feat(account): revocable API tokens managed from account settings - #10624
feat(account): revocable API tokens managed from account settings#10624dnplkndll wants to merge 18 commits into
Conversation
|
Connected to Huly®: UBERF-15850 |
c84d786 to
efdbe1b
Compare
Follow-up: REST API should apply sensible defaults for
|
c7d2bf2 to
aaa8348
Compare
|
Hi @dnplkndll |
8663ec8 to
865fd71
Compare
|
@ArtyomSavchenko Thanks for the review! Formatting has been fixed — the issue was a prettier version mismatch (local 3.8.1 vs project's 3.6.2). New code now matches the existing codebase style with no formatting noise on existing lines. Changes in this update3 commits:
What's new since last push
Suggestions for future consideration
|
c39720c to
058c7da
Compare
|
|
||
| import { AccountRole, type MeasureContext, type PersonUuid, type WorkspaceUuid } from '@hcengineering/core' | ||
| import platform, { PlatformError, Severity, Status } from '@hcengineering/platform' | ||
| import { decodeTokenVerbose, generateToken } from '@hcengineering/server-token' |
There was a problem hiding this comment.
Could you please check warnings in this file?
CI formatting step is failed due to this:
https://github.com/hcengineering/platform/actions/runs/23317689234/job/67851578625?pr=10624
|
Hi @dnplkndll |
|
sure what is your opinion on the use of MCP versus api? My motivation was that I would prefer to have tool access in a controlled predicable / scriptable way with permissions on things. then you can test on a staging build out script you can repeat. |
f8cf105 to
4f332bb
Compare
ebdcb02 to
370a096
Compare
| "ApiTokenScopePreset": "Permissions", | ||
| "ApiTokenScopeReadOnly": "Read Only", | ||
| "ApiTokenScopeReadWrite": "Read & Write", | ||
| "ApiTokenScopeFullAccess": "Full Access" |
There was a problem hiding this comment.
Could you please check translations in this file and others?
Looks like many sentences were not translated and remained in English.
| return window.location.origin | ||
| } | ||
|
|
||
| $: baseApiUrl = getBaseUrl() + '/_transactor/api/v1' |
There was a problem hiding this comment.
That's not correct, transactor endpoint address cannot be built in this way, it is returned by account service when user is authenticated.
|
|
||
| async function resolveScopeLabels(): Promise<void> { | ||
| const lang = $themeStore.language | ||
| const labels = await Promise.all([ |
There was a problem hiding this comment.
There is no need to manually translate strings, we have dropdown version that uses i18n strings
| // Re-check if stale or missing | ||
| if (cached == null || now - cached.checkedAt > REVOCATION_CACHE_TTL_MS) { | ||
| try { | ||
| const revoked = await accountClient.checkApiTokenRevoked(apiTokenId) |
There was a problem hiding this comment.
This does not make sense.
- User comes with revokable token that contains token Id (that is how we identify revokable token).
- We use this token (that is potentially revoked or expired at the moment) to check whether the token is revoked
If the token is revoked, the account cannot reply anything other than 401 or 403, because the token is revoked/expired.
The account method, that returns boolean is redundant. We can use selectWorkspace or getWorkspaceInfo for this.
Additionally, I would implement token expiration / revocation logic and lower level. Maybe at the same level as decodeToken, for example we can add a new verifyToken method next to the decodeToken, that:
- Decodes token
- Checks whether the token is expired
- If the token is revokable, checks whether token is expired
- Returns decoded token
This will allow reuse token expiration / revocation logic in services to prevent accessing blobs, or other data with expired tokens.
But in order to do this, we will have to additionally set up some metadata in the server-token plugin (either address of the account endpoint, or method to verify whether the token is expired).
|
Thanks for the contribution We also have a button to generate token on workspace settings page, we will need to remove it in favor of the new approach. |
370a096 to
cf87e38
Compare
e0e0743 to
a3a8019
Compare
Add UI and backend support for creating, listing, and revoking API tokens scoped to workspaces. Includes owner-level workspace token visibility, OpenAPI documentation, Mongo/Postgres persistence, and i18n translations. Signed-off-by: Don Kendall <kendall@donkendall.com>
Embed apiTokenId in JWT extra field and add a per-token revocation cache (60s TTL) in the transactor REST handler. Revoked tokens are now rejected within ~60 seconds instead of remaining valid until JWT expiry. Adds checkApiTokenRevoked account service method for the transactor to query individual token revocation status. Signed-off-by: Don Kendall <kendall@donkendall.com>
Add coarse-grained scope enforcement for API tokens. Tokens can now be created with scopes ['read:*'], ['read:*','write:*'], or ['read:*','write:*','delete:*']. Existing tokens without scopes retain full access (backward compatible). - DB: v26 migration adds scopes TEXT[] column to api_tokens - Types: add scopes field to ApiToken and ApiTokenInfo - Operations: createApiToken accepts/validates/persists scopes, embeds in JWT via extra.scopes - Enforcement: withSession checks scopes against method; tx handler additionally requires delete:* for TxRemoveDoc - Client: createApiToken signature accepts optional scopes param - UI: scope preset dropdown in create popup (default: Read Only), permissions column in token list with i18n labels - Also fixes 3 pre-existing TS2322/TS2345 errors in operations.ts Signed-off-by: Don Kendall <kendall@donkendall.com>
- scopes.test.ts: 8 tests for hasScope() and getRequiredScope() logic - apiTokenScopes.test.ts: 7 tests for createApiToken scope validation (valid scopes, multiple scopes, no scopes backward compat, invalid format rejection, empty array rejection, domain-scope rejection) and listApiTokens scopes inclusion - Export hasScope/getRequiredScope from rpc.ts for testability Signed-off-by: Don Kendall <kendall@donkendall.com>
…tting - Restrict API token creation/revocation to AccountRole.User or higher (guests cannot use API tokens), per reviewer suggestion - Add 5 missing translation keys (ApiTokenPermissions, ApiTokenScopePreset, ApiTokenScopeReadOnly, ApiTokenScopeReadWrite, ApiTokenScopeFullAccess) to all non-en locale files to fix locale parity CI test - Fix prettier formatting in apiTokenScopes.test.ts - Rename local `extra` to `tokenExtra` in createApiToken to avoid shadowing the decoded token's `extra` field Signed-off-by: Don Kendall <kendall@donkendall.com>
- rpc.ts: use system service token for checkApiTokenRevoked so the revocation check is not coupled to the user's potentially-revoked bearer token; systemAccountUuid + service:'server' ensures account service always accepts the call - ApiDocsSection.svelte: derive transactor base URL from login.metadata.LoginEndpoint (set on auth) instead of window.location.origin, which is not necessarily the transactor host - ApiTokenCreatePopup.svelte: replace manual translate() calls and themeStore language watch with DropdownLabelsIntl + DropdownIntlItem[], which handle i18n automatically; error state is now IntlString - General.svelte: remove legacy GenerateApiToken button, handler, and ApiTokenPopup import in favour of the new ApiTokens settings panel Signed-off-by: Don Kendall <kendall@donkendall.com>
Signed-off-by: Don Kendall <kendall@donkendall.com>
Signed-off-by: Don Kendall <dkendall@ledoweb.com>
Signed-off-by: Don Kendall <dkendall@ledoweb.com>
a3a8019 to
dfa360b
Compare
The feat/* branches got rebased onto upstream/develop to clear the DCO + merge-conflict blockers on PRs hcengineering#10705 and hcengineering#10624. That puts commits on those branches that depend on files only present in develop > v0.7.423, so the prior 'merge feat/* onto v0.7.423 tag' flow now conflicts. Three changes: 1. ledoent/upstream-tag.txt → develop tip (fa2930d). The file now carries any ref — tag, branch, or SHA. We pin to a SHA so each aggregate run is reproducible; bump it when we want to follow develop forward. 2. gitaggregate.yml — resolve the ref through 'refs/tags/<ref>' first, then 'upstream/<ref>', falling back to the raw value. Output and variable renamed upstream_tag → upstream_ref to match the new semantics. 3. common/scripts/show_tag.js — read common/scripts/version.txt before trying 'git describe --tags'. Develop has no tag in its ancestry so the previous git-describe-only path printed the hardcoded "0.6.0" fallback, which then showed up next to the Help & Support button. Matches the pattern already used by show_version.js. Signed-off-by: Don Kendall <dkendall@ledoweb.com>
The feat/* branches got rebased onto upstream/develop to clear the DCO + merge-conflict blockers on PRs hcengineering#10705 and hcengineering#10624. That puts commits on those branches that depend on files only present in develop > v0.7.423, so the prior 'merge feat/* onto v0.7.423 tag' flow now conflicts. Three changes: 1. ledoent/upstream-tag.txt → develop tip (fa2930d). The file now carries any ref — tag, branch, or SHA. We pin to a SHA so each aggregate run is reproducible; bump it when we want to follow develop forward. 2. gitaggregate.yml — resolve the ref through 'refs/tags/<ref>' first, then 'upstream/<ref>', falling back to the raw value. Output and variable renamed upstream_tag → upstream_ref to match the new semantics. 3. common/scripts/show_tag.js — read common/scripts/version.txt before trying 'git describe --tags'. Develop has no tag in its ancestry so the previous git-describe-only path printed the hardcoded "0.6.0" fallback, which then showed up next to the Help & Support button. Matches the pattern already used by show_version.js. Signed-off-by: Don Kendall <dkendall@ledoweb.com>
Resolve conflicts: - migrations: keep develop's V26 (pending_configuration); api_tokens table (with scopes column) becomes V27 - lang files: union of develop's keys and the API token strings Signed-off-by: Don Kendall <dkendall@ledoweb.com>
Address @aonnikov's review: the bespoke checkApiTokenRevoked RPC and the ad-hoc revocation cache in the transactor are replaced by a reusable verifyToken in server-token that checks signature, expiry, and (for revokable API tokens) revocation via a pluggable checker. - server-token: add verifyToken + isTokenExpired + setApiTokenRevocationChecker (the 'method to verify' metadata the plugin needs, without depending on the account client). Revocation cache (60s TTL) now lives here, reusable by any service (transactor, blob access, etc.). - account: the account is now authoritative — wrap() rejects revoked/expired API tokens, so any account method (selectWorkspace/getWorkspaceInfo/...) naturally 401s. Removed the redundant checkApiTokenRevoked method. - account-client: drop checkApiTokenRevoked. - transactor: withSession uses verifyToken; the registered checker reuses the existing getLoginInfoByToken instead of a dedicated boolean RPC. Scopes are parsed once and threaded through (no re-decode in the tx handler). - tests: verifyToken/isTokenExpired unit coverage. Signed-off-by: Don Kendall <dkendall@ledoweb.com>
…ndpoint Address @aonnikov: the REST API host must come from the transactor endpoint returned by the account service (the login endpoint, a ws(s):// URL), not be constructed from window.location. Convert ws->http and append /api/v1, matching the existing ServerManagerGeneral pattern. Signed-off-by: Don Kendall <dkendall@ledoweb.com>
Address @ArtyomSavchenko: the new API token UI strings were left in English in non-en locales. Provide translations for ru/de/es/fr/it/pt/pt-br/zh/ja/cs/tr. The en/ru locale-parity test passes (the prior failure was an en/ru key mismatch, resolved by the develop merge). Signed-off-by: Don Kendall <dkendall@ledoweb.com>
…DocsSection) Signed-off-by: Don Kendall <dkendall@ledoweb.com>
|
Thanks for the reviews — pushed an update addressing the feedback. Conflicts with
Unit tests added for |
|
Hi @dnplkndll |
Conflicts, all from upstream adding strings and error handling next to ours: - plugins/setting-assets/lang/*.json (12 locales): three-way key merge, keeping upstream's new keys and re-applying ours. Locale parity fills requested in review (es/pt/pt-br/cs) are preserved. - pods/server/src/rpc.ts: kept our delete:* scope check ahead of upstream's new try/catch around the tx dispatch. Two fixes the merge surfaced, needed for the tree to build and pass CI: - token.test.ts: Token.account is AccountUuid, so the test constant can no longer be cast to PersonUuid. - server_http.ts: prettier rewrap of a line upstream's changes pushed over the width limit. Claude-Session: https://claude.ai/code/session_01ANdoXbdn5k2hZy734EwKe7 Signed-off-by: Don Kendall <dkendall@ledoweb.com>
The scopes were only ever consulted in the transactor's REST handler, so they promised a boundary the platform did not keep: - the WebSocket transport decodes the token and never looks at scopes, so a read-only token could open a socket and issue any transaction; - even on the REST path, delete:* only matched a top-level TxRemoveDoc and was bypassed by nesting the removal in a TxApplyIf; - nothing stopped a scoped token from calling createApiToken and minting an unscoped one. Narrowing a token's rights has to happen in the pipeline, where it covers every transport, and that is a larger design than this feature. Until then a token carries its account's rights and says so, rather than displaying a restriction that does not hold. Scopes are removed end to end. What is kept is made to work: - API tokens can no longer create, list or revoke API tokens. Otherwise a leaked token renews itself and revokes the tokens meant to stop it. - Revocation fails closed. The account is the only authority on it, so an unreachable account now rejects the token instead of admitting a revoked one to whoever can keep the account busy. Verdicts stay trusted for the cache TTL so brief outages do not cut off healthy tokens. - The revocation cache is bounded and re-checks negative verdicts. - Revoking your own token no longer requires a role in its workspace, so leaving a workspace cannot strand a credential you can never revoke. - The per-account limit counts only usable tokens; revoked and expired ones are kept for the audit trail and used to lock out anyone rotating. - Rejected REST tokens are logged with a reason, since expired, revoked and unverifiable are one opaque 401 from outside. The unused listWorkspaceApiTokens/revokeWorkspaceApiToken pair is removed; it had no caller and no test. Tests cover the role restriction, the API-token guard, ownership on revoke, the limit accounting and fail-closed revocation. Removing any of those checks fails them. Claude-Session: https://claude.ai/code/session_01ANdoXbdn5k2hZy734EwKe7 Signed-off-by: Don Kendall <dkendall@ledoweb.com>
- Moves the page from workspace settings to account settings. Tokens belong to the account: the page already lists them across every workspace, and creating one only needs the User role the account service checks, not Owner as the category required. - Surfaces failures. A failed revoke was logged to the console and the row re-rendered unchanged; loading workspaces could reject unhandled and leave an empty dropdown with no explanation. - Outside a secure context there is no clipboard API, so the OK button did nothing and the dialog had no way out but Cancel. It now closes, leaving the token selectable. - Replaces hardcoded English labels, adds aria-expanded on the docs disclosure and labels on the copy targets. - Fills in the Korean and Polish translations, which were the only two locales missing these keys, and drops the API access strings orphaned when this PR replaced the old Generate API token button. Claude-Session: https://claude.ai/code/session_01ANdoXbdn5k2hZy734EwKe7 Signed-off-by: Don Kendall <dkendall@ledoweb.com>
Unrelated to this feature and wrong for this repo: it documents a /_tokens minting service and a tools/mint-token CLI that do not exist here, uses reverse-proxy prefixes from a self-hosted deployment rather than the transactor's /api/v1 routes, and states that revocation is a future enhancement, which is what this PR implements. Nothing references it. REST API docs belong in their own change, generated against the routes that exist. Claude-Session: https://claude.ai/code/session_01ANdoXbdn5k2hZy734EwKe7 Signed-off-by: Don Kendall <dkendall@ledoweb.com>
Summary
Personal API tokens with a built-in REST API, managed from account settings. Tokens are JWTs with a configurable expiry (7–365 days) that can be revoked, with expiry and revocation enforced centrally so the policy is reusable across services.
What changed since the last review
Thanks for the review and the patience — conflicts are resolved and every comment is addressed. Two things also changed on their own merits:
Token scopes are gone. They were only consulted in the transactor's REST handler, so they advertised a boundary the platform did not keep. The WebSocket transport decodes the token and never looks at scopes, so a "Read Only" token could open a socket and issue any transaction; on the REST path itself
delete:*only matched a top-levelTxRemoveDocand was bypassed by nesting the removal in aTxApplyIf; and nothing stopped a scoped token from callingcreateApiTokento mint an unscoped one. Narrowing a token's rights belongs in the pipeline, next toGuestPermissionsMiddleware, where it covers every transport — that is a larger design than this feature, and worth its own PR. Until then a token carries its account's rights and the UI says so, instead of showing a restriction that does not hold.docs/openapi.yamlis dropped. It described a/_tokensminting service and atools/mint-tokenCLI that do not exist in this repo, used reverse-proxy prefixes rather than the transactor's/api/v1routes, and stated that revocation was a future enhancement — which is what this PR implements. Nothing referenced it.Review comments
verifyToken()now sits alongsidedecodeToken()and verifies signature, expiry and, for revokable tokens, revocation through a pluggable checker (setApiTokenRevocationChecker). The account is authoritative:wrap()rejects revoked or expired tokens, so the transactor's checker just reusesgetLoginInfoByToken. The bespokecheckApiTokenRevokedRPC is gone.ServerManagerGeneraluses.createApiToken, and now covered by a test that fails if the check is removed.en.json; Korean and Polish were the two still missing these keys. The API access strings orphaned when this PR replaced the old "Generate API token" button are removed from all 14 locales.Hardening
listWorkspaceApiTokens/revokeWorkspaceApiTokenpair is removed — no caller, no test.Placement and UI
The settings page moved from workspace settings to account settings: tokens belong to the account, the page already listed them across every workspace, and creating one only needs
User— the category requiredOwner, so the very users the role check was written for could not reach it. Failed loads and revokes are now surfaced instead of logged to the console, the copy dialog no longer dead-ends outside a secure context, and the docs disclosure and copy targets carry the expected aria attributes.Verification
rush validate(full repo),rush check,check-versions,fast-format,svelte-check— all cleanrushx testinserver/account— 499 passed; the 4postgres-realfailures need a live database and fail identically on a clean checkoutKnown limitation
Revocation is checked when a WebSocket session is established, not continuously, so an already-open session outlives revocation until it reconnects. The REST path re-checks within the cache TTL. Closing that gap means re-validating live sessions in the session manager, which I've kept out of this PR.
https://claude.ai/code/session_01ANdoXbdn5k2hZy734EwKe7