Skip to content

Commit 8be2708

Browse files
committed
Implement PKCE for OIDC #7734
1 parent b6d8c96 commit 8be2708

File tree

3 files changed

+70
-2
lines changed

3 files changed

+70
-2
lines changed

src/Security/Authentication/OpenIdConnect/src/OpenIdConnectHandler.cs

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
using System.Text.Json;
1616
using System.Threading.Tasks;
1717
using Microsoft.AspNetCore.Http;
18+
using Microsoft.AspNetCore.WebUtilities;
1819
using Microsoft.Extensions.Logging;
1920
using Microsoft.Extensions.Options;
2021
using Microsoft.Extensions.Primitives;
@@ -30,8 +31,11 @@ namespace Microsoft.AspNetCore.Authentication.OpenIdConnect
3031
public class OpenIdConnectHandler : RemoteAuthenticationHandler<OpenIdConnectOptions>, IAuthenticationSignOutHandler
3132
{
3233
private const string NonceProperty = "N";
33-
3434
private const string HeaderValueEpocDate = "Thu, 01 Jan 1970 00:00:00 GMT";
35+
private const string CodeVerifierKey = "code_verifier";
36+
private const string CodeChallengeKey = "code_challenge";
37+
private const string CodeChallengeMethodKey = "code_challenge_method";
38+
private const string CodeChallengeMethodS256 = "S256";
3539

3640
private static readonly RandomNumberGenerator CryptoRandom = RandomNumberGenerator.Create();
3741

@@ -366,6 +370,24 @@ private async Task HandleChallengeAsyncInternal(AuthenticationProperties propert
366370
Scope = string.Join(" ", properties.GetParameter<ICollection<string>>(OpenIdConnectParameterNames.Scope) ?? Options.Scope),
367371
};
368372

373+
// https://tools.ietf.org/html/rfc7636
374+
if (Options.UsePkse)
375+
{
376+
var verifierBytes = new byte[32];
377+
CryptoRandom.GetBytes(verifierBytes);
378+
var codeVerifier = WebEncoders.Base64UrlEncode(verifierBytes);
379+
380+
// Store this for use during the code redemption. See RunAuthorizationCodeReceivedEventAsync.
381+
properties.Items.Add(CodeVerifierKey, codeVerifier);
382+
383+
using var sha256 = SHA256.Create();
384+
var challengeBytes = sha256.ComputeHash(Encoding.UTF8.GetBytes(codeVerifier));
385+
var codeChallenge = WebEncoders.Base64UrlEncode(challengeBytes);
386+
387+
message.Parameters.Add(CodeChallengeKey, codeChallenge);
388+
message.Parameters.Add(CodeChallengeMethodKey, CodeChallengeMethodS256);
389+
}
390+
369391
// Add the 'max_age' parameter to the authentication request if MaxAge is not null.
370392
// See http://openid.net/specs/openid-connect-core-1_0.html#AuthRequest
371393
var maxAge = properties.GetParameter<TimeSpan?>(OpenIdConnectParameterNames.MaxAge) ?? Options.MaxAge;
@@ -1097,6 +1119,12 @@ private async Task<AuthorizationCodeReceivedContext> RunAuthorizationCodeReceive
10971119
RedirectUri = properties.Items[OpenIdConnectDefaults.RedirectUriForCodePropertiesKey]
10981120
};
10991121

1122+
// PKCE https://tools.ietf.org/html/rfc7636#section-4.5, see HandleChallengeAsyncInternal
1123+
if (properties.Items.TryGetValue(CodeVerifierKey, out var codeVerifier))
1124+
{
1125+
tokenEndpointRequest.Parameters.Add(CodeVerifierKey, codeVerifier);
1126+
}
1127+
11001128
var context = new AuthorizationCodeReceivedContext(Context, Scheme, Options, properties)
11011129
{
11021130
ProtocolMessage = authorizationResponse,

src/Security/Authentication/OpenIdConnect/src/OpenIdConnectOptions.cs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -299,6 +299,13 @@ public CookieBuilder NonceCookie
299299
set => _nonceCookieBuilder = value ?? throw new ArgumentNullException(nameof(value));
300300
}
301301

302+
/// <summary>
303+
/// Enables or disables the use of the Proof Key for Code Exchange (PKCE) standard.
304+
/// See https://tools.ietf.org/html/rfc7636.
305+
/// The default value is `true`.
306+
/// </summary>
307+
public bool UsePkse { get; set; } = true;
308+
302309
private class OpenIdConnectNonceCookieBuilder : RequestPathBaseCookieBuilder
303310
{
304311
private readonly OpenIdConnectOptions _options;

src/Security/Authentication/test/OpenIdConnect/OpenIdConnectChallengeTests.cs

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,39 @@ public async Task ChallengeRedirectIsIssuedCorrectly()
4848
OpenIdConnectParameterNames.VersionTelemetry);
4949
}
5050

51+
[Theory]
52+
[InlineData(true)]
53+
[InlineData(false)]
54+
public async Task ChallengeIncludesPkceIfRequested(bool include)
55+
{
56+
var settings = new TestSettings(
57+
opt =>
58+
{
59+
opt.Authority = TestServerBuilder.DefaultAuthority;
60+
opt.AuthenticationMethod = OpenIdConnectRedirectBehavior.RedirectGet;
61+
opt.ClientId = "Test Id";
62+
opt.UsePkse = include;
63+
});
64+
65+
var server = settings.CreateTestServer();
66+
var transaction = await server.SendAsync(ChallengeEndpoint);
67+
68+
var res = transaction.Response;
69+
Assert.Equal(HttpStatusCode.Redirect, res.StatusCode);
70+
Assert.NotNull(res.Headers.Location);
71+
72+
if (include)
73+
{
74+
Assert.Contains("code_challenge=", res.Headers.Location.Query);
75+
Assert.Contains("code_challenge_method=S256", res.Headers.Location.Query);
76+
}
77+
else
78+
{
79+
Assert.DoesNotContain("code_challenge=", res.Headers.Location.Query);
80+
Assert.DoesNotContain("code_challenge_method=", res.Headers.Location.Query);
81+
}
82+
}
83+
5184
[Fact]
5285
public async Task AuthorizationRequestDoesNotIncludeTelemetryParametersWhenDisabled()
5386
{
@@ -613,4 +646,4 @@ public async Task Challenge_HasOverwrittenMaxAgeParaFromBaseAuthenticationProper
613646
Assert.Contains("max_age=1234", res.Headers.Location.Query);
614647
}
615648
}
616-
}
649+
}

0 commit comments

Comments
 (0)