diff --git a/src/OpenIddict.Client/OpenIddictClientHandlers.cs b/src/OpenIddict.Client/OpenIddictClientHandlers.cs index 018729d2..e608d6ff 100644 --- a/src/OpenIddict.Client/OpenIddictClientHandlers.cs +++ b/src/OpenIddict.Client/OpenIddictClientHandlers.cs @@ -4581,8 +4581,9 @@ public static partial class OpenIddictClientHandlers => (GrantTypes.Implicit, ResponseTypes.IdToken), // Note: response types combinations containing "token" are always tested last as some - // authorization servers - like OpenIddict - are known to block authorization requests - // asking for an access token if Proof Key for Code Exchange is used in the same request. + // authorization servers (e.g OpenIddict when response type permissions are disabled) + // are known to mitigate downgrade attacks by blocking authorization requests asking + // for an access token if Proof Key for Code Exchange is used in the same request. // // Returning an identity token directly from the authorization endpoint also has privacy // concerns that code-based flows - that require a backchannel request - typically don't diff --git a/src/OpenIddict.Server/OpenIddictServerHandlers.Authentication.cs b/src/OpenIddict.Server/OpenIddictServerHandlers.Authentication.cs index 933a8dcd..0be08e40 100644 --- a/src/OpenIddict.Server/OpenIddictServerHandlers.Authentication.cs +++ b/src/OpenIddict.Server/OpenIddictServerHandlers.Authentication.cs @@ -996,19 +996,6 @@ public static partial class OpenIddictServerHandlers return default; } - // Reject authorization requests that contain response_type=token when a code_challenge is specified. - if (context.Request.HasResponseType(ResponseTypes.Token)) - { - context.Logger.LogInformation(SR.GetResourceString(SR.ID6043)); - - context.Reject( - error: Errors.InvalidRequest, - description: SR.FormatID2041(Parameters.ResponseType), - uri: SR.FormatID8000(SR.ID2041)); - - return default; - } - return default; } } @@ -1075,27 +1062,34 @@ public static partial class OpenIddictServerHandlers } /// - /// Contains the logic responsible for rejecting authorization requests that use a - /// response_type containing token if the application is a confidential client. - /// Note: this handler is not used when the degraded mode is enabled - /// or when response type permissions enforcement is not disabled. + /// Contains the logic responsible for rejecting authorization requests that use an unsafe response type. /// public sealed class ValidateResponseType : IOpenIddictServerHandler { - private readonly IOpenIddictApplicationManager _applicationManager; + private readonly IOpenIddictApplicationManager? _applicationManager; - public ValidateResponseType() => throw new InvalidOperationException(SR.GetResourceString(SR.ID0016)); + [Obsolete("This constructor is no longer supported and will be removed in a future version.", error: true)] + public ValidateResponseType() => throw new NotSupportedException(SR.GetResourceString(SR.ID0403)); - public ValidateResponseType(IOpenIddictApplicationManager applicationManager) - => _applicationManager = applicationManager ?? throw new ArgumentNullException(nameof(applicationManager)); + public ValidateResponseType(IOpenIddictApplicationManager? applicationManager = null) + => _applicationManager = applicationManager; /// /// Gets the default descriptor definition assigned to this handler. /// public static OpenIddictServerHandlerDescriptor Descriptor { get; } = OpenIddictServerHandlerDescriptor.CreateBuilder() - .AddFilter() - .UseScopedHandler() + .UseScopedHandler(static provider => + { + // Note: the application manager is only resolved if the degraded mode was not enabled to ensure + // invalid core configuration exceptions are not thrown even if the managers were registered. + var options = provider.GetRequiredService>().CurrentValue; + + return options.EnableDegradedMode ? + new ValidateResponseType(applicationManager: null) : + new ValidateResponseType(provider.GetService() ?? + throw new InvalidOperationException(SR.GetResourceString(SR.ID0016))); + }) .SetOrder(ValidateAuthentication.Descriptor.Order + 1_000) .SetType(OpenIddictServerHandlerType.BuiltIn) .Build(); @@ -1108,33 +1102,61 @@ public static partial class OpenIddictServerHandlers throw new ArgumentNullException(nameof(context)); } - Debug.Assert(!string.IsNullOrEmpty(context.ClientId), SR.FormatID4000(Parameters.ClientId)); - - var application = await _applicationManager.FindByClientIdAsync(context.ClientId) ?? - throw new InvalidOperationException(SR.GetResourceString(SR.ID0032)); - - // To prevent downgrade attacks, ensure that authorization requests returning an access token directly - // from the authorization endpoint are rejected if the client_id corresponds to a confidential application - // and if response type permissions enforcement was explicitly disabled in the server options. - // Users who want to enable this advanced scenario are encouraged to re-enable permissions validation. + // Note: this handler is responsible for enforcing additional response_type requirements when + // response type permissions are not used (and thus cannot be finely controlled per client). // - // Alternatively, this handler can be removed from the handlers list using the events model APIs. - if (!context.Options.IgnoreResponseTypePermissions || !context.Request.HasResponseType(ResponseTypes.Token)) + // Users who want to support the scenarios disallowed by this event handler are encouraged + // to re-enable permissions validation. Alternatively, this handler can be removed from + // the handlers list and replaced by a custom version using the events model APIs. + if (!context.Options.IgnoreResponseTypePermissions) { return; } - if (await _applicationManager.HasClientTypeAsync(application, ClientTypes.Confidential)) + Debug.Assert(!string.IsNullOrEmpty(context.ClientId), SR.FormatID4000(Parameters.ClientId)); + + // When PKCE is used, reject authorization requests returning an access token directly + // from the authorization endpoint to prevent a malicious client from retrieving a valid + // access token - even with a limited scope - without sending the correct code_verifier. + if (!string.IsNullOrEmpty(context.Request.CodeChallenge) && + context.Request.HasResponseType(ResponseTypes.Token)) { - context.Logger.LogInformation(SR.GetResourceString(SR.ID6045), context.ClientId); + context.Logger.LogInformation(SR.GetResourceString(SR.ID6043)); context.Reject( error: Errors.UnauthorizedClient, - description: SR.FormatID2043(Parameters.ResponseType), - uri: SR.FormatID8000(SR.ID2043)); + description: SR.FormatID2041(Parameters.ResponseType), + uri: SR.FormatID8000(SR.ID2041)); return; } + + if (!context.Options.EnableDegradedMode) + { + if (_applicationManager is null) + { + throw new InvalidOperationException(SR.GetResourceString(SR.ID0016)); + } + + var application = await _applicationManager.FindByClientIdAsync(context.ClientId) ?? + throw new InvalidOperationException(SR.GetResourceString(SR.ID0032)); + + // To prevent downgrade attacks, ensure that authorization requests returning + // an access token directly from the authorization endpoint are rejected if + // the client_id corresponds to a confidential application. + if (context.Request.HasResponseType(ResponseTypes.Token) && + await _applicationManager.HasClientTypeAsync(application, ClientTypes.Confidential)) + { + context.Logger.LogInformation(SR.GetResourceString(SR.ID6045), context.ClientId); + + context.Reject( + error: Errors.UnauthorizedClient, + description: SR.FormatID2043(Parameters.ResponseType), + uri: SR.FormatID8000(SR.ID2043)); + + return; + } + } } } diff --git a/test/OpenIddict.Server.IntegrationTests/OpenIddictServerIntegrationTests.Authentication.cs b/test/OpenIddict.Server.IntegrationTests/OpenIddictServerIntegrationTests.Authentication.cs index 9f363d00..930d0c94 100644 --- a/test/OpenIddict.Server.IntegrationTests/OpenIddictServerIntegrationTests.Authentication.cs +++ b/test/OpenIddict.Server.IntegrationTests/OpenIddictServerIntegrationTests.Authentication.cs @@ -1045,33 +1045,6 @@ public abstract partial class OpenIddictServerIntegrationTests Assert.NotNull(response.Code); } - [Theory] - [InlineData("code id_token token")] - [InlineData("code token")] - public async Task ValidateAuthorizationRequest_PkceRequestWithForbiddenResponseTypeIsRejected(string type) - { - // Arrange - await using var server = await CreateServerAsync(options => options.EnableDegradedMode()); - await using var client = await server.CreateClientAsync(); - - // Act - var response = await client.PostAsync("/connect/authorize", new OpenIddictRequest - { - ClientId = "Fabrikam", - CodeChallenge = "E9Melhoa2OwvFrEMTJguCHaoeK1t8URWbuGJSstw-cM", - CodeChallengeMethod = CodeChallengeMethods.Sha256, - Nonce = "n-0S6_WzA2Mj", - RedirectUri = "http://www.fabrikam.com/path", - ResponseType = type, - Scope = Scopes.OpenId - }); - - // Assert - Assert.Equal(Errors.InvalidRequest, response.Error); - Assert.Equal(SR.FormatID2041(Parameters.ResponseType), response.ErrorDescription); - Assert.Equal(SR.FormatID8000(SR.ID2041), response.ErrorUri); - } - [Fact] public async Task ValidateAuthorizationRequest_RequestIsRejectedWhenRedirectUriIsMissing() { @@ -1183,6 +1156,154 @@ public abstract partial class OpenIddictServerIntegrationTests Mock.Get(manager).Verify(manager => manager.FindByClientIdAsync("Fabrikam", It.IsAny()), Times.AtLeastOnce()); } + [Theory] + [InlineData("code id_token token")] + [InlineData("code token")] + public async Task ValidateAuthorizationRequest_PkceRequestWithSensitiveResponseTypeIsRejectedWhenDegradedModeIsEnabled(string type) + { + // Arrange + await using var server = await CreateServerAsync(options => options.EnableDegradedMode()); + await using var client = await server.CreateClientAsync(); + + // Act + var response = await client.PostAsync("/connect/authorize", new OpenIddictRequest + { + ClientId = "Fabrikam", + CodeChallenge = "E9Melhoa2OwvFrEMTJguCHaoeK1t8URWbuGJSstw-cM", + CodeChallengeMethod = CodeChallengeMethods.Sha256, + Nonce = "n-0S6_WzA2Mj", + RedirectUri = "http://www.fabrikam.com/path", + ResponseType = type, + Scope = Scopes.OpenId + }); + + // Assert + Assert.Equal(Errors.UnauthorizedClient, response.Error); + Assert.Equal(SR.FormatID2041(Parameters.ResponseType), response.ErrorDescription); + Assert.Equal(SR.FormatID8000(SR.ID2041), response.ErrorUri); + } + + [Theory] + [InlineData("code id_token token")] + [InlineData("code token")] + public async Task ValidateAuthorizationRequest_PkceRequestWithSensitiveResponseTypeIsRejectedWhenPermissionsAreIgnored(string type) + { + // Arrange + var application = new OpenIddictApplication(); + + var manager = CreateApplicationManager(mock => + { + mock.Setup(manager => manager.FindByClientIdAsync("Fabrikam", It.IsAny())) + .ReturnsAsync(application); + + mock.Setup(manager => manager.ValidateRedirectUriAsync(application, "http://www.fabrikam.com/path", It.IsAny())) + .ReturnsAsync(true); + + mock.Setup(manager => manager.GetSettingsAsync(application, It.IsAny())) + .ReturnsAsync(ImmutableDictionary.Create()); + }); + + await using var server = await CreateServerAsync(options => + { + options.Services.AddSingleton(manager); + + options.AddEventHandler(builder => + builder.UseInlineHandler(context => + { + context.Principal = new ClaimsPrincipal(new ClaimsIdentity("Bearer")) + .SetClaim(Claims.Subject, "Bob le Magnifique"); + + return default; + })); + }); + + await using var client = await server.CreateClientAsync(); + + // Act + var response = await client.PostAsync("/connect/authorize", new OpenIddictRequest + { + ClientId = "Fabrikam", + CodeChallenge = "E9Melhoa2OwvFrEMTJguCHaoeK1t8URWbuGJSstw-cM", + CodeChallengeMethod = CodeChallengeMethods.Sha256, + Nonce = "n-0S6_WzA2Mj", + RedirectUri = "http://www.fabrikam.com/path", + ResponseType = type, + Scope = Scopes.OpenId + }); + + // Assert + Assert.Equal(Errors.UnauthorizedClient, response.Error); + Assert.Equal(SR.FormatID2041(Parameters.ResponseType), response.ErrorDescription); + Assert.Equal(SR.FormatID8000(SR.ID2041), response.ErrorUri); + } + + [Theory] + [InlineData("code id_token token")] + [InlineData("code token")] + public async Task ValidateAuthorizationRequest_PkceRequestWithSensitiveResponseTypeIsValidatedWhenPermissionsAreEnforced(string type) + { + // Arrange + var application = new OpenIddictApplication(); + + var manager = CreateApplicationManager(mock => + { + mock.Setup(manager => manager.FindByClientIdAsync("Fabrikam", It.IsAny())) + .ReturnsAsync(application); + + mock.Setup(manager => manager.GetPermissionsAsync(application, It.IsAny())) + .ReturnsAsync(ImmutableArray.Create("rst:" + type)); + + mock.Setup(manager => manager.ValidateRedirectUriAsync(application, "http://www.fabrikam.com/path", It.IsAny())) + .ReturnsAsync(true); + + mock.Setup(manager => manager.GetSettingsAsync(application, It.IsAny())) + .ReturnsAsync(ImmutableDictionary.Create()); + }); + + await using var server = await CreateServerAsync(options => + { + options.SetDeviceEndpointUris(Array.Empty()); + options.SetRevocationEndpointUris(Array.Empty()); + options.Configure(options => options.GrantTypes.Remove(GrantTypes.DeviceCode)); + options.DisableAuthorizationStorage(); + options.DisableTokenStorage(); + options.DisableSlidingRefreshTokenExpiration(); + + options.Configure(options => options.IgnoreResponseTypePermissions = false); + + options.Services.AddSingleton(manager); + + options.AddEventHandler(builder => + builder.UseInlineHandler(context => + { + context.Principal = new ClaimsPrincipal(new ClaimsIdentity("Bearer")) + .SetClaim(Claims.Subject, "Bob le Magnifique"); + + return default; + })); + }); + + await using var client = await server.CreateClientAsync(); + + // Act + var response = await client.PostAsync("/connect/authorize", new OpenIddictRequest + { + ClientId = "Fabrikam", + CodeChallenge = "E9Melhoa2OwvFrEMTJguCHaoeK1t8URWbuGJSstw-cM", + CodeChallengeMethod = CodeChallengeMethods.Sha256, + Nonce = "n-0S6_WzA2Mj", + RedirectUri = "http://www.fabrikam.com/path", + ResponseType = type, + Scope = Scopes.OpenId + }); + + // 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")]