Authz: Skip cache in List request if option provided (#110864)
* Authz: Skip cache in List request if option provided * return timestamp with list response * update authlib * add skipCache option test * refactor * fix tests * update workspaces * Set zookies depending on cache hit * update workspaces * Fix nil pointer
This commit is contained in:
@@ -22,4 +22,9 @@ type ListRequest struct {
|
||||
Resource string
|
||||
Verb string
|
||||
Action string
|
||||
Options *ListRequestOptions
|
||||
}
|
||||
|
||||
type ListRequestOptions struct {
|
||||
SkipCache bool
|
||||
}
|
||||
|
||||
@@ -18,6 +18,7 @@ import (
|
||||
authzv1 "github.com/grafana/authlib/authz/proto/v1"
|
||||
"github.com/grafana/authlib/cache"
|
||||
"github.com/grafana/authlib/types"
|
||||
|
||||
"github.com/grafana/grafana/pkg/apimachinery/utils"
|
||||
|
||||
"github.com/grafana/grafana/pkg/infra/log"
|
||||
@@ -209,10 +210,17 @@ func (s *Service) List(ctx context.Context, req *authzv1.ListRequest) (*authzv1.
|
||||
attribute.String("action", listReq.Action),
|
||||
)
|
||||
|
||||
permissions, err := s.getCachedIdentityPermissions(ctx, listReq.Namespace, listReq.IdentityType, listReq.UserUID, listReq.Action)
|
||||
if err == nil {
|
||||
s.metrics.permissionCacheUsage.WithLabelValues("true", listReq.Action).Inc()
|
||||
} else {
|
||||
var permissions map[string]bool
|
||||
cacheHit := false
|
||||
|
||||
if !listReq.Options.SkipCache {
|
||||
permissions, err = s.getCachedIdentityPermissions(ctx, listReq.Namespace, listReq.IdentityType, listReq.UserUID, listReq.Action)
|
||||
if err == nil {
|
||||
s.metrics.permissionCacheUsage.WithLabelValues("true", listReq.Action).Inc()
|
||||
cacheHit = true
|
||||
}
|
||||
}
|
||||
if err != nil || listReq.Options.SkipCache {
|
||||
s.metrics.permissionCacheUsage.WithLabelValues("false", listReq.Action).Inc()
|
||||
|
||||
permissions, err = s.getIdentityPermissions(ctx, listReq.Namespace, listReq.IdentityType, listReq.UserUID, listReq.Action)
|
||||
@@ -224,6 +232,10 @@ func (s *Service) List(ctx context.Context, req *authzv1.ListRequest) (*authzv1.
|
||||
}
|
||||
|
||||
resp, err := s.listPermission(ctx, permissions, listReq)
|
||||
if cacheHit && time.Duration(time.Now().Unix()-resp.Zookie.Timestamp) < s.settings.CacheTTL {
|
||||
resp.Zookie = &authzv1.Zookie{Timestamp: time.Now().Add(-s.settings.CacheTTL).Unix()}
|
||||
}
|
||||
|
||||
s.metrics.requestCount.WithLabelValues(strconv.FormatBool(err != nil), "true", req.GetVerb(), req.GetGroup(), req.GetResource()).Inc()
|
||||
return resp, err
|
||||
}
|
||||
@@ -280,6 +292,14 @@ func (s *Service) validateListRequest(ctx context.Context, req *authzv1.ListRequ
|
||||
return nil, err
|
||||
}
|
||||
|
||||
authzOptions := req.GetOptions()
|
||||
if authzOptions == nil {
|
||||
authzOptions = &authzv1.ListRequestOptions{}
|
||||
}
|
||||
options := &ListRequestOptions{
|
||||
SkipCache: authzOptions.Skipcache,
|
||||
}
|
||||
|
||||
listReq := &ListRequest{
|
||||
Namespace: ns,
|
||||
UserUID: userUID,
|
||||
@@ -288,6 +308,7 @@ func (s *Service) validateListRequest(ctx context.Context, req *authzv1.ListRequ
|
||||
Group: req.GetGroup(),
|
||||
Resource: req.GetResource(),
|
||||
Verb: req.GetVerb(),
|
||||
Options: options,
|
||||
}
|
||||
return listReq, nil
|
||||
}
|
||||
@@ -706,7 +727,10 @@ func (s *Service) buildFolderTree(ctx context.Context, ns types.NamespaceInfo) (
|
||||
|
||||
func (s *Service) listPermission(ctx context.Context, scopeMap map[string]bool, req *ListRequest) (*authzv1.ListResponse, error) {
|
||||
if scopeMap["*"] {
|
||||
return &authzv1.ListResponse{All: true}, nil
|
||||
return &authzv1.ListResponse{
|
||||
All: true,
|
||||
Zookie: &authzv1.Zookie{Timestamp: time.Now().Unix()},
|
||||
}, nil
|
||||
}
|
||||
|
||||
ctx, span := s.tracer.Start(ctx, "authz_direct_db.service.listPermission")
|
||||
@@ -720,9 +744,14 @@ func (s *Service) listPermission(ctx context.Context, scopeMap map[string]bool,
|
||||
}
|
||||
|
||||
var tree folderTree
|
||||
cacheHit := false
|
||||
if t.HasFolderSupport() {
|
||||
var err error
|
||||
tree, ok = s.getCachedFolderTree(ctx, req.Namespace)
|
||||
ok = false
|
||||
if !req.Options.SkipCache {
|
||||
tree, ok = s.getCachedFolderTree(ctx, req.Namespace)
|
||||
cacheHit = true
|
||||
}
|
||||
if !ok {
|
||||
tree, err = s.buildFolderTree(ctx, req.Namespace)
|
||||
if err != nil {
|
||||
@@ -739,6 +768,12 @@ func (s *Service) listPermission(ctx context.Context, scopeMap map[string]bool,
|
||||
res = buildItemList(scopeMap, tree, t.Prefix())
|
||||
}
|
||||
|
||||
if cacheHit {
|
||||
res.Zookie = &authzv1.Zookie{Timestamp: time.Now().Add(-s.settings.CacheTTL).Unix()}
|
||||
} else {
|
||||
res.Zookie = &authzv1.Zookie{Timestamp: time.Now().Unix()}
|
||||
}
|
||||
|
||||
span.SetAttributes(attribute.Int("num_folders", len(res.Folders)), attribute.Int("num_items", len(res.Items)))
|
||||
return res, nil
|
||||
}
|
||||
|
||||
@@ -367,6 +367,43 @@ func TestService_checkPermission_folderCacheMissRecovery(t *testing.T) {
|
||||
assert.Equal(t, 1, folderStore.calls)
|
||||
}
|
||||
|
||||
func TestService_listPermission_skipCache(t *testing.T) {
|
||||
s := setupService()
|
||||
ctx := context.Background()
|
||||
|
||||
// User has root folder access
|
||||
userPermissions := map[string]bool{
|
||||
"folders:uid:root": true,
|
||||
}
|
||||
|
||||
// Populate store with folders
|
||||
folderStore := &fakeStore{
|
||||
folders: []store.Folder{{UID: "root"}, {UID: "sub", ParentUID: strPtr("root")}},
|
||||
disableNsCheck: true,
|
||||
}
|
||||
s.folderStore = folderStore
|
||||
|
||||
// Sub folder is missing from the cache
|
||||
s.folderCache.Set(ctx, folderCacheKey("default"), newFolderTree([]store.Folder{{UID: "root"}}))
|
||||
|
||||
// Perform list
|
||||
listReq := ListRequest{
|
||||
Action: "folders:read",
|
||||
Group: "folder.grafana.app",
|
||||
Resource: "folders",
|
||||
Namespace: types.NamespaceInfo{Value: "default", OrgID: 1},
|
||||
Options: &ListRequestOptions{SkipCache: true},
|
||||
}
|
||||
|
||||
res, err := s.listPermission(ctx, userPermissions, &listReq)
|
||||
require.NoError(t, err)
|
||||
// Check that all folders are in returned list
|
||||
assert.Len(t, res.GetItems(), 2)
|
||||
|
||||
// Check that folder store was queried
|
||||
assert.Equal(t, 1, folderStore.calls)
|
||||
}
|
||||
|
||||
func TestService_getUserTeams(t *testing.T) {
|
||||
type testCase struct {
|
||||
name string
|
||||
@@ -612,6 +649,7 @@ func TestService_listPermission(t *testing.T) {
|
||||
Action: "dashboards:read",
|
||||
Group: "dashboard.grafana.app",
|
||||
Resource: "dashboards",
|
||||
Options: &ListRequestOptions{},
|
||||
},
|
||||
expectedAll: true,
|
||||
},
|
||||
@@ -648,6 +686,7 @@ func TestService_listPermission(t *testing.T) {
|
||||
Action: "dashboards:read",
|
||||
Group: "dashboard.grafana.app",
|
||||
Resource: "dashboards",
|
||||
Options: &ListRequestOptions{},
|
||||
},
|
||||
expectedItems: []string{"some_dashboard"},
|
||||
expectedFolders: []string{"some_folder_1", "some_folder_2"},
|
||||
@@ -675,6 +714,7 @@ func TestService_listPermission(t *testing.T) {
|
||||
Action: "dashboards:read",
|
||||
Group: "dashboard.grafana.app",
|
||||
Resource: "dashboards",
|
||||
Options: &ListRequestOptions{},
|
||||
},
|
||||
expectedFolders: []string{"some_folder_parent", "some_folder_child", "some_folder_subchild1", "some_folder_subchild2", "some_folder_subsubchild"},
|
||||
},
|
||||
@@ -704,6 +744,7 @@ func TestService_listPermission(t *testing.T) {
|
||||
Action: "dashboards:read",
|
||||
Group: "dashboard.grafana.app",
|
||||
Resource: "dashboards",
|
||||
Options: &ListRequestOptions{},
|
||||
},
|
||||
expectedItems: []string{"some_dashboard"},
|
||||
expectedFolders: []string{"some_folder_parent", "some_folder_child"},
|
||||
@@ -736,6 +777,7 @@ func TestService_listPermission(t *testing.T) {
|
||||
Action: "dashboards:read",
|
||||
Group: "dashboard.grafana.app",
|
||||
Resource: "dashboards",
|
||||
Options: &ListRequestOptions{},
|
||||
},
|
||||
expectedFolders: []string{"some_folder_parent", "some_folder_child", "some_folder_child2", "some_folder_subchild"},
|
||||
},
|
||||
@@ -750,6 +792,7 @@ func TestService_listPermission(t *testing.T) {
|
||||
Action: "dashboards:read",
|
||||
Group: "dashboard.grafana.app",
|
||||
Resource: "dashboards",
|
||||
Options: &ListRequestOptions{},
|
||||
},
|
||||
},
|
||||
{
|
||||
@@ -771,6 +814,7 @@ func TestService_listPermission(t *testing.T) {
|
||||
Action: "folders:read",
|
||||
Group: "folder.grafana.app",
|
||||
Resource: "folders",
|
||||
Options: &ListRequestOptions{},
|
||||
},
|
||||
expectedItems: []string{"some_folder_parent", "some_folder_child"},
|
||||
},
|
||||
|
||||
Reference in New Issue
Block a user