auth.service.ts doesn't refresh authMethods after AUTHENTICATED_ERROR

[philipprumpf]

Describe the bug We've observed for a while that after the backend is restarted while a user is logged-in, reloading the logged-in browser tab results in an "empty" login menu, like this:

We've seen this problem with our DSpace-CRIS installation as well as a recent main branch of dspace-angular, and with Firefox 91.11.0esr as well as Chromium 103.0.5060.53.

To Reproduce Steps to reproduce the behavior:

1. Start frontend and backend
2. Open a browser
3. Log into the frontend in the browser window
4. Restart the backend
5. Reload the browser tab
6. Open login menu

Expected behavior The authentication methods are expected to appear.

Analysis I've finally found the time to investigate and it appears that while AUTHENTICATED_ERROR fires correctly, all that's done in response is to remove the token from storage (which is why things will work again if you reload the tab twice); there's no working code path that results in a RetrieveAuthMethodsAction or a forced page reload.

I say "no working code path" because this code in trackTokenExpiration comes quite close to handling this (and may provide the appropriate UI response to a non-expired token that unexpectedly fails to work):

  public trackTokenExpiration(): void {
    let token: AuthTokenInfo;
    let currentlyRefreshingToken = false;
    this.store.pipe(select(getAuthenticationToken)).subscribe((authTokenInfo: AuthTokenInfo) => {
      // If new token is undefined an it wasn't previously => Refresh failed
      if (currentlyRefreshingToken && token !== undefined && authTokenInfo === undefined) {
        // Token refresh failed => Error notification => 10 second wait => Page reloads & user logged out
        this.notificationService.error(this.translateService.get('auth.messages.token-refresh-failed'));
        setTimeout(() => this.navigateToRedirectUrl(this.hardRedirectService.getCurrentRoute()), 10000);
        currentlyRefreshingToken = false;
      }

Unfortunately, the code above is never called in our case, because the authentication token never assumes a value other than undefined. As an experiment, I've modified the AUTHENTICATED_ERROR handler to use null rather than undefined:

    case AuthActionTypes.AUTHENTICATED_ERROR:
    case AuthActionTypes.RETRIEVE_AUTHENTICATED_EPERSON_ERROR:
      return Object.assign({}, state, {
        authenticated: false,
        authToken: null,
        error: (action as AuthenticationErrorAction).payload.message,
        loaded: true,
        blocking: false,
        loading: false
      });

and adjusted the code in trackTokenExpiration accordingly. That appears to work.

Proposed fixes I think the easiest fix would be to trigger a CheckAuthenticationTokenCookieAction (which is very different from a CheckAuthenticationTokenAction) in response to an AUTHENTICATED_ERROR. However, as that action may itself result in an AUTHENTICATED_ERROR, we must do so only if we've actually deleted a token from storage, in order to avoid an infinite loop if the backend is actually throwing an error.

That would, however, result in a transparent log-out as observed by the user, which might not be desired.

[tdonohue]

@philipprumpf : Based on your description above, it's unclear to me if you've found a fix you are satisfied with or not? If you have a fix you'd propose, feel free to create a PR & I can find a tester/reviewer. If you feel this is better assigned to someone else for additional analysis, I can find someone on our DSpace 7 team to try to help out here...just let me know.

[philipprumpf]

I got stuck trying to figure out how to write a test for this, to be honest. For now, I've submitted a PR without the test.

[tdonohue]

Closing, as this is no longer reproducible in pre-7.4. It appears it was fixed via a separate bug fix.