Skip to content

Commit 8072d9f

Browse files
iQQBotclaude
andcommitted
fix: validate OAuth callback origin against SCM provider domain
Update NonceService.validateOrigin to check request origin against the expected SCM provider domain instead of Gitpod's own domain. This fixes the CSRF protection logic for OAuth callbacks which legitimately come from external providers (github.com, gitlab.com, etc.). 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
1 parent b33555f commit 8072d9f

File tree

3 files changed

+46
-18
lines changed

3 files changed

+46
-18
lines changed

components/server/src/auth/authenticator.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -191,11 +191,12 @@ export class Authenticator {
191191
}
192192

193193
// Validate origin for additional CSRF protection
194-
if (!this.nonceService.validateOrigin(req)) {
194+
if (!this.nonceService.validateOrigin(req, host)) {
195195
log.error(`CSRF protection: Origin validation failed`, {
196196
url: req.url,
197197
origin: req.get("Origin"),
198198
referer: req.get("Referer"),
199+
expectedHost: host,
199200
});
200201
res.status(403).send("Invalid request");
201202
return;

components/server/src/auth/nonce-service.spec.ts

Lines changed: 38 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
import { expect } from "chai";
88
import { Container } from "inversify";
9+
import express from "express";
910
import { Config } from "../config";
1011
import { NonceService } from "./nonce-service";
1112

@@ -66,15 +67,15 @@ describe("NonceService", () => {
6667
});
6768

6869
describe("validateOrigin", () => {
69-
it("should accept requests from same origin", () => {
70+
it("should accept requests from expected SCM provider origin", () => {
7071
const req = {
7172
get: (header: string) => {
72-
if (header === "Origin") return "https://gitpod.io";
73+
if (header === "Origin") return "https://github.com";
7374
return undefined;
7475
},
75-
} as any;
76+
} as Partial<express.Request> as express.Request;
7677

77-
const isValid = nonceService.validateOrigin(req);
78+
const isValid = nonceService.validateOrigin(req, "github.com");
7879
expect(isValid).to.be.true;
7980
});
8081

@@ -84,31 +85,56 @@ describe("NonceService", () => {
8485
if (header === "Origin") return "https://evil.com";
8586
return undefined;
8687
},
87-
} as any;
88+
} as Partial<express.Request> as express.Request;
8889

89-
const isValid = nonceService.validateOrigin(req);
90+
const isValid = nonceService.validateOrigin(req, "github.com");
9091
expect(isValid).to.be.false;
9192
});
9293

9394
it("should reject requests without origin or referer", () => {
9495
const req = {
9596
get: () => undefined,
96-
} as any;
97+
} as Partial<express.Request> as express.Request;
9798

98-
const isValid = nonceService.validateOrigin(req);
99+
const isValid = nonceService.validateOrigin(req, "github.com");
99100
expect(isValid).to.be.false;
100101
});
101102

102-
it("should accept requests with valid referer", () => {
103+
it("should accept requests with valid referer from expected host", () => {
103104
const req = {
104105
get: (header: string) => {
105-
if (header === "Referer") return "https://gitpod.io/login";
106+
if (header === "Referer") return "https://gitlab.com/oauth/authorize";
106107
return undefined;
107108
},
108-
} as any;
109+
} as Partial<express.Request> as express.Request;
109110

110-
const isValid = nonceService.validateOrigin(req);
111+
const isValid = nonceService.validateOrigin(req, "gitlab.com");
111112
expect(isValid).to.be.true;
112113
});
114+
115+
it("should work with different SCM providers", () => {
116+
const testCases = [
117+
{ origin: "https://github.com", expectedHost: "github.com", shouldPass: true },
118+
{ origin: "https://gitlab.com", expectedHost: "gitlab.com", shouldPass: true },
119+
{ origin: "https://bitbucket.org", expectedHost: "bitbucket.org", shouldPass: true },
120+
{ origin: "https://github.com", expectedHost: "gitlab.com", shouldPass: false },
121+
{ origin: "https://evil.com", expectedHost: "github.com", shouldPass: false },
122+
];
123+
124+
testCases.forEach(({ origin, expectedHost, shouldPass }) => {
125+
const req = {
126+
get: (header: string) => {
127+
if (header === "Origin") return origin;
128+
return undefined;
129+
},
130+
} as Partial<express.Request> as express.Request;
131+
132+
const isValid = nonceService.validateOrigin(req, expectedHost);
133+
expect(isValid).to.equal(
134+
shouldPass,
135+
`${origin} vs ${expectedHost} should ${shouldPass ? "pass" : "fail"}`,
136+
);
137+
});
138+
});
113139
});
114140
});

components/server/src/auth/nonce-service.ts

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -79,9 +79,9 @@ export class NonceService {
7979
}
8080

8181
/**
82-
* Validates that the request origin is from a trusted domain
82+
* Validates that the request origin is from the expected SCM provider domain
8383
*/
84-
validateOrigin(req: express.Request): boolean {
84+
validateOrigin(req: express.Request, expectedHost: string): boolean {
8585
const origin = req.get("Origin");
8686
const referer = req.get("Referer");
8787

@@ -95,10 +95,11 @@ export class NonceService {
9595

9696
try {
9797
const sourceUrl = new URL(requestSource);
98-
const configUrl = new URL(this.config.hostUrl.url.toString());
9998

100-
// Validate that the request comes from the same origin as our configured host
101-
return sourceUrl.origin === configUrl.origin;
99+
// Validate that the request comes from the expected SCM provider host
100+
// expectedHost could be "github.com", "gitlab.com", etc.
101+
const expectedOrigin = `https://${expectedHost}`;
102+
return sourceUrl.origin === expectedOrigin;
102103
} catch (error) {
103104
// Invalid URL format
104105
return false;

0 commit comments

Comments
 (0)