diff --git a/src/OpenIddict.Server/OpenIddictServerConfiguration.cs b/src/OpenIddict.Server/OpenIddictServerConfiguration.cs index e6e6a895..0cdfb1f5 100644 --- a/src/OpenIddict.Server/OpenIddictServerConfiguration.cs +++ b/src/OpenIddict.Server/OpenIddictServerConfiguration.cs @@ -248,6 +248,8 @@ namespace OpenIddict.Server if (options.GrantTypes.Contains(GrantTypes.AuthorizationCode)) { + options.CodeChallengeMethods.Add(CodeChallengeMethods.Sha256); + options.ResponseTypes.Add(ResponseTypes.Code); } diff --git a/src/OpenIddict.Server/OpenIddictServerHandlers.Authentication.cs b/src/OpenIddict.Server/OpenIddictServerHandlers.Authentication.cs index d1820de5..cc7c785b 100644 --- a/src/OpenIddict.Server/OpenIddictServerHandlers.Authentication.cs +++ b/src/OpenIddict.Server/OpenIddictServerHandlers.Authentication.cs @@ -950,6 +950,35 @@ namespace OpenIddict.Server return default; } + // If the plain code challenge method was not explicitly enabled, + // reject the request indicating that a method must be set. + if (string.IsNullOrEmpty(context.Request.CodeChallengeMethod) && + !context.Options.CodeChallengeMethods.Contains(CodeChallengeMethods.Plain)) + { + context.Logger.LogError("The authorization request was rejected because the " + + "required 'code_challenge_method' parameter was missing."); + + context.Reject( + error: Errors.InvalidRequest, + description: "The 'code_challenge_method' parameter must be specified."); + + return default; + } + + // If a code_challenge_method was specified, ensure the algorithm is supported. + if (!string.IsNullOrEmpty(context.Request.CodeChallengeMethod) && + !context.Options.CodeChallengeMethods.Contains(context.Request.CodeChallengeMethod)) + { + context.Logger.LogError("The authorization request was rejected because " + + "the specified code challenge method was not supported."); + + context.Reject( + error: Errors.InvalidRequest, + description: "The specified 'code_challenge_method' parameter is not supported."); + + return default; + } + // When code_challenge or code_challenge_method is specified, ensure the response_type includes "code". if (!context.Request.HasResponseType(ResponseTypes.Code)) { @@ -977,21 +1006,6 @@ namespace OpenIddict.Server return default; } - // If a code_challenge_method was specified, ensure the algorithm is supported. - if (!string.IsNullOrEmpty(context.Request.CodeChallengeMethod) && - !string.Equals(context.Request.CodeChallengeMethod, CodeChallengeMethods.Plain, StringComparison.Ordinal) && - !string.Equals(context.Request.CodeChallengeMethod, CodeChallengeMethods.Sha256, StringComparison.Ordinal)) - { - context.Logger.LogError("The authorization request was rejected because " + - "the specified code challenge method was not supported."); - - context.Reject( - error: Errors.InvalidRequest, - description: "The specified code_challenge_method is not supported."); - - return default; - } - return default; } } diff --git a/src/OpenIddict.Server/OpenIddictServerHandlers.Discovery.cs b/src/OpenIddict.Server/OpenIddictServerHandlers.Discovery.cs index 6bc42abb..7103e42d 100644 --- a/src/OpenIddict.Server/OpenIddictServerHandlers.Discovery.cs +++ b/src/OpenIddict.Server/OpenIddictServerHandlers.Discovery.cs @@ -671,14 +671,7 @@ namespace OpenIddict.Server throw new ArgumentNullException(nameof(context)); } - // Only populate code_challenge_methods_supported if the code flow was enabled. - if (context.GrantTypes.Contains(GrantTypes.AuthorizationCode)) - { - // Note: supporting S256 is mandatory for authorization servers that implement PKCE. - // See https://tools.ietf.org/html/rfc7636#section-4.2 for more information. - context.CodeChallengeMethods.Add(CodeChallengeMethods.Plain); - context.CodeChallengeMethods.Add(CodeChallengeMethods.Sha256); - } + context.CodeChallengeMethods.UnionWith(context.Options.CodeChallengeMethods); return default; } diff --git a/src/OpenIddict.Server/OpenIddictServerHandlers.Exchange.cs b/src/OpenIddict.Server/OpenIddictServerHandlers.Exchange.cs index cc5ab2c3..3c086556 100644 --- a/src/OpenIddict.Server/OpenIddictServerHandlers.Exchange.cs +++ b/src/OpenIddict.Server/OpenIddictServerHandlers.Exchange.cs @@ -1636,7 +1636,7 @@ namespace OpenIddict.Server var method = context.Principal.GetClaim(Claims.Private.CodeChallengeMethod); if (string.IsNullOrEmpty(method)) { - method = CodeChallengeMethods.Sha256; + throw new InvalidOperationException("The code challenge method cannot be retrieved from the authorization code."); } // Note: when using the "plain" code challenge method, no hashing is actually performed. @@ -1656,13 +1656,7 @@ namespace OpenIddict.Server else { - context.Logger.LogError("The token request was rejected because the 'code_challenge_method' was invalid."); - - context.Reject( - error: Errors.InvalidGrant, - description: "The specified 'code_challenge_method' is invalid."); - - return default; + throw new InvalidOperationException("The specified code challenge method is not supported."); } // Compare the verifier and the code challenge: if the two don't match, return an error. diff --git a/src/OpenIddict.Server/OpenIddictServerHandlers.cs b/src/OpenIddict.Server/OpenIddictServerHandlers.cs index 3ed24a3b..0f46d385 100644 --- a/src/OpenIddict.Server/OpenIddictServerHandlers.cs +++ b/src/OpenIddict.Server/OpenIddictServerHandlers.cs @@ -1908,10 +1908,10 @@ namespace OpenIddict.Server { principal.SetClaim(Claims.Private.CodeChallenge, context.Request.CodeChallenge); - // Default to S256 if no explicit code challenge method was specified. + // Default to plain if no explicit code challenge method was specified. principal.SetClaim(Claims.Private.CodeChallengeMethod, !string.IsNullOrEmpty(context.Request.CodeChallengeMethod) ? - context.Request.CodeChallengeMethod : CodeChallengeMethods.Sha256); + context.Request.CodeChallengeMethod : CodeChallengeMethods.Plain); } // Attach the nonce so that it can be later returned by diff --git a/src/OpenIddict.Server/OpenIddictServerOptions.cs b/src/OpenIddict.Server/OpenIddictServerOptions.cs index 1b32fff1..c8fdc0e5 100644 --- a/src/OpenIddict.Server/OpenIddictServerOptions.cs +++ b/src/OpenIddict.Server/OpenIddictServerOptions.cs @@ -237,6 +237,12 @@ namespace OpenIddict.Server /// public bool DisableScopeValidation { get; set; } + /// + /// Gets the OAuth 2.0 code challenge methods enabled for this application. + /// By default, only the S256 method is allowed (if the code flow is enabled). + /// + public ISet CodeChallengeMethods { get; } = new HashSet(StringComparer.Ordinal); + /// /// Gets the OAuth 2.0/OpenID Connect flows enabled for this application. /// diff --git a/test/OpenIddict.Server.IntegrationTests/OpenIddictServerIntegrationTests.Authentication.cs b/test/OpenIddict.Server.IntegrationTests/OpenIddictServerIntegrationTests.Authentication.cs index 355a2538..3505d888 100644 --- a/test/OpenIddict.Server.IntegrationTests/OpenIddictServerIntegrationTests.Authentication.cs +++ b/test/OpenIddict.Server.IntegrationTests/OpenIddictServerIntegrationTests.Authentication.cs @@ -452,7 +452,7 @@ namespace OpenIddict.Server.FunctionalTests // Assert Assert.Equal(Errors.InvalidRequest, response.Error); - Assert.Equal("The specified code_challenge_method is not supported.", response.ErrorDescription); + Assert.Equal("The specified 'code_challenge_method' parameter is not supported.", response.ErrorDescription); } [Fact] @@ -720,7 +720,7 @@ namespace OpenIddict.Server.FunctionalTests Assert.Equal("The specified 'response_mode' parameter is not supported.", response.ErrorDescription); } - [Fact(Skip = "The handler responsible of rejecting such requests has not been ported.")] + [Fact] public async Task ValidateAuthorizationRequest_RequestIsRejectedWhenCodeChallengeMethodIsMissing() { // Arrange @@ -741,8 +741,34 @@ namespace OpenIddict.Server.FunctionalTests Assert.Equal("The 'code_challenge_method' parameter must be specified.", response.ErrorDescription); } - [Fact(Skip = "The handler responsible of rejecting such requests has not been ported.")] - public async Task ValidateAuthorizationRequest_RequestIsRejectedWhenCodeChallengeMethodIsPlain() + [Fact] + public async Task ValidateAuthorizationRequest_RequestIsRejectedWhenCodeChallengeMethodIsNotEnabled() + { + // Arrange + var client = CreateClient(options => + { + options.EnableDegradedMode(); + options.Services.PostConfigure(options => + options.CodeChallengeMethods.Clear()); + }); + + // Act + var response = await client.PostAsync("/connect/authorize", new OpenIddictRequest + { + ClientId = "Fabrikam", + CodeChallenge = "E9Melhoa2OwvFrEMTJguCHaoeK1t8URWbuGJSstw-cM", + CodeChallengeMethod = CodeChallengeMethods.Sha256, + RedirectUri = "http://www.fabrikam.com/path", + ResponseType = ResponseTypes.Code + }); + + // Assert + Assert.Equal(Errors.InvalidRequest, response.Error); + Assert.Equal("The specified 'code_challenge_method' parameter is not supported.", response.ErrorDescription); + } + + [Fact] + public async Task ValidateAuthorizationRequest_RequestIsRejectedWhenPlainCodeChallengeMethodIsNotExplicitlyEnabled() { // Arrange var client = CreateClient(options => options.EnableDegradedMode()); @@ -759,13 +785,44 @@ namespace OpenIddict.Server.FunctionalTests // Assert Assert.Equal(Errors.InvalidRequest, response.Error); - Assert.Equal("The specified 'code_challenge_method' parameter is not allowed.", response.ErrorDescription); + Assert.Equal("The specified 'code_challenge_method' parameter is not supported.", response.ErrorDescription); + } + + [Theory] + [InlineData(CodeChallengeMethods.Plain)] + [InlineData(CodeChallengeMethods.Sha256)] + [InlineData("custom_code_challenge_method")] + public async Task ValidateAuthorizationRequest_RequestIsValidatedWhenCodeChallengeMethodIsRegistered(string method) + { + // Arrange + var client = CreateClient(options => + { + options.EnableDegradedMode(); + options.Configure(options => options.CodeChallengeMethods.Clear()); + options.Configure(options => options.CodeChallengeMethods.Add(method)); + }); + + // Act + var response = await client.PostAsync("/connect/authorize", new OpenIddictRequest + { + ClientId = "Fabrikam", + CodeChallenge = "E9Melhoa2OwvFrEMTJguCHaoeK1t8URWbuGJSstw-cM", + CodeChallengeMethod = method, + RedirectUri = "http://www.fabrikam.com/path", + ResponseType = ResponseTypes.Code + }); + + // Assert + Assert.Null(response.Error); + Assert.Null(response.ErrorDescription); + Assert.Null(response.ErrorUri); + Assert.NotNull(response.Code); } [Theory] [InlineData("code id_token token")] [InlineData("code token")] - public async Task ValidateAuthorizationRequest_CodeChallengeRequestWithForbiddenResponseTypeIsRejected(string type) + public async Task ValidateAuthorizationRequest_PkceRequestWithForbiddenResponseTypeIsRejected(string type) { // Arrange var client = CreateClient(options => options.EnableDegradedMode()); diff --git a/test/OpenIddict.Server.IntegrationTests/OpenIddictServerIntegrationTests.Exchange.cs b/test/OpenIddict.Server.IntegrationTests/OpenIddictServerIntegrationTests.Exchange.cs index 2a7e61fb..858d922c 100644 --- a/test/OpenIddict.Server.IntegrationTests/OpenIddictServerIntegrationTests.Exchange.cs +++ b/test/OpenIddict.Server.IntegrationTests/OpenIddictServerIntegrationTests.Exchange.cs @@ -697,6 +697,90 @@ namespace OpenIddict.Server.FunctionalTests Assert.Equal("The mandatory 'code_verifier' parameter is missing.", response.ErrorDescription); } + [Fact] + public async Task ValidateTokenRequest_AuthorizationCodeCausesAnErrorWhenCodeChallengeMethodIsMIssing() + { + // Arrange + var client = CreateClient(options => + { + options.EnableDegradedMode(); + + options.AddEventHandler(builder => + { + builder.UseInlineHandler(context => + { + Assert.Equal("SplxlOBeZQQYbYS6WxSbIA", context.Token); + Assert.Equal(TokenTypeHints.AuthorizationCode, context.TokenType); + + context.Principal = new ClaimsPrincipal(new ClaimsIdentity("Bearer")) + .SetPresenters("Fabrikam") + .SetClaim(Claims.Private.CodeChallenge, "E9Melhoa2OwvFrEMTJguCHaoeK1t8URWbuGJSstw-cM") + .SetClaim(Claims.Private.CodeChallengeMethod, null); + + return default; + }); + + builder.SetOrder(ValidateIdentityModelToken.Descriptor.Order - 500); + }); + }); + + // Act and assert + var exception = await Assert.ThrowsAsync(delegate + { + return client.PostAsync("/connect/token", new OpenIddictRequest + { + ClientId = "Fabrikam", + Code = "SplxlOBeZQQYbYS6WxSbIA", + CodeVerifier = "E9Melhoa2OwvFrEMTJguCHaoeK1t8URWbuGJSstw-cM", + GrantType = GrantTypes.AuthorizationCode + }); + }); + + Assert.Equal("The code challenge method cannot be retrieved from the authorization code.", exception.Message); + } + + [Fact] + public async Task ValidateTokenRequest_AuthorizationCodeCausesAnErrorWhenCodeChallengeMethodIsInvalid() + { + // Arrange + var client = CreateClient(options => + { + options.EnableDegradedMode(); + + options.AddEventHandler(builder => + { + builder.UseInlineHandler(context => + { + Assert.Equal("SplxlOBeZQQYbYS6WxSbIA", context.Token); + Assert.Equal(TokenTypeHints.AuthorizationCode, context.TokenType); + + context.Principal = new ClaimsPrincipal(new ClaimsIdentity("Bearer")) + .SetPresenters("Fabrikam") + .SetClaim(Claims.Private.CodeChallenge, "E9Melhoa2OwvFrEMTJguCHaoeK1t8URWbuGJSstw-cM") + .SetClaim(Claims.Private.CodeChallengeMethod, "custom_code_challenge_method"); + + return default; + }); + + builder.SetOrder(ValidateIdentityModelToken.Descriptor.Order - 500); + }); + }); + + // Act and assert + var exception = await Assert.ThrowsAsync(delegate + { + return client.PostAsync("/connect/token", new OpenIddictRequest + { + ClientId = "Fabrikam", + Code = "SplxlOBeZQQYbYS6WxSbIA", + CodeVerifier = "E9Melhoa2OwvFrEMTJguCHaoeK1t8URWbuGJSstw-cM", + GrantType = GrantTypes.AuthorizationCode + }); + }); + + Assert.Equal("The specified code challenge method is not supported.", exception.Message); + } + [Theory] [InlineData(CodeChallengeMethods.Plain, "challenge", "invalid_verifier")] [InlineData(CodeChallengeMethods.Sha256, "challenge", "invalid_verifier")]