Skip to content

refactor(controlplane): remove regex support in authz middleware #3246

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
migmartri merged 3 commits into from
Jun 26, 2026
+18 −84
Merged

refactor(controlplane): remove regex support in authz middleware #3246

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c26d7e9
refactor(controlplane): remove regex support in authz middleware
jiparis Jun 26, 2026
5720b7f
Merge remote-tracking branch 'origin' into jiparis/remove-middleware-…
jiparis Jun 26, 2026
1c67b42
revert: "fix(authz): close DeleteMembership authorization bypass (#32…
jiparis Jun 26, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 6 additions & 12 deletions app/controlplane/pkg/authz/authz.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,10 +160,8 @@ var (
PolicyOrganizationCreate = &Policy{Organization, ActionCreate}
PolicyOrganizationDelete = &Policy{Organization, ActionDelete}
// User Membership
PolicyOrganizationRead = &Policy{Organization, ActionRead}
PolicyOrganizationListMemberships = &Policy{OrganizationMemberships, ActionList}
PolicyOrganizationMembershipsDelete = &Policy{OrganizationMemberships, ActionDelete}
PolicyOrganizationMembershipsUpdate = &Policy{OrganizationMemberships, ActionUpdate}
PolicyOrganizationRead = &Policy{Organization, ActionRead}
PolicyOrganizationListMemberships = &Policy{OrganizationMemberships, ActionList}

// Group Memberships
PolicyGroupListPendingInvitations = &Policy{ResourceGroup, ActionList}
Expand Down Expand Up @@ -205,8 +203,6 @@ var RolesMap = map[Role][]*Policy{
RoleOwner: {
PolicyOrganizationDelete,
PolicyOrganizationManageOwners,
PolicyOrganizationMembershipsDelete,
PolicyOrganizationMembershipsUpdate,
},
// RoleAdmin is an org-scoped role that provides super admin privileges (it's the higher role)
RoleAdmin: {
Expand All @@ -217,8 +213,6 @@ var RolesMap = map[Role][]*Policy{
PolicyOrganizationInvitationsCreate,
// Being able to read from the default backend
PolicyDefaultBackendArtifactRead,
PolicyOrganizationMembershipsDelete,
PolicyOrganizationMembershipsUpdate,
// + all the policies from the viewer role inherited automatically
},
// RoleViewer is an org-scoped role that provides read-only access to all resources
Expand Down Expand Up @@ -391,7 +385,9 @@ var ServerOperationsMap = map[string]*OperationPolicy{
"/controlplane.v1.IntegrationsService/Attach": {Policies: []*Policy{PolicyAttachedIntegrationAttach}},
"/controlplane.v1.IntegrationsService/Detach": {Policies: []*Policy{PolicyAttachedIntegrationDetach}},
// Metrics
"/controlplane.v1.OrgMetricsService/.*": {Policies: []*Policy{PolicyOrgMetricsRead}},
"/controlplane.v1.OrgMetricsService/Totals": {Policies: []*Policy{PolicyOrgMetricsRead}},
"/controlplane.v1.OrgMetricsService/TopWorkflowsByRunsCount": {Policies: []*Policy{PolicyOrgMetricsRead}},
Comment thread
jiparis marked this conversation as resolved.
"/controlplane.v1.OrgMetricsService/DailyRunsCount": {Policies: []*Policy{PolicyOrgMetricsRead}},
// Robot Account
"/controlplane.v1.RobotAccountService/List": {Policies: []*Policy{PolicyRobotAccountList}},
"/controlplane.v1.RobotAccountService/Create": {Policies: []*Policy{PolicyRobotAccountCreate}},
Expand Down Expand Up @@ -423,9 +419,7 @@ var ServerOperationsMap = map[string]*OperationPolicy{
"/controlplane.v1.OrganizationService/Delete": {},

// List global memberships
"/controlplane.v1.OrganizationService/ListMemberships": {Policies: []*Policy{PolicyOrganizationListMemberships}},
"/controlplane.v1.OrganizationService/DeleteMembership": {Policies: []*Policy{PolicyOrganizationMembershipsDelete}},
"/controlplane.v1.OrganizationService/UpdateMembership": {Policies: []*Policy{PolicyOrganizationMembershipsUpdate}},
"/controlplane.v1.OrganizationService/ListMemberships": {Policies: []*Policy{PolicyOrganizationListMemberships}},

// NOTE: this is about listing my own memberships, not about listing all the memberships in the organization
"/controlplane.v1.UserService/ListMemberships": {},
Expand Down
17 changes: 2 additions & 15 deletions app/controlplane/pkg/authz/middleware/middleware.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ package middleware
import (
"context"
"errors"
"regexp"

errorsAPI "github.com/go-kratos/kratos/v2/errors"

Expand Down Expand Up @@ -103,25 +102,13 @@ func checkPolicies(ctx context.Context, subject, apiOperation string, enforcer E
return nil
}

// policiesLookup returns the policies required for a given API operation
// it performs a two run lookup
// 1 - It checks if there is an entry in the map
// 2 - if there is not, it runs a regex match in each key in case one of those keys contains a regex
// policiesLookup returns the policies required for a given API operation.
// It looks up the operation directly in the ServerOperationsMap.
func policiesLookup(apiOperation string) ([]*authz.Policy, error) {
// Direct match
entry, found := authz.ServerOperationsMap[apiOperation]
if found {
return entry.Policies, nil
}

// second pass trying to match a regex
// i.e "/controlplane.v1.OrgMetricsService/.*" -> "/controlplane.v1.OrgMetricsService/Totals"
for k, entry := range authz.ServerOperationsMap {
found, _ := regexp.MatchString(k, apiOperation)
if found {
return entry.Policies, nil
}
}

return nil, errors.New("operation not allowed")
}
67 changes: 10 additions & 57 deletions app/controlplane/pkg/authz/middleware/middleware_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -193,35 +193,32 @@ func TestPoliciesLookup(t *testing.T) {
operation: "/controlplane.v1.WorkflowContractService/List",
},
{
name: "operation found with regexp",
operation: "/controlplane.v1.OrgMetricsService/List",
name: "org metrics operation found",
operation: "/controlplane.v1.OrgMetricsService/Totals",
},
{
name: "operation found with regexp 2",
name: "org metrics operation found 2",
operation: "/controlplane.v1.OrgMetricsService/DailyRunsCount",
},
{
name: "non-existing org metrics operation, error not found",
operation: "/controlplane.v1.OrgMetricsService/boom",
wantErr: true,
},
{
name: "operation found with regexp, error wrong prefix",
name: "operation error wrong prefix",
operation: "/boom/controlplane.v1.OrgMetricsService",
wantErr: true,
},
{
name: "operation found with regexp, error not found",
name: "operation error not found",
operation: "/controlplane.v1.OrgMetricsService",
wantErr: true,
},
{
name: "contract apply operation found",
operation: "/controlplane.v1.WorkflowContractService/Apply",
},
{
name: "organization delete membership operation found",
operation: "/controlplane.v1.OrganizationService/DeleteMembership",
},
{
name: "organization update membership operation found",
operation: "/controlplane.v1.OrganizationService/UpdateMembership",
},
}

for _, tc := range testCases {
Expand All @@ -242,47 +239,3 @@ func TestPoliciesLookupContractApply(t *testing.T) {
assert.NoError(t, err)
assert.Equal(t, []*authz.Policy{authz.PolicyWorkflowContractCreate, authz.PolicyWorkflowContractUpdate}, policies)
}

func TestPoliciesLookupDeleteMembership(t *testing.T) {
policies, err := policiesLookup("/controlplane.v1.OrganizationService/DeleteMembership")
assert.NoError(t, err)
assert.Equal(t, []*authz.Policy{authz.PolicyOrganizationMembershipsDelete}, policies)
}

func TestPoliciesLookupUpdateMembership(t *testing.T) {
policies, err := policiesLookup("/controlplane.v1.OrganizationService/UpdateMembership")
assert.NoError(t, err)
assert.Equal(t, []*authz.Policy{authz.PolicyOrganizationMembershipsUpdate}, policies)
}

func TestViewerDeniedDeleteMembership(t *testing.T) {
logger := log.NewHelper(log.NewStdLogger(io.Discard))

ctx := context.Background()
ctx = usercontext.WithAuthzSubject(ctx, string(authz.RoleViewer))
ctx = transport.NewServerContext(ctx, &mockTransport{operation: "/controlplane.v1.OrganizationService/DeleteMembership"})

e := NewMockEnforcer(t)
e.On("Enforce", mock.Anything, string(authz.RoleViewer), authz.PolicyOrganizationMembershipsDelete).Return(false, nil)

m := WithAuthzMiddleware(e, logger)
_, err := m(emptyHandler)(ctx, nil)
assert.Error(t, err)
assert.True(t, errors.IsForbidden(err))
}

func TestViewerDeniedUpdateMembership(t *testing.T) {
logger := log.NewHelper(log.NewStdLogger(io.Discard))

ctx := context.Background()
ctx = usercontext.WithAuthzSubject(ctx, string(authz.RoleViewer))
ctx = transport.NewServerContext(ctx, &mockTransport{operation: "/controlplane.v1.OrganizationService/UpdateMembership"})

e := NewMockEnforcer(t)
e.On("Enforce", mock.Anything, string(authz.RoleViewer), authz.PolicyOrganizationMembershipsUpdate).Return(false, nil)

m := WithAuthzMiddleware(e, logger)
_, err := m(emptyHandler)(ctx, nil)
assert.Error(t, err)
assert.True(t, errors.IsForbidden(err))
}
Loading