[Bug] InteractiveBrowserCredential not failing in non-interactive environments

[sinedied]

• Package Name: @azure/identity
• Package Version: 2.1.0
• Operating system: MacOS 12.4
• nodejs
  • version: 16.14.0
• browser
  • name/version:
• typescript
  • version:
• Is the bug related to documentation in
  • README.md
  • source code documentation
  • SDK API docs on https://docs.microsoft.com

Describe the bug
When using InteractiveBrowserCredential in a non-UI or non-interactive environment (CI, GitHub Actions...) it's stuck waiting indefinitely for an interactive login. When using ChainedTokenCredential, I would expect it to fail and fall back to the next credentials method chained.

To Reproduce
Steps to reproduce the behavior:

1. Set up an app to log in with something like this:

  const browserCredential = new InteractiveBrowserCredential({
    redirectUri: `http://localhost:31337`
  });

2. Try to run the app in a Docker container, for example.

Expected behavior
InteractiveBrowserCredential fails because of non-UI or non-interactive environment.

Additional context
Because of this issue, in SWA CLI (https://github.com/Azure/static-web-apps) the app never fall back to using DeviceTokenCredential in non-UI environment or gets CI stuck indefinitely in non-interactive environments.

[xirzec]

@sinedied I'm trying to understand your scenario, is the idea that this code needs to run both in the browser for customers (using InteractiveBrowserCredential) but also in CI? Can I ask how you are configuring ChainedTokenCredential?

As a possible workaround, would it be possible for you to switch with credential you use based on the current environment?

Perhaps the fix could be allowing developers to pass a timeout to InteractiveBrowserCredential so we don't block indefinitely?

[sinedied]

Hi @xirzec , here's the code we're using: https://github.com/Azure/static-web-apps-cli/blob/main/src/core/account.ts#L52-L76

We are trying to support multiple scenarios:

• unattended non-interactive environments (CI for example) using EnvironmentCredential or ClientSecretCredential
• interactive environments with GUI available using InteractiveBrowserCredential
• interactive environments with only CLI available, using DeviceTokenCredential (for example, a Docker container)

A timeout wouldn't fix the issue for the first scenario, and would not be convenient for the third one.

As a user, I would expect these two behaviors:

• If a CI or non-interactive environment is detected, InteractiveBrowserCredential and DeviceTokenCredential should be bypassed. There are many different ways to detect this using npm packages or even the core Node.js API.

• If I'm running an interactive environment with no GUI available, if opening the browser fails (as done by InteractiveBrowserCredential) it should immediately switch to the next method.

[xirzec]

I'm curious to hear more about "If a CI or non-interactive environment is detected", specifically what checks you are thinking of having us perform. Perhaps something like this? https://github.com/sindresorhus/is-interactive/blob/dc8037ae1a61d828cfb42761c345404055b1e036/index.js

It is concerning that InteractiveBrowserCredential is hanging forever though, since I would expect it to throw a CredentialUnavailableError and proceed with the next credential in the chain, as you suggest.

I'm a bit suspicious of the open dependency, given that it still hasn't addressed a previous issue: sindresorhus/open#287

[sinedied]

Perhaps something like this? https://github.com/sindresorhus/is-interactive/blob/dc8037ae1a61d828cfb42761c345404055b1e036/index.js

Yes exactly! Regarding the open dependency I do not know a good alternative, but yes it has issues (we also faced some with the SWA CLI). For the InteractiveBrowserCredential issue though, it might be possible to check the spawned child process result to see if the browser was effectively opened or not?

[KarishmaGhiya]

@sinedied We do not support InteractiveBrowserCredential in a non-interactive environment or CI-like environment. It's also not part of ChainedTokenCredential.

[sinedied]

@KarishmaGhiya Is this specified somewhere in the docs? I couldn't find any reference to that information, for example here: https://github.com/Azure/azure-sdk-for-js/blob/main/sdk/identity/identity/samples/AzureIdentityExamples.md

[KarishmaGhiya]

Currently, Interactive Browser Credentials involve a redirect and it doesn't preserve the state of the application. As such, it may not fall back to using the next credential in the loop of ChainedTokenCredential. It is a drawback of the credential. So this may not work as desired. However, are you experiencing the same issue of IBC not working at all when used as is? @sinedied

[KarishmaGhiya]

Also, @sinedied do you think this issue was related? Or is this one different? #21726

[sinedied]

Also, @sinedied do you think this issue was related? Or is this one different? #21726

I think it was a different one, as the cause was a dangling promise due to the open dependency.

Currently, Interactive Browser Credentials involve a redirect and it doesn't preserve the state of the application. As such, it may not fall back to using the next credential in the loop of ChainedTokenCredential. It is a drawback of the credential. So this may not work as desired. However, are you experiencing the same issue of IBC not working at all when used as is? @sinedied

When used in the correct environment (ie interactive terminal, with a accessible UI) IBC works as intended. Knowing that it's not supported in ChainedTokenCredentials means that we'll probably have to put extra logic from our side to detect non-interactive environment, but the @azure/identity documentation should be updated to precise that.

However, taking non-interactive environments aside, there's still I issue when using InteractiveBrowserCredential alone when a UI environment is not available: it's stuck until the timeout, whereas I would expect to catch the open error returned if a browser could not be opened, and let me know via an error or exception. Otherwise, I have no way to do the fall back on my side. Would that be an acceptable change?

[xirzec]

Setting aside the feature work of detecting non-interactive environments and exiting early, it seems like we should be handling spawn failing to open the browser.

We don't do anything complicated with open:

azure-sdk-for-js/sdk/identity/identity/src/msal/nodeFlows/msalOpenBrowser.ts

Line 247 in 05174d3

throw new CredentialUnavailableError(

I think what might be happening is that there is a bug with non-zero exit codes in open: https://github.com/sindresorhus/open/blob/05ba9e150cc1a2629e518a9cc19b586c6ca3f269/index.js#L212

I believe the behavior for allowNonzeroExitCode is flipped on accident right now, so it's resolving rather than rejecting. Perhaps we could try looking at the returned object's exitCode and checking if it's non-zero?

[sinedied]

Perhaps we could try looking at the returned object's exitCode and checking if it's non-zero?

That would be helpful I think for solving this! Another option would be to propose a PR to fix the issue in open as negative error codes are not considered errors.

[xirzec]

That would be helpful I think for solving this! Another option would be to propose a PR to fix the issue in open as negative error codes are not considered errors.

sindresorhus/open#296

[KarishmaGhiya]

@sinedied Is this issue resolved with the PR merging to open library? Do you need anything else from the sdk side?

[github-actions]

Hi @sinedied. Thank you for opening this issue and giving us the opportunity to assist. To help our team better understand your issue and the details of your scenario please provide a response to the question asked above or the information requested above. This will help us more accurately address your issue.

[github-actions]

Hi @sinedied, we're sending this friendly reminder because we haven't heard back from you in 7 days. We need more information about this issue to help address it. Please be sure to give us your input. If we don't hear back from you within 14 days of this comment the issue will be automatically closed. Thank you!

[sinedied]

I just checked in Codespaces with v3.2.4 of @azure/identity, this is the error I get (coming from the identity SDK), so it's not better