diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 662fe08..9a49f8e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -61,6 +61,7 @@ jobs: env: TEST_REVIEW_CONSENT_MONGO: mongodb://127.0.0.1:28743/?replicaSet=review-consent-test TEST_REVIEW_CONSENT_PERF_REPORT: review-consent-perf.json + TEST_REVIEW_COMPLETION_PERF_REPORT: review-completion-perf.json TEST_REVIEW_CONSENT_HTTP_PERF: review-consent-http-perf.json run: npx mocha test/review-owner-consent.test.js test/review-consent-http.test.js - uses: actions/upload-artifact@v4 @@ -69,6 +70,7 @@ jobs: path: | review-consent-perf.json review-consent-http-perf.json + review-completion-perf.json - name: Remove disposable database if: always() run: docker rm -f review-consent-test || true diff --git a/docs/review-completion-exchange.md b/docs/review-completion-exchange.md new file mode 100644 index 0000000..6337d5a --- /dev/null +++ b/docs/review-completion-exchange.md @@ -0,0 +1,15 @@ +# Completion codes and binding receipts + +The owner consent backend now saves the authenticated review intent with its immutable consent receipt. Existing consent records that lack this scope cannot issue completion codes. No caller may supply replacement submission, entitlement or policy fields. + +`completion(actor, ticket)` requires the same current browser account and session, a matching confirmed consent and current repository authority. It issues one code per intent, expiring at the earliest of five minutes, intent expiry or retention expiry. Repeating issuance returns the same code without extending its lifetime. The database stores a random nonce and a code hash. A domain-separated HMAC using the private consent signing key makes the code recoverable after response loss without storing it in plaintext. Retain that key for the code lifetime; changing it fails closed for outstanding issuance retries. + +`exchange(authenticatedClientId, command)` requires a separately verified service client. The argument is an internal authority boundary, not authentication by itself. First exchange checks the code, expiry, current account and repository authority, then atomically inserts a pending binding, a client-scoped request key and the original receipt. A later write failure rolls all three back. Competing transactions may report unavailable; callers explicitly retry the original command. The backend never automatically repeats a transaction. + +Exact authenticated exchanges recover the original receipt after response loss, code expiry or later owner changes. They cannot create another binding. Changed codes under the committed request ID conflict, a spent code cannot be used with another request ID, and an existing request ID cannot bind another intent. `receipt(authenticatedClientId, intentId)` provides post-expiry reconciliation without accepting an expired code for first use. Both operations remain client scoped and expose only protocol identifiers and policy. They never return source coordinates, account IDs or repository IDs. + +Bindings remain pending. No snapshot, artifact access, retained bytes or entitlement fulfillment is claimed. Browser completion POSTs, callback registration, service HTTP authentication, private configuration, startup wiring and activation remain separate tasks. The existing browser confirmation response intentionally does not expose the completion code yet. + +Receipt recovery uses majority reads outside transactions. The factory limits active authority transactions and service reads to four each. MongoDB queries have three-second limits, majority writes have a three-second acknowledgment timeout and commits have a five-second limit. The serving connection must also have bounded selection/socket/pool waits. Secrets and encoded completion payloads must never be logged. + +Tests use a disposable local MongoDB replica set and cover current ownership, absent consent, cross-session and cross-client attempts, altered codes, expiry, explicit replay, concurrent exchange, request-key reuse, rollback and persistence without raw tokens or codes. Set `TEST_REVIEW_CONSENT_MONGO` to the disposable replica-set URI and run `npx mocha test/review-owner-consent.test.js test/review-consent-http.test.js`. `TEST_REVIEW_COMPLETION_PERF_REPORT` records 100 immutable receipt replays; it excludes browser, TLS and provider latency. diff --git a/src/server/service/review-owner-consent.ts b/src/server/service/review-owner-consent.ts index 4897fe8..8c816a4 100644 --- a/src/server/service/review-owner-consent.ts +++ b/src/server/service/review-owner-consent.ts @@ -1,4 +1,4 @@ -import { createHash, createHmac, timingSafeEqual } from "crypto"; +import { createHash, createHmac, timingSafeEqual, randomBytes } from "crypto"; import { Connection, ClientSession, Types } from "mongoose"; import { ConsumedReviewIntent, @@ -37,6 +37,27 @@ type SavedConsent = { requestId: string; ticketHash: string; receipt: ConsentReceipt; + intent: ConsumedReviewIntent; +}; +type BindingReceipt = { + contract: "4open.artifacts/1"; + clientId: string; + intentId: string; + bindingId: string; + submissionRef: string; + policy: ConsumedReviewIntent["policy"]; + entitlementId: string; +}; +type SavedCompletion = { + _id: string; + accountId: string; + repositoryId: string; + ticketHash: string; + intent: ConsumedReviewIntent; + nonce: string; + codeHash: string; + expiresAt: string; + exchange?: { requestId: string; receipt: BindingReceipt }; }; export class ReviewConsentError extends Error { constructor( @@ -79,6 +100,7 @@ export function createReviewOwnerConsent( if (!Buffer.isBuffer(signingKey) || signingKey.length !== 32) deny("invalid"); const key = Buffer.from(signingKey); let activeTransactions = 0; + let activeReads = 0; const signature = (body: string) => createHmac("sha256", key) .update("4open.review-consent/1." + body) @@ -202,7 +224,252 @@ export function createReviewOwnerConsent( await session.endSession().catch(() => undefined); } } + const completionCode = (id: string, nonce: string) => + createHmac("sha256", key) + .update("4open.review-completion/1." + id + "." + nonce) + .digest("hex"); return Object.freeze({ + // Called only with the current authenticated browser principal. The HTTP + // adapter must retain its session reload, same-origin and CSRF checks. + async completion(actor: Principal, ticket: string) { + const who = principal(actor), + quote = decode(ticket, who); + return transaction( + who, + { _id: new Types.ObjectId(quote.repositoryId) }, + async (authority, session) => { + decode(ticket, who); + const id = quote.intent.clientId + ":" + quote.intent.intentId; + const consent = await connection.db + .collection("review_owner_consents") + .findOne({ _id: id }, { session, maxTimeMS: 3000 }); + if ( + !consent || + consent.ticketHash !== hash(ticket) || + consent.accountId !== who.accountId || + consent.repositoryId !== authority.repositoryId || + !consent.intent + ) + return deny("forbidden"); + const collection = + connection.db.collection("review_completions"); + let saved = await collection.findOne( + { _id: id }, + { session, maxTimeMS: 3000 }, + ); + if ( + saved && + (saved.ticketHash !== consent.ticketHash || + saved.accountId !== who.accountId || + saved.repositoryId !== authority.repositoryId) + ) + return deny("conflict"); + if (!saved) { + const nonce = randomBytes(32).toString("hex"); + saved = { + _id: id, + accountId: who.accountId, + repositoryId: authority.repositoryId, + ticketHash: consent.ticketHash, + intent: consent.intent, + nonce, + codeHash: hash(completionCode(id, nonce)), + expiresAt: new Date( + Math.floor( + Math.min( + Date.now() + 300000, + Date.parse(consent.intent.expiresAt), + Date.parse(consent.intent.policy.retainUntil), + ) / 1000, + ) * 1000, + ) + .toISOString() + .replace(/\.000Z$/, "Z"), + }; + await collection.insertOne(saved, { session, maxTimeMS: 3000 }); + } + if (Date.parse(saved.expiresAt) <= Date.now()) return deny("expired"); + const code = completionCode(id, saved.nonce); + // A key rotation must not silently issue a different code for a saved + // intent. Keep the issuing key available for the short code lifetime. + if (hash(code) !== saved.codeHash) return deny("conflict"); + return Object.freeze({ + contract: "4open.artifacts/1", + clientId: saved.intent.clientId, + intentId: saved.intent.intentId, + code, + expiresAt: saved.expiresAt, + }); + }, + ); + }, + // authenticatedClientId must come from service credential verification, + // never browser identity or the request body's clientId alone. + async exchange( + authenticatedClientId: string, + input: { + contract: string; + clientId: string; + intentId: string; + code: string; + requestId: string; + }, + ): Promise { + if ( + !input || + Object.keys(input).sort().join(",") !== + "clientId,code,contract,intentId,requestId" || + input.contract !== "4open.artifacts/1" || + ![input.clientId, input.intentId, input.requestId].every( + (value) => typeof value === "string" && /^[a-f0-9]{32}$/.test(value), + ) || + typeof input.code !== "string" || + !/^[a-f0-9]{64}$/.test(input.code) + ) + return deny("invalid"); + const command = { ...input }; + if (authenticatedClientId !== command.clientId) return deny("forbidden"); + if (connection.readyState !== 1 || activeReads >= 4) + return deny("unavailable"); + const id = command.clientId + ":" + command.intentId; + const collection = + connection.db.collection("review_completions"); + const read = async (session?: ClientSession) => { + const saved = await collection.findOne( + { _id: id }, + { session, maxTimeMS: 3000, ...(session ? {} : { readConcern: { level: "majority" as const } }) }, + ); + if (!saved) return deny("forbidden"); + if ( + !timingSafeEqual( + Buffer.from(saved.codeHash, "hex"), + Buffer.from(hash(command.code), "hex"), + ) + ) + return deny( + saved.exchange?.requestId === command.requestId + ? "conflict" + : "forbidden", + ); + return saved; + }; + activeReads++; + try { + const before = await read(); + if (before.exchange) { + if (before.exchange.requestId !== command.requestId) + return deny("conflict"); + return Object.freeze(before.exchange.receipt); + } + return await transaction( + { accountId: before.accountId, sessionHash: "" }, + { _id: new Types.ObjectId(before.repositoryId) }, + async (_authority, session) => { + const saved = await read(session); + if (saved.exchange) { + if (saved.exchange.requestId !== command.requestId) + return deny("conflict"); + return Object.freeze(saved.exchange.receipt); + } + if ( + Date.parse(saved.expiresAt) <= Date.now() || + Date.parse(saved.intent.policy.retainUntil) <= Date.now() + ) + return deny("expired"); + // The unique request document detects a reused key on another intent. + const requests = connection.db.collection<{ + _id: string; + intentId: string; + }>("review_completion_requests"); + const requestKey = command.clientId + ":" + command.requestId; + if ( + await requests.findOne( + { _id: requestKey }, + { session, maxTimeMS: 3000 }, + ) + ) + return deny("conflict"); + const receipt: BindingReceipt = { + contract: "4open.artifacts/1", + clientId: saved.intent.clientId, + intentId: saved.intent.intentId, + bindingId: randomBytes(16).toString("hex"), + submissionRef: saved.intent.submissionRef, + policy: { ...saved.intent.policy }, + entitlementId: saved.intent.entitlementId, + }; + await connection.db + .collection<{ + _id: string; + clientId: string; + intentId: string; + accountId: string; + repositoryId: string; + state: "pending"; + revision: number; + receipt: BindingReceipt; + }>("review_artifact_bindings") + .insertOne( + { + _id: receipt.bindingId, + clientId: command.clientId, + intentId: command.intentId, + accountId: saved.accountId, + repositoryId: saved.repositoryId, + state: "pending", + revision: 1, + receipt, + }, + { session, maxTimeMS: 3000 }, + ); + await requests.insertOne( + { _id: requestKey, intentId: command.intentId }, + { session, maxTimeMS: 3000 }, + ); + await collection.updateOne( + { _id: id }, + { $set: { exchange: { requestId: command.requestId, receipt } } }, + { session, maxTimeMS: 3000 }, + ); + return Object.freeze(receipt); + }, + ); + } catch (error) { + if (error instanceof ReviewConsentError) throw error; + return deny("unavailable"); + } finally { + activeReads--; + } + }, + async receipt( + authenticatedClientId: string, + intentId: string, + ): Promise { + if ( + ![authenticatedClientId, intentId].every( + (value) => typeof value === "string" && /^[a-f0-9]{32}$/.test(value), + ) + ) + return deny("invalid"); + if (connection.readyState !== 1 || activeReads >= 4) + return deny("unavailable"); + activeReads++; + try { + const saved = await connection.db + .collection("review_completions") + .findOne( + { _id: authenticatedClientId + ":" + intentId }, + { maxTimeMS: 3000, readConcern: { level: "majority" }, projection: { exchange: 1 } }, + ); + if (!saved?.exchange) return deny("forbidden"); + return Object.freeze(saved.exchange.receipt); + } catch (error) { + if (error instanceof ReviewConsentError) throw error; + return deny("unavailable"); + } finally { + activeReads--; + } + }, async preview( actor: Principal, repositoryName: string, @@ -315,6 +582,7 @@ export function createReviewOwnerConsent( requestId, ticketHash, receipt, + intent: quote.intent, }, { session, maxTimeMS: 3000 }, ); diff --git a/test/review-owner-consent.test.js b/test/review-owner-consent.test.js index eb45004..8329e99 100644 --- a/test/review-owner-consent.test.js +++ b/test/review-owner-consent.test.js @@ -69,7 +69,13 @@ describe("review owner consent transactions", function () { maxPoolSize: 10, }) .asPromise(); - await connection.db.createCollection("review_owner_consents"); + for (const name of [ + "review_owner_consents", + "review_completions", + "review_completion_requests", + "review_artifact_bindings", + ]) + await connection.db.createCollection(name); }); after(async () => { if (connection) { @@ -79,9 +85,14 @@ describe("review owner consent transactions", function () { }); beforeEach(async () => { await Promise.all( - ["users", "anonymizedrepositories", "review_owner_consents"].map((name) => - connection.db.collection(name).deleteMany({}), - ), + [ + "users", + "anonymizedrepositories", + "review_owner_consents", + "review_completions", + "review_completion_requests", + "review_artifact_bindings", + ].map((name) => connection.db.collection(name).deleteMany({})), ); await connection.db.collection("users").insertMany([ { @@ -506,4 +517,270 @@ describe("review owner consent transactions", function () { ) + "\n", ); }); + async function completed() { + const quoted = await store.preview( + actor(), + "synthetic-repository", + request(), + ); + await store.confirm(actor(), quoted.ticket, confirmation()); + const completion = await store.completion(actor(), quoted.ticket); + const command = { + contract: completion.contract, + clientId: completion.clientId, + intentId: completion.intentId, + code: completion.code, + requestId: "9".repeat(32), + }; + return { quoted, completion, command }; + } + it("issues one recoverable completion and atomically creates one pending binding", async function () { + const { quoted, completion, command } = await completed(); + expect(completion.code).match(/^[a-f0-9]{64}$/); + expect(Date.parse(completion.expiresAt) - Date.now()).within(1, 300000); + expect(await store.completion(actor(), quoted.ticket)).deep.equal( + completion, + ); + const receipt = await store.exchange(command.clientId, command); + expect(receipt.submissionRef).equal(intent().submissionRef); + expect(receipt.entitlementId).equal(intent().entitlementId); + expect(receipt.policy).deep.equal(intent().policy); + expect(receipt.bindingId).match(/^[a-f0-9]{32}$/); + expect(await store.exchange(command.clientId, command)).deep.equal(receipt); + expect(await store.receipt(command.clientId, command.intentId)).deep.equal( + receipt, + ); + await rejected( + store.exchange(command.clientId, { ...command, code: "0".repeat(64) }), + "conflict", + ); + expect( + await connection.db + .collection("review_artifact_bindings") + .countDocuments({}), + ).equal(1); + const saved = await connection.db + .collection("review_artifact_bindings") + .findOne({}); + expect(saved.state).equal("pending"); + for (const name of [ + "review_owner_consents", + "review_completions", + "review_completion_requests", + "review_artifact_bindings", + ]) { + const text = JSON.stringify( + await connection.db.collection(name).find({}).toArray(), + ); + expect(text) + .not.include(completion.code) + .not.include(request().token) + .not.include(actor().sessionId) + .not.include(quoted.ticket); + } + }); + it("rejects missing consent, another session, another service client and altered codes", async function () { + const quoted = await store.preview( + actor(), + "synthetic-repository", + request(), + ); + await rejected(store.completion(actor(), quoted.ticket), "forbidden"); + await store.confirm(actor(), quoted.ticket, confirmation()); + await rejected( + store.completion(actor(actorId, "t".repeat(32)), quoted.ticket), + "forbidden", + ); + const completion = await store.completion(actor(), quoted.ticket); + const command = { + contract: completion.contract, + clientId: completion.clientId, + intentId: completion.intentId, + code: completion.code, + requestId: "9".repeat(32), + }; + await rejected(store.exchange("f".repeat(32), command), "forbidden"); + await rejected( + store.exchange(command.clientId, { ...command, code: "0".repeat(64) }), + "forbidden", + ); + await rejected( + store.receipt("f".repeat(32), command.intentId), + "forbidden", + ); + expect( + await connection.db + .collection("review_artifact_bindings") + .countDocuments({}), + ).equal(0); + }); + it("recovers committed receipts after expiry but rejects first use of expired codes", async function () { + const { command } = await completed(); + const receipt = await store.exchange(command.clientId, command); + await connection.db + .collection("review_completions") + .updateMany({}, { $set: { expiresAt: "2000-01-01T00:00:00Z" } }); + await connection.db + .collection("users") + .updateOne({ _id: actorId }, { $set: { status: "disabled" } }); + expect(await store.exchange(command.clientId, command)).deep.equal(receipt); + expect(await store.receipt(command.clientId, command.intentId)).deep.equal( + receipt, + ); + await rejected( + store.exchange(command.clientId, { + ...command, + requestId: "a".repeat(32), + }), + "conflict", + ); + await connection.db + .collection("users") + .updateOne({ _id: actorId }, { $set: { status: "active" } }); + await connection.db + .collection("review_completions") + .updateMany({}, { $unset: { exchange: "" } }); + await rejected(store.exchange(command.clientId, command), "expired"); + }); + it("checks current ownership before a first exchange", async function () { + const { command } = await completed(); + await connection.db + .collection("anonymizedrepositories") + .updateOne({ _id: repoId }, { $set: { owner: otherId, coauthors: [] } }); + await rejected(store.exchange(command.clientId, command), "forbidden"); + expect( + await connection.db + .collection("review_artifact_bindings") + .countDocuments({}), + ).equal(0); + expect( + await connection.db + .collection("review_completion_requests") + .countDocuments({}), + ).equal(0); + }); + it("serializes concurrent exchanges and retains one immutable binding", async function () { + const { command } = await completed(); + const results = await Promise.allSettled([ + store.exchange(command.clientId, command), + store.exchange(command.clientId, command), + ]); + const successes = results.filter((result) => result.status === "fulfilled"); + expect(successes.length).greaterThan(0); + for (const result of results) + if (result.status === "rejected") + expect(result.reason.kind).equal("unavailable"); + const receipt = await store.exchange(command.clientId, command); + successes.forEach((result) => expect(result.value).deep.equal(receipt)); + expect( + await connection.db + .collection("review_artifact_bindings") + .countDocuments({}), + ).equal(1); + }); + it("does not reuse an exchange request ID for a different intent", async function () { + const first = await completed(); + await store.exchange(first.command.clientId, first.command); + upstream = async () => ({ ...intent(), intentId: "b".repeat(32) }); + const quoted = await store.preview(actor(), "synthetic-repository", { + ...request(), + intentId: "b".repeat(32), + }); + await store.confirm(actor(), quoted.ticket, confirmation()); + const second = await store.completion(actor(), quoted.ticket); + await rejected( + store.exchange(second.clientId, { + ...first.command, + intentId: second.intentId, + code: second.code, + }), + "conflict", + ); + expect( + await connection.db + .collection("review_artifact_bindings") + .countDocuments({}), + ).equal(1); + }); + it("measures 100 immutable binding receipt replays", async function () { + const { command } = await completed(); + const receipt = await store.exchange(command.clientId, command); + const times = []; + for (let i = 0; i < 100; i++) { + const start = process.hrtime.bigint(); + expect(await store.exchange(command.clientId, command)).deep.equal( + receipt, + ); + times.push(Number(process.hrtime.bigint() - start) / 1e6); + } + times.sort((a, b) => a - b); + if (process.env.TEST_REVIEW_COMPLETION_PERF_REPORT) + require("fs").writeFileSync( + process.env.TEST_REVIEW_COMPLETION_PERF_REPORT, + JSON.stringify( + { + calls: 100, + workload: + "MongoDB immutable binding receipt replay, disposable loopback replica set", + medianMs: times[49], + p95Ms: times[94], + }, + null, + 2, + ) + "\n", + ); + }); + + it("rolls back a binding when a later transaction write fails", async function () { + const { command } = await completed(); + const original = connection.db.collection.bind(connection.db); + connection.db.collection = (...args) => { + const collection = original(...args); + if (args[0] === "review_completion_requests") + collection.insertOne = async () => { + throw new Error("synthetic write failure"); + }; + return collection; + }; + try { + await rejected(store.exchange(command.clientId, command), "unavailable"); + } finally { + connection.db.collection = original; + } + expect( + await connection.db + .collection("review_artifact_bindings") + .countDocuments({}), + ).equal(0); + expect( + await connection.db + .collection("review_completion_requests") + .countDocuments({}), + ).equal(0); + expect( + (await connection.db.collection("review_completions").findOne({})) + .exchange, + ).equal(undefined); + expect((await store.exchange(command.clientId, command)).bindingId).match( + /^[a-f0-9]{32}$/, + ); + }); + it('never extends an issued completion or substitutes a code after key rotation', async function () { + const { quoted, completion } = await completed(); + const rotated = createReviewOwnerConsent(connection, { consume: async () => intent() }, randomBytes(32)); + await rejected(rotated.completion(actor(), quoted.ticket), 'invalid'); + const saved = await connection.db.collection('review_completions').findOne({}); + expect(saved.codeHash).equal(require('crypto').createHash('sha256').update(completion.code).digest('hex')); + await connection.db.collection('review_completions').updateMany({}, { $set: { expiresAt: '2000-01-01T00:00:00Z' } }); + await rejected(store.completion(actor(), quoted.ticket), 'expired'); + expect((await connection.db.collection('review_completions').findOne({})).expiresAt).equal('2000-01-01T00:00:00Z'); + }); + it('rejects legacy consent without its authenticated intent scope', async function () { + const quoted = await store.preview(actor(), 'synthetic-repository', request()); + await store.confirm(actor(), quoted.ticket, confirmation()); + await connection.db.collection('review_owner_consents').updateMany({}, { $unset: { intent: '' } }); + await rejected(store.completion(actor(), quoted.ticket), 'forbidden'); + expect(await connection.db.collection('review_completions').countDocuments({})).equal(0); + }); + });