diff --git a/pkg/modules/auth/openid/openid.go b/pkg/modules/auth/openid/openid.go index ada0ac1edb..383bab6a99 100644 --- a/pkg/modules/auth/openid/openid.go +++ b/pkg/modules/auth/openid/openid.go @@ -202,7 +202,7 @@ func HandleCallback(c echo.Context) error { // does the oidc token contain well formed "vikunja_groups" through vikunja_scope log.Debugf("Checking for vikunja_groups in token %v", cl.VikunjaGroups) teamData, errs := getTeamDataFromToken(cl.VikunjaGroups, provider) - if teamData != nil && len(teamData) > 0 { + if len(teamData) > 0 { for _, err := range errs { log.Errorf("Error creating teams for user and vikunja groups %s: %v", cl.VikunjaGroups, err) } @@ -221,7 +221,12 @@ func HandleCallback(c echo.Context) error { if err != nil { log.Errorf("Found error while leaving teams %v", err) } - err = RemoveEmptySSOTeams(s, u, teamIDsToLeave) + errors := RemoveEmptySSOTeams(s, teamIDsToLeave) + if len(errors) > 0 { + for _, err := range errors { + log.Errorf("Found error while removing empty teams %v", err) + } + } } err = s.Commit() if err != nil { @@ -258,15 +263,18 @@ func AssignOrCreateUserToTeams(s *xorm.Session, u *user.User, teamData []models. return oidcTeams, err } -func RemoveEmptySSOTeams(s *xorm.Session, u *user.User, teamIDs []int64) (err error) { +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{}) + _, _err := s.Where("id = ?", teamID).Delete(&models.Team{}) + if _err != nil { + errs = append(errs, _err) + } } } - return err + return errs } func RemoveUserFromTeamsByIds(s *xorm.Session, u *user.User, teamIDs []int64) (err error) { diff --git a/pkg/modules/auth/openid/openid_test.go b/pkg/modules/auth/openid/openid_test.go index 873db6451e..f6c13b21af 100644 --- a/pkg/modules/auth/openid/openid_test.go +++ b/pkg/modules/auth/openid/openid_test.go @@ -151,16 +151,8 @@ func TestGetOrCreateUser(t *testing.T) { for _, err := range errs { assert.NoError(t, err) } - // oldOidcTeams, err := models.FindAllOidcTeamIDsForUser(s, u.ID) - //assert.NoError(t, err) oidcTeams, err := AssignOrCreateUserToTeams(s, u, teamData) assert.NoError(t, err) - // teamIDsToLeave := utils.NotIn(oldOidcTeams, oidcTeams) - // assert.NoError(t, err) - // err = RemoveUserFromTeamsByIds(s, u, teamIDsToLeave) - // assert.NoError(t, err) - // err = RemoveEmptySSOTeams(s, u, teamIDsToLeave) - // assert.NoError(t, err) err = s.Commit() assert.NoError(t, err) @@ -194,8 +186,14 @@ func TestGetOrCreateUser(t *testing.T) { assert.NoError(t, err) err = RemoveUserFromTeamsByIds(s, u, teamIDsToLeave) assert.NoError(t, err) - // err = RemoveEmptySSOTeams(s, u, teamIDsToLeave) - // assert.NoError(t, err) + errs = RemoveEmptySSOTeams(s, teamIDsToLeave) + for _, err = range errs { + assert.NoError(t, err) + } + errs = RemoveEmptySSOTeams(s, teamIDsToLeave) + for _, err = range errs { + assert.NoError(t, err) + } err = s.Commit() assert.NoError(t, err) @@ -229,11 +227,12 @@ func TestGetOrCreateUser(t *testing.T) { assert.NoError(t, err) err = RemoveUserFromTeamsByIds(s, u, teamIDsToLeave) assert.NoError(t, err) - err = RemoveEmptySSOTeams(s, u, teamIDsToLeave) - assert.NoError(t, err) + errs = RemoveEmptySSOTeams(s, teamIDsToLeave) + for _, err := range errs { + assert.NoError(t, err) + } err = s.Commit() assert.NoError(t, err) - db.AssertMissing(t, "teams", map[string]interface{}{ "id": oidcTeams, })