diff --git a/src/IdentityServer/Stores/Default/DefaultGrantStore.cs b/src/IdentityServer/Stores/Default/DefaultGrantStore.cs index e5fa28ea8..127282e0a 100644 --- a/src/IdentityServer/Stores/Default/DefaultGrantStore.cs +++ b/src/IdentityServer/Stores/Default/DefaultGrantStore.cs @@ -72,7 +72,11 @@ protected DefaultGrantStore(string grantType, } private const string KeySeparator = ":"; - const string HexEncodingFormatSuffix = "-1"; + + /// + /// The suffix added to keys to indicate that hex encoding should be used. + /// + protected const string HexEncodingFormatSuffix = "-1"; /// /// Creates a handle. diff --git a/src/IdentityServer/Stores/Default/DefaultUserConsentStore.cs b/src/IdentityServer/Stores/Default/DefaultUserConsentStore.cs index ca72e56a9..bb98ef06b 100644 --- a/src/IdentityServer/Stores/Default/DefaultUserConsentStore.cs +++ b/src/IdentityServer/Stores/Default/DefaultUserConsentStore.cs @@ -31,9 +31,15 @@ public DefaultUserConsentStore( { } - private string GetConsentKey(string subjectId, string clientId) + private string GetConsentKey(string subjectId, string clientId, bool useHexEncoding = true) { - return clientId + "|" + subjectId; + if(useHexEncoding) + { + return $"{clientId}|{subjectId}{HexEncodingFormatSuffix}"; + } else + { + return $"{clientId}|{subjectId}"; + } } /// @@ -55,12 +61,24 @@ public Task StoreUserConsentAsync(Consent consent) /// The subject identifier. /// The client identifier. /// - public Task GetUserConsentAsync(string subjectId, string clientId) + public async Task GetUserConsentAsync(string subjectId, string clientId) { using var activity = Tracing.StoreActivitySource.StartActivity("DefaultUserConsentStore.GetUserConsent"); var key = GetConsentKey(subjectId, clientId); - return GetItemAsync(key); + var consent = await GetItemAsync(key); + if(consent == null) + { + var legacyKey = GetConsentKey(subjectId, clientId, useHexEncoding: false); + consent = await GetItemAsync(legacyKey); + if(consent != null) + { + await StoreUserConsentAsync(consent); // Write back the consent record to update its key + await RemoveItemAsync(legacyKey); + } + } + + return consent; } /// diff --git a/test/IdentityServer.IntegrationTests/Endpoints/Authorize/ConsentTests.cs b/test/IdentityServer.IntegrationTests/Endpoints/Authorize/ConsentTests.cs index 8933fd109..dfbb1532f 100644 --- a/test/IdentityServer.IntegrationTests/Endpoints/Authorize/ConsentTests.cs +++ b/test/IdentityServer.IntegrationTests/Endpoints/Authorize/ConsentTests.cs @@ -6,16 +6,21 @@ using System.Collections.Generic; using System.Net; using System.Security.Claims; +using System.Security.Cryptography; +using System.Text; +using System.Text.Json; using System.Threading.Tasks; using Duende.IdentityServer; using Duende.IdentityServer.Models; using Duende.IdentityServer.Stores; using Duende.IdentityServer.Stores.Default; +using Duende.IdentityServer.Stores.Serialization; using Duende.IdentityServer.Test; using FluentAssertions; using IntegrationTests.Common; using Microsoft.Extensions.DependencyInjection; using Xunit; +using static Duende.IdentityServer.Models.IdentityResources; namespace IntegrationTests.Endpoints.Authorize; @@ -359,4 +364,80 @@ public async Task consent_response_of_unmet_authentication_requirements_should_r authorization.Error.Should().Be("unmet_authentication_requirements"); authorization.ErrorDescription.Should().Be("some description"); } + + [Fact] + [Trait("Category", Category)] + public async Task legacy_consents_should_apply_and_be_migrated_to_hex_encoding() + { + var clientId = "client2"; + var subjectId = "bob"; + + // Create and serialize a consent record + _mockPipeline.Options.PersistentGrants.DataProtectData = false; + var serializer = _mockPipeline.Resolve(); + var serialized = serializer.Serialize(new Consent + { + ClientId = clientId, + SubjectId = subjectId, + CreationTime = DateTime.UtcNow, + Scopes = new List { "openid" } + }); + + // Store the consent using the legacy key format + var persistedGrantStore = _mockPipeline.Resolve(); + var legacyKey = $"{clientId}|{subjectId}:{IdentityServerConstants.PersistedGrantTypes.UserConsent}".Sha256(); + var legacyConsent = new PersistedGrant + { + Key = legacyKey, + Type = IdentityServerConstants.PersistedGrantTypes.UserConsent, + ClientId = clientId, + SubjectId = subjectId, + SessionId = Guid.NewGuid().ToString(), + Description = null, + CreationTime = DateTime.UtcNow, + Expiration = null, + ConsumedTime = null, + Data = serialized + }; + await persistedGrantStore.StoreAsync(legacyConsent); + + // Create a session cookie + await _mockPipeline.LoginAsync("bob"); + + // Start a challenge + var url = _mockPipeline.CreateAuthorizeUrl( + clientId: "client2", + responseType: "id_token", + scope: "openid", + redirectUri: "https://client2/callback", + state: "123_state", + nonce: "123_nonce" + ); + _mockPipeline.BrowserClient.AllowAutoRedirect = false; + var response = await _mockPipeline.BrowserClient.GetAsync(url); + + // The existing legacy consent should apply - user isn't show consent screen + response.StatusCode.Should().Be(HttpStatusCode.Redirect); + response.Headers.Location.ToString().Should().StartWith("https://client2/callback"); + _mockPipeline.ConsentWasCalled.Should().BeFalse(); + + // The legacy consent should be migrated to use a new key... + + // Old key shouldn't find anything + var grant = await persistedGrantStore.GetAsync(legacyKey); + grant.Should().BeNull(); + + // New key should + var hexEncodedKeyNoHash = $"{clientId}|{subjectId}-1:{IdentityServerConstants.PersistedGrantTypes.UserConsent}"; + using (var sha = SHA256.Create()) + { + var bytes = Encoding.UTF8.GetBytes(hexEncodedKeyNoHash); + var hash = sha.ComputeHash(bytes); + var hexEncodedKey = BitConverter.ToString(hash).Replace("-", ""); + grant = await persistedGrantStore.GetAsync(hexEncodedKey); + grant.Should().NotBeNull(); + grant.ClientId.Should().Be(clientId); + grant.SubjectId.Should().Be(subjectId); + } + } }