fix: align the legacy and optional network paths with V3 authority
LEG-01. AuthSessionPort.recover now takes the request's lifetime context, and the raw recovery helper returns data only. The sign-out notification moved to the site that adopts the result, so a recovery that answers after the deadline or a caller abort is observed and discarded instead of logging the user out of a request nobody is waiting on. LEG-02. The V2 client shares V3's credential admission validator instead of checking the allowed set alone. A bearer profile whose patch omits, empties, duplicates or corrupts Authorization now fails closed with zero fetches rather than dispatching an anonymous request under an authenticated profile. OPT-NET-01. A cursor loader rejection is re-thrown exactly as it is with no signal at all. Only a signal that has actually aborted classifies the outcome as PAGINATION_ABORTED, so a real upstream failure stops being filed as a user cancellation. OPT-NET-02. defineMutationIntent and the V3 admission site now share the single isValidIdempotencyKey authority, closing the drift that let a control character through intent definition. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
f4bfdf0365
commit
ca210d3bc5
+85
-17
@@ -18,11 +18,24 @@ import {
|
||||
durationBucket,
|
||||
statusGroup,
|
||||
} from "../../contracts/diagnostics.ts";
|
||||
import type { AuthSessionPort } from "../../application/ports/auth-session-port.ts";
|
||||
import type {
|
||||
AuthSessionPort,
|
||||
CredentialOperationContext,
|
||||
} from "../../application/ports/auth-session-port.ts";
|
||||
import { isValidIdempotencyKey } from "../../contracts/mutation-intent.ts";
|
||||
|
||||
/** Sentinel for a credential wait ended by the attempt lifetime. */
|
||||
const ATTEMPT_ABORTED = Symbol("ATTEMPT_ABORTED");
|
||||
|
||||
/**
|
||||
* LEG-02. The pre-V2 operations have no installed auth profile, so this is the
|
||||
* legacy allowance the shared validator applies to them. It requires nothing,
|
||||
* which preserves their existing behaviour exactly.
|
||||
*/
|
||||
const LEGACY_ALLOWED_CREDENTIAL_HEADERS = Object.freeze([
|
||||
"authorization",
|
||||
"x-csrf-token",
|
||||
] as const);
|
||||
import type { ClockPort } from "../../application/ports/clock-port.ts";
|
||||
import type { DiagnosticsPort } from "../../application/ports/diagnostics-port.ts";
|
||||
import type { TelemetryPort } from "../../application/ports/telemetry-port.ts";
|
||||
@@ -30,6 +43,7 @@ import type { ApiOperation } from "../../contracts/api-operations.ts";
|
||||
import type { ApiFailure } from "../../contracts/errors.ts";
|
||||
import type { OperationRequestInput } from "./request-builder.ts";
|
||||
import { readBoundedJson } from "./bounded-json.ts";
|
||||
import { admitCredentialHeaders } from "./http-contract-bridge.ts";
|
||||
import type { MappingResult } from "../../contracts/boundary-mapper.ts";
|
||||
import {
|
||||
createRestProviderProfile,
|
||||
@@ -328,14 +342,29 @@ export function createHttpClient(
|
||||
authSession.onUnauthenticated();
|
||||
return finalize(outcome, "failed");
|
||||
}
|
||||
let recovered: Awaited<ReturnType<typeof recoverSession>>;
|
||||
let recovered: SessionRecoveryOutcome;
|
||||
// LEG-01. The recovery collaborator receives the request lifetime, and
|
||||
// the transport races the same signal so a non-cooperative owner cannot
|
||||
// hold the request open.
|
||||
const recoveryLifetime = new AbortController();
|
||||
try {
|
||||
recovered = await withinLogicalDeadline(
|
||||
recoverSession(authSession, operation, outcome.error),
|
||||
recoverSession(
|
||||
authSession,
|
||||
operation,
|
||||
outcome.error,
|
||||
Object.freeze({
|
||||
signal: recoveryLifetime.signal,
|
||||
deadlineAtMonotonicMs: deadlineAt,
|
||||
}),
|
||||
),
|
||||
deadlineAt,
|
||||
input.signal,
|
||||
);
|
||||
} catch (error) {
|
||||
// The request is over. Whatever the recovery answers next is observed
|
||||
// by its own owner, never adopted here.
|
||||
recoveryLifetime.abort();
|
||||
return finalize(
|
||||
{
|
||||
ok: false,
|
||||
@@ -356,7 +385,14 @@ export function createHttpClient(
|
||||
error instanceof LogicalDeadlineError ? "failed" : "aborted",
|
||||
);
|
||||
}
|
||||
if (!recovered.ok) return finalize(recovered, "failed");
|
||||
if (!recovered.ok) {
|
||||
// The result is adopted here, so the notification happens here.
|
||||
if (recovered.notifyUnauthenticated) authSession.onUnauthenticated();
|
||||
return finalize(
|
||||
{ ok: false, error: recovered.error },
|
||||
"failed",
|
||||
);
|
||||
}
|
||||
if (operation.idempotency === "none") {
|
||||
return finalize(
|
||||
{
|
||||
@@ -641,15 +677,30 @@ export function createHttpClient(
|
||||
};
|
||||
}
|
||||
const patch = raced;
|
||||
for (const [name, value] of Object.entries(patch.headers)) {
|
||||
const normalized = name.toLowerCase();
|
||||
const allowedHeaders =
|
||||
// LEG-02. The same admission validator V3 uses. Checking only the
|
||||
// allowed set let a bearer profile dispatch with no Authorization at
|
||||
// all, which is precisely the anonymous downgrade the required set
|
||||
// exists to prevent.
|
||||
const admission = admitCredentialHeaders(patch.headers, {
|
||||
allowedCredentialHeaders:
|
||||
security?.auth.allowedCredentialHeaders ??
|
||||
(["authorization", "x-csrf-token"] as const);
|
||||
if (!allowedHeaders.includes(normalized as never)) {
|
||||
throw new TypeError("Credential patch contains a forbidden header");
|
||||
}
|
||||
headers.set(normalized, value);
|
||||
LEGACY_ALLOWED_CREDENTIAL_HEADERS,
|
||||
requiredCredentialHeaders:
|
||||
security?.auth.requiredCredentialHeaders ?? [],
|
||||
});
|
||||
if (!admission.ok) {
|
||||
return {
|
||||
ok: false,
|
||||
error: failure(
|
||||
"AUTH_INTEGRATION_FAILURE",
|
||||
operation.operationId,
|
||||
attempt,
|
||||
{ code: "AUTH_ATTACH_FAILED" },
|
||||
),
|
||||
};
|
||||
}
|
||||
for (const [name, value] of Object.entries(admission.headers)) {
|
||||
headers.set(name, value);
|
||||
}
|
||||
} catch {
|
||||
// An ordinary owner rejection stays an integration failure.
|
||||
@@ -939,20 +990,36 @@ async function parseResponse(
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* LEG-01. Recovery returns data only.
|
||||
*
|
||||
* The sign-out notification is a user-visible side effect, so it belongs to
|
||||
* whoever adopts this result — not to the raw recovery call. A recovery that
|
||||
* loses the race against the deadline or a caller abort still settles, and
|
||||
* signing the user out then would attribute a request nobody is waiting on to
|
||||
* an expired session.
|
||||
*/
|
||||
type SessionRecoveryOutcome =
|
||||
| Readonly<{ ok: true }>
|
||||
| Readonly<{
|
||||
ok: false;
|
||||
error: HttpFailure;
|
||||
notifyUnauthenticated: boolean;
|
||||
}>;
|
||||
|
||||
async function recoverSession(
|
||||
authSession: HttpAuthSession,
|
||||
operation: ApiOperation,
|
||||
originalFailure: HttpFailure,
|
||||
): Promise<
|
||||
Readonly<{ ok: true }> | Readonly<{ ok: false; error: HttpFailure }>
|
||||
> {
|
||||
context: CredentialOperationContext,
|
||||
): Promise<SessionRecoveryOutcome> {
|
||||
try {
|
||||
const result = await authSession.recover();
|
||||
const result = await authSession.recover(context);
|
||||
if (result === "restored") return { ok: true };
|
||||
if (result === "no-session") {
|
||||
authSession.onUnauthenticated();
|
||||
return {
|
||||
ok: false,
|
||||
notifyUnauthenticated: true,
|
||||
error: failure("AUTH_REQUIRED", operation.operationId, originalFailure.attemptCount - 1, {
|
||||
code: "AUTH_REQUIRED",
|
||||
httpStatus: 401,
|
||||
@@ -965,6 +1032,7 @@ async function recoverSession(
|
||||
|
||||
return {
|
||||
ok: false,
|
||||
notifyUnauthenticated: false,
|
||||
error: failure(
|
||||
"AUTH_INTEGRATION_FAILURE",
|
||||
operation.operationId,
|
||||
|
||||
@@ -6,7 +6,9 @@ import {
|
||||
import type { CacheScopeSnapshot } from "../../contracts/server-state-scope.ts";
|
||||
import {
|
||||
defineMutationIntent,
|
||||
MUTATION_INTENT_BOUNDS,
|
||||
// OPT-NET-02. One shared key authority, so the intent factory and this
|
||||
// admission site cannot drift apart.
|
||||
isValidIdempotencyKey,
|
||||
type MutationIntent,
|
||||
} from "../../contracts/mutation-intent.ts";
|
||||
import {
|
||||
@@ -342,7 +344,7 @@ function validateMutationIntent(
|
||||
}
|
||||
|
||||
const key = validated.idempotencyKey;
|
||||
if (requiresKey && !validIdempotencyKey(key)) {
|
||||
if (requiresKey && !isValidIdempotencyKey(key)) {
|
||||
return Object.freeze({
|
||||
ok: false,
|
||||
violation: "MISSING_IDEMPOTENCY_KEY",
|
||||
@@ -357,31 +359,6 @@ function validateMutationIntent(
|
||||
return Object.freeze({ ok: true, intent: validated });
|
||||
}
|
||||
|
||||
const UTF8 = new TextEncoder();
|
||||
|
||||
function validIdempotencyKey(value: unknown): value is string {
|
||||
return (
|
||||
typeof value === "string" &&
|
||||
value.trim().length > 0 &&
|
||||
UTF8.encode(value).byteLength <=
|
||||
MUTATION_INTENT_BOUNDS.idempotencyKeyMaxBytes &&
|
||||
!hasControlCharacter(value)
|
||||
);
|
||||
}
|
||||
|
||||
function hasControlCharacter(value: string): boolean {
|
||||
for (const character of value) {
|
||||
const codePoint = character.codePointAt(0) ?? 0;
|
||||
if (
|
||||
codePoint <= 0x1f ||
|
||||
(codePoint >= 0x7f && codePoint <= 0x9f)
|
||||
) {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
export function createContractHttpExecutor(
|
||||
dependencies: ContractHttpExecutorDependencies,
|
||||
): ContractHttpExecutor {
|
||||
|
||||
@@ -12,6 +12,11 @@ const ABORTED = Symbol("PAGINATION_ABORTED");
|
||||
* Resolves as soon as the operation settles or the signal aborts, whichever
|
||||
* comes first. A late operation result is observed and discarded, never thrown
|
||||
* as an unhandled rejection.
|
||||
*
|
||||
* OPT-NET-01. A loader rejection is *not* an abort. The presence of a signal
|
||||
* says nothing about why the loader failed, so a rejection is re-thrown exactly
|
||||
* as it would be with no signal at all; only a signal that has actually
|
||||
* aborted classifies the outcome as cancellation.
|
||||
*/
|
||||
async function raceAbort<Value>(
|
||||
operation: Promise<Value>,
|
||||
@@ -20,7 +25,7 @@ async function raceAbort<Value>(
|
||||
operation.catch(() => {});
|
||||
if (!signal) return await operation;
|
||||
if (signal.aborted) return ABORTED;
|
||||
return await new Promise<Value | typeof ABORTED>((resolve) => {
|
||||
return await new Promise<Value | typeof ABORTED>((resolve, reject) => {
|
||||
const onAbort = () => resolve(ABORTED);
|
||||
signal.addEventListener("abort", onAbort, { once: true });
|
||||
operation.then(
|
||||
@@ -28,9 +33,13 @@ async function raceAbort<Value>(
|
||||
signal.removeEventListener("abort", onAbort);
|
||||
resolve(value);
|
||||
},
|
||||
() => {
|
||||
(reason: unknown) => {
|
||||
signal.removeEventListener("abort", onAbort);
|
||||
resolve(ABORTED);
|
||||
if (signal.aborted) {
|
||||
resolve(ABORTED);
|
||||
return;
|
||||
}
|
||||
reject(reason);
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
@@ -9,7 +9,16 @@ export type SessionGateway = Readonly<{
|
||||
subscribe(listener: () => void): () => void;
|
||||
beginSignIn(returnTo?: string): Promise<void>;
|
||||
signOut(): Promise<void>;
|
||||
recover(): Promise<"restored" | "no-session">;
|
||||
/**
|
||||
* LEG-01. Recovery is part of a request's lifetime, so it receives the same
|
||||
* context a credential attach does. The context is optional for one release
|
||||
* to keep existing owners working; the transport races the signal either way,
|
||||
* and a recovery that answers after the request already ended is observed but
|
||||
* never turned into a user-visible sign-out.
|
||||
*/
|
||||
recover(
|
||||
context?: CredentialOperationContext,
|
||||
): Promise<"restored" | "no-session">;
|
||||
}>;
|
||||
|
||||
export type CredentialRequestBinding = Readonly<{
|
||||
|
||||
@@ -70,11 +70,11 @@ export function defineMutationIntent(intent: MutationIntent): MutationIntent {
|
||||
intent.canonicalInputIdentity,
|
||||
MUTATION_INTENT_BOUNDS.canonicalInputIdentityMaxBytes,
|
||||
) ||
|
||||
// OPT-NET-02. Intent definition and executor admission share one key
|
||||
// authority; a second, looser rule here is how a control character reaches
|
||||
// an `Idempotency-Key` header.
|
||||
(intent.idempotencyKey !== undefined &&
|
||||
!validBoundedString(
|
||||
intent.idempotencyKey,
|
||||
MUTATION_INTENT_BOUNDS.idempotencyKeyMaxBytes,
|
||||
)) ||
|
||||
!isValidIdempotencyKey(intent.idempotencyKey)) ||
|
||||
!Number.isFinite(intent.createdAtMonotonicMs) ||
|
||||
intent.createdAtMonotonicMs < 0
|
||||
) {
|
||||
|
||||
Reference in New Issue
Block a user