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

Namerequest UI: "isAuthenticated" is not refreshed on login #18115

Closed
severinbeauvais opened this issue Oct 6, 2023 · 9 comments
Closed

Namerequest UI: "isAuthenticated" is not refreshed on login #18115

severinbeauvais opened this issue Oct 6, 2023 · 9 comments
Assignees
Labels
bug Something isn't working Names Team Name Request Name Examination Team Priority2

Comments

@severinbeauvais
Copy link
Collaborator

severinbeauvais commented Oct 6, 2023

This issue was found in #18023, in which the logged-in breadcrumb was not being displayed until the app is reloaded.

  1. don't be logged in
  2. go to Namerequest UI
  3. observe the non-logged in breadcrumb:

Private Zenhub Image

  1. log in
  2. observe that the incorrect breadcrumb is displayed (same as above)
  3. reload the page (F5)
  4. observe that the correct logged-in breadcrumb is displayed:

Private Zenhub Image

Analysis

isAuthenticated is a store getter that checks the existence of a Keycloak token in session storage. However, the KC token is not populated until a few seconds after the user logs in (it's async in the SbcHeader component), and isAuthenticated is not responsive to it because it's in session storage. Bottom line: isAuthenticated does not get updated after a successful login.

The impact is that any code that checks if the user is logged in will not work correctly. This bug has existed since NR UI was first implemented, but now it's visible in the breadcrumb (which itself is low severity).

This only happens if the user goes to NR UI while logged out and then logs in. If the user is already logged in and goes to NR UI then this bug doesn't manifest.

@severinbeauvais severinbeauvais added ENTITY Business Team bug Something isn't working labels Oct 6, 2023
@Mihai-QuickSilverDev Mihai-QuickSilverDev added Priority2 Names Team Name Request Name Examination Team labels Oct 6, 2023
@Mihai-QuickSilverDev
Copy link
Collaborator

@ozamani9gh Something for your consideration.

@severinbeauvais
Copy link
Collaborator Author

severinbeauvais commented Oct 6, 2023

I have a couple of proper technical solutions to propose, which I will document here when I have a moment. done, see below

@severinbeauvais
Copy link
Collaborator Author

severinbeauvais commented Oct 6, 2023

Proposed solution # 1 (simplest)

Use a store variable to maintain the "is authenticated" state. Update it when the app starts (in App.vue::created()) and when login has completed (in Signin.vue::onReady()).

Sample code change: bcgov/namerequest#726.

Proposed solution # 2

Wait for current account to be available, as is done here: https://github.com/bcgov/business-edit-ui/blob/05d30b4c5a59ad29050bf27b52573f6a3c74cbac/src/App.vue#L557

Proposed solution # 3 (best)

Link to the auth store and read its state directly, as is done here: https://github.com/bcgov/business-filings-ui/blob/23ee32c04343d48ca84161843e8bb10a5eab9f90/src/stores/authenticationStore.ts#L25

@ozamani9gh
Copy link
Collaborator

ozamani9gh commented Oct 6, 2023

@Mihai-QuickSilverDev @severinbeauvais are you hoping Names Team picks up this work. If so:

Does this need to be groomed, worked on as part of WoN Release, or can we start this after the implementation. our team is at velocity this sprint, so I would need to move priorities around

@severinbeauvais
Copy link
Collaborator Author

This bug has been around for years and I think very few users will bump into it, so it's not P1 and can be fixed later (IMHO).

@Mihai-QuickSilverDev Mihai-QuickSilverDev removed the ENTITY Business Team label Oct 12, 2023
@Mihai-QuickSilverDev
Copy link
Collaborator

@ozamani9gh Hi Omid, yes, this is a Names ticket, and would leave it to you to schedule. It can be done after the WoN go-live on October 17.

@eve-git
Copy link
Collaborator

eve-git commented Oct 27, 2023

PR: bcgov/namerequest#742

@eve-git
Copy link
Collaborator

eve-git commented Nov 3, 2023

Ready to test in DEV and TEST environments.

@oanyahuru
Copy link
Collaborator

I tested it and the correct breadcrumb displayed when I logged in..
Private Zenhub Image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Names Team Name Request Name Examination Team Priority2
Projects
None yet
Development

No branches or pull requests

6 participants