Skip to content

Commit f5b90cf

Browse files
committed
refactor: reuse Check method in filter candidates, remove duplicate code
Address CR feedback from yosiharan: - Remove redundant projectCache and mgmtSDK parameters from filter methods - Replace checkRelationsWithCache with direct calls to existing Check method - Delete duplicate checkRelationsWithCache method (~30 lines removed)
1 parent c20b874 commit f5b90cf

File tree

1 file changed

+6
-40
lines changed

1 file changed

+6
-40
lines changed

internal/services/authz.go

Lines changed: 6 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,7 @@ func (a *authzCache) WhoCanAccess(ctx context.Context, resource, relationDefinit
149149
}
150150
candidates, cacheHit := projectCache.GetWhoCanAccessCached(ctx, resource, relationDefinition, namespace)
151151
if cacheHit && len(candidates) > 0 {
152-
verified, err := a.filterWhoCanAccessCandidates(ctx, projectCache, mgmtSDK, resource, relationDefinition, namespace, candidates)
152+
verified, err := a.filterWhoCanAccessCandidates(ctx, resource, relationDefinition, namespace, candidates)
153153
if err != nil {
154154
return nil, err // notest
155155
}
@@ -168,7 +168,7 @@ func (a *authzCache) WhoCanAccess(ctx context.Context, resource, relationDefinit
168168
return targets, nil
169169
}
170170

171-
func (a *authzCache) filterWhoCanAccessCandidates(ctx context.Context, projectCache caches.ProjectAuthzCache, mgmtSDK sdk.Management, resource, relationDefinition, namespace string, candidates []string) ([]string, error) {
171+
func (a *authzCache) filterWhoCanAccessCandidates(ctx context.Context, resource, relationDefinition, namespace string, candidates []string) ([]string, error) {
172172
relations := make([]*descope.FGARelation, len(candidates))
173173
for i, target := range candidates {
174174
relations[i] = &descope.FGARelation{
@@ -178,7 +178,7 @@ func (a *authzCache) filterWhoCanAccessCandidates(ctx context.Context, projectCa
178178
Target: target,
179179
}
180180
}
181-
checks, err := a.checkRelationsWithCache(ctx, projectCache, mgmtSDK, relations)
181+
checks, err := a.Check(ctx, relations)
182182
if err != nil {
183183
return nil, err
184184
}
@@ -198,7 +198,7 @@ func (a *authzCache) WhatCanTargetAccess(ctx context.Context, target string) ([]
198198
}
199199
candidates, cacheHit := projectCache.GetWhatCanTargetAccessCached(ctx, target)
200200
if cacheHit && len(candidates) > 0 {
201-
verified, err := a.filterWhatCanTargetAccessCandidates(ctx, projectCache, mgmtSDK, target, candidates)
201+
verified, err := a.filterWhatCanTargetAccessCandidates(ctx, target, candidates)
202202
if err != nil {
203203
return nil, err // notest
204204
}
@@ -217,7 +217,7 @@ func (a *authzCache) WhatCanTargetAccess(ctx context.Context, target string) ([]
217217
return relations, nil
218218
}
219219

220-
func (a *authzCache) filterWhatCanTargetAccessCandidates(ctx context.Context, projectCache caches.ProjectAuthzCache, mgmtSDK sdk.Management, target string, candidates []*descope.AuthzRelation) ([]*descope.AuthzRelation, error) {
220+
func (a *authzCache) filterWhatCanTargetAccessCandidates(ctx context.Context, target string, candidates []*descope.AuthzRelation) ([]*descope.AuthzRelation, error) {
221221
relations := make([]*descope.FGARelation, len(candidates))
222222
for i, r := range candidates {
223223
relations[i] = &descope.FGARelation{
@@ -227,7 +227,7 @@ func (a *authzCache) filterWhatCanTargetAccessCandidates(ctx context.Context, pr
227227
Target: target,
228228
}
229229
}
230-
checks, err := a.checkRelationsWithCache(ctx, projectCache, mgmtSDK, relations)
230+
checks, err := a.Check(ctx, relations)
231231
if err != nil {
232232
return nil, err
233233
}
@@ -240,40 +240,6 @@ func (a *authzCache) filterWhatCanTargetAccessCandidates(ctx context.Context, pr
240240
return verified, nil
241241
}
242242

243-
func (a *authzCache) checkRelationsWithCache(ctx context.Context, projectCache caches.ProjectAuthzCache, mgmtSDK sdk.Management, relations []*descope.FGARelation) ([]*descope.FGACheck, error) {
244-
var cachedChecks []*descope.FGACheck
245-
var toCheckViaSDK []*descope.FGARelation
246-
indexToCachedChecks := make(map[int]*descope.FGACheck, len(relations))
247-
for i, r := range relations {
248-
if allowed, direct, ok := projectCache.CheckRelation(ctx, r); ok {
249-
check := &descope.FGACheck{Allowed: allowed, Relation: r, Info: &descope.FGACheckInfo{Direct: direct}}
250-
cachedChecks = append(cachedChecks, check)
251-
indexToCachedChecks[i] = check
252-
} else {
253-
toCheckViaSDK = append(toCheckViaSDK, r)
254-
}
255-
}
256-
if len(toCheckViaSDK) == 0 {
257-
return cachedChecks, nil
258-
}
259-
sdkChecks, err := mgmtSDK.FGA().Check(ctx, toCheckViaSDK)
260-
if err != nil {
261-
return nil, err
262-
}
263-
projectCache.UpdateCacheWithChecks(ctx, sdkChecks)
264-
var result []*descope.FGACheck
265-
var j int
266-
for i := range relations {
267-
if check, ok := indexToCachedChecks[i]; ok {
268-
result = append(result, check)
269-
} else {
270-
result = append(result, sdkChecks[j])
271-
j++
272-
}
273-
}
274-
return result, nil
275-
}
276-
277243
func (a *authzCache) getOrCreateProjectCache(ctx context.Context) (caches.ProjectAuthzCache, sdk.Management, error) {
278244
projectID := cctx.ProjectID(ctx)
279245
if p, ok := a.projects.Load(projectID); ok {

0 commit comments

Comments
 (0)