Return artifact consent through registered POST callbacks

docs/review-callback-handoff.md

@@ -0,0 +1,13 @@
+# Returning owner consent to review
+
+An optional callback registry maps the authenticated intent's client and callback IDs to one exact HTTPS URL. Configuration rejects credentials, queries, fragments, noncanonical paths, duplicate IDs and duplicate JSON fields. It permits at most 32 entries and a 4 KiB form-action policy. Callback selection never uses a browser-supplied address.
+
+After recording consent, the browser can request `POST /api/review-consent/handoff` with its signed ticket. The existing transport requires the current authenticated session, same origin and a session-bound CSRF token. The backend resolves the signed intent's callback before issuing a completion code and checks the current repository authority and saved consent. The transport reloads the session before returning the result. A missing registry leaves this endpoint unavailable.
+
+The page submits the completion envelope as the sole `completion` field in a native HTML POST form. Codes never enter URL parameters or browser storage. The page's form-action policy allows only the configured destinations and its own origin. It removes the temporary form when CSP blocks submission and shows a retry/status message. A late result after an account change cannot submit a form. The signed ticket stays in a controller closure for a retry; it is not submitted to the callback.
+
+The initial cross-site POST does not carry a review session cookie configured with SameSite=Strict. The review receiver must serve a non-caching landing page, then use a same-origin authenticated request and saved initiating-browser state before exchanging the code. Receiving the POST alone must not establish a binding. That receiver and startup activation are separate unfinished work. This change does not mount the upstream consent router or enable an integration.
+
+Validation covers callback configuration, current-session HTTP access, concurrent logout, signed callback scope, consent requirements, browser POST contents, account changes and malformed completions. The disposable MongoDB suite contains 23 passing tests. The local 100-replay binding benchmark measured median 2.20 ms and p95 2.69 ms on a one-CPU, 1 GiB container; this excludes provider traffic. Browser fixture results use synthetic HTTP backend responses and test TLS certificates, so they do not establish provider or certificate-verification behavior.
+
+The full local suite passed 762 tests, with 74 pending tests for optional fixtures. The targeted browser/page suite passed 17 tests, registry/HTTP tests passed 16 with one optional performance case pending, and TypeScript, targeted lint and the UI build passed. Chromium at 1280 and 320 pixels completed the native cross-site POST with no referrer or Strict cookie, no overflow, no external requests, no script errors and no WCAG A/AA violations. A third case blocked the destination with CSP and verified zero callback deliveries and removal of the code form. The first browser harness waited for a navigation that CSP intentionally blocked; changing the assertion to inspect the current document completed this case without changing application code.

public/partials/reviewConsent.htm

@@ -4,7 +4,8 @@ <h1 class="mt-3">Approve artifact access</h1>
   <p>This records your approval as the repository owner or an authorized coauthor.</p>
   <p v-if="!user?.username">Sign in first, then reopen the artifact link from your submission. <a href="/signin">Sign in</a></p>
   <p v-if="error" role="alert">{{ error }}</p>
-  <p v-if="saved" role="status">Consent recorded. Artifact linking is not complete. Return to your submission to check its status.</p>
+  <p v-if="saved" role="status">Consent recorded. <span v-if="handoffEnabled">Continue to review to finish linking.</span><span v-else>Artifact linking is not complete. Return to your submission to check its status.</span></p>
+  <button v-if="saved && handoffEnabled" class="btn btn-primary" type="button" @click="continueReview" :disabled="busy">Continue to review</button>
   <form v-if="user?.username && !saved" @submit.prevent="loadPreview">
     <label for="review-repository">Anonymous repository ID</label>
     <input id="review-repository" class="form-control" v-model="repositoryId" maxlength="128" autocomplete="off" :disabled="busy || uncertain" aria-describedby="review-repository-help">

public/script/review-consent.js

@@ -13,11 +13,12 @@ export function takeReviewFragment(browser) {
 export function reviewConsentController(state, services) {
   const { http, promises, window: browser } = services;
   let handoff = services.takeReviewHandoff();
+  const scope = handoff ? { clientId: handoff.clientId, intentId: handoff.intentId } : null;
   let ticket, csrf, confirmId, consumeId, identity;
   let active = true, expiryTimer;
   const stop = promises.defer();
   const requestId = () => [...browser.crypto.getRandomValues(new Uint8Array(16))].map(byte => byte.toString(16).padStart(2, "0")).join("");
-  Object.assign(state, { repositoryId: "", busy: false, preview: null, acceptAccess: false, acceptRetention: false, uncertain: false, saved: false, expired: false, error: handoff ? "" : "Open a new artifact link from your submission. This link is missing or invalid." });
+  Object.assign(state, { repositoryId: "", handoffEnabled: false, busy: false, preview: null, acceptAccess: false, acceptRetention: false, uncertain: false, saved: false, expired: false, error: handoff ? "" : "Open a new artifact link from your submission. This link is missing or invalid." });
   const current = () => active && state.user?.username === identity;
   function dispose() {
     active = false; handoff = ticket = csrf = undefined;
@@ -28,7 +29,7 @@ export function reviewConsentController(state, services) {
   state.watch(() => state.user?.username, username => {
     if (!identity && username) identity = username;
     else if (identity && username !== identity) {
-      dispose(); state.preview = null; state.busy = false;
+      dispose(); state.preview = null; state.repositoryId = ""; state.busy = false; state.saved = false; state.handoffEnabled = false;
       state.error = "Your account changed. Open a new artifact link from your submission.";
     }
   });
@@ -44,8 +45,10 @@ export function reviewConsentController(state, services) {
     browser.clearTimeout(expiryTimer);
     consumeId ||= requestId();
     try {
-      csrf = (await http.get("/api/review-consent/csrf", { timeout: stop.promise })).data.csrf;
+      const protection = (await http.get("/api/review-consent/csrf", { timeout: stop.promise })).data;
       if (!current()) return;
+      csrf = protection.csrf;
+      state.handoffEnabled = protection.handoffEnabled === true;
       if (!/^[a-f0-9]{64}$/.test(csrf || "")) throw new Error("invalid_csrf");
       const result = (await http.post("/api/review-consent/preview", { repositoryId, intent: { ...handoff, requestId: consumeId } }, options())).data;
       if (!current()) return;
@@ -64,9 +67,36 @@ export function reviewConsentController(state, services) {
       const result = (await http.post("/api/review-consent/confirm", { ticket, requestId: confirmId, acceptAccess: true, acceptRetention: true }, options())).data;
       if (!current()) return;
       if (result?.requestId !== confirmId || !Number.isFinite(Date.parse(result.confirmedAt))) throw new Error("invalid_receipt");
-      state.saved = true; state.uncertain = false; handoff = ticket = undefined;
+      state.saved = true; state.uncertain = false; handoff = undefined; if (!state.handoffEnabled) ticket = undefined;
     } catch (error) {
       if (current()) { state.uncertain = true; state.error = "The confirmation was not verified. Retry with the same approval, or return to your submission to check its status."; }
     } finally { if (current()) state.busy = false; }
   };
+  state.continueReview = async () => {
+    if (!current() || state.busy || !state.saved || !state.handoffEnabled || !ticket || !scope) return;
+    state.busy = true; state.error = "";
+    let form;
+    const cleanup = () => { form?.remove(); browser.removeEventListener("securitypolicyviolation", blocked); };
+    const blocked = event => {
+      if (event.violatedDirective?.startsWith("form-action")) {
+        cleanup(); if (current()) { state.busy = false; state.error = "The return address was blocked. Return to your submission to check the artifact status."; }
+      }
+    };
+    try {
+      const result = (await http.post("/api/review-consent/handoff", { ticket }, options())).data;
+      if (!current()) return;
+      const completion = result?.completion;
+      const target = new URL(result?.callbackUrl);
+      if (!completion || Object.keys(completion).sort().join(",") !== "clientId,code,contract,expiresAt,intentId" || completion.contract !== "4open.artifacts/1" || completion.clientId !== scope.clientId || completion.intentId !== scope.intentId || !/^[a-f0-9]{64}$/.test(completion.code || "") || !Number.isFinite(Date.parse(completion.expiresAt)) || Date.parse(completion.expiresAt) <= Date.now() || Date.parse(completion.expiresAt) > Date.now() + 300000 || target.protocol !== "https:" || target.href !== result.callbackUrl || target.username || target.password || result.callbackUrl.includes("?") || result.callbackUrl.includes("#")) throw new Error("invalid_handoff");
+      form = browser.document.createElement("form"); form.method = "POST"; form.action = target.href;
+      const input = browser.document.createElement("input"); input.type = "hidden"; input.name = "completion"; input.value = JSON.stringify(completion);
+      form.appendChild(input); browser.document.body.appendChild(form);
+      browser.addEventListener("securitypolicyviolation", blocked);
+      state.on("dispose", cleanup);
+      form.submit();
+    } catch {
+      cleanup(); if (current()) { state.busy = false; state.error = "The return to review could not be prepared. Retry from this page, or return to your submission to check its status."; }
+    }
+  };
+
 }

src/server/review-consent-page.ts

@@ -1,3 +1,4 @@
+import { ReviewCallbacks } from "./service/review-callbacks";
 import { Request, Response } from "express";
 import { existsSync, readFileSync } from "fs";
 import { resolve } from "path";
@@ -17,6 +18,7 @@ export function isReviewConsentPagePath(path: string): boolean {
 
 export function createReviewConsentPage(
   manifestPath = resolve("public", "asset-manifest.json"),
+  callbacks?: ReviewCallbacks,
 ) {
   return (req: Request, res: Response): void => {
     res.set({
@@ -25,7 +27,7 @@ export function createReviewConsentPage(
       "X-Content-Type-Options": "nosniff",
       "X-Frame-Options": "DENY",
       "Content-Security-Policy":
-        "default-src 'self'; script-src 'self'; connect-src 'self'; img-src 'self' data:; style-src 'self' 'unsafe-inline'; font-src 'self' data:; frame-src 'none'; object-src 'none'; base-uri 'none'; form-action 'self'; frame-ancestors 'none'",
+        "default-src 'self'; script-src 'self'; connect-src 'self'; img-src 'self' data:; style-src 'self' 'unsafe-inline'; font-src 'self' data:; frame-src 'none'; object-src 'none'; base-uri 'none'; form-action " + (callbacks?.formAction || "'self'") + "; frame-ancestors 'none'",
     });
     if (
       req.originalUrl !== "/review-link" ||

src/server/service/review-callbacks.ts

@@ -0,0 +1,28 @@
+import { decodeReviewJSON } from "./review-json";
+const invalid = (): never => { throw new Error("Invalid review callback registry"); };
+export function createReviewCallbacks(raw: string) {
+  let value: unknown;
+  try { value = decodeReviewJSON(Buffer.from(raw, "utf8")); } catch { return invalid(); }
+  if (!value || typeof value !== "object" || Array.isArray(value) || Object.keys(value).sort().join(",") !== "callbacks,version") return invalid();
+  const config = value as { version: unknown; callbacks: unknown };
+  if (config.version !== 1 || !Array.isArray(config.callbacks) || !config.callbacks.length || config.callbacks.length > 32) return invalid();
+  const callbacks = new Map<string, string>(), destinations = new Set<string>();
+  for (const item of config.callbacks) {
+    if (!item || typeof item !== "object" || Array.isArray(item) || Object.keys(item).sort().join(",") !== "callbackId,clientId,url") return invalid();
+    const { clientId, callbackId, url } = item;
+    if (![clientId, callbackId].every(id => typeof id === "string" && /^[a-f0-9]{32}$/.test(id)) || typeof url !== "string" || url.length > 2048) return invalid();
+    let parsed: URL;
+    try { parsed = new URL(url); } catch { return invalid(); }
+    if (parsed.protocol !== "https:" || parsed.href !== url || parsed.username || parsed.password || parsed.search || parsed.hash || url.includes("?") || url.includes("#") || !/^\/[A-Za-z0-9_-]+(?:\/[A-Za-z0-9_-]+)*$/.test(parsed.pathname)) return invalid();
+    const key = clientId + ":" + callbackId;
+    if (callbacks.has(key)) return invalid();
+    callbacks.set(key, url); destinations.add(url);
+  }
+  const formAction = "'self' " + [...destinations].join(" ");
+  if (Buffer.byteLength(formAction) > 4096) return invalid();
+  return Object.freeze({
+    formAction,
+    resolve(clientId: string, callbackId: string): string | undefined { return callbacks.get(clientId + ":" + callbackId); },
+  });
+}
+export type ReviewCallbacks = ReturnType<typeof createReviewCallbacks>;

src/server/service/review-consent-http.ts

@@ -1,3 +1,4 @@
+import { ReviewCallbacks } from "./review-callbacks";
 import { createHmac, timingSafeEqual } from "crypto";
 import * as express from "express";
 import { Request, Response } from "express";
@@ -51,6 +52,7 @@ export function createReviewConsentRouter(
   origin: string,
   backend: ReturnType<typeof createReviewOwnerConsent>,
   csrfKey: Buffer,
+  callbacks?: ReviewCallbacks,
 ) {
   try {
     if (new URL(origin).origin !== origin || !origin.startsWith("https://"))
@@ -77,7 +79,7 @@ export function createReviewConsentRouter(
     res.setHeader("Referrer-Policy", "no-referrer");
     res.setHeader("X-Content-Type-Options", "nosniff");
     res.removeHeader("Access-Control-Allow-Origin");
-    if (!["/csrf", "/preview", "/confirm"].includes(req.url))
+    if (!["/csrf", "/preview", "/confirm", ...(callbacks ? ["/handoff"] : [])].includes(req.url))
       return reject(res, 404, "not-found");
     if (req.method !== (req.url === "/csrf" ? "GET" : "POST"))
       return reject(res, 405, "invalid-request");
@@ -180,17 +182,24 @@ export function createReviewConsentRouter(
         return reject(res, 401, "unauthorized");
       // Derive without saving the session: a late CSRF response must never
       // resurrect a session that a concurrent logout has destroyed.
-      res.json({ csrf: csrf(fresh).toString("hex") });
+      res.json({ csrf: csrf(fresh).toString("hex"), handoffEnabled: !!callbacks });
     } catch {
       reject(res, 503, "unavailable");
     }
   });
   router.use(express.json({ limit: 12288, strict: true, inflate: false }));
-  router.post(["/preview", "/confirm"], async (req, res) => {
+  router.post(["/preview", "/confirm", "/handoff"], async (req, res) => {
     const actor = res.locals.consentActor as Context;
     try {
       let result: unknown;
-      if (req.url === "/preview") {
+      if (req.url === "/handoff") {
+        if (!callbacks || !fields(req.body, ["ticket"]) || typeof req.body.ticket !== "string" || !req.body.ticket || req.body.ticket.length > 8192) return reject(res, 400, "invalid-request");
+        const result = await backend.handoff(actor, req.body.ticket, callbacks.resolve);
+        if (res.destroyed || res.headersSent) return;
+        const fresh = await current(req);
+        if (!fresh || fresh.accountId !== actor.accountId || fresh.sessionId !== actor.sessionId) return reject(res, 401, "unauthorized");
+        res.json({ completion: { contract: result.completion.contract, clientId: result.completion.clientId, intentId: result.completion.intentId, code: result.completion.code, expiresAt: result.completion.expiresAt }, callbackUrl: result.callbackUrl });
+      } else if (req.url === "/preview") {
         if (
           !fields(req.body, ["repositoryId", "intent"]) ||
           typeof req.body.repositoryId !== "string" ||

src/server/service/review-owner-consent.ts

@@ -228,7 +228,14 @@ export function createReviewOwnerConsent(
     createHmac("sha256", key)
       .update("4open.review-completion/1." + id + "." + nonce)
       .digest("hex");
-  return Object.freeze({
+  const api = {
+    async handoff(actor: Principal, ticket: string, resolveCallback: (clientId: string, callbackId: string) => string | undefined) {
+      const quote = decode(ticket, principal(actor));
+      const callbackUrl = resolveCallback(quote.intent.clientId, quote.intent.callbackId);
+      if (!callbackUrl) return deny("forbidden");
+      const completion = await api.completion(actor, ticket);
+      return Object.freeze({ completion, callbackUrl });
+    },
     // 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) {
@@ -590,5 +597,6 @@ export function createReviewOwnerConsent(
         },
       );
     },
-  });
+  };
+  return Object.freeze(api);
 }

test/review-callbacks.test.js

@@ -0,0 +1,22 @@
+require('ts-node/register/transpile-only');
+const { expect } = require('chai');
+const { createReviewCallbacks } = require('../src/server/service/review-callbacks');
+const entry = () => ({ clientId:'1'.repeat(32),callbackId:'2'.repeat(32),url:'https://review.example.test/api/v1/artifacts/callback' });
+const raw = callbacks => JSON.stringify({version:1,callbacks});
+describe('registered review callback destinations', () => {
+ it('binds exact HTTPS destinations to both client and callback IDs', () => {
+  const registry=createReviewCallbacks(raw([entry()]));
+  expect(registry.resolve(entry().clientId,entry().callbackId)).equal(entry().url);
+  expect(registry.resolve('3'.repeat(32),entry().callbackId)).equal(undefined);
+  expect(registry.resolve(entry().clientId,'4'.repeat(32))).equal(undefined);
+  expect(registry.formAction).equal("'self' "+entry().url);
+ });
+ it('rejects redirect parameters, URL credentials, aliases and unsafe CSP characters', () => {
+  for(const url of ['http://review.example.test/callback','https://owner:secret@review.example.test/callback','https://review.example.test/callback?','https://review.example.test/callback#','https://review.example.test/callback?redirect=https://other.test','https://review.example.test/a/../callback','https://review.example.test/%63allback','https://review.example.test/callback/','https://review.example.test/callback\n','https://review.example.test/','https://review.example.test/callback;script-src']) expect(()=>createReviewCallbacks(raw([{...entry(),url}]))).to.throw('Invalid review callback registry');
+ });
+ it('rejects duplicate decoded configuration fields and bounded registry overflows', () => {
+  for(const value of [raw([]),raw([entry(),entry()]),raw([{...entry(),extra:'x'}]),raw(Array.from({length:33},(_,i)=>({...entry(),callbackId:i.toString(16).padStart(32,'0')}))),raw([entry()]).replace('"version":1','"version":1,"ver\\u0073ion":1'),'{']) expect(()=>createReviewCallbacks(value)).to.throw('Invalid review callback registry');
+  const entries=Array.from({length:4},(_,i)=>({...entry(),callbackId:String(i).repeat(32),url:'https://review.example.test/'+('x'.repeat(1100))+i}));
+  expect(()=>createReviewCallbacks(raw(entries))).to.throw('Invalid review callback registry');
+ });
+});