diff --git a/packages/realm-server/tests/card-operations-dispatch-test.ts b/packages/realm-server/tests/card-operations-dispatch-test.ts index 3b911eb0dc2..6e971c7ac25 100644 --- a/packages/realm-server/tests/card-operations-dispatch-test.ts +++ b/packages/realm-server/tests/card-operations-dispatch-test.ts @@ -24,6 +24,7 @@ import { type OperationScope, type OperationTarget, } from '@cardstack/runtime-common/card-operations'; +import { GateTrace } from '@cardstack/runtime-common/card-operations/gate-trace'; import { fileContentToBytes } from '@cardstack/runtime-common/stream'; import { urlNamesFile } from '@cardstack/runtime-common/file-def-code-ref'; import type { CodeRef } from '@cardstack/runtime-common/code-ref'; @@ -1751,7 +1752,6 @@ module(basename(import.meta.filename), function () { { kind: 'unattributed' }, 'a scope built without naming a caller has none', ); - assert.strictEqual(newOperationScope(core).proposed, undefined); }); test('a scope with a caller resolves exactly as one without', async function (assert) { @@ -1773,7 +1773,8 @@ module(basename(import.meta.filename), function () { let batch = newOperationScope(core, { caller: scopeCallerFor('@someone:example.com'), }); - let entry = batch.derive({ proposed: { title: 'Q3' } }); + let trace = new GateTrace(); + let entry = batch.derive({ trace }); await batch.peekInstance(url); await entry.peekInstance(url); assert.strictEqual( @@ -1782,18 +1783,18 @@ module(basename(import.meta.filename), function () { 'the two invocations read the row once between them', ); assert.deepEqual(entry.caller, batch.caller); - assert.deepEqual(entry.proposed, { title: 'Q3' }); + assert.strictEqual(entry.trace, trace); assert.strictEqual( - batch.proposed, + batch.trace, undefined, 'deriving leaves the scope it came from as it was', ); let unattributed = entry.derive({ caller: { kind: 'unattributed' } }); assert.deepEqual(unattributed.caller, { kind: 'unattributed' }); assert.strictEqual( - unattributed.proposed, + unattributed.trace, undefined, - 'a proposed document is one invocation’s and does not carry over', + 'a trace is one invocation’s and does not carry over', ); }); diff --git a/packages/realm-server/tests/card-operations-transforms-test.ts b/packages/realm-server/tests/card-operations-transforms-test.ts index 0975aabd97d..52b551f2cb7 100644 --- a/packages/realm-server/tests/card-operations-transforms-test.ts +++ b/packages/realm-server/tests/card-operations-transforms-test.ts @@ -698,16 +698,17 @@ module(basename(import.meta.filename), function () { const realmConfig = async () => ({}); - test('a create’s scope carries the document its stages produced, not the caller’s payload', async function (assert) { + test('a create is staged from the document its stages produced, not the caller’s payload', async function (assert) { let core = stub(); let sent = { lid: 'draft-1', meta: { x: 1 }, note: 'from the caller' }; + let scope = newOperationScope(core, { caller: scopeCallerFor(ACTOR) }); let staged = await stageWriteEntry( { entry: createEntry(sent), target: { kind: 'type', codeRef: PERSON, realm: REALM }, definition: TITLED_CREATE, decision: { kind: 'coarse' }, - scope: newOperationScope(core, { caller: scopeCallerFor(ACTOR) }), + scope, }, { name: 'draft', @@ -717,24 +718,24 @@ module(basename(import.meta.filename), function () { }, ); assert.deepEqual( - staged.scope.proposed, + paramsFor(staged.entry), { note: 'from the caller', title: 'Untitled' }, - 'the proposed document holds the param the input supplied, and ' + - 'not the envelope members a named create’s params leave out', + 'the params hold the one the input supplied, and not the envelope ' + + 'members a named create’s params leave out', ); assert.strictEqual( (sent as Record).title, undefined, 'which the caller never sent', ); - assert.deepEqual( - staged.scope.caller, - { kind: 'user', actor: ACTOR }, - 'and the batch’s caller travels with it', + assert.strictEqual( + staged.scope, + scope, + 'and the entry keeps the scope it was resolved under', ); }); - test('a plain create’s proposed document is the resource it is minted from', async function (assert) { + test('a plain create is staged from the whole resource it is minted from', async function (assert) { let resource = { lid: 'draft-1', attributes: { title: 'Q3' }, @@ -751,13 +752,13 @@ module(basename(import.meta.filename), function () { { name: 'create', params: {}, actor: ACTOR, realmConfig }, ); assert.deepEqual( - staged.scope.proposed, + staged.entry.data, resource, 'the whole resource, local id and type included, as it is staged', ); }); - test('a create the params check refuses stages no proposed document', async function (assert) { + test('a create the params check refuses is not staged', async function (assert) { let { input: _input, ...withoutInput } = TITLED_CREATE; let error = await refusal(() => stageWriteEntry( @@ -774,31 +775,6 @@ module(basename(import.meta.filename), function () { assert.strictEqual(error.code, 'invalid-params'); }); - test('a write that is not a create carries no proposed document', async function (assert) { - let staged = await stageWriteEntry( - { - entry: { - op: 'invoke', - position: 0, - name: 'addComment', - href: CARD, - data: { body: 'hello' }, - }, - target: { kind: 'instance', url: CARD }, - definition: { base: 'transform', deterministic: true }, - decision: { kind: 'coarse' }, - scope: newOperationScope(stub(), { caller: scopeCallerFor(ACTOR) }), - }, - { - name: 'addComment', - params: { body: 'hello' }, - actor: ACTOR, - realmConfig, - }, - ); - assert.strictEqual(staged.scope.proposed, undefined); - }); - test('a batch entry’s projection has to remain a result object', async function (assert) { let entry: EnvelopeEntry = { op: 'invoke', diff --git a/packages/realm-server/tests/realm-policy-anonymous-named-operation-test.ts b/packages/realm-server/tests/realm-policy-anonymous-named-operation-test.ts index 5857677c1ed..1a72f16f06b 100644 --- a/packages/realm-server/tests/realm-policy-anonymous-named-operation-test.ts +++ b/packages/realm-server/tests/realm-policy-anonymous-named-operation-test.ts @@ -37,10 +37,11 @@ import { setupCatalogTestSubset } from './helpers/catalog-test-subset.ts'; // registering for updates. Both are made as the submitter the realm's own // `realm.json` names. Closing a petition is opened to signed-in callers only, // and `claim`, which reads the caller, is opened to everyone but admits only -// signed-in callers. +// signed-in callers. A notice declares `sign` too and no grant names it, and a +// ballot's only grant is made as a key the realm's config doesn't hold. const CIVIC = 'http://127.0.0.1:4444/civic/'; -// A board anyone may read, whose policy opens only a declared write, `sign`, -// to callers who aren't signed in. +// A board anyone may read, whose policy opens only a declared write, `sign` of +// an open petition, to callers who aren't signed in. const BOARD = 'http://127.0.0.1:4444/board/'; const ORG = 'http://127.0.0.1:4444/org/'; const CIVIC_POLICY = `${ORG}policies/civic`; @@ -88,6 +89,18 @@ const CIVIC_MODULE = ` set: { signature: actor() }, }; } + export class Notice extends CardDef { + @field title = contains(StringField); + @field status = contains(StringField); + @field signature = contains(StringField); + + @operation static sign = { + base: 'transform', + params: { name: StringField }, + set: { signature: params('name') }, + }; + } + export class Ballot extends Notice {} export class Signup extends CardDef { @field email = contains(StringField); @field source = contains(StringField); @@ -111,8 +124,11 @@ const SIGNUP = type(CIVIC, 'Signup'); const OPEN = `${CIVIC}petitions/open`; const LOCAL = `${CIVIC}petitions/local`; const CLOSED = `${CIVIC}petitions/closed`; +const NOTICE = `${CIVIC}notices/open`; +const BALLOT = `${CIVIC}ballots/open`; const MISSING = `${CIVIC}petitions/nowhere`; const BOARD_PETITION = `${BOARD}petitions/open`; +const BOARD_CLOSED = `${BOARD}petitions/closed`; function petition(title: string, status: string, name = 'Petition') { return JSON.stringify({ @@ -155,6 +171,9 @@ const POLICY = policyCard([ rule(SIGNUP, [ { operation: 'register', anonymous: true, actingUser: 'submitter' }, ]), + rule(type(CIVIC, 'Ballot'), [ + { operation: 'sign', anonymous: true, actingUser: 'missing' }, + ]), ]); module(basename(import.meta.filename), function (hooks) { @@ -197,6 +216,8 @@ module(basename(import.meta.filename), function (hooks) { 'open', 'LocalPetition', ), + 'notices/open.json': petition('Quiet hours', 'open', 'Notice'), + 'ballots/open.json': petition('Name the park', 'open', 'Ballot'), }, permissions: { [EDITOR]: ['read', 'write', 'realm-owner'], @@ -214,6 +235,7 @@ module(basename(import.meta.filename), function (hooks) { }), 'civic.gts': CIVIC_MODULE, 'petitions/open.json': petition('Paint the fence', 'open'), + 'petitions/closed.json': petition('Fix the gate', 'closed'), }, permissions: { '*': ['read'], @@ -228,7 +250,12 @@ module(basename(import.meta.filename), function (hooks) { 'policies/civic.json': POLICY, 'policies/board.json': policyCard([ rule(type(BOARD, 'Petition'), [ - { operation: 'sign', anonymous: true, actingUser: 'submitter' }, + { + operation: 'sign', + anonymous: true, + actingUser: 'submitter', + where: '.status == "open"', + }, ]), ]), }, @@ -357,7 +384,11 @@ module(basename(import.meta.filename), function (hooks) { return `Bearer ${createJWT(civic, EDITOR, ['read', 'write', 'realm-owner'])}`; } - async function explain(target: string, operation: string) { + async function explain( + target: string, + operation: string, + policy = CIVIC_POLICY, + ) { let response = await request .post(`${new URL(ORG).pathname}_operations`) .set('X-HTTP-Method-Override', 'QUERY') @@ -371,7 +402,7 @@ module(basename(import.meta.filename), function (hooks) { JSON.stringify({ 'boxel:operations': [ invoke('explain', { - href: CIVIC_POLICY, + href: policy, data: { actor: '', target, operation }, }), ], @@ -612,4 +643,40 @@ module(basename(import.meta.filename), function (hooks) { 'a closed petition', ); }); + + test('explain reports a refusal as the caller who is not signed in receives it', async function (assert) { + let refusals: [string, string, string][] = [ + ['a predicate that does not hold', CLOSED, 'predicate-false'], + ['an operation that reads the caller', LOCAL, 'reads-actor'], + ['a type no grant names', NOTICE, 'no-grant'], + ['a grant whose acting user is not in the config', BALLOT, 'no-grant'], + ]; + for (let [label, target, reason] of refusals) { + let explained = await explain(target, 'sign'); + assert.strictEqual(explained.reason, reason, `${label}: ${reason}`); + let response = await sign(target, 'Ada'); + unauthenticated(response, label, assert); + assert.deepEqual( + explained.refusal, + { status: 401, code: 'actor-required' }, + `${label}: explain reports the 401 the realm sends`, + ); + } + // A realm anyone may read tells such a caller to authenticate for a write + // it refused too, and explain says the same. + let explained = await explain(BOARD_CLOSED, 'sign', BOARD_POLICY); + assert.strictEqual(explained.reason, 'predicate-false'); + unauthenticated( + await operations(BOARD, [ + invoke('sign', { href: BOARD_CLOSED, data: { name: 'Ada' } }), + ]), + 'a realm anyone may read', + assert, + ); + assert.deepEqual( + explained.refusal, + { status: 401, code: 'actor-required' }, + 'a realm anyone may read: explain reports the 401 the realm sends', + ); + }); }); diff --git a/packages/runtime-common/card-operations/dispatch.ts b/packages/runtime-common/card-operations/dispatch.ts index 5720895ac48..e320c3f349b 100644 --- a/packages/runtime-common/card-operations/dispatch.ts +++ b/packages/runtime-common/card-operations/dispatch.ts @@ -349,11 +349,13 @@ export interface RunOperationOptions { // snapshot of it. The memo lives for the invocation and no longer: a core is // long-lived and must never hold a card's row across requests. // -// A scope also says who the invocation is for, whether the realm ACL declined -// them, and, for a create, what it would write. Those are the facts the policy -// gate reads that the target does not carry. They travel here rather than as +// A scope also says who the invocation is for and whether the realm ACL +// declined them. Those are the facts the policy gate reads that the target +// does not carry (see `GateScope`). They travel here rather than as // parameters of `resolveOperation` because the scope already reaches every -// place an operation is resolved. +// place an operation is resolved. A create's payload is not among them: a +// create against a type is judged by the card it mints once staged, and a +// named create anchored on a card by that card. export interface OperationScope { peekInstance(url: URL): Promise; readonly caller: ScopeCaller; @@ -361,13 +363,6 @@ export interface OperationScope { // Where it declined an invocation, the policy gate decides whether it runs; // everywhere else the gate does nothing at all. readonly coarseDeclined: CoarseDeclined; - // The payload a `create` entry is staged from, as its `input` stage and its - // `params` check left it — what the operation would actually write, not the - // bytes the caller sent. For a plain create that is the resource the card is - // minted from; for a named one, the params its template is filled with. - // Absent for every other invocation, and for a create until those two - // stages have run. - readonly proposed: Record | undefined; // Where the policy gate records how it reached its decision, for an explain // to report, or for a capability check to read why the gate refused. Absent // on every invocation a caller makes. @@ -390,8 +385,7 @@ export interface OperationScope { // memo so the invocations of one request still cost one read of each row // between them. The caller, the ACL's verdict, the seal and whether the // request is advisory carry over unless named, and so does its route; a - // proposed document belongs to one invocation and never does, and neither - // does a trace. + // trace belongs to one invocation and never does. derive(invocation: ScopeInvocation): OperationScope; } @@ -415,7 +409,6 @@ export type ScopeCaller = export interface ScopeInvocation { caller?: ScopeCaller; coarseDeclined?: CoarseDeclined; - proposed?: Record; trace?: GateTrace; seal?: Error; advisory?: boolean; @@ -466,7 +459,6 @@ export function newOperationScope( let scopeFor = ( caller: ScopeCaller, coarseDeclined: CoarseDeclined, - proposed: Record | undefined, trace: GateTrace | undefined, seal: Error | undefined, advisory: boolean, @@ -475,7 +467,6 @@ export function newOperationScope( peekInstance, caller, coarseDeclined, - proposed, trace, seal, advisory, @@ -485,7 +476,6 @@ export function newOperationScope( scopeFor( next.caller ?? caller, next.coarseDeclined ?? coarseDeclined, - next.proposed, next.trace, next.seal ?? seal, next.advisory ?? advisory, @@ -495,7 +485,6 @@ export function newOperationScope( return scopeFor( invocation.caller ?? { kind: 'unattributed' }, invocation.coarseDeclined ?? 'none', - invocation.proposed, invocation.trace, invocation.seal, invocation.advisory ?? false, diff --git a/packages/runtime-common/card-operations/envelope.ts b/packages/runtime-common/card-operations/envelope.ts index 70f10c1ac17..3e40ac5ed2a 100644 --- a/packages/runtime-common/card-operations/envelope.ts +++ b/packages/runtime-common/card-operations/envelope.ts @@ -652,8 +652,7 @@ export interface ResolvedEnvelopeEntry { // whose admission still rests on a predicate. decision: GateDecision; // This entry's own view of the batch's scope: the batch's caller and row - // memo, and — once `stageWriteEntry` has run — the document a create would - // write. + // memo. scope: OperationScope; } @@ -1011,15 +1010,13 @@ export function entryWithPayload( // payload at all, so a declaration requiring one would be carried out over a // card the caller had not said enough to remove. // -// A create's scope comes back carrying the payload as these two steps left it, -// since that — not what the caller sent — is what the card would be minted -// from. The transformed entry keeps its `position` and the envelope's own -// members; only the payload moves. +// The transformed entry keeps its `position` and the envelope's own members; +// only the payload moves. export async function stageWriteEntry( write: ResolvedEnvelopeEntry, ctx: TransformContext, ): Promise { - let { entry, definition, scope } = write; + let { entry, definition } = write; if (definition.input) { entry = entryWithPayload( entry, @@ -1030,15 +1027,7 @@ export async function stageWriteEntry( name: entry.name, ...(entry.href ? { id: entry.href } : {}), }); - if (definition.base === 'create') { - // Read the way `batchEntryFor` stages it: a named create is filled from - // its params, which leave out the members the envelope reads for itself, - // and a plain create is minted from the whole resource, local id included. - scope = scope.derive({ - proposed: definition.of ? paramsFor(entry) : (entry.data ?? {}), - }); - } - return { ...write, entry, scope }; + return { ...write, entry }; } function own( diff --git a/packages/runtime-common/card-operations/explain.ts b/packages/runtime-common/card-operations/explain.ts index c3821e571a6..365fe37caa8 100644 --- a/packages/runtime-common/card-operations/explain.ts +++ b/packages/runtime-common/card-operations/explain.ts @@ -44,6 +44,7 @@ import { isOperationFailure, isWrite, refusalForNonReader, + refusalSeenBy, type ExplainedGrant, type ExplainedGrantOutcome, type ExplainedIndexLag, @@ -867,7 +868,7 @@ async function explain( refused( base, refusalReason(trace, error.status), - seenBy(coarseDeclined, error), + seenBy(actor, coarseDeclined, error), error.status >= 500 ? 'failed' : 'denied', ), trace, @@ -907,7 +908,7 @@ async function explain( return refused( explained, refusalReason(trace, refusal.status), - seenBy(coarseDeclined, refusal), + seenBy(actor, coarseDeclined, refusal), threw ? 'failed' : 'denied', ); } @@ -926,6 +927,11 @@ async function explain( // this is follows from the behavior the operation resolves to. That is // resolved first, as a caller the ACL allows would resolve it. An operation // that does not resolve travels as a read would. +// +// The realm asks whether a caller it declined a write could read the realm +// only of a signed-in caller, so a write from a caller who isn't signed in is +// declined outright even where anyone may read the realm, and is judged and +// answered as such. async function coarseDeclinedFor( core: OperationCore, target: OperationTarget, @@ -948,7 +954,7 @@ async function coarseDeclinedFor( if (writes ? acl.write : acl.read) { return 'none'; } - return acl.read ? 'writes' : 'all'; + return acl.read && actor.kind === 'user' ? 'writes' : 'all'; } function refused( @@ -960,13 +966,16 @@ function refused( return { ...explanation, decision, reason, refusal }; } -// The refusal as the actor would receive it. A caller who may not read the -// realm is told that a card they were refused is not there. +// The refusal as the actor would receive it (see `refusalSeenBy`). function seenBy( + actor: ScopeCaller, coarseDeclined: CoarseDeclined, error: OperationError, ): { status: number; code: OperationError['code'] } { - let seen = coarseDeclined === 'all' ? refusalForNonReader(error) : error; + let seen = refusalSeenBy(error, { + readDeclined: coarseDeclined === 'all', + signedIn: actor.kind === 'user', + }); return { status: seen.status, code: seen.code }; } diff --git a/packages/runtime-common/card-operations/index.ts b/packages/runtime-common/card-operations/index.ts index ddee16cba04..b5ea8ad81b4 100644 --- a/packages/runtime-common/card-operations/index.ts +++ b/packages/runtime-common/card-operations/index.ts @@ -243,7 +243,9 @@ export { linkStrategyOf, readLinkStrategyOf, refusalForNonReader, + refusalSeenBy, unshareableFormatsOf, + AUTHENTICATION_REQUIRED, } from './types.ts'; export type { BaseOperation, diff --git a/packages/runtime-common/card-operations/types.ts b/packages/runtime-common/card-operations/types.ts index 0c157fda7c5..cec0dee7c60 100644 --- a/packages/runtime-common/card-operations/types.ts +++ b/packages/runtime-common/card-operations/types.ts @@ -1565,3 +1565,41 @@ export function refusalForNonReader(error: OperationError): OperationError { ...(entry !== undefined ? { meta: { entry } } : {}), }; } + +// What a realm with a policy tells a request that authenticated nobody, +// wherever nothing admits it: to authenticate. The detail is the realm's +// missing-credentials message (`AuthenticationErrorMessages.MissingAuthHeader`). +export const AUTHENTICATION_REQUIRED: OperationError = { + status: 401, + code: 'actor-required', + title: 'Authentication required', + detail: 'Missing Authorization header', +}; + +// An operation's refusal as its caller is told it. A caller the realm ACL +// would not let read the realm is never told which cards exist (see +// `refusalForNonReader`), and one of them who isn't signed in is told to +// authenticate wherever such a caller is told nothing is there, since that is +// the answer every request that authenticated nobody gets. A caller the ACL +// lets read the realm is told the refusal as it is, where the realm asks: it +// asks whether a caller it declined a write could read only of a signed-in +// caller, so a visitor's refused write is told as a non-reader's even on a +// realm anyone may read. +// +// The operations envelope and the card+json writes answer their callers with +// this, and an explain reports it as the refusal its actor would receive. The +// card+json read and HEAD and the byte serves apply `refusalForNonReader` +// themselves, and give a caller who isn't signed in the same answer by their +// own route. +export function refusalSeenBy( + error: OperationError, + caller: { readDeclined: boolean; signedIn: boolean }, +): OperationError { + if (!caller.readDeclined) { + return error; + } + let seen = refusalForNonReader(error); + return !caller.signedIn && seen.code === 'target-not-found' + ? AUTHENTICATION_REQUIRED + : seen; +} diff --git a/packages/runtime-common/realm.ts b/packages/runtime-common/realm.ts index 87601601c16..08e372a7a38 100644 --- a/packages/runtime-common/realm.ts +++ b/packages/runtime-common/realm.ts @@ -364,6 +364,8 @@ import { isSourceResult, isWrite, refusalForNonReader, + refusalSeenBy, + AUTHENTICATION_REQUIRED, type EntryPosition, type OperationError, type OperationRequest, @@ -2401,15 +2403,6 @@ interface RequestDispatch { handle: () => Promise; } -// What a realm with a policy tells a request that authenticated nobody, wherever -// nothing admits it: to authenticate. -const AUTHENTICATION_REQUIRED: OperationError = { - status: 401, - code: 'actor-required', - title: 'Authentication required', - detail: AuthenticationErrorMessages.MissingAuthHeader, -}; - // A route as `Realm.routeDescriptions` lists it: a router route, the hashed // stylesheet serve under its path prefix, or the fallback file and module // serve for one method, which answers whatever path and media type no router @@ -7313,16 +7306,10 @@ export class Realm { error: OperationError, requestContext: RequestContext, ): OperationError { - if (this.#coarseDeclined(requestContext) !== 'all') { - return error; - } - let seen = refusalForNonReader(error); - // A caller who isn't signed in is told to authenticate wherever a caller - // who may not read the realm is told nothing is there, so the answer is - // the one every request that authenticated nobody gets. - return requestContext.anonymousCaller && seen.code === 'target-not-found' - ? AUTHENTICATION_REQUIRED - : seen; + return refusalSeenBy(error, { + readDeclined: this.#coarseDeclined(requestContext) === 'all', + signedIn: !requestContext.anonymousCaller, + }); } // Who an operation dispatched from an HTTP request is running for. The actor