Repository navigation
feat(account): admin backend foundation — account disable/enable, admin audit log, admin list queries - #11055
MichaelUray wants to merge 5 commits into
Conversation
…ration) Add the data model for admin-driven account lifecycle management: - account.disabled_at, account.token_version and account.last_activity_at - admin_audit_log table with target/admin/time indexes - supporting indexes for admin account listing queries The schema is introduced by a single new migration, account_db_v28_account_lifecycle_admin_audit. Every statement is idempotent (IF NOT EXISTS), so it also applies cleanly on deployments that already created these objects under the earlier admin migration ids (account_db_v27_admin_user_management .. v30). A real-database upgrade test reproduces that state on PostgreSQL and CockroachDB. AccountDB gains an adminAuditLog collection (insert only for now) for PostgreSQL and MongoDB; the account SELECT returns the new fields. Signed-off-by: Michael Uray <michaeluray@users.noreply.github.com>
…, disabled checks on login paths Make a disabled account unusable everywhere, independent of how its credentials were issued: - wrap() loads the account of every UUID token principal and rejects the request with Unauthorized when the account is disabled. This covers session tokens, API tokens and user-scoped service tokens for every account RPC, and therefore also transactor/REST token verification (getLoginInfoByToken) and WebSocket session establishment (getLoginWithWorkspaceInfo). System, guest and read-only guest principals, non-UUID principals and principals without an account row are exempt. The loaded row is passed to the method as meta.principalAccount so hot paths do not read it twice; a caller-supplied value is never forwarded. - Session tokens carry the account's token_version (when > 0); getLoginInfoByToken, getLoginWithWorkspaceInfo and selectWorkspace reject session tokens issued before the last version bump. API tokens (extra.apiTokenId) and service tokens (extra.service) are not version-validated (a re-minted one may carry the claim, which is ignored), so they keep working after a bump; they are still rejected while the account is disabled. There is no version check in wrap() itself. - The token-less login paths (password, OTP, 2FA, provider and grant sign-in) refuse disabled accounts with account:status:AccountDisabled. Password login only reports it after the password matched. - createApiToken additionally refuses disabled accounts and stale session tokens (defense in depth). - Successful logins and workspace selection record last_activity_at, throttled to one write per five minutes. API tokens stay unversioned and keep their own revocation check. Signed-off-by: Michael Uray <michaeluray@users.noreply.github.com>
…state+audit write Add two admin-only account RPCs (token with the admin claim, current token version): - disableAccount sets disabled_at and bumps token_version. It refuses to disable the caller (account:status:CannotDisableSelf) and the last active instance admin from ADMIN_EMAILS (account:status:LastAdmin), re-checking the last-admin invariant after the write and undoing the disable if a concurrent change left no active admin. Disabling an already disabled account changes nothing. - enableAccount clears disabled_at and bumps token_version, so session tokens issued before the disable stay invalid. Enabling an active account changes nothing. Both record an admin_audit_log row (no-op calls are recorded with details.noop). State change and audit row go through the new AccountDB.applyAccountLifecycle, which runs them in a single transaction on PostgreSQL/CockroachDB (a failed audit insert rolls the change back) and sequentially on MongoDB. Refused attempts are audited best-effort as admin_action_denied, throttled per admin, reason and target. API tokens are deliberately not revoked: the disabled-principal gate rejects them, like user-scoped service tokens, while the account is disabled. Neither is version-validated, so both work again after enable, while session tokens issued before the disable stay invalid. The account client gains disableAccount and enableAccount. Signed-off-by: Michael Uray <michaeluray@users.noreply.github.com>
…dInvite, resendInvite and changePassword These operations mint new access (guest links, invites) or change credentials based on the caller's token. They now reject session tokens issued before the account's last token version bump, using the account row they already load. API and service tokens are not version-validated here; they are still refused while the owner is disabled. API token revocation and expiry are still enforced by wrap(). Signed-off-by: Michael Uray <michaeluray@users.noreply.github.com>
…L pushdown), escapeLike Add two admin-only read RPCs: - listAccountsAdmin lists accounts with search, status, auth method, name/email, workspace membership, workspace count, last activity, orphan and instance-admin filters, sorting and offset pagination. Filtering, sorting and paging are pushed down to SQL; user input is bound as parameters, sort columns come from a whitelist (with a uuid tie-breaker for stable pages) and LIKE wildcards are escaped (escapeLike). - listAuditAdmin lists admin_audit_log entries newest first, filtered by target account, admin account, action and time range, with offset pagination and the admin/target names resolved. Request parameters are reduced to well-typed values before they reach the database; malformed workspace UUIDs and non-integer numbers (pagination, workspace count and last-activity bounds) are rejected with BadRequest. Both queries are implemented for PostgreSQL/CockroachDB; on MongoDB they return account:status:NotSupportedOnBackend, as listAccounts is likewise not implemented there. The account client gains listAccountsAdmin and listAuditAdmin with their DTOs; @hcengineering/account now depends on @hcengineering/account-client for the shared row type. Signed-off-by: Michael Uray <michaeluray@users.noreply.github.com>
| return await db.getWorkspaceUsersWithPermission(workspace, permission) | ||
| } | ||
|
|
||
| /* ======== ADMIN: ACCOUNT LIFECYCLE ======== */ |
There was a problem hiding this comment.
It seems to me that this file already contains too much. What do you think about moving the new admin operations to a separate file?
| throw new PlatformError(new Status(Severity.ERROR, platform.status.BadRequest, {})) | ||
| } | ||
|
|
||
| const { account, extra } = decodeTokenVerbose(ctx, token) |
There was a problem hiding this comment.
Looks like an old session can mint a current session after re-enable. createWorkspace accepts a session token without checking its version, then issues a token stamped with the account’s current version. A session issued before disable can therefore create a workspace after re-enable and obtain a usable token, bypassing the intended session invalidation. I suggest to check the incoming version before issuing a replacement token; audit the other token-returning RPCs for the same path.
|
|
||
| // isAdmin filter | ||
| if (params.isAdmin === true) { | ||
| conds.push(`LOWER(s.primary_email) = ANY(${ph(adminEmails.map((e) => e.toLowerCase()))}::TEXT[])`) |
There was a problem hiding this comment.
Looks like the admin list can omit an actual admin. The admin filter examine only the minimum verified email. Login grants the admin claim using the email used to sign in. If an account has multiple verified emails and a configured admin email is not the minimum, it can act as an admin but disappears from isAdmin: true results and is labelled non-admin.
|
Here are review results by AI agent and the comments seem valid from my point of view. Must fix
Should address
|
|
Thanks for the thorough review. The points are valid; I'm reworking the PR accordingly, including the split into two PRs, and will push the updated version soon. |
Context
Backend-only foundation for instance-admin tooling, as agreed in #10883 (#10883 (comment)). This PR has no UI. The admin panel UI from #10883 will be stacked on top of it in a follow-up.
It is a fresh port against current
develop, not a rebase of #10883. It lives entirely inserver/accountandfoundations/core/packages/account-client(plus a 3-line lockfile entry for the new workspace dependency).What's in (one commit each)
account.disabled_at,account.token_version,account.last_activity_at, theadmin_audit_logtable, and indexes for the admin listing. One new migration,account_db_v28_account_lifecycle_admin_audit.AccountDB.adminAuditLogis added for PostgreSQL and MongoDB.wrap()loads the account of every UUID token principal and answersUnauthorizedif the account is disabled. This applies to session tokens, API tokens and user-scoped service tokens on every account RPC. Because the transactor/REST token check (getLoginInfoByToken) and WebSocket session setup (getLoginWithWorkspaceInfo) also go throughwrap(), they are covered too.meta.principalAccount, so the hot paths do not read the account a second time.wrap()never forwards a value supplied by the caller.token_versionwhen it is greater than 0.getLoginInfoByToken,getLoginWithWorkspaceInfoandselectWorkspacereject session tokens issued before the last version bump. API tokens (extra.apiTokenId) and service tokens (extra.service) are not version-validated (a token re-minted from one, e.g. bygetLoginInfoByTokenorselectWorkspace, may carrytoken_version, but it is ignored), so they keep working after a bump; they are still rejected while the account is disabled. There is no version check inwrap()itself.account:status:AccountDisabled: password, OTP, 2FA, provider (OAuth/OIDC) and grant sign-in. Password login reports this only after the password has matched, so the disabled state is not disclosed to callers who do not know the password.createApiTokenadditionally refuses disabled accounts and stale session tokens (defense in depth).last_activity_at, at most one write per 5 minutes per account.disableAccount/enableAccount(admin-only RPCs and client methods)disabled_atand bumpstoken_version. It refuses to disable the caller (CannotDisableSelf) and the last active instance admin fromADMIN_EMAILS(LastAdmin). The last-admin invariant is checked again after the write; if a concurrent disable left no active admin, the change is undone.disabled_atand bumpstoken_version, so session tokens issued before the disable stay invalid.details.noop.AccountDB.applyAccountLifecycle. On PostgreSQL/CockroachDB both happen in one transaction, and a failed audit insert rolls the change back. On MongoDB they are written one after the other (documented as not atomic).admin_action_denied. This is best-effort and throttled per admin, reason and target.createAccessLink,sendInvite,resendInviteandchangePasswordreject stale session tokens. API and service tokens are not version-checked there (they are still refused while the owner is disabled); the API-token revocation and expiry check inwrap()is unchanged.listAccountsAdmin/listAuditAdmin(admin-only RPCs and client methods)listAccountsAdminfilters by search, status, auth method, name/email, workspace membership, workspace count, last activity, orphan and instance admin. It supports sorting and offset pagination, all pushed down to SQL.escapeLike).listAuditAdminreturns entries newest first. Filters: target account, admin account, action and time range. It uses offset pagination and resolves admin and target names.BadRequestinstead of failing in the database;limitis clamped to 500.API tokens (#10624)
API tokens are not version-validated and are not revoked when an account is disabled. While the owner is disabled, the gate rejects them everywhere. That includes the transactor's revocation checker, which treats
UnauthorizedfromgetLoginInfoByTokenas revoked. After enable they work again, including throughgetLoginInfoByToken, because the token-version check applies only to session tokens. Session tokens issued before the disable stay invalid after enable on the version-checked paths (getLoginInfoByToken,getLoginWithWorkspaceInfo,selectWorkspace, the four operations above andcreateApiToken) because of the version bump. The revocation and expiry check for API tokens inwrap()is unchanged.Service tokens
Service tokens minted with the system account are exempt from the gate. Service tokens minted with a user's UUID are gated like sessions: telegram-bot, mail worker, events processor, the GitHub user token, and the AI bot's own account. So a disabled user's integrations stop working until the account is enabled again (see question 2). Like API tokens, they are not version-validated, so they work again after enable (including
getLoginWithWorkspaceInfoandselectWorkspace).Migration
account_db_v28_account_lifecycle_admin_auditcontains onlyIF NOT EXISTSstatements (ADD COLUMN,CREATE TABLE,CREATE INDEX). It has no backfill and no constraints.account_db_v27_admin_user_management...v30). An upgrade test reproduces exactly that state on PostgreSQL and CockroachDB: the legacy DDL is copied verbatim into a test fixture, applied, and theninit()runs. The test then checks the schema and runtime behaviour, including listing legacy audit rows without a target account.init()twice on a fresh database and executes the DDL twice.adminAuditLogcollection.listAccountsAdmin/listAuditAdminreturnaccount:status:NotSupportedOnBackendthere, the same way the existinglistAccountsis not implemented for MongoDB.disableAccount/enableAccountand the audit insert work on both backends.Deliberately out (follow-ups)
createAccountAdmin,triggerPasswordReset, bulk operations, and workspace-operation audit rows. These need a nullabletarget_accountand a batch id and would come as a second backend step.decodeFilterParam. I mentioneddecodeFilterParamearlier, but I dropped it here because its only consumer is the CSV export, which belongs to the UI stage.Cost
One primary-key
accountlookup per authenticated account RPC, inwrap(). System-account service calls do not pay it. The row is reused through request meta, sogetLoginInfoByToken,getLoginWithWorkspaceInfo,assertAdminandcreateApiTokendo not read it again. There is no cross-request cache because account pods scale out and there is no cheap invalidation. The transactor already cachesgetLoginInfoByTokenfor 60 s.Compatibility
ADMIN_EMAILSadmin gate is unchanged. TheserveAccountsignature is unchanged.Metagains an optional field.sendEmailConfirmationnow takesdb(exported helper; no callers outsideserver/account).@hcengineering/accountnow depends on@hcengineering/account-clientfor the shared row type.account:status:AccountDisabled,CannotDisableSelf,LastAdmin,NotSupportedOnBackend.Tests
server/account: 736 tests (530 before this PR), including the real-database suite against CockroachDB and PostgreSQL 18, as in CI.account-client: 42 tests (38 before).getLoginInfoByToken,getLoginWithWorkspaceInfo,selectWorkspaceand an ordinary wrapped RPC) for session, API and user-scoped service tokens;admin_action_deniedthrottling; the four token-minting operations, called directly and throughwrap();listAccountsAdminBadRequestfor malformed workspace UUIDs and fractional pagination at the RPC level.applyAccountLifecyclecommit and rollback on a failed audit insert, everylistAccountsAdminsort field in both directions, filters, andlistAuditAdmin.rush validate) and passes the suites on its own.Open questions
statusblock to the account plugin (AccountDisabled,CannotDisableSelf,LastAdmin,NotSupportedOnBackend) without client translations. Would you rather reuseplatform.status.Forbidden/Unauthorizedwith details? If not, where should the translations live?wrap(): is one primary-key lookup per authenticated RPC acceptable, or would you prefer a narrower projection without the password join?