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

Set Secure setting on AuthSession cookie for HTTPS pages #3182

Closed
alxndrsn opened this issue Mar 1, 2017 · 8 comments
Closed

Set Secure setting on AuthSession cookie for HTTPS pages #3182

alxndrsn opened this issue Mar 1, 2017 · 8 comments

Comments

@alxndrsn
Copy link
Contributor

alxndrsn commented Mar 1, 2017

This should be done in api's login controller.

@nice-snek nice-snek added the Status: Blocked waiting for triage Blocked waiting for prioritisation label Jul 4, 2017
@nice-snek
Copy link

hi frand @mandric, please triage before the end of this sprint.

@mandric
Copy link
Contributor

mandric commented Jul 4, 2017

@alxndrsn please triage (close or schedule)

@mandric mandric removed their assignment Jul 4, 2017
@garethbowen garethbowen added 1 - Scheduled and removed Status: Blocked waiting for triage Blocked waiting for prioritisation labels Jul 10, 2017
@garethbowen garethbowen added this to the June 26 - July 10 milestone Jul 10, 2017
@garethbowen
Copy link
Member

Scheduling - this should be a quick win.

@garethbowen garethbowen self-assigned this Jul 11, 2017
garethbowen added a commit to medic/medic-api that referenced this issue Jul 11, 2017
Uses secure, httpOnly, and sameSite flags where possible to
protect users from cookie hacking.

medic/cht-core#3182
@garethbowen
Copy link
Member

Code review please @SCdF

Once this is merged leave it open and assign it back to me so I can test on an actual server using https.

@SCdF
Copy link
Contributor

SCdF commented Jul 11, 2017

Back to you @garethbowen. Mostly fine, just a comment about implementation and security therein.

@garethbowen
Copy link
Member

Yeah good call. I've changed it to use the NODE_ENV environment variable which I think we're setting to production in medic-os. It's another thing I'll test when this gets merged.

Please re-review @SCdF

@garethbowen garethbowen assigned SCdF and unassigned garethbowen Jul 12, 2017
@SCdF
Copy link
Contributor

SCdF commented Jul 12, 2017

LGTM @garethbowen, back to you to test etc

@SCdF SCdF removed their assignment Jul 12, 2017
garethbowen added a commit to medic/medic-api that referenced this issue Jul 12, 2017
Uses secure, httpOnly, and sameSite flags where possible to
protect users from cookie hacking.

medic/cht-core#3182
garethbowen added a commit that referenced this issue Jul 12, 2017
@garethbowen
Copy link
Member

Tested on alpha.dev works as expected. Leaving in AT so another pair of eyes can check it.

For AT:

  1. Log out and log in again
  2. Check your cookies in the dev console. In Chrome it's in the Application tab, in Firefox it's in the Storage tab.
  3. Find the AuthSession cookie, and ensure it's "httpOnly" and "secure", eg:

cookie

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

No branches or pull requests

6 participants