[PR #392] [MERGED] fix: use transactions when operations involve multiple database queries #832

Closed
opened 2025-10-07 00:23:08 +03:00 by OVERLORD · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/pocket-id/pocket-id/pull/392
Author: @ItalyPaleAle
Created: 3/27/2025
Status: Merged
Merged: 4/6/2025
Merged by: @stonith404

Base: mainHead: tx


📝 Commits (9)

📊 Changes

33 files changed (+1398 additions, -498 deletions)

View changed files

📝 backend/internal/bootstrap/bootstrap.go (+6 -2)
📝 backend/internal/bootstrap/router_bootstrap.go (+7 -6)
📝 backend/internal/common/errors.go (+1 -1)
📝 backend/internal/controller/api_key_controller.go (+3 -3)
📝 backend/internal/controller/app_config_controller.go (+9 -10)
📝 backend/internal/controller/audit_log_controller.go (+4 -2)
📝 backend/internal/controller/custom_claim_controller.go (+3 -3)
📝 backend/internal/controller/oidc_controller.go (+16 -15)
📝 backend/internal/controller/user_controller.go (+13 -13)
📝 backend/internal/controller/user_group_controller.go (+9 -7)
📝 backend/internal/controller/webauthn_controller.go (+7 -7)
📝 backend/internal/job/db_cleanup_job.go (+34 -17)
📝 backend/internal/job/file_cleanup_job.go (+11 -5)
📝 backend/internal/job/job.go (+3 -1)
📝 backend/internal/job/ldap_job.go (+12 -9)
📝 backend/internal/middleware/api_key_auth.go (+1 -1)
📝 backend/internal/service/api_key_service.go (+36 -20)
📝 backend/internal/service/app_config_service.go (+137 -57)
📝 backend/internal/service/audit_log_service.go (+40 -16)
📝 backend/internal/service/custom_claim_service.go (+86 -45)

...and 13 more files

📄 Description

Currently, the backend does not use database transactions, even when there are operations performed in sequence. For example: 4d049bbe24/backend/internal/service/user_service.go (L166-L194) where the user is loaded from the DB and then updates

This can cause a number of problems if multiple requests happen in parallel, and in some cases could also have security implications.

This PR updates the backend's to add transactions where they are needed.


🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/pocket-id/pocket-id/pull/392 **Author:** [@ItalyPaleAle](https://github.com/ItalyPaleAle) **Created:** 3/27/2025 **Status:** ✅ Merged **Merged:** 4/6/2025 **Merged by:** [@stonith404](https://github.com/stonith404) **Base:** `main` ← **Head:** `tx` --- ### 📝 Commits (9) - [`9c8d281`](https://github.com/pocket-id/pocket-id/commit/9c8d28172dd665d02dd90536acda486db3328a37) use transactions when operations involve multiple database queries - [`ae0daef`](https://github.com/pocket-id/pocket-id/commit/ae0daef18304ce1ec7e7730bfb20e9ad444ab238) Update more methods for transactions and contexts - [`008c9ac`](https://github.com/pocket-id/pocket-id/commit/008c9ac3d75f22f85c8b5983b9c812aa8b74d2b3) Merge branch 'main' into tx - [`db96490`](https://github.com/pocket-id/pocket-id/commit/db9649070a6d4c282f4961a0fe23ae5eb90acffa) Merge branch 'main' into tx - [`d8b9e73`](https://github.com/pocket-id/pocket-id/commit/d8b9e7318aab8493e16b868db3e870cf8110fade) Merge branch 'main' of https://github.com/pocket-id/pocket-id into tx - [`e748c47`](https://github.com/pocket-id/pocket-id/commit/e748c474d172ca6a35789778aa88adf7a857143f) Missing these from merge - [`3d5a05f`](https://github.com/pocket-id/pocket-id/commit/3d5a05f16ae615a63916c6420589bd300e69622d) Updated newly-added methods too - [`3827b50`](https://github.com/pocket-id/pocket-id/commit/3827b5074af31506109934937f6f04e9b67b8bc3) Merge branch 'main' of https://github.com/pocket-id/pocket-id into tx - [`3b89df0`](https://github.com/pocket-id/pocket-id/commit/3b89df0c51107d7af2513bc8548de28fc95dbc2f) Re-add transaction needed to roll-back ### 📊 Changes **33 files changed** (+1398 additions, -498 deletions) <details> <summary>View changed files</summary> 📝 `backend/internal/bootstrap/bootstrap.go` (+6 -2) 📝 `backend/internal/bootstrap/router_bootstrap.go` (+7 -6) 📝 `backend/internal/common/errors.go` (+1 -1) 📝 `backend/internal/controller/api_key_controller.go` (+3 -3) 📝 `backend/internal/controller/app_config_controller.go` (+9 -10) 📝 `backend/internal/controller/audit_log_controller.go` (+4 -2) 📝 `backend/internal/controller/custom_claim_controller.go` (+3 -3) 📝 `backend/internal/controller/oidc_controller.go` (+16 -15) 📝 `backend/internal/controller/user_controller.go` (+13 -13) 📝 `backend/internal/controller/user_group_controller.go` (+9 -7) 📝 `backend/internal/controller/webauthn_controller.go` (+7 -7) 📝 `backend/internal/job/db_cleanup_job.go` (+34 -17) 📝 `backend/internal/job/file_cleanup_job.go` (+11 -5) 📝 `backend/internal/job/job.go` (+3 -1) 📝 `backend/internal/job/ldap_job.go` (+12 -9) 📝 `backend/internal/middleware/api_key_auth.go` (+1 -1) 📝 `backend/internal/service/api_key_service.go` (+36 -20) 📝 `backend/internal/service/app_config_service.go` (+137 -57) 📝 `backend/internal/service/audit_log_service.go` (+40 -16) 📝 `backend/internal/service/custom_claim_service.go` (+86 -45) _...and 13 more files_ </details> ### 📄 Description Currently, the backend does not use database transactions, even when there are operations performed in sequence. For example: https://github.dev/pocket-id/pocket-id/blob/4d049bbe24aa0b714c6f97369f1562064076a3b4/backend/internal/service/user_service.go#L166-L194 where the user is loaded from the DB and then updates This can cause a number of problems if multiple requests happen in parallel, and in some cases _could_ also have security implications. This PR updates the backend's to add transactions where they are needed. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
OVERLORD added the pull-request label 2025-10-07 00:23:08 +03:00
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: starred/pocket-id#832