diff --git a/.changeset/group-import-existing-account.md b/.changeset/group-import-existing-account.md new file mode 100644 index 0000000..c8bf41c --- /dev/null +++ b/.changeset/group-import-existing-account.md @@ -0,0 +1,17 @@ +--- +'group-service': minor +--- + +You can now turn an existing account into a group, instead of always creating a brand-new one. + +**Affects:** Client app developers, Operators + +**Client app developers:** a new procedure `app.certified.group.import` is the sibling of `app.certified.group.register`. Where `register` creates a new account on the group PDS, `import` reuses an account that already exists. + +- Call it directly (like `register`, not via the proxy), with a service-auth JWT (`aud` = the group service DID, `lxm` = `app.certified.group.import`). +- Request body: `{ groupDid, appPassword, ownerDid }`. `groupDid` is the existing account's DID; `appPassword` is an app password for that account so the service can act on its behalf; `ownerDid` must match the JWT `iss` and is seeded as owner. The service resolves the account's PDS and handle from its DID document — no `handle` input. +- Response: `{ groupDid, handle }` (handle resolved from the account). +- Errors: `InvalidRequest` (missing/invalid fields or unresolvable DID), `InvalidAppPassword` (`401` — the app password is wrong/revoked or the account is not on the resolved PDS), `GroupAlreadyRegistered` (`409`). +- Unlike registered groups, the service holds **no recovery key** for an imported account, and `import` does **not** modify the account's DID document. See `docs/integration-guide.md` (Step 1b) and `docs/design/group-import.md`. + +**Operators:** `import` is served as a standard XRPC method on `/xrpc/app.certified.group.import` (service-auth, like `register`). No new environment variables. Imported groups are stored in the `groups` table with `encrypted_recovery_key` left `NULL` (the column is already nullable), so they are distinguishable from registered groups, and `PdsAgentPool` drives them via the per-group `pds_url` resolved at import time — which may differ from `GROUP_PDS_URL`. diff --git a/docs/api-reference.md b/docs/api-reference.md index 33895d6..3ff372e 100644 --- a/docs/api-reference.md +++ b/docs/api-reference.md @@ -578,6 +578,7 @@ Every audited operation produces one of the following `action` strings. Denied o | Action | Trigger | `detail` fields | | ------------------- | ----------------------------------------------------------------- | -------------------------------------- | | `group.register` | Group created via `app.certified.group.register` | `{ handle }` | +| `group.import` | Existing account imported via `app.certified.group.import` | `{ handle }` | | `member.add` | Member added via `member.add` | `{ memberDid, role }` | | `member.remove` | Member removed via `member.remove` | `{ memberDid }` | | `role.set` | Role changed via `role.set` | `{ memberDid, previousRole, newRole }` | diff --git a/docs/design/group-import.md b/docs/design/group-import.md new file mode 100644 index 0000000..3d1f555 --- /dev/null +++ b/docs/design/group-import.md @@ -0,0 +1,395 @@ +# Design: Importing an existing PDS account as a group + +Status: **Draft / proposal** + +Tracking issues: + +- [HYPER-469 — Add `app.certified.group.import` XRPC to promote an existing PDS + account into a group](https://linear.app/hypercerts/issue/HYPER-469) + (the work this doc designs) +- [HYPER-442 — Upgrade existing Bluesky accounts to certified + groups](https://linear.app/hypercerts/issue/HYPER-442) + (the product-level feature this primitive serves) +- [HYPER-440 — Account migration from normal atproto/Bluesky account to group + account](https://linear.app/hypercerts/issue/HYPER-440) + (the concrete case: Mangaroa Farms already had a Bluesky account) +- [HYPER-453 — ePDS DID cache serves genesis doc without + `#certified_group`](https://linear.app/hypercerts/issue/HYPER-453) + (why service proxying is **not currently relied upon** — clients call CGS + directly, which does not depend on the DID-document service entry) + +## Motivation + +`app.certified.group.register` **creates** a brand-new account on the group's +PDS and brings it under group-service management in one shot. But some accounts +already exist as ordinary atproto/Bluesky accounts before anyone decides to run +them as a group — Mangaroa Farms ([HYPER-440](https://linear.app/hypercerts/issue/HYPER-440)) +is the motivating case. Forcing those owners to create a _second_ account and +abandon the first is poor UX and fragments their identity and history. + +`app.certified.group.import` is the sibling primitive: take an account that +already exists and **promote** it to a group, reusing the existing DID, handle, +and repo rather than minting new ones. + +## Goals + +- An owner can bring an **existing** PDS account under group-service management + without creating a new account. +- The imported group behaves identically to a registered group for all + subsequent operations (`member.*`, `role.*`, `repo.*`, `audit.*`): same + per-group DB, same RBAC, same direct-client access path. +- Reuse `register`'s machinery wherever the two genuinely overlap (input + validation, owner verification, per-group DB seeding, audit logging, + credential storage) — **do not** duplicate it. +- Do **not** gate import on something that production does not currently + depend on. In particular, service proxying (and therefore the + `#certified_group` DID-document entry) is **not** currently in use — see + _The DID-document entry_ below — so import must not block on it. + +## Non-goals (this iteration) + +- The client-side "upgrade" UX in certified-app. Tracked under + [HYPER-442](https://linear.app/hypercerts/issue/HYPER-442). +- Adding the `#certified_group` DID-document service entry as part of `import`. + It is neither required for current operation nor possible with the + credentials `import` accepts — see _The DID-document entry_ below. +- Ownership transfer, multi-owner import, or importing an account that is + already a group. + +--- + +## The DID-document entry (and why import does not gate on it) + +For a registered group, `register` adds a `certified_group` service entry to the +group's DID document. This entry exists to support **service proxying** — a +client doing `agent.withProxy("certified_group", groupDid)` resolves that entry +to find the CGS endpoint. + +The thing to be clear about up front: **service proxying is not currently in +use, and `import` must not gate on the DID-document entry.** Two independent +reasons, below. The first is why we _need not_; the second is why we _could +not_ even if we wanted to. + +### Service proxying is not currently relied upon + +Per [HYPER-453](https://linear.app/hypercerts/issue/HYPER-453), proxied calls +break immediately after `register` because the caller's ePDS DID cache serves +the genesis document (which has only `#atproto_pds`) before the second PLC op +adding `#certified_group` has propagated. The **shipped workaround** was to stop +using service proxying and call CGS directly instead — an approach that has no +dependence on the group's `#certified_group` service entry being resolvable. + +So in production today, clients reach CGS **directly**, and nothing in the live +path depends on the group's `#certified_group` entry resolving. A group is fully +functional — `member.*`, `role.*`, `repo.*`, `audit.*` all work — without it. +The entry is forward-looking infrastructure for an eventual proxy path, not a +current operational requirement. + +It follows that **`import` must not block on the entry being present.** Gating +import on a DID-document precondition that production does not currently depend +on — and that even `register` does not reliably satisfy at call time (the very +cache race HYPER-453 describes) — would reject perfectly importable accounts for +no operational benefit. + +### CGS cannot add the entry for an imported account anyway + +Even setting aside the above, CGS _could not_ add the entry as part of `import`, +because of how `register` adds it versus what `import` is given. + +`register` controls the account **from genesis**: it calls +`com.atproto.server.createAccount` with a `recoveryKey` it generated +(`src/api/group/register.ts`), so that key lands in the new DID's **rotation key +set**. CGS holds it and can therefore sign a PLC operation itself +(`signPlcOperation`, `src/pds/plc.ts`) to add the entry. An **imported** +account's DID already exists with rotation keys CGS does not hold, so that +genesis trick is unavailable. + +The atproto-native fallback — `com.atproto.identity.requestPlcOperationSignature` +then `signPlcOperation` — requires the **`ACCESS_FULL`** auth scope on the PDS: + +```ts +// packages/pds/src/auth-scope.ts +export enum AuthScope { + Access = 'com.atproto.access', + Refresh = 'com.atproto.refresh', + AppPass = 'com.atproto.appPass', + AppPassPrivileged = 'com.atproto.appPassPrivileged', + ... +} +export const ACCESS_FULL = [AuthScope.Access] as const + +// packages/pds/src/api/com/atproto/identity/requestPlcOperationSignature.ts +scopes: ACCESS_FULL, +``` + +An **app-password** login is issued `AppPass` or `AppPassPrivileged` — **never** +`Access`. `ACCESS_FULL` contains only `AuthScope.Access`. Therefore: + +> **An app password categorically cannot trigger a DID-document update.** It +> cannot call `requestPlcOperationSignature`, cannot call `signPlcOperation`, +> and cannot otherwise mutate rotation keys or services. + +Since `import`'s premise is "the owner hands CGS an app password", CGS cannot add +the entry. This is a deliberate, correct atproto scope boundary — a delegated app +credential should not be able to rewrite an account's identity document. + +### Decision: leave the entry to the owner, out-of-band, if and when proxying matters + +`import` neither adds nor requires the `certified_group` entry. The imported +group works today via the direct-client path regardless. + +If and when service proxying becomes a real requirement (i.e. HYPER-453 is +resolved and clients move back to a proxy client), the entry is added the only +way it can be for an existing account: **by the owner, out-of-band**, using a +credential that carries `ACCESS_FULL` (the account's full password via their +PDS's email-token PLC flow, or their PDS's UI). The entry to add is the same +shape `register` writes: + +```json +"certified_group": { + "type": "CertifiedGroupService", + "endpoint": "" +} +``` + +`import` _may_ resolve the DID document and **log a non-fatal advisory** when the +entry is absent (so operators have a breadcrumb if proxying is later switched +on), but it does not fail the import. There is no `MissingServiceEntry` error. + +--- + +## The procedure + +### Lexicon: `app.certified.group.import` + +```jsonc +{ + "lexicon": 1, + "id": "app.certified.group.import", + "defs": { + "main": { + "type": "procedure", + "description": "Import an existing PDS account as a group. Stores the supplied app password so the service can act on the account's behalf, and seeds the caller as owner. (Does not modify the account's DID document.)", + "input": { + "encoding": "application/json", + "schema": { + "type": "object", + "required": ["groupDid", "appPassword", "ownerDid"], + "properties": { + "groupDid": { + "type": "string", + "format": "did", + "description": "DID of the existing account to import.", + }, + "appPassword": { + "type": "string", + "description": "An app password for the account, so the service can act on its behalf. Stored encrypted, exactly as supplied; the owner manages its lifecycle and may revoke it.", + }, + "ownerDid": { "type": "string", "format": "did" }, + }, + }, + }, + "output": { + "encoding": "application/json", + "schema": { + "type": "object", + "required": ["groupDid", "handle"], + "properties": { + "groupDid": { "type": "string", "format": "did" }, + "handle": { + "type": "string", + "description": "Handle resolved from the imported account.", + }, + }, + }, + }, + "errors": [ + { "name": "InvalidRequest" }, + { "name": "InvalidAppPassword" }, + { "name": "GroupAlreadyRegistered" }, + ], + }, + }, +} +``` + +Notes on the shape, contrasted with `register`: + +- **No `handle` input.** The account already has one; we resolve it, we don't + assign it. +- **No `email` input.** No account is being created, so there is no recovery + email to set. The owner's existing recovery arrangements are untouched. +- **No `accountPassword` output.** `register` returns the primary password it + generated for credible exit; `import` generates no such password — the owner + already holds their own credentials. (See _Credible exit_ below.) +- **`appPassword` is a required input** rather than something CGS mints. This is + the defining difference from `register`. + +### Handler: `src/api/group/import.ts` + +Flow, with explicit reuse of existing helpers: + +1. **Validate inputs.** `groupDid`, `appPassword`, `ownerDid` all present; + `ensureValidDid` on `groupDid` and `ownerDid`. (Same validation idiom as + `register`.) +2. **Verify the caller controls `ownerDid`.** Use `verifyServiceAuth`, **not** + `verifyRegistration`: the latter hardcodes + `lxm = 'app.certified.group.register'` (`REGISTER_NSID` in + `src/auth/verifier.ts`) and would reject a JWT minted for `import`. + `verifyServiceAuth` checks `aud === serviceDid`, derives `lxm` from the + request's own NSID, and applies the same lifetime + nonce-replay checks. + Require `iss === ownerDid` — this is the authority to seed an owner. +3. **Resolve the account's PDS and authenticate.** An imported account may live + on a different PDS than `config.groupPdsUrl`, so resolve its + `#atproto_pds` service endpoint from the account's DID document, via + `ctx.idResolver` (the `IdResolver` constructed in `src/index.ts` — exposed on + `AppContext` for this; see below). Then `new AtpAgent({ service: +resolvedPdsUrl })` and `agent.login({ identifier: groupDid, password: +appPassword })`. This is a PDS-local `createSession` against the host PDS + itself — no entryway involved. A failed login (bad/revoked credential, or + account not on that PDS) → `InvalidAppPassword`. Success yields the resolved + `handle`. Store `resolvedPdsUrl` in `groups.pds_url` — `PdsAgentPool` reads it + back verbatim (`src/pds/agent.ts`), so per-group PDS URLs already work. +4. **Encrypt and store.** `encrypt(appPassword, encryptionKey)` → + `groups.encrypted_app_password`. Set `groups.encrypted_recovery_key = NULL` + (column is already nullable, migration `002_recovery_key`). Insert into + `groups`; catch UNIQUE/PK constraint → `GroupAlreadyRegistered` (same + `ConflictError` mapping as `register`). +5. **Migrate the per-group DB** — `ctx.groupDbs.migrateGroup(groupDid)`. +6. **Seed the owner** — `ctx.memberIndex.add(groupRaw, groupDid, ownerDid, +'owner', ownerDid)`. Identical to `register`. +7. **Audit-log** the operation as `group.import` (new action string alongside + `group.register`). +8. **Respond** `{ groupDid, handle }`. + +Steps 4–8 are byte-for-byte the back half of `register`. The honest reuse story +is: factor that tail (encrypt-store-migrate-seed-audit) into a shared helper +both handlers call, rather than copy-pasting. Step 1 (input validation) is +shared too. The genuinely new logic is step 3 (resolve the account's PDS + log +in to an existing account) and the _absence_ of account creation and PLC +signing. Step 2 differs only in which verifier method is used +(`verifyServiceAuth` vs `register`'s `verifyRegistration`), for the NSID reason +noted above. + +**Plumbing prerequisite:** `IdResolver` is constructed in `src/index.ts` but is +not currently on `AppContext` (only `AuthVerifier` holds a private reference). +`import` needs DID resolution directly, so expose `idResolver` on `AppContext` +and wire it through `src/index.ts` and the test context helper +(`tests/helpers/mock-server.ts`). + +`import` does **not** touch the DID document — no PLC op, no service-entry +verification (see _The DID-document entry_ for why neither is needed nor +possible). It may optionally log a non-fatal advisory if the `certified_group` +entry is absent, but never fails on it. + +### App-password handling: store as supplied + +CGS stores the app password the owner provides, encrypted, exactly as given — +matching how `PdsAgentPool` already logs in (`identifier: groupDid, password: +`). We do **not** try to re-mint our own app password +via `createAppPassword`: that endpoint requires a privileged +(`AppPassPrivileged`) session, and a plain app password may not be able to mint +another, so attempting it would be fragile and is unnecessary. The owner owns +the credential's lifecycle and may revoke it; if they do, proxied calls start +failing with auth errors, which is the correct, observable consequence. + +--- + +## Credible exit for imported groups + +Registered groups get a strong credible-exit guarantee: CGS generated the +account's recovery key and stores it (`encrypted_recovery_key`), and returns the +primary `accountPassword` to the owner at registration. The owner can walk away +from the service and still control the account. + +Imported groups are **deliberately different**, and the difference must be +stated plainly to owners: + +- **CGS holds no recovery key for an imported account.** + `encrypted_recovery_key` is `NULL`. CGS never had genesis control and the app + password cannot grant key control. +- **The owner already retains their own credentials** — the full account + password and whatever recovery key/email their PDS set up when the account was + created. _That_ is their credible exit, and it is arguably stronger than the + registered-group story, because it never depended on CGS at all. +- **Exit = revoke the app password.** The owner revokes the app password they + gave CGS, severing CGS's ability to act on the account. This is an owner-side + action CGS cannot block, which is the point. (If a `certified_group` service + entry was ever added for proxying, the owner removes that too — but that is a + no-op today, since nothing is proxied.) + +So imported groups are not _worse_ off for exit; the trust model is just +inverted — control was never delegated to CGS in the first place. The +documentation and client UX must make this explicit so owners understand that, +unlike registered groups, CGS is not a custodian of their account keys. + +> This section resolves open question #2 from +> [HYPER-469](https://linear.app/hypercerts/issue/HYPER-469): nothing goes in +> `encrypted_recovery_key`; the owner's pre-existing credentials are the exit. + +--- + +## Schema impact + +**None required.** `encrypted_recovery_key` is already nullable +(`src/db/schema.ts`, added by `002_recovery_key`), and `pds_url` is already +per-group. Imported and registered groups share the `groups` table unchanged; an +imported group is simply one with `encrypted_recovery_key IS NULL`. + +(If we later want to _distinguish_ imported from registered groups for reporting +or for differing exit messaging, a nullable `origin` column would be the place — +not needed for correctness now, noted as a future option.) + +--- + +## Security considerations + +- **No privilege escalation via `import`.** `import` cannot do anything to the + DID document; the scope boundary that blocks the app password from PLC + operations is the same boundary that means importing an account grants CGS no + identity-level control over it. CGS gets exactly what an app password grants: + repo read/write and proxied XRPC, nothing more. +- **Owner verification is unchanged** from `register`: the caller must prove + control of `ownerDid` with a service-auth JWT. Supplying someone else's + `appPassword` does not let you make yourself owner of _their_ account unless + you also control the `ownerDid` you claim — and the seeded owner is whoever + the verified JWT issuer is, not an arbitrary input. +- **The app password is a stored secret.** It is encrypted at rest with the same + AES-256-GCM scheme as registered groups' app passwords + (`src/pds/credentials.ts`). Its blast radius is bounded by the app-password + scope (no identity control), and the owner can revoke it unilaterally. + +--- + +## Open questions (carried from HYPER-469) + +1. **DID-document update** — _resolved._ `import` neither adds nor requires the + `certified_group` entry: service proxying is not currently relied upon, and + CGS could not add the entry with an app password anyway (scope boundary, + proven above). Owner adds it out-of-band if and when proxying matters. See + _The DID-document entry_. +2. **Credible exit** — _resolved._ No CGS-held recovery key; the owner's own + pre-existing credentials are the exit. See _Credible exit_. +3. **App-password lifecycle** — _resolved._ Store the supplied app password + as-is; do not re-mint. Owner-managed, revocable. See _App-password handling_. + +Remaining smaller decisions: + +- Whether `import` should log a non-fatal advisory when the `certified_group` + entry is absent, or ignore the DID document entirely (leaning advisory — cheap + breadcrumb for if/when proxying is switched on). +- Whether to add a nullable `origin` column now or defer (deferred above). + +## Acceptance criteria + +(Mirrors [HYPER-469](https://linear.app/hypercerts/issue/HYPER-469).) + +- New lexicon `lexicons/app/certified/group/import.json`. +- New handler `src/api/group/import.ts`, registered in `src/api/index.ts`, + sharing the encrypt-store-migrate-seed-audit tail with `register`. +- Tests in `tests/import.test.ts`: successful import, invalid/revoked app + password, already-registered group, owner seeding, per-group PDS URL + resolution. +- Docs updated (`docs/api-reference.md`, integration guide), including the + credible-exit difference. +- Changeset added (per the `writing-changesets` skill). diff --git a/docs/integration-guide.md b/docs/integration-guide.md index cfa3b5d..51f0531 100644 --- a/docs/integration-guide.md +++ b/docs/integration-guide.md @@ -86,7 +86,51 @@ async function registerGroup(agent: AtpAgent, handle: string, ownerDid: string, - `ownerDid` — the DID of the user who will own this group. Must match the JWT's `iss` claim. They're immediately seeded as the owner. - `email` — optional recovery email for the group account. If omitted, a placeholder is generated. Providing a real email enables the forgot-password flow for credible exit. -Registration is the **only** endpoint called directly (not via proxy). All subsequent calls go through the proxy agent. +Registration (and import, below) are called **directly**, not via proxy. All subsequent calls go through the proxy agent. + +## Step 1b (alternative): Import an existing account + +If the account already exists — e.g. a Bluesky/atproto account you want to "promote" to a group rather than creating a fresh one — use `app.certified.group.import` instead of `register`. It reuses the existing DID, handle, and repo. + +```typescript +async function importGroup( + agent: AtpAgent, + groupDid: string, + appPassword: string, + ownerDid: string, +) { + const { + data: { token }, + } = await agent.com.atproto.server.getServiceAuth({ + aud: GROUP_SERVICE_DID, + lxm: 'app.certified.group.import', + }) + + const res = await fetch(`${GROUP_SERVICE}/xrpc/app.certified.group.import`, { + method: 'POST', + headers: { + 'Content-Type': 'application/json', + Authorization: `Bearer ${token}`, + }, + body: JSON.stringify({ groupDid, appPassword, ownerDid }), + }) + + if (!res.ok) throw new Error(`Import failed: ${res.status}`) + + // Response: { groupDid: "did:plc:abc123", handle: "existing.pds.example.com" } + return res.json() +} +``` + +- `groupDid` — the DID of the existing account to import. The group service resolves its PDS and handle from the DID document. +- `appPassword` — an [app password](https://bsky.app/settings/app-passwords) for that account, so the service can act on its behalf. Stored encrypted; **the owner manages its lifecycle and can revoke it at any time** to sever the service's access. +- `ownerDid` — as for `register`, must match the JWT's `iss` claim; seeded as owner. + +**How import differs from register:** + +- The account is **not** created — it already exists, and its DID/handle/repo are reused. +- The group service holds **no recovery key** for an imported account (unlike registered groups, where it generates one). The owner's own pre-existing account credentials are their credible exit; the service is not a custodian of the account's keys. +- Import does **not** modify the account's DID document. (Service proxying is not currently relied upon; and an app password cannot perform the PLC operation required to add a service entry. See `docs/design/group-import.md`.) ## Step 2: Create a proxy agent with custom lexicons @@ -445,6 +489,7 @@ All error responses follow this shape: | NSID | Type | Required role | Description | | --------------------------------------- | --------- | ------------- | ----------------------------------------------- | | `app.certified.group.register` | procedure | service auth | Register a new group (direct call, not proxied) | +| `app.certified.group.import` | procedure | service auth | Import an existing account as a group (direct) | | `app.certified.group.repo.createRecord` | procedure | member | Create a record | | `app.certified.group.repo.putRecord` | procedure | member/admin | Update or create a record | | `app.certified.group.repo.deleteRecord` | procedure | member/admin | Delete a record | diff --git a/lexicons/app/certified/group/import.json b/lexicons/app/certified/group/import.json new file mode 100644 index 0000000..f17eed4 --- /dev/null +++ b/lexicons/app/certified/group/import.json @@ -0,0 +1,48 @@ +{ + "lexicon": 1, + "id": "app.certified.group.import", + "defs": { + "main": { + "type": "procedure", + "description": "Import an existing PDS account as a group. Sibling to app.certified.group.register, but reuses an existing account rather than creating a new one. Stores the supplied app password so the service can act on the account's behalf, and seeds the caller as owner. Does not modify the account's DID document.", + "input": { + "encoding": "application/json", + "schema": { + "type": "object", + "required": ["groupDid", "appPassword", "ownerDid"], + "properties": { + "groupDid": { + "type": "string", + "format": "did", + "description": "DID of the existing account to import as a group." + }, + "appPassword": { + "type": "string", + "description": "An app password for the account, so the service can act on its behalf. Stored encrypted, exactly as supplied; the owner manages its lifecycle and may revoke it." + }, + "ownerDid": { "type": "string", "format": "did" } + } + } + }, + "output": { + "encoding": "application/json", + "schema": { + "type": "object", + "required": ["groupDid", "handle"], + "properties": { + "groupDid": { "type": "string", "format": "did" }, + "handle": { + "type": "string", + "description": "Handle resolved from the imported account." + } + } + } + }, + "errors": [ + { "name": "InvalidRequest" }, + { "name": "InvalidAppPassword" }, + { "name": "GroupAlreadyRegistered" } + ] + } + } +} diff --git a/src/api/group/finalize.ts b/src/api/group/finalize.ts new file mode 100644 index 0000000..e78a537 --- /dev/null +++ b/src/api/group/finalize.ts @@ -0,0 +1,73 @@ +import type { AppContext } from '../../context.js' +import { ConflictError } from '../../errors.js' +import { encrypt } from '../../pds/credentials.js' + +export interface FinalizeGroupParams { + /** DID of the group account (created by register, or pre-existing for import). */ + groupDid: string + /** PDS hosting the group account; stored verbatim and reused by PdsAgentPool. */ + pdsUrl: string + /** App password the service uses to act on the account's behalf (plaintext). */ + appPassword: string + /** DID seeded as the immutable owner. */ + ownerDid: string + /** + * Recovery-key material to store, already base64url-encoded, or `null` when + * the service holds no recovery key for this group (the import case). The + * `groups.encrypted_recovery_key` column is nullable. + */ + recoveryKeyMaterial: string | null + /** Audit action: distinguishes how the group entered the service. */ + action: 'group.register' | 'group.import' + /** Resolved full handle, recorded in the audit detail. */ + handle: string +} + +/** + * Shared tail of `group.register` and `group.import`: persist the group's + * credentials, initialise its per-group database, seed the owner, and audit-log + * the operation. Everything before this differs between the two (register + * creates an account and signs a PLC op; import logs in to an existing one), + * but from credential storage onward the two are identical save for the + * recovery key and the audit action. + * + * Throws `ConflictError('GroupAlreadyRegistered')` if the group DID is already + * present in the `groups` table. + */ +export async function finalizeGroup(ctx: AppContext, params: FinalizeGroupParams): Promise { + const { groupDid, pdsUrl, appPassword, ownerDid, recoveryKeyMaterial, action, handle } = params + + const encryptionKey = Buffer.from(ctx.config.encryptionKey, 'hex') + const encryptedAppPassword = encrypt(appPassword, encryptionKey) + const encryptedRecoveryKey = + recoveryKeyMaterial === null ? null : encrypt(recoveryKeyMaterial, encryptionKey) + + try { + await ctx.globalDb + .insertInto('groups') + .values({ + did: groupDid, + pds_url: pdsUrl, + encrypted_app_password: encryptedAppPassword, + encrypted_recovery_key: encryptedRecoveryKey, + }) + .execute() + } catch (err: unknown) { + const msg = err instanceof Error ? err.message : String(err) + if (msg.includes('UNIQUE constraint failed') || msg.includes('PRIMARY KEY constraint failed')) { + throw new ConflictError('Group already registered', 'GroupAlreadyRegistered') + } + throw err + } + + // Initialize per-group database and run migrations + await ctx.groupDbs.migrateGroup(groupDid) + + // Seed owner (atomic write to both group DB and member_index) + const groupDb = ctx.groupDbs.get(groupDid) + const groupRaw = ctx.groupDbs.getRaw(groupDid) + ctx.memberIndex.add(groupRaw, groupDid, ownerDid, 'owner', ownerDid) + + // Audit log the group creation/import + await ctx.audit.log(groupDb, ownerDid, action, 'permitted', { handle }) +} diff --git a/src/api/group/import.ts b/src/api/group/import.ts new file mode 100644 index 0000000..d9bde2f --- /dev/null +++ b/src/api/group/import.ts @@ -0,0 +1,100 @@ +import type { Server } from '@atproto/xrpc-server' +import { AtpAgent } from '@atproto/api' +import { ensureValidDid } from '@atproto/syntax' +import { AuthRequiredError, InvalidRequestError } from '@atproto/xrpc-server' +import type { AppContext } from '../../context.js' +import { registerServiceAuthMethod, jsonResponse } from '../util.js' +import { finalizeGroup } from './finalize.js' + +/** + * app.certified.group.import — promote an existing PDS account into a group. + * + * Sibling to group.register: where register creates a new account on the group + * PDS and signs a PLC op to advertise the certified_group service, import + * reuses an account that already exists. The caller supplies an app password so + * the service can act on the account's behalf. + * + * import deliberately does NOT touch the account's DID document: service + * proxying is not currently relied upon (clients call CGS directly), and an app + * password cannot perform PLC operations anyway (that needs the ACCESS_FULL + * scope). See docs/design/group-import.md. + * + * Auth is service-level (aud = the service DID), because the group does not yet + * exist in the service. The handler additionally verifies the authenticated + * caller matches the ownerDid it is about to seed. + */ +export default function (server: Server, ctx: AppContext) { + registerServiceAuthMethod(server, 'app.certified.group.import', ctx, { + handler: async ({ auth, input }) => { + const { callerDid } = auth.credentials + const { groupDid, appPassword, ownerDid } = input?.body as { + groupDid: string + appPassword: string + ownerDid: string + } + + // Validate inputs (the lexicon enforces presence + did format; we also + // guard explicitly so a malformed DID fails as a clean 400) + try { + ensureValidDid(groupDid) + } catch { + throw new InvalidRequestError('Invalid groupDid') + } + try { + ensureValidDid(ownerDid) + } catch { + throw new InvalidRequestError('Invalid ownerDid') + } + + // The authenticated caller must be the owner they are seeding + if (callerDid !== ownerDid) { + throw new AuthRequiredError('Service auth token issuer does not match ownerDid') + } + + // Resolve the account's PDS and handle from its DID document. An imported + // account may live on a PDS other than config.groupPdsUrl, so we use the + // account's own #atproto_pds endpoint rather than assuming a host. + let atprotoData + try { + atprotoData = await ctx.idResolver.did.resolveAtprotoData(groupDid) + } catch { + throw new InvalidRequestError(`Could not resolve DID document for ${groupDid}`) + } + const pdsUrl = atprotoData.pds + const handle = atprotoData.handle + + // Authenticate to the account's PDS with the supplied app password. This + // is a PDS-local createSession against the host PDS itself (no entryway), + // and both proves the credential works and confirms the account is there. + const agent = new AtpAgent({ service: pdsUrl }) + try { + await agent.login({ identifier: groupDid, password: appPassword }) + } catch (err) { + const e = err as { status?: number; error?: string; message?: string } + // Bad/revoked app password, or the account is not on the resolved PDS. + if (e?.status === 401 || e?.status === 400) { + throw new AuthRequiredError( + 'Could not authenticate to the account PDS with the supplied app password', + 'InvalidAppPassword', + ) + } + throw err + } + + // Persist credentials, init per-group DB, seed owner, audit-log. + // No recovery key: the service never had genesis control of this account, + // and an app password cannot grant key control (see design doc). + await finalizeGroup(ctx, { + groupDid, + pdsUrl, + appPassword, + ownerDid, + recoveryKeyMaterial: null, + action: 'group.import', + handle, + }) + + return jsonResponse({ groupDid, handle }) + }, + }) +} diff --git a/src/api/group/register.ts b/src/api/group/register.ts index c76c44c..28684b8 100644 --- a/src/api/group/register.ts +++ b/src/api/group/register.ts @@ -1,36 +1,47 @@ import { randomBytes } from 'node:crypto' -import type { Express } from 'express' +import type { Server } from '@atproto/xrpc-server' import { AtpAgent } from '@atproto/api' import { ensureValidDid } from '@atproto/syntax' import { AuthRequiredError, InvalidRequestError } from '@atproto/xrpc-server' import type { AppContext } from '../../context.js' import { ConflictError } from '../../errors.js' -import { encrypt } from '../../pds/credentials.js' +import { registerServiceAuthMethod, jsonResponse } from '../util.js' import { generateRecoveryKey, getLatestPlcCid, signPlcOperation, submitPlcOperation, } from '../../pds/plc.js' - -export default function (app: Express, ctx: AppContext) { - app.post('/xrpc/app.certified.group.register', async (req, res, next) => { - try { - const { handle, ownerDid, email } = req.body - - // Validate inputs - if (!handle || !ownerDid) { - throw new InvalidRequestError('Missing required fields: handle, ownerDid') +import { finalizeGroup } from './finalize.js' + +/** + * app.certified.group.register — create a new group account and bring it under + * service management. + * + * Auth is service-level (aud = the service DID), because the group does not yet + * exist in the service. The handler additionally verifies the authenticated + * caller matches the ownerDid it is about to seed. + */ +export default function (server: Server, ctx: AppContext) { + registerServiceAuthMethod(server, 'app.certified.group.register', ctx, { + handler: async ({ auth, input }) => { + const { callerDid } = auth.credentials + const { handle, ownerDid, email } = input?.body as { + handle: string + ownerDid: string + email?: string } + + // Validate inputs (the lexicon enforces presence + did format; we also + // guard explicitly for the handle charset and a clean DID error) try { ensureValidDid(ownerDid) } catch { throw new InvalidRequestError('Invalid ownerDid') } - // Verify the caller controls the claimed ownerDid - const { iss } = await ctx.authVerifier.verifyRegistration(req) - if (iss !== ownerDid) { + // The authenticated caller must be the owner they are seeding + if (callerDid !== ownerDid) { throw new AuthRequiredError('Service auth token issuer does not match ownerDid') } if (!/^[a-zA-Z0-9-]+$/.test(handle)) { @@ -126,54 +137,19 @@ export default function (app: Express, ctx: AppContext) { }) const appPassword = appPasswordRes.data.password - // Encrypt and store - const encryptionKey = Buffer.from(ctx.config.encryptionKey, 'hex') - const encrypted = encrypt(appPassword, encryptionKey) + // Persist credentials, init per-group DB, seed owner, audit-log const recoveryKeyBytes = await recoveryKey.export() - const encryptedRecoveryKey = encrypt( - Buffer.from(recoveryKeyBytes).toString('base64url'), - encryptionKey, - ) - try { - await ctx.globalDb - .insertInto('groups') - .values({ - did: groupDid, - pds_url: pdsUrl, - encrypted_app_password: encrypted, - encrypted_recovery_key: encryptedRecoveryKey, - }) - .execute() - } catch (err: unknown) { - const msg = err instanceof Error ? err.message : String(err) - if ( - msg.includes('UNIQUE constraint failed') || - msg.includes('PRIMARY KEY constraint failed') - ) { - throw new ConflictError('Group already registered', 'GroupAlreadyRegistered') - } - throw err - } - - // Initialize per-group database and run migrations - await ctx.groupDbs.migrateGroup(groupDid) - - // Seed owner (atomic write to both group DB and member_index) - const groupDb = ctx.groupDbs.get(groupDid) - const groupRaw = ctx.groupDbs.getRaw(groupDid) - ctx.memberIndex.add(groupRaw, groupDid, ownerDid, 'owner', ownerDid) - - // Audit log the group creation - await ctx.audit.log(groupDb, ownerDid, 'group.register', 'permitted', { - handle: fullHandle, - }) - - res.json({ + await finalizeGroup(ctx, { groupDid, + pdsUrl, + appPassword, + ownerDid, + recoveryKeyMaterial: Buffer.from(recoveryKeyBytes).toString('base64url'), + action: 'group.register', handle: fullHandle, }) - } catch (err) { - next(err) - } + + return jsonResponse({ groupDid, handle: fullHandle }) + }, }) } diff --git a/src/api/index.ts b/src/api/index.ts index 3b5c2ed..8da0156 100644 --- a/src/api/index.ts +++ b/src/api/index.ts @@ -1,5 +1,4 @@ import type { Server } from '@atproto/xrpc-server' -import type { Express } from 'express' import type { AppContext } from '../context.js' import createRecord from './repo/createRecord.js' @@ -13,6 +12,7 @@ import roleSet from './role/set.js' import auditQuery from './audit/query.js' import membershipList from './membership/list.js' import groupRegister from './group/register.js' +import groupImport from './group/import.js' export function registerXrpcMethods(server: Server, ctx: AppContext): void { createRecord(server, ctx) @@ -25,9 +25,6 @@ export function registerXrpcMethods(server: Server, ctx: AppContext): void { roleSet(server, ctx) auditQuery(server, ctx) membershipList(server, ctx) -} - -/** Routes that live outside the XRPC server (unauthenticated, non-standard). */ -export function registerRawRoutes(app: Express, ctx: AppContext): void { - groupRegister(app, ctx) + groupRegister(server, ctx) + groupImport(server, ctx) } diff --git a/src/api/util.ts b/src/api/util.ts index 97fe6df..ba3c63b 100644 --- a/src/api/util.ts +++ b/src/api/util.ts @@ -1,7 +1,7 @@ import type { Server, MethodHandler, RouteOptions } from '@atproto/xrpc-server' import type { Kysely } from 'kysely' import type { AppContext } from '../context.js' -import type { GroupAuthResult } from '../auth/verifier.js' +import type { GroupAuthResult, ServiceAuthResult } from '../auth/verifier.js' import type { AuditEventDetail } from '../audit.js' import type { Operation } from '../rbac/permissions.js' import type { GroupDatabase } from '../db/schema.js' @@ -14,6 +14,11 @@ export interface AuthedMethodConfig { handler: MethodHandler } +export interface ServiceAuthMethodConfig { + opts?: RouteOptions + handler: MethodHandler +} + export function jsonResponse(body: T) { return { encoding: 'application/json' as const, body } } @@ -92,3 +97,24 @@ export function registerAuthedMethod( handler: config.handler, }) } + +/** + * Register a group-bootstrapping XRPC method (register, import) — one whose + * audience is the service's own DID rather than a group DID, and whose target + * group does not yet exist in the service. Unlike registerAuthedMethod, the + * auth verifier does not open a per-group DB or check group membership; it only + * proves the caller controls the issuing DID. The handler is responsible for + * any ownerDid / authorship checks. + */ +export function registerServiceAuthMethod( + server: Server, + nsid: string, + ctx: AppContext, + config: ServiceAuthMethodConfig, +): void { + server.method(nsid, { + auth: ctx.authVerifier.xrpcServiceAuth(), + opts: config.opts, + handler: config.handler, + }) +} diff --git a/src/auth/verifier.ts b/src/auth/verifier.ts index 6b681ee..865ac62 100644 --- a/src/auth/verifier.ts +++ b/src/auth/verifier.ts @@ -21,8 +21,6 @@ export interface ServiceAuthCredentials { } export type ServiceAuthResult = { credentials: ServiceAuthCredentials } -const REGISTER_NSID = 'app.certified.group.register' - export class AuthVerifier { private verifyJwtFn: typeof defaultVerifyJwt private parseReqNsidFn: typeof defaultParseReqNsid @@ -92,41 +90,6 @@ export class AuthVerifier { return { iss: payload.iss, aud: payload.aud } } - /** - * Verify a service auth JWT for the registration endpoint. - * Proves the caller controls the claimed DID by checking the JWT signature - * against their DID document's signing key. Audience must be this service's DID. - */ - async verifyRegistration(req: Request): Promise<{ iss: string }> { - const authHeader = req.headers.authorization - if (!authHeader?.startsWith('Bearer ')) { - throw new AuthRequiredError('Missing auth token') - } - const jwtStr = authHeader.slice(7) - - const payload = await this.verifyJwtFn( - jwtStr, - this.serviceDid, - REGISTER_NSID, - async (did: string, forceRefresh: boolean): Promise => { - const atprotoData = await this.idResolver.did.resolveAtprotoData(did, forceRefresh) - return atprotoData.signingKey - }, - ) - - this.assertTokenLifetime(payload) - - if (!payload.jti) { - throw new AuthRequiredError('Missing jti in service auth token') - } - const isNew = await this.nonceCache.checkAndStore(payload.jti) - if (!isNew) { - throw new AuthRequiredError('Replayed token') - } - - return { iss: payload.iss } - } - xrpcAuth(): MethodAuthVerifier { return async ({ req }) => { const { iss, aud } = await this.verify(req) diff --git a/src/context.ts b/src/context.ts index a575195..334b031 100644 --- a/src/context.ts +++ b/src/context.ts @@ -1,5 +1,6 @@ import type { Kysely } from 'kysely' import type { Logger } from 'pino' +import type { IdResolver } from '@atproto/identity' import type { Config } from './config.js' import type { GlobalDatabase } from './db/schema.js' import type { GroupDbPool } from './db/group-db-pool.js' @@ -15,6 +16,7 @@ export interface AppContext { globalDbPath: string groupDbs: GroupDbPool authVerifier: AuthVerifier + idResolver: IdResolver rbac: RbacChecker pdsAgents: PdsAgentPool audit: AuditLogger diff --git a/src/index.ts b/src/index.ts index 2535d61..9a35b00 100644 --- a/src/index.ts +++ b/src/index.ts @@ -11,7 +11,7 @@ import { loadConfig } from './config.js' import { AuthVerifier } from './auth/verifier.js' import { NonceCache } from './auth/nonce.js' import { RbacChecker } from './rbac/check.js' -import { registerXrpcMethods, registerRawRoutes } from './api/index.js' +import { registerXrpcMethods } from './api/index.js' import { createFallbackErrorHandler } from './api/error-handler.js' import { runGlobalMigrations } from './db/migrate.js' import { openSqliteDb } from './db/sqlite.js' @@ -76,6 +76,7 @@ async function main() { globalDbPath, groupDbs, authVerifier, + idResolver, rbac, pdsAgents, audit, @@ -93,17 +94,14 @@ async function main() { } }) - // group.register needs JSON parsing (outside XRPC server) - app.use('/xrpc/app.certified.group.register', express.json({ limit: '1mb' })) - registerRawRoutes(app, ctx) - - // XRPC server — handles all other /xrpc/* routes + // XRPC server — handles all /xrpc/* routes, including group.register and + // group.import (service-auth methods) and per-group methods const __dirname = dirname(fileURLToPath(import.meta.url)) const xrpcServer = createGroupServer(join(__dirname, '..', 'lexicons')) registerXrpcMethods(xrpcServer, ctx) app.use(xrpcServer.router) - // Fallback error handler for non-XRPC routes (group.register) + // Fallback error handler for any non-XRPC routes (e.g. /health) app.use(createFallbackErrorHandler(logger)) const server = app.listen(config.port, () => { diff --git a/tests/helpers/mock-server.ts b/tests/helpers/mock-server.ts index 5394da4..1858b3c 100644 --- a/tests/helpers/mock-server.ts +++ b/tests/helpers/mock-server.ts @@ -94,6 +94,7 @@ export async function createTestContext(overrides?: Partial): Promis globalDbPath: ':memory:', groupDbs: mockGroupDbs as any, authVerifier: mockAuth('did:plc:testuser'), + idResolver: mockIdResolver(), rbac: new RbacChecker(), pdsAgents: mockPdsAgents as any, audit: new AuditLogger(), @@ -199,7 +200,6 @@ export function createTestApp( export function mockAuth(iss: string, aud: string = 'did:plc:testgroup') { return { verify: async () => ({ iss, aud }), - verifyRegistration: async () => ({ iss }), verifyServiceAuth: async () => ({ iss }), xrpcAuth() { return async ({ req }: { req: any }) => { @@ -208,9 +208,29 @@ export function mockAuth(iss: string, aud: string = 'did:plc:testgroup') { } }, xrpcServiceAuth() { - return async () => { + return async ({ req }: { req: any }) => { + const { iss } = await this.verifyServiceAuth(req) return { credentials: { callerDid: iss } } } }, } as any } + +/** + * Minimal IdResolver mock. By default resolves any DID to atproto data whose + * `pds` points at the test PDS, so the group.import success path works out of + * the box. Tests override `did.resolveAtprotoData` to exercise other cases + * (e.g. a different PDS, or a throw for an unresolvable DID). + */ +export function mockIdResolver(pdsUrl = 'https://pds.example.com') { + return { + did: { + resolveAtprotoData: async (did: string) => ({ + did, + signingKey: 'did:key:zQ3shmockSigningKey', + handle: 'imported.pds.example.com', + pds: pdsUrl, + }), + }, + } as any +} diff --git a/tests/import.test.ts b/tests/import.test.ts new file mode 100644 index 0000000..afa5f2b --- /dev/null +++ b/tests/import.test.ts @@ -0,0 +1,206 @@ +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest' +import express from 'express' +import request from 'supertest' +import { AuthRequiredError } from '@atproto/xrpc-server' +import { + createTestContext, + createTestApp, + mockAuth, + mockIdResolver, +} from './helpers/mock-server.js' +import groupImportHandler from '../src/api/group/import.js' +import type { AppContext } from '../src/context.js' +import type { Kysely } from 'kysely' +import type { GlobalDatabase, GroupDatabase } from '../src/db/schema.js' + +// Mock AtpAgent — import only logs in (no account creation). Default: success. +vi.mock('@atproto/api', () => { + return { + AtpAgent: vi.fn().mockImplementation(() => ({ + login: vi.fn().mockResolvedValue(undefined), + })), + } +}) + +import { AtpAgent } from '@atproto/api' + +const ENDPOINT = '/xrpc/app.certified.group.import' + +const validBody = { + groupDid: 'did:plc:existingaccount', + appPassword: 'abcd-efgh-ijkl-mnop', + ownerDid: 'did:plc:owner', +} + +describe('group.import', () => { + let ctx: AppContext + let globalDb: Kysely + let groupDb: Kysely + let app: express.Express + + beforeEach(async () => { + vi.clearAllMocks() + const test = await createTestContext({ + // Owner is did:plc:owner; idResolver resolves to the test PDS by default. + authVerifier: mockAuth('did:plc:owner'), + }) + ctx = test.ctx + globalDb = test.globalDb + groupDb = test.groupDb + app = createTestApp(ctx, groupImportHandler) + }) + + afterEach(async () => { + await globalDb.destroy() + await groupDb.destroy() + }) + + it('imports an existing account and registers the group', async () => { + const res = await request(app).post(ENDPOINT).send(validBody) + expect(res.status).toBe(200) + expect(res.body.groupDid).toBe('did:plc:existingaccount') + expect(res.body.handle).toBe('imported.pds.example.com') + + // Logged in to the resolved PDS with the supplied app password + const mockAgent = vi.mocked(AtpAgent).mock.results[0].value + expect(mockAgent.login).toHaveBeenCalledWith({ + identifier: 'did:plc:existingaccount', + password: 'abcd-efgh-ijkl-mnop', + }) + + // Group stored in global DB with the resolved PDS and NO recovery key + const group = await globalDb + .selectFrom('groups') + .where('did', '=', 'did:plc:existingaccount') + .selectAll() + .executeTakeFirst() + expect(group).toBeDefined() + expect(group!.pds_url).toBe('https://pds.example.com') + expect(group!.encrypted_app_password).toBeDefined() + expect(group!.encrypted_recovery_key).toBeNull() + }) + + it('seeds the caller as owner', async () => { + const res = await request(app).post(ENDPOINT).send(validBody) + expect(res.status).toBe(200) + + const owner = await groupDb + .selectFrom('group_members') + .where('member_did', '=', 'did:plc:owner') + .selectAll() + .executeTakeFirst() + expect(owner).toBeDefined() + expect(owner!.role).toBe('owner') + }) + + it('stores the resolved PDS url, not the configured group PDS', async () => { + // Resolve the account to a different PDS than config.groupPdsUrl + const test = await createTestContext({ + authVerifier: mockAuth('did:plc:owner'), + idResolver: mockIdResolver('https://other-pds.example.net'), + }) + const otherApp = createTestApp(test.ctx, groupImportHandler) + + const res = await request(otherApp).post(ENDPOINT).send(validBody) + expect(res.status).toBe(200) + + const mockAgent = vi.mocked(AtpAgent).mock.results.at(-1)!.value + expect(mockAgent.login).toHaveBeenCalled() + + const group = await test.globalDb + .selectFrom('groups') + .where('did', '=', 'did:plc:existingaccount') + .selectAll() + .executeTakeFirst() + expect(group!.pds_url).toBe('https://other-pds.example.net') + + await test.globalDb.destroy() + await test.groupDb.destroy() + }) + + it('returns InvalidAppPassword when login fails (bad/revoked credential)', async () => { + vi.mocked(AtpAgent).mockImplementationOnce( + () => + ({ + login: vi.fn().mockRejectedValue( + Object.assign(new Error('Invalid identifier or password'), { + status: 401, + error: 'AuthenticationRequired', + }), + ), + }) as any, + ) + + const res = await request(app).post(ENDPOINT).send(validBody) + expect(res.status).toBe(401) + expect(res.body.error).toBe('InvalidAppPassword') + + // Nothing persisted on failure + const group = await globalDb + .selectFrom('groups') + .where('did', '=', 'did:plc:existingaccount') + .selectAll() + .executeTakeFirst() + expect(group).toBeUndefined() + }) + + it('returns GroupAlreadyRegistered when the group already exists', async () => { + const first = await request(app).post(ENDPOINT).send(validBody) + expect(first.status).toBe(200) + + const second = await request(app).post(ENDPOINT).send(validBody) + expect(second.status).toBe(409) + expect(second.body.error).toBe('GroupAlreadyRegistered') + }) + + it('returns 400 for missing fields', async () => { + const res = await request(app).post(ENDPOINT).send({ groupDid: 'did:plc:existingaccount' }) + expect(res.status).toBe(400) + }) + + it('returns 400 for invalid groupDid', async () => { + const res = await request(app) + .post(ENDPOINT) + .send({ ...validBody, groupDid: 'not-a-did' }) + expect(res.status).toBe(400) + }) + + it('returns 400 for invalid ownerDid', async () => { + const res = await request(app) + .post(ENDPOINT) + .send({ ...validBody, ownerDid: 'not-a-did' }) + expect(res.status).toBe(400) + }) + + it('rejects when the auth issuer does not match ownerDid', async () => { + const test = await createTestContext({ + // Caller authenticates as someone other than the claimed owner + authVerifier: mockAuth('did:plc:someoneelse'), + }) + const otherApp = createTestApp(test.ctx, groupImportHandler) + + const res = await request(otherApp).post(ENDPOINT).send(validBody) + expect(res.status).toBe(401) + + await test.globalDb.destroy() + await test.groupDb.destroy() + }) + + it('rejects unauthenticated requests', async () => { + const test = await createTestContext({ + authVerifier: { + ...mockAuth('did:plc:owner'), + verifyServiceAuth: async () => { + throw new AuthRequiredError('Missing auth token') + }, + }, + }) + const otherApp = createTestApp(test.ctx, groupImportHandler) + + const res = await request(otherApp).post(ENDPOINT).send(validBody) + expect(res.status).toBe(401) + + await test.globalDb.destroy() + await test.groupDb.destroy() + }) +}) diff --git a/tests/register.test.ts b/tests/register.test.ts index 55e41bf..e2f452a 100644 --- a/tests/register.test.ts +++ b/tests/register.test.ts @@ -2,9 +2,8 @@ import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest' import express from 'express' import request from 'supertest' import { AuthRequiredError } from '@atproto/xrpc-server' -import { createTestContext, silentLogger } from './helpers/mock-server.js' +import { createTestContext, createTestApp, mockAuth } from './helpers/mock-server.js' import groupRegisterHandler from '../src/api/group/register.js' -import { createFallbackErrorHandler } from '../src/api/error-handler.js' import type { AppContext } from '../src/context.js' import type { Kysely } from 'kysely' import type { GlobalDatabase, GroupDatabase } from '../src/db/schema.js' @@ -77,11 +76,7 @@ import { } from '../src/pds/plc.js' function createApp(ctx: AppContext) { - const app = express() - app.use(express.json()) - groupRegisterHandler(app, ctx) - app.use(createFallbackErrorHandler(silentLogger as any)) - return app + return createTestApp(ctx, groupRegisterHandler) } const validBody = { @@ -98,21 +93,7 @@ describe('group.register', () => { beforeEach(async () => { vi.clearAllMocks() const test = await createTestContext({ - authVerifier: { - verify: async () => ({ - iss: 'did:plc:owner', - aud: 'did:plc:testgroup', - }), - verifyRegistration: async () => ({ iss: 'did:plc:owner' }), - xrpcAuth() { - return async () => ({ - credentials: { - callerDid: 'did:plc:owner', - groupDid: 'did:plc:testgroup', - }, - }) - }, - } as any, + authVerifier: mockAuth('did:plc:owner'), }) ctx = test.ctx globalDb = test.globalDb @@ -255,22 +236,11 @@ describe('group.register', () => { it('rejects unauthenticated requests', async () => { const test = await createTestContext({ authVerifier: { - verify: async () => ({ - iss: 'did:plc:owner', - aud: 'did:plc:testgroup', - }), - verifyRegistration: async () => { + ...mockAuth('did:plc:owner'), + verifyServiceAuth: async () => { throw new AuthRequiredError('Missing auth token') }, - xrpcAuth() { - return async () => ({ - credentials: { - callerDid: 'did:plc:owner', - groupDid: 'did:plc:testgroup', - }, - }) - }, - } as any, + }, }) const unauthApp = createApp(test.ctx) @@ -283,21 +253,7 @@ describe('group.register', () => { it('rejects when token issuer does not match ownerDid', async () => { const test = await createTestContext({ - authVerifier: { - verify: async () => ({ - iss: 'did:plc:attacker', - aud: 'did:plc:testgroup', - }), - verifyRegistration: async () => ({ iss: 'did:plc:attacker' }), - xrpcAuth() { - return async () => ({ - credentials: { - callerDid: 'did:plc:attacker', - groupDid: 'did:plc:testgroup', - }, - }) - }, - } as any, + authVerifier: mockAuth('did:plc:attacker'), }) const mismatchApp = createApp(test.ctx) diff --git a/tests/verifier.test.ts b/tests/verifier.test.ts index 1891c36..e966e13 100644 --- a/tests/verifier.test.ts +++ b/tests/verifier.test.ts @@ -170,7 +170,7 @@ describe('AuthVerifier', () => { expect(result).toEqual({ iss: 'did:plc:caller', aud: 'did:plc:testgroup' }) }) - it('enforces token lifetime in verifyRegistration', async () => { + it('enforces token lifetime in verifyServiceAuth', async () => { const now = Math.floor(Date.now() / 1000) fakeVerifyJwt.mockResolvedValue({ iss: 'did:plc:caller', @@ -179,19 +179,19 @@ describe('AuthVerifier', () => { exp: now + NONCE_TTL_SECONDS + 60, }) const regReq = makeReq({ authorization: 'Bearer jwt' }, '/xrpc/app.certified.group.register') - await expect(verifier.verifyRegistration(regReq)).rejects.toThrow( + await expect(verifier.verifyServiceAuth(regReq)).rejects.toThrow( 'Token lifetime exceeds nonce window', ) }) - it('rejects missing iat in verifyRegistration', async () => { + it('rejects missing iat in verifyServiceAuth', async () => { fakeVerifyJwt.mockResolvedValue({ iss: 'did:plc:caller', jti: 'jti-reg-no-iat', exp: Math.floor(Date.now() / 1000) + 60, }) const regReq = makeReq({ authorization: 'Bearer jwt' }, '/xrpc/app.certified.group.register') - await expect(verifier.verifyRegistration(regReq)).rejects.toThrow( + await expect(verifier.verifyServiceAuth(regReq)).rejects.toThrow( 'Missing iat in service auth token', ) })