From cbfa62cce1e64ec7af07281a662ac2576cef9db3 Mon Sep 17 00:00:00 2001 From: Alan Parra Date: Wed, 17 Apr 2024 11:11:31 -0300 Subject: [PATCH] [v15] fix: Correctly handle passwordless in MFA audit events (#40617) * fix: Correctly handle passwordless in MFA audit events (#40607) * Record passwordless username in the mfa_auth_challenge.validate event * Correctly render mfa_auth_challenge.create events without an user * Update snapshots * Update snapshots --- lib/auth/auth.go | 93 +++++++++------ .../__snapshots__/Audit.story.test.tsx.snap | 112 +++++++++++++++++- .../teleport/src/Audit/fixtures/index.ts | 24 ++++ .../teleport/src/services/audit/makeEvent.ts | 8 +- 4 files changed, 200 insertions(+), 37 deletions(-) diff --git a/lib/auth/auth.go b/lib/auth/auth.go index 9d795ff04c816..9bcb079195617 100644 --- a/lib/auth/auth.go +++ b/lib/auth/auth.go @@ -6147,7 +6147,66 @@ func (a *Server) validateMFAAuthResponseForRegister(ctx context.Context, resp *p // required challenge extensions will be checked against the stored challenge when // applicable (webauthn only). Returns the authentication data derived from the solved // challenge. -func (a *Server) ValidateMFAAuthResponse(ctx context.Context, resp *proto.MFAAuthenticateResponse, user string, requiredExtensions *mfav1.ChallengeExtensions) (mfaAuthData *authz.MFAAuthData, err error) { +func (a *Server) ValidateMFAAuthResponse( + ctx context.Context, + resp *proto.MFAAuthenticateResponse, + user string, + requiredExtensions *mfav1.ChallengeExtensions, +) (*authz.MFAAuthData, error) { + + authData, validateErr := a.validateMFAAuthResponseInternal(ctx, resp, user, requiredExtensions) + // validateErr handled after audit. + + // Read ClusterName for audit. + var clusterName string + if cn, err := a.GetClusterName(); err != nil { + log.WithError(err).Warn("Failed to read cluster name") + // err swallowed on purpose. + } else { + clusterName = cn.GetClusterName() + } + + // Take the user from the authData if the user param is empty. + // This happens for passwordless. + if user == "" && authData != nil { + user = authData.User + } + + // Emit audit event. + auditEvent := &apievents.ValidateMFAAuthResponse{ + Metadata: apievents.Metadata{ + Type: events.ValidateMFAAuthResponseEvent, + ClusterName: clusterName, + }, + UserMetadata: authz.ClientUserMetadataWithUser(ctx, user), + ChallengeScope: requiredExtensions.Scope.String(), + } + if validateErr != nil { + auditEvent.Code = events.ValidateMFAAuthResponseFailureCode + auditEvent.Success = false + auditEvent.UserMessage = validateErr.Error() + auditEvent.Error = validateErr.Error() + } else { + auditEvent.Code = events.ValidateMFAAuthResponseCode + auditEvent.Success = true + deviceMetadata := mfaDeviceEventMetadata(authData.Device) + auditEvent.MFADevice = &deviceMetadata + auditEvent.ChallengeAllowReuse = authData.AllowReuse == mfav1.ChallengeAllowReuse_CHALLENGE_ALLOW_REUSE_YES + } + if err := a.emitter.EmitAuditEvent(ctx, auditEvent); err != nil { + log.WithError(err).Warn("Failed to emit ValidateMFAAuthResponse event") + // err swallowed on purpose. + } + + return authData, trace.Wrap(validateErr) +} + +func (a *Server) validateMFAAuthResponseInternal( + ctx context.Context, + resp *proto.MFAAuthenticateResponse, + user string, + requiredExtensions *mfav1.ChallengeExtensions, +) (*authz.MFAAuthData, error) { if requiredExtensions == nil { return nil, trace.BadParameter("required challenge extensions parameter required") } @@ -6159,38 +6218,6 @@ func (a *Server) ValidateMFAAuthResponse(ctx context.Context, resp *proto.MFAAut return nil, trace.BadParameter("user required") } - clusterName, err := a.GetClusterName() - if err != nil { - return nil, trace.Wrap(err) - } - - defer func() { - auditEvent := &apievents.ValidateMFAAuthResponse{ - Metadata: apievents.Metadata{ - Type: events.ValidateMFAAuthResponseEvent, - ClusterName: clusterName.GetClusterName(), - }, - UserMetadata: authz.ClientUserMetadataWithUser(ctx, user), - ChallengeScope: requiredExtensions.Scope.String(), - } - if err != nil { - auditEvent.Code = events.ValidateMFAAuthResponseFailureCode - auditEvent.Success = false - auditEvent.UserMessage = err.Error() - auditEvent.Error = err.Error() - } else { - auditEvent.Code = events.ValidateMFAAuthResponseCode - auditEvent.Status.Success = true - deviceMetadata := mfaDeviceEventMetadata(mfaAuthData.Device) - auditEvent.MFADevice = &deviceMetadata - auditEvent.ChallengeAllowReuse = mfaAuthData.AllowReuse == mfav1.ChallengeAllowReuse_CHALLENGE_ALLOW_REUSE_YES - } - - if err := a.emitter.EmitAuditEvent(ctx, auditEvent); err != nil { - log.WithError(err).Warn("Failed to emit ValidateMFAAuthResponse event.") - } - }() - switch res := resp.Response.(type) { // cases in order of preference case *proto.MFAAuthenticateResponse_Webauthn: diff --git a/web/packages/teleport/src/Audit/__snapshots__/Audit.story.test.tsx.snap b/web/packages/teleport/src/Audit/__snapshots__/Audit.story.test.tsx.snap index 6f8b3c8ffdd5c..67e3cf112bb1c 100644 --- a/web/packages/teleport/src/Audit/__snapshots__/Audit.story.test.tsx.snap +++ b/web/packages/teleport/src/Audit/__snapshots__/Audit.story.test.tsx.snap @@ -406,12 +406,12 @@ exports[`list of all events 1`] = ` - - 236 + 238 of - 236 + 238 + + + + +
+ + + + + + + + MFA Authentication Attempt +
+ + + User [llama] requested an MFA authentication challenge + + + 2024-04-16T21:46:59.317Z + + + + + - `User [${user}] requested an MFA authentication challenge`, + format: ({ user }) => { + if (user) { + return `User [${user}] requested an MFA authentication challenge`; + } + return `Passwordless user requested an MFA authentication challenge`; + }, }, [eventCodes.VALIDATE_MFA_AUTH_RESPONSE]: { type: 'mfa_auth_challenge.validate',