Skip to content

Commit

Permalink
[v15] fix: Correctly handle passwordless in MFA audit events (#40617)
Browse files Browse the repository at this point in the history 
* 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
  • Loading branch information
@codingllama
codingllama authored Apr 17, 2024
1 parent a399863 commit cbfa62c
Show file tree
Hide file tree
Showing 4 changed files with 200 additions and 37 deletions.
93 changes: 60 additions & 33 deletions lib/auth/auth.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
Expand All @@ -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:
Expand Down
112 changes: 110 additions & 2 deletions web/packages/teleport/src/Audit/__snapshots__/Audit.story.test.tsx.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -406,12 +406,12 @@ exports[`list of all events 1`] = `
</strong>
-
<strong>
236
238
</strong>
of

<strong>
236
238
</strong>
</div>
<button
Expand Down Expand Up @@ -537,6 +537,114 @@ exports[`list of all events 1`] = `
</tr>
</thead>
<tbody>
<tr>
<td
style="vertical-align: inherit;"
>
<div
class="c18"
>
<span
class="c19 icon icon-info"
>
<svg
fill="currentColor"
height="20"
viewBox="0 0 24 24"
width="20"
>
<path
d="M11.625 8.8125C12.1428 8.8125 12.5625 8.39277 12.5625 7.875C12.5625 7.35723 12.1428 6.9375 11.625 6.9375C11.1072 6.9375 10.6875 7.35723 10.6875 7.875C10.6875 8.39277 11.1072 8.8125 11.625 8.8125Z"
/>
<path
d="M10.5 11.25C10.5 10.8358 10.8358 10.5 11.25 10.5C11.6478 10.5 12.0294 10.658 12.3107 10.9393C12.592 11.2206 12.75 11.6022 12.75 12V15.75C13.1642 15.75 13.5 16.0858 13.5 16.5C13.5 16.9142 13.1642 17.25 12.75 17.25C12.3522 17.25 11.9706 17.092 11.6893 16.8107C11.408 16.5294 11.25 16.1478 11.25 15.75V12C10.8358 12 10.5 11.6642 10.5 11.25Z"
/>
<path
clip-rule="evenodd"
d="M12 2.25C6.61522 2.25 2.25 6.61522 2.25 12C2.25 17.3848 6.61522 21.75 12 21.75C17.3848 21.75 21.75 17.3848 21.75 12C21.75 6.61522 17.3848 2.25 12 2.25ZM3.75 12C3.75 7.44365 7.44365 3.75 12 3.75C16.5563 3.75 20.25 7.44365 20.25 12C20.25 16.5563 16.5563 20.25 12 20.25C7.44365 20.25 3.75 16.5563 3.75 12Z"
fill-rule="evenodd"
/>
</svg>
</span>
MFA Authentication Attempt
</div>
</td>
<td
style="word-break: break-word;"
>
Passwordless user requested an MFA authentication challenge
</td>
<td
style="min-width: 120px;"
>
2024-04-16T21:47:20.69Z
</td>
<td
align="right"
>
<button
class="c20"
kind="border"
width="87px"
>
Details
</button>
</td>
</tr>
<tr>
<td
style="vertical-align: inherit;"
>
<div
class="c18"
>
<span
class="c19 icon icon-info"
>
<svg
fill="currentColor"
height="20"
viewBox="0 0 24 24"
width="20"
>
<path
d="M11.625 8.8125C12.1428 8.8125 12.5625 8.39277 12.5625 7.875C12.5625 7.35723 12.1428 6.9375 11.625 6.9375C11.1072 6.9375 10.6875 7.35723 10.6875 7.875C10.6875 8.39277 11.1072 8.8125 11.625 8.8125Z"
/>
<path
d="M10.5 11.25C10.5 10.8358 10.8358 10.5 11.25 10.5C11.6478 10.5 12.0294 10.658 12.3107 10.9393C12.592 11.2206 12.75 11.6022 12.75 12V15.75C13.1642 15.75 13.5 16.0858 13.5 16.5C13.5 16.9142 13.1642 17.25 12.75 17.25C12.3522 17.25 11.9706 17.092 11.6893 16.8107C11.408 16.5294 11.25 16.1478 11.25 15.75V12C10.8358 12 10.5 11.6642 10.5 11.25Z"
/>
<path
clip-rule="evenodd"
d="M12 2.25C6.61522 2.25 2.25 6.61522 2.25 12C2.25 17.3848 6.61522 21.75 12 21.75C17.3848 21.75 21.75 17.3848 21.75 12C21.75 6.61522 17.3848 2.25 12 2.25ZM3.75 12C3.75 7.44365 7.44365 3.75 12 3.75C16.5563 3.75 20.25 7.44365 20.25 12C20.25 16.5563 16.5563 20.25 12 20.25C7.44365 20.25 3.75 16.5563 3.75 12Z"
fill-rule="evenodd"
/>
</svg>
</span>
MFA Authentication Attempt
</div>
</td>
<td
style="word-break: break-word;"
>
User [llama] requested an MFA authentication challenge
</td>
<td
style="min-width: 120px;"
>
2024-04-16T21:46:59.317Z
</td>
<td
align="right"
>
<button
class="c20"
kind="border"
width="87px"
>
Details
</button>
</td>
</tr>
<tr>
<td
style="vertical-align: inherit;"
Expand Down
24 changes: 24 additions & 0 deletions web/packages/teleport/src/Audit/fixtures/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3477,6 +3477,30 @@ export const events = [
user: 'bot-test12',
user_kind: 2,
},
{
challenge_allow_reuse: false,
challenge_scope: 'CHALLENGE_SCOPE_LOGIN',
cluster_name: 'zarq',
code: 'T1015I',
ei: 0,
event: 'mfa_auth_challenge.create',
time: '2024-04-16T21:46:59.317Z',
uid: '815bbcf4-fb05-4e08-917c-7259e9332d69',
user: 'llama',
user_kind: 1,
},
{
challenge_allow_reuse: false,
challenge_scope: 'CHALLENGE_SCOPE_PASSWORDLESS_LOGIN',
cluster_name: 'zarq',
code: 'T1015I',
ei: 0,
event: 'mfa_auth_challenge.create',
time: '2024-04-16T21:47:20.69Z',
uid: 'e4946d15-3a6f-4f0a-b456-1704a7586572',
// user is missing for passwordless challenges.
user_kind: 1,
},
].map(makeEvent);

// Do not add new events to this array, add it to `events` list.
Expand Down
8 changes: 6 additions & 2 deletions web/packages/teleport/src/services/audit/makeEvent.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -777,8 +777,12 @@ export const formatters: Formatters = {
[eventCodes.CREATE_MFA_AUTH_CHALLENGE]: {
type: 'mfa_auth_challenge.create',
desc: 'MFA Authentication Attempt',
format: ({ user }) =>
`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',
Expand Down

0 comments on commit cbfa62c

Please sign in to comment.