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

Hash not being cleared after silent refresh in Firefox #408

Closed
shambarick opened this issue Aug 17, 2018 · 15 comments
Closed

Hash not being cleared after silent refresh in Firefox #408

shambarick opened this issue Aug 17, 2018 · 15 comments

Comments

@shambarick
Copy link

In Firefox, when the token is refreshed by silent refresh, it adds a # automatically in the url and it's not removed from the url.
It only happens in Firefox, I don't have the issue in Chrome.
I even tried to add clearHashAfterLogin: true but it doesn't seem to do anything, the hash is not removed.

I'm using angular-oauth2-oidc(v4.0.2) with Keycloak (4.2.1.Final).
I use the default routing configuration in Angular, so my URLs don't contain any hash.

My auth.config.ts

export const authConfig: AuthConfig = {
  // Url of the Identity Provider
  issuer: 'http://keycloak.local/auth/realms/helloworld',
  requireHttps: false,
  // URL of the SPA to redirect the user to after login
  redirectUri: window.location.origin + '/index.html',
  // URL for silent refresh
  silentRefreshRedirectUri: window.location.origin + '/silent-refresh.html',
  // Timeout before silent refresh is fired, 0.4 => fired when access token expiration time reaches 40%
  timeoutFactor: 0.5,
  showDebugInformation: true,
  // The SPA's id. The SPA is registerd with this id at the auth-server
  clientId: 'my-app',
  scope: 'openid profile email',
};

My app.component.ts

export class AppComponent {
  title = 'app';

  constructor(private oauthService: OAuthService) {
    this.configureWithNewConfigApi();
  }

  private configureWithNewConfigApi() {
    this.oauthService.configure(authConfig);
    this.oauthService.tokenValidationHandler = new JwksValidationHandler();
    this.oauthService.loadDiscoveryDocumentAndLogin();
    this.oauthService.setupAutomaticSilentRefresh();
}
@jeroenheijmans
Copy link
Collaborator

jeroenheijmans commented Aug 17, 2018

Could you try doing:

this.oauthService.events.subscribe(event => {
  if (event instanceof OAuthErrorEvent) {
    console.error(event);
  } else {
    console.warn(event);
  }
});

and then check if the same (order of) events pop up in Chrome and Firefox ("Preserve logs").

Let us know if that helps you find the root cause.

Also, at some point we had similar issues in different browsers, and we just chained this to the discovery loading and login promise chain, at the end:

this.oauthService
  .loadDiscoveryDocumentAnd....()
  // etc
  .then(() => { location.hash = ''; return Promise.resolve(); });

Which I think doesn't hurt (but is a hack/workaround, of course).

@shambarick
Copy link
Author

The order of events is the same in Chrome and Firefox.
Your workaround doesn't seem to work in my case. The hash still appears after the silent refresh.

Firefox (version 61.0.1)

app.component.ts:31:8
Object { type: "discovery_document_loaded", info: {…} }
app.component.ts:31:8
Object { type: "token_expires", info: "id_token" }
app.component.ts:31:8
Object { type: "token_received", info: null }
app.component.ts:31:8
Object { type: "silently_refreshed", info: null }
app.component.ts:31:8
Object { type: "token_expires", info: "id_token" }
app.component.ts:31:8
Object { type: "token_received", info: null }
app.component.ts:31:8
Object { type: "silently_refreshed", info: null }
app.component.ts:31:8
Object { type: "token_expires", info: "id_token" }
app.component.ts:31:8
Object { type: "token_received", info: null }
app.component.ts:31:8
Object { type: "silently_refreshed", info: null }

Chrome (version 68)

app.component.ts:31 OAuthSuccessEvent {type: "discovery_document_loaded", info: {…}}
(anonymous) @ app.component.ts:31
app.component.ts:31 OAuthInfoEvent {type: "token_expires", info: "id_token"}
(anonymous) @ app.component.ts:31
app.component.ts:31 OAuthSuccessEvent {type: "token_received", info: null}
(anonymous) @ app.component.ts:31
app.component.ts:31 OAuthSuccessEvent {type: "silently_refreshed", info: null}
(anonymous) @ app.component.ts:31
app.component.ts:31 OAuthInfoEvent {type: "token_expires", info: "id_token"}
(anonymous) @ app.component.ts:31
app.component.ts:31 OAuthSuccessEvent {type: "token_received", info: null}
(anonymous) @ app.component.ts:31
app.component.ts:31 OAuthSuccessEvent {type: "silently_refreshed", info: null}
(anonymous) @ app.component.ts:31
app.component.ts:31 OAuthInfoEvent {type: "token_expires", info: "id_token"}
(anonymous) @ app.component.ts:31
app.component.ts:31 OAuthSuccessEvent {type: "token_received", info: null}
(anonymous) @ app.component.ts:31
app.component.ts:31 OAuthSuccessEvent {type: "silently_refreshed", info: null}
(anonymous) @ app.component.ts:31

@jeroenheijmans
Copy link
Collaborator

Wait, what? I didn't read your original post correctly and had not seen that the silent refresh is causing this. Can you verify/confirm that merely commenting out the setupAutomaticSilentRefresh() will make the problem go away?

If it doesn't, perhaps you could elaborate a bit more about that part? The reason it's confusing to me is that the silent refresh should only do URL-based stuff to the iframe, the location of the top frame (your app) shouldn't be touched. You can see this in the listener for the iframe posting a message which sends the hash in a custom manner to tryLogin (grabbing it from the posted message.

I'm also somewhat confused about the stream of events, because it seems as if there is no output from the ...AndLogin(...) part of your routines. Bit lost why that would be the case.

One final other note, you could try using the slightly different flow I suggested in #359, that doesn't use the convenience overloads but chains all parts itself, allowing you to add some debug statements to find out what's going on in your scenario.

@shambarick
Copy link
Author

shambarick commented Aug 17, 2018

Commenting out the setupAutomaticSilentRefresh() makes the problem goes away.
Actually, as soon as this.authService.silentRefresh() is called, the hash appears.

I tried your flow and it didn't work as expected. this.authService.hasValidAccessToken() always returns true. And even if I remove the condition, since this.authService.silentRefresh() is called, the hash appears.

Edit: I just created a new Angular project and it works as expected... No hash in the url after a silent refresh in Firefox
Edit2: Actually it doesn't work even with a new fresh project. I actually forgot to create the silent-refresh.html file and I thought it worked, but it didn't. After creating the file, it still adds a hash in the url after the silent refresh in Firefox. In Chrome, it still works fine.

@jeroenheijmans
Copy link
Collaborator

Actually it doesn't work even with a new fresh project.

That's somewhat good news! Could you please create a StackBllitz example or GitHub repo with the repro? That makes it a lot easier to help.

In addition (or possibly as an alternative) you could have a look at this minimal repro, clone it, and check if that reproduces your problem as well?

Let us know what you find.

@shambarick
Copy link
Author

Sadly, your repo gives me the same result.
I tried Firefox, Chrome and Edge, only Chrome works fine.

@jeroenheijmans
Copy link
Collaborator

Huh weird, not sure how I can further help then. I've checkout out ca0bbab ran ng serve and opened http://localhost:4200 in a fresh Firefox Private Browsing window. It redirects me to the IdentityServer3 where I log (max/geheim) in and give consent, then I go back to the app and the hash fragment is cleared.

I've also tried this with network throttling, same deal: just works.

I'm using Windows 10 and these versions:

  • Firefox 61.0.2 (64 bit), no addons
  • Node 10.8.0
  • npm 6.3.0
  • the node_modules tree is pinned using the package-lock so you should have the same

In Edge I also get a cleared hash.

Perhaps there's something in your local setup that's affecting this. Perhaps the above helps you debug what is. If it doesn't, then the only reasonable path for root cause analysis left is using an unminified build of the library and debug it with the Firefox dev tools to see why it's not clearing.

Good luck!

@shambarick
Copy link
Author

I tried on Ubuntu 16.04 and 18.04 and these versions:

  • Firefox 61.0.2 (64 bit), no addons, in private browsing
  • Node 8.11.3 and 10.9.0

I get the hash as soon as I click on force silent refresh.

If you have Docker installed, you can run the following container docker run --rm -it -p 4200:80 tazounet/sample-angular-oauth2-oidc

@jeroenheijmans
Copy link
Collaborator

I have to apologize, for not reading your problem description carefully enough...

I thought the hash fragment wasn't being cleared, and that you were worried about the tokens remaining visible in the URL bar. After running your docker image I finally realized that you were talking about the hash itself. Sorry for the confusion!

I actually can reproduce what you mention, I too get the hash in Firefox when clicking 'force silent refresh'.

You're running into this issue because this oauth library uses this code to clear the hash:

location.hash = '';

You can try that yourself, it behaves differently between Chrome and Firefox. Open up a simple site, type in #blahblah at the end of the URL, and then execute location.hash = '' in your devtools. Chrome will clear the hashbang character too, Firefox leaves it.

There is a modern solution to this which can also be polyfilled of course. Place it both in your login chain as well as after your silent refreshes (see lines A/B/C below):

import { Injectable } from '@angular/core';
import { OAuthService, OAuthErrorEvent } from 'angular-oauth2-oidc';
import { filter } from 'rxjs/operators';

@Injectable()
export class AuthService {

  // A!
  private clearHash() {
    // By @AndyE from https://stackoverflow.com/a/5298684/419956
    history.pushState('', document.title, window.location.pathname + window.location.search);
  }

  constructor(private oauthService: OAuthService) {
    // Useful for debugging:
    this.oauthService.events.subscribe(event => {
      if (event instanceof OAuthErrorEvent) {
        console.error(event);
      } else {
        console.warn(event);
      }
    });

    this.oauthService.events
      .pipe(filter(e => e.type === 'silently_refreshed'))
      .subscribe(e => this.clearHash()); // B!

    // login chain
    this.oauthService.loadDiscoveryDocument()
      .then(() => this.oauthService.tryLogin())
      .then(tryLoginResult => {
        if (this.oauthService.hasValidAccessToken()) {
          return tryLoginResult;
        }

        return this.oauthService.silentRefresh()
          .catch(result => {
            const errorResponsesRequiringUserInteraction = [
              'interaction_required',
              'login_required',
              'account_selection_required',
              'consent_required',
            ];

            if (result && result.reason && errorResponsesRequiringUserInteraction.indexOf(result.reason.error) >= 0) {
              this.oauthService.initImplicitFlow();
              return Promise.resolve();
            }

            return Promise.reject(result);
          });
      })
      .then(() => this.clearHash()); // C!
  }
}

I'm not sure about this library's claimed browser support, but from linked SO post it seems it might be good to use the history based solution too.

jeroenheijmans added a commit to jeroenheijmans/angular-oauth2-oidc that referenced this issue Aug 19, 2018
@shambarick
Copy link
Author

Your fix works but it has a drawback. It adds a new state in the history each time a silent refresh is triggered.

I'm on a page and the silent refresh is triggered. If I want to go back to the previous page, I'm not going to be on the previous page. It will take me back to the current page with the hash.

@jeroenheijmans
Copy link
Collaborator

Then I think your only other quick option left is living with the fact that Firefox (and friends) will have a hash in the URL after silent refreshes.

The more involved fix could perhaps be to make the library not reset the location.hash to empty string if the fragment wasn't retrieved from the URL in the first place. Then again, that would only solve things for silent refreshes, not for the initial tryLogin call that would use a fragment too.

@shambarick
Copy link
Author

Could you take a look at oidc-client-js (https://github.com/IdentityModel/oidc-client-js) ?
I don't have the issue with it. However, I don't understand how it handles the silent refresh (JS is not really my thing... Otherwise I would have already done it).

@jeroenheijmans
Copy link
Collaborator

😄 Hmm, no sorry, I'm not very familiar with the other library, you will have to investigate yourself or with help of a colleague, friend, or (possibly paid) trainer to see how it works. (I also think the GitHub issues list for angular-oauth2-oidc is not really the place to discuss how another library works...)

@shambarick
Copy link
Author

Yes, I understand. Thank you for your help 😄

@pfbrowning
Copy link

@shambarick I've posted a workaround to this in #493. In short, you can side-step the issue by turning off clearHashAfterLogin after processing your initial login.

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

No branches or pull requests

3 participants