Skip to content

Commit 4bf77f2

Browse files
thomasballingerConvex, Inc.
authored and
Convex, Inc.
committed
Configure OAuth to allow any subdomain (#36022)
GitOrigin-RevId: 19c132d541fbcf039a4dbaadadd9bd2f8619555c
1 parent 3db527f commit 4bf77f2

File tree

2 files changed

+139
-2
lines changed

2 files changed

+139
-2
lines changed

Diff for: npm-packages/dashboard/src/components/AuthorizeProject.test.tsx

+89
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,15 @@ describe("AuthorizeProject", () => {
5858
allowedRedirects: ["https://test-app.com/callback"],
5959
allowImplicitFlow: true,
6060
},
61+
"test-subdomain-client": {
62+
name: "Test App With Any Subdomain Allowed",
63+
allowedRedirects: [],
64+
allowedRedirectsAnySubdomain: [
65+
"https://a.pages.dev/foo",
66+
"http://b.com/bar",
67+
],
68+
allowImplicitFlow: true,
69+
},
6170
},
6271
};
6372

@@ -333,4 +342,84 @@ describe("AuthorizeProject", () => {
333342
screen.getByText("Authorize access to your project"),
334343
);
335344
});
345+
346+
describe("allowedRedirectsAnySubdomain", () => {
347+
test("accepts a subdomain of an allowed domain with matching path", () => {
348+
mockRouter.setCurrentUrl(
349+
"/?client_id=test-subdomain-client&redirect_uri=https://b.a.pages.dev/foo&response_type=token",
350+
);
351+
352+
render(<AuthorizeProject />);
353+
expect(
354+
screen.getByText("Authorize access to your project"),
355+
).toBeInTheDocument();
356+
});
357+
358+
test("accepts multiple levels of subdomains with matching path", () => {
359+
mockRouter.setCurrentUrl(
360+
"/?client_id=test-subdomain-client&redirect_uri=https://c.b.a.pages.dev/foo&response_type=token",
361+
);
362+
363+
render(<AuthorizeProject />);
364+
expect(
365+
screen.getByText("Authorize access to your project"),
366+
).toBeInTheDocument();
367+
});
368+
369+
test("rejects if domain structure is changed", () => {
370+
mockRouter.setCurrentUrl(
371+
"/?client_id=test-subdomain-client&redirect_uri=https://b.a.c.pages.dev/foo&response_type=token",
372+
);
373+
374+
render(<AuthorizeProject />);
375+
expect(screen.getByTestId("invalid-redirect-uri")).toBeInTheDocument();
376+
});
377+
378+
test("rejects if path is different", () => {
379+
mockRouter.setCurrentUrl(
380+
"/?client_id=test-subdomain-client&redirect_uri=https://b.a.pages.dev/foo/bar&response_type=token",
381+
);
382+
383+
render(<AuthorizeProject />);
384+
expect(screen.getByTestId("invalid-redirect-uri")).toBeInTheDocument();
385+
});
386+
387+
test("rejects if protocol is different", () => {
388+
mockRouter.setCurrentUrl(
389+
"/?client_id=test-subdomain-client&redirect_uri=http://b.a.pages.dev/foo&response_type=token",
390+
);
391+
392+
render(<AuthorizeProject />);
393+
expect(screen.getByTestId("invalid-redirect-uri")).toBeInTheDocument();
394+
});
395+
396+
test("accepts a subdomain for second allowed domain with matching path", () => {
397+
mockRouter.setCurrentUrl(
398+
"/?client_id=test-subdomain-client&redirect_uri=http://sub.b.com/bar&response_type=token",
399+
);
400+
401+
render(<AuthorizeProject />);
402+
expect(
403+
screen.getByText("Authorize access to your project"),
404+
).toBeInTheDocument();
405+
});
406+
407+
test("rejects if port is different", () => {
408+
mockRouter.setCurrentUrl(
409+
"/?client_id=test-subdomain-client&redirect_uri=https://b.a.pages.dev:8080/foo&response_type=token",
410+
);
411+
412+
render(<AuthorizeProject />);
413+
expect(screen.getByTestId("invalid-redirect-uri")).toBeInTheDocument();
414+
});
415+
416+
test("rejects query parameters", () => {
417+
mockRouter.setCurrentUrl(
418+
"/?client_id=test-subdomain-client&redirect_uri=https://b.a.pages.dev/foo?query=param&response_type=token",
419+
);
420+
421+
render(<AuthorizeProject />);
422+
expect(screen.getByTestId("invalid-redirect-uri")).toBeInTheDocument();
423+
});
424+
});
336425
});

Diff for: npm-packages/dashboard/src/components/AuthorizeProject.tsx

+50-2
Original file line numberDiff line numberDiff line change
@@ -330,7 +330,12 @@ function validateOAuthConfig(
330330
config: OAuthConfig,
331331
oauthProviderConfiguration: Record<
332332
string,
333-
{ name: string; allowedRedirects: string[]; allowImplicitFlow?: boolean }
333+
{
334+
name: string;
335+
allowedRedirects: string[];
336+
allowImplicitFlow?: boolean;
337+
allowedRedirectsAnySubdomain?: string[];
338+
}
334339
>,
335340
): {
336341
callingApplication: { name: string; allowedRedirects: string[] };
@@ -357,7 +362,7 @@ function validateOAuthConfig(
357362
};
358363
}
359364

360-
if (!callingApplication.allowedRedirects.includes(config.redirectUri)) {
365+
if (!redirectUriAllowed(callingApplication, config.redirectUri)) {
361366
// Don't include the invalid redirectUri in the validated config
362367
return {
363368
callingApplication,
@@ -417,6 +422,49 @@ function validateOAuthConfig(
417422
};
418423
}
419424

425+
function redirectUriAllowed(
426+
callingApplication: {
427+
allowedRedirectsAnySubdomain?: string[];
428+
allowedRedirects: string[];
429+
},
430+
redirectUri: string,
431+
): boolean {
432+
if (callingApplication.allowedRedirects.includes(redirectUri)) {
433+
return true;
434+
}
435+
436+
return (callingApplication.allowedRedirectsAnySubdomain || []).some(
437+
(allowedPattern) => {
438+
// https:// a.pages.dev/foo allows both
439+
// https:// b.a.pages.dev/foo and
440+
// https://c.b.a.pages.dev/foo.
441+
let allowedUrl: URL;
442+
let redirectUrl: URL;
443+
try {
444+
allowedUrl = new URL(allowedPattern);
445+
redirectUrl = new URL(redirectUri);
446+
} catch (e) {
447+
captureException(e);
448+
return false;
449+
}
450+
451+
if (
452+
redirectUrl.hostname !== allowedUrl.hostname &&
453+
!redirectUrl.hostname.endsWith(`.${allowedUrl.hostname}`)
454+
) {
455+
return false;
456+
}
457+
458+
return (
459+
redirectUrl.protocol === allowedUrl.protocol &&
460+
redirectUrl.port === allowedUrl.port &&
461+
redirectUrl.pathname === allowedUrl.pathname &&
462+
redirectUrl.search === allowedUrl.search
463+
);
464+
},
465+
);
466+
}
467+
420468
function buildOAuthRedirectUrl(
421469
validatedConfig: ValidatedOAuthConfig | undefined,
422470
params: {

0 commit comments

Comments
 (0)