Fix regenerating recovery codes webauthn scope#40633
Conversation
|
The PR changelog entry failed validation: Changelog entry not found in the PR body. Please add a "no-changelog" label to the PR, or changelog lines starting with |
zmb3
left a comment
There was a problem hiding this comment.
This has regressed more than once, so it would be great to figure out a way to test it.
I've refactored the privilege-token-based re-authentication flow to ensure the Other than that, we'd need to add an e2e test that checks that the scope provided on the front end matches the scope expected on the backend, which I'm not sure is necessary. |
codingllama
left a comment
There was a problem hiding this comment.
Seems reasonable to me, but I think a frontend person should take a look.
| if ('onMfaResponse' in props) { | ||
| auth | ||
| .getWebauthnResponse(scope) | ||
| .getWebauthnResponse(props.challengeScope) |
There was a problem hiding this comment.
Could we document the challengeScope field as irrelevant if we are going this way? (Ideally, the presence of an event handler should not even affect the logic here, but for the sake of making minimal changes, we can omit this.)
There was a problem hiding this comment.
The challengeScope is relevant here for getWebauthnResponse, it's just not relevant for createPrivilegeTokenWithWebauthn.
I agree that using the event handler's presence as a logic switch is odd, but the way it actually works is that props can only be one of two different mutually exclusive types:
// MfaResponseProps defines a function
// that accepts a MFA response. No
// authentication has been done at this point.
type MfaResponseProps = BaseProps & {
onMfaResponse(res: MfaAuthnResponse): void;
/**
* The MFA challenge scope of the action to perform, as defined in webauthn.proto.
*/
challengeScope: MfaChallengeScope;
onAuthenticated?: never;
};
// DefaultProps defines a function that
// accepts a privilegeTokenId that is only
// obtained after MFA response has been
// validated.
type DefaultProps = BaseProps & {
onAuthenticated(privilegeTokenId: string): void;
onMfaResponse?: never;
// TODO(Joerger): change type to 'never' once it is no longer expected in /e
challengeScope?: MfaChallengeScope;
};I'm not sure what additional info to document, so feel free to leave any specific suggestions, or resolve this comment if you're ok with merging as is.
* Always provide MANAGE_DEVICES webauthn scope for creating privileged tokens. * Make challenge scope exclusive to onMfaResponse flow. * Fix UI lint.
Changelog: Fix regenerating cloud account recovery codes
The
CreatePrivilegeTokenendpoint always expects theMANAGE_DEVICESwebauthn scope. Since regenerating recovery codes uses a privileged token instead of using a webauthn response directly, using theACCOUNT_RECOVERYscope for this flow would result in an error:Note:
ACCOUNT_RECOVERYis exclusively expected by theVerifyAccountRecoveryendpoint. It should be used for other one-shot recovery endpoints as well.Fixes #40528
TODO: follow up on
TODOonce https://github.com/gravitational/teleport.e/pull/3980 is merged as well.