Issue 978: Fix Custom cookie name breaks redis for session #1949
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
JoelSpeed
reviewed
Jun 12, 2023
tuunit
requested changes
Aug 24, 2023
There was a problem hiding this comment.
@miguelborges99 I would like to request that you split the test you adapted into two and explicitly test the old and new method and name the tests accordingly. Everything else LGTM.
tuunit
requested changes
Sep 1, 2023
tuunit
requested changes
Sep 10, 2023
There was a problem hiding this comment.
Hi @miguelborges99 please rebase your branch with the current master. Furthermore please wrap the errors as mentioned in the comments, to have a better error tracing.
tuunit
requested changes
Sep 10, 2023
tuunit
requested changes
Sep 13, 2023
|
Hi @miguelborges99, do you think we can get the tests done until friday? This way we would be able to merge it and release it as part of the 7.5.1 patch release on friday. |
@miguelborges99 will you be able to work on the comments mentioned in the review? Otherwise you could give me collaborator access on your fork and I'll be able to finalize the PR for you :) |
|
@miguelborges99 If you can fix up the changelog and wrap the errors I highlighted, then I think we can get this merged |
Sorry, I can only see this probably in the weekend. |
|
@tuunit : Sorry for the delay. The request for changes were done. |
No worries, its on your free time therefore I'm thankful you keep the PRs up to date :) |
tuunit
requested changes
Nov 18, 2023
JoelSpeed
approved these changes
Nov 18, 2023
miguelborges99
deleted the
custom_cookie_name_breaks_redis_for_session
branch
tuunit
pushed a commit
to tuunit/oauth2-proxy
that referenced
this pull request
Nov 20, 2023
…oxy#1949) * Issue 978: Fix Custom cookie name breaks redis for session (see oauth2-proxy#978) * Issue 978: Fix Custom cookie name breaks redis for session (see oauth2-proxy#978) * Update CHANGELOG.md * Issue 978: Fix Custom cookie name breaks redis for session * Issue 978: Fix Custom cookie name breaks redis for session * Issue 978: Fix Custom cookie name breaks redis for session * Issue 978: Fix Custom cookie name breaks redis for session * Issue 978: Fix Custom cookie name breaks redis for session * Issue 978: Fix Custom cookie name breaks redis for session * Update CHANGELOG.md --------- Co-authored-by: Nuno Borges <Nuno.Borges@ctw.bmwgroup.com> Co-authored-by: Joel Speed <Joel.speed@hotmail.co.uk>
tuunit
pushed a commit
to tuunit/oauth2-proxy
that referenced
this pull request
Nov 20, 2023
…oxy#1949) * Issue 978: Fix Custom cookie name breaks redis for session (see oauth2-proxy#978) * Issue 978: Fix Custom cookie name breaks redis for session (see oauth2-proxy#978) * Update CHANGELOG.md * Issue 978: Fix Custom cookie name breaks redis for session * Issue 978: Fix Custom cookie name breaks redis for session * Issue 978: Fix Custom cookie name breaks redis for session * Issue 978: Fix Custom cookie name breaks redis for session * Issue 978: Fix Custom cookie name breaks redis for session * Issue 978: Fix Custom cookie name breaks redis for session * Update CHANGELOG.md --------- Co-authored-by: Nuno Borges <Nuno.Borges@ctw.bmwgroup.com> Co-authored-by: Joel Speed <Joel.speed@hotmail.co.uk>
tuunit
pushed a commit
to tuunit/oauth2-proxy
that referenced
this pull request
Dec 17, 2023
…oxy#1949) * Issue 978: Fix Custom cookie name breaks redis for session (see oauth2-proxy#978) * Issue 978: Fix Custom cookie name breaks redis for session (see oauth2-proxy#978) * Update CHANGELOG.md * Issue 978: Fix Custom cookie name breaks redis for session * Issue 978: Fix Custom cookie name breaks redis for session * Issue 978: Fix Custom cookie name breaks redis for session * Issue 978: Fix Custom cookie name breaks redis for session * Issue 978: Fix Custom cookie name breaks redis for session * Issue 978: Fix Custom cookie name breaks redis for session * Update CHANGELOG.md --------- Co-authored-by: Nuno Borges <Nuno.Borges@ctw.bmwgroup.com> Co-authored-by: Joel Speed <Joel.speed@hotmail.co.uk>
tuunit
pushed a commit
to tuunit/oauth2-proxy
that referenced
this pull request
Dec 18, 2023
…oxy#1949) * Issue 978: Fix Custom cookie name breaks redis for session (see oauth2-proxy#978) * Issue 978: Fix Custom cookie name breaks redis for session (see oauth2-proxy#978) * Update CHANGELOG.md * Issue 978: Fix Custom cookie name breaks redis for session * Issue 978: Fix Custom cookie name breaks redis for session * Issue 978: Fix Custom cookie name breaks redis for session * Issue 978: Fix Custom cookie name breaks redis for session * Issue 978: Fix Custom cookie name breaks redis for session * Issue 978: Fix Custom cookie name breaks redis for session * Update CHANGELOG.md --------- Co-authored-by: Nuno Borges <Nuno.Borges@ctw.bmwgroup.com> Co-authored-by: Joel Speed <Joel.speed@hotmail.co.uk>
JoelSpeed
added a commit
that referenced
this pull request
Dec 18, 2023
* Add GitHub groups (orgs/teams) support * align code of getTeams with getOrgs to support Github Enterprise Server instances with different domain * add documentation * add missing import after rebase * add nightly build and push (#2297) * add nightly build and push * add date based nightly build tags * only keep single multiarch image build and push * add changelog * add images to internal docs static files * add docu for nightly builds * remove unnecessary spaces * update nightly repository * Issue 978: Fix Custom cookie name breaks redis for session (#1949) * Issue 978: Fix Custom cookie name breaks redis for session (see #978) * Issue 978: Fix Custom cookie name breaks redis for session (see #978) * Update CHANGELOG.md * Issue 978: Fix Custom cookie name breaks redis for session * Issue 978: Fix Custom cookie name breaks redis for session * Issue 978: Fix Custom cookie name breaks redis for session * Issue 978: Fix Custom cookie name breaks redis for session * Issue 978: Fix Custom cookie name breaks redis for session * Issue 978: Fix Custom cookie name breaks redis for session * Update CHANGELOG.md --------- Co-authored-by: Nuno Borges <Nuno.Borges@ctw.bmwgroup.com> Co-authored-by: Joel Speed <Joel.speed@hotmail.co.uk> * Support http.AllowQuerySemicolons (#2248) * Support http.AllowQuerySemicolons * Docs * Make it clear we are overriding the handler * Update documentation for allow-query-semicolons * Fix changelog format * Fix formatting --------- Co-authored-by: MickMake <github@mickmake.com> * Add GitHub groups (orgs/teams) support * align code of getTeams with getOrgs to support Github Enterprise Server instances with different domain * add documentation * fix changelog & documentation * fix missing import --------- Co-authored-by: Tobias Mayer <github@tobiasm.de> Co-authored-by: Nuno Miguel Micaelo Borges <miguelborges99@gmail.com> Co-authored-by: Nuno Borges <Nuno.Borges@ctw.bmwgroup.com> Co-authored-by: Joel Speed <Joel.speed@hotmail.co.uk> Co-authored-by: Tim White <tim.white@su.org.au> Co-authored-by: MickMake <github@mickmake.com>
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.
…://github.com//issues/978)
Allow cookie names with dots in redis sessions
Description
Ticket decoding was failing when cookie name had dots in it and using redis. To avoid such issues, ticket ID is encoded with base64. The ticket is now encoded using the format: {encoding version}.{ticketID base64}.{ticketSecret base 64}. This will allow to decode properly for different enconding versions.
Motivation and Context
Some users configure cookie name with dots and oauth2-proxy fails.
See issue 978.
How Has This Been Tested?
Needs to be tested.
Checklist: