Skip to content

Conversation

@glucaci
Copy link
Contributor

@glucaci glucaci commented Feb 9, 2026

Add client registration handling to OpenIdConnect authentication flow

Summary

Adds per-request extension points to OpenIdConnectHandler to support multi-tenant scenarios where the OIDC client identity, credentials, and token validation settings vary per request — without requiring a full handler fork.

Motivation

Applications that serve multiple tenants through a single OIDC authentication scheme (e.g., resolving Entra ID app registrations per-request) currently have no way to override client_id, client_secret, or TokenValidationParameters on a per-request basis. The only option today is to fork the entire handler. This PR adds minimal, non-breaking extension points that close that gap.

Changes

New type: OpenIdConnectClientRegistration

A simple, unsealed class grouping the three per-request client values:

ClientId — used in authorize, token, PAR requests and protocol validation
ClientSecret — used in token and PAR requests (nullable for public clients / assertion-based auth)
TokenValidationParameters — used for ID token validation

New virtual method: ResolveClientRegistrationAsync

protected virtual ValueTask<OpenIdConnectClientRegistration> ResolveClientRegistrationAsync(
    AuthenticationProperties properties)

Default implementation returns values from OpenIdConnectOptions(with a cloned TokenValidationParameters), preserving existing behavior. Override to resolve a different client registration per request.

New event method: AuthorizationCodeReceivedContext.HandleClientAuthentication()

Mirrors the existing PushedAuthorizationContext.HandleClientAuthentication() pattern. When called in OnAuthorizationCodeReceived, the handler removes client_secret from the token endpoint request, allowing the event to provide alternative credentials (e.g., client_assertion / private_key_jwt).

Design notes

  • ValueTask<T> — The default implementation is synchronous (ValueTask.FromResult), avoiding a Task allocation on the hot path.
  • HandleClientAuthentication on AuthorizationCodeReceivedContext — Follows the identical pattern already shipped on PushedAuthorizationContext
  • Scope / Prompt / MaxAge excluded — These already have per-request overrides via AuthenticationProperties.GetParameter<T>()

@glucaci glucaci requested a review from halter73 as a code owner February 9, 2026 08:11
Copilot AI review requested due to automatic review settings February 9, 2026 08:11
@github-actions github-actions bot added the area-auth Includes: Authn, Authz, OAuth, OIDC, Bearer label Feb 9, 2026
@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Feb 9, 2026
@dotnet-policy-service
Copy link
Contributor

Thanks for your PR, @@glucaci. Someone from the team will get assigned to your PR shortly and we'll get it reviewed.

Copy link
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

Adds per-request extension points to OpenIdConnectHandler to support multi-tenant OpenID Connect scenarios by dynamically resolving client registration and allowing events to take over token endpoint client authentication.

Changes:

  • Introduces OpenIdConnectClientRegistration and OpenIdConnectHandler.ResolveClientRegistrationAsync(...) for per-request client settings.
  • Adds AuthorizationCodeReceivedContext.HandleClientAuthentication() / HandledClientAuthentication to allow alternative token endpoint authentication (e.g., client assertions).
  • Adds a comprehensive new test suite covering default and overridden behaviors across challenge, PAR, token redemption, and protocol validation.

Reviewed changes

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

Show a summary per file
File Description
src/Security/Authentication/test/OpenIdConnect/OpenIdConnectExtensionTests.cs Adds tests validating dynamic client registration and handled client authentication behavior.
src/Security/Authentication/OpenIdConnect/src/PublicAPI.Unshipped.txt Records newly added public APIs for the OpenIdConnect package.
src/Security/Authentication/OpenIdConnect/src/OpenIdConnectHandler.cs Wires dynamic client registration through challenge, PAR, token redemption, and protocol validation; honors handled client authentication.
src/Security/Authentication/OpenIdConnect/src/OpenIdConnectClientRegistration.cs Introduces the per-request client registration data container (id/secret/TVP).
src/Security/Authentication/OpenIdConnect/src/Events/AuthorizationCodeReceivedContext.cs Adds event-controlled client authentication handling for code redemption.

JwtSecurityToken? jwt = null;
string? nonce = null;
var validationParameters = Options.TokenValidationParameters.Clone();
var registration = await ResolveClientRegistrationAsync(properties);
Copy link

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

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

properties is used elsewhere in this method as nullable (e.g., passed as properties! later), but here it's passed as non-null to ResolveClientRegistrationAsync(AuthenticationProperties properties). This is a nullability mismatch and may not compile (or can introduce a nullability warning-as-error). Use properties! here (consistent with the existing usage) or ensure properties is non-nullable by construction before this point.

Suggested change
var registration = await ResolveClientRegistrationAsync(properties);
var registration = await ResolveClientRegistrationAsync(properties!);

Copilot uses AI. Check for mistakes.
Assert.NotNull(capturedTokenParams);
// ClientSecret was null in the registration, so no client_secret should be sent
// (the OpenIdConnectMessage won't include a null/empty parameter)
Assert.False(capturedTokenParams!.ContainsKey("client_secret") && !string.IsNullOrEmpty(capturedTokenParams["client_secret"]));
Copy link

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

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

This assertion would still pass if client_secret is present but empty, which weakens the test and could mask a regression (the comment says it should be omitted). Prefer asserting that the key is not present at all (e.g., Assert.DoesNotContain(\"client_secret\", capturedTokenParams.Keys)).

Suggested change
Assert.False(capturedTokenParams!.ContainsKey("client_secret") && !string.IsNullOrEmpty(capturedTokenParams["client_secret"]));
Assert.DoesNotContain("client_secret", capturedTokenParams!.Keys);

Copilot uses AI. Check for mistakes.
Comment on lines +45 to +55
var server = CreateServerWithEvents(options =>
{
options.ClientId = "default-client-id";
options.Events.OnRedirectToIdentityProvider = ctx =>
{
capturedClientId = ctx.ProtocolMessage.ClientId;
ctx.HandleResponse();
ctx.Response.StatusCode = StatusCodes.Status200OK;
return Task.CompletedTask;
};
});
Copy link

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

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

The tests create TestServer instances but never dispose them. This can leak resources across the test run and cause flakiness. Update the tests to dispose servers (e.g., await using var server = ...; / using var server = ...;) consistently after each test.

Copilot uses AI. Check for mistakes.
Comment on lines +848 to +854
foreach (string key in query)
{
if (key is not null)
{
parameters[key] = query[key]!;
}
}
Copy link

Copilot AI Feb 9, 2026

Choose a reason for hiding this comment

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

HttpUtility.ParseQueryString(...) returns a NameValueCollection whose enumerator can yield null keys. Declaring the loop variable as non-nullable string makes the key is not null check ineffective (the assignment would already have failed). Use foreach (string? key in query) or iterate query.AllKeys to accurately reflect nullability and simplify the code.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-auth Includes: Authn, Authz, OAuth, OIDC, Bearer community-contribution Indicates that the PR has been added by a community member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant