Skip to content

Conversation

@christian-posta
Copy link

Fix override merging for app token

Description

Fix some missing override parameters including requestAppToken

Fixes #3643

@christian-posta christian-posta requested a review from a team as a code owner December 5, 2025 21:16
Copy link
Member

@bgavrilMS bgavrilMS left a comment

Choose a reason for hiding this comment

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

I would add a test that uses reflection to check that each property is copied.

@christian-posta
Copy link
Author

LMK if this is what you had in mind @bgavrilMS

Copy link
Collaborator

@jmprieur jmprieur left a comment

Choose a reason for hiding this comment

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

LGTM
Thanks @christian-posta for raising and fixing

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

This PR fixes missing override parameter merging in the DownstreamApiOptionsMerger class by adding support for several previously unhandled properties including RequestAppToken, HTTP-related properties (BaseUrl, HttpMethod, ContentType, AcceptHeader), and additional AcquireTokenOptions properties.

Key Changes:

  • Added merging logic for RequestAppToken to enable proper app token vs user delegation override control
  • Added merging support for HTTP configuration properties (BaseUrl, HttpMethod, ContentType, AcceptHeader)
  • Added merging support for additional token acquisition options (LongRunningWebApiSessionKey, PopPublicKey, CorrelationId, ManagedIdentity)

@bgavrilMS bgavrilMS force-pushed the ceposta-fix-requestapptoken branch from 93b33c5 to 5748d52 Compare December 11, 2025 13:04
@keegan-caruso
Copy link
Contributor

@christian-posta - Can you fix the failing unit test?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

RequestAppToken for sidecar is not respected

4 participants