Address remaining PR feedback + misc cleanup

This commit is contained in:
Hao Kung 2015-10-15 16:51:12 -07:00
parent 409b50269a
commit 8359038927
19 changed files with 146 additions and 99 deletions

View File

@ -19,7 +19,7 @@ namespace Microsoft.AspNet.Authentication.Cookies
/// <summary>
/// The prefix used to provide a default CookieAuthenticationOptions.CookieName
/// </summary>
public const string CookiePrefix = ".AspNet.";
public static readonly string CookiePrefix = ".AspNet.";
/// <summary>
/// The default value used by CookieAuthenticationMiddleware for the
@ -45,6 +45,6 @@ namespace Microsoft.AspNet.Authentication.Cookies
/// <summary>
/// The default value of the CookieAuthenticationOptions.ReturnUrlParameter
/// </summary>
public const string ReturnUrlParameter = "ReturnUrl";
public static readonly string ReturnUrlParameter = "ReturnUrl";
}
}

View File

@ -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<AuthenticationTicket> TicketDataFormat { get; set; }
/// <summary>
/// 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.
/// </summary>
[EditorBrowsable(EditorBrowsableState.Never)]
public ISystemClock SystemClock { get; set; }
/// <summary>

View File

@ -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";
}
}

View File

@ -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";
}
}

View File

@ -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;
/// <summary>
/// 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.
/// </summary>
[EditorBrowsable(EditorBrowsableState.Never)]
public ISystemClock SystemClock { get; set; } = new SystemClock();
/// <summary>

View File

@ -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";
}
}

View File

@ -11,7 +11,7 @@ namespace Microsoft.AspNet.Authentication.OpenIdConnect
/// <summary>
/// Constant used to identify state in openIdConnect protocol message.
/// </summary>
public const string AuthenticationPropertiesKey = "OpenIdConnect.AuthenticationProperties";
public static readonly string AuthenticationPropertiesKey = "OpenIdConnect.AuthenticationProperties";
/// <summary>
/// The default value used for OpenIdConnectOptions.AuthenticationScheme.
@ -21,26 +21,26 @@ namespace Microsoft.AspNet.Authentication.OpenIdConnect
/// <summary>
/// The default value for OpenIdConnectOptions.Caption.
/// </summary>
public const string Caption = "OpenIdConnect";
public static readonly string Caption = "OpenIdConnect";
/// <summary>
/// The prefix used to for the nonce in the cookie.
/// </summary>
public const string CookieNoncePrefix = ".AspNet.OpenIdConnect.Nonce.";
public static readonly string CookieNoncePrefix = ".AspNet.OpenIdConnect.Nonce.";
/// <summary>
/// The prefix used for the state in the cookie.
/// </summary>
public const string CookieStatePrefix = ".AspNet.OpenIdConnect.State.";
public static readonly string CookieStatePrefix = ".AspNet.OpenIdConnect.State.";
/// <summary>
/// The property for the RedirectUri that was used when asking for a 'authorizationCode'.
/// </summary>
public const string RedirectUriForCodePropertiesKey = "OpenIdConnect.Code.RedirectUri";
public static readonly string RedirectUriForCodePropertiesKey = "OpenIdConnect.Code.RedirectUri";
/// <summary>
/// Constant used to identify userstate inside AuthenticationProperties that have been serialized in the 'state' parameter.
/// </summary>
public const string UserstatePropertiesKey = "OpenIdConnect.Userstate";
public static readonly string UserstatePropertiesKey = "OpenIdConnect.Userstate";
}
}

View File

@ -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;
}
/// <summary>
@ -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);

View File

@ -39,7 +39,8 @@ namespace Microsoft.AspNet.Authentication.OpenIdConnect
IUrlEncoder encoder,
IServiceProvider services,
IOptions<SharedAuthenticationOptions> 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; }
/// <summary>
/// Provides the <see cref="AuthenticationHandler"/> object for processing authentication-related requests.
/// </summary>
/// <returns>An <see cref="AuthenticationHandler"/> configured with the <see cref="OpenIdConnectOptions"/> supplied to the constructor.</returns>
protected override AuthenticationHandler<OpenIdConnectOptions> CreateHandler()
{
return new OpenIdConnectHandler(Backchannel);
return new OpenIdConnectHandler(Backchannel, HtmlEncoder);
}
private class StringSerializer : IDataSerializer<string>

View File

@ -87,12 +87,6 @@ namespace Microsoft.AspNet.Authentication.OpenIdConnect
/// </summary>
public IConfigurationManager<OpenIdConnectConfiguration> ConfigurationManager { get; set; }
/// <summary>
/// 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.
/// </summary>
public bool DefaultToCurrentUriOnRedirect { get; set; }
/// <summary>
/// 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.
/// </summary>
@ -202,10 +196,5 @@ namespace Microsoft.AspNet.Authentication.OpenIdConnect
/// You can set this property to <c>false</c> to reduce the size of the final authentication cookie.
/// </summary>
public bool SaveTokensAsClaims { get; set; } = true;
/// <summary>
/// Gets or sets the <see cref="IHtmlEncoder"/> used to sanitize HTML outputs.
/// </summary>
public IHtmlEncoder HtmlEncoder { get; set; }
}
}

View File

@ -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
/// </summary>
public class AuthenticationTicket
{
/// <summary>
/// Initializes a new instance of the <see cref="AuthenticationTicket"/> class
/// </summary>
/// <param name="properties">additional properties that can be consumed by the user or runtime.</param>
/// <param name="authenticationScheme">the authentication middleware that was responsible for this ticket.</param>
public AuthenticationTicket(AuthenticationProperties properties, string authenticationScheme) : this(null, properties, authenticationScheme) { }
/// <summary>
/// Initializes a new instance of the <see cref="AuthenticationTicket"/> class
/// </summary>
@ -26,6 +20,10 @@ namespace Microsoft.AspNet.Authentication
/// <param name="authenticationScheme">the authentication middleware that was responsible for this ticket.</param>
public AuthenticationTicket(ClaimsPrincipal principal, AuthenticationProperties properties, string authenticationScheme)
{
if (principal == null)
{
throw new ArgumentNullException(nameof(principal));
}
AuthenticationScheme = authenticationScheme;
Principal = principal;
Properties = properties ?? new AuthenticationProperties();

View File

@ -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)

View File

@ -16,6 +16,7 @@ namespace Microsoft.AspNet.Authentication
: base(context)
{
Options = options;
AuthenticationTicket = ticket;
if (ticket != null)
{
Principal = ticket.Principal;

View File

@ -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<AuthenticateResult> HandleRemoteAuthenticateAsync();

View File

@ -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<AuthenticateResult> 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<AuthenticateResult> 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<AuthenticateResult> 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")));
}
}

View File

@ -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<ArgumentNullException>(() => serializer.Write(writer, ticket));
}
}
[Fact]
public void CanRoundTripEmptyPrincipal()
{

View File

@ -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);
}

View File

@ -15,7 +15,7 @@ namespace Microsoft.AspNet.Authentication.Tests.OpenIdConnect
/// </summary>
public class OpenIdConnectHandlerForTestingAuthenticate : OpenIdConnectHandler
{
public OpenIdConnectHandlerForTestingAuthenticate() : base(null)
public OpenIdConnectHandlerForTestingAuthenticate() : base(null, null)
{
}

View File

@ -28,9 +28,10 @@ namespace Microsoft.AspNet.Authentication.Tests.OpenIdConnect
IServiceProvider services,
IOptions<SharedAuthenticationOptions> 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;