From 9f1fa1aa4954f7d0f5913682bcf0ef0a4eccd575 Mon Sep 17 00:00:00 2001 From: khalidabuhakmeh Date: Tue, 6 May 2025 14:17:19 -0400 Subject: [PATCH 1/4] Add unit tests and refactor ProtectedResourceErrorResult Introduce tests for ProtectedResourceErrorResult to ensure WWW-Authenticate header formatting is correct. Refactor header construction values into a single string. --- .../Results/ProtectedResourceErrorResult.cs | 23 ++++--- .../ProtectedResourceErrorResultTests.cs | 63 +++++++++++++++++++ 2 files changed, 79 insertions(+), 7 deletions(-) create mode 100644 identity-server/test/IdentityServer.UnitTests/Endpoints/Results/ProtectedResourceErrorResultTests.cs diff --git a/identity-server/src/IdentityServer/Endpoints/Results/ProtectedResourceErrorResult.cs b/identity-server/src/IdentityServer/Endpoints/Results/ProtectedResourceErrorResult.cs index ba73ef94f..f0504e8d5 100644 --- a/identity-server/src/IdentityServer/Endpoints/Results/ProtectedResourceErrorResult.cs +++ b/identity-server/src/IdentityServer/Endpoints/Results/ProtectedResourceErrorResult.cs @@ -9,6 +9,7 @@ using Microsoft.Extensions.Primitives; using Microsoft.AspNetCore.Http; using Microsoft.Net.Http.Headers; using Duende.IdentityModel; +using System.Collections.Generic; namespace Duende.IdentityServer.Endpoints.Results; @@ -59,16 +60,24 @@ internal class ProtectedResourceErrorHttpWriter : IHttpResponseWriter { - context.Response.Headers.Append(HeaderNames.WWWAuthenticate, new StringValues(new[] { "Bearer realm=\"IdentityServer\"", errorString })); - } - else + """ + Bearer realm="IdentityServer" + """, + $""" + error="{error}" + """ + }; + + if (!errorDescription.IsMissing()) { - var errorDescriptionString = string.Format($"error_description=\"{errorDescription}\""); - context.Response.Headers.Append(HeaderNames.WWWAuthenticate, new StringValues(new[] { "Bearer realm=\"IdentityServer\"", errorString, errorDescriptionString })); + values.Add($""" + error_description="{errorDescription}" + """); } + + context.Response.Headers.Append(HeaderNames.WWWAuthenticate, string.Join(",", values)); return Task.CompletedTask; } diff --git a/identity-server/test/IdentityServer.UnitTests/Endpoints/Results/ProtectedResourceErrorResultTests.cs b/identity-server/test/IdentityServer.UnitTests/Endpoints/Results/ProtectedResourceErrorResultTests.cs new file mode 100644 index 000000000..a39398722 --- /dev/null +++ b/identity-server/test/IdentityServer.UnitTests/Endpoints/Results/ProtectedResourceErrorResultTests.cs @@ -0,0 +1,63 @@ +// Copyright (c) Duende Software. All rights reserved. +// See LICENSE in the project root for license information. + +using Duende.IdentityServer.Endpoints.Results; +using FluentAssertions; +using Microsoft.AspNetCore.Http; +using Microsoft.Net.Http.Headers; +using Xunit; + +namespace UnitTests.Endpoints.Results; + +public class ProtectedResourceErrorResultTests +{ + private readonly ProtectedResourceErrorHttpWriter writer = new(); + + [Fact] + public void WwwAuthenticate_header_with_error_and_description_should_be_a_single_line() + { + var context = new DefaultHttpContext(); + + writer.WriteHttpResponse( + new ProtectedResourceErrorResult("oops", "big oops"), + context + ); + + var wwwAuthHeader = context.Response.Headers[HeaderNames.WWWAuthenticate].ToString(); + wwwAuthHeader.Should().BeEquivalentTo( + """ + Bearer realm="IdentityServer",error="oops",error_description="big oops" + """); + } + + [Fact] + public void WwwAuthenticate_header_with_error_should_be_a_single_line() + { + var context = new DefaultHttpContext(); + + writer.WriteHttpResponse( + new ProtectedResourceErrorResult("oops"), + context + ); + + var wwwAuthHeader = context.Response.Headers[HeaderNames.WWWAuthenticate].ToString(); + wwwAuthHeader.Should().BeEquivalentTo( + """ + Bearer realm="IdentityServer",error="oops" + """); + } + + [Fact] + public void WwwAuthenticate_header_should_always_be_a_single_string_value() + { + var context = new DefaultHttpContext(); + + writer.WriteHttpResponse( + new ProtectedResourceErrorResult("oops", "big oops"), + context + ); + + var wwwAuthHeader = context.Response.Headers[HeaderNames.WWWAuthenticate]; + wwwAuthHeader.Count.Should().Be(1); + } +} \ No newline at end of file From 8d233567fb24a37e79c071bbb00c530a968822cd Mon Sep 17 00:00:00 2001 From: Joe DeCock Date: Tue, 6 May 2025 20:36:52 -0500 Subject: [PATCH 2/4] Add vscode settings to .gitignore --- .gitignore | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 .gitignore diff --git a/.gitignore b/.gitignore new file mode 100644 index 000000000..b04fff14a --- /dev/null +++ b/.gitignore @@ -0,0 +1,2 @@ +# Visual Studio Code workspace options +.vscode/settings.json From a960bf47e175acf1012000c98f916778c3807694 Mon Sep 17 00:00:00 2001 From: Joe DeCock Date: Tue, 6 May 2025 20:38:29 -0500 Subject: [PATCH 3/4] Add Shoudly in identity-server test projects This is only adding the dependency, not updating all the tests. This gives us an easy way to write tests that will merge cleanly into main. --- identity-server/Directory.Build.targets | 1 + .../Configuration.IntegrationTests.csproj | 1 + .../EntityFramework.IntegrationTests.csproj | 1 + .../EntityFramework.Storage.IntegrationTests.csproj | 5 +++-- .../EntityFramework.Storage.UnitTests.csproj | 1 + .../IdentityServer.IntegrationTests.csproj | 1 + .../IdentityServer.UnitTests/IdentityServer.UnitTests.csproj | 1 + 7 files changed, 9 insertions(+), 2 deletions(-) diff --git a/identity-server/Directory.Build.targets b/identity-server/Directory.Build.targets index 7b3436797..a1f512d4c 100644 --- a/identity-server/Directory.Build.targets +++ b/identity-server/Directory.Build.targets @@ -27,6 +27,7 @@ + diff --git a/identity-server/test/Configuration.IntegrationTests/Configuration.IntegrationTests.csproj b/identity-server/test/Configuration.IntegrationTests/Configuration.IntegrationTests.csproj index 2f225ae68..89291c4a0 100644 --- a/identity-server/test/Configuration.IntegrationTests/Configuration.IntegrationTests.csproj +++ b/identity-server/test/Configuration.IntegrationTests/Configuration.IntegrationTests.csproj @@ -23,6 +23,7 @@ all + diff --git a/identity-server/test/EntityFramework.IntegrationTests/EntityFramework.IntegrationTests.csproj b/identity-server/test/EntityFramework.IntegrationTests/EntityFramework.IntegrationTests.csproj index 90ab0a162..7d232874c 100644 --- a/identity-server/test/EntityFramework.IntegrationTests/EntityFramework.IntegrationTests.csproj +++ b/identity-server/test/EntityFramework.IntegrationTests/EntityFramework.IntegrationTests.csproj @@ -9,6 +9,7 @@ + diff --git a/identity-server/test/EntityFramework.Storage.IntegrationTests/EntityFramework.Storage.IntegrationTests.csproj b/identity-server/test/EntityFramework.Storage.IntegrationTests/EntityFramework.Storage.IntegrationTests.csproj index 2eaec819d..75e1c0799 100644 --- a/identity-server/test/EntityFramework.Storage.IntegrationTests/EntityFramework.Storage.IntegrationTests.csproj +++ b/identity-server/test/EntityFramework.Storage.IntegrationTests/EntityFramework.Storage.IntegrationTests.csproj @@ -13,6 +13,7 @@ + @@ -21,7 +22,7 @@