Skip to content

azure: Use BearerChallengePolicy from auth instead of local duplicate#2232

Merged
alexweininger merged 3 commits intomainfrom
azure/use-auth-challenge-policy
Mar 20, 2026
Merged

azure: Use BearerChallengePolicy from auth instead of local duplicate#2232
alexweininger merged 3 commits intomainfrom
azure/use-auth-challenge-policy

Conversation

@alexweininger
Copy link
Copy Markdown
Member

Summary

Replaces the local AzExtBearerChallengePolicy and getDefaultScopeFromEndpoint in the azure package with imports from @microsoft/vscode-azext-azureauth, consolidating the challenge policy logic in one place.

Changes

  • Import BearerChallengePolicy from @microsoft/vscode-azext-azureauth
  • Remove duplicate AzExtBearerChallengePolicy class and getDefaultScopeFromEndpoint function
  • Wrap the challenge callback with try/catch for robust telemetry (challengeSuccess = 'false' on failure)
  • Precompute the challenge handler instead of recreating per call
  • Add @microsoft/vscode-azext-azureauth: ^6.0.0-alpha.6 as dependency

Depends on

⚠️ Requires #2231 to be merged and @microsoft/vscode-azext-azureauth@6.0.0-alpha.6 published first.

Replace AzExtBearerChallengePolicy and getDefaultScopeFromEndpoint in the
azure package with imports from @microsoft/vscode-azext-azureauth. The
telemetry properties (challenge, challengeSuccess) are preserved by wrapping
the callback at the call site with try/catch for robustness.

This adds @microsoft/vscode-azext-azureauth as a dependency of the azure
package, consolidating the challenge policy logic in one place.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@alexweininger alexweininger requested a review from a team as a code owner March 19, 2026 18:13
Copilot AI review requested due to automatic review settings March 19, 2026 18:13
Copy link
Copy Markdown
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 consolidates Azure bearer challenge handling by replacing a locally duplicated challenge policy with BearerChallengePolicy from @microsoft/vscode-azext-azureauth.

Changes:

  • Replaced local AzExtBearerChallengePolicy/getDefaultScopeFromEndpoint with BearerChallengePolicy from @microsoft/vscode-azext-azureauth
  • Added try/catch around the challenge callback and precomputed the challenge handler
  • Added @microsoft/vscode-azext-azureauth@^6.0.0-alpha.6 dependency

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File Description
azure/src/createAzureClient.ts Swaps in shared BearerChallengePolicy, adds telemetry + error handling around challenge callback, removes local duplicate policy/helpers
azure/package.json Adds dependency on @microsoft/vscode-azext-azureauth for the shared challenge policy

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

Comment thread azure/src/createAzureClient.ts Outdated
Comment on lines 55 to 76
const handleChallenge = getChallengeHandlerFromCredential(context.createCredentialsForScopes);
addAzExtPipeline(
context,
client.pipeline,
context.environment.resourceManagerEndpointUrl,
undefined,
undefined,
new AzExtBearerChallengePolicy(
context,
getChallengeHandlerFromCredential(context.createCredentialsForScopes),
context.environment.resourceManagerEndpointUrl
new BearerChallengePolicy(
async (challenge) => {
context.telemetry.properties.challenge = 'true';
try {
const token = await handleChallenge(challenge);
context.telemetry.properties.challengeSuccess = token ? 'true' : 'false';
return token;
} catch {
context.telemetry.properties.challengeSuccess = 'false';
return undefined;
}
},
context.environment.resourceManagerEndpointUrl,
)
);
Comment thread azure/src/createAzureClient.ts Outdated
Comment on lines +69 to +72
} catch {
context.telemetry.properties.challengeSuccess = 'false';
return undefined;
}
Comment thread azure/src/createAzureClient.ts Outdated
Comment on lines 55 to 76
const handleChallenge = getChallengeHandlerFromCredential(context.createCredentialsForScopes);
addAzExtPipeline(
context,
client.pipeline,
context.environment.resourceManagerEndpointUrl,
undefined,
undefined,
new AzExtBearerChallengePolicy(
context,
getChallengeHandlerFromCredential(context.createCredentialsForScopes),
context.environment.resourceManagerEndpointUrl
new BearerChallengePolicy(
async (challenge) => {
context.telemetry.properties.challenge = 'true';
try {
const token = await handleChallenge(challenge);
context.telemetry.properties.challengeSuccess = token ? 'true' : 'false';
return token;
} catch {
context.telemetry.properties.challengeSuccess = 'false';
return undefined;
}
},
context.environment.resourceManagerEndpointUrl,
)
);
Comment thread azure/package.json
- Extract createBearerChallengePolicy() helper to deduplicate policy
  creation between createAzureClient and createAzureSubscriptionClient
- Rethrow errors from challenge handler instead of swallowing them
- Log parseError(err).message to telemetry as challengeError on failure

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

@bwateratmsft bwateratmsft left a comment

Choose a reason for hiding this comment

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

LGTM, though of course we need the new package version released

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

@bwateratmsft bwateratmsft left a comment

Choose a reason for hiding this comment

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

With all these new packages added to the lockfile, we should try to install this package in, e.g., the Resources extension and make sure the bundle size hasn't gone haywire.

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.

3 participants