alephdata / aleph

Search and browse documents and data; find the people and companies you look for.
http://docs.aleph.occrp.org
MIT License
2k stars 267 forks source link

Do not allow duplicate role memberships #3815

Open tillprochaska opened 1 month ago

tillprochaska commented 1 month ago

Aleph users can be part of user groups. Users and groups are both modeled as "roles" in Aleph. To add a user to a group, a row is inserted into the role_membership table.

We’ve occasionally had issues with duplicate rows in this table, likely due to a race condition when Aleph handles parallel OAuth requests for the same user (https://github.com/alephdata/aleph/issues/3811#issuecomment-2232953036), but we haven’t been able to reproduce or pinpoint the exact root cause.

The effect of a duplicate role membership is that users are permanently blocked from authenticating until the duplicates are removed.

This PR adds a primary key constraint to the role_membership table. While this does not address the root cause of the problem, it prevents duplicates from being inserted into the table. In practice, this means that in rare cases, OAuth requests may fail when the constraint is violated, but users can simply retry the authentication in that case.


In addition, I’ve implemented two related changes:


[!IMPORTANT]
This PR contains a migration that adds a primary key constraint to an existing table. The migration will fail if the existing table violates the constraint. It is necessary to manually check and remove whether duplicates exist in this table before applying the migration. See https://github.com/alephdata/aleph/issues/3811#issuecomment-2223355962 for a query that returns duplicates.

tillprochaska commented 1 month ago

I suggest we keep this PR open and merge it when we are ready to release/deploy the changes (after a stable 4.0 release), to ensure we do not forget about the breaking changes/manual checks required.