From 835903892771b9e833985c43bc3b967f66f31892 Mon Sep 17 00:00:00 2001 From: Hao Kung Date: Thu, 15 Oct 2015 16:51:12 -0700 Subject: [PATCH] Address remaining PR feedback + misc cleanup --- .../CookieAuthenticationDefaults.cs | 4 +- .../CookieAuthenticationOptions.cs | 5 +- .../FacebookDefaults.cs | 6 +- .../GoogleDefaults.cs | 6 +- .../JwtBearerOptions.cs | 5 +- .../MicrosoftAccountDefaults.cs | 6 +- .../OpenIdConnectDefaults.cs | 12 +-- .../OpenIdConnectHandler.cs | 28 +++---- .../OpenIdConnectMiddleware.cs | 17 ++-- .../OpenIdConnectOptions.cs | 11 --- .../AuthenticationTicket.cs | 12 ++- .../DataHandler/TicketSerializer.cs | 7 +- .../Events/TicketReceivedContext.cs | 1 + .../RemoteAuthenticationHandler.cs | 16 ++-- .../AuthenticationHandlerFacts.cs | 7 +- .../DataHandler/TicketSerializerTests.cs | 16 ---- .../Google/GoogleMiddlewareTests.cs | 81 ++++++++++++++++++- ...nIdConnectHandlerForTestingAuthenticate.cs | 2 +- ...ConnectMiddlewareForTestingAuthenticate.cs | 3 +- 19 files changed, 146 insertions(+), 99 deletions(-) diff --git a/src/Microsoft.AspNet.Authentication.Cookies/CookieAuthenticationDefaults.cs b/src/Microsoft.AspNet.Authentication.Cookies/CookieAuthenticationDefaults.cs index d47cca2052..715c251008 100644 --- a/src/Microsoft.AspNet.Authentication.Cookies/CookieAuthenticationDefaults.cs +++ b/src/Microsoft.AspNet.Authentication.Cookies/CookieAuthenticationDefaults.cs @@ -19,7 +19,7 @@ namespace Microsoft.AspNet.Authentication.Cookies /// /// The prefix used to provide a default CookieAuthenticationOptions.CookieName /// - public const string CookiePrefix = ".AspNet."; + public static readonly string CookiePrefix = ".AspNet."; /// /// The default value used by CookieAuthenticationMiddleware for the @@ -45,6 +45,6 @@ namespace Microsoft.AspNet.Authentication.Cookies /// /// The default value of the CookieAuthenticationOptions.ReturnUrlParameter /// - public const string ReturnUrlParameter = "ReturnUrl"; + public static readonly string ReturnUrlParameter = "ReturnUrl"; } } diff --git a/src/Microsoft.AspNet.Authentication.Cookies/CookieAuthenticationOptions.cs b/src/Microsoft.AspNet.Authentication.Cookies/CookieAuthenticationOptions.cs index 472ea80eac..8bb1d39598 100644 --- a/src/Microsoft.AspNet.Authentication.Cookies/CookieAuthenticationOptions.cs +++ b/src/Microsoft.AspNet.Authentication.Cookies/CookieAuthenticationOptions.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; +using System.ComponentModel; using System.Diagnostics.CodeAnalysis; using Microsoft.AspNet.Http; using Microsoft.Extensions.OptionsModel; @@ -133,9 +134,9 @@ namespace Microsoft.AspNet.Authentication.Cookies public ISecureDataFormat TicketDataFormat { get; set; } /// - /// The SystemClock provides access to the system's current time coordinates. If it is not provided a default instance is - /// used which calls DateTimeOffset.UtcNow. This is typically not replaced except for unit testing. + /// For testing purposes only. /// + [EditorBrowsable(EditorBrowsableState.Never)] public ISystemClock SystemClock { get; set; } /// diff --git a/src/Microsoft.AspNet.Authentication.Facebook/FacebookDefaults.cs b/src/Microsoft.AspNet.Authentication.Facebook/FacebookDefaults.cs index 765871274a..4877b25fc6 100644 --- a/src/Microsoft.AspNet.Authentication.Facebook/FacebookDefaults.cs +++ b/src/Microsoft.AspNet.Authentication.Facebook/FacebookDefaults.cs @@ -7,10 +7,10 @@ namespace Microsoft.AspNet.Authentication.Facebook { public const string AuthenticationScheme = "Facebook"; - public const string AuthorizationEndpoint = "https://www.facebook.com/v2.2/dialog/oauth"; + public static readonly string AuthorizationEndpoint = "https://www.facebook.com/v2.2/dialog/oauth"; - public const string TokenEndpoint = "https://graph.facebook.com/v2.2/oauth/access_token"; + public static readonly string TokenEndpoint = "https://graph.facebook.com/v2.2/oauth/access_token"; - public const string UserInformationEndpoint = "https://graph.facebook.com/v2.2/me"; + public static readonly string UserInformationEndpoint = "https://graph.facebook.com/v2.2/me"; } } diff --git a/src/Microsoft.AspNet.Authentication.Google/GoogleDefaults.cs b/src/Microsoft.AspNet.Authentication.Google/GoogleDefaults.cs index 4085778d35..28fa46bc57 100644 --- a/src/Microsoft.AspNet.Authentication.Google/GoogleDefaults.cs +++ b/src/Microsoft.AspNet.Authentication.Google/GoogleDefaults.cs @@ -7,10 +7,10 @@ namespace Microsoft.AspNet.Authentication.Google { public const string AuthenticationScheme = "Google"; - public const string AuthorizationEndpoint = "https://accounts.google.com/o/oauth2/auth"; + public static readonly string AuthorizationEndpoint = "https://accounts.google.com/o/oauth2/auth"; - public const string TokenEndpoint = "https://accounts.google.com/o/oauth2/token"; + public static readonly string TokenEndpoint = "https://accounts.google.com/o/oauth2/token"; - public const string UserInformationEndpoint = "https://www.googleapis.com/plus/v1/people/me"; + public static readonly string UserInformationEndpoint = "https://www.googleapis.com/plus/v1/people/me"; } } diff --git a/src/Microsoft.AspNet.Authentication.JwtBearer/JwtBearerOptions.cs b/src/Microsoft.AspNet.Authentication.JwtBearer/JwtBearerOptions.cs index 5aca4a1392..bd1f9ded6e 100644 --- a/src/Microsoft.AspNet.Authentication.JwtBearer/JwtBearerOptions.cs +++ b/src/Microsoft.AspNet.Authentication.JwtBearer/JwtBearerOptions.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.ComponentModel; using System.IdentityModel.Tokens; using System.IdentityModel.Tokens.Jwt; using System.Net.Http; @@ -85,9 +86,9 @@ namespace Microsoft.AspNet.Authentication.JwtBearer public bool RefreshOnIssuerKeyNotFound { get; set; } = true; /// - /// Used to know what the current clock time is when calculating or validating token expiration. When not assigned default is based on - /// DateTimeOffset.UtcNow. This is typically needed only for unit testing. + /// For testing purposes only. /// + [EditorBrowsable(EditorBrowsableState.Never)] public ISystemClock SystemClock { get; set; } = new SystemClock(); /// diff --git a/src/Microsoft.AspNet.Authentication.MicrosoftAccount/MicrosoftAccountDefaults.cs b/src/Microsoft.AspNet.Authentication.MicrosoftAccount/MicrosoftAccountDefaults.cs index 4cfd9dda03..be0732ae6d 100644 --- a/src/Microsoft.AspNet.Authentication.MicrosoftAccount/MicrosoftAccountDefaults.cs +++ b/src/Microsoft.AspNet.Authentication.MicrosoftAccount/MicrosoftAccountDefaults.cs @@ -7,10 +7,10 @@ namespace Microsoft.AspNet.Authentication.MicrosoftAccount { public const string AuthenticationScheme = "Microsoft"; - public const string AuthorizationEndpoint = "https://login.live.com/oauth20_authorize.srf"; + public static readonly string AuthorizationEndpoint = "https://login.live.com/oauth20_authorize.srf"; - public const string TokenEndpoint = "https://login.live.com/oauth20_token.srf"; + public static readonly string TokenEndpoint = "https://login.live.com/oauth20_token.srf"; - public const string UserInformationEndpoint = "https://apis.live.net/v5.0/me"; + public static readonly string UserInformationEndpoint = "https://apis.live.net/v5.0/me"; } } diff --git a/src/Microsoft.AspNet.Authentication.OpenIdConnect/OpenIdConnectDefaults.cs b/src/Microsoft.AspNet.Authentication.OpenIdConnect/OpenIdConnectDefaults.cs index 3ddeb0fc88..e280a6afb1 100644 --- a/src/Microsoft.AspNet.Authentication.OpenIdConnect/OpenIdConnectDefaults.cs +++ b/src/Microsoft.AspNet.Authentication.OpenIdConnect/OpenIdConnectDefaults.cs @@ -11,7 +11,7 @@ namespace Microsoft.AspNet.Authentication.OpenIdConnect /// /// Constant used to identify state in openIdConnect protocol message. /// - public const string AuthenticationPropertiesKey = "OpenIdConnect.AuthenticationProperties"; + public static readonly string AuthenticationPropertiesKey = "OpenIdConnect.AuthenticationProperties"; /// /// The default value used for OpenIdConnectOptions.AuthenticationScheme. @@ -21,26 +21,26 @@ namespace Microsoft.AspNet.Authentication.OpenIdConnect /// /// The default value for OpenIdConnectOptions.Caption. /// - public const string Caption = "OpenIdConnect"; + public static readonly string Caption = "OpenIdConnect"; /// /// The prefix used to for the nonce in the cookie. /// - public const string CookieNoncePrefix = ".AspNet.OpenIdConnect.Nonce."; + public static readonly string CookieNoncePrefix = ".AspNet.OpenIdConnect.Nonce."; /// /// The prefix used for the state in the cookie. /// - public const string CookieStatePrefix = ".AspNet.OpenIdConnect.State."; + public static readonly string CookieStatePrefix = ".AspNet.OpenIdConnect.State."; /// /// The property for the RedirectUri that was used when asking for a 'authorizationCode'. /// - public const string RedirectUriForCodePropertiesKey = "OpenIdConnect.Code.RedirectUri"; + public static readonly string RedirectUriForCodePropertiesKey = "OpenIdConnect.Code.RedirectUri"; /// /// Constant used to identify userstate inside AuthenticationProperties that have been serialized in the 'state' parameter. /// - public const string UserstatePropertiesKey = "OpenIdConnect.Userstate"; + public static readonly string UserstatePropertiesKey = "OpenIdConnect.Userstate"; } } diff --git a/src/Microsoft.AspNet.Authentication.OpenIdConnect/OpenIdConnectHandler.cs b/src/Microsoft.AspNet.Authentication.OpenIdConnect/OpenIdConnectHandler.cs index 9a62cba956..c6abe26eef 100644 --- a/src/Microsoft.AspNet.Authentication.OpenIdConnect/OpenIdConnectHandler.cs +++ b/src/Microsoft.AspNet.Authentication.OpenIdConnect/OpenIdConnectHandler.cs @@ -17,6 +17,7 @@ using Microsoft.AspNet.Http; using Microsoft.AspNet.Http.Authentication; using Microsoft.AspNet.Http.Features.Authentication; using Microsoft.Extensions.Logging; +using Microsoft.Extensions.WebEncoders; using Microsoft.IdentityModel.Protocols.OpenIdConnect; using Microsoft.Net.Http.Headers; using Newtonsoft.Json.Linq; @@ -52,9 +53,12 @@ namespace Microsoft.AspNet.Authentication.OpenIdConnect protected HttpClient Backchannel { get; private set; } - public OpenIdConnectHandler(HttpClient backchannel) + protected IHtmlEncoder HtmlEncoder { get; private set; } + + public OpenIdConnectHandler(HttpClient backchannel, IHtmlEncoder htmlEncoder) { Backchannel = backchannel; + HtmlEncoder = htmlEncoder; } /// @@ -129,14 +133,14 @@ namespace Microsoft.AspNet.Authentication.OpenIdConnect var inputs = new StringBuilder(); foreach (var parameter in message.Parameters) { - var name = Options.HtmlEncoder.HtmlEncode(parameter.Key); - var value = Options.HtmlEncoder.HtmlEncode(parameter.Value); + var name = HtmlEncoder.HtmlEncode(parameter.Key); + var value = HtmlEncoder.HtmlEncode(parameter.Value); var input = string.Format(CultureInfo.InvariantCulture, InputTagFormat, name, value); inputs.AppendLine(input); } - var issuer = Options.HtmlEncoder.HtmlEncode(message.IssuerAddress); + var issuer = HtmlEncoder.HtmlEncode(message.IssuerAddress); var content = string.Format(CultureInfo.InvariantCulture, HtmlFormFormat, issuer, inputs); var buffer = Encoding.UTF8.GetBytes(content); @@ -170,18 +174,14 @@ namespace Microsoft.AspNet.Authentication.OpenIdConnect // order for local RedirectUri // 1. challenge.Properties.RedirectUri - // 2. CurrentUri if Options.DefaultToCurrentUriOnRedirect is true) + // 2. CurrentUri if RedirectUri is not set) var properties = new AuthenticationProperties(context.Properties); - if (!string.IsNullOrEmpty(properties.RedirectUri)) + if (string.IsNullOrEmpty(properties.RedirectUri)) { - Logger.LogDebug(Resources.OIDCH_0030_Using_Properties_RedirectUri, properties.RedirectUri); - } - else if (Options.DefaultToCurrentUriOnRedirect) - { - Logger.LogDebug(Resources.OIDCH_0032_UsingCurrentUriRedirectUri, CurrentUri); properties.RedirectUri = CurrentUri; } + Logger.LogDebug(Resources.OIDCH_0030_Using_Properties_RedirectUri, properties.RedirectUri); if (_configuration == null && Options.ConfigurationManager != null) { @@ -270,14 +270,14 @@ namespace Microsoft.AspNet.Authentication.OpenIdConnect var inputs = new StringBuilder(); foreach (var parameter in message.Parameters) { - var name = Options.HtmlEncoder.HtmlEncode(parameter.Key); - var value = Options.HtmlEncoder.HtmlEncode(parameter.Value); + var name = HtmlEncoder.HtmlEncode(parameter.Key); + var value = HtmlEncoder.HtmlEncode(parameter.Value); var input = string.Format(CultureInfo.InvariantCulture, InputTagFormat, name, value); inputs.AppendLine(input); } - var issuer = Options.HtmlEncoder.HtmlEncode(message.IssuerAddress); + var issuer = HtmlEncoder.HtmlEncode(message.IssuerAddress); var content = string.Format(CultureInfo.InvariantCulture, HtmlFormFormat, issuer, inputs); var buffer = Encoding.UTF8.GetBytes(content); diff --git a/src/Microsoft.AspNet.Authentication.OpenIdConnect/OpenIdConnectMiddleware.cs b/src/Microsoft.AspNet.Authentication.OpenIdConnect/OpenIdConnectMiddleware.cs index 82a393ae4b..c0ecff39a8 100644 --- a/src/Microsoft.AspNet.Authentication.OpenIdConnect/OpenIdConnectMiddleware.cs +++ b/src/Microsoft.AspNet.Authentication.OpenIdConnect/OpenIdConnectMiddleware.cs @@ -39,7 +39,8 @@ namespace Microsoft.AspNet.Authentication.OpenIdConnect IUrlEncoder encoder, IServiceProvider services, IOptions sharedOptions, - OpenIdConnectOptions options) + OpenIdConnectOptions options, + IHtmlEncoder htmlEncoder) : base(next, options, loggerFactory, encoder) { if (next == null) @@ -77,6 +78,11 @@ namespace Microsoft.AspNet.Authentication.OpenIdConnect throw new ArgumentNullException(nameof(options)); } + if (htmlEncoder == null) + { + throw new ArgumentNullException(nameof(htmlEncoder)); + } + if (!Options.CallbackPath.HasValue) { throw new ArgumentException("Options.CallbackPath must be provided."); @@ -91,10 +97,7 @@ namespace Microsoft.AspNet.Authentication.OpenIdConnect throw new ArgumentException("Options.SignInScheme is required."); } - if (Options.HtmlEncoder == null) - { - Options.HtmlEncoder = services.GetHtmlEncoder(); - } + HtmlEncoder = htmlEncoder; if (Options.StateDataFormat == null) { @@ -170,13 +173,15 @@ namespace Microsoft.AspNet.Authentication.OpenIdConnect protected HttpClient Backchannel { get; private set; } + protected IHtmlEncoder HtmlEncoder { get; private set; } + /// /// Provides the object for processing authentication-related requests. /// /// An configured with the supplied to the constructor. protected override AuthenticationHandler CreateHandler() { - return new OpenIdConnectHandler(Backchannel); + return new OpenIdConnectHandler(Backchannel, HtmlEncoder); } private class StringSerializer : IDataSerializer diff --git a/src/Microsoft.AspNet.Authentication.OpenIdConnect/OpenIdConnectOptions.cs b/src/Microsoft.AspNet.Authentication.OpenIdConnect/OpenIdConnectOptions.cs index c79451d225..b310ac4c2f 100644 --- a/src/Microsoft.AspNet.Authentication.OpenIdConnect/OpenIdConnectOptions.cs +++ b/src/Microsoft.AspNet.Authentication.OpenIdConnect/OpenIdConnectOptions.cs @@ -87,12 +87,6 @@ namespace Microsoft.AspNet.Authentication.OpenIdConnect /// public IConfigurationManager ConfigurationManager { get; set; } - /// - /// Gets or sets a value controlling if the 'CurrentUri' should be used as the 'local redirect' post authentication - /// if AuthenticationProperties.RedirectUri is null or empty. - /// - public bool DefaultToCurrentUriOnRedirect { get; set; } - /// /// Boolean to set whether the middleware should go to user info endpoint to retrieve additional claims or not after creating an identity from id_token received from token endpoint. /// @@ -202,10 +196,5 @@ namespace Microsoft.AspNet.Authentication.OpenIdConnect /// You can set this property to false to reduce the size of the final authentication cookie. /// public bool SaveTokensAsClaims { get; set; } = true; - - /// - /// Gets or sets the used to sanitize HTML outputs. - /// - public IHtmlEncoder HtmlEncoder { get; set; } } } diff --git a/src/Microsoft.AspNet.Authentication/AuthenticationTicket.cs b/src/Microsoft.AspNet.Authentication/AuthenticationTicket.cs index e72385d879..dffcebc08e 100644 --- a/src/Microsoft.AspNet.Authentication/AuthenticationTicket.cs +++ b/src/Microsoft.AspNet.Authentication/AuthenticationTicket.cs @@ -1,6 +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 System; using System.Security.Claims; using Microsoft.AspNet.Http.Authentication; @@ -11,13 +12,6 @@ namespace Microsoft.AspNet.Authentication /// public class AuthenticationTicket { - /// - /// Initializes a new instance of the class - /// - /// additional properties that can be consumed by the user or runtime. - /// the authentication middleware that was responsible for this ticket. - public AuthenticationTicket(AuthenticationProperties properties, string authenticationScheme) : this(null, properties, authenticationScheme) { } - /// /// Initializes a new instance of the class /// @@ -26,6 +20,10 @@ namespace Microsoft.AspNet.Authentication /// the authentication middleware that was responsible for this ticket. public AuthenticationTicket(ClaimsPrincipal principal, AuthenticationProperties properties, string authenticationScheme) { + if (principal == null) + { + throw new ArgumentNullException(nameof(principal)); + } AuthenticationScheme = authenticationScheme; Principal = principal; Properties = properties ?? new AuthenticationProperties(); diff --git a/src/Microsoft.AspNet.Authentication/DataHandler/TicketSerializer.cs b/src/Microsoft.AspNet.Authentication/DataHandler/TicketSerializer.cs index 44d304004f..b27027d11b 100644 --- a/src/Microsoft.AspNet.Authentication/DataHandler/TicketSerializer.cs +++ b/src/Microsoft.AspNet.Authentication/DataHandler/TicketSerializer.cs @@ -53,13 +53,8 @@ namespace Microsoft.AspNet.Authentication writer.Write(FormatVersion); writer.Write(ticket.AuthenticationScheme); - var principal = ticket.Principal; - if (principal == null) - { - throw new ArgumentNullException("model.Principal"); - } - // Write the number of identities contained in the principal. + var principal = ticket.Principal; writer.Write(principal.Identities.Count()); foreach (var identity in principal.Identities) diff --git a/src/Microsoft.AspNet.Authentication/Events/TicketReceivedContext.cs b/src/Microsoft.AspNet.Authentication/Events/TicketReceivedContext.cs index 2021209820..b2c5adbc58 100644 --- a/src/Microsoft.AspNet.Authentication/Events/TicketReceivedContext.cs +++ b/src/Microsoft.AspNet.Authentication/Events/TicketReceivedContext.cs @@ -16,6 +16,7 @@ namespace Microsoft.AspNet.Authentication : base(context) { Options = options; + AuthenticationTicket = ticket; if (ticket != null) { Principal = ticket.Principal; diff --git a/src/Microsoft.AspNet.Authentication/RemoteAuthenticationHandler.cs b/src/Microsoft.AspNet.Authentication/RemoteAuthenticationHandler.cs index d9563e628f..7e382edd06 100644 --- a/src/Microsoft.AspNet.Authentication/RemoteAuthenticationHandler.cs +++ b/src/Microsoft.AspNet.Authentication/RemoteAuthenticationHandler.cs @@ -62,18 +62,16 @@ namespace Microsoft.AspNet.Authentication return false; } - if (context.Principal != null) + await Context.Authentication.SignInAsync(Options.SignInScheme, context.Principal, context.Properties); + + // Default redirect path is the base path + if (string.IsNullOrEmpty(context.ReturnUri)) { - await Context.Authentication.SignInAsync(Options.SignInScheme, context.Principal, context.Properties); + context.ReturnUri = "/"; } - if (context.ReturnUri != null) - { - Response.Redirect(context.ReturnUri); - return true; - } - - return false; + Response.Redirect(context.ReturnUri); + return true; } protected abstract Task HandleRemoteAuthenticateAsync(); diff --git a/test/Microsoft.AspNet.Authentication.Test/AuthenticationHandlerFacts.cs b/test/Microsoft.AspNet.Authentication.Test/AuthenticationHandlerFacts.cs index e8423dbba1..765065045a 100644 --- a/test/Microsoft.AspNet.Authentication.Test/AuthenticationHandlerFacts.cs +++ b/test/Microsoft.AspNet.Authentication.Test/AuthenticationHandlerFacts.cs @@ -3,6 +3,7 @@ using System; using System.IO; +using System.Security.Claims; using System.Threading.Tasks; using Microsoft.AspNet.Http; using Microsoft.AspNet.Http.Authentication; @@ -98,7 +99,7 @@ namespace Microsoft.AspNet.Authentication protected override Task HandleAuthenticateAsync() { AuthCount++; - return Task.FromResult(AuthenticateResult.Success(new AuthenticationTicket(new Http.Authentication.AuthenticationProperties(), "whatever"))); + return Task.FromResult(AuthenticateResult.Success(new AuthenticationTicket(new ClaimsPrincipal(), new AuthenticationProperties(), "whatever"))); } } @@ -122,7 +123,7 @@ namespace Microsoft.AspNet.Authentication protected override Task HandleAuthenticateAsync() { - return Task.FromResult(AuthenticateResult.Success(new AuthenticationTicket(new Http.Authentication.AuthenticationProperties(), "whatever"))); + return Task.FromResult(AuthenticateResult.Success(new AuthenticationTicket(new ClaimsPrincipal(), new AuthenticationProperties(), "whatever"))); } } @@ -156,7 +157,7 @@ namespace Microsoft.AspNet.Authentication protected override Task HandleAuthenticateAsync() { - return Task.FromResult(AuthenticateResult.Success(new AuthenticationTicket(new Http.Authentication.AuthenticationProperties(), "whatever"))); + return Task.FromResult(AuthenticateResult.Success(new AuthenticationTicket(new ClaimsPrincipal(), new AuthenticationProperties(), "whatever"))); } } diff --git a/test/Microsoft.AspNet.Authentication.Test/DataHandler/TicketSerializerTests.cs b/test/Microsoft.AspNet.Authentication.Test/DataHandler/TicketSerializerTests.cs index a218209d44..0ab8bce417 100644 --- a/test/Microsoft.AspNet.Authentication.Test/DataHandler/TicketSerializerTests.cs +++ b/test/Microsoft.AspNet.Authentication.Test/DataHandler/TicketSerializerTests.cs @@ -13,22 +13,6 @@ namespace Microsoft.AspNet.Authentication { public class TicketSerializerTests { - [Fact] - public void NullPrincipalThrows() - { - var serializer = new TicketSerializer(); - var properties = new AuthenticationProperties(); - properties.RedirectUri = "bye"; - var ticket = new AuthenticationTicket(properties, "Hello"); - - using (var stream = new MemoryStream()) - using (var writer = new BinaryWriter(stream)) - using (var reader = new BinaryReader(stream)) - { - Assert.Throws(() => serializer.Write(writer, ticket)); - } - } - [Fact] public void CanRoundTripEmptyPrincipal() { diff --git a/test/Microsoft.AspNet.Authentication.Test/Google/GoogleMiddlewareTests.cs b/test/Microsoft.AspNet.Authentication.Test/Google/GoogleMiddlewareTests.cs index 48d513ad7b..9a19a5635b 100644 --- a/test/Microsoft.AspNet.Authentication.Test/Google/GoogleMiddlewareTests.cs +++ b/test/Microsoft.AspNet.Authentication.Test/Google/GoogleMiddlewareTests.cs @@ -263,7 +263,7 @@ namespace Microsoft.AspNet.Authentication.Google { OnRemoteError = ctx => { - ctx.Response.Redirect("/error?ErrorMessage=" + ctx.Error.Message); + ctx.Response.Redirect("/error?ErrorMessage=" + UrlEncoder.Default.UrlEncode(ctx.Error.Message)); ctx.HandleResponse(); return Task.FromResult(0); } @@ -388,7 +388,7 @@ namespace Microsoft.AspNet.Authentication.Google { OnRemoteError = ctx => { - ctx.Response.Redirect("/error?ErrorMessage=" + ctx.Error.Message); + ctx.Response.Redirect("/error?ErrorMessage=" + UrlEncoder.Default.UrlEncode(ctx.Error.Message)); ctx.HandleResponse(); return Task.FromResult(0); } @@ -446,7 +446,7 @@ namespace Microsoft.AspNet.Authentication.Google { OnRemoteError = ctx => { - ctx.Response.Redirect("/error?ErrorMessage=" + ctx.Error.Message); + ctx.Response.Redirect("/error?ErrorMessage=" + UrlEncoder.Default.UrlEncode(ctx.Error.Message)); ctx.HandleResponse(); return Task.FromResult(0); } @@ -554,6 +554,79 @@ namespace Microsoft.AspNet.Authentication.Google Assert.Equal("Test Refresh Token", transaction.FindClaimValue("RefreshToken")); } + [Fact] + public async Task NullRedirectUriWillRedirectToSlash() + { + var stateFormat = new PropertiesDataFormat(new EphemeralDataProtectionProvider().CreateProtector("GoogleTest")); + var server = CreateServer(options => + { + options.ClientId = "Test Id"; + options.ClientSecret = "Test Secret"; + options.StateDataFormat = stateFormat; + options.BackchannelHttpHandler = new TestHttpMessageHandler + { + Sender = req => + { + if (req.RequestUri.AbsoluteUri == "https://accounts.google.com/o/oauth2/token") + { + return ReturnJsonResponse(new + { + access_token = "Test Access Token", + expires_in = 3600, + token_type = "Bearer", + refresh_token = "Test Refresh Token" + }); + } + else if (req.RequestUri.GetLeftPart(UriPartial.Path) == "https://www.googleapis.com/plus/v1/people/me") + { + return ReturnJsonResponse(new + { + id = "Test User ID", + displayName = "Test Name", + name = new + { + familyName = "Test Family Name", + givenName = "Test Given Name" + }, + url = "Profile link", + emails = new[] + { + new + { + value = "Test email", + type = "account" + } + } + }); + } + + return null; + } + }; + options.Events = new OAuthEvents + { + OnTicketReceived = context => + { + context.AuthenticationTicket.Properties.RedirectUri = null; + return Task.FromResult(0); + } + }; + }); + var properties = new AuthenticationProperties(); + var correlationKey = ".AspNet.Correlation.Google"; + var correlationValue = "TestCorrelationId"; + properties.Items.Add(correlationKey, correlationValue); + var state = stateFormat.Protect(properties); + var transaction = await server.SendAsync( + "https://example.com/signin-google?code=TestCode&state=" + UrlEncoder.Default.UrlEncode(state), + correlationKey + "=" + correlationValue); + Assert.Equal(HttpStatusCode.Redirect, transaction.Response.StatusCode); + Assert.Equal("/", transaction.Response.Headers.GetValues("Location").First()); + Assert.Equal(2, transaction.SetCookie.Count); + Assert.Contains(correlationKey, transaction.SetCookie[0]); + Assert.Contains(".AspNet." + TestExtensions.CookieAuthenticationScheme, transaction.SetCookie[1]); + } + [Fact] public async Task ValidateAuthenticatedContext() { @@ -667,7 +740,7 @@ namespace Microsoft.AspNet.Authentication.Google { OnRemoteError = ctx => { - ctx.Response.Redirect("/error?ErrorMessage=" + ctx.Error.Message); + ctx.Response.Redirect("/error?ErrorMessage=" + UrlEncoder.Default.UrlEncode(ctx.Error.Message)); ctx.HandleResponse(); return Task.FromResult(0); } diff --git a/test/Microsoft.AspNet.Authentication.Test/OpenIdConnect/OpenIdConnectHandlerForTestingAuthenticate.cs b/test/Microsoft.AspNet.Authentication.Test/OpenIdConnect/OpenIdConnectHandlerForTestingAuthenticate.cs index ccdd74e3ff..0be9b7d69b 100644 --- a/test/Microsoft.AspNet.Authentication.Test/OpenIdConnect/OpenIdConnectHandlerForTestingAuthenticate.cs +++ b/test/Microsoft.AspNet.Authentication.Test/OpenIdConnect/OpenIdConnectHandlerForTestingAuthenticate.cs @@ -15,7 +15,7 @@ namespace Microsoft.AspNet.Authentication.Tests.OpenIdConnect /// public class OpenIdConnectHandlerForTestingAuthenticate : OpenIdConnectHandler { - public OpenIdConnectHandlerForTestingAuthenticate() : base(null) + public OpenIdConnectHandlerForTestingAuthenticate() : base(null, null) { } diff --git a/test/Microsoft.AspNet.Authentication.Test/OpenIdConnect/OpenIdConnectMiddlewareForTestingAuthenticate.cs b/test/Microsoft.AspNet.Authentication.Test/OpenIdConnect/OpenIdConnectMiddlewareForTestingAuthenticate.cs index b144e838d9..c39e0798d9 100644 --- a/test/Microsoft.AspNet.Authentication.Test/OpenIdConnect/OpenIdConnectMiddlewareForTestingAuthenticate.cs +++ b/test/Microsoft.AspNet.Authentication.Test/OpenIdConnect/OpenIdConnectMiddlewareForTestingAuthenticate.cs @@ -28,9 +28,10 @@ namespace Microsoft.AspNet.Authentication.Tests.OpenIdConnect IServiceProvider services, IOptions sharedOptions, OpenIdConnectOptions options, + IHtmlEncoder htmlEncoder, OpenIdConnectHandler handler = null ) - : base(next, dataProtectionProvider, loggerFactory, encoder, services, sharedOptions, options) + : base(next, dataProtectionProvider, loggerFactory, encoder, services, sharedOptions, options, htmlEncoder) { _handler = handler; var customFactory = loggerFactory as InMemoryLoggerFactory;