From bc6440ae9e40d0f58db75301e3c44aae8812e0a9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Chalet?= Date: Fri, 9 Feb 2018 16:26:32 +0100 Subject: [PATCH] Avoid changing the status of the token used in the token request when revoking previous tokens --- src/OpenIddict/OpenIddictProvider.Exchange.cs | 1 + src/OpenIddict/OpenIddictProvider.Helpers.cs | 44 ++++++++++++------- .../OpenIddictProviderTests.cs | 2 + 3 files changed, 32 insertions(+), 15 deletions(-) diff --git a/src/OpenIddict/OpenIddictProvider.Exchange.cs b/src/OpenIddict/OpenIddictProvider.Exchange.cs index 4412d52c..adcd590c 100644 --- a/src/OpenIddict/OpenIddictProvider.Exchange.cs +++ b/src/OpenIddict/OpenIddictProvider.Exchange.cs @@ -270,6 +270,7 @@ namespace OpenIddict // response will be returned to the client application. await TryRevokeAuthorizationAsync(context.Ticket); await TryRevokeTokensAsync(context.Ticket); + await TryRevokeTokenAsync(token); Logger.LogError("The token request was rejected because the authorization code " + "or refresh token '{Identifier}' has already been redeemed.", identifier); diff --git a/src/OpenIddict/OpenIddictProvider.Helpers.cs b/src/OpenIddict/OpenIddictProvider.Helpers.cs index 8600cb18..88ac81d0 100644 --- a/src/OpenIddict/OpenIddictProvider.Helpers.cs +++ b/src/OpenIddict/OpenIddictProvider.Helpers.cs @@ -402,6 +402,31 @@ namespace OpenIddict } } + private async Task TryRevokeTokenAsync([NotNull] TToken token) + { + var identifier = await Tokens.GetIdAsync(token); + Debug.Assert(!string.IsNullOrEmpty(identifier), "The token identifier shouldn't be null or empty."); + + try + { + // Note: the request cancellation token is deliberately not used here to ensure the caller + // cannot prevent this operation from being executed by resetting the TCP connection. + await Tokens.RevokeAsync(token); + + Logger.LogInformation("The token '{Identifier}' was automatically revoked.", identifier); + + return true; + } + + catch (Exception exception) + { + Logger.LogWarning(exception, "An exception occurred while trying to revoke " + + "the token '{Identifier}'.", identifier); + + return false; + } + } + private async Task TryRevokeTokensAsync([NotNull] AuthenticationTicket ticket) { // Note: if the authorization identifier is null, return true as no tokens need to be revoked. @@ -413,24 +438,13 @@ namespace OpenIddict foreach (var token in await Tokens.FindByAuthorizationIdAsync(identifier)) { - try + // Don't change the status of the token used in the token request. + if (string.Equals(ticket.GetTokenId(), await Tokens.GetIdAsync(token), StringComparison.Ordinal)) { - // Note: the request cancellation token is deliberately not used here to ensure the caller - // cannot prevent this operation from being executed by resetting the TCP connection. - await Tokens.RevokeAsync(token); - - Logger.LogInformation("The token '{Identifier}' was automatically revoked.", - await Tokens.GetIdAsync(token)); + continue; } - catch (Exception exception) - { - Logger.LogWarning(exception, "An exception occurred while trying to revoke the tokens " + - "associated with the token '{Identifier}'.", - await Tokens.GetIdAsync(token)); - - return false; - } + await TryRevokeTokenAsync(token); } return true; diff --git a/test/OpenIddict.Tests/OpenIddictProviderTests.cs b/test/OpenIddict.Tests/OpenIddictProviderTests.cs index 13860dd2..dfbc48c6 100644 --- a/test/OpenIddict.Tests/OpenIddictProviderTests.cs +++ b/test/OpenIddict.Tests/OpenIddictProviderTests.cs @@ -824,6 +824,7 @@ namespace OpenIddict.Tests Assert.NotNull(response.RefreshToken); Mock.Get(manager).Verify(mock => mock.FindByIdAsync("60FFF7EA-F98E-437B-937E-5073CC313103", It.IsAny()), Times.Once()); + Mock.Get(manager).Verify(mock => mock.RevokeAsync(tokens[0], It.IsAny()), Times.Never()); Mock.Get(manager).Verify(mock => mock.RevokeAsync(tokens[1], It.IsAny()), Times.Once()); Mock.Get(manager).Verify(mock => mock.RevokeAsync(tokens[2], It.IsAny()), Times.Once()); } @@ -893,6 +894,7 @@ namespace OpenIddict.Tests Assert.Null(response.RefreshToken); Mock.Get(manager).Verify(mock => mock.FindByIdAsync("60FFF7EA-F98E-437B-937E-5073CC313103", It.IsAny()), Times.Once()); + Mock.Get(manager).Verify(mock => mock.RevokeAsync(tokens[0], It.IsAny()), Times.Never()); Mock.Get(manager).Verify(mock => mock.RevokeAsync(tokens[1], It.IsAny()), Times.Never()); Mock.Get(manager).Verify(mock => mock.RevokeAsync(tokens[2], It.IsAny()), Times.Never()); }