diff --git a/src/Microsoft.AspNetCore.Authentication.OpenIdConnect/OpenIdConnectHandler.cs b/src/Microsoft.AspNetCore.Authentication.OpenIdConnect/OpenIdConnectHandler.cs index 341abbf5a..9e6d34bc9 100644 --- a/src/Microsoft.AspNetCore.Authentication.OpenIdConnect/OpenIdConnectHandler.cs +++ b/src/Microsoft.AspNetCore.Authentication.OpenIdConnect/OpenIdConnectHandler.cs @@ -275,8 +275,7 @@ public async virtual Task SignOutAsync(AuthenticationProperties properties) /// A task executing the callback procedure protected virtual Task HandleSignOutCallbackAsync() { - StringValues protectedState; - if (Request.Query.TryGetValue(OpenIdConnectParameterNames.State, out protectedState)) + if (Request.Query.TryGetValue(OpenIdConnectParameterNames.State, out StringValues protectedState)) { var properties = Options.StateDataFormat.Unprotect(protectedState); if (!string.IsNullOrEmpty(properties?.RedirectUri)) @@ -505,8 +504,7 @@ protected override async Task HandleRemoteAuthenticateAsync return HandleRequestResult.Fail(Resources.MessageStateIsInvalid); } - string userstate = null; - properties.Items.TryGetValue(OpenIdConnectDefaults.UserstatePropertiesKey, out userstate); + properties.Items.TryGetValue(OpenIdConnectDefaults.UserstatePropertiesKey, out string userstate); authorizationResponse.State = userstate; if (!ValidateCorrelationId(properties)) @@ -859,8 +857,7 @@ private void SaveTokens(AuthenticationProperties properties, OpenIdConnectMessag if (!string.IsNullOrEmpty(message.ExpiresIn)) { - int value; - if (int.TryParse(message.ExpiresIn, NumberStyles.Integer, CultureInfo.InvariantCulture, out value)) + if (int.TryParse(message.ExpiresIn, NumberStyles.Integer, CultureInfo.InvariantCulture, out int value)) { var expiresAt = Clock.UtcNow + TimeSpan.FromSeconds(value); // https://www.w3.org/TR/xmlschema-2/#dateTime @@ -885,21 +882,12 @@ private void WriteNonceCookie(string nonce) throw new ArgumentNullException(nameof(nonce)); } - var options = new CookieOptions - { - HttpOnly = true, - SameSite = Http.SameSiteMode.None, - Path = OriginalPathBase + Options.CallbackPath, - Secure = Request.IsHttps, - Expires = Clock.UtcNow.Add(Options.ProtocolValidator.NonceLifetime) - }; - - Options.ConfigureNonceCookie?.Invoke(Context, options); + var cookieOptions = Options.NonceCookie.Build(Context, Clock.UtcNow); Response.Cookies.Append( - OpenIdConnectDefaults.CookieNoncePrefix + Options.StringDataFormat.Protect(nonce), + Options.NonceCookie.Name + Options.StringDataFormat.Protect(nonce), NonceProperty, - options); + cookieOptions); } /// @@ -918,23 +906,14 @@ private string ReadNonceCookie(string nonce) foreach (var nonceKey in Request.Cookies.Keys) { - if (nonceKey.StartsWith(OpenIdConnectDefaults.CookieNoncePrefix)) + if (nonceKey.StartsWith(Options.NonceCookie.Name)) { try { - var nonceDecodedValue = Options.StringDataFormat.Unprotect(nonceKey.Substring(OpenIdConnectDefaults.CookieNoncePrefix.Length, nonceKey.Length - OpenIdConnectDefaults.CookieNoncePrefix.Length)); + var nonceDecodedValue = Options.StringDataFormat.Unprotect(nonceKey.Substring(Options.NonceCookie.Name.Length, nonceKey.Length - Options.NonceCookie.Name.Length)); if (nonceDecodedValue == nonce) { - var cookieOptions = new CookieOptions - { - HttpOnly = true, - Path = OriginalPathBase + Options.CallbackPath, - SameSite = Http.SameSiteMode.None, - Secure = Request.IsHttps - }; - - Options.ConfigureNonceCookie?.Invoke(Context, cookieOptions); - + var cookieOptions = Options.NonceCookie.Build(Context, Clock.UtcNow); Response.Cookies.Delete(nonceKey, cookieOptions); return nonce; } @@ -1170,8 +1149,7 @@ private ClaimsPrincipal ValidateToken(string idToken, AuthenticationProperties p validationParameters.IssuerSigningKeys = validationParameters.IssuerSigningKeys?.Concat(_configuration.SigningKeys) ?? _configuration.SigningKeys; } - SecurityToken validatedToken = null; - var principal = Options.SecurityTokenValidator.ValidateToken(idToken, validationParameters, out validatedToken); + var principal = Options.SecurityTokenValidator.ValidateToken(idToken, validationParameters, out SecurityToken validatedToken); jwt = validatedToken as JwtSecurityToken; if (jwt == null) { diff --git a/src/Microsoft.AspNetCore.Authentication.OpenIdConnect/OpenIdConnectOptions.cs b/src/Microsoft.AspNetCore.Authentication.OpenIdConnect/OpenIdConnectOptions.cs index 8bcedaec2..23169b5bc 100644 --- a/src/Microsoft.AspNetCore.Authentication.OpenIdConnect/OpenIdConnectOptions.cs +++ b/src/Microsoft.AspNetCore.Authentication.OpenIdConnect/OpenIdConnectOptions.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Generic; using System.IdentityModel.Tokens.Jwt; +using Microsoft.AspNetCore.Authentication.Internal; using Microsoft.AspNetCore.Authentication.OAuth.Claims; using Microsoft.AspNetCore.Http; using Microsoft.IdentityModel.Protocols; @@ -17,6 +18,8 @@ namespace Microsoft.AspNetCore.Authentication.OpenIdConnect /// public class OpenIdConnectOptions : RemoteAuthenticationOptions { + private CookieBuilder _nonceCookieBuilder; + /// /// Initializes a new /// @@ -65,6 +68,14 @@ public OpenIdConnectOptions() ClaimActions.MapUniqueJsonKey("family_name", "family_name"); ClaimActions.MapUniqueJsonKey("profile", "profile"); ClaimActions.MapUniqueJsonKey("email", "email"); + + _nonceCookieBuilder = new OpenIdConnectNonceCookieBuilder(this) + { + Name = OpenIdConnectDefaults.CookieNoncePrefix, + HttpOnly = true, + SameSite = SameSiteMode.None, + SecurePolicy = CookieSecurePolicy.SameAsRequest, + }; } /// @@ -145,8 +156,8 @@ public override void Validate() /// public new OpenIdConnectEvents Events { - get { return (OpenIdConnectEvents)base.Events; } - set { base.Events = value; } + get => (OpenIdConnectEvents)base.Events; + set => base.Events = value; } /// @@ -259,9 +270,40 @@ public override void Validate() public bool DisableTelemetry { get; set; } /// - /// Gets or sets an action that can override the nonce cookie options before the + /// Determines the settings used to create the nonce cookie before the /// cookie gets added to the response. /// - public Action ConfigureNonceCookie { get; set; } + /// + /// The value of is treated as the prefix to the cookie name, and defaults to . + /// + public CookieBuilder NonceCookie + { + get => _nonceCookieBuilder; + set => _nonceCookieBuilder = value ?? throw new ArgumentNullException(nameof(value)); + } + + private class OpenIdConnectNonceCookieBuilder : RequestPathBaseCookieBuilder + { + private readonly OpenIdConnectOptions _options; + + public OpenIdConnectNonceCookieBuilder(OpenIdConnectOptions oidcOptions) + { + _options = oidcOptions; + } + + protected override string AdditionalPath => _options.CallbackPath; + + public override CookieOptions Build(HttpContext context, DateTimeOffset expiresFrom) + { + var cookieOptions = base.Build(context, expiresFrom); + + if (!Expiration.HasValue || !cookieOptions.Expires.HasValue) + { + cookieOptions.Expires = expiresFrom.Add(_options.ProtocolValidator.NonceLifetime); + } + + return cookieOptions; + } + } } } diff --git a/src/Microsoft.AspNetCore.Authentication.Twitter/TwitterHandler.cs b/src/Microsoft.AspNetCore.Authentication.Twitter/TwitterHandler.cs index 7fcc01eee..ddcd095d5 100644 --- a/src/Microsoft.AspNetCore.Authentication.Twitter/TwitterHandler.cs +++ b/src/Microsoft.AspNetCore.Authentication.Twitter/TwitterHandler.cs @@ -10,7 +10,6 @@ using System.Text; using System.Text.Encodings.Web; using System.Threading.Tasks; -using Microsoft.AspNetCore.DataProtection; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.WebUtilities; using Microsoft.Extensions.Logging; @@ -23,7 +22,6 @@ namespace Microsoft.AspNetCore.Authentication.Twitter internal class TwitterHandler : RemoteAuthenticationHandler { private static readonly DateTime Epoch = new DateTime(1970, 1, 1, 0, 0, 0, DateTimeKind.Utc); - private const string StateCookie = "__TwitterState"; private const string RequestTokenEndpoint = "https://api.twitter.com/oauth/request_token"; private const string AuthenticationEndpoint = "https://api.twitter.com/oauth/authenticate?oauth_token="; private const string AccessTokenEndpoint = "https://api.twitter.com/oauth/access_token"; @@ -50,7 +48,7 @@ protected override async Task HandleRemoteAuthenticateAsync { AuthenticationProperties properties = null; var query = Request.Query; - var protectedRequestToken = Request.Cookies[StateCookie]; + var protectedRequestToken = Request.Cookies[Options.StateCookie.Name]; var requestToken = Options.StateDataFormat.Unprotect(protectedRequestToken); @@ -80,16 +78,9 @@ protected override async Task HandleRemoteAuthenticateAsync return HandleRequestResult.Fail("Missing or blank oauth_verifier"); } - var cookieOptions = new CookieOptions - { - HttpOnly = true, - SameSite = SameSiteMode.Lax, - Secure = Request.IsHttps - }; - - Options.ConfigureStateCookie?.Invoke(Context, cookieOptions); + var cookieOptions = Options.StateCookie.Build(Context, Clock.UtcNow); - Response.Cookies.Delete(StateCookie, cookieOptions); + Response.Cookies.Delete(Options.StateCookie.Name, cookieOptions); var accessToken = await ObtainAccessTokenAsync(requestToken, oauthVerifier); @@ -144,17 +135,9 @@ protected override async Task HandleChallengeAsync(AuthenticationProperties prop var requestToken = await ObtainRequestTokenAsync(BuildRedirectUri(Options.CallbackPath), properties); var twitterAuthenticationEndpoint = AuthenticationEndpoint + requestToken.Token; - var cookieOptions = new CookieOptions - { - HttpOnly = true, - SameSite = SameSiteMode.Lax, - Secure = Request.IsHttps, - Expires = Clock.UtcNow.Add(Options.RemoteAuthenticationTimeout), - }; - - Options.ConfigureStateCookie?.Invoke(Context, cookieOptions); + var cookieOptions = Options.StateCookie.Build(Context, Clock.UtcNow); - Response.Cookies.Append(StateCookie, Options.StateDataFormat.Protect(requestToken), cookieOptions); + Response.Cookies.Append(Options.StateCookie.Name, Options.StateDataFormat.Protect(requestToken), cookieOptions); var redirectContext = new RedirectContext(Context, Scheme, Options, properties, twitterAuthenticationEndpoint); await Events.RedirectToAuthorizationEndpoint(redirectContext); diff --git a/src/Microsoft.AspNetCore.Authentication.Twitter/TwitterOptions.cs b/src/Microsoft.AspNetCore.Authentication.Twitter/TwitterOptions.cs index 8b57f2502..0190f21a6 100644 --- a/src/Microsoft.AspNetCore.Authentication.Twitter/TwitterOptions.cs +++ b/src/Microsoft.AspNetCore.Authentication.Twitter/TwitterOptions.cs @@ -3,10 +3,7 @@ using System; using System.Security.Claims; -using Microsoft.AspNetCore.Authentication; using Microsoft.AspNetCore.Authentication.OAuth.Claims; -using Microsoft.AspNetCore.Authentication.Twitter; -using Microsoft.AspNetCore.DataProtection; using Microsoft.AspNetCore.Http; namespace Microsoft.AspNetCore.Authentication.Twitter @@ -16,6 +13,10 @@ namespace Microsoft.AspNetCore.Authentication.Twitter /// public class TwitterOptions : RemoteAuthenticationOptions { + private const string DefaultStateCookieName = "__TwitterState"; + + private CookieBuilder _stateCookieBuilder; + /// /// Initializes a new instance of the class. /// @@ -26,6 +27,14 @@ public TwitterOptions() Events = new TwitterEvents(); ClaimActions.MapJsonKey(ClaimTypes.Email, "email", ClaimValueTypes.Email); + + _stateCookieBuilder = new TwitterCookieBuilder(this) + { + Name = DefaultStateCookieName, + SecurePolicy = CookieSecurePolicy.SameAsRequest, + HttpOnly = true, + SameSite = SameSiteMode.Lax, + }; } /// @@ -59,18 +68,42 @@ public TwitterOptions() public ISecureDataFormat StateDataFormat { get; set; } /// - /// Gets or sets an action that can override the state cookie options before the - /// cookie gets added to the response. + /// Gets or sets the used to handle authentication events. /// - public Action ConfigureStateCookie { get; set; } + public new TwitterEvents Events + { + get => (TwitterEvents)base.Events; + set => base.Events = value; + } /// - /// Gets or sets the used to handle authentication events. + /// Determines the settings used to create the state cookie before the + /// cookie gets added to the response. /// - public new TwitterEvents Events + public CookieBuilder StateCookie + { + get => _stateCookieBuilder; + set => _stateCookieBuilder = value ?? throw new ArgumentNullException(nameof(value)); + } + + private class TwitterCookieBuilder : CookieBuilder { - get { return (TwitterEvents)base.Events; } - set { base.Events = value; } + private readonly TwitterOptions _twitterOptions; + + public TwitterCookieBuilder(TwitterOptions twitterOptions) + { + _twitterOptions = twitterOptions; + } + + public override CookieOptions Build(HttpContext context, DateTimeOffset expiresFrom) + { + var options = base.Build(context, expiresFrom); + if (!Expiration.HasValue) + { + options.Expires = expiresFrom.Add(_twitterOptions.RemoteAuthenticationTimeout); + } + return options; + } } } } diff --git a/src/Microsoft.AspNetCore.Authentication/Internal/RequestPathBaseCookieBuilder.cs b/src/Microsoft.AspNetCore.Authentication/Internal/RequestPathBaseCookieBuilder.cs new file mode 100644 index 000000000..f42617cb2 --- /dev/null +++ b/src/Microsoft.AspNetCore.Authentication/Internal/RequestPathBaseCookieBuilder.cs @@ -0,0 +1,38 @@ +// 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.AspNetCore.Http; + +namespace Microsoft.AspNetCore.Authentication.Internal +{ + /// + /// A cookie builder that sets to the request path base. + /// + public class RequestPathBaseCookieBuilder : CookieBuilder + { + /// + /// Gets an optional value that is appended to the request path base. + /// + protected virtual string AdditionalPath { get; } + + public override CookieOptions Build(HttpContext context, DateTimeOffset expiresFrom) + { + // check if the user has overridden the default value of path. If so, use that instead of our default value. + var path = Path; + if (path == null) + { + var originalPathBase = context.Features.Get()?.OriginalPathBase ?? context.Request.PathBase; + path = originalPathBase + AdditionalPath; + } + + var options = base.Build(context, expiresFrom); + + options.Path = !string.IsNullOrEmpty(path) + ? path + : "/"; + + return options; + } + } +} diff --git a/src/Microsoft.AspNetCore.Authentication/RemoteAuthenticationHandler.cs b/src/Microsoft.AspNetCore.Authentication/RemoteAuthenticationHandler.cs index 62213a171..7bd3b0773 100644 --- a/src/Microsoft.AspNetCore.Authentication/RemoteAuthenticationHandler.cs +++ b/src/Microsoft.AspNetCore.Authentication/RemoteAuthenticationHandler.cs @@ -5,7 +5,6 @@ using System.Security.Cryptography; using System.Text.Encodings.Web; using System.Threading.Tasks; -using Microsoft.AspNetCore.Http; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; @@ -14,7 +13,6 @@ namespace Microsoft.AspNetCore.Authentication public abstract class RemoteAuthenticationHandler : AuthenticationHandler, IAuthenticationRequestHandler where TOptions : RemoteAuthenticationOptions, new() { - private const string CorrelationPrefix = ".AspNetCore.Correlation."; private const string CorrelationProperty = ".xsrf"; private const string CorrelationMarker = "N"; private const string AuthSchemeKey = ".AuthScheme"; @@ -187,20 +185,11 @@ protected virtual void GenerateCorrelationId(AuthenticationProperties properties CryptoRandom.GetBytes(bytes); var correlationId = Base64UrlTextEncoder.Encode(bytes); - var cookieOptions = new CookieOptions - { - HttpOnly = true, - SameSite = SameSiteMode.None, - Secure = Request.IsHttps, - Path = OriginalPathBase + Options.CallbackPath, - Expires = Clock.UtcNow.Add(Options.RemoteAuthenticationTimeout), - }; - - Options.ConfigureCorrelationIdCookie?.Invoke(Context, cookieOptions); + var cookieOptions = Options.CorrelationCookie.Build(Context, Clock.UtcNow); properties.Items[CorrelationProperty] = correlationId; - var cookieName = CorrelationPrefix + Scheme.Name + "." + correlationId; + var cookieName = Options.CorrelationCookie.Name + Scheme.Name + "." + correlationId; Response.Cookies.Append(cookieName, CorrelationMarker, cookieOptions); } @@ -212,16 +201,15 @@ protected virtual bool ValidateCorrelationId(AuthenticationProperties properties throw new ArgumentNullException(nameof(properties)); } - string correlationId; - if (!properties.Items.TryGetValue(CorrelationProperty, out correlationId)) + if (!properties.Items.TryGetValue(CorrelationProperty, out string correlationId)) { - Logger.CorrelationPropertyNotFound(CorrelationPrefix); + Logger.CorrelationPropertyNotFound(Options.CorrelationCookie.Name); return false; } properties.Items.Remove(CorrelationProperty); - var cookieName = CorrelationPrefix + Scheme.Name + "." + correlationId; + var cookieName = Options.CorrelationCookie.Name + Scheme.Name + "." + correlationId; var correlationCookie = Request.Cookies[cookieName]; if (string.IsNullOrEmpty(correlationCookie)) @@ -230,15 +218,7 @@ protected virtual bool ValidateCorrelationId(AuthenticationProperties properties return false; } - var cookieOptions = new CookieOptions - { - HttpOnly = true, - Path = OriginalPathBase + Options.CallbackPath, - SameSite = SameSiteMode.None, - Secure = Request.IsHttps - }; - - Options.ConfigureCorrelationIdCookie?.Invoke(Context, cookieOptions); + var cookieOptions = Options.CorrelationCookie.Build(Context, Clock.UtcNow); Response.Cookies.Delete(cookieName, cookieOptions); diff --git a/src/Microsoft.AspNetCore.Authentication/RemoteAuthenticationOptions.cs b/src/Microsoft.AspNetCore.Authentication/RemoteAuthenticationOptions.cs index a5f0bb44b..3b34cf43e 100644 --- a/src/Microsoft.AspNetCore.Authentication/RemoteAuthenticationOptions.cs +++ b/src/Microsoft.AspNetCore.Authentication/RemoteAuthenticationOptions.cs @@ -3,6 +3,7 @@ using System; using System.Net.Http; +using Microsoft.AspNetCore.Authentication.Internal; using Microsoft.AspNetCore.DataProtection; using Microsoft.AspNetCore.Http; @@ -13,6 +14,24 @@ namespace Microsoft.AspNetCore.Authentication /// public class RemoteAuthenticationOptions : AuthenticationSchemeOptions { + private const string CorrelationPrefix = ".AspNetCore.Correlation."; + + private CookieBuilder _correlationCookieBuilder; + + /// + /// Initializes a new . + /// + public RemoteAuthenticationOptions() + { + _correlationCookieBuilder = new CorrelationCookieBuilder(this) + { + Name = CorrelationPrefix, + HttpOnly = true, + SameSite = SameSiteMode.None, + SecurePolicy = CookieSecurePolicy.SameAsRequest, + }; + } + /// /// Check that the options are valid. Should throw an exception if things are not ok. /// @@ -71,8 +90,8 @@ public override void Validate() public new RemoteAuthenticationEvents Events { - get { return (RemoteAuthenticationEvents)base.Events; } - set { base.Events = value; } + get => (RemoteAuthenticationEvents)base.Events; + set => base.Events = value; } /// @@ -84,9 +103,37 @@ public override void Validate() public bool SaveTokens { get; set; } /// - /// Gets or sets an action that can override the correlation id cookie options before the + /// Determines the settings used to create the correlation cookie before the /// cookie gets added to the response. /// - public Action ConfigureCorrelationIdCookie { get; set; } + public CookieBuilder CorrelationCookie + { + get => _correlationCookieBuilder; + set => _correlationCookieBuilder = value ?? throw new ArgumentNullException(nameof(value)); + } + + private class CorrelationCookieBuilder : RequestPathBaseCookieBuilder + { + private readonly RemoteAuthenticationOptions _options; + + public CorrelationCookieBuilder(RemoteAuthenticationOptions remoteAuthenticationOptions) + { + _options = remoteAuthenticationOptions; + } + + protected override string AdditionalPath => _options.CallbackPath; + + public override CookieOptions Build(HttpContext context, DateTimeOffset expiresFrom) + { + var cookieOptions = base.Build(context, expiresFrom); + + if (!Expiration.HasValue || !cookieOptions.Expires.HasValue) + { + cookieOptions.Expires = expiresFrom.Add(_options.RemoteAuthenticationTimeout); + } + + return cookieOptions; + } + } } } diff --git a/test/Microsoft.AspNetCore.Authentication.Test/OAuthTests.cs b/test/Microsoft.AspNetCore.Authentication.Test/OAuthTests.cs index bdd79533b..62d11e52a 100644 --- a/test/Microsoft.AspNetCore.Authentication.Test/OAuthTests.cs +++ b/test/Microsoft.AspNetCore.Authentication.Test/OAuthTests.cs @@ -190,7 +190,7 @@ public async Task RedirectToAuthorizeEndpoint_CorrelationIdCookieOptions_CanBeOv opt.AuthorizationEndpoint = "https://example.com/provider/login"; opt.TokenEndpoint = "https://example.com/provider/token"; opt.CallbackPath = "/oauth-callback"; - opt.ConfigureCorrelationIdCookie = (ctx, options) => options.Path = "/"; + opt.CorrelationCookie.Path = "/"; }), ctx => { diff --git a/test/Microsoft.AspNetCore.Authentication.Test/OpenIdConnect/OpenIdConnectTests.cs b/test/Microsoft.AspNetCore.Authentication.Test/OpenIdConnect/OpenIdConnectTests.cs index 7f9d42b1c..b7ac1f82d 100644 --- a/test/Microsoft.AspNetCore.Authentication.Test/OpenIdConnect/OpenIdConnectTests.cs +++ b/test/Microsoft.AspNetCore.Authentication.Test/OpenIdConnect/OpenIdConnectTests.cs @@ -92,7 +92,7 @@ public async Task RedirectToIdentityProvider_NonceCookieOptions_CanBeOverriden() { AuthorizationEndpoint = "https://example.com/provider/login" }; - opt.ConfigureNonceCookie = (ctx, options) => options.Path = "/"; + opt.NonceCookie.Path = "/"; }); var server = setting.CreateTestServer(); @@ -143,7 +143,7 @@ public async Task RedirectToIdentityProvider_CorrelationIdCookieOptions_CanBeOve { AuthorizationEndpoint = "https://example.com/provider/login" }; - opt.ConfigureCorrelationIdCookie = (ctx, options) => options.Path = "/"; + opt.CorrelationCookie.Path = "/"; }); var server = setting.CreateTestServer();