ADR 0043 — In-app workspace membership (`workspace_members`)¶

Status: Accepted (2026-09-05)
Date: 2026-09-04
Deciders: @TheurgicDuke771
Amends: ADR 0032 — the env sign-up allowlist stops being the whole member list and becomes a starting seed plus an emergency way back in. ADR 0033 — the Admin page can now also decide who is in, not only what each person can do; no capability in its matrix changes.
Related: ADR 0026 (a PAT acts as its owner, so membership must be checked when the PAT is used, not only when it is created), 0027 (per-suite sharing, unchanged), 0041 (every change here is audited), 0010 (we never read anything provider-specific).

Amended 2026-09-05, after the post-merge review of the build. Four decisions below were written before the code existed and the code disagreed with them in ways that mattered:

Decision 1 — the one normalization rule now lives in core/identity.py, not otp_service.normalize_email. It is shared by the auth seam, the OTP service and the membership service, and a rule that lives inside one of its callers cannot be shared without an import cycle.

Decision 3 — the env grant is derived inside the predicate rather than passed in by each door. Passing it made the grant-only rule a per-caller decision, and two of the four callers passed False: an env-listed address could sign in and then be refused on every following request. The predicate now computes one tri-state — granted / denied / no allowlist — over the OTP allowlist, the OIDC allowlist and WORKSPACE_ADMIN_EMAILS. Only allowlists belonging to a mode the deployment actually runs can deny; an Azure-AD workspace, which has no app-side allowlist, keeps exactly today's behaviour.

Decision 5 — the exemption keys on Settings.dev_bypass_active, not dev_bypass_allowed. The latter stays true beside email OTP, where the ladder selects OTP and the bypass identity is never minted, so exempting on it would switch the gate off on a real OTP deployment. This is why the narrowing exists and belongs here rather than in a comment in the module.

Decision 8 — the last-admin guard is cross-table. Guarding each table on its own left a two-step gap: remove B's membership (A is still a stored admin), then demote A (users still shows two), and nobody who can sign in is an admin. One admin_service.assert_admin_remains counts admins who are stored-role admins and still members, locking users before workspace_members. The switch-on import additionally takes a transaction advisory lock that concurrent sign-ins take shared, so it cannot read users a moment before a first sign-in commits and leave that person unimported.

One consequence to state plainly: enforcement_active (the table is non-empty) and enforced (a door is actually checking) are different questions, and under developer bypass the second is false. A missing workspace_members table — an image rolled ahead of its migration — reads as "not enforced" with a WARNING rather than raising, because an empty table means the same thing.

Context¶

An admin cannot add a member today. A users row appears only when someone signs in for the first time, and who is allowed to sign in is set outside the app: OIDC_ALLOWED_EMAILS/_DOMAINS for generic OIDC, AUTH_OTP_ALLOWED_EMAILS/_DOMAINS for email OTP (there, the allowlist is the member list), and plain tenant membership for Azure AD, which has no app-side check at all. "Add Priya" means: edit env vars, restart, then wait until she signs in before you can even give her a role. ADR 0033 let admins manage the people already inside; nobody controls the door.

Removing someone works differently at each door, and you cannot see that. Checked against the code:

Credential	Where it is resolved	Is membership re-checked?
Generic-OIDC bearer	`core.auth._resolve_generic_oidc_user` → `_oidc_access_allowed`	Yes, every request — a removed address gets 403 on its next call
Azure AD bearer	`core.auth._get_current_user_real` → `_upsert_user`	No app-side check at all — the tenant is the only gate
OTP session cookie	`session_service.resolve_token`	No — only hash, revoked, expiry; a live `dq_sess_` cookie works until it expires
PAT (`dq_live_`, REST and `/mcp`)	`api_key_service.resolve_token`	No — only hash, revoked, expiry; membership is never looked at after the key is created

So removing an address from AUTH_OTP_ALLOWED_EMAILS stops new sign-in codes (otp_service.is_signup_eligible, at both the request and the verify step) and stops nothing else. The person keeps their browser session until it expires and keeps every PAT they hold for ever — and a PAT reaches all 50 MCP tools, including the ones that change things. That is why this is a security fix, not a convenience.

Scope (user direction, 2026-08-29): DataQ controls its own door, nothing more. The account at the identity provider must already exist. We do not call Entra, Cognito or any provisioning API, we do not do SCIM, and we do not create accounts anyone can sign in with. Adding a member means "this person may enter this workspace once their IdP lets them sign in". The UI must say that plainly in OIDC and Azure modes, or admins will read "Add member" as "create account".

Decision¶

One new table, workspace_members: id, email (normalized, unique on lower(email) like uq_users_email_lower; the one normalization rule is core.identity.normalize_email), initial_role, source (admin or auto_import, CHECK-constrained — see decision 8), invited_by (FK to users.id, null for imported rows), created_at. No FK to users: a member is admitted before a user row exists, and the row must survive the user leaving. Additive migration; users is untouched.
Admin add/remove at /admin/members, behind the existing admin role gate, audited (workspace_member.add / .remove with actor and before/after). Logs carry counts, domains and digests, never a bare email address — the same rule the OTP code already follows.
The switch is whether the table is empty. There is no flag. Empty table ⇒ exactly today's behaviour for every auth mode, so an upgrade changes nothing and locks nobody out. Non-empty table ⇒ a person is a member if they are in the env allowlist OR in the table, checked on every request. The env half is computed by the predicate itself, never handed to it by a door — see the amendment above. A separate flag was rejected because a flag and the data can disagree, and nobody notices when they do.
Four places to enforce it, one per credential kind:
core.auth._upsert_user — Azure AD and generic OIDC, REST and MCP. Every identity sign-in passes through here, including the _claim_unlinked_user branch, which is only reachable from inside it.
session_service.resolve_token — OTP browser sessions.
api_key_service.resolve_token — PATs, REST and /mcp. MCP's _PatOrJwtVerifier.verify_token calls this, and mcp.auth.resolve_current_user then just loads the user row; that load is not a second check and must not be mistaken for one.
otp_service.is_signup_eligible — both callers (code request and code verify), so the check that stops the email is the check that stops the code being used.

The HTTP status differs by door, and pretending otherwise would be wrong. On REST it is 403 with a membership reason, as the OIDC sign-up gate already does: the credential itself is valid, and a 401 would send the SPA round the sign-in loop for ever. At /mcp it is 401, because _PatOrJwtVerifier.verify_token swallows the resolver's error and returns nothing, which FastMCP reports as "not authenticated". Changing that verifier is out of scope; this is recorded so it is not a surprise. On both doors the reason goes in the audit log, which is where an operator answers "why did their PAT stop working?".

Dev-bypass is exempt, and the exemption lives inside the check. _upsert_user is also how the dev-bypass user is created, on REST and on MCP. A blanket gate there would make the local and eval stacks unbootable. The exemption keys on Settings.dev_bypass_active — the mode the auth ladder actually selected — not on comparing against DEV_BYPASS_EMAIL, which an attacker on a real deployment could send, and not on dev_bypass_allowed, which stays true beside email OTP.
The check normalizes the email itself. The generic-OIDC path normalizes before calling _upsert_user; the Azure REST and MCP paths pass preferred_username / upn / email through as-is. A lookup on a lower(email) index that trusts its caller would admit or refuse people based on letter case. Normalize at the check.
The env allowlists become a starting seed plus an emergency way in, and can only ever grant. This is exactly how WORKSPACE_ADMIN_EMAILS already works (ADR 0033 decision 6). An env entry can let someone in; it can never remove someone the table admits. That is what makes the switch safe: fill the table and find a gap, and an env entry restores access; empty the table and you are back to today.

ADR 0032's boot check stays as it is, on purpose. Settings._validate_otp_auth refuses to start OTP mode with an empty AUTH_OTP_ALLOWED_*, and OTP mode is selected by that block. So in OTP mode the env allowlist must stay non-empty even after the table is filled. We do not relax it to "allowlist OR table", because the first admin has to come from somewhere: with both empty, nobody can sign in to write the first row. The change is about authority (env entries can only grant), not presence. The known cost: removing an env-listed address still means an env edit and a restart, same as for WORKSPACE_ADMIN_EMAILS. Seed the minimum, add the rest in-app.

Three guards against locking the workspace out of itself.
The first row written imports every existing users row as a member, in the same transaction, marked as provisional. Turning enforcement on can never throw out a current user. It is part of the first insert, not a migration or a background job, otherwise it is a race. But the import is a safety net, not proof that those people belong: nothing deletes users rows today, so it re-admits everyone who ever signed in, including someone who left and still holds a PAT. So imported rows get source = 'auto_import' (a deliberate add gets 'admin'), the Members page shows them under a "review imported members" banner until an admin confirms or removes each one, and the count is shown at switch-on. Filtering the import by last_seen_at was rejected: a quiet current member and a departed one look the same by that signal, and a safety net that guesses is not a safety net. Importing everyone and flagging them is safe in the direction that matters — too many provisional members is visible and one click from correct; too few silently locks a real member out.
Removing the last admin-role member is refused by the SAME guard the role editor uses, over both tables: decide from a SELECT … FOR UPDATE lock on users then workspace_members, count stored-role admins who are still members (an allowlist admin can disappear on the next deploy, so it cannot be what keeps the workspace recoverable). Two guards, one per table, leave a two-step gap.
Removing yourself needs an explicit confirmation — allowed, since an admin may be leaving after a handover, but never as a quiet click.
initial_role sets the role only when the user row is first created. After that the Members role editor is the source of truth, and an existing row is never downgraded on sign-in (ADR 0033 decision 7). This cannot reuse _upsert_user's role= parameter: that parameter is written into both the insert values and the on-conflict update, so passing initial_role through it would silently overwrite an in-app role change on every sign-in. It needs its own insert-only path.
Out of scope, so it is not re-argued: OTP invite emails (the code email already reaches them); creating users at the IdP or SCIM; deactivating people and handing over their suites (that is the offboarding work, which builds on this); more than one workspace.

Consequences¶

Good — the biggest live gap closes: removal takes effect on the next request whatever credential is used, so a departed member's PAT and browser session die with their membership. Adding a member no longer needs a deploy. Azure AD gets an app-side gate for the first time. initial_role removes the "wait for their first login, then set the role" two-step. Nothing changes for any deployment until an admin deliberately writes the first row.

Costs we accept — four enforcement points is four gates, and a fifth credential kind added later must join the list or it silently bypasses membership; the mitigation is to declare the gates as data and sweep them in tests, as tests/support/mcp_gates.py does for the MCP tools. Every authenticated request gains one indexed lookup (the "is the table empty" answer can be cached per process, but a cache is a delay in revocation, so the TTL is a decision — start with none). An email now lives in two tables, users and workspace_members, and the two can drift if someone's address changes at the IdP. Membership is per workspace with no groups or teams; this leaves room for them without adding them.

Alternatives considered¶

Create users at the IdP (SCIM / Graph / Cognito admin APIs) — rejected, and it is the user's explicit direction. It is the deepest lock-in possible (one integration per provider, against ADR 0010), it would need DataQ to hold write credentials for the IdP — far more dangerous than anything it holds now — and nobody asked for it. The IdP owns identity; DataQ owns admission.
Invite emails with a token — rejected for now: OTP already emails the person when they request a code, and in OIDC/Azure modes there is no credential to hand over, so the invite would carry nothing useful. Revisit if self-service sign-up is ever wanted.
Fix each door separately (OTP session revocation, PAT membership, and so on) — rejected. That is the shape of the current bug: three doors that each decided on their own, two of which decided nothing. One membership rule at four resolvers is one rule; three fixes are three rules that will drift apart.
A migration that fills the table (non-empty by default) — rejected, and be precise about why. The problem is a silent backfill: it turns a zero-risk deploy into one that switches enforcement on everywhere at once with nobody watching. Decision 8's import reads the same rows, but it runs when an admin has chosen to switch, in the same transaction as that choice, and every row it writes is flagged and shown for review — because a users row proves someone once signed in, not that they still belong. Same data, opposite meaning: the migration would assert membership, the import proposes it.
A MEMBERSHIP_ENFORCED env flag — rejected: it duplicates what the table already says, somewhere that can disagree with it, and brings back the env-edit-and-restart this ADR exists to remove.
Drop the env allowlists entirely — rejected: an env-only way in is how you recover a workspace whose table has locked everyone out, exactly as WORKSPACE_ADMIN_EMAILS recovers one with no admins.

Verification bar¶

Required for the build PRs, based on what has actually bitten this repo:

Every enforcement point is mutation-verified. Delete the gate; the sweep must go red. A gate whose removal leaves tests green is not a gate — that trap hit four times on the MCP tool surface, each time because the probe failed earlier for an acceptable-looking reason. Probes must use a real identity that was admitted and then removed, for each credential kind.
Revocation is proven per credential, not per door: a live dq_sess_ cookie and a live dq_live_ PAT, each created while admitted and then used after removal, on REST and /mcp.
The import's transaction is proven on real Postgres — the first row and the import commit together or not at all. The test suite reads through its own transaction and cannot tell whether anything committed; use the real-DB harness the audit work introduced.
The last-admin guard is proven under real concurrency, with genuinely interleaved sessions. Three earlier regression tests passed against unfixed code because they ran one after the other; mutation-check this one before trusting it.
The initial_role insert-only claim is tested from the other side: sign in, change the role in-app, sign in again, assert the in-app value survived.
Superuser blindness still applies: tests run as an owner that ignores REVOKE, so anything that rests on a privilege needs the real-privilege harness.

Migration & rollout¶

One additive migration: CREATE TABLE workspace_members plus the lower(email) unique index. Nothing is dropped, nothing on users changes, so it deploys on its own and rolls back cleanly.

Two steps per CLAUDE.md §6: step 1 ships the migration alone; step 2 ships the resolver checks, the /admin/members endpoints and the Members panel. Because the table ships empty, neither step changes behaviour until an admin writes the first row — the switch is an admin's deliberate act, not a deploy event. The backend and the Members panel can land independently.

ADR 0043 — In-app workspace membership (workspace_members)¶