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

Provide authentication guard #707

Closed
rbschiefer opened this issue Jan 23, 2020 · 2 comments
Closed

Provide authentication guard #707

rbschiefer opened this issue Jan 23, 2020 · 2 comments
Labels
feature-request Improvements and additions to the library.

Comments

@rbschiefer
Copy link

rbschiefer commented Jan 23, 2020

I am using angular-oauth2-oidc with simple oauth implicit flow. Using this library as described in the documentation, you see the page you want to navigate to for a very short time (as long as the redirect kicks in).

A solution for me is a simple auth guard checking for valid id and access token.

I would like to have this kind of standard guard provided by the library instead of implementing it on my own.

See a very simple implementation here:
https://angular-prwr5z.stackblitz.io - Although, the redirect is not working due to the not registered domain, you see the problem: the content is shown at loading time.

Link for editing: https://stackblitz.com/edit/angular-prwr5z

@jeroenheijmans jeroenheijmans added the feature-request Improvements and additions to the library. label Jan 24, 2020
@jeroenheijmans
Copy link
Collaborator

jeroenheijmans commented Jan 24, 2020

I think the link for editing may be an old one? It has no code for redirection or logging in at all?

When I first started using this library I also felt the need for such a guard. But after working with it for some time it feels less logical to me to have it in the library, as there's many small variations possible, depending on your application and context.

Instead, it might be nice to have some "additional documentation" to suggest a starting point for applications?


FWIW, and possibly useful for such 'additional documentation', here's a clean variant of the guards I tend to use. First one that sends you to the login server immediately if you want to access a protected route:

@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));
  }
}

and one that just prevents navigation:

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

  canActivate(
    route: ActivatedRouteSnapshot,
    state: RouterStateSnapshot,
  ): Observable<boolean> {
    return this.authService.canActivateProtectedRoutes$
      .pipe(tap(x => console.log('You tried to go to ' + state.url + ' and this guard said ' + x)));
  }
}

In both cases they rely on some observables in an application service (wrapper around the OAuthService):

  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)));

Those in turn are set up by an application-specific bit of logic:

    this.oauthService.events
      .subscribe(_ => {
        this.isAuthenticatedSubject$.next(this.oauthService.hasValidAccessToken());
      });
    this.oauthService.loadDiscoveryDocument()
      .then(() => new Promise(resolve => setTimeout(() => resolve(), 1000))) // for debugging
      .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' ];
            return (result && result.reason && errorResponsesRequiringUserInteraction.indexOf(result.reason.error) >= 0)
              ? Promise.resolve()
              : Promise.reject(result);
          });
      })
      .then(() => {
        this.isDoneLoadingSubject$.next(true);
        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));

@jeroenheijmans
Copy link
Collaborator

I think a guard as part of the library currently makes no sense, given that CanActivate (a guard) would imply a dependency on @angular/router too, where this library doesn't require it currently.

For reference, the other options are:

  • this issue (even if it is closed) documents above one approach
  • my sample repo's guards should help
  • and if someone would make a PR with "Additional Documentation" to make the above more visible would still be welcome, I think

Closing the issue for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request Improvements and additions to the library.
Projects
None yet
Development

No branches or pull requests

2 participants