Add CSRF protection to ReverseProxy authentication on API #22221
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Author-Change-Id: IB#1129006
Signed-off-by: Andrew Thornton <[email protected]>
zeripath
added
type/bug
outdated/backport/v1.18
|
Now one of the things against this PR as it stands is that it adds another CORS library - I don't fully understand why this is the case. Realistically for this to be mergable IMO we need to use the same library in both the API and the main code |
GiteaBot
added
the
lgtm/need 2
Signed-off-by: Andrew Thornton <[email protected]>
According to https://cheatsheetseries.owasp.org/cheatsheets/Cross-Site_Request_Forgery_Prevention_Cheat_Sheet.html#javascript-guidance-for-auto-inclusion-of-csrf-tokens-as-an-ajax-request-header GET, HEAD and OPTIONS should not be used for changing state.
…r-and-make-updatable
|
I wonder if all this CSFR stuff is actually needed when we set |
Signed-off-by: Andrew Thornton <[email protected]>
zeripath
commented
Dec 22, 2022
Signed-off-by: Andrew Thornton <[email protected]>
lunny
pushed a commit
that referenced
this pull request
Dec 27, 2022
Since we changed the /api/v1/ routes to disallow session authentication we also removed their reliance on CSRF. However, we left the ReverseProxy authentication here - but this means that POSTs to the API are no longer protected by CSRF. Now, ReverseProxy authentication is a kind of session authentication, and is therefore inconsistent with the removal of session from the API. This PR proposes that we simply remove the ReverseProxy authentication from the API and therefore users of the API must explicitly use tokens or basic authentication. Replace #22077 Close #22221 Close #22077 Signed-off-by: Andrew Thornton <[email protected]>
lunny
pushed a commit
to lunny/gitea
that referenced
this pull request
Dec 27, 2022
Since we changed the /api/v1/ routes to disallow session authentication we also removed their reliance on CSRF. However, we left the ReverseProxy authentication here - but this means that POSTs to the API are no longer protected by CSRF. Now, ReverseProxy authentication is a kind of session authentication, and is therefore inconsistent with the removal of session from the API. This PR proposes that we simply remove the ReverseProxy authentication from the API and therefore users of the API must explicitly use tokens or basic authentication. Replace go-gitea#22077 Close go-gitea#22221 Close go-gitea#22077 Signed-off-by: Andrew Thornton <[email protected]>
lunny
pushed a commit
to lunny/gitea
that referenced
this pull request
Dec 27, 2022
Since we changed the /api/v1/ routes to disallow session authentication we also removed their reliance on CSRF. However, we left the ReverseProxy authentication here - but this means that POSTs to the API are no longer protected by CSRF. Now, ReverseProxy authentication is a kind of session authentication, and is therefore inconsistent with the removal of session from the API. This PR proposes that we simply remove the ReverseProxy authentication from the API and therefore users of the API must explicitly use tokens or basic authentication. Replace go-gitea#22077 Close go-gitea#22221 Close go-gitea#22077 Signed-off-by: Andrew Thornton <[email protected]>
KN4CK3R
pushed a commit
that referenced
this pull request
Dec 27, 2022
backport from #22219 Since we changed the /api/v1/ routes to disallow session authentication we also removed their reliance on CSRF. However, we left the ReverseProxy authentication here - but this means that POSTs to the API are no longer protected by CSRF. Now, ReverseProxy authentication is a kind of session authentication, and is therefore inconsistent with the removal of session from the API. This PR proposes that we simply remove the ReverseProxy authentication from the API and therefore users of the API must explicitly use tokens or basic authentication. Replace #22077 Close #22221 Close #22077 Signed-off-by: Andrew Thornton <[email protected]> Signed-off-by: Andrew Thornton <[email protected]> Co-authored-by: zeripath <[email protected]>
zeripath
deleted the
replace-22077-fix-copyright-header-and-make-updatable
branch
lafriks
pushed a commit
that referenced
this pull request
Dec 30, 2022
backport #22219 Since we changed the /api/v1/ routes to disallow session authentication we also removed their reliance on CSRF. However, we left the ReverseProxy authentication here - but this means that POSTs to the API are no longer protected by CSRF. Now, ReverseProxy authentication is a kind of session authentication, and is therefore inconsistent with the removal of session from the API. This PR proposes that we simply remove the ReverseProxy authentication from the API and therefore users of the API must explicitly use tokens or basic authentication. Replace #22077 Close #22221 Close #22077 Signed-off-by: Andrew Thornton <[email protected]> Signed-off-by: Andrew Thornton <[email protected]> Co-authored-by: zeripath <[email protected]>
zeripath
removed
the
outdated/backport/v1.18
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add CSRF protection to the API when using ReverseProxy authentication.
This is a recreation of #22077 with the copyright headers left unchanged, and adding HEAD to the allowed methods.
Replace #22077
Close #22219
Close #22077