Skip to content
This repository was archived by the owner on Dec 13, 2018. It is now read-only.

Commit 0a740b2

Browse files
author
Nate McMaster
committed
Replace configure method on Twitter, RemoteAuthentication, and OpenIdConnect options with CookieBuilder
1 parent ff9f145 commit 0a740b2

File tree

9 files changed

+203
-99
lines changed

9 files changed

+203
-99
lines changed

‎src/Microsoft.AspNetCore.Authentication.OpenIdConnect/OpenIdConnectHandler.cs

Lines changed: 13 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -275,8 +275,7 @@ public async virtual Task SignOutAsync(AuthenticationProperties properties)
275275
/// <returns>A task executing the callback procedure</returns>
276276
protected virtual Task<bool> HandleSignOutCallbackAsync()
277277
{
278-
StringValues protectedState;
279-
if (Request.Query.TryGetValue(OpenIdConnectParameterNames.State, out protectedState))
278+
if (Request.Query.TryGetValue(OpenIdConnectParameterNames.State, out StringValues protectedState))
280279
{
281280
var properties = Options.StateDataFormat.Unprotect(protectedState);
282281
if (!string.IsNullOrEmpty(properties?.RedirectUri))
@@ -505,8 +504,7 @@ protected override async Task<HandleRequestResult> HandleRemoteAuthenticateAsync
505504
return HandleRequestResult.Fail(Resources.MessageStateIsInvalid);
506505
}
507506

508-
string userstate = null;
509-
properties.Items.TryGetValue(OpenIdConnectDefaults.UserstatePropertiesKey, out userstate);
507+
properties.Items.TryGetValue(OpenIdConnectDefaults.UserstatePropertiesKey, out string userstate);
510508
authorizationResponse.State = userstate;
511509

512510
if (!ValidateCorrelationId(properties))
@@ -859,8 +857,7 @@ private void SaveTokens(AuthenticationProperties properties, OpenIdConnectMessag
859857

860858
if (!string.IsNullOrEmpty(message.ExpiresIn))
861859
{
862-
int value;
863-
if (int.TryParse(message.ExpiresIn, NumberStyles.Integer, CultureInfo.InvariantCulture, out value))
860+
if (int.TryParse(message.ExpiresIn, NumberStyles.Integer, CultureInfo.InvariantCulture, out int value))
864861
{
865862
var expiresAt = Clock.UtcNow + TimeSpan.FromSeconds(value);
866863
// https://www.w3.org/TR/xmlschema-2/#dateTime
@@ -885,21 +882,12 @@ private void WriteNonceCookie(string nonce)
885882
throw new ArgumentNullException(nameof(nonce));
886883
}
887884

888-
var options = new CookieOptions
889-
{
890-
HttpOnly = true,
891-
SameSite = Http.SameSiteMode.None,
892-
Path = OriginalPathBase + Options.CallbackPath,
893-
Secure = Request.IsHttps,
894-
Expires = Clock.UtcNow.Add(Options.ProtocolValidator.NonceLifetime)
895-
};
896-
897-
Options.ConfigureNonceCookie?.Invoke(Context, options);
885+
var cookieOptions = Options.NonceCookie.Build(Context, Clock.UtcNow);
898886

899887
Response.Cookies.Append(
900-
OpenIdConnectDefaults.CookieNoncePrefix + Options.StringDataFormat.Protect(nonce),
888+
Options.NonceCookie.Name + Options.StringDataFormat.Protect(nonce),
901889
NonceProperty,
902-
options);
890+
cookieOptions);
903891
}
904892

905893
/// <summary>
@@ -918,22 +906,18 @@ private string ReadNonceCookie(string nonce)
918906

919907
foreach (var nonceKey in Request.Cookies.Keys)
920908
{
921-
if (nonceKey.StartsWith(OpenIdConnectDefaults.CookieNoncePrefix))
909+
if (nonceKey.StartsWith(Options.NonceCookie.Name))
922910
{
923911
try
924912
{
925-
var nonceDecodedValue = Options.StringDataFormat.Unprotect(nonceKey.Substring(OpenIdConnectDefaults.CookieNoncePrefix.Length, nonceKey.Length - OpenIdConnectDefaults.CookieNoncePrefix.Length));
913+
var nonceDecodedValue = Options.StringDataFormat.Unprotect(nonceKey.Substring(Options.NonceCookie.Name.Length, nonceKey.Length - Options.NonceCookie.Name.Length));
926914
if (nonceDecodedValue == nonce)
927915
{
928-
var cookieOptions = new CookieOptions
916+
var cookieOptions = Options.NonceCookie.Build(Context);
917+
if (Options.NonceCookie.Path == null)
929918
{
930-
HttpOnly = true,
931-
Path = OriginalPathBase + Options.CallbackPath,
932-
SameSite = Http.SameSiteMode.None,
933-
Secure = Request.IsHttps
934-
};
935-
936-
Options.ConfigureNonceCookie?.Invoke(Context, cookieOptions);
919+
cookieOptions.Path = OriginalPathBase + Options.CallbackPath;
920+
}
937921

938922
Response.Cookies.Delete(nonceKey, cookieOptions);
939923
return nonce;
@@ -1170,8 +1154,7 @@ private ClaimsPrincipal ValidateToken(string idToken, AuthenticationProperties p
11701154
validationParameters.IssuerSigningKeys = validationParameters.IssuerSigningKeys?.Concat(_configuration.SigningKeys) ?? _configuration.SigningKeys;
11711155
}
11721156

1173-
SecurityToken validatedToken = null;
1174-
var principal = Options.SecurityTokenValidator.ValidateToken(idToken, validationParameters, out validatedToken);
1157+
var principal = Options.SecurityTokenValidator.ValidateToken(idToken, validationParameters, out SecurityToken validatedToken);
11751158
jwt = validatedToken as JwtSecurityToken;
11761159
if (jwt == null)
11771160
{

‎src/Microsoft.AspNetCore.Authentication.OpenIdConnect/OpenIdConnectOptions.cs

Lines changed: 46 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
using System;
55
using System.Collections.Generic;
66
using System.IdentityModel.Tokens.Jwt;
7+
using Microsoft.AspNetCore.Authentication.Internal;
78
using Microsoft.AspNetCore.Authentication.OAuth.Claims;
89
using Microsoft.AspNetCore.Http;
910
using Microsoft.IdentityModel.Protocols;
@@ -17,6 +18,8 @@ namespace Microsoft.AspNetCore.Authentication.OpenIdConnect
1718
/// </summary>
1819
public class OpenIdConnectOptions : RemoteAuthenticationOptions
1920
{
21+
private CookieBuilder _nonceCookieBuilder;
22+
2023
/// <summary>
2124
/// Initializes a new <see cref="OpenIdConnectOptions"/>
2225
/// </summary>
@@ -65,6 +68,14 @@ public OpenIdConnectOptions()
6568
ClaimActions.MapUniqueJsonKey("family_name", "family_name");
6669
ClaimActions.MapUniqueJsonKey("profile", "profile");
6770
ClaimActions.MapUniqueJsonKey("email", "email");
71+
72+
_nonceCookieBuilder = new OpenIdConnectNonceCookieBuilder(this)
73+
{
74+
Name = OpenIdConnectDefaults.CookieNoncePrefix,
75+
HttpOnly = true,
76+
SameSite = SameSiteMode.None,
77+
SecurePolicy = CookieSecurePolicy.SameAsRequest,
78+
};
6879
}
6980

7081
/// <summary>
@@ -145,8 +156,8 @@ public override void Validate()
145156
/// </summary>
146157
public new OpenIdConnectEvents Events
147158
{
148-
get { return (OpenIdConnectEvents)base.Events; }
149-
set { base.Events = value; }
159+
get => (OpenIdConnectEvents)base.Events;
160+
set => base.Events = value;
150161
}
151162

152163
/// <summary>
@@ -259,9 +270,40 @@ public override void Validate()
259270
public bool DisableTelemetry { get; set; }
260271

261272
/// <summary>
262-
/// Gets or sets an action that can override the nonce cookie options before the
273+
/// Determines the settings used to create the nonce cookie before the
263274
/// cookie gets added to the response.
264275
/// </summary>
265-
public Action<HttpContext, CookieOptions> ConfigureNonceCookie { get; set; }
276+
/// <remarks>
277+
/// The value of <see cref="CookieBuilder.Name"/> is treated as the prefix to the cookie name, and defaults to <seealso cref="OpenIdConnectDefaults.CookieNoncePrefix"/>.
278+
/// </remarks>
279+
public CookieBuilder NonceCookie
280+
{
281+
get => _nonceCookieBuilder;
282+
set => _nonceCookieBuilder = value ?? throw new ArgumentNullException(nameof(value));
283+
}
284+
285+
private class OpenIdConnectNonceCookieBuilder : PathScopingCookieBuilder
286+
{
287+
private readonly OpenIdConnectOptions _options;
288+
289+
public OpenIdConnectNonceCookieBuilder(OpenIdConnectOptions oidcOptions)
290+
{
291+
_options = oidcOptions;
292+
}
293+
294+
protected override string PathScope => _options.CallbackPath;
295+
296+
public override CookieOptions Build(HttpContext context, DateTimeOffset expiresFrom)
297+
{
298+
var cookieOptions = base.Build(context, expiresFrom);
299+
300+
if (!Expiration.HasValue || !cookieOptions.Expires.HasValue)
301+
{
302+
cookieOptions.Expires = expiresFrom.Add(_options.ProtocolValidator.NonceLifetime);
303+
}
304+
305+
return cookieOptions;
306+
}
307+
}
266308
}
267309
}

‎src/Microsoft.AspNetCore.Authentication.Twitter/TwitterHandler.cs

Lines changed: 5 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010
using System.Text;
1111
using System.Text.Encodings.Web;
1212
using System.Threading.Tasks;
13-
using Microsoft.AspNetCore.DataProtection;
1413
using Microsoft.AspNetCore.Http;
1514
using Microsoft.AspNetCore.WebUtilities;
1615
using Microsoft.Extensions.Logging;
@@ -23,7 +22,6 @@ namespace Microsoft.AspNetCore.Authentication.Twitter
2322
internal class TwitterHandler : RemoteAuthenticationHandler<TwitterOptions>
2423
{
2524
private static readonly DateTime Epoch = new DateTime(1970, 1, 1, 0, 0, 0, DateTimeKind.Utc);
26-
private const string StateCookie = "__TwitterState";
2725
private const string RequestTokenEndpoint = "https://api.twitter.com/oauth/request_token";
2826
private const string AuthenticationEndpoint = "https://api.twitter.com/oauth/authenticate?oauth_token=";
2927
private const string AccessTokenEndpoint = "https://api.twitter.com/oauth/access_token";
@@ -50,7 +48,7 @@ protected override async Task<HandleRequestResult> HandleRemoteAuthenticateAsync
5048
{
5149
AuthenticationProperties properties = null;
5250
var query = Request.Query;
53-
var protectedRequestToken = Request.Cookies[StateCookie];
51+
var protectedRequestToken = Request.Cookies[Options.StateCookie.Name];
5452

5553
var requestToken = Options.StateDataFormat.Unprotect(protectedRequestToken);
5654

@@ -80,16 +78,9 @@ protected override async Task<HandleRequestResult> HandleRemoteAuthenticateAsync
8078
return HandleRequestResult.Fail("Missing or blank oauth_verifier");
8179
}
8280

83-
var cookieOptions = new CookieOptions
84-
{
85-
HttpOnly = true,
86-
SameSite = SameSiteMode.Lax,
87-
Secure = Request.IsHttps
88-
};
89-
90-
Options.ConfigureStateCookie?.Invoke(Context, cookieOptions);
81+
var cookieOptions = Options.StateCookie.Build(Context, Clock.UtcNow);
9182

92-
Response.Cookies.Delete(StateCookie, cookieOptions);
83+
Response.Cookies.Delete(Options.StateCookie.Name, cookieOptions);
9384

9485
var accessToken = await ObtainAccessTokenAsync(requestToken, oauthVerifier);
9586

@@ -144,17 +135,9 @@ protected override async Task HandleChallengeAsync(AuthenticationProperties prop
144135
var requestToken = await ObtainRequestTokenAsync(BuildRedirectUri(Options.CallbackPath), properties);
145136
var twitterAuthenticationEndpoint = AuthenticationEndpoint + requestToken.Token;
146137

147-
var cookieOptions = new CookieOptions
148-
{
149-
HttpOnly = true,
150-
SameSite = SameSiteMode.Lax,
151-
Secure = Request.IsHttps,
152-
Expires = Clock.UtcNow.Add(Options.RemoteAuthenticationTimeout),
153-
};
154-
155-
Options.ConfigureStateCookie?.Invoke(Context, cookieOptions);
138+
var cookieOptions = Options.StateCookie.Build(Context, Clock.UtcNow);
156139

157-
Response.Cookies.Append(StateCookie, Options.StateDataFormat.Protect(requestToken), cookieOptions);
140+
Response.Cookies.Append(Options.StateCookie.Name, Options.StateDataFormat.Protect(requestToken), cookieOptions);
158141

159142
var redirectContext = new RedirectContext<TwitterOptions>(Context, Scheme, Options, properties, twitterAuthenticationEndpoint);
160143
await Events.RedirectToAuthorizationEndpoint(redirectContext);

‎src/Microsoft.AspNetCore.Authentication.Twitter/TwitterOptions.cs

Lines changed: 43 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,7 @@
33

44
using System;
55
using System.Security.Claims;
6-
using Microsoft.AspNetCore.Authentication;
76
using Microsoft.AspNetCore.Authentication.OAuth.Claims;
8-
using Microsoft.AspNetCore.Authentication.Twitter;
9-
using Microsoft.AspNetCore.DataProtection;
107
using Microsoft.AspNetCore.Http;
118

129
namespace Microsoft.AspNetCore.Authentication.Twitter
@@ -16,6 +13,10 @@ namespace Microsoft.AspNetCore.Authentication.Twitter
1613
/// </summary>
1714
public class TwitterOptions : RemoteAuthenticationOptions
1815
{
16+
private const string DefaultStateCookieName = "__TwitterState";
17+
18+
private CookieBuilder _stateCookieBuilder;
19+
1920
/// <summary>
2021
/// Initializes a new instance of the <see cref="TwitterOptions"/> class.
2122
/// </summary>
@@ -26,6 +27,14 @@ public TwitterOptions()
2627
Events = new TwitterEvents();
2728

2829
ClaimActions.MapJsonKey(ClaimTypes.Email, "email", ClaimValueTypes.Email);
30+
31+
_stateCookieBuilder = new TwitterCookieBuilder(this)
32+
{
33+
Name = DefaultStateCookieName,
34+
SecurePolicy = CookieSecurePolicy.SameAsRequest,
35+
HttpOnly = true,
36+
SameSite = SameSiteMode.Lax,
37+
};
2938
}
3039

3140
/// <summary>
@@ -59,18 +68,42 @@ public TwitterOptions()
5968
public ISecureDataFormat<RequestToken> StateDataFormat { get; set; }
6069

6170
/// <summary>
62-
/// Gets or sets an action that can override the state cookie options before the
63-
/// cookie gets added to the response.
71+
/// Gets or sets the <see cref="TwitterEvents"/> used to handle authentication events.
6472
/// </summary>
65-
public Action<HttpContext, CookieOptions> ConfigureStateCookie { get; set; }
73+
public new TwitterEvents Events
74+
{
75+
get => (TwitterEvents)base.Events;
76+
set => base.Events = value;
77+
}
6678

6779
/// <summary>
68-
/// Gets or sets the <see cref="TwitterEvents"/> used to handle authentication events.
80+
/// Determines the settings used to create the state cookie before the
81+
/// cookie gets added to the response.
6982
/// </summary>
70-
public new TwitterEvents Events
83+
public CookieBuilder StateCookie
84+
{
85+
get => _stateCookieBuilder;
86+
set => _stateCookieBuilder = value ?? throw new ArgumentNullException(nameof(value));
87+
}
88+
89+
private class TwitterCookieBuilder : CookieBuilder
7190
{
72-
get { return (TwitterEvents)base.Events; }
73-
set { base.Events = value; }
91+
private readonly TwitterOptions _twitterOptions;
92+
93+
public TwitterCookieBuilder(TwitterOptions twitterOptions)
94+
{
95+
_twitterOptions = twitterOptions;
96+
}
97+
98+
public override CookieOptions Build(HttpContext context, DateTimeOffset expiresFrom)
99+
{
100+
var options = base.Build(context, expiresFrom);
101+
if (!Expiration.HasValue)
102+
{
103+
options.Expires = expiresFrom.Add(_twitterOptions.RemoteAuthenticationTimeout);
104+
}
105+
return options;
106+
}
74107
}
75108
}
76109
}

‎src/Microsoft.AspNetCore.Authentication/Internal/PathScopingCookieBuilder.cs

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
// Copyright (c) .NET Foundation. All rights reserved.
2+
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
3+
4+
using System;
5+
using Microsoft.AspNetCore.Http;
6+
7+
namespace Microsoft.AspNetCore.Authentication.Internal
8+
{
9+
/// <summary>
10+
/// A cookie builder that set <see cref="CookieOptions.Path"/> to the original path base plus some scope.
11+
/// </summary>
12+
public abstract class PathScopingCookieBuilder : CookieBuilder
13+
{
14+
/// <summary>
15+
/// Path that is appended to the request path base.
16+
/// </summary>
17+
protected abstract string PathScope { get; }
18+
19+
public override CookieOptions Build(HttpContext context, DateTimeOffset expiresFrom)
20+
{
21+
// check if the user has overridden the default value of path. If so, use that instead of our default value.
22+
var path = Path;
23+
if (path == null)
24+
{
25+
var originalPathBase = context.Features.Get<IAuthenticationFeature>()?.OriginalPathBase ?? context.Request.PathBase;
26+
path = originalPathBase + PathScope;
27+
}
28+
29+
var options = base.Build(context, expiresFrom);
30+
31+
options.Path = path ?? "/";
32+
33+
return options;
34+
}
35+
}
36+
}

0 commit comments

Comments
 (0)