Skip to content

Commit c7ed635

Browse files
committed
Send JSON errors for API 403s, and test Origin check explicitly
1 parent c6a2dc5 commit c7ed635

File tree

3 files changed

+54
-7
lines changed

3 files changed

+54
-7
lines changed

custom-typings/cors-gate.d.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ declare module 'cors-gate' {
55
origin: string;
66
strict?: boolean;
77
allowSafe?: boolean;
8+
failure: (req: express.Request, res: express.Response, next: express.NextFunction) => void;
89
}
910

1011
function corsGate(options: Options): express.RequestHandler;

src/api/api-server.ts

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -64,15 +64,24 @@ export class HttpToolkitServerApi extends events.EventEmitter {
6464

6565
this.server.use(corsGate({
6666
strict: true, // MUST send an allowed origin
67-
allowSafe: false, // Even for HEAD/GET requests (should be none anyway)
68-
origin: '' // No origin - we accept *no* same-origin requests
67+
allowSafe: false, // Even for HEAD/GET requests
68+
origin: '', // No origin - we accept *no* same-origin requests
69+
70+
// Extend default failure response to add a helpful error body.
71+
failure: (_req, res, _next) => {
72+
res.statusCode = 403;
73+
res.send({ error: { message: 'Invalid CORS headers' }});
74+
}
6975
}));
7076

7177
this.server.use((req, res, next) => {
7278
if (req.path === '/' && req.method !== 'POST') {
7379
// We allow only POST to GQL, because that's all we expect for GraphQL queries,
7480
// and this helps derisk some (admittedly unlikely) XSRF possibilities.
75-
res.status(405).send('Only POST requests are supported');
81+
82+
res.status(405).send({
83+
error: { message: 'Only POST requests are supported' }
84+
});
7685

7786
// XSRF is less of a risk elsewhere, as REST GET endpoints don't do dangerous
7887
// things. Also we're enforcing Origin headers everywhere so it should be
@@ -92,7 +101,9 @@ export class HttpToolkitServerApi extends events.EventEmitter {
92101
const token = tokenMatch[1];
93102

94103
if (token !== config.authToken) {
95-
res.status(403).send('Valid token required');
104+
res.status(403).send({
105+
error: { message: 'Valid token required' }
106+
});
96107
} else {
97108
next();
98109
}

test/integration-test.spec.ts

Lines changed: 38 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,10 +42,13 @@ async function setupServerPath() {
4242
return path.join(tmpDir, 'httptoolkit-server', 'bin', 'run');
4343
}
4444

45-
const buildGraphql = (url: string) => getGraphQL(url, {
46-
asJSON: true,
45+
const buildGraphql = (
46+
url: string,
4747
// Pretend to be a browser on the real site:
48-
headers: { 'origin': 'https://app.httptoolkit.tech' }
48+
headers = { 'origin': 'https://app.httptoolkit.tech' }
49+
) => getGraphQL(url, {
50+
asJSON: true,
51+
headers
4952
});
5053

5154
describe('Integration test', function () {
@@ -152,6 +155,38 @@ describe('Integration test', function () {
152155
});
153156
});
154157

158+
it('rejects all requests with invalid origins', async () => {
159+
const graphql = buildGraphql('http://localhost:45457/', {
160+
origin: 'https://unknown.test'
161+
});
162+
163+
const restWrongOriginResponse = await fetch('http://localhost:45457/version', {
164+
headers: { 'origin': 'https://unknown.test' }
165+
});
166+
167+
const restNoOriginResponse = await fetch('http://localhost:45457/version', {
168+
headers: {}
169+
});
170+
171+
expect(restWrongOriginResponse.status).to.equal(403);
172+
expect(restNoOriginResponse.status).to.equal(403);
173+
174+
try {
175+
await graphql(`
176+
query getVersion {
177+
version
178+
}
179+
`)();
180+
expect.fail('GraphQL request with invalid origin should fail');
181+
} catch (errorResponse: any) {
182+
// GraphQL.js handles errors weirdly, and just throws the response body. Oh well,
183+
// it's good enough to test this anyway:
184+
expect(errorResponse).to.deep.equal({
185+
error: { message: 'Invalid CORS headers' }
186+
});
187+
}
188+
});
189+
155190
it('exposes the system configuration via REST', async () => {
156191
const response = await fetch('http://localhost:45457/config?proxyPort=8000', {
157192
headers: { 'origin': 'https://app.httptoolkit.tech' }

0 commit comments

Comments
 (0)