Skip to content

Commit 29308ba

Browse files
os-zhuangclaude
andauthored
fix(plugin-auth): run the D5.1 /oauth2/authorize env-access gate for a signed bearer (#8102) (#8175)
* fix(plugin-auth): run the D5.1 /oauth2/authorize env-access gate for a signed bearer (#8102) ADR-0069 D5.1's cloud-as-IdP gate resolved its subject with an inline copy of resolveActor -- line for line the same logic, in a second place. #8101 fixed one bug in the shared resolver and left the copy untouched, so the two diverged. The shared resolver learned that a bearer credential must have its signature stripped before lookup: bearer() hands clients the signed form in set-auth-token (the documented API-lane credential) and accepts it back, while session.token stores the unsigned value. The copy guarding /oauth2/authorize kept looking the signed credential up verbatim and resolved nothing. The unresolved case here is deliberately fail-open, so that miss did not deny the request -- it skipped the check entirely. An authenticated caller on the documented API lane was read as unauthenticated, and against a skip_consent client was issued an authorization code the gate would have refused. Delete the copy and call resolveActor. Two resolution sites are what let them diverge, so a second corrected copy would not have fixed the class. The fail-open default for genuinely unauthenticated callers is preserved unchanged. resolveActor also returns activeOrgId; this gate deliberately does not consume it -- the D5.1 host contract is (userId, clientId) and the control plane derives org membership from the user itself. Pinned by a dogfood gate that arms a DENYING gate and drives /oauth2/authorize over the cookie lane and both accepted bearer spellings, asserting the gate was actually invoked with the caller as its subject and the request refused rather than issued a code. The cookie and raw-token lanes are controls that pass on the broken build too; the signed-bearer lane is the pin. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PEVB6w7D7uCszR9Mw1BL73 * test(qa): assert the fixture's own discriminator claim in the #8102 pin The Lane.pinsTheDefect field was declared on every lane and read by nothing -- the same declared-but-unenforced shape this file exists to pin. Assert it: the suite now fails if a later edit makes a second lane the discriminator or drops the signed-bearer lane, either of which would leave the file green while measuring something other than #8102. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PEVB6w7D7uCszR9Mw1BL73 --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 37785ed commit 29308ba

3 files changed

Lines changed: 388 additions & 37 deletions

File tree

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
---
2+
"@objectstack/plugin-auth": patch
3+
---
4+
5+
fix(plugin-auth): the D5.1 `/oauth2/authorize` env-access gate now runs for a signed bearer credential (#8102)
6+
7+
ADR-0069 D5.1's cloud-as-IdP gate (`oidcAuthorizeGate`) is what enforces
8+
org-membership / app-assignment before the OP issues an authorization code. It
9+
resolved its subject with an **inline copy** of the shared `resolveActor`
10+
line for line the same logic, in a second place — and the two diverged the
11+
moment one of them was fixed.
12+
13+
`#8049` taught `resolveActor` that a bearer credential must have its signature
14+
stripped before lookup: `bearer()` hands clients the signed form in the
15+
`set-auth-token` response header (the documented API-lane credential) and
16+
accepts it back, while `session.token` stores the **unsigned** value. The copy
17+
guarding `/oauth2/authorize` kept looking the signed credential up verbatim and
18+
so resolved nothing.
19+
20+
**Why that is a security defect and not a lookup miss.** The unresolved case at
21+
this endpoint is deliberately **fail-open** — an anonymous caller must fall
22+
through so the OP can redirect them to log in. So an *authenticated* caller
23+
holding the signed bearer was read as unauthenticated, and the env-access check
24+
was not denied but **never evaluated at all**: the request proceeded, and
25+
against a `skip_consent` client it was issued an authorization code that the
26+
gate, had it run, would have refused. A declared control enforced for one
27+
credential spelling and silently absent for the other.
28+
29+
Impact is bounded: `oidcAuthorizeGate` is set only on the cloud control plane
30+
(unset in open editions / self-host, where there is no gate at all), and the
31+
OP's authorize endpoint is normally browser/cookie-driven — the cookie branch
32+
always normalized and was never affected.
33+
34+
**Fix.** The inline copy is deleted; the branch calls the shared
35+
`resolveActor`, so there is one resolution site instead of two. The fail-open
36+
default for genuinely unauthenticated callers is unchanged and deliberately
37+
preserved.
38+
39+
Pinned by a new dogfood gate that arms a **denying** gate and drives
40+
`/oauth2/authorize` over the cookie lane and both accepted bearer spellings,
41+
asserting on each that the gate was actually invoked with the caller as its
42+
subject and that the request was refused rather than issued a code.

packages/plugins/plugin-auth/src/auth-manager.ts

Lines changed: 22 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -1323,47 +1323,32 @@ export class AuthManager {
13231323
// (open editions / self-host) → no gate. Unauthenticated → fall
13241324
// through so the OP redirects to login; the gate runs on the
13251325
// return pass (or immediately for a bearer/cookie session).
1326+
//
1327+
// [#8102] The acting subject is resolved through the shared,
1328+
// hook-order-independent {@link resolveActor} — NOT a second inline
1329+
// copy of it. This branch used to carry its own token lookup, line
1330+
// for line the same logic, and the two diverged the moment one was
1331+
// fixed: #8101 taught `resolveActor` to strip the signature from a
1332+
// bearer credential (`session.token` stores the UNSIGNED value while
1333+
// `bearer()` hands clients the SIGNED form in `set-auth-token`), and
1334+
// the copy here kept looking the signed credential up verbatim. It
1335+
// resolved nothing — and because the unresolved case is deliberately
1336+
// fail-open, an AUTHENTICATED caller on the documented API lane was
1337+
// read as unauthenticated and this env-access check never evaluated
1338+
// at all. Two resolution sites are what let them diverge, so the fix
1339+
// is to delete one, not to correct it in place.
13261340
if (ctx?.path === '/oauth2/authorize' && this.config.oidcAuthorizeGate) {
13271341
const clientId = ctx?.query?.client_id;
13281342
if (clientId) {
1329-
let gateUserId: string | undefined;
1330-
// (a) standard resolver — handles the cookie session.
1331-
try {
1332-
const { getSessionFromCtx } = await import('better-auth/api');
1333-
const s: any = await getSessionFromCtx(ctx as any);
1334-
gateUserId = s?.user?.id ?? s?.session?.userId;
1335-
} catch { /* fall through to explicit resolution */ }
1336-
// (b) explicit token resolution — hook-order-independent. The
1337-
// bearer plugin may convert `Authorization: Bearer` to a session
1338-
// AFTER this global before-hook, so getSessionFromCtx can miss a
1339-
// bearer (or non-default cookie) request here. Resolve the token
1340-
// (bearer or the session cookie's token part) and look it up.
1341-
if (!gateUserId) {
1342-
try {
1343-
const hdr = (k: string): string =>
1344-
((ctx?.headers?.get?.(k) ?? ctx?.request?.headers?.get?.(k)) as string) || '';
1345-
let token: string | undefined;
1346-
const bm = /^Bearer\s+(.+)$/i.exec(hdr('authorization'));
1347-
if (bm?.[1]) token = bm[1].trim();
1348-
if (!token) {
1349-
const cm = /(?:^|;\s*)(?:__Secure-|__Host-)?better-auth\.session_token=([^;]+)/.exec(hdr('cookie'));
1350-
if (cm?.[1]) token = decodeURIComponent(cm[1]).split('.')[0];
1351-
}
1352-
if (token) {
1353-
const sess: any = await (ctx as any).context.adapter.findOne({
1354-
model: 'session',
1355-
where: [{ field: 'token', value: token }],
1356-
});
1357-
const exp = sess?.expiresAt ?? sess?.expires_at;
1358-
if (sess && (!exp || new Date(exp).getTime() > Date.now())) {
1359-
gateUserId = String(sess.userId ?? sess.user_id ?? '') || undefined;
1360-
}
1361-
}
1362-
} catch { /* unresolved → fall through, OP handles auth */ }
1363-
}
1364-
if (gateUserId) {
1343+
const actor = await this.resolveActor(ctx);
1344+
// Unauthenticated → fall through, per the contract above. Only an
1345+
// AUTHENTICATED subject is gated here. `resolveActor` also returns
1346+
// `activeOrgId`; this gate deliberately does not consume it — the
1347+
// D5.1 host contract is `(userId, clientId)` and the control plane
1348+
// derives org membership / app assignment from the user itself.
1349+
if (actor?.userId) {
13651350
const allowed = await this.config.oidcAuthorizeGate({
1366-
userId: gateUserId,
1351+
userId: actor.userId,
13671352
clientId: String(clientId),
13681353
});
13691354
if (!allowed) {

0 commit comments

Comments
 (0)