Skip to content

Commit 06e5c98

Browse files
fix: allow whitelisted paths like onboarding as team/user slugs on org domains (calcom#24984)
* fix: allow whitelisted paths like onboarding as team/user slugs on org domains - Add whitelistedPaths array to pagesAndRewritePaths.js with 'onboarding' - Update getRegExpMatchingAllReservedRoutes to accept exclusions parameter - Modify org route patterns to exclude whitelisted paths from reserved routes - Add tests to verify onboarding can be used as a slug on org domains - Fixes issue where acme.cal.com/onboarding would 404 This allows teams/users on org domains to use 'onboarding' as their slug while still preserving the /onboarding app route on non-org domains. Co-Authored-By: hariom@cal.com <hariombalhara@gmail.com> * Remove accidental change by AI * Add special character handling test --------- Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
1 parent 528b27f commit 06e5c98

4 files changed

Lines changed: 125 additions & 17 deletions

File tree

apps/web/pagesAndRewritePaths.js

Lines changed: 35 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,26 @@
1+
/* eslint-env node */
2+
/* eslint-disable @typescript-eslint/no-require-imports, no-undef */
13
const glob = require("glob");
24
const { nextJsOrgRewriteConfig } = require("./getNextjsOrgRewriteConfig");
3-
/** Needed to rewrite public booking page, gets all static pages but [user] */
4-
// Pages found here are excluded from redirects in beforeFiles in next.config.js
5-
let pages = (exports.pages = glob
5+
6+
// Top-level route names that are explicitly allowed for org rewrite (whitelist)
7+
8+
const topLevelRouteNamesWhitelistedForRewrite = exports.topLevelRouteNamesWhitelistedForRewrite = [
9+
// We don't allow all dashboard route names to be used as slug because people are probably accustomed to access links like acme.cal.com/workflows, acme.cal.com/event-types etc.
10+
// So, we carefully allow, what is absolutely needed.
11+
// Allowed to be a team/user slug in organization because onboarding is a common team name
12+
'onboarding',
13+
]
14+
15+
/**
16+
* Extracts top-level route names from all pages/app files and excludes them from org rewrite.
17+
* For example: /abc/def/ghi -> 'abc'
18+
*
19+
* These top-level route names are excluded from rewrites in beforeFiles in next.config.js
20+
* to prevent conflicts with organization slug rewrites.
21+
*/
22+
/* eslint-disable no-undef */
23+
let topLevelRoutesExcludedFromOrgRewrite = exports.topLevelRoutesExcludedFromOrgRewrite = glob
624
.sync(
725
"{pages,app,app/(booking-page-wrapper),app/(use-page-wrapper),app/(use-page-wrapper)/(main-nav)}/**/[^_]*.{tsx,js,ts}",
826
{
@@ -11,11 +29,14 @@ let pages = (exports.pages = glob
1129
)
1230
.map((filename) =>
1331
filename
32+
// Remove the directory prefix (pages/, app/ and route group folders.)
1433
.replace(
1534
/^(app\/\(use-page-wrapper\)\/\(main-nav\)|app\/\(use-page-wrapper\)|app\/\(booking-page-wrapper\)|pages|app)\//,
1635
""
1736
)
37+
// Remove file extensions
1838
.replace(/(\.tsx|\.js|\.ts)/, "")
39+
// Extract only the top-level route name (e.g., /abc/def -> abc)
1940
.replace(/\/.*/, "")
2041
)
2142
.filter(
@@ -34,40 +55,47 @@ let pages = (exports.pages = glob
3455
"ShellMainAppDir",
3556
"ShellMainAppDirBackButton",
3657
].some((prefix) => v.startsWith(prefix))
37-
));
58+
)
59+
.filter((page) => {
60+
return !topLevelRouteNamesWhitelistedForRewrite.includes(page);
61+
});
3862

3963
// .* matches / as well(Note: *(i.e wildcard) doesn't match / but .*(i.e. RegExp) does)
4064
// It would match /free/30min but not /bookings/upcoming because 'bookings' is an item in pages
4165
// It would also not match /free/30min/embed because we are ensuring just two slashes
4266
// ?!book ensures it doesn't match /free/book page which doesn't have a corresponding new-booker page.
4367
// [^/]+ makes the RegExp match the full path, it seems like a partial match doesn't work.
4468
// book$ ensures that only /book is excluded from rewrite(which is at the end always) and not /booked
45-
69+
/* eslint-disable no-undef */
4670
exports.nextJsOrgRewriteConfig = nextJsOrgRewriteConfig;
4771

4872
/**
4973
* Returns a regex that matches all existing routes, virtual routes (like /forms, /router, /success etc) and nextjs special paths (_next, public)
74+
* @param {string} suffix - The suffix to append to each route in the regex
5075
*/
5176
function getRegExpMatchingAllReservedRoutes(suffix) {
5277
// Following routes don't exist but they work by doing rewrite. Thus they need to be excluded from matching the orgRewrite patterns
5378
// Make sure to keep it upto date as more nonExistingRouteRewrites are added.
5479
const otherNonExistingRoutePrefixes = ["forms", "router", "success", "cancel"];
80+
5581
// Most files/dirs in public dir must not be rewritten to org pages. Ideally it should be all the content of public dir, but that can be done later
5682
// It is important to exclude the embed pages separately here because with SINGLE_ORG_SLUG enabled, the entire domain is eligible for rewrite vs just the org subdomain otherwise
5783
const staticAssets = ["embed"];
84+
5885
// FIXME: I am not sure why public is needed here, an asset 'test' in public isn't accessible through "/public/test" but only through "/test"
5986
// We should infact scan through all files in public and exclude them instead.
6087
const nextJsSpecialPaths = ["_next", "public"];
6188

62-
let beforeRewriteExcludePages = pages
89+
let allTopLevelRoutesExcludedFromOrgRewrite = topLevelRoutesExcludedFromOrgRewrite
6390
.concat(otherNonExistingRoutePrefixes)
6491
.concat(nextJsSpecialPaths)
6592
.concat(staticAssets);
66-
return beforeRewriteExcludePages.join(`${suffix}|`) + suffix;
93+
return allTopLevelRoutesExcludedFromOrgRewrite.join(`${suffix}|`) + suffix;
6794
}
6895

6996
// To handle /something
7097
exports.orgUserRoutePath = `/:user((?!${getRegExpMatchingAllReservedRoutes("/?$")})[a-zA-Z0-9\-_]+)`;
98+
7199
// To handle /something/somethingelse
72100
exports.orgUserTypeRoutePath = `/:user((?!${getRegExpMatchingAllReservedRoutes(
73101
"/"

apps/web/test/lib/next-config.test.ts

Lines changed: 76 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { it, expect, describe, beforeAll } from "vitest";
22

33
import { getRegExpThatMatchesAllOrgDomains } from "../../getNextjsOrgRewriteConfig";
44

5-
// eslint-disable-next-line @typescript-eslint/no-var-requires
5+
/* eslint-disable @typescript-eslint/no-require-imports */
66
const { match, pathToRegexp } = require("next/dist/compiled/path-to-regexp");
77
type MatcherRes = (path: string) => { params: Record<string, string> };
88
let orgUserTypeRouteMatch: MatcherRes;
@@ -15,7 +15,6 @@ beforeAll(async () => {
1515
const {
1616
orgUserRoutePath,
1717
orgUserTypeRoutePath,
18-
// eslint-disable-next-line @typescript-eslint/no-var-requires
1918
} = require("../../pagesAndRewritePaths");
2019

2120
orgUserTypeRouteMatch = match(orgUserTypeRoutePath);
@@ -162,5 +161,80 @@ describe("next.config.js - Org Rewrite", () => {
162161
expect(orgUserRouteMatch("/forms/xdsdf-sd")).toEqual(false);
163162
expect(orgUserRouteMatch("/router?form=")).toEqual(false);
164163
});
164+
165+
it("Whitelisted routes should match on org domains", () => {
166+
expect(orgUserRouteMatch("/onboarding")?.params).toEqual({
167+
user: "onboarding",
168+
});
169+
expect(orgUserTypeRouteMatch("/onboarding/30min")?.params).toEqual({
170+
user: "onboarding",
171+
type: "30min",
172+
});
173+
});
174+
175+
describe("Character validation in org slugs", () => {
176+
it("should allow valid characters: alphanumeric, hyphens, and underscores", () => {
177+
// Valid usernames with allowed characters
178+
expect(orgUserRouteMatch("/john-doe")?.params).toEqual({
179+
user: "john-doe",
180+
});
181+
expect(orgUserRouteMatch("/john_doe")?.params).toEqual({
182+
user: "john_doe",
183+
});
184+
expect(orgUserRouteMatch("/user123")?.params).toEqual({
185+
user: "user123",
186+
});
187+
expect(orgUserRouteMatch("/ABC-123_xyz")?.params).toEqual({
188+
user: "ABC-123_xyz",
189+
});
190+
});
191+
192+
it("should reject invalid punctuation characters in org slugs", () => {
193+
// These should NOT match because they contain invalid characters
194+
// The character class [a-zA-Z0-9-_] when hyphen is unescaped between 9 and _
195+
// creates a range from - (ASCII 45) to _ (ASCII 95), which includes : ; < = > ? @ [ \ ] ^
196+
197+
// Colon (:) - ASCII 58
198+
expect(orgUserRouteMatch("/user:name")).toEqual(false);
199+
200+
// Semicolon (;) - ASCII 59
201+
expect(orgUserRouteMatch("/user;name")).toEqual(false);
202+
203+
// Less than (<) - ASCII 60
204+
expect(orgUserRouteMatch("/user<name")).toEqual(false);
205+
206+
// Equals (=) - ASCII 61
207+
expect(orgUserRouteMatch("/user=name")).toEqual(false);
208+
209+
// Greater than (>) - ASCII 62
210+
expect(orgUserRouteMatch("/user>name")).toEqual(false);
211+
212+
// Question mark (?) - ASCII 63
213+
expect(orgUserRouteMatch("/user?name")).toEqual(false);
214+
215+
// At symbol (@) - ASCII 64
216+
expect(orgUserRouteMatch("/user@name")).toEqual(false);
217+
218+
// Square brackets ([, ]) - ASCII 91, 93
219+
expect(orgUserRouteMatch("/user[name")).toEqual(false);
220+
expect(orgUserRouteMatch("/user]name")).toEqual(false);
221+
222+
// Backslash (\) - ASCII 92
223+
expect(orgUserRouteMatch("/user\\name")).toEqual(false);
224+
225+
// Caret (^) - ASCII 94
226+
expect(orgUserRouteMatch("/user^name")).toEqual(false);
227+
228+
// Other common punctuation that should also be rejected
229+
expect(orgUserRouteMatch("/user.name")).toEqual(false);
230+
expect(orgUserRouteMatch("/user!name")).toEqual(false);
231+
expect(orgUserRouteMatch("/user#name")).toEqual(false);
232+
expect(orgUserRouteMatch("/user$name")).toEqual(false);
233+
expect(orgUserRouteMatch("/user%name")).toEqual(false);
234+
expect(orgUserRouteMatch("/user&name")).toEqual(false);
235+
expect(orgUserRouteMatch("/user*name")).toEqual(false);
236+
expect(orgUserRouteMatch("/user+name")).toEqual(false);
237+
});
238+
});
165239
});
166240
});

apps/web/test/lib/pagesAndRewritePaths.test.ts

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
import { it, expect, describe } from "vitest";
22

3-
import { pages } from "../../pagesAndRewritePaths.js";
3+
import { topLevelRoutesExcludedFromOrgRewrite, topLevelRouteNamesWhitelistedForRewrite } from "../../pagesAndRewritePaths.js";
44

55
describe("pagesAndRewritePaths", () => {
6-
describe("beforeFiles must exclude routes in pages/app router", () => {
7-
const BEFORE_REWRITE_EXCLUDE_PAGES = [
6+
describe("beforeFiles must exclude top-level routes in pages/app router", () => {
7+
const ROUTES_EXCLUDED_FROM_ORG_REWRITE = [
88
"apps",
99
"availability",
1010
"booking",
@@ -30,12 +30,18 @@ describe("pagesAndRewritePaths", () => {
3030
"routing-forms",
3131
"signup",
3232
"team",
33-
"d",
33+
"d"
3434
];
3535

36-
it("should include all required routes", () => {
37-
BEFORE_REWRITE_EXCLUDE_PAGES.forEach((route) => {
38-
expect(pages).toContain(route);
36+
it("should include all required top-level route names", () => {
37+
ROUTES_EXCLUDED_FROM_ORG_REWRITE.forEach((route) => {
38+
expect(topLevelRoutesExcludedFromOrgRewrite).toContain(route);
39+
});
40+
});
41+
42+
it("should NOT include whitelisted routes", () => {
43+
topLevelRouteNamesWhitelistedForRewrite.forEach((route) => {
44+
expect(topLevelRoutesExcludedFromOrgRewrite).not.toContain(route);
3945
});
4046
});
4147
});

packages/prisma/schema.prisma

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// This is your Prisma Schema file
1+
// This is your Prisma Schema file
22
// learn more about it in the docs: https://pris.ly/d/prisma-schema
33

44
datasource db {

0 commit comments

Comments
 (0)