Skip to content

Commit

Permalink
Rework fix for logout token issuer bug
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
josephdecock committed Dec 14, 2023
1 parent 2f575dc commit ea5617b
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 15 deletions.
11 changes: 5 additions & 6 deletions src/IdentityServer/IdentityServerTools.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,26 +49,34 @@ public class DefaultBackChannelLogoutService : IBackChannelLogoutService
/// </summary>
protected ILogger<IBackChannelLogoutService> Logger { get; }

/// <summary>
/// Ths issuer name service.
/// </summary>
protected IIssuerNameService IssuerNameService { get; }

/// <summary>
/// Constructor.
/// </summary>
/// <param name="clock"></param>
/// <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;
Tools = tools;
LogoutNotificationService = logoutNotificationService;
HttpClient = backChannelLogoutHttpClient;
Logger = logger;
IssuerNameService = issuerNameService;
}

/// <inheritdoc/>
Expand Down Expand Up @@ -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>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand All @@ -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",
Expand Down

0 comments on commit ea5617b

Please sign in to comment.