finos / finos/git-proxy

Deleting or renaming a user leaves their repo permission grants behind (all sink backends)

Open
#1,705 1 comment 0 reactions 0 assignees View on GitHub
Dominant language
TypeScript
Stars
249
Forks
176
Avg merge
3d 8h
Merged PRs (30d)
20

Description

## Summary

Repo permissions are keyed by plain username in every sink backend (the `users.canPush` / `users.canAuthorise` arrays in mongo/fs, the `repo_users.username` column in postgres), with no link to the user record itself. `deleteUser` does not touch these grants, and `updateUser` renaming a username does not carry them over.

Consequences:

- **Grant inheritance**: delete user `alice`, create a new, unrelated `alice` later — the new account silently inherits every `canPush` / `canAuthorise` grant the old one held.
- **Orphaned grants on rename**: renaming a username leaves the old name's grants behind, and the renamed user loses their permissions.

Since those arrays gate pushing and approving, grant inheritance is effectively a privilege-escalation-by-username-reuse hole.

## Where

- `src/db/file` and `src/db/mongo`: usernames embedded in each repo document's `users` arrays; `deleteUser` only removes the user record.
- `src/db/postgres`: `repo_users.username` is `TEXT` with no foreign key to `users`; `deleteUser` deletes only from `users`.

## Suggested direction

Fix it uniformly across all three backends so behaviour stays at parity (see `.agents/skills/sink-parity/SKILL.md`):

1. `deleteUser` removes the username from every repo's grants (all backends).
2. Username rename (where supported) migrates grants, or is rejected.
3. Optionally, postgres adds a real foreign key from `repo_users.username` once usernames are stable, making the invariant self-enforcing there.

## Context

Raised during review of #1687 (https://github.com/finos/git-proxy/pull/1687#discussion_r3885772973). The postgres backend merely normalised the same pre-existing hole the document backends carry, so the fix belongs in a cross-backend change rather than that PR.

Contributor guide

Open the contributing guide

Research direction

Read .agents/skills/sink-parity/SKILL.md first, then inspect deleteUser and username-update paths under src/db/file, src/db/mongo, and src/db/postgres. Trace how repo grants are stored and changed in each backend. Done means deletion cannot leave grants behind and rename behavior is consistent across the supported backends.

Written by the indexing model from the issue text.

Assessment

Tech stack
mongodb, postgresql, typescript
Domain
backend, databases, security
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.