Skip to content

Commit d9453d4

Browse files
committed
Return forbidden for rejected switch origins
1 parent ffa4b3c commit d9453d4

3 files changed

Lines changed: 26 additions & 9 deletions

File tree

docs/strategy_switch_architecture_security_review.zh-CN.md

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ requireSameOrigin(request, { requireOrigin: true });
5151

5252
Impact:`/api/switch``/api/admin/config``/api/logout` 都是状态变更路径。要求浏览器 POST 带同源 `Origin`,能减少 cookie-auth endpoint 被跨站触发的空间。
5353

54-
Fix:`requireSameOrigin()` 现在支持 `requireOrigin`,缺失或跨站 Origin 都会拒绝。OAuth GET callback 不受影响。
54+
Fix:`requireSameOrigin()` 现在支持 `requireOrigin`,缺失或跨站 Origin 都会以 403 拒绝。OAuth GET callback 不受影响。
5555

5656
False positive notes:非浏览器脚本如果手动带 session cookie 调 POST,也必须提供正确 `Origin` header。
5757

@@ -95,4 +95,3 @@ Fix:主切换页删除了 `.innerHTML` 动态渲染路径,并在 [tests/stra
9595
- `node --check --input-type=module < web/strategy-switch-console/worker.js`
9696
- `python3 scripts/runtime_settings.py validate`
9797
- `python3 -m unittest discover -s tests -v`
98-

tests/strategy_switch_worker_validation.mjs

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,22 +24,33 @@ assert.doesNotThrow(() => __test.requireSameOrigin(
2424
}),
2525
{ requireOrigin: true },
2626
));
27-
assert.throws(
27+
const missingOriginError = captureError(
2828
() => __test.requireSameOrigin(new Request("https://switch.example/api/switch", { method: "POST" }), {
2929
requireOrigin: true,
3030
}),
31-
/Origin header is required/,
3231
);
33-
assert.throws(
32+
assert.match(missingOriginError.message, /Origin header is required/);
33+
assert.equal(missingOriginError.status, 403);
34+
const crossOriginError = captureError(
3435
() => __test.requireSameOrigin(
3536
new Request("https://switch.example/api/switch", {
3637
method: "POST",
3738
headers: { Origin: "https://evil.example" },
3839
}),
3940
{ requireOrigin: true },
4041
),
41-
/cross-origin request rejected/,
4242
);
43+
assert.match(crossOriginError.message, /cross-origin request rejected/);
44+
assert.equal(crossOriginError.status, 403);
45+
46+
function captureError(fn) {
47+
try {
48+
fn();
49+
} catch (error) {
50+
return error;
51+
}
52+
assert.fail("Expected function to throw");
53+
}
4354

4455
const strategyProfiles = __test.normalizeStrategyProfilesPayload(
4556
[

web/strategy-switch-console/worker.js

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -62,11 +62,18 @@ export default {
6262
if (url.pathname === "/api/switch" && request.method === "POST") return dispatchSwitch(request, env);
6363
return html(PAGE_HTML);
6464
} catch (error) {
65-
return json({ ok: false, error: error.message || "unexpected error" }, 500);
65+
return json({ ok: false, error: error.message || "unexpected error" }, error.status || 500);
6666
}
6767
},
6868
};
6969

70+
class HttpError extends Error {
71+
constructor(message, status) {
72+
super(message);
73+
this.status = status;
74+
}
75+
}
76+
7077
async function startLogin(request, env) {
7178
requireEnv(env, "GITHUB_CLIENT_ID");
7279
const url = new URL(request.url);
@@ -868,10 +875,10 @@ function cleanLabel(value, field) {
868875
function requireSameOrigin(request, options = {}) {
869876
const origin = request.headers.get("Origin");
870877
if (!origin) {
871-
if (options.requireOrigin) throw new Error("Origin header is required");
878+
if (options.requireOrigin) throw new HttpError("Origin header is required", 403);
872879
return;
873880
}
874-
if (origin !== new URL(request.url).origin) throw new Error("cross-origin request rejected");
881+
if (origin !== new URL(request.url).origin) throw new HttpError("cross-origin request rejected", 403);
875882
}
876883

877884
async function fetchGithubVariable(token, repository, scope, githubEnvironment, name) {

0 commit comments

Comments
 (0)