diff --git a/pkg/models/error.go b/pkg/models/error.go index e4757d198..1a6c9d7ab 100644 --- a/pkg/models/error.go +++ b/pkg/models/error.go @@ -1241,7 +1241,7 @@ func (err ErrTeamsDoNotExist) HTTPError() web.HTTPError { // ErrOIDCTeamDoesNotExist represents an error where a team with specified name and specified oidcId property does not exist type ErrOIDCTeamDoesNotExist struct { - OidcId string + OidcID string Name string } @@ -1252,7 +1252,7 @@ func IsErrOIDCTeamDoesNotExist(err error) bool { } func (err ErrOIDCTeamDoesNotExist) Error() string { - return fmt.Sprintf("No Team with that name and valid property oidcId could be found [Team Name: %v] [OidcId : %v] ", err.Name, err.OidcId) + return fmt.Sprintf("No Team with that name and valid property oidcId could be found [Team Name: %v] [OidcId : %v] ", err.Name, err.OidcID) } // ErrCodeTeamDoesNotExist holds the unique world-error code of this error diff --git a/pkg/modules/auth/openid/openid.go b/pkg/modules/auth/openid/openid.go index 00d02e9cb..bd0a6dbb3 100644 --- a/pkg/modules/auth/openid/openid.go +++ b/pkg/modules/auth/openid/openid.go @@ -199,38 +199,18 @@ func HandleCallback(c echo.Context) error { // does the oidc token contain well formed "vikunja_groups" through vikunja_scope teamData, errs := getTeamDataFromToken(cl.VikunjaGroups, provider) - if len(errs) > 0 { - for _, err := range errs { - log.Errorf("Error creating teams for user and vikunja groups %s: %v", cl.VikunjaGroups, err) - } + for _, err := range errs { + log.Errorf("Error creating teams for user and vikunja groups %s: %v", cl.VikunjaGroups, err) } //find old teams for user through oidc oldOidcTeams, err := models.FindAllOidcTeamIDsForUser(s, u.ID) if err != nil { - log.Errorf("No Oidc Teams found for user", err) + log.Errorf("No Oidc Teams found for user %v", err) } - var oidcTeams []int64 - if len(teamData) > 0 { - // check if we have seen these teams before. - // find or create Teams and assign user as teammember. - log.Debugf("TeamData is set %v", teamData) - teams, err := GetOrCreateTeamsByOIDCAndNames(s, teamData, u) - if err != nil { - log.Errorf("Error verifying team for name %v, got %v", cl.Name, teams, err) - return err - } - for _, team := range teams { - tm := models.TeamMember{TeamID: team.ID, UserID: u.ID, Username: u.Username} - exists, err := tm.CheckMembership(s) - if !exists { - err = tm.Create(s, u) - if err != nil { - log.Errorf("Could not assign %v to %v. %v", u.Username, team.Name, err) - } - } - oidcTeams = append(oidcTeams, team.ID) - } + oidcTeams, err := AssignOrCreateUserToTeams(s, u, teamData) + if err != nil { + log.Errorf("Could not proceed with group routine %v", err) } errs = SignOutFromOrDeleteTeamsByID(s, u, utils.NotIn(oldOidcTeams, oidcTeams)) for _, err := range errs { @@ -246,16 +226,40 @@ func HandleCallback(c echo.Context) error { return auth.NewUserAuthTokenResponse(u, c, false) } +func AssignOrCreateUserToTeams(s *xorm.Session, u *user.User, teamData []models.TeamData) (oidcTeams []int64, err error) { + if len(teamData) > 0 { + // check if we have seen these teams before. + // find or create Teams and assign user as teammember. + teams, err := GetOrCreateTeamsByOIDCAndNames(s, teamData, u) + if err != nil { + log.Errorf("Error verifying team for %v, got %v. Error: %v", u.Name, teams, err) + return nil, err + } + for _, team := range teams { + tm := models.TeamMember{TeamID: team.ID, UserID: u.ID, Username: u.Username} + exists, _ := tm.CheckMembership(s) + if !exists { + err = tm.Create(s, u) + if err != nil { + log.Errorf("Could not assign %v to %v. %v", u.Username, team.Name, err) + } + } + oidcTeams = append(oidcTeams, team.ID) + } + } + return oidcTeams, err + +} func SignOutFromOrDeleteTeamsByID(s *xorm.Session, u *user.User, teamIDs []int64) (errs []error) { errs = []error{} for _, teamID := range teamIDs { tm := models.TeamMember{TeamID: teamID, UserID: u.ID, Username: u.Username} - exists, err := tm.CheckMembership(s) + exists, _ := tm.CheckMembership(s) memberCount, _ := tm.GetMemberCount(s) if !exists { continue } - err = tm.Delete(s, u) + err := tm.Delete(s, u) // if you cannot delete the team_member if err != nil || memberCount <= 1 { team, err := models.GetTeamByID(s, teamID)