Skip to content

feat(auth): add scoped API keys for Astra - #244

Merged
aptend merged 5 commits into
matrixorigin:mainfrom
loveRhythm1990:feat/astra-scoped-keys
Sep 8, 2026
Merged

feat(auth): add scoped API keys for Astra#244
aptend merged 5 commits into
matrixorigin:mainfrom
loveRhythm1990:feat/astra-scoped-keys

Conversation

@loveRhythm1990

@loveRhythm1990 loveRhythm1990 commented Sep 4, 2026

Copy link
Copy Markdown
Collaborator

What type of PR is this?

  • feat (new feature)
  • fix (bug fix)
  • docs (documentation)
  • style (formatting, no code change)
  • refactor (code change that neither fixes a bug nor adds a feature)
  • perf (performance improvement)
  • test (adding or updating tests)
  • chore (maintenance, tooling)
  • build / ci (build or CI changes)

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/whoami identity 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:write for memory_apply, memory_rebuild_index and memory_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.
  • HTTP tests used isolated local MatrixOne databases. The new regression checks all three missed write tools, notification handling, write-authorized positive controls, readable tools, identity-only denial and unknown/malformed names.
  • Persisted-state assertions confirm denied requests/notifications do not tune retrieval parameters, while the same write-authorized operation does.
  • Targeted Rust formatting and git diff --check: passed.
  • No user test data was cleared or local Memoria service replaced.

Previously recorded verification

Local validation before push:

  • Memoria API compile checks and targeted auth/MCP scope unit tests passed.
  • Real-database scoped-key API tests passed, including group-administration denial and uncached whoami revocation.
  • Seven legacy API-key regression tests passed.
  • Astra-to-real-Memoria API v1 contract passed for identity-only, read-only, read-write, master-key rejection, stable account identity and revocation.
  • Latest upstream main was merged; its intervening changes have no net source-tree delta relative to the tested Memoria implementation.
  • git diff --check passed.

Production OAuth and GitOps deployment are not covered by these API tests.

Cross-repository dependencies

No MatrixOne source changes are included.

Comment thread memoria/crates/memoria-api/src/routes/mcp.rs Outdated

@aptend aptend left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Request changes for two authorization lifecycle issues:

  1. [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 call user_sql_store, which provisions a missing personal memory database. An identity-only key can therefore call a forbidden tool such as memory_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.

  2. [P1] Cached keys remain usable after expires_at. Non-whoami validation returns a cached principal before consulting the database, while CachedApiKeyPrincipal does 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/check expires_at on cache hits (or cap cache lifetime to it) and add a regression covering warm cache -> expiry -> REST and MCP authentication failure.

@gouhongshen gouhongshen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.
mergify Bot pushed a commit to matrixorigin/Astra that referenced this pull request Sep 8, 2026
## 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 aptend left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@aptend
aptend merged commit 58012cf into matrixorigin:main Sep 8, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants