auth0-angular icon indicating copy to clipboard operation
auth0-angular copied to clipboard

Double login redirect on ionic + angular workflow

Open mrentmeister-tt opened this issue 8 months ago • 20 comments

Checklist

  • [x] The issue can be reproduced in the auth0-angular sample app (or N/A).
  • [x] I have looked into the Readme, Examples, and FAQ and have not found a suitable solution or answer.
  • [x] I have looked into the API documentation and have not found a suitable solution or answer.
  • [x] I have searched the issues and have not found a suitable solution or answer.
  • [x] I have searched the Auth0 Community forums and have not found a suitable solution or answer.
  • [x] I agree to the terms within the Auth0 Code of Conduct.

Description

I followed the instructions with how to setup Auth0 in a capacitor + Angular app listed here: https://auth0.com/docs/quickstart/native/ionic-angular. I then added the AuthGuard to my root path, because I wanted all of my main routes to be protected. After completing a successful login, the app redirects a second time to the login screen, and then immediately redirects back (or makes you select your organization again if doing that flow).

Expected behavor: The app should not need to authenticate you twice in a row.

Reproduction

  1. Setup the sample capacitor + angular demo app: https://auth0.com/docs/quickstart/native/ionic-angular
  2. Setup your main route to be protected by the AuthGuard
// app.routes.ts
export const APP_ROUTES: Route[] = [
  {
    path: 'login-failed',
    component: LoginFailedComponent
  },
  {
    path: 'logout',
    component: LogoutComponent
  },
  {
    path: '',
    runGuardsAndResolvers: 'always',
    canActivate: [AuthGuard],
    children: [
      {
        path: 'tenant-select',
        component: TenantSelectComponent
      },
      {
        path: 'usage',
        component: UsageComponent
      },
      // ... More routes
      {
        path: '',
        pathMatch: 'full',
        redirectTo: 'tenant-select'
      }
    ]
  },
  {
    path: '**',
    redirectTo: 'tenant-select'
  }
];
  1. Launch the app and login. Once logged in, you will be redirect back to the login page again.

Additional context

Looking through the auth0 code, this is happening because of the following sequence of events:

  1. The app launches, goes to '/' route. This triggers the AuthGuard to call loginWithRedirect, because it's not authenticated: https://github.com/auth0/auth0-angular/blob/main/projects/auth0-angular/src/lib/auth.guard.ts#L45
  2. After successfully logging in, the appUrlOpen code executes handleRedirectCallback:
ngOnInit(): void {
    // Use Capacitor's App plugin to subscribe to the `appUrlOpen` event
    App.addListener('appUrlOpen', ({ url }) => {
      // Must run inside an NgZone for Angular to pick up the changes
      // https://capacitorjs.com/docs/guides/angular
      ngZone.run(() => {
        if (url?.startsWith(callbackUri)) {
          // If the URL is an authentication callback URL..
          if (
            url.includes('state=') &&
            (url.includes('error=') || url.includes('code='))
          ) {
            // Call handleRedirectCallback and close the browser
            this.auth
              .handleRedirectCallback(url)
              .pipe(mergeMap(() => Browser.close()))
              .subscribe();
          } else {
            Browser.close();
          }
        }
      });
    });
  }
  1. The code in handleRedirectCallback calls this.authState.refresh(); (this ASYNCHRONOUSLY updates the state), and then continues to SYNCHRONOUSLY redirect back to '/'. Nothing waits for the state to be updated before redirecting to '/'. https://github.com/auth0/auth0-angular/blob/main/projects/auth0-angular/src/lib/auth.service.ts#L305
  2. The redirect to '/' triggers the AuthGuard to execute again, checking the isAuthenticated$ observable (which is still false), and triggers the loginWithRedirect AGAIN.

As best as I can tell from running the code, since isAuthenticated$ uses shareReplay(1), the AuthGuard is getting the replayed value of false BEFORE the isAuthenticatedTrigger$ can be re-run from the authState.refresh() call.

auth0-angular version

2.2.3

Angular version

19.1.3

Which browsers have you tested in?

Safari, Edge

mrentmeister-tt avatar Apr 21 '25 13:04 mrentmeister-tt

What I have done to mitigate this behavior in the meantime, as a less-than-ideal solution I admit, is created my own auth guard that waits 100ms (to allow the appState time to be refreshed), and then redirects if it's not authenticated.

export const traxAuthGuardFn: CanActivateFn = (_, state: RouterStateSnapshot) => {
  const authService = inject(AuthService);
  const logger = inject(LoggerService);

  logger.info('traxAuthGuardFn - Checking authentication state');

  return timer(100).pipe(
    switchMap(() => authService.isAuthenticated$),
    tap(isAuthenticated => {
      logger.info(`traxAuthGuardFn - Authentication state isAuthenticated: ${isAuthenticated}`);
      if (!isAuthenticated) {
        authService.loginWithRedirect({
          appState: { target: state.url }
        });
      }
    })
  );
};

mrentmeister-tt avatar Apr 21 '25 13:04 mrentmeister-tt

I am encountering the same issue as @mrentmeister-tt.

Mekell avatar Apr 21 '25 14:04 Mekell

Also note that this affects logging out as well. The app state is refreshed asynchronously, so my guard is getting a value saying that I'm authenticated, even when I'm actually not because I just logged out.

mrentmeister-tt avatar Apr 29 '25 18:04 mrentmeister-tt

A bit late here, apologies.

It's important to have a public route that is used as redirect_uri, you can not have auth0 redirect you back to a protected route, as the user isn't logged in yet. Sure, there are ways to work around that, as is being done in #671, but in the end the actual root-cause is what I mentioned.

It relates a bit to this entry in our FAQ.

Can you confirm this solves it?

frederikprijck avatar Jun 23 '25 12:06 frederikprijck

@frederikprijck - Thank you for your reply. It's important to note that this is a hybrid mobile app. It's opening up a native browser, redirecting back to my app when login is complete, and then I am triggering this.auth.handleRedirectCallback(url) manually (like shown in the demo provided by Auth0). There is no routing that NEEDS to take place. Auth0 is the one that is redirecting to my protected route when it doesn't need to. The problem is that it's redirecting to a protected route and not waiting for a new isAuthenticated value.

mrentmeister-tt avatar Jun 23 '25 13:06 mrentmeister-tt

So if I understand, we can also avoid the issue in your case by not redirecting back to /, correct?

frederikprijck avatar Jun 23 '25 13:06 frederikprijck

That could probably work too, yes. However, I might still have to still await something before I do an internal navigation to a protected route though, to give the routeGuard time to process the new authentication, but I'm not positive. The ideal solution would be to not have handleRedirectCallback complete until isAuthenticated$ has an updated value -- which is what my pull request tries to achieve.

mrentmeister-tt avatar Jun 23 '25 13:06 mrentmeister-tt

I am not sure I understand the PR.

We have an isLoading observable, which kind of does the same thing as the refresh state you are adding, see https://github.com/auth0/auth0-angular/blob/main/projects/auth0-angular/src/lib/auth.state.ts#L61.

The goal here is that if isLoading is true, isAuthenticated isnt going to emit and waits for isLoading to be false.

We update the isLoading here, which gets called here.

Perhaps something isnt working as intended, but I do not think we need to have an additional thing to signal the same situation.

frederikprijck avatar Jun 23 '25 14:06 frederikprijck

While it's true that your have in isLoading observable, it does not work as intended by the original author, because isAuthenticated$ uses shareReplay(1). The AuthGuard is getting the replayed value of false BEFORE the isAuthenticatedTrigger$ can be re-run from the authState.refresh() call. What my PR does is wait for the authState refresh to fully complete before navigating, and before returning from handleRedirectCallback or logout

mrentmeister-tt avatar Jun 23 '25 14:06 mrentmeister-tt

The ideal solution would be to not have handleRedirectCallback complete until isAuthenticated$ has an updated value -- which is what my pull request tries to achieve.

Thanks, I see what you are trying to do and I think I agree that handleRedirect should not complete before isAuthenticated has an updated value.

I am not convinced on the solution in the PR, as I feel like its worth considering if this can be solved without the additional state tracking, as it feels like we have all the information available to achieve the same.

I think the main issue is this code:

  handleRedirectCallback(
    url?: string
  ): Observable<RedirectLoginResult<TAppState>> {
    return defer(() =>
      this.auth0Client.handleRedirectCallback<TAppState>(url)
    ).pipe(
      withLatestFrom(this.authState.isLoading$),
      tap(([result, isLoading]) => {
        if (!isLoading) {
          this.authState.refresh();
        }
        const appState = result?.appState;
        const target = appState?.target ?? '/';

        if (appState) {
          this.appStateSubject$.next(appState);
        }

        this.navigator.navigateByUrl(target);
      }),
      map(([result]) => result)
    );
  }

More specifically, it should not emit anything before isLoading is done to begin with. I do not think that on its own is going to solve it, but I agree this should not emit until isLoading is done and until the state is refreshed.

I can take a look at this when I have some time, not saying we can not end up going with your PR, but I want to consider if there's something else we can do.

frederikprijck avatar Jun 23 '25 15:06 frederikprijck

Correct me if I'm wrong, but it looks to me like isLoading$ is only used when the service is loading for the first time, not when calling refreshState. I don't really see anything in the code that calls setIsLoading when refreshState is called. The only thing that happens when refreshState is called is emitting a value on the refreshSubject.

What my PR does it change refreshSubject to a Subject<RefreshState>, so that it accurately keeps track of if it's currently being refreshed or not. (I did an enum instead of a boolean, just in case more enum values would want to be added in the future, like Error.

mrentmeister-tt avatar Jun 23 '25 15:06 mrentmeister-tt

Correct me if I'm wrong, but it looks to me like isLoading$ is only used when the service is loading for the first time, not when calling refreshState. I don't really see anything in the code that calls setIsLoading when refreshState is called. The only thing that happens when refreshState is called is emitting a value on the refreshSubject.

It is, here, which is called after calling refresh() (but I think this is part of the cause of the behavior).

so that it accurately keeps track of if it's currently being refreshed or not.

This is what isLoading is for. I think we have an issue with the combination of isLoading and isAuthenticated (isLoading false means you can reliably read the isAuthenticated value, but that isnt the case apparantly) that we need to resolve, rather than introduce something new (if possible).

frederikprijck avatar Jun 23 '25 15:06 frederikprijck

Yeah, I saw that, but nothing is ever setting it back to true when it starts refreshing.

mrentmeister-tt avatar Jun 23 '25 15:06 mrentmeister-tt

If you fix the isLoading, it would also be nice if you don't trigger the navigation until after isLoading is set back to false, and don't return from logout until isLoading is back to false as well.

mrentmeister-tt avatar Jun 23 '25 15:06 mrentmeister-tt

Yeah, I saw that, but nothing is ever setting it back to true when it starts refreshing.

Yes, I think that is also an additional issue, where the SDK is written for the web (where we know that loginWithRedirect will leave the app, come back, and apply the default of true for isLoading).

However, this may not work as expected in Ionic. We may need to consider ensuring we set isLoading to true when calling loginWithRedirect.

Curious to understand how that would impact the behavior you are seeing?

If you fix the isLoading, it would also be nice if you don't trigger the navigation until after isLoading is set back to false

Yes, I agree 👍

frederikprijck avatar Jun 23 '25 15:06 frederikprijck

Curious to understand how that would impact the behavior you are seeing?

If isLoading is fixed to be toggled back to true when refreshState is called, and toggled back to false when complete, that would likely solve my issue as well. If you wanted to create a branch with the changes, I can test it out in my live app to see if it would work.

mrentmeister-tt avatar Jun 23 '25 15:06 mrentmeister-tt

Here is a branch showing what I mean: https://github.com/auth0/auth0-angular/tree/fix/is-loading

I doubt it's this simple, but I am actualy curious to understand if this would change anything in the behavior you are seeing.

frederikprijck avatar Jun 23 '25 15:06 frederikprijck

Here is a branch showing what I mean: https://github.com/auth0/auth0-angular/tree/fix/is-loading

I doubt it's this simple, but I am actualy curious to understand if this would change anything in the behavior you are seeing.

I can check, but my suspicion without debugging is no, because we are not waiting to redirect, and the authGuard doesn't check isLoading$

mrentmeister-tt avatar Jun 23 '25 15:06 mrentmeister-tt

Yeah fair, and the shareReplay used in isAuthenticated will still make it return the previous value, instead of no value which you would get when you run this in a web application and actualy navigate away and come back.

I will need to give this a deeper look.

frederikprijck avatar Jun 23 '25 16:06 frederikprijck

I will need to give this a deeper look.

If this is not high priority to Auth0, I understand. I am more than willing to take another stab at a PR where I fix isLoading$, instead of changing refreshSubject. Let me know if you'd like me to proceed at a first-pass.

mrentmeister-tt avatar Jun 23 '25 16:06 mrentmeister-tt