feat: add operation authorization forward middleware#3021
feat: add operation authorization forward middleware#3021Piskoo wants to merge 3 commits intochainloop-dev:mainfrom
Conversation
Signed-off-by: Sylwester Piskozub <sylwesterpiskozub@gmail.com>
There was a problem hiding this comment.
1 issue found across 10 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="app/controlplane/internal/usercontext/operation_authorization_middleware.go">
<violation number="1" location="app/controlplane/internal/usercontext/operation_authorization_middleware.go:96">
P1: Cache key omits `orgID`, but the authorization request includes it. If the external endpoint's decision depends on the organization, a cached "allowed" result for one org will incorrectly apply to another org for the same user and operation.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review, or fix all with cubic.
app/controlplane/internal/usercontext/operation_authorization_middleware.go
Outdated
Show resolved
Hide resolved
Signed-off-by: Sylwester Piskozub <sylwesterpiskozub@gmail.com>
| ) | ||
|
|
||
| // Operations that require authorization checks | ||
| var operationAuthTargets = map[string]bool{ |
There was a problem hiding this comment.
woudl it be possible to add this property to authz.go? As in an extension of the current system?
| return nil, fmt.Errorf("marshaling request: %w", err) | ||
| } | ||
|
|
||
| httpReq, err := http.NewRequestWithContext(ctx, http.MethodPost, url, bytes.NewReader(body)) |
There was a problem hiding this comment.
we might want to forward the API token as a header
There was a problem hiding this comment.
That should be the way. @Piskoo check how we do with policy providers. We use plain Bearer header so that providers can authenticate it automatically using standard middlewares.
| orgID = org.ID | ||
| } | ||
|
|
||
| cacheKey := fmt.Sprintf("%s:%s:%s", user.ID, orgID, operation) |
There was a problem hiding this comment.
Not sure the kind of operations we want to authorize this way, but I think we should add the parameters here. Most RBAC operations are done against specific resources.
| return nil, fmt.Errorf("marshaling request: %w", err) | ||
| } | ||
|
|
||
| httpReq, err := http.NewRequestWithContext(ctx, http.MethodPost, url, bytes.NewReader(body)) |
There was a problem hiding this comment.
That should be the way. @Piskoo check how we do with policy providers. We use plain Bearer header so that providers can authenticate it automatically using standard middlewares.
Summary
Added a middleware that forwards operations to an external authorization endpoint before allowing them to proceed. Disabled by default