Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Dec 12, 2025

Plan to Fix GetTokenAcquirer Option Propagation Issue ✅ COMPLETE

Understanding the Issue

The class hierarchy is:

  • IdentityApplicationOptions (base class)
  • MicrosoftEntraApplicationOptions extends IdentityApplicationOptions (adds: Name, Instance, TenantId, AppHomeTenantId, AzureRegion, ClientCapabilities, SendX5C)
  • MicrosoftIdentityApplicationOptions extends MicrosoftEntraApplicationOptions (adds B2C properties)

In DefaultTokenAcquirerFactoryImplementation.GetTokenAcquirer(IdentityApplicationOptions), when the parameter is MicrosoftEntraApplicationOptions (but not MicrosoftIdentityApplicationOptions), the method only copies base IdentityApplicationOptions properties and misses the MicrosoftEntraApplicationOptions specific properties.

Tasks

  • Understand the existing code structure and class hierarchy
  • Identify the exact location where properties need to be copied
  • Implement the fix to propagate MicrosoftEntraApplicationOptions properties
  • Review and identify existing tests for DefaultTokenAcquirerFactoryImplementation
  • Create or update tests to validate the fix
  • Build and test the changes (682 tests passed)
  • Run broader test suite to ensure no regressions
  • Run code review (No issues found)
  • Run security scan (CodeQL timed out - typical for large repos, no security concerns in our minimal changes)
  • Address code review feedback: Rename variables to camelCase and copy Authority property

Summary of Changes

1. DefaultTokenAcquirerFactoryImplementation.cs (Updated)

  • ✅ Renamed MicrosoftIdentityApplicationOptions variable to microsoftIdentityApplicationOptions (camelCase)
  • ✅ Renamed MicrosoftEntraApplicationOptions variable to microsoftEntraApplicationOptions (camelCase)
  • ✅ Added copying of Authority property from microsoftEntraApplicationOptions
  • All property assignments from the intermediate class in the hierarchy:
    • Authority (newly added)
    • Name
    • Instance
    • TenantId
    • AppHomeTenantId
    • AzureRegion
    • ClientCapabilities
    • SendX5C

2. DefaultTokenAcquirerFactoryImplementationTests.cs

Created comprehensive unit tests to validate:

  • ✅ MicrosoftEntraApplicationOptions properties are properly propagated
  • ✅ IdentityApplicationOptions base properties continue to work
  • ✅ MicrosoftIdentityApplicationOptions instances are used as-is without modification

All tests pass successfully! (682 tests passed)

Original prompt

This section details on the original issue you should resolve

<issue_title>GetTokenAcquirer(IdentityApplicationOptions IdentityApplicationOptions) doesn't propagate all options</issue_title>
<issue_description>### Microsoft.Identity.Web Library

Microsoft.Identity.Web

Microsoft.Identity.Web version

4.1.1

Web app

Not Applicable

Web API

Not Applicable

Token cache serialization

Not Applicable

Description

MicrosoftIdentityApplicationOptions extends MicrosoftEntraApplicationOptions which extends IdentityApplicationOptions.

public ITokenAcquirer GetTokenAcquirer(IdentityApplicationOptions IdentityApplicationOptions)

In DefaultTokenAcquirerFactoryImplementation, GetTokenAcquirer(IdentityApplicationOptions IdentityApplicationOptions) doesn't propagate some options if IdentityApplicationOptions is of type MicrosoftEntraApplicationOptions

Reproduction steps

  • Create an instance of MicrosoftEntraApplicationOptions and set class-specific options.
  • Pass it to GetTokenAcquirer(IdentityApplicationOptions IdentityApplicationOptions)
  • Check that the resulting merged options have the previously set option values.

Error message

No response

Id Web logs

No response

Relevant code snippets

n/a

Regression

No response

Expected behavior

In DefaultTokenAcquirerFactoryImplementation, GetTokenAcquirer(IdentityApplicationOptions IdentityApplicationOptions), if IdentityApplicationOptions is not MicrosoftIdentityApplicationOptions and is of MicrosoftEntraApplicationOptions type, after creating a new MicrosoftIdentityApplicationOptions instance and copying over the options from IdentityApplicationOptions, if IdentityApplicationOptions is of type MicrosoftEntraApplicationOptions, copy over those options too.

Update/add any relevant tests.</issue_description>

Comments on the Issue (you are @copilot in this section)


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

…operties

Co-authored-by: pmaytak <34331512+pmaytak@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix token options propagation in GetTokenAcquirer Fix GetTokenAcquirer to propagate MicrosoftEntraApplicationOptions properties Dec 12, 2025
Copilot AI requested a review from pmaytak December 12, 2025 20:15
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.

Great catch!

…oftEntraApplicationOptions

Co-authored-by: pmaytak <34331512+pmaytak@users.noreply.github.com>
Copilot AI requested a review from pmaytak December 12, 2025 23:56
@pmaytak pmaytak marked this pull request as ready for review December 13, 2025 00:28
@pmaytak pmaytak requested a review from a team as a code owner December 13, 2025 00:28
@kllysng kllysng merged commit 4cbb58a into master Dec 13, 2025
11 checks passed
@kllysng kllysng deleted the copilot/fix-gettokenacquirer-options branch December 13, 2025 05:15
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.

GetTokenAcquirer(IdentityApplicationOptions IdentityApplicationOptions) doesn't propagate all options

4 participants