We're updating the issue view to help you get more done. 

Fix frontend-auth getAuthenticationState bug.

Description

As part of ARCH-873, it was discovered that calls to `authClient.getAuthenticationState()` were unreliable.

The fix implemented for was to pass the token used to authenticate in the callback passed to `authClient.ensureAuthenticationAndCookies()`. The `user_id` is pulled from the returned token, and passed to the frontend-analytics `identifyAuthenticatedUser` call, in place of calling `authClient.getAuthenticationState()` directly from inside frontend-analytics.

  • UPDATE: Even the token returned by refresh token is sometimes empty, and frontend-auth should do something smarter than return bad data (an empty token) to what was meant to be a success callback.

    • Note: some logging was added to the service, but what is going wrong is still unclear.

    • We may want to fail and ensure bad data is not returned as a preliminary step, with a follow-up to try to actually solve the issue if it happens frequently enough. Of course, the more apps that use frontend-auth, the more frequent this will become.

Additionally, the callback solution above still has the following problems:
1. We still expose `getAuthenticationState()` publicly, and it is known to be buggy. Applications continue to use this call in other locations. This interface needs though, including whether or not it should cache and return the most recent state.
2. The frontend-analytics call `identifyAuthenticatedUser` no longer encapsulates the user_id call, and thus no longer ensures consistency of implementation around the user's identity, which was a past problem we were trying to fix.
3. Additionally, a separate but related bug might be that `isRoutePublic` doesn't check-for authentication, and thus doesn't return anything to the callback. It seems possible to have a route be public, but still want to know whether it is being viewed by an anonymous or an authenticated user.
4. Also separate but related, ensurePublicOrAuthenticationAndCookies sometimes returns a promise and sometimes returns the result of callback. This should either be clearly documented, or we should simply remove all the returns. (Not sure if the returns were used to help unit testing though?)

A potential solution might look like:
1. Make `getDecodedAccessToken` private to frontend-auth, so it can only be used immediately before checking the token as part of the authentication code, like `ensureAuthenticationAndCookies` and any other locations in the library.
2. Whenever we have a token that passes authentication according to the library, we should cache it so that it could be returned later by `getAuthenticationState`.
3. Restore the old frontend-analytics code that used `getAuthenticationState` to get the user_id.

Another potential solution would be to either drop the function `getAuthenticationState`, and force users to check authentication every time they want the state.

NOTE: Please remove any `TODO: ARCH-948` comments from the code after implementing this.

Environment

None

Status

Epic Link

Story Points

None

Reporter

Robert Raposa

Assignee

Unassigned

Labels

None

Sprint

None

Priority

Unset