Skip to content

feat(auth): add User model and per-user repository ownership (E1.1)#70

Open
SHAURYAKSHARMA24 wants to merge 2 commits into
devfrom
feature/auth-user-model
Open

feat(auth): add User model and per-user repository ownership (E1.1)#70
SHAURYAKSHARMA24 wants to merge 2 commits into
devfrom
feature/auth-user-model

Conversation

@SHAURYAKSHARMA24

Copy link
Copy Markdown
Collaborator

Summary

Implements E1.1 — the identity foundation for M1's auth epic. Adds a User table and scopes every repository to an owner, so a request can only ever read or mutate its own data. Deliberately independent of the native-JWT-vs-Auth0 decision still open on #60: a User row and owner_id scoping are needed either way, so this unblocks E1.2+ without pre-committing that choice.

Backend only. No frontend, no application behaviour change for the existing single-tenant flow (everything is transparently attributed to a seed user until sign-in lands).

What's here

  • User model — UUID id, unique email, is_active, timestamps. No credential column yet; password/token handling is E1.2's job.
  • owner_id FK on repositories, indexed. Alembic 0002 creates the users table, seeds a system user (00000000-…-0000), and backfills existing repositories to it, so the migration is non-destructive on a populated database.
  • Owner-scoped accessRepositoryService filters every read by the current user and returns 404, not 403, for another user's repository so existence never leaks. Import de-duplication is now per-owner (two users may import the same repo).
  • Temporary current-user seamget_current_user attributes requests to the seed user, with an X-Dev-User header override honoured only in development/test so multi-tenant behaviour is testable before real sign-in. E1.2 swaps the body of this one dependency for token verification; its callers don't change.

Scoping note: the unscoped repository queries remain for internal callers (analysis / ai / documentation). Moving those onto the current user is E1.3 by design, so this PR stays task-sized.

Type

  • Bug fix
  • Feature
  • Engineering task
  • Documentation
  • Security

Verification

  • Frontend build/lint run where relevant — n/a, backend only
  • Backend tests run where relevant
  • Docker Compose config/runtime readiness checked where relevant — see note on Postgres below
  • Release/deployment docs updated where relevant — n/a
  • Manual verification described below

Commands/results:

# Full backend suite (adds 5 tests):
$ python -m pytest
92 passed, 1 skipped

# New tests:
tests/test_repository_ownership.py  — cross-user GET/list/DELETE return 404/empty;
                                      owner keeps access; seed user isolated from dev users
tests/test_migrations.py            — full revision chain up -> down -> up on a fresh DB

# Migration verified on SQLite (up / down / up), including backfill of a pre-existing
# repository created under 0001:
$ alembic upgrade 0001_initial   # then inserted a legacy repo with no owner_id
$ alembic upgrade head
  legacy repo owner_id backfilled to: 00000000-0000-0000-0000-000000000000
  users seeded with: [('00000000-0000-0000-0000-000000000000', 'system@partha.local')]
$ alembic downgrade base   # drops owner_id, FK, index, users table cleanly
$ alembic upgrade head     # re-applies clean
  repositories.owner_id: VARCHAR(36), FK -> users, index ix_repositories_owner_id

Postgres: I could not run Postgres locally (no Docker on this machine), so the migration is verified on SQLite only here. The Docker Compose CI job exercises the Postgres path — the backend container runs alembic upgrade head against Postgres on start (AUTO_CREATE_TABLES=false), then the readiness probe boots the app. The migration is written to be dialect-safe (batch operations; the seed user is inserted before the FK is validated so the constraint holds on a populated table). If the Compose job surfaces anything, I'll fix it here.

Risk and Rollback

  • Risk level: Medium (schema migration + a change to how every repository read is scoped).
  • The migration is additive and backfills existing rows, so no data is lost; downgrade is implemented and verified. Existing single-tenant behaviour is preserved because unauthenticated requests resolve to the seed user that owns all pre-existing data.
  • Security note for review: the X-Dev-User seam is honoured only when APP_ENV is development/test, and ignored in production. It does not lower the current security posture — the app is already open/single-tenant pre-auth — and it is removed by E1.2. Flagging it as a conscious, temporary choice rather than hidden.
  • Rollback: revert this PR, then alembic downgrade 0001_initial (or redeploy the prior image, which pins the earlier head).

Checklist

  • No unrelated behavior changes
  • No secrets, local env files, generated build artifacts, or local databases committed
  • API behavior documented if changed — repository reads are now owner-scoped; described above
  • UI states covered for loading, empty, error, and success where relevant — n/a, no UI in this PR
  • Security/privacy impact considered — cross-user isolation added; the temporary dev seam is gated to non-production and noted above

Closes #61

Introduce identity persistence ahead of authentication. Adds a User table and
scopes repositories to an owner so a request can only ever see its own data.
This is the foundation E1.2+ build on, and it is independent of the
native-JWT-vs-Auth0 decision still open on the E1 epic.

- User model: UUID id, unique email, is_active, timestamps. No credential
  column yet; that belongs to E1.2.
- owner_id FK on repositories, indexed. Alembic 0002 creates the users table,
  seeds a system user, and backfills existing repositories to it so the
  upgrade is non-destructive. upgrade/downgrade/upgrade all verified.
- Owner-scoped data access: RepositoryService filters every read by the
  current user and returns 404 (not 403) for another user's repository, so
  existence never leaks. Import de-duplication is now per-owner.
- Temporary current-user seam (get_current_user) attributes requests to the
  seed user, with an X-Dev-User override honoured only in development/test so
  multi-tenant behaviour is testable before sign-in exists. E1.2 replaces the
  body of this dependency with token verification; its callers do not change.

The unscoped repository queries remain for internal callers (analysis, ai,
documentation) and move onto the current user in E1.3.

Tests: cross-user read/list/delete denial and a migration round-trip. Full
backend suite green (92 passed, 1 skipped).
from app.core.config import get_settings
from app.models.base import Base
from app.models.repository import RepositoryRecord
from app.models.user import User
Comment thread apps/backend/app/main.py
from app.core.logging import configure_logging
from app.core.observability import new_request_id, reset_request_id, runtime_metrics, set_request_id
from app.models import RepositoryRecord # noqa: F401 - imported so metadata includes model
from app.models import RepositoryRecord, User # noqa: F401 - imported so metadata includes models
…ion(32)

The 0002 revision id was 35 characters. Alembic's default
alembic_version.version_num column is VARCHAR(32); SQLite ignores the length
(so local tests passed) but PostgreSQL enforces it and the Docker Compose CI
job failed writing the version row. Rename the revision (and its file) to
0002_users_and_repo_owner (25 chars). No schema or data change.
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.

E1.1 — Add User model and auth tables + migration

1 participant