-
-
Notifications
You must be signed in to change notification settings - Fork 2k
feat: add a cookie-csrf-per-request-limit attribute #2615
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
Conversation
bh-tt
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Few points I'd like some feedback on.
Signed-off-by: test <bert@transtrend.com>
|
This pull request has been inactive for 60 days. If the pull request is still relevant please comment to re-activate the pull request. If no action is taken within 7 days, the pull request will be marked closed. |
|
This is still relevant |
|
I really do not like these 7 day windows to respond to these bots. I was at home for 2 weeks due to a medical issue, I come back and my PR is closed. This is absolutely still relevant. |
Sorry for the inconvenience. Hope you are doing better! From time to time, I go through all closed PRs and check if they are still relevant and reopen them if necessary. |
|
Can this PR be reviewed and implemented? Experiencing this issue and need the fix. |
Co-authored-by: Jan Larwig <jan@larwig.com>
Co-authored-by: Jan Larwig <jan@larwig.com>
Co-authored-by: Jan Larwig <jan@larwig.com>
Co-authored-by: Jan Larwig <jan@larwig.com>
…ter rename Signed-off-by: Bert Helderman <bert@transtrend.com>
|
Workflows seem to fail here adn there but all tests do succeed. |
|
This feature would be very helpful to me, but it looks like the PR has been stuck for a bit - is there something that can be done to un-stall this? |
|
By now we've implemented a custom http proxy doing the openid login and with for us flexible rules, also containing this feature, since this fairly small non-breaking PR took more than a year. |
Really sorry that we lost track of this PR :/ As I cannot edit the content of this PR. I copied your branch and commits (for full attribution of your contribution) and fixed the branch conflicts and test case: |
|
Will be merged as part of #3134 |
Description
This PR adds the option for a limit in the number of per-request CSRF cookies oauth2-proxy sets. During the doOAuthStart method we call this method in order to remove the oldest cookies.
Motivation and Context
See #2383
How Has This Been Tested?
Using a unit test, I ran
go test ./pgk/cookiesChecklist: