docs: plan per-user tenancy model
This commit is contained in:
@@ -0,0 +1,161 @@
|
||||
# Per-user tenancy implementation plan
|
||||
|
||||
## Scope and current state
|
||||
|
||||
Gitea issue #10 asks AMCS to isolate memories by user, tenant, or workspace. The current branch already carries the first implementation pass, so this plan records the intended model and the remaining hardening work another worker should verify or finish.
|
||||
|
||||
Observed code paths:
|
||||
|
||||
- Authentication enters through `internal/auth/middleware.go`. API-key auth uses the configured header, defaulting to `x-brain-key`; bearer tokens can resolve through OAuth `TokenStore` or API keyring; HTTP Basic resolves OAuth client credentials.
|
||||
- `internal/auth/middleware.go` writes both `auth.key_id` and `tenancy.tenant_key` into the request context. The tenant key is intentionally opaque and currently equals the authenticated key id or OAuth client id.
|
||||
- `internal/tenancy/tenancy.go` exposes `WithTenantKey` and `KeyFromContext` only; callers should not infer user semantics from the tenant string.
|
||||
- Schema already has `tenant_key` on `projects`, `thoughts`, `stored_files`, `learnings`, `plans`, and `chat_histories` in `schema/*.dbml`.
|
||||
- `internal/store/tenancy.go` provides helper functions for appending tenant predicates to SQL.
|
||||
- Tenant scoping is already present in project, thought, and stored-file store paths. Some other project-owned domains still need explicit tenant enforcement.
|
||||
|
||||
## Tenant/user identity source
|
||||
|
||||
Use the authenticated principal id as the tenant boundary:
|
||||
|
||||
1. API key: resolve token through `auth.Keyring.Lookup`; use returned key id as `tenant_key`.
|
||||
2. OAuth bearer token: resolve token through `TokenStore.Lookup`; use returned client id/key id as `tenant_key`.
|
||||
3. OAuth Basic client credentials: resolve through `OAuthRegistry.Lookup`; use returned client id as `tenant_key`.
|
||||
4. Unauthenticated requests must not get tenant context and must not reach protected MCP/API handlers.
|
||||
|
||||
Do not store raw API keys or bearer tokens in tenant columns. Store only stable configured key ids/client ids. Yes, it is less flashy than inventing an account service before breakfast, but it keeps the trust boundary small and auditable.
|
||||
|
||||
## Required schema/model changes
|
||||
|
||||
Source-of-truth DBML changes:
|
||||
|
||||
- Add nullable `tenant_key text` to all user-owned tables:
|
||||
- `projects`
|
||||
- `thoughts`
|
||||
- `stored_files`
|
||||
- `learnings`
|
||||
- `plans`
|
||||
- `chat_histories`
|
||||
- Add tenant indexes:
|
||||
- single-column `tenant_key` for direct filtering
|
||||
- `(tenant_key, name)` unique on `projects`, replacing global `projects.name` uniqueness
|
||||
- `(tenant_key, project_id)` on `thoughts` for common project-scoped memory lookups
|
||||
- Regenerate SQL migrations and generated models from DBML; do not hand-edit generated Go models except as a temporary debugging step.
|
||||
|
||||
Important follow-up: `agent_skills`, `agent_guardrails`, `agent_personas`, `agent_parts`, traits, and arcs are currently global catalogs. Keep them global unless product requirements say skills/personas are private per tenant. Project join tables inherit protection through tenant-scoped project ids, but direct join queries must verify the project belongs to the tenant.
|
||||
|
||||
## Query scoping strategy
|
||||
|
||||
All request-facing store methods that touch tenant-owned rows must include tenant predicates whenever `tenancy.KeyFromContext(ctx)` returns a key.
|
||||
|
||||
Rules:
|
||||
|
||||
- Inserts must populate `tenant_key` from context.
|
||||
- Gets/updates/deletes by id or guid must add `and tenant_key = $n`.
|
||||
- Lists/searches must add `tenant_key = $n` before other filters.
|
||||
- Joins must scope the tenant-owned root table. Example: project summaries should count only thoughts that belong to the same tenant as the project, not merely thoughts with the same `project_id`.
|
||||
- Background maintenance jobs must either run per tenant, carry tenant context explicitly, or intentionally operate cross-tenant with an internal-only code path documented in the job.
|
||||
|
||||
Already-scoped paths to keep:
|
||||
|
||||
- `internal/store/projects.go`: create/get/list/touch projects.
|
||||
- `internal/store/thoughts.go`: create/list/get/update/delete/archive/search/stats/embedding repair paths.
|
||||
- `internal/store/files.go`: insert/get/list stored files.
|
||||
|
||||
Paths needing audit/hardening:
|
||||
|
||||
- `internal/store/learnings.go`: `CreateLearning`, `GetLearning`, and `ListLearnings` should populate/filter by tenant.
|
||||
- `internal/store/plans.go`: `CreatePlan`, `GetPlan`, `GetPlanDetail`, `UpdatePlan`, `DeletePlan`, `ListPlans`, dependency/related-plan operations, and plan skill/guardrail joins should scope to tenant-owned plans.
|
||||
- `internal/store/chat_histories.go`: chat history create/get/list/update paths should populate/filter by tenant.
|
||||
- `internal/store/thought_learning_links.go`: links traverse tenant-owned thoughts/learnings; queries should join and scope both sides or at least the tenant-owned root.
|
||||
- `internal/store/skills.go` and `internal/store/project_personas.go`: project-scoped joins should verify the project is visible to the current tenant before returning linked global catalog records.
|
||||
- ResolveSpec/admin CRUD endpoints exposing generated models must not bypass tenant-aware store methods. If they use direct generic model access, add middleware-level filter injection or disable tenant-owned tables from generic admin writes.
|
||||
|
||||
## Migration and backfill
|
||||
|
||||
Migration approach for existing single-tenant installs:
|
||||
|
||||
1. Add nullable `tenant_key` columns first; do not make them `not null` initially because existing deployments have historical rows without a principal.
|
||||
2. Backfill existing rows to a configured default tenant only if the instance enables multi-tenant mode or defines `auth.default_tenant_key`. Otherwise leave `NULL` rows visible only to unauthenticated/internal single-tenant contexts.
|
||||
3. Replace global project-name uniqueness with `(tenant_key, name)` uniqueness. For PostgreSQL, preserve legacy `NULL` semantics carefully: multiple null-tenant projects with the same name may be possible unless a partial unique index is added for null tenant rows.
|
||||
4. Add indexes concurrently where practical for production-sized tables.
|
||||
5. Document that after enabling auth-backed tenancy, legacy null-tenant data is not visible to authenticated tenants unless backfilled.
|
||||
|
||||
Recommended config addition:
|
||||
|
||||
- `auth.default_tenant_key` or `tenancy.default_key` for one-time/self-hosted backfill and development.
|
||||
- Optional `tenancy.mode: single|authenticated` so operators can keep current single-user behavior deliberately instead of discovering isolation by accident. An accident in auth is just a breach with better branding.
|
||||
|
||||
## Authorization checks
|
||||
|
||||
Authorization is row ownership by tenant key:
|
||||
|
||||
- A request may only see or mutate rows whose `tenant_key` equals the authenticated tenant key.
|
||||
- Cross-tenant ids/gids should behave as not found, not forbidden, to avoid existence leaks.
|
||||
- Tenant key is server-derived only. Ignore any client-provided `tenant_key` fields on tool/API inputs.
|
||||
- Project id references in create/update operations must be validated against the same tenant before use. This prevents attaching a new thought/file/plan to another tenant's project id.
|
||||
- Relationship operations must verify both endpoints are in the same tenant before creating links.
|
||||
- Global catalogs can be read across tenants only if intentionally shared; project-specific associations must be tenant-checked through the project.
|
||||
|
||||
## Affected endpoints/services/UI surfaces
|
||||
|
||||
Backend/MCP tools:
|
||||
|
||||
- Project tools: create/get/list/set active project.
|
||||
- Thought tools: capture, retrieve, update, delete/archive, semantic search, text search, stats, metadata and embedding repair queues.
|
||||
- File tools and binary file upload/download APIs.
|
||||
- Learning tools and thought-learning link tools.
|
||||
- Plan/task tools including dependencies, related plans, skills, and guardrails.
|
||||
- Chat history/session persistence tools.
|
||||
- Persona/skill/guardrail project-association tools.
|
||||
|
||||
HTTP/API boundaries:
|
||||
|
||||
- MCP SSE and streamable HTTP handlers must run behind auth middleware when auth is configured.
|
||||
- Any non-MCP REST endpoints under the admin/API server must either use tenant-aware stores or explicitly be internal/admin-only.
|
||||
- ResolveSpec admin CRUD views need tenant filter injection or table-level access restrictions for tenant-owned models.
|
||||
|
||||
UI:
|
||||
|
||||
- Project selector/list should naturally show tenant-scoped projects.
|
||||
- Admin tables for projects/thoughts/files/learnings/plans/chat histories must not show `tenant_key` as an editable field.
|
||||
- If a tenant switcher is ever added, it must map to a server-side authenticated principal or admin impersonation path, not a client-side query parameter. Obviously.
|
||||
|
||||
## Test cases
|
||||
|
||||
Minimum test matrix:
|
||||
|
||||
1. Auth middleware propagates tenant key for API-key header auth.
|
||||
2. Auth middleware propagates tenant key for bearer token via OAuth token store.
|
||||
3. Auth middleware propagates tenant key for HTTP Basic OAuth client credentials.
|
||||
4. Tenant A and tenant B can create projects with the same name; each only lists its own project.
|
||||
5. Tenant A cannot get/update/delete/archive Tenant B's thought by guid or numeric id.
|
||||
6. Semantic and text search return only same-tenant thoughts, including project-filtered searches.
|
||||
7. Stored file get/list/download is scoped; a file id from another tenant returns not found.
|
||||
8. Learning create/list/get is scoped, including links to thoughts.
|
||||
9. Plan create/list/get/update/delete and dependency/related-plan operations are scoped.
|
||||
10. Project skill/persona/guardrail association reads verify the project belongs to the tenant.
|
||||
11. Background metadata/embedding retry queues do not cross tenants unless intentionally internal.
|
||||
12. Migration test covers legacy null-tenant rows and backfill to default tenant.
|
||||
13. ResolveSpec/admin API tests prove tenant-owned resources are filtered or unavailable without admin override.
|
||||
|
||||
## Assumptions and blockers
|
||||
|
||||
Assumptions:
|
||||
|
||||
- The first tenancy boundary is authenticated principal id, not human account, org, or workspace. This is consistent with the current auth system and avoids adding an account model prematurely.
|
||||
- Null `tenant_key` remains the compatibility path for existing single-tenant/internal flows.
|
||||
- Global skills/personas/guardrails remain shared catalogs until a separate product decision makes them tenant-private.
|
||||
|
||||
Blockers/decisions needed:
|
||||
|
||||
- Decide whether legacy rows should be backfilled automatically to a configured default tenant during migration or left null until an operator runs an explicit backfill.
|
||||
- Decide whether ResolveSpec admin endpoints are trusted admin-only or must enforce tenant predicates like MCP tools.
|
||||
- Decide whether future OAuth subjects should distinguish user id from client id; the current implementation only has client/key id available.
|
||||
|
||||
## Implementation order
|
||||
|
||||
1. Finish tenant scoping audits for learnings, plans, chat histories, thought-learning links, and project catalog joins.
|
||||
2. Add tests proving cross-tenant invisibility for each store/tool domain before widening coverage. Yes, tests first; future us has enough enemies.
|
||||
3. Add config/backfill migration behavior and document operator steps.
|
||||
4. Lock down or filter ResolveSpec/admin tenant-owned model access.
|
||||
5. Run `make test` and `make build`; include exact results in the issue/PR comment.
|
||||
Reference in New Issue
Block a user