Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(auth): Run token bulk update statements in atomic transaction #37676

Merged
merged 1 commit into from
Apr 17, 2023

Conversation

ChristophWurst
Copy link
Member

@ChristophWurst ChristophWurst commented Apr 11, 2023

  • Resolves: #

Summary

Ensures that if we want to update all tokens of a user there are no changes between reading the tokens and updating them individually. This change uses the atomic helper function https://docs.nextcloud.com/server/latest/developer_manual/basics/storage/database.html#transactions.

Concurrent requests may otherwise insert, update or delete one of the rows.

This is best reviewed as https://github.com/nextcloud/server/pull/37676/files?w=1 to ignore whitespace changes.

TODO

  • Wrap operations in transaction

Checklist

@ChristophWurst ChristophWurst added the 2. developing Work in progress label Apr 11, 2023
@ChristophWurst ChristophWurst self-assigned this Apr 11, 2023
@ChristophWurst ChristophWurst added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Apr 12, 2023
@ChristophWurst ChristophWurst added this to the Nextcloud 27 milestone Apr 12, 2023
@ChristophWurst ChristophWurst requested a review from rullzer April 12, 2023 07:35
@ChristophWurst ChristophWurst marked this pull request as ready for review April 12, 2023 07:35
@ChristophWurst ChristophWurst changed the title fix(auth): Run token statements in atomic transaction fix(auth): Run token bulk update statements in atomic transaction Apr 12, 2023
All or nothing

Signed-off-by: Christoph Wurst <christoph@winzerhof-wurst.at>
@ChristophWurst ChristophWurst force-pushed the fix/auth/atomic-password-db-statements branch from 39098ff to 5eb768a Compare April 12, 2023 13:55
@ChristophWurst ChristophWurst added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Apr 12, 2023
@szaimen
Copy link
Contributor

szaimen commented Apr 17, 2023

CI failure unrelated

@szaimen szaimen merged commit 0a76382 into master Apr 17, 2023
@szaimen szaimen deleted the fix/auth/atomic-password-db-statements branch April 17, 2023 21:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4. to release Ready to be released and/or waiting for tests to finish feature: authentication
Projects
Development

Successfully merging this pull request may close these issues.

5 participants