Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Authorization Code Flow support #19

Closed
lgleim opened this issue Feb 14, 2017 · 39 comments
Closed

Authorization Code Flow support #19

lgleim opened this issue Feb 14, 2017 · 39 comments
Labels
feature-request Improvements and additions to the library.

Comments

@lgleim
Copy link

lgleim commented Feb 14, 2017

When are you planning to add support for the Authorization Code Flow?

Should be reasonably easy given that you already implemented all logic for identity, access, and refresh tokens, including refreshing the access token with the refresh token - or am I missing something?

@manfredsteyer
Copy link
Owner

can you create a pull request?

@DavidParks8
Copy link

Is there a technical reason you want support for the authorization code flow? It was not designed for use with client side applications. According to the specification, the Authorization Code flow is suitable for Clients that can securely maintain a Client Secret between themselves and the Authorization Server. Angular apps are not able to maintain such a secret.

@lgleim
Copy link
Author

lgleim commented Apr 9, 2017

Some OIDC Servers implement authorization code flow with shortlived refresh and id tokens. Keycloak (cf. https://keycloak.gitbooks.io/documentation/content/server_admin/topics/sso-protocols/oidc.html, js adapter code: https://github.com/keycloak/keycloak/tree/master/adapters/oidc/js/src/main/resources) would be an example of that.
And when you consider it a problem for the client to be able to access the refresh/id token, supporting the direct access grant flow is certainly much more of a concern. Nevertheless there is a plethora of (especially hybrid mobile) apps that handle password entry right in the web app.

@manfredsteyer
Copy link
Owner

Yes, Code Flow for a SPA seems to be a gray zone and can be used safely. But it is not the indented way. I would prefere implicit flow with token refresh (which is possible without refresh_tokens by leveraging a well known hack).

@manfredsteyer
Copy link
Owner

As the next version of this lib (lands i a few days) is supporting silent refresh with implicit flow there is IMHO no need for code flow for browser scenarios. When it comes to hybrid scenarios (cordova, ionic) it would make sense. I would accept PR on this and there is a pull request for hybrid flow which contains a lot of code that can be used when writing support for code flow.

I would suggest to introduce a new method initCodeFlow(...) for this.

@manfredsteyer manfredsteyer added the feature-request Improvements and additions to the library. label Aug 14, 2017
@manfredsteyer
Copy link
Owner

Oh, if anyone creates a PR for this, please have a look at the RFC about Code Flow and mobile apps.

@sbueringer
Copy link

What do you thing about the Authorization Code Flow with PKCE for SPAs? (https://tools.ietf.org/html/rfc7636)

@manfredsteyer
Copy link
Owner

If think, when we are implementing Code Flow we should also implement PKCE alongside b/c this is THE way to go for mobile apps/ hybrid apps. When it comes to web apps, I would prefer Implicit Flow + Silent Refresh.

Do you have a current project where code flow + PKCE is needed?
If yes, when would you need it?
If yes, would you be interested into contributing such an addition?

@sbueringer
Copy link

I'm not sure if we're going to use code flow with PKCE, yet. I'll get back to you when I know.

If we're going to use it, I would absolutely be interested in contributing this addition.

@manfredsteyer
Copy link
Owner

Cool, just reach out in case.

@CallumBrankin
Copy link

+1 for Authorization Code Flow.

@khamlaoui
Copy link

+1 Authorization Code Flow

@srenatus
Copy link
Contributor

srenatus commented Nov 9, 2017

Adding some context here, it seems like parts of "the industry" (hi there 👋) have moved on when it comes to using the implicit flow for SPAs:

Note: Previously, it was recommended that browser-based apps use the "Implicit" flow, which returns an access token immediately and does not have a token exchange step. In the time since the spec was originally written, the industry best practice has changed to recommend that the authorization code flow be used without the client secret. This provides more opportunities to create a secure flow, such as using the state parameter. (source)

So, to accommodate that, I'd +1 the request for authorization code flow.

Authorization Code Flow with PKCE for SPAs

Anyways, I'm not convinced that PKCE makes sense for SPAs. The crucial point in the App use case could be that the client (the app) can keep its state (including the challenge) a secret; or at least hidden from a malicious app on the same smartphone. I don't believe that this makes sense for an in-browser JS application. (But I might very well be missing something, not an expert.)

@manfredsteyer
Copy link
Owner

You're right. PKCE does not make sense in the browser. When it comes to hybrid apps, we should have it.

@grmaciel
Copy link

+1 for the Authorization Code Flow.

Are there any plans for it? We were planning on use this library for our product here but now we hit a roadblock.

@phausero
Copy link

+1 for the Authorization Code Flow.

2 similar comments
@masiorama
Copy link

+1 for the Authorization Code Flow.

@bechhansen
Copy link
Contributor

+1 for the Authorization Code Flow.

@srenatus
Copy link
Contributor

srenatus commented Dec 8, 2017

I suppose @manfredsteyer wouldn't be blocking a code contribution... (nudge, nudge at y'all who want this so dearly 😉)

@bechhansen
Copy link
Contributor

bechhansen commented Dec 9, 2017

Hi @manfredsteyer I'm about to implement support for authorization code flow for your library.
I have forked the projekt but I'm a bit confused abut the versioning of the the latest release.
When installing we get version 3.0.3. https://www.npmjs.com/package/angular-oauth2-oidc also also have version 3.0.3.

In the github repository https://github.com/manfredsteyer/angular-oauth2-oidc the latest version is 3.0.1.
The version in package.json of the master branch is 3.0.2 ?!?
Where is the code for 3.0.3.

Also

@bechhansen
Copy link
Contributor

@manfredsteyer also how do you build the library?
If I build it using npm pack and then install the tar, I get the error:
Module build failed: Error: ///node_modules/angular-oauth2-oidc/index.ts is not part of the compilation output.

@pako-g
Copy link

pako-g commented Dec 9, 2017

+1 Authorization Code Flow

@spawnrider
Copy link

+1 Authorization Code Flow
I need it too

@manfredsteyer
Copy link
Owner

@bechhansen Thx for taking care about this.

I guess I've used git push and npm version minor && npm publish in the wrong order. That's why we have a gap here. But it should not matter and I will correct this soon.

Regarding the build failure: It's about Angular 5. They don't support it to have ts files in the referenced package anymore. It was never indented to support this but by coincident it worked before. The quick workaround for this is to run it with the --aot flag. In this case this is still supported.

npm start -- --aot

@bechhansen
Copy link
Contributor

I have added a pull request for this functionality for you to review:
#195

@pako-g
Copy link

pako-g commented Feb 27, 2018

+1 Authorization Code Flow

@HichamBI
Copy link

+1 for the Authorization Code Flow

@Larsera
Copy link

Larsera commented Apr 12, 2018

+1 for Authorization Code Flow

@pokkia
Copy link

pokkia commented Apr 13, 2018

+1

@TomWhiteOmni
Copy link

+1 for PKCE

@bechhansen
Copy link
Contributor

Hi

I think this great library need to include the authorization code flow to be complete, and I think its sad it can’t be part of this package. I personally prefer to use the authorization code flow as I think the silent refresh mechanism of the implicit flow is a dirty hack.

I have forked this repository and added support for the authorization code flow. PKCE is currently not implemented.

Go check it out at https://www.npmjs.com/package/angular-oauth2-oidc-codeflow

(Name, versioning, structure, etc. might change in the future)

@manfredsteyer
Copy link
Owner

@bechhansen: Thanks for the PR and for the fork. Very appreciated.

As mentioned in an other thread, I cannot support that much flows and so creating additional solutions/ forks seem to be the best way to meet all the different needs.

@benze
Copy link

benze commented Sep 18, 2018

I'm confused by all the requests for Auth Code Flow for SPAs. Having a long-lived refresh token in an SPA would seem like an exploitable security hole. Why should I prefer Auth Code over Implicit? What advantages does Auth Code provide? Is it limited to the fact that the access token is never passed over the URL (via the fragment) and instead only via body content? Or is there something else I am missing?

@andifalk
Copy link

According the new OAuth2 draft spec implicit flow MUST NOT be used any more for SPA
https://tools.ietf.org/html/draft-parecki-oauth-browser-based-apps-00#section-7.8
See also https://medium.com/@torsten_lodderstedt/why-you-should-stop-using-the-oauth-implicit-grant-2436ced1c926

@bechhansen
Copy link
Contributor

@andifalk very very interesting. Too bad the library do not support Code flow ;-)

@pako-g
Copy link

pako-g commented Nov 12, 2018

+1 for the Authorization Code Flow.

@jeroenheijmans
Copy link
Collaborator

A fresh issue was opened in #470 (triggered by the newest RFC, so it seems appropriate IMO to start a fresh issue for it).

@foadshariat7
Copy link

+1 for the Authorization Code Flow.

@bschnabel
Copy link

it's going to come. see #549

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request Improvements and additions to the library.
Projects
None yet
Development

No branches or pull requests