Clean up some tests

- avoid mocking `HttpContext`
- change `DefaultAntiforgeryTest` to mock token generators consistently

test/Microsoft.AspNetCore.Antiforgery.Test/Internal/DefaultAntiforgeryTest.cs

@@ -22,7 +22,6 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal
         {
             // Arrange
             var httpContext = new DefaultHttpContext();
-
             var options = new AntiforgeryOptions()
             {
                 RequireSsl = true
@@ -32,7 +31,7 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal
 
             // Act & Assert
             var exception = await Assert.ThrowsAsync<InvalidOperationException>(
-                        async () => await antiforgery.ValidateRequestAsync(httpContext));
+                () => antiforgery.ValidateRequestAsync(httpContext));
             Assert.Equal(
                 @"The antiforgery system has the configuration value AntiforgeryOptions.RequireSsl = true, " +
                 "but the current request is not an SSL request.",
@@ -44,7 +43,6 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal
         {
             // Arrange
             var httpContext = new DefaultHttpContext();
-
             var options = new AntiforgeryOptions()
             {
                 RequireSsl = true
@@ -54,7 +52,7 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal
 
             // Act & Assert
             var exception = await Assert.ThrowsAsync<InvalidOperationException>(
-                        async () => await antiforgery.IsRequestValidAsync(httpContext));
+                () => antiforgery.IsRequestValidAsync(httpContext));
             Assert.Equal(
                 @"The antiforgery system has the configuration value AntiforgeryOptions.RequireSsl = true, " +
                 "but the current request is not an SSL request.",
@@ -66,14 +64,12 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal
         {
             // Arrange
             var httpContext = new DefaultHttpContext();
-
             var options = new AntiforgeryOptions()
             {
                 RequireSsl = true
             };
 
             var antiforgery = GetAntiforgery(options);
-
             var tokenSet = new AntiforgeryTokenSet("hello", "world", "form", "header");
 
             // Act & Assert
@@ -90,7 +86,6 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal
         {
             // Arrange
             var httpContext = new DefaultHttpContext();
-
             var options = new AntiforgeryOptions()
             {
                 RequireSsl = true
@@ -111,7 +106,6 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal
         {
             // Arrange
             var httpContext = new DefaultHttpContext();
-
             var options = new AntiforgeryOptions()
             {
                 RequireSsl = true
@@ -133,7 +127,6 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal
         {
             // Arrange
             var httpContext = new DefaultHttpContext();
-
             var options = new AntiforgeryOptions()
             {
                 RequireSsl = true
@@ -155,7 +148,6 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal
         {
             // Arrange
             var httpContext = new DefaultHttpContext();
-
             var options = new AntiforgeryOptions()
             {
                 RequireSsl = true
@@ -420,15 +412,19 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal
                 .Setup(o => o.Deserialize("form-token"))
                 .Returns(context.TestTokenSet.RequestToken);
 
-            // You can't really do Moq with out-parameters :(
+            var message = "my-message";
-            var tokenGenerator = new TestTokenGenerator()
+            context.TokenGenerator
-            {
+                .Setup(o => o.TryValidateTokenSet(
-                Message = "my-message",
+                    context.HttpContext,
-            };
+                    context.TestTokenSet.OldCookieToken,
+                    context.TestTokenSet.RequestToken,
+                    out message))
+                .Returns(false)
+                .Verifiable();
 
             var antiforgery = new DefaultAntiforgery(
                 new TestOptionsManager(),
-                tokenGenerator,
+                context.TokenGenerator.Object,
                 context.TokenSerializer.Object,
                 tokenStore: null);
 
@@ -440,6 +436,7 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal
                         context.HttpContext,
                         tokenSet));
             Assert.Equal("my-message", exception.Message);
+            context.TokenGenerator.Verify();
         }
 
         [Fact]
@@ -447,7 +444,6 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal
         {
             // Arrange
             var context = CreateMockContext(new AntiforgeryOptions());
-
             context.TokenSerializer
                 .Setup(o => o.Deserialize("cookie-token"))
                 .Returns(context.TestTokenSet.OldCookieToken);
@@ -482,7 +478,6 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal
             // Arrange
             var context = CreateMockContext(new AntiforgeryOptions());
             var antiforgery = GetAntiforgery(context);
-
             var tokenSet = new AntiforgeryTokenSet("form-token", null, "form", "header");
 
             // Act
@@ -548,15 +543,19 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal
             // Arrange
             var context = CreateMockContext(new AntiforgeryOptions());
 
-            // You can't really do Moq with out-parameters :(
+            var message = "my-message";
-            var tokenGenerator = new TestTokenGenerator()
+            context.TokenGenerator
-            {
+                .Setup(o => o.TryValidateTokenSet(
-                Message = "my-message",
+                    context.HttpContext,
-            };
+                    context.TestTokenSet.OldCookieToken,
+                    context.TestTokenSet.RequestToken,
+                    out message))
+                .Returns(false)
+                .Verifiable();
 
             var antiforgery = new DefaultAntiforgery(
                 new TestOptionsManager(),
-                tokenGenerator,
+                context.TokenGenerator.Object,
                 context.TokenSerializer.Object,
                 context.TokenStore.Object);
 
@@ -564,6 +563,7 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal
             var exception = await Assert.ThrowsAsync<AntiforgeryValidationException>(
                         async () => await antiforgery.ValidateRequestAsync(context.HttpContext));
             Assert.Equal("my-message", exception.Message);
+            context.TokenGenerator.Verify();
         }
 
         [Fact]
@@ -788,35 +788,5 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal
         {
             public AntiforgeryOptions Value { get; set; } = new AntiforgeryOptions();
         }
-
-        private class TestTokenGenerator : IAntiforgeryTokenGenerator
-        {
-            public string Message { get; set; }
-
-            public AntiforgeryToken GenerateCookieToken()
-            {
-                throw new NotImplementedException();
-            }
-
-            public AntiforgeryToken GenerateRequestToken(HttpContext httpContext, AntiforgeryToken cookieToken)
-            {
-                throw new NotImplementedException();
-            }
-
-            public bool IsCookieTokenValid(AntiforgeryToken cookieToken)
-            {
-                throw new NotImplementedException();
-            }
-
-            public bool TryValidateTokenSet(
-                HttpContext httpContext,
-                AntiforgeryToken cookieToken,
-                AntiforgeryToken requestToken,
-                out string message)
-            {
-                message = Message;
-                return false;
-            }
-        }
     }
 }

test/Microsoft.AspNetCore.Antiforgery.Test/Internal/DefaultAntiforgeryTokenStoreTest.cs

@@ -25,17 +25,7 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal
         public void GetCookieToken_CookieDoesNotExist_ReturnsNull()
         {
             // Arrange
-            var requestCookies = new Mock<IRequestCookieCollection>();
+            var httpContext = GetHttpContext(new RequestCookieCollection());
-            requestCookies
-                .Setup(o => o[It.IsAny<string>()])
-                .Returns(string.Empty);
-            var mockHttpContext = new Mock<HttpContext>();
-            mockHttpContext
-                .Setup(o => o.Request.Cookies)
-                .Returns(requestCookies.Object);
-            var contextAccessor = new DefaultAntiforgeryContextAccessor();
-            mockHttpContext.SetupGet(o => o.RequestServices)
-                           .Returns(GetServiceProvider(contextAccessor));
             var options = new AntiforgeryOptions()
             {
                 CookieName = _cookieName
@@ -46,7 +36,7 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal
                 tokenSerializer: Mock.Of<IAntiforgeryTokenSerializer>());
 
             // Act
-            var token = tokenStore.GetCookieToken(mockHttpContext.Object);
+            var token = tokenStore.GetCookieToken(httpContext);
 
             // Assert
             Assert.Null(token);
@@ -56,22 +46,14 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal
         public void GetCookieToken_CookieIsMissingInRequest_LooksUpCookieInAntiforgeryContext()
         {
             // Arrange
-            var requestCookies = new Mock<IRequestCookieCollection>();
-            requestCookies
-                .Setup(o => o[It.IsAny<string>()])
-                .Returns(string.Empty);
-            var mockHttpContext = new Mock<HttpContext>();
-            mockHttpContext
-                .Setup(o => o.Request.Cookies)
-                .Returns(requestCookies.Object);
             var contextAccessor = new DefaultAntiforgeryContextAccessor();
-            mockHttpContext.SetupGet(o => o.RequestServices)
+            var httpContext = GetHttpContext(_cookieName, string.Empty, contextAccessor);
-                           .Returns(GetServiceProvider(contextAccessor));
 
             // add a cookie explicitly.
             var cookie = new AntiforgeryToken();
             contextAccessor.Value = new AntiforgeryContext() { CookieToken = cookie };
-            var options = new AntiforgeryOptions()
+
+            var options = new AntiforgeryOptions
             {
                 CookieName = _cookieName
             };
@@ -81,7 +63,7 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal
                 tokenSerializer: Mock.Of<IAntiforgeryTokenSerializer>());
 
             // Act
-            var token = tokenStore.GetCookieToken(mockHttpContext.Object);
+            var token = tokenStore.GetCookieToken(httpContext);
 
             // Assert
             Assert.Equal(cookie, token);
@@ -91,8 +73,7 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal
         public void GetCookieToken_CookieIsEmpty_ReturnsNull()
         {
             // Arrange
-            var mockHttpContext = GetMockHttpContext(_cookieName, string.Empty);
+            var httpContext = GetHttpContext(_cookieName, string.Empty);
-
             var options = new AntiforgeryOptions()
             {
                 CookieName = _cookieName
@@ -103,7 +84,7 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal
                 tokenSerializer: Mock.Of<IAntiforgeryTokenSerializer>());
 
             // Act
-            var token = tokenStore.GetCookieToken(mockHttpContext);
+            var token = tokenStore.GetCookieToken(httpContext);
 
             // Assert
             Assert.Null(token);
@@ -113,7 +94,7 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal
         public void GetCookieToken_CookieIsInvalid_PropagatesException()
         {
             // Arrange
-            var mockHttpContext = GetMockHttpContext(_cookieName, "invalid-value");
+            var httpContext = GetHttpContext(_cookieName, "invalid-value");
 
             var expectedException = new AntiforgeryValidationException("some exception");
             var mockSerializer = new Mock<IAntiforgeryTokenSerializer>();
@@ -131,7 +112,7 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal
                 tokenSerializer: mockSerializer.Object);
 
             // Act & assert
-            var ex = Assert.Throws<AntiforgeryValidationException>(() => tokenStore.GetCookieToken(mockHttpContext));
+            var ex = Assert.Throws<AntiforgeryValidationException>(() => tokenStore.GetCookieToken(httpContext));
             Assert.Same(expectedException, ex);
         }
 
@@ -140,7 +121,7 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal
         {
             // Arrange
             var expectedToken = new AntiforgeryToken();
-            var mockHttpContext = GetMockHttpContext(_cookieName, "valid-value");
+            var httpContext = GetHttpContext(_cookieName, "valid-value");
 
             var mockSerializer = new Mock<IAntiforgeryTokenSerializer>();
             mockSerializer
@@ -157,19 +138,18 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal
                 tokenSerializer: mockSerializer.Object);
 
             // Act
-            AntiforgeryToken retVal = tokenStore.GetCookieToken(mockHttpContext);
+            var token = tokenStore.GetCookieToken(httpContext);
 
             // Assert
-            Assert.Same(expectedToken, retVal);
+            Assert.Same(expectedToken, token);
         }
 
         [Fact]
         public async Task GetRequestTokens_CookieIsEmpty_Throws()
         {
             // Arrange
-            var httpContext = new DefaultHttpContext();
+            var httpContext = GetHttpContext(new RequestCookieCollection());
-            httpContext.Request.Form = new FormCollection(new Dictionary<string, StringValues>());
+            httpContext.Request.Form = FormCollection.Empty;
-            httpContext.Request.Cookies = new RequestCookieCollection(new Dictionary<string, string>());
 
             var options = new AntiforgeryOptions()
             {
@@ -193,15 +173,11 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal
         public async Task GetRequestTokens_NonFormContentType_HeaderDisabled_Throws()
         {
             // Arrange
-            var httpContext = new DefaultHttpContext();
+            var httpContext = GetHttpContext("cookie-name", "cookie-value");
             httpContext.Request.ContentType = "application/json";
 
             // Will not be accessed
             httpContext.Request.Form = null;
-            httpContext.Request.Cookies = new RequestCookieCollection(new Dictionary<string, string>()
-            {
-                { "cookie-name", "cookie-value" },
-            });
 
             var options = new AntiforgeryOptions()
             {
@@ -226,16 +202,9 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal
         public async Task GetRequestTokens_FormContentType_FallbackHeaderToken()
         {
             // Arrange
-            var httpContext = new DefaultHttpContext();
+            var httpContext = GetHttpContext("cookie-name", "cookie-value");
-            httpContext.Request.ContentType = "application/json";
-
-            // Will not be accessed
             httpContext.Request.ContentType = "application/x-www-form-urlencoded";
-            httpContext.Request.Form = new FormCollection(new Dictionary<string, StringValues>());
+            httpContext.Request.Form = FormCollection.Empty;
-            httpContext.Request.Cookies = new RequestCookieCollection(new Dictionary<string, string>()
-            {
-                { "cookie-name", "cookie-value" },
-            });
             httpContext.Request.Headers.Add("header-name", "header-value");
 
             var options = new AntiforgeryOptions()
@@ -261,17 +230,12 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal
         public async Task GetRequestTokens_NonFormContentType_UsesHeaderToken()
         {
             // Arrange
-            var httpContext = new DefaultHttpContext();
+            var httpContext = GetHttpContext("cookie-name", "cookie-value");
             httpContext.Request.ContentType = "application/json";
+            httpContext.Request.Headers.Add("header-name", "header-value");
 
             // Will not be accessed
             httpContext.Request.Form = null;
-            httpContext.Request.Cookies = new RequestCookieCollection(new Dictionary<string, string>()
-            {
-                { "cookie-name", "cookie-value" },
-            });
-
-            httpContext.Request.Headers.Add("header-name", "header-value");
 
             var options = new AntiforgeryOptions()
             {
@@ -296,15 +260,11 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal
         public async Task GetRequestTokens_NonFormContentType_UsesHeaderToken_ThrowsOnMissingValue()
         {
             // Arrange
-            var httpContext = new DefaultHttpContext();
+            var httpContext = GetHttpContext("cookie-name", "cookie-value");
             httpContext.Request.ContentType = "application/json";
 
             // Will not be accessed
             httpContext.Request.Form = null;
-            httpContext.Request.Cookies = new RequestCookieCollection(new Dictionary<string, string>()
-            {
-                { "cookie-name", "cookie-value" },
-            });
 
             var options = new AntiforgeryOptions()
             {
@@ -329,13 +289,9 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal
         public async Task GetRequestTokens_BothFieldsEmpty_Throws()
         {
             // Arrange
-            var httpContext = new DefaultHttpContext();
+            var httpContext = GetHttpContext("cookie-name", "cookie-value");
             httpContext.Request.ContentType = "application/x-www-form-urlencoded";
-            httpContext.Request.Form = new FormCollection(new Dictionary<string, StringValues>());
+            httpContext.Request.Form = FormCollection.Empty;
-            httpContext.Request.Cookies = new RequestCookieCollection(new Dictionary<string, string>()
-            {
-                { "cookie-name", "cookie-value" },
-            });
 
             var options = new AntiforgeryOptions()
             {
@@ -363,16 +319,12 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal
         public async Task GetFormToken_FormFieldIsValid_ReturnsToken()
         {
             // Arrange
-            var httpContext = new DefaultHttpContext();
+            var httpContext = GetHttpContext("cookie-name", "cookie-value");
             httpContext.Request.ContentType = "application/x-www-form-urlencoded";
-            httpContext.Request.Form = new FormCollection(new Dictionary<string, StringValues>()
+            httpContext.Request.Form = new FormCollection(new Dictionary<string, StringValues>
             {
                 { "form-field-name", "form-value" },
             });
-            httpContext.Request.Cookies = new RequestCookieCollection(new Dictionary<string, string>()
-            {
-                { "cookie-name", "cookie-value" },
-            });
             httpContext.Request.Headers.Add("header-name", "header-value"); // form value has priority.
 
             var options = new AntiforgeryOptions()
@@ -401,22 +353,23 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal
         {
             // Arrange
             var token = new AntiforgeryToken();
-            var mockCookies = new Mock<IResponseCookies>();
-
             bool defaultCookieSecureValue = expectedCookieSecureFlag ?? false; // pulled from config; set by ctor
             var cookies = new MockResponseCookieCollection();
 
-            cookies.Count = 0;
             var mockHttpContext = new Mock<HttpContext>();
-            mockHttpContext.Setup(o => o.Response.Cookies)
+            mockHttpContext
-                           .Returns(cookies);
+                .Setup(o => o.Response.Cookies)
+                .Returns(cookies);
+
             var contextAccessor = new DefaultAntiforgeryContextAccessor();
-            mockHttpContext.SetupGet(o => o.RequestServices)
+            mockHttpContext
-                           .Returns(GetServiceProvider(contextAccessor));
+                .SetupGet(o => o.RequestServices)
+                .Returns(GetServiceProvider(contextAccessor));
 
             var mockSerializer = new Mock<IAntiforgeryTokenSerializer>();
-            mockSerializer.Setup(o => o.Serialize(token))
+            mockSerializer
-                          .Returns("serialized-value");
+                .Setup(o => o.Serialize(token))
+                .Returns("serialized-value");
 
             var options = new AntiforgeryOptions()
             {
@@ -441,22 +394,30 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal
             Assert.Equal(defaultCookieSecureValue, cookies.Options.Secure);
         }
 
-        private HttpContext GetMockHttpContext(string cookieName, string cookieValue)
+        private HttpContext GetHttpContext(
+            string cookieName,
+            string cookieValue,
+            IAntiforgeryContextAccessor contextAccessor = null)
         {
-            var requestCookies = new MockCookieCollection(new Dictionary<string, string>() { { cookieName, cookieValue } });
+            var cookies = new RequestCookieCollection(new Dictionary<string, string>
+            {
+                { cookieName, cookieValue },
+            });
 
-            var request = new Mock<HttpRequest>();
+            return GetHttpContext(cookies, contextAccessor);
-            request.Setup(o => o.Cookies)
+        }
-                   .Returns(requestCookies);
-            var mockHttpContext = new Mock<HttpContext>();
-            mockHttpContext.Setup(o => o.Request)
-                           .Returns(request.Object);
 
-            var contextAccessor = new DefaultAntiforgeryContextAccessor();
+        private HttpContext GetHttpContext(
-            mockHttpContext.SetupGet(o => o.RequestServices)
+            IRequestCookieCollection cookies,
-                           .Returns(GetServiceProvider(contextAccessor));
+            IAntiforgeryContextAccessor contextAccessor = null)
+        {
+            var httpContext = new DefaultHttpContext();
+            httpContext.Request.Cookies = cookies;
 
-            return mockHttpContext.Object;
+            contextAccessor = contextAccessor ?? new DefaultAntiforgeryContextAccessor();
+            httpContext.RequestServices = GetServiceProvider(contextAccessor);
+
+            return httpContext;
         }
 
         private static IServiceProvider GetServiceProvider(IAntiforgeryContextAccessor contextAccessor)
@@ -496,65 +457,5 @@ namespace Microsoft.AspNetCore.Antiforgery.Internal
                 throw new NotImplementedException();
             }
         }
-
-        private class MockCookieCollection : IRequestCookieCollection
-        {
-            private Dictionary<string, string> _dictionary;
-
-            public int Count
-            {
-                get
-                {
-                    return _dictionary.Count;
-                }
-            }
-
-            public ICollection<string> Keys
-            {
-                get
-                {
-                    return _dictionary.Keys;
-                }
-            }
-
-            public MockCookieCollection(Dictionary<string, string> dictionary)
-            {
-                _dictionary = dictionary;
-            }
-
-            public static MockCookieCollection GetDummyInstance(string key, string value)
-            {
-                return new MockCookieCollection(new Dictionary<string, string>() { { key, value } });
-            }
-
-            public bool ContainsKey(string key)
-            {
-                return _dictionary.ContainsKey(key);
-            }
-
-            public string this[string key]
-            {
-                get
-                {
-                    string value;
-                    return _dictionary.TryGetValue(key, out value) ? value : null;
-                }
-            }
-
-            public bool TryGetValue(string key, out string value)
-            {
-                return _dictionary.TryGetValue(key, out value);
-            }
-
-            public IEnumerator<KeyValuePair<string, string>> GetEnumerator()
-            {
-                throw new NotImplementedException();
-            }
-
-            System.Collections.IEnumerator System.Collections.IEnumerable.GetEnumerator()
-            {
-                throw new NotImplementedException();
-            }
-        }
     }
 }