From d69fc28125585293569ecaad070ad12ee7f8600c Mon Sep 17 00:00:00 2001
From: waza-ari <daniel.herrmann@makerspace-darmstadt.de>
Date: Tue, 5 Mar 2024 22:08:39 +0000
Subject: [PATCH] fix(openid): OIDC teams should not have admins (#2161)

This PR fixes an issue discussed in #2152. Before this PR, the user who triggered team creation automatically got the admin flag set for this group, which makes perfect sense for the normal UI workflow. OIDC managed teams cannot be edited in Vikunja, and they're created automatically by the first user logging in having this team assigned. This PR therefore makes sure that OIDC managed team members do not receive the admin flag.

Co-authored-by: Daniel Herrmann <daniel.herrmann1@gmail.com>
Reviewed-on: https://kolaente.dev/vikunja/vikunja/pulls/2161
Reviewed-by: konrad <k@knt.li>
Co-authored-by: waza-ari <daniel.herrmann@makerspace-darmstadt.de>
Co-committed-by: waza-ari <daniel.herrmann@makerspace-darmstadt.de>
---
 pkg/models/teams.go               | 62 +++++++++++++++++++------------
 pkg/modules/auth/openid/openid.go |  2 +-
 2 files changed, 40 insertions(+), 24 deletions(-)

diff --git a/pkg/models/teams.go b/pkg/models/teams.go
index 5b38d8aac..648cbe919 100644
--- a/pkg/models/teams.go
+++ b/pkg/models/teams.go
@@ -211,6 +211,42 @@ func addMoreInfoToTeams(s *xorm.Session, teams []*Team) (err error) {
 	return
 }
 
+// CreateNewTeam creates a new team and assignes the user that has caused creation
+// as the first member of the team
+// If firstUserShouldBeAdmin is true, the user will be an admin of the team
+// Note: this function has been extracted from the Create method to allow
+// an additional parameter to control whether the user should become admin of the team
+func (t *Team) CreateNewTeam(s *xorm.Session, a web.Auth, firstUserShouldBeAdmin bool) (err error) {
+
+	doer, err := user.GetFromAuth(a)
+	if err != nil {
+		return err
+	}
+
+	// Check if we have a name
+	if t.Name == "" {
+		return ErrTeamNameCannotBeEmpty{}
+	}
+
+	t.CreatedByID = doer.ID
+	t.CreatedBy = doer
+
+	_, err = s.Insert(t)
+	if err != nil {
+		return
+	}
+
+	tm := TeamMember{TeamID: t.ID, Username: doer.Username, Admin: firstUserShouldBeAdmin}
+	if err = tm.Create(s, doer); err != nil {
+		return err
+	}
+
+	return events.Dispatch(&TeamCreatedEvent{
+		Team: t,
+		Doer: a,
+	})
+}
+
 // ReadOne implements the CRUD method to get one team
 // @Summary Gets one team
 // @Description Returns a team by its ID.
@@ -291,33 +327,13 @@ func (t *Team) ReadAll(s *xorm.Session, a web.Auth, search string, page int, per
 // @Failure 500 {object} models.Message "Internal error"
 // @Router /teams [put]
 func (t *Team) Create(s *xorm.Session, a web.Auth) (err error) {
-	doer, err := user.GetFromAuth(a)
+
+	err = t.CreateNewTeam(s, a, true)
 	if err != nil {
 		return err
 	}
 
-	// Check if we have a name
-	if t.Name == "" {
-		return ErrTeamNameCannotBeEmpty{}
-	}
-
-	t.CreatedByID = doer.ID
-	t.CreatedBy = doer
-
-	_, err = s.Insert(t)
-	if err != nil {
-		return
-	}
-
-	tm := TeamMember{TeamID: t.ID, Username: doer.Username, Admin: true}
-	if err = tm.Create(s, doer); err != nil {
-		return err
-	}
-
-	return events.Dispatch(&TeamCreatedEvent{
-		Team: t,
-		Doer: a,
-	})
+	return
 }
 
 // Delete deletes a team
diff --git a/pkg/modules/auth/openid/openid.go b/pkg/modules/auth/openid/openid.go
index f7411887c..c66aaef8b 100644
--- a/pkg/modules/auth/openid/openid.go
+++ b/pkg/modules/auth/openid/openid.go
@@ -340,7 +340,7 @@ func CreateOIDCTeam(s *xorm.Session, teamData *models.OIDCTeam, u *user.User, is
 		OidcID:      teamData.OidcID,
 		Issuer:      issuer,
 	}
-	err = team.Create(s, u)
+	err = team.CreateNewTeam(s, u, false)
 	return team, err
 }