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

Support ignoring redirects during oauth2 flow #326

Merged
merged 10 commits into from
Aug 17, 2022

Conversation

robert-blackman
Copy link
Contributor

Hello, we came across an issue with the OAuth2 login flow.

During a typical login, gate will respond with a 302 redirecting to deck. In the case that the predefined redirect URL is undefined (in our case it is undefined) localhost:8085 is set as the location. Due to the default behaviour of the go http client, this redirection is followed causing an error when the connection is inevitably refused.

This PR adds support for an ignoreRedirects configuration and/or --ignore-redirects command line arg that instructs the client not to follow redirects, the default behaviour should remain the same however. Additionally, the error message has been improved to help diagnose further failures.

Thanks!

@dbyron-sf
Copy link
Contributor

@Mergifyio update

@mergify
Copy link
Contributor

mergify bot commented Mar 4, 2022

update

❌ Pull request can't be updated with latest base branch changes

Mergify needs the author permission to update the base branch of the pull request.
monster-next needs to authorize modification on its head branch.
err-code: C4623

@dbyron-sf
Copy link
Contributor

@robert-blackman looks like there are some test failures.

@robert-blackman
Copy link
Contributor Author

Hey @dbyron-sf tests are passing locally, is there more here for us to do?

@dbyron-sf
Copy link
Contributor

Hey @dbyron-sf tests are passing locally, is there more here for us to do?

I'm not sure why, but the PR build checks don't seem to be triggering...maybe try pushing an empty commit to see if that wakes things up?

@robert-blackman
Copy link
Contributor Author

Hey @dbyron-sf tests are passing locally, is there more here for us to do?

I'm not sure why, but the PR build checks don't seem to be triggering...maybe try pushing an empty commit to see if that wakes things up?

@dbyron-sf gave it a kick, looks like the workflow needs to be approved

@dbyron-sf dbyron-sf added the ready to merge Approved and Reviewed and ready for merge label Aug 17, 2022
@mergify mergify bot added the auto merged label Aug 17, 2022
@mergify mergify bot merged commit 259a670 into spinnaker:master Aug 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto merged ready to merge Approved and Reviewed and ready for merge target-release/1.28
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants