-
Notifications
You must be signed in to change notification settings - Fork 322
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
Fix redirect for IDV/IDP flow for OV enrollment on Android #3782
base: master
Are you sure you want to change the base?
Conversation
// Do not show "Open Okta Verify" button for "redirect-idp" remediation | ||
// converted to "success-redirect" with `convertRedirectIdPToSuccessRedirectIffOneIdp()` | ||
// (Which can happen if there is a IdP route configured for a user) | ||
const isSuccessRedirect = idx?.rawIdxState?.success?.href; |
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.
Without this fix it will show unnecessary page with "Open Okta Verify" button before redirect to IDP (not IDP authenticator).
In my example I've set up Github IdP and routing rule for my user:
screen-20250204-204028.mp4
src/v2/controllers/FormController.ts
Outdated
@@ -259,9 +259,10 @@ export default Controller.extend({ | |||
// Because SIW sort of finished its current /transaction/ | |||
sessionStorageHelper.removeStateHandle(); |
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 think we need to move this down into the else
conditional and also copy it into the AutoRedirectView
, because if the user is shown the redirect button (didn't get auto-redirected) but then refreshes the page for some reason, they will get returned to introspect rather than have the state preserved.
} else { | ||
// Clear when navigates away from SIW page, e.g. success, IdP Authenticator. | ||
// Because SIW sort of finished its current /transaction/ | ||
sessionStorageHelper.removeStateHandle(); |
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.
Should we also add this in the button's click method?
https://github.com/okta/okta-signin-widget/pull/3782/files#diff-c992b801cae4c3d84efb925a6c392d7485753b37da4566e0992c90f07be564faR98-R100
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 think yes
Description:
PR Checklist
Issue:
Reviewers:
Screenshot/Video:
Downstream Monolith Build: