Skip to content

feat: Add Interactive Browser Credential as a fallback #307

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

Conversation

srathi-msft
Copy link

@srathi-msft srathi-msft commented Jul 18, 2025

  • Try all credentials in sequence --> Azure CLI (if tenantID given), Default, then Interactive
  • Removed ChainedTokenCredential as it fails if Azure CLI is not installed on client's machine
  • Tested the different auth mechanisms locally

#306

Associated Risks

PR Checklist

  • I have read the contribution guidelines
  • I have read the code of conduct guidelines
  • Title of the pull request is clear and informative.
  • 👌 Code hygiene
  • 🔭 Telemetry added, updated, or N/A
  • 📄 Documentation added, updated, or N/A
  • 🛡️ Automated tests added, or N/A

🧪 How did you test it?

Locally used the server with and without the Azure CLI installed.

- Try all credentials in sequence --> Azure CLI (if tenantID given),
  Default, then Interactive
- Removed ChainedTokenCredential as it fails if Azure CLI is not
  installed on client's machine
- Tested the different auth mechanisms locally
@srathi-msft srathi-msft requested a review from a team as a code owner July 18, 2025 13:08

// Try DefaultAzureCredential first (includes managed identity, environment variables, etc.)
try {
const defaultCredential = new DefaultAzureCredential(); // CodeQL [SM05138] resolved by explicitly setting AZURE_TOKEN_CREDENTIALS
Copy link
Contributor

Choose a reason for hiding this comment

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

But this will bring back CodeQL warning?

Copy link
Author

@srathi-msft srathi-msft Jul 18, 2025

Choose a reason for hiding this comment

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

Hi @kboom,

The comment indicated that the fix for the warning is to explicitly set the environment variable AZURE_TOKEN_CREDENTIALS, which we are doing on lines 42-46.

I could not find any references on that particular warning [SM05138] on the internet, so not sure about its contents/reasons/fixes.

@srathi-msft
Copy link
Author

Closing the pull request, as it is against the contributor guidelines to contribute to an unapproved issue.

@srathi-msft
Copy link
Author

@microsoft-github-policy-service agree company="Microsoft"

@danhellem danhellem added the Needs Review 👓 needs review by the product team label Jul 21, 2025
Copy link
Contributor

@aaudzei aaudzei left a comment

Choose a reason for hiding this comment

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

We are moving towards OAuth flow as a default mode. It'll be completed once the App Registration is approved. See #368

@aaudzei aaudzei closed this Jul 31, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Review 👓 needs review by the product team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants