avoid updating user when MfaWeakestDevice doesn't change (#47390)

This PR introduces a small change that skips user updates when adding or removing MFA devices if the end value is equal to the start value of `MfaWeakestDevice`. Signed-off-by: Tiago Silva <[email protected]>
gravitational · Oct 9, 2024 · 8492e07 · 8492e07
1 parent a6037e6
commit 8492e07
Show file tree

Hide file tree

Showing 2 changed files with 9 additions and 0 deletions.
diff --git a/lib/services/local/users.go b/lib/services/local/users.go
@@ -1282,6 +1282,10 @@ func (s *IdentityService) upsertUserStatusMFADevice(ctx context.Context, user st
 		user,
 		false, /*withSecrets*/
 		func(u types.User) (bool, error) {
+			// If the user already has the weakest device, don't update.
+			if u.GetWeakestDevice() == mfaState {
+				return false, nil
+			}
 			u.SetWeakestDevice(mfaState)
 			return true, nil
 		})

diff --git a/lib/services/local/users_test.go b/lib/services/local/users_test.go
@@ -1431,6 +1431,7 @@ func TestWeakestMFADeviceKind(t *testing.T) {
 	got, err = identity.GetUser(ctx, "bob", false)
 	require.NoError(t, err)
 	require.Equal(t, types.MFADeviceKind_MFA_DEVICE_KIND_TOTP, got.GetWeakestDevice())
+	oldRevision := got.GetMetadata().Revision
 
 	u2fDev := &types.MFADevice{
 		Metadata: types.Metadata{
@@ -1451,6 +1452,7 @@ func TestWeakestMFADeviceKind(t *testing.T) {
 	got, err = identity.GetUser(ctx, "bob", false)
 	require.NoError(t, err)
 	require.Equal(t, types.MFADeviceKind_MFA_DEVICE_KIND_TOTP, got.GetWeakestDevice())
+	require.Equal(t, oldRevision, got.GetMetadata().Revision, "revision should not change")
 
 	// Create webauthn device but state should still be MFA_DEVICE_KIND_TOTP
 	// because it shows the weakest state.
@@ -1477,6 +1479,7 @@ func TestWeakestMFADeviceKind(t *testing.T) {
 	got, err = identity.GetUser(ctx, "bob", false)
 	require.NoError(t, err)
 	require.Equal(t, types.MFADeviceKind_MFA_DEVICE_KIND_TOTP, got.GetWeakestDevice())
+	require.Equal(t, oldRevision, got.GetMetadata().Revision, "revision should not change")
 
 	// Delete the TOTP device and the state should be MFA_DEVICE_KIND_WEBAUTHN
 	err = identity.DeleteMFADevice(ctx, "bob", totpDevice.Id)
@@ -1485,6 +1488,7 @@ func TestWeakestMFADeviceKind(t *testing.T) {
 	got, err = identity.GetUser(ctx, "bob", false)
 	require.NoError(t, err)
 	require.Equal(t, types.MFADeviceKind_MFA_DEVICE_KIND_WEBAUTHN, got.GetWeakestDevice())
+	oldRevision = got.GetMetadata().Revision
 
 	// Delete the U2F device and the state should be MFA_DEVICE_KIND_WEBAUTHN
 	err = identity.DeleteMFADevice(ctx, "bob", u2fDev.Id)
@@ -1493,6 +1497,7 @@ func TestWeakestMFADeviceKind(t *testing.T) {
 	got, err = identity.GetUser(ctx, "bob", false)
 	require.NoError(t, err)
 	require.Equal(t, types.MFADeviceKind_MFA_DEVICE_KIND_WEBAUTHN, got.GetWeakestDevice())
+	require.Equal(t, oldRevision, got.GetMetadata().Revision, "revision should not change")
 
 	// Delete the Webauthn device and the state should be MFA_DEVICE_KIND_UNSET
 	err = identity.DeleteMFADevice(ctx, "bob", webauthnDevice.Id)