Skip to content

feat: add operation authorization forward middleware#3021

Draft
Piskoo wants to merge 3 commits intochainloop-dev:mainfrom
Piskoo:pfm-5407
Draft

feat: add operation authorization forward middleware#3021
Piskoo wants to merge 3 commits intochainloop-dev:mainfrom
Piskoo:pfm-5407

Conversation

@Piskoo
Copy link
Copy Markdown
Collaborator

@Piskoo Piskoo commented Apr 10, 2026

Summary

Added a middleware that forwards operations to an external authorization endpoint before allowing them to proceed. Disabled by default

Piskoo added 2 commits April 10, 2026 09:49
Signed-off-by: Sylwester Piskozub <sylwesterpiskozub@gmail.com>
Signed-off-by: Sylwester Piskozub <sylwesterpiskozub@gmail.com>
@Piskoo Piskoo marked this pull request as ready for review April 10, 2026 08:54
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Signed-off-by: Sylwester Piskozub <sylwesterpiskozub@gmail.com>
)

// Operations that require authorization checks
var operationAuthTargets = map[string]bool{
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

woudl it be possible to add this property to authz.go? As in an extension of the current system?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, that would be great

return nil, fmt.Errorf("marshaling request: %w", err)
}

httpReq, err := http.NewRequestWithContext(ctx, http.MethodPost, url, bytes.NewReader(body))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we might want to forward the API token as a header

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@Piskoo Piskoo marked this pull request as draft April 10, 2026 14:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants