Skip to content
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

Add redirect capability to sign_out #314

Merged
merged 3 commits into from
Nov 19, 2019
Merged

Add redirect capability to sign_out #314

merged 3 commits into from
Nov 19, 2019

Conversation

morarucostel
Copy link
Contributor

Fix for the #311

Description

Added the redirect capability for the sign_out path.

Motivation and Context

When installed multiple instances of oauth-proxy in the same k8s cluster, for example one instance for app1 and a second one for app2, after the sign_out, the request will get redirected to the same endpoint without the possibility to differenciate among the two paths.

How Has This Been Tested?

Tested in our environments.

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.

@morarucostel morarucostel requested a review from a team November 18, 2019 19:13
@morarucostel morarucostel changed the title addint redirect capability to sign_out Add redirect capability to sign_out Nov 18, 2019
JoelSpeed
JoelSpeed previously approved these changes Nov 19, 2019
Copy link
Member

@JoelSpeed JoelSpeed left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, @syscll got anything to add? I think this one is pretty simple and it looks like GetRedirect should return / in the default case which maintains existing behaviour which is nice

loshz
loshz previously approved these changes Nov 19, 2019
Copy link
Contributor

@loshz loshz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

Just need to resolve conflicts then we're good to go!

@JoelSpeed JoelSpeed dismissed stale reviews from loshz and themself via 8af4058 November 19, 2019 17:06
@JoelSpeed
Copy link
Member

I fixed up the conflict, going to merge this

Thanks @morarucostel for your work :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants