From 7cb7290a3e94c1c5e764866f18db686eac573832 Mon Sep 17 00:00:00 2001 From: Gabriel MABILLE Date: Tue, 17 May 2022 15:47:31 +0200 Subject: [PATCH] AccessControl: Enforce user check when enterprise accesscontrol is on (#49003) * AccessControl: Enforce user check when enterprise accesscontrol is on * Update the test not to fail enterprise build * Adding a log as suggested by Kalle Co-authored-by: Kalle Persson * Update log message Co-authored-by: Kalle Persson --- pkg/services/sqlstore/org_test.go | 32 +++++++++++++++++++++++++++--- pkg/services/sqlstore/org_users.go | 5 ++++- 2 files changed, 33 insertions(+), 4 deletions(-) diff --git a/pkg/services/sqlstore/org_test.go b/pkg/services/sqlstore/org_test.go index 437a1ed64d0..4ddfc18d6c9 100644 --- a/pkg/services/sqlstore/org_test.go +++ b/pkg/services/sqlstore/org_test.go @@ -203,7 +203,13 @@ func TestAccountDataAccess(t *testing.T) { err = sqlStore.UpdateOrgUser(context.Background(), &updateCmd) require.NoError(t, err) - orgUsersQuery := models.GetOrgUsersQuery{OrgId: ac1.OrgId} + orgUsersQuery := models.GetOrgUsersQuery{ + OrgId: ac1.OrgId, + User: &models.SignedInUser{ + OrgId: ac1.OrgId, + Permissions: map[int64]map[string][]string{ac1.OrgId: {accesscontrol.ActionOrgUsersRead: {accesscontrol.ScopeUsersAll}}}, + }, + } err = sqlStore.GetOrgUsers(context.Background(), &orgUsersQuery) require.NoError(t, err) @@ -233,7 +239,13 @@ func TestAccountDataAccess(t *testing.T) { }) t.Run("Can get organization users", func(t *testing.T) { - query := models.GetOrgUsersQuery{OrgId: ac1.OrgId} + query := models.GetOrgUsersQuery{ + OrgId: ac1.OrgId, + User: &models.SignedInUser{ + OrgId: ac1.OrgId, + Permissions: map[int64]map[string][]string{ac1.OrgId: {accesscontrol.ActionOrgUsersRead: {accesscontrol.ScopeUsersAll}}}, + }, + } err := sqlStore.GetOrgUsers(context.Background(), &query) require.NoError(t, err) @@ -245,6 +257,10 @@ func TestAccountDataAccess(t *testing.T) { query := models.GetOrgUsersQuery{ OrgId: ac1.OrgId, Query: "ac1", + User: &models.SignedInUser{ + OrgId: ac1.OrgId, + Permissions: map[int64]map[string][]string{ac1.OrgId: {accesscontrol.ActionOrgUsersRead: {accesscontrol.ScopeUsersAll}}}, + }, } err := sqlStore.GetOrgUsers(context.Background(), &query) @@ -258,6 +274,10 @@ func TestAccountDataAccess(t *testing.T) { OrgId: ac1.OrgId, Query: "ac", Limit: 1, + User: &models.SignedInUser{ + OrgId: ac1.OrgId, + Permissions: map[int64]map[string][]string{ac1.OrgId: {accesscontrol.ActionOrgUsersRead: {accesscontrol.ScopeUsersAll}}}, + }, } err := sqlStore.GetOrgUsers(context.Background(), &query) @@ -338,7 +358,13 @@ func TestAccountDataAccess(t *testing.T) { err = sqlStore.AddOrgUser(context.Background(), &orgUserCmd) require.NoError(t, err) - query := models.GetOrgUsersQuery{OrgId: ac1.OrgId} + query := models.GetOrgUsersQuery{ + OrgId: ac1.OrgId, + User: &models.SignedInUser{ + OrgId: ac1.OrgId, + Permissions: map[int64]map[string][]string{ac1.OrgId: {accesscontrol.ActionOrgUsersRead: {accesscontrol.ScopeUsersAll}}}, + }, + } err = sqlStore.GetOrgUsers(context.Background(), &query) require.NoError(t, err) // require.Equal(t, len(query.Result), 3) diff --git a/pkg/services/sqlstore/org_users.go b/pkg/services/sqlstore/org_users.go index cf00e095141..65eb4a67c57 100644 --- a/pkg/services/sqlstore/org_users.go +++ b/pkg/services/sqlstore/org_users.go @@ -109,7 +109,10 @@ func (ss *SQLStore) GetOrgUsers(ctx context.Context, query *models.GetOrgUsersQu whereConditions = append(whereConditions, fmt.Sprintf("%s.is_service_account = ?", ss.Dialect.Quote("user"))) whereParams = append(whereParams, ss.Dialect.BooleanStr(false)) - if ss.Cfg.IsEnterprise && !accesscontrol.IsDisabled(ss.Cfg) && query.User != nil { + if query.User == nil { + ss.log.Warn("Query user not set for filtering.") + } + if ss.Cfg.IsEnterprise && !accesscontrol.IsDisabled(ss.Cfg) { acFilter, err := accesscontrol.Filter(query.User, "org_user.user_id", "users:id:", accesscontrol.ActionOrgUsersRead) if err != nil { return err