Browse Source

fix(ui) Fix origin handling logic for CSRF tokens (#56912)

With the addition of region domains for API requests our existing
cross-origin logic for CSRF tokens is not robust enough. We now need to
send CSRF tokens when the origin is sentry.sentry.io and the request is
going to one of `sentry.io`, `sentry.sentry.io` or `us.sentry.io`.
Mark Story 1 year ago
parent
commit
e1406c1c5c
2 changed files with 59 additions and 6 deletions
  1. 33 1
      static/app/api.spec.tsx
  2. 26 5
      static/app/api.tsx

+ 33 - 1
static/app/api.spec.tsx

@@ -1,4 +1,4 @@
-import {Request, resolveHostname} from 'sentry/api';
+import {isSimilarOrigin, Request, resolveHostname} from 'sentry/api';
 import {PROJECT_MOVED} from 'sentry/constants/apiErrorCodes';
 
 import ConfigStore from './stores/configStore';
@@ -182,3 +182,35 @@ describe('resolveHostname', function () {
     expect(result).toBe('https://de.sentry.io/api/0/organizations/slug/issues/');
   });
 });
+
+describe('isSimilarOrigin', function () {
+  test.each([
+    // Same domain
+    ['https://sentry.io', 'https://sentry.io', true],
+    ['https://example.io', 'https://example.io', true],
+
+    // Not the same
+    ['https://example.io', 'https://sentry.io', false],
+    ['https://sentry.io', 'https://io.sentry', false],
+
+    // Sibling domains
+    ['https://us.sentry.io', 'https://sentry.sentry.io', true],
+    ['https://us.sentry.io', 'https://acme.sentry.io', true],
+    ['https://us.sentry.io', 'https://eu.sentry.io', true],
+    ['https://woof.sentry.io', 'https://woof-org.sentry.io', true],
+    ['https://woof.sentry.io/issues/1234/', 'https://woof-org.sentry.io', true],
+
+    // Subdomain
+    ['https://sentry.io/api/0/broadcasts/', 'https://woof.sentry.io', true],
+    ['https://sentry.io/api/0/users/', 'https://sentry.sentry.io', true],
+    ['https://sentry.io/api/0/users/', 'https://io.sentry.io', true],
+
+    // Not siblings
+    ['https://sentry.io/api/0/broadcasts/', 'https://sentry.example.io', false],
+    ['https://woof.example.sentry.io', 'https://example.sentry.io', false],
+    ['https://woof.example.io', 'https://woof.sentry.io', false],
+    ['https://woof.sentry.io', 'https://sentry.woof.io', false],
+  ])('allows sibling domains %s and %s is %s', (target, origin, expected) => {
+    expect(isSimilarOrigin(target, origin)).toBe(expected);
+  });
+});

+ 26 - 5
static/app/api.tsx

@@ -80,11 +80,35 @@ export type ResponseMeta<R = any> = {
 /**
  * Check if the requested method does not require CSRF tokens
  */
-function csrfSafeMethod(method?: string) {
+function csrfSafeMethod(method?: string): boolean {
   // these HTTP methods do not require CSRF protection
   return /^(GET|HEAD|OPTIONS|TRACE)$/.test(method ?? '');
 }
 
+/**
+ * Check if we a request is going to the same or similar origin.
+ * similar origins are those that share an ancestor. Example `sentry.sentry.io` and `us.sentry.io`
+ * are similar origins, but sentry.sentry.io and sentry.example.io are not.
+ */
+export function isSimilarOrigin(target: string, origin: string): boolean {
+  const targetUrl = new URL(target, origin);
+  const originUrl = new URL(origin);
+  if (originUrl.hostname.endsWith(targetUrl.hostname)) {
+    return true;
+  }
+  // Check if the target and origin are on sibiling subdomains.
+  const targetHost = targetUrl.hostname.split('.');
+  const originHost = originUrl.hostname.split('.');
+
+  // Remove the subdomains. If don't have at least 2 segments we aren't subdomains.
+  targetHost.shift();
+  originHost.shift();
+  if (targetHost.length < 2 || originHost.length < 2) {
+    return false;
+  }
+  return targetHost.join('.') === originHost.join('.');
+}
+
 // TODO: Need better way of identifying anonymous pages that don't trigger redirect
 const ALLOWED_ANON_PAGES = [
   /^\/accept\//,
@@ -464,10 +488,7 @@ export class Client {
 
     // Do not set the X-CSRFToken header when making a request outside of the
     // current domain. Because we use subdomains we loosely compare origins
-    const absoluteUrl = new URL(fullUrl, window.location.origin);
-    const originUrl = new URL(window.location.origin);
-    const isSameOrigin = originUrl.hostname.endsWith(absoluteUrl.hostname);
-    if (!csrfSafeMethod(method) && isSameOrigin) {
+    if (!csrfSafeMethod(method) && isSimilarOrigin(fullUrl, window.location.origin)) {
       headers.set('X-CSRFToken', getCsrfToken());
     }