Azure / azure-sdk-for-js

This repository is for active development of the Azure SDK for JavaScript (NodeJS & Browser). For consumers of the SDK we recommend visiting our public developer docs at https://docs.microsoft.com/javascript/azure/ or our versioned developer docs at https://azure.github.io/azure-sdk-for-js.
MIT License
2.07k stars 1.19k forks source link

msal browserFlow error when using instrumental & middleware in Next.js #28904

Closed bjamdort closed 7 months ago

bjamdort commented 7 months ago

Describe the bug When using the Next.JS instrumentation feature for getting KeyVault values and using Next.JS' middleware I get the following error:

error - node_modules\@azure\identity\dist-esm\src\msal\browserFlows\msalAuthCode.js (9:0) @ <unknown>
error - Cannot read properties of undefined (reading 'hash')

It appears to be trying to get the URL hash from window, but failing.

This doesn't happen when I'm not using middleware.ts which makes it even more confusing.

To Reproduce Steps to reproduce the behavior:

  1. Create a Next.JS application with middleware.ts and instrumentation.ts files
  2. Create a SecretClient in instrumentation.ts using DefaultAzureCredential
  3. Get secrets via the client.
  4. Run app

Expected behavior Secret is returned from the client.

Additional context I'm using Next.JS version 13.3.3 but it also appears to be happening on newer versions.

github-actions[bot] commented 7 months ago

@KarishmaGhiya @maorleger

github-actions[bot] commented 7 months ago

Thank you for your feedback. Tagging and routing to the team member best able to assist.

maorleger commented 7 months ago

👋 Hi @bjamdort - thanks for reaching out about this. I was able to repro this but with slightly different results.

What I did:

  1. ran npx create-next-app@latest
  2. Added a file called middleware.ts to the root level
  3. Used the following bit of code:
export async function middleware(request: NextRequest) {
  const secretClient = new SecretClient(
    "<my vault url>",
    new DefaultAzureCredential({})
  );
  await secretClient.getSecret("my-secret");
  return NextResponse.next();
}

What I got was an error: DefaultAzureCredential is not supported in the browser. Use InteractiveBrowserCredential instead.

Checkpoint: Did you get that as well?

Then, I switched to using InteractiveBrowserCredential and got the error you described

 ⨯ node_modules/@azure/identity/dist-esm/src/msal/browserFlows/msalAuthCode.js (9:0) @ <unknown>
 ⨯ Cannot read properties of undefined (reading 'hash')

I'll keep investigating but I'm not too familiar with next.js so I'll need to learn a bit about where middleware is actually run (server or browser). In the meanwhile, could you confirm that we're looking at the same thing and that you are using InteractiveBrowserCredetial? If you are not, where is your middleware.ts file located?

Under app? src? root level?

The reason I ask is because:

  1. If middleware runs on the server as I suspect it does, I'll want to understand why nextJS is picking our browser replacements
  2. If it runs on the frontend, I'll want to investigate why self.location is not defined

Thanks in advance!

maorleger commented 7 months ago

Update: looking closely at the issue again, it sounds like instrumentation.ts is where you are trying to hook into Azure and the presence of middleware causes that to stop working is that right?

Looking at Next.JS docs it does look like middleware can only be run on the Edge Runtime. We support NodeJS and browsers today, and while we make a best-effort attempt at supporting other runtimes we cannot guarantee compatibility.

The Edge Runtime does not support all Node APIs, so they recommend using the Node.js runtime if you need to use packages that depend on those APIs. See documentation. This may also explain why adding middleware causes issues to pop up.

Could you try using the Node runtime instead? I'm not sure how the presence of middleware.ts changes nextJS's compilation and environment so I would recommend avoiding that path if possible

github-actions[bot] commented 7 months ago

Hi @bjamdort. 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.

bjamdort commented 7 months ago

It seems like instrumentation.ts and middleware.ts are connected and Next.js will choose the Edge runtime for both when you are using middleware.ts so I can't choose the Node runtime while using middleware.ts. For our case we won't be using middleware.ts because we need the Node runtime.

Thanks for the quick reaction, it helped us figure out why we couldn't get it to work. I'll close this issue because I don't think it's an issue with Azure Identity.