From ea5617b437b0c9ace1a1868f69fe4ad0e06ec329 Mon Sep 17 00:00:00 2001
From: Joe DeCock <josephdecock@gmail.com>
Date: Wed, 13 Dec 2023 21:01:55 -0600
Subject: [PATCH] Rework fix for logout token issuer bug
Moved the fix from IdentityServerTools into the back channel logout
service to avoid breaking changes in IdentityServerTools. While
technically this is a breaking change in the back channel service too,
this makes the impact much smaller, and more likely to be caught by the
compiler. The previous implementation changed the semantics of a string
parameter in a way that could have been surprising, vs this way adds a
new constructor parameter in a way that will be totally obvious and
caught by the compiler.
---
src/IdentityServer/IdentityServerTools.cs | 11 +++++-----
.../DefaultBackChannelLogoutService.cs | 11 +++++++++-
.../DefaultBackChannelLogoutServiceTests.cs | 20 +++++++++++--------
3 files changed, 27 insertions(+), 15 deletions(-)
diff --git a/src/IdentityServer/IdentityServerTools.cs b/src/IdentityServer/IdentityServerTools.cs
index 02ed1e09b..c9ca8dc68 100644
--- a/src/IdentityServer/IdentityServerTools.cs
+++ b/src/IdentityServer/IdentityServerTools.cs
@@ -48,23 +48,22 @@ public IdentityServerTools(IServiceProvider serviceProvider, IIssuerNameService
/// <exception cref="System.ArgumentNullException">claims</exception>
public virtual async Task<string> IssueJwtAsync(int lifetime, IEnumerable<Claim> claims)
{
- var tokenType = OidcConstants.TokenTypes.AccessToken;
var issuer = await IssuerNameService.GetCurrentAsync();
- return await IssueJwtAsync(lifetime, issuer, tokenType, claims);
+ return await IssueJwtAsync(lifetime, issuer, claims);
}
/// <summary>
/// Issues a JWT.
/// </summary>
/// <param name="lifetime">The lifetime.</param>
- /// <param name="tokenType">The token type.</param>
+ /// <param name="issuer">The issuer.</param>
/// <param name="claims">The claims.</param>
/// <returns></returns>
/// <exception cref="System.ArgumentNullException">claims</exception>
- public virtual async Task<string> IssueJwtAsync(int lifetime, string tokenType, IEnumerable<Claim> claims)
+ public virtual Task<string> IssueJwtAsync(int lifetime, string issuer, IEnumerable<Claim> claims)
{
- var issuer = await IssuerNameService.GetCurrentAsync();
- return await IssueJwtAsync(lifetime, issuer, tokenType, claims);
+ var tokenType = OidcConstants.TokenTypes.AccessToken;
+ return IssueJwtAsync(lifetime, issuer, tokenType, claims);
}
/// <summary>
diff --git a/src/IdentityServer/Services/Default/DefaultBackChannelLogoutService.cs b/src/IdentityServer/Services/Default/DefaultBackChannelLogoutService.cs
index dbd9e4ad6..54b8d5e94 100644
--- a/src/IdentityServer/Services/Default/DefaultBackChannelLogoutService.cs
+++ b/src/IdentityServer/Services/Default/DefaultBackChannelLogoutService.cs
@@ -49,6 +49,11 @@ public class DefaultBackChannelLogoutService : IBackChannelLogoutService
/// </summary>
protected ILogger<IBackChannelLogoutService> Logger { get; }
+ /// <summary>
+ /// Ths issuer name service.
+ /// </summary>
+ protected IIssuerNameService IssuerNameService { get; }
+
/// <summary>
/// Constructor.
/// </summary>
@@ -56,12 +61,14 @@ public class DefaultBackChannelLogoutService : IBackChannelLogoutService
/// <param name="tools"></param>
/// <param name="logoutNotificationService"></param>
/// <param name="backChannelLogoutHttpClient"></param>
+ /// <param name="issuerNameService"></param>
/// <param name="logger"></param>
public DefaultBackChannelLogoutService(
ISystemClock clock,
IdentityServerTools tools,
ILogoutNotificationService logoutNotificationService,
IBackChannelLogoutHttpClient backChannelLogoutHttpClient,
+ IIssuerNameService issuerNameService,
ILogger<IBackChannelLogoutService> logger)
{
Clock = clock;
@@ -69,6 +76,7 @@ public DefaultBackChannelLogoutService(
LogoutNotificationService = logoutNotificationService;
HttpClient = backChannelLogoutHttpClient;
Logger = logger;
+ IssuerNameService = issuerNameService;
}
/// <inheritdoc/>
@@ -150,7 +158,8 @@ protected virtual async Task<string> CreateTokenAsync(BackChannelLogoutRequest r
return await Tools.IssueJwtAsync(DefaultLogoutTokenLifetime, request.Issuer, IdentityServerConstants.TokenTypes.LogoutToken, claims);
}
- return await Tools.IssueJwtAsync(DefaultLogoutTokenLifetime, IdentityServerConstants.TokenTypes.LogoutToken, claims);
+ var issuer = await IssuerNameService.GetCurrentAsync();
+ return await Tools.IssueJwtAsync(DefaultLogoutTokenLifetime, issuer, IdentityServerConstants.TokenTypes.LogoutToken, claims);
}
/// <summary>
diff --git a/test/IdentityServer.UnitTests/Services/Default/DefaultBackChannelLogoutServiceTests.cs b/test/IdentityServer.UnitTests/Services/Default/DefaultBackChannelLogoutServiceTests.cs
index f8228fd36..a864ce618 100644
--- a/test/IdentityServer.UnitTests/Services/Default/DefaultBackChannelLogoutServiceTests.cs
+++ b/test/IdentityServer.UnitTests/Services/Default/DefaultBackChannelLogoutServiceTests.cs
@@ -24,15 +24,18 @@ public class DefaultBackChannelLogoutServiceTests
{
private class ServiceTestHarness : DefaultBackChannelLogoutService
{
- public ServiceTestHarness(ISystemClock clock,
- IdentityServerTools tools,
- ILogoutNotificationService logoutNotificationService,
- IBackChannelLogoutHttpClient backChannelLogoutHttpClient,
- ILogger<IBackChannelLogoutService> logger)
- : base(clock, tools, logoutNotificationService, backChannelLogoutHttpClient, logger)
+ public ServiceTestHarness(
+ ISystemClock clock,
+ IdentityServerTools tools,
+ ILogoutNotificationService logoutNotificationService,
+ IBackChannelLogoutHttpClient backChannelLogoutHttpClient,
+ IIssuerNameService issuerNameService,
+ ILogger<IBackChannelLogoutService> logger)
+ : base(clock, tools, logoutNotificationService, backChannelLogoutHttpClient, issuerNameService, logger)
{
}
+
// CreateTokenAsync is protected, so we use this wrapper to exercise it in our tests
public async Task<string> ExerciseCreateTokenAsync(BackChannelLogoutRequest request)
{
@@ -51,14 +54,15 @@ public async Task CreateTokenAsync_Should_Set_Issuer_Correctly()
var tokenCreation = new DefaultTokenCreationService(new MockClock(), mockKeyMaterialService, TestIdentityServerOptions.Create(), TestLogger.Create<DefaultTokenCreationService>());
+ var issuerNameService = new TestIssuerNameService(expected);
var tools = new IdentityServerTools(
null, // service provider is unused
- new TestIssuerNameService(expected),
+ issuerNameService,
tokenCreation,
new MockClock()
);
- var subject = new ServiceTestHarness(null, tools, null, null, null);
+ var subject = new ServiceTestHarness(null, tools, null, null, issuerNameService, null);
var rawToken = await subject.ExerciseCreateTokenAsync(new BackChannelLogoutRequest
{
ClientId = "test_client",