2954 - Remove SESSION_TIMEOUT

[jtimpe]

Summary of Changes

Pull request closes #2954

• removes the old/irrelevant SESSION_TIMEOUT which hasn't controlled the session timeout since 2822 fix webinspect medium priority - persistent session #2911
• extend SESSION_COOKIE_AGE to match the expiration of the jwt provided by login.gov - the expiration in the dev environment was only 15 minutes
• Update the documentation to reflect the change in session management

How to Test

cd tdrs-backend && docker-compose up
cd tdrs-frontend && docker-compose up --build

1. Open http://localhost:3000/ and sign in.

That's basically it, nothing changes about our signin workflow. The values set by SESSION_TIMEOUT were being overriden by SESSION_EXPIRE_AT_BROWSER_CLOSE's behavior of removing the Expires= from the cookie. You can run the tests from #2911 that verify the session cookie behavior.

Deliverables

More details on how deliverables herein are assessed included here.

Deliverable 1: Accepted Features

Checklist of ACs:

• Identify session timeout/expiration for the jwt provided by login.gov
• Parameterize environment variable value for SESSION_COOKIE_AGE (match or lower than login.gov jwt expiration)
• Remove unused SESSION_TIMEOUT variable in common.py and usages throughout the authentication api
• lfrohlich and/or adpennington confirmed that ACs are met.

Deliverable 2: Tested Code

• Are all areas of code introduced in this PR meaningfully tested?
  • If this PR introduces backend code changes, are they meaningfully tested?
  • If this PR introduces frontend code changes, are they meaningfully tested?
• Are code coverage minimums met?
  • Frontend coverage: [insert coverage %] (see CodeCov Report comment in PR)
  • Backend coverage: [insert coverage %] (see CodeCov Report comment in PR)

Deliverable 3: Properly Styled Code

• Are backend code style checks passing on CircleCI?
• Are frontend code style checks passing on CircleCI?
• Are code maintainability principles being followed?

Deliverable 4: Accessible

• Does this PR complete the epic?
• Are links included to any other gov-approved PRs associated with epic?
• Does PR include documentation for Raft's a11y review?
• Did automated and manual testing with iamjolly and ttran-hub using Accessibility Insights reveal any errors introduced in this PR?

Deliverable 5: Deployed

• Was the code successfully deployed via automated CircleCI process to development on Cloud.gov?

Deliverable 6: Documented

• Does this PR provide background for why coding decisions were made?
• If this PR introduces backend code, is that code easy to understand and sufficiently documented, both inline and overall?
• If this PR introduces frontend code, is that code easy to understand and sufficiently documented, both inline and overall?
• If this PR introduces dependencies, are their licenses documented?
• Can reviewer explain and take ownership of these elements presented in this code review?

Deliverable 7: Secure

• Does the OWASP Scan pass on CircleCI?
• Do manual code review and manual testing detect any new security issues?
• If new issues detected, is investigation and/or remediation plan documented?

Deliverable 8: User Research

Research product(s) clearly articulate(s):

• the purpose of the research
• methods used to conduct the research
• who participated in the research
• what was tested and how
• impact of research on TDP
• (if applicable) final design mockups produced for TDP development

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.71%. Comparing base (f416519) to head (ceedc61).
Report is 2 commits behind head on develop.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #3020      +/-   ##
===========================================
- Coverage    92.71%   92.71%   -0.01%     
===========================================
  Files          277      277              
  Lines         7486     7478       -8     
  Branches       672      672              
===========================================
- Hits          6941     6933       -8     
  Misses         443      443              
  Partials       102      102              

Flag	Coverage Δ
dev-backend	92.72% <100.00%> (-0.01%)	⬇️
dev-frontend	92.60% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files	Coverage Δ
tdrs-backend/tdpservice/settings/common.py	99.31% <100.00%> (-0.01%)	⬇️
tdrs-backend/tdpservice/users/api/middleware.py	100.00% <100.00%> (ø)
tdrs-backend/tdpservice/users/api/utils.py	100.00% <100.00%> (ø)

---

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2e954b4...ceedc61. Read the comment docs.

[elipe17]

Small nit: should be # 15 minutes

[elipe17]

I would be curious to know if Alex knows that the timeout is going to be even shorter now? I know the 30 minute timeout was not something she liked. Otherwise, this all looks good!

[jtimpe]

find out what prod exp is

[raftmsohani]

Nit: If @ADPennington want to have extended session cookie, then you should be able to set the session at login to a different value for admin users.

[raftmsohani]

LGTM!

[ADPennington]

@jtimpe lgtm 🚀 thanks for cleaning up the session-related code. Please note:

• tested that session ends after 15 minutes for acf and login.gov users
• the session is ultimately too short, especially for ACF users actively using the system during the day, so @robgendron we recommend moving up As a TDP user, I need to stay logged in when I'm actively using the system #3060 in the sprint backlog. cc: @lfrohlich @ttran-hub