Audit the AddCredential action after we commit to the DB (#9324)

Resolve NuGet/Engineering#4660

Author: joelverhagen

src/NuGetGallery.Services/Authentication/AuthenticationService.cs

@@ -473,6 +473,9 @@ public virtual async Task ReplaceCredential(User user, Credential credential)
         {
             await ReplaceCredentialInternal(user, credential);
             await Entities.SaveChangesAsync();
+
+            await Auditing.SaveAuditRecordAsync(new UserAuditRecord(
+                user, AuditedUserAction.AddCredential, credential));
         }
 
         public virtual async Task<Credential> ResetPasswordWithToken(string username, string token, string newPassword)
@@ -501,6 +504,10 @@ public virtual async Task<Credential> ResetPasswordWithToken(string username, st
                 user.FailedLoginCount = 0;
                 user.LastFailedLoginUtc = null;
                 await Entities.SaveChangesAsync();
+
+                await Auditing.SaveAuditRecordAsync(new UserAuditRecord(
+                    user, AuditedUserAction.AddCredential, cred));
+
                 return cred;
             }
 
@@ -590,6 +597,10 @@ public virtual async Task<bool> ChangePassword(User user, string oldPassword, st
 
             // Save changes
             await Entities.SaveChangesAsync();
+
+            await Auditing.SaveAuditRecordAsync(new UserAuditRecord(
+                user, AuditedUserAction.AddCredential, passwordCredential));
+
             return true;
         }
 
@@ -623,10 +634,10 @@ public virtual async Task AddCredential(User user, Credential credential)
                 throw new InvalidOperationException(ServicesStrings.OrganizationsCannotCreateCredentials);
             }
 
-            await Auditing.SaveAuditRecordAsync(new UserAuditRecord(user, AuditedUserAction.AddCredential, credential));
             user.Credentials.Add(credential);
             await Entities.SaveChangesAsync();
 
+            await Auditing.SaveAuditRecordAsync(new UserAuditRecord(user, AuditedUserAction.AddCredential, credential));
             _telemetryService.TrackNewCredentialCreated(user, credential);
         }
 
@@ -838,9 +849,6 @@ await Auditing.SaveAuditRecordAsync(new UserAuditRecord(
             }
 
             user.Credentials.Add(credential);
-
-            await Auditing.SaveAuditRecordAsync(new UserAuditRecord(
-                user, AuditedUserAction.AddCredential, credential));
         }
 
         private static CredentialKind GetCredentialKind(string type)
@@ -1024,15 +1032,20 @@ private async Task MigrateCredentials(User user, List<Credential> creds, string
             await Auditing.SaveAuditRecordAsync(new UserAuditRecord(user, AuditedUserAction.RemoveCredential, toRemove));
 
             // Now add one if there are no credentials left
+            Credential newCred = null;
             if (creds.Count == 0)
             {
-                var newCred = _credentialBuilder.CreatePasswordCredential(password);
-                await Auditing.SaveAuditRecordAsync(new UserAuditRecord(user, AuditedUserAction.AddCredential, newCred));
+                newCred = _credentialBuilder.CreatePasswordCredential(password);
                 user.Credentials.Add(newCred);
             }
 
             // Save changes, if any
             await Entities.SaveChangesAsync();
+
+            if (newCred != null)
+            {
+                await Auditing.SaveAuditRecordAsync(new UserAuditRecord(user, AuditedUserAction.AddCredential, newCred));
+            }
         }
     }
 }

tests/NuGetGallery.Facts/Authentication/AuthenticationServiceFacts.cs

@@ -15,6 +15,8 @@
 using NuGetGallery.Auditing;
 using NuGetGallery.Authentication.Providers;
 using NuGetGallery.Authentication.Providers.MicrosoftAccount;
+using NuGetGallery.Configuration;
+using NuGetGallery.Diagnostics;
 using NuGetGallery.Framework;
 using NuGetGallery.Infrastructure.Authentication;
 using Xunit;
@@ -2071,6 +2073,51 @@ public async Task WritesAuditRecordForTheNewCredential()
                     ar.AffectedCredential[0].Type == cred.Type &&
                     ar.AffectedCredential[0].Identity == cred.Identity));
             }
+
+            [Fact]
+            public async Task WritesAuditRecordAfterDbCommit()
+            {
+                // Arrange
+                var entitiesContext = new Mock<IEntitiesContext>();
+                var auditingService = new Mock<IAuditingService>();
+                var credentialBuilder = new CredentialBuilder();
+
+                var authService = new AuthenticationService(
+                    entitiesContext.Object,
+                    Get<IAppConfiguration>(),
+                    Get<IDiagnosticsService>(),
+                    auditingService.Object,
+                    Enumerable.Empty<Authenticator>(),
+                    credentialBuilder,
+                    Get<ICredentialValidator>(),
+                    Get<IDateTimeProvider>(),
+                    Get<ITelemetryService>(),
+                    Get<IContentObjectService>(),
+                    Get<IFeatureFlagService>());
+                var operations = new List<string>();
+
+                var fakes = Get<Fakes>();
+                var user = fakes.CreateUser("test", credentialBuilder.CreatePasswordCredential(Fakes.Password));
+                var cred = credentialBuilder.CreateExternalCredential("flarg", "glarb", "blarb");
+                Mock
+                    .Get(authService.Auditing)
+                    .Setup(x => x.SaveAuditRecordAsync(It.IsAny<AuditRecord>()))
+                    .Returns(Task.CompletedTask)
+                    .Callback(() => operations.Add(nameof(IAuditingService.SaveAuditRecordAsync)));
+                Mock
+                    .Get(authService.Entities)
+                    .Setup(x => x.SaveChangesAsync())
+                    .ReturnsAsync(() => 0)
+                    .Callback(() => operations.Add(nameof(IEntitiesContext.SaveChangesAsync)));
+
+                // Act
+                await authService.AddCredential(user, cred);
+
+                // Assert
+                Assert.Equal(
+                    new List<string> { nameof(IEntitiesContext.SaveChangesAsync), nameof(IAuditingService.SaveAuditRecordAsync) },
+                    operations);
+            }
         }
 
         public class TheDescribeCredentialMethod : TestContainer