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.

Sign up for GitHub

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

Jump to bottom

Mark _csrf cookie as secure if COOKIE_SECURE is enabled #3833

Closed
wants to merge 1 commit into from
Closed

Mark _csrf cookie as secure if COOKIE_SECURE is enabled #3833

wants to merge 1 commit into from

Conversation

thehowl
Copy link
Contributor

@thehowl thehowl commented Apr 22, 2018

Fixes #1734.

@codecov-io
Copy link

Codecov Report

Merging #3833 into master will decrease coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #3833      +/-   ##
=========================================
- Coverage   23.02%     23%   -0.02%     
=========================================
  Files         126     126              
  Lines       24894   24894              
=========================================
- Hits         5731    5728       -3     
- Misses      18287   18289       +2     
- Partials      876     877       +1
Impacted Files Coverage Δ
modules/process/manager.go 69.56% <0%> (-4.35%) ⬇️

Continue to review full report at Codecov.

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

@bkcsoft bkcsoft added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Apr 22, 2018
@lunny lunny added the topic/security Something leaks user information or is otherwise vulnerable. Should be fixed! label Apr 23, 2018
@lunny lunny added this to the 1.5.0 milestone Apr 23, 2018
@lunny
Copy link
Member

lunny commented Apr 23, 2018

LGTM

@bkcsoft bkcsoft added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Apr 23, 2018
@thehowl thehowl mentioned this pull request Apr 23, 2018
Merged
@thehowl
Copy link
Contributor Author

thehowl commented Apr 23, 2018

@AleksandrBulyshchenko spotted a problem in the upstream csrf repo - I think we can move this bugfix to #3839.

@thehowl thehowl closed this Apr 23, 2018
@lunny lunny removed this from the 1.5.0 milestone Apr 24, 2018
@go-gitea go-gitea locked and limited conversation to collaborators Nov 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
lgtm/need 1 This PR needs approval from one additional maintainer to be merged. topic/security Something leaks user information or is otherwise vulnerable. Should be fixed!
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

COOKIE_SECURE doesn't flag _csrf cookie as 'Secure'
4 participants
@thehowl @codecov-io @lunny @bkcsoft