Improve API key expiration warnings

src/NuGetGallery/Filters/ApiAuthorizeAttribute.cs

@@ -42,9 +42,16 @@ public override void OnAuthorization(AuthorizationContext filterContext)
                     else if (expirationPeriod.TotalDays <= controller.NuGetContext.Config.Current.WarnAboutExpirationInDaysForApiKeyV1)
                     {
                         // about to expire warning
+                        var expirationDays = Math.Round(expirationPeriod.TotalDays, 0);
+                        var expiration = expirationDays + " " + (expirationDays == 1 ? "day" : "days");
+
                         filterContext.HttpContext.Response.Headers.Add(
                             GalleryConstants.WarningHeaderName,
-                            string.Format(CultureInfo.InvariantCulture, Strings.WarningApiKeyAboutToExpire, Math.Round(expirationPeriod.TotalDays, 0), accountUrl));
+                            string.Format(
+                                CultureInfo.InvariantCulture,
+                                Strings.WarningApiKeyAboutToExpire,
+                                expiration,
+                                accountUrl));
                     }
                 }
             }

src/NuGetGallery/Strings.resx

@@ -329,7 +329,7 @@
     <value>API key can not be the default Guid.</value>
   </data>
   <data name="WarningApiKeyAboutToExpire" xml:space="preserve">
-    <value>Your API key expires in {0} days. Visit {1} to regenerate your API key.</value>
+    <value>Your API key expires in {0}. Visit {1} to regenerate your API key.</value>
   </data>
   <data name="WarningApiKeyExpired" xml:space="preserve">
     <value>Your API key has expired. Visit {0} to generate a new API key.</value>

tests/NuGetGallery.Facts/Filters/ApiAuthorizeAttributeFacts.cs

@@ -1,14 +1,18 @@
 // 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.Collections.Specialized;
 using System.Linq;
 using System.Security.Claims;
 using System.Web;
 using System.Web.Mvc;
+using System.Web.Routing;
 using Moq;
 using NuGet.Services.Entities;
 using NuGetGallery.Authentication;
+using NuGetGallery.Configuration;
 using Xunit;
 using AuthenticationTypes = NuGetGallery.Authentication.AuthenticationTypes;
 using AuthorizationContext = System.Web.Mvc.AuthorizationContext;
@@ -36,6 +40,32 @@ public void Returns401ForUnauthenticatedUser()
                 Assert.Equal(AuthenticationTypes.ApiKey, owinContext.Authentication.AuthenticationResponseChallenge.AuthenticationTypes[0]);
             }
 
+            [Theory]
+            [InlineData(-1, "Your API key has expired. Visit https://www.nuget.org/account/apikeys to generate a new API key.")]
+            [InlineData(10, "Your API key expires in 0 days. Visit https://www.nuget.org/account/apikeys to regenerate your API key.")]
+            [InlineData(30, "Your API key expires in 1 day. Visit https://www.nuget.org/account/apikeys to regenerate your API key.")]
+            [InlineData(50, "Your API key expires in 2 days. Visit https://www.nuget.org/account/apikeys to regenerate your API key.")]
+            public void AddsExpirationWarnings(int expirationHours, string expectedWarning)
+            {
+                // Create an API key. We set the expiration using the property as the constructor does not
+                // allow creating already expired keys.
+                var apiKey = TestCredentialHelper.CreateV4ApiKey(expiration: null, out _);
+                apiKey.Expires = DateTime.UtcNow.AddHours(expirationHours);
+
+                var context = BuildAuthorizationContext(authenticated: true, credential: apiKey).Object;
+                var attribute = new ApiAuthorizeAttribute();
+
+                // Act
+                attribute.OnAuthorization(context);
+
+                var owinContext = context.HttpContext.GetOwinContext();
+
+                // Assert
+                Assert.Equal(200, owinContext.Response.StatusCode);
+                Assert.True(context.HttpContext.Response.Headers.AllKeys.Contains("X-NuGet-Warning"));
+                Assert.Equal(expectedWarning, context.HttpContext.Response.Headers["X-NuGet-Warning"]);
+            }
+
             [Fact]
             public void SucceedsForAuthenticatedUser()
             {
@@ -51,18 +81,27 @@ public void SucceedsForAuthenticatedUser()
                 Assert.Equal(200, owinContext.Response.StatusCode);
             }
 
-            private Mock<AuthorizationContext> BuildAuthorizationContext(bool authenticated = true)
+            private Mock<AuthorizationContext> BuildAuthorizationContext(
+                bool authenticated = true,
+                Credential credential = null)
             {
-                var mockController = new Mock<AppController>();
-                var user = new User();
-                user.Credentials.Add(TestCredentialHelper.CreateV4ApiKey(expiration: null, plaintextApiKey: out string plaintextApiKey));
+                var configs = new Mock<IGalleryConfigurationService>();
+                configs.SetupGet(c => c.Current.RequireSSL).Returns(true);
+                configs.SetupGet(c => c.Current.WarnAboutExpirationInDaysForApiKeyV1).Returns(10);
+                configs.Setup(c => c.GetSiteRoot(true)).Returns("https://www.nuget.org");
 
-                mockController.Setup(c => c.GetCurrentUser()).Returns(user);
+                UrlHelperExtensions.SetConfigurationService(configs.Object);
+
+                credential = credential ?? TestCredentialHelper.CreateV4ApiKey(expiration: null, plaintextApiKey: out string plaintextApiKey);
+
+                var user = new User();
+                user.Credentials.Add(credential);
 
                 var mockHttpContext = new Mock<HttpContextBase>();
-                mockHttpContext.SetupGet(c => c.Items).Returns(new Dictionary<object, object> {
-                { "owin.Environment", new Dictionary<string, object>() }
-            });
+                mockHttpContext.SetupGet(c => c.Items).Returns(new Dictionary<object, object>
+                {
+                    { "owin.Environment", new Dictionary<string, object>() }
+                });
 
                 var mockIdentity = new Mock<ClaimsIdentity>();
 
@@ -71,16 +110,25 @@ private Mock<AuthorizationContext> BuildAuthorizationContext(bool authenticated
                 mockIdentity.SetupGet(i => i.AuthenticationType).Returns(AuthenticationTypes.ApiKey);
 
                 mockHttpContext.SetupGet(c => c.User.Identity).Returns(mockIdentity.Object);
-                mockHttpContext.SetupGet(c => c.Response.Cache).Returns(new Mock<HttpCachePolicyBase>().Object);
+                mockHttpContext.SetupGet(c => c.Response.Cache).Returns(Mock.Of<HttpCachePolicyBase>());
+                mockHttpContext.SetupGet(c => c.Response.Headers).Returns(new NameValueCollection());
+                mockHttpContext.SetupGet(c => c.Request.IsSecureConnection).Returns(true);
 
                 var mockActionDescriptor = new Mock<ActionDescriptor>();
                 mockActionDescriptor.Setup(c => c.ControllerDescriptor).Returns(new Mock<ControllerDescriptor>().Object);
 
+                var mockController = new Mock<AppController>();
+                mockController.Setup(c => c.GetCurrentUser()).Returns(user);
+
+                var controller = mockController.Object;
+                controller.NuGetContext.Config = configs.Object;
+                TestUtility.SetupUrlHelperForUrlGeneration(controller);
+
                 var mockAuthContext = new Mock<AuthorizationContext>(MockBehavior.Strict);
                 mockAuthContext.SetupGet(c => c.HttpContext).Returns(mockHttpContext.Object);
                 mockAuthContext.SetupGet(c => c.ActionDescriptor).Returns(mockActionDescriptor.Object);
-                mockAuthContext.SetupGet(c => c.Controller).Returns(mockController.Object);
-                mockAuthContext.SetupGet(c => c.RouteData).Returns(new Mock<System.Web.Routing.RouteData>().Object);
+                mockAuthContext.SetupGet(c => c.Controller).Returns(controller);
+                mockAuthContext.SetupGet(c => c.RouteData).Returns(Mock.Of<RouteData>());
 
                 return mockAuthContext;
             }