Telemetry for temp keys

src/NuGetGallery/Constants.cs

@@ -52,6 +52,7 @@ public static class Constants
         public const string UrlValidationErrorMessage = "This doesn't appear to be a valid HTTP/HTTPS URL";
 
         internal const string ApiKeyHeaderName = "X-NuGet-ApiKey";
+        internal const string ClientVersionHeaderName = "X-NuGet-Client-Version";
         internal const string WarningHeaderName = "X-NuGet-Warning";
 
         public static readonly string ReturnUrlParameterName = "ReturnUrl";

src/NuGetGallery/Controllers/ApiController.cs

@@ -219,7 +219,10 @@ public async virtual Task<ActionResult> CreatePackageVerificationKeyAsync(string
             // validation until the VerifyPackageKey call.
             var credential = CredentialBuilder.CreatePackageVerificationApiKey(id);
 
-            await AuthenticationService.AddCredential(GetCurrentUser(), credential);
+            var user = GetCurrentUser();
+            await AuthenticationService.AddCredential(user, credential);
+
+            TelemetryService.TrackCreatePackageVerificationKeyEvent(id, version, user, User.Identity);
 
             return Json(new
             {
@@ -245,11 +248,13 @@ public async virtual Task<ActionResult> VerifyPackageKeyAsync(string id, string
             {
                 await AuthenticationService.RemoveCredential(user, credential);
             }
+
+            TelemetryService.TrackVerifyPackageKeyEvent(id, version, user, User.Identity, result?.StatusCode ?? 200);
 
-            return result;
+            return (ActionResult)result ?? new EmptyResult();
         }
 
-        private ActionResult VerifyPackageKeyInternal(User user, Credential credential, string id, string version)
+        private HttpStatusCodeWithBodyResult VerifyPackageKeyInternal(User user, Credential credential, string id, string version)
         {
             // Verify that the user has permission to push for the specific Id \ version combination.
             var package = PackageService.FindPackageByIdAndVersion(id, version);
@@ -281,7 +286,7 @@ private ActionResult VerifyPackageKeyInternal(User user, Credential credential,
                 }
             }
 
-            return new EmptyResult();
+            return null;
         }
 
         [HttpPut]
@@ -483,7 +488,7 @@ await AuditingService.SaveAuditRecordAsync(
                             Url.Action("ReportMyPackage", "Packages", routeValues: new { id = package.PackageRegistration.Id, version = package.Version }, protocol: Request.Url.Scheme),
                             Url.Action("Account", "Users", routeValues: null, protocol: Request.Url.Scheme));
 
-                        TelemetryService.TrackPackagePushEvent(user, User.Identity);
+                        TelemetryService.TrackPackagePushEvent(package, user, User.Identity);
 
                         return new HttpStatusCodeResult(HttpStatusCode.Created);
                     }

src/NuGetGallery/Controllers/PackagesController.cs

@@ -1224,7 +1224,7 @@ await _auditingService.SaveAuditRecordAsync(
             // delete the uploaded binary in the Uploads container
             await _uploadFileService.DeleteUploadFileAsync(currentUser.Key);
 
-            _telemetryService.TrackPackagePushEvent(currentUser, User.Identity);
+            _telemetryService.TrackPackagePushEvent(package, currentUser, User.Identity);
 
             TempData["Message"] = String.Format(
                 CultureInfo.CurrentCulture, Strings.SuccessfullyUploadedPackage, package.PackageRegistration.Id, package.Version);

src/NuGetGallery/ExtensionMethods.cs

@@ -389,20 +389,28 @@ public static string GetAuthenticationType(this IIdentity self)
             return identity?.GetClaimOrDefault(ClaimTypes.AuthenticationMethod);
         }
 
-        public static bool IsScopedAuthentication(this IIdentity self)
+        private static string GetScopeClaim(this IIdentity self)
         {
             var identity = self as ClaimsIdentity;
 
-            if (identity == null)
-            {
-                return false;
-            }
+            return identity?.GetClaimOrDefault(NuGetClaims.Scope);
+        }
 
-            var scopeClaim = identity.GetClaimOrDefault(NuGetClaims.Scope);
+        public static bool IsScopedAuthentication(this IIdentity self)
+        {
+            var scopeClaim = self.GetScopeClaim();
 
             return !ScopeEvaluator.IsEmptyScopeClaim(scopeClaim);
         }
 
+        public static bool HasVerifyScope(this IIdentity self)
+        {
+            var scopeClaim = self.GetScopeClaim();
+
+            return ScopeEvaluator.ScopeClaimsAllowsActionForSubject(scopeClaim, null,
+                new [] { NuGetScopes.PackageVerify });
+        }
+
         // This is a method because the first call will perform a database call
         /// <summary>
         /// Get the current user, from the database, or if someone in this request has already

src/NuGetGallery/Services/ITelemetryService.cs

@@ -9,6 +9,10 @@ public interface ITelemetryService
     {
         void TrackODataQueryFilterEvent(string callContext, bool isEnabled, bool isAllowed, string queryPattern);
 
-        void TrackPackagePushEvent(User user, IIdentity identity);
+        void TrackPackagePushEvent(Package package, User user, IIdentity identity);
+
+        void TrackCreatePackageVerificationKeyEvent(string packageId, string packageVersion, User user, IIdentity identity);
+
+        void TrackVerifyPackageKeyEvent(string packageId, string packageVersion, User user, IIdentity identity, int statusCode);
     }
 }

src/NuGetGallery/Services/TelemetryService.cs

@@ -4,38 +4,76 @@
 using System;
 using System.Collections.Generic;
 using System.Security.Principal;
+using System.Web;
 
 namespace NuGetGallery
 {
     public class TelemetryService : ITelemetryService
     {
-        // ODataQueryFilter
-        public const string ODataQueryFilter = "ODataQueryFilter";
+        // Event types
+        public const string ODataQueryFilterEvent = "ODataQueryFilter";
+        public const string PackagePushEvent = "PackagePush";
+        public const string CreatePackageVerificationKeyEvent = "CreatePackageVerificationKeyEvent";
+        public const string VerifyPackageKeyEvent = "VerifyPackageKeyEvent";
+
+        // ODataQueryFilter properties
         public const string CallContext = "CallContext";
         public const string IsEnabled = "IsEnabled";
         public const string IsAllowed = "IsAllowed";
         public const string QueryPattern = "QueryPattern";
 
-        // Package push
-        public const string PackagePush = "PackagePush";
-        public const string AuthenticatinMethod = "AuthenticationMethod";
+        // Package push properties
+        public const string AuthenticationMethod = "AuthenticationMethod";
         public const string AccountCreationDate = "AccountCreationDate";
+        public const string ClientVersion = "ClientVersion";
         public const string IsScoped = "IsScoped";
+        public const string PackageId = "PackageId";
+        public const string PackageVersion = "PackageVersion";
+
+        // Verify package properties
+        public const string HasVerifyScope = "HasVerifyScope";
+        public const string VerifyPackageKeyStatusCode = "VerifyPackageKeyStatusCode";
 
         public void TrackODataQueryFilterEvent(string callContext, bool isEnabled, bool isAllowed, string queryPattern)
         {
-            var telemetryProperties = new Dictionary<string, string>();
+            TrackEvent(ODataQueryFilterEvent, properties =>
+            {
+                properties.Add(CallContext, callContext);
+                properties.Add(IsEnabled, $"{isEnabled}");
 
-            telemetryProperties.Add(CallContext, callContext);
-            telemetryProperties.Add(IsEnabled, $"{isEnabled}");
+                properties.Add(IsAllowed, $"{isAllowed}");
+                properties.Add(QueryPattern, queryPattern);
+            });
+        }
 
-            telemetryProperties.Add(IsAllowed, $"{isAllowed}");
-            telemetryProperties.Add(QueryPattern, queryPattern);
+        public void TrackPackagePushEvent(Package package, User user, IIdentity identity)
+        {
+            if (package == null)
+            {
+                throw new ArgumentNullException(nameof(package));
+            }
 
-            Telemetry.TrackEvent(ODataQueryFilter, telemetryProperties, metrics: null);
+            if (user == null)
+            {
+                throw new ArgumentNullException(nameof(user));
+            }
+
+            if (identity == null)
+            {
+                throw new ArgumentNullException(nameof(identity));
+            }
+
+            TrackEvent(PackagePushEvent, properties => {
+                properties.Add(ClientVersion, GetClientVersion());
+                properties.Add(PackageId, package.PackageRegistration.Id);
+                properties.Add(PackageVersion, package.Version);
+                properties.Add(AuthenticationMethod, identity.GetAuthenticationType());
+                properties.Add(AccountCreationDate, GetAccountCreationDate(user));
+                properties.Add(IsScoped, identity.IsScopedAuthentication().ToString());
+            });
         }
 
-        public void TrackPackagePushEvent(User user, IIdentity identity)
+        public void TrackCreatePackageVerificationKeyEvent(string packageId, string packageVersion, User user, IIdentity identity)
         {
             if (user == null)
             {
@@ -47,15 +85,53 @@ public void TrackPackagePushEvent(User user, IIdentity identity)
                 throw new ArgumentNullException(nameof(identity));
             }
 
-            string authenticationMethod = identity.GetAuthenticationType();
-            bool isScoped = identity.IsScopedAuthentication();
+            TrackEvent(CreatePackageVerificationKeyEvent, properties => {
+                properties.Add(ClientVersion, GetClientVersion());
+                properties.Add(PackageId, packageId);
+                properties.Add(PackageVersion, packageVersion);
+                properties.Add(AccountCreationDate, GetAccountCreationDate(user));
+                properties.Add(IsScoped, identity.IsScopedAuthentication().ToString());
+            });
+        }
 
+        public void TrackVerifyPackageKeyEvent(string packageId, string packageVersion, User user, IIdentity identity, int statusCode)
+        {
+            if (user == null)
+            {
+                throw new ArgumentNullException(nameof(user));
+            }
+
+            if (identity == null)
+            {
+                throw new ArgumentNullException(nameof(identity));
+            }
+
+            TrackEvent(VerifyPackageKeyEvent, properties =>
+            {
+                properties.Add(PackageId, packageId);
+                properties.Add(PackageVersion, packageVersion);
+                properties.Add(HasVerifyScope, identity.HasVerifyScope().ToString());
+                properties.Add(VerifyPackageKeyStatusCode, statusCode.ToString());
+            });
+        }
+
+        private static string GetClientVersion()
+        {
+            return HttpContext.Current?.Request?.Headers[Constants.ClientVersionHeaderName];
+        }
+
+        private static string GetAccountCreationDate(User user)
+        {
+            return user.CreatedUtc != null ? user.CreatedUtc.Value.ToString("d") : "N/A";
+        }
+
+        private static void TrackEvent(string eventName, Action<Dictionary<string, string>> addProperties)
+        {
             var telemetryProperties = new Dictionary<string, string>();
-            telemetryProperties.Add(AuthenticatinMethod, authenticationMethod);
-            telemetryProperties.Add(AccountCreationDate, user.CreatedUtc != null ? user.CreatedUtc.Value.ToString("d") : "N/A");
-            telemetryProperties.Add(IsScoped, isScoped.ToString());
 
-            Telemetry.TrackEvent(PackagePush, telemetryProperties, metrics: null);
+            addProperties(telemetryProperties);
+
+            Telemetry.TrackEvent(eventName, telemetryProperties, metrics: null);
         }
     }
 }

tests/NuGetGallery.Facts/Controllers/ApiControllerFacts.cs

@@ -631,7 +631,7 @@ public async Task WillSendPackagePushEvent()
 
                 await controller.CreatePackagePut();
 
-                controller.MockTelemetryService.Verify(x => x.TrackPackagePushEvent(user, controller.OwinContext.Request.User.Identity), Times.Once);
+                controller.MockTelemetryService.Verify(x => x.TrackPackagePushEvent(It.IsAny<Package>(), user, controller.OwinContext.Request.User.Identity), Times.Once);
             }
         }
 
@@ -1059,6 +1059,9 @@ public async void CreatePackageKeyReturnsPackageVerificationKey(string scope)
                 Assert.True(DateTime.TryParse(json.Expires, out expires));
 
                 controller.MockAuthenticationService.Verify(s => s.AddCredential(It.IsAny<User>(), It.IsAny<Credential>()), Times.Once);
+
+                controller.MockTelemetryService.Verify(x => x.TrackCreatePackageVerificationKeyEvent("foo", "1.0.0",
+                    It.IsAny<User>(), controller.OwinContext.Request.User.Identity), Times.Once);
             }
         }
 
@@ -1083,6 +1086,9 @@ public async void VerifyPackageKeyReturns404IfPackageDoesNotExist_ApiKeyV2(strin
                     String.Format(CultureInfo.CurrentCulture, Strings.PackageWithIdAndVersionNotFound, "foo", "1.0.0"));
 
                 controller.MockAuthenticationService.Verify(s => s.RemoveCredential(It.IsAny<User>(), It.IsAny<Credential>()), Times.Never);
+
+                controller.MockTelemetryService.Verify(x => x.TrackVerifyPackageKeyEvent("foo", "1.0.0",
+                    It.IsAny<User>(), controller.OwinContext.Request.User.Identity, 404), Times.Once);
             }
 
             [Theory]
@@ -1102,6 +1108,9 @@ public async void VerifyPackageKeyReturns404IfPackageDoesNotExist_ApiKeyVerifyV1
                     String.Format(CultureInfo.CurrentCulture, Strings.PackageWithIdAndVersionNotFound, "foo", "1.0.0"));
 
                 controller.MockAuthenticationService.Verify(s => s.RemoveCredential(It.IsAny<User>(), It.IsAny<Credential>()), Times.Once);
+
+                controller.MockTelemetryService.Verify(x => x.TrackVerifyPackageKeyEvent("foo", "1.0.0",
+                    It.IsAny<User>(), controller.OwinContext.Request.User.Identity, 404), Times.Once);
             }
 
             [Theory]
@@ -1128,6 +1137,9 @@ public async void VerifyPackageKeyReturns403IfUserIsNotAnOwner_ApiKeyV2(string s
                     Strings.ApiKeyNotAuthorized);
 
                 controller.MockAuthenticationService.Verify(s => s.RemoveCredential(It.IsAny<User>(), It.IsAny<Credential>()), Times.Never);
+
+                controller.MockTelemetryService.Verify(x => x.TrackVerifyPackageKeyEvent("foo", "1.0.0",
+                    It.IsAny<User>(), controller.OwinContext.Request.User.Identity, 403), Times.Once);
             }
 
             [Theory]
@@ -1152,6 +1164,9 @@ public async void VerifyPackageKeyReturns403IfUserIsNotAnOwner_ApiKeyVerifyV1(st
                     Strings.ApiKeyNotAuthorized);
 
                 controller.MockAuthenticationService.Verify(s => s.RemoveCredential(It.IsAny<User>(), It.IsAny<Credential>()), Times.Once);
+
+                controller.MockTelemetryService.Verify(x => x.TrackVerifyPackageKeyEvent("foo", "1.0.0",
+                    It.IsAny<User>(), controller.OwinContext.Request.User.Identity, 403), Times.Once);
             }
 
             [Theory]
@@ -1181,6 +1196,9 @@ public async void VerifyPackageKeyReturns403IfScopeDoesNotMatch_ApiKeyV2(string
                     Strings.ApiKeyNotAuthorized);
 
                 controller.MockAuthenticationService.Verify(s => s.RemoveCredential(It.IsAny<User>(), It.IsAny<Credential>()), Times.Never);
+
+                controller.MockTelemetryService.Verify(x => x.TrackVerifyPackageKeyEvent("foo", "1.0.0",
+                    It.IsAny<User>(), controller.OwinContext.Request.User.Identity, 403), Times.Once);
             }
 
             [Theory]
@@ -1210,6 +1228,9 @@ public async void VerifyPackageKeyReturns403IfScopeDoesNotMatch_ApiKeyVerifyV1(s
                     Strings.ApiKeyNotAuthorized);
 
                 controller.MockAuthenticationService.Verify(s => s.RemoveCredential(It.IsAny<User>(), It.IsAny<Credential>()), Times.Once);
+
+                controller.MockTelemetryService.Verify(x => x.TrackVerifyPackageKeyEvent("foo", "1.0.0",
+                    It.IsAny<User>(), controller.OwinContext.Request.User.Identity, 403), Times.Once);
             }
 
             [Theory]
@@ -1233,6 +1254,9 @@ public async void VerifyPackageKeyReturns200_ApiKeyV2(string scope)
                 ResultAssert.IsEmpty(result);
 
                 controller.MockAuthenticationService.Verify(s => s.RemoveCredential(It.IsAny<User>(), It.IsAny<Credential>()), Times.Never);
+
+                controller.MockTelemetryService.Verify(x => x.TrackVerifyPackageKeyEvent("foo", "1.0.0",
+                    It.IsAny<User>(), controller.OwinContext.Request.User.Identity, 200), Times.Once);
             }
 
             [Theory]
@@ -1254,6 +1278,9 @@ public async void VerifyPackageKeyReturns200_ApiKeyVerifyV1(string scope)
                 ResultAssert.IsEmpty(result);
 
                 controller.MockAuthenticationService.Verify(s => s.RemoveCredential(It.IsAny<User>(), It.IsAny<Credential>()), Times.Once);
+
+                controller.MockTelemetryService.Verify(x => x.TrackVerifyPackageKeyEvent("foo", "1.0.0",
+                    It.IsAny<User>(), controller.OwinContext.Request.User.Identity, 200), Times.Once);
             }
         }
 

tests/NuGetGallery.Facts/Controllers/PackagesControllerFacts.cs

@@ -1879,7 +1879,7 @@ public async Task WillSendPackagePublishedEvent()
                     await controller.VerifyPackage(new VerifyPackageRequest { Listed = true, Edit = null });
 
                     // Assert
-                    fakeTelemetryService.Verify(x => x.TrackPackagePushEvent(TestUtility.FakeUser, controller.OwinContext.Request.User.Identity), Times.Once);
+                    fakeTelemetryService.Verify(x => x.TrackPackagePushEvent(It.IsAny<Package>(), TestUtility.FakeUser, controller.OwinContext.Request.User.Identity), Times.Once);
                 }
             }
         }