Authz: Remove use of SignedInUser copy for permission evaluation (#78448)

* remove use of SignedInUserCopies

* add extra safety to not cross assign permissions

unwind circular dependency

dashboardacl->dashboardaccess

fix missing import

* correctly set teams for permissions

* fix missing inits

* nit: check err

* exit early for api keys
This commit is contained in:
Jo
2023-11-22 14:20:22 +01:00
committed by GitHub
parent 392a4342a8
commit 0de66a8099
44 changed files with 422 additions and 337 deletions

View File

@@ -8,7 +8,7 @@ import (
"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/dashboards/dashboardaccess"
"github.com/grafana/grafana/pkg/services/search/model"
)
@@ -103,15 +103,15 @@ type SearchTeamsQuery struct {
}
type TeamDTO struct {
ID int64 `json:"id" xorm:"id"`
UID string `json:"uid" xorm:"uid"`
OrgID int64 `json:"orgId" xorm:"org_id"`
Name string `json:"name"`
Email string `json:"email"`
AvatarURL string `json:"avatarUrl"`
MemberCount int64 `json:"memberCount"`
Permission dashboards.PermissionType `json:"permission"`
AccessControl map[string]bool `json:"accessControl"`
ID int64 `json:"id" xorm:"id"`
UID string `json:"uid" xorm:"uid"`
OrgID int64 `json:"orgId" xorm:"org_id"`
Name string `json:"name"`
Email string `json:"email"`
AvatarURL string `json:"avatarUrl"`
MemberCount int64 `json:"memberCount"`
Permission dashboardaccess.PermissionType `json:"permission"`
AccessControl map[string]bool `json:"accessControl"`
}
type SearchTeamQueryResult struct {
@@ -128,7 +128,7 @@ type TeamMember struct {
TeamID int64 `xorm:"team_id"`
UserID int64 `xorm:"user_id"`
External bool // Signals that the membership has been created by an external systems, such as LDAP
Permission dashboards.PermissionType
Permission dashboardaccess.PermissionType
Created time.Time
Updated time.Time
@@ -138,18 +138,18 @@ type TeamMember struct {
// COMMANDS
type AddTeamMemberCommand struct {
UserID int64 `json:"userId" binding:"Required"`
OrgID int64 `json:"-"`
TeamID int64 `json:"-"`
External bool `json:"-"`
Permission dashboards.PermissionType `json:"-"`
UserID int64 `json:"userId" binding:"Required"`
OrgID int64 `json:"-"`
TeamID int64 `json:"-"`
External bool `json:"-"`
Permission dashboardaccess.PermissionType `json:"-"`
}
type UpdateTeamMemberCommand struct {
UserID int64 `json:"-"`
OrgID int64 `json:"-"`
TeamID int64 `json:"-"`
Permission dashboards.PermissionType `json:"permission"`
UserID int64 `json:"-"`
OrgID int64 `json:"-"`
TeamID int64 `json:"-"`
Permission dashboardaccess.PermissionType `json:"permission"`
}
type RemoveTeamMemberCommand struct {
@@ -174,16 +174,16 @@ type GetTeamMembersQuery struct {
// Projections and DTOs
type TeamMemberDTO struct {
OrgID int64 `json:"orgId" xorm:"org_id"`
TeamID int64 `json:"teamId" xorm:"team_id"`
TeamUID string `json:"teamUID" xorm:"uid"`
UserID int64 `json:"userId" xorm:"user_id"`
External bool `json:"-"`
AuthModule string `json:"auth_module"`
Email string `json:"email"`
Name string `json:"name"`
Login string `json:"login"`
AvatarURL string `json:"avatarUrl" xorm:"avatar_url"`
Labels []string `json:"labels"`
Permission dashboards.PermissionType `json:"permission"`
OrgID int64 `json:"orgId" xorm:"org_id"`
TeamID int64 `json:"teamId" xorm:"team_id"`
TeamUID string `json:"teamUID" xorm:"uid"`
UserID int64 `json:"userId" xorm:"user_id"`
External bool `json:"-"`
AuthModule string `json:"auth_module"`
Email string `json:"email"`
Name string `json:"name"`
Login string `json:"login"`
AvatarURL string `json:"avatarUrl" xorm:"avatar_url"`
Labels []string `json:"labels"`
Permission dashboardaccess.PermissionType `json:"permission"`
}

View File

@@ -3,7 +3,7 @@ package team
import (
"context"
"github.com/grafana/grafana/pkg/services/dashboards"
"github.com/grafana/grafana/pkg/services/dashboards/dashboardaccess"
)
type Service interface {
@@ -14,7 +14,7 @@ type Service interface {
GetTeamByID(ctx context.Context, query *GetTeamByIDQuery) (*TeamDTO, error)
GetTeamsByUser(ctx context.Context, query *GetTeamsByUserQuery) ([]*TeamDTO, error)
GetTeamIDsByUser(ctx context.Context, query *GetTeamIDsByUserQuery) ([]int64, error)
AddTeamMember(userID, orgID, teamID int64, isExternal bool, permission dashboards.PermissionType) error
AddTeamMember(userID, orgID, teamID int64, isExternal bool, permission dashboardaccess.PermissionType) error
UpdateTeamMember(ctx context.Context, cmd *UpdateTeamMemberCommand) error
IsTeamMember(orgId int64, teamId int64, userId int64) (bool, error)
RemoveTeamMember(ctx context.Context, cmd *RemoveTeamMemberCommand) error

View File

@@ -10,7 +10,7 @@ import (
"github.com/grafana/grafana/pkg/services/accesscontrol"
"github.com/grafana/grafana/pkg/services/auth/identity"
contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model"
"github.com/grafana/grafana/pkg/services/dashboards"
"github.com/grafana/grafana/pkg/services/dashboards/dashboardaccess"
"github.com/grafana/grafana/pkg/services/preference/prefapi"
"github.com/grafana/grafana/pkg/services/team"
"github.com/grafana/grafana/pkg/services/team/sortopts"
@@ -58,7 +58,7 @@ func (tapi *TeamAPI) createTeam(c *contextmodel.ReqContext) response.Response {
break
}
if err := addOrUpdateTeamMember(c.Req.Context(), tapi.teamPermissionsService, userID, c.SignedInUser.GetOrgID(),
t.ID, dashboards.PERMISSION_ADMIN.String()); err != nil {
t.ID, dashboardaccess.PERMISSION_ADMIN.String()); err != nil {
c.Logger.Error("Could not add creator to team", "error", err)
}
default:

View File

@@ -11,7 +11,7 @@ import (
"github.com/grafana/grafana/pkg/api/response"
"github.com/grafana/grafana/pkg/services/accesscontrol"
contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model"
"github.com/grafana/grafana/pkg/services/dashboards"
"github.com/grafana/grafana/pkg/services/dashboards/dashboardaccess"
"github.com/grafana/grafana/pkg/services/login"
"github.com/grafana/grafana/pkg/services/team"
"github.com/grafana/grafana/pkg/util"
@@ -141,7 +141,7 @@ func (tapi *TeamAPI) updateTeamMember(c *contextmodel.ReqContext) response.Respo
return response.Success("Team member updated")
}
func getPermissionName(permission dashboards.PermissionType) string {
func getPermissionName(permission dashboardaccess.PermissionType) string {
permissionName := permission.String()
// Team member permission is 0, which maps to an empty string.
// However, we want the team permission service to display "Member" for team members. This is a hack to make it work.

View File

@@ -10,7 +10,7 @@ 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/dashboards/dashboardaccess"
"github.com/grafana/grafana/pkg/services/team"
"github.com/grafana/grafana/pkg/setting"
"github.com/grafana/grafana/pkg/util"
@@ -25,7 +25,7 @@ type store interface {
GetByUser(ctx context.Context, query *team.GetTeamsByUserQuery) ([]*team.TeamDTO, error)
GetIDsByUser(ctx context.Context, query *team.GetTeamIDsByUserQuery) ([]int64, error)
RemoveUsersMemberships(ctx context.Context, userID int64) error
AddMember(userID, orgID, teamID int64, isExternal bool, permission dashboards.PermissionType) error
AddMember(userID, orgID, teamID int64, isExternal bool, permission dashboardaccess.PermissionType) error
UpdateMember(ctx context.Context, cmd *team.UpdateTeamMemberCommand) error
IsMember(orgId int64, teamId int64, userId int64) (bool, error)
RemoveMember(ctx context.Context, cmd *team.RemoveTeamMemberCommand) error
@@ -354,7 +354,7 @@ WHERE tm.user_id=? AND tm.org_id=?;`, query.UserID, query.OrgID).Find(&queryResu
}
// AddTeamMember adds a user to a team
func (ss *xormStore) AddMember(userID, orgID, teamID int64, isExternal bool, permission dashboards.PermissionType) error {
func (ss *xormStore) AddMember(userID, orgID, teamID int64, isExternal bool, permission dashboardaccess.PermissionType) error {
return ss.db.WithTransactionalDbSession(context.Background(), func(sess *db.Session) error {
if isMember, err := isTeamMember(sess, orgID, teamID, userID); err != nil {
return err
@@ -412,7 +412,7 @@ func isTeamMember(sess *db.Session, orgId int64, teamId int64, userId int64) (bo
// AddOrUpdateTeamMemberHook is called from team resource permission service
// it adds user to a team or updates user permissions in a team within the given transaction session
func AddOrUpdateTeamMemberHook(sess *db.Session, userID, orgID, teamID int64, isExternal bool, permission dashboards.PermissionType) error {
func AddOrUpdateTeamMemberHook(sess *db.Session, userID, orgID, teamID int64, isExternal bool, permission dashboardaccess.PermissionType) error {
isMember, err := isTeamMember(sess, orgID, teamID, userID)
if err != nil {
return err
@@ -427,7 +427,7 @@ func AddOrUpdateTeamMemberHook(sess *db.Session, userID, orgID, teamID int64, is
return err
}
func addTeamMember(sess *db.Session, orgID, teamID, userID int64, isExternal bool, permission dashboards.PermissionType) error {
func addTeamMember(sess *db.Session, orgID, teamID, userID int64, isExternal bool, permission dashboardaccess.PermissionType) error {
if _, err := teamExists(orgID, teamID, sess); err != nil {
return err
}
@@ -446,13 +446,13 @@ func addTeamMember(sess *db.Session, orgID, teamID, userID int64, isExternal boo
return err
}
func updateTeamMember(sess *db.Session, orgID, teamID, userID int64, permission dashboards.PermissionType) error {
func updateTeamMember(sess *db.Session, orgID, teamID, userID int64, permission dashboardaccess.PermissionType) error {
member, err := getTeamMember(sess, orgID, teamID, userID)
if err != nil {
return err
}
if permission != dashboards.PERMISSION_ADMIN {
if permission != dashboardaccess.PERMISSION_ADMIN {
permission = 0 // make sure we don't get invalid permission levels in store
}

View File

@@ -12,7 +12,7 @@ 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/dashboards/dashboardaccess"
"github.com/grafana/grafana/pkg/services/org/orgimpl"
"github.com/grafana/grafana/pkg/services/quota/quotaimpl"
"github.com/grafana/grafana/pkg/services/serviceaccounts"
@@ -174,7 +174,7 @@ func TestIntegrationTeamCommandsAndQueries(t *testing.T) {
UserID: userId,
OrgID: testOrgID,
TeamID: team1.ID,
Permission: dashboards.PERMISSION_ADMIN,
Permission: dashboardaccess.PERMISSION_ADMIN,
})
require.NoError(t, err)
@@ -182,7 +182,7 @@ func TestIntegrationTeamCommandsAndQueries(t *testing.T) {
qAfterUpdate := &team.GetTeamMembersQuery{OrgID: testOrgID, TeamID: team1.ID, SignedInUser: testUser}
qAfterUpdateResult, err := teamSvc.GetTeamMembers(context.Background(), qAfterUpdate)
require.NoError(t, err)
require.Equal(t, qAfterUpdateResult[0].Permission, dashboards.PERMISSION_ADMIN)
require.Equal(t, qAfterUpdateResult[0].Permission, dashboardaccess.PERMISSION_ADMIN)
})
t.Run("Should default to member permission level when updating a user with invalid permission level", func(t *testing.T) {
@@ -197,7 +197,7 @@ func TestIntegrationTeamCommandsAndQueries(t *testing.T) {
require.NoError(t, err)
require.EqualValues(t, qBeforeUpdateResult[0].Permission, 0)
invalidPermissionLevel := dashboards.PERMISSION_EDIT
invalidPermissionLevel := dashboardaccess.PERMISSION_EDIT
err = teamSvc.UpdateTeamMember(context.Background(), &team.UpdateTeamMemberCommand{
UserID: userID,
OrgID: testOrgID,
@@ -220,7 +220,7 @@ func TestIntegrationTeamCommandsAndQueries(t *testing.T) {
UserID: 1,
OrgID: testOrgID,
TeamID: team1.ID,
Permission: dashboards.PERMISSION_ADMIN,
Permission: dashboardaccess.PERMISSION_ADMIN,
})
require.Error(t, err, team.ErrTeamMemberNotFound)
@@ -330,7 +330,7 @@ func TestIntegrationTeamCommandsAndQueries(t *testing.T) {
})
t.Run("Should have empty teams", func(t *testing.T) {
err = teamSvc.AddTeamMember(userIds[0], testOrgID, team1.ID, false, dashboards.PERMISSION_ADMIN)
err = teamSvc.AddTeamMember(userIds[0], testOrgID, team1.ID, false, dashboardaccess.PERMISSION_ADMIN)
require.NoError(t, err)
t.Run("A user should be able to remove the admin permission for the last admin", func(t *testing.T) {
@@ -347,10 +347,10 @@ func TestIntegrationTeamCommandsAndQueries(t *testing.T) {
sqlStore = db.InitTestDB(t)
setup()
err = teamSvc.AddTeamMember(userIds[0], testOrgID, team1.ID, false, dashboards.PERMISSION_ADMIN)
err = teamSvc.AddTeamMember(userIds[0], testOrgID, team1.ID, false, dashboardaccess.PERMISSION_ADMIN)
require.NoError(t, err)
err = teamSvc.AddTeamMember(userIds[1], testOrgID, team1.ID, false, dashboards.PERMISSION_ADMIN)
err = teamSvc.AddTeamMember(userIds[1], testOrgID, team1.ID, false, dashboardaccess.PERMISSION_ADMIN)
require.NoError(t, err)
err = teamSvc.UpdateTeamMember(context.Background(), &team.UpdateTeamMemberCommand{OrgID: testOrgID, TeamID: team1.ID, UserID: userIds[0], Permission: 0})
require.NoError(t, err)

View File

@@ -4,7 +4,7 @@ import (
"context"
"github.com/grafana/grafana/pkg/infra/db"
"github.com/grafana/grafana/pkg/services/dashboards"
"github.com/grafana/grafana/pkg/services/dashboards/dashboardaccess"
"github.com/grafana/grafana/pkg/services/team"
"github.com/grafana/grafana/pkg/setting"
)
@@ -45,7 +45,7 @@ func (s *Service) GetTeamIDsByUser(ctx context.Context, query *team.GetTeamIDsBy
return s.store.GetIDsByUser(ctx, query)
}
func (s *Service) AddTeamMember(userID, orgID, teamID int64, isExternal bool, permission dashboards.PermissionType) error {
func (s *Service) AddTeamMember(userID, orgID, teamID int64, isExternal bool, permission dashboardaccess.PermissionType) error {
return s.store.AddMember(userID, orgID, teamID, isExternal, permission)
}

View File

@@ -3,7 +3,7 @@ package teamtest
import (
"context"
"github.com/grafana/grafana/pkg/services/dashboards"
"github.com/grafana/grafana/pkg/services/dashboards/dashboardaccess"
"github.com/grafana/grafana/pkg/services/team"
)
@@ -45,7 +45,7 @@ func (s *FakeService) GetTeamsByUser(ctx context.Context, query *team.GetTeamsBy
return s.ExpectedTeamsByUser, s.ExpectedError
}
func (s *FakeService) AddTeamMember(userID, orgID, teamID int64, isExternal bool, permission dashboards.PermissionType) error {
func (s *FakeService) AddTeamMember(userID, orgID, teamID int64, isExternal bool, permission dashboardaccess.PermissionType) error {
return s.ExpectedError
}