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

Silent renew iframe is not loaded when there is no session #744

Closed
GabrielGil opened this issue May 18, 2020 · 12 comments
Closed

Silent renew iframe is not loaded when there is no session #744

GabrielGil opened this issue May 18, 2020 · 12 comments

Comments

@GabrielGil
Copy link
Contributor

Describe the bug
Silent renew, should (and it used to), be triggered when enabled, so the application can be authorised directly without redirects in case the user is already authenticated in the STS server.

To Reproduce
Steps to reproduce the behaviour:

  1. Configure a project using the sample: https://github.com/damienbod/angular-auth-oidc-client/blob/master/docs/samples.md#code-flow-with-pkce-basic-with-silent-renew
  2. Open the application in Tab 1
  3. Login (At this point, the silent renew iframe is loaded and working)
  4. Open the application in Tab 2
  5. The silent renew iframe is not loaded, forcing the user to click on "Login" again.

Expected behaviour
From the previous steps, I believe the plugins should load the iframe to silently renew. This will cause the STS to give the app new tokens directly, as the app was previously authorised already.

This is the behaviour of the silent renew as per version 10.0.14.

Also, I'd like to clarify that this is not auto-login, given that auto-login will redirect the user to the STS login interaction, regardless of the current user state in the STS whereas silent renew will only get a fresh set of tokens in case the user is still correctly authenticated and the client app authorised in the STS server.

Screenshots
image

Desktop (please complete the following information):

  • OS: Mac OS 10.15.4
  • Browser: Chrome, Firefox, Edge, Safari.
  • Version 81, 75, 84, 13.1

Smartphone N/A:

  • Device: [e.g. iPhone6]
  • OS: [e.g. iOS8.1]
  • Browser [e.g. stock browser, safari]
  • Version [e.g. 22]

Additional context
N/A

@damienbod
Copy link
Owner

Hi @GabrielGil yes we changed the startup here. We wanted to simplify the logic, and removed all our timing issues. You can achieve the same with the following code:

 ngOnInit() {
        this.oidcSecurityService.checkAuth().subscribe((isAuthenticated) => {
            console.log('app authenticated', isAuthenticated);
            if (!isAuthenticated) {
                console.log('run login without UI', isAuthenticated);
                this.oidcSecurityService.authorize({ customParams: { prompt: 'none' } });
            }
        });
    }

Then it will work like you want.

Hope this helps Damien

@GabrielGil
Copy link
Contributor Author

Hi @damienbod. Thanks for your answer.

That is not exactly precise, as it is not the same result and it has a number of side effects:

  1. It does not keep the originating url. Since the user agent is redirected to the STS, this will just redirect to the callback url, and not to the one originally indicated by the user).
  2. It forces an unnecessary redirect, and error in case the user is not logged in in the STS, or has never authorised the app.

The silent renew approach seems to be the de facto method to effectively get new tokens when the client app has no session in the current tab, quite well described in this article by Dennis Stötzel (Security consultant), "How To Securely Implement Authentication in Single Page Applications".

Please, let me know your thoughts/opinions.
Gg.

@damienbod
Copy link
Owner

damienbod commented May 19, 2020

Hi @GabrielGil nice blog, read it. yes the redirect is for the whole application and not just the silent renew. But this could be solved using the auto login but...

I think it would makes sense to add support for forceRefreshSession() as a possiblity to login, if already logged in like the code above but with prompt none and no redirect, so you can keep your URL without and requiring an auto login.

The error will happen anyway, just in the silent renew, and if it fails, the session will be reset anyway.

Would this work for you?

@GabrielGil
Copy link
Contributor Author

Thanks again for reading the docs I provided. I really appreciate that @damienbod.

Auto-login would get closer to the expected behaviour, but despite the redirect (and manually keep the originating url in storage), the problem with that approach is that it will add a perceivable redirect to every single user, even if that user has never authenticated in the STS.

I agree with you that having this option included would perfectly cover this scenario. I understand that the error will still be there, as the loaded URL will also contain prompt=none, but this will be completely unnoticeable to the user.

Lastly, as long as the first emission of isAuthenticated$ waits for the forcedRefresh this will work for me, inline with the described in the article. Otherwise, guards may incorrectly redirect the user from a guarded route.

@damienbod
Copy link
Owner

damienbod commented May 21, 2020

@GabrielGil We will consider a new API

ngOnInit() {
	this.oidcSecurityService.checkAuthIncludingServer().subscribe((isAuthenticated) => {
		console.log('app authenticated', isAuthenticated);
	});
}

Should be ready in the next few days. We need to do the testing, and docs

This will only work with silent renew using iframes, and never with refresh tokens. Creating a new session with refresh tokens requires consent or a refresh token, which is not possible in a new tab using session storage.

Greetings Damien

@GabrielGil
Copy link
Contributor Author

GabrielGil commented May 21, 2020

That is a really elegant solution in. my. opinion and it fits with the use case we're describing here @damienbod. I appreciate to have an approximate ETA as well.

I agree with the limitations with refresh tokens, and it complies with the spec which require user consent to get refresh tokens, thus not. possible in the iframe.

I want to thank you one more time for your dedication and good will.

Awaiting that new release,
Gg.

@damienbod
Copy link
Owner

@GabrielGil Added support for this in version 11.1.2

https://github.com/damienbod/angular-auth-oidc-client/blob/master/docs/public-api.md#checkauthincludingserver-observable

Thanks for reporting.

Greetings Damien

@GabrielGil
Copy link
Contributor Author

Thank you once again, @damienbod. I have updated and it works exactly as expected. Much appreciated.

G.

@GabrielGil
Copy link
Contributor Author

Sorry to come up with this again, however I think I found a small bug. Or maybe I have misunderstood what was achievable.

The thing is that using checkAuthIncludingServer will prevent the guards to work as the isAuthenticated$ observable, emits false directly without waiting for the iframe to emit, so directly accessing /protected-route causes the redirect expected if the user was logged out, but miliseconds after this redirection, the user is correctly authenticated.

Is this the expected behaviour? Is this something that could be improved? Here again, this is the behaviour that was in place in the old 10.0.14 release.

Thanks in advance again @damienbod.

@GabrielGil
Copy link
Contributor Author

Hey @damienbod, not sure whether you saw this comment here or should I open a new issue.

@GabrielGil
Copy link
Contributor Author

Hey guys, was there any progress in this investigation? Is there anything I can do to help with issue? (Details, plunkr etc?)

@damienbod
Copy link
Owner

Hi @GabrielGil I try to clean up old issues. Can you open a new issue if you still have problems here?

Sorry if I close this incorrectly

Greetings Damien

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants