opensource/grafana
1
0
Fork 0
mirror of https://github.com/grafana/grafana.git synced 2025-08-01 18:13:09 +08:00
Code Issues Packages Projects Releases Wiki Activity

RBAC: Add an additional check on UserID before fetching the permissions (#53002)

Browse Source 
* RBAC: add an additional check before fetching permissions

* Nit.

* Readd removed test

* change message
This commit is contained in:
Gabriel MABILLE
2022-08-03 11:06:06 +02:00
committed by GitHub
parent 2054414d37
commit 00ff61cb9e
2 changed files with 31 additions and 8 deletions
Download Patch File Download Diff File Expand all files Collapse all files

22
pkg/services/accesscontrol/database/database.go
View File

@ -60,8 +60,13 @@ func (s *AccessControlStore) GetUserPermissions(ctx context.Context, query acces
} }
func userRolesFilter(orgID, userID int64, roles []string) (string, []interface{}) { func userRolesFilter(orgID, userID int64, roles []string) (string, []interface{}) {
q := ` params := []interface{}{}
WHERE role.id IN ( q := `WHERE role.id IN (`
// This is an additional security. We should never have permissions granted to userID 0.
// Only allow real users to get user/team permissions (anonymous/apikeys)
if userID > 0 {
q += `
SELECT ur.role_id SELECT ur.role_id
FROM user_role AS ur FROM user_role AS ur
WHERE ur.user_id = ? WHERE ur.user_id = ?
@ -69,15 +74,18 @@ func userRolesFilter(orgID, userID int64, roles []string) (string, []interface{}
UNION UNION
SELECT tr.role_id FROM team_role as tr SELECT tr.role_id FROM team_role as tr
INNER JOIN team_member as tm ON tm.team_id = tr.team_id INNER JOIN team_member as tm ON tm.team_id = tr.team_id
WHERE tm.user_id = ? AND tr.org_id = ? WHERE tm.user_id = ? AND tr.org_id = ?`
` params = []interface{}{userID, orgID, globalOrgID, userID, orgID}
params := []interface{}{userID, orgID, globalOrgID, userID, orgID} }
if len(roles) != 0 { if len(roles) != 0 {
if userID > 0 {
q += `
UNION`
}
q += ` q += `
UNION
SELECT br.role_id FROM builtin_role AS br SELECT br.role_id FROM builtin_role AS br
WHERE role IN (? ` + strings.Repeat(", ?", len(roles)-1) + `) WHERE br.role IN (? ` + strings.Repeat(", ?", len(roles)-1) + `)
` `
for _, role := range roles { for _, role := range roles {
params = append(params, role) params = append(params, role)

17
pkg/services/accesscontrol/database/database_test.go
View File

@ -16,6 +16,7 @@ import (
type getUserPermissionsTestCase struct { type getUserPermissionsTestCase struct {
desc string desc string
anonymousUser bool
orgID int64 orgID int64
role string role string
userPermissions []string userPermissions []string
@ -74,6 +75,16 @@ func TestAccessControlStore_GetUserPermissions(t *testing.T) {
expected: 0, expected: 0,
actions: []string{}, actions: []string{},
}, },
{
desc: "should only get br permissions for anonymous user",
anonymousUser: true,
orgID: 1,
role: "Admin",
userPermissions: []string{"1", "2", "10"},
teamPermissions: []string{"100", "2"},
builtinPermissions: []string{"5", "6"},
expected: 2,
},
} }
for _, tt := range tests { for _, tt := range tests {
t.Run(tt.desc, func(t *testing.T) { t.Run(tt.desc, func(t *testing.T) {
@ -118,9 +129,13 @@ func TestAccessControlStore_GetUserPermissions(t *testing.T) {
} }
} }
userID := user.ID
if tt.anonymousUser {
userID = 0
}
permissions, err := store.GetUserPermissions(context.Background(), accesscontrol.GetUserPermissionsQuery{ permissions, err := store.GetUserPermissions(context.Background(), accesscontrol.GetUserPermissionsQuery{
OrgID: tt.orgID, OrgID: tt.orgID,
UserID: user.ID, UserID: userID,
Roles: roles, Roles: roles,
Actions: tt.actions, Actions: tt.actions,
}) })