Browse Source

Add a new option allowing to make PKCE mandatory

pull/758/head
Kévin Chalet 7 years ago
committed by GitHub
parent
commit
8ff0e97586
No known key found for this signature in database GPG Key ID: 4AEE18F83AFDEB23
  1. 10
      src/OpenIddict.Server/OpenIddictServerBuilder.cs
  2. 8
      src/OpenIddict.Server/OpenIddictServerOptions.cs
  3. 15
      src/OpenIddict.Server/OpenIddictServerProvider.Authentication.cs
  4. 38
      src/OpenIddict.Server/OpenIddictServerProvider.Exchange.cs
  5. 16
      test/OpenIddict.Server.Tests/OpenIddictServerBuilderTests.cs
  6. 22
      test/OpenIddict.Server.Tests/OpenIddictServerProviderTests.Authentication.cs
  7. 23
      test/OpenIddict.Server.Tests/OpenIddictServerProviderTests.Exchange.cs

10
src/OpenIddict.Server/OpenIddictServerBuilder.cs

@ -674,6 +674,16 @@ namespace Microsoft.Extensions.DependencyInjection
return Configure(options => options.Scopes.UnionWith(scopes)); return Configure(options => options.Scopes.UnionWith(scopes));
} }
/// <summary>
/// Configures OpenIddict to force client applications to use Proof Key for Code Exchange
/// (PKCE) when requesting an authorization code (e.g when using the code or hybrid flows).
/// When enforced, authorization requests that lack the code_challenge or
/// code_challenge_method parameters will be automatically rejected by OpenIddict.
/// </summary>
/// <returns>The <see cref="OpenIddictServerBuilder"/>.</returns>
public OpenIddictServerBuilder RequireProofKeyForCodeExchange()
=> Configure(options => options.RequireProofKeyForCodeExchange = true);
/// <summary> /// <summary>
/// Sets the access token lifetime, after which client applications must retrieve /// Sets the access token lifetime, after which client applications must retrieve
/// a new access token by making a grant_type=refresh_token token request /// a new access token by making a grant_type=refresh_token token request

8
src/OpenIddict.Server/OpenIddictServerOptions.cs

@ -122,6 +122,14 @@ namespace OpenIddict.Server
SlidingExpiration = TimeSpan.FromMinutes(30) SlidingExpiration = TimeSpan.FromMinutes(30)
}; };
/// <summary>
/// Gets or sets a boolean indicating whether PKCE must be used by client applications
/// when requesting an authorization code (e.g when using the code or hybrid flows).
/// If this property is set to <c>true</c>, authorization requests that lack the
/// code_challenge/code_challenge_method parameters will be automatically rejected.
/// </summary>
public bool RequireProofKeyForCodeExchange { get; set; }
/// <summary> /// <summary>
/// Gets the OAuth2/OpenID Connect scopes enabled for this application. /// Gets the OAuth2/OpenID Connect scopes enabled for this application.
/// </summary> /// </summary>

15
src/OpenIddict.Server/OpenIddictServerProvider.Authentication.cs

@ -239,6 +239,21 @@ namespace OpenIddict.Server
return; return;
} }
// If OpenIddict was configured to require PKCE, reject the request if the code challenge
// is missing and if an authorization code was requested by the client application.
if (options.RequireProofKeyForCodeExchange && string.IsNullOrEmpty(context.Request.CodeChallenge) &&
context.Request.HasResponseType(OpenIddictConstants.ResponseTypes.Code))
{
_logger.LogError("The authorization request was rejected because the " +
"required 'code_challenge' parameter was missing.");
context.Reject(
error: OpenIddictConstants.Errors.InvalidRequest,
description: "The mandatory 'code_challenge' parameter is missing.");
return;
}
// Note: the OpenID Connect server middleware always ensures a // Note: the OpenID Connect server middleware always ensures a
// code_challenge_method can't be specified without code_challenge. // code_challenge_method can't be specified without code_challenge.
if (!string.IsNullOrEmpty(context.Request.CodeChallenge)) if (!string.IsNullOrEmpty(context.Request.CodeChallenge))

38
src/OpenIddict.Server/OpenIddictServerProvider.Exchange.cs

@ -54,18 +54,36 @@ namespace OpenIddict.Server
return; return;
} }
// Optimization: the OpenID Connect server middleware automatically rejects grant_type=authorization_code if (context.Request.IsAuthorizationCodeGrantType())
// requests missing the redirect_uri parameter if one was specified in the initial authorization request.
// Since OpenIddict doesn't allow redirect_uri-less authorization requests, an earlier check can be made here,
// which saves the OpenID Connect server middleware from having to deserialize the authorization code ticket.
// See http://openid.net/specs/openid-connect-core-1_0.html#TokenRequestValidation for more information.
if (context.Request.IsAuthorizationCodeGrantType() && string.IsNullOrEmpty(context.Request.RedirectUri))
{ {
context.Reject( // Optimization: the OpenID Connect server middleware automatically rejects grant_type=authorization_code
error: OpenIddictConstants.Errors.InvalidRequest, // requests missing the redirect_uri parameter if one was specified in the initial authorization request.
description: "The mandatory 'redirect_uri' parameter is missing."); // Since OpenIddict doesn't allow redirect_uri-less authorization requests, an earlier check can be made here,
// which saves the OpenID Connect server middleware from having to deserialize the authorization code ticket.
// See http://openid.net/specs/openid-connect-core-1_0.html#TokenRequestValidation for more information.
if (string.IsNullOrEmpty(context.Request.RedirectUri))
{
context.Reject(
error: OpenIddictConstants.Errors.InvalidRequest,
description: "The mandatory 'redirect_uri' parameter is missing.");
return; return;
}
// Optimization: the OpenID Connect server middleware automatically rejects grant_type=authorization_code
// requests missing the code_verifier parameter when a challenge was specified in the authorization request.
// That check requires decrypting the authorization code and determining whether a code challenge was set.
// If OpenIddict was configured to require PKCE, this can be potentially avoided by making an early check here.
if (options.RequireProofKeyForCodeExchange && string.IsNullOrEmpty(context.Request.CodeVerifier))
{
_logger.LogError("The token request was rejected because the required 'code_verifier' parameter was missing.");
context.Reject(
error: OpenIddictConstants.Errors.InvalidRequest,
description: "The mandatory 'code_verifier' parameter is missing.");
return;
}
} }
// Note: the OpenID Connect server middleware allows returning a refresh token with grant_type=client_credentials, // Note: the OpenID Connect server middleware allows returning a refresh token with grant_type=client_credentials,

16
test/OpenIddict.Server.Tests/OpenIddictServerBuilderTests.cs

@ -848,6 +848,22 @@ namespace OpenIddict.Server.Tests
Assert.Contains("custom_scope_2", options.Scopes); Assert.Contains("custom_scope_2", options.Scopes);
} }
[Fact]
public void RequireProofKeyForCodeExchange_PkceIsEnforced()
{
// Arrange
var services = CreateServices();
var builder = CreateBuilder(services);
// Act
builder.RequireProofKeyForCodeExchange();
var options = GetOptions(services);
// Assert
Assert.True(options.RequireProofKeyForCodeExchange);
}
[Fact] [Fact]
public void UseDataProtectionProvider_DefaultProviderIsReplaced() public void UseDataProtectionProvider_DefaultProviderIsReplaced()
{ {

22
test/OpenIddict.Server.Tests/OpenIddictServerProviderTests.Authentication.cs

@ -392,6 +392,28 @@ namespace OpenIddict.Server.Tests
Assert.Equal("The mandatory 'redirect_uri' parameter is missing.", response.ErrorDescription); Assert.Equal("The mandatory 'redirect_uri' parameter is missing.", response.ErrorDescription);
} }
[Fact]
public async Task ValidateAuthorizationRequest_RequestIsRejectedWhenPkceIsRequiredAndCodeChallengeIsMissing()
{
// Arrange
var server = CreateAuthorizationServer(builder => builder.RequireProofKeyForCodeExchange());
var client = new OpenIdConnectClient(server.CreateClient());
// Act
var response = await client.PostAsync(AuthorizationEndpoint, new OpenIdConnectRequest
{
ClientId = "Fabrikam",
CodeChallenge = null,
RedirectUri = "http://www.fabrikam.com/path",
ResponseType = OpenIddictConstants.ResponseTypes.Code
});
// Assert
Assert.Equal(OpenIddictConstants.Errors.InvalidRequest, response.Error);
Assert.Equal("The mandatory 'code_challenge' parameter is missing.", response.ErrorDescription);
}
[Fact] [Fact]
public async Task ValidateAuthorizationRequest_RequestIsRejectedWhenCodeChallengeMethodIsMissing() public async Task ValidateAuthorizationRequest_RequestIsRejectedWhenCodeChallengeMethodIsMissing()
{ {

23
test/OpenIddict.Server.Tests/OpenIddictServerProviderTests.Exchange.cs

@ -99,6 +99,29 @@ namespace OpenIddict.Server.Tests
Assert.Equal("The mandatory 'redirect_uri' parameter is missing.", response.ErrorDescription); Assert.Equal("The mandatory 'redirect_uri' parameter is missing.", response.ErrorDescription);
} }
[Fact]
public async Task ValidateTokenRequest_AuthorizationCodeRequestIsRejectedWhenCodeVerifierIsMissing()
{
// Arrange
var server = CreateAuthorizationServer(builder => builder.RequireProofKeyForCodeExchange());
var client = new OpenIdConnectClient(server.CreateClient());
// Act
var response = await client.PostAsync(TokenEndpoint, new OpenIdConnectRequest
{
ClientId = "Fabrikam",
Code = "SplxlOBeZQQYbYS6WxSbIA",
CodeVerifier = null,
GrantType = OpenIddictConstants.GrantTypes.AuthorizationCode,
RedirectUri = "http://www.fabrikam.com/path"
});
// Assert
Assert.Equal(OpenIddictConstants.Errors.InvalidRequest, response.Error);
Assert.Equal("The mandatory 'code_verifier' parameter is missing.", response.ErrorDescription);
}
[Fact] [Fact]
public async Task ValidateTokenRequest_RequestIsRejectedWhenUnregisteredScopeIsSpecified() public async Task ValidateTokenRequest_RequestIsRejectedWhenUnregisteredScopeIsSpecified()
{ {

Loading…
Cancel
Save