SQLite: migration 002 never dropped UNIQUE(project_id, user_id) on project_access #133

New issue

Closed

opened 2026-08-30 17:16:45 +02:00 by qwc · 0 comments

qwc commented

2026-08-30 17:16:45 +02:00

Owner

Found while fixing #126 (see PR #132). This is a separate defect and needs its own migration.

What's wrong

internal/database/migrations/sqlite/002_auth_groups.up.sql intends to replace the per-user uniqueness on project_access with one that includes source:

DROP INDEX IF EXISTS idx_project_access_unique;
CREATE UNIQUE INDEX idx_project_access_source ON project_access(project_id, user_id, source);

But in SQLite the original constraint is a table-level UNIQUE(project_id, user_id) declared inside CREATE TABLE (001_initial.up.sql). Its backing index is sqlite_autoindex_project_access_1 and cannot be removed with DROP INDEX. The statement above matches nothing and succeeds silently, so both constraints are live.

Verified against a migrated test database — sqlite_master still shows:

table project_access :: CREATE TABLE project_access (
    ...
    role TEXT NOT NULL DEFAULT 'viewer', source TEXT NOT NULL DEFAULT 'manual',
    UNIQUE(project_id, user_id)
)
index sqlite_autoindex_project_access_1 :: <auto-index from table constraint>
index idx_project_access_source :: CREATE UNIQUE INDEX ... (project_id, user_id, source)

Postgres (DROP CONSTRAINT ... project_access_project_id_user_id_key) and MySQL (DROP INDEX uq_project_user) both do drop theirs correctly. SQLite is the default driver, so most deployments have the wrong schema.

Impact

A user can hold only one project_access row per project on SQLite, so multi-source grants are impossible:

If a user already has a manual grant and an LDAP/OAuth2 group mapping then grants them access, ProjectAccessStore.Grant hits the UNIQUE(project_id, user_id) constraint. Its ON CONFLICT(project_id, user_id, source) clause targets the other index, so the violation isn't handled and the insert fails with UNIQUE constraint failed: project_access.project_id, project_access.user_id. The group access is never applied.
The reverse fails the same way: granting manual access to a user who already has a synced grant errors out, which likely looks like a dead "Grant Access" button.

Fix sketch

A SQLite migration that rebuilds the table without the inline UNIQUE(project_id, user_id), in the style of the existing 002_auth_groups.down.sql rebuild:

CREATE TABLE project_access_new (...) — same columns, no table-level UNIQUE, FKs preserved.
Copy rows across.
DROP TABLE project_access; ALTER TABLE project_access_new RENAME TO project_access;
Recreate idx_project_access_source and any other indexes.

Existing data is safe: the surviving rows are already unique per (project_id, user_id), which is stricter than the target constraint. Postgres and MySQL need no change, so the migration is SQLite-only.

Worth a test asserting a user can hold a manual and an ldap grant on the same project at once. PR #132 originally had that test; it could not pass against this schema, so it was narrowed to a single-row source-targeting check with a comment pointing here — restore the two-row version alongside the migration.

Reported by Claude Opus 5.

Found while fixing #126 (see PR #132). This is a separate defect and needs its own migration. ## What's wrong `internal/database/migrations/sqlite/002_auth_groups.up.sql` intends to replace the per-user uniqueness on `project_access` with one that includes `source`: ```sql DROP INDEX IF EXISTS idx_project_access_unique; CREATE UNIQUE INDEX idx_project_access_source ON project_access(project_id, user_id, source); ``` But in SQLite the original constraint is a **table-level** `UNIQUE(project_id, user_id)` declared inside `CREATE TABLE` (`001_initial.up.sql`). Its backing index is `sqlite_autoindex_project_access_1` and cannot be removed with `DROP INDEX`. The statement above matches nothing and succeeds silently, so both constraints are live. Verified against a migrated test database — `sqlite_master` still shows: ``` table project_access :: CREATE TABLE project_access ( ... role TEXT NOT NULL DEFAULT 'viewer', source TEXT NOT NULL DEFAULT 'manual', UNIQUE(project_id, user_id) ) index sqlite_autoindex_project_access_1 :: <auto-index from table constraint> index idx_project_access_source :: CREATE UNIQUE INDEX ... (project_id, user_id, source) ``` Postgres (`DROP CONSTRAINT ... project_access_project_id_user_id_key`) and MySQL (`DROP INDEX uq_project_user`) both do drop theirs correctly. **SQLite is the default driver**, so most deployments have the wrong schema. ## Impact A user can hold only one `project_access` row per project on SQLite, so multi-source grants are impossible: - If a user already has a `manual` grant and an LDAP/OAuth2 group mapping then grants them access, `ProjectAccessStore.Grant` hits the `UNIQUE(project_id, user_id)` constraint. Its `ON CONFLICT(project_id, user_id, source)` clause targets the *other* index, so the violation isn't handled and the insert fails with `UNIQUE constraint failed: project_access.project_id, project_access.user_id`. The group access is never applied. - The reverse fails the same way: granting manual access to a user who already has a synced grant errors out, which likely looks like a dead "Grant Access" button. ## Fix sketch A SQLite migration that rebuilds the table without the inline `UNIQUE(project_id, user_id)`, in the style of the existing `002_auth_groups.down.sql` rebuild: 1. `CREATE TABLE project_access_new (...)` — same columns, no table-level UNIQUE, FKs preserved. 2. Copy rows across. 3. `DROP TABLE project_access; ALTER TABLE project_access_new RENAME TO project_access;` 4. Recreate `idx_project_access_source` and any other indexes. Existing data is safe: the surviving rows are already unique per `(project_id, user_id)`, which is stricter than the target constraint. Postgres and MySQL need no change, so the migration is SQLite-only. Worth a test asserting a user can hold a `manual` and an `ldap` grant on the same project at once. PR #132 originally had that test; it could not pass against this schema, so it was narrowed to a single-row source-targeting check with a comment pointing here — restore the two-row version alongside the migration. --- Reported by Claude Opus 5.

qwc referenced this issue from a commit

2026-08-30 17:17:02 +02:00

Point the revoke test comment at issue #133

qwc referenced this issue from a commit

2026-08-30 17:34:27 +02:00

Drop the leftover per-user unique constraint on SQLite (fixes #133)

qwc referenced this issue from a pull request that will close it,

2026-08-30 17:34:43 +02:00

Drop the leftover per-user unique constraint on SQLite (fixes #133) #134

qwc closed this issue

2026-08-30 18:04:52 +02:00

qwc referenced this issue from a commit