diff --git a/pkg/db/fixtures/teams.yml b/pkg/db/fixtures/teams.yml index defdedd62..aaba624a3 100644 --- a/pkg/db/fixtures/teams.yml +++ b/pkg/db/fixtures/teams.yml @@ -32,4 +32,5 @@ - id: 14 name: testteam14 created_by_id: 7 - oidc_id: 14 \ No newline at end of file + oidc_id: 14 + issuer: "https://some.issuer" \ No newline at end of file diff --git a/pkg/migration/20240304153738.go b/pkg/migration/20240304153738.go new file mode 100644 index 000000000..4661897bb --- /dev/null +++ b/pkg/migration/20240304153738.go @@ -0,0 +1,43 @@ +// 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 . + +package migration + +import ( + "src.techknowlogick.com/xormigrate" + "xorm.io/xorm" +) + +type teams20240304153738 struct { + Issuer string `xorm:"text null" json:"-"` +} + +func (teams20240304153738) TableName() string { + return "teams" +} + +func init() { + migrations = append(migrations, &xormigrate.Migration{ + ID: "20240304153738", + Description: "Add issuer column to teams to track the OIDC provider this issue has bene created by", + Migrate: func(tx *xorm.Engine) error { + return tx.Sync2(teams20240304153738{}) + }, + Rollback: func(tx *xorm.Engine) error { + return nil + }, + }) +} diff --git a/pkg/models/error.go b/pkg/models/error.go index 2e31a6e58..a3b78c445 100644 --- a/pkg/models/error.go +++ b/pkg/models/error.go @@ -1177,10 +1177,10 @@ func (err ErrTeamDoesNotHaveAccessToProject) HTTPError() web.HTTPError { return web.HTTPError{HTTPCode: http.StatusForbidden, Code: ErrCodeTeamDoesNotHaveAccessToProject, Message: "This team does not have access to the project."} } -// ErrOIDCTeamDoesNotExist represents an error where a team with specified name and specified oidcId property does not exist +// ErrOIDCTeamDoesNotExist represents an error where a team with specified oidcId property does not exist for a given issuer type ErrOIDCTeamDoesNotExist struct { OidcID string - Name string + Issuer string } // IsErrOIDCTeamDoesNotExist checks if an error is ErrOIDCTeamDoesNotExist. @@ -1191,7 +1191,7 @@ func IsErrOIDCTeamDoesNotExist(err error) bool { // ErrTeamDoesNotExist represents an error where a team does not exist func (err ErrOIDCTeamDoesNotExist) Error() string { - return fmt.Sprintf("No team with that name and valid oidcId could be found. [Team Name: %v] [OidcID : %v] ", err.Name, err.OidcID) + return fmt.Sprintf("No team could be found for the given oidcId and issuer. [OIDC ID : %v] [Issuer: %v] ", err.OidcID, err.Issuer) } // ErrCodeTeamDoesNotExist holds the unique world-error code of this error @@ -1199,7 +1199,7 @@ const ErrCodeOIDCTeamDoesNotExist = 6008 // HTTPError holds the http error description func (err ErrOIDCTeamDoesNotExist) HTTPError() web.HTTPError { - return web.HTTPError{HTTPCode: http.StatusNotFound, Code: ErrCodeTeamDoesNotExist, Message: "No team with that name and valid oidcId could be found."} + return web.HTTPError{HTTPCode: http.StatusNotFound, Code: ErrCodeTeamDoesNotExist, Message: "No team could be found for the given OIDC ID and issuer."} } // ErrOIDCTeamsDoNotExistForUser represents an error where an oidcTeam does not exist for the user diff --git a/pkg/models/teams.go b/pkg/models/teams.go index c6442f7b9..5b38d8aac 100644 --- a/pkg/models/teams.go +++ b/pkg/models/teams.go @@ -40,6 +40,8 @@ type Team struct { CreatedByID int64 `xorm:"bigint not null INDEX" json:"-"` // The team's oidc id delivered by the oidc provider OidcID string `xorm:"varchar(250) null" maxLength:"250" json:"oidc_id"` + // Contains the issuer extracted from the vikunja_groups claim if this team was created through oidc + Issuer string `xorm:"text null" json:"-"` // The user who created this team. CreatedBy *user.User `xorm:"-" json:"created_by"` @@ -129,16 +131,16 @@ func GetTeamByID(s *xorm.Session, id int64) (team *Team, err error) { return } -// GetTeamByOidcIDAndName gets teams where oidc_id and name match parameters +// GetTeamByOidcID returns a team matching the given oidc_id // For oidc team creation oidcID and Name need to be set -func GetTeamByOidcIDAndName(s *xorm.Session, oidcID string, teamName string) (*Team, error) { +func GetTeamByOidcIDAndIssuer(s *xorm.Session, oidcID string, issuer string) (*Team, error) { team := &Team{} has, err := s. Table("teams"). - Where("oidc_id = ? AND name = ?", oidcID, teamName). + Where("oidc_id = ? AND issuer = ?", oidcID, issuer). Get(team) if !has || err != nil { - return nil, ErrOIDCTeamDoesNotExist{teamName, oidcID} + return nil, ErrOIDCTeamDoesNotExist{issuer, oidcID} } return team, nil } diff --git a/pkg/modules/auth/openid/openid.go b/pkg/modules/auth/openid/openid.go index 7ed4b8b2d..f7411887c 100644 --- a/pkg/modules/auth/openid/openid.go +++ b/pkg/modules/auth/openid/openid.go @@ -215,7 +215,7 @@ func HandleCallback(c echo.Context) error { if err != nil { log.Debugf("No oidc teams found for user %v", err) } - oidcTeams, err := AssignOrCreateUserToTeams(s, u, teamData) + oidcTeams, err := AssignOrCreateUserToTeams(s, u, teamData, idToken.Issuer) if err != nil { log.Errorf("Could not proceed with group routine %v", err) } @@ -241,13 +241,13 @@ func HandleCallback(c echo.Context) error { return auth.NewUserAuthTokenResponse(u, c, false) } -func AssignOrCreateUserToTeams(s *xorm.Session, u *user.User, teamData []*models.OIDCTeam) (oidcTeams []int64, err error) { +func AssignOrCreateUserToTeams(s *xorm.Session, u *user.User, teamData []*models.OIDCTeam, issuer string) (oidcTeams []int64, err error) { if len(teamData) == 0 { return } // check if we have seen these teams before. // find or create Teams and assign user as teammember. - teams, err := GetOrCreateTeamsByOIDCAndNames(s, teamData, u) + teams, err := GetOrCreateTeamsByOIDC(s, teamData, u, issuer) if err != nil { log.Errorf("Error verifying team for %v, got %v. Error: %v", u.Name, teams, err) return nil, err @@ -333,28 +333,29 @@ func getOIDCTeamName(name string) string { return name + " (OIDC)" } -func CreateOIDCTeam(s *xorm.Session, teamData *models.OIDCTeam, u *user.User) (team *models.Team, err error) { +func CreateOIDCTeam(s *xorm.Session, teamData *models.OIDCTeam, u *user.User, issuer string) (team *models.Team, err error) { team = &models.Team{ Name: getOIDCTeamName(teamData.Name), Description: teamData.Description, OidcID: teamData.OidcID, + Issuer: issuer, } err = team.Create(s, u) return team, err } -// GetOrCreateTeamsByOIDCAndNames returns a slice of teams which were generated from the oidc data. If a team did not exist previously it is automatically created. -func GetOrCreateTeamsByOIDCAndNames(s *xorm.Session, teamData []*models.OIDCTeam, u *user.User) (te []*models.Team, err error) { +// GetOrCreateTeamsByOIDC returns a slice of teams which were generated from the oidc data. If a team did not exist previously it is automatically created. +func GetOrCreateTeamsByOIDC(s *xorm.Session, teamData []*models.OIDCTeam, u *user.User, issuer string) (te []*models.Team, err error) { te = []*models.Team{} // Procedure can only be successful if oidcID is set for _, oidcTeam := range teamData { - team, err := models.GetTeamByOidcIDAndName(s, oidcTeam.OidcID, oidcTeam.Name) + team, err := models.GetTeamByOidcIDAndIssuer(s, oidcTeam.OidcID, issuer) if err != nil && !models.IsErrOIDCTeamDoesNotExist(err) { return nil, err } if err != nil && models.IsErrOIDCTeamDoesNotExist(err) { log.Debugf("Team with oidc_id %v and name %v does not exist. Creating team… ", oidcTeam.OidcID, oidcTeam.Name) - newTeam, err := CreateOIDCTeam(s, oidcTeam, u) + newTeam, err := CreateOIDCTeam(s, oidcTeam, u, issuer) if err != nil { return te, err } diff --git a/pkg/modules/auth/openid/openid_test.go b/pkg/modules/auth/openid/openid_test.go index a0393dc7e..6cafbd222 100644 --- a/pkg/modules/auth/openid/openid_test.go +++ b/pkg/modules/auth/openid/openid_test.go @@ -118,7 +118,7 @@ func TestGetOrCreateUser(t *testing.T) { require.NoError(t, err) } require.NoError(t, err) - oidcTeams, err := AssignOrCreateUserToTeams(s, u, teamData) + oidcTeams, err := AssignOrCreateUserToTeams(s, u, teamData, "https://some.issuer") require.NoError(t, err) err = s.Commit() require.NoError(t, err) @@ -152,7 +152,7 @@ func TestGetOrCreateUser(t *testing.T) { for _, err := range errs { require.NoError(t, err) } - oidcTeams, err := AssignOrCreateUserToTeams(s, u, teamData) + oidcTeams, err := AssignOrCreateUserToTeams(s, u, teamData, "https://some.issuer") require.NoError(t, err) err = s.Commit() require.NoError(t, err) @@ -181,7 +181,7 @@ func TestGetOrCreateUser(t *testing.T) { } oldOidcTeams, err := models.FindAllOidcTeamIDsForUser(s, u.ID) require.NoError(t, err) - oidcTeams, err := AssignOrCreateUserToTeams(s, u, teamData) + oidcTeams, err := AssignOrCreateUserToTeams(s, u, teamData, "https://some.issuer") require.NoError(t, err) teamIDsToLeave := utils.NotIn(oldOidcTeams, oidcTeams) require.NoError(t, err) @@ -222,7 +222,7 @@ func TestGetOrCreateUser(t *testing.T) { } oldOidcTeams, err := models.FindAllOidcTeamIDsForUser(s, u.ID) require.NoError(t, err) - oidcTeams, err := AssignOrCreateUserToTeams(s, u, teamData) + oidcTeams, err := AssignOrCreateUserToTeams(s, u, teamData, "https://some.issuer") require.NoError(t, err) teamIDsToLeave := utils.NotIn(oldOidcTeams, oidcTeams) require.NoError(t, err)