Skip to content

Commit df392be

Browse files
iQQBotona-agent
andcommitted
1
Co-authored-by: Ona <[email protected]>
1 parent 6768661 commit df392be

File tree

3 files changed

+371
-11
lines changed

3 files changed

+371
-11
lines changed

components/server/src/auth/authenticator.ts

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
import { BUILTIN_INSTLLATION_ADMIN_USER_ID, TeamDB } from "@gitpod/gitpod-db/lib";
88
import { User } from "@gitpod/gitpod-protocol";
99
import { log } from "@gitpod/gitpod-protocol/lib/util/logging";
10+
import { GitpodHostUrl } from "@gitpod/gitpod-protocol/lib/util/gitpod-host-url";
1011
import express from "express";
1112
import { inject, injectable, postConstruct } from "inversify";
1213
import passport from "passport";
@@ -21,6 +22,82 @@ import { SignInJWT } from "./jwt";
2122
import { NonceService } from "./nonce-service";
2223
import { ensureUrlHasFragment } from "./fragment-utils";
2324

25+
/**
26+
* Common validation logic for returnTo URLs.
27+
* @param returnTo The URL to validate
28+
* @param hostUrl The host URL configuration
29+
* @param allowedPatterns Array of regex patterns that are allowed for the pathname
30+
* @returns true if the URL is valid, false otherwise
31+
*/
32+
function validateReturnToUrlWithPatterns(returnTo: string, hostUrl: GitpodHostUrl, allowedPatterns: RegExp[]): boolean {
33+
try {
34+
const url = new URL(returnTo);
35+
const baseUrl = hostUrl.url;
36+
37+
// Must be same origin OR www.gitpod.io exception
38+
const isSameOrigin = url.origin === baseUrl.origin;
39+
const isGitpodWebsite = url.protocol === "https:" && url.hostname === "www.gitpod.io";
40+
41+
if (!isSameOrigin && !isGitpodWebsite) {
42+
return false;
43+
}
44+
45+
// For www.gitpod.io, only allow root path
46+
if (isGitpodWebsite) {
47+
return url.pathname === "/";
48+
}
49+
50+
// Check if pathname matches any allowed pattern
51+
const isAllowedPath = allowedPatterns.some((pattern) => pattern.test(url.pathname));
52+
if (!isAllowedPath) {
53+
return false;
54+
}
55+
56+
// For complete-auth, require ONLY message parameter (used by OAuth flows)
57+
if (url.pathname === "/complete-auth") {
58+
const searchParams = new URLSearchParams(url.search);
59+
const paramKeys = Array.from(searchParams.keys());
60+
return paramKeys.length === 1 && paramKeys[0] === "message" && searchParams.has("message");
61+
}
62+
63+
return true;
64+
} catch (error) {
65+
// Invalid URL
66+
return false;
67+
}
68+
}
69+
70+
/**
71+
* Validates returnTo URLs for login API endpoints.
72+
* Login API allows broader navigation after authentication.
73+
*/
74+
export function validateLoginReturnToUrl(returnTo: string, hostUrl: GitpodHostUrl): boolean {
75+
const allowedPatterns = [
76+
// We have already verified the domain above, and we do not restrict the redirect location for loginReturnToUrl.
77+
/^\/.*$/,
78+
];
79+
80+
return validateReturnToUrlWithPatterns(returnTo, hostUrl, allowedPatterns);
81+
}
82+
83+
/**
84+
* Validates returnTo URLs for authorize API endpoints.
85+
* Authorize API allows complete-auth callbacks and dashboard pages for scope elevation.
86+
*/
87+
export function validateAuthorizeReturnToUrl(returnTo: string, hostUrl: GitpodHostUrl): boolean {
88+
const allowedPatterns = [
89+
// 1. complete-auth callback for OAuth popup windows
90+
/^\/complete-auth$/,
91+
92+
// 2. Dashboard pages (for scope elevation flows)
93+
/^\/$/, // Root
94+
/^\/new$/, // Create workspace page
95+
/^\/quickstart$/, // Quickstart page
96+
];
97+
98+
return validateReturnToUrlWithPatterns(returnTo, hostUrl, allowedPatterns);
99+
}
100+
24101
@injectable()
25102
export class Authenticator {
26103
protected passportInitialize: express.Handler;
@@ -192,6 +269,13 @@ export class Authenticator {
192269
let returnToParam: string | undefined = req.query.returnTo?.toString();
193270
if (returnToParam) {
194271
log.info(`Stored returnTo URL: ${returnToParam}`, { "login-flow": true });
272+
273+
// Validate returnTo URL against allowlist for login API
274+
if (!validateLoginReturnToUrl(returnToParam, this.config.hostUrl)) {
275+
log.warn(`Invalid returnTo URL rejected for login: ${returnToParam}`, { "login-flow": true });
276+
res.redirect(this.getSorryUrl(`Invalid return URL.`));
277+
return;
278+
}
195279
}
196280
// returnTo defaults to workspaces url
197281
const workspaceUrl = this.config.hostUrl.asDashboard().toString();
@@ -306,6 +390,13 @@ export class Authenticator {
306390
return;
307391
}
308392

393+
// Validate returnTo URL against allowlist for authorize API
394+
if (!validateAuthorizeReturnToUrl(returnToParam, this.config.hostUrl)) {
395+
log.warn(`Invalid returnTo URL rejected for authorize: ${returnToParam}`, { "authorize-flow": true });
396+
res.redirect(this.getSorryUrl(`Invalid return URL.`));
397+
return;
398+
}
399+
309400
// Ensure returnTo URL has a fragment to prevent OAuth token inheritance attacks
310401
const returnTo = ensureUrlHasFragment(returnToParam);
311402

Lines changed: 255 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,255 @@
1+
/**
2+
* Copyright (c) 2024 Gitpod GmbH. All rights reserved.
3+
* Licensed under the GNU Affero General Public License (AGPL).
4+
* See License.AGPL.txt in the project root for license information.
5+
*/
6+
7+
import { expect } from "chai";
8+
import { GitpodHostUrl } from "@gitpod/gitpod-protocol/lib/util/gitpod-host-url";
9+
import { validateLoginReturnToUrl, validateAuthorizeReturnToUrl } from "./authenticator";
10+
11+
describe("ReturnTo URL Validation", () => {
12+
const hostUrl = new GitpodHostUrl("https://gitpod.io");
13+
14+
describe("validateLoginReturnToUrl", () => {
15+
it("should accept any valid path after domain validation", () => {
16+
const validUrls = [
17+
"https://gitpod.io/",
18+
"https://gitpod.io/workspaces",
19+
"https://gitpod.io/workspaces/123",
20+
"https://gitpod.io/settings",
21+
"https://gitpod.io/settings/integrations",
22+
"https://gitpod.io/user-settings",
23+
"https://gitpod.io/user-settings/account",
24+
"https://gitpod.io/integrations",
25+
"https://gitpod.io/integrations/github",
26+
"https://gitpod.io/repositories",
27+
"https://gitpod.io/repositories/123",
28+
"https://gitpod.io/prebuilds",
29+
"https://gitpod.io/prebuilds/456",
30+
"https://gitpod.io/members",
31+
"https://gitpod.io/billing",
32+
"https://gitpod.io/usage",
33+
"https://gitpod.io/insights",
34+
"https://gitpod.io/new",
35+
"https://gitpod.io/login",
36+
"https://gitpod.io/login/org-slug",
37+
"https://gitpod.io/quickstart",
38+
"https://gitpod.io/admin", // Now allowed since login doesn't restrict paths
39+
"https://gitpod.io/api/workspace", // Now allowed
40+
"https://gitpod.io/any-path", // Any path is allowed
41+
];
42+
43+
validUrls.forEach((url) => {
44+
const result = validateLoginReturnToUrl(url, hostUrl);
45+
expect(result).to.equal(true, `Should accept valid login URL: ${url}`);
46+
});
47+
});
48+
49+
it("should accept complete-auth with ONLY message parameter", () => {
50+
const validCompleteAuthUrls = [
51+
"https://gitpod.io/complete-auth?message=success:123",
52+
"https://gitpod.io/complete-auth?message=success",
53+
];
54+
55+
validCompleteAuthUrls.forEach((url) => {
56+
const result = validateLoginReturnToUrl(url, hostUrl);
57+
expect(result).to.equal(true, `Should accept complete-auth with only message: ${url}`);
58+
});
59+
});
60+
61+
it("should reject complete-auth with additional parameters", () => {
62+
const invalidCompleteAuthUrls = [
63+
"https://gitpod.io/complete-auth?message=success&other=param",
64+
"https://gitpod.io/complete-auth?other=param&message=success",
65+
"https://gitpod.io/complete-auth",
66+
"https://gitpod.io/complete-auth?other=param",
67+
"https://gitpod.io/complete-auth?msg=success", // Wrong parameter name
68+
];
69+
70+
invalidCompleteAuthUrls.forEach((url) => {
71+
const result = validateLoginReturnToUrl(url, hostUrl);
72+
expect(result).to.equal(false, `Should reject complete-auth with extra params: ${url}`);
73+
});
74+
});
75+
76+
it("should accept www.gitpod.io root only", () => {
77+
const result = validateLoginReturnToUrl("https://www.gitpod.io/", hostUrl);
78+
expect(result).to.equal(true, "Should accept www.gitpod.io root");
79+
80+
const invalidWwwUrls = [
81+
"https://www.gitpod.io/workspaces",
82+
"https://www.gitpod.io/login",
83+
"http://www.gitpod.io/", // Wrong protocol
84+
];
85+
86+
invalidWwwUrls.forEach((url) => {
87+
const result = validateLoginReturnToUrl(url, hostUrl);
88+
expect(result).to.equal(false, `Should reject www.gitpod.io non-root: ${url}`);
89+
});
90+
});
91+
});
92+
93+
describe("validateAuthorizeReturnToUrl", () => {
94+
it("should accept only specific allowlisted paths", () => {
95+
const validUrls = [
96+
"https://gitpod.io/", // Root
97+
"https://gitpod.io/new", // Create workspace page
98+
"https://gitpod.io/quickstart", // Quickstart page
99+
];
100+
101+
validUrls.forEach((url) => {
102+
const result = validateAuthorizeReturnToUrl(url, hostUrl);
103+
expect(result).to.equal(true, `Should accept valid authorize URL: ${url}`);
104+
});
105+
});
106+
107+
it("should accept complete-auth with ONLY message parameter", () => {
108+
const validCompleteAuthUrls = [
109+
"https://gitpod.io/complete-auth?message=success:123",
110+
"https://gitpod.io/complete-auth?message=success",
111+
];
112+
113+
validCompleteAuthUrls.forEach((url) => {
114+
const result = validateAuthorizeReturnToUrl(url, hostUrl);
115+
expect(result).to.equal(true, `Should accept complete-auth with only message: ${url}`);
116+
});
117+
});
118+
119+
it("should reject paths not in authorize allowlist", () => {
120+
const rejectedUrls = [
121+
"https://gitpod.io/workspaces",
122+
"https://gitpod.io/workspaces/123",
123+
"https://gitpod.io/user-settings",
124+
"https://gitpod.io/integrations",
125+
"https://gitpod.io/prebuilds",
126+
"https://gitpod.io/members",
127+
"https://gitpod.io/billing",
128+
"https://gitpod.io/usage",
129+
"https://gitpod.io/insights",
130+
"https://gitpod.io/login",
131+
"https://gitpod.io/settings",
132+
"https://gitpod.io/repositories",
133+
"https://gitpod.io/admin",
134+
"https://gitpod.io/api/workspace",
135+
];
136+
137+
rejectedUrls.forEach((url) => {
138+
const result = validateAuthorizeReturnToUrl(url, hostUrl);
139+
expect(result).to.equal(false, `Should reject authorize URL: ${url}`);
140+
});
141+
});
142+
143+
it("should accept www.gitpod.io root only", () => {
144+
const result = validateAuthorizeReturnToUrl("https://www.gitpod.io/", hostUrl);
145+
expect(result).to.equal(true, "Should accept www.gitpod.io root");
146+
});
147+
});
148+
149+
describe("Common validation tests", () => {
150+
it("should reject different origins", () => {
151+
const invalidOriginUrls = [
152+
"https://evil.com/workspaces",
153+
"http://gitpod.io/workspaces", // Different protocol
154+
"https://gitpod.io:8080/workspaces", // Different port
155+
"https://subdomain.gitpod.io/workspaces", // Different subdomain
156+
];
157+
158+
invalidOriginUrls.forEach((url) => {
159+
expect(validateLoginReturnToUrl(url, hostUrl)).to.equal(false, `Login should reject: ${url}`);
160+
expect(validateAuthorizeReturnToUrl(url, hostUrl)).to.equal(false, `Authorize should reject: ${url}`);
161+
});
162+
});
163+
164+
it("should have different path restrictions for login vs authorize", () => {
165+
const pathsAllowedInLoginOnly = [
166+
"https://gitpod.io/admin",
167+
"https://gitpod.io/workspace-123",
168+
"https://gitpod.io/api/workspace",
169+
"https://gitpod.io/workspaces",
170+
"https://gitpod.io/settings",
171+
"https://gitpod.io/any-arbitrary-path",
172+
];
173+
174+
pathsAllowedInLoginOnly.forEach((url) => {
175+
expect(validateLoginReturnToUrl(url, hostUrl)).to.equal(true, `Login should allow: ${url}`);
176+
expect(validateAuthorizeReturnToUrl(url, hostUrl)).to.equal(false, `Authorize should reject: ${url}`);
177+
});
178+
});
179+
180+
it("should reject invalid URLs", () => {
181+
const invalidUrls = [
182+
"not-a-url",
183+
"javascript:alert('xss')",
184+
"data:text/html,<script>alert('xss')</script>",
185+
"",
186+
"//evil.com/workspaces",
187+
"ftp://gitpod.io/workspaces",
188+
];
189+
190+
invalidUrls.forEach((url) => {
191+
expect(validateLoginReturnToUrl(url, hostUrl)).to.equal(false, `Login should reject: ${url}`);
192+
expect(validateAuthorizeReturnToUrl(url, hostUrl)).to.equal(false, `Authorize should reject: ${url}`);
193+
});
194+
});
195+
196+
it("should work with different host configurations", () => {
197+
const previewHostUrl = new GitpodHostUrl("https://preview.gitpod-dev.com");
198+
199+
// Login should accept any path on same origin
200+
const validLoginUrls = [
201+
"https://preview.gitpod-dev.com/workspaces",
202+
"https://preview.gitpod-dev.com/complete-auth?message=success",
203+
"https://preview.gitpod-dev.com/any-path",
204+
];
205+
206+
validLoginUrls.forEach((url) => {
207+
expect(validateLoginReturnToUrl(url, previewHostUrl)).to.equal(true, `Login should accept: ${url}`);
208+
});
209+
210+
// Authorize should only accept allowlisted paths
211+
const validAuthorizeUrls = [
212+
"https://preview.gitpod-dev.com/", // Root
213+
"https://preview.gitpod-dev.com/new", // Create workspace page
214+
"https://preview.gitpod-dev.com/quickstart", // Quickstart page
215+
"https://preview.gitpod-dev.com/complete-auth?message=success",
216+
];
217+
218+
validAuthorizeUrls.forEach((url) => {
219+
expect(validateAuthorizeReturnToUrl(url, previewHostUrl)).to.equal(
220+
true,
221+
`Authorize should accept: ${url}`,
222+
);
223+
});
224+
225+
// Should reject /workspaces for authorize since it's not in allowlist
226+
expect(validateAuthorizeReturnToUrl("https://preview.gitpod-dev.com/workspaces", previewHostUrl)).to.equal(
227+
false,
228+
);
229+
230+
// Should reject URLs for different hosts
231+
expect(validateLoginReturnToUrl("https://gitpod.io/workspaces", previewHostUrl)).to.equal(false);
232+
expect(validateAuthorizeReturnToUrl("https://gitpod.io/workspaces", previewHostUrl)).to.equal(false);
233+
});
234+
235+
it("should validate www.gitpod.io specifically", () => {
236+
// Test with production gitpod.io
237+
const prodHostUrl = new GitpodHostUrl("https://gitpod.io");
238+
expect(validateLoginReturnToUrl("https://www.gitpod.io/", prodHostUrl)).to.equal(true);
239+
expect(validateAuthorizeReturnToUrl("https://www.gitpod.io/", prodHostUrl)).to.equal(true);
240+
241+
// Test with different deployment - www.gitpod.io should still work as it's hardcoded
242+
const customHostUrl = new GitpodHostUrl("https://gitpod.example.com");
243+
expect(validateLoginReturnToUrl("https://www.gitpod.io/", customHostUrl)).to.equal(true);
244+
expect(validateAuthorizeReturnToUrl("https://www.gitpod.io/", customHostUrl)).to.equal(true);
245+
246+
// Test that other www subdomains don't work
247+
expect(validateLoginReturnToUrl("https://www.gitpod.example.com/", prodHostUrl)).to.equal(false);
248+
expect(validateAuthorizeReturnToUrl("https://www.gitpod.example.com/", prodHostUrl)).to.equal(false);
249+
250+
// Test domain injection prevention
251+
expect(validateLoginReturnToUrl("https://www.gitpod.io.evil.com/", prodHostUrl)).to.equal(false);
252+
expect(validateAuthorizeReturnToUrl("https://www.gitpod.io.evil.com/", prodHostUrl)).to.equal(false);
253+
});
254+
});
255+
});

0 commit comments

Comments
 (0)