From 1b935868b6733452f20f3fca731274a5002409d9 Mon Sep 17 00:00:00 2001 From: viehlieb Date: Wed, 12 Oct 2022 12:17:04 +0200 Subject: [PATCH 1/3] add groups to claims and assign user as no admin --- pkg/models/error.go | 106 +++++++----------------------- pkg/models/teams.go | 61 ++++++++++++++++- pkg/modules/auth/openid/openid.go | 73 ++++++++++++++++++-- 3 files changed, 150 insertions(+), 90 deletions(-) diff --git a/pkg/models/error.go b/pkg/models/error.go index 580ba055..65183aa6 100644 --- a/pkg/models/error.go +++ b/pkg/models/error.go @@ -283,34 +283,6 @@ func (err *ErrListCannotBelongToAPseudoNamespace) HTTPError() web.HTTPError { } } -// ErrListMustBelongToANamespace represents an error where a list must belong to a namespace -type ErrListMustBelongToANamespace struct { - ListID int64 - NamespaceID int64 -} - -// IsErrListMustBelongToANamespace checks if an error is a list must belong to a namespace error. -func IsErrListMustBelongToANamespace(err error) bool { - _, ok := err.(*ErrListMustBelongToANamespace) - return ok -} - -func (err *ErrListMustBelongToANamespace) Error() string { - return fmt.Sprintf("List must belong to a namespace [ListID: %d, NamespaceID: %d]", err.ListID, err.NamespaceID) -} - -// ErrCodeListMustBelongToANamespace holds the unique world-error code of this error -const ErrCodeListMustBelongToANamespace = 3010 - -// HTTPError holds the http error description -func (err *ErrListMustBelongToANamespace) HTTPError() web.HTTPError { - return web.HTTPError{ - HTTPCode: http.StatusPreconditionFailed, - Code: ErrCodeListMustBelongToANamespace, - Message: "This list must belong to a namespace.", - } -} - // ================ // List task errors // ================ @@ -819,62 +791,6 @@ func (err ErrInvalidTaskFilterValue) HTTPError() web.HTTPError { } } -// ErrAttachmentDoesNotBelongToTask represents an error where the provided task cover attachment does not belong to the same task -type ErrAttachmentDoesNotBelongToTask struct { - TaskID int64 - AttachmentID int64 -} - -// IsErrAttachmentAndCoverMustBelongToTheSameTask checks if an error is ErrAttachmentDoesNotBelongToTask. -func IsErrAttachmentAndCoverMustBelongToTheSameTask(err error) bool { - _, ok := err.(ErrAttachmentDoesNotBelongToTask) - return ok -} - -func (err ErrAttachmentDoesNotBelongToTask) Error() string { - return fmt.Sprintf("Task attachment and cover image do not belong to the same task [TaskID: %d, AttachmentID: %d]", err.TaskID, err.AttachmentID) -} - -// ErrCodeAttachmentDoesNotBelongToTask holds the unique world-error code of this error -const ErrCodeAttachmentDoesNotBelongToTask = 4020 - -// HTTPError holds the http error description -func (err ErrAttachmentDoesNotBelongToTask) HTTPError() web.HTTPError { - return web.HTTPError{ - HTTPCode: http.StatusBadRequest, - Code: ErrCodeAttachmentDoesNotBelongToTask, - Message: "This attachment does not belong to that task.", - } -} - -// ErrUserAlreadyAssigned represents an error where the user is already assigned to this task -type ErrUserAlreadyAssigned struct { - TaskID int64 - UserID int64 -} - -// IsErrUserAlreadyAssigned checks if an error is ErrUserAlreadyAssigned. -func IsErrUserAlreadyAssigned(err error) bool { - _, ok := err.(ErrUserAlreadyAssigned) - return ok -} - -func (err ErrUserAlreadyAssigned) Error() string { - return fmt.Sprintf("User is already assigned to task [TaskID: %d, UserID: %d]", err.TaskID, err.UserID) -} - -// ErrCodeUserAlreadyAssigned holds the unique world-error code of this error -const ErrCodeUserAlreadyAssigned = 4021 - -// HTTPError holds the http error description -func (err ErrUserAlreadyAssigned) HTTPError() web.HTTPError { - return web.HTTPError{ - HTTPCode: http.StatusBadRequest, - Code: ErrCodeUserAlreadyAssigned, - Message: "This user is already assigned to that task.", - } -} - // ================= // Namespace errors // ================= @@ -1095,6 +1011,28 @@ func (err ErrTeamDoesNotExist) HTTPError() web.HTTPError { return web.HTTPError{HTTPCode: http.StatusNotFound, Code: ErrCodeTeamDoesNotExist, Message: "This team does not exist."} } +type ErrTeamsDoNotExist struct { + Name string +} + +// IsErrTeamDoNotExist checks if an error is ErrTeamDoesNotExist. +func IsErrTeamsDoNotExist(err error) bool { + _, ok := err.(ErrTeamsDoNotExist) + return ok +} + +func (err ErrTeamsDoNotExist) Error() string { + return fmt.Sprintf("Team does not exist [Team ID: %d]", err.Name) +} + +// ErrCodeTeamDoesNotExist holds the unique world-error code of this error +const ErrCodeTeamsDoNotExist = 6002 + +// HTTPError holds the http error description +func (err ErrTeamsDoNotExist) HTTPError() web.HTTPError { + return web.HTTPError{HTTPCode: http.StatusNotFound, Code: ErrCodeTeamDoesNotExist, Message: "No team with given name exists."} +} + // ErrTeamAlreadyHasAccess represents an error where a team already has access to a list/namespace type ErrTeamAlreadyHasAccess struct { TeamID int64 diff --git a/pkg/models/teams.go b/pkg/models/teams.go index 0b21a5bd..1b1d09c0 100644 --- a/pkg/models/teams.go +++ b/pkg/models/teams.go @@ -79,7 +79,7 @@ type TeamMember struct { } // TableName makes beautiful table names -func (*TeamMember) TableName() string { +func (TeamMember) TableName() string { return "team_members" } @@ -119,6 +119,34 @@ func GetTeamByID(s *xorm.Session, id int64) (team *Team, err error) { return } +func GetTeamsByName(s *xorm.Session, name string) (teams []*Team, err error) { + if name == "" { + return teams, ErrTeamsDoNotExist{name} + } + + var ts []*Team + + exists := s. + Where("name = ?", name). + Find(&ts) + if exists != nil { + return + } + if len(ts) == 0 { + return ts, ErrTeamsDoNotExist{name} + } + + // //for each ts + // teamSlice := []*Team{ts} + // err = addMoreInfoToTeams(s, teamSlice) + // if err != nil { + // return + // } + + teams = ts + + return +} func addMoreInfoToTeams(s *xorm.Session, teams []*Team) (err error) { @@ -282,6 +310,37 @@ func (t *Team) Create(s *xorm.Session, a web.Auth) (err error) { }) } +func (t *Team) CreateNoAdmin(s *xorm.Session, a web.Auth) (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 + } + + // Insert the current user as member and admin + tm := TeamMember{TeamID: t.ID, Username: doer.Username, Admin: false} + if err = tm.Create(s, doer); err != nil { + return err + } + + return events.Dispatch(&TeamCreatedEvent{ + Team: t, + Doer: a, + }) +} + // Delete deletes a team // @Summary Deletes a team // @Description Delets a team. This will also remove the access for all users in that team. diff --git a/pkg/modules/auth/openid/openid.go b/pkg/modules/auth/openid/openid.go index c282bea8..1c54c354 100644 --- a/pkg/modules/auth/openid/openid.go +++ b/pkg/modules/auth/openid/openid.go @@ -52,16 +52,18 @@ type Provider struct { OriginalAuthURL string `json:"-"` AuthURL string `json:"auth_url"` ClientID string `json:"client_id"` + Scope string `json:"scope"` ClientSecret string `json:"-"` openIDProvider *oidc.Provider Oauth2Config *oauth2.Config `json:"-"` } type claims struct { - Email string `json:"email"` - Name string `json:"name"` - PreferredUsername string `json:"preferred_username"` - Nickname string `json:"nickname"` + Email string `json:"email"` + Name string `json:"name"` + PreferredUsername string `json:"preferred_username"` + Nickname string `json:"nickname"` + Group []string `json:"groups"` } func init() { @@ -188,22 +190,83 @@ func HandleCallback(c echo.Context) error { // Check if we have seen this user before u, err := getOrCreateUser(s, cl, idToken.Issuer, idToken.Subject) + + log.Errorf("Issuer %s: %v", idToken.Issuer, err) + if err != nil { _ = s.Rollback() log.Errorf("Error creating new user for provider %s: %v", provider.Name, err) return handler.HandleHTTPError(err, c) } + // Check if we have seen this user before + teams := GetOrCreateTeamsByNames(s, cl.Group, u) + if err != nil { + log.Errorf("Error verifying team for name %v, got %v", cl.Name, teams, err) + return err + } else { + for _, team := range teams { + tm := models.TeamMember{TeamID: team.ID, Username: u.Username} + if err = tm.Create(s, u); err != nil { + switch t := err.(type) { + case *models.ErrUserIsMemberOfTeam: + log.Errorf("ErrUserIsMemberOfTeam", t) + break + default: + log.Errorf("Error assigning User to team", t) + } + } + } + } + err = s.Commit() if err != nil { return handler.HandleHTTPError(err, c) } - // Create token return auth.NewUserAuthTokenResponse(u, c, false) } +func GetOrCreateTeamsByNames(s *xorm.Session, teamNames []string, u *user.User) (te []models.Team) { + // Check if a team with given name exists should be after user creation + + //TODO: 1. Create team if not exist + te = []models.Team{} + for _, t := range teamNames { + team, err := models.GetTeamsByName(s, t) + + if models.IsErrTeamsDoNotExist(err) { + log.Errorf("No such Team: %v, got %v", t, team, err) + tea := &models.Team{ + Name: t, + } + // TODO: here the user who creates the Team is automatically admin. That shoud not be the case..? + err := tea.CreateNoAdmin(s, u) + if err != nil { + log.Errorf("Teams: %v, err: %v", tea, err) + } else { + te = append(te, *tea) + } + } else { + // if multiple teams with same name are found, + if len(team) == 1 { + te = append(te, *team[len(team)-1]) + } else { + log.Errorf("Multiple Teams have the same name: %v, ", team[len(team)-1].Name) + } + } + } + return te +} + +// assign user to team +// remove user from team if not in group +// if multiple teams found with same name -> do nothing +// optional: assign by id +// + func getOrCreateUser(s *xorm.Session, cl *claims, issuer, subject string) (u *user.User, err error) { + // Check if the user exists for that issuer and subject u, err = user.GetUserWithEmail(s, &user.User{ Issuer: issuer, From 93abfc41fc395e83fd8185850feb4ebcd79840e3 Mon Sep 17 00:00:00 2001 From: viehlieb Date: Wed, 12 Oct 2022 12:26:47 +0200 Subject: [PATCH 2/3] integrate errors --- pkg/models/error.go | 84 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 84 insertions(+) diff --git a/pkg/models/error.go b/pkg/models/error.go index 65183aa6..b9b0509e 100644 --- a/pkg/models/error.go +++ b/pkg/models/error.go @@ -283,6 +283,34 @@ func (err *ErrListCannotBelongToAPseudoNamespace) HTTPError() web.HTTPError { } } +// ErrListMustBelongToANamespace represents an error where a list must belong to a namespace +type ErrListMustBelongToANamespace struct { + ListID int64 + NamespaceID int64 +} + +// IsErrListMustBelongToANamespace checks if an error is a list must belong to a namespace error. +func IsErrListMustBelongToANamespace(err error) bool { + _, ok := err.(*ErrListMustBelongToANamespace) + return ok +} + +func (err *ErrListMustBelongToANamespace) Error() string { + return fmt.Sprintf("List must belong to a namespace [ListID: %d, NamespaceID: %d]", err.ListID, err.NamespaceID) +} + +// ErrCodeListMustBelongToANamespace holds the unique world-error code of this error +const ErrCodeListMustBelongToANamespace = 3010 + +// HTTPError holds the http error description +func (err *ErrListMustBelongToANamespace) HTTPError() web.HTTPError { + return web.HTTPError{ + HTTPCode: http.StatusPreconditionFailed, + Code: ErrCodeListMustBelongToANamespace, + Message: "This list must belong to a namespace.", + } +} + // ================ // List task errors // ================ @@ -791,6 +819,62 @@ func (err ErrInvalidTaskFilterValue) HTTPError() web.HTTPError { } } +// ErrAttachmentDoesNotBelongToTask represents an error where the provided task cover attachment does not belong to the same task +type ErrAttachmentDoesNotBelongToTask struct { + TaskID int64 + AttachmentID int64 +} + +// IsErrAttachmentAndCoverMustBelongToTheSameTask checks if an error is ErrAttachmentDoesNotBelongToTask. +func IsErrAttachmentAndCoverMustBelongToTheSameTask(err error) bool { + _, ok := err.(ErrAttachmentDoesNotBelongToTask) + return ok +} + +func (err ErrAttachmentDoesNotBelongToTask) Error() string { + return fmt.Sprintf("Task attachment and cover image do not belong to the same task [TaskID: %d, AttachmentID: %d]", err.TaskID, err.AttachmentID) +} + +// ErrCodeAttachmentDoesNotBelongToTask holds the unique world-error code of this error +const ErrCodeAttachmentDoesNotBelongToTask = 4020 + +// HTTPError holds the http error description +func (err ErrAttachmentDoesNotBelongToTask) HTTPError() web.HTTPError { + return web.HTTPError{ + HTTPCode: http.StatusBadRequest, + Code: ErrCodeAttachmentDoesNotBelongToTask, + Message: "This attachment does not belong to that task.", + } +} + +// ErrUserAlreadyAssigned represents an error where the user is already assigned to this task +type ErrUserAlreadyAssigned struct { + TaskID int64 + UserID int64 +} + +// IsErrUserAlreadyAssigned checks if an error is ErrUserAlreadyAssigned. +func IsErrUserAlreadyAssigned(err error) bool { + _, ok := err.(ErrUserAlreadyAssigned) + return ok +} + +func (err ErrUserAlreadyAssigned) Error() string { + return fmt.Sprintf("User is already assigned to task [TaskID: %d, UserID: %d]", err.TaskID, err.UserID) +} + +// ErrCodeUserAlreadyAssigned holds the unique world-error code of this error +const ErrCodeUserAlreadyAssigned = 4021 + +// HTTPError holds the http error description +func (err ErrUserAlreadyAssigned) HTTPError() web.HTTPError { + return web.HTTPError{ + HTTPCode: http.StatusBadRequest, + Code: ErrCodeUserAlreadyAssigned, + Message: "This user is already assigned to that task.", + } +} + // ================= // Namespace errors // ================= From 959811ae31991a871e580d9e0e7ca1352843352e Mon Sep 17 00:00:00 2001 From: viehlieb Date: Wed, 12 Oct 2022 13:55:36 +0200 Subject: [PATCH 3/3] fix minor issue in error.go --- pkg/models/error.go | 2 +- pkg/modules/auth/openid/openid.go | 10 +++------- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/pkg/models/error.go b/pkg/models/error.go index b9b0509e..c63ee71e 100644 --- a/pkg/models/error.go +++ b/pkg/models/error.go @@ -1106,7 +1106,7 @@ func IsErrTeamsDoNotExist(err error) bool { } func (err ErrTeamsDoNotExist) Error() string { - return fmt.Sprintf("Team does not exist [Team ID: %d]", err.Name) + return fmt.Sprintf("Team does not exist [Team Name: %v]", err.Name) } // 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 1c54c354..d0febc13 100644 --- a/pkg/modules/auth/openid/openid.go +++ b/pkg/modules/auth/openid/openid.go @@ -200,7 +200,7 @@ func HandleCallback(c echo.Context) error { } // Check if we have seen this user before - teams := GetOrCreateTeamsByNames(s, cl.Group, u) + teams, err := GetOrCreateTeamsByNames(s, cl.Group, u) if err != nil { log.Errorf("Error verifying team for name %v, got %v", cl.Name, teams, err) return err @@ -227,10 +227,7 @@ func HandleCallback(c echo.Context) error { return auth.NewUserAuthTokenResponse(u, c, false) } -func GetOrCreateTeamsByNames(s *xorm.Session, teamNames []string, u *user.User) (te []models.Team) { - // Check if a team with given name exists should be after user creation - - //TODO: 1. Create team if not exist +func GetOrCreateTeamsByNames(s *xorm.Session, teamNames []string, u *user.User) (te []models.Team, err error) { te = []models.Team{} for _, t := range teamNames { team, err := models.GetTeamsByName(s, t) @@ -240,7 +237,6 @@ func GetOrCreateTeamsByNames(s *xorm.Session, teamNames []string, u *user.User) tea := &models.Team{ Name: t, } - // TODO: here the user who creates the Team is automatically admin. That shoud not be the case..? err := tea.CreateNoAdmin(s, u) if err != nil { log.Errorf("Teams: %v, err: %v", tea, err) @@ -256,7 +252,7 @@ func GetOrCreateTeamsByNames(s *xorm.Session, teamNames []string, u *user.User) } } } - return te + return te, err } // assign user to team