From 4f36184c984ccbfe77e86b0ece241622f257dccc Mon Sep 17 00:00:00 2001 From: Daniel Herrmann Date: Mon, 4 Mar 2024 22:43:35 +0100 Subject: [PATCH 1/3] fix: OIDC teams should not have admins --- pkg/models/teams.go | 53 ++++++++++++++++++++----------- pkg/modules/auth/openid/openid.go | 2 +- 2 files changed, 35 insertions(+), 20 deletions(-) diff --git a/pkg/models/teams.go b/pkg/models/teams.go index 5b38d8aac..9c4c5034f 100644 --- a/pkg/models/teams.go +++ b/pkg/models/teams.go @@ -211,6 +211,38 @@ 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 def_adm 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, adm 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: adm} + if err = tm.Create(s, doer); err != nil { + return err + } + return +} + // ReadOne implements the CRUD method to get one team // @Summary Gets one team // @Description Returns a team by its ID. @@ -291,29 +323,12 @@ 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, 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 } -- 2.45.1 From 6781e5e6cd5b0ad7d43c55ba775f7d8ac87b370c Mon Sep 17 00:00:00 2001 From: Daniel Herrmann Date: Tue, 5 Mar 2024 15:00:01 +0100 Subject: [PATCH 2/3] fix: naming change for parameter --- pkg/models/teams.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/models/teams.go b/pkg/models/teams.go index 9c4c5034f..e6037fb11 100644 --- a/pkg/models/teams.go +++ b/pkg/models/teams.go @@ -216,7 +216,7 @@ func addMoreInfoToTeams(s *xorm.Session, teams []*Team) (err error) { // If def_adm 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, adm bool) (err error) { +func (t *Team) CreateNewTeam(s *xorm.Session, a web.Auth, firstUserShouldBeAdmin bool) (err error) { doer, err := user.GetFromAuth(a) if err != nil { @@ -236,7 +236,7 @@ func (t *Team) CreateNewTeam(s *xorm.Session, a web.Auth, adm bool) (err error) return } - tm := TeamMember{TeamID: t.ID, Username: doer.Username, Admin: adm} + tm := TeamMember{TeamID: t.ID, Username: doer.Username, Admin: firstUserShouldBeAdmin} if err = tm.Create(s, doer); err != nil { return err } -- 2.45.1 From 07dcfd82496173b4070db62ccc52bf7bb319667e Mon Sep 17 00:00:00 2001 From: Daniel Herrmann Date: Tue, 5 Mar 2024 22:13:07 +0100 Subject: [PATCH 3/3] fix: move event dispatch to new function --- pkg/models/teams.go | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/pkg/models/teams.go b/pkg/models/teams.go index e6037fb11..648cbe919 100644 --- a/pkg/models/teams.go +++ b/pkg/models/teams.go @@ -213,7 +213,7 @@ func addMoreInfoToTeams(s *xorm.Session, teams []*Team) (err error) { // CreateNewTeam creates a new team and assignes the user that has caused creation // as the first member of the team -// If def_adm is true, the user will be an admin 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) { @@ -240,7 +240,11 @@ func (t *Team) CreateNewTeam(s *xorm.Session, a web.Auth, firstUserShouldBeAdmin if err = tm.Create(s, doer); err != nil { return err } - return + + return events.Dispatch(&TeamCreatedEvent{ + Team: t, + Doer: a, + }) } // ReadOne implements the CRUD method to get one team @@ -329,10 +333,7 @@ func (t *Team) Create(s *xorm.Session, a web.Auth) (err error) { return err } - return events.Dispatch(&TeamCreatedEvent{ - Team: t, - Doer: a, - }) + return } // Delete deletes a team -- 2.45.1