From c39e640bfedfe7c99361ba51adb43fec22c4ad9d Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Thu, 14 Jul 2022 06:54:06 -0700 Subject: [PATCH] fix(core): Add `sentry_client` to auth headers (#5413) This adds `sentry_client` to the auth headers* we send with every envelope request, as described [in the develop docs](https://develop.sentry.dev/sdk/overview/#authentication). In order to have access to the SDK metadata, the full `ClientOptions` object is now passed to `getEnvelopeEndpointWithUrlEncodedAuth`. Despite the fact that this is really an internal function, it's exported, so in order to keep everything backwards-compatible, for the moment it will also accept a string as the second argument, as it has in the past. Finally, all of our internal uses of the function have been switched to passing `options`, and there's a `TODO` in place so that we remember to remove the backwards compatibility in v8. Note that this change doesn't affect anyone using a tunnel, as no auth headers are sent in that case, in order to better cloak store requests from ad blockers. _\*The "headers" are actually querystring values, so as not to trigger CORS issues, but the effect is the same_ Fixes https://github.com/getsentry/sentry-javascript/issues/5406 --- packages/browser/src/client.ts | 2 +- packages/core/src/api.ts | 22 ++++++++++++--- packages/core/src/baseclient.ts | 2 +- packages/core/test/lib/api.test.ts | 43 +++++++++++++++++++++++++++--- 4 files changed, 59 insertions(+), 10 deletions(-) diff --git a/packages/browser/src/client.ts b/packages/browser/src/client.ts index eef289c3b00a..dae905672af1 100644 --- a/packages/browser/src/client.ts +++ b/packages/browser/src/client.ts @@ -161,7 +161,7 @@ export class BrowserClient extends BaseClient { __DEBUG_BUILD__ && logger.log('Sending outcomes:', outcomes); - const url = getEnvelopeEndpointWithUrlEncodedAuth(this._dsn, this._options.tunnel); + const url = getEnvelopeEndpointWithUrlEncodedAuth(this._dsn, this._options); const envelope = createClientReportEnvelope(outcomes, this._options.tunnel && dsnToString(this._dsn)); try { diff --git a/packages/core/src/api.ts b/packages/core/src/api.ts index 31375593fd4a..f454e148cc6c 100644 --- a/packages/core/src/api.ts +++ b/packages/core/src/api.ts @@ -1,4 +1,4 @@ -import { DsnComponents, DsnLike } from '@sentry/types'; +import { ClientOptions, DsnComponents, DsnLike, SdkInfo } from '@sentry/types'; import { dsnToString, makeDsn, urlEncode } from '@sentry/utils'; const SENTRY_API_VERSION = '7'; @@ -16,12 +16,13 @@ function _getIngestEndpoint(dsn: DsnComponents): string { } /** Returns a URL-encoded string with auth config suitable for a query string. */ -function _encodedAuth(dsn: DsnComponents): string { +function _encodedAuth(dsn: DsnComponents, sdkInfo: SdkInfo | undefined): string { return urlEncode({ // We send only the minimum set of required information. See // https://github.com/getsentry/sentry-javascript/issues/2572. sentry_key: dsn.publicKey, sentry_version: SENTRY_API_VERSION, + ...(sdkInfo && { sentry_client: `${sdkInfo.name}/${sdkInfo.version}` }), }); } @@ -30,8 +31,21 @@ function _encodedAuth(dsn: DsnComponents): string { * * Sending auth as part of the query string and not as custom HTTP headers avoids CORS preflight requests. */ -export function getEnvelopeEndpointWithUrlEncodedAuth(dsn: DsnComponents, tunnel?: string): string { - return tunnel ? tunnel : `${_getIngestEndpoint(dsn)}?${_encodedAuth(dsn)}`; +export function getEnvelopeEndpointWithUrlEncodedAuth( + dsn: DsnComponents, + // TODO (v8): Remove `tunnelOrOptions` in favor of `options`, and use the substitute code below + // options: ClientOptions = {} as ClientOptions, + tunnelOrOptions: string | ClientOptions = {} as ClientOptions, +): string { + // TODO (v8): Use this code instead + // const { tunnel, _metadata = {} } = options; + // return tunnel ? tunnel : `${_getIngestEndpoint(dsn)}?${_encodedAuth(dsn, _metadata.sdk)}`; + + const tunnel = typeof tunnelOrOptions === 'string' ? tunnelOrOptions : tunnelOrOptions.tunnel; + const sdkInfo = + typeof tunnelOrOptions === 'string' || !tunnelOrOptions._metadata ? undefined : tunnelOrOptions._metadata.sdk; + + return tunnel ? tunnel : `${_getIngestEndpoint(dsn)}?${_encodedAuth(dsn, sdkInfo)}`; } /** Returns the url to the report dialog endpoint. */ diff --git a/packages/core/src/baseclient.ts b/packages/core/src/baseclient.ts index 09c40a5aadc8..6518a7feec0f 100644 --- a/packages/core/src/baseclient.ts +++ b/packages/core/src/baseclient.ts @@ -104,7 +104,7 @@ export abstract class BaseClient implements Client { this._options = options; if (options.dsn) { this._dsn = makeDsn(options.dsn); - const url = getEnvelopeEndpointWithUrlEncodedAuth(this._dsn, options.tunnel); + const url = getEnvelopeEndpointWithUrlEncodedAuth(this._dsn, options); this._transport = options.transport({ recordDroppedEvent: this.recordDroppedEvent.bind(this), ...options.transportOptions, diff --git a/packages/core/test/lib/api.test.ts b/packages/core/test/lib/api.test.ts index 0a5ba52196e7..752f23844aad 100644 --- a/packages/core/test/lib/api.test.ts +++ b/packages/core/test/lib/api.test.ts @@ -1,4 +1,5 @@ /* eslint-disable deprecation/deprecation */ +import { ClientOptions, DsnComponents } from '@sentry/types'; import { makeDsn } from '@sentry/utils'; import { getEnvelopeEndpointWithUrlEncodedAuth, getReportDialogEndpoint } from '../../src/api'; @@ -6,15 +7,49 @@ import { getEnvelopeEndpointWithUrlEncodedAuth, getReportDialogEndpoint } from ' const ingestDsn = 'https://abc@xxxx.ingest.sentry.io:1234/subpath/123'; const dsnPublic = 'https://abc@sentry.io:1234/subpath/123'; const tunnel = 'https://hello.com/world'; +const _metadata = { sdk: { name: 'sentry.javascript.browser', version: '12.31.12' } } as ClientOptions['_metadata']; const dsnPublicComponents = makeDsn(dsnPublic); describe('API', () => { - test('getEnvelopeEndpoint', () => { - expect(getEnvelopeEndpointWithUrlEncodedAuth(dsnPublicComponents)).toEqual( - 'https://sentry.io:1234/subpath/api/123/envelope/?sentry_key=abc&sentry_version=7', + describe('getEnvelopeEndpointWithUrlEncodedAuth', () => { + it.each([ + [ + "doesn't include `sentry_client` when called with only DSN", + dsnPublicComponents, + undefined, + 'https://sentry.io:1234/subpath/api/123/envelope/?sentry_key=abc&sentry_version=7', + ], + ['uses `tunnel` value when called with `tunnel` as string', dsnPublicComponents, tunnel, tunnel], + [ + 'uses `tunnel` value when called with `tunnel` in options', + dsnPublicComponents, + { tunnel } as ClientOptions, + tunnel, + ], + [ + 'uses `tunnel` value when called with `tunnel` and `_metadata` in options', + dsnPublicComponents, + { tunnel, _metadata } as ClientOptions, + tunnel, + ], + [ + 'includes `sentry_client` when called with `_metadata` in options and no tunnel', + dsnPublicComponents, + { _metadata } as ClientOptions, + 'https://sentry.io:1234/subpath/api/123/envelope/?sentry_key=abc&sentry_version=7&sentry_client=sentry.javascript.browser%2F12.31.12', + ], + ])( + '%s', + ( + _testName: string, + dsnComponents: DsnComponents, + tunnelOrOptions: string | ClientOptions | undefined, + expected: string, + ) => { + expect(getEnvelopeEndpointWithUrlEncodedAuth(dsnComponents, tunnelOrOptions)).toBe(expected); + }, ); - expect(getEnvelopeEndpointWithUrlEncodedAuth(dsnPublicComponents, tunnel)).toEqual(tunnel); }); describe('getReportDialogEndpoint', () => {