Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Merged
merged 3 commits into from
Dec 12, 2022
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3f894ba
Audit the AddCredential action after we commit to the DB
joelverhagen Dec 9, 2022
4999456
Fixed a line
joelverhagen Dec 9, 2022
1afb42a
Remove using
joelverhagen Dec 9, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 19 additions & 6 deletions src/NuGetGallery.Services/Authentication/AuthenticationService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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));
}
}
}
}
47 changes: 47 additions & 0 deletions tests/NuGetGallery.Facts/Authentication/AuthenticationServiceFacts.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down