AccessControl: Implement SQL filters for team members filtering (#44898) (#45219)

* AccessControl: Filter team members

* Modify GetTeamMembersByUser comment

* Fix postgres failing test due to quoting

* Rename GetTeamMembersByUser to GetUserTeamMemberships

* Update TeamStore interface

(cherry picked from commit 78fc0258b1)
This commit is contained in:
Gabriel MABILLE
2022-02-10 15:49:02 +01:00
committed by GitHub
parent 82fc75cacc
commit e6d0f5367f
9 changed files with 251 additions and 34 deletions
@@ -197,6 +197,10 @@ func (m SQLStoreMock) RemoveTeamMember(ctx context.Context, cmd *models.RemoveTe
return nil // TODO: Implement
}
func (m SQLStoreMock) GetUserTeamMemberships(ctx context.Context, orgID, userID int64, external bool) ([]*models.TeamMemberDTO, error) {
return nil, m.ExpectedError
}
func (m SQLStoreMock) GetTeamMembers(ctx context.Context, query *models.GetTeamMembersQuery) error {
return nil // TODO: Implement
}
+1
View File
@@ -53,6 +53,7 @@ type Store interface {
UpdateTeamMember(ctx context.Context, cmd *models.UpdateTeamMemberCommand) error
IsTeamMember(orgId int64, teamId int64, userId int64) (bool, error)
RemoveTeamMember(ctx context.Context, cmd *models.RemoveTeamMemberCommand) error
GetUserTeamMemberships(ctx context.Context, orgID, userID int64, external bool) ([]*models.TeamMemberDTO, error)
GetTeamMembers(ctx context.Context, query *models.GetTeamMembersQuery) error
NewSession(ctx context.Context) *DBSession
WithDbSession(ctx context.Context, callback DBTransactionFunc) error
+44 -2
View File
@@ -34,6 +34,7 @@ type TeamStore interface {
UpdateTeamMember(ctx context.Context, cmd *models.UpdateTeamMemberCommand) error
RemoveTeamMember(ctx context.Context, cmd *models.RemoveTeamMemberCommand) error
GetTeamMembers(ctx context.Context, cmd *models.GetTeamMembersQuery) error
GetUserTeamMemberships(ctx context.Context, orgID, userID int64, external bool) ([]*models.TeamMemberDTO, error)
AddOrUpdateTeamMember(userID, orgID, teamID int64, isExternal bool, permission models.PermissionType) error
}
@@ -492,11 +493,52 @@ func isLastAdmin(sess *DBSession, orgId int64, teamId int64, userId int64) (bool
return false, err
}
// GetTeamMembers return a list of members for the specified team
// GetUserTeamMemberships return a list of memberships to teams granted to a user
// If external is specified, only memberships provided by an external auth provider will be listed
// This function doesn't perform any accesscontrol filtering.
func (ss *SQLStore) GetUserTeamMemberships(ctx context.Context, orgID, userID int64, external bool) ([]*models.TeamMemberDTO, error) {
query := &models.GetTeamMembersQuery{
OrgId: orgID,
UserId: userID,
External: external,
Result: []*models.TeamMemberDTO{},
}
err := ss.getTeamMembers(ctx, query, nil)
return query.Result, err
}
// GetTeamMembers return a list of members for the specified team filtered based on the user's permissions
func (ss *SQLStore) GetTeamMembers(ctx context.Context, query *models.GetTeamMembersQuery) error {
acFilter := &ac.SQLFilter{}
var err error
// With accesscontrol we filter out users based on the SignedInUser's permissions
// Note we assume that checking SignedInUser is allowed to see team members for this team has already been performed
// If the signed in user is not set no member will be returned
if ss.Cfg.IsFeatureToggleEnabled(featuremgmt.FlagAccesscontrol) {
*acFilter, err = ac.Filter(ctx,
fmt.Sprintf("%s.%s", x.Dialect().Quote("user"), x.Dialect().Quote("id")),
"users", ac.ActionOrgUsersRead, query.SignedInUser,
)
if err != nil {
return err
}
}
return ss.getTeamMembers(ctx, query, acFilter)
}
// getTeamMembers return a list of members for the specified team
func (ss *SQLStore) getTeamMembers(ctx context.Context, query *models.GetTeamMembersQuery, acUserFilter *ac.SQLFilter) error {
query.Result = make([]*models.TeamMemberDTO, 0)
sess := x.Table("team_member")
sess.Join("INNER", x.Dialect().Quote("user"), fmt.Sprintf("team_member.user_id=%s.id", x.Dialect().Quote("user")))
sess.Join("INNER", x.Dialect().Quote("user"),
fmt.Sprintf("team_member.user_id=%s.%s", x.Dialect().Quote("user"), x.Dialect().Quote("id")),
)
if acUserFilter != nil {
sess.Where(acUserFilter.Where, acUserFilter.Args...)
}
// Join with only most recent auth module
authJoinCondition := `(
+107 -2
View File
@@ -8,11 +8,12 @@ import (
"fmt"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"github.com/grafana/grafana/pkg/models"
ac "github.com/grafana/grafana/pkg/services/accesscontrol"
"github.com/grafana/grafana/pkg/services/featuremgmt"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
func TestTeamCommandsAndQueries(t *testing.T) {
@@ -419,3 +420,107 @@ func TestSQLStore_SearchTeams(t *testing.T) {
})
}
}
// TestSQLStore_GetTeamMembers_ACFilter tests the accesscontrol filtering of
// team members based on the signed in user permissions
func TestSQLStore_GetTeamMembers_ACFilter(t *testing.T) {
testOrgID := int64(2)
userIds := make([]int64, 4)
// Seed 2 teams with 2 members
setup := func(store *SQLStore) {
team1, errCreateTeam := store.CreateTeam("group1 name", "test1@example.org", testOrgID)
require.NoError(t, errCreateTeam)
team2, errCreateTeam := store.CreateTeam("group2 name", "test2@example.org", testOrgID)
require.NoError(t, errCreateTeam)
for i := 0; i < 4; i++ {
userCmd := models.CreateUserCommand{
Email: fmt.Sprint("user", i, "@example.org"),
Name: fmt.Sprint("user", i),
Login: fmt.Sprint("loginuser", i),
}
user, errCreateUser := store.CreateUser(context.Background(), userCmd)
require.NoError(t, errCreateUser)
userIds[i] = user.Id
}
errAddMember := store.AddTeamMember(userIds[0], testOrgID, team1.Id, false, 0)
require.NoError(t, errAddMember)
errAddMember = store.AddTeamMember(userIds[1], testOrgID, team1.Id, false, 0)
require.NoError(t, errAddMember)
errAddMember = store.AddTeamMember(userIds[2], testOrgID, team2.Id, false, 0)
require.NoError(t, errAddMember)
errAddMember = store.AddTeamMember(userIds[3], testOrgID, team2.Id, false, 0)
require.NoError(t, errAddMember)
}
store := InitTestDB(t)
store.Cfg.IsFeatureToggleEnabled = featuremgmt.WithFeatures(featuremgmt.FlagAccesscontrol).IsEnabled
setup(store)
type getTeamMembersTestCase struct {
desc string
query *models.GetTeamMembersQuery
expectedNumUsers int
}
tests := []getTeamMembersTestCase{
{
desc: "should return all team members",
query: &models.GetTeamMembersQuery{
OrgId: testOrgID,
SignedInUser: &models.SignedInUser{
OrgId: testOrgID,
Permissions: map[int64]map[string][]string{testOrgID: {ac.ActionOrgUsersRead: {ac.ScopeUsersAll}}},
},
},
expectedNumUsers: 4,
},
{
desc: "should return no team members",
query: &models.GetTeamMembersQuery{
OrgId: testOrgID,
SignedInUser: &models.SignedInUser{
OrgId: testOrgID,
Permissions: map[int64]map[string][]string{testOrgID: {ac.ActionOrgUsersRead: {""}}},
},
},
expectedNumUsers: 0,
},
{
desc: "should return some team members",
query: &models.GetTeamMembersQuery{
OrgId: testOrgID,
SignedInUser: &models.SignedInUser{
OrgId: testOrgID,
Permissions: map[int64]map[string][]string{testOrgID: {ac.ActionOrgUsersRead: {
ac.Scope("users", "id", fmt.Sprintf("%d", userIds[0])),
ac.Scope("users", "id", fmt.Sprintf("%d", userIds[2])),
ac.Scope("users", "id", fmt.Sprintf("%d", userIds[3])),
}}},
},
},
expectedNumUsers: 3,
},
}
for _, tt := range tests {
t.Run(tt.desc, func(t *testing.T) {
err := store.GetTeamMembers(context.Background(), tt.query)
require.NoError(t, err)
assert.Len(t, tt.query.Result, tt.expectedNumUsers)
if !hasWildcardScope(tt.query.SignedInUser, ac.ActionOrgUsersRead) {
for _, member := range tt.query.Result {
assert.Contains(t,
tt.query.SignedInUser.Permissions[tt.query.SignedInUser.OrgId][ac.ActionOrgUsersRead],
ac.Scope("users", "id", fmt.Sprintf("%d", member.UserId)),
)
}
}
})
}
}