-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
Fixes SSO Redirect #9943
Fixes SSO Redirect #9943
Conversation
Build failed.
|
recheck |
Build succeeded.
|
We are gonna have the #9908 in first |
@@ -61,13 +61,16 @@ function AWXLogin({ alt, i18n, isAuthenticated }) { | |||
const logoSrc = custom_logo | |||
? `data:image/jpeg;${custom_logo}` | |||
: loginLogoSrc; | |||
if (login_redirect_override && !isAuthenticated(document.cookie)) { | |||
return window.location.replace(login_redirect_override); |
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.
I like the simplicity. That said, burying this deep within an async callback (and conditionally changing its return structure) makes me a little uneasy. To me, it's an unexpected thing to do in an asynchronous api response handler.
We have other conditional redirects further down in the component definition that use the typical react-router
semantics and, if possible, I think we should do something similar.
I think all you'd need to do is add:
if (login_redirect_override) {
return <Redirect to={login_redirect_override} />;
}
to this line
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.
or, if react-router doesn't like external urls:
if (login_redirect_override) {
window.location.replace(login_redirect_override);
return null;
}
to this line
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.
React router does not like redirecting outside of the app. Using option 1
Build succeeded.
|
Build failed.
|
recheck |
Build failed.
|
Hey, @AlexSCorey - once the check failure is fixed I can take a look at this. I'll be out next week so we may need to reassign if I can't get it hammered out |
Build succeeded.
|
I've verified this is working correctly. SSO verification is a manual process currently (though I'd like to automate it in the future). |
Build succeeded (gate pipeline).
|
SUMMARY
This fixes #9115 by simply checking if there is a redirect url in and then replacing it with the existing url in history, navigating the user to the correct login url.
ISSUE TYPE
COMPONENT NAME
AWX VERSION
ADDITIONAL INFORMATION