AzureAD / microsoft-authentication-library-for-js

Microsoft Authentication Library (MSAL) for JS
http://aka.ms/aadv2
MIT License
3.66k stars 2.65k forks source link

[msal-node] LoopbackClient listen should not crash with unhandled rejection #6449

Closed mpodwysocki closed 1 year ago

mpodwysocki commented 1 year ago

Core Library

MSAL Node (@azure/msal-node)

Core Library Version

1.18.3

Wrapper Library

Not Applicable

Wrapper Library Version

None

Public or Confidential Client?

Public

Description

Trying to integrate the MSAL PublicClientApplication in the acquireTokenInteractive method. This calls into the LoopbackClient for the listenForAuthCode method. This method should set up an HTTP server and listen for when the server is actually listening. The code should acquire a server and handle listening gracefully. However, this crashes with an unhandled Promise rejection exception.

Error Message

[node-tests] AuthError [NodeAuthError]: loopback_server_timeout: Timed out waiting for auth code listener to be registered.
[node-tests]     at new AuthError (/Users/matthewp/git/azure-sdk-for-js/common/temp/node_modules/.pnpm/@azure+msal-common@13.3.0/node_modules/@azure/msal-common/src/error/AuthError.ts:49:9)
[node-tests]     at new NodeAuthError (/Users/matthewp/git/azure-sdk-for-js/common/temp/node_modules/.pnpm/@azure+msal-node@1.18.3/node_modules/@azure/msal-node/src/error/NodeAuthError.ts:44:9)
[node-tests]     at Function.createLoopbackServerTimeoutError (/Users/matthewp/git/azure-sdk-for-js/common/temp/node_modules/.pnpm/@azure+msal-node@1.18.3/node_modules/@azure/msal-node/src/error/NodeAuthError.ts:92:16)
[node-tests]     at Timeout._onTimeout (/Users/matthewp/git/azure-sdk-for-js/common/temp/node_modules/.pnpm/@azure+msal-node@1.18.3/node_modules/@azure/msal-node/src/network/LoopbackClient.ts:54:41)
[node-tests]     at listOnTimeout (node:internal/timers:569:17)
[node-tests]     at processTimers (node:internal/timers:512:7) {
[node-tests]   errorCode: 'loopback_server_timeout',
[node-tests]   errorMessage: 'Timed out waiting for auth code listener to be registered.',
[node-tests]   subError: ''
[node-tests] }

Msal Logs

No response

MSAL Configuration

{
    auth: {
      clientId: "some-client-id",
      authority: "https://login.microsoftonline.com/common",
    },
}

Relevant Code Snippets

This is the code that is causing the issue:

        await new Promise<void>((resolve) => {
            let ticks = 0;
            const id = setInterval(() => {
                if (
                    LOOPBACK_SERVER_CONSTANTS.TIMEOUT_MS /
                        LOOPBACK_SERVER_CONSTANTS.INTERVAL_MS <
                    ticks
                ) {
                    throw NodeAuthError.createLoopbackServerTimeoutError();
                }

                if (this.server.listening) {
                    clearInterval(id);
                    resolve();
                }
                ticks++;
            }, LOOPBACK_SERVER_CONSTANTS.INTERVAL_MS);
        });

The code should be calling the listen method with a callback for the listener instead such as:

this.server.listen(0, (event) => {
  // Do something with listening to the server
});

### Reproduction Steps

This test should run without crashing the test-server

  it("Throws an expected error if no browser is available", async function (this: Context) {
    // The SinonStub type does not include this second parameter to throws().
    const testErrorMessage = "No browsers available on this test.";
    (sandbox.stub(interactiveBrowserMockable, "open") as any).throws(
      "BrowserConfigurationAuthError",
      testErrorMessage
    );

    const credential = new InteractiveBrowserCredential(
      recorder.configureClientOptions({
        redirectUri: "http://localhost:8081",
        tenantId: env.AZURE_TENANT_ID,
        clientId: env.AZURE_CLIENT_ID,
      })
    );

    let error: Error | undefined;
    try {
      await credential.getToken(scope);
    } catch (e: any) {
      error = e;
    }

    assert.equal(error?.name, "BrowserConfigurationAuthError");
    assert.equal(error?.message, "No browsers available on this test.");
  });

### Expected Behavior

This should not crash with the following:

[node-tests] AuthError [NodeAuthError]: loopback_server_timeout: Timed out waiting for auth code listener to be registered. [node-tests] at new AuthError (/Users/matthewp/git/azure-sdk-for-js/common/temp/node_modules/.pnpm/@azure+msal-common@13.3.0/node_modules/@azure/msal-common/src/error/AuthError.ts:49:9) [node-tests] at new NodeAuthError (/Users/matthewp/git/azure-sdk-for-js/common/temp/node_modules/.pnpm/@azure+msal-node@1.18.3/node_modules/@azure/msal-node/src/error/NodeAuthError.ts:44:9) [node-tests] at Function.createLoopbackServerTimeoutError (/Users/matthewp/git/azure-sdk-for-js/common/temp/node_modules/.pnpm/@azure+msal-node@1.18.3/node_modules/@azure/msal-node/src/error/NodeAuthError.ts:92:16) [node-tests] at Timeout._onTimeout (/Users/matthewp/git/azure-sdk-for-js/common/temp/node_modules/.pnpm/@azure+msal-node@1.18.3/node_modules/@azure/msal-node/src/network/LoopbackClient.ts:54:41) [node-tests] at listOnTimeout (node:internal/timers:569:17) [node-tests] at processTimers (node:internal/timers:512:7) { [node-tests] errorCode: 'loopback_server_timeout', [node-tests] errorMessage: 'Timed out waiting for auth code listener to be registered.', [node-tests] subError: '' [node-tests] }



### Identity Provider

Azure AD / MSA

### Browsers Affected (Select all that apply)

None (Server)

### Regression

_No response_

### Source

Internal (Microsoft)
konstantin-msft commented 1 year ago

@tnorling Could you please take a look when you get a chance?

microsoft-github-policy-service[bot] commented 1 year ago

@mpodwysocki This issue has been automatically marked as stale because it is marked as requiring author feedback but has not had any activity for 5 days. If your issue has been resolved please let us know by closing the issue. If your issue has not been resolved please leave a comment to keep this open. It will be closed automatically in 7 days if it remains stale.