Skip to content

Commit f2c1759

Browse files
authored
Adds User's MFA status when an API key is created (#8947)
1 parent e3c8719 commit f2c1759

7 files changed

Lines changed: 235 additions & 0 deletions

File tree

src/NuGet.Services.Entities/Credential.cs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,8 @@ public Credential(string type, string value, TimeSpan? expiration)
8484

8585
public CredentialRevocationSource? RevocationSourceKey { get; set; }
8686

87+
public bool? WasCreatedSecurely { get; set; }
88+
8789
public virtual User User { get; set; }
8890

8991
public virtual ICollection<Scope> Scopes { get; set; }

src/NuGetGallery/Controllers/UsersController.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1053,6 +1053,7 @@ private async Task<CredentialViewModel> GenerateApiKeyInternal(string descriptio
10531053
var newCredential = _credentialBuilder.CreateApiKey(expiration, out string plaintextApiKey);
10541054
newCredential.Description = description;
10551055
newCredential.Scopes = scopes;
1056+
newCredential.WasCreatedSecurely = User.WasMultiFactorAuthenticated();
10561057

10571058
await AuthenticationService.AddCredential(user, newCredential);
10581059

src/NuGetGallery/Migrations/202202010109383_AddCreatedSecurelyToCredentials.Designer.cs

Lines changed: 29 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
namespace NuGetGallery.Migrations
2+
{
3+
using System;
4+
using System.Data.Entity.Migrations;
5+
6+
public partial class AddCreatedSecurelyToCredentials : DbMigration
7+
{
8+
public override void Up()
9+
{
10+
AddColumn("dbo.Credentials", "WasCreatedSecurely", c => c.Boolean());
11+
}
12+
13+
public override void Down()
14+
{
15+
DropColumn("dbo.Credentials", "WasCreatedSecurely");
16+
}
17+
}
18+
}

src/NuGetGallery/Migrations/202202010109383_AddCreatedSecurelyToCredentials.resx

Lines changed: 126 additions & 0 deletions
Large diffs are not rendered by default.

src/NuGetGallery/NuGetGallery.csproj

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -319,6 +319,10 @@
319319
<Compile Include="Migrations\202201242159337_AddUserStatusKeyColumn.designer.cs">
320320
<DependentUpon>202201242159337_AddUserStatusKeyColumn.cs</DependentUpon>
321321
</Compile>
322+
<Compile Include="Migrations\202202010109383_AddCreatedSecurelyToCredentials.cs" />
323+
<Compile Include="Migrations\202202010109383_AddCreatedSecurelyToCredentials.designer.cs">
324+
<DependentUpon>202202010109383_AddCreatedSecurelyToCredentials.cs</DependentUpon>
325+
</Compile>
322326
<Compile Include="Modules\CookieComplianceHttpModule.cs" />
323327
<Compile Include="RequestModels\DeletePackagesApiRequest.cs" />
324328
<Compile Include="RequestModels\UpdateListedRequest.cs" />
@@ -1660,6 +1664,9 @@
16601664
<EmbeddedResource Include="Migrations\202201242159337_AddUserStatusKeyColumn.resx">
16611665
<DependentUpon>202201242159337_AddUserStatusKeyColumn.cs</DependentUpon>
16621666
</EmbeddedResource>
1667+
<EmbeddedResource Include="Migrations\202202010109383_AddCreatedSecurelyToCredentials.resx">
1668+
<DependentUpon>202202010109383_AddCreatedSecurelyToCredentials.cs</DependentUpon>
1669+
</EmbeddedResource>
16631670
<EmbeddedResource Include="OData\QueryAllowed\Data\apiv1packages.json" />
16641671
<EmbeddedResource Include="OData\QueryAllowed\Data\apiv1search.json" />
16651672
<EmbeddedResource Include="OData\QueryAllowed\Data\apiv2getupdates.json" />

tests/NuGetGallery.Facts/Controllers/UsersControllerFacts.cs

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -906,6 +906,58 @@ await controller.GenerateApiKey(
906906
Assert.NotNull(apiKey);
907907
Assert.Equal(description, apiKey.Description);
908908
Assert.Equal(expectedScopes.Length, apiKey.Scopes.Count);
909+
Assert.False(apiKey.WasCreatedSecurely);
910+
911+
foreach (var expectedScope in expectedScopes)
912+
{
913+
var actualScope =
914+
apiKey.Scopes.First(x => x.AllowedAction == expectedScope.AllowedAction &&
915+
x.Subject == expectedScope.Subject);
916+
Assert.NotNull(actualScope);
917+
}
918+
}
919+
920+
[MemberData(nameof(CreatesNewApiKeyCredential_Input))]
921+
[Theory]
922+
public async Task CreatesNewApiKeyCredentialSecurely(string description, string[] scopes, string[] subjects, Scope[] expectedScopes)
923+
{
924+
// Arrange
925+
var user = new User("the-username");
926+
GetMock<IUserService>()
927+
.Setup(u => u.FindByUsername(user.Username, false))
928+
.Returns(user);
929+
930+
GetMock<AuthenticationService>()
931+
.Setup(u => u.AddCredential(
932+
It.IsAny<User>(),
933+
It.IsAny<Credential>()))
934+
.Callback<User, Credential>((u, c) =>
935+
{
936+
u.Credentials.Add(c);
937+
c.User = u;
938+
})
939+
.Completes()
940+
.Verifiable();
941+
942+
var controller = GetController<UsersController>();
943+
controller.SetCurrentUser(user);
944+
controller.OwinContext.AddClaim(NuGetClaims.WasMultiFactorAuthenticated);
945+
946+
// Act
947+
await controller.GenerateApiKey(
948+
description: description,
949+
owner: user.Username,
950+
scopes: scopes,
951+
subjects: subjects,
952+
expirationInDays: null);
953+
954+
// Assert
955+
var apiKey = user.Credentials.FirstOrDefault(x => x.Type == CredentialTypes.ApiKey.V4);
956+
957+
Assert.NotNull(apiKey);
958+
Assert.Equal(description, apiKey.Description);
959+
Assert.Equal(expectedScopes.Length, apiKey.Scopes.Count);
960+
Assert.True(apiKey.WasCreatedSecurely);
909961

910962
foreach (var expectedScope in expectedScopes)
911963
{

0 commit comments

Comments
 (0)