Skip to content

Commit

Permalink
fix: remove setting MfaWeakestDevice when loading users from auth (#…
Browse files Browse the repository at this point in the history
…51226) (#51231)

This PR fixes a bug that causes compare and swap to fail when reading
users with secrets because we were mutating `MfaWeakestDevice` value and
comparison with database failed.

This PR removes the auto-filling of the field and transitions the
computation to CUD methods.

Fixes #51209

Signed-off-by: Tiago Silva <tiago.silva@goteleport.com>
  • Loading branch information
tigrato authored Jan 20, 2025
1 parent 2cfed43 commit a06e8b5
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 9 deletions.
5 changes: 0 additions & 5 deletions lib/services/local/resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -342,11 +342,6 @@ func userFromUserItems(name string, items userItems) (*types.UserV2, error) {
}
user.SetLocalAuth(auth)

if auth != nil {
// when reading with secrets, we can populate the data automatically.
user.SetWeakestDevice(getWeakestMFADeviceKind(auth.MFA))
}

return user, nil
}

Expand Down
8 changes: 4 additions & 4 deletions lib/services/local/users.go
Original file line number Diff line number Diff line change
Expand Up @@ -1228,7 +1228,7 @@ func (s *IdentityService) upsertUserStatusMFADevice(ctx context.Context, user st
if err != nil {
return trace.Wrap(err)
}
mfaState := getWeakestMFADeviceKind(devs)
mfaState := GetWeakestMFADeviceKind(devs)

_, err = s.UpdateAndSwapUser(
ctx,
Expand Down Expand Up @@ -1266,15 +1266,15 @@ func (s *IdentityService) buildWeakestMFADeviceKind(ctx context.Context, user st
if err != nil {
return types.MFADeviceKind_MFA_DEVICE_KIND_UNSET, trace.Wrap(err)
}
return getWeakestMFADeviceKind(append(devs, upsertingMFA...)), nil
return GetWeakestMFADeviceKind(append(devs, upsertingMFA...)), nil
}

// getWeakestMFADeviceKind returns the weakest MFA state based on the devices the user
// GetWeakestMFADeviceKind returns the weakest MFA state based on the devices the user
// has.
// When a user has no MFA device, it's set to `MFADeviceKind_MFA_DEVICE_KIND_UNSET`.
// When a user has at least one TOTP device, it's set to `MFADeviceKind_MFA_DEVICE_KIND_TOTP`.
// When a user ONLY has webauthn devices, it's set to `MFADeviceKind_MFA_DEVICE_KIND_WEBAUTHN`.
func getWeakestMFADeviceKind(devs []*types.MFADevice) types.MFADeviceKind {
func GetWeakestMFADeviceKind(devs []*types.MFADevice) types.MFADeviceKind {
mfaState := types.MFADeviceKind_MFA_DEVICE_KIND_UNSET
for _, d := range devs {
if (d.GetWebauthn() != nil || d.GetU2F() != nil) && mfaState == types.MFADeviceKind_MFA_DEVICE_KIND_UNSET {
Expand Down

0 comments on commit a06e8b5

Please sign in to comment.