From ea2f3720684ccbb02224563d2c47239d9b19efb1 Mon Sep 17 00:00:00 2001 From: Joe DeCock Date: Wed, 30 Aug 2023 08:36:07 -0500 Subject: [PATCH 1/6] Add test for public clients refreshing with DPoP --- .../Endpoints/Token/DPoPTokenEndpointTests.cs | 75 ++++++++++++++++++- 1 file changed, 74 insertions(+), 1 deletion(-) diff --git a/test/IdentityServer.IntegrationTests/Endpoints/Token/DPoPTokenEndpointTests.cs b/test/IdentityServer.IntegrationTests/Endpoints/Token/DPoPTokenEndpointTests.cs index f1f5cf58f..71311efca 100644 --- a/test/IdentityServer.IntegrationTests/Endpoints/Token/DPoPTokenEndpointTests.cs +++ b/test/IdentityServer.IntegrationTests/Endpoints/Token/DPoPTokenEndpointTests.cs @@ -73,8 +73,9 @@ public DPoPTokenEndpointTests() RedirectUris = { "https://client1/callback" }, RequirePkce = false, AllowOfflineAccess = true, + RefreshTokenUsage = TokenUsage.ReUse, AllowedScopes = new List { "openid", "profile", "scope1" }, - }, + } }); _mockPipeline.Users.Add(new TestUser @@ -689,6 +690,78 @@ public async Task public_client_should_not_be_able_to_use_different_dpop_key_for rtResponse.Error.Should().Be("invalid_dpop_proof"); } + [Fact] + [Trait("Category", Category)] + public async Task public_client_using_same_dpop_key_for_refresh_token_request_should_succeed() + { + _dpopClient.RequireClientSecret = false; + + await _mockPipeline.LoginAsync("bob"); + + _mockPipeline.BrowserClient.AllowAutoRedirect = false; + + var url = _mockPipeline.CreateAuthorizeUrl( + clientId: "client1", + responseType: "code", + responseMode: "query", + scope: "openid scope1 offline_access", + redirectUri: "https://client1/callback"); + var response = await _mockPipeline.BrowserClient.GetAsync(url); + + response.StatusCode.Should().Be(HttpStatusCode.Redirect); + response.Headers.Location.ToString().Should().StartWith("https://client1/callback"); + + var authorization = new AuthorizeResponse(response.Headers.Location.ToString()); + authorization.IsError.Should().BeFalse(); + + var codeRequest = new AuthorizationCodeTokenRequest + { + Address = IdentityServerPipeline.TokenEndpoint, + ClientId = "client1", + ClientSecret = "secret", + Code = authorization.Code, + RedirectUri = "https://client1/callback", + }; + codeRequest.Headers.Add("DPoP", CreateDPoPProofToken()); + + var codeResponse = await _mockPipeline.BackChannelClient.RequestAuthorizationCodeTokenAsync(codeRequest); + codeResponse.IsError.Should().BeFalse(); + codeResponse.TokenType.Should().Be("DPoP"); + GetJKTFromAccessToken(codeResponse).Should().Be(_JKT); + + var firstRefreshRequest = new RefreshTokenRequest + { + Address = IdentityServerPipeline.TokenEndpoint, + ClientId = "client1", + ClientSecret = "secret", + RefreshToken = codeResponse.RefreshToken + }; + firstRefreshRequest.Headers.Add("DPoP", CreateDPoPProofToken()); + + var firstRefreshResponse = await _mockPipeline.BackChannelClient.RequestRefreshTokenAsync(firstRefreshRequest); + firstRefreshResponse.IsError.Should().BeFalse(); + firstRefreshResponse.TokenType.Should().Be("DPoP"); + GetJKTFromAccessToken(firstRefreshResponse).Should().Be(_JKT); + + var secondRefreshRequest = new RefreshTokenRequest + { + Address = IdentityServerPipeline.TokenEndpoint, + ClientId = "client1", + ClientSecret = "secret", + RefreshToken = codeResponse.RefreshToken + }; + secondRefreshRequest.Headers.Add("DPoP", CreateDPoPProofToken()); + + firstRefreshRequest.Headers.GetValues("DPoP").FirstOrDefault().Should().NotBe( + secondRefreshRequest.Headers.GetValues("DPoP").FirstOrDefault()); + + var secondRefreshResponse = await _mockPipeline.BackChannelClient.RequestRefreshTokenAsync(secondRefreshRequest); + secondRefreshResponse.IsError.Should().BeFalse(secondRefreshResponse.Error); + secondRefreshResponse.TokenType.Should().Be("DPoP"); + GetJKTFromAccessToken(secondRefreshResponse).Should().Be(_JKT); + } + + [Fact] [Trait("Category", Category)] public async Task missing_proof_token_when_required_on_refresh_token_request_should_fail() From 95b63bade53dee82285694ea1b43dcc795d18cf1 Mon Sep 17 00:00:00 2001 From: Joe DeCock Date: Wed, 30 Aug 2023 15:13:08 -0500 Subject: [PATCH 2/6] Add test for getting mtls thumprints on refresh --- .../Extensions/TokenExtensionsTests.cs | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/test/IdentityServer.UnitTests/Extensions/TokenExtensionsTests.cs b/test/IdentityServer.UnitTests/Extensions/TokenExtensionsTests.cs index 0eb6bc2a3..6042416ae 100644 --- a/test/IdentityServer.UnitTests/Extensions/TokenExtensionsTests.cs +++ b/test/IdentityServer.UnitTests/Extensions/TokenExtensionsTests.cs @@ -7,7 +7,9 @@ using Duende.IdentityServer.Models; using IdentityModel; using Microsoft.AspNetCore.Authentication; +using System; using System.Collections.Generic; +using System.Linq; using System.Security.Claims; using System.Text.Json; using UnitTests.Common; @@ -43,4 +45,27 @@ public void TestClaimValueTypes(string type, string value, string valueType, str Assert.Contains(expected, payloadJson); } + + [Fact] + public void refresh_token_should_get_mtls_x5t_thumprint() + { + var expected = "some hash normally goes here"; + var refreshToken = new RefreshToken() + { + AccessTokens = new Dictionary + { + { "token", new Token() + { + Confirmation = $$""" + { + "x5t#S256": "{{expected}}" + } + """ + } + } + } + }; + var thumbprint = refreshToken.GetProofKeyThumbprints().Single().Thumbprint; + Assert.Equal(expected, thumbprint); + } } \ No newline at end of file From afd500db460c80542533a0794a5bba9603a44b97 Mon Sep 17 00:00:00 2001 From: Joe DeCock Date: Wed, 30 Aug 2023 15:14:02 -0500 Subject: [PATCH 3/6] Fix retrieval of refresh token thumbprints --- src/IdentityServer/Extensions/TokenExtensions.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/IdentityServer/Extensions/TokenExtensions.cs b/src/IdentityServer/Extensions/TokenExtensions.cs index 1a3bc7d04..a5e8655ff 100644 --- a/src/IdentityServer/Extensions/TokenExtensions.cs +++ b/src/IdentityServer/Extensions/TokenExtensions.cs @@ -146,7 +146,7 @@ private static object AddObject(Claim claim) if (claim.ValueType == ClaimValueTypes.Integer64) { - return long.Parse(claim.Value); + return long.Parse(claim.Value); } if (claim.ValueType == ClaimValueTypes.Double) @@ -180,11 +180,11 @@ private static ProofKeyThumbprint GetProofKeyThumbprint(string cnf) { if (cnf.IsPresent()) { - var data = JsonSerializer.Deserialize>(cnf); + var data = JsonSerializer.Deserialize>(cnf); if (data.TryGetValue(JwtClaimTypes.ConfirmationMethods.JwkThumbprint, out var jkt)) { - var thumbprint = jkt as string; + var thumbprint = jkt.ToString(); if (thumbprint.IsPresent()) { return new ProofKeyThumbprint { Type = ProofType.DPoP, Thumbprint = thumbprint }; @@ -193,7 +193,7 @@ private static ProofKeyThumbprint GetProofKeyThumbprint(string cnf) if (data.TryGetValue("x5t#S256", out var x5t)) { - var thumbprint = x5t as string; + var thumbprint = x5t.ToString(); if (thumbprint.IsPresent()) { return new ProofKeyThumbprint { Type = ProofType.ClientCertificate, Thumbprint = thumbprint }; From 1e0bc2a706140ccdc5ce86e305f08df584bda70e Mon Sep 17 00:00:00 2001 From: Joe DeCock Date: Wed, 30 Aug 2023 15:14:14 -0500 Subject: [PATCH 4/6] Add vscode config for dpop client --- .vscode/launch.json | 20 ++++++++++++++++++++ .vscode/tasks.json | 12 ++++++++++++ 2 files changed, 32 insertions(+) diff --git a/.vscode/launch.json b/.vscode/launch.json index 77feba9ad..aef00e052 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -516,6 +516,26 @@ "group": "20-clients", } }, + { + "name": "client: MvcDPoP", + "type": "coreclr", + "request": "launch", + "preLaunchTask": "build-client-MvcDPoP", + "program": "${workspaceFolder}/clients/src/MvcDPoP/bin/Debug/net8.0/MvcDPoP.dll", + "args": [], + "cwd": "${workspaceFolder}/clients/src/MvcDPoP", + "serverReadyAction": { + "action": "openExternally", + "pattern": "\\bNow listening on:\\s+(https?://\\S+)" + }, + "env": { + "ASPNETCORE_ENVIRONMENT": "Development" + }, + "presentation": { + "hidden": false, + "group": "20-clients", + } + }, { "name": "client: MvcHybridBackChannel", "type": "coreclr", diff --git a/.vscode/tasks.json b/.vscode/tasks.json index bd435262d..99de65126 100644 --- a/.vscode/tasks.json +++ b/.vscode/tasks.json @@ -387,6 +387,18 @@ ], "problemMatcher": "$msCompile" }, + { + "label": "build-client-MvcDPoP", + "type": "process", + "command": "dotnet", + "args": [ + "build", + "${workspaceFolder}/clients/src/MvcDPoP/MvcDPoP.csproj", + "/property:GenerateFullPaths=true", + "/consoleloggerparameters:NoSummary" + ], + "problemMatcher": "$msCompile" + }, { "label": "build-client-MvcHybridBackChannel", "type": "process", From fbc82dd0ddb39a4a4108179a792343249a6cd4c4 Mon Sep 17 00:00:00 2001 From: Joe DeCock Date: Wed, 30 Aug 2023 15:23:10 -0500 Subject: [PATCH 5/6] Fix whitespace --- src/IdentityServer/Extensions/TokenExtensions.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/IdentityServer/Extensions/TokenExtensions.cs b/src/IdentityServer/Extensions/TokenExtensions.cs index a5e8655ff..936ab8d63 100644 --- a/src/IdentityServer/Extensions/TokenExtensions.cs +++ b/src/IdentityServer/Extensions/TokenExtensions.cs @@ -146,7 +146,7 @@ private static object AddObject(Claim claim) if (claim.ValueType == ClaimValueTypes.Integer64) { - return long.Parse(claim.Value); + return long.Parse(claim.Value); } if (claim.ValueType == ClaimValueTypes.Double) From 5314d8f71cffe6c5b8bcc08aa855c527c9759c2f Mon Sep 17 00:00:00 2001 From: Joe DeCock Date: Wed, 30 Aug 2023 16:05:02 -0500 Subject: [PATCH 6/6] Switch test to not use raw string literal This keeps the test usable on our 6.3 branch, which targets .NET 6 which doesn't support C# 11/raw string literals --- .../Extensions/TokenExtensionsTests.cs | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/test/IdentityServer.UnitTests/Extensions/TokenExtensionsTests.cs b/test/IdentityServer.UnitTests/Extensions/TokenExtensionsTests.cs index 6042416ae..7854e3614 100644 --- a/test/IdentityServer.UnitTests/Extensions/TokenExtensionsTests.cs +++ b/test/IdentityServer.UnitTests/Extensions/TokenExtensionsTests.cs @@ -50,17 +50,19 @@ public void TestClaimValueTypes(string type, string value, string valueType, str public void refresh_token_should_get_mtls_x5t_thumprint() { var expected = "some hash normally goes here"; + + var cnf = new Dictionary + { + { "x5t#S256", expected } + }; + var refreshToken = new RefreshToken() { AccessTokens = new Dictionary { { "token", new Token() { - Confirmation = $$""" - { - "x5t#S256": "{{expected}}" - } - """ + Confirmation = JsonSerializer.Serialize(cnf) } } }