Skip to content

Commit 4c46ee0

Browse files
os-zhuangclaude
andauthored
refactor(security): migrate handleSecurity admin gate to shouldDenyAnonymous (#2567 follow-up) (#2997)
The /security/suggested-bindings admin surface was the last HTTP seam still hand-rolling the anonymous check. It now delegates to the shared shouldDenyAnonymous decision with requireAuth:true hardcoded, preserving its UNCONDITIONAL semantics (an admin surface denies anonymous even on a requireAuth:false deployment — pinned by the existing test that constructs the dispatcher with no options and still expects 401). The 401 body adopts the shared shape (code: 'unauthenticated'), asserted in the test. Deliberately NOT migrated: handleNotification's !userId check — a "needs a user identity" predicate (inbox is keyed by userId; a system context has no inbox), not an anonymous-posture decision. Verified: http-dispatcher + http-dispatcher.requireauth suites 185/185, lint clean, check-single-authz-resolver intact. Claude-Session: https://claude.ai/code/session_01ShknNUYoSTspJLBiqorD8V Co-authored-by: Claude <noreply@anthropic.com>
1 parent 23925e9 commit 4c46ee0

3 files changed

Lines changed: 32 additions & 2 deletions

File tree

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
---
2+
'@objectstack/runtime': patch
3+
---
4+
5+
refactor(security): migrate the handleSecurity admin gate to shouldDenyAnonymous (#2567 follow-up)
6+
7+
The dispatcher's `/security/suggested-bindings` admin surface was the last HTTP
8+
seam still hand-rolling the `!userId && !isSystem → 401` check. It now delegates
9+
to the shared `shouldDenyAnonymous` decision like every other seam — with
10+
`requireAuth: true` hardcoded, preserving its UNCONDITIONAL semantics (an admin
11+
surface denies anonymous callers even on a `requireAuth: false` demo deployment).
12+
The 401 body adopts the shared shape (`code: 'unauthenticated'`).
13+
14+
Deliberately NOT migrated: `handleNotification`'s `!userId` check — that is a
15+
"needs a user identity" predicate (the inbox is keyed by userId; a system
16+
context has no inbox), not an anonymous-posture decision; migrating it would
17+
change semantics.

packages/runtime/src/http-dispatcher.test.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -558,10 +558,15 @@ describe('HttpDispatcher', () => {
558558

559559
it('401s an anonymous request without touching the service', async () => {
560560
const service = makeService();
561+
// NOTE: no options — requireAuth defaults false, yet the admin
562+
// surface still denies anonymous (the gate is UNCONDITIONAL,
563+
// hardcoded requireAuth:true into shouldDenyAnonymous — #2567).
561564
const d = new HttpDispatcher(secKernel(service));
562565
const result = await d.handleSecurity('/suggested-bindings', 'GET', undefined, {}, ctx());
563566
expect(result.handled).toBe(true);
564567
expect(result.response?.status).toBe(401);
568+
// Shared anonymous-deny body shape (locks the seam migration).
569+
expect(result.response?.body?.error?.details?.code).toBe('unauthenticated');
565570
expect(service.listAudienceBindingSuggestions).not.toHaveBeenCalled();
566571
});
567572

packages/runtime/src/http-dispatcher.ts

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2337,8 +2337,16 @@ export class HttpDispatcher {
23372337
}
23382338

23392339
const ec = context.executionContext;
2340-
if (!ec?.userId && !ec?.isSystem) {
2341-
return { handled: true, response: this.error('Authentication required', 401) };
2340+
// Admin surface — anonymous is denied UNCONDITIONALLY (`requireAuth:
2341+
// true` hardcoded), independent of the deployment posture: even a
2342+
// `requireAuth: false` demo must not let anonymous callers list or
2343+
// confirm audience bindings. Shares the decision + body with every
2344+
// other HTTP seam (#2567).
2345+
if (shouldDenyAnonymous({ requireAuth: true, userId: ec?.userId, isSystem: ec?.isSystem })) {
2346+
return {
2347+
handled: true,
2348+
response: this.error(ANONYMOUS_DENY_MESSAGE, ANONYMOUS_DENY_STATUS, { code: ANONYMOUS_DENY_CODE }),
2349+
};
23422350
}
23432351

23442352
const m = method.toUpperCase();

0 commit comments

Comments
 (0)