refactor: rollback db changes with defer everywhere

backend/internal/service/app_config_service.go

@@ -207,7 +207,6 @@ func (s *AppConfigService) UpdateAppConfig(ctx context.Context, input dto.AppCon
 
 	tx := s.db.Begin()
 	defer func() {
-		// This is a no-op if the transaction has been committed already
 		tx.Rollback()
 	}()
 
@@ -345,7 +344,6 @@ func (s *AppConfigService) UpdateImage(ctx context.Context, uploadedFile *multip
 func (s *AppConfigService) InitDbConfig(ctx context.Context) (err error) {
 	tx := s.db.Begin()
 	defer func() {
-		// This is a no-op if the transaction has been committed already
 		tx.Rollback()
 	}()
 

backend/internal/service/custom_claim_service.go

@@ -75,7 +75,6 @@ func (s *CustomClaimService) updateCustomClaims(ctx context.Context, idType idTy
 
 	tx := s.db.Begin()
 	defer func() {
-		// This is a no-op if the transaction has been committed already
 		tx.Rollback()
 	}()
 

backend/internal/service/oidc_service.go

@@ -44,7 +44,6 @@ func NewOidcService(db *gorm.DB, jwtService *JwtService, appConfigService *AppCo
 func (s *OidcService) Authorize(ctx context.Context, input dto.AuthorizeOidcClientRequestDto, userID, ipAddress, userAgent string) (string, string, error) {
 	tx := s.db.Begin()
 	defer func() {
-		// This is a no-op if the transaction has been committed already
 		tx.Rollback()
 	}()
 
@@ -194,7 +193,6 @@ func (s *OidcService) CreateTokens(ctx context.Context, code, grantType, clientI
 func (s *OidcService) createTokenFromAuthorizationCode(ctx context.Context, code, clientID, clientSecret, codeVerifier string) (idToken string, accessToken string, refreshToken string, exp int, err error) {
 	tx := s.db.Begin()
 	defer func() {
-		// This is a no-op if the transaction has been committed already
 		tx.Rollback()
 	}()
 
@@ -284,7 +282,6 @@ func (s *OidcService) createTokenFromRefreshToken(ctx context.Context, refreshTo
 
 	tx := s.db.Begin()
 	defer func() {
-		// This is a no-op if the transaction has been committed already
 		tx.Rollback()
 	}()
 
@@ -421,7 +418,6 @@ func (s *OidcService) CreateClient(ctx context.Context, input dto.OidcClientCrea
 func (s *OidcService) UpdateClient(ctx context.Context, clientID string, input dto.OidcClientCreateDto) (model.OidcClient, error) {
 	tx := s.db.Begin()
 	defer func() {
-		// This is a no-op if the transaction has been committed already
 		tx.Rollback()
 	}()
 
@@ -474,7 +470,6 @@ func (s *OidcService) DeleteClient(ctx context.Context, clientID string) error {
 func (s *OidcService) CreateClientSecret(ctx context.Context, clientID string) (string, error) {
 	tx := s.db.Begin()
 	defer func() {
-		// This is a no-op if the transaction has been committed already
 		tx.Rollback()
 	}()
 
@@ -548,7 +543,6 @@ func (s *OidcService) UpdateClientLogo(ctx context.Context, clientID string, fil
 
 	tx := s.db.Begin()
 	defer func() {
-		// This is a no-op if the transaction has been committed already
 		tx.Rollback()
 	}()
 
@@ -588,7 +582,6 @@ func (s *OidcService) UpdateClientLogo(ctx context.Context, clientID string, fil
 func (s *OidcService) DeleteClientLogo(ctx context.Context, clientID string) error {
 	tx := s.db.Begin()
 	defer func() {
-		// This is a no-op if the transaction has been committed already
 		tx.Rollback()
 	}()
 
@@ -630,7 +623,6 @@ func (s *OidcService) DeleteClientLogo(ctx context.Context, clientID string) err
 func (s *OidcService) GetUserClaimsForClient(ctx context.Context, userID string, clientID string) (map[string]interface{}, error) {
 	tx := s.db.Begin()
 	defer func() {
-		// This is a no-op if the transaction has been committed already
 		tx.Rollback()
 	}()
 
@@ -722,7 +714,6 @@ func (s *OidcService) getUserClaimsForClientInternal(ctx context.Context, userID
 func (s *OidcService) UpdateAllowedUserGroups(ctx context.Context, id string, input dto.OidcUpdateAllowedUserGroupsDto) (client model.OidcClient, err error) {
 	tx := s.db.Begin()
 	defer func() {
-		// This is a no-op if the transaction has been committed already
 		tx.Rollback()
 	}()
 

backend/internal/service/user_group_service.go

@@ -64,6 +64,9 @@ func (s *UserGroupService) getInternal(ctx context.Context, id string, tx *gorm.
 
 func (s *UserGroupService) Delete(ctx context.Context, id string) error {
 	tx := s.db.Begin()
+	defer func() {
+		tx.Rollback()
+	}()
 
 	var group model.UserGroup
 	err := tx.
@@ -77,10 +80,6 @@ func (s *UserGroupService) Delete(ctx context.Context, id string) error {
 
 	// Disallow deleting the group if it is an LDAP group and LDAP is enabled
 	if group.LdapID != nil && s.appConfigService.DbConfig.LdapEnabled.IsTrue() {
-		err = tx.Rollback().Error
-		if err != nil {
-			return err
-		}
 		return &common.LdapUserGroupUpdateError{}
 	}
 
@@ -125,16 +124,17 @@ func (s *UserGroupService) createInternal(ctx context.Context, input dto.UserGro
 
 func (s *UserGroupService) Update(ctx context.Context, id string, input dto.UserGroupCreateDto, allowLdapUpdate bool) (group model.UserGroup, err error) {
 	tx := s.db.Begin()
+	defer func() {
+		tx.Rollback()
+	}()
 
 	group, err = s.updateInternal(ctx, id, input, allowLdapUpdate, tx)
 	if err != nil {
-		tx.Rollback()
 		return model.UserGroup{}, err
 	}
 
 	err = tx.Commit().Error
 	if err != nil {
-		tx.Rollback()
 		return model.UserGroup{}, err
 	}
 
@@ -171,16 +171,17 @@ func (s *UserGroupService) updateInternal(ctx context.Context, id string, input
 
 func (s *UserGroupService) UpdateUsers(ctx context.Context, id string, userIds []string) (group model.UserGroup, err error) {
 	tx := s.db.Begin()
+	defer func() {
+		tx.Rollback()
+	}()
 
 	group, err = s.updateUsersInternal(ctx, id, userIds, tx)
 	if err != nil {
-		tx.Rollback()
 		return model.UserGroup{}, err
 	}
 
 	err = tx.Commit().Error
 	if err != nil {
-		tx.Rollback()
 		return model.UserGroup{}, err
 	}
 

backend/internal/service/user_service.go

@@ -205,7 +205,6 @@ func (s *UserService) deleteUserInternal(ctx context.Context, userID string, all
 func (s *UserService) CreateUser(ctx context.Context, input dto.UserCreateDto) (model.User, error) {
 	tx := s.db.Begin()
 	defer func() {
-		// This is a no-op if the transaction has been committed already
 		tx.Rollback()
 	}()
 
@@ -251,7 +250,6 @@ func (s *UserService) createUserInternal(ctx context.Context, input dto.UserCrea
 func (s *UserService) UpdateUser(ctx context.Context, userID string, updatedUser dto.UserCreateDto, updateOwnUser bool, allowLdapUpdate bool) (model.User, error) {
 	tx := s.db.Begin()
 	defer func() {
-		// This is a no-op if the transaction has been committed already
 		tx.Rollback()
 	}()
 
@@ -313,7 +311,6 @@ func (s *UserService) updateUserInternal(ctx context.Context, userID string, upd
 func (s *UserService) RequestOneTimeAccessEmail(ctx context.Context, emailAddress, redirectPath string) error {
 	tx := s.db.Begin()
 	defer func() {
-		// This is a no-op if the transaction has been committed already
 		tx.Rollback()
 	}()
 
@@ -409,7 +406,6 @@ func (s *UserService) createOneTimeAccessTokenInternal(ctx context.Context, user
 func (s *UserService) ExchangeOneTimeAccessToken(ctx context.Context, token string, ipAddress, userAgent string) (model.User, string, error) {
 	tx := s.db.Begin()
 	defer func() {
-		// This is a no-op if the transaction has been committed already
 		tx.Rollback()
 	}()
 
@@ -453,7 +449,6 @@ func (s *UserService) ExchangeOneTimeAccessToken(ctx context.Context, token stri
 func (s *UserService) UpdateUserGroups(ctx context.Context, id string, userGroupIds []string) (user model.User, err error) {
 	tx := s.db.Begin()
 	defer func() {
-		// This is a no-op if the transaction has been committed already
 		tx.Rollback()
 	}()
 
@@ -502,7 +497,6 @@ func (s *UserService) UpdateUserGroups(ctx context.Context, id string, userGroup
 func (s *UserService) SetupInitialAdmin(ctx context.Context) (model.User, string, error) {
 	tx := s.db.Begin()
 	defer func() {
-		// This is a no-op if the transaction has been committed already
 		tx.Rollback()
 	}()
 

backend/internal/service/webauthn_service.go

@@ -49,7 +49,6 @@ func NewWebAuthnService(db *gorm.DB, jwtService *JwtService, auditLogService *Au
 func (s *WebAuthnService) BeginRegistration(ctx context.Context, userID string) (*model.PublicKeyCredentialCreationOptions, error) {
 	tx := s.db.Begin()
 	defer func() {
-		// This is a no-op if the transaction has been committed already
 		tx.Rollback()
 	}()
 
@@ -104,7 +103,6 @@ func (s *WebAuthnService) BeginRegistration(ctx context.Context, userID string)
 func (s *WebAuthnService) VerifyRegistration(ctx context.Context, sessionID, userID string, r *http.Request) (model.WebauthnCredential, error) {
 	tx := s.db.Begin()
 	defer func() {
-		// This is a no-op if the transaction has been committed already
 		tx.Rollback()
 	}()
 
@@ -206,7 +204,6 @@ func (s *WebAuthnService) BeginLogin(ctx context.Context) (*model.PublicKeyCrede
 func (s *WebAuthnService) VerifyLogin(ctx context.Context, sessionID string, credentialAssertionData *protocol.ParsedCredentialAssertionData, ipAddress, userAgent string) (model.User, string, error) {
 	tx := s.db.Begin()
 	defer func() {
-		// This is a no-op if the transaction has been committed already
 		tx.Rollback()
 	}()
 
@@ -284,7 +281,6 @@ func (s *WebAuthnService) DeleteCredential(ctx context.Context, userID, credenti
 func (s *WebAuthnService) UpdateCredential(ctx context.Context, userID, credentialID, name string) (model.WebauthnCredential, error) {
 	tx := s.db.Begin()
 	defer func() {
-		// This is a no-op if the transaction has been committed already
 		tx.Rollback()
 	}()