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

Commit 879f0b7

Browse files
javiercnjaviercn
committed
[Fixes #1133] Limit the path on the nonce and correlation id cookies
1 parent 200ce72 commit 879f0b7

File tree

8 files changed

+213
-8
lines changed

8 files changed

+213
-8
lines changed

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

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -886,16 +886,21 @@ private void WriteNonceCookie(string nonce)
886886
throw new ArgumentNullException(nameof(nonce));
887887
}
888888

889+
var options = new CookieOptions
890+
{
891+
HttpOnly = true,
892+
SameSite = Http.SameSiteMode.None,
893+
Path = OriginalPathBase + Options.CallbackPath,
894+
Secure = Request.IsHttps,
895+
Expires = Clock.UtcNow.Add(Options.ProtocolValidator.NonceLifetime)
896+
};
897+
898+
Options.ConfigureNonceCookie?.Invoke(Context, options);
899+
889900
Response.Cookies.Append(
890901
OpenIdConnectDefaults.CookieNoncePrefix + Options.StringDataFormat.Protect(nonce),
891902
NonceProperty,
892-
new CookieOptions
893-
{
894-
HttpOnly = true,
895-
SameSite = Http.SameSiteMode.None,
896-
Secure = Request.IsHttps,
897-
Expires = Clock.UtcNow.Add(Options.ProtocolValidator.NonceLifetime)
898-
});
903+
options);
899904
}
900905

901906
/// <summary>
@@ -924,10 +929,13 @@ private string ReadNonceCookie(string nonce)
924929
var cookieOptions = new CookieOptions
925930
{
926931
HttpOnly = true,
932+
Path = OriginalPathBase + Options.CallbackPath,
927933
SameSite = Http.SameSiteMode.None,
928934
Secure = Request.IsHttps
929935
};
930936

937+
Options.ConfigureNonceCookie?.Invoke(Context, cookieOptions);
938+
931939
Response.Cookies.Delete(nonceKey, cookieOptions);
932940
return nonce;
933941
}

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -262,5 +262,11 @@ public override void Validate()
262262
/// remote OpenID Connect provider as an authorization/logout request parameter.
263263
/// </summary>
264264
public bool DisableTelemetry { get; set; }
265+
266+
/// <summary>
267+
/// Gets or sets an action that can override the nonce cookie options before the
268+
/// cookie gets added to the response.
269+
/// </summary>
270+
public Action<HttpContext, CookieOptions> ConfigureNonceCookie { get; set; }
265271
}
266272
}

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,8 @@ protected override async Task<AuthenticateResult> HandleRemoteAuthenticateAsync(
8787
Secure = Request.IsHttps
8888
};
8989

90+
Options.ConfigureStateCookie?.Invoke(Context, cookieOptions);
91+
9092
Response.Cookies.Delete(StateCookie, cookieOptions);
9193

9294
var accessToken = await ObtainAccessTokenAsync(requestToken, oauthVerifier);
@@ -159,6 +161,8 @@ protected override async Task HandleChallengeAsync(AuthenticationProperties prop
159161
Expires = Clock.UtcNow.Add(Options.RemoteAuthenticationTimeout),
160162
};
161163

164+
Options.ConfigureStateCookie?.Invoke(Context, cookieOptions);
165+
162166
Response.Cookies.Append(StateCookie, Options.StateDataFormat.Protect(requestToken), cookieOptions);
163167

164168
var redirectContext = new TwitterRedirectToAuthorizationEndpointContext(Context, Scheme, Options, properties, twitterAuthenticationEndpoint);

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,12 @@ public TwitterOptions()
5858
/// </summary>
5959
public ISecureDataFormat<RequestToken> StateDataFormat { get; set; }
6060

61+
/// <summary>
62+
/// Gets or sets an action that can override the state cookie options before the
63+
/// cookie gets added to the response.
64+
/// </summary>
65+
public Action<HttpContext, CookieOptions> ConfigureStateCookie { get; set; }
66+
6167
/// <summary>
6268
/// Gets or sets the <see cref="TwitterEvents"/> used to handle authentication events.
6369
/// </summary>

‎src/Microsoft.AspNetCore.Authentication/RemoteAuthenticationHandler.cs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -205,9 +205,12 @@ protected virtual void GenerateCorrelationId(AuthenticationProperties properties
205205
HttpOnly = true,
206206
SameSite = SameSiteMode.None,
207207
Secure = Request.IsHttps,
208+
Path = OriginalPathBase + Options.CallbackPath,
208209
Expires = Clock.UtcNow.Add(Options.RemoteAuthenticationTimeout),
209210
};
210211

212+
Options.ConfigureCorrelationIdCookie?.Invoke(Context, cookieOptions);
213+
211214
properties.Items[CorrelationProperty] = correlationId;
212215

213216
var cookieName = CorrelationPrefix + Scheme.Name + "." + correlationId;
@@ -243,9 +246,13 @@ protected virtual bool ValidateCorrelationId(AuthenticationProperties properties
243246
var cookieOptions = new CookieOptions
244247
{
245248
HttpOnly = true,
249+
Path = OriginalPathBase + Options.CallbackPath,
246250
SameSite = SameSiteMode.None,
247251
Secure = Request.IsHttps
248252
};
253+
254+
Options.ConfigureCorrelationIdCookie?.Invoke(Context, cookieOptions);
255+
249256
Response.Cookies.Delete(cookieName, cookieOptions);
250257

251258
if (!string.Equals(correlationCookie, CorrelationMarker, StringComparison.Ordinal))

‎src/Microsoft.AspNetCore.Authentication/RemoteAuthenticationOptions.cs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,5 +87,11 @@ public override void Validate()
8787
/// the size of the final authentication cookie.
8888
/// </summary>
8989
public bool SaveTokens { get; set; }
90+
91+
/// <summary>
92+
/// Gets or sets an action that can override the correlation id cookie options before the
93+
/// cookie gets added to the response.
94+
/// </summary>
95+
public Action<HttpContext, CookieOptions> ConfigureCorrelationIdCookie { get; set; }
9096
}
9197
}

‎test/Microsoft.AspNetCore.Authentication.Test/OAuthTests.cs

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
using System;
55
using System.Net;
66
using System.Threading.Tasks;
7+
using Microsoft.AspNetCore.Authentication.Cookies;
78
using Microsoft.AspNetCore.Builder;
89
using Microsoft.AspNetCore.Hosting;
910
using Microsoft.AspNetCore.Http;
@@ -165,6 +166,70 @@ public async Task ThrowsIfSignInSchemeMissing()
165166
Assert.Equal(HttpStatusCode.OK, transaction.Response.StatusCode);
166167
}
167168

169+
[Fact]
170+
public async Task RedirectToIdentityProvider_SetsCorrelationIdCookiePath_ToCallBackPath()
171+
{
172+
var server = CreateServer(
173+
app => { },
174+
s => s.AddOAuthAuthentication(
175+
"Weblie",
176+
opt =>
177+
{
178+
opt.ClientId = "Test Id";
179+
opt.ClientSecret = "secret";
180+
opt.SignInScheme = CookieAuthenticationDefaults.AuthenticationScheme;
181+
opt.AuthorizationEndpoint = "https://example.com/provider/login";
182+
opt.TokenEndpoint = "https://example.com/provider/token";
183+
opt.CallbackPath = "/oauth-callback";
184+
}),
185+
ctx =>
186+
{
187+
ctx.ChallengeAsync("Weblie").ConfigureAwait(false).GetAwaiter().GetResult();
188+
return true;
189+
});
190+
191+
var transaction = await server.SendAsync("https://www.example.com/challenge");
192+
var res = transaction.Response;
193+
194+
Assert.Equal(HttpStatusCode.Redirect, res.StatusCode);
195+
Assert.NotNull(res.Headers.Location);
196+
var setCookie = Assert.Single(res.Headers, h => h.Key == "Set-Cookie");
197+
var correlation = Assert.Single(setCookie.Value, v => v.StartsWith(".AspNetCore.Correlation."));
198+
Assert.Contains("path=/oauth-callback", correlation);
199+
}
200+
201+
[Fact]
202+
public async Task RedirectToAuthorizeEndpoint_CorrelationIdCookieOptions_CanBeOverriden()
203+
{
204+
var server = CreateServer(
205+
app => { },
206+
s => s.AddOAuthAuthentication(
207+
"Weblie",
208+
opt =>
209+
{
210+
opt.ClientId = "Test Id";
211+
opt.ClientSecret = "secret";
212+
opt.SignInScheme = CookieAuthenticationDefaults.AuthenticationScheme;
213+
opt.AuthorizationEndpoint = "https://example.com/provider/login";
214+
opt.TokenEndpoint = "https://example.com/provider/token";
215+
opt.CallbackPath = "/oauth-callback";
216+
opt.ConfigureCorrelationIdCookie = (ctx, options) => options.Path = "/";
217+
}),
218+
ctx =>
219+
{
220+
ctx.ChallengeAsync("Weblie").ConfigureAwait(false).GetAwaiter().GetResult();
221+
return true;
222+
});
223+
224+
var transaction = await server.SendAsync("https://www.example.com/challenge");
225+
var res = transaction.Response;
226+
227+
Assert.Equal(HttpStatusCode.Redirect, res.StatusCode);
228+
Assert.NotNull(res.Headers.Location);
229+
var setCookie = Assert.Single(res.Headers, h => h.Key == "Set-Cookie");
230+
var correlation = Assert.Single(setCookie.Value, v => v.StartsWith(".AspNetCore.Correlation."));
231+
Assert.Contains("path=/", correlation);
232+
}
168233

169234
private static TestServer CreateServer(Action<IApplicationBuilder> configure, Action<IServiceCollection> configureServices, Func<HttpContext, bool> handler)
170235
{

‎test/Microsoft.AspNetCore.Authentication.Test/OpenIdConnect/OpenIdConnectTests.cs

Lines changed: 104 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,108 @@ public async Task SignOutSettingMessage()
6060
OpenIdConnectParameterNames.VersionTelemetry);
6161
}
6262

63+
[Fact]
64+
public async Task RedirectToIdentityProvider_SetsNonceCookiePath_ToCallBackPath()
65+
{
66+
var setting = new TestSettings(opt =>
67+
{
68+
opt.ClientId = "Test Id";
69+
opt.SignInScheme = CookieAuthenticationDefaults.AuthenticationScheme;
70+
opt.Configuration = new OpenIdConnectConfiguration
71+
{
72+
AuthorizationEndpoint = "https://example.com/provider/login"
73+
};
74+
});
75+
76+
var server = setting.CreateTestServer();
77+
78+
var transaction = await server.SendAsync(DefaultHost + TestServerBuilder.Challenge);
79+
var res = transaction.Response;
80+
81+
Assert.Equal(HttpStatusCode.Redirect, res.StatusCode);
82+
Assert.NotNull(res.Headers.Location);
83+
var setCookie = Assert.Single(res.Headers, h => h.Key == "Set-Cookie");
84+
var nonce = Assert.Single(setCookie.Value, v => v.StartsWith(OpenIdConnectDefaults.CookieNoncePrefix));
85+
Assert.Contains("path=/signin-oidc", nonce);
86+
}
87+
88+
[Fact]
89+
public async Task RedirectToIdentityProvider_NonceCookieOptions_CanBeOverriden()
90+
{
91+
var setting = new TestSettings(opt =>
92+
{
93+
opt.ClientId = "Test Id";
94+
opt.SignInScheme = CookieAuthenticationDefaults.AuthenticationScheme;
95+
opt.Configuration = new OpenIdConnectConfiguration
96+
{
97+
AuthorizationEndpoint = "https://example.com/provider/login"
98+
};
99+
opt.ConfigureNonceCookie = (ctx, options) => options.Path = "/";
100+
});
101+
102+
var server = setting.CreateTestServer();
103+
104+
var transaction = await server.SendAsync(DefaultHost + TestServerBuilder.Challenge);
105+
var res = transaction.Response;
106+
107+
Assert.Equal(HttpStatusCode.Redirect, res.StatusCode);
108+
Assert.NotNull(res.Headers.Location);
109+
var setCookie = Assert.Single(res.Headers, h => h.Key == "Set-Cookie");
110+
var nonce = Assert.Single(setCookie.Value, v => v.StartsWith(OpenIdConnectDefaults.CookieNoncePrefix));
111+
Assert.Contains("path=/", nonce);
112+
}
113+
114+
[Fact]
115+
public async Task RedirectToIdentityProvider_SetsCorrelationIdCookiePath_ToCallBackPath()
116+
{
117+
var setting = new TestSettings(opt =>
118+
{
119+
opt.ClientId = "Test Id";
120+
opt.SignInScheme = CookieAuthenticationDefaults.AuthenticationScheme;
121+
opt.Configuration = new OpenIdConnectConfiguration
122+
{
123+
AuthorizationEndpoint = "https://example.com/provider/login"
124+
};
125+
});
126+
127+
var server = setting.CreateTestServer();
128+
129+
var transaction = await server.SendAsync(DefaultHost + TestServerBuilder.Challenge);
130+
var res = transaction.Response;
131+
132+
Assert.Equal(HttpStatusCode.Redirect, res.StatusCode);
133+
Assert.NotNull(res.Headers.Location);
134+
var setCookie = Assert.Single(res.Headers, h => h.Key == "Set-Cookie");
135+
var correlation = Assert.Single(setCookie.Value, v => v.StartsWith(".AspNetCore.Correlation."));
136+
Assert.Contains("path=/signin-oidc", correlation);
137+
}
138+
139+
[Fact]
140+
public async Task RedirectToIdentityProvider_CorrelationIdCookieOptions_CanBeOverriden()
141+
{
142+
var setting = new TestSettings(opt =>
143+
{
144+
opt.ClientId = "Test Id";
145+
opt.SignInScheme = CookieAuthenticationDefaults.AuthenticationScheme;
146+
opt.Configuration = new OpenIdConnectConfiguration
147+
{
148+
AuthorizationEndpoint = "https://example.com/provider/login"
149+
};
150+
opt.ConfigureCorrelationIdCookie = (ctx, options) => options.Path = "/";
151+
});
152+
153+
var server = setting.CreateTestServer();
154+
155+
var transaction = await server.SendAsync(DefaultHost + TestServerBuilder.Challenge);
156+
var res = transaction.Response;
157+
158+
Assert.Equal(HttpStatusCode.Redirect, res.StatusCode);
159+
Assert.NotNull(res.Headers.Location);
160+
var setCookie = Assert.Single(res.Headers, h => h.Key == "Set-Cookie");
161+
var correlation = Assert.Single(setCookie.Value, v => v.StartsWith(".AspNetCore.Correlation."));
162+
Assert.Contains("path=/", correlation);
163+
}
164+
63165
[Fact]
64166
public async Task EndSessionRequestDoesNotIncludeTelemetryParametersWhenDisabled()
65167
{
@@ -173,7 +275,8 @@ public async Task SignOutWith_Specific_RedirectUri_From_Authentication_Properite
173275
[Fact]
174276
public async Task SignOut_WithMissingConfig_Throws()
175277
{
176-
var setting = new TestSettings(opt => {
278+
var setting = new TestSettings(opt =>
279+
{
177280
opt.ClientId = "Test Id";
178281
opt.Configuration = new OpenIdConnectConfiguration();
179282
});

0 commit comments

Comments
 (0)