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

197 & 278: Session Timeout #421

Merged
merged 167 commits into from
Nov 25, 2020
Merged

197 & 278: Session Timeout #421

merged 167 commits into from
Nov 25, 2020

Conversation

hilvitzs
Copy link

@hilvitzs hilvitzs commented Nov 17, 2020

This pull request changes...

This makes it so that a user will be logged out after being inactive. There are two new environment variables I created for the timeout:

REACT_APP_TIMEOUT_TIME=6000 - 6 seconds of inactivity before the modal pops up and, when the modal pops up, 3 seconds before the user is logged out.
REACT_APP_LOGOUT_TIME=10000 - User has 10 seconds once the modal pops up to make a selection to either stay logged in or log out.
REACT_APP_DEBOUNCE_TIME=1000 - This is a debounce time of 1 second for calls to /v1/auth_check to reset the user token to ensure the user is not logged out.

Also included in this PR is a fix for #425 where the mobile menu not being entirely keyboard navigable. In order to fix this, I needed to make a small design change where the Sign In/Sign Out button is always visible but the text changes based on whether the user is authenticated. This has to do with the way that USWDS wrote the javascript for tabbing through the navigation menus.

FUNCTIONALITY

  • 197 Session Timeout: A modal will pop up explaining to the user that they have been inactive and will be logged out if they don't choose to remain logged in. Right now, for testing, the times are set at 3 seconds for the initial modal and then five seconds to be logged out. These will be updated to 20 minutes and 3 minutes once testing is no longer necessary.

  • There are two buttons on the modal - one to remain logged in and one to sign out immediately.

  • 278 User ability to extend session: The user will be able to interact with the modal to sign out immediately or renew their session.

TO TEST

  • Go to https://tdp-frontend.app.cloud.gov/ and log in.
  • Don't interact with the page for 3 seconds - the modal should pop up.
  • Don't click one of the buttons on the modal for another 3 seconds - the user should be logged out.
  • Click on the Sign Out button - the user should be logged out.
  • Click on the Stay Signed In button - the user should remain logged in. The modal will reopen if you are idle again for 3 seconds.
  • When the modal opens focus should go the h1 element and voiceover should read out the h1 text.

This pull request is ready to merge when...

  • Meets acceptance criteria for the issue
  • Code is reviewed by someone other than the original author
  • Code is tested
  • Experience is approved by UX
  • Meets a11y checklist
  • Meets Raft's Manual QASP Checklist🔒 (only applicable for PR to main HHS)
  • Documentation is updated
    • OpenAPI
    • Readme
    • Entity Relationship Diagram (ERD)

@hilvitzs
Copy link
Author

@alexsoble Sorry that I missed the throttle work. I added that along with the onAction function. Those are deployed on the cloud environment.

@alexsoble
Copy link

@hilvitzs Great, thanks. For what it's worth, the session management strategy doc says that a combination of debounce and throttle will be used: #364.

I could be happy with debounce, throttle, or both — but I think the approach we implement here should match whatever is stated in the strategy doc, and we should have a rationale or explanation for whatever strategy we go with.

@carltonsmith
Copy link

@hilvitzs Great, thanks. For what it's worth, the session management strategy doc says that a combination of debounce and throttle will be used: #364.

I could be happy with debounce, throttle, or both — but I think the approach we implement here should match whatever is stated in the strategy doc, and we should have a rationale or explanation for whatever strategy we go with.

@alexsoble This is probably my fault as we planned on figuring out the strategy as we got into the code. I should have been more clear in the documentation.

@alexsoble
Copy link

@carltonsmith No worries!

@carltonsmith
Copy link

@hilvitzs Great, thanks. For what it's worth, the session management strategy doc says that a combination of debounce and throttle will be used: #364.
I could be happy with debounce, throttle, or both — but I think the approach we implement here should match whatever is stated in the strategy doc, and we should have a rationale or explanation for whatever strategy we go with.

@alexsoble This is probably my fault as we planned on figuring out the strategy as we got into the code. I should have been more clear in the documentation.

The documentation in #364 has been updated accordingly

@alexsoble
Copy link

Just noting here that this PR corresponds to ATO Control AC-02(05): https://nvd.nist.gov/800-53/Rev4/control/AC-2 to help myself situate how this fits into the overall security/compliance picture. 🖼️

@carltonsmith carltonsmith added QASP Review and removed raft review This issue is ready for raft review labels Nov 20, 2020
@hilvitzs hilvitzs force-pushed the 197-session-timeout branch from 81658fb to 2d289d9 Compare November 20, 2020 20:42
@carltonsmith carltonsmith merged commit 12a33cc into raft-tdp-main Nov 25, 2020
@hilvitzs hilvitzs deleted the 197-session-timeout branch December 8, 2020 00:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.