HandleAPIClientRedirectURIsPut in src/authserver/internal/handlers/apihandlers/handler_api_clients.go replaces a client's whole redirect URI set, and it performs every write outside a transaction. It calls database.ClientLoadRedirectURIs(nil, client) to read the current list, then database.CreateRedirectURI(nil, ...) once per added URI and database.DeleteRedirectURI(nil, ...) once per removed one, each on its own connection.
Two things follow.
A failure part way through commits part of the save. A database error on the second of three writes leaves the first one persisted, answers 500 INTERNAL_ERROR, and leaves the client's list as neither what the administrator sent nor what it was before. The case where that matters is the case an administrator is most likely to be in a hurry for: removing a compromised callback and adding its replacement in one save, which can end up having done only the removal, or only the addition.
Two concurrent saves produce the union of both lists. Each request reads the same current list and writes its own diff of it, so an administrator removing a URI while another adds one can find the removed URI still registered. The read is not serialized against the writes by anything.
HandleAPIClientWebOriginsPut, the endpoint next door, was repaired for exactly this in #250: BeginTransaction, a deferred RollbackTransaction, AcquireClientRow to serialize the read against a concurrent save, the current-list read, the inserts and deletes, and CommitTransaction. updateClientNotOwningAuthenticationMode in the same file has run that sequence since #245. Nothing new is needed on the Database interface: all four methods are already on it and already covered by the data tier.
Redirect URIs were deliberately left out of #250 to keep that change from widening twice; it fixed the flow gate on this endpoint and nothing else about it.
What closing it takes: the same wrapping, plus a unit case on handler_api_clients_test.go's strict-mock harness asserting that the current list is read under AcquireClientRow inside the writing transaction, and one asserting that a failed write reaches RollbackTransaction and never CommitTransaction. Both shapes exist for the web-origins endpoint and can be copied. No data tier and no migration.
Related but distinct: #260 defect 3 (whole-client column projection), #249 (the same family on user_session_clients and user_consents).
HandleAPIClientRedirectURIsPutinsrc/authserver/internal/handlers/apihandlers/handler_api_clients.goreplaces a client's whole redirect URI set, and it performs every write outside a transaction. It callsdatabase.ClientLoadRedirectURIs(nil, client)to read the current list, thendatabase.CreateRedirectURI(nil, ...)once per added URI anddatabase.DeleteRedirectURI(nil, ...)once per removed one, each on its own connection.Two things follow.
A failure part way through commits part of the save. A database error on the second of three writes leaves the first one persisted, answers
500 INTERNAL_ERROR, and leaves the client's list as neither what the administrator sent nor what it was before. The case where that matters is the case an administrator is most likely to be in a hurry for: removing a compromised callback and adding its replacement in one save, which can end up having done only the removal, or only the addition.Two concurrent saves produce the union of both lists. Each request reads the same current list and writes its own diff of it, so an administrator removing a URI while another adds one can find the removed URI still registered. The read is not serialized against the writes by anything.
HandleAPIClientWebOriginsPut, the endpoint next door, was repaired for exactly this in #250:BeginTransaction, a deferredRollbackTransaction,AcquireClientRowto serialize the read against a concurrent save, the current-list read, the inserts and deletes, andCommitTransaction.updateClientNotOwningAuthenticationModein the same file has run that sequence since #245. Nothing new is needed on theDatabaseinterface: all four methods are already on it and already covered by the data tier.Redirect URIs were deliberately left out of #250 to keep that change from widening twice; it fixed the flow gate on this endpoint and nothing else about it.
What closing it takes: the same wrapping, plus a unit case on
handler_api_clients_test.go's strict-mock harness asserting that the current list is read underAcquireClientRowinside the writing transaction, and one asserting that a failed write reachesRollbackTransactionand neverCommitTransaction. Both shapes exist for the web-origins endpoint and can be copied. No data tier and no migration.Related but distinct: #260 defect 3 (whole-client column projection), #249 (the same family on
user_session_clientsanduser_consents).