Task NuGet/Engineering#724 (#4856)

src/NuGetGallery.Core/Auditing/AuditActor.cs

@@ -7,8 +7,10 @@
 using System.Net;
 using System.Net.NetworkInformation;
 using System.Net.Sockets;
+using System.Security.Claims;
 using System.Threading.Tasks;
 using System.Web;
+using NuGetGallery.Authentication;
 
 namespace NuGetGallery.Auditing
 {
@@ -18,21 +20,23 @@ public class AuditActor
         public string MachineIP { get; set; }
         public string UserName { get; set; }
         public string AuthenticationType { get; set; }
+        public string CredentialKey { get; set; }
         public DateTime TimestampUtc { get; set; }
 
         public AuditActor OnBehalfOf { get; set; }
 
-        public AuditActor(string machineName, string machineIP, string userName, string authenticationType, DateTime timeStampUtc)
-            : this(machineName, machineIP, userName, authenticationType, timeStampUtc, null) { }
+        public AuditActor(string machineName, string machineIP, string userName, string authenticationType, string credentialKey, DateTime timeStampUtc)
+            : this(machineName, machineIP, userName, authenticationType, credentialKey, timeStampUtc, null) { }
 
-        public AuditActor(string machineName, string machineIP, string userName, string authenticationType, DateTime timeStampUtc, AuditActor onBehalfOf)
+        public AuditActor(string machineName, string machineIP, string userName, string authenticationType, string credentialKey, DateTime timeStampUtc, AuditActor onBehalfOf)
         {
             MachineName = machineName;
             MachineIP = machineIP;
             UserName = userName;
             AuthenticationType = authenticationType;
             TimestampUtc = timeStampUtc;
             OnBehalfOf = onBehalfOf;
+            CredentialKey = credentialKey;
         }
 
         public static Task<AuditActor> GetAspNetOnBehalfOfAsync()
@@ -68,17 +72,23 @@ public static Task<AuditActor> GetAspNetOnBehalfOfAsync(HttpContextBase context)
 
             string user = null;
             string authType = null;
+            string credentialKey = null;
+
             if (context.User != null)
             {
                 user = context.User.Identity.Name;
                 authType = context.User.Identity.AuthenticationType;
+
+                var claimsIdentity = context.User.Identity as ClaimsIdentity;
+                credentialKey = claimsIdentity?.GetClaimOrDefault(NuGetClaims.CredentialKey);
             }
 
             return Task.FromResult(new AuditActor(
                 null,
                 clientIpAddress,
                 user,
                 authType,
+                credentialKey,
                 DateTime.UtcNow));
         }
 
@@ -97,6 +107,7 @@ public static async Task<AuditActor> GetCurrentMachineActorAsync(AuditActor onBe
                 ipAddress,
                 $@"{Environment.UserDomainName}\{Environment.UserName}",
                 "MachineUser",
+                string.Empty,
                 DateTime.UtcNow,
                 onBehalfOf);
         }

src/NuGetGallery.Core/Authentication/AuthenticationExtensions.cs

@@ -0,0 +1,26 @@
+// Copyright (c) .NET Foundation. All rights reserved.
+// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
+
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Security.Claims;
+
+namespace NuGetGallery.Authentication
+{
+    public static class AuthenticationExtensions
+    {
+        public static string GetClaimOrDefault(this ClaimsIdentity self, string claimType)
+        {
+            return self.Claims.GetClaimOrDefault(claimType);
+        }
+
+        public static string GetClaimOrDefault(this IEnumerable<Claim> self, string claimType)
+        {
+            return self
+                .Where(c => string.Equals(c.Type, claimType, StringComparison.OrdinalIgnoreCase))
+                .Select(c => c.Value)
+                .FirstOrDefault();
+        }
+    }
+}

src/NuGetGallery/Authentication/NuGetClaims.cs → src/NuGetGallery.Core/Authentication/NuGetClaims.cs

@@ -10,5 +10,8 @@ public static class NuGetClaims
         public const string ApiKey = "https://claims.nuget.org/apikey";
 
         public const string Scope = "https://claims.nuget.org/scope";
+
+        // Allows identifying the credential that was used by his DB key.
+        public const string CredentialKey = "https://claims.nuget.org/credentialkey";
     }
-}
+}

src/NuGetGallery.Core/NuGetGallery.Core.csproj

@@ -143,6 +143,7 @@
     <Compile Include="Auditing\PackageAuditRecord.cs" />
     <Compile Include="Auditing\ScopeAuditRecord.cs" />
     <Compile Include="Auditing\UserAuditRecord.cs" />
+    <Compile Include="Authentication\AuthenticationExtensions.cs" />
     <Compile Include="Cookies\CookieComplianceServiceBase.cs" />
     <Compile Include="Cookies\CookieConsentMessage.cs" />
     <Compile Include="Cookies\ICookieComplianceService.cs" />
@@ -191,6 +192,7 @@
     <Compile Include="ICloudStorageStatusDependency.cs" />
     <Compile Include="Infrastructure\AzureEntityList.cs" />
     <Compile Include="Infrastructure\TableErrorLog.cs" />
+    <Compile Include="Authentication\NuGetClaims.cs" />
     <Compile Include="PackageReaderCoreExtensions.cs" />
     <Compile Include="PackageStatus.cs" />
     <Compile Include="Packaging\InvalidPackageException.cs" />

src/NuGetGallery/Authentication/Providers/ApiKey/ApiKeyAuthenticationHandler.cs

@@ -28,24 +28,9 @@ internal ApiKeyAuthenticationHandler() { }
 
         public ApiKeyAuthenticationHandler(ILogger logger, AuthenticationService auth, ICredentialBuilder credentialBuilder)
         {
-            if (logger == null)
-            {
-                throw new ArgumentNullException(nameof(logger));
-            }
-
-            if (auth == null)
-            {
-                throw new ArgumentNullException(nameof(auth));
-            }
-
-            if (credentialBuilder == null)
-            {
-                throw new ArgumentNullException(nameof(credentialBuilder));
-            }
-
-            Logger = logger;
-            Auth = auth;
-            CredentialBuilder = credentialBuilder;
+            Logger = logger ?? throw new ArgumentNullException(nameof(logger));
+            Auth = auth ?? throw new ArgumentNullException(nameof(auth));
+            CredentialBuilder = credentialBuilder ?? throw new ArgumentNullException(nameof(credentialBuilder));
         }
 
         internal Task InitializeAsync(ApiKeyAuthenticationOptions options, IOwinContext context)
@@ -109,7 +94,8 @@ protected override async Task<AuthenticationTicket> AuthenticateCoreAsync()
                                 authUser.User, 
                                 AuthenticationTypes.ApiKey, 
                                 new Claim(NuGetClaims.ApiKey, apiKey),
-                                new Claim(NuGetClaims.Scope, scopes)),
+                                new Claim(NuGetClaims.Scope, scopes),
+                                new Claim(NuGetClaims.CredentialKey, authUser.CredentialUsed.Key.ToString())),
                             new AuthenticationProperties());
                 }
                 else

src/NuGetGallery/ExtensionMethods.cs

@@ -464,19 +464,6 @@ public static string GetClaimOrDefault(this ClaimsPrincipal self, string claimTy
             return self.Claims.GetClaimOrDefault(claimType);
         }
 
-        public static string GetClaimOrDefault(this ClaimsIdentity self, string claimType)
-        {
-            return self.Claims.GetClaimOrDefault(claimType);
-        }
-
-        public static string GetClaimOrDefault(this IEnumerable<Claim> self, string claimType)
-        {
-            return self
-                .Where(c => string.Equals(c.Type, claimType, StringComparison.OrdinalIgnoreCase))
-                .Select(c => c.Value)
-                .FirstOrDefault();
-        }
-
         public static bool HasScopeThatAllowsActionForSubject(
             this IIdentity self, 
             string subject,

src/NuGetGallery/NuGetGallery.csproj

@@ -749,7 +749,6 @@
     <Compile Include="Authentication\AuthenticationService.cs" />
     <Compile Include="Authentication\AuthenticationTypes.cs" />
     <Compile Include="Authentication\AuthDependenciesModule.cs" />
-    <Compile Include="Authentication\NuGetClaims.cs" />
     <Compile Include="Authentication\Providers\ApiKey\ApiKeyAuthenticator.cs" />
     <Compile Include="Authentication\Providers\ApiKey\ApiKeyAuthenticatorConfiguration.cs" />
     <Compile Include="Authentication\Providers\Authenticator.cs" />

tests/NuGetGallery.Core.Facts/Auditing/AuditActorTests.cs

@@ -8,10 +8,12 @@
 using System.Net;
 using System.Net.NetworkInformation;
 using System.Net.Sockets;
+using System.Security.Claims;
 using System.Security.Principal;
 using System.Threading.Tasks;
 using System.Web;
 using Moq;
+using NuGetGallery.Authentication;
 using Xunit;
 
 namespace NuGetGallery.Auditing
@@ -26,12 +28,14 @@ public void Constructor_WithoutOnBehalfOf_AcceptsNullValues()
                 machineIP: null,
                 userName: null,
                 authenticationType: null,
+                credentialKey: null,
                 timeStampUtc: DateTime.MinValue);
 
             Assert.Null(actor.MachineName);
             Assert.Null(actor.MachineIP);
             Assert.Null(actor.UserName);
             Assert.Null(actor.AuthenticationType);
+            Assert.Null(actor.CredentialKey);
         }
 
         [Fact]
@@ -41,6 +45,7 @@ public void Constructor_WithOnBehalfOf_AcceptsNullValues()
                 machineIP: null,
                 userName: null,
                 authenticationType: null,
+                credentialKey: null,
                 timeStampUtc: DateTime.MinValue,
                 onBehalfOf: null);
 
@@ -49,39 +54,42 @@ public void Constructor_WithOnBehalfOf_AcceptsNullValues()
             Assert.Null(actor.UserName);
             Assert.Null(actor.AuthenticationType);
             Assert.Null(actor.OnBehalfOf);
+            Assert.Null(actor.CredentialKey);
         }
 
         [Fact]
         public void Constructor_WithoutOnBehalfOf_AcceptsEmptyStringValues()
         {
             var actor = new AuditActor(
-                machineName: "",
-                machineIP: "",
-                userName: "",
-                authenticationType: "",
+                machineName: string.Empty,
+                machineIP: string.Empty,
+                userName: string.Empty,
+                authenticationType: string.Empty,
+                credentialKey: string.Empty,
                 timeStampUtc: DateTime.MinValue);
 
-            Assert.Equal("", actor.MachineName);
-            Assert.Equal("", actor.MachineIP);
-            Assert.Equal("", actor.UserName);
-            Assert.Equal("", actor.AuthenticationType);
+            Assert.Equal(string.Empty, actor.MachineName);
+            Assert.Equal(string.Empty, actor.MachineIP);
+            Assert.Equal(string.Empty, actor.UserName);
+            Assert.Equal(string.Empty, actor.AuthenticationType);
         }
 
         [Fact]
         public void Constructor_WithOnBehalfOf_AcceptsEmptyStringValues()
         {
             var actor = new AuditActor(
-                machineName: "",
-                machineIP: "",
-                userName: "",
-                authenticationType: "",
+                machineName: string.Empty,
+                machineIP: string.Empty,
+                userName: string.Empty,
+                authenticationType: string.Empty,
+                credentialKey: string.Empty,
                 timeStampUtc: DateTime.MinValue,
                 onBehalfOf: null);
 
-            Assert.Equal("", actor.MachineName);
-            Assert.Equal("", actor.MachineIP);
-            Assert.Equal("", actor.UserName);
-            Assert.Equal("", actor.AuthenticationType);
+            Assert.Equal(string.Empty, actor.MachineName);
+            Assert.Equal(string.Empty, actor.MachineIP);
+            Assert.Equal(string.Empty, actor.UserName);
+            Assert.Equal(string.Empty, actor.AuthenticationType);
         }
 
         [Fact]
@@ -92,12 +100,14 @@ public void Constructor_WithoutOnBehalfOf_SetsProperties()
                 machineIP: "b",
                 userName: "c",
                 authenticationType: "d",
+                credentialKey: "e",
                 timeStampUtc: DateTime.MinValue);
 
             Assert.Equal("a", actor.MachineName);
             Assert.Equal("b", actor.MachineIP);
             Assert.Equal("c", actor.UserName);
             Assert.Equal("d", actor.AuthenticationType);
+            Assert.Equal("e", actor.CredentialKey);
             Assert.Equal(DateTime.MinValue, actor.TimestampUtc);
         }
 
@@ -109,19 +119,22 @@ public void Constructor_WithOnBehalfOf_SetsProperties()
                 machineIP: null,
                 userName: null,
                 authenticationType: null,
+                credentialKey: null,
                 timeStampUtc: DateTime.MinValue);
             var actor = new AuditActor(
                 machineName: "a",
                 machineIP: "b",
                 userName: "c",
                 authenticationType: "d",
+                credentialKey: "e",
                 timeStampUtc: DateTime.MinValue,
                 onBehalfOf: onBehalfOfActor);
 
             Assert.Equal("a", actor.MachineName);
             Assert.Equal("b", actor.MachineIP);
             Assert.Equal("c", actor.UserName);
             Assert.Equal("d", actor.AuthenticationType);
+            Assert.Equal("e", actor.CredentialKey);
             Assert.Equal(DateTime.MinValue, actor.TimestampUtc);
             Assert.Same(onBehalfOfActor, actor.OnBehalfOf);
         }
@@ -288,6 +301,41 @@ public async Task GetAspNetOnBehalfOfAsync_WithContext_SupportsNullUser()
             Assert.Null(actor.UserName);
         }
 
+        [Fact]
+        public async Task GetAspNetOnBehalfOfAsync_WithContext_ResturnActorWithCredentialKey()
+        {
+            var request = new Mock<HttpRequestBase>();
+            var identity = new Mock<IIdentity>();
+            var user = new Mock<IPrincipal>();
+            var context = new Mock<HttpContextBase>();
+
+            request.SetupGet(x => x.ServerVariables)
+                .Returns(new NameValueCollection() { { "HTTP_X_FORWARDED_FOR", "1.2.3.4" } });
+            identity.Setup(x => x.Name)
+                .Returns("b");
+            identity.Setup(x => x.AuthenticationType)
+                .Returns("c");
+
+            var cliamsIdentity = new ClaimsIdentity(identity.Object, new List<Claim> { new Claim(NuGetClaims.CredentialKey, "99") });
+            user.Setup(x => x.Identity)
+                .Returns(cliamsIdentity);
+            context.Setup(x => x.Request)
+                .Returns(request.Object);
+            context.Setup(x => x.User)
+                .Returns(user.Object);
+
+            var actor = await AuditActor.GetAspNetOnBehalfOfAsync(context.Object);
+
+            Assert.NotNull(actor);
+            Assert.Equal("c", actor.AuthenticationType);
+            Assert.Equal("99", actor.CredentialKey);
+            Assert.Equal("1.2.3.0", actor.MachineIP);
+            Assert.Null(actor.MachineName);
+            Assert.Null(actor.OnBehalfOf);
+            Assert.InRange(actor.TimestampUtc, DateTime.UtcNow.AddMinutes(-1), DateTime.UtcNow.AddMinutes(1));
+            Assert.Equal("b", actor.UserName);
+        }
+
         [Fact]
         public async Task GetCurrentMachineActorAsync_WithoutOnBehalfOf()
         {

tests/NuGetGallery.Core.Facts/Auditing/AuditEntryTests.cs

@@ -27,6 +27,7 @@ public void Constructor_SetsProperties()
                 machineIP: null,
                 userName: null,
                 authenticationType: null,
+                credentialKey: null,
                 timeStampUtc: DateTime.MinValue);
             var entry = new AuditEntry(record.Object, actor);
 

tests/NuGetGallery.Core.Facts/Auditing/FileSystemAuditingServiceTests.cs

@@ -141,9 +141,10 @@ private static Task<AuditActor> GetOnBehalfOf()
                 machineIP: "b",
                 userName: "c",
                 authenticationType: "d",
+                credentialKey: "e",
                 timeStampUtc: DateTime.MinValue);
 
-            return Task.FromResult<AuditActor>(actor);
+            return Task.FromResult(actor);
         }
 
         private static bool IsValidMachineIpValue(string ipAddress)