Angular-auth-oidc-client: Auth Guard is fired before OidcSecurityService.checkAuth() finishes

[mvanheijningen-aquadis]

Hi,

im running into a weird issue. My Authguard is returning false because isAuthenticated retuns false because checkauth is not fully completed.

also the token, keys and userdata servicecalls are called multiple times and i have no clue why.

App.Module.ts

export function configureAuth(oidcConfigService: OidcConfigService) {
  const configSvc = new ConfigService();
  const config = configSvc.get('oidc');

  return () =>

    oidcConfigService.withConfig({
      stsServer: config.stsServer,
      redirectUrl: config.redirectUrl,
      postLogoutRedirectUri: config.postLogoutRedirectUri,
      clientId: config.clientId,
      scope: config.scope,
      responseType: 'code',
      renewTimeBeforeTokenExpiresInSeconds: 10,
      logLevel: LogLevel.Debug,
    });
}

@NgModule({
  declarations: [
    AppComponent,
  ],
  imports: [
    BrowserModule,
    AuthModule.forRoot(),
    AppRoutingModule,
    ServiceWorkerModule.register('ngsw-worker.js',
      {
        enabled: environment.production,
        registrationStrategy: 'registerImmediately'
      }),
    BrowserAnimationsModule,
    xxxCoreModule,
    xxxSharedModule
  ],
  providers: [
    OidcConfigService,
    {
      provide: APP_INITIALIZER,
      useFactory: configureAuth,
      deps: [OidcConfigService],
      multi: true,
    },
  ],
  exports: [],
  bootstrap: [AppComponent]
})
export class AppModule { }

app.component.ts

import { Component, OnInit } from '@angular/core';
import { ConnectionService } from './core/services/connection.service';
import { SwUpdateService, ConfigService } from './core/services';
import { AuthService } from './core/authentication/auth.service';
import { EventTypes, PublicEventsService } from 'angular-auth-oidc-client';
import { delay, filter } from 'rxjs/operators';

@Component({
  selector: 'app-root',
  templateUrl: './app.component.html',
  styleUrls: ['./app.component.scss']
})


export class AppComponent implements OnInit {

  appconfig: any;

  constructor(
    private authService: AuthService,
    private eventService: PublicEventsService,
    private swUpdate: SwUpdateService,
    private connection: ConnectionService,
    private config: ConfigService) {
    this.appconfig = this.config.get('app');
  }

  ngOnInit() {
this.authService.checkAuth().subscribe((isAuthenticated) => {
      console.warn('app authenticated', isAuthenticated);
      const at = this.authService.getToken();
      console.warn(at);
    });

    this.eventService
      .registerForEvents()
      .pipe(filter((notification) => notification.type === EventTypes.CheckSessionReceived))
      .subscribe((value) => console.log('CheckSessionReceived with value from app', value));

    this.connection.monitor();
    this.swUpdate.monitor();
  }

}

Auth.service.ts

import { Injectable } from '@angular/core';
import { OidcSecurityService } from 'angular-auth-oidc-client';
import { promise } from 'protractor';
import { BehaviorSubject, Observable } from 'rxjs';
import { filter, take } from 'rxjs/operators';

@Injectable()
export class AuthService {

  constructor(private oidcSecurityService: OidcSecurityService) { }

  public getUserData(): Observable<any> {
    return this.oidcSecurityService.userData$;
  }

  public getToken(): string {
    return this.oidcSecurityService.getToken();
  }

  public get isAuthenticated$(): Observable<boolean> {
    return this.oidcSecurityService.isAuthenticated$;
  }

  public checkAuth(): Observable<boolean> {
    return  this.oidcSecurityService.checkAuth();
  }

  public login(): void {
    this.oidcSecurityService.authorize();
  }

  public logout(): void {
    this.oidcSecurityService.logoff();
  }
}

Auth.guard.ts

import { Injectable } from '@angular/core';
import { CanActivate, ActivatedRouteSnapshot, RouterStateSnapshot, UrlTree, Router } from '@angular/router';
import { Observable } from 'rxjs';
import { map } from 'rxjs/operators';
import { AuthService } from '../authentication/auth.service';

@Injectable()
export class AuthGuard implements CanActivate {
  constructor(private authService: AuthService, private router: Router) { }

  canActivate(
    next: ActivatedRouteSnapshot,
    state: RouterStateSnapshot): Observable<boolean | UrlTree> | Promise<boolean | UrlTree> | boolean | UrlTree {

    return this.authService.isAuthenticated$.pipe(
      map((isAuthorized: boolean) => {
        console.log('AuthorizationGuard, canActivate isAuthorized: ' + isAuthorized);

        if (!isAuthorized) {
          this.authService.login();
          return false;
        }

        return true;
      })
    );

  }
}

[mvanheijningen-aquadis]

as an addition to my text above:

[valdian]

Yes, this is how it currently works - if you subscribe to isAuthenticated$ before checkAuth() does its job, isAuthenticated$ immediately emits its default value false. And after the tokens are checked, it emits another value indicating the current authentication state.

What you can do, is to ignore values from isAuthenticated$ before checkAuth() emits a value. You could change your auth-service for example like this:

export class AuthService {
    private checkAuthCompleted$ = new ReplaySubject(1);

    constructor(private oidcSecurityService: OidcSecurityService) {}

    // ...

    public get isAuthenticated$(): Observable<boolean> {
        return this.checkAuthCompleted$.pipe(
            first(),
            switchMap((_) => this.oidcSecurityService.isAuthenticated$)
        );
    }

    public checkAuth(): Observable<boolean> {
        return this.oidcSecurityService.checkAuth().pipe(tap((_) => this.checkAuthCompleted$.next()));
    }

    // ...
}

@damienbod @FabianGosebrink I would personally prefer ReplaySubject(1) to BehaviorSubject in these places so that no default value is emitted before the authentication is checked:
https://github.com/damienbod/angular-auth-oidc-client/blob/cffb44bff0e6d689def032fd1fc03fba6e783524/projects/angular-auth-oidc-client/src/lib/authState/auth-state.service.ts#L13
https://github.com/damienbod/angular-auth-oidc-client/blob/cffb44bff0e6d689def032fd1fc03fba6e783524/projects/angular-auth-oidc-client/src/lib/iframe/check-session.service.ts#L24
https://github.com/damienbod/angular-auth-oidc-client/blob/cffb44bff0e6d689def032fd1fc03fba6e783524/projects/angular-auth-oidc-client/src/lib/userData/user-service.ts#L15
Is there any reason why BehaviorSubject should be used? If no, I could try to implement the change. But it should probably be considered a breaking change.

[FabianGosebrink]

Hey @mvanheijningen-aquadis and @valdian , thanks for your inputs and for being constructive. I do understand the issue that when you hit a route, the mechanism has not finished yet so the isAuthorized is false (better pending. this would be cleaner). However, when you navigate back to your app I normally take the approach to hit a route called callback or something which is open and not protected, with nothing on it but to check everything and then, when all is set, redirects you to the route where you really wanted to go. Is it correct that with this approach you would fix the issue, because you would not hit the protected route in the first place? Or am I understanding it wrong? Cheers and thanks once again

[mvanheijningen-aquadis]

Hi @FabianGosebrink, this is entirely correct.
unfortunatly the whole app we are building must be secured so each and every page has a guard.

[mvanheijningen-aquadis]

Yes, this is how it currently works - if you subscribe to isAuthenticated$ before checkAuth() does its job, isAuthenticated$ immediately emits its default value false. And after the tokens are checked, it emits another value indicating the current authentication state.

What you can do, is to ignore values from isAuthenticated$ before checkAuth() emits a value. You could change your auth-service for example like this:

export class AuthService {
    private checkAuthCompleted$ = new ReplaySubject(1);

    constructor(private oidcSecurityService: OidcSecurityService) {}

    // ...

    public get isAuthenticated$(): Observable<boolean> {
        return this.checkAuthCompleted$.pipe(
            first(),
            switchMap((_) => this.oidcSecurityService.isAuthenticated$)
        );
    }

    public checkAuth(): Observable<boolean> {
        return this.oidcSecurityService.checkAuth().pipe(tap((_) => this.checkAuthCompleted$.next()));
    }

    // ...
}

@damienbod @FabianGosebrink I would personally prefer ReplaySubject(1) to BehaviorSubject in these places so that no default value is emitted before the authentication is checked:
https://github.com/damienbod/angular-auth-oidc-client/blob/cffb44bff0e6d689def032fd1fc03fba6e783524/projects/angular-auth-oidc-client/src/lib/authState/auth-state.service.ts#L13

https://github.com/damienbod/angular-auth-oidc-client/blob/cffb44bff0e6d689def032fd1fc03fba6e783524/projects/angular-auth-oidc-client/src/lib/iframe/check-session.service.ts#L24

https://github.com/damienbod/angular-auth-oidc-client/blob/cffb44bff0e6d689def032fd1fc03fba6e783524/projects/angular-auth-oidc-client/src/lib/userData/user-service.ts#L15

Is there any reason why BehaviorSubject should be used? If no, I could try to implement the change. But it should probably be considered a breaking change.

Goging to try this right away!

[mvanheijningen-aquadis]

After this implementation i see one thing:

it is not getting the configuration from the well known endpoint.

[valdian]

@mvanheijningen-aquadis Could you share more details? For example your console logs.

[mvanheijningen-aquadis]

Sorry for my late reply. i figuerd that in my intercepters the next() function was not called because the isAuthenticated$ is now not returning anything. so i modified my Interceptors with the following lines:

import { Injectable } from '@angular/core';
import { HttpInterceptor, HttpRequest, HttpHandler, HttpEvent } from '@angular/common/http';
import { Observable } from 'rxjs';
import { AuthService } from '../authentication/auth.service';
import { ConfigService } from '../services';
import { mergeMap } from 'rxjs/operators';

@Injectable()
export class HttpTokenInterceptor implements HttpInterceptor {

  constructor(private authService: AuthService, private config: ConfigService) { }

  intercept(req: HttpRequest<any>, next: HttpHandler): Observable<HttpEvent<any>> {
    const apiConfig = this.config.get('api');

    return this.authService.isAuthenticated$.pipe(mergeMap((auth: boolean) => {
      if (auth && req.url.indexOf(apiConfig.endpoints.baseAddress, 0) > -1) {
        const token = this.authService.getToken();
        req = req.clone({
          setHeaders: {
            Authorization: 'Bearer' + token
          }
        });
      }

      return next.handle(req);
    **}),
      () => next.handle(req));
  }**
}

[mvanheijningen-aquadis]

turns out, the above code works perfectly when not logged in,.. but when logged in the 2nd function is fired faster than the switchmap. so now it always skips the auth check :( any help would be appriciated

[mvanheijningen-aquadis]

looks like i need some way to timeout the pipe if isAuthenticated$ does not return anything. after the timeout the next.handle(req) should be fired without changes

[FabianGosebrink]

Is this problem still existing?

[luis-abeno]

This problem was fixed in another version?