Skip to content

Added support for Token revocation support #567

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 14 commits into
base: main
Choose a base branch
from
Open

Conversation

4gust
Copy link
Collaborator

@4gust 4gust commented Apr 3, 2025

PR Title: Add Token Revocation Handling and Enhanced Testing for App Service in Managed Identity

Description:
This PR introduces a new feature to handle token revocation scenarios in App Service managed identity and enhances testing to ensure robust behavior. The changes focus on handling claims challenges, bad access tokens from the cache, and validating client capabilities.


Changes:

Feature Implementation:

  1. Token Revocation Handling:

    • Ensures that the client retries the request with the claims parameter included in the URL to obtain a fresh token.
  2. Bad Access Token Handling:

    • Introduced logic to detect bad access tokens retrieved from the cache when claims are passed and ensures that a fresh token is acquired when necessary.

New Unit Tests:

  1. TestAppServiceWithClaimsAndBadAccessToken

    • Validates the scenario when claims are passed, and a bad access token is retrieved from the cache.
    • Ensures that the client retries with the claims parameter and successfully obtains a valid token.
  2. TestAppServiceWithClientCapabilities

    • Tests the inclusion of client capabilities in the request.
    • Verifies that the client_capabilities parameter is correctly passed and processed.

@4gust 4gust requested review from bgavrilMS and rayluo as code owners April 3, 2025 12:31
@4gust 4gust requested a review from chlowell April 3, 2025 14:33
chlowell
chlowell previously approved these changes Apr 3, 2025
Copy link
Collaborator

@chlowell chlowell left a comment

Choose a reason for hiding this comment

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

I approve of the Go content. Someone from the MSAL side should review the doc content

@@ -82,7 +84,7 @@ const (
tokenName = "Tokens"

// App Service
appServiceAPIVersion = "2019-08-01"
appServiceAPIVersion = "2025-03-30"

Choose a reason for hiding this comment

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

this looks good and matches the app service new version. We just want to make sure not to merge this PR yet, as App Service rollout is still happening.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

How does it work in MSAL .net about guarding the release or the version blocking ?

@gladjohn
Copy link

@4gust this looks good, did not approve it because of the app service api-version update. anyway we can guard it?

Copy link

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