Enhancing Token Refresh Control in MSAL: Introducing .WithAccessTokenSha256ToRefresh() in AcquireTokenForClient Flows

[gladjohn]

Fixes #5111

Specification - https://github.com/AzureAD/microsoft-authentication-library-for-dotnet/blob/main/docs/msiv1_token_revocation.md

Changes proposed in this request
This pull request includes several changes to enhance token acquisition and revocation capabilities in the MSAL library. The most important changes focus on adding support for token hash-based refresh, improving cache handling, and updating error handling.

Enhancements to Token Acquisition and Revocation:

• Added a new method WithAccessTokenSha256ToRefresh to the AcquireTokenForClientParameterBuilder for configuring the SDK to bypass the cache for a specific token based on its SHA-256 hash.
• Introduced a new property AccessTokenHashToRefresh in AcquireTokenForClientParameters to store the hash of the token to be refreshed.
• Updated ClientCredentialRequest to handle token hash-based refresh logic, including checking the cache for matching tokens and bypassing the cache if necessary. [1] [2] [3]

Error Handling Improvements:

• Added a new error ForceRefreshNotCompatibleWithTokenHash to indicate that force refresh and token hash cannot be used together. [1] [2] [3]

Internal Code Changes:

• Changed the visibility of the Parameters property in AcquireTokenForClientParameterBuilder from private to internal.
• Added validation in AcquireTokenForClientParameterBuilder to throw an exception if both force refresh and token hash are used together.

Documentation Updates:

• Removed outdated documentation on MSI v1 token revocation and capabilities from docs/msiv1_token_revocation.md.

These changes collectively improve the flexibility and robustness of token handling in the MSAL library.

Testing
unit tests

Performance impact
none

Documentation

• All relevant documentation is updated.

[rayluo]

Overall looks good. Do we plan to implement the managed identity part of functionality in a different PR? As this PR currently stands, it is not sufficient to "fix #5111", so, better remove the first line in the PR description and remove #5111 from "Successfully merging this pull request may close these issues."

[gladjohn]

Overall looks good. Do we plan to implement the managed identity part of functionality in a different PR?

yes @rayluo, here is the MIA PR for Token Revocation, I had marked this as blocked since we had to finalize the design. I can work on this now - #5139

[gladjohn]

@rayluo @Robbie-Microsoft can I get a review from you as well please

[rayluo]

Functionality wise, this PR shall work. Adding a couple of minor comments below, but overall LGTM.

[bgavrilMS]

Approved with comments. Let's discuss about telemetry separately.

[gladjohn]

Approved with comments. Let's discuss about telemetry separately.

All comments addressed and removed experimental flags check from the tests, as we are GA'ing the API.

[neha-bhargava]

Suggested change

	logger.Info("[ClientCredentialRequest] Skipped looking for a cached access token because ForceRefresh or Claims were set.");
	logger.Info("[ClientCredentialRequest] Skipped looking for a cached access token because either of ForceRefresh, Claims or AccessTokenHashToRefresh were set.");

[gladjohn]

Committed.

[rayluo]

Nice catch. Actually, the new change was still inaccurate. The accurate statement shall be "skipped ... cache ... because either ForceRefresh was set, or Claims was set without AccessTokenHashToRefresh".

[rayluo]

Being addressed in #5201