This repository was archived by the owner on Apr 26, 2024. It is now read-only.
-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Setting a user's external_id via the admin API returns 500 and deletes users existing external mappings if that external ID is already mapped #10846
Copy link
Copy link
Closed
Labels
S-MinorBlocks non-critical functionality, workarounds exist.Blocks non-critical functionality, workarounds exist.T-DefectBugs, crashes, hangs, security vulnerabilities, or other reported issues.Bugs, crashes, hangs, security vulnerabilities, or other reported issues.good first issueGood for newcomersGood for newcomers
Description
Attempting to use the create or modify user account admin api to add an external ID to a user that is already mapped to another user fails with a unique constraint db error. It should really return a HTTP 400 error instead.
In addition, any existing external IDs for the user that we were trying to modify will be erased (all entries for that user are removed from the user_external_ids
table. This is due to the fact that the removal and addition of old and new IDs are separate actions:
synapse/synapse/rest/admin/users.py
Lines 285 to 299 in 220f901
# remove old external_ids | |
for auth_provider, external_id in del_external_ids: | |
await self.store.remove_user_external_id( | |
auth_provider, | |
external_id, | |
user_id, | |
) | |
# add new external_ids | |
for auth_provider, external_id in add_external_ids: | |
await self.store.record_user_external_id( | |
auth_provider, | |
external_id, | |
user_id, | |
) |
They should really be bundled into a single database transaction in a new storage function so that it's not possible for the user to be left without any mappings.
Metadata
Metadata
Assignees
Labels
S-MinorBlocks non-critical functionality, workarounds exist.Blocks non-critical functionality, workarounds exist.T-DefectBugs, crashes, hangs, security vulnerabilities, or other reported issues.Bugs, crashes, hangs, security vulnerabilities, or other reported issues.good first issueGood for newcomersGood for newcomers