Skip to content

Conversation

@bh-tt
Copy link
Contributor

@bh-tt bh-tt commented Apr 23, 2024

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/cookies

Checklist:

  • My change requires a change to the documentation or CHANGELOG.
  • I have updated the documentation/CHANGELOG accordingly.
  • I have created a feature (non-master) branch for my PR.
  • I have written tests for my code changes.

@bh-tt bh-tt marked this pull request as ready for review April 23, 2024 15:08
@bh-tt bh-tt requested a review from a team as a code owner April 23, 2024 15:08
@github-actions github-actions bot added the docs label Apr 23, 2024
Copy link
Contributor Author

@bh-tt bh-tt left a 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>
@github-actions
Copy link
Contributor

github-actions bot commented Sep 3, 2024

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.

@github-actions github-actions bot added the Stale label Sep 3, 2024
@github-actions github-actions bot closed this Sep 11, 2024
@rbange
Copy link

rbange commented Sep 11, 2024

This is still relevant

@bh-tt
Copy link
Contributor Author

bh-tt commented Sep 19, 2024

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.

@tuunit tuunit reopened this Sep 30, 2024
@tuunit
Copy link
Member

tuunit commented Sep 30, 2024

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.

@github-actions github-actions bot removed the Stale label Oct 1, 2024
@staplWaldo
Copy link

Can this PR be reviewed and implemented? Experiencing this issue and need the fix.

@tuunit tuunit changed the title Add a cookie-csrf-per-request-limit attribute feat: add a cookie-csrf-per-request-limit attribute Nov 7, 2024
bh-tt and others added 6 commits November 8, 2024 08:10
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>
@bh-tt bh-tt requested a review from tuunit November 11, 2024 14:33
@bh-tt
Copy link
Contributor Author

bh-tt commented Jan 28, 2025

Workflows seem to fail here adn there but all tests do succeed.

@danielluke
Copy link

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?

@bh-tt
Copy link
Contributor Author

bh-tt commented Jul 18, 2025

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.

@tuunit tuunit mentioned this pull request Jul 20, 2025
4 tasks
@tuunit
Copy link
Member

tuunit commented Jul 20, 2025

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:

0bb82be

@tuunit
Copy link
Member

tuunit commented Jul 20, 2025

Will be merged as part of #3134

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants