Angular-oauth2-oidc: Impossible to redirect to previous page after logging in

Created on 7 Sep 2018  路  17Comments  路  Source: manfredsteyer/angular-oauth2-oidc

Hello!

I am using this library for an oauth protected application. Using an auth-guard, the secured pages trigger a login page which calls an authservice configured as in the code snippet.

Somehow it seems impossible to redirect to the chosen page after logging in, as only one redirect can be set. Anyone an idea how to configure it? Listening to the event "token_received" does not seem to work.

```@Injectable()
export class AuthService {

constructor(
private router: Router,
private oauthService: OAuthService
) {
this.oauthService.configure(authConfig);
this.oauthService.setupAutomaticSilentRefresh();

this.oauthService.tokenValidationHandler = new JwksValidationHandler();
this.oauthService.tryLogin({});

}

login() {
this.oauthService.initImplicitFlow();
}

}```

docs pr-welcome

Most helpful comment

I'm using the same library, and in my production application I have a flow similar to what you describe (which I documented in a toy-problem version in a sample repo). It's still on my to do list to add the feature you describe (navigate to the intended page after getting back from the ID Server, right?) to my production application, but I have played around with it a bit.

I also found that responding to events like you mention does not work for this feature. I'm guessing the implementation of state upon returning is lagging behind the library's move towards events. But that's a guess.

As for a solution, I found that this can work:

Send the intended url along with the user when they go to the ID Server (you might need to do this in the auth guard based on the target URL instead of the current URL):

public login() { this.oauthService.initImplicitFlow(encodeURIComponent(this.router.url)); }

PS. The encodeURIComponent is because of #415 (e.g. ? characters get lost otherwise).

Then when the user gets back you cannot use the event as you noted, but I found that this does work:

this.oauthService.loadDiscoveryDocument()
  .then(() => this.oauthService.tryLogin({
    onTokenReceived: () => this.router.navigateByUrl(this.oauthService.state),
  }))
  // etc.

Edit: see updated/improved solution below.

Hope that helps.

All 17 comments

I'm using the same library, and in my production application I have a flow similar to what you describe (which I documented in a toy-problem version in a sample repo). It's still on my to do list to add the feature you describe (navigate to the intended page after getting back from the ID Server, right?) to my production application, but I have played around with it a bit.

I also found that responding to events like you mention does not work for this feature. I'm guessing the implementation of state upon returning is lagging behind the library's move towards events. But that's a guess.

As for a solution, I found that this can work:

Send the intended url along with the user when they go to the ID Server (you might need to do this in the auth guard based on the target URL instead of the current URL):

public login() { this.oauthService.initImplicitFlow(encodeURIComponent(this.router.url)); }

PS. The encodeURIComponent is because of #415 (e.g. ? characters get lost otherwise).

Then when the user gets back you cannot use the event as you noted, but I found that this does work:

this.oauthService.loadDiscoveryDocument()
  .then(() => this.oauthService.tryLogin({
    onTokenReceived: () => this.router.navigateByUrl(this.oauthService.state),
  }))
  // etc.

Edit: see updated/improved solution below.

Hope that helps.

Thank you for your extended answer! I really appreciate it.
It almost works out, the redirect is correct now. However, it ends up in an endless loop calling the redirect url + ?acces_token=xxx over and over again. Does someone have an idea to do this redirect only once?

Hmm, I've seen that before, it might depend on specifics. At first we had similar problems too, until we refactored to a chain of promises like this, and blocking the auth guard until that chain is fairly complete.

Combining that with what you posted, in the code you showed the tryLogin({}) stands on its own, but it returns a promise. Could you try somehow blocking your auth-guard until tryLogin() has completed? I've tried an approach with exposing that returned promise to my auth-guard and .thenning on it, but ended up with a solution that puts an isDoneLoading$ observable in between.

Could you share what your auth-guard looks like? And possibly any other relevant timing-related code?

Yes for sure!
Meanwhile I also discovered that the nonce was not validated, apparently, it then tries to acquire to access token again, but that is a real mystery to me, why it is doing that and why it is invalid.

My authgard:

@Injectable()
export class AuthGuard implements CanActivate {

  constructor(
    private router: Router,
    private authService: AuthService,
  ) { }

  canActivate(
    route: ActivatedRouteSnapshot,
    state: RouterStateSnapshot,
  ) {
    if (this.authService.isUserLoggedIn()) {
      return true;
    }

    this.authService.login();

    return false;
  }
}

My authService:

@Injectable()
export class AuthService {

  constructor(
    private router: Router,
    private oauthService: OAuthService
  ) {
    this.oauthService.configure(authConfig);
    this.oauthService.setStorage(localStorage);
    // this.oauthService.setupAutomaticSilentRefresh();

    this.oauthService.tokenValidationHandler = new JwksValidationHandler();
  }

  login() {
    this.oauthService.initImplicitFlow();
  }

  isUserLoggedIn(): boolean {
    let isLoggedIn = false;
    this.oauthService.tryLogin({
        onTokenReceived: () => {
          this.router.navigateByUrl(encodeURIComponent(this.oauthService.state));
          isLoggedIn =  true;
        }
      });
    return isLoggedIn;
  }
}

I tried to solve the timing issue by create this isUserLoggedIn() method

That code helps indeed: it looks a lot like what we had at first too.

You could try making canActivate into one that returns a Promise<boolean> | Observable<boolean> instead of a boolean, things will hopefully improve if you do. Put differently, I speculate that return isLoggedIn; runs _before_ isLoggedIn = true, though I'd have to try your code to be sure (no time today anymore, unfortunately).

Let me know if you solve things in the mean time, otherwise I might have a go later on, since we need a solution for this as well.

The biggest problem is initImplicitFlow(). This one should return a Promise. Once that call is done, the Promise can be resolved and redirect to the url called in the guard. Atm I have no clue how to fix this and actually wondering whether we should continue with this library.

I'm not sure if I follow why initImplicitFlow() should return a Promise? Its job is to redirect the user to the ID Server, so its return value doesn't matter, right?

Although I agree that this library can be hard to use at times, I think that this specific point is more a hard thing about the implicit flow itself. As it includes redirection (even for the silent refreshes, though that's done without prompts and hidden iframes), it will be hard to preserve state with any SPA library I think.

Either way, I got around to implementing this for my own app, by first building it in my toy repository. You can clone it and check out the full solution, but here's the important bits for state preservation:

My canActivate guard looks like this:

@Injectable()
export class AuthGuardWithForcedLogin implements CanActivate {
  private isAuthenticated: boolean;

  constructor(
    private authService: AuthService,
  ) {
    this.authService.isAuthenticated$.subscribe(i => this.isAuthenticated = i);
  }

  canActivate(
    route: ActivatedRouteSnapshot,
    state: RouterStateSnapshot,
  ): Observable<boolean>{
    return this.authService.isDoneLoading$
      .pipe(filter(isDone => isDone))
      .pipe(tap(_ => (this.isAuthenticated || this.authService.login(state.url))))
      .pipe(map(_ => this.isAuthenticated));
  }
}

It returns an observable based off authService.isDoneLoading$, which only produces values if the entire asynchronous login chain has finished (or thrown an error).

It taps the observable, and sends the user to login(...) sending the intended URL along. Note that if it does so, most likely the canActivate observable never completes anymore because the user gets redirected to log in.

Put differently, if you try to activate a route guarded by this service, the user will:

  • have to wait for a first observable value until the login process is complete
  • be redirected to login right away once the login process completes but the user isn't logged in
  • be sent along to the target route if the login process completes and the user _is_ logged in

Once the user gets back from the server after being sent off to log in, this is my process in my auth-service wrapper:

private isAuthenticatedSubject$ = new BehaviorSubject<boolean>(false);
public isAuthenticated$ = this.isAuthenticatedSubject$.asObservable();

private isDoneLoadingSubject$ = new ReplaySubject<boolean>();
public isDoneLoading$ = this.isDoneLoadingSubject$.asObservable();

public canActivateProtectedRoutes$: Observable<boolean> = combineLatest(
  this.isAuthenticated$,
  this.isDoneLoading$
).pipe(map(values => values.every(b => b)));
return this.oauthService.loadDiscoveryDocument()
  // For demo purposes, we pretend the previous call was very slow
  .then(() => new Promise(resolve => setTimeout(() => resolve(), 1000)))
  .then(() => this.oauthService.tryLogin())
  .then(() => {
    if (this.oauthService.hasValidAccessToken()) {
      return Promise.resolve();
    }

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

        if (result && result.reason && errorResponsesRequiringUserInteraction.indexOf(result.reason.error) >= 0) {
          console.warn('User interaction is needed to log in, we will wait for the user to manually log in.');
          return Promise.resolve();
        }

        return Promise.reject(result);
      });
  })
  .then(() => {
    this.isDoneLoadingSubject$.next(true);

    // Check for the strings 'undefined' and 'null' just to be sure. Our current
    // login(...) should never have this, but in case someone ever calls
    // initImplicitFlow(undefined | null) this could happen.
    if (this.oauthService.state && this.oauthService.state !== 'undefined' && this.oauthService.state !== 'null') {
      console.log('There was state, so we are sending you to: ' + this.oauthService.state);
      this.router.navigateByUrl(this.oauthService.state);
    }
  })
  .catch(() => this.isDoneLoadingSubject$.next(true));

I opted _not_ to use the onTokenReceived. Instead, at the end of the chain, I check if there was any state and if so, I send the user there. The reason I do this is because a user might try to go to a protected URL e.g. with a bookmark, and then you need to wait for the silentRefresh() to complete before knowing if the user can go there.

Effectively, that this.router.navigateByUrl(this.oauthService.state) call at the end will happen when the ID Server had sent the user back and all turns out to be well.

Hope this helps (either with an implementation, or with deciding not to use the library :smile:).

As a final note and separate comment, I think that the "Preserving State" documentation is a bit too limited. Perhaps either this GitHub issue, or a new fresh one could be used to note that it should be updated / expanded?

@jeroenheijmans @PaulienVa
I'm experiencing problems redirecting to the state returned by the provider. I'm getting Router Event: NavigationCancel without any reason given. Before it starts a new Router Event: NavigationStart right after. Is this the same issue?
I'm using a version of your setup @jeroenheijmans.

public runAuthenticationProcess(): Promise<void> {
    // 1. Load discovery document
    return this.oauthService.loadDiscoveryDocument()
    // 2. Try login from hash
    .then(() => this.oauthService.tryLogin())
    // 3. Check if login vas valid, otherwise try silent refresh
    .then(() => {
      if (this.oauthService.hasValidAccessToken()) {
        return Promise.resolve();
      }
      return this.oauthService.silentRefresh()
      .then(() => Promise.resolve())
      .catch(result => {
        const silent_errors = ['interaction_required', 'login_required', 'account_selection_required', 'consent_required'];
        if (result && result.reason && silent_errors.indexOf(result.reason.error) >= 0) {
          return Promise.resolve();
        }
        return Promise.reject(result);
      });
    })
    // 4. Finish loading and redirect to state if found
    .then(() => {
      this.isLoadedSubject$.next(true);
      if (this.oauthService.state && this.oauthService.state !== 'undefined' && this.oauthService.state !== 'null') {
        this.router.navigateByUrl(this.oauthService.state);
      }
    })
    // 5. Catch if failed and mark as loaded
    .catch(() => {
      this.isLoadedSubject$.next(true);
    });
  }

@ggjersund That sounds like a related but quite different issue. I recommend creating a minimal repro, and asking a Stack Overflow question (or possibly report a new GitHub issue if you think you found a bug with the library).

@jeroenheijmans I've created Stackoverflow question here

@ggjersund I've had similar issue, while trying to redirect user back to requested url on onTokenReceived event. Fixed it by setting clearHashAfterLogin setting to false, therefore the router is not triggered on location.hash = '';, so that this.router.navigateByUrl works correctly.

@LeonsBuntis How do you handle the case when a state is not provided? The hash would still be in the url wouldn't it?

@ggjersund I'm using Implicit Flow, so when user is trying to access the web site it gets redirected to authorization server, after login user is redirected back to main web site. But there's no possibility to start accessing web site from authorization server, therefore there can't be situation where state is not provided.
But in any way, if you don't have any state, that means that there was no previously requested url, so then you can redirect it back to some sort of home-page?

I am having trouble with the navigation as it seems navigationcancel returns false which cancel the navigation. Below is the code for AuthGuard and AuthenticationService

AuthGuard

import { Injectable } from '@angular/core'
import { Router, CanActivate, ActivatedRouteSnapshot, RouterStateSnapshot } from '@angular/router'
import { AuthenticationService } from './authentication.service'

@Injectable()
export class AuthGuardService implements CanActivate {
constructor(private auth: AuthenticationService, private router: Router){}

canActivate(route: ActivatedRouteSnapshot, state: RouterStateSnapshot,) {
    if(this.auth.isLoggedIn()){
        this.router.navigate(['/dashboard'])
        return true
    }
    return false
  }
}

AuthenticationService

import { Injectable } from '@angular/core'
import { HttpClient } from '@angular/common/http'
import { Observable, of } from 'rxjs'
import { map } from 'rxjs/operators'
import { Router } from '@angular/router'

export interface LoginDetails {
    _id: string
    first_name: string
    last_name: string
    email: string
    password: string
    cat: string
    exp: number
    iat: number
}

interface TokenResponse {
    token: string
}

export interface TokenPayload {
    _id: string
    first_name: string
    last_name: string
    email: string
    password: string
    cat: string
}
@Injectable()
export class AuthenticationService {
    private token : string

    constructor(private http: HttpClient, private router: Router){}

    private saveToken(token : string) : void{
        localStorage.setItem('userToken', token)
        this.token = token
    }

    private getToken(): string {
        if(!this.token){
            this.token = localStorage.getItem('userToken')
        }
        return this.token
    }

    public getLoginDetails(): LoginDetails {
        const token = this.getToken()
        let payload
        if(payload){
            payload = token.split('.')[1]
            payload = window.atob(payload)
            return JSON.parse(payload)
        }else{
            return null
        }
    }

    public isLoggedIn(): boolean {
        const user = this.getLoginDetails()
        if(user){
            return user.exp > Date.now() / 1000
        }else{
            return false
        }
    }

    public login(user: TokenPayload): Observable<any>{
        const base = this.http.post('http://localhost:5000/login', user);

        const request = base.pipe(
            map((data:TokenResponse) => {
                if(data.token) {
                    console.log("Executed",data.token);
                    this.saveToken(data.token);
                //this.router.navigate(['doctor']);
                //console.log("Navigated");
            }
            return data
        })
    )
    return request
    }

}

I got this working using @jeroenheijmans suggestion in this thread https://github.com/manfredsteyer/angular-oauth2-oidc/issues/424#issuecomment-419713360

But, I did have to call decodeURIComponent on the state before navigating:

this.router.navigateByUrl(decodeURIComponent(this.oauthService.state));

Given the various emoji responses on specific comments, I think we could close this issue by improving the docs. If anyone want to document this case in a prominent place I presume that would be welcomed.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

andrea-spotsoftware picture andrea-spotsoftware  路  3Comments

phrouv picture phrouv  路  4Comments

uzzafar picture uzzafar  路  4Comments

medokin picture medokin  路  4Comments

kneefer picture kneefer  路  3Comments