From 5fc0f4cad8daa761422c08bad0aa5c0e9f32284e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Chalet?= Date: Fri, 9 Feb 2018 18:19:09 +0100 Subject: [PATCH] Avoid returning an error when extending the lifetime of a refresh token or revoking previous tokens fails --- src/OpenIddict/OpenIddictProvider.cs | 31 ++++++++----------- .../OpenIddictProviderTests.cs | 5 ++- 2 files changed, 15 insertions(+), 21 deletions(-) diff --git a/src/OpenIddict/OpenIddictProvider.cs b/src/OpenIddict/OpenIddictProvider.cs index ac7a62ae..7efb0d70 100644 --- a/src/OpenIddict/OpenIddictProvider.cs +++ b/src/OpenIddict/OpenIddictProvider.cs @@ -152,6 +152,7 @@ namespace OpenIddict // If rolling tokens are enabled or if the request is a grant_type=authorization_code request, // mark the authorization code or the refresh token as redeemed to prevent future reuses. + // If the operation fails, return an error indicating the code/token is no longer valid. // See https://tools.ietf.org/html/rfc6749#section-6 for more information. if (options.UseRollingTokens || context.Request.IsAuthorizationCodeGrantType()) { @@ -169,30 +170,24 @@ namespace OpenIddict if (context.Request.IsRefreshTokenGrantType()) { - // When rolling tokens are enabled, revoke all the previously issued tokens associated - // with the authorization if the request is a grant_type=refresh_token request. - // If the operation fails, return an error indicating the token is not valid. - if (options.UseRollingTokens && !await TryRevokeTokensAsync(context.Ticket)) + // When rolling tokens are enabled, try to revoke all the previously issued tokens + // associated with the authorization if the request is a refresh_token request. + // If the operation fails, silently ignore the error and keep processing the request: + // this may indicate that one of the revoked tokens was modified by a concurrent request. + if (options.UseRollingTokens) { - context.Reject( - error: OpenIdConnectConstants.Errors.InvalidGrant, - description: "The specified refresh token is no longer valid."); - - return; + await TryRevokeTokensAsync(context.Ticket); } - // When rolling tokens are disabled, extend the expiration date + // When rolling tokens are disabled, try to extend the expiration date // of the existing token instead of returning a new refresh token // with a new expiration date if sliding expiration was not disabled. - // If the operation fails, return an error indicating the token is not valid. - if (!options.UseRollingTokens && options.UseSlidingExpiration && - !await TryExtendTokenAsync(token, context.Ticket, options)) + // If the operation fails, silently ignore the error and keep processing + // the request: this may indicate that a concurrent refresh token request + // already updated the expiration date associated with the refresh token. + if (!options.UseRollingTokens && options.UseSlidingExpiration) { - context.Reject( - error: OpenIdConnectConstants.Errors.InvalidGrant, - description: "The specified refresh token is no longer valid."); - - return; + await TryExtendTokenAsync(token, context.Ticket, options); } } } diff --git a/test/OpenIddict.Tests/OpenIddictProviderTests.cs b/test/OpenIddict.Tests/OpenIddictProviderTests.cs index dfbc48c6..d13cf8dd 100644 --- a/test/OpenIddict.Tests/OpenIddictProviderTests.cs +++ b/test/OpenIddict.Tests/OpenIddictProviderTests.cs @@ -1035,7 +1035,7 @@ namespace OpenIddict.Tests } [Fact] - public async Task ProcessSigninResponse_ReturnsErrorResponseWhenExtendingLifetimeOfExistingTokenFailed() + public async Task ProcessSigninResponse_IgnoresErrorWhenExtendingLifetimeOfExistingTokenFailed() { // Arrange var ticket = new AuthenticationTicket( @@ -1098,8 +1098,7 @@ namespace OpenIddict.Tests }); // Assert - Assert.Equal(OpenIdConnectConstants.Errors.InvalidGrant, response.Error); - Assert.Equal("The specified refresh token is no longer valid.", response.ErrorDescription); + Assert.NotNull(response.AccessToken); Mock.Get(manager).Verify(mock => mock.ExtendAsync(token, new DateTimeOffset(2017, 01, 15, 00, 00, 00, TimeSpan.Zero),