From 2c9f43a16045d395b8dcc68ea72d9c8d7860e008 Mon Sep 17 00:00:00 2001 From: Hao Kung Date: Wed, 8 Oct 2014 16:27:29 -0700 Subject: [PATCH] Enable and fix Async/Cancellation tests --- .../BuilderExtensions.cs | 4 - .../ClaimsIdentityFactory.cs | 2 +- .../ISecurityStampValidator.cs | 4 +- .../IdentityServiceCollectionExtensions.cs | 8 +- .../SecurityStampValidator.cs | 10 +- .../SignInManager.cs | 6 +- .../TotpSecurityStampBasedTokenProvider.cs | 6 +- src/Microsoft.AspNet.Identity/UserManager.cs | 31 ++-- .../ApiConsistencyTest.cs | 17 ++ test/Shared/ApiConsistencyTestBase.cs | 153 ++++++++++++++++++ 10 files changed, 207 insertions(+), 34 deletions(-) create mode 100644 test/Microsoft.AspNet.Identity.Test/ApiConsistencyTest.cs create mode 100644 test/Shared/ApiConsistencyTestBase.cs diff --git a/src/Microsoft.AspNet.Identity/BuilderExtensions.cs b/src/Microsoft.AspNet.Identity/BuilderExtensions.cs index 97a925efce..4049f04ffd 100644 --- a/src/Microsoft.AspNet.Identity/BuilderExtensions.cs +++ b/src/Microsoft.AspNet.Identity/BuilderExtensions.cs @@ -3,10 +3,6 @@ using System; using Microsoft.AspNet.Identity; -using Microsoft.Framework.OptionsModel; -using Microsoft.Framework.DependencyInjection; -using Microsoft.AspNet.Security.Cookies; -using Microsoft.Framework.ConfigurationModel; namespace Microsoft.AspNet.Builder { diff --git a/src/Microsoft.AspNet.Identity/ClaimsIdentityFactory.cs b/src/Microsoft.AspNet.Identity/ClaimsIdentityFactory.cs index c788651965..9fc5cb760f 100644 --- a/src/Microsoft.AspNet.Identity/ClaimsIdentityFactory.cs +++ b/src/Microsoft.AspNet.Identity/ClaimsIdentityFactory.cs @@ -1,11 +1,11 @@ // Copyright (c) Microsoft Open Technologies, Inc. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. -using Microsoft.Framework.OptionsModel; using System; using System.Security.Claims; using System.Threading; using System.Threading.Tasks; +using Microsoft.Framework.OptionsModel; namespace Microsoft.AspNet.Identity { diff --git a/src/Microsoft.AspNet.Identity/ISecurityStampValidator.cs b/src/Microsoft.AspNet.Identity/ISecurityStampValidator.cs index cd40753cb9..f50a163e46 100644 --- a/src/Microsoft.AspNet.Identity/ISecurityStampValidator.cs +++ b/src/Microsoft.AspNet.Identity/ISecurityStampValidator.cs @@ -2,6 +2,7 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System.Security.Claims; +using System.Threading; using System.Threading.Tasks; using Microsoft.AspNet.Security.Cookies; @@ -9,6 +10,7 @@ namespace Microsoft.AspNet.Identity { public interface ISecurityStampValidator { - Task Validate(CookieValidateIdentityContext context, ClaimsIdentity identity); + Task ValidateAsync(CookieValidateIdentityContext context, ClaimsIdentity identity, + CancellationToken cancellationToken = default(CancellationToken)); } } \ No newline at end of file diff --git a/src/Microsoft.AspNet.Identity/IdentityServiceCollectionExtensions.cs b/src/Microsoft.AspNet.Identity/IdentityServiceCollectionExtensions.cs index e4d97c5dc8..60fdc61754 100644 --- a/src/Microsoft.AspNet.Identity/IdentityServiceCollectionExtensions.cs +++ b/src/Microsoft.AspNet.Identity/IdentityServiceCollectionExtensions.cs @@ -2,12 +2,12 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; -using Microsoft.AspNet.Identity; -using Microsoft.Framework.ConfigurationModel; -using Microsoft.AspNet.Security.DataProtection; -using Microsoft.AspNet.Security.Cookies; using Microsoft.AspNet.Http; +using Microsoft.AspNet.Identity; using Microsoft.AspNet.Security; +using Microsoft.AspNet.Security.Cookies; +using Microsoft.AspNet.Security.DataProtection; +using Microsoft.Framework.ConfigurationModel; namespace Microsoft.Framework.DependencyInjection { diff --git a/src/Microsoft.AspNet.Identity/SecurityStampValidator.cs b/src/Microsoft.AspNet.Identity/SecurityStampValidator.cs index 755afcf4c2..73323893d4 100644 --- a/src/Microsoft.AspNet.Identity/SecurityStampValidator.cs +++ b/src/Microsoft.AspNet.Identity/SecurityStampValidator.cs @@ -4,6 +4,7 @@ using System; using System.Security.Claims; using System.Security.Principal; +using System.Threading; using System.Threading.Tasks; using Microsoft.AspNet.Security.Cookies; using Microsoft.Framework.DependencyInjection; @@ -18,11 +19,12 @@ namespace Microsoft.AspNet.Identity /// ClaimsIdentity /// /// - public virtual async Task Validate(CookieValidateIdentityContext context, ClaimsIdentity identity) + public virtual async Task ValidateAsync(CookieValidateIdentityContext context, ClaimsIdentity identity, + CancellationToken cancellationToken = default(CancellationToken)) { var manager = context.HttpContext.RequestServices.GetRequiredService>(); var userId = identity.GetUserId(); - var user = await manager.ValidateSecurityStampAsync(identity, userId); + var user = await manager.ValidateSecurityStampAsync(identity, userId, cancellationToken); if (user != null) { var isPersistent = false; @@ -30,7 +32,7 @@ namespace Microsoft.AspNet.Identity { isPersistent = context.Properties.IsPersistent; } - await manager.SignInAsync(user, isPersistent); + await manager.SignInAsync(user, isPersistent, authenticationMethod: null, cancellationToken: cancellationToken); } else { @@ -71,7 +73,7 @@ namespace Microsoft.AspNet.Identity if (validate) { var validator = context.HttpContext.RequestServices.GetRequiredService(); - return validator.Validate(context, context.Identity); + return validator.ValidateAsync(context, context.Identity); } return Task.FromResult(0); } diff --git a/src/Microsoft.AspNet.Identity/SignInManager.cs b/src/Microsoft.AspNet.Identity/SignInManager.cs index a6e096d59f..df1e0144ad 100644 --- a/src/Microsoft.AspNet.Identity/SignInManager.cs +++ b/src/Microsoft.AspNet.Identity/SignInManager.cs @@ -2,6 +2,8 @@ // 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; using System.Security.Principal; using System.Threading; @@ -10,8 +12,6 @@ using Microsoft.AspNet.Http; using Microsoft.AspNet.Http.Security; using Microsoft.Framework.DependencyInjection; using Microsoft.Framework.OptionsModel; -using System.Collections.Generic; -using System.Linq; namespace Microsoft.AspNet.Identity { @@ -214,7 +214,7 @@ namespace Microsoft.AspNet.Identity Context.Response.SignIn(new AuthenticationProperties { IsPersistent = true }, rememberBrowserIdentity); } - public virtual Task ForgetTwoFactorClientAsync() + public virtual Task ForgetTwoFactorClientAsync(CancellationToken cancellationToken = default(CancellationToken)) { Context.Response.SignOut(IdentityOptions.TwoFactorRememberMeCookieAuthenticationType); return Task.FromResult(0); diff --git a/src/Microsoft.AspNet.Identity/TotpSecurityStampBasedTokenProvider.cs b/src/Microsoft.AspNet.Identity/TotpSecurityStampBasedTokenProvider.cs index 19d6732b52..57a7519cd2 100644 --- a/src/Microsoft.AspNet.Identity/TotpSecurityStampBasedTokenProvider.cs +++ b/src/Microsoft.AspNet.Identity/TotpSecurityStampBasedTokenProvider.cs @@ -42,7 +42,7 @@ namespace Microsoft.AspNet.Identity { throw new ArgumentNullException("manager"); } - var token = await manager.CreateSecurityTokenAsync(user); + var token = await manager.CreateSecurityTokenAsync(user, cancellationToken); var modifier = await GetUserModifierAsync(purpose, manager, user); return Rfc6238AuthenticationService.GenerateCode(token, modifier).ToString("D6", CultureInfo.InvariantCulture); } @@ -63,11 +63,11 @@ namespace Microsoft.AspNet.Identity throw new ArgumentNullException("manager"); } int code; - if (!Int32.TryParse(token, out code)) + if (!int.TryParse(token, out code)) { return false; } - var securityToken = await manager.CreateSecurityTokenAsync(user); + var securityToken = await manager.CreateSecurityTokenAsync(user, cancellationToken); var modifier = await GetUserModifierAsync(purpose, manager, user); return securityToken != null && Rfc6238AuthenticationService.ValidateCode(securityToken, code, modifier); } diff --git a/src/Microsoft.AspNet.Identity/UserManager.cs b/src/Microsoft.AspNet.Identity/UserManager.cs index 5fd7a7f0ad..5a95a98c85 100644 --- a/src/Microsoft.AspNet.Identity/UserManager.cs +++ b/src/Microsoft.AspNet.Identity/UserManager.cs @@ -61,7 +61,8 @@ namespace Microsoft.AspNet.Identity UserNameNormalizer = userNameNormalizer; // TODO: Email/Sms/Token services - if (tokenProviders != null) { + if (tokenProviders != null) + { foreach (var tokenProvider in tokenProviders) { RegisterTokenProvider(tokenProvider); @@ -310,7 +311,7 @@ namespace Microsoft.AspNet.Identity { await GetUserLockoutStore().SetLockoutEnabledAsync(user, true, cancellationToken); } - await UpdateNormalizedUserName(user, cancellationToken); + await UpdateNormalizedUserNameAsync(user, cancellationToken); await Store.CreateAsync(user, cancellationToken); return IdentityResult.Success; } @@ -334,7 +335,7 @@ namespace Microsoft.AspNet.Identity { return result; } - await UpdateNormalizedUserName(user, cancellationToken); + await UpdateNormalizedUserNameAsync(user, cancellationToken); await Store.UpdateAsync(user, cancellationToken); return IdentityResult.Success; } @@ -443,7 +444,7 @@ namespace Microsoft.AspNet.Identity /// /// /// - public virtual async Task UpdateNormalizedUserName(TUser user, + public virtual async Task UpdateNormalizedUserNameAsync(TUser user, CancellationToken cancellationToken = default(CancellationToken)) { var userName = await GetUserNameAsync(user, cancellationToken); @@ -489,7 +490,7 @@ namespace Microsoft.AspNet.Identity private async Task UpdateUserName(TUser user, string userName, CancellationToken cancellationToken) { await Store.SetUserNameAsync(user, userName, cancellationToken); - await UpdateNormalizedUserName(user, cancellationToken); + await UpdateNormalizedUserNameAsync(user, cancellationToken); } /// @@ -1362,7 +1363,7 @@ namespace Microsoft.AspNet.Identity { throw new ArgumentNullException("user"); } - if (!await VerifyChangePhoneNumberTokenAsync(user, token, phoneNumber)) + if (!await VerifyChangePhoneNumberTokenAsync(user, token, phoneNumber, cancellationToken)) { return IdentityResult.Failed(Resources.InvalidToken); } @@ -1392,10 +1393,10 @@ namespace Microsoft.AspNet.Identity // Two factor APIS - internal async Task CreateSecurityTokenAsync(TUser user) + internal async Task CreateSecurityTokenAsync(TUser user, CancellationToken cancellationToken) { return - new SecurityToken(Encoding.Unicode.GetBytes(await GetSecurityStampAsync(user))); + new SecurityToken(Encoding.Unicode.GetBytes(await GetSecurityStampAsync(user, cancellationToken))); } /// @@ -1404,12 +1405,13 @@ namespace Microsoft.AspNet.Identity /// /// /// - public virtual async Task GenerateChangePhoneNumberTokenAsync(TUser user, string phoneNumber) + public virtual async Task GenerateChangePhoneNumberTokenAsync(TUser user, string phoneNumber, + CancellationToken cancellationToken = default(CancellationToken)) { ThrowIfDisposed(); - return - Rfc6238AuthenticationService.GenerateCode(await CreateSecurityTokenAsync(user), phoneNumber) - .ToString(CultureInfo.InvariantCulture); + return Rfc6238AuthenticationService.GenerateCode( + await CreateSecurityTokenAsync(user, cancellationToken), phoneNumber) + .ToString(CultureInfo.InvariantCulture); } /// @@ -1419,10 +1421,11 @@ namespace Microsoft.AspNet.Identity /// /// /// - public virtual async Task VerifyChangePhoneNumberTokenAsync(TUser user, string token, string phoneNumber) + public virtual async Task VerifyChangePhoneNumberTokenAsync(TUser user, string token, string phoneNumber, + CancellationToken cancellationToken = default(CancellationToken)) { ThrowIfDisposed(); - var securityToken = await CreateSecurityTokenAsync(user); + var securityToken = await CreateSecurityTokenAsync(user, cancellationToken); int code; if (securityToken != null && Int32.TryParse(token, out code)) { diff --git a/test/Microsoft.AspNet.Identity.Test/ApiConsistencyTest.cs b/test/Microsoft.AspNet.Identity.Test/ApiConsistencyTest.cs new file mode 100644 index 0000000000..d02c0979eb --- /dev/null +++ b/test/Microsoft.AspNet.Identity.Test/ApiConsistencyTest.cs @@ -0,0 +1,17 @@ +// Copyright (c) Microsoft Open Technologies, Inc. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System.Linq; +using System.Reflection; +using Xunit; + +namespace Microsoft.AspNet.Identity.Test +{ + public class ApiConsistencyTest : ApiConsistencyTestBase + { + protected override Assembly TargetAssembly + { + get { return typeof(IdentityOptions).Assembly; } + } + } +} diff --git a/test/Shared/ApiConsistencyTestBase.cs b/test/Shared/ApiConsistencyTestBase.cs new file mode 100644 index 0000000000..11daf5f9c6 --- /dev/null +++ b/test/Shared/ApiConsistencyTestBase.cs @@ -0,0 +1,153 @@ +// Copyright (c) Microsoft Open Technologies, Inc. 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.Reflection; +using System.Threading; +using System.Threading.Tasks; +using Xunit; + +namespace Microsoft.AspNet.Identity.Test +{ + public abstract class ApiConsistencyTestBase + { + protected const BindingFlags PublicInstance + = BindingFlags.Instance | BindingFlags.Public; + + //protected const BindingFlags AnyInstance + // = BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic; + + //[Fact] + //public void Public_inheritable_apis_should_be_virtual() + //{ + // var nonVirtualMethods + // = (from type in GetAllTypes(TargetAssembly.GetTypes()) + // where type.IsVisible + // && !type.IsSealed + // && type.GetConstructors(AnyInstance).Any(c => c.IsPublic || c.IsFamily || c.IsFamilyOrAssembly) + // && type.Namespace != null + // && !type.Namespace.EndsWith(".Compiled") + // from method in type.GetMethods(PublicInstance) + // where GetBasestTypeInAssembly(method.DeclaringType) == type + // && !(method.IsVirtual && !method.IsFinal) + // select type.Name + "." + method.Name) + // .ToList(); + + // Assert.False( + // nonVirtualMethods.Any(), + // "\r\n-- Missing virtual APIs --\r\n" + string.Join("\r\n", nonVirtualMethods)); + //} + + //[Fact] + //public void Public_api_arguments_should_have_not_null_annotation() + //{ + // var parametersMissingAttribute + // = (from type in GetAllTypes(TargetAssembly.GetTypes()) + // where type.IsVisible && !typeof(Delegate).IsAssignableFrom(type) + // let interfaceMappings = type.GetInterfaces().Select(type.GetInterfaceMap) + // let events = type.GetEvents() + // from method in type.GetMethods(PublicInstance | BindingFlags.Static) + // .Concat(type.GetConstructors()) + // where GetBasestTypeInAssembly(method.DeclaringType) == type + // where type.IsInterface || !interfaceMappings.Any(im => im.TargetMethods.Contains(method)) + // where !events.Any(e => e.AddMethod == method || e.RemoveMethod == method) + // from parameter in method.GetParameters() + // where !parameter.ParameterType.IsValueType + // && !parameter.GetCustomAttributes() + // .Any( + // a => a.GetType().Name == "NotNullAttribute" + // || a.GetType().Name == "CanBeNullAttribute") + // select type.Name + "." + method.Name + "[" + parameter.Name + "]") + // .ToList(); + + // Assert.False( + // parametersMissingAttribute.Any(), + // "\r\n-- Missing NotNull annotations --\r\n" + string.Join("\r\n", parametersMissingAttribute)); + //} + + [Fact] + public void Async_methods_should_have_overload_with_cancellation_token_and_end_with_async_suffix() + { + var asyncMethods + = (from type in GetAllTypes(TargetAssembly.GetTypes()) + where type.IsVisible + from method in type.GetMethods(PublicInstance/* | BindingFlags.Static*/) + where GetBasestTypeInAssembly(method.DeclaringType) == type + where typeof(Task).IsAssignableFrom(method.ReturnType) + select method).ToList(); + + var asyncMethodsWithToken + = (from method in asyncMethods + where method.GetParameters().Any(pi => pi.ParameterType == typeof(CancellationToken)) + select method).ToList(); + + var asyncMethodsWithoutToken + = (from method in asyncMethods + where method.GetParameters().All(pi => pi.ParameterType != typeof(CancellationToken)) + select method).ToList(); + + var missingOverloads + = (from methodWithoutToken in asyncMethodsWithoutToken + where !asyncMethodsWithToken + .Any(methodWithToken => methodWithoutToken.Name == methodWithToken.Name + && methodWithoutToken.ReflectedType == methodWithToken.ReflectedType) + // ReSharper disable once PossibleNullReferenceException + select methodWithoutToken.DeclaringType.Name + "." + methodWithoutToken.Name) + .Except(GetCancellationTokenExceptions()) + .ToList(); + + Assert.False( + missingOverloads.Any(), + "\r\n-- Missing async overloads --\r\n" + string.Join("\r\n", missingOverloads)); + + var missingSuffixMethods + = asyncMethods + .Where(method => !method.Name.EndsWith("Async")) + .Select(method => method.DeclaringType.Name + "." + method.Name) + .Except(GetAsyncSuffixExceptions()) + .ToList(); + + Assert.False( + missingSuffixMethods.Any(), + "\r\n-- Missing async suffix --\r\n" + string.Join("\r\n", missingSuffixMethods)); + } + + protected virtual IEnumerable GetCancellationTokenExceptions() + { + return Enumerable.Empty(); + } + + protected virtual IEnumerable GetAsyncSuffixExceptions() + { + return Enumerable.Empty(); + } + + protected abstract Assembly TargetAssembly { get; } + + protected virtual IEnumerable GetAllTypes(IEnumerable types) + { + foreach (var type in types) + { + yield return type; + + foreach (var nestedType in GetAllTypes(type.GetNestedTypes())) + { + yield return nestedType; + } + } + } + + protected Type GetBasestTypeInAssembly(Type type) + { + while (type.BaseType != null + && type.BaseType.Assembly == type.Assembly) + { + type = type.BaseType; + } + + return type; + } + } +}