feat(auth): add scoped API keys for Astra - #244
Conversation
aptend
left a comment
There was a problem hiding this comment.
Request changes for two authorization lifecycle issues:
-
[P1] Denied MCP calls still provision per-user storage. The handler records tool usage before evaluating
authorization_error, and the denial paths enqueue per-user call-log entries. In multi-DB mode both batchers later calluser_sql_store, which provisions a missing personal memory database. An identity-only key can therefore call a forbidden tool such asmemory_store, receive-32003, and still cause its memory DB to be created at the next flush or shutdown. The group guard also performs storage work before the authorization error is returned. Please complete scope admission before any per-user instrumentation/storage access, or persist denied-call telemetry in the shared auth store, and add a real-DB assertion that an identity-only denied request leaves no user database/registry row. -
[P1] Cached keys remain usable after
expires_at. Non-whoamivalidation returns a cached principal before consulting the database, whileCachedApiKeyPrincipaldoes not carry the key expiry and the cache TTL is 300 seconds. A key warmed shortly before expiration can therefore continue reading or writing through REST/MCP for up to five minutes after its database expiry. Please carry/checkexpires_aton cache hits (or cap cache lifetime to it) and add a regression covering warm cache -> expiry -> REST and MCP authentication failure.
gouhongshen
left a comment
There was a problem hiding this comment.
Re-reviewed the latest head. No remaining P0/P1 findings. MCP tool authorization is now explicit and fail-closed, including write tools, and all required checks are green. Approving.
Check absolute API key expiry on every cache read and reject unauthorized MCP calls before per-user telemetry or group storage access. Add expiry and database-provisioning regressions and document the authorization boundary.
## Summary Implement hosted BYOK model configuration and scoped Memoria account sign-in, including the fixes requested in the review of this PR. - Add authenticated owner-scoped `/me/models` APIs and CLI model configuration, default selection and probing. - Support OpenAI, Anthropic, DeepSeek and OpenAI-compatible endpoints through the existing provider/runtime path. - Protect user-supplied model endpoints with public-address admission, DNS/transport validation and explicit server-owned proxy configuration. - Resolve Memoria identities through the canonical provider/subject mapping, preserving issuer provenance instead of issuing an internal identity. - Commit the identity, encrypted connection credential and refresh session atomically; serialize concurrent binding changes and add disconnect/retention behavior. - Carry one application-scoped provider and credential resolver through proxy, recall, extraction, compaction and governance. - Treat missing/none memory access as disabled and deny write-oriented extraction before memory reads or LLM work for read-only users. - Discover the login website from the target Server, preserve self-hosted password login, and harden loopback callback parsing and Windows browser-command construction. - Add fresh Memoria account reauthentication for device trust, reenrollment and forced Work takeover, bound to issuer, subject, credential generation and purpose; update the SDK/Web consumer. - Revalidate background memory inference through canonical model admission on every provider attempt, avoiding stale credentials and deployment-key fallback for hosted users. - Add explicit TCP-only BYOK DNS configuration without UDP fallback; keep public-address validation and test synthetic DNS answers. - Isolate database fixtures and gate external Memoria/provider contracts behind an explicit opt-in feature so ordinary fork CI does not require unprovisioned services. ## Related issue Related: matrixorigin/memoria#245 Cross-repository dependencies: - Memoria scoped-key API: matrixorigin/memoria#244 - Website follow-up: matrixorigin/memoria-website#216 - Initial website integration (merged): matrixorigin/memoria-website#213 These references intentionally do not automatically close the cross-repository issue. ## Change type - [x] Feature - [x] Bug fix - [x] Documentation - [x] Refactor or performance improvement - [x] Test - [x] Build, CI, or maintenance ## User and compatibility impact - Hosted BYOK stores user LLM credentials encrypted on Astra Server. This does not implement Runner-local inference or claim that provider credentials remain on the user's device. - Memoria sign-in does not implicitly grant memory sharing. Identity-only keys remain valid for login. - `MEMORIA_WEB_URL` is a Server setting exposed through `GET /auth/methods`; the CLI no longer chooses its own website. Remote browser login URLs require HTTPS. - Self-hosted password login and administrator-managed model configuration remain supported. - `MEMORIA_ISSUER` gives the integration a stable identity namespace. Existing pre-issuer mappings require an explicit `MEMORIA_LEGACY_ISSUER` assertion and fresh login; migration preserves the Astra account, models and Work/history. - Disconnect removes Astra's stored Memoria credential and revokes Astra sessions, but does not delete Memoria memories or revoke upstream keys on the user's behalf. - GitOps/Helm deployment integration, production domain provisioning and TaaS are outside this change. ## Architecture and complexity delta - Canonical owner extended: authentication owns verification, provider identity, sessions and credential binding; model access/admission owns personal model execution; MemoriaPort owns memory operation admission. - Existing implementations/callers searched: provider identity mapping, auth sessions, model service/admission, CLI login/model commands, memory proxy, run/subrun builders, recall, extraction, compaction and governance. - Superseded paths removed: subject-only identity creation, handler-owned credential writes after session commit, ambient memory transport selection in generic pool setters, independent runtime provider configuration reads, and the unconditional hosted CLI login journey. - The old Memoria-only identity table remains a read-only migration source. Rows are removed after explicitly issuer-authorized migration. Verified-provider principals remain distinct from request-authorized external providers. - User models reuse the existing provider adapters and canonical execution validation rather than adding another agent loop or provider client. ## Verification ### Latest review-fix update (7a04fef) - `cargo fmt --all -- --check`: passed. - SDK `npm test -- src/__tests__/work-contract.test.ts`: 60 passed. - Web `npm test -- __tests__/components/app/work-turn-composer.test.tsx __tests__/lib/work-criteria-actions.test.ts`: 38 passed. - `git diff --check`: passed. - Attempted `CARGO_INCREMENTAL=0 CARGO_PROFILE_DEV_DEBUG=0 CARGO_PROFILE_TEST_DEBUG=0 cargo test --locked -p astra-services --lib byok_endpoint`, but the local `quote` dependency build-script artifact could not execute (exit 126; `file` identifies it as data). No Rust test ran in this attempt. This is not reported as a passing Rust gate; CI and a clean/local-cache Rust rerun remain required. - Website integration counterpart independently passed its 3 isolated MatrixOne/SMTP regression tests and deployed Nginx route smoke. This does not substitute for a full Astra Rust or production OAuth test. ### Previously recorded verification After merging the current upstream main and preserving the existing remote PR history: - `cargo check --locked -p astra-runtime -p astra-cli` with `CARGO_INCREMENTAL=0`: passed. - `cargo fmt --all -- --check`: passed. - `python3 -m unittest discover -s scripts/schema -p 'test_*.py'`: 47 passed. - `git diff --check github/main...HEAD`: passed. Earlier local validation of the feature/review-fix implementation, before this final upstream synchronization: - CLI auth-flow suite: 14 passed. - Memory extraction, prefetch, compaction and governance suites: 185 passed. - Public HTTP contracts: 11 passed, including login/disconnect and user model authorization. - Real-database Memoria auth tests: 2 passed in cloud-byok and again in self-hosted mode; covered concurrent first login/relink, issuer separation, partial-failure rollback, migration, disconnect and inactive-account credential cleanup. - Real Memoria API v1 contract: passed for all three scoped modes, identity continuity, master-key rejection and upstream revocation. - Targeted services auth tests and strict library Clippy for services/runtime/CLI passed. - Additional personal-model real-database and cross-project permission/CLI smoke tests passed during feature validation. Live GitHub/Google OAuth callbacks and actual Windows browser launch have not been exercised. The Windows test validates command construction, not an actual Windows browser. Full post-merge test/CI validation remains required; the earlier test results are not presented as results for the newly synchronized head. ## Final checklist - [x] I added or updated tests at the layer that owns the behavior. - [x] I updated public/design documentation for contract changes. - [x] I checked the feature diff for credentials, private URLs, customer data and generated files; local runtime configs and build output are excluded. - [x] The PR title follows the repository's Conventional Commit format. Approved-by: XuPeng-SH <39627130+XuPeng-SH@users.noreply.github.com> Approved-by: gouhongshen <26336316+gouhongshen@users.noreply.github.com>
aptend
left a comment
There was a problem hiding this comment.
Re-reviewed 40f7dbd. The two previously blocking authorization lifecycle issues are resolved: cached principals now enforce absolute key expiry, and denied or identity-only MCP traffic is admitted before any per-user telemetry or storage provisioning. The new unit and real-database regressions cover REST/MCP expiry, notifications, flush/shutdown behavior, headers, and group storage lookup. Local memoria-api library tests passed (96/96), and the current Check & Clippy, Unit Tests, and DB Tests are green. No remaining P0/P1 findings.
What type of PR is this?
Which issue(s) this PR fixes
Related: #245
This reference intentionally does not automatically close the cross-repository issue.
What this PR does / why we need it
Provide reusable, least-privilege credentials for Astra account sign-in and optional memory sharing.
Add API-key scopes and stable
GET /auth/whoamiidentity metadata.Enforce identity/read/write/key-management scopes across REST and MCP.
Prevent identity-only/read-only/read-write connection keys from administering groups, managing keys, or reaching unclassified authenticated routes.
Preserve scopes during rotation and maintain explicit compatibility for legacy/default keys.
Reject unauthorized memory access before per-user memory provisioning.
Make whoami verify the current database grant rather than trust a warm replica-local cache, so login/refresh observes revocation made through another API replica.
Decouple MCP authorization from metrics invalidation. Explicitly require
memory:writeformemory_apply,memory_rebuild_indexandmemory_tune_params.Classify exact dispatch names rather than sanitized tracking labels; reject unclassified tools by default, including with master credentials.
Apply the same admission to requests and notifications, with real HTTP/database regression coverage.
Verification
Latest MCP review-fix update (58744ff)
cargo test --locked -p memoria-api --lib: 93 passed.cargo test --locked -p memoria-api --test api_e2e test_scoped_ -- --nocapture: 3 passed.cargo test --locked -p memoria-api --test api_e2e test_mcp_ -- --nocapture: 16 passed.git diff --check: passed.Previously recorded verification
Local validation before push:
git diff --checkpassed.Production OAuth and GitOps deployment are not covered by these API tests.
Cross-repository dependencies
No MatrixOne source changes are included.