Fixed Oauth redirect not working on Android Chrome #18513
Conversation
This fixes the issue described in hashicorp#16778. Navigation is blocked in Android chrome while redirecting back after OIDC authentication. The issue is explained by the lead maintainer of AppAuth(https://stackoverflow.com/a/41882732). The latest Chrome version redirects to the app only if triggered by the user and not automatically redirect. Hence, a link is added in the UI to redirect back to the app.
|
|
|
Thanks for this contribution, @prajnamohan1! @siepkes would you be able to see if this fixes your issue? |
There was a problem hiding this comment.
Thank you for catching this and making the fix! One small suggestion below so that we can still test this workflow, and we will need to replace all the UI tests that have assert.dom('[data-test-oidc-redirect]').hasTextContaining(...) with assert.dom('[data-test-oidc-redirect]').exists(...)
Co-authored-by: Chelsea Shaw <82459713+hashishaw@users.noreply.github.com>
|
@austingebauer I can confirm this fixes the issue (I verified it). But to clarify @prajnamohan1 is a co-worker of mine. I asked her to investigate this issue. |
prajnamohan1
requested review from
hashishaw
and removed request for
austingebauer
|
@hashishaw Hi! I've made the requested changes! Is there something else I need to change? |
|
Great to see the PR was approved! Is there anything you need from @prajnamohan1 or me in order to be able to merge the change? Let us know! |
There was a problem hiding this comment.
Sorry for the delay! Looking at this again, I think there are some changes we'd like to make sure we're using the best practices for Ember. I've added some comments, please let me know if anything is unclear.
One other request: please add a changelog (instructions can be found here).
Thank you for your contribution! Apologies again for the delay
| @@ -21,8 +21,9 @@ | |||
| @redirect={{this.model.consent.redirect}} | |||
| @onSuccess={{this._handleSuccess}} | |||
| /> | |||
| {{else if this.model.redirectUrl}} | |||
| <div data-test-oidc-redirect>{{this.model.redirectUrl}}</div> | |||
| {{else if this.redirect_uri}} | |||
There was a problem hiding this comment.
| {{else if this.redirect_uri}} | |
| {{else if this.model.redirectUrl}} |
There was a problem hiding this comment.
In the tests we can replace lines 166-169 and lines 192-194 each with the following snippet:
assert
.dom('[data-test-oidc-redirect]')
.hasTextContaining(`click here to go back to app`, 'Shows link back to app');
const link = document.querySelector('[data-test-oidc-redirect]').getAttribute('href');
assert.ok(link.includes('/callback?code='), 'Redirects to correct url');
| @@ -94,6 +94,7 @@ export default class VaultClusterOidcProviderRoute extends Route { | |||
| _handleSuccess(response, baseUrl, state) { | |||
| const { code } = response; | |||
| const redirectUrl = this._buildUrl(baseUrl, { code, state }); | |||
| this.redirect_uri = redirectUrl; | |||
There was a problem hiding this comment.
Instead of setting this, we can try to replace the location and return the redirectUrl at the end if it is blocked by the browser. So we would change both this and the _handleError block below to:
if (!Ember.testing) {
this.win.location.replace(redirectUrl);
}
return { redirectUrl };
| <div data-test-oidc-redirect>{{this.model.redirectUrl}}</div> | ||
| {{else if this.redirect_uri}} | ||
| <VaultLogoSpinner /> | ||
| <a href={{this.redirect_uri}} data-test-oidc-redirect>Click here to redirect back to app</a> |
There was a problem hiding this comment.
Whoops, forgot one change request:
| <a href={{this.redirect_uri}} data-test-oidc-redirect>Click here to redirect back to app</a> | |
| <p>If you are not automatically redirected, | |
| <a href={{this.model.redirectUrl}} data-test-oidc-redirect>click here to go back to app.</a></p> |
This fixes the issue described in #16778. Navigation is blocked in Android chrome while redirecting back after OIDC authentication. The issue is explained by the lead maintainer of
AppAuth(https://stackoverflow.com/a/41882732).
The latest Chrome version redirects to the app only if triggered by the user and not automatically redirect. Hence, a link is added in the UI to redirect back to the app.