From 15629c038086cb9d4cc3f9eee4df98996b70f202 Mon Sep 17 00:00:00 2001 From: Erwin van der Valk Date: Tue, 17 Mar 2026 14:44:19 +0100 Subject: [PATCH 1/3] fix circular dependency --- .../Internal/ConfigureOpenIdConnectOptions.cs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/access-token-management/src/AccessTokenManagement.OpenIdConnect/Internal/ConfigureOpenIdConnectOptions.cs b/access-token-management/src/AccessTokenManagement.OpenIdConnect/Internal/ConfigureOpenIdConnectOptions.cs index 2780d959..24009c0f 100644 --- a/access-token-management/src/AccessTokenManagement.OpenIdConnect/Internal/ConfigureOpenIdConnectOptions.cs +++ b/access-token-management/src/AccessTokenManagement.OpenIdConnect/Internal/ConfigureOpenIdConnectOptions.cs @@ -22,10 +22,11 @@ internal class ConfigureOpenIdConnectOptions( IHttpContextAccessor httpContextAccessor, IOptions userAccessTokenManagementOptions, IAuthenticationSchemeProvider schemeProvider, - IClientAssertionService clientAssertionService, + IServiceProvider serviceProvider, ILoggerFactory loggerFactory) : IConfigureNamedOptions { private readonly Scheme _configScheme = GetConfigScheme(userAccessTokenManagementOptions.Value, schemeProvider); + private IClientAssertionService ClientAssertionService => serviceProvider.GetRequiredService(); private ClientCredentialsClientName ClientName => _configScheme.ToClientName(); @@ -119,7 +120,7 @@ async Task Callback(AuthorizationCodeReceivedContext context) } // Automatically send client assertion during code exchange if a service is registered - var assertion = await clientAssertionService + var assertion = await ClientAssertionService .GetClientAssertionAsync(ClientName, ct: context.HttpContext.RequestAborted) .ConfigureAwait(false); @@ -162,7 +163,7 @@ async Task Callback(PushedAuthorizationContext context) await inner.Invoke(context); // --- Client assertion --- - var assertion = await clientAssertionService + var assertion = await ClientAssertionService .GetClientAssertionAsync(ClientName, ct: context.HttpContext.RequestAborted) .ConfigureAwait(false); From fbb9f47e0cd4e36e022108bc27f44d62b7f926fa Mon Sep 17 00:00:00 2001 From: Erwin van der Valk Date: Tue, 17 Mar 2026 17:51:51 +0100 Subject: [PATCH 2/3] add test for circular dependencies --- .../CircularDependencyTests.cs | 88 +++++++++++++++++++ 1 file changed, 88 insertions(+) create mode 100644 access-token-management/test/AccessTokenManagement.Tests/CircularDependencyTests.cs diff --git a/access-token-management/test/AccessTokenManagement.Tests/CircularDependencyTests.cs b/access-token-management/test/AccessTokenManagement.Tests/CircularDependencyTests.cs new file mode 100644 index 00000000..4557a3c0 --- /dev/null +++ b/access-token-management/test/AccessTokenManagement.Tests/CircularDependencyTests.cs @@ -0,0 +1,88 @@ +// Copyright (c) Duende Software. All rights reserved. +// Licensed under the Apache License, Version 2.0. See LICENSE in the project root for license information. + +using Duende.AccessTokenManagement.OpenIdConnect; +using Duende.IdentityModel; +using Duende.IdentityModel.Client; +using Microsoft.AspNetCore.Authentication; +using Microsoft.AspNetCore.Authentication.Cookies; +using Microsoft.Extensions.DependencyInjection; + +namespace Duende.AccessTokenManagement; + +/// +/// Tests that verify the DI registration does not create circular dependencies. +/// See https://github.com/DuendeSoftware/foss/pull/347 for context. +/// +public class CircularDependencyTests +{ + /// + /// Reproduces the circular dependency described in PR #347: + /// + /// IClientAssertionService (user impl) + /// → IOpenIdConnectConfigurationService + /// → IOptionsMonitor<OpenIdConnectOptions> + /// → IConfigureOptions<OpenIdConnectOptions> (ConfigureOpenIdConnectOptions) + /// → IClientAssertionService ← CYCLE + /// + /// The fix in ConfigureOpenIdConnectOptions resolves IClientAssertionService + /// lazily via IServiceProvider instead of constructor injection, breaking the cycle. + /// + [Fact] + public void IClientAssertionService_depending_on_IOpenIdConnectConfigurationService_should_not_cause_circular_dependency() + { + var services = new ServiceCollection(); + + // Register authentication with an OpenIdConnect scheme (minimal setup). + services.AddAuthentication(options => + { + options.DefaultChallengeScheme = "oidc"; + options.DefaultSignInScheme = "cookie"; + }) + .AddCookie("cookie") + .AddOpenIdConnect("oidc", options => + { + options.Authority = "https://demo.duendesoftware.com"; + options.ClientId = "test-client"; + options.ClientSecret = "secret"; + }); + + // Register ATM's OpenIdConnect services (includes ConfigureOpenIdConnectOptions). + services.AddOpenIdConnectAccessTokenManagement(); + + // Register a custom IClientAssertionService that depends on + // IOpenIdConnectConfigurationService — the exact pattern from the + // WebJarJwt sample that triggered the circular dependency before the fix. + services.AddTransient(); + + // ValidateOnBuild detects circular dependencies at container build time. + // Before the fix, this would throw: + // "A circular dependency was detected for the service of type + // 'Duende.AccessTokenManagement.IClientAssertionService'." + var act = () => services.BuildServiceProvider(new ServiceProviderOptions + { + ValidateOnBuild = true, + ValidateScopes = true, + }); + + act.ShouldNotThrow(); + } + + /// + /// A test implementation of IClientAssertionService that depends on + /// IOpenIdConnectConfigurationService, reproducing the dependency chain + /// from the WebJarJwt sample that caused the circular dependency. + /// + private sealed class ClientAssertionServiceWithOidcDependency( + IOpenIdConnectConfigurationService configurationService) : IClientAssertionService + { + // Keep a reference to prove DI resolved the dependency successfully. + private readonly IOpenIdConnectConfigurationService _configurationService = configurationService; + + public Task GetClientAssertionAsync( + ClientCredentialsClientName? clientName = null, + TokenRequestParameters? parameters = null, + CancellationToken ct = default) => + Task.FromResult(null); + } +} From 7a2c178a316f8134f560aedf03b0c4b92dfa213b Mon Sep 17 00:00:00 2001 From: Erwin van der Valk Date: Tue, 17 Mar 2026 17:53:47 +0100 Subject: [PATCH 3/3] fixup --- .../AccessTokenManagement.Tests/CircularDependencyTests.cs | 3 --- 1 file changed, 3 deletions(-) diff --git a/access-token-management/test/AccessTokenManagement.Tests/CircularDependencyTests.cs b/access-token-management/test/AccessTokenManagement.Tests/CircularDependencyTests.cs index 4557a3c0..307e6a2d 100644 --- a/access-token-management/test/AccessTokenManagement.Tests/CircularDependencyTests.cs +++ b/access-token-management/test/AccessTokenManagement.Tests/CircularDependencyTests.cs @@ -2,10 +2,7 @@ // Licensed under the Apache License, Version 2.0. See LICENSE in the project root for license information. using Duende.AccessTokenManagement.OpenIdConnect; -using Duende.IdentityModel; using Duende.IdentityModel.Client; -using Microsoft.AspNetCore.Authentication; -using Microsoft.AspNetCore.Authentication.Cookies; using Microsoft.Extensions.DependencyInjection; namespace Duende.AccessTokenManagement;