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

Bug: Can use "Edit FAQ Page" even with an expired session #1842

Closed
NilakshiS opened this issue Sep 11, 2024 · 4 comments · Fixed by #1904 or #1925
Closed

Bug: Can use "Edit FAQ Page" even with an expired session #1842

NilakshiS opened this issue Sep 11, 2024 · 4 comments · Fixed by #1904 or #1925
Assignees
Labels
bug Release Note: Shows as Error Correction level: medium p-feature: FAQ screen /faq priority: MUST HAVE role: front-end Front End Developer size: 2pt Can be done in 7-12 hours
Milestone

Comments

@NilakshiS
Copy link
Member

Describe the bug

I can access the "FAQ" page and partially use the "Edit FAQ Page" function even with an expired session which should not be possible. The changes made to the FAQ page are incorrectly displayed as saved but navigating to FAQ page again does not show the changes

Because the bug incorrectly shows the FAQ changes as saved, the user might not realize that the edits to the page did not take effect and any new data entered is lost.

Currently affects the tdmdev login - Admin ([email protected]) account, unsure if more users are affected.

Steps to reproduce the issue

  1. Login to the dev site.

  2. Go to "FAQ" page.

  3. Wait for the session to expire or "Expire" the session using dev tools.

    Instructions for using Chrome Dev tools to expire the session
    • Open Developer tools in the browser.
    • Go to the "Application" tab in dev tools.
    • From the menu on left, under "Storage" select "Cookies".
    • Click on the item "https://tdm-dev.azurewebsites.net/" if not already selected.
    • In the list of cookies that opens on the right, find the item with the name "jwt".
    • Double click on the value in "Expires/ Max Age" column for the "jwt" item.
    • Change the date to a day before, for example, if it's 2024-09-12T07:44:24.722Z, change it to 2024-09-11T07:44:24.722Z.
    • Click outside the panel to change the value, the "jwt" item will disappear from the list.
    • Close the dev tools.
  4. Navigate to "About" page then go back to the "FAQ" page.

What's the expected result?

After the last step, clicking on the "FAQ" page nav button should cause the user to be logged out and be redirected to the login page with an expired session notification displayed.

What's the actual result?

The user can still edit the FAQ page and try to save their changes. The changes are also displayed on the FAQ page but disappear as soon as the page is reload.

Additional details / screenshot

An image of the FAQ page, notice that the user is still shown as logged in and the "Edit FAQ Page" button is displayed

image

An image of adding a new FAQ

image

The FAQ page after the edits were saved by clicking the "Save Edits" button

image

The FAQ page after the page was reload

image

Device configuration

  • Device: desktop computer
  • OS version: Windows 11 OS
  • Browser: Chrome
  • Browser version: 128.0.6613.121
@roslynwythe
Copy link
Member

roslynwythe commented Sep 26, 2024

@entrotech From your comments in the meeting I understand that a polling strategy is the likely solution, and that's fine, but just to consider alternatives, can I ask:

  • Could we make "FAQ" a route and then use validateUser from jwt-session.js from server/middleware/jwt-session.js ?
  • As an alternative to a polling strategy, would you consider a SSE (server-sent event) approach?

@roslynwythe roslynwythe self-assigned this Sep 26, 2024
@entrotech
Copy link
Member

Caveat: Our whole authentication and authorization system is home-grown, and we could do a complete overhaul to incorporate modern best practices, etc. The city, in fact, wants to replace it with an enterprise-wide 3rd party authentication product (they partially implemented a solution from Okta, but are now looking at switching to a different product from Okta). But it remains to be seen when they might be ready to implement it. So my current position is that we can't count on the City to implement anything soon and should just keep going with our home-grown security, expecting it to be used in production.

The way it works now is that when the user logs in, it uses the /api/accounts/login Web API endpoint. If the login is successful, the results of the call are:

  • A response cookie is returned with a JSON Web token that serves to authenticate subsequent requests for up to 12 hours. After 12 hours, any web api requests that require authentication will return an HTTP 401 response.
  • The login response also includes a userAccount object containing the user properties id, firstName, lastName, email, isAdmin, emailConfirmed and isSecurityAdmin. In the application, this object is stored as a state variable in the TdmAuthProvider component for use by the userContext to make the userAccount object available to various parts of the client application. As a convenience to users, the same object is also persisted to local storage, and will be used to restore the userAccount object to the TdmAuthProvider if the user closes and then re-opens the browser session, so they do not have to re-login if they re-open the application.

Various react components use the userContext to access the userAccount object to grant or revoke permission to a client-side route. If the account object is not null, then it considers the user to be "logged in", and will not be aware if the JWT token has expired. As a consequence the app will allow access to pages and components as if the user were logged in. The user can manipulate the app as if the user were logged in, but when they attempt to commit any changes to the back end, the Web API will reject any requests requiring user validation as mentioned above. This problem causes a number of bugs like this one, and #1841.

The main problem is that the userAccount object on the client should reset to null at the time the JWT token expires, so the client will behave the way it should when there is no logged in user. One thought that comes to mind is that we could augment the userAccount object returned by the login api call with an expiration datetime, and modify the TdmAuthProvider to set the userAccount object to null at that time. However, I'm willing to entertain other ideas you might have.

We could also get rid of the feature that stores the userAccount object to local storage, but that might be a good feature to have when users start opening links to snapshots shared with them (that's a different story), so I'm hesitant to remove that at this time.

@entrotech entrotech removed the Dependency Issues that cannot be worked on until another issue is closed label Oct 2, 2024
@roslynwythe
Copy link
Member

roslynwythe commented Oct 4, 2024

@entrotech Thank you for the summary of the current functionality. The solution that you propose:

One thought that comes to mind is that we could augment the userAccount object returned by the login api call with an
expiration datetime, and modify the TdmAuthProvider to set the userAccount object to null at that time.

sounds good. The only drawback would be that since we are not confirming user status with the server, it would be possible for a user to be logged in to the application in two browser tabs, and if they Logout in one browser tab, they will remain logged in in the other browser tab.

@roslynwythe

This comment was marked as outdated.

@github-project-automation github-project-automation bot moved this from In progress (actively working) to On Dev - not yet pushed to Prod in P: TDM: project board Oct 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Release Note: Shows as Error Correction level: medium p-feature: FAQ screen /faq priority: MUST HAVE role: front-end Front End Developer size: 2pt Can be done in 7-12 hours
Projects
Status: Released
4 participants