Skip to content

Commit

Permalink
[v17] Add a null check to getMfaChallengeResponse (#50611)
Browse files Browse the repository at this point in the history
* Add a null check to getMfaChallengeResponse.

* Add other null checks, just in case.

* Update signatures to include undefined, consistently return undefined

---------

Co-authored-by: joerger <bjoerger@goteleport.com>
Co-authored-by: Rafał Cieślak <rafal.cieslak@goteleport.com>
  • Loading branch information
3 people authored Dec 30, 2024
1 parent 2b88ebc commit 10a8072
Show file tree
Hide file tree
Showing 4 changed files with 12 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,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 @@ -82,16 +82,16 @@ class Tty extends EventEmitterMfaSender {
this.socket.send(bytearray.buffer);
}

sendChallengeResponse(data: MfaChallengeResponse) {
sendChallengeResponse(resp: 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 18
const backwardCompatibleData = {
...data.webauthn_response,
...data,
...resp?.webauthn_response,
...resp,
};
const encoded = this._proto.encodeChallengeResponse(
JSON.stringify(backwardCompatibleData)
Expand Down
10 changes: 6 additions & 4 deletions web/packages/teleport/src/services/auth/auth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,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 @@ -280,7 +280,9 @@ const auth = {
challenge: MfaAuthenticateChallenge,
mfaType?: DeviceType,
totpCode?: string
): Promise<MfaChallengeResponse> {
): Promise<MfaChallengeResponse | undefined> {
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
// whichever method we can succeed with first.
Expand All @@ -303,7 +305,7 @@ const auth = {
}

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

async getWebAuthnChallengeResponse(
Expand Down Expand Up @@ -387,7 +389,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
4 changes: 2 additions & 2 deletions web/packages/teleport/src/services/mfa/makeMfa.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,13 +63,13 @@ export function parseMfaRegistrationChallengeJson(
// parseMfaChallengeJson formats fetched authenticate challenge JSON.
export function parseMfaChallengeJson(
challenge: MfaAuthenticateChallengeJson
): MfaAuthenticateChallenge {
): MfaAuthenticateChallenge | undefined {
if (
!challenge.sso_challenge &&
!challenge.webauthn_challenge &&
!challenge.totp_challenge
) {
return null;
return;
}

// WebAuthn challenge contains Base64URL(byte) fields that needs to
Expand Down

0 comments on commit 10a8072

Please sign in to comment.