getsentry / sentry-javascript

Official Sentry SDKs for JavaScript
https://sentry.io
MIT License
7.98k stars 1.57k forks source link

Tunnel endpoint basic auth override #8341

Closed filip-kis-fsc closed 5 months ago

filip-kis-fsc commented 1 year ago

Environment

SaaS (https://sentry.io/)

Steps to Reproduce

  1. use sentry tunnel with nextjs app
  2. authenticate sentry via env variable
  3. nextjs app has own basic auth unrelated to sentry
  4. open app, type in basic auth credentials to login and observe network for sentry tunnel request

Expected Result

sentry tunnel request is successful

Actual Result

sentry tunnel request fails with status 400

causes: ["invalid project key"]
detail: "bad sentry DSN public key"

This appears to happen due to header 'authorization: Basic <xyz>' being sent to the tunnel endpoint. Note that this basic auth has nothing to do with sentry but the app itself.

Is there a way to avoid this override?

Sentry is already using SENTRY_AUTH_TOKEN env variable and has dsn specified in sentry.client.config.js, sentry.server.config.js and sentry.edge.config.js files.

Product Area

Other

Link

No response

DSN

https://00cd68d0276b4de3a6f342955581ac69@o430938.ingest.sentry.io/6122809

Version

7.55.0

getsantry[bot] commented 1 year ago

Assigning to @getsentry/support for routing, due by (sfo). ⏲️

getsantry[bot] commented 1 year ago

Failed to route for Product Area: Other. Defaulting to @getsentry/open-source for triage, due by (sea). ⏲️

hubertdeng123 commented 1 year ago

Looks like the team that owns javascript SDK may be able to be of help here, transferring this over there.

AbhiPrasad commented 1 year ago

Hey @filip-kis-fsc, thanks for writing in. Are you using the tunnelRoute option to set up a tunnel? https://docs.sentry.io/platforms/javascript/guides/nextjs/manual-setup/#configure-tunneling-to-avoid-ad-blockers

If you enable the debug: true option in your sentry.client.config.js and sentry.server.config.js, what gets logged out?

Sentry.init({
  debug: true,
});
iscekic commented 1 year ago

I'm a colleague of @filip-kis-fsc, he's OOO today. 😄

Anyway yes, we are using tunneling. I'll leave the debug logs in a follow-up comment.

filip-kis-fsc commented 1 year ago

@AbhiPrasad thanks for responding! We are using the tunnelRoute option to set up a tunnel.

Just to reiterate, the issue only occurs when a basic auth header (non-sentry related) is sent to the tunnel endpoint - sentry appears to interpret it as a dsn public key.

You can find the requested logs below.

Server logs:

[1] Sentry Logger [log]: Initializing SDK...
[1] Sentry Logger [log]: Integration installed: InboundFilters
[1] Sentry Logger [log]: Integration installed: FunctionToString
[1] Sentry Logger [log]: Integration installed: Console
[1] Sentry Logger [log]: Integration installed: Http
[1] Sentry Logger [log]: Integration installed: Undici
[1] Sentry Logger [log]: Integration installed: OnUncaughtException
[1] Sentry Logger [log]: Integration installed: OnUnhandledRejection
[1] Sentry Logger [log]: Integration installed: ContextLines
[1] Sentry Logger [log]: Integration installed: LocalVariables
[1] Sentry Logger [log]: Integration installed: Context
[1] Sentry Logger [log]: Integration installed: Modules
[1] Sentry Logger [log]: Integration installed: RequestData
[1] Sentry Logger [log]: Integration installed: LinkedErrors
[1] Sentry Logger [log]: Integration installed: RewriteFrames
[1] Sentry Logger [log]: SDK successfully initialized

Browser logs:

Screenshot 2023-06-20 at 13 18 36

Note that the tunnel request works fine when the basic auth header is not sent.

GitHelge commented 1 year ago

Same issue for us. Thanks for raising the point. Good thing is that the prod app is not behind a basic auth... :D

AbhiPrasad commented 1 year ago

I guess we should strip this header - @lforst does that seem right?

lforst commented 1 year ago

I am wondering, how are basic auth headers ending up in the request to the tunnel endpoint? 🤔

The tunnel endpoint just forwards everything it receives. Can you adjust your application logic not to attach auth headers to that request?

iscekic commented 1 year ago

This occurs in our non-prod environments which are "protected" by http basic auth. So it's not us attaching the header, it's the browser. 😄

lforst commented 1 year ago

@iscekic I don't think we can strip headers in Next.js rewrites. Would it be an option for you to just not use the tunnelRoute option inside your non-prod envs?

iscekic commented 1 year ago

We can, but it's not ideal as we want to minimize differences between our envs.

lforst commented 1 year ago

I don't think we will invest work into this for now, as it is essentially user behaviour to add this to the request and there is a very simple workaround (by not setting the tunnelRoute option). PRs are always welcome though!

j2ghz commented 1 year ago

A workaround is to strip the Authorization header after checking it before calling the final NextResponse. In our case I replaced return NextResponse.next(); with

// after successfully checking basic auth
const headers = new Headers(request.headers);
headers.delete('authorization');
return NextResponse.next({ request: { headers } });
Tomekmularczyk commented 1 year ago

Same problem, we are using basic NextJS basic auth as in the example and get 400 from tunnelRoute about incorrect DSN.

lforst commented 12 months ago

@Tomekmularczyk You likely want to exclude the tunnel endpoint from your middleware matcher.

getsantry[bot] commented 5 months ago

This issue has gone three weeks without activity. In another week, I will close it.

But! If you comment or otherwise update it, I will reset the clock, and if you remove the label Waiting for: Community, I will leave it alone ... forever!


"A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀

iscekic commented 5 months ago

Though low prio, this shouldn't be closed I think.

lforst commented 5 months ago

Unfortunately, I don't think there is anything we can do here from within SDK land. We already talk about limitations arount the tunnelRoute option in the docs. If you feel like the SDK can do something better, feel free to outline the solution here.

mhfortuna commented 1 week ago

I tried adding this middleware in front of the API, in my case /monitoring is the tunnel. Stopped the error on the client

import { NextResponse } from "next/server";
import type { NextRequest } from "next/server";

export function middleware(request: NextRequest) {
  const headers = new Headers(request.headers);

  headers.delete("authorization");

  return NextResponse.next({
    request: {
      headers: headers,
    },
  });
}

export const config = {
  matcher: [
    {
      source: "/monitoring",
      has: [{ type: "header", key: "authorization" }],
    },
  ],
};