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

OAuth POST /auth/authorize #147

Merged
merged 6 commits into from
Dec 14, 2016
Merged

OAuth POST /auth/authorize #147

merged 6 commits into from
Dec 14, 2016

Conversation

nono
Copy link
Member

@nono nono commented Dec 13, 2016

No description provided.

@nono nono requested review from jinroh and aenario December 13, 2016 10:39
@codecov-io
Copy link

codecov-io commented Dec 13, 2016

Current coverage is 54.96% (diff: 89.53%)

Merging #147 into master will increase coverage by 0.66%

@@             master       #147   diff @@
==========================================
  Files            31         32     +1   
  Lines          2926       2978    +52   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits           1589       1637    +48   
- Misses         1093       1096     +3   
- Partials        244        245     +1   

Powered by Codecov. Last update 7305124...d31d3fb

CookieMaxAge: 3600, // 1 hour
CookieHTTPOnly: true,
CookieSecure: true,
})
Copy link
Contributor

Choose a reason for hiding this comment

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

could it be interesting to store the CSRF in the session instead of using another cookie ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Well, I think another cookie is fine. I don't see a pro for using the session for CSRF, and there is a con: the echo middleware won't support that out of the box.

@jinroh jinroh merged commit cc9aa96 into cozy:master Dec 14, 2016
@nono nono deleted the oauth-authorize-2 branch December 16, 2016 11:08
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.

None yet

4 participants