checkAuthIncludingServer cannot complete without credentials

[1619digital]

Hi, owing to the rewrite of forceRefreshSession (thank you) there are circumstances where the checkAuthIncludingServer and / or forceRefeshSession observables will never return a value. I believe its a race error, but is apparent (for me) if either is subscribed to without any existing credentials in the sessionStorage.

What's happening -

1. The silentRenewEventHandler creates the callback to handle the IFrame result via codeFlowCallbackSilentRenewIframe , receives an error from the IFrame querystring because the refresh request didn't succeed and immediately throws
2. silentRenewEventHandler catches the error in its subscription to the callback, and updates refreshSessionWithIFrameCompletedInternal$ to null - all correctly
3. However, all this happens before startRefreshSession() actually returns a result - and because the results are expected consecutively, forceRefreshSession will now wait forever for a result from refreshSessionWithIFrameCompleted$

I guess it may or may not happen depending on speed of network.

I suggest a fix is to deal with the observable concurrently

    return this.startRefreshSession().pipe(
      switchMap(() => this.silentRenewService.refreshSessionWithIFrameCompleted$),
      take(1),
      map((callbackContext) => {
           ...
      })
    );

... to ...

    return forkJoin(
        this.startRefreshSession(), 
        this.silentRenewService.refreshSessionWithIFrameCompleted$.pipe(take(1)))
    .pipe(
      map(([callbackContext, _]) => {
           ...
      })
    );

Thank you!

[damienbod]

Hi @PartyArk

We used this initially but the refreshSession ws returning early and so we decided to change it. Is they a way to reproduce this?

Greetings Damien

[Expelz]

Hi @damienbod!

I've been faced with the same problem. Steps for reproduce:

1. Clear all cookies and local storage related to your SPA-oidc.
2. After 'withConfig()' configuration finished invoke 'checkAuthIncludingServer()' and subscribe on it.
3. Then if your IS worked correctly you will get next error message:
   
   It is expected error which indicates that you try to do renew (/authorize endpoint) without authentication cookies.
4. Then you will discover that your subscription from step 2 will never be executed.

Hope this help you.
Thank you!

[1619digital]

Thanks @Expelz - yes this is reasonably easy to reproduce -

1. fire up angular-auth-oidc-client-master and start
2. mess around trying to get https localhost to work :)
3. login to the built-in sts server offeringsolutions-sts.azurewebsites.net using PKCE code flow on vanilla sample-code-flow, and make sure localhost:4200 is logged in
4. clear Session Storage
5. reload (you have to be quite quick, I think the refresh timings are quite tight on the test setup)
6. Note that console.warn('app authenticated', isAuthenticated) is never hit because checkAuthIncludingServer never completes

It's a bit of an insidious error, because the way the test-server is setup, it's not obvious that there is a problem at all. However, if you're using the result of checkAuthIncludingServer in a route guard (e.g. canLoad or canActivate) then you're in real trouble!

Thanks again.

[FabianGosebrink]

Hey @PartyArk , just tried it out with your steps and I could not reproduce. Am I doing something wrong?

1. sample-code-flow
2. cleared storage
3. immediatelly refresh

[1619digital]

Well I've just tried again and yours isn't the same as mine, so there must be some difference!

I'm opening up the code in VS Code on Windows, doing npm install and start. Login in, delete session state, and here's the console on the next reload.

As expected app authenticated is never hit and the subscription never completes.

It looks quite different to yours (bit hard to read because of the animation) but clearly your authentication process is getting much further. I think I'm the same as @Expelz - the login_required error is where it all goes wrong.

---

Ok, so I've taken another look and it looks like this isn't happening in Edge (Chrome engine) or Firefox. So it's something local. After digging around there is this setting here - I think it's on by default in Chrome (could be wrong) -

.... and when I load the page now I get this little icon top right

... which tells me I'm blocking third-party cookies. If I allow third party cookies the oidc process works. So, I think this is the key.

EDIT - Thinking about it, blocking all third party cookies completely is always going to cause silent refresh to fail (unless your sts and client are on the same domain). It's quite a common corporate-enforced setting in my experience.

And that's ok, because silent-refresh is a cookie based system. The bug is that it causes the oidc authentication to effectively hang permanently if you're using it as part of a route guard, and at least never to return a yes / no.

Thanks for taking the time to look at this.

[FabianGosebrink]

Hey, I just changed something, can you try out this branch? Thanks. https://github.com/damienbod/angular-auth-oidc-client/tree/fabiangosebrink/fixing-missing-catch-error

[1619digital]

Hi, I'm afraid a simple catch isn't going to work - the issue is one of timing on the subscription, where the error (and hence completion) of refreshSessionWithIFrameCompletedInternal happens before silentRenewEventHandler completes, and so expecting them to happen consecutively causes the method to hang.

I set out in the original post in more detail why it's not completing. Hopefully you can reproduce your end. Thanks!

[FabianGosebrink]

Thanks for the answer. We tested the code you wrote above (funnily we have had that code without looking at it :-) ) but during testing it did not work for us. How about this: We are accepting PRs. So you can fix it as you think it should be fixed and we will run your code against our test as well. If all is good we are happy to merge. Will look into it further as well.

[1619digital]

Ok I will try that, would be interested to know what didn't work.

As a general point, you way want to run more tests against blocked third party cookies, might throw something else up.

[FabianGosebrink]

Yeah that'd be really interesting. Would be interested as well. Thanks, yeah, will def try that!

[1619digital]

I've had another look at this - I have tried forcing karma to launch a browser with third-party-cookies-disabled but I'm not really sure how. Nor do I know how I would write a test, sorry.

However, the fix is really simple - there's a glitch in the forkJoin I proposed and you said you'd tried too. The returned array parameters are in the wrong order. Should be:

    return forkJoin(
        this.startRefreshSession(),
        this.silentRenewService.refreshSessionWithIFrameCompleted$.pipe(take(1)))
        .pipe(
            map(([_, callbackContext]) => {
                   ....
            })
        );

All tests pass with this in place, and my "manual" testing indicates that the hanging silent-renew bug goes away too. But I think it would be better to have some proper tests in place.

[FabianGosebrink]

Hey @PartyArk, thanks for the fix. We have not been far away then, thanks! Does it also work not only one but multiple times? Cheers

[1619digital]

Should do, yes.

Having thought about it some more, I suggest the 3rd party cookies and clearing the session state is a bit of a distraction. The essence of the bug is simply this - if silent renew comes back with in an error state, then the process can't complete.

There's a thousand combinations of blocked cookies, SameSite settings, latency, broken connections and all the other things that might go wrong with the request to the sts server. If you can simulate an sts error state in your tests, then you should be covered for these scenarios and this bug.

[Expelz]

Hi guys!
As @PartyArk mentioned the problem is occurred here:

return this.startRefreshSession().pipe(
      switchMap(() => this.silentRenewService.refreshSessionWithIFrameCompleted$),
      take(1),
      map((callbackContext) => {
           ...
      })
    );

It happens because next value into refreshSessionWithIFrameCompleted$ will be sent before startRefreshSession() will return observable object.

After long investigation why it happens and in what situations, I finally found root cause of the problem:

1. startRefreshSession() will execute this function:
   

   angular-auth-oidc-client/projects/angular-auth-oidc-client/src/lib/iframe/refresh-session-iframe.service.ts

   Lines 26 to 41 in 0799173

   	private sendAuthorizeReqestUsingSilentRenew(url: string): Observable<boolean> {
   	const sessionIframe = this.silentRenewService.getOrCreateIframe();
   	this.initSilentRenewRequest();
   	this.loggerService.logDebug('sendAuthorizeReqestUsingSilentRenew for URL:' + url);
   	return new Observable((observer) => {
   	const onLoadHandler = () => {
   	sessionIframe.removeEventListener('load', onLoadHandler);
   	this.loggerService.logDebug('removed event listener from IFrame');
   	observer.next(true);
   	observer.complete();
   	};
   	sessionIframe.addEventListener('load', onLoadHandler);
   	sessionIframe.src = url;
   	});
   	}

   
   This code will register handler for 'load' event and then redirect iframe to IS authorize endpoint - sessionIframe.src = url;
2. Then only after unsuccessful authorize (authentication cookies expired or doesn't exist or any other IS error like invalid grant_type etc.) the iframe will be redirected back to SPA special page for silent renew - "https://********/silent-renew.html".
3. Inside this page we have inline script with onload handler function, which will dispatch event and after will start execution of silent renew event handler function:
   

   angular-auth-oidc-client/projects/angular-auth-oidc-client/src/lib/iframe/silent-renew.service.ts

   Lines 100 to 128 in 0799173

   	silentRenewEventHandler(e: CustomEvent) {
   	this.loggerService.logDebug('silentRenewEventHandler');
   	if (!e.detail) {
   	return;
   	}
   	let callback$ = of(null);
   	const isCodeFlow = this.flowHelper.isCurrentFlowCodeFlow();
   	if (isCodeFlow) {
   	const urlParts = e.detail.toString().split('?');
   	callback$ = this.codeFlowCallbackSilentRenewIframe(urlParts);
   	} else {
   	callback$ = this.implicitFlowCallbackService.authorizedImplicitFlowCallback(e.detail);
   	}
   	callback$.subscribe(
   	(callbackContext) => {
   	this.refreshSessionWithIFrameCompletedInternal$.next(callbackContext);
   	this.flowsDataService.resetSilentRenewRunning();
   	},
   	(err: any) => {
   	this.loggerService.logError('Error: ' + err);
   	this.refreshSessionWithIFrameCompletedInternal$.next(null);
   	this.flowsDataService.resetSilentRenewRunning();
   	}
   	);
   	}

And here is occurred main problem - we have two event handlers for 'load' event for iframe.
Event handler from inline script (silent-renew.html) will be executed first.
And given that the authorization was unsuccessful (step 2), execution of this 'load' event handler will be finished after throw error inside silent renew service:

angular-auth-oidc-client/projects/angular-auth-oidc-client/src/lib/iframe/silent-renew.service.ts

Lines 56 to 73 in 0799173

	codeFlowCallbackSilentRenewIframe(urlParts) {
	const params = new HttpParams({
	fromString: urlParts[1],
	});
	const error = params.get('error');
	if (error) {
	this.authStateService.updateAndPublishAuthState({
	authorizationState: AuthorizedState.Unauthorized,
	validationResult: ValidationResult.LoginRequired,
	isRenewProcess: true,
	});
	this.flowsService.resetAuthorizationData();
	this.flowsDataService.setNonce('');
	this.intervallService.stopPeriodicallTokenCheck();
	return throwError(error);
	}

Than this error will be handled by subscriber:

angular-auth-oidc-client/projects/angular-auth-oidc-client/src/lib/iframe/silent-renew.service.ts

Lines 117 to 127 in 0799173

	callback$.subscribe(
	(callbackContext) => {
	this.refreshSessionWithIFrameCompletedInternal$.next(callbackContext);
	this.flowsDataService.resetSilentRenewRunning();
	},
	(err: any) => {
	this.loggerService.logError('Error: ' + err);
	this.refreshSessionWithIFrameCompletedInternal$.next(null);
	this.flowsDataService.resetSilentRenewRunning();
	}
	);

Finally here we see call to refreshSessionWithIFrameCompleted$.next().
And now pay attention that all this actions happened before startRefreshSession() will return observable object.

Reasonable question: "Why doesn't this happen with successful authorization (silent renew)?"
The answer: because after success authorization our iframe will do code exchange (code from IS which we should exchange to get token(s)). And it will be done by httpClient through post request it allows to switch execution context to second 'onload' handler (from step 1) which finally will return observable object startRefreshSession() before result from refreshSessionWithIFrameCompleted$.

I agree with @PartyArk forkJoin will handle it.
If you want reproduce such behavior you need clear not only local or sessions storage but also Cookies (there is located authentication cookie from IS).

[FabianGosebrink]

Hey all, first of all: Thanks, you guys are amazing. This is what makes OSS. Thanks. I already pushed @PartyArk s solution into the branch I created. @damienbod could you run the tests, then we can merge and fix this issue. Thanks to all!

[damienbod]

@FabianGosebrink @PartyArk @Expelz Thanks for you work. I'll test this tomorrow, then PR, release, good?

[sekoch]

Hey @damienbod , any update on this issue? I ran into the same problem :-(. Thanks in advance and stay healthy.

[damienbod]

@FabianGosebrink @PartyArk @Expelz

testing this now (with a slight delay, sorry) see the PR

At the moment it looks good.

[damienbod]

will release in version 11.1.4

[gubo]

i have 11.1.4 and this still never completes:
3093 checkAuthIncludingServer() ... isAuthenticated is false ...
2984 return forkJoin([this.startRefreshSession(), this.silentRenewService.refreshSessionWithIFrameCompleted$.pipe
are u still working on this issue ?
let me know if i should prepare stackblitz/sample ...