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

Add secureProxy option to the cookie session #294

Merged
merged 52 commits into from
Feb 16, 2021
Merged

Conversation

kryswisnaskas
Copy link
Collaborator

@kryswisnaskas kryswisnaskas commented Feb 12, 2021

Description of change
Small change to add explicit option to the cookie used for session management.

plus the changes for issue 5

How to test

Issue(s)

Checklist

  • Meets issue criteria
  • Code tested
  • Meets accessibility standards (WCAG 2.1 Levels A, AA)
  • Documentation updated

kryswisnaskas and others added 5 commits February 12, 2021 17:39
To eliminate `Call retries were exceeded` error for frontend tests
Add --maxWorkers=50% as suggested at
jestjs/jest#8769
…tests

Try limiting workers for frontend tests
Extend yarn commands and add resources for backend
Copy link
Contributor

@rahearn rahearn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please make this slight tweak to ensure that non-ssl local development still works with HSES login.

src/app.js Outdated
@@ -29,6 +29,7 @@ app.use(cookieSession({

// Cookie Options. httpOnly is set by default to true for https
sameSite: 'lax',
secureProxy: true,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great in dev, but seems to be breaking cookies in my local environment, preventing any logins when BYPASS_AUTH is false.

Suggested change
secureProxy: true,
secureProxy: (process.env.NODE_ENV === 'production'),

@@ -0,0 +1,25 @@
# TTADP Backend
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome add!

@kryswisnaskas
Copy link
Collaborator Author

Thanks for finding this. Created adhocteam#174 since I wasn't able to commit the suggested change here.

Make secureProxy setting conditional
@rahearn rahearn merged commit e823b25 into HHS:main Feb 16, 2021
rahearn pushed a commit that referenced this pull request May 26, 2021
[TTAHUB-121]: Create ADR for using New Relic for web analytics.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Session cookies secure flag not properly set Display list of Activity Reports
4 participants