diff --git a/.env.example b/.env.example index 0cf5ce2..2ed543b 100644 --- a/.env.example +++ b/.env.example @@ -23,6 +23,12 @@ LOG_LEVEL=debug # Set to true/1/yes when behind a reverse proxy (e.g. Caddy) so client IP is read from X-Forwarded-For # TRUST_PROXY=true +# Refresh-cookie Secure flag. Defaults to true when NODE_ENV=production. A Secure cookie is dropped by +# the browser over plain HTTP on a non-localhost host, which logs users out on every page reload (the +# reload restores the session only by exchanging this cookie). If you serve the app over plain HTTP on +# a trusted network (e.g. a LAN-only http://host:port), set this to false. Prefer HTTPS where possible. +# COOKIE_SECURE=false + # CORS (comma-separated origins; keep strict — do not use * with credentials) CORS_ORIGINS=http://localhost:5173 diff --git a/README.md b/README.md index cc7bcc7..7ca5d75 100644 --- a/README.md +++ b/README.md @@ -183,7 +183,7 @@ This tool implements the full [NIP-46 Nostr Remote Signing](https://nips.nostr.c - **URI Support**: Parse and generate both `bunker://` and `nostrconnect://` URIs - **All RPC Methods**: `connect`, `sign_event`, `ping`, `get_public_key`, `nip04_encrypt/decrypt`, `nip44_encrypt/decrypt`, `switch_relays` -- **Fine-grained Permissions**: Per-connection method and event kind restrictions +- **Fine-grained Permissions**: Per-connection method and event-kind restrictions, enforced default-deny (a connection with no permissions can sign/decrypt nothing). New connections get a conservative default set (common signing kinds, no decryption). Following the NIP-46 model for a non-interactive signer, a connecting client may declare the scope it needs in its `connect` request and that becomes the connection's permission set, so **only connect clients you trust with the bound key**; operators can tighten any connection's permissions in the dashboard at any time. - **NIP-44 Encryption**: All communication encrypted using NIP-44 - **Auth Challenges**: Support for out-of-band authentication - **Relay Management**: Configurable relays per connection with automatic reconnection diff --git a/apps/server/prisma/migrations/20260624130000_grandfather_connection_permissions/migration.sql b/apps/server/prisma/migrations/20260624130000_grandfather_connection_permissions/migration.sql new file mode 100644 index 0000000..b823651 --- /dev/null +++ b/apps/server/prisma/migrations/20260624130000_grandfather_connection_permissions/migration.sql @@ -0,0 +1,26 @@ +-- Grandfather existing permission-less connections into the new default-deny permission model. +-- +-- Before this change an empty permission set meant "allow everything" (fail-open). The NIP-46 RPC +-- handler is now default-deny, and newly created connections are seeded with a conservative default +-- set in application code. Existing ACTIVE/PENDING connections that currently have NO permission +-- rows were operating under the old allow-all behaviour; denying them outright would break live +-- signers. To avoid downtime we make their previous implicit access EXPLICIT: one method-level row +-- (kind = NULL = all kinds for sign_event) per gated capability method. Operators should review and +-- tighten these in the dashboard afterwards. New connections are unaffected (seeded at creation). +-- +-- Idempotent: re-running is a no-op because the NOT EXISTS guard skips any connection that already +-- has permission rows. +INSERT INTO "connection_permissions" ("id", "connection_id", "method", "kind", "allowed") +SELECT gen_random_uuid()::text, c."id", m."method", NULL, true +FROM "bunker_connections" c +CROSS JOIN (VALUES + ('sign_event'), + ('nip04_encrypt'), + ('nip04_decrypt'), + ('nip44_encrypt'), + ('nip44_decrypt') +) AS m("method") +WHERE c."status" IN ('ACTIVE', 'PENDING') + AND NOT EXISTS ( + SELECT 1 FROM "connection_permissions" p WHERE p."connection_id" = c."id" + ); diff --git a/apps/server/src/auth/auth.controller.spec.ts b/apps/server/src/auth/auth.controller.spec.ts index 8c46662..19bd34d 100644 --- a/apps/server/src/auth/auth.controller.spec.ts +++ b/apps/server/src/auth/auth.controller.spec.ts @@ -257,6 +257,40 @@ describe('AuthController', () => { await expect(controller.refresh(req, mockReply)).rejects.toThrow('Missing refresh token'); expect(mockAuthService.refreshTokens).not.toHaveBeenCalled(); }); + + // The Secure flag governs whether a plain-HTTP (e.g. LAN) deployment can keep a session across + // reloads: a Secure cookie is dropped over http, so the reload-restore via /api/auth/refresh fails. + describe('Secure cookie flag', () => { + const req = { + headers: {}, + cookies: { refresh_token: 'rt-from-cookie' }, + } as unknown as FastifyRequest; + const original = process.env['COOKIE_SECURE']; + afterEach(() => { + if (original === undefined) delete process.env['COOKIE_SECURE']; + else process.env['COOKIE_SECURE'] = original; + }); + + it('marks the cookie Secure=false when COOKIE_SECURE=false (plain-HTTP deployment)', async () => { + process.env['COOKIE_SECURE'] = 'false'; + await controller.refresh(req, mockReply); + expect(mockReply.setCookie).toHaveBeenCalledWith( + 'refresh_token', + 'rt2', + expect.objectContaining({ secure: false }), + ); + }); + + it('marks the cookie Secure=true when COOKIE_SECURE=true', async () => { + process.env['COOKIE_SECURE'] = 'true'; + await controller.refresh(req, mockReply); + expect(mockReply.setCookie).toHaveBeenCalledWith( + 'refresh_token', + 'rt2', + expect.objectContaining({ secure: true }), + ); + }); + }); }); describe('logout', () => { diff --git a/apps/server/src/auth/auth.controller.ts b/apps/server/src/auth/auth.controller.ts index 94469f2..736a62e 100644 --- a/apps/server/src/auth/auth.controller.ts +++ b/apps/server/src/auth/auth.controller.ts @@ -29,6 +29,21 @@ import type { FastifyRequest, FastifyReply } from 'fastify'; /** httpOnly cookie that carries the refresh token; never exposed to browser JavaScript. */ const REFRESH_COOKIE = 'refresh_token'; +/** + * Whether the refresh cookie is marked `Secure` (sent only over HTTPS). Defaults to true in + * production so a real deployment never ships the refresh token over plaintext. A `Secure` cookie + * is silently dropped by the browser when the app is served over plain HTTP on a non-localhost host, + * which makes every page reload log the user out (the reload can only restore the session by + * exchanging this cookie). Operators serving over plain HTTP on a trusted network (e.g. a LAN-only + * http://host:port) must therefore set `COOKIE_SECURE=false`; ideally, serve the app over HTTPS. + */ +function isCookieSecure(): boolean { + const override = process.env['COOKIE_SECURE']; + if (override === 'true') return true; + if (override === 'false') return false; + return process.env['NODE_ENV'] === 'production'; +} + /** * Cookie attributes shared by set and clear. Browsers only delete a cookie when the clear call's * attributes match those it was set with, so both paths must use the same values. @@ -36,7 +51,7 @@ const REFRESH_COOKIE = 'refresh_token'; function refreshCookieAttrs() { return { httpOnly: true, - secure: process.env['NODE_ENV'] === 'production', + secure: isCookieSecure(), sameSite: 'lax' as const, // Scope to the auth routes that consume it (refresh, logout), so it is not sent on every request. path: '/api/auth', diff --git a/apps/server/src/bunker/bunker-rpc.handler.spec.ts b/apps/server/src/bunker/bunker-rpc.handler.spec.ts new file mode 100644 index 0000000..33196ad --- /dev/null +++ b/apps/server/src/bunker/bunker-rpc.handler.spec.ts @@ -0,0 +1,300 @@ +import { describe, it, expect, beforeEach, vi } from 'vitest'; +import { BunkerRpcHandler } from './bunker-rpc.handler.js'; +import type { ConnectionsService } from '../connections/connections.service.js'; +import type { LoggingService } from '../logging/logging.service.js'; +import type { EventsService } from '../events/events.service.js'; +import type { EncryptionService } from '../common/crypto/encryption.service.js'; +import type { Nip46Request } from '@bunker46/shared-types'; + +type Perm = { method: string; kind?: number; allowed?: boolean }; + +describe('BunkerRpcHandler', () => { + let handler: BunkerRpcHandler; + let connections: Partial; + let loggingService: Partial; + let eventsService: Partial; + let encryption: Partial; + + const signEvent = vi.fn().mockResolvedValue('{"sig":"deadbeef"}'); + const nip04Encrypt = vi.fn().mockResolvedValue('enc04'); + const nip04Decrypt = vi.fn().mockResolvedValue('dec04'); + const nip44Encrypt = vi.fn().mockResolvedValue('enc44'); + const nip44Decrypt = vi.fn().mockResolvedValue('dec44'); + const getPublicKeyFromNsec = vi.fn().mockReturnValue('signer-pubkey-hex'); + + const CLIENT = 'client-pubkey-hex'; + const SIGNER = 'signer-pubkey-hex'; + + function makeConnection(permissions: Perm[], status = 'ACTIVE', secret?: string) { + return { + id: 'conn-1', + userId: 'user-1', + name: 'Test App', + clientPubkey: CLIENT, + status, + relays: ['wss://relay.example.com'], + secret, + permissions: permissions.map((p) => ({ + method: p.method, + kind: p.kind ?? null, + allowed: p.allowed ?? true, + })), + nsecKey: { encryptedNsec: 'encrypted-nsec' }, + }; + } + + function request(method: string, params: string[] = [], id = 'req-1'): Nip46Request { + return { id, method, params } as Nip46Request; + } + + function handle(req: Nip46Request) { + return handler.handleRequest( + CLIENT, + SIGNER, + req, + signEvent, + nip04Encrypt, + nip04Decrypt, + nip44Encrypt, + nip44Decrypt, + getPublicKeyFromNsec, + ); + } + + const signEventJson = (kind: number) => JSON.stringify({ kind, content: 'hello', tags: [] }); + + beforeEach(() => { + vi.clearAllMocks(); + connections = { + findByClientAndSigner: vi.fn().mockResolvedValue(null), + updateConnectionStatus: vi.fn().mockResolvedValue(undefined as never), + setPermissions: vi.fn().mockResolvedValue(undefined as never), + requestPermissions: vi.fn().mockResolvedValue(undefined as never), + touchActivity: vi.fn().mockResolvedValue(undefined as never), + }; + loggingService = { logSigningAction: vi.fn().mockResolvedValue(undefined as never) }; + eventsService = { publishUserActivity: vi.fn().mockResolvedValue(undefined as never) }; + encryption = { decrypt: vi.fn().mockReturnValue('nsec-hex') }; + handler = new BunkerRpcHandler( + connections as ConnectionsService, + loggingService as LoggingService, + eventsService as EventsService, + encryption as EncryptionService, + ); + }); + + describe('unknown / revoked connections', () => { + it('rejects a non-connect request from an unknown client', async () => { + vi.mocked(connections.findByClientAndSigner!).mockResolvedValue(null as never); + const res = await handle(request('sign_event', [signEventJson(1)])); + expect(res).toEqual({ id: 'req-1', error: 'Unknown client' }); + expect(signEvent).not.toHaveBeenCalled(); + // No connection => no activity/logging side effects. + expect(loggingService.logSigningAction).not.toHaveBeenCalled(); + }); + + it('rejects requests on a REVOKED connection', async () => { + vi.mocked(connections.findByClientAndSigner!).mockResolvedValue( + makeConnection([{ method: 'sign_event' }], 'REVOKED') as never, + ); + const res = await handle(request('sign_event', [signEventJson(1)])); + expect(res).toEqual({ id: 'req-1', error: 'Connection revoked' }); + expect(signEvent).not.toHaveBeenCalled(); + }); + }); + + describe('signer binding (M1)', () => { + it('resolves the connection scoped to the signer pubkey the request was addressed to', async () => { + vi.mocked(connections.findByClientAndSigner!).mockResolvedValue( + makeConnection([{ method: 'sign_event', kind: 1 }]) as never, + ); + await handle(request('sign_event', [signEventJson(1)])); + // The relay #p tag / listener key is passed through so a request for signer A cannot be served + // by a connection bound to signer B. + expect(connections.findByClientAndSigner).toHaveBeenCalledWith(CLIENT, SIGNER); + }); + }); + + describe('default-deny permission model', () => { + it('denies sign_event when the connection has NO permissions (not fail-open)', async () => { + vi.mocked(connections.findByClientAndSigner!).mockResolvedValue(makeConnection([]) as never); + const res = await handle(request('sign_event', [signEventJson(1)])); + expect(res.error).toBe('Permission denied for sign_event kind:1'); + expect(res.result).toBeUndefined(); + expect(signEvent).not.toHaveBeenCalled(); + }); + + it('denies nip44_decrypt with no permissions (no blanket decryption oracle)', async () => { + vi.mocked(connections.findByClientAndSigner!).mockResolvedValue(makeConnection([]) as never); + const res = await handle(request('nip44_decrypt', ['third-party-pubkey', 'ciphertext'])); + expect(res.error).toBe('Permission denied for nip44_decrypt'); + expect(nip44Decrypt).not.toHaveBeenCalled(); + }); + + it('denies nip04_decrypt with no permissions', async () => { + vi.mocked(connections.findByClientAndSigner!).mockResolvedValue(makeConnection([]) as never); + const res = await handle(request('nip04_decrypt', ['third-party-pubkey', 'ciphertext'])); + expect(res.error).toBe('Permission denied for nip04_decrypt'); + expect(nip04Decrypt).not.toHaveBeenCalled(); + }); + }); + + describe('permission enforcement', () => { + it('allows sign_event for a kind explicitly permitted', async () => { + vi.mocked(connections.findByClientAndSigner!).mockResolvedValue( + makeConnection([{ method: 'sign_event', kind: 1 }]) as never, + ); + const res = await handle(request('sign_event', [signEventJson(1)])); + expect(res.error).toBeUndefined(); + expect(res.result).toBe('{"sig":"deadbeef"}'); + expect(signEvent).toHaveBeenCalledWith(signEventJson(1), 'nsec-hex'); + }); + + it('denies sign_event for a kind that is not permitted', async () => { + vi.mocked(connections.findByClientAndSigner!).mockResolvedValue( + makeConnection([{ method: 'sign_event', kind: 1 }]) as never, + ); + const res = await handle(request('sign_event', [signEventJson(2)])); + expect(res.error).toBe('Permission denied for sign_event kind:2'); + expect(signEvent).not.toHaveBeenCalled(); + }); + + it('a method-level sign_event permission (no kind) allows any kind', async () => { + vi.mocked(connections.findByClientAndSigner!).mockResolvedValue( + makeConnection([{ method: 'sign_event' }]) as never, + ); + const res = await handle(request('sign_event', [signEventJson(30023)])); + expect(res.error).toBeUndefined(); + expect(signEvent).toHaveBeenCalledTimes(1); + }); + + it('allows nip44_decrypt when explicitly permitted', async () => { + vi.mocked(connections.findByClientAndSigner!).mockResolvedValue( + makeConnection([{ method: 'nip44_decrypt' }]) as never, + ); + const res = await handle(request('nip44_decrypt', ['third-party-pubkey', 'ciphertext'])); + expect(res.error).toBeUndefined(); + expect(res.result).toBe('dec44'); + expect(nip44Decrypt).toHaveBeenCalledWith('ciphertext', 'third-party-pubkey', 'nsec-hex'); + }); + }); + + describe('unguarded informational methods', () => { + it('responds to ping without requiring a permission', async () => { + vi.mocked(connections.findByClientAndSigner!).mockResolvedValue(makeConnection([]) as never); + const res = await handle(request('ping')); + expect(res.result).toBe('pong'); + expect(res.error).toBeUndefined(); + }); + + it('returns get_public_key without requiring a permission', async () => { + vi.mocked(connections.findByClientAndSigner!).mockResolvedValue(makeConnection([]) as never); + const res = await handle(request('get_public_key')); + expect(res.result).toBe('signer-pubkey-hex'); + expect(res.error).toBeUndefined(); + }); + }); + + describe('connect handshake', () => { + it('activates a PENDING connection and echoes its secret', async () => { + vi.mocked(connections.findByClientAndSigner!).mockResolvedValue( + makeConnection([], 'PENDING', 'the-secret') as never, + ); + const res = await handle(request('connect', ['', ''])); + expect(connections.updateConnectionStatus).toHaveBeenCalledWith('conn-1', 'ACTIVE'); + expect(res.result).toBe('the-secret'); + }); + + it('records client-requested permissions as PENDING (not granted) on connect', async () => { + vi.mocked(connections.findByClientAndSigner!).mockResolvedValue( + makeConnection([], 'PENDING') as never, + ); + await handle(request('connect', ['', '', 'sign_event:1,nip44_decrypt'])); + // Requested, not granted: recorded for operator approval, never applied directly. + expect(connections.requestPermissions).toHaveBeenCalledWith('conn-1', [ + { method: 'sign_event', kind: 1 }, + { method: 'nip44_decrypt', kind: undefined }, + ]); + expect(connections.setPermissions).not.toHaveBeenCalled(); + }); + }); + + describe('pending permissions do not grant access', () => { + it('denies a capability that is only PENDING (allowed=false), even if requested on connect', async () => { + // A pending nip44_decrypt (operator has not approved it) must not authorize decryption. + vi.mocked(connections.findByClientAndSigner!).mockResolvedValue( + makeConnection([{ method: 'nip44_decrypt', allowed: false }]) as never, + ); + const res = await handle(request('nip44_decrypt', ['third-party-pubkey', 'ciphertext'])); + expect(res.error).toBe('Permission denied for nip44_decrypt'); + expect(nip44Decrypt).not.toHaveBeenCalled(); + }); + + it('allows the same capability once it has been approved (allowed=true)', async () => { + vi.mocked(connections.findByClientAndSigner!).mockResolvedValue( + makeConnection([{ method: 'nip44_decrypt', allowed: true }]) as never, + ); + const res = await handle(request('nip44_decrypt', ['third-party-pubkey', 'ciphertext'])); + expect(res.error).toBeUndefined(); + expect(nip44Decrypt).toHaveBeenCalledTimes(1); + }); + }); + + describe('denied gated requests surface as pending', () => { + it('records a denied sign_event kind as a PENDING permission request for operator approval', async () => { + // Granted only kind 1; the client signs kind 30078 (app data) which is not permitted. + vi.mocked(connections.findByClientAndSigner!).mockResolvedValue( + makeConnection([{ method: 'sign_event', kind: 1 }]) as never, + ); + const res = await handle(request('sign_event', [signEventJson(30078)])); + expect(res.error).toBe('Permission denied for sign_event kind:30078'); + expect(connections.requestPermissions).toHaveBeenCalledWith('conn-1', [ + { method: 'sign_event', kind: 30078 }, + ]); + }); + + it('records a denied nip44_decrypt as a PENDING (method-level) request', async () => { + vi.mocked(connections.findByClientAndSigner!).mockResolvedValue(makeConnection([]) as never); + await handle(request('nip44_decrypt', ['third-party-pubkey', 'ciphertext'])); + expect(connections.requestPermissions).toHaveBeenCalledWith('conn-1', [ + { method: 'nip44_decrypt' }, + ]); + }); + + it('does NOT record a pending request when the capability is already permitted', async () => { + vi.mocked(connections.findByClientAndSigner!).mockResolvedValue( + makeConnection([{ method: 'sign_event', kind: 1 }]) as never, + ); + await handle(request('sign_event', [signEventJson(1)])); + expect(connections.requestPermissions).not.toHaveBeenCalled(); + }); + }); + + describe('audit logging', () => { + it('logs an approved signing action and publishes activity', async () => { + vi.mocked(connections.findByClientAndSigner!).mockResolvedValue( + makeConnection([{ method: 'sign_event', kind: 1 }]) as never, + ); + await handle(request('sign_event', [signEventJson(1)])); + expect(connections.touchActivity).toHaveBeenCalledWith('conn-1'); + expect(loggingService.logSigningAction).toHaveBeenCalledWith( + expect.objectContaining({ + connectionId: 'conn-1', + userId: 'user-1', + method: 'sign_event', + eventKind: 1, + result: 'APPROVED', + }), + ); + expect(eventsService.publishUserActivity).toHaveBeenCalledWith('user-1'); + }); + + it('logs a permission-denied signing action as a non-approved result', async () => { + vi.mocked(connections.findByClientAndSigner!).mockResolvedValue(makeConnection([]) as never); + await handle(request('sign_event', [signEventJson(1)])); + expect(loggingService.logSigningAction).toHaveBeenCalledWith( + expect.objectContaining({ method: 'sign_event', result: 'ERROR' }), + ); + }); + }); +}); diff --git a/apps/server/src/bunker/bunker-rpc.handler.ts b/apps/server/src/bunker/bunker-rpc.handler.ts index f7d2839..b7a3e53 100644 --- a/apps/server/src/bunker/bunker-rpc.handler.ts +++ b/apps/server/src/bunker/bunker-rpc.handler.ts @@ -11,6 +11,14 @@ import { type Nip46Response, } from '@bunker46/shared-types'; +/** Resolved pending bunker:// secret: who/which key to bind, plus the operator's chosen permission seed. */ +type PendingSecretResult = { + userId: string; + nsecKeyId: string; + name: string; + permissions?: PermissionDescriptor[]; +}; + @Injectable() export class BunkerRpcHandler { private readonly logger = new Logger(BunkerRpcHandler.name); @@ -18,7 +26,7 @@ export class BunkerRpcHandler { private pendingSecretLookup?: ( signerPubkey: string, secret: string, - ) => { userId: string; nsecKeyId: string; name: string } | undefined; + ) => PendingSecretResult | undefined; constructor( private readonly connections: ConnectionsService, @@ -28,10 +36,7 @@ export class BunkerRpcHandler { ) {} setPendingSecretLookup( - fn: ( - signerPubkey: string, - secret: string, - ) => { userId: string; nsecKeyId: string; name: string } | undefined, + fn: (signerPubkey: string, secret: string) => PendingSecretResult | undefined, ) { this.pendingSecretLookup = fn; } @@ -56,7 +61,7 @@ export class BunkerRpcHandler { getPublicKeyFromNsec: (nsecHex: string) => string, ): Promise { const start = Date.now(); - let connection = await this.connections.findByClientPubkey(clientPubkey); + let connection = await this.connections.findByClientAndSigner(clientPubkey, signerPubkey); if (!connection && request.method === 'connect') { connection = await this.handleNewConnect(clientPubkey, signerPubkey, request); @@ -73,16 +78,24 @@ export class BunkerRpcHandler { return { id: request.id, error: 'Connection revoked' }; } - const permissions: PermissionDescriptor[] = connection.permissions.map((p) => ({ - method: p.method as PermissionDescriptor['method'], - kind: p.kind ?? undefined, - })); + // Only GRANTED permissions (allowed = true) are enforced. Pending requests (allowed = false), + // recorded when a client asks for capabilities on connect, grant nothing until the operator + // approves them in the dashboard — so a client can never self-escalate. + const permissions: PermissionDescriptor[] = connection.permissions + .filter((p) => p.allowed) + .map((p) => ({ + method: p.method as PermissionDescriptor['method'], + kind: p.kind ?? undefined, + })); const nsecHex = this.encryption.decrypt(connection.nsecKey.encryptedNsec); let result: string | undefined; let error: string | undefined; let eventKind: number | undefined; + // A gated method denied for want of permission is captured here and, after the request is + // handled, recorded as a PENDING permission request the operator can approve in the dashboard. + let deniedPermission: PermissionDescriptor | undefined; try { switch (request.method) { @@ -94,14 +107,19 @@ export class BunkerRpcHandler { ); } + // Per NIP-46 a client may declare the capabilities it needs in the connect request's perms + // param. These are a REQUEST, not a grant: we record them as PENDING for the operator to + // approve in the dashboard (interactive approval). They grant nothing until approved, so a + // client can never self-escalate — only the seeded/operator-granted permissions are enforced. + // Already-granted (or already-pending) method/kinds are skipped, so re-connects are idempotent. const connectPerms = request.params[2]; if (connectPerms) { try { const parsed = parsePermissionList(connectPerms); if (parsed.length > 0) { - await this.connections.setPermissions(connection.id, parsed); + await this.connections.requestPermissions(connection.id, parsed); this.logger.log( - `Stored ${parsed.length} permissions from connect for ${clientPubkey.slice(0, 12)}...: ${connectPerms}`, + `Recorded ${parsed.length} pending permission request(s) from connect for ${clientPubkey.slice(0, 12)}...: ${connectPerms}`, ); } } catch { @@ -136,11 +154,12 @@ export class BunkerRpcHandler { `sign_event kind:${eventKind} from ${clientPubkey.slice(0, 12)}... (conn: ${connection.name})`, ); - if ( - permissions.length > 0 && - !checkPermission(permissions, { method: 'sign_event', kind: eventKind }) - ) { + // Default-deny: every capability method (sign_event, nip04/nip44 encrypt/decrypt) requires + // an explicit matching permission. A connection with no stored permissions can perform none + // of them — unlike the previous fail-open behaviour where an empty set allowed everything. + if (!checkPermission(permissions, { method: 'sign_event', kind: eventKind })) { error = `Permission denied for sign_event kind:${eventKind}`; + deniedPermission = { method: 'sign_event', kind: eventKind }; break; } @@ -149,11 +168,9 @@ export class BunkerRpcHandler { } case 'nip04_encrypt': { - if ( - permissions.length > 0 && - !checkPermission(permissions, { method: 'nip04_encrypt' }) - ) { + if (!checkPermission(permissions, { method: 'nip04_encrypt' })) { error = 'Permission denied for nip04_encrypt'; + deniedPermission = { method: 'nip04_encrypt' }; break; } const [tp04e, pt04] = request.params; @@ -166,11 +183,9 @@ export class BunkerRpcHandler { } case 'nip04_decrypt': { - if ( - permissions.length > 0 && - !checkPermission(permissions, { method: 'nip04_decrypt' }) - ) { + if (!checkPermission(permissions, { method: 'nip04_decrypt' })) { error = 'Permission denied for nip04_decrypt'; + deniedPermission = { method: 'nip04_decrypt' }; break; } const [tp04d, ct04] = request.params; @@ -183,11 +198,9 @@ export class BunkerRpcHandler { } case 'nip44_encrypt': { - if ( - permissions.length > 0 && - !checkPermission(permissions, { method: 'nip44_encrypt' }) - ) { + if (!checkPermission(permissions, { method: 'nip44_encrypt' })) { error = 'Permission denied for nip44_encrypt'; + deniedPermission = { method: 'nip44_encrypt' }; break; } const [tp44e, pt44] = request.params; @@ -200,11 +213,9 @@ export class BunkerRpcHandler { } case 'nip44_decrypt': { - if ( - permissions.length > 0 && - !checkPermission(permissions, { method: 'nip44_decrypt' }) - ) { + if (!checkPermission(permissions, { method: 'nip44_decrypt' })) { error = 'Permission denied for nip44_decrypt'; + deniedPermission = { method: 'nip44_decrypt' }; break; } const [tp44d, ct44] = request.params; @@ -232,6 +243,18 @@ export class BunkerRpcHandler { const durationMs = Date.now() - start; await this.connections.touchActivity(connection.id); + // Surface a denied gated request as a PENDING permission request (allowed = false) so the operator + // can approve it in the dashboard — the interactive approval flow extended to capabilities the + // client exercises at runtime, not just those it declared on connect. Recorded with skipDuplicates, + // so a client retrying the same method/kind cannot spam the queue, and it never downgrades a grant. + if (deniedPermission) { + try { + await this.connections.requestPermissions(connection.id, [deniedPermission]); + } catch (err) { + this.logger.warn(`Failed to record pending permission request: ${err}`); + } + } + await this.loggingService.logSigningAction({ connectionId: connection.id, userId: connection.userId, @@ -280,13 +303,19 @@ export class BunkerRpcHandler { ); try { - await this.connections.createConnection(info.userId, info.nsecKeyId, clientPubkey, { - name: info.name, - relays: [], - secret, - }); + await this.connections.createConnection( + info.userId, + info.nsecKeyId, + clientPubkey, + { + name: info.name, + relays: [], + secret, + }, + info.permissions, + ); - return this.connections.findByClientPubkey(clientPubkey); + return this.connections.findByClientAndSigner(clientPubkey, signerPubkey); } catch (err) { this.logger.error(`Failed to auto-create connection: ${err}`); return null; diff --git a/apps/server/src/bunker/bunker.controller.ts b/apps/server/src/bunker/bunker.controller.ts index 1deda88..3be0246 100644 --- a/apps/server/src/bunker/bunker.controller.ts +++ b/apps/server/src/bunker/bunker.controller.ts @@ -9,7 +9,7 @@ import { Inject, } from '@nestjs/common'; import { ApiTags, ApiBearerAuth } from '@nestjs/swagger'; -import { SafeRelayUrlsSchema } from '@bunker46/shared-types'; +import { SafeRelayUrlsSchema, PermissionDescriptorSchema } from '@bunker46/shared-types'; import { JwtAuthGuard } from '../auth/guards/jwt-auth.guard.js'; import { TotpVerifiedGuard } from '../auth/guards/totp-verified.guard.js'; import { BunkerService } from './bunker.service.js'; @@ -48,12 +48,20 @@ export class BunkerController { } @Post('generate-bunker-uri') - async generateBunkerUri(@Req() req: AuthReq, @Body() body: { nsecKeyId: string; name?: string }) { + async generateBunkerUri( + @Req() req: AuthReq, + @Body() body: { nsecKeyId: string; name?: string; permissions?: unknown }, + ) { const key = await this.prisma.nsecKey.findUnique({ where: { id: body.nsecKeyId } }); if (!key || key.userId !== req.user.sub) { throw new Error('Key not found'); } + // Operator-chosen permission seed (default-deny, operator-authoritative). Validated and deduped; + // an absent/empty/invalid list leaves the connection to fall back to the conservative defaults. + const parsedPerms = PermissionDescriptorSchema.array().safeParse(body.permissions); + const permissions = parsedPerms.success ? parsedPerms.data : undefined; + const relays = await this.getActiveRelaysForUser(req.user.sub); const secret = randomBytes(16).toString('hex'); @@ -61,9 +69,10 @@ export class BunkerController { userId: req.user.sub, nsecKeyId: body.nsecKeyId, name: body.name || 'Bunker46', + permissions, }); - await this.bunkerService.ensureListeningForConnection(body.nsecKeyId, relays); + await this.bunkerService.ensureListeningForConnection(body.nsecKeyId, req.user.sub, relays); const uri = this.uriService.buildBunkerUri(key.publicKey, relays, secret); return { uri, secret, signerPubkey: key.publicKey, relays }; diff --git a/apps/server/src/bunker/bunker.service.spec.ts b/apps/server/src/bunker/bunker.service.spec.ts index 1c5f4f2..01e05b4 100644 --- a/apps/server/src/bunker/bunker.service.spec.ts +++ b/apps/server/src/bunker/bunker.service.spec.ts @@ -63,6 +63,23 @@ describe('BunkerService', () => { const result = service.consumePendingSecret('a'.repeat(64), 'unknown'); expect(result).toBeUndefined(); }); + + it('preserves the operator-chosen permission seed through register/consume', () => { + const pubkey = 'b'.repeat(64); + const secret = 'secret-with-perms'; + const info = { + userId: 'u1', + nsecKeyId: 'k1', + name: 'Test', + permissions: [ + { method: 'sign_event' as const, kind: 30078 }, + { method: 'nip44_decrypt' as const }, + ], + }; + service.registerPendingSecret(pubkey, secret, info); + // The connect handler reads these back to seed the auto-created connection's granted permissions. + expect(service.consumePendingSecret(pubkey, secret)).toEqual(info); + }); }); describe('listeners', () => { diff --git a/apps/server/src/bunker/bunker.service.ts b/apps/server/src/bunker/bunker.service.ts index 2adf50d..5c9f391 100644 --- a/apps/server/src/bunker/bunker.service.ts +++ b/apps/server/src/bunker/bunker.service.ts @@ -11,7 +11,11 @@ import { hexToBytes } from '@noble/hashes/utils.js'; import * as nip44 from 'nostr-tools/nip44'; import * as nip04 from 'nostr-tools/nip04'; import { BunkerRpcHandler } from './bunker-rpc.handler.js'; -import { Nip46RequestSchema, SafeRelayUrlSchema } from '@bunker46/shared-types'; +import { + Nip46RequestSchema, + SafeRelayUrlSchema, + type PermissionDescriptor, +} from '@bunker46/shared-types'; import { NOSTR_CONSTANTS, NOSTR_DEFAULT_RELAYS_INJECTION_TOKEN } from '@bunker46/config'; import { PrismaService } from '../prisma/prisma.service.js'; import { EncryptionService } from '../common/crypto/encryption.service.js'; @@ -32,6 +36,8 @@ export interface PendingSecretInfo { userId: string; nsecKeyId: string; name: string; + /** Operator-chosen granted permissions to seed the auto-created connection with (bunker:// flow). */ + permissions?: PermissionDescriptor[]; } @Injectable() @@ -248,8 +254,15 @@ export class BunkerService implements OnModuleInit, OnModuleDestroy { ); } - async ensureListeningForConnection(nsecKeyId: string, connectionRelays: string[]) { - const key = await this.prisma.nsecKey.findUnique({ where: { id: nsecKeyId } }); + async ensureListeningForConnection( + nsecKeyId: string, + userId: string, + connectionRelays: string[], + ) { + // Defense-in-depth (same class as the C1 fix): scope the key lookup to its owner so this method + // can never start a listener that decrypts with another user's key, even if a future caller forgets + // to validate ownership first. + const key = await this.prisma.nsecKey.findFirst({ where: { id: nsecKeyId, userId } }); if (!key) return; const nsecHex = this.encryption.decrypt(key.encryptedNsec); @@ -365,11 +378,14 @@ export class BunkerService implements OnModuleInit, OnModuleDestroy { async sendConnectResponse( nsecKeyId: string, + userId: string, clientPubkey: string, secret: string, relays: string[], ) { - const key = await this.prisma.nsecKey.findUnique({ where: { id: nsecKeyId } }); + // Defense-in-depth (same class as the C1 fix): scope the key lookup to its owner so this method can + // never sign/publish a NIP-46 response with another user's key. + const key = await this.prisma.nsecKey.findFirst({ where: { id: nsecKeyId, userId } }); if (!key) { this.logger.error('sendConnectResponse: nsec key not found'); return; diff --git a/apps/server/src/connections/connections.controller.ts b/apps/server/src/connections/connections.controller.ts index c10aebe..308f5c2 100644 --- a/apps/server/src/connections/connections.controller.ts +++ b/apps/server/src/connections/connections.controller.ts @@ -123,20 +123,27 @@ export class ConnectionsController { if (body.perms) { try { const permissions = parsePermissionList(body.perms); - await this.connectionsService.setPermissions(conn.id, permissions); + // Only override the seeded default permissions when an explicit, non-empty set was supplied; + // an empty/invalid perms string leaves the conservative defaults in place (not a locked-out + // connection under the default-deny handler). + if (permissions.length > 0) { + await this.connectionsService.setPermissions(conn.id, permissions); + } } catch { - // Invalid perms string - continue without setting permissions + // Invalid perms string - continue with the seeded default permissions } } await this.bunkerService.ensureListeningForConnection( body.nsecKeyId, + req.user.sub, validatedRelays ?? body.relays ?? [], ); if (body.type === 'nostrconnect' && body.secret) { await this.bunkerService.sendConnectResponse( body.nsecKeyId, + req.user.sub, body.clientPubkey, body.secret, validatedRelays ?? body.relays ?? [], @@ -158,6 +165,30 @@ export class ConnectionsController { return { success: true }; } + // Approve client-requested (pending) permissions; omit `permissions` to approve all pending. + @Post(':id/permissions/approve') + @HttpCode(HttpStatus.OK) + async approvePermissionRequests( + @Req() req: AuthReq, + @Param('id') id: string, + @Body() body: { permissions?: PermissionDescriptor[] }, + ) { + await this.connectionsService.approveRequests(id, req.user.sub, body?.permissions); + return { success: true }; + } + + // Deny (delete) client-requested (pending) permissions; omit `permissions` to deny all pending. + @Post(':id/permissions/deny') + @HttpCode(HttpStatus.OK) + async denyPermissionRequests( + @Req() req: AuthReq, + @Param('id') id: string, + @Body() body: { permissions?: PermissionDescriptor[] }, + ) { + await this.connectionsService.denyRequests(id, req.user.sub, body?.permissions); + return { success: true }; + } + @Patch(':id/status') async updateStatus( @Req() req: AuthReq, diff --git a/apps/server/src/connections/connections.service.spec.ts b/apps/server/src/connections/connections.service.spec.ts index af691bd..08cb126 100644 --- a/apps/server/src/connections/connections.service.spec.ts +++ b/apps/server/src/connections/connections.service.spec.ts @@ -17,12 +17,16 @@ describe('ConnectionsService', () => { create: vi.fn().mockResolvedValue({ id: 'key-1', publicKey: 'pub', label: 'My Key' }), findMany: vi.fn().mockResolvedValue([]), findUnique: vi.fn().mockResolvedValue(null), + findFirst: vi.fn().mockResolvedValue(null), delete: vi.fn().mockResolvedValue(undefined), }, bunkerConnection: { create: vi.fn().mockResolvedValue({ id: 'conn-1', status: 'PENDING' }), findMany: vi.fn().mockResolvedValue([]), findUnique: vi.fn().mockResolvedValue(null), + findUniqueOrThrow: vi + .fn() + .mockResolvedValue({ id: 'conn-1', status: 'PENDING', permissions: [] }), findFirst: vi.fn().mockResolvedValue(null), update: vi.fn().mockResolvedValue({}), delete: vi.fn().mockResolvedValue(undefined), @@ -31,6 +35,7 @@ describe('ConnectionsService', () => { connectionPermission: { deleteMany: vi.fn().mockResolvedValue({ count: 0 }), createMany: vi.fn().mockResolvedValue({ count: 0 }), + updateMany: vi.fn().mockResolvedValue({ count: 0 }), findMany: vi.fn().mockResolvedValue([]), }, }; @@ -84,8 +89,18 @@ describe('ConnectionsService', () => { }); describe('createConnection', () => { - it('should create connection and publish activity', async () => { + it('should create connection, seed default permissions and publish activity', async () => { + vi.mocked(prisma.nsecKey!.findFirst!).mockResolvedValue({ + id: 'key-1', + userId: 'user-1', + } as never); + await service.createConnection('user-1', 'key-1', 'client-pub', { name: 'App' }); + + // C1: ownership of the nsec key is verified, scoped by the authenticated userId. + expect(prisma.nsecKey?.findFirst).toHaveBeenCalledWith({ + where: { id: 'key-1', userId: 'user-1' }, + }); expect(prisma.bunkerConnection?.create).toHaveBeenCalledWith( expect.objectContaining({ data: expect.objectContaining({ @@ -97,8 +112,69 @@ describe('ConnectionsService', () => { }), }), ); + // H2: a conservative default permission set is seeded so the connection is not fail-open. + const seedArg = vi.mocked(prisma.connectionPermission!.createMany!).mock.calls[0]?.[0] as { + data: Array<{ connectionId: string; method: string; kind: number | null }>; + }; + expect(seedArg.data.length).toBeGreaterThan(0); + expect(seedArg.data.every((p) => p.connectionId === 'conn-1')).toBe(true); + expect(seedArg.data.every((p) => p.method === 'sign_event')).toBe(true); + expect(seedArg.data.map((p) => p.kind)).toEqual(expect.arrayContaining([0, 1, 3, 4, 7])); expect(eventsService.publishUserActivity).toHaveBeenCalledWith('user-1'); }); + + it('seeds the operator-chosen permissions (granted) instead of the defaults when provided', async () => { + vi.mocked(prisma.nsecKey!.findFirst!).mockResolvedValue({ + id: 'key-1', + userId: 'user-1', + } as never); + + // Operator picked these while generating the bunker:// URI (operator-authoritative seed). + await service.createConnection('user-1', 'key-1', 'client-pub', { name: 'App' }, [ + { method: 'sign_event', kind: 30078 }, + { method: 'nip44_decrypt' }, + ]); + + const seedArg = vi.mocked(prisma.connectionPermission!.createMany!).mock.calls[0]?.[0] as { + data: Array<{ + connectionId: string; + method: string; + kind: number | null; + allowed: boolean; + }>; + }; + expect(seedArg.data).toEqual([ + { connectionId: 'conn-1', method: 'sign_event', kind: 30078, allowed: true }, + { connectionId: 'conn-1', method: 'nip44_decrypt', kind: null, allowed: true }, + ]); + }); + + it('falls back to the default seed when the operator-chosen list is empty', async () => { + vi.mocked(prisma.nsecKey!.findFirst!).mockResolvedValue({ + id: 'key-1', + userId: 'user-1', + } as never); + + await service.createConnection('user-1', 'key-1', 'client-pub', { name: 'App' }, []); + + const seedArg = vi.mocked(prisma.connectionPermission!.createMany!).mock.calls[0]?.[0] as { + data: Array<{ method: string; kind: number | null }>; + }; + expect(seedArg.data.map((p) => p.kind)).toEqual(expect.arrayContaining([0, 1, 3, 4, 7])); + }); + + it('should throw NotFoundException when the nsec key does not belong to the user (C1)', async () => { + // findFirst scoped by { id, userId } returns null for a foreign or non-existent key. + vi.mocked(prisma.nsecKey!.findFirst!).mockResolvedValue(null as never); + + await expect( + service.createConnection('attacker', 'victim-key', 'client-pub', { name: 'Evil' }), + ).rejects.toThrow(NotFoundException); + + // The connection is never created and no key is bound. + expect(prisma.bunkerConnection?.create).not.toHaveBeenCalled(); + expect(prisma.connectionPermission?.createMany).not.toHaveBeenCalled(); + }); }); describe('getConnection', () => { @@ -176,19 +252,113 @@ describe('ConnectionsService', () => { }); describe('setPermissions', () => { - it('should delete existing and create new permissions', async () => { + it('replaces granted permissions while preserving pending requests', async () => { await service.setPermissions('conn-1', [ { method: 'sign_event', kind: 1 }, { method: 'ping' }, ]); + // Only granted rows are cleared; pending rows survive. + expect(prisma.connectionPermission?.deleteMany).toHaveBeenCalledWith({ + where: { connectionId: 'conn-1', allowed: true }, + }); + // Pending rows for the now-granted method/kinds are dropped to avoid a unique conflict. expect(prisma.connectionPermission?.deleteMany).toHaveBeenCalledWith({ - where: { connectionId: 'conn-1' }, + where: { + connectionId: 'conn-1', + allowed: false, + OR: [ + { method: 'sign_event', kind: 1 }, + { method: 'ping', kind: null }, + ], + }, }); expect(prisma.connectionPermission?.createMany).toHaveBeenCalledWith({ data: [ - { connectionId: 'conn-1', method: 'sign_event', kind: 1 }, - { connectionId: 'conn-1', method: 'ping', kind: null }, + { connectionId: 'conn-1', method: 'sign_event', kind: 1, allowed: true }, + { connectionId: 'conn-1', method: 'ping', kind: null, allowed: true }, + ], + }); + }); + }); + + describe('requestPermissions', () => { + it('records requested permissions as PENDING (allowed=false), skipping duplicates', async () => { + await service.requestPermissions('conn-1', [ + { method: 'sign_event', kind: 1 }, + { method: 'nip44_decrypt' }, + ]); + expect(prisma.connectionPermission?.createMany).toHaveBeenCalledWith({ + data: [ + { connectionId: 'conn-1', method: 'sign_event', kind: 1, allowed: false }, + { connectionId: 'conn-1', method: 'nip44_decrypt', kind: null, allowed: false }, ], + skipDuplicates: true, + }); + }); + + it('is a no-op for an empty request', async () => { + await service.requestPermissions('conn-1', []); + expect(prisma.connectionPermission?.createMany).not.toHaveBeenCalled(); + }); + }); + + describe('approveRequests', () => { + it('throws when the connection does not belong to the user', async () => { + vi.mocked(prisma.bunkerConnection!.findUnique!).mockResolvedValue({ + id: 'conn-1', + userId: 'other', + } as never); + await expect(service.approveRequests('conn-1', 'user-1')).rejects.toThrow(NotFoundException); + expect(prisma.connectionPermission?.updateMany).not.toHaveBeenCalled(); + }); + + it('approves all pending requests by flipping allowed to true', async () => { + vi.mocked(prisma.bunkerConnection!.findUnique!).mockResolvedValue({ + id: 'conn-1', + userId: 'user-1', + } as never); + await service.approveRequests('conn-1', 'user-1'); + expect(prisma.connectionPermission?.updateMany).toHaveBeenCalledWith({ + where: { connectionId: 'conn-1', allowed: false }, + data: { allowed: true }, + }); + }); + + it('approves only the given subset when provided', async () => { + vi.mocked(prisma.bunkerConnection!.findUnique!).mockResolvedValue({ + id: 'conn-1', + userId: 'user-1', + } as never); + await service.approveRequests('conn-1', 'user-1', [{ method: 'nip44_decrypt' }]); + expect(prisma.connectionPermission?.updateMany).toHaveBeenCalledWith({ + where: { + connectionId: 'conn-1', + allowed: false, + OR: [{ method: 'nip44_decrypt', kind: null }], + }, + data: { allowed: true }, + }); + }); + }); + + describe('denyRequests', () => { + it('throws when the connection does not belong to the user', async () => { + vi.mocked(prisma.bunkerConnection!.findUnique!).mockResolvedValue({ + id: 'conn-1', + userId: 'other', + } as never); + await expect(service.denyRequests('conn-1', 'user-1')).rejects.toThrow(NotFoundException); + expect(prisma.connectionPermission?.deleteMany).not.toHaveBeenCalled(); + }); + + it('deletes pending requests for the owner', async () => { + vi.mocked(prisma.bunkerConnection!.findUnique!).mockResolvedValue({ + id: 'conn-1', + userId: 'user-1', + } as never); + await service.denyRequests('conn-1', 'user-1'); + expect(prisma.connectionPermission?.deleteMany).toHaveBeenCalledWith({ + where: { connectionId: 'conn-1', allowed: false }, }); }); }); diff --git a/apps/server/src/connections/connections.service.ts b/apps/server/src/connections/connections.service.ts index 003c0be..cdc475e 100644 --- a/apps/server/src/connections/connections.service.ts +++ b/apps/server/src/connections/connections.service.ts @@ -3,7 +3,7 @@ import { PrismaService } from '../prisma/prisma.service.js'; import { EncryptionService } from '../common/crypto/encryption.service.js'; import { EventsService } from '../events/events.service.js'; import type { ConnectionStatus as PrismaConnectionStatus } from '@/generated/prisma/client.js'; -import type { PermissionDescriptor } from '@bunker46/shared-types'; +import { DEFAULT_CONNECTION_PERMISSIONS, type PermissionDescriptor } from '@bunker46/shared-types'; @Injectable() export class ConnectionsService { @@ -44,7 +44,20 @@ export class ConnectionsService { secret?: string; remotePubkey?: string; }, + /** + * Operator-chosen permission seed (granted). When the operator picks permissions while generating + * a bunker:// URI, those are the authoritative grants for the auto-created connection. Omitted (or + * empty) falls back to DEFAULT_CONNECTION_PERMISSIONS so a connection is never left fail-open. + */ + seedPermissions?: PermissionDescriptor[], ) { + // Authorization (C1): bind a connection only to an nsec key the caller actually owns. Without + // this, an authenticated user could create a connection referencing another user's nsecKeyId and + // have the bunker sign/decrypt with the victim's private key. Scope the lookup by userId so a + // foreign or non-existent key is indistinguishable. + const key = await this.prisma.nsecKey.findFirst({ where: { id: nsecKeyId, userId } }); + if (!key) throw new NotFoundException('Key not found'); + const conn = await this.prisma.bunkerConnection.create({ data: { userId, @@ -57,10 +70,31 @@ export class ConnectionsService { remotePubkey: data.remotePubkey, status: 'PENDING', }, - include: { permissions: true }, }); + + // Seed the connection's granted permissions so it is usable under the default-deny RPC handler + // without being fail-open. Prefer the operator's explicit choice (e.g. picked while generating the + // bunker:// URI); otherwise fall back to a conservative default set. Callers with an explicit list + // (controller body.perms / the connect request's perms param) can still adjust these afterwards. + const seed = + seedPermissions && seedPermissions.length > 0 + ? seedPermissions + : DEFAULT_CONNECTION_PERMISSIONS; + await this.prisma.connectionPermission.createMany({ + data: seed.map((p) => ({ + connectionId: conn.id, + method: p.method, + kind: p.kind ?? null, + allowed: true, + })), + }); + await this.eventsService.publishUserActivity(userId); - return conn; + + return this.prisma.bunkerConnection.findUniqueOrThrow({ + where: { id: conn.id }, + include: { permissions: true }, + }); } async listConnections(userId: string) { @@ -95,22 +129,94 @@ export class ConnectionsService { }); } + /** + * Replace the connection's GRANTED permission set (operator-authoritative). Granted rows + * (`allowed = true`) are what the signer actually enforces, as a whitelist under the default-deny + * RPC handler. Pending requests (`allowed = false`) are PRESERVED, so editing the whitelist never + * silently discards a client's outstanding permission request. + */ async setPermissions(connectionId: string, permissions: PermissionDescriptor[]) { - await this.prisma.connectionPermission.deleteMany({ where: { connectionId } }); + await this.prisma.connectionPermission.deleteMany({ where: { connectionId, allowed: true } }); if (permissions.length > 0) { + // Drop any pending row that is now being granted, to satisfy the (connectionId, method, kind) + // unique constraint before inserting the granted rows. + await this.prisma.connectionPermission.deleteMany({ + where: { + connectionId, + allowed: false, + OR: permissions.map((p) => ({ method: p.method, kind: p.kind ?? null })), + }, + }); await this.prisma.connectionPermission.createMany({ data: permissions.map((p) => ({ connectionId, method: p.method, kind: p.kind ?? null, + allowed: true, })), }); } } + /** + * Record permissions a client requested on `connect` as PENDING (`allowed = false`) for the operator + * to approve. Pending rows do NOT grant anything (the handler enforces only `allowed = true`), so a + * client can never self-escalate. Rows that already exist (granted or pending) for the same + * method/kind are skipped, so this never downgrades a granted permission and is idempotent. + */ + async requestPermissions(connectionId: string, permissions: PermissionDescriptor[]) { + if (permissions.length === 0) return; + await this.prisma.connectionPermission.createMany({ + data: permissions.map((p) => ({ + connectionId, + method: p.method, + kind: p.kind ?? null, + allowed: false, + })), + skipDuplicates: true, + }); + } + + /** Operator approves pending permission requests (all, or a given subset), turning them into grants. */ + async approveRequests( + connectionId: string, + userId: string, + permissions?: PermissionDescriptor[], + ) { + const conn = await this.prisma.bunkerConnection.findUnique({ where: { id: connectionId } }); + if (!conn || conn.userId !== userId) throw new NotFoundException(); + await this.prisma.connectionPermission.updateMany({ + where: { + connectionId, + allowed: false, + ...(permissions?.length + ? { OR: permissions.map((p) => ({ method: p.method, kind: p.kind ?? null })) } + : {}), + }, + data: { allowed: true }, + }); + await this.eventsService.publishUserActivity(userId); + } + + /** Operator denies (deletes) pending permission requests (all, or a given subset). */ + async denyRequests(connectionId: string, userId: string, permissions?: PermissionDescriptor[]) { + const conn = await this.prisma.bunkerConnection.findUnique({ where: { id: connectionId } }); + if (!conn || conn.userId !== userId) throw new NotFoundException(); + await this.prisma.connectionPermission.deleteMany({ + where: { + connectionId, + allowed: false, + ...(permissions?.length + ? { OR: permissions.map((p) => ({ method: p.method, kind: p.kind ?? null })) } + : {}), + }, + }); + await this.eventsService.publishUserActivity(userId); + } + async getPermissions(connectionId: string): Promise { const perms = await this.prisma.connectionPermission.findMany({ - where: { connectionId }, + where: { connectionId, allowed: true }, }); return perms.map((p) => ({ method: p.method as PermissionDescriptor['method'], @@ -162,9 +268,20 @@ export class ConnectionsService { await this.prisma.nsecKey.delete({ where: { id: nsecKeyId } }); } - async findByClientPubkey(clientPubkey: string) { + /** + * Resolve the connection for an incoming NIP-46 request by BOTH the client pubkey and the signer + * key the request was addressed to (the relay #p tag / listener key). Binding on the signer key + * (M1) prevents key-confusion: a client that holds connections to several of the user's keys + * cannot have a request addressed to key A served by a connection bound to key B — which could + * sign/decrypt with the wrong key under a different, possibly broader permission set. + */ + async findByClientAndSigner(clientPubkey: string, signerPubkey: string) { return this.prisma.bunkerConnection.findFirst({ - where: { clientPubkey, status: { in: ['ACTIVE', 'PENDING'] } }, + where: { + clientPubkey, + status: { in: ['ACTIVE', 'PENDING'] }, + nsecKey: { publicKey: signerPubkey }, + }, include: { permissions: true, nsecKey: true }, orderBy: { createdAt: 'desc' }, }); diff --git a/apps/server/src/users/users.controller.ts b/apps/server/src/users/users.controller.ts index de24fe3..19c0fba 100644 --- a/apps/server/src/users/users.controller.ts +++ b/apps/server/src/users/users.controller.ts @@ -31,10 +31,16 @@ export class UsersController { @Patch('me/password') @HttpCode(HttpStatus.OK) async changePassword( - @Req() req: FastifyRequest & { user: { sub: string } }, + @Req() req: FastifyRequest & { user: { sub: string; sessionId?: string } }, @Body() body: { currentPassword: string; newPassword: string }, ) { - await this.usersService.updatePassword(req.user.sub, body.currentPassword, body.newPassword); + // Pass the current session id so it is kept while all other sessions are revoked (H1). + await this.usersService.updatePassword( + req.user.sub, + body.currentPassword, + body.newPassword, + req.user.sessionId, + ); return { success: true }; } diff --git a/apps/server/src/users/users.service.spec.ts b/apps/server/src/users/users.service.spec.ts index 73e38a6..d341167 100644 --- a/apps/server/src/users/users.service.spec.ts +++ b/apps/server/src/users/users.service.spec.ts @@ -42,6 +42,9 @@ describe('UsersService', () => { update: vi.fn().mockResolvedValue(mockUser), count: vi.fn().mockResolvedValue(0), }, + session: { + deleteMany: vi.fn().mockResolvedValue({ count: 0 }), + }, }; // Run interactive transactions inline against the same mocked client. prisma.$transaction = vi.fn((cb: (tx: PrismaService) => unknown) => @@ -157,7 +160,7 @@ describe('UsersService', () => { ); }); - it('should update password when current is correct', async () => { + it('should update password and revoke all sessions when no session is kept', async () => { vi.mocked(prisma.user!.findUnique!).mockResolvedValue(mockUser); vi.mocked(prisma.user!.update!).mockResolvedValue({ ...mockUser, passwordHash: 'newhash' }); await usersService.updatePassword('user-1', 'current', 'newpass'); @@ -165,6 +168,25 @@ describe('UsersService', () => { where: { id: 'user-1' }, data: { passwordHash: 'hashed-password' }, }); + // H1: every session is invalidated on password change. + expect(prisma.session?.deleteMany).toHaveBeenCalledWith({ where: { userId: 'user-1' } }); + }); + + it('should keep the current session and revoke the others (H1)', async () => { + vi.mocked(prisma.user!.findUnique!).mockResolvedValue(mockUser); + await usersService.updatePassword('user-1', 'current', 'newpass', 'session-current'); + expect(prisma.session?.deleteMany).toHaveBeenCalledWith({ + where: { userId: 'user-1', id: { not: 'session-current' } }, + }); + }); + + it('should not revoke any session when the current password is wrong', async () => { + vi.mocked(prisma.user!.findUnique!).mockResolvedValue(mockUser); + vi.mocked(argon2.verify).mockResolvedValue(false); + await expect(usersService.updatePassword('user-1', 'wrong', 'newpass')).rejects.toThrow( + UnauthorizedException, + ); + expect(prisma.session?.deleteMany).not.toHaveBeenCalled(); }); }); diff --git a/apps/server/src/users/users.service.ts b/apps/server/src/users/users.service.ts index 12e6770..eec4982 100644 --- a/apps/server/src/users/users.service.ts +++ b/apps/server/src/users/users.service.ts @@ -93,12 +93,23 @@ export class UsersService { userId: string, currentPassword: string, newPassword: string, + keepSessionId?: string, ): Promise { const user = await this.findById(userId); const valid = await argon2.verify(user.passwordHash, currentPassword); if (!valid) throw new UnauthorizedException('Current password is incorrect'); const passwordHash = await argon2.hash(newPassword); - await this.prisma.user.update({ where: { id: userId }, data: { passwordHash } }); + // H1: write the new hash AND invalidate every other refresh-token session atomically, so a crash + // between the two cannot persist the new password while leaving old sessions valid. A leaked or + // stale token must not survive a password change — the canonical post-compromise remediation. + // Access tokens are stateless and expire on their own (~15m); this kills the long-lived refresh + // sessions. The caller's current session is preserved so the user stays signed in here. + await this.prisma.$transaction(async (tx) => { + await tx.user.update({ where: { id: userId }, data: { passwordHash } }); + await tx.session.deleteMany({ + where: keepSessionId ? { userId, id: { not: keepSessionId } } : { userId }, + }); + }); } async updateTotpSecret(userId: string, encryptedSecret: string): Promise { diff --git a/apps/web/src/views/ConnectionDetailView.vue b/apps/web/src/views/ConnectionDetailView.vue index 59dc06c..14a5d57 100644 --- a/apps/web/src/views/ConnectionDetailView.vue +++ b/apps/web/src/views/ConnectionDetailView.vue @@ -25,17 +25,18 @@ const logs = ref([]); const loading = ref(true); const logsLoading = ref(false); -const NIP46_METHODS = [ - 'connect', +// Methods the signer gates behind explicit permission (default-deny). The others +// (connect, ping, get_public_key, switch_relays) are always available. +const GATED_METHODS = [ 'sign_event', - 'ping', - 'get_public_key', 'nip04_encrypt', 'nip04_decrypt', 'nip44_encrypt', 'nip44_decrypt', ]; +type Perm = { method: string; kind?: number | null; allowed?: boolean }; + onMounted(async () => { try { const [conn, logsRes] = await Promise.all([ @@ -51,40 +52,101 @@ onMounted(async () => { } }); -const isUnrestricted = computed(() => { - return !connection.value?.permissions?.length; -}); +const grantedPerms = computed(() => + ((connection.value?.permissions ?? []) as Perm[]).filter((p) => p.allowed), +); +const pendingPerms = computed(() => + ((connection.value?.permissions ?? []) as Perm[]).filter((p) => !p.allowed), +); -function isMethodExplicit(method: string) { - return connection.value?.permissions?.some((p: any) => p.method === method) ?? false; +function permLabel(p: Perm) { + return p.kind != null ? `${p.method}:${p.kind}` : p.method; } -async function togglePermission(method: string) { - if (!connection.value) return; - const current = connection.value.permissions as Array<{ method: string; kind?: number }>; - let updated: Array<{ method: string; kind?: number }>; +// A method-level grant (no kind) authorizes every kind for that method. The toggle reflects ONLY this +// all-kinds state — a connection that has just kind-scoped grants (e.g. sign_event:1) shows the toggle +// OFF, so the control never overstates what is allowed. The specific kinds appear as chips above. +function isMethodAllKinds(method: string) { + return grantedPerms.value.some((p) => p.method === method && p.kind == null); +} + +const wirePerm = (p: Perm) => ({ method: p.method, ...(p.kind != null ? { kind: p.kind } : {}) }); + +async function reloadConnection() { + connection.value = await api.get(`/connections/${route.params.id}`); +} - if (isMethodExplicit(method)) { - updated = current.filter((p: any) => p.method !== method); - } else { - updated = [...current, { method }]; +// Replace the GRANTED whitelist; the backend preserves any pending requests. +async function setGranted(perms: Perm[]) { + await api.put(`/connections/${route.params.id}/permissions`, { + permissions: perms.map(wirePerm), + }); + await reloadConnection(); +} + +// Grant a method for ALL kinds: replace any kind-scoped grants for this method with a single +// method-level (kindless) grant, which the signer treats as "any kind". +async function grantAllKinds(method: string) { + await setGranted([...grantedPerms.value.filter((p) => p.method !== method), { method }]); +} + +// Remove every grant for this method (both the all-kinds row and any kind-scoped rows). +async function revokeMethod(method: string) { + await setGranted(grantedPerms.value.filter((p) => p.method !== method)); +} + +const newKind = ref(''); + +// Grant a single specific event kind (e.g. sign_event:30078) typed by the operator. No-op on an +// invalid number or a kind that is already granted (either specifically or via an all-kinds grant). +async function grantKind(method: string, kindStr: string) { + const kind = Number.parseInt(kindStr, 10); + if (!Number.isInteger(kind) || kind < 0) return; + const alreadyGranted = grantedPerms.value.some( + (p) => p.method === method && (p.kind == null || p.kind === kind), + ); + if (alreadyGranted) { + newKind.value = ''; + return; } + await setGranted([...grantedPerms.value, { method, kind }]); + newKind.value = ''; +} - await api.put(`/connections/${route.params.id}/permissions`, { permissions: updated }); - connection.value.permissions = updated; +async function revokePerm(perm: Perm) { + await setGranted(grantedPerms.value.filter((p) => permLabel(p) !== permLabel(perm))); } -async function setUnrestricted() { - if (!connection.value) return; - await api.put(`/connections/${route.params.id}/permissions`, { permissions: [] }); - connection.value.permissions = []; +async function grantAll() { + await setGranted(GATED_METHODS.map((m) => ({ method: m }))); } -async function setRestrictedDefaults() { - if (!connection.value) return; - const defaults = NIP46_METHODS.map((m) => ({ method: m })); - await api.put(`/connections/${route.params.id}/permissions`, { permissions: defaults }); - connection.value.permissions = defaults; +async function revokeAll() { + await setGranted([]); +} + +async function approvePerm(perm: Perm) { + await api.post(`/connections/${route.params.id}/permissions/approve`, { + permissions: [wirePerm(perm)], + }); + await reloadConnection(); +} + +async function denyPerm(perm: Perm) { + await api.post(`/connections/${route.params.id}/permissions/deny`, { + permissions: [wirePerm(perm)], + }); + await reloadConnection(); +} + +async function approveAllPending() { + await api.post(`/connections/${route.params.id}/permissions/approve`, {}); + await reloadConnection(); +} + +async function denyAllPending() { + await api.post(`/connections/${route.params.id}/permissions/deny`, {}); + await reloadConnection(); } async function toggleLogging() { @@ -232,67 +294,155 @@ function formatTime(ts: string) {

Permissions

+
-

Unrestricted

-

- No permission whitelist configured. All NIP-46 methods are allowed. Click "Restrict to - whitelist" to only allow specific methods. -

+
+

+ {{ pendingPerms.length }} permission request{{ pendingPerms.length > 1 ? 's' : '' }} + awaiting approval +

+
+ + +
+
+
+
+ {{ permLabel(perm) }} +
+ + +
+
+
-
-

Whitelist mode

+
+

Default-deny

- Only explicitly enabled methods are allowed. Toggle individual methods below. + Only the granted permissions below are enforced. connect, + ping, get_public_key and + switch_relays are always available.

-
+ +
+

Granted

+
+ + {{ permLabel(perm) }} + + +
+
+

+ No permissions granted — this connection can only use connect, ping, get_public_key and + switch_relays. +

+ + +
+

Allow all kinds

-
- {{ method }} - allowed -
+ {{ method }}
+ + +
+

Allow a specific sign_event kind

+
+ + +
+
diff --git a/apps/web/src/views/ConnectionsView.vue b/apps/web/src/views/ConnectionsView.vue index 6feff81..2d0fb46 100644 --- a/apps/web/src/views/ConnectionsView.vue +++ b/apps/web/src/views/ConnectionsView.vue @@ -2,7 +2,7 @@ import { ref, watch, onMounted, computed } from 'vue'; import { useActivityStream } from '@/composables/useActivityStream'; import { useRouter, useRoute } from 'vue-router'; -import { Link2, KeyRound, Plug, Lock, Unlock, Search } from '@lucide/vue'; +import { Link2, KeyRound, Plug, Lock, Search } from '@lucide/vue'; import { api } from '@/lib/api'; import { useUiStore } from '@/stores/ui'; import { useFormatting } from '@/composables/useFormatting'; @@ -17,6 +17,9 @@ const route = useRoute(); const ui = useUiStore(); const { formatDateTime } = useFormatting(); +const grantedCount = (c: Connection) => c.permissions.filter((p) => p.allowed).length; +const pendingCount = (c: Connection) => c.permissions.filter((p) => !p.allowed).length; + interface Connection { id: string; name: string; @@ -28,7 +31,7 @@ interface Connection { lastActivity?: string; createdAt: string; nsecKey: { publicKey: string; label: string }; - permissions: Array<{ method: string; kind?: number }>; + permissions: Array<{ method: string; kind?: number; allowed?: boolean }>; _count: { logs: number }; } @@ -84,6 +87,61 @@ const generating = ref(false); const copied = ref(false); const error = ref(''); +// Operator-chosen permission seed for the bunker:// flow. Since the operator generates the URI, they +// are authoritative: these become the connection's GRANTED permissions on connect (default-deny). The +// signer gates these methods; connect/ping/get_public_key/switch_relays are always available. +type PickPerm = { method: string; kind?: number }; +const GATED_METHODS = [ + 'sign_event', + 'nip04_encrypt', + 'nip04_decrypt', + 'nip44_encrypt', + 'nip44_decrypt', +]; +const DEFAULT_SEED: PickPerm[] = [ + { method: 'sign_event', kind: 0 }, + { method: 'sign_event', kind: 1 }, + { method: 'sign_event', kind: 3 }, + { method: 'sign_event', kind: 4 }, + { method: 'sign_event', kind: 7 }, +]; +const seedPerms = ref([...DEFAULT_SEED]); +const newSeedKind = ref(''); + +const permLabel = (p: PickPerm) => (p.kind != null ? `${p.method}:${p.kind}` : p.method); +const isSeedAllKinds = (method: string) => + seedPerms.value.some((p) => p.method === method && p.kind == null); + +function toggleSeedMethod(method: string) { + // Toggle a method-level (all-kinds) grant. Granting all kinds replaces any kind-scoped rows for it. + seedPerms.value = isSeedAllKinds(method) + ? seedPerms.value.filter((p) => p.method !== method) + : [...seedPerms.value.filter((p) => p.method !== method), { method }]; +} + +function removeSeedPerm(p: PickPerm) { + seedPerms.value = seedPerms.value.filter((x) => permLabel(x) !== permLabel(p)); +} + +// Grant every gated method for all kinds, or clear the whole selection. +const allMethodsAllKinds = computed(() => GATED_METHODS.every((m) => isSeedAllKinds(m))); +function grantAllSeed() { + seedPerms.value = GATED_METHODS.map((method) => ({ method })); +} +function revokeAllSeed() { + seedPerms.value = []; +} + +function addSeedKind() { + const kind = Number.parseInt(newSeedKind.value, 10); + if (!Number.isInteger(kind) || kind < 0) return; + const exists = seedPerms.value.some( + (p) => p.method === 'sign_event' && (p.kind == null || p.kind === kind), + ); + if (!exists) seedPerms.value = [...seedPerms.value, { method: 'sign_event', kind }]; + newSeedKind.value = ''; +} + const bunkerUri = ref(''); const uriPreviewImage = ref(''); const creating = ref(false); @@ -177,6 +235,8 @@ function reset() { bunkerUri.value = ''; uriPreviewImage.value = ''; creating.value = false; + seedPerms.value = [...DEFAULT_SEED]; + newSeedKind.value = ''; } function enterMode(m: 'generate' | 'import') { @@ -199,6 +259,7 @@ async function generateBunkerUri() { const res = await api.post<{ uri: string }>('/bunker/generate-bunker-uri', { nsecKeyId: selectedKeyId.value, name: connectionName.value || undefined, + permissions: seedPerms.value, }); generatedUri.value = res.uri; } catch (err) { @@ -312,6 +373,102 @@ function statusVariant(status: string) {
+ + +
+ +

+ Granted to this connection on connect. Anything the client later needs beyond these is + denied and shows up as a request you can approve. connect, ping, get_public_key and + switch_relays are always available. +

+ +
+ + {{ permLabel(perm) }} + + +
+

+ No permissions selected — the connection starts able to do nothing until you grant some. +

+ +
+ +
+ All permissions + +
+
+ + {{ method }} + all kinds + + +
+
+ +
+ + +
+
+

Add an nsec key @@ -512,21 +669,15 @@ function statusVariant(status: string) { />

+ + + {{ grantedCount(conn) }} granted + - - - {{ - conn.permissions.length === 0 - ? 'Unrestricted' - : `${conn.permissions.length} permissions` - }} + {{ pendingCount(conn) }} pending {{ conn._count.logs }} logs Last: {{ formatDateTime(conn.lastActivity) }} diff --git a/docker-compose.yml b/docker-compose.yml index b3a667a..8fb97f1 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -45,6 +45,10 @@ services: JWT_REFRESH_EXPIRES_IN: ${JWT_REFRESH_EXPIRES_IN:-7d} ENCRYPTION_KEY: ${ENCRYPTION_KEY:?set ENCRYPTION_KEY to a strong random value (openssl rand -base64 48)} CORS_ORIGINS: ${CORS_ORIGINS:-http://localhost:8080} + # Defaults to Secure refresh cookies in production. Set COOKIE_SECURE=false when serving over + # plain HTTP on a trusted network (e.g. http://host:8080), otherwise the browser drops the Secure + # cookie and users are logged out on every page reload. Prefer serving over HTTPS. + COOKIE_SECURE: ${COOKIE_SECURE:-} REDIS_URL: redis://redis:6379 WEBAUTHN_RP_NAME: ${WEBAUTHN_RP_NAME:-Bunker46} WEBAUTHN_RP_ID: ${WEBAUTHN_RP_ID:-localhost} diff --git a/packages/shared-types/src/permissions.ts b/packages/shared-types/src/permissions.ts index 838a64d..7d661d2 100644 --- a/packages/shared-types/src/permissions.ts +++ b/packages/shared-types/src/permissions.ts @@ -7,6 +7,23 @@ export const PermissionDescriptorSchema = z.object({ }); export type PermissionDescriptor = z.infer; +/** + * Conservative default permissions seeded on a newly created connection when neither the client + * (via the connect request) nor the operator (via the dashboard) supplies an explicit set. + * + * Bounded to common signing kinds (profile, note, contacts, DM, reaction). It deliberately grants + * NO nip04/nip44 decrypt (nor encrypt), so a fresh connection can never act as a blanket decryption + * oracle on the user's key. The RPC handler is default-deny, so a connection with zero permissions + * can perform no signing/encryption until these — or explicit permissions — are granted. + */ +export const DEFAULT_CONNECTION_PERMISSIONS: readonly PermissionDescriptor[] = [ + { method: 'sign_event', kind: 0 }, // profile metadata + { method: 'sign_event', kind: 1 }, // short text note + { method: 'sign_event', kind: 3 }, // contacts / follow list + { method: 'sign_event', kind: 4 }, // encrypted direct message + { method: 'sign_event', kind: 7 }, // reaction +]; + export function parsePermissionString(perm: string): PermissionDescriptor { const [method, kindStr] = perm.split(':'); const parsed = Nip46Method.parse(method);