WebUI MFA types refactor #49678
WebUI MFA types refactor #49678
Conversation
| }; | ||
| } | ||
|
|
||
| return opt as MfaOption; |
There was a problem hiding this comment.
Isn't opt already MfaOption here?
There was a problem hiding this comment.
It's of type services/mfa/MfaOption but this method needs to return utils/MfaOption. I'm not sure how to how to make the former a type alias of the latter since they have the same name, so I just did a type cast. Is there another way to make this work?
There was a problem hiding this comment.
would using satisfies MfaOption work here to coerce it w/out asserting?
| return mfaOptions; | ||
| // Deprecated: use getMfaRegisterOptions or getMfaChallengeOptions instead. | ||
| // TODO(Joerger): Delete once no longer used. | ||
| export default function createMfaOptions(opts: Options): MfaOption[] { |
There was a problem hiding this comment.
Now that this no longer handles required, should this just accept auth2faType?
There was a problem hiding this comment.
Yeah, but I can't change the function signature until the RecoveryFlow in /e starts using the new MfaOptions methods. I'm going to remove the method all existing calls to createMfaOptions in a couple of follow up PRs to avoid merge conflicts and /e shenanigans.
| mfaOptions.push({ value: 'webauthn', label: 'Passkey or Security Key' }); | ||
| } | ||
|
|
||
| if (mfaChallenge?.totpChallenge) { | ||
| mfaOptions.push({ value: 'totp', label: 'Authenticator App' }); | ||
| } | ||
|
|
||
| if (mfaChallenge?.ssoChallenge) { | ||
| mfaOptions.push({ | ||
| value: 'sso', | ||
| label: | ||
| mfaChallenge.ssoChallenge.device.displayName || | ||
| mfaChallenge.ssoChallenge.device.connectorId, | ||
| }); | ||
| } | ||
|
|
||
| return mfaOptions; | ||
| } | ||
|
|
||
| export function getMfaRegisterOptions(auth2faType: Auth2faType) { | ||
| const mfaOptions: MfaOption[] = []; | ||
|
|
||
| if (auth2faType === 'webauthn' || auth2faType === 'on') { | ||
| mfaOptions.push({ value: 'webauthn', label: 'Passkey or Security Key' }); | ||
| } | ||
|
|
||
| if (auth2faType === 'otp' || auth2faType === 'on') { | ||
| mfaOptions.push({ value: 'totp', label: 'Authenticator App' }); |
There was a problem hiding this comment.
Seems like these objects can be defined outside of the functions and then it's just mfaOptions.push(WEBAUTHN_OPTION) etc in both methods
There was a problem hiding this comment.
Good point, added const webauthnOption, const totpOption, and func getSsoOption.
SSO option is derived from the challenge so I'm not sure if an enum works here (that's generally what all caps like WEBAUTHN_OPTION is for right?).
Joerger
added
no-changelog
Joerger
force-pushed
the
joerger/unify-mfa-types
branch
from
a190b68
to
7ae2721
Compare
Joerger
force-pushed
the
joerger/unify-mfa-types
branch
from
7ae2721
to
e0ee68e
Compare
Joerger
force-pushed
the
joerger/unify-mfa-types
branch
from
e0ee68e
to
9755a70
Compare
Changes:
web/packages/teleport/src/services/mfaauth2faTypePrerequisite for SSO MFA changes (TODO).
TODO: Follow up PRs to remove remaining uses of
createMfaOptions.