Skip to content

Conversation

@Haard30
Copy link
Contributor

@Haard30 Haard30 commented Dec 5, 2024

Investigation findings:

  1. Cached auth = publicClientApp.getTokenSilentAsync(....), which means it calls getTokenSilent() method of PCA class and the way silent Auth is performed when calling this method depends on the way each AuthFlow builds and passes its own PCA wrapper object.
  2. CachedAuth (which is always added by default ) builds PCAWrapper without Broker configs. This when it is used to call the getTokenSilent method, detects windows platform, and makes the call non-compliant when the RT expires/some conditions change because it needs to fetch new RT (and thus API call).
  3. Broker builds the PCAWrapper using withBroker(options) and also calls CachedAuth with its own built PCAWraapper. This means that when someone calls AzureAuth and specifies to use only Broker mode, we try CachedAuth 2 times, one with PCAWrapper that CachedAuth itself builds (because cached auth is always added) without Broker config and then when Broker flow gets executed, it will pass on the PCA Wrapper that it built withbroker(options).

Workaround:

It might be important for us to reconsider the Auth flows and make it more close to current MSAL ideology of silent v/s interactive. However, to address current compliance issues, I am implementing following workaround - Whenever Broker is present in AuthMode (default/all/separate invocation to Broker), we leverage the CachedAuth call from Broker and ignore the initial CachedAuth call which is non-compliant.

Testing:

For the following commands, make sure the default/all mode skips initial CachedAuth call and uses CachedAuth from Broker. For --mode Broker make sure it only calls Broker and not separate CachedAuth.
For --mode web, --mode device-code make sure CachedAuth is still present and works.
On mac the current behavior should remain unchanged.

  1. azureauth --help
  2. azureauth aad
  3. azureauth ado pat
  4. azureauth ado token

@Haard30 Haard30 marked this pull request as ready for review December 5, 2024 19:05
@Haard30 Haard30 requested a review from a team as a code owner December 5, 2024 19:05
@Haard30 Haard30 self-assigned this Dec 5, 2024
@Haard30 Haard30 changed the title Ignore duplicate CachedAuth if Broker is present in AuthMode on win 10 or 11 Ignore CachedAuth if Broker is present in AuthMode on win 10 or 11 Dec 5, 2024
@Haard30 Haard30 enabled auto-merge (squash) December 5, 2024 23:20
@Haard30 Haard30 merged commit eaebd77 into main Dec 9, 2024
9 checks passed
@Haard30 Haard30 deleted the user/haashah/remove-cached-auth-if-broker-present branch December 9, 2024 17:41
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.

4 participants