From 92100b066514c55c536f5c8ea0138ba18dd42f87 Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Fri, 4 Sep 2026 15:12:51 +0900 Subject: [PATCH 1/4] Add Instance.localInstance GraphQL field The database models an Instance as owning zero or one LocalInstance through the nullable instances.localId foreign key, but nothing in the GraphQL schema returned a LocalInstance. The type was an orphan, reachable only through node(id:), so clients had no way to get from an Instance to its slug, expiry, or actor quota. Expose that edge as a nullable Instance.localInstance field, which resolves to the backing record for instances this deployment hosts and to null for remote ones. The name repeats the parent type rather than using the shorter "local" on purpose: in the fediverse, "local" reads as a boolean distinguishing local from remote, and a field of type LocalInstance named "local" invites that misreading. Fix the two relation names this builds on in the models package. The forward relation was named localInstances even though it is r.one, and the reverse block was keyed localInstance, which matches no table -- defineRelations keys are schema table export names, with cardinality carried by r.one/r.many -- so it was registered nowhere. Also correct three LocalInstance descriptions that had been copied from Instance and described the wrong type. Provenance: Claude Code was asked to expose the LocalInstance a given Instance owns as a GraphQL field, to choose between the names "local" and "localInstance", and to document the field. It explored the schema, relations, and Pothos setup, then proposed a plan; the contributor chose localInstance, limited the documentation work to the new field plus the incorrect descriptions, and questioned whether the singular relations.ts key was intentional, which surfaced that the key must be a table name. The assistant wrote the field, the relation fixes, and the tests. A review loop followed: Codex found that the test fixture reused one UUID for instances.id, instances.localId, and localInstances.id, so a relation joining the wrong column would still pass; the fixture now uses a distinct local-instance ID, verified by mutation testing that the test fails when the join column is changed. A second Codex round and two Claude Fable 5 rounds reported no further findings. Verified with mise run check, mise run build, and mise run test. Assisted-by: Claude Code:claude-opus-5 Assisted-by: Codex:gpt-5.6-sol --- packages/graphql/src/instance.test.ts | 91 ++++++++++++++++++++++++++- packages/graphql/src/instance.ts | 18 +++++- packages/models/src/relations.ts | 6 +- 3 files changed, 107 insertions(+), 8 deletions(-) diff --git a/packages/graphql/src/instance.test.ts b/packages/graphql/src/instance.test.ts index cd092c4..960c46c 100644 --- a/packages/graphql/src/instance.test.ts +++ b/packages/graphql/src/instance.test.ts @@ -27,11 +27,13 @@ const accepted = new Date("2026-06-24T00:00:00.000Z"); const created = new Date("2026-06-24T00:00:00.000Z"); const expires = new Date("2026-07-24T00:00:00.000Z"); const ok = 200; +const defaultMaxActors = 10; const accountId = "00000000-0000-4000-8000-000000000001"; const memberId = "00000000-0000-4000-8000-000000000002"; const pendingMemberId = "00000000-0000-4000-8000-000000000003"; const instanceId = "00000000-0000-4000-8000-000000000101"; +const localInstanceId = "00000000-0000-4000-8000-000000000103"; const duplicateRemoteInstanceId = "00000000-0000-4000-8000-000000000102"; const sessionId = "00000000-0000-4000-8000-000000000201"; const accessToken = "test-access-token"; @@ -51,6 +53,26 @@ const remoteInstanceQuery = ` } `; +const localInstanceQuery = ` + query LocalInstance($uuid: UUID!) { + accountByUuid(uuid: $uuid) { + instances { + edges { + node { + host + localInstance { + uuid + slug + expires + maxActors + } + } + } + } + } + } +`; + const instanceMembersQuery = ` query InstanceMembers($uuid: UUID!) { accountByUuid(uuid: $uuid) { @@ -249,6 +271,71 @@ describe("Mutation.createInstance", () => { }); }); +describe("Instance.localInstance", () => { + it("returns the `LocalInstance` backing a local `Instance`", async () => { + await withTestHarness(async ({ db, post }) => { + await seedInstanceMembers(db); + + const response = await post({ + query: localInstanceQuery, + variables: { uuid: accountId }, + }); + + assert.equal(response.status, ok); + assert.deepEqual(await response.json(), { + data: { + accountByUuid: { + instances: { + edges: [ + { + node: { + host: "test-instance.drfed.org", + localInstance: { + uuid: localInstanceId, + slug: "test-instance", + expires: expires.toISOString(), + maxActors: defaultMaxActors, + }, + }, + }, + ], + }, + }, + }, + }); + }); + }); + + it("returns null for a remote `Instance`", async () => { + await withTestHarness(async ({ db, post }) => { + await seedRemoteInstance(db); + + const response = await post({ + query: localInstanceQuery, + variables: { uuid: accountId }, + }); + + assert.equal(response.status, ok); + assert.deepEqual(await response.json(), { + data: { + accountByUuid: { + instances: { + edges: [ + { + node: { + host: "remote.example.com", + localInstance: null, + }, + }, + ], + }, + }, + }, + }); + }); + }); +}); + describe("Remote instance", () => { it("returns a created remote instance", async () => { await withTestHarness(async ({ db, post }) => { @@ -364,13 +451,13 @@ async function seedInstanceMembers(db: Database): Promise { }, ]); await db.insert(schema.localInstances).values({ - id: instanceId, + id: localInstanceId, slug: "test-instance", expires, }); await db.insert(schema.instances).values({ id: instanceId, - localId: instanceId, + localId: localInstanceId, created, host: "test-instance.drfed.org", }); diff --git a/packages/graphql/src/instance.ts b/packages/graphql/src/instance.ts index d5a2d05..fbe97bb 100644 --- a/packages/graphql/src/instance.ts +++ b/packages/graphql/src/instance.ts @@ -59,17 +59,19 @@ export const Instance: DrFedObjectRef = InstanceRef; const LocalInstanceRef = builder.drizzleNode("localInstances", { name: "LocalInstance", - description: "Represents an `Instance` in the DrFed platform.", + description: + "Represents a `LocalInstance`, i.e., an `Instance` hosted by this DrFed " + + "deployment.", id: { column(instance) { return instance.id; }, - description: "The unique identifier of the `Instance`.", + description: "The unique identifier of the `LocalInstance`.", }, fields: (t) => ({ uuid: t.expose("id", { type: "UUID", - description: "The UUID of the `Instance`.", + description: "The UUID of the `LocalInstance`.", }), slug: t.exposeString("slug"), expires: t.expose("expires", { @@ -82,6 +84,16 @@ const LocalInstanceRef = builder.drizzleNode("localInstances", { export const LocalInstance: DrFedObjectRef = LocalInstanceRef; +builder.drizzleObjectField(InstanceRef, "localInstance", (t) => + t.relation("localInstance", { + nullable: true, + description: + "The `LocalInstance` backing the `Instance` when it is hosted by " + + "this DrFed deployment. `null` if the `Instance` is remote, i.e., " + + "hosted by another server on the fediverse.", + }), +); + const instanceMembersConnection = drizzleConnectionHelpers( builder, "instanceMembers", diff --git a/packages/models/src/relations.ts b/packages/models/src/relations.ts index 299db4c..1b8a8f3 100644 --- a/packages/models/src/relations.ts +++ b/packages/models/src/relations.ts @@ -72,13 +72,13 @@ export const relations = defineRelations(schema, (r) => ({ accepted: { isNotNull: true }, }, }), - localInstances: r.one.localInstances({ + localInstance: r.one.localInstances({ from: r.instances.localId, to: r.localInstances.id, }), }, - localInstance: { - instances: r.one.instances({ + localInstances: { + instance: r.one.instances({ from: r.localInstances.id, to: r.instances.localId, }), From 9239725d4c2e04d41cb2f128f8e88ae350332ebd Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Fri, 4 Sep 2026 15:38:08 +0900 Subject: [PATCH 2/4] Add Query.localInstanceBySlug Instance.localInstance let a client walk from an Instance down to its local record, but there was still no way to start from a slug -- the label that appears in an instance's host name and the argument createInstance takes. The root Query offered only accountByUuid, viewer, node, and nodes, so a known slug was not a usable entry point. Add localInstanceBySlug(slug: String!), named to match the existing accountByUuid(uuid:) so the two root lookups follow one predictable pattern. It is nullable and returns null for an unknown slug; the slug column is unique, so findFirst resolves at most one row. The resolver does no slug-format validation, because a slug that could never satisfy the insert-time check constraint is simply a miss. The field requires authentication. Slugs match ^[a-z0-9-]{4,63}$ and are trivially enumerable, while LocalInstance exposes expires and maxActors. Note that this blocks enumeration through this entry point but does not make LocalInstance private: the type stays reachable unauthenticated through node(id:) and through accountByUuid, and scoping the type itself is separate work. Also expose the reverse edge LocalInstance.instance. Without it a slug lookup could not reach the host name, which would leave the new query half-useful. It is nullable: the foreign key runs the other way, so an orphaned LocalInstance is representable even though createInstance always creates the pair in one transaction. Provenance: Claude Code was asked to add a root Query field fetching a LocalInstance by slug, with documentation, and offered localInstanceBySlug over the suggested localInstance for consistency with accountByUuid. The contributor chose that name, chose to require authentication after the assistant raised slug enumeration as a concern, and accepted the assistant's proposal to add the reverse LocalInstance.instance field in the same change. The assistant wrote the field, the reverse relation, and the tests, splitting createSession out of the authenticate test helper to avoid a primary-key clash with seedInstanceMembers. The unauthenticated response shape was determined by probing the running server rather than assumed. Both behaviors were mutation-tested: removing authScopes fails the denial test, and pointing the reverse relation at the wrong join column fails the lookup test. Codex and Claude Fable 5 each reviewed the result and reported no findings. Verified with mise run check, mise run build, and mise run test. Assisted-by: Claude Code:claude-opus-5 --- packages/graphql/src/instance.test.ts | 102 ++++++++++++++++++++++++++ packages/graphql/src/instance.ts | 32 ++++++++ 2 files changed, 134 insertions(+) diff --git a/packages/graphql/src/instance.test.ts b/packages/graphql/src/instance.test.ts index 960c46c..ba8cf7c 100644 --- a/packages/graphql/src/instance.test.ts +++ b/packages/graphql/src/instance.test.ts @@ -73,6 +73,21 @@ const localInstanceQuery = ` } `; +const localInstanceBySlugQuery = ` + query LocalInstanceBySlug($slug: String!) { + localInstanceBySlug(slug: $slug) { + uuid + slug + expires + maxActors + instance { + uuid + host + } + } + } +`; + const instanceMembersQuery = ` query InstanceMembers($uuid: UUID!) { accountByUuid(uuid: $uuid) { @@ -336,6 +351,80 @@ describe("Instance.localInstance", () => { }); }); +describe("Query.localInstanceBySlug", () => { + it("returns the `LocalInstance` with the given slug", async () => { + await withTestHarness(async ({ db, post }) => { + await seedInstanceMembers(db); + const auth = await createSession(db); + + const response = await post( + { + query: localInstanceBySlugQuery, + variables: { slug: "test-instance" }, + }, + auth, + ); + + assert.equal(response.status, ok); + assert.deepEqual(await response.json(), { + data: { + localInstanceBySlug: { + uuid: localInstanceId, + slug: "test-instance", + expires: expires.toISOString(), + maxActors: defaultMaxActors, + instance: { + uuid: instanceId, + host: "test-instance.drfed.org", + }, + }, + }, + }); + }); + }); + + it("returns null when no `LocalInstance` has the slug", async () => { + await withTestHarness(async ({ db, post }) => { + await seedInstanceMembers(db); + const auth = await createSession(db); + + const response = await post( + { + query: localInstanceBySlugQuery, + variables: { slug: "no-such-instance" }, + }, + auth, + ); + + assert.equal(response.status, ok); + assert.deepEqual(await response.json(), { + data: { localInstanceBySlug: null }, + }); + }); + }); + + it("denies the field to an unauthenticated viewer", async () => { + await withTestHarness(async ({ db, post }) => { + await seedInstanceMembers(db); + + const response = await post({ + query: localInstanceBySlugQuery, + variables: { slug: "test-instance" }, + }); + + assert.equal(response.status, ok); + const body = await response.json(); + assert.equal(body.data.localInstanceBySlug, null); + assert.equal(body.errors.length, 1); + assert.equal( + body.errors[0].message, + "Not authorized to resolve Query.localInstanceBySlug", + ); + assert.deepEqual(body.errors[0].path, ["localInstanceBySlug"]); + }); + }); +}); + describe("Remote instance", () => { it("returns a created remote instance", async () => { await withTestHarness(async ({ db, post }) => { @@ -405,6 +494,19 @@ async function authenticate( ...(maxInstances == null ? {} : { maxInstances }), created, }); + return createSession(db); +} + +/** + * Opens an authenticated session for the account seeded as {@link accountId}, + * then returns the request options carrying the session's bearer token. Use + * this instead of {@link authenticate} when the account already exists, e.g. + * after {@link seedInstanceMembers}. + * + * @param db The database to seed. + * @returns Request options with an `Authorization` header for {@link post}. + */ +async function createSession(db: Database): Promise { await db.insert(schema.sessions).values({ id: sessionId, accountId, diff --git a/packages/graphql/src/instance.ts b/packages/graphql/src/instance.ts index fbe97bb..06bfe7b 100644 --- a/packages/graphql/src/instance.ts +++ b/packages/graphql/src/instance.ts @@ -94,6 +94,15 @@ builder.drizzleObjectField(InstanceRef, "localInstance", (t) => }), ); +builder.drizzleObjectField(LocalInstanceRef, "instance", (t) => + t.relation("instance", { + nullable: true, + description: + "The `Instance` this `LocalInstance` backs, which carries the " + + "federation-facing data such as the host name.", + }), +); + const instanceMembersConnection = drizzleConnectionHelpers( builder, "instanceMembers", @@ -191,6 +200,29 @@ builder.drizzleObjectField(InstanceRef, "members", (t) => ), ); +builder.queryFields((t) => ({ + localInstanceBySlug: t.drizzleField({ + type: LocalInstanceRef, + nullable: true, + description: + "Get a `LocalInstance` by its slug. Returns `null` if no " + + "`LocalInstance` has the given slug.", + authScopes: { authenticated: true }, + args: { + slug: t.arg({ + type: "String", + required: true, + description: + "The slug of the `LocalInstance` to retrieve, i.e., the label " + + "that forms the first part of its host name.", + }), + }, + resolve(query, _root, { slug }, ctx) { + return ctx.db.query.localInstances.findFirst(query({ where: { slug } })); + }, + }), +})); + export const CreateInstanceErrorType = builder.enumType( "CreateInstanceErrorType", { From fd7e88edc226b53b0e3eca352d8065a7c5477cd9 Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Fri, 4 Sep 2026 17:36:11 +0900 Subject: [PATCH 3/4] Restrict LocalInstance to instance members LocalInstance carries operational details -- the expiry date and the actor quota -- but any caller could read it. The type was reachable through node(id:), through nodes(ids:), and through Instance. localInstance off the unauthenticated accountByUuid query, so those details were effectively public. Scope the type to the people it concerns: accepted members of the Instance it backs, plus site administrators. Membership is a new parameterized auth scope, localInstanceMember, taking the LocalInstance UUID; the type combines it with the existing admin scope under $any. Pothos caches scope results per request by scope name and parameter, so a LocalInstance appearing repeatedly in one response costs one query. The membership predicate uses an explicit join rather than the instances.instanceMembers relation. That relation carries a baked-in accepted IS NOT NULL filter, and it is not obvious whether a query-time where clause merges with that filter or replaces it. If it replaced it, invited-but-not-accepted members would silently pass, so the predicate states all three conditions itself. Set runScopesOnType so the check runs on the type rather than only on each field. Without it a selection touching no scoped field still resolves, and node(id:) { __typename } would answer "LocalInstance" to a viewer who may not read one. This closes the data channel only: the presence of the authorization error still reveals that an ID resolves to an existing LocalInstance, since an unknown ID yields a plain null with no error. That residual signal is accepted, as it is for slugs, and it additionally requires already knowing a UUID. The field-level authenticated scope on Query.localInstanceBySlug stays. It runs before the resolver, so an unauthenticated caller gets the same response whether or not the slug exists; relying on the type scope alone there would open an unauthenticated existence oracle over slugs, which are guessable in a way UUIDs are not. Provenance: the contributor asked for authentication on LocalInstance, and Claude Code first implemented a plain authenticated scope; the contributor then reconsidered and asked for a membership check via a dedicated auth scope, pointing at the Pothos scope-auth documentation, and chose to let site administrators through and to accept the existence oracle for logged-in non-members. The assistant read the plugin's type definitions and the documentation, wrote the scope, the predicate, and the tests, and determined every unauthorized response shape by probing a running server rather than assuming it. Each branch of the policy was mutation-tested: dropping the accepted filter, the admin branch, the membership branch, or the localId join each fails a specific test. Codex reviewed twice and found three coverage gaps -- no nodes(ids:) or localInstanceBySlug test for a logged-in non-member, an allow case that could not distinguish an accepted member from an instance admin, and a non-member case that could not detect losing the localId join -- all three fixed and mutation-verified. Claude Fable 5 reviewed twice and found that a comment overclaimed what runScopesOnType prevents; the comment now states what is actually true. Verified with mise run check, mise run build, and mise run test. Assisted-by: Claude Code:claude-opus-5 Assisted-by: Codex:gpt-5.6-sol Assisted-by: Claude Code:claude-fable-5 --- packages/graphql/src/builder.ts | 49 +++- packages/graphql/src/instance.test.ts | 389 +++++++++++++++++++++++++- packages/graphql/src/instance.ts | 21 +- 3 files changed, 444 insertions(+), 15 deletions(-) diff --git a/packages/graphql/src/builder.ts b/packages/graphql/src/builder.ts index d6925cc..1d940e0 100644 --- a/packages/graphql/src/builder.ts +++ b/packages/graphql/src/builder.ts @@ -15,7 +15,12 @@ // along with this program. If not, see . import { type Database, normalizeEmail, relations } from "@drfed/models"; -import type { Account, Session } from "@drfed/models/schema"; +import { + type Account, + type Session, + instanceMembers, + instances, +} from "@drfed/models/schema"; import { Template } from "@fedify/uri-template"; import SchemaBuilder, { type ObjectRef } from "@pothos/core"; import DrizzlePlugin from "@pothos/plugin-drizzle"; @@ -24,6 +29,7 @@ import RelayPlugin from "@pothos/plugin-relay"; import ScopeAuthPlugin from "@pothos/plugin-scope-auth"; import type { Transport } from "@upyo/core"; import { getTableConfig } from "drizzle-orm/pg-core"; +import { and, eq, isNotNull } from "drizzle-orm/sql/expressions"; import { DateTimeResolver, UUIDResolver } from "graphql-scalars"; /** @@ -104,6 +110,11 @@ export interface SchemaTypes { AuthScopes: { authenticated: boolean; admin: boolean; + /** + * Whether the viewer is an accepted member of the `Instance` backed by + * the `LocalInstance` with the given UUID. + */ + localInstanceMember: string; }; } @@ -136,11 +147,47 @@ export const builder = new SchemaBuilder({ return { authenticated: Boolean(context.session), admin: Boolean(context.account?.admin), + localInstanceMember(localInstanceId) { + return isLocalInstanceMember(context, localInstanceId); + }, }; }, }, }); +/** + * Determines whether the viewer is an accepted member of the `Instance` that + * the given `LocalInstance` backs. Pending members, i.e. those who have been + * invited but have not accepted yet, do not count. + * + * Pothos caches scope results per request by scope name and parameter, so this + * runs at most once per `LocalInstance` per request. + * + * @param context The request context, whose `account` is the viewer. + * @param localInstanceId The UUID of the `LocalInstance` to check. + * @returns Whether the viewer is an accepted member. + */ +async function isLocalInstanceMember( + context: UserContext, + localInstanceId: string, +): Promise { + const { account } = context; + if (account == null) return false; + const rows = await context.db + .select({ instanceId: instanceMembers.instanceId }) + .from(instanceMembers) + .innerJoin(instances, eq(instanceMembers.instanceId, instances.id)) + .where( + and( + eq(instances.localId, localInstanceId), + eq(instanceMembers.accountId, account.id), + isNotNull(instanceMembers.accepted), + ), + ) + .limit(1); + return rows.length > 0; +} + builder.addScalarType("DateTime", DateTimeResolver); builder.scalarType("Email", { diff --git a/packages/graphql/src/instance.test.ts b/packages/graphql/src/instance.test.ts index ba8cf7c..2d14423 100644 --- a/packages/graphql/src/instance.test.ts +++ b/packages/graphql/src/instance.test.ts @@ -34,9 +34,16 @@ const memberId = "00000000-0000-4000-8000-000000000002"; const pendingMemberId = "00000000-0000-4000-8000-000000000003"; const instanceId = "00000000-0000-4000-8000-000000000101"; const localInstanceId = "00000000-0000-4000-8000-000000000103"; +const otherInstanceId = "00000000-0000-4000-8000-000000000104"; +const otherLocalInstanceId = "00000000-0000-4000-8000-000000000105"; +const localInstanceNodeId = btoa(`LocalInstance:${localInstanceId}`); const duplicateRemoteInstanceId = "00000000-0000-4000-8000-000000000102"; const sessionId = "00000000-0000-4000-8000-000000000201"; +const otherSessionId = "00000000-0000-4000-8000-000000000202"; +const strangerId = "00000000-0000-4000-8000-000000000004"; +const siteAdminId = "00000000-0000-4000-8000-000000000005"; const accessToken = "test-access-token"; +const otherAccessToken = "other-access-token"; const remoteInstanceQuery = ` query RemoteInstance($uuid: UUID!) { @@ -88,6 +95,34 @@ const localInstanceBySlugQuery = ` } `; +const localInstanceNodeQuery = ` + query LocalInstanceNode($id: ID!) { + node(id: $id) { + __typename + ... on LocalInstance { + uuid + slug + } + } + } +`; + +const localInstanceNodesQuery = ` + query LocalInstanceNodes($ids: [ID!]!) { + nodes(ids: $ids) { + __typename + } + } +`; + +const localInstanceTypenameQuery = ` + query LocalInstanceTypename($id: ID!) { + node(id: $id) { + __typename + } + } +`; + const instanceMembersQuery = ` query InstanceMembers($uuid: UUID!) { accountByUuid(uuid: $uuid) { @@ -290,11 +325,12 @@ describe("Instance.localInstance", () => { it("returns the `LocalInstance` backing a local `Instance`", async () => { await withTestHarness(async ({ db, post }) => { await seedInstanceMembers(db); + const auth = await createSession(db); - const response = await post({ - query: localInstanceQuery, - variables: { uuid: accountId }, - }); + const response = await post( + { query: localInstanceQuery, variables: { uuid: accountId } }, + auth, + ); assert.equal(response.status, ok); assert.deepEqual(await response.json(), { @@ -425,6 +461,327 @@ describe("Query.localInstanceBySlug", () => { }); }); +describe("LocalInstance authorization", () => { + it("denies `Instance.localInstance` to an unauthenticated viewer", async () => { + await withTestHarness(async ({ db, post }) => { + await seedInstanceMembers(db); + + const response = await post({ + query: localInstanceQuery, + variables: { uuid: accountId }, + }); + + assert.equal(response.status, ok); + const body = await response.json(); + const [edge] = body.data.accountByUuid.instances.edges; + // The field is nullable, so it absorbs the error and the rest of the + // `Instance` still resolves. + assert.equal(edge.node.localInstance, null); + assert.equal(edge.node.host, "test-instance.drfed.org"); + assert.equal(body.errors.length, 1); + assert.equal( + body.errors[0].message, + "Not authorized to read fields for LocalInstance", + ); + }); + }); + + it("denies `node(id:)` on a `LocalInstance` to an unauthenticated viewer", async () => { + await withTestHarness(async ({ db, post }) => { + await seedInstanceMembers(db); + + const response = await post({ + query: localInstanceNodeQuery, + variables: { id: localInstanceNodeId }, + }); + + assert.equal(response.status, ok); + const body = await response.json(); + assert.equal(body.data.node, null); + assert.equal(body.errors.length, 1); + assert.equal( + body.errors[0].message, + "Not authorized to read fields for LocalInstance", + ); + }); + }); + + it("hides a `LocalInstance` from an unauthenticated `__typename` probe", async () => { + await withTestHarness(async ({ db, post }) => { + await seedInstanceMembers(db); + + const response = await post({ + query: localInstanceTypenameQuery, + variables: { id: localInstanceNodeId }, + }); + + assert.equal(response.status, ok); + const body = await response.json(); + // `runScopesOnType` is what keeps `__typename` out of the response: a + // selection of only `__typename` touches no scoped field, so without it + // `data.node.__typename` would come back as `"LocalInstance"`. The + // error below still reveals that the ID resolves to something; only the + // data channel is closed here. + assert.equal(body.data.node, null); + assert.equal(body.errors.length, 1); + assert.equal( + body.errors[0].message, + "Not authorized to read fields for LocalInstance", + ); + }); + }); + + it("denies a logged-in viewer who is not a member", async () => { + await withTestHarness(async ({ db, post }) => { + await seedInstanceMembers(db); + await db.insert(schema.accounts).values({ + id: strangerId, + email: "stranger@example.com", + name: "Stranger", + created, + }); + const auth = await createSession(db, { + id: otherSessionId, + account: strangerId, + token: otherAccessToken, + }); + + const response = await post( + { + query: localInstanceNodeQuery, + variables: { id: localInstanceNodeId }, + }, + auth, + ); + + assert.equal(response.status, ok); + const body = await response.json(); + assert.equal(body.data.node, null); + assert.equal( + body.errors[0].message, + "Not authorized to read fields for LocalInstance", + ); + }); + }); + + it("denies a member who has not accepted the invitation yet", async () => { + await withTestHarness(async ({ db, post }) => { + // `seedInstanceMembers` seeds this account with `accepted: null`. + await seedInstanceMembers(db); + const auth = await createSession(db, { + id: otherSessionId, + account: pendingMemberId, + token: otherAccessToken, + }); + + const response = await post( + { + query: localInstanceNodeQuery, + variables: { id: localInstanceNodeId }, + }, + auth, + ); + + assert.equal(response.status, ok); + const body = await response.json(); + assert.equal(body.data.node, null); + assert.equal( + body.errors[0].message, + "Not authorized to read fields for LocalInstance", + ); + }); + }); + + it("allows a site administrator who is not a member", async () => { + await withTestHarness(async ({ db, post }) => { + await seedInstanceMembers(db); + await db.insert(schema.accounts).values({ + id: siteAdminId, + email: "admin@example.com", + name: "Site Admin", + admin: true, + created, + }); + const auth = await createSession(db, { + id: otherSessionId, + account: siteAdminId, + token: otherAccessToken, + }); + + const response = await post( + { + query: localInstanceNodeQuery, + variables: { id: localInstanceNodeId }, + }, + auth, + ); + + assert.equal(response.status, ok); + assert.deepEqual(await response.json(), { + data: { + node: { + __typename: "LocalInstance", + uuid: localInstanceId, + slug: "test-instance", + }, + }, + }); + }); + }); + + it("denies `nodes(ids:)` to a logged-in non-member", async () => { + await withTestHarness(async ({ db, post }) => { + await seedInstanceMembers(db); + await db.insert(schema.accounts).values({ + id: strangerId, + email: "stranger@example.com", + name: "Stranger", + created, + }); + const auth = await createSession(db, { + id: otherSessionId, + account: strangerId, + token: otherAccessToken, + }); + + const response = await post( + { + query: localInstanceNodesQuery, + variables: { ids: [localInstanceNodeId] }, + }, + auth, + ); + + assert.equal(response.status, ok); + const body = await response.json(); + assert.deepEqual(body.data.nodes, [null]); + assert.equal( + body.errors[0].message, + "Not authorized to read fields for LocalInstance", + ); + }); + }); + + it("denies `localInstanceBySlug` to a logged-in non-member", async () => { + await withTestHarness(async ({ db, post }) => { + await seedInstanceMembers(db); + await db.insert(schema.accounts).values({ + id: strangerId, + email: "stranger@example.com", + name: "Stranger", + created, + }); + const auth = await createSession(db, { + id: otherSessionId, + account: strangerId, + token: otherAccessToken, + }); + + // The field's own `authenticated` scope passes here, so this exercises + // the type scope rather than the pre-resolver one. + const response = await post( + { + query: localInstanceBySlugQuery, + variables: { slug: "test-instance" }, + }, + auth, + ); + + assert.equal(response.status, ok); + const body = await response.json(); + assert.equal(body.data.localInstanceBySlug, null); + assert.equal( + body.errors[0].message, + "Not authorized to read fields for LocalInstance", + ); + }); + }); + + it("denies a viewer who is a member of a different `Instance`", async () => { + await withTestHarness(async ({ db, post }) => { + await seedInstanceMembers(db); + await db.insert(schema.accounts).values({ + id: strangerId, + email: "stranger@example.com", + name: "Stranger", + created, + }); + // An accepted membership somewhere else must not grant access here; this + // is what pins the `instances.localId` condition in the predicate. + await db.insert(schema.localInstances).values({ + id: otherLocalInstanceId, + slug: "other-instance", + expires, + }); + await db.insert(schema.instances).values({ + id: otherInstanceId, + localId: otherLocalInstanceId, + created, + host: "other-instance.drfed.org", + }); + await db.insert(schema.instanceMembers).values({ + accountId: strangerId, + instanceId: otherInstanceId, + accepted, + created, + }); + const auth = await createSession(db, { + id: otherSessionId, + account: strangerId, + token: otherAccessToken, + }); + + const response = await post( + { + query: localInstanceNodeQuery, + variables: { id: localInstanceNodeId }, + }, + auth, + ); + + assert.equal(response.status, ok); + const body = await response.json(); + assert.equal(body.data.node, null); + assert.equal( + body.errors[0].message, + "Not authorized to read fields for LocalInstance", + ); + }); + }); + + it("allows an accepted member of the backing `Instance`", async () => { + await withTestHarness(async ({ db, post }) => { + // `memberId` is an accepted member with `admin: false`, unlike the owner + // `accountId`, so this cannot pass by way of the per-instance admin flag. + await seedInstanceMembers(db); + const auth = await createSession(db, { + id: otherSessionId, + account: memberId, + token: otherAccessToken, + }); + + const response = await post( + { + query: localInstanceNodeQuery, + variables: { id: localInstanceNodeId }, + }, + auth, + ); + + assert.equal(response.status, ok); + assert.deepEqual(await response.json(), { + data: { + node: { + __typename: "LocalInstance", + uuid: localInstanceId, + slug: "test-instance", + }, + }, + }); + }); + }); +}); + describe("Remote instance", () => { it("returns a created remote instance", async () => { await withTestHarness(async ({ db, post }) => { @@ -498,21 +855,27 @@ async function authenticate( } /** - * Opens an authenticated session for the account seeded as {@link accountId}, - * then returns the request options carrying the session's bearer token. Use - * this instead of {@link authenticate} when the account already exists, e.g. - * after {@link seedInstanceMembers}. + * Opens an authenticated session for an already-seeded account, then returns + * the request options carrying the session's bearer token. Use this instead + * of {@link authenticate} when the account already exists, e.g. after + * {@link seedInstanceMembers}. * * @param db The database to seed. + * @param options The session id, account id, and bearer token to use. Each + * defaults to the instance owner's. * @returns Request options with an `Authorization` header for {@link post}. */ -async function createSession(db: Database): Promise { +async function createSession( + db: Database, + options: { id?: string; account?: string; token?: string } = {}, +): Promise { + const { id = sessionId, account = accountId, token = accessToken } = options; await db.insert(schema.sessions).values({ - id: sessionId, - accountId, - tokenHash: await hashSecret(accessToken), + id, + accountId: account, + tokenHash: await hashSecret(token), }); - return { headers: { authorization: `Bearer ${accessToken}` } }; + return { headers: { authorization: `Bearer ${token}` } }; } /** diff --git a/packages/graphql/src/instance.ts b/packages/graphql/src/instance.ts index 06bfe7b..73321d3 100644 --- a/packages/graphql/src/instance.ts +++ b/packages/graphql/src/instance.ts @@ -61,7 +61,26 @@ const LocalInstanceRef = builder.drizzleNode("localInstances", { name: "LocalInstance", description: "Represents a `LocalInstance`, i.e., an `Instance` hosted by this DrFed " + - "deployment.", + "deployment. Only accepted members of the `Instance` it backs, and " + + "site administrators, can read it, because it carries operational " + + "details such as the expiry date and the actor quota.", + authScopes(localInstance) { + return { + $any: { + admin: true, + localInstanceMember: localInstance.id, + }, + }; + }, + // Check the scopes on the type itself rather than only on each field. + // Without this, a selection that touches no scoped field still resolves, so + // `node(id: $id) { __typename }` would answer `"LocalInstance"` to a viewer + // who may not read one. Note this closes the data channel only: the + // presence of the authorization error still tells such a viewer that the ID + // resolves to an existing `LocalInstance`, since an unknown ID yields a + // plain `null` with no error. That residual signal is accepted, as it is + // for slugs, and it additionally requires already knowing a UUID. + runScopesOnType: true, id: { column(instance) { return instance.id; From 7f824a57a99625716ad52ffc140d6041302c274e Mon Sep 17 00:00:00 2001 From: Hong Minhee Date: Fri, 4 Sep 2026 19:22:43 +0900 Subject: [PATCH 4/4] Restrict private Account fields to their owner Query.accountByUuid was unauthenticated and Account.email was readable by anyone, so knowing or guessing an account UUID exposed an email address -- and through Account.instances and Instance.members, the email of every member of every instance that account belongs to. A pre-existing test demonstrated it: a request with no Authorization header received two members' addresses. Scope email, admin, and instances to the account itself and to site administrators, via a new accountSelf auth scope combined with the existing admin scope under $any. Unlike localInstanceMember, this scope is a plain comparison against the viewer's account id and issues no query. Leave uuid, name, and created public: Instance.members exists so members can see one another, and scoping the whole type would either break that outright or, if merely gated on being signed in, barely help. Add authenticated to Query.accountByUuid as well, so the cheapest enumeration entry point closes before the resolver runs. The three fields become nullable, which is a breaking schema change and is the point. Left non-null, one denied email deep in a members list nulled the entire response through null propagation -- the viewer's own name, the instance list, every member's name -- because the null bubbles up to the first nullable ancestor. This was measured against a running server, not assumed. Nullable confines the denial to the field that was denied. packages/web needed a matching null guard on viewer().instances, which the schema change forces. Granting the login response its own account required care. completeLoginChallenge resolves on a request that has no session yet, so accountSelf cannot see the viewer and the response would have lost its own email. Session.account now grants ownAccount, which the field scopes accept. This is sound because Session is returned only by completeLoginChallenge, is not a Node, and is reachable nowhere else, so the account behind a session is always the viewer's own; and the grant covers only that account's direct fields, not other accounts reachable beneath it. Provenance: after Claude Code reported, as an out-of-scope finding from the previous change's review, that member emails were readable anonymously, the contributor asked whether Account should be scoped too. The assistant found that accountByUuid is unused by the frontend and that closing it alone would not help, since node(id:) reaches Instance and its members regardless, and recommended field-level scopes over a type-level one because Account mixes public and private data. The contributor chose that approach and chose to authenticate accountByUuid. Both the null-propagation blast radius and every unauthorized response shape were determined by probing a running server. A frontend type error was caught only after re-running check:types following build, since the first run typechecked stale Relay artifacts. Each branch was mutation-tested: removing accountSelf, the admin branch, the ownAccount grant, or the field scopes each fails specific tests. Codex reviewed twice and found that no test covered the node(id:) entry path; two were added, one authenticated and one anonymous. Claude Fable 5 reviewed twice, read the installed Pothos plugin sources to confirm the grant cannot cascade past Session.account's direct fields, and found that the containment of that grant was asserted nowhere; a test now pins it. Verified with mise run build, then mise run check, then mise run test. Assisted-by: Claude Code:claude-opus-5 Assisted-by: Codex:gpt-5.6-sol Assisted-by: Claude Code:claude-fable-5 --- packages/graphql/src/account.test.ts | 262 +++++++++++++++++++- packages/graphql/src/account.ts | 40 ++- packages/graphql/src/auth.test.ts | 105 +++++++- packages/graphql/src/auth/challenge.ts | 7 +- packages/graphql/src/builder.ts | 7 + packages/graphql/src/instance.test.ts | 64 +++-- packages/web/src/routes/workspace/index.tsx | 2 +- 7 files changed, 457 insertions(+), 30 deletions(-) diff --git a/packages/graphql/src/account.test.ts b/packages/graphql/src/account.test.ts index bc00e39..ec1b03a 100644 --- a/packages/graphql/src/account.test.ts +++ b/packages/graphql/src/account.test.ts @@ -30,6 +30,12 @@ const memberId = "00000000-0000-4000-8000-000000000002"; const pendingMemberId = "00000000-0000-4000-8000-000000000003"; const acceptedInstanceId = "00000000-0000-4000-8000-000000000101"; const pendingInstanceId = "00000000-0000-4000-8000-000000000102"; +const sessionId = "00000000-0000-4000-8000-000000000201"; +const accessToken = "test-access-token"; +const otherSessionId = "00000000-0000-4000-8000-000000000202"; +const otherAccessToken = "other-access-token"; +const siteAdminId = "00000000-0000-4000-8000-000000000005"; +const memberNodeId = btoa(`Account:${memberId}`); const accountByUuidQuery = ` query Account($uuid: UUID!) { @@ -41,6 +47,37 @@ const accountByUuidQuery = ` } `; +const accountNodeQuery = ` + query AccountNode($id: ID!) { + node(id: $id) { + __typename + ... on Account { + uuid + name + email + admin + instances { + totalCount + } + } + } + } +`; + +const accountPrivateFieldsQuery = ` + query AccountPrivateFields($uuid: UUID!) { + accountByUuid(uuid: $uuid) { + uuid + name + email + admin + instances { + totalCount + } + } + } +`; + const accountInstancesQuery = ` query AccountInstances($uuid: UUID!) { accountByUuid(uuid: $uuid) { @@ -93,6 +130,180 @@ const accountInstancesResponse = { describe("accountByUuid", () => { it("returns an account by UUID", async () => { + await withTestHarness(async ({ db, post }) => { + await seedAccounts(db); + const auth = await createSession(db); + + const response = await post( + { query: accountByUuidQuery, variables: { uuid: accountId } }, + auth, + ); + + assert.equal(response.status, ok); + assert.deepEqual(await response.json(), accountByUuidResponse); + }); + }); +}); + +describe("Account field authorization", () => { + it("hides another account's private fields from a logged-in viewer", async () => { + await withTestHarness(async ({ db, post }) => { + await seedAccounts(db); + // Authenticated as `accountId`, asking about `memberId`. + const auth = await createSession(db); + + const response = await post( + { query: accountPrivateFieldsQuery, variables: { uuid: memberId } }, + auth, + ); + + assert.equal(response.status, ok); + const body = await response.json(); + // Public fields survive; each private field is individually null rather + // than nulling the whole `Account`. + assert.deepEqual(body.data.accountByUuid, { + uuid: memberId, + name: "Member", + email: null, + admin: null, + instances: null, + }); + assert.deepEqual( + body.errors.map((e: { message: string }) => e.message).sort(), + [ + "Not authorized to resolve Account.admin", + "Not authorized to resolve Account.email", + "Not authorized to resolve Account.instances", + ], + ); + }); + }); + + it("shows an account its own private fields", async () => { + await withTestHarness(async ({ db, post }) => { + await seedAccounts(db); + const auth = await createSession(db); + + const response = await post( + { query: accountPrivateFieldsQuery, variables: { uuid: accountId } }, + auth, + ); + + assert.equal(response.status, ok); + assert.deepEqual(await response.json(), { + data: { + accountByUuid: { + uuid: accountId, + name: "Owner", + email: "owner@example.com", + admin: false, + instances: { totalCount: 0 }, + }, + }, + }); + }); + }); + + it("shows a site administrator another account's private fields", async () => { + await withTestHarness(async ({ db, post }) => { + await seedAccounts(db); + await db.insert(schema.accounts).values({ + id: siteAdminId, + email: "admin@example.com", + name: "Site Admin", + admin: true, + created, + }); + const auth = await createSession(db, { + id: otherSessionId, + account: siteAdminId, + token: otherAccessToken, + }); + + const response = await post( + { query: accountPrivateFieldsQuery, variables: { uuid: memberId } }, + auth, + ); + + assert.equal(response.status, ok); + assert.deepEqual(await response.json(), { + data: { + accountByUuid: { + uuid: memberId, + name: "Member", + email: "member@example.com", + admin: false, + instances: { totalCount: 0 }, + }, + }, + }); + }); + }); + + it("hides private fields reached through `node(id:)` from another account", async () => { + await withTestHarness(async ({ db, post }) => { + await seedAccounts(db); + const auth = await createSession(db); + + const response = await post( + { query: accountNodeQuery, variables: { id: memberNodeId } }, + auth, + ); + + assert.equal(response.status, ok); + const body = await response.json(); + assert.deepEqual(body.data.node, { + __typename: "Account", + uuid: memberId, + name: "Member", + email: null, + admin: null, + instances: null, + }); + assert.deepEqual( + body.errors.map((e: { message: string }) => e.message).sort(), + [ + "Not authorized to resolve Account.admin", + "Not authorized to resolve Account.email", + "Not authorized to resolve Account.instances", + ], + ); + }); + }); + + it("hides private fields reached through `node(id:)` from an anonymous viewer", async () => { + await withTestHarness(async ({ db, post }) => { + await seedAccounts(db); + + // `node(id:)` carries no `authenticated` scope, unlike `accountByUuid`, + // so this is the entry path that the field scopes alone must hold. + const response = await post({ + query: accountNodeQuery, + variables: { id: memberNodeId }, + }); + + assert.equal(response.status, ok); + const body = await response.json(); + assert.deepEqual(body.data.node, { + __typename: "Account", + uuid: memberId, + name: "Member", + email: null, + admin: null, + instances: null, + }); + assert.deepEqual( + body.errors.map((e: { message: string }) => e.message).sort(), + [ + "Not authorized to resolve Account.admin", + "Not authorized to resolve Account.email", + "Not authorized to resolve Account.instances", + ], + ); + }); + }); + + it("denies `accountByUuid` to an unauthenticated viewer", async () => { await withTestHarness(async ({ db, post }) => { await seedAccounts(db); @@ -102,7 +313,12 @@ describe("accountByUuid", () => { }); assert.equal(response.status, ok); - assert.deepEqual(await response.json(), accountByUuidResponse); + const body = await response.json(); + assert.equal(body.data.accountByUuid, null); + assert.equal( + body.errors[0].message, + "Not authorized to resolve Query.accountByUuid", + ); }); }); }); @@ -111,11 +327,12 @@ describe("Account.instances", () => { it("returns the account's accepted instances", async () => { await withTestHarness(async ({ db, post }) => { await seedMembershipGraph(db); + const auth = await createSession(db); - const response = await post({ - query: accountInstancesQuery, - variables: { uuid: accountId }, - }); + const response = await post( + { query: accountInstancesQuery, variables: { uuid: accountId } }, + auth, + ); assert.equal(response.status, ok); assert.deepEqual(await response.json(), accountInstancesResponse); @@ -123,6 +340,41 @@ describe("Account.instances", () => { }); }); +/** + * Opens an authenticated session for an already-seeded account, then returns + * the request options carrying the session's bearer token. + * + * @param db The database to seed. + * @param options The session id, account id, and bearer token to use. Each + * defaults to {@link accountId}'s. + * @returns Request options with an `Authorization` header for {@link post}. + */ +async function createSession( + db: Database, + options: { id?: string; account?: string; token?: string } = {}, +): Promise { + const { id = sessionId, account = accountId, token = accessToken } = options; + await db.insert(schema.sessions).values({ + id, + accountId: account, + tokenHash: await hashSecret(token), + }); + return { headers: { authorization: `Bearer ${token}` } }; +} + +/** + * Computes the SHA-256 hex digest the server stores for a bearer token, + * mirroring `hashSecret` in *auth/hash.ts*. + * + * @param raw The raw access token. + * @returns The lowercase hex-encoded SHA-256 digest. + */ +async function hashSecret(raw: string): Promise { + return new Uint8Array( + await crypto.subtle.digest("SHA-256", new TextEncoder().encode(raw)), + ).toHex(); +} + async function seedAccounts(db: Database): Promise { await db.insert(schema.accounts).values([ { diff --git a/packages/graphql/src/account.ts b/packages/graphql/src/account.ts index f07b118..d6ec689 100644 --- a/packages/graphql/src/account.ts +++ b/packages/graphql/src/account.ts @@ -21,6 +21,26 @@ import builder, { type DrFedObjectRef } from "./builder.ts"; // oxlint-disable-next-line import/no-cycle import { Instance } from "./instance.ts"; +/** + * The auth scopes for `Account` fields that only the `Account` itself and + * site administrators may read. + * + * @param account The `Account` the field belongs to. + * @returns A scope map granting access to the account itself or a site + * administrator. + */ +function selfOrSiteAdmin(account: { readonly id: string }) { + return { + $any: { + admin: true, + accountSelf: account.id, + // Granted by `Session.account`, whose account is always the viewer's own + // but which is resolved on a request that carries no session yet. + $granted: "ownAccount", + }, + }; +} + const AccountRef = builder.drizzleNode("accounts", { name: "Account", description: @@ -39,13 +59,21 @@ const AccountRef = builder.drizzleNode("accounts", { }), email: t.expose("email", { type: "Email", - description: "The email address of the `Account`.", + nullable: true, + description: + "The email address of the `Account`. `null` unless the viewer is " + + "the `Account` itself or a site administrator.", + authScopes: selfOrSiteAdmin, }), name: t.exposeString("name", { description: "The display name of the `Account`.", }), admin: t.exposeBoolean("admin", { - description: "Whether the `Account` has administrator privileges.", + nullable: true, + description: + "Whether the `Account` has administrator privileges. `null` unless " + + "the viewer is the `Account` itself or a site administrator.", + authScopes: selfOrSiteAdmin, }), created: t.expose("created", { type: "DateTime", @@ -83,7 +111,12 @@ builder.drizzleObjectField(AccountRef, "instances", (t) => t.connection( { type: Instance, - description: "The `Instance`s that the `Account` belongs to.", + nullable: true, + description: + "The `Instance`s that the `Account` belongs to. `null` unless the " + + "viewer is the `Account` itself or a site administrator; an empty " + + "connection means the `Account` belongs to no `Instance`.", + authScopes: selfOrSiteAdmin, select(args, ctx, nestedSelection) { return { with: { @@ -164,6 +197,7 @@ builder.queryFields((t) => ({ }, description: "Get an `Account` by its UUID.", nullable: true, + authScopes: { authenticated: true }, resolve(query, _, { uuid }, ctx) { return ctx.db.query.accounts.findFirst(query({ where: { id: uuid } })); }, diff --git a/packages/graphql/src/auth.test.ts b/packages/graphql/src/auth.test.ts index fc5dd4f..2ae1bbd 100644 --- a/packages/graphql/src/auth.test.ts +++ b/packages/graphql/src/auth.test.ts @@ -21,12 +21,15 @@ import { deepEqual, equal, ok } from "node:assert/strict"; import { schema } from "@drfed/models"; import { describe, it } from "@logtape/testing-node/autoload"; -import { withTestHarness } from "./harness.test.ts"; +import { type TestHarness, withTestHarness } from "./harness.test.ts"; const okStatus = 200; const accountId = "00000000-0000-4000-8000-000000000001"; const email = "noreply@drfed.org"; const verifyUrl = "https://drfed.org/transports/mock?token={token}&code={code}"; +const memberId = "00000000-0000-4000-8000-000000000002"; +const memberEmail = "member@example.com"; +const instanceId = "00000000-0000-4000-8000-000000000101"; const loginMutation = ` mutation Login($email: Email!, $verifyUrl: URITemplate) { @@ -51,6 +54,31 @@ const completeLoginMutation = ` } `; +const completeLoginReachingOthersMutation = ` + mutation CompleteLoginReachingOthers($token: UUID!, $code: String!) { + completeLoginChallenge(token: $token, code: $code) { + account { + uuid + email + instances { + edges { + node { + members { + edges { + node { + uuid + email + } + } + } + } + } + } + } + } + } +`; + const viewerQuery = ` query Viewer { viewer { @@ -71,7 +99,82 @@ const revokeSessionMutation = ` const loginUrlPattern = /https:\/\/drfed\.org\/transports\/mock\?token=[0-9a-f-]+&code=[0-9a-z]+/u; +/** + * Runs the login-by-email flow far enough to obtain a challenge token and the + * one-time code mailed with it. + * + * @param post The harness request helper. + * @param mailer The harness mock mailer. + * @returns The challenge token and its one-time code. + */ +async function requestLoginCode( + post: TestHarness["post"], + mailer: TestHarness["mailer"], +): Promise<{ token: string; code: string }> { + const loginResponse = await post({ + query: loginMutation, + variables: { email, verifyUrl }, + }); + equal(loginResponse.status, okStatus); + const loginBody = await loginResponse.json(); + equal(loginBody.errors, undefined); + const { token } = loginBody.data.loginByEmail; + + const [message] = mailer.getSentMessages(); + ok(message); + const urlMatch = message.content.text?.match(loginUrlPattern); + ok(urlMatch); + const code = new URL(urlMatch[0]).searchParams.get("code"); + ok(code); + return { token, code }; +} + describe("email authentication", () => { + it("does not let the login grant reach another account's email", async () => { + await withTestHarness(async ({ db, mailer, post }) => { + await db.insert(schema.accounts).values([ + { id: accountId, email, name: "Login Test" }, + { id: memberId, email: memberEmail, name: "Fellow Member" }, + ]); + await db + .insert(schema.instances) + .values({ id: instanceId, host: "shared.example.com" }); + await db.insert(schema.instanceMembers).values([ + { accountId, instanceId, accepted: new Date() }, + { accountId: memberId, instanceId, accepted: new Date() }, + ]); + + const { token, code } = await requestLoginCode(post, mailer); + + // `Session.account` grants `ownAccount`, but the grant must cover only + // the viewer's own account -- not every account reachable underneath it. + // This request carries no Authorization header at all. + const response = await post({ + query: completeLoginReachingOthersMutation, + variables: { token, code }, + }); + + equal(response.status, okStatus); + const body = await response.json(); + const { account } = body.data.completeLoginChallenge; + equal(account.uuid, accountId); + equal(account.email, email); + + const members = account.instances.edges[0].node.members.edges; + const fellow = members.find( + (edge: { node: { uuid: string } }) => edge.node.uuid === memberId, + ); + ok(fellow); + equal(fellow.node.email, null); + ok( + body.errors.some( + (e: { message: string }) => + e.message === "Not authorized to resolve Account.email", + ), + ); + }); + }); + it("logs in, authenticates the viewer, and revokes the session", async () => { await withTestHarness(async ({ db, mailer, post }) => { await db.insert(schema.accounts).values({ diff --git a/packages/graphql/src/auth/challenge.ts b/packages/graphql/src/auth/challenge.ts index b953dbd..34638e2 100644 --- a/packages/graphql/src/auth/challenge.ts +++ b/packages/graphql/src/auth/challenge.ts @@ -36,7 +36,12 @@ const SessionRef = builder.drizzleObject("sessions", { return typeof accessToken === "string" ? accessToken : null; }, }), - account: t.relation("account"), + // The only `Session` a client can obtain is the one `completeLoginChallenge` + // just created for it, and `Session` is not a `Node`, so the account behind + // a session is always the viewer's own. Granting `ownAccount` here lets + // the login response carry the viewer's private fields even though the + // request that produced it had no session yet. + account: t.relation("account", { grantScopes: ["ownAccount"] }), created: t.expose("created", { type: "DateTime" }), expires: t.expose("expires", { type: "DateTime" }), }), diff --git a/packages/graphql/src/builder.ts b/packages/graphql/src/builder.ts index 1d940e0..d13edbf 100644 --- a/packages/graphql/src/builder.ts +++ b/packages/graphql/src/builder.ts @@ -115,6 +115,10 @@ export interface SchemaTypes { * the `LocalInstance` with the given UUID. */ localInstanceMember: string; + /** + * Whether the viewer is the `Account` with the given UUID. + */ + accountSelf: string; }; } @@ -150,6 +154,9 @@ export const builder = new SchemaBuilder({ localInstanceMember(localInstanceId) { return isLocalInstanceMember(context, localInstanceId); }, + accountSelf(accountId) { + return context.account?.id === accountId; + }, }; }, }, diff --git a/packages/graphql/src/instance.test.ts b/packages/graphql/src/instance.test.ts index 2d14423..59977d3 100644 --- a/packages/graphql/src/instance.test.ts +++ b/packages/graphql/src/instance.test.ts @@ -37,6 +37,7 @@ const localInstanceId = "00000000-0000-4000-8000-000000000103"; const otherInstanceId = "00000000-0000-4000-8000-000000000104"; const otherLocalInstanceId = "00000000-0000-4000-8000-000000000105"; const localInstanceNodeId = btoa(`LocalInstance:${localInstanceId}`); +const instanceNodeId = btoa(`Instance:${instanceId}`); const duplicateRemoteInstanceId = "00000000-0000-4000-8000-000000000102"; const sessionId = "00000000-0000-4000-8000-000000000201"; const otherSessionId = "00000000-0000-4000-8000-000000000202"; @@ -107,6 +108,19 @@ const localInstanceNodeQuery = ` } `; +const instanceNodeQuery = ` + query InstanceNode($id: ID!) { + node(id: $id) { + ... on Instance { + host + localInstance { + slug + } + } + } + } +`; + const localInstanceNodesQuery = ` query LocalInstanceNodes($ids: [ID!]!) { nodes(ids: $ids) { @@ -175,7 +189,7 @@ const instanceMembersResponse = { admin: false, node: { uuid: memberId, - email: "member@example.com", + email: null, name: "Member", }, }, @@ -193,14 +207,23 @@ describe("Instance.members", () => { it("returns the instance's accepted members", async () => { await withTestHarness(async ({ db, post }) => { await seedInstanceMembers(db); + const auth = await createSession(db); - const response = await post({ - query: instanceMembersQuery, - variables: { uuid: accountId }, - }); + const response = await post( + { query: instanceMembersQuery, variables: { uuid: accountId } }, + auth, + ); assert.equal(response.status, ok); - assert.deepEqual(await response.json(), instanceMembersResponse); + const body = await response.json(); + // The viewer reads their own email but not a fellow member's; the + // authorization error is confined to that one field. + assert.deepEqual(body.data, instanceMembersResponse.data); + assert.equal(body.errors.length, 1); + assert.equal( + body.errors[0].message, + "Not authorized to resolve Account.email", + ); }); }); }); @@ -360,11 +383,12 @@ describe("Instance.localInstance", () => { it("returns null for a remote `Instance`", async () => { await withTestHarness(async ({ db, post }) => { await seedRemoteInstance(db); + const auth = await createSession(db); - const response = await post({ - query: localInstanceQuery, - variables: { uuid: accountId }, - }); + const response = await post( + { query: localInstanceQuery, variables: { uuid: accountId } }, + auth, + ); assert.equal(response.status, ok); assert.deepEqual(await response.json(), { @@ -466,18 +490,19 @@ describe("LocalInstance authorization", () => { await withTestHarness(async ({ db, post }) => { await seedInstanceMembers(db); + // `Instance` carries no scope, so an anonymous viewer still reaches it + // through `node(id:)`; only `localInstance` is denied. const response = await post({ - query: localInstanceQuery, - variables: { uuid: accountId }, + query: instanceNodeQuery, + variables: { id: instanceNodeId }, }); assert.equal(response.status, ok); const body = await response.json(); - const [edge] = body.data.accountByUuid.instances.edges; // The field is nullable, so it absorbs the error and the rest of the // `Instance` still resolves. - assert.equal(edge.node.localInstance, null); - assert.equal(edge.node.host, "test-instance.drfed.org"); + assert.equal(body.data.node.localInstance, null); + assert.equal(body.data.node.host, "test-instance.drfed.org"); assert.equal(body.errors.length, 1); assert.equal( body.errors[0].message, @@ -786,11 +811,12 @@ describe("Remote instance", () => { it("returns a created remote instance", async () => { await withTestHarness(async ({ db, post }) => { await seedRemoteInstance(db); + const auth = await createSession(db); - const response = await post({ - query: remoteInstanceQuery, - variables: { uuid: accountId }, - }); + const response = await post( + { query: remoteInstanceQuery, variables: { uuid: accountId } }, + auth, + ); assert.equal(response.status, ok); assert.deepEqual(await response.json(), { diff --git a/packages/web/src/routes/workspace/index.tsx b/packages/web/src/routes/workspace/index.tsx index 9dc94b3..0fe150b 100644 --- a/packages/web/src/routes/workspace/index.tsx +++ b/packages/web/src/routes/workspace/index.tsx @@ -61,7 +61,7 @@ export default function WorkspacePage() {
You're not signed in.

}> {(viewer) => ( - + {(edge) => } )}