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

Firefox silent refresh changes main page URL #493

Closed
wentzt-engtech opened this issue Jan 2, 2019 · 3 comments · Fixed by #676
Closed

Firefox silent refresh changes main page URL #493

wentzt-engtech opened this issue Jan 2, 2019 · 3 comments · Fixed by #676
Labels
bug For tagging faulty or unexpected behavior.

Comments

@wentzt-engtech
Copy link

I ran into an issue in Firefox where the silent refresh was triggering a location change which is causing an issue for us. This was brought up before in #408. The clearing of the hash in Firefox results in the '#' character being added to the URL. There is code to prevent clearing the hash if it is not an oidc flow by checking preventClearHashAfterLogin which appears to only be set when doing a silent refresh.

if (this.clearHashAfterLogin && !options.preventClearHashAfterLogin) {

However, for oidc, the hash gets cleared in the processIdToken then() just a little bit below, and preventClearHashAfterLogin does not get checked.

if (this.clearHashAfterLogin) {

It looks like the additional check should be added to that line so that silent refreshes don't affect the main document location/URL when using oidc.

I also noticed that the iframe url keeps the tokens in the URL hash. I don't know if that is the expected behavior. Either way, from the discussion in #408 (comment) it doesn't sound like the silent refresh should affect the main document's URL/location.

Thanks!

@LouisBernath
Copy link

LouisBernath commented Jan 18, 2019

I have the same issue.
From what I saw in the debugger, location.hash gets cleared correctly in the code. But Firefox if window.location.hash is empty and the code (you mentioned above) re-sets with an empty string '', firefox will add an '#' at the end of the url.

I'm not sure if this is standard compliant behaviour or a firefox bug...

Edit: here a simple javascript repro that demonstrates the problem
https://gist.github.com/Burnaster/ba69a7307b7a423acd78186937185ed2

@jeroenheijmans jeroenheijmans added the investigation-needed Indication that the maintainer or involved community members may need to investigate more. label Aug 19, 2019
@pfbrowning
Copy link

pfbrowning commented Oct 16, 2019

This issue affects me as well. My workaround is to turn off clearHashAfterLogin after processing the initial login:

this.oauthService.loadDiscoveryDocumentAndTryLogin()
	.then(() => {
		this.oauthService.setupAutomaticSilentRefresh();
		this.oauthService.clearHashAfterLogin = false;
	})

You can set clearHashAfterLogin in your configuration. However I'm waiting until after a successful initial login before turning it off because I do want to clear hash on the initial login: I just don't want to clear it on silent refresh.

This seems to solve the problem, but it's a workaround rather than a proper fix. As originally suggested by @jeroenheijmans in #408, I think that a proper solution would involve updating this:

if (this.clearHashAfterLogin) {

such that we're also checking to ensure that there actually is a hash value before clearing it. Something like:

if (this.clearHashAfterLogin && location.hash != '') {
	location.hash = '';
}

@jeroenheijmans jeroenheijmans added bug For tagging faulty or unexpected behavior. and removed investigation-needed Indication that the maintainer or involved community members may need to investigate more. labels Nov 20, 2019
@jeroenheijmans
Copy link
Collaborator

jeroenheijmans commented Nov 20, 2019

I confirmed this bug using my sample repo after disabling the very same clearHashAfterLogin = true workaround. We also encounter this in our production application, where setting an empty # causes Firefox to scroll to the top, which is especially annoying if a silent refresh triggers it!

I also confirmed that adding the extra check for ... && !options.preventClearHashAfterLogin does the trick.

I'll add a PR to update this.

jeroenheijmans pushed a commit to jeroenheijmans/angular-oauth2-oidc that referenced this issue Nov 20, 2019
Firefox will show annoying behavior for setting location.hash to an
empty string if it already is empty: it refreshes the page or scrolls
to the top.

The preventClearHashAfterLogin option is passed down to functions to
prevent this behavior when it's not needed (e.g. during the results
of a silent refresh). However, that option wasn't checked for in a
consistent manner. This commit fixes that.

Fixes manfredsteyer#493
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug For tagging faulty or unexpected behavior.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants