Angular-auth-oidc-client: getIdFromToken returns incorrect data, APP callback from STS emits 2 auth events bug

Created on 14 May 2020  路  12Comments  路  Source: damienbod/angular-auth-oidc-client

If you take the link you provided above and replace console.log with:

const userData = this.oidcSecurityService.getPayloadFromIdToken();
console.log (userData);

You'll see that any changes to the id_token won't be reflected with a call from getPayloadFromIdToken(), but if you look in localStorage and decode the id_token in xxx_authorizationDataIdToken it does have it.

Here's the general flow:

  1. Update some field via api that the idp encodes in the id_token
    2 Call forceRefreshSession()
  2. UI updates based on new values from the idp in the id_token from call to getPayloadFromIdToken that sets fields.

Preferably it would be:

  1. Update some field
  2. forceRefreshSession
  3. checkAuth involked automatically by this library because the auth changed because the tokens changed.
  4. Respond to event like normal which would cause the right stuff to be updated which would change the UI.

So I'd say that the bug is that getIdFromToken still has a cached copy even though it was updated and stored elsewhere so it never gets the new version, and the forceRefreshSession doesn't compare the auth and id tokens it gets back and then call checkAuth if they aren't the same. (doesn't matter about the content, only that they're not identical)

bug investigate

All 12 comments

Hi @JohnGalt1717 I think this works. You need to wait for the auth event and the id_tokens match. Can you validate if this works for you?

 constructor(public oidcSecurityService: OidcSecurityService, public eventService: PublicEventsService) {}

 ngOnInit() {
        this.configuration = this.oidcSecurityService.configuration;
        this.userData$ = this.oidcSecurityService.userData$;
        this.isAuthenticated$ = this.oidcSecurityService.isAuthenticated$;

        this.eventService
            .registerForEvents()
            .pipe(filter((notification) => notification.type === EventTypes.NewAuthorizationResult))
            .subscribe((value) => {
                const data = this.oidcSecurityService.getIdToken();
                console.warn(data);
            });
    }

    async refreshSession() {
        this.oidcSecurityService.forceRefreshSession().toPromise();
    }

Greetings Damien

So that works as far as it goes, but then on initial login the checkAuth and that fire so you're double calling.

It would be better if forceRefreshSession waited for this in it's code and only returned once it executed completely (or something similar) otherwise this breaks the checkAuth event flow for this case.

(and it should call checkAuth automatically once done if the auth or id tokens change as a result)

@JohnGalt1717 yes, you don't need the notification event, you can just use the this.oidcSecurityService.isAuthenticated$

??

I would expect that forceRefreshToken() observable doesn't fire until all of this is done.

Are you saying that I should just be observing isAuthenticated on first start not checkAuth per the docs or something? I'd still get a double call that way.

Really trying to use checkAuth to tell me once and only once whenever something changes in auth, and forceRefreshToken() should get the updated tokens and if there's a change, it should finish loading then call checkAuth because the auth changed.

Will search for a solution tomorrow, but yes, I get 2 event after a login, otherwise only one...

   ngOnInit() {
        this.configuration = this.oidcSecurityService.configuration;
        this.userData$ = this.oidcSecurityService.userData$;

        this.oidcSecurityService.isAuthenticated$.subscribe((authenticated) => {
            this.isAuthenticated = authenticated;
            if (authenticated) {
                const data = this.oidcSecurityService.getIdToken();
                console.warn(data);
            }
        });
    }

@JohnGalt1717 fixed the 2 events emitted bug on login will publish after testing and must add some unit test for this first. You only get one event now. The forceRefreshToken() kicks off a multiple step flow. If this was refresh tokens, you would get the token back in the observable. But we use iframes here, so the function starts this process, and at some stage in the future, the iframe calls back with the code, and then we get the tokens. After which the auth event is fired.

So after you start the forceRefreshSession, the next auth event will be fired when the new tokens have be processed and ready to use.

Ok... too bad that there isn't a way to pool for the iframe result and then return. Would make it way more elegant. Thanks for all of the efforts!

yes, but is 6 months, maybe a little longer, the iframe flows will probably dissappear

Why? I thought refresh tokens were insecure on the web?

At the moment yes, but Safari is moving to block cookies cross site, and Chrome will follow, so iframe renew will stop working at some stage in the future. Only other option it looks like will be the refresh tokens, which have problems... It will become a little safer in the future when token rotation is supported (server side) and revocation is supported to reject the tokens, and one time usage, all on the server part.

At the moment iframe renew is still the best way.

Greetings Damien

Ok. Thanks for the heads up.

I wish the browser companies would embed a native openIdConnect client into the browser where the site could call a JavaScript function with the idP and have a login/logout function, an event for auth changes, and a method to get the decoded id token and the access token.

That way the browser would do all of the work, there wouldn't be any login loop, and it would be completely secure because refresh tokens etc. wouldn't be shared with JavaScript and thus not be publicly available.

@JohnGalt1717 closing this now, fixed in 11.1.1 Thanks for reporting and your help

Greetings Damien

Was this page helpful?
0 / 5 - 0 ratings

Related issues

nizarkhsib picture nizarkhsib  路  4Comments

yelhouti picture yelhouti  路  4Comments

vit100 picture vit100  路  4Comments

sdev95 picture sdev95  路  3Comments

cgatian picture cgatian  路  4Comments