From e0ec2da711aa97f70e4dac25aa4a99a1230ca4f3 Mon Sep 17 00:00:00 2001 From: Pranav K Date: Fri, 11 Sep 2015 16:26:06 -0700 Subject: [PATCH] Replacing NotNull with thrown exceptions --- .../AntiforgeryOptions.cs | 38 +++++++++++-- .../DefaultAntiforgery.cs | 54 +++++++++++++++---- .../DefaultAntiforgeryTokenGenerator.cs | 10 ++++ .../DefaultAntiforgeryTokenSerializer.cs | 15 ++++-- .../DefaultAntiforgeryTokenStore.cs | 37 +++++++++++-- .../IAntiforgery.cs | 15 +++--- .../IAntiforgeryTokenGenerator.cs | 9 ++-- .../IAntiforgeryTokenStore.cs | 7 ++- .../ServiceCollectionExtensions.cs | 24 +++++++-- src/Microsoft.AspNet.Antiforgery/project.json | 1 - .../DefaultAntiforgeryTokenStoreTest.cs | 15 +++--- 11 files changed, 175 insertions(+), 50 deletions(-) diff --git a/src/Microsoft.AspNet.Antiforgery/AntiforgeryOptions.cs b/src/Microsoft.AspNet.Antiforgery/AntiforgeryOptions.cs index 6955481157..db66421a15 100644 --- a/src/Microsoft.AspNet.Antiforgery/AntiforgeryOptions.cs +++ b/src/Microsoft.AspNet.Antiforgery/AntiforgeryOptions.cs @@ -1,7 +1,7 @@ // 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 Microsoft.Framework.Internal; +using System; namespace Microsoft.AspNet.Antiforgery { @@ -11,6 +11,8 @@ namespace Microsoft.AspNet.Antiforgery public class AntiforgeryOptions { private const string AntiforgeryTokenFieldName = "__RequestVerificationToken"; + private string _cookieName; + private string _formFieldName = AntiforgeryTokenFieldName; /// /// Specifies the name of the cookie that is used by the antiforgery @@ -20,12 +22,42 @@ namespace Microsoft.AspNet.Antiforgery /// If an explicit name is not provided, the system will automatically /// generate a name. /// - public string CookieName { get; [param: NotNull] set; } + public string CookieName + { + get + { + return _cookieName; + } + set + { + if (value == null) + { + throw new ArgumentNullException(nameof(value)); + } + + _cookieName = value; + } + } /// /// Specifies the name of the antiforgery token field that is used by the antiforgery system. /// - public string FormFieldName { get; [param: NotNull] set; } = AntiforgeryTokenFieldName; + public string FormFieldName + { + get + { + return _formFieldName; + } + set + { + if (value == null) + { + throw new ArgumentNullException(nameof(value)); + } + + _formFieldName = value; + } + } /// /// Specifies whether SSL is required for the antiforgery system diff --git a/src/Microsoft.AspNet.Antiforgery/DefaultAntiforgery.cs b/src/Microsoft.AspNet.Antiforgery/DefaultAntiforgery.cs index b68ae093b3..c2c66444ce 100644 --- a/src/Microsoft.AspNet.Antiforgery/DefaultAntiforgery.cs +++ b/src/Microsoft.AspNet.Antiforgery/DefaultAntiforgery.cs @@ -5,7 +5,6 @@ using System; using System.Diagnostics; using System.Threading.Tasks; using Microsoft.AspNet.Http; -using Microsoft.Framework.Internal; using Microsoft.Framework.OptionsModel; using Microsoft.Framework.WebEncoders; @@ -38,8 +37,13 @@ namespace Microsoft.AspNet.Antiforgery } /// - public string GetHtml([NotNull] HttpContext context) + public string GetHtml(HttpContext context) { + if (context == null) + { + throw new ArgumentNullException(nameof(context)); + } + CheckSSLConfig(context); var tokenSet = GetAndStoreTokens(context); @@ -53,27 +57,42 @@ namespace Microsoft.AspNet.Antiforgery } /// - public AntiforgeryTokenSet GetAndStoreTokens([NotNull] HttpContext context) + public AntiforgeryTokenSet GetAndStoreTokens(HttpContext context) { + if (context == null) + { + throw new ArgumentNullException(nameof(context)); + } + CheckSSLConfig(context); - + var tokenSet = GetTokensInternal(context); SaveCookieTokenAndHeader(context, tokenSet.CookieToken); return Serialize(tokenSet); } /// - public AntiforgeryTokenSet GetTokens([NotNull] HttpContext context) + public AntiforgeryTokenSet GetTokens(HttpContext context) { + if (context == null) + { + throw new ArgumentNullException(nameof(context)); + } + CheckSSLConfig(context); - + var tokenSet = GetTokensInternal(context); return Serialize(tokenSet); } /// - public async Task ValidateRequestAsync([NotNull] HttpContext context) + public async Task ValidateRequestAsync(HttpContext context) { + if (context == null) + { + throw new ArgumentNullException(nameof(context)); + } + CheckSSLConfig(context); var tokens = await _tokenStore.GetRequestTokensAsync(context); @@ -81,8 +100,13 @@ namespace Microsoft.AspNet.Antiforgery } /// - public void ValidateTokens([NotNull] HttpContext context, AntiforgeryTokenSet antiforgeryTokenSet) + public void ValidateTokens(HttpContext context, AntiforgeryTokenSet antiforgeryTokenSet) { + if (context == null) + { + throw new ArgumentNullException(nameof(context)); + } + CheckSSLConfig(context); if (string.IsNullOrEmpty(antiforgeryTokenSet.CookieToken)) @@ -111,8 +135,13 @@ namespace Microsoft.AspNet.Antiforgery } /// - public void SetCookieTokenAndHeader([NotNull] HttpContext context) + public void SetCookieTokenAndHeader(HttpContext context) { + if (context == null) + { + throw new ArgumentNullException(nameof(context)); + } + CheckSSLConfig(context); var cookieToken = GetCookieTokenDoesNotThrow(context); @@ -135,9 +164,14 @@ namespace Microsoft.AspNet.Antiforgery } private void SaveCookieTokenAndHeader( - [NotNull] HttpContext context, + HttpContext context, AntiforgeryToken cookieToken) { + if (context == null) + { + throw new ArgumentNullException(nameof(context)); + } + if (cookieToken != null) { // Persist the new cookie if it is not null. diff --git a/src/Microsoft.AspNet.Antiforgery/DefaultAntiforgeryTokenGenerator.cs b/src/Microsoft.AspNet.Antiforgery/DefaultAntiforgeryTokenGenerator.cs index 700ecaaa6e..c23661ccc4 100644 --- a/src/Microsoft.AspNet.Antiforgery/DefaultAntiforgeryTokenGenerator.cs +++ b/src/Microsoft.AspNet.Antiforgery/DefaultAntiforgeryTokenGenerator.cs @@ -35,6 +35,11 @@ namespace Microsoft.AspNet.Antiforgery HttpContext httpContext, AntiforgeryToken cookieToken) { + if (httpContext == null) + { + throw new ArgumentNullException(nameof(httpContext)); + } + Debug.Assert(IsCookieTokenValid(cookieToken)); var formToken = new AntiforgeryToken() @@ -92,6 +97,11 @@ namespace Microsoft.AspNet.Antiforgery AntiforgeryToken sessionToken, AntiforgeryToken fieldToken) { + if (httpContext == null) + { + throw new ArgumentNullException(nameof(httpContext)); + } + if (sessionToken == null) { throw new ArgumentNullException( diff --git a/src/Microsoft.AspNet.Antiforgery/DefaultAntiforgeryTokenSerializer.cs b/src/Microsoft.AspNet.Antiforgery/DefaultAntiforgeryTokenSerializer.cs index 971af9c3a0..c5eb46a413 100644 --- a/src/Microsoft.AspNet.Antiforgery/DefaultAntiforgeryTokenSerializer.cs +++ b/src/Microsoft.AspNet.Antiforgery/DefaultAntiforgeryTokenSerializer.cs @@ -5,7 +5,6 @@ using System; using System.IO; using Microsoft.AspNet.DataProtection; using Microsoft.AspNet.WebUtilities; -using Microsoft.Framework.Internal; namespace Microsoft.AspNet.Antiforgery { @@ -16,8 +15,13 @@ namespace Microsoft.AspNet.Antiforgery private readonly IDataProtector _cryptoSystem; private const byte TokenVersion = 0x01; - public DefaultAntiforgeryTokenSerializer([NotNull] IDataProtectionProvider provider) + public DefaultAntiforgeryTokenSerializer(IDataProtectionProvider provider) { + if (provider == null) + { + throw new ArgumentNullException(nameof(provider)); + } + _cryptoSystem = provider.CreateProtector(Purpose); } @@ -102,8 +106,13 @@ namespace Microsoft.AspNet.Antiforgery return deserializedToken; } - public string Serialize([NotNull] AntiforgeryToken token) + public string Serialize(AntiforgeryToken token) { + if (token == null) + { + throw new ArgumentNullException(nameof(token)); + } + using (var stream = new MemoryStream()) { using (var writer = new BinaryWriter(stream)) diff --git a/src/Microsoft.AspNet.Antiforgery/DefaultAntiforgeryTokenStore.cs b/src/Microsoft.AspNet.Antiforgery/DefaultAntiforgeryTokenStore.cs index cd0667f0fb..0e67c14827 100644 --- a/src/Microsoft.AspNet.Antiforgery/DefaultAntiforgeryTokenStore.cs +++ b/src/Microsoft.AspNet.Antiforgery/DefaultAntiforgeryTokenStore.cs @@ -6,7 +6,6 @@ using System.Diagnostics; using System.Threading.Tasks; using Microsoft.AspNet.Http; using Microsoft.Framework.DependencyInjection; -using Microsoft.Framework.Internal; using Microsoft.Framework.OptionsModel; namespace Microsoft.AspNet.Antiforgery @@ -18,15 +17,30 @@ namespace Microsoft.AspNet.Antiforgery private readonly IAntiforgeryTokenSerializer _tokenSerializer; public DefaultAntiforgeryTokenStore( - [NotNull] IOptions optionsAccessor, - [NotNull] IAntiforgeryTokenSerializer tokenSerializer) + IOptions optionsAccessor, + IAntiforgeryTokenSerializer tokenSerializer) { + if (optionsAccessor == null) + { + throw new ArgumentNullException(nameof(optionsAccessor)); + } + + if (tokenSerializer == null) + { + throw new ArgumentNullException(nameof(tokenSerializer)); + } + _options = optionsAccessor.Value; _tokenSerializer = tokenSerializer; } public AntiforgeryToken GetCookieToken(HttpContext httpContext) { + if (httpContext == null) + { + throw new ArgumentNullException(nameof(httpContext)); + } + var services = httpContext.RequestServices; var contextAccessor = services.GetRequiredService(); if (contextAccessor.Value != null) @@ -44,8 +58,13 @@ namespace Microsoft.AspNet.Antiforgery return _tokenSerializer.Deserialize(requestCookie); } - public async Task GetRequestTokensAsync([NotNull] HttpContext httpContext) + public async Task GetRequestTokensAsync(HttpContext httpContext) { + if (httpContext == null) + { + throw new ArgumentNullException(nameof(httpContext)); + } + var requestCookie = httpContext.Request.Cookies[_options.CookieName]; if (string.IsNullOrEmpty(requestCookie)) { @@ -74,6 +93,16 @@ namespace Microsoft.AspNet.Antiforgery public void SaveCookieToken(HttpContext httpContext, AntiforgeryToken token) { + if (httpContext == null) + { + throw new ArgumentNullException(nameof(httpContext)); + } + + if (token == null) + { + throw new ArgumentNullException(nameof(token)); + } + // Add the cookie to the request based context. // This is useful if the cookie needs to be reloaded in the context of the same request. diff --git a/src/Microsoft.AspNet.Antiforgery/IAntiforgery.cs b/src/Microsoft.AspNet.Antiforgery/IAntiforgery.cs index 564dbd2b83..f57c5689ae 100644 --- a/src/Microsoft.AspNet.Antiforgery/IAntiforgery.cs +++ b/src/Microsoft.AspNet.Antiforgery/IAntiforgery.cs @@ -1,9 +1,8 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// 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.Threading.Tasks; using Microsoft.AspNet.Http; -using Microsoft.Framework.Internal; namespace Microsoft.AspNet.Antiforgery { @@ -25,7 +24,7 @@ namespace Microsoft.AspNet.Antiforgery /// This method has a side effect: /// A response cookie is set if there is no valid cookie associated with the request. /// - string GetHtml([NotNull] HttpContext context); + string GetHtml(HttpContext context); /// /// Generates an for this request and stores the cookie token @@ -37,7 +36,7 @@ namespace Microsoft.AspNet.Antiforgery /// This method has a side effect: /// A response cookie is set if there is no valid cookie associated with the request. /// - AntiforgeryTokenSet GetAndStoreTokens([NotNull] HttpContext context); + AntiforgeryTokenSet GetAndStoreTokens(HttpContext context); /// /// Generates an for this request. @@ -48,13 +47,13 @@ namespace Microsoft.AspNet.Antiforgery /// is responsible for setting the response cookie and injecting the returned /// form token as appropriate. /// - AntiforgeryTokenSet GetTokens([NotNull] HttpContext context); + AntiforgeryTokenSet GetTokens(HttpContext context); /// /// Validates an antiforgery token that was supplied as part of the request. /// /// The associated with the current request. - Task ValidateRequestAsync([NotNull] HttpContext context); + Task ValidateRequestAsync(HttpContext context); /// /// Validates an for the current request. @@ -63,12 +62,12 @@ namespace Microsoft.AspNet.Antiforgery /// /// The (cookie and form token) for this request. /// - void ValidateTokens([NotNull] HttpContext context, AntiforgeryTokenSet antiforgeryTokenSet); + void ValidateTokens(HttpContext context, AntiforgeryTokenSet antiforgeryTokenSet); /// /// Generates and stores an antiforgery cookie token if one is not available or not valid. /// /// The associated with the current request. - void SetCookieTokenAndHeader([NotNull] HttpContext context); + void SetCookieTokenAndHeader(HttpContext context); } } diff --git a/src/Microsoft.AspNet.Antiforgery/IAntiforgeryTokenGenerator.cs b/src/Microsoft.AspNet.Antiforgery/IAntiforgeryTokenGenerator.cs index bdf01af11e..0918d26917 100644 --- a/src/Microsoft.AspNet.Antiforgery/IAntiforgeryTokenGenerator.cs +++ b/src/Microsoft.AspNet.Antiforgery/IAntiforgeryTokenGenerator.cs @@ -2,7 +2,6 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using Microsoft.AspNet.Http; -using Microsoft.Framework.Internal; namespace Microsoft.AspNet.Antiforgery { @@ -17,7 +16,7 @@ namespace Microsoft.AspNet.Antiforgery // Given a cookie token, generates a corresponding form token. // The incoming cookie token must be valid. AntiforgeryToken GenerateFormToken( - [NotNull] HttpContext httpContext, + HttpContext httpContext, AntiforgeryToken cookieToken); // Determines whether an existing cookie token is valid (well-formed). @@ -26,8 +25,8 @@ namespace Microsoft.AspNet.Antiforgery // Validates a (cookie, form) token pair. void ValidateTokens( - [NotNull] HttpContext httpContext, - [NotNull] AntiforgeryToken cookieToken, - [NotNull] AntiforgeryToken formToken); + HttpContext httpContext, + AntiforgeryToken cookieToken, + AntiforgeryToken formToken); } } \ No newline at end of file diff --git a/src/Microsoft.AspNet.Antiforgery/IAntiforgeryTokenStore.cs b/src/Microsoft.AspNet.Antiforgery/IAntiforgeryTokenStore.cs index afa43014d6..c600d68e53 100644 --- a/src/Microsoft.AspNet.Antiforgery/IAntiforgeryTokenStore.cs +++ b/src/Microsoft.AspNet.Antiforgery/IAntiforgeryTokenStore.cs @@ -3,14 +3,13 @@ using System.Threading.Tasks; using Microsoft.AspNet.Http; -using Microsoft.Framework.Internal; namespace Microsoft.AspNet.Antiforgery { // Provides an abstraction around how tokens are persisted and retrieved for a request public interface IAntiforgeryTokenStore { - AntiforgeryToken GetCookieToken([NotNull] HttpContext httpContext); + AntiforgeryToken GetCookieToken(HttpContext httpContext); /// /// Gets the cookie and form tokens from the request. Will throw an exception if either token is @@ -18,8 +17,8 @@ namespace Microsoft.AspNet.Antiforgery /// /// The for the current request. /// The . - Task GetRequestTokensAsync([NotNull] HttpContext httpContext); + Task GetRequestTokensAsync(HttpContext httpContext); - void SaveCookieToken([NotNull] HttpContext httpContext, [NotNull] AntiforgeryToken token); + void SaveCookieToken(HttpContext httpContext, AntiforgeryToken token); } } \ No newline at end of file diff --git a/src/Microsoft.AspNet.Antiforgery/ServiceCollectionExtensions.cs b/src/Microsoft.AspNet.Antiforgery/ServiceCollectionExtensions.cs index 297a76a36d..e33c314b6c 100644 --- a/src/Microsoft.AspNet.Antiforgery/ServiceCollectionExtensions.cs +++ b/src/Microsoft.AspNet.Antiforgery/ServiceCollectionExtensions.cs @@ -1,18 +1,22 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// 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 Microsoft.AspNet.Antiforgery; using Microsoft.Framework.DependencyInjection.Extensions; -using Microsoft.Framework.Internal; using Microsoft.Framework.OptionsModel; namespace Microsoft.Framework.DependencyInjection { public static class ServiceCollectionExtensions { - public static IServiceCollection AddAntiforgery([NotNull] this IServiceCollection services) + public static IServiceCollection AddAntiforgery(this IServiceCollection services) { + if (services == null) + { + throw new ArgumentNullException(nameof(services)); + } + services.AddDataProtection(); services.AddWebEncoders(); @@ -31,9 +35,19 @@ namespace Microsoft.Framework.DependencyInjection } public static IServiceCollection ConfigureAntiforgery( - [NotNull] this IServiceCollection services, - [NotNull] Action setupAction) + this IServiceCollection services, + Action setupAction) { + if (services == null) + { + throw new ArgumentNullException(nameof(services)); + } + + if (setupAction == null) + { + throw new ArgumentNullException(nameof(setupAction)); + } + services.Configure(setupAction); return services; } diff --git a/src/Microsoft.AspNet.Antiforgery/project.json b/src/Microsoft.AspNet.Antiforgery/project.json index c5de78d2a6..4bc77d47bd 100644 --- a/src/Microsoft.AspNet.Antiforgery/project.json +++ b/src/Microsoft.AspNet.Antiforgery/project.json @@ -10,7 +10,6 @@ "Microsoft.AspNet.Http.Abstractions": "1.0.0-*", "Microsoft.AspNet.WebUtilities": "1.0.0-*", "Microsoft.Framework.DependencyInjection.Abstractions": "1.0.0-*", - "Microsoft.Framework.NotNullAttribute.Sources": { "type": "build", "version": "1.0.0-*" }, "Microsoft.Framework.OptionsModel": "1.0.0-*", "Microsoft.Framework.WebEncoders": "1.0.0-*" }, diff --git a/test/Microsoft.AspNet.Antiforgery.Test/DefaultAntiforgeryTokenStoreTest.cs b/test/Microsoft.AspNet.Antiforgery.Test/DefaultAntiforgeryTokenStoreTest.cs index 8e73c9d69e..98f56039e2 100644 --- a/test/Microsoft.AspNet.Antiforgery.Test/DefaultAntiforgeryTokenStoreTest.cs +++ b/test/Microsoft.AspNet.Antiforgery.Test/DefaultAntiforgeryTokenStoreTest.cs @@ -5,6 +5,7 @@ using System; using System.Collections.Generic; using System.Threading.Tasks; +using Microsoft.AspNet.DataProtection; using Microsoft.AspNet.Http; using Microsoft.AspNet.Http.Internal; using Microsoft.Framework.DependencyInjection; @@ -40,7 +41,7 @@ namespace Microsoft.AspNet.Antiforgery var tokenStore = new DefaultAntiforgeryTokenStore( optionsAccessor: new TestOptionsManager(options), - tokenSerializer: null); + tokenSerializer: Mock.Of()); // Act var token = tokenStore.GetCookieToken(mockHttpContext.Object); @@ -75,7 +76,7 @@ namespace Microsoft.AspNet.Antiforgery var tokenStore = new DefaultAntiforgeryTokenStore( optionsAccessor: new TestOptionsManager(options), - tokenSerializer: null); + tokenSerializer: Mock.Of()); // Act var token = tokenStore.GetCookieToken(mockHttpContext.Object); @@ -97,7 +98,7 @@ namespace Microsoft.AspNet.Antiforgery var tokenStore = new DefaultAntiforgeryTokenStore( optionsAccessor: new TestOptionsManager(options), - tokenSerializer: null); + tokenSerializer: Mock.Of()); // Act var token = tokenStore.GetCookieToken(mockHttpContext); @@ -176,7 +177,7 @@ namespace Microsoft.AspNet.Antiforgery var tokenStore = new DefaultAntiforgeryTokenStore( optionsAccessor: new TestOptionsManager(options), - tokenSerializer: null); + tokenSerializer: Mock.Of()); // Act var exception = await Assert.ThrowsAsync( @@ -208,7 +209,7 @@ namespace Microsoft.AspNet.Antiforgery var tokenStore = new DefaultAntiforgeryTokenStore( optionsAccessor: new TestOptionsManager(options), - tokenSerializer: null); + tokenSerializer: new DefaultAntiforgeryTokenSerializer(new EphemeralDataProtectionProvider())); // Act var exception = await Assert.ThrowsAsync( @@ -238,7 +239,7 @@ namespace Microsoft.AspNet.Antiforgery var tokenStore = new DefaultAntiforgeryTokenStore( optionsAccessor: new TestOptionsManager(options), - tokenSerializer: null); + tokenSerializer: Mock.Of()); // Act var exception = await Assert.ThrowsAsync( @@ -271,7 +272,7 @@ namespace Microsoft.AspNet.Antiforgery var tokenStore = new DefaultAntiforgeryTokenStore( optionsAccessor: new TestOptionsManager(options), - tokenSerializer: null); + tokenSerializer: Mock.Of()); // Act var tokens = await tokenStore.GetRequestTokensAsync(httpContext);