Skip to content

fix circular dependency#347

Merged
Erwinvandervalk merged 3 commits intomainfrom
ev/atm/solve-cirular-dep-in-IClientAssertionService
Mar 18, 2026
Merged

fix circular dependency#347
Erwinvandervalk merged 3 commits intomainfrom
ev/atm/solve-cirular-dep-in-IClientAssertionService

Conversation

@Erwinvandervalk
Copy link
Copy Markdown
Contributor

@Erwinvandervalk Erwinvandervalk commented Mar 17, 2026

While running one of the samples, I found that there is a circular dependency.

This can be resolved in the sample, but I figured it's better to solve it in the library, as other people might not run the sample directly.

 ---> (Inner Exception #10) System.InvalidOperationException: Error while validating the service descriptor 'ServiceType: Duende.AccessTokenManagement.IClientAssertionService Lifetime: Transient ImplementationType: WebJarJwt.ClientAssertionService': A circular dependency was detected for the service of type 'Duende.AccessTokenManagement.IClientAssertionService'.
Duende.AccessTokenManagement.IClientAssertionService(WebJarJwt.ClientAssertionService) -> Duende.AccessTokenManagement.OpenIdConnect.IOpenIdConnectConfigurationService(Duende.AccessTokenManagement.OpenIdConnect.Internal.OpenIdConnectConfigurationService) -> Microsoft.Extensions.Options.IOptionsMonitor<Microsoft.AspNetCore.Authentication.OpenIdConnect.OpenIdConnectOptions>(Microsoft.Extensions.Options.OptionsMonitor<Microsoft.AspNetCore.Authentication.OpenIdConnect.OpenIdConnectOptions>) -> Microsoft.Extensions.Options.IOptionsFactory<Microsoft.AspNetCore.Authentication.OpenIdConnect.OpenIdConnectOptions>(Microsoft.Extensions.Options.OptionsFactory<Microsoft.AspNetCore.Authentication.OpenIdConnect.OpenIdConnectOptions>) -> System.Collections.Generic.IEnumerable<Microsoft.Extensions.Options.IConfigureOptions<Microsoft.AspNetCore.Authentication.OpenIdConnect.OpenIdConnectOptions>> -> Microsoft.Extensions.Options.IConfigureOptions<Microsoft.AspNetCore.Authentication.OpenIdConnect.OpenIdConnectOptions>(Duende.AccessTokenManagement.OpenIdConnect.Internal.ConfigureOpenIdConnectOptions) -> Duende.AccessTokenManagement.IClientAssertionService

@Erwinvandervalk Erwinvandervalk self-assigned this Mar 17, 2026
@Erwinvandervalk Erwinvandervalk added the area/foss/atm Issues related to Access Token Management label Mar 17, 2026
@Erwinvandervalk Erwinvandervalk added this to the atm-4.2.0 milestone Mar 17, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Resolves a DI circular dependency involving IClientAssertionService when OpenIdConnect options are configured, and adds a regression test to ensure the container can be validated/built without cycles.

Changes:

  • Updates ConfigureOpenIdConnectOptions to avoid constructor-injecting IClientAssertionService (breaking the circular dependency).
  • Adds a new test that reproduces the circular dependency scenario and asserts DI validation succeeds.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
access-token-management/src/AccessTokenManagement.OpenIdConnect/Internal/ConfigureOpenIdConnectOptions.cs Reworks client-assertion resolution in OIDC option configuration to break the DI cycle.
access-token-management/test/AccessTokenManagement.Tests/CircularDependencyTests.cs Adds a regression test that validates the service provider can be built without circular dependencies.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

@Erwinvandervalk Erwinvandervalk merged commit b15ff68 into main Mar 18, 2026
6 checks passed
@Erwinvandervalk Erwinvandervalk deleted the ev/atm/solve-cirular-dep-in-IClientAssertionService branch March 18, 2026 07:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/foss/atm Issues related to Access Token Management

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants