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

Consider supporting iframe silent refresh in Authorization Code + PKCE flow #600

Closed
jeroenheijmans opened this issue Aug 11, 2019 · 4 comments · Fixed by #609
Closed

Consider supporting iframe silent refresh in Authorization Code + PKCE flow #600

jeroenheijmans opened this issue Aug 11, 2019 · 4 comments · Fixed by #609
Labels
feature-request Improvements and additions to the library.

Comments

@jeroenheijmans
Copy link
Collaborator

jeroenheijmans commented Aug 11, 2019

Recently released versions 8.x introduced support for "Code Flow" (Authorization Code + PKCE). While trying to upgrade my sample repositlory to utilize this flow I'm running into an issue.

It seems that silent refresh via an iframe is not supported with Code Flow in angular-oauth2-oidc?

Current situation

Even though you could ask for offline_access as suggested by the Code Flow docs for this library, and then utilize refresh() instead, I think the iframe method can be at least as useful, if not more. It allows you to initiate a silent refresh when starting your application (if you want this negates the need for localStorage too), and in general prevents having to ask for refresh tokens (which are often deemed too powerful for SPA's).

I shortly wondered if "the iframe silent refresh trick" would even work with Code Flow, but found some evidence that it should do so:

So, I think our library could and should support it just fine.

The code

Here's the relevant tryLogin method:

public tryLogin(options: LoginOptions = null): Promise<boolean> {
if (this.config.responseType === 'code') {
return this.tryLoginCodeFlow().then(_ => true);
}
else {
return this.tryLoginImplicitFlow(options);
}
}

This method is called:

  • For initial login sequences, possibly grabbing the code hash fragment parameter and others from the window.location
  • When called based on silent renew iframe messages fired when the auth server successfully does a "no prompt" login, redirecting the iframe back to the silent-refresh.html page

As you can see in the code above, tryLoginCodeFlow is called without arguments, that is the iframe's message is discarded in the second scenario.

This makes sense, as the method does not support being called with a customHashFragment grabbed from the iframe's message:

public tryLoginCodeFlow(): Promise<void> {
const parts = this.parseQueryString(window.location.search)

Proposed change

I propose we change tryLoginCodeFlow to support passing along something like LoginOptions. We most likely would need to tweak things, as with the code flow the response data is in the query string parameter, not the hash fragment (I think?).

@ErazerBrecht
Copy link
Contributor

ErazerBrecht commented Jan 25, 2020

Another reason to prefer the silent refresh is because this will keep your SSO session on your Identity Server alive (if you are using sliding cookies). This is better UX when you have multiple applications using your Identity Server.

With refresh tokens you are working completely outside the user session, that's why it's called offline_access.

@jeroenheijmans
Copy link
Collaborator Author

In this commit in my sample repo you can see some of the hoops I'd have to jump through in my production applications too. Would be really nice to have the silentRefresh() also supported in code flow :D - hoping the offered PR is indeed as simple as it can be!

@manfredsteyer
Copy link
Owner

@ErazerBrecht:

With refresh tokens you are working completely outside the user session, that's why it's called offline_access.

Honestly, this is an implementation/ configuration detail. The current best practices document suggest to limit the usage of such a refresh-token for the time of the session.

The only concern that stays -- also after respecting the current best practices document -- is that the refresh_token can be stolen via XSS. The iframe trick can help here b/c it allows to use http-only cookies.

@manfredsteyer
Copy link
Owner

@jeroenheijmans I'll make this a priority.

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

Successfully merging a pull request may close this issue.

3 participants