From 30274a4f88ce8b3bb7bae4e3744e345129463427 Mon Sep 17 00:00:00 2001 From: Jo Date: Wed, 2 Aug 2023 10:43:56 +0200 Subject: [PATCH] Auth: Move Team service to SignedInUserInterface (#72674) * move SignedInUser to specific file * add primitive interface for signedInUser --- pkg/services/accesscontrol/filter.go | 9 +- pkg/services/auth/identity/requester.go | 9 ++ pkg/services/team/model.go | 10 +- pkg/services/team/teamimpl/store.go | 8 +- pkg/services/team/teamimpl/store_test.go | 5 +- pkg/services/user/identity.go | 119 +++++++++++++++++++++++ pkg/services/user/model.go | 91 ----------------- 7 files changed, 146 insertions(+), 105 deletions(-) create mode 100644 pkg/services/auth/identity/requester.go create mode 100644 pkg/services/user/identity.go diff --git a/pkg/services/accesscontrol/filter.go b/pkg/services/accesscontrol/filter.go index 2df980d2f96..828393d05eb 100644 --- a/pkg/services/accesscontrol/filter.go +++ b/pkg/services/accesscontrol/filter.go @@ -5,7 +5,7 @@ import ( "strconv" "strings" - "github.com/grafana/grafana/pkg/services/user" + "github.com/grafana/grafana/pkg/services/auth/identity" ) var sqlIDAcceptList = map[string]struct{}{ @@ -33,18 +33,19 @@ type SQLFilter struct { // Filter creates a where clause to restrict the view of a query based on a users permissions // Scopes that exists for all actions will be parsed and compared against the supplied sqlID // Prefix parameter is the prefix of the scope that we support (e.g. "users:id:") -func Filter(user *user.SignedInUser, sqlID, prefix string, actions ...string) (SQLFilter, error) { +func Filter(user identity.Requester, sqlID, prefix string, actions ...string) (SQLFilter, error) { if _, ok := sqlIDAcceptList[sqlID]; !ok { return denyQuery, errors.New("sqlID is not in the accept list") } - if user == nil || user.Permissions == nil || user.Permissions[user.OrgID] == nil { + + if user == nil || user.IsNil() { return denyQuery, errors.New("missing permissions") } wildcards := 0 result := make(map[interface{}]int) for _, a := range actions { - ids, hasWildcard := ParseScopes(prefix, user.Permissions[user.OrgID][a]) + ids, hasWildcard := ParseScopes(prefix, user.GetPermissions(user.GetOrgID())[a]) if hasWildcard { wildcards += 1 continue diff --git a/pkg/services/auth/identity/requester.go b/pkg/services/auth/identity/requester.go new file mode 100644 index 00000000000..353b26b2573 --- /dev/null +++ b/pkg/services/auth/identity/requester.go @@ -0,0 +1,9 @@ +package identity + +type Requester interface { + GetIsGrafanaAdmin() bool + GetLogin() string + GetOrgID() int64 + GetPermissions(orgID int64) map[string][]string + IsNil() bool +} diff --git a/pkg/services/team/model.go b/pkg/services/team/model.go index b93056d80d4..c4e16e9d647 100644 --- a/pkg/services/team/model.go +++ b/pkg/services/team/model.go @@ -4,10 +4,12 @@ import ( "errors" "time" + v1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "github.com/grafana/grafana/pkg/kinds/team" + "github.com/grafana/grafana/pkg/services/auth/identity" "github.com/grafana/grafana/pkg/services/dashboards" "github.com/grafana/grafana/pkg/services/user" - v1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) // Typed errors @@ -70,7 +72,7 @@ type DeleteTeamCommand struct { type GetTeamByIDQuery struct { OrgID int64 ID int64 - SignedInUser *user.SignedInUser + SignedInUser identity.Requester HiddenUsers map[string]struct{} } @@ -80,7 +82,7 @@ const FilterIgnoreUser int64 = 0 type GetTeamsByUserQuery struct { OrgID int64 UserID int64 `json:"userId"` - SignedInUser *user.SignedInUser + SignedInUser identity.Requester } type SearchTeamsQuery struct { @@ -89,7 +91,7 @@ type SearchTeamsQuery struct { Limit int Page int OrgID int64 `xorm:"org_id"` - SignedInUser *user.SignedInUser + SignedInUser identity.Requester HiddenUsers map[string]struct{} } diff --git a/pkg/services/team/teamimpl/store.go b/pkg/services/team/teamimpl/store.go index 31682739918..0bd6761a5b8 100644 --- a/pkg/services/team/teamimpl/store.go +++ b/pkg/services/team/teamimpl/store.go @@ -9,9 +9,9 @@ import ( "github.com/grafana/grafana/pkg/infra/db" ac "github.com/grafana/grafana/pkg/services/accesscontrol" + "github.com/grafana/grafana/pkg/services/auth/identity" "github.com/grafana/grafana/pkg/services/dashboards" "github.com/grafana/grafana/pkg/services/team" - "github.com/grafana/grafana/pkg/services/user" "github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/util" ) @@ -37,14 +37,14 @@ type xormStore struct { cfg *setting.Cfg } -func getFilteredUsers(signedInUser *user.SignedInUser, hiddenUsers map[string]struct{}) []string { +func getFilteredUsers(signedInUser identity.Requester, hiddenUsers map[string]struct{}) []string { filteredUsers := make([]string, 0, len(hiddenUsers)) - if signedInUser == nil || signedInUser.IsGrafanaAdmin { + if signedInUser == nil || signedInUser.IsNil() || signedInUser.GetIsGrafanaAdmin() { return filteredUsers } for u := range hiddenUsers { - if u == signedInUser.Login { + if u == signedInUser.GetLogin() { continue } filteredUsers = append(filteredUsers, u) diff --git a/pkg/services/team/teamimpl/store_test.go b/pkg/services/team/teamimpl/store_test.go index 8e434dea227..3b1215cb618 100644 --- a/pkg/services/team/teamimpl/store_test.go +++ b/pkg/services/team/teamimpl/store_test.go @@ -461,9 +461,10 @@ func TestIntegrationSQLStore_SearchTeams(t *testing.T) { assert.Len(t, queryResult.Teams, tt.expectedTeamCount) assert.Equal(t, queryResult.TotalCount, int64(tt.expectedTeamCount)) - if !hasWildcardScope(tt.query.SignedInUser, ac.ActionTeamsRead) { + castSignedInUser := tt.query.SignedInUser.(*user.SignedInUser) + if !hasWildcardScope(castSignedInUser, ac.ActionTeamsRead) { for _, team := range queryResult.Teams { - assert.Contains(t, tt.query.SignedInUser.Permissions[tt.query.SignedInUser.OrgID][ac.ActionTeamsRead], fmt.Sprintf("teams:id:%d", team.ID)) + assert.Contains(t, castSignedInUser.Permissions[castSignedInUser.OrgID][ac.ActionTeamsRead], fmt.Sprintf("teams:id:%d", team.ID)) } } }) diff --git a/pkg/services/user/identity.go b/pkg/services/user/identity.go new file mode 100644 index 00000000000..75a1a2b1b07 --- /dev/null +++ b/pkg/services/user/identity.go @@ -0,0 +1,119 @@ +package user + +import ( + "fmt" + "time" + + "github.com/grafana/grafana/pkg/models/roletype" +) + +type SignedInUser struct { + UserID int64 `xorm:"user_id"` + OrgID int64 `xorm:"org_id"` + OrgName string + OrgRole roletype.RoleType + Login string + Name string + Email string + AuthenticatedBy string + ApiKeyID int64 `xorm:"api_key_id"` + IsServiceAccount bool `xorm:"is_service_account"` + IsGrafanaAdmin bool + IsAnonymous bool + IsDisabled bool + HelpFlags1 HelpFlags1 + LastSeenAt time.Time + Teams []int64 + // Permissions grouped by orgID and actions + Permissions map[int64]map[string][]string `json:"-"` +} + +func (u *SignedInUser) ShouldUpdateLastSeenAt() bool { + return u.UserID > 0 && time.Since(u.LastSeenAt) > time.Minute*5 +} + +func (u *SignedInUser) NameOrFallback() string { + if u.Name != "" { + return u.Name + } + if u.Login != "" { + return u.Login + } + return u.Email +} + +func (u *SignedInUser) ToUserDisplayDTO() *UserDisplayDTO { + return &UserDisplayDTO{ + ID: u.UserID, + Login: u.Login, + Name: u.Name, + } +} + +func (u *SignedInUser) HasRole(role roletype.RoleType) bool { + if u.IsGrafanaAdmin { + return true + } + + return u.OrgRole.Includes(role) +} + +// IsRealUser returns true if the user is a real user and not a service account +func (u *SignedInUser) IsRealUser() bool { + // backwards compatibility + // checking if userId the user is a real user + // previously we used to check if the UserId was 0 or -1 + // and not a service account + return u.UserID > 0 && !u.IsServiceAccountUser() +} + +func (u *SignedInUser) IsApiKeyUser() bool { + return u.ApiKeyID > 0 +} + +// IsServiceAccountUser returns true if the user is a service account +func (u *SignedInUser) IsServiceAccountUser() bool { + return u.IsServiceAccount +} + +func (u *SignedInUser) HasUniqueId() bool { + return u.IsRealUser() || u.IsApiKeyUser() || u.IsServiceAccountUser() +} + +func (u *SignedInUser) GetCacheKey() (string, error) { + if u.IsRealUser() { + return fmt.Sprintf("%d-user-%d", u.OrgID, u.UserID), nil + } + if u.IsApiKeyUser() { + return fmt.Sprintf("%d-apikey-%d", u.OrgID, u.ApiKeyID), nil + } + if u.IsServiceAccountUser() { // not considered a real user + return fmt.Sprintf("%d-service-%d", u.OrgID, u.UserID), nil + } + return "", ErrNoUniqueID +} + +func (u *SignedInUser) GetIsGrafanaAdmin() bool { + return u.IsGrafanaAdmin +} + +func (u *SignedInUser) GetLogin() string { + return u.Login +} + +func (u *SignedInUser) GetOrgID() int64 { + return u.OrgID +} + +func (u *SignedInUser) GetPermissions(orgID int64) map[string][]string { + if u.Permissions == nil { + return make(map[string][]string) + } + + return u.Permissions[orgID] +} + +// FIXME: remove this method once all services are using an interface +func (u *SignedInUser) IsNil() bool { + return u == nil +} diff --git a/pkg/services/user/model.go b/pkg/services/user/model.go index c3a80ddd61d..d30ad324980 100644 --- a/pkg/services/user/model.go +++ b/pkg/services/user/model.go @@ -5,8 +5,6 @@ import ( "fmt" "strings" "time" - - "github.com/grafana/grafana/pkg/models/roletype" ) type HelpFlags1 uint64 @@ -201,27 +199,6 @@ type AnalyticsSettings struct { IntercomIdentifier string } -type SignedInUser struct { - UserID int64 `xorm:"user_id"` - OrgID int64 `xorm:"org_id"` - OrgName string - OrgRole roletype.RoleType - Login string - Name string - Email string - AuthenticatedBy string - ApiKeyID int64 `xorm:"api_key_id"` - IsServiceAccount bool `xorm:"is_service_account"` - IsGrafanaAdmin bool - IsAnonymous bool - IsDisabled bool - HelpFlags1 HelpFlags1 - LastSeenAt time.Time - Teams []int64 - // Permissions grouped by orgID and actions - Permissions map[int64]map[string][]string `json:"-"` -} - func (u *User) NameOrFallback() string { if u.Name != "" { return u.Name @@ -251,74 +228,6 @@ type UserDisplayDTO struct { AvatarURL string `json:"avatarUrl"` } -// ------------------------ -// DTO & Projections - -func (u *SignedInUser) ShouldUpdateLastSeenAt() bool { - return u.UserID > 0 && time.Since(u.LastSeenAt) > time.Minute*5 -} - -func (u *SignedInUser) NameOrFallback() string { - if u.Name != "" { - return u.Name - } - if u.Login != "" { - return u.Login - } - return u.Email -} - -func (u *SignedInUser) ToUserDisplayDTO() *UserDisplayDTO { - return &UserDisplayDTO{ - ID: u.UserID, - Login: u.Login, - Name: u.Name, - } -} - -func (u *SignedInUser) HasRole(role roletype.RoleType) bool { - if u.IsGrafanaAdmin { - return true - } - - return u.OrgRole.Includes(role) -} - -// IsRealUser returns true if the user is a real user and not a service account -func (u *SignedInUser) IsRealUser() bool { - // backwards compatibility - // checking if userId the user is a real user - // previously we used to check if the UserId was 0 or -1 - // and not a service account - return u.UserID > 0 && !u.IsServiceAccountUser() -} - -func (u *SignedInUser) IsApiKeyUser() bool { - return u.ApiKeyID > 0 -} - -// IsServiceAccountUser returns true if the user is a service account -func (u *SignedInUser) IsServiceAccountUser() bool { - return u.IsServiceAccount -} - -func (u *SignedInUser) HasUniqueId() bool { - return u.IsRealUser() || u.IsApiKeyUser() || u.IsServiceAccountUser() -} - -func (u *SignedInUser) GetCacheKey() (string, error) { - if u.IsRealUser() { - return fmt.Sprintf("%d-user-%d", u.OrgID, u.UserID), nil - } - if u.IsApiKeyUser() { - return fmt.Sprintf("%d-apikey-%d", u.OrgID, u.ApiKeyID), nil - } - if u.IsServiceAccountUser() { // not considered a real user - return fmt.Sprintf("%d-service-%d", u.OrgID, u.UserID), nil - } - return "", ErrNoUniqueID -} - func (e *ErrCaseInsensitiveLoginConflict) Unwrap() error { return ErrCaseInsensitive }