From e1d78178252daf9fe8f72300b3d9a2c3e2a78027 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Chalet?= Date: Thu, 4 Aug 2022 16:01:27 +0200 Subject: [PATCH] Update the client and server stacks to redeem tokens earlier --- .../OpenIddictResources.resx | 3 + .../OpenIddictClientHandlers.Protection.cs | 35 +++- .../OpenIddictClientHandlers.cs | 115 ++++++----- .../OpenIddictServerHandlers.cs | 191 +++++++++++------- ...enIddictServerIntegrationTests.Exchange.cs | 95 +++++++++ 5 files changed, 315 insertions(+), 124 deletions(-) diff --git a/src/OpenIddict.Abstractions/OpenIddictResources.resx b/src/OpenIddict.Abstractions/OpenIddictResources.resx index e6dbccb6..2c59f27b 100644 --- a/src/OpenIddict.Abstractions/OpenIddictResources.resx +++ b/src/OpenIddict.Abstractions/OpenIddictResources.resx @@ -1730,6 +1730,9 @@ Alternatively, you can disable the token storage feature by calling 'services.Ad The current address doesn't match the address of the redirection endpoint selected during the initial authorization request. + + The specified state token has already been redeemed. + The '{0}' parameter shouldn't be null or empty at this point. diff --git a/src/OpenIddict.Client/OpenIddictClientHandlers.Protection.cs b/src/OpenIddict.Client/OpenIddictClientHandlers.Protection.cs index de89f99a..e5b221f1 100644 --- a/src/OpenIddict.Client/OpenIddictClientHandlers.Protection.cs +++ b/src/OpenIddict.Client/OpenIddictClientHandlers.Protection.cs @@ -575,8 +575,41 @@ public static partial class OpenIddictClientHandlers return; } + // If the token entry cannot be found, return a generic error. var token = await _tokenManager.FindByIdAsync(identifier); - if (token is null || !await _tokenManager.HasStatusAsync(token, Statuses.Valid)) + if (token is null) + { + context.Reject( + error: Errors.InvalidToken, + description: SR.GetResourceString(SR.ID2019), + uri: SR.FormatID8000(SR.ID2019)); + + return; + } + + if (await _tokenManager.HasStatusAsync(token, Statuses.Redeemed)) + { + context.Logger.LogInformation(SR.GetResourceString(SR.ID6002), identifier); + + context.Reject( + error: Errors.InvalidToken, + description: context.Principal.GetTokenType() switch + { + TokenTypeHints.StateToken => SR.GetResourceString(SR.ID2139), + + _ => SR.GetResourceString(SR.ID2013) + }, + uri: context.Principal.GetTokenType() switch + { + TokenTypeHints.StateToken => SR.FormatID8000(SR.ID2139), + + _ => SR.FormatID8000(SR.ID2013) + }); + + return; + } + + if (!await _tokenManager.HasStatusAsync(token, Statuses.Valid)) { context.Logger.LogInformation(SR.GetResourceString(SR.ID6005), identifier); diff --git a/src/OpenIddict.Client/OpenIddictClientHandlers.cs b/src/OpenIddict.Client/OpenIddictClientHandlers.cs index ea239074..898a431f 100644 --- a/src/OpenIddict.Client/OpenIddictClientHandlers.cs +++ b/src/OpenIddict.Client/OpenIddictClientHandlers.cs @@ -30,6 +30,7 @@ public static partial class OpenIddictClientHandlers ResolveValidatedStateToken.Descriptor, ValidateRequiredStateToken.Descriptor, ValidateStateToken.Descriptor, + RedeemStateTokenEntry.Descriptor, ResolveClientRegistrationFromStateToken.Descriptor, ValidateIssuerParameter.Descriptor, ValidateFrontchannelErrorParameters.Descriptor, @@ -85,8 +86,6 @@ public static partial class OpenIddictClientHandlers ValidateUserinfoTokenWellknownClaims.Descriptor, ValidateUserinfoTokenSubject.Descriptor, - RedeemStateTokenEntry.Descriptor, - /* * Challenge processing: */ @@ -374,6 +373,66 @@ public static partial class OpenIddictClientHandlers } } + /// + /// Contains the logic responsible for redeeming the token entry corresponding to the received state token. + /// Note: this handler is not used when the degraded mode is enabled. + /// + public class RedeemStateTokenEntry : IOpenIddictClientHandler + { + private readonly IOpenIddictTokenManager _tokenManager; + + public RedeemStateTokenEntry() => throw new InvalidOperationException(SR.GetResourceString(SR.ID0318)); + + public RedeemStateTokenEntry(IOpenIddictTokenManager tokenManager) + => _tokenManager = tokenManager ?? throw new ArgumentNullException(nameof(tokenManager)); + + /// + /// Gets the default descriptor definition assigned to this handler. + /// + public static OpenIddictClientHandlerDescriptor Descriptor { get; } + = OpenIddictClientHandlerDescriptor.CreateBuilder() + .AddFilter() + .AddFilter() + .UseScopedHandler() + // Note: this handler is deliberately executed early in the pipeline to ensure + // that the state token entry is always marked as redeemed even if the authentication + // demand is rejected later in the pipeline (e.g because an error was returned). + .SetOrder(ValidateStateToken.Descriptor.Order + 1_000) + .SetType(OpenIddictClientHandlerType.BuiltIn) + .Build(); + + /// + public async ValueTask HandleAsync(ProcessAuthenticationContext context) + { + if (context is null) + { + throw new ArgumentNullException(nameof(context)); + } + + Debug.Assert(context.StateTokenPrincipal is { Identity: ClaimsIdentity }, SR.GetResourceString(SR.ID4006)); + + // Extract the token identifier from the state token principal. + // If no token identifier can be found, this indicates that the token has no backing database entry. + var identifier = context.StateTokenPrincipal.GetTokenId(); + if (string.IsNullOrEmpty(identifier)) + { + return; + } + + // Mark the token as redeemed to prevent future reuses. + var token = await _tokenManager.FindByIdAsync(identifier); + if (token is not null && !await _tokenManager.TryRedeemAsync(token)) + { + context.Reject( + error: Errors.InvalidToken, + description: SR.GetResourceString(SR.ID2139), + uri: SR.FormatID8000(SR.ID2139)); + + return; + } + } + } + /// /// Contains the logic responsible for resolving the client registration /// based on the authorization server identity stored in the state token. @@ -387,7 +446,7 @@ public static partial class OpenIddictClientHandlers = OpenIddictClientHandlerDescriptor.CreateBuilder() .AddFilter() .UseSingletonHandler() - .SetOrder(ValidateStateToken.Descriptor.Order + 1_000) + .SetOrder(RedeemStateTokenEntry.Descriptor.Order + 1_000) .Build(); /// @@ -3194,56 +3253,6 @@ public static partial class OpenIddictClientHandlers } } - /// - /// Contains the logic responsible for redeeming the token entry corresponding to the received state token. - /// Note: this handler is not used when the degraded mode is enabled. - /// - public class RedeemStateTokenEntry : IOpenIddictClientHandler - { - private readonly IOpenIddictTokenManager _tokenManager; - - public RedeemStateTokenEntry() => throw new InvalidOperationException(SR.GetResourceString(SR.ID0318)); - - public RedeemStateTokenEntry(IOpenIddictTokenManager tokenManager) - => _tokenManager = tokenManager ?? throw new ArgumentNullException(nameof(tokenManager)); - - /// - /// Gets the default descriptor definition assigned to this handler. - /// - public static OpenIddictClientHandlerDescriptor Descriptor { get; } - = OpenIddictClientHandlerDescriptor.CreateBuilder() - .AddFilter() - .AddFilter() - .UseScopedHandler() - .SetOrder(ValidateUserinfoTokenSubject.Descriptor.Order + 1_000) - .SetType(OpenIddictClientHandlerType.BuiltIn) - .Build(); - - /// - public async ValueTask HandleAsync(ProcessAuthenticationContext context) - { - if (context is null) - { - throw new ArgumentNullException(nameof(context)); - } - - Debug.Assert(context.StateTokenPrincipal is { Identity: ClaimsIdentity }, SR.GetResourceString(SR.ID4006)); - - // Extract the token identifier from the state token principal. - // If no token identifier can be found, this indicates that the token has no backing database entry. - var identifier = context.StateTokenPrincipal.GetTokenId(); - if (!string.IsNullOrEmpty(identifier)) - { - // Mark the token as redeemed to prevent future reuses. - var token = await _tokenManager.FindByIdAsync(identifier); - if (token is not null) - { - await _tokenManager.TryRedeemAsync(token); - } - } - } - } - /// /// Contains the logic responsible for rejecting invalid challenge demands. /// diff --git a/src/OpenIddict.Server/OpenIddictServerHandlers.cs b/src/OpenIddict.Server/OpenIddictServerHandlers.cs index 023b0826..11647735 100644 --- a/src/OpenIddict.Server/OpenIddictServerHandlers.cs +++ b/src/OpenIddict.Server/OpenIddictServerHandlers.cs @@ -47,6 +47,7 @@ public static partial class OpenIddictServerHandlers * Sign-in processing: */ ValidateSignInDemand.Descriptor, + RedeemTokenEntry.Descriptor, RestoreInternalClaims.Descriptor, AttachDefaultScopes.Descriptor, AttachDefaultPresenters.Descriptor, @@ -61,8 +62,6 @@ public static partial class OpenIddictServerHandlers PrepareIdentityTokenPrincipal.Descriptor, PrepareUserCodePrincipal.Descriptor, - RedeemTokenEntry.Descriptor, - GenerateAccessToken.Descriptor, GenerateAuthorizationCode.Descriptor, GenerateDeviceCode.Descriptor, @@ -1172,6 +1171,124 @@ public static partial class OpenIddictServerHandlers } } + /// + /// Contains the logic responsible for redeeming the token entry corresponding to + /// the received authorization code, device code, user code or refresh token. + /// Note: this handler is not used when the degraded mode is enabled. + /// + public class RedeemTokenEntry : IOpenIddictServerHandler + { + private readonly IOpenIddictTokenManager _tokenManager; + + public RedeemTokenEntry() => throw new InvalidOperationException(SR.GetResourceString(SR.ID0016)); + + public RedeemTokenEntry(IOpenIddictTokenManager tokenManager) + => _tokenManager = tokenManager ?? throw new ArgumentNullException(nameof(tokenManager)); + + /// + /// Gets the default descriptor definition assigned to this handler. + /// + public static OpenIddictServerHandlerDescriptor Descriptor { get; } + = OpenIddictServerHandlerDescriptor.CreateBuilder() + .AddFilter() + .AddFilter() + .UseScopedHandler() + // Note: this handler is deliberately executed early in the pipeline to ensure + // that the token database entry is always marked as redeemed even if the sign-in + // demand is rejected later in the pipeline (e.g because an error was returned). + .SetOrder(ValidateSignInDemand.Descriptor.Order + 1_000) + .SetType(OpenIddictServerHandlerType.BuiltIn) + .Build(); + + /// + public async ValueTask HandleAsync(ProcessSignInContext context) + { + if (context is null) + { + throw new ArgumentNullException(nameof(context)); + } + + switch (context.EndpointType) + { + case OpenIddictServerEndpointType.Token when context.Request.IsAuthorizationCodeGrantType(): + case OpenIddictServerEndpointType.Token when context.Request.IsDeviceCodeGrantType(): + case OpenIddictServerEndpointType.Token when context.Request.IsRefreshTokenGrantType() && + !context.Options.DisableRollingRefreshTokens: + case OpenIddictServerEndpointType.Verification: + break; + + default: return; + } + + var notification = context.Transaction.GetProperty( + typeof(ProcessAuthenticationContext).FullName!) ?? + throw new InvalidOperationException(SR.GetResourceString(SR.ID0007)); + + var principal = context.EndpointType switch + { + OpenIddictServerEndpointType.Token when context.Request.IsAuthorizationCodeGrantType() + => notification.AuthorizationCodePrincipal, + + OpenIddictServerEndpointType.Token when context.Request.IsDeviceCodeGrantType() + => notification.DeviceCodePrincipal, + + OpenIddictServerEndpointType.Token when context.Request.IsRefreshTokenGrantType() + => notification.RefreshTokenPrincipal, + + OpenIddictServerEndpointType.Verification => notification.UserCodePrincipal, + + _ => null + }; + + Debug.Assert(principal is { Identity: ClaimsIdentity }, SR.GetResourceString(SR.ID4006)); + + // Extract the token identifier from the authentication principal. + // If no token identifier can be found, this indicates that the token has no backing database entry. + var identifier = principal.GetTokenId(); + if (string.IsNullOrEmpty(identifier)) + { + return; + } + + var token = await _tokenManager.FindByIdAsync(identifier); + if (token is null) + { + return; + } + + // Mark the token as redeemed to prevent future reuses. If the request is a refresh token request, ignore + // errors returned while trying to mark the entry as redeemed (that may be caused by concurrent requests). + if (context.EndpointType is OpenIddictServerEndpointType.Token && context.Request.IsRefreshTokenGrantType()) + { + await _tokenManager.TryRedeemAsync(token); + } + + else if (!await _tokenManager.TryRedeemAsync(token)) + { + context.Reject( + error: Errors.InvalidToken, + description: principal.GetTokenType() switch + { + TokenTypeHints.AuthorizationCode => SR.GetResourceString(SR.ID2010), + TokenTypeHints.DeviceCode => SR.GetResourceString(SR.ID2011), + TokenTypeHints.RefreshToken => SR.GetResourceString(SR.ID2012), + + _ => SR.GetResourceString(SR.ID2013) + }, + uri: principal.GetTokenType() switch + { + TokenTypeHints.AuthorizationCode => SR.FormatID8000(SR.ID2010), + TokenTypeHints.DeviceCode => SR.FormatID8000(SR.ID2011), + TokenTypeHints.RefreshToken => SR.FormatID8000(SR.ID2012), + + _ => SR.FormatID8000(SR.ID2013) + }); + + return; + } + } + } + /// /// Contains the logic responsible for re-attaching internal claims to the authentication principal. /// @@ -1183,7 +1300,7 @@ public static partial class OpenIddictServerHandlers public static OpenIddictServerHandlerDescriptor Descriptor { get; } = OpenIddictServerHandlerDescriptor.CreateBuilder() .UseSingletonHandler() - .SetOrder(ValidateSignInDemand.Descriptor.Order + 1_000) + .SetOrder(RedeemTokenEntry.Descriptor.Order + 1_000) .SetType(OpenIddictServerHandlerType.BuiltIn) .Build(); @@ -2125,72 +2242,6 @@ public static partial class OpenIddictServerHandlers } } - /// - /// Contains the logic responsible for redeeming the token entry corresponding to - /// the received authorization code, device code, user code or refresh token. - /// Note: this handler is not used when the degraded mode is enabled. - /// - public class RedeemTokenEntry : IOpenIddictServerHandler - { - private readonly IOpenIddictTokenManager _tokenManager; - - public RedeemTokenEntry() => throw new InvalidOperationException(SR.GetResourceString(SR.ID0016)); - - public RedeemTokenEntry(IOpenIddictTokenManager tokenManager) - => _tokenManager = tokenManager ?? throw new ArgumentNullException(nameof(tokenManager)); - - /// - /// Gets the default descriptor definition assigned to this handler. - /// - public static OpenIddictServerHandlerDescriptor Descriptor { get; } - = OpenIddictServerHandlerDescriptor.CreateBuilder() - .AddFilter() - .AddFilter() - .UseScopedHandler() - .SetOrder(100_000) - .SetType(OpenIddictServerHandlerType.BuiltIn) - .Build(); - - /// - public async ValueTask HandleAsync(ProcessSignInContext context) - { - if (context is null) - { - throw new ArgumentNullException(nameof(context)); - } - - switch (context.EndpointType) - { - case OpenIddictServerEndpointType.Token when context.Request.IsAuthorizationCodeGrantType(): - case OpenIddictServerEndpointType.Token when context.Request.IsDeviceCodeGrantType(): - case OpenIddictServerEndpointType.Token when context.Request.IsRefreshTokenGrantType() && - !context.Options.DisableRollingRefreshTokens: - case OpenIddictServerEndpointType.Verification: - break; - - default: return; - } - - Debug.Assert(context.Principal is { Identity: ClaimsIdentity }, SR.GetResourceString(SR.ID4006)); - - // Extract the token identifier from the authentication principal. - // If no token identifier can be found, this indicates that the token has no backing database entry. - var identifier = context.Principal.GetTokenId(); - if (string.IsNullOrEmpty(identifier)) - { - return; - } - - // If rolling tokens are enabled or if the request is a a code or device code token request - // or a user code verification request, mark the token as redeemed to prevent future reuses. - var token = await _tokenManager.FindByIdAsync(identifier); - if (token is not null) - { - await _tokenManager.TryRedeemAsync(token); - } - } - } - /// /// Contains the logic responsible for generating an access token for the current sign-in operation. /// @@ -2208,7 +2259,7 @@ public static partial class OpenIddictServerHandlers = OpenIddictServerHandlerDescriptor.CreateBuilder() .AddFilter() .UseScopedHandler() - .SetOrder(RedeemTokenEntry.Descriptor.Order + 1_000) + .SetOrder(100_000) .SetType(OpenIddictServerHandlerType.BuiltIn) .Build(); diff --git a/test/OpenIddict.Server.IntegrationTests/OpenIddictServerIntegrationTests.Exchange.cs b/test/OpenIddict.Server.IntegrationTests/OpenIddictServerIntegrationTests.Exchange.cs index 99069604..a33857fa 100644 --- a/test/OpenIddict.Server.IntegrationTests/OpenIddictServerIntegrationTests.Exchange.cs +++ b/test/OpenIddict.Server.IntegrationTests/OpenIddictServerIntegrationTests.Exchange.cs @@ -3805,6 +3805,101 @@ public abstract partial class OpenIddictServerIntegrationTests Mock.Get(manager).Verify(manager => manager.HasStatusAsync(authorization, Statuses.Valid, It.IsAny()), Times.Once()); } + [Fact] + public async Task HandleTokenRequest_RequestIsRejectedWhenAuthorizationCodeCannotBeRedeemed() + { + // Arrange + var token = new OpenIddictToken(); + + var manager = CreateTokenManager(mock => + { + mock.Setup(manager => manager.FindByIdAsync("3E228451-1555-46F7-A471-951EFBA23A56", It.IsAny())) + .ReturnsAsync(token); + + mock.Setup(manager => manager.GetIdAsync(token, It.IsAny())) + .ReturnsAsync("3E228451-1555-46F7-A471-951EFBA23A56"); + + mock.Setup(manager => manager.GetTypeAsync(token, It.IsAny())) + .ReturnsAsync(TokenTypeHints.AuthorizationCode); + + mock.Setup(manager => manager.HasStatusAsync(token, Statuses.Redeemed, It.IsAny())) + .ReturnsAsync(false); + + mock.Setup(manager => manager.HasStatusAsync(token, Statuses.Valid, It.IsAny())) + .ReturnsAsync(true); + + mock.Setup(manager => manager.TryRedeemAsync(token, It.IsAny())) + .ReturnsAsync(false); + + mock.Setup(manager => manager.CreateAsync(It.IsAny(), It.IsAny())) + .ReturnsAsync(new OpenIddictToken()); + }); + + await using var server = await CreateServerAsync(options => + { + options.AddEventHandler(builder => + { + builder.UseInlineHandler(context => + { + Assert.Equal("SplxlOBeZQQYbYS6WxSbIA", context.Token); + Assert.Equal(new[] { TokenTypeHints.AuthorizationCode }, context.ValidTokenTypes); + + context.Principal = new ClaimsPrincipal(new ClaimsIdentity("Bearer")) + .SetTokenType(TokenTypeHints.AuthorizationCode) + .SetPresenters("Fabrikam") + .SetTokenId("3E228451-1555-46F7-A471-951EFBA23A56") + .SetClaim(Claims.Subject, "Bob le Bricoleur"); + + return default; + }); + + builder.SetOrder(ValidateIdentityModelToken.Descriptor.Order - 500); + }); + + options.AddEventHandler(builder => + builder.UseInlineHandler(context => + { + context.Principal = new ClaimsPrincipal(new ClaimsIdentity("Bearer")) + .SetClaim(Claims.Subject, "Bob le Magnifique"); + + return default; + })); + + options.Services.AddSingleton(CreateApplicationManager(mock => + { + var application = new OpenIddictApplication(); + + mock.Setup(manager => manager.FindByClientIdAsync("Fabrikam", It.IsAny())) + .ReturnsAsync(application); + + mock.Setup(manager => manager.HasClientTypeAsync(application, ClientTypes.Public, It.IsAny())) + .ReturnsAsync(true); + })); + + options.Services.AddSingleton(manager); + + options.DisableAuthorizationStorage(); + }); + + await using var client = await server.CreateClientAsync(); + + // Act + var response = await client.PostAsync("/connect/token", new OpenIddictRequest + { + ClientId = "Fabrikam", + Code = "SplxlOBeZQQYbYS6WxSbIA", + GrantType = GrantTypes.AuthorizationCode, + RedirectUri = "http://www.fabrikam.com/path" + }); + + // Assert + Assert.Equal(Errors.InvalidGrant, response.Error); + Assert.Equal(SR.GetResourceString(SR.ID2010), response.ErrorDescription); + Assert.Equal(SR.FormatID8000(SR.ID2010), response.ErrorUri); + + Mock.Get(manager).Verify(manager => manager.TryRedeemAsync(token, It.IsAny()), Times.Once()); + } + [Theory] [InlineData(GrantTypes.AuthorizationCode)] [InlineData(GrantTypes.ClientCredentials)]