chore(auth): refactor removing empty openid teams to cron job

pkg/initialize/init.go

@@ -95,6 +95,7 @@ func FullInit() {
 	models.RegisterUserDeletionCron()
 	models.RegisterOldExportCleanupCron()
 	openid.CleanupSavedOpenIDProviders()
+	openid.RegisterEmptyOpenIDTeamCleanupCron()
 
 	// Start processing events
 	go func() {

pkg/modules/auth/openid/cron.go

@@ -0,0 +1,68 @@
+// Vikunja is a to-do list application to facilitate your life.
+// Copyright 2018-present Vikunja and contributors. All rights reserved.
+//
+// This program is free software: you can redistribute it and/or modify
+// it under the terms of the GNU Affero General Public Licensee as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// This program is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU Affero General Public Licensee for more details.
+//
+// You should have received a copy of the GNU Affero General Public Licensee
+// along with this program.  If not, see <https://www.gnu.org/licenses/>.
+
+package openid
+
+import (
+	"code.vikunja.io/api/pkg/log"
+	"code.vikunja.io/api/pkg/models"
+	"xorm.io/builder"
+	"xorm.io/xorm"
+
+	"code.vikunja.io/api/pkg/cron"
+	"code.vikunja.io/api/pkg/db"
+)
+
+func RemoveEmptySSOTeams(s *xorm.Session) (err error) {
+	teams := []*models.Team{}
+	err = s.
+		Where(
+			builder.NotIn("id", builder.Expr("select team_members.team_id from team_members")),
+			builder.Or(builder.Neq{"oidc_id": ""}, builder.NotNull{"oidc_id"}),
+		).
+		Find(&teams)
+	if err != nil {
+		return err
+	}
+
+	teamIDs := make([]int64, 0, len(teams))
+	for _, team := range teams {
+		teamIDs = append(teamIDs, team.ID)
+	}
+
+	log.Debugf("Deleting empty teams: %v", teamIDs)
+
+	_, err = s.In("id", teamIDs).Delete(&models.Team{})
+	return err
+}
+
+func RegisterEmptyOpenIDTeamCleanupCron() {
+	const logPrefix = "[Empty openid Team Cleanup Cron] "
+
+	err := cron.Schedule("* * * * *", func() {
+		s := db.NewSession()
+		defer s.Close()
+
+		err := RemoveEmptySSOTeams(s)
+		if err != nil {
+			log.Errorf(logPrefix+"Error removing empty openid team: %s", err)
+			return
+		}
+	})
+	if err != nil {
+		log.Fatalf("Could not empty openid teams cleanup cron: %s", err)
+	}
+}

pkg/modules/auth/openid/openid.go

@@ -222,13 +222,7 @@ func HandleCallback(c echo.Context) error {
 		teamIDsToLeave := utils.NotIn(oldOidcTeams, oidcTeams)
 		err = RemoveUserFromTeamsByIDs(s, u, teamIDsToLeave)
 		if err != nil {
-			log.Errorf("Found error while leaving teams %v", err)
+			log.Errorf("Error while leaving teams %v", err)
-		}
-		errs := RemoveEmptySSOTeams(s, teamIDsToLeave)
-		if len(errs) > 0 {
-			for _, err := range errs {
-				log.Errorf("Found error while removing empty teams %v", err)
-			}
 		}
 	}
 	err = s.Commit()
@@ -266,20 +260,6 @@ func AssignOrCreateUserToTeams(s *xorm.Session, u *user.User, teamData []*models
 	return oidcTeams, err
 }
 
-func RemoveEmptySSOTeams(s *xorm.Session, teamIDs []int64) (errs []error) {
-	for _, teamID := range teamIDs {
-		count, err := s.Where("team_id = ?", teamID).Count(&models.TeamMember{})
-		if count == 0 && err == nil {
-			log.Debugf("SSO team with id %v has no members. It will be deleted", teamID)
-			_, _err := s.Where("id = ?", teamID).Delete(&models.Team{})
-			if _err != nil {
-				errs = append(errs, _err)
-			}
-		}
-	}
-	return errs
-}
-
 func RemoveUserFromTeamsByIDs(s *xorm.Session, u *user.User, teamIDs []int64) (err error) {
 
 	if len(teamIDs) < 1 {

pkg/modules/auth/openid/openid_test.go

@@ -220,14 +220,6 @@ func TestGetOrCreateUser(t *testing.T) {
 		require.NoError(t, err)
 		err = RemoveUserFromTeamsByIDs(s, u, teamIDsToLeave)
 		require.NoError(t, err)
-		errs = RemoveEmptySSOTeams(s, teamIDsToLeave)
-		for _, err = range errs {
-			require.NoError(t, err)
-		}
-		errs = RemoveEmptySSOTeams(s, teamIDsToLeave)
-		for _, err = range errs {
-			require.NoError(t, err)
-		}
 		err = s.Commit()
 		require.NoError(t, err)
 
@@ -235,38 +227,6 @@ func TestGetOrCreateUser(t *testing.T) {
 			"team_id": oidcTeams,
 			"user_id": u.ID,
 		})
-	})
-	t.Run("existing user, remove from existing team and delete team", func(t *testing.T) {
-		db.LoadAndAssertFixtures(t)
-		s := db.NewSession()
-		defer s.Close()
-
-		cl := &claims{
-			Email:         "other-email-address@some.service.com",
-			VikunjaGroups: []map[string]interface{}{},
-		}
-
-		u := &user.User{ID: 10}
-		teamData, errs := getTeamDataFromToken(cl.VikunjaGroups, nil)
-		if len(errs) > 0 {
-			for _, err := range errs {
-				require.NoError(t, err)
-			}
-		}
-		oldOidcTeams, err := models.FindAllOidcTeamIDsForUser(s, u.ID)
-		require.NoError(t, err)
-		oidcTeams, err := AssignOrCreateUserToTeams(s, u, teamData, "https://some.issuer")
-		require.NoError(t, err)
-		teamIDsToLeave := utils.NotIn(oldOidcTeams, oidcTeams)
-		require.NoError(t, err)
-		err = RemoveUserFromTeamsByIDs(s, u, teamIDsToLeave)
-		require.NoError(t, err)
-		errs = RemoveEmptySSOTeams(s, teamIDsToLeave)
-		for _, err := range errs {
-			require.NoError(t, err)
-		}
-		err = s.Commit()
-		require.NoError(t, err)
 		db.AssertMissing(t, "teams", map[string]interface{}{
 			"id": oidcTeams,
 		})