Skip to content

Commit

Permalink
Return an empty challenge and challenge response instead of undefined.
Browse files Browse the repository at this point in the history
  • Loading branch information
Joerger committed Jan 16, 2025
1 parent f0c562d commit 21430bc
Show file tree
Hide file tree
Showing 4 changed files with 12 additions and 19 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ export function ChangePasswordWizard({
const reauthState = useReAuthenticate({
challengeScope: MfaChallengeScope.CHANGE_PASSWORD,
onMfaResponse: async mfaResponse =>
setWebauthnResponse(mfaResponse?.webauthn_response),
setWebauthnResponse(mfaResponse.webauthn_response),
});

const [reauthMethod, setReauthMethod] = useState<ReauthenticationMethod>();
Expand Down
6 changes: 3 additions & 3 deletions web/packages/teleport/src/lib/term/tty.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,16 +80,16 @@ class Tty extends EventEmitterMfaSender {
this.socket.send(bytearray.buffer);
}

sendChallengeResponse(resp: MfaChallengeResponse) {
sendChallengeResponse(data: MfaChallengeResponse) {
// we want to have the backend listen on a single message type
// for any responses. so our data will look like data.webauthn, data.sso, etc
// but to be backward compatible, we need to still spread the existing webauthn only fields
// as "top level" fields so old proxies can still respond to webauthn challenges.
// in 19, we can just pass "data" without this extra step
// TODO (avatus): DELETE IN 19.0.0
const backwardCompatibleData = {
...resp?.webauthn_response,
...resp,
...data.webauthn_response,
...data,
};
const encoded = this._proto.encodeChallengeResponse(
JSON.stringify(backwardCompatibleData)
Expand Down
13 changes: 7 additions & 6 deletions web/packages/teleport/src/services/auth/auth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ const auth = {
.then(res => {
const request = {
action: 'accept',
webauthnAssertionResponse: res?.webauthn_response,
webauthnAssertionResponse: res.webauthn_response,
};

return api.put(cfg.getHeadlessSsoPath(transactionId), request);
Expand Down Expand Up @@ -274,13 +274,14 @@ const auth = {
},

// getChallengeResponse gets an MFA challenge response for the provided parameters.
// If is_mfa_required_req is provided and it is found that MFA is not required, returns null instead.
// If challenge is undefined or has no viable challenge options, returns empty.
async getMfaChallengeResponse(
challenge: MfaAuthenticateChallenge,
mfaType?: DeviceType,
totpCode?: string
): Promise<MfaChallengeResponse | undefined> {
if (!challenge) return;
): Promise<MfaChallengeResponse> {
// No challenge, return empty response.
if (!challenge) return {};

// TODO(Joerger): If mfaType is not provided by a parent component, use some global context
// to display a component, similar to the one used in useMfa. For now we just default to
Expand Down Expand Up @@ -310,7 +311,7 @@ const auth = {
}

// No viable challenge, return empty response.
return;
return {};
},

async getWebAuthnChallengeResponse(
Expand Down Expand Up @@ -439,7 +440,7 @@ const auth = {
return auth
.getMfaChallenge({ scope, allowReuse, isMfaRequiredRequest }, abortSignal)
.then(challenge => auth.getMfaChallengeResponse(challenge, 'webauthn'))
.then(res => res?.webauthn_response);
.then(res => res.webauthn_response);
},

getMfaChallengeResponseForAdminAction(allowReuse?: boolean) {
Expand Down
10 changes: 1 addition & 9 deletions web/packages/teleport/src/services/mfa/makeMfa.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,15 +63,7 @@ export function parseMfaRegistrationChallengeJson(
// parseMfaChallengeJson formats fetched authenticate challenge JSON.
export function parseMfaChallengeJson(
challenge: MfaAuthenticateChallengeJson
): MfaAuthenticateChallenge | undefined {
if (
!challenge.sso_challenge &&
!challenge.webauthn_challenge &&
!challenge.totp_challenge
) {
return;
}

): MfaAuthenticateChallenge {
// WebAuthn challenge contains Base64URL(byte) fields that needs to
// be converted to ArrayBuffer expected by navigator.credentials.get:
// - challenge
Expand Down

0 comments on commit 21430bc

Please sign in to comment.