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

Commit 06f8139

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

File tree

6 files changed

+199
-10
lines changed

6 files changed

+199
-10
lines changed

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

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -332,7 +332,18 @@ protected override async Task HandleChallengeAsync(AuthenticationProperties prop
332332
if (Options.ProtocolValidator.RequireNonce)
333333
{
334334
message.Nonce = Options.ProtocolValidator.GenerateNonce();
335-
WriteNonceCookie(message.Nonce);
335+
var cookieOptions = new CookieOptions
336+
{
337+
HttpOnly = true,
338+
SameSite = Http.SameSiteMode.None,
339+
Path = OriginalPathBase + Options.CallbackPath,
340+
Secure = Request.IsHttps,
341+
Expires = Clock.UtcNow.Add(Options.ProtocolValidator.NonceLifetime)
342+
};
343+
344+
Options.ConfigureNonceCookie?.Invoke(Context, cookieOptions);
345+
346+
WriteNonceCookie(message.Nonce, cookieOptions);
336347
}
337348

338349
GenerateCorrelationId(properties);
@@ -877,9 +888,10 @@ private void SaveTokens(AuthenticationProperties properties, OpenIdConnectMessag
877888
/// Adds the nonce to <see cref="HttpResponse.Cookies"/>.
878889
/// </summary>
879890
/// <param name="nonce">the nonce to remember.</param>
891+
/// <param name="options">The options for the cookie.</param>
880892
/// <remarks><see cref="M:IResponseCookies.Append"/> of <see cref="HttpResponse.Cookies"/> is called to add a cookie with the name: 'OpenIdConnectAuthenticationDefaults.Nonce + <see cref="M:ISecureDataFormat{TData}.Protect"/>(nonce)' of <see cref="OpenIdConnectOptions.StringDataFormat"/>.
881893
/// The value of the cookie is: "N".</remarks>
882-
private void WriteNonceCookie(string nonce)
894+
private void WriteNonceCookie(string nonce, CookieOptions options)
883895
{
884896
if (string.IsNullOrEmpty(nonce))
885897
{
@@ -889,13 +901,7 @@ private void WriteNonceCookie(string nonce)
889901
Response.Cookies.Append(
890902
OpenIdConnectDefaults.CookieNoncePrefix + Options.StringDataFormat.Protect(nonce),
891903
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-
});
904+
options);
899905
}
900906

901907
/// <summary>

‎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/RemoteAuthenticationHandler.cs

Lines changed: 3 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;

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