diff --git a/src/OpenIddict.Server/OpenIddictServerBuilder.cs b/src/OpenIddict.Server/OpenIddictServerBuilder.cs index 6cfb3243..1793d59d 100644 --- a/src/OpenIddict.Server/OpenIddictServerBuilder.cs +++ b/src/OpenIddict.Server/OpenIddictServerBuilder.cs @@ -674,6 +674,16 @@ namespace Microsoft.Extensions.DependencyInjection return Configure(options => options.Scopes.UnionWith(scopes)); } + /// + /// 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. + /// + /// The . + public OpenIddictServerBuilder RequireProofKeyForCodeExchange() + => Configure(options => options.RequireProofKeyForCodeExchange = true); + /// /// Sets the access token lifetime, after which client applications must retrieve /// a new access token by making a grant_type=refresh_token token request diff --git a/src/OpenIddict.Server/OpenIddictServerOptions.cs b/src/OpenIddict.Server/OpenIddictServerOptions.cs index 63b054c7..277fb3e6 100644 --- a/src/OpenIddict.Server/OpenIddictServerOptions.cs +++ b/src/OpenIddict.Server/OpenIddictServerOptions.cs @@ -122,6 +122,14 @@ namespace OpenIddict.Server SlidingExpiration = TimeSpan.FromMinutes(30) }; + /// + /// 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 true, authorization requests that lack the + /// code_challenge/code_challenge_method parameters will be automatically rejected. + /// + public bool RequireProofKeyForCodeExchange { get; set; } + /// /// Gets the OAuth2/OpenID Connect scopes enabled for this application. /// diff --git a/src/OpenIddict.Server/OpenIddictServerProvider.Authentication.cs b/src/OpenIddict.Server/OpenIddictServerProvider.Authentication.cs index 5ba1f4a2..af9f2dc8 100644 --- a/src/OpenIddict.Server/OpenIddictServerProvider.Authentication.cs +++ b/src/OpenIddict.Server/OpenIddictServerProvider.Authentication.cs @@ -239,6 +239,21 @@ namespace OpenIddict.Server 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 // code_challenge_method can't be specified without code_challenge. if (!string.IsNullOrEmpty(context.Request.CodeChallenge)) diff --git a/src/OpenIddict.Server/OpenIddictServerProvider.Exchange.cs b/src/OpenIddict.Server/OpenIddictServerProvider.Exchange.cs index 768dbfb9..e67fa85b 100644 --- a/src/OpenIddict.Server/OpenIddictServerProvider.Exchange.cs +++ b/src/OpenIddict.Server/OpenIddictServerProvider.Exchange.cs @@ -54,18 +54,36 @@ namespace OpenIddict.Server return; } - // Optimization: the OpenID Connect server middleware automatically rejects grant_type=authorization_code - // 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)) + if (context.Request.IsAuthorizationCodeGrantType()) { - context.Reject( - error: OpenIddictConstants.Errors.InvalidRequest, - description: "The mandatory 'redirect_uri' parameter is missing."); + // Optimization: the OpenID Connect server middleware automatically rejects grant_type=authorization_code + // 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 (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, diff --git a/test/OpenIddict.Server.Tests/OpenIddictServerBuilderTests.cs b/test/OpenIddict.Server.Tests/OpenIddictServerBuilderTests.cs index 8fa273ec..7379d6d9 100644 --- a/test/OpenIddict.Server.Tests/OpenIddictServerBuilderTests.cs +++ b/test/OpenIddict.Server.Tests/OpenIddictServerBuilderTests.cs @@ -848,6 +848,22 @@ namespace OpenIddict.Server.Tests 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] public void UseDataProtectionProvider_DefaultProviderIsReplaced() { diff --git a/test/OpenIddict.Server.Tests/OpenIddictServerProviderTests.Authentication.cs b/test/OpenIddict.Server.Tests/OpenIddictServerProviderTests.Authentication.cs index 5f12541e..7c685c1b 100644 --- a/test/OpenIddict.Server.Tests/OpenIddictServerProviderTests.Authentication.cs +++ b/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); } + [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] public async Task ValidateAuthorizationRequest_RequestIsRejectedWhenCodeChallengeMethodIsMissing() { diff --git a/test/OpenIddict.Server.Tests/OpenIddictServerProviderTests.Exchange.cs b/test/OpenIddict.Server.Tests/OpenIddictServerProviderTests.Exchange.cs index cf35ebbc..18a742d3 100644 --- a/test/OpenIddict.Server.Tests/OpenIddictServerProviderTests.Exchange.cs +++ b/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); } + [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] public async Task ValidateTokenRequest_RequestIsRejectedWhenUnregisteredScopeIsSpecified() {