fix(authenticator): Move autoSignIn logic out of signUp actor
#1910
Conversation
This should fail because we aren't setting intents yet.
🦋 Changeset detectedLatest commit: f6d95ff The changes in this PR will be included in the next version bump. This PR includes changesets to release 4 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
This comment was marked as duplicate.
This comment was marked as duplicate.
This comment was marked as duplicate.
This comment was marked as duplicate.
This comment was marked as duplicate.
This comment was marked as duplicate.
This comment was marked as duplicate.
This comment was marked as duplicate.
This comment was marked as spam.
This comment was marked as spam.
wlee221
force-pushed
the
autosignin-refactor
branch
from
ab20b2a to
f0d954e
Compare
wlee221
changed the title
signIn actor
wlee221
changed the title
signIn actorautoSignIn logic to signIn actor
wlee221
changed the title
autoSignIn logic to signIn actorautoSignIn logic out of signUp actor`
wlee221
changed the title
autoSignIn logic out of signUp actor`autoSignIn logic out of signUp actor
| @@ -373,6 +374,7 @@ export function createAuthenticatorMachine() { | |||
| event.data?.intent === 'confirmSignUp', | |||
| shouldRedirectToResetPassword: (_, event) => | |||
| event.data?.intent === 'confirmPasswordReset', | |||
| shouldAutoSignIn: (_, event) => event.data?.intent === 'autoSignIn', | |||
There was a problem hiding this comment.
I realize after few months that intent is a bad name. Really what intent is the redirectTarget.
e.g. { intent: 'autoSignIn' } means that authenticator should go to autoSignIn state next.
| @@ -16,6 +16,7 @@ const getRouteComponent = (route: string) => { | |||
| case 'authenticated': | |||
| case 'idle': | |||
| case 'setup': | |||
| case 'autoSignIn': | |||
There was a problem hiding this comment.
We might need to add this case for Vue an Angular too. I noticed an unusual visual regression where after you enter your code in the confirm sign up page and click submit, a new floating bar at the top appears for a second or two before the authenticated page appears. It also occurs at sign out.
This only happens for Angular and Vue.
Otherwise looks good. 👍
There was a problem hiding this comment.
Good catch! That's been frustrating to me. I'll create a new PR that targets this branch.
Co-authored-by: Shane Laymance <shane.laymance@gmail.com>
Description of changes
Previously,
autoSignInflow was part of thesignUpactor. ButautoSignInis quite incomplete compared to whatsignInactor does;autoSignInonly triesAuth.signInand does not check for any auth challenges or edge cases like sms mfa, totp, etc.Instead of having incomplete
autoSignIninsignUpactor, this movesautoSignIntosignInactor. This makes semantic sense, becauseautoSignInis part of sign in flow anyway andsignUpactor should only be dealing with sign up logic.Issue #, if available
Fixes #1660.
Description of how you validated changes
I confirmed manually that end-users are be automatically redirected to
confirmSignInafter confirm sign up.Also added new e2e test that goes through this process.
Checklist
yarn testpassessideEffectsfield updatedBy submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.