Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/organization-settings-verification.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@executor-js/cloud": patch
---

Require authenticator verification for organization settings while preserving shared workspace reads and API-key access.
16 changes: 14 additions & 2 deletions apps/cloud/src/account/account-api.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import { env } from "cloudflare:workers";
import { ADMIN_MFA_COOKIE, readAdminMfaProof } from "../auth/admin-mfa-proof";
import { HttpRouter, HttpServerRequest } from "effect/unstable/http";
import { Effect, Layer } from "effect";
import { Clock, Effect, Layer } from "effect";

import {
AccountProvider,
Expand Down Expand Up @@ -75,6 +77,16 @@ const AccountProviderMiddleware = HttpRouter.middleware<{
// session is `""` (vs `SessionAuthLive`, which keeps the inbound cookie).
const session: Session | null = resolved ? sessionFromSealed(resolved, "") : null;

const proof = resolved
? yield* readAdminMfaProof(
env.WORKOS_COOKIE_PASSWORD,
{ userId: resolved.userId, sessionId: resolved.sessionId },
"verified",
request.cookies[ADMIN_MFA_COOKIE],
yield* Clock.currentTimeMillis,
)
: null;

// Built inside the request body so the WorkOS account service closes
// over the per-request `UserStoreService` (postgres socket) supplied by
// the combined request-scoped layer. `local` keeps that promise: the
Expand All @@ -84,7 +96,7 @@ const AccountProviderMiddleware = HttpRouter.middleware<{
AccountProvider.asEffect(),
workosAccountProvider.pipe(
Layer.provide(ApiKeyService.WorkOS),
Layer.provide(Layer.succeed(AccountCaller)({ session })),
Layer.provide(Layer.succeed(AccountCaller)({ session, adminVerified: proof !== null })),
),
{ local: true },
);
Expand Down
20 changes: 15 additions & 5 deletions apps/cloud/src/account/workos-account-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
// the same `WorkOSClient.authenticateSealedSession` the rest of cloud uses.
export class AccountCaller extends Context.Service<
AccountCaller,
{ readonly session: Session | null }
{ readonly session: Session | null; readonly adminVerified?: boolean }
>()("@executor-js/cloud/AccountCaller") {}

// ---------------------------------------------------------------------------
Expand Down Expand Up @@ -146,6 +146,16 @@
const requireAdmin = (org: { readonly memberRole: "admin" | "member" }) =>
org.memberRole === "admin" ? Effect.void : Effect.fail(new AccountForbidden());

// Settings mutations require MFA; shared member reads and key management do not.
const requireVerifiedSettings = (org: { readonly memberRole: "admin" | "member" }) =>
Effect.gen(function* () {
yield* requireAdmin(org);
if (caller.adminVerified !== true)
return yield* new AccountForbidden({

Check failure on line 154 in apps/cloud/src/account/workos-account-service.ts

View workflow job for this annotation

GitHub Actions / Test

src/auth/mirror-feeders.node.test.ts > account service writes through to the mirror > updateMemberRole writes the role WorkOS returned

AccountForbidden: Verify your identity to change organization settings. ❯ Array.<anonymous> src/account/workos-account-service.ts:154:25 ❯ next ../../node_modules/.bun/effect@4.0.0-beta.59/node_modules/effect/src/internal/effect.ts:1276:25 ❯ Object.~effect/Effect/evaluate ../../node_modules/.bun/effect@4.0.0-beta.59/node_modules/effect/src/internal/effect.ts:1289:23 ❯ FiberImpl.runLoop ../../node_modules/.bun/effect@4.0.0-beta.59/node_modules/effect/src/internal/effect.ts:633:39 ❯ runLoop ../../node_modules/.bun/effect@4.0.0-beta.59/node_modules/effect/src/internal/effect.ts:593:22 ❯ evaluate ../../node_modules/.bun/effect@4.0.0-beta.59/node_modules/effect/src/internal/effect.ts:1030:14 ❯ resume ../../node_modules/.bun/effect@4.0.0-beta.59/node_modules/effect/src/internal/effect.ts:991:15 ⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯ Serialized Error: { _tag: 'AccountForbidden' }

Check failure on line 154 in apps/cloud/src/account/workos-account-service.ts

View workflow job for this annotation

GitHub Actions / Test

src/auth/mirror-feeders.node.test.ts > account service writes through to the mirror > removeMember tombstones the mirror row after the WorkOS delete

AccountForbidden: Verify your identity to change organization settings. ❯ Array.<anonymous> src/account/workos-account-service.ts:154:25 ❯ next ../../node_modules/.bun/effect@4.0.0-beta.59/node_modules/effect/src/internal/effect.ts:1276:25 ❯ Object.~effect/Effect/evaluate ../../node_modules/.bun/effect@4.0.0-beta.59/node_modules/effect/src/internal/effect.ts:1289:23 ❯ FiberImpl.runLoop ../../node_modules/.bun/effect@4.0.0-beta.59/node_modules/effect/src/internal/effect.ts:633:39 ❯ runLoop ../../node_modules/.bun/effect@4.0.0-beta.59/node_modules/effect/src/internal/effect.ts:593:22 ❯ evaluate ../../node_modules/.bun/effect@4.0.0-beta.59/node_modules/effect/src/internal/effect.ts:1030:14 ❯ resume ../../node_modules/.bun/effect@4.0.0-beta.59/node_modules/effect/src/internal/effect.ts:991:15 ⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯ Serialized Error: { _tag: 'AccountForbidden' }

Check failure on line 154 in apps/cloud/src/account/workos-account-service.ts

View workflow job for this annotation

GitHub Actions / Test

src/auth/mirror-feeders.node.test.ts > account service writes through to the mirror > inviteMember mirrors the pending membership WorkOS created for the invitee

AccountForbidden: Verify your identity to change organization settings. ❯ Array.<anonymous> src/account/workos-account-service.ts:154:25 ❯ next ../../node_modules/.bun/effect@4.0.0-beta.59/node_modules/effect/src/internal/effect.ts:1276:25 ❯ Object.~effect/Effect/evaluate ../../node_modules/.bun/effect@4.0.0-beta.59/node_modules/effect/src/internal/effect.ts:1289:23 ❯ FiberImpl.runLoop ../../node_modules/.bun/effect@4.0.0-beta.59/node_modules/effect/src/internal/effect.ts:633:39 ❯ runLoop ../../node_modules/.bun/effect@4.0.0-beta.59/node_modules/effect/src/internal/effect.ts:593:22 ❯ evaluate ../../node_modules/.bun/effect@4.0.0-beta.59/node_modules/effect/src/internal/effect.ts:1030:14 ❯ resume ../../node_modules/.bun/effect@4.0.0-beta.59/node_modules/effect/src/internal/effect.ts:991:15 ⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯ Serialized Error: { _tag: 'AccountForbidden' }
message: "Verify your identity to change organization settings.",
});
});

// Ownership check so an admin can't mutate a membership id from another
// org: the id must name a row the mirror holds for THIS org (any status —
// revoking a pending invite is a delete too). One point read on the
Expand Down Expand Up @@ -390,7 +400,7 @@
inviteMember: (headers, body) =>
Effect.gen(function* () {
const { org } = yield* requireOrganization(headers);
yield* requireAdmin(org);
yield* requireVerifiedSettings(org);
yield* reserveMemberSlot(org.id);
const invitation = yield* workos
.sendInvitation({
Expand Down Expand Up @@ -422,7 +432,7 @@
removeMember: (headers, membershipId) =>
Effect.gen(function* () {
const { org } = yield* requireOrganization(headers);
yield* requireAdmin(org);
yield* requireVerifiedSettings(org);
const membership = yield* assertMembershipInOrg(org.id, membershipId);
yield* workos
.deleteOrgMembership(membershipId)
Expand Down Expand Up @@ -453,7 +463,7 @@
updateMemberRole: (headers, membershipId, roleSlug) =>
Effect.gen(function* () {
const { org } = yield* requireOrganization(headers);
yield* requireAdmin(org);
yield* requireVerifiedSettings(org);
yield* assertMembershipInOrg(org.id, membershipId);
const updated = yield* workos
.updateOrgMembershipRole(membershipId, roleSlug)
Expand All @@ -467,7 +477,7 @@
updateOrgName: (headers, name) =>
Effect.gen(function* () {
const { org } = yield* requireOrganization(headers);
yield* requireAdmin(org);
yield* requireVerifiedSettings(org);
const updated = yield* workos
.updateOrganization(org.id, name)
.pipe(Effect.catchTag("WorkOSError", toAccountError));
Expand Down
2 changes: 2 additions & 0 deletions apps/cloud/src/api/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { UserStoreService } from "../auth/context";
import { WorkOsMirror } from "../auth/workos-mirror";
import { DbService } from "../db/db";
import { makeAccountApiLive } from "../account/account-api";
import { AdminMfaRoutes } from "../auth/admin-mfa-routes";

import { AutumnRoutesLive } from "../extensions/billing/route";
import { CloudDocsLive } from "../extensions/docs";
Expand Down Expand Up @@ -41,6 +42,7 @@ export const makeApiLive = (
Layer.provide(requestScopedMiddleware(requestScopedLive).layer),
);
return Layer.mergeAll(
AdminMfaRoutes.pipe(Layer.provide(requestScopedMiddleware(requestScopedLive).layer)),
makeNonProtectedApiLive(requestScopedLive),
makeOrgApiLive(requestScopedLive),
makeAccountApiLive(requestScopedLive),
Expand Down
89 changes: 89 additions & 0 deletions apps/cloud/src/auth/admin-mfa-proof.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
import { Data, Effect, Option, Schema } from "effect";
import { SignJWT, jwtVerify } from "jose";

/** HttpOnly cookies used only for the administrative verification flow. */
export const ADMIN_MFA_COOKIE = "__Host-executor-admin-mfa";
/** The pending challenge is bound to the same user and WorkOS session. */
export const ADMIN_MFA_CHALLENGE_COOKIE = "__Host-executor-admin-challenge";
/** Verification is session-bound, with a seven-day maximum matching the login cookie. */
export const ADMIN_MFA_TTL_SECONDS = 7 * 24 * 60 * 60;

/** A verified WorkOS session, supplied by the authentication adapter. */
export interface AdminMfaIdentity {
readonly userId: string;
readonly sessionId: string;
}

const Proof = Schema.Struct({
factorId: Schema.String,
challengeId: Schema.String,
mode: Schema.Literals(["enroll", "challenge"]),
exp: Schema.Number,
});
const decodeProof = Schema.decodeUnknownOption(Proof);

/** Signing failures are server failures; invalid input cookies are simply refused. */
export class AdminMfaProofError extends Data.TaggedError("AdminMfaProofError")<{
readonly cause: unknown;
}> {}

type Purpose = "challenge" | "verified";
const issuer = (purpose: Purpose) => `executor:admin-mfa:${purpose}`;
const key = (secret: string) => new TextEncoder().encode(secret);

/** Sign a purpose-specific, session-bound proof with an explicit expiration. */
export const signAdminMfaProof = (
secret: string,
identity: AdminMfaIdentity,
purpose: Purpose,
proof: typeof Proof.Type,
now: number,
) =>
Effect.tryPromise({
try: () =>
new SignJWT({ factorId: proof.factorId, challengeId: proof.challengeId, mode: proof.mode })
.setProtectedHeader({ alg: "HS256" })
.setIssuer(issuer(purpose))
.setSubject(identity.userId)
.setAudience(identity.sessionId)
.setIssuedAt(Math.floor(now / 1000))
.setExpirationTime(proof.exp)
.sign(key(secret)),
catch: (cause) => new AdminMfaProofError({ cause }),
});

/** Reject expired, tampered, cross-user, cross-session, and wrong-purpose proofs. */
export const readAdminMfaProof = (
secret: string,
identity: AdminMfaIdentity,
purpose: Purpose,
token: string | undefined,
now: number,
) => {
if (!token) return Effect.succeed(null);
return Effect.tryPromise({
try: () =>
jwtVerify(token, key(secret), {
algorithms: ["HS256"],
issuer: issuer(purpose),
subject: identity.userId,
audience: identity.sessionId,
requiredClaims: ["exp", "iat", "sub", "aud"],
maxTokenAge: purpose === "challenge" ? 300 : ADMIN_MFA_TTL_SECONDS,
currentDate: new Date(now),
}),
catch: (cause) => new AdminMfaProofError({ cause }),
}).pipe(
Effect.map(({ payload }) => {
const maxAge = purpose === "challenge" ? 300 : ADMIN_MFA_TTL_SECONDS;
if (
typeof payload.iat !== "number" ||
typeof payload.exp !== "number" ||
payload.exp > payload.iat + maxAge
)
return null;
return Option.getOrNull(decodeProof(payload));
}),
Effect.catchTag("AdminMfaProofError", () => Effect.succeed(null)),
);
};
Loading
Loading