From 4af1f96308c75b60e5fc2d3120655e2420968166 Mon Sep 17 00:00:00 2001 From: Dominick Baier Date: Mon, 4 Jan 2021 11:35:12 +0100 Subject: [PATCH 1/4] add option --- .../DependencyInjection/Options/IdentityServerOptions.cs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/IdentityServer/Configuration/DependencyInjection/Options/IdentityServerOptions.cs b/src/IdentityServer/Configuration/DependencyInjection/Options/IdentityServerOptions.cs index 976621ea1..678ecebad 100644 --- a/src/IdentityServer/Configuration/DependencyInjection/Options/IdentityServerOptions.cs +++ b/src/IdentityServer/Configuration/DependencyInjection/Options/IdentityServerOptions.cs @@ -37,12 +37,17 @@ public class IdentityServerOptions /// Emits an aud claim with the format issuer/resources. That's needed for some older access token validation plumbing. Defaults to false. /// public bool EmitStaticAudienceClaim { get; set; } = false; - + /// /// Specifies whether scopes in JWTs are emitted as array or string /// public bool EmitScopesAsSpaceDelimitedStringInJwt { get; set; } = false; + /// + /// Specifies whether the s_hash claim gets emitted in identity tokens. Defaults to false. + /// + public bool EmitStateHash { get; set; } = false; + /// /// Specifies whether the JWT typ and content-type for JWT secured authorization requests is checked according to IETF spec. /// This might break older OIDC conformant request objects. From 7a70af0dc214ffcab39ef5e6ace718aa4f23a84d Mon Sep 17 00:00:00 2001 From: Dominick Baier Date: Mon, 4 Jan 2021 11:35:22 +0100 Subject: [PATCH 2/4] check option in response generator --- .../Default/AuthorizeResponseGenerator.cs | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/IdentityServer/ResponseHandling/Default/AuthorizeResponseGenerator.cs b/src/IdentityServer/ResponseHandling/Default/AuthorizeResponseGenerator.cs index 751529def..dc902cb1c 100644 --- a/src/IdentityServer/ResponseHandling/Default/AuthorizeResponseGenerator.cs +++ b/src/IdentityServer/ResponseHandling/Default/AuthorizeResponseGenerator.cs @@ -14,7 +14,6 @@ using Duende.IdentityServer.Services; using Duende.IdentityServer.Validation; using Microsoft.AspNetCore.Authentication; -using Microsoft.AspNetCore.Http; namespace Duende.IdentityServer.ResponseHandling { @@ -24,6 +23,11 @@ namespace Duende.IdentityServer.ResponseHandling /// public class AuthorizeResponseGenerator : IAuthorizeResponseGenerator { + /// + /// The options + /// + protected IdentityServerOptions Options; + /// /// The token service /// @@ -57,6 +61,7 @@ public class AuthorizeResponseGenerator : IAuthorizeResponseGenerator /// /// Initializes a new instance of the class. /// + /// The options. /// The clock. /// The logger. /// The token service. @@ -64,6 +69,7 @@ public class AuthorizeResponseGenerator : IAuthorizeResponseGenerator /// The authorization code store. /// The events. public AuthorizeResponseGenerator( + IdentityServerOptions options, ISystemClock clock, ITokenService tokenService, IKeyMaterialService keyMaterialService, @@ -71,6 +77,7 @@ public AuthorizeResponseGenerator( ILogger logger, IEventService events) { + Options = options; Clock = clock; TokenService = tokenService; KeyMaterialService = keyMaterialService; @@ -181,7 +188,8 @@ protected virtual async Task CreateImplicitFlowResponseAsync( if (responseTypes.Contains(OidcConstants.ResponseTypes.IdToken)) { string stateHash = null; - if (request.State.IsPresent()) + + if (Options.EmitStateHash && request.State.IsPresent()) { var credential = await KeyMaterialService.GetSigningCredentialsAsync(request.Client.AllowedIdentityTokenSigningAlgorithms); if (credential == null) @@ -229,7 +237,7 @@ protected virtual async Task CreateImplicitFlowResponseAsync( protected virtual async Task CreateCodeAsync(ValidatedAuthorizeRequest request) { string stateHash = null; - if (request.State.IsPresent()) + if (Options.EmitStateHash && request.State.IsPresent()) { var credential = await KeyMaterialService.GetSigningCredentialsAsync(request.Client.AllowedIdentityTokenSigningAlgorithms); if (credential == null) From 56090a031de5fdcc08427357ad6dd4161a1f4f35 Mon Sep 17 00:00:00 2001 From: Dominick Baier Date: Mon, 4 Jan 2021 12:19:52 +0100 Subject: [PATCH 3/4] update tests --- .../Common/IdentityServerPipeline.cs | 7 +++++- .../Conformance/Basic/CodeFlowTests.cs | 22 +++++++++++++++---- 2 files changed, 24 insertions(+), 5 deletions(-) diff --git a/test/IdentityServer.IntegrationTests/Common/IdentityServerPipeline.cs b/test/IdentityServer.IntegrationTests/Common/IdentityServerPipeline.cs index 604f4e94d..da7507cd3 100644 --- a/test/IdentityServer.IntegrationTests/Common/IdentityServerPipeline.cs +++ b/test/IdentityServer.IntegrationTests/Common/IdentityServerPipeline.cs @@ -126,7 +126,10 @@ public void ConfigureServices(IServiceCollection services) services.AddIdentityServer(options => { - Options = options; + if (Options != null) + { + options.EmitStateHash = Options.EmitStateHash; + } options.Events = new EventsOptions { @@ -136,6 +139,8 @@ public void ConfigureServices(IServiceCollection services) RaiseSuccessEvents = true }; options.KeyManagement.Enabled = false; + + Options = options; }) .AddInMemoryClients(Clients) .AddInMemoryIdentityResources(IdentityScopes) diff --git a/test/IdentityServer.IntegrationTests/Conformance/Basic/CodeFlowTests.cs b/test/IdentityServer.IntegrationTests/Conformance/Basic/CodeFlowTests.cs index 69e0c93a1..a7dbab598 100644 --- a/test/IdentityServer.IntegrationTests/Conformance/Basic/CodeFlowTests.cs +++ b/test/IdentityServer.IntegrationTests/Conformance/Basic/CodeFlowTests.cs @@ -112,10 +112,16 @@ public async Task No_state_should_not_result_in_shash() s_hash.Should().BeNull(); } - [Fact] + [Theory] + [InlineData(true)] + [InlineData(false)] [Trait("Category", Category)] - public async Task State_should_result_in_shash() + public async Task StateHash_should_be_emitted_based_on_options(bool emitStateHash) { + // todo: is there a better way to supply per test options? + _pipeline.Options.EmitStateHash = emitStateHash; + _pipeline.Initialize(); + await _pipeline.LoginAsync("bob"); var nonce = Guid.NewGuid().ToString(); @@ -158,8 +164,16 @@ public async Task State_should_result_in_shash() var token = new JwtSecurityToken(tokenResult.IdentityToken); var s_hash = token.Claims.FirstOrDefault(c => c.Type == "s_hash"); - s_hash.Should().NotBeNull(); - s_hash.Value.Should().Be(CryptoHelper.CreateHashClaimValue("state", "RS256")); + + if (emitStateHash) + { + s_hash.Should().NotBeNull(); + s_hash.Value.Should().Be(CryptoHelper.CreateHashClaimValue("state", "RS256")); + } + else + { + s_hash.Should().BeNull(); + } } } } \ No newline at end of file From 48405b29e09f44d640d5719149d00f4845cb5003 Mon Sep 17 00:00:00 2001 From: Brock Allen Date: Tue, 5 Jan 2021 10:10:03 -0500 Subject: [PATCH 4/4] update tests for s_hash --- .../Common/IdentityServerPipeline.cs | 8 ++++---- .../Conformance/Basic/CodeFlowTests.cs | 2 -- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/test/IdentityServer.IntegrationTests/Common/IdentityServerPipeline.cs b/test/IdentityServer.IntegrationTests/Common/IdentityServerPipeline.cs index da7507cd3..eecefe571 100644 --- a/test/IdentityServer.IntegrationTests/Common/IdentityServerPipeline.cs +++ b/test/IdentityServer.IntegrationTests/Common/IdentityServerPipeline.cs @@ -126,10 +126,10 @@ public void ConfigureServices(IServiceCollection services) services.AddIdentityServer(options => { - if (Options != null) - { - options.EmitStateHash = Options.EmitStateHash; - } + //if (Options != null) + //{ + // options.EmitStateHash = Options.EmitStateHash; + //} options.Events = new EventsOptions { diff --git a/test/IdentityServer.IntegrationTests/Conformance/Basic/CodeFlowTests.cs b/test/IdentityServer.IntegrationTests/Conformance/Basic/CodeFlowTests.cs index a7dbab598..054ac0120 100644 --- a/test/IdentityServer.IntegrationTests/Conformance/Basic/CodeFlowTests.cs +++ b/test/IdentityServer.IntegrationTests/Conformance/Basic/CodeFlowTests.cs @@ -118,9 +118,7 @@ public async Task No_state_should_not_result_in_shash() [Trait("Category", Category)] public async Task StateHash_should_be_emitted_based_on_options(bool emitStateHash) { - // todo: is there a better way to supply per test options? _pipeline.Options.EmitStateHash = emitStateHash; - _pipeline.Initialize(); await _pipeline.LoginAsync("bob");