Set cookie policy to SameSite=lax and httpOnly (partial mitigation for #2496) #2498
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
|
GolangCI step fails with error message: $ GOLANGCI_COM_RUN=1 golangci-lint run --out-format=json --issues-exit-code=0 --timeout=5m --new=false --new-from-rev= --new-from-patch=../changes.patch
Can't get config for command line: can't parse args: unknown flag: --timeout |
|
It seems GolangCI changed the option Wait for them to fix (hopefully), or update our version request of |
|
Well, whatever we do, I have opened an issue with them: golangci/golangci#48 |
|
They fixed support for old versions, so this build turned green automatically. |
alexmt
approved these changes
Oct 17, 2019
|
Thank you @jannfis . Cherry-picking it into 1.3 |
alexmt
pushed a commit
that referenced
this pull request
Oct 17, 2019
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Checklist:
I've created an enhancement proposal and I feel I've gotten a green light from the community.This fixes a bugThis PR sets the attributes
SameSite=laxandhttpOnlyfor the authentication cookie used by the web UI. It partially mitigates the CSRF vulnerability reported in #2496. It will work with most more or less up-to-date browsers in the wild, but not with all (IE 10 and IE11 on some platforms, most notably).I chose
laxasSameSitepolicy so that links to ArgoCD (i.e. from a company internal documentation or bookmark page) will still work when the user accidentally closed the tab without logging out, and opening ArgoCD viahreflink. I tested basic reproduction steps as mentioned in #2496 and found the solution protects against this scenario (although giving 401 response, not 403, which I consider fine and appropriate).Cross-site requests using
GET,HEAD,TRACEandOPTIONSmethods will also still work, but since we obey REST principles everywhere (no data manipulation using those methods), this should be "fine".Also as mentioned in discussion in #2496, this is no complete CSRF protection but only a "quick win" that we should also keep when thinking about things like CSRF tokens for all data manipulating HTTP methods.