-
Notifications
You must be signed in to change notification settings - Fork 8
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
Change Hybrid Flow to Authorization Code Flow and add PKCE support #8
base: master
Are you sure you want to change the base?
Conversation
I'll look into it, but I don't have the time today :) Will get back to you. |
Hello, |
Hi @pliolis! As far as I know, it's not. A token refresh function is implemented, but you have to call it yourself, by writing an interceptor for example. |
Thank you my friend. |
let request = await AuthorizationRequest.fromJson(json); | ||
|
||
let response = await this.storageBackend.getItem(AUTHORIZATION_RESPONSE_KEY); | ||
let parts = response.split('#'); | ||
let parts = response.split('?'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The identity server default is # have you changed this on your server and this is why you modified this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I'm not mistaken (it's been a long time now), this part interprets the authorization response of the oidc server. The following RFC describes this response: https://tools.ietf.org/html/rfc6749#section-4.1.2. The response must be something like that :
HTTP/1.1 302 Found
Location: https://client.example.com/cb?code=SplxlOBeZQQYbYS6WxSbIA&state=xyz
As you can see, it's a ?
and not a #
.
I used Identity Server 3 and it sent a ?
. Recently, I've migrated to Identity Server 4 and it works the same.
However, I'm not the one who set up Identity Server, so I may be unaware of a specific configuration somewhere...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I noticed that the #
is used with the Implicit grant according to the RFC (https://tools.ietf.org/html/rfc6749#section-4.2.2). The code of this commit handles the Authorization Code grant.
Hello,
Thanks for this repository which has been a time saver for me!
I had to implement an Authorization Code Flow client on Ionic - as recommended by IETF, so I used this repository and I changed some details.
I also got the code verifier from #7 and I put it in the sources to implement PKCE.
I don't know if you want to do the change from Hybrid to Authorization code flow, so be free to accept or not this PR. Perhaps it could be merged in another branch so we keep both flows.