Skip to content

Commit 2fedced

Browse files
mckaragozmckaragoz
authored
Fix External Hub Antiforgery Cookies and Strengthen CSRF Regression Tests (#148)
1 parent dfc17fc commit 2fedced

4 files changed

Lines changed: 154 additions & 33 deletions

File tree

‎src/CodeBeam.UltimateAuth.Server/Extensions/ServiceCollectionExtensions.cs‎

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
1-
using Microsoft.AspNetCore.Builder;
2-
using CodeBeam.UltimateAuth.Authorization;
1+
using CodeBeam.UltimateAuth.Authorization;
32
using CodeBeam.UltimateAuth.Core;
43
using CodeBeam.UltimateAuth.Core.Abstractions;
54
using CodeBeam.UltimateAuth.Core.Defaults;
@@ -11,6 +10,7 @@
1110
using CodeBeam.UltimateAuth.Core.Options;
1211
using CodeBeam.UltimateAuth.Core.Runtime;
1312
using CodeBeam.UltimateAuth.Credentials;
13+
using CodeBeam.UltimateAuth.Credentials.Extensions;
1414
using CodeBeam.UltimateAuth.Policies.Abstractions;
1515
using CodeBeam.UltimateAuth.Policies.Defaults;
1616
using CodeBeam.UltimateAuth.Policies.Registry;
@@ -30,13 +30,15 @@
3030
using CodeBeam.UltimateAuth.Server.Stores;
3131
using CodeBeam.UltimateAuth.Users;
3232
using CodeBeam.UltimateAuth.Users.Contracts;
33+
using Microsoft.AspNetCore.Antiforgery;
3334
using Microsoft.AspNetCore.Authentication;
3435
using Microsoft.AspNetCore.Authorization;
36+
using Microsoft.AspNetCore.Builder;
37+
using Microsoft.AspNetCore.Http;
3538
using Microsoft.Extensions.DependencyInjection;
3639
using Microsoft.Extensions.DependencyInjection.Extensions;
3740
using Microsoft.Extensions.Options;
3841
using Microsoft.IdentityModel.Tokens;
39-
using CodeBeam.UltimateAuth.Credentials.Extensions;
4042

4143
namespace CodeBeam.UltimateAuth.Server.Extensions;
4244

@@ -321,6 +323,23 @@ private static IServiceCollection AddUltimateAuthServerInternal(this IServiceCol
321323
options.FormFieldName = "__RequestVerificationToken";
322324
});
323325

326+
services.AddOptions<AntiforgeryOptions>().PostConfigure<IOptions<UAuthServerOptions>>((antiforgery, serverOptions) =>
327+
{
328+
var server = serverOptions.Value;
329+
330+
var sameSite = server.Antiforgery.CookieSameSite
331+
?? (server.HubDeploymentMode == UAuthHubDeploymentMode.External
332+
? SameSiteMode.None
333+
: SameSiteMode.Strict);
334+
335+
antiforgery.Cookie.SameSite = sameSite;
336+
337+
if (sameSite == SameSiteMode.None)
338+
{
339+
antiforgery.Cookie.SecurePolicy = CookieSecurePolicy.Always;
340+
}
341+
});
342+
324343
services.AddScoped<UAuthCsrfPolicyEvaluator>();
325344
services.AddScoped<UAuthCsrfValidator>();
326345

‎src/CodeBeam.UltimateAuth.Server/Options/UAuthAntiforgeryOptions.cs‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
1-
namespace CodeBeam.UltimateAuth.Server.Options;
1+
using Microsoft.AspNetCore.Http;
2+
3+
namespace CodeBeam.UltimateAuth.Server.Options;
24

35
/// <summary>
46
/// Configures CSRF protection for UltimateAuth endpoints.
@@ -13,8 +15,15 @@ public sealed class UAuthAntiforgeryOptions
1315
/// </summary>
1416
public string TokenEndpoint { get; set; } = "/antiforgery/token";
1517

18+
/// <summary>
19+
/// Overrides the antiforgery cookie SameSite policy.
20+
/// When null, External Hub deployments use None, other deployments retain ASP.NET Core's Strict default.
21+
/// </summary>
22+
public SameSiteMode? CookieSameSite { get; set; }
23+
1624
public UAuthAntiforgeryOptions Clone() => new()
1725
{
18-
TokenEndpoint = TokenEndpoint
26+
TokenEndpoint = TokenEndpoint,
27+
CookieSameSite = CookieSameSite
1928
};
2029
}

‎tests/CodeBeam.UltimateAuth.Tests.Integration/Security/AntiforgeryCookiePolicyTests.cs‎

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+

2+
using AwesomeAssertions;
3+
using CodeBeam.UltimateAuth.Core;
4+
using CodeBeam.UltimateAuth.Server.Options;
5+
using CodeBeam.UltimateAuth.Tests.Integration.Infrastructure;
6+
using Microsoft.AspNetCore.Antiforgery;
7+
using Microsoft.AspNetCore.Http;
8+
using Microsoft.AspNetCore.Mvc.Testing;
9+
using Microsoft.Extensions.DependencyInjection;
10+
using Microsoft.Extensions.Options;
11+
12+
namespace CodeBeam.UltimateAuth.Tests.Integration.Security;
13+
14+
public sealed class AntiforgeryCookiePolicyTests
15+
{
16+
[Theory]
17+
[InlineData(UAuthHubDeploymentMode.Embedded, SameSiteMode.Strict)]
18+
[InlineData(UAuthHubDeploymentMode.Integrated, SameSiteMode.Strict)]
19+
[InlineData(UAuthHubDeploymentMode.External, SameSiteMode.None)]
20+
public async Task AntiforgeryCookie_UsesDeploymentPolicy(
21+
UAuthHubDeploymentMode deployment,
22+
SameSiteMode expected)
23+
{
24+
await using var factory = AuthServerFactory.Create(
25+
options => options.HubDeploymentMode = deployment);
26+
27+
var antiforgery = factory.Services
28+
.GetRequiredService<IOptions<AntiforgeryOptions>>()
29+
.Value;
30+
31+
antiforgery.Cookie.SameSite.Should().Be(expected);
32+
33+
if (expected == SameSiteMode.None)
34+
{
35+
antiforgery.Cookie.SecurePolicy
36+
.Should().Be(CookieSecurePolicy.Always);
37+
}
38+
}
39+
40+
[Fact]
41+
public async Task ExternalHub_AntiforgeryTokenCookie_IsSecureAndCrossSite()
42+
{
43+
await using var factory = AuthServerFactory.Create(
44+
options => options.HubDeploymentMode =
45+
UAuthHubDeploymentMode.External);
46+
47+
using var client = factory.CreateClient(
48+
new WebApplicationFactoryClientOptions
49+
{
50+
BaseAddress = new Uri("https://localhost"),
51+
AllowAutoRedirect = false
52+
});
53+
54+
using var response = await client.GetAsync(
55+
"/auth/antiforgery/token",
56+
TestContext.Current.CancellationToken);
57+
58+
response.EnsureSuccessStatusCode();
59+
60+
response.Headers.TryGetValues("Set-Cookie", out var cookies)
61+
.Should().BeTrue();
62+
63+
var cookie = cookies!.Single(x =>
64+
x.StartsWith(
65+
".AspNetCore.Antiforgery.",
66+
StringComparison.OrdinalIgnoreCase));
67+
68+
cookie.Contains(
69+
"samesite=none",
70+
StringComparison.OrdinalIgnoreCase).Should().BeTrue();
71+
72+
cookie.Split(';', StringSplitOptions.TrimEntries)
73+
.Any(x => x.Equals(
74+
"secure",
75+
StringComparison.OrdinalIgnoreCase))
76+
.Should().BeTrue();
77+
}
78+
79+
[Fact]
80+
public async Task ExplicitAntiforgeryCookiePolicy_OverridesDeploymentDefault()
81+
{
82+
await using var factory = AuthServerFactory.Create(options =>
83+
{
84+
options.HubDeploymentMode = UAuthHubDeploymentMode.External;
85+
options.Antiforgery.CookieSameSite = SameSiteMode.Lax;
86+
});
87+
88+
var antiforgery = factory.Services
89+
.GetRequiredService<IOptions<AntiforgeryOptions>>()
90+
.Value;
91+
92+
antiforgery.Cookie.SameSite.Should().Be(SameSiteMode.Lax);
93+
}
94+
}

‎tests/CodeBeam.UltimateAuth.Tests.Integration/Security/CsrfAttackTests.cs‎

Lines changed: 27 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -33,25 +33,29 @@ public async Task Login_WithoutCsrfToken_MustNotCreateSession()
3333
public async Task Login_WithForgedCsrfToken_MustNotCreateSession()
3434
{
3535
await using var factory = new AuthServerFactory();
36-
var attacker = await factory.CreateLoginUserAsync(ct: TestContext.Current.CancellationToken);
36+
var ct = TestContext.Current.CancellationToken;
37+
38+
var user = await factory.CreateLoginUserAsync(ct: ct);
3739

3840
using var victimBrowser = CreateBrowser(factory);
3941

40-
using var request = new HttpRequestMessage(
41-
HttpMethod.Post, "/auth/login")
42+
// Obtain a real antiforgery cookie and request token.
43+
var tokens = await GetTokens(victimBrowser);
44+
45+
using var request = new HttpRequestMessage(HttpMethod.Post, "/auth/login")
4246
{
4347
Content = JsonContent.Create(new
4448
{
45-
identifier = attacker.Identifier,
46-
secret = attacker.Secret
49+
identifier = user.Identifier,
50+
secret = user.Secret
4751
})
4852
};
4953

50-
request.Headers.TryAddWithoutValidation(
51-
"RequestVerificationToken",
52-
"attacker-forged-token");
54+
// Keep the legitimate cookie, but forge the request token.
55+
request.Headers.TryAddWithoutValidation(tokens.HeaderName, "attacker-forged-token");
56+
request.Headers.TryAddWithoutValidation("Cookie", tokens.Cookie);
5357

54-
using var response = await victimBrowser.SendAsync(request, TestContext.Current.CancellationToken);
58+
using var response = await victimBrowser.SendAsync(request, ct);
5559

5660
await AssertCsrfRejected(response);
5761
AssertNoSessionCookie(response);
@@ -167,28 +171,26 @@ public async Task RevokeOthers_CsrfAttack_MustNotRevokeSessions()
167171
// The attack must not revoke the other session.
168172
await AssertAuthenticated(otherBrowser);
169173

170-
// ATTACK 2: Forged antiforgery token.
171-
using (var forgedRequest = new HttpRequestMessage(
172-
HttpMethod.Post,
173-
"/auth/me/sessions/revoke-others"))
174+
// ATTACK 2: Valid antiforgery cookie with a forged request token.
175+
var forgedTokens = await GetTokens(currentBrowser);
176+
177+
using (var forgedRequest = new HttpRequestMessage(HttpMethod.Post, "/auth/me/sessions/revoke-others"))
174178
{
175-
forgedRequest.Headers.TryAddWithoutValidation(
176-
"RequestVerificationToken",
177-
"forged-token");
179+
forgedRequest.Headers.TryAddWithoutValidation(forgedTokens.HeaderName, "attacker-forged-token");
178180

179-
using var forged = await currentBrowser.SendAsync(
180-
forgedRequest, TestContext.Current.CancellationToken);
181+
// Preserve the authenticated session cookie and
182+
// include the legitimate antiforgery cookie.
183+
forgedRequest.Headers.TryAddWithoutValidation("Cookie", $"{currentCookie}; {forgedTokens.Cookie}");
181184

185+
using var forged = await currentBrowser.SendAsync(forgedRequest, TestContext.Current.CancellationToken);
182186
await AssertCsrfRejected(forged);
183187
}
184188

185-
// The second attack must not revoke the other session.
189+
// A rejected CSRF request must not revoke the other session.
186190
await AssertAuthenticated(otherBrowser);
187191

188192
// CONTROL: Valid CSRF token must allow the operation.
189-
using (var valid = await CsrfTestClient.PostAsync(
190-
currentBrowser,
191-
"/auth/me/sessions/revoke-others"))
193+
using (var valid = await CsrfTestClient.PostAsync(currentBrowser, "/auth/me/sessions/revoke-others"))
192194
{
193195
valid.IsSuccessStatusCode.Should().BeTrue(
194196
$"valid revoke failed: " +
@@ -199,15 +201,12 @@ public async Task RevokeOthers_CsrfAttack_MustNotRevokeSessions()
199201
await AssertAuthenticated(currentBrowser);
200202

201203
// The other session must now be revoked.
202-
using var revokedCheck = await CsrfTestClient.PostJsonAsync(
203-
otherBrowser,
204-
"/auth/me/identifiers/get",
205-
new { });
204+
using var revokedCheck = await CsrfTestClient.PostJsonAsync(otherBrowser, "/auth/me/identifiers/get", new { });
206205

207-
revokedCheck.StatusCode.Should().Be(
208-
HttpStatusCode.Unauthorized);
206+
revokedCheck.StatusCode.Should().Be(HttpStatusCode.Unauthorized);
209207
}
210208

209+
211210
private static async Task<string> LoginAndGetSessionCookie(
212211
HttpClient browser,
213212
IntegrationTestUser user)

0 commit comments

Comments
 (0)