From 8aafddaad0ddd852c539440e2b7318dfd0f2b958 Mon Sep 17 00:00:00 2001 From: Miguel Martinez Date: Sat, 15 Nov 2025 23:26:22 +0100 Subject: [PATCH 1/5] fix enforcer Signed-off-by: Miguel Martinez --- app/controlplane/pkg/authz/enforcer.go | 2 +- .../pkg/authz/middleware/middleware.go | 4 +- .../pkg/authz/middleware/middleware_test.go | 2 +- .../pkg/authz/middleware/mocks_test.go | 38 +++++----- app/controlplane/pkg/biz/authz.go | 71 +++++++++++++++++++ app/controlplane/pkg/biz/biz.go | 1 + 6 files changed, 92 insertions(+), 26 deletions(-) create mode 100644 app/controlplane/pkg/biz/authz.go diff --git a/app/controlplane/pkg/authz/enforcer.go b/app/controlplane/pkg/authz/enforcer.go index 46968f001..7e6b5ad4d 100644 --- a/app/controlplane/pkg/authz/enforcer.go +++ b/app/controlplane/pkg/authz/enforcer.go @@ -57,7 +57,7 @@ func (e *Enforcer) Enforce(sub string, p *Policy) (bool, error) { // This is used for ACL-based authorization (e.g., API tokens) where policies are stored in the database // rather than in Casbin. Returns true if the required policy is found in the allowed list. // in the future we will use this function to check if the policy is allowed for the subject by running the enforcer with the subject -func (e *Enforcer) EnforceWithPolicies(_ string, p *Policy, allowedPolicies []*Policy) (bool, error) { +func (e *Enforcer) EnforceWithPolicies(p *Policy, allowedPolicies []*Policy) (bool, error) { for _, allowed := range allowedPolicies { if allowed.Resource == p.Resource && allowed.Action == p.Action { return true, nil diff --git a/app/controlplane/pkg/authz/middleware/middleware.go b/app/controlplane/pkg/authz/middleware/middleware.go index b2d31c021..27ea11d21 100644 --- a/app/controlplane/pkg/authz/middleware/middleware.go +++ b/app/controlplane/pkg/authz/middleware/middleware.go @@ -33,7 +33,7 @@ import ( type Enforcer interface { Enforce(sub string, p *authz.Policy) (bool, error) - EnforceWithPolicies(sub string, p *authz.Policy, allowedPolicies []*authz.Policy) (bool, error) + EnforceWithPolicies(p *authz.Policy, allowedPolicies []*authz.Policy) (bool, error) } // Check Authorization for the current API operation against the current user/token @@ -92,7 +92,7 @@ func checkPolicies(ctx context.Context, subject, apiOperation string, enforcer E } for _, p := range policies { - ok, err := enforcer.EnforceWithPolicies(subject, p, token.Policies) + ok, err := enforcer.EnforceWithPolicies(p, token.Policies) if err != nil { return errorsAPI.InternalServer("internal error", err.Error()) } diff --git a/app/controlplane/pkg/authz/middleware/middleware_test.go b/app/controlplane/pkg/authz/middleware/middleware_test.go index e4e7a90b7..bfc4d8e03 100644 --- a/app/controlplane/pkg/authz/middleware/middleware_test.go +++ b/app/controlplane/pkg/authz/middleware/middleware_test.go @@ -159,7 +159,7 @@ func TestWithAuthMiddleware(t *testing.T) { e := NewMockEnforcer(t) e.On("Enforce", subject, mock.Anything).Maybe().Return(tc.hasPermissions, nil) - e.On("EnforceWithPolicies", subject, mock.Anything, mock.Anything).Maybe().Return(tc.hasPermissions, nil) + e.On("EnforceWithPolicies", mock.Anything, mock.Anything).Maybe().Return(tc.hasPermissions, nil) m := WithAuthzMiddleware(e, logger) _, err := m(emptyHandler)(ctx, nil) diff --git a/app/controlplane/pkg/authz/middleware/mocks_test.go b/app/controlplane/pkg/authz/middleware/mocks_test.go index 5ff102e86..eb54326bb 100644 --- a/app/controlplane/pkg/authz/middleware/mocks_test.go +++ b/app/controlplane/pkg/authz/middleware/mocks_test.go @@ -103,8 +103,8 @@ func (_c *MockEnforcer_Enforce_Call) RunAndReturn(run func(sub string, p *authz. } // EnforceWithPolicies provides a mock function for the type MockEnforcer -func (_mock *MockEnforcer) EnforceWithPolicies(sub string, p *authz.Policy, allowedPolicies []*authz.Policy) (bool, error) { - ret := _mock.Called(sub, p, allowedPolicies) +func (_mock *MockEnforcer) EnforceWithPolicies(p *authz.Policy, allowedPolicies []*authz.Policy) (bool, error) { + ret := _mock.Called(p, allowedPolicies) if len(ret) == 0 { panic("no return value specified for EnforceWithPolicies") @@ -112,16 +112,16 @@ func (_mock *MockEnforcer) EnforceWithPolicies(sub string, p *authz.Policy, allo var r0 bool var r1 error - if returnFunc, ok := ret.Get(0).(func(string, *authz.Policy, []*authz.Policy) (bool, error)); ok { - return returnFunc(sub, p, allowedPolicies) + if returnFunc, ok := ret.Get(0).(func(*authz.Policy, []*authz.Policy) (bool, error)); ok { + return returnFunc(p, allowedPolicies) } - if returnFunc, ok := ret.Get(0).(func(string, *authz.Policy, []*authz.Policy) bool); ok { - r0 = returnFunc(sub, p, allowedPolicies) + if returnFunc, ok := ret.Get(0).(func(*authz.Policy, []*authz.Policy) bool); ok { + r0 = returnFunc(p, allowedPolicies) } else { r0 = ret.Get(0).(bool) } - if returnFunc, ok := ret.Get(1).(func(string, *authz.Policy, []*authz.Policy) error); ok { - r1 = returnFunc(sub, p, allowedPolicies) + if returnFunc, ok := ret.Get(1).(func(*authz.Policy, []*authz.Policy) error); ok { + r1 = returnFunc(p, allowedPolicies) } else { r1 = ret.Error(1) } @@ -134,31 +134,25 @@ type MockEnforcer_EnforceWithPolicies_Call struct { } // EnforceWithPolicies is a helper method to define mock.On call -// - sub string // - p *authz.Policy // - allowedPolicies []*authz.Policy -func (_e *MockEnforcer_Expecter) EnforceWithPolicies(sub interface{}, p interface{}, allowedPolicies interface{}) *MockEnforcer_EnforceWithPolicies_Call { - return &MockEnforcer_EnforceWithPolicies_Call{Call: _e.mock.On("EnforceWithPolicies", sub, p, allowedPolicies)} +func (_e *MockEnforcer_Expecter) EnforceWithPolicies(p interface{}, allowedPolicies interface{}) *MockEnforcer_EnforceWithPolicies_Call { + return &MockEnforcer_EnforceWithPolicies_Call{Call: _e.mock.On("EnforceWithPolicies", p, allowedPolicies)} } -func (_c *MockEnforcer_EnforceWithPolicies_Call) Run(run func(sub string, p *authz.Policy, allowedPolicies []*authz.Policy)) *MockEnforcer_EnforceWithPolicies_Call { +func (_c *MockEnforcer_EnforceWithPolicies_Call) Run(run func(p *authz.Policy, allowedPolicies []*authz.Policy)) *MockEnforcer_EnforceWithPolicies_Call { _c.Call.Run(func(args mock.Arguments) { - var arg0 string + var arg0 *authz.Policy if args[0] != nil { - arg0 = args[0].(string) + arg0 = args[0].(*authz.Policy) } - var arg1 *authz.Policy + var arg1 []*authz.Policy if args[1] != nil { - arg1 = args[1].(*authz.Policy) - } - var arg2 []*authz.Policy - if args[2] != nil { - arg2 = args[2].([]*authz.Policy) + arg1 = args[1].([]*authz.Policy) } run( arg0, arg1, - arg2, ) }) return _c @@ -169,7 +163,7 @@ func (_c *MockEnforcer_EnforceWithPolicies_Call) Return(b bool, err error) *Mock return _c } -func (_c *MockEnforcer_EnforceWithPolicies_Call) RunAndReturn(run func(sub string, p *authz.Policy, allowedPolicies []*authz.Policy) (bool, error)) *MockEnforcer_EnforceWithPolicies_Call { +func (_c *MockEnforcer_EnforceWithPolicies_Call) RunAndReturn(run func(p *authz.Policy, allowedPolicies []*authz.Policy) (bool, error)) *MockEnforcer_EnforceWithPolicies_Call { _c.Call.Return(run) return _c } diff --git a/app/controlplane/pkg/biz/authz.go b/app/controlplane/pkg/biz/authz.go new file mode 100644 index 000000000..06d003687 --- /dev/null +++ b/app/controlplane/pkg/biz/authz.go @@ -0,0 +1,71 @@ +// +// Copyright 2025 The Chainloop Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package biz + +import ( + "context" + "strings" + + "github.com/chainloop-dev/chainloop/app/controlplane/pkg/authz" + "github.com/go-kratos/kratos/v2/log" + "github.com/google/uuid" +) + +type AuthzUseCase struct { + log *log.Helper + *authz.Enforcer + apiTokenRepo APITokenRepo +} + +func NewAuthzUseCase(enforcer *authz.Enforcer, apiTokenRepo APITokenRepo, logger log.Logger) *AuthzUseCase { + return &AuthzUseCase{ + log: log.NewHelper(log.With(logger, "component", "biz/authz")), + apiTokenRepo: apiTokenRepo, + Enforcer: enforcer, + } +} + +// Wrapper around the Enforcer.Enforce method that takes into account some of our nuances +// with regards to policies retrieval and handling for API tokens. +func (e *AuthzUseCase) Enforce(ctx context.Context, sub string, p *authz.Policy) (bool, error) { + // Check if this is an API token (subject starts with "api-token:") + if strings.HasPrefix(sub, "api-token:") { + // load the token using the ID that's the second part of the subject + tokenID := strings.Split(sub, ":")[1] + tokenIDUUID, err := uuid.Parse(tokenID) + if err != nil { + return false, err + } + + token, err := e.apiTokenRepo.FindByID(ctx, tokenIDUUID) + if err != nil { + return false, err + } + + if token == nil { + return false, NewErrNotFound("API token") + } + + // For API tokens, use ACL-based enforcement with token's policies + ok, err := e.Enforcer.EnforceWithPolicies(p, token.Policies) + if err != nil { + return false, err + } + + return ok, nil + } + return e.Enforcer.Enforce(sub, p) +} diff --git a/app/controlplane/pkg/biz/biz.go b/app/controlplane/pkg/biz/biz.go index 81c51b54a..f8484b2ad 100644 --- a/app/controlplane/pkg/biz/biz.go +++ b/app/controlplane/pkg/biz/biz.go @@ -57,6 +57,7 @@ var ProviderSet = wire.NewSet( NewUserAccessSyncerUseCase, NewGroupUseCase, NewCASBackendChecker, + NewAuthzUseCase, wire.Bind(new(PromObservable), new(*PrometheusUseCase)), wire.Struct(new(NewIntegrationUseCaseOpts), "*"), wire.Struct(new(NewUserUseCaseParams), "*"), From 52731711717a0805e8cbd8cf8e7db015ebb1a247 Mon Sep 17 00:00:00 2001 From: Miguel Martinez Date: Sat, 15 Nov 2025 23:34:52 +0100 Subject: [PATCH 2/5] fix enforcer Signed-off-by: Miguel Martinez --- app/controlplane/cmd/wire_gen.go | 16 +-- app/controlplane/internal/server/grpc.go | 5 +- .../pkg/authz/middleware/middleware.go | 44 ++------ .../pkg/authz/middleware/mocks_test.go | 106 ++++-------------- 4 files changed, 44 insertions(+), 127 deletions(-) diff --git a/app/controlplane/cmd/wire_gen.go b/app/controlplane/cmd/wire_gen.go index 71c4808ba..addf98c16 100644 --- a/app/controlplane/cmd/wire_gen.go +++ b/app/controlplane/cmd/wire_gen.go @@ -30,6 +30,11 @@ import ( // Injectors from wire.go: func wireApp(bootstrap *conf.Bootstrap, readerWriter credentials.ReaderWriter, logger log.Logger, availablePlugins sdk.AvailablePlugins) (*app, func(), error) { + config := authzConfig(bootstrap) + enforcer, err := authz.NewInMemoryEnforcer(config) + if err != nil { + return nil, nil, err + } confData := bootstrap.Data data_Database := confData.Database databaseConfig := newDataConf(data_Database) @@ -37,6 +42,8 @@ func wireApp(bootstrap *conf.Bootstrap, readerWriter credentials.ReaderWriter, l if err != nil { return nil, nil, err } + apiTokenRepo := data.NewAPITokenRepo(dataData, logger) + authzUseCase := biz.NewAuthzUseCase(enforcer, apiTokenRepo, logger) userRepo := data.NewUserRepo(dataData, logger) groupRepo := data.NewGroupRepo(dataData, logger) membershipRepo := data.NewMembershipRepo(dataData, groupRepo, logger) @@ -110,14 +117,7 @@ func wireApp(bootstrap *conf.Bootstrap, readerWriter credentials.ReaderWriter, l cleanup() return nil, nil, err } - apiTokenRepo := data.NewAPITokenRepo(dataData, logger) apiTokenJWTConfig := newJWTConfig(auth) - config := authzConfig(bootstrap) - enforcer, err := authz.NewInMemoryEnforcer(config) - if err != nil { - cleanup() - return nil, nil, err - } apiTokenUseCase, err := biz.NewAPITokenUseCase(apiTokenRepo, apiTokenJWTConfig, enforcer, organizationUseCase, auditorUseCase, logger) if err != nil { cleanup() @@ -244,6 +244,7 @@ func wireApp(bootstrap *conf.Bootstrap, readerWriter credentials.ReaderWriter, l return nil, nil, err } opts := &server.Opts{ + AuthzUseCase: authzUseCase, UserUseCase: userUseCase, RobotAccountUseCase: robotAccountUseCase, CASBackendUseCase: casBackendUseCase, @@ -282,7 +283,6 @@ func wireApp(bootstrap *conf.Bootstrap, readerWriter credentials.ReaderWriter, l FederatedConfig: federatedAuthentication, BootstrapConfig: bootstrap, Credentials: readerWriter, - Enforcer: enforcer, Validator: validator, } grpcServer, err := server.NewGRPCServer(opts) diff --git a/app/controlplane/internal/server/grpc.go b/app/controlplane/internal/server/grpc.go index bec5c35ba..2a830b2dc 100644 --- a/app/controlplane/internal/server/grpc.go +++ b/app/controlplane/internal/server/grpc.go @@ -25,7 +25,6 @@ import ( conf "github.com/chainloop-dev/chainloop/app/controlplane/internal/conf/controlplane/config/v1" "github.com/chainloop-dev/chainloop/app/controlplane/internal/sentrycontext" "github.com/chainloop-dev/chainloop/app/controlplane/internal/usercontext/attjwtmiddleware" - "github.com/chainloop-dev/chainloop/app/controlplane/pkg/authz" authzMiddleware "github.com/chainloop-dev/chainloop/app/controlplane/pkg/authz/middleware" "github.com/chainloop-dev/chainloop/app/controlplane/pkg/biz" "github.com/chainloop-dev/chainloop/app/controlplane/pkg/jwt/user" @@ -51,6 +50,7 @@ import ( type Opts struct { // UseCases + AuthzUseCase *biz.AuthzUseCase UserUseCase *biz.UserUseCase RobotAccountUseCase *biz.RobotAccountUseCase CASBackendUseCase *biz.CASBackendUseCase @@ -91,7 +91,6 @@ type Opts struct { FederatedConfig *conf.FederatedAuthentication BootstrapConfig *conf.Bootstrap Credentials credentials.ReaderWriter - Enforcer *authz.Enforcer Validator *protovalidate.Validator } @@ -198,7 +197,7 @@ func craftMiddleware(opts *Opts) []middleware.Middleware { // 2.d- Set its organization usercontext.WithCurrentOrganizationMiddleware(opts.UserUseCase, logHelper), // 3 - Check user/token authorization - authzMiddleware.WithAuthzMiddleware(opts.Enforcer, logHelper), + authzMiddleware.WithAuthzMiddleware(opts.AuthzUseCase, logHelper), ).Match(requireAllButOrganizationOperationsMatcher()).Build(), // Store all memberships in the context usercontext.WithCurrentMembershipsMiddleware(opts.MembershipUseCase), diff --git a/app/controlplane/pkg/authz/middleware/middleware.go b/app/controlplane/pkg/authz/middleware/middleware.go index 27ea11d21..6a3e9fcdb 100644 --- a/app/controlplane/pkg/authz/middleware/middleware.go +++ b/app/controlplane/pkg/authz/middleware/middleware.go @@ -1,5 +1,5 @@ // -// Copyright 2024 The Chainloop Authors. +// Copyright 2024-2025 The Chainloop Authors. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -19,12 +19,10 @@ import ( "context" "errors" "regexp" - "strings" errorsAPI "github.com/go-kratos/kratos/v2/errors" "github.com/chainloop-dev/chainloop/app/controlplane/internal/usercontext" - "github.com/chainloop-dev/chainloop/app/controlplane/internal/usercontext/entities" "github.com/chainloop-dev/chainloop/app/controlplane/pkg/authz" "github.com/go-kratos/kratos/v2/log" "github.com/go-kratos/kratos/v2/middleware" @@ -32,8 +30,7 @@ import ( ) type Enforcer interface { - Enforce(sub string, p *authz.Policy) (bool, error) - EnforceWithPolicies(p *authz.Policy, allowedPolicies []*authz.Policy) (bool, error) + Enforce(ctx context.Context, sub string, p *authz.Policy) (bool, error) } // Check Authorization for the current API operation against the current user/token @@ -83,37 +80,16 @@ func checkPolicies(ctx context.Context, subject, apiOperation string, enforcer E return errorsAPI.Forbidden("forbidden", err.Error()) } - // Check if this is an API token (subject starts with "api-token:") - if strings.HasPrefix(subject, "api-token:") { - // For API tokens, use ACL-based enforcement with token's policies - token := entities.CurrentAPIToken(ctx) - if token == nil { - return errorsAPI.InternalServer("internal error", "API token not found in context") + // For users, use role-based enforcement via Casbin + for _, p := range policies { + ok, err := enforcer.Enforce(ctx, subject, p) + if err != nil { + return errorsAPI.InternalServer("internal error", err.Error()) } - for _, p := range policies { - ok, err := enforcer.EnforceWithPolicies(p, token.Policies) - if err != nil { - return errorsAPI.InternalServer("internal error", err.Error()) - } - - if !ok { - logger.Infow("msg", "[authZ] policy not found in token policies", "sub", subject, "operation", apiOperation, "resource", p.Resource, "action", p.Action) - return errorsAPI.Forbidden("forbidden", "operation not allowed") - } - } - } else { - // For users, use role-based enforcement via Casbin - for _, p := range policies { - ok, err := enforcer.Enforce(subject, p) - if err != nil { - return errorsAPI.InternalServer("internal error", err.Error()) - } - - if !ok { - logger.Infow("msg", "[authZ] policy not found", "sub", subject, "operation", apiOperation, "resource", p.Resource, "action", p.Action) - return errorsAPI.Forbidden("forbidden", "operation not allowed") - } + if !ok { + logger.Infow("msg", "[authZ] policy not found", "sub", subject, "operation", apiOperation, "resource", p.Resource, "action", p.Action) + return errorsAPI.Forbidden("forbidden", "operation not allowed") } } diff --git a/app/controlplane/pkg/authz/middleware/mocks_test.go b/app/controlplane/pkg/authz/middleware/mocks_test.go index eb54326bb..84b2ccb54 100644 --- a/app/controlplane/pkg/authz/middleware/mocks_test.go +++ b/app/controlplane/pkg/authz/middleware/mocks_test.go @@ -5,6 +5,8 @@ package middleware import ( + "context" + "github.com/chainloop-dev/chainloop/app/controlplane/pkg/authz" mock "github.com/stretchr/testify/mock" ) @@ -37,8 +39,8 @@ func (_m *MockEnforcer) EXPECT() *MockEnforcer_Expecter { } // Enforce provides a mock function for the type MockEnforcer -func (_mock *MockEnforcer) Enforce(sub string, p *authz.Policy) (bool, error) { - ret := _mock.Called(sub, p) +func (_mock *MockEnforcer) Enforce(ctx context.Context, sub string, p *authz.Policy) (bool, error) { + ret := _mock.Called(ctx, sub, p) if len(ret) == 0 { panic("no return value specified for Enforce") @@ -46,16 +48,16 @@ func (_mock *MockEnforcer) Enforce(sub string, p *authz.Policy) (bool, error) { var r0 bool var r1 error - if returnFunc, ok := ret.Get(0).(func(string, *authz.Policy) (bool, error)); ok { - return returnFunc(sub, p) + if returnFunc, ok := ret.Get(0).(func(context.Context, string, *authz.Policy) (bool, error)); ok { + return returnFunc(ctx, sub, p) } - if returnFunc, ok := ret.Get(0).(func(string, *authz.Policy) bool); ok { - r0 = returnFunc(sub, p) + if returnFunc, ok := ret.Get(0).(func(context.Context, string, *authz.Policy) bool); ok { + r0 = returnFunc(ctx, sub, p) } else { r0 = ret.Get(0).(bool) } - if returnFunc, ok := ret.Get(1).(func(string, *authz.Policy) error); ok { - r1 = returnFunc(sub, p) + if returnFunc, ok := ret.Get(1).(func(context.Context, string, *authz.Policy) error); ok { + r1 = returnFunc(ctx, sub, p) } else { r1 = ret.Error(1) } @@ -68,102 +70,42 @@ type MockEnforcer_Enforce_Call struct { } // Enforce is a helper method to define mock.On call +// - ctx context.Context // - sub string // - p *authz.Policy -func (_e *MockEnforcer_Expecter) Enforce(sub interface{}, p interface{}) *MockEnforcer_Enforce_Call { - return &MockEnforcer_Enforce_Call{Call: _e.mock.On("Enforce", sub, p)} +func (_e *MockEnforcer_Expecter) Enforce(ctx interface{}, sub interface{}, p interface{}) *MockEnforcer_Enforce_Call { + return &MockEnforcer_Enforce_Call{Call: _e.mock.On("Enforce", ctx, sub, p)} } -func (_c *MockEnforcer_Enforce_Call) Run(run func(sub string, p *authz.Policy)) *MockEnforcer_Enforce_Call { +func (_c *MockEnforcer_Enforce_Call) Run(run func(ctx context.Context, sub string, p *authz.Policy)) *MockEnforcer_Enforce_Call { _c.Call.Run(func(args mock.Arguments) { - var arg0 string + var arg0 context.Context if args[0] != nil { - arg0 = args[0].(string) + arg0 = args[0].(context.Context) } - var arg1 *authz.Policy + var arg1 string if args[1] != nil { - arg1 = args[1].(*authz.Policy) + arg1 = args[1].(string) } - run( - arg0, - arg1, - ) - }) - return _c -} - -func (_c *MockEnforcer_Enforce_Call) Return(b bool, err error) *MockEnforcer_Enforce_Call { - _c.Call.Return(b, err) - return _c -} - -func (_c *MockEnforcer_Enforce_Call) RunAndReturn(run func(sub string, p *authz.Policy) (bool, error)) *MockEnforcer_Enforce_Call { - _c.Call.Return(run) - return _c -} - -// EnforceWithPolicies provides a mock function for the type MockEnforcer -func (_mock *MockEnforcer) EnforceWithPolicies(p *authz.Policy, allowedPolicies []*authz.Policy) (bool, error) { - ret := _mock.Called(p, allowedPolicies) - - if len(ret) == 0 { - panic("no return value specified for EnforceWithPolicies") - } - - var r0 bool - var r1 error - if returnFunc, ok := ret.Get(0).(func(*authz.Policy, []*authz.Policy) (bool, error)); ok { - return returnFunc(p, allowedPolicies) - } - if returnFunc, ok := ret.Get(0).(func(*authz.Policy, []*authz.Policy) bool); ok { - r0 = returnFunc(p, allowedPolicies) - } else { - r0 = ret.Get(0).(bool) - } - if returnFunc, ok := ret.Get(1).(func(*authz.Policy, []*authz.Policy) error); ok { - r1 = returnFunc(p, allowedPolicies) - } else { - r1 = ret.Error(1) - } - return r0, r1 -} - -// MockEnforcer_EnforceWithPolicies_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'EnforceWithPolicies' -type MockEnforcer_EnforceWithPolicies_Call struct { - *mock.Call -} - -// EnforceWithPolicies is a helper method to define mock.On call -// - p *authz.Policy -// - allowedPolicies []*authz.Policy -func (_e *MockEnforcer_Expecter) EnforceWithPolicies(p interface{}, allowedPolicies interface{}) *MockEnforcer_EnforceWithPolicies_Call { - return &MockEnforcer_EnforceWithPolicies_Call{Call: _e.mock.On("EnforceWithPolicies", p, allowedPolicies)} -} - -func (_c *MockEnforcer_EnforceWithPolicies_Call) Run(run func(p *authz.Policy, allowedPolicies []*authz.Policy)) *MockEnforcer_EnforceWithPolicies_Call { - _c.Call.Run(func(args mock.Arguments) { - var arg0 *authz.Policy - if args[0] != nil { - arg0 = args[0].(*authz.Policy) - } - var arg1 []*authz.Policy - if args[1] != nil { - arg1 = args[1].([]*authz.Policy) + var arg2 *authz.Policy + if args[2] != nil { + arg2 = args[2].(*authz.Policy) } run( arg0, arg1, + arg2, ) }) return _c } -func (_c *MockEnforcer_EnforceWithPolicies_Call) Return(b bool, err error) *MockEnforcer_EnforceWithPolicies_Call { +func (_c *MockEnforcer_Enforce_Call) Return(b bool, err error) *MockEnforcer_Enforce_Call { _c.Call.Return(b, err) return _c } -func (_c *MockEnforcer_EnforceWithPolicies_Call) RunAndReturn(run func(p *authz.Policy, allowedPolicies []*authz.Policy) (bool, error)) *MockEnforcer_EnforceWithPolicies_Call { +func (_c *MockEnforcer_Enforce_Call) RunAndReturn(run func(ctx context.Context, sub string, p *authz.Policy) (bool, error)) *MockEnforcer_Enforce_Call { _c.Call.Return(run) return _c } From 38a013aadf0d893adc22000b3acdaeca1b776a38 Mon Sep 17 00:00:00 2001 From: Miguel Martinez Date: Sun, 16 Nov 2025 00:22:19 +0100 Subject: [PATCH 3/5] fix enforcer Signed-off-by: Miguel Martinez --- CLAUDE.md | 3 +- app/controlplane/cmd/wire.go | 20 +++-- app/controlplane/cmd/wire_gen.go | 34 +++++--- .../internal/service/cascredential.go | 10 +-- app/controlplane/internal/service/group.go | 2 +- .../internal/service/organization.go | 4 +- app/controlplane/internal/service/service.go | 10 +-- app/controlplane/pkg/authz/authz_test.go | 10 +-- app/controlplane/pkg/authz/enforcer.go | 86 ++++--------------- .../pkg/authz/middleware/middleware.go | 6 +- .../pkg/authz/middleware/middleware_test.go | 3 +- app/controlplane/pkg/biz/apitoken.go | 6 +- app/controlplane/pkg/biz/authz.go | 41 ++++++--- app/controlplane/pkg/biz/group.go | 10 +-- app/controlplane/pkg/biz/project.go | 12 +-- app/controlplane/pkg/biz/testhelpers/wire.go | 14 ++- .../pkg/biz/testhelpers/wire_gen.go | 19 +++- 17 files changed, 146 insertions(+), 144 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index bd0777e60..9a4091f24 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -268,4 +268,5 @@ Code reviews are required for all submissions via GitHub pull requests. - if you add any new dependency to a constructor, remember to run wire ./... - when creating PR message, keep it high-level, what functionality was added, don't add info about testing, no icons, no info about how the message was generated. - app/controlplane/api/gen/frontend/google/protobuf/descriptor.ts is a special case that we don't want to upgrade, so if it upgrades, put it back to main -- when creating a commit or PR message, NEVER add co-authored by or generated by Claude code \ No newline at end of file +- when creating a commit or PR message, NEVER add co-authored by or generated by Claude code +- any call to authorization Enforce done from the biz or svc layer must be done using biz.AuthzUseCase \ No newline at end of file diff --git a/app/controlplane/cmd/wire.go b/app/controlplane/cmd/wire.go index fba4229fc..a52478a8a 100644 --- a/app/controlplane/cmd/wire.go +++ b/app/controlplane/cmd/wire.go @@ -53,7 +53,7 @@ func wireApp(*conf.Bootstrap, credentials.ReaderWriter, log.Logger, sdk.Availabl wire.FieldsOf(new(*conf.Bootstrap), "Server", "Auth", "Data", "CasServer", "ReferrerSharedIndex", "Onboarding", "PrometheusIntegration", "PolicyProviders", "NatsServer", "FederatedAuthentication"), wire.FieldsOf(new(*conf.Data), "Database"), dispatcher.New, - authz.NewInMemoryEnforcer, + authz.NewEnforcer, policies.NewRegistry, newApp, newProtoValidator, @@ -65,13 +65,23 @@ func wireApp(*conf.Bootstrap, credentials.ReaderWriter, log.Logger, sdk.Availabl newAuthAllowList, newJWTConfig, authzConfig, + authzUseCaseConfig, biz.NewIndexConfig, ), ) } -func authzConfig(conf *conf.Bootstrap) *authz.Config { - return &authz.Config{RolesMap: authz.RolesMap, RestrictOrgCreation: conf.RestrictOrgCreation} +func authzConfig() *authz.Config { + return &authz.Config{RolesMap: authz.RolesMap} +} + +func authzUseCaseConfig(conf *conf.Bootstrap, enforcer *authz.Enforcer, apiTokenRepo biz.APITokenRepo, logger log.Logger) *biz.AuthzUseCaseConfig { + return &biz.AuthzUseCaseConfig{ + Enforcer: enforcer, + APITokenRepo: apiTokenRepo, + RestrictOrgCreation: conf.RestrictOrgCreation, + Logger: logger, + } } func newJWTConfig(conf *conf.Auth) *biz.APITokenJWTConfig { @@ -96,10 +106,10 @@ func newPolicyProviderConfig(in []*conf.PolicyProvider) []*policies.NewRegistryC return out } -func serviceOpts(l log.Logger, enforcer *authz.Enforcer, pUC *biz.ProjectUseCase, gUC *biz.GroupUseCase) []service.NewOpt { +func serviceOpts(l log.Logger, authzUC *biz.AuthzUseCase, pUC *biz.ProjectUseCase, gUC *biz.GroupUseCase) []service.NewOpt { return []service.NewOpt{ service.WithLogger(l), - service.WithEnforcer(enforcer), + service.WithEnforcer(authzUC), service.WithProjectUseCase(pUC), service.WithGroupUseCase(gUC), } diff --git a/app/controlplane/cmd/wire_gen.go b/app/controlplane/cmd/wire_gen.go index addf98c16..d0166e823 100644 --- a/app/controlplane/cmd/wire_gen.go +++ b/app/controlplane/cmd/wire_gen.go @@ -30,8 +30,8 @@ import ( // Injectors from wire.go: func wireApp(bootstrap *conf.Bootstrap, readerWriter credentials.ReaderWriter, logger log.Logger, availablePlugins sdk.AvailablePlugins) (*app, func(), error) { - config := authzConfig(bootstrap) - enforcer, err := authz.NewInMemoryEnforcer(config) + config := authzConfig() + enforcer, err := authz.NewEnforcer(config) if err != nil { return nil, nil, err } @@ -43,7 +43,8 @@ func wireApp(bootstrap *conf.Bootstrap, readerWriter credentials.ReaderWriter, l return nil, nil, err } apiTokenRepo := data.NewAPITokenRepo(dataData, logger) - authzUseCase := biz.NewAuthzUseCase(enforcer, apiTokenRepo, logger) + bizAuthzUseCaseConfig := authzUseCaseConfig(bootstrap, enforcer, apiTokenRepo, logger) + authzUseCase := biz.NewAuthzUseCase(bizAuthzUseCaseConfig) userRepo := data.NewUserRepo(dataData, logger) groupRepo := data.NewGroupRepo(dataData, logger) membershipRepo := data.NewMembershipRepo(dataData, groupRepo, logger) @@ -118,7 +119,7 @@ func wireApp(bootstrap *conf.Bootstrap, readerWriter credentials.ReaderWriter, l return nil, nil, err } apiTokenJWTConfig := newJWTConfig(auth) - apiTokenUseCase, err := biz.NewAPITokenUseCase(apiTokenRepo, apiTokenJWTConfig, enforcer, organizationUseCase, auditorUseCase, logger) + apiTokenUseCase, err := biz.NewAPITokenUseCase(apiTokenRepo, apiTokenJWTConfig, authzUseCase, organizationUseCase, auditorUseCase, logger) if err != nil { cleanup() return nil, nil, err @@ -140,9 +141,9 @@ func wireApp(bootstrap *conf.Bootstrap, readerWriter credentials.ReaderWriter, l cleanup() return nil, nil, err } - groupUseCase := biz.NewGroupUseCase(logger, groupRepo, membershipRepo, userRepo, orgInvitationUseCase, auditorUseCase, orgInvitationRepo, enforcer, membershipUseCase) - projectUseCase := biz.NewProjectsUseCase(logger, projectsRepo, membershipRepo, auditorUseCase, groupUseCase, membershipUseCase, orgInvitationUseCase, orgInvitationRepo, enforcer) - v5 := serviceOpts(logger, enforcer, projectUseCase, groupUseCase) + groupUseCase := biz.NewGroupUseCase(logger, groupRepo, membershipRepo, userRepo, orgInvitationUseCase, auditorUseCase, orgInvitationRepo, authzUseCase, membershipUseCase) + projectUseCase := biz.NewProjectsUseCase(logger, projectsRepo, membershipRepo, auditorUseCase, groupUseCase, membershipUseCase, orgInvitationUseCase, orgInvitationRepo, authzUseCase) + v5 := serviceOpts(logger, authzUseCase, projectUseCase, groupUseCase) workflowService := service.NewWorkflowService(workflowUseCase, workflowContractUseCase, projectUseCase, v5...) confServer := bootstrap.Server authService, err := service.NewAuthService(userUseCase, organizationUseCase, membershipUseCase, orgInvitationUseCase, auth, confServer, auditorUseCase, v5...) @@ -206,7 +207,7 @@ func wireApp(bootstrap *conf.Bootstrap, readerWriter credentials.ReaderWriter, l attestationService := service.NewAttestationService(newAttestationServiceOpts) workflowContractService := service.NewWorkflowSchemaService(workflowContractUseCase, v5...) contextService := service.NewContextService(casBackendUseCase, userUseCase, v5...) - casCredentialsService := service.NewCASCredentialsService(casCredentialsUseCase, casMappingUseCase, casBackendUseCase, enforcer, v5...) + casCredentialsService := service.NewCASCredentialsService(casCredentialsUseCase, casMappingUseCase, casBackendUseCase, authzUseCase, v5...) orgMetricsService := service.NewOrgMetricsService(orgMetricsUseCase, v5...) integrationsService := service.NewIntegrationsService(integrationUseCase, workflowUseCase, availablePlugins, v5...) organizationService := service.NewOrganizationService(membershipUseCase, organizationUseCase, v5...) @@ -319,8 +320,17 @@ var ( // wire.go: -func authzConfig(conf2 *conf.Bootstrap) *authz.Config { - return &authz.Config{RolesMap: authz.RolesMap, RestrictOrgCreation: conf2.RestrictOrgCreation} +func authzConfig() *authz.Config { + return &authz.Config{RolesMap: authz.RolesMap} +} + +func authzUseCaseConfig(conf2 *conf.Bootstrap, enforcer *authz.Enforcer, apiTokenRepo biz.APITokenRepo, logger log.Logger) *biz.AuthzUseCaseConfig { + return &biz.AuthzUseCaseConfig{ + Enforcer: enforcer, + APITokenRepo: apiTokenRepo, + RestrictOrgCreation: conf2.RestrictOrgCreation, + Logger: logger, + } } func newJWTConfig(conf2 *conf.Auth) *biz.APITokenJWTConfig { @@ -345,8 +355,8 @@ func newPolicyProviderConfig(in []*conf.PolicyProvider) []*policies.NewRegistryC return out } -func serviceOpts(l log.Logger, enforcer *authz.Enforcer, pUC *biz.ProjectUseCase, gUC *biz.GroupUseCase) []service.NewOpt { - return []service.NewOpt{service.WithLogger(l), service.WithEnforcer(enforcer), service.WithProjectUseCase(pUC), service.WithGroupUseCase(gUC)} +func serviceOpts(l log.Logger, authzUC *biz.AuthzUseCase, pUC *biz.ProjectUseCase, gUC *biz.GroupUseCase) []service.NewOpt { + return []service.NewOpt{service.WithLogger(l), service.WithEnforcer(authzUC), service.WithProjectUseCase(pUC), service.WithGroupUseCase(gUC)} } func newCASServerOptions(in *conf.Bootstrap_CASServer) *biz.CASServerDefaultOpts { diff --git a/app/controlplane/internal/service/cascredential.go b/app/controlplane/internal/service/cascredential.go index f98ba77fc..5f8514724 100644 --- a/app/controlplane/internal/service/cascredential.go +++ b/app/controlplane/internal/service/cascredential.go @@ -1,5 +1,5 @@ // -// Copyright 2023 The Chainloop Authors. +// Copyright 2023-2025 The Chainloop Authors. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -34,10 +34,10 @@ type CASCredentialsService struct { casUC *biz.CASCredentialsUseCase casBackendUC *biz.CASBackendUseCase casMappingUC *biz.CASMappingUseCase - authz *authz.Enforcer + authzUC *biz.AuthzUseCase } -func NewCASCredentialsService(casUC *biz.CASCredentialsUseCase, casmUC *biz.CASMappingUseCase, casBUC *biz.CASBackendUseCase, authz *authz.Enforcer, opts ...NewOpt) *CASCredentialsService { +func NewCASCredentialsService(casUC *biz.CASCredentialsUseCase, casmUC *biz.CASMappingUseCase, casBUC *biz.CASBackendUseCase, authzUC *biz.AuthzUseCase, opts ...NewOpt) *CASCredentialsService { return &CASCredentialsService{ service: newService(opts...), casUC: casUC, @@ -45,7 +45,7 @@ func NewCASCredentialsService(casUC *biz.CASCredentialsUseCase, casmUC *biz.CASM casMappingUC: casmUC, // we use the casBackendUC to find the default upload backend casBackendUC: casBUC, - authz: authz, + authzUC: authzUC, } } @@ -79,7 +79,7 @@ func (s *CASCredentialsService) Get(ctx context.Context, req *pb.CASCredentialsS } // Enforce required role - if ok, err := s.authz.Enforce(currentAuthzSubject, policyToCheck); err != nil { + if ok, err := s.authzUC.Enforce(ctx, currentAuthzSubject, policyToCheck); err != nil { return nil, handleUseCaseErr(err, s.log) } else if !ok { return nil, errors.Forbidden("forbidden", "not allowed to perform this operation") diff --git a/app/controlplane/internal/service/group.go b/app/controlplane/internal/service/group.go index 16fa43af0..0ba45abda 100644 --- a/app/controlplane/internal/service/group.go +++ b/app/controlplane/internal/service/group.go @@ -620,7 +620,7 @@ func (g *GroupService) userHasPermissionOnGroupMembershipsWithPolicy(ctx context m := entities.CurrentMembership(ctx) for _, rm := range m.Resources { if rm.ResourceType == authz.ResourceTypeGroup && rm.ResourceID == resolvedGroupID { - pass, err := g.enforcer.Enforce(string(rm.Role), policy) + pass, err := g.authz.Enforce(ctx, string(rm.Role), policy) if err != nil { return handleUseCaseErr(err, g.log) } diff --git a/app/controlplane/internal/service/organization.go b/app/controlplane/internal/service/organization.go index e5711368c..6a6f62f03 100644 --- a/app/controlplane/internal/service/organization.go +++ b/app/controlplane/internal/service/organization.go @@ -212,7 +212,7 @@ func (s *OrganizationService) UpdateMembership(ctx context.Context, req *pb.Orga func (s *OrganizationService) canCreateOrganization(ctx context.Context) (bool, error) { // Restricted org creation is disabled, allow creation - if !s.enforcer.RestrictOrgCreation { + if !s.authz.RestrictOrgCreation { return true, nil } @@ -222,7 +222,7 @@ func (s *OrganizationService) canCreateOrganization(ctx context.Context) (bool, continue } - pass, err := s.enforcer.Enforce(string(rm.Role), authz.PolicyOrganizationCreate) + pass, err := s.authz.Enforce(ctx, string(rm.Role), authz.PolicyOrganizationCreate) if err != nil { return false, handleUseCaseErr(err, s.log) } diff --git a/app/controlplane/internal/service/service.go b/app/controlplane/internal/service/service.go index bf5f31dab..06275775d 100644 --- a/app/controlplane/internal/service/service.go +++ b/app/controlplane/internal/service/service.go @@ -133,7 +133,7 @@ func newService(opts ...NewOpt) *service { type service struct { log *log.Helper - enforcer *authz.Enforcer + authz *biz.AuthzUseCase projectUseCase *biz.ProjectUseCase groupUseCase *biz.GroupUseCase } @@ -146,9 +146,9 @@ func WithLogger(logger log.Logger) NewOpt { } } -func WithEnforcer(enforcer *authz.Enforcer) NewOpt { +func WithEnforcer(authzUC *biz.AuthzUseCase) NewOpt { return func(s *service) { - s.enforcer = enforcer + s.authz = authzUC } } @@ -231,7 +231,7 @@ func (s *service) authorizeResource(ctx context.Context, op *authz.Policy, resou // Try to enforce the policy with each matching role // If any role passes, authorize the request for _, rm := range matchingResources { - pass, err := s.enforcer.Enforce(string(rm.Role), op) + pass, err := s.authz.Enforce(ctx, string(rm.Role), op) if err != nil { return handleUseCaseErr(err, s.log) } @@ -289,7 +289,7 @@ func (s *service) userCanCreateProject(ctx context.Context) error { } orgRole := usercontext.CurrentAuthzSubject(ctx) - pass, err := s.enforcer.Enforce(orgRole, authz.PolicyProjectCreate) + pass, err := s.authz.Enforce(ctx, orgRole, authz.PolicyProjectCreate) if err != nil { return handleUseCaseErr(err, s.log) } diff --git a/app/controlplane/pkg/authz/authz_test.go b/app/controlplane/pkg/authz/authz_test.go index 0b7fef372..76b32701a 100644 --- a/app/controlplane/pkg/authz/authz_test.go +++ b/app/controlplane/pkg/authz/authz_test.go @@ -76,7 +76,7 @@ func TestDoSync(t *testing.T) { } // load custom policies - err := doSync(e, &Config{RolesMap: policiesM}) + err := syncRBACRoles(e, &Config{RolesMap: policiesM}) assert.NoError(t, err) got, err := e.GetPolicy() assert.NoError(t, err) @@ -92,7 +92,7 @@ func TestDoSync(t *testing.T) { }, } - err = doSync(e, &Config{RolesMap: policiesM}) + err = syncRBACRoles(e, &Config{RolesMap: policiesM}) assert.NoError(t, err) got, err = e.GetPolicy() assert.NoError(t, err) @@ -105,7 +105,7 @@ func TestDoSync(t *testing.T) { }, } - err = doSync(e, &Config{RolesMap: policiesM}) + err = syncRBACRoles(e, &Config{RolesMap: policiesM}) assert.NoError(t, err) got, err = e.GetPolicy() assert.NoError(t, err) @@ -117,7 +117,7 @@ func TestDoSync(t *testing.T) { PolicyAttachedIntegrationDetach, }, } - err = doSync(e, &Config{RolesMap: policiesM}) + err = syncRBACRoles(e, &Config{RolesMap: policiesM}) assert.NoError(t, err) got, err = e.GetPolicy() assert.NoError(t, err) @@ -135,7 +135,7 @@ func testEnforcer(t *testing.T) (*Enforcer, io.Closer) { require.FailNow(t, err.Error()) } - enforcer, err := NewFiletypeEnforcer(f.Name(), &Config{}) + enforcer, err := NewEnforcer(&Config{}) require.NoError(t, err) return enforcer, f } diff --git a/app/controlplane/pkg/authz/enforcer.go b/app/controlplane/pkg/authz/enforcer.go index 7e6b5ad4d..33b4ff9be 100644 --- a/app/controlplane/pkg/authz/enforcer.go +++ b/app/controlplane/pkg/authz/enforcer.go @@ -17,13 +17,13 @@ package authz import ( _ "embed" + "errors" "fmt" "slices" + "strings" "github.com/casbin/casbin/v2" "github.com/casbin/casbin/v2/model" - "github.com/casbin/casbin/v2/persist" - fileadapter "github.com/casbin/casbin/v2/persist/file-adapter" ) type SubjectAPIToken struct { @@ -38,38 +38,29 @@ func (t *SubjectAPIToken) String() string { var modelFile []byte type Config struct { - RolesMap map[Role][]*Policy - RestrictOrgCreation bool + RolesMap map[Role][]*Policy } type Enforcer struct { *casbin.Enforcer - config *Config - RestrictOrgCreation bool + config *Config } func (e *Enforcer) Enforce(sub string, p *Policy) (bool, error) { - return e.Enforcer.Enforce(sub, p.Resource, p.Action) -} - -// EnforceWithPolicies checks if the required policy exists in the provided list of allowed policies. -// This is used for ACL-based authorization (e.g., API tokens) where policies are stored in the database -// rather than in Casbin. Returns true if the required policy is found in the allowed list. -// in the future we will use this function to check if the policy is allowed for the subject by running the enforcer with the subject -func (e *Enforcer) EnforceWithPolicies(p *Policy, allowedPolicies []*Policy) (bool, error) { - for _, allowed := range allowedPolicies { - if allowed.Resource == p.Resource && allowed.Action == p.Action { - return true, nil - } + // This enforcer does not support API token subjects + // this is due to the fact that API tokens are not stored in casbin yet + // To use them, make sure you use the AuthzUseCase.Enforce method instead + if strings.HasPrefix(sub, "api-token:") { + return false, errors.New("API token subjects not supported") } - return false, nil + + return e.Enforcer.Enforce(sub, p.Resource, p.Action) } -// NewInMemoryEnforcer creates a new casbin authorization enforcer with in-memory storage. -// Only static role policies from RolesMap are loaded. API token policies are checked separately -// using EnforceWithPolicies and are not stored in Casbin. -func NewInMemoryEnforcer(config *Config) (*Enforcer, error) { +// NewEnforcer creates a new casbin authorization enforcer with in-memory storage. +// Only static role policies from RolesMap are loaded +func NewEnforcer(config *Config) (*Enforcer, error) { // load model defined in model.conf m, err := model.NewModelFromString(string(modelFile)) if err != nil { @@ -82,45 +73,7 @@ func NewInMemoryEnforcer(config *Config) (*Enforcer, error) { return nil, fmt.Errorf("failed to create enforcer: %w", err) } - e := &Enforcer{enforcer, config, config.RestrictOrgCreation} - - // Initialize the enforcer with the roles map - if err := syncRBACRoles(e, config); err != nil { - return nil, fmt.Errorf("failed to sync roles: %w", err) - } - - return e, nil -} - -// NewFileAdapter creates a new casbin authorization enforcer -// based on a CSV file as policies storage backend -func NewFiletypeEnforcer(path string, config *Config) (*Enforcer, error) { - // policy storage in filesystem - a := fileadapter.NewAdapter(path) - e, err := newEnforcer(a, config) - if err != nil { - return nil, fmt.Errorf("failed to create enforcer: %w", err) - } - - return e, nil -} - -// NewEnforcer creates a new casbin authorization enforcer for the policies stored -// in the database and the model defined in model.conf -func newEnforcer(a persist.Adapter, config *Config) (*Enforcer, error) { - // load model defined in model.conf - m, err := model.NewModelFromString(string(modelFile)) - if err != nil { - return nil, fmt.Errorf("failed to create model: %w", err) - } - - // create enforcer for authorization - enforcer, err := casbin.NewEnforcer(m, a) - if err != nil { - return nil, fmt.Errorf("failed to create enforcer: %w", err) - } - - e := &Enforcer{enforcer, config, config.RestrictOrgCreation} + e := &Enforcer{enforcer, config} // Initialize the enforcer with the roles map if err := syncRBACRoles(e, config); err != nil { @@ -130,14 +83,7 @@ func newEnforcer(a persist.Adapter, config *Config) (*Enforcer, error) { return e, nil } -// Load the roles map into the enforcer -// This is done by adding all the policies defined in the roles map -// and removing all the policies that are not -func syncRBACRoles(e *Enforcer, config *Config) error { - return doSync(e, config) -} - -func doSync(e *Enforcer, c *Config) error { +func syncRBACRoles(e *Enforcer, c *Config) error { // allow to override config during sync conf := c if conf == nil { diff --git a/app/controlplane/pkg/authz/middleware/middleware.go b/app/controlplane/pkg/authz/middleware/middleware.go index 6a3e9fcdb..f7d7d6d2c 100644 --- a/app/controlplane/pkg/authz/middleware/middleware.go +++ b/app/controlplane/pkg/authz/middleware/middleware.go @@ -58,7 +58,7 @@ func WithAuthzMiddleware(enforcer Enforcer, logger *log.Helper) middleware.Middl // so for now we skip the authorization check for admin users since they are allowed to do anything // TODO: fill out the rest of the policies in authz.ServerOperationsMap and remove this check if authz.Role(subject).IsAdmin() { - logger.Infow("msg", "[authZ] skipped", "sub", subject, "operation", apiOperation) + logger.Infow("msg", "[authZ] skipped", "sub", subject, "operation", apiOperation, "component", "authz/middleware") return handler(ctx, req) } @@ -73,7 +73,7 @@ func WithAuthzMiddleware(enforcer Enforcer, logger *log.Helper) middleware.Middl } func checkPolicies(ctx context.Context, subject, apiOperation string, enforcer Enforcer, logger *log.Helper) error { - logger.Infow("msg", "[authZ] checking authorization", "sub", subject, "operation", apiOperation) + logger.Infow("msg", "[authZ] checking authorization", "sub", subject, "operation", apiOperation, "component", "authz/middleware") // If there is no entry in the map for this API operation, we deny access policies, err := policiesLookup(apiOperation) if err != nil { @@ -88,7 +88,7 @@ func checkPolicies(ctx context.Context, subject, apiOperation string, enforcer E } if !ok { - logger.Infow("msg", "[authZ] policy not found", "sub", subject, "operation", apiOperation, "resource", p.Resource, "action", p.Action) + logger.Infow("msg", "[authZ] policy not found", "sub", subject, "operation", apiOperation, "resource", p.Resource, "action", p.Action, "component", "authz/middleware") return errorsAPI.Forbidden("forbidden", "operation not allowed") } } diff --git a/app/controlplane/pkg/authz/middleware/middleware_test.go b/app/controlplane/pkg/authz/middleware/middleware_test.go index bfc4d8e03..91b70ae18 100644 --- a/app/controlplane/pkg/authz/middleware/middleware_test.go +++ b/app/controlplane/pkg/authz/middleware/middleware_test.go @@ -158,8 +158,7 @@ func TestWithAuthMiddleware(t *testing.T) { ctx = transport.NewServerContext(ctx, &mockTransport{operation: tc.operationName}) e := NewMockEnforcer(t) - e.On("Enforce", subject, mock.Anything).Maybe().Return(tc.hasPermissions, nil) - e.On("EnforceWithPolicies", mock.Anything, mock.Anything).Maybe().Return(tc.hasPermissions, nil) + e.On("Enforce", mock.Anything, subject, mock.Anything).Maybe().Return(tc.hasPermissions, nil) m := WithAuthzMiddleware(e, logger) _, err := m(emptyHandler)(ctx, nil) diff --git a/app/controlplane/pkg/biz/apitoken.go b/app/controlplane/pkg/biz/apitoken.go index 0daba9b62..edd58aef0 100644 --- a/app/controlplane/pkg/biz/apitoken.go +++ b/app/controlplane/pkg/biz/apitoken.go @@ -73,20 +73,20 @@ type APITokenUseCase struct { apiTokenRepo APITokenRepo logger *log.Helper jwtBuilder *apitoken.Builder - enforcer *authz.Enforcer + authz *AuthzUseCase DefaultAuthzPolicies []*authz.Policy // Use Cases orgUseCase *OrganizationUseCase auditorUC *AuditorUseCase } -func NewAPITokenUseCase(apiTokenRepo APITokenRepo, jwtConfig *APITokenJWTConfig, authzE *authz.Enforcer, orgUseCase *OrganizationUseCase, auditorUC *AuditorUseCase, logger log.Logger) (*APITokenUseCase, error) { +func NewAPITokenUseCase(apiTokenRepo APITokenRepo, jwtConfig *APITokenJWTConfig, authzUC *AuthzUseCase, orgUseCase *OrganizationUseCase, auditorUC *AuditorUseCase, logger log.Logger) (*APITokenUseCase, error) { uc := &APITokenUseCase{ apiTokenRepo: apiTokenRepo, orgUseCase: orgUseCase, auditorUC: auditorUC, logger: servicelogger.ScopedHelper(logger, "biz/APITokenUseCase"), - enforcer: authzE, + authz: authzUC, DefaultAuthzPolicies: []*authz.Policy{ // Add permissions to workflow run authz.PolicyWorkflowRunList, authz.PolicyWorkflowRunRead, diff --git a/app/controlplane/pkg/biz/authz.go b/app/controlplane/pkg/biz/authz.go index 06d003687..e9b3c2d5d 100644 --- a/app/controlplane/pkg/biz/authz.go +++ b/app/controlplane/pkg/biz/authz.go @@ -26,21 +26,33 @@ import ( type AuthzUseCase struct { log *log.Helper - *authz.Enforcer - apiTokenRepo APITokenRepo + // actual CASBIN enforcer + enforcer *authz.Enforcer + apiTokenRepo APITokenRepo + RestrictOrgCreation bool } -func NewAuthzUseCase(enforcer *authz.Enforcer, apiTokenRepo APITokenRepo, logger log.Logger) *AuthzUseCase { +type AuthzUseCaseConfig struct { + Enforcer *authz.Enforcer + APITokenRepo APITokenRepo + RestrictOrgCreation bool + Logger log.Logger +} + +func NewAuthzUseCase(config *AuthzUseCaseConfig) *AuthzUseCase { return &AuthzUseCase{ - log: log.NewHelper(log.With(logger, "component", "biz/authz")), - apiTokenRepo: apiTokenRepo, - Enforcer: enforcer, + log: log.NewHelper(log.With(config.Logger, "component", "biz/authz")), + apiTokenRepo: config.APITokenRepo, + enforcer: config.Enforcer, + RestrictOrgCreation: config.RestrictOrgCreation, } } // Wrapper around the Enforcer.Enforce method that takes into account some of our nuances // with regards to policies retrieval and handling for API tokens. -func (e *AuthzUseCase) Enforce(ctx context.Context, sub string, p *authz.Policy) (bool, error) { +func (e *AuthzUseCase) Enforce(ctx context.Context, sub string, p *authz.Policy) (ok bool, err error) { + defer e.log.Infow("msg", "policy enforcement result", "sub", sub, "policy", p, "result", ok) + // Check if this is an API token (subject starts with "api-token:") if strings.HasPrefix(sub, "api-token:") { // load the token using the ID that's the second part of the subject @@ -59,13 +71,16 @@ func (e *AuthzUseCase) Enforce(ctx context.Context, sub string, p *authz.Policy) return false, NewErrNotFound("API token") } - // For API tokens, use ACL-based enforcement with token's policies - ok, err := e.Enforcer.EnforceWithPolicies(p, token.Policies) - if err != nil { - return false, err + for _, allowed := range token.Policies { + if allowed.Resource == p.Resource && allowed.Action == p.Action { + return true, nil + } } - return ok, nil + // Tokens for now only support ACL-based authorization + // in the future we'll continue to use the enforcer to check if the policy is allowed for the subject + return false, nil } - return e.Enforcer.Enforce(sub, p) + + return e.enforcer.Enforce(sub, p) } diff --git a/app/controlplane/pkg/biz/group.go b/app/controlplane/pkg/biz/group.go index 4f370d782..7a2f53c2e 100644 --- a/app/controlplane/pkg/biz/group.go +++ b/app/controlplane/pkg/biz/group.go @@ -188,8 +188,8 @@ type ListProjectsByGroupOpts struct { type GroupUseCase struct { // logger is used to log messages. - logger *log.Helper - enforcer *authz.Enforcer + logger *log.Helper + authz *AuthzUseCase // Repositories groupRepo GroupRepo membershipRepo MembershipRepo @@ -201,7 +201,7 @@ type GroupUseCase struct { membershipUC *MembershipUseCase } -func NewGroupUseCase(logger log.Logger, groupRepo GroupRepo, membershipRepo MembershipRepo, userRepo UserRepo, orgInvitationUC *OrgInvitationUseCase, auditorUC *AuditorUseCase, invitationRepo OrgInvitationRepo, enforcer *authz.Enforcer, membershipUseCase *MembershipUseCase) *GroupUseCase { +func NewGroupUseCase(logger log.Logger, groupRepo GroupRepo, membershipRepo MembershipRepo, userRepo UserRepo, orgInvitationUC *OrgInvitationUseCase, auditorUC *AuditorUseCase, invitationRepo OrgInvitationRepo, authzUC *AuthzUseCase, membershipUseCase *MembershipUseCase) *GroupUseCase { return &GroupUseCase{ logger: log.NewHelper(log.With(logger, "component", "biz/group")), groupRepo: groupRepo, @@ -210,7 +210,7 @@ func NewGroupUseCase(logger log.Logger, groupRepo GroupRepo, membershipRepo Memb orgInvitationUC: orgInvitationUC, auditorUC: auditorUC, orgInvitationRepo: invitationRepo, - enforcer: enforcer, + authz: authzUC, membershipUC: membershipUseCase, } } @@ -548,7 +548,7 @@ func (uc *GroupUseCase) handleNonExistingUser(ctx context.Context, orgID, groupI return nil, fmt.Errorf("failed to check requester's role: %w", err) } - pass, err := uc.enforcer.Enforce(string(requesterMembership.Role), authz.PolicyOrganizationInvitationsCreate) + pass, err := uc.authz.Enforce(ctx, string(requesterMembership.Role), authz.PolicyOrganizationInvitationsCreate) if err != nil { return nil, fmt.Errorf("failed to check requester's role: %w", err) } diff --git a/app/controlplane/pkg/biz/project.go b/app/controlplane/pkg/biz/project.go index a07abfc27..bc29b2077 100644 --- a/app/controlplane/pkg/biz/project.go +++ b/app/controlplane/pkg/biz/project.go @@ -1,5 +1,5 @@ // -// Copyright 2024 The Chainloop Authors. +// Copyright 2024-2025 The Chainloop Authors. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -51,8 +51,8 @@ type ProjectsRepo interface { // ProjectUseCase is a use case for projects type ProjectUseCase struct { - logger *log.Helper - enforcer *authz.Enforcer + logger *log.Helper + authz *AuthzUseCase // Use Cases auditorUC *AuditorUseCase groupUC *GroupUseCase @@ -164,7 +164,7 @@ type AddMemberToProjectResult struct { InvitationSent bool } -func NewProjectsUseCase(logger log.Logger, projectsRepository ProjectsRepo, membershipRepository MembershipRepo, auditorUC *AuditorUseCase, groupUC *GroupUseCase, membershipUC *MembershipUseCase, orgInvitationUC *OrgInvitationUseCase, orgInvitationRepo OrgInvitationRepo, enforcer *authz.Enforcer) *ProjectUseCase { +func NewProjectsUseCase(logger log.Logger, projectsRepository ProjectsRepo, membershipRepository MembershipRepo, auditorUC *AuditorUseCase, groupUC *GroupUseCase, membershipUC *MembershipUseCase, orgInvitationUC *OrgInvitationUseCase, orgInvitationRepo OrgInvitationRepo, authzUC *AuthzUseCase) *ProjectUseCase { return &ProjectUseCase{ logger: servicelogger.ScopedHelper(logger, "biz/project"), projectsRepository: projectsRepository, @@ -174,7 +174,7 @@ func NewProjectsUseCase(logger log.Logger, projectsRepository ProjectsRepo, memb membershipUC: membershipUC, orgInvitationUC: orgInvitationUC, orgInvitationRepo: orgInvitationRepo, - enforcer: enforcer, + authz: authzUC, } } @@ -394,7 +394,7 @@ func (uc *ProjectUseCase) handleNonExistingUser(ctx context.Context, orgID, proj return nil, fmt.Errorf("failed to check requester's role: %w", err) } - pass, err := uc.enforcer.Enforce(string(requesterMembership.Role), authz.PolicyOrganizationInvitationsCreate) + pass, err := uc.authz.Enforce(ctx, string(requesterMembership.Role), authz.PolicyOrganizationInvitationsCreate) if err != nil { return nil, fmt.Errorf("failed to check requester's role: %w", err) } diff --git a/app/controlplane/pkg/biz/testhelpers/wire.go b/app/controlplane/pkg/biz/testhelpers/wire.go index 4d8d4ffbc..1e88cefe9 100644 --- a/app/controlplane/pkg/biz/testhelpers/wire.go +++ b/app/controlplane/pkg/biz/testhelpers/wire.go @@ -1,5 +1,5 @@ // -// Copyright 2024 The Chainloop Authors. +// Copyright 2024-2025 The Chainloop Authors. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -54,7 +54,7 @@ func WireTestData(*TestDatabase, *testing.T, log.Logger, credentials.ReaderWrite NewPromSpec, NewPolicyProviderConfig, policies.NewRegistry, - authz.NewInMemoryEnforcer, + authz.NewEnforcer, newNatsConnection, auditor.NewAuditLogPublisher, NewCASBackendConfig, @@ -62,6 +62,7 @@ func WireTestData(*TestDatabase, *testing.T, log.Logger, credentials.ReaderWrite newAuthAllowList, newJWTConfig, authzConfig, + authzUseCaseConfig, biz.NewIndexConfig, ), ) @@ -71,6 +72,15 @@ func authzConfig() *authz.Config { return &authz.Config{RolesMap: authz.RolesMap} } +func authzUseCaseConfig(conf *conf.Bootstrap, enforcer *authz.Enforcer, apiTokenRepo biz.APITokenRepo, logger log.Logger) *biz.AuthzUseCaseConfig { + return &biz.AuthzUseCaseConfig{ + Enforcer: enforcer, + APITokenRepo: apiTokenRepo, + RestrictOrgCreation: conf.RestrictOrgCreation, + Logger: logger, + } +} + func newJWTConfig(conf *conf.Auth) *biz.APITokenJWTConfig { return &biz.APITokenJWTConfig{ SymmetricHmacKey: conf.GeneratedJwsHmacSecret, diff --git a/app/controlplane/pkg/biz/testhelpers/wire_gen.go b/app/controlplane/pkg/biz/testhelpers/wire_gen.go index dddd11111..8a57c38b5 100644 --- a/app/controlplane/pkg/biz/testhelpers/wire_gen.go +++ b/app/controlplane/pkg/biz/testhelpers/wire_gen.go @@ -139,12 +139,14 @@ func WireTestData(testDatabase *TestDatabase, t *testing.T, logger log.Logger, r apiTokenRepo := data.NewAPITokenRepo(dataData, logger) apiTokenJWTConfig := newJWTConfig(auth) config := authzConfig() - enforcer, err := authz.NewInMemoryEnforcer(config) + enforcer, err := authz.NewEnforcer(config) if err != nil { cleanup() return nil, nil, err } - apiTokenUseCase, err := biz.NewAPITokenUseCase(apiTokenRepo, apiTokenJWTConfig, enforcer, organizationUseCase, auditorUseCase, logger) + bizAuthzUseCaseConfig := authzUseCaseConfig(bootstrap, enforcer, apiTokenRepo, logger) + authzUseCase := biz.NewAuthzUseCase(bizAuthzUseCaseConfig) + apiTokenUseCase, err := biz.NewAPITokenUseCase(apiTokenRepo, apiTokenJWTConfig, authzUseCase, organizationUseCase, auditorUseCase, logger) if err != nil { cleanup() return nil, nil, err @@ -157,8 +159,8 @@ func WireTestData(testDatabase *TestDatabase, t *testing.T, logger log.Logger, r } projectVersionRepo := data.NewProjectVersionRepo(dataData, logger) projectVersionUseCase := biz.NewProjectVersionUseCase(projectVersionRepo, logger) - groupUseCase := biz.NewGroupUseCase(logger, groupRepo, membershipRepo, userRepo, orgInvitationUseCase, auditorUseCase, orgInvitationRepo, enforcer, membershipUseCase) - projectUseCase := biz.NewProjectsUseCase(logger, projectsRepo, membershipRepo, auditorUseCase, groupUseCase, membershipUseCase, orgInvitationUseCase, orgInvitationRepo, enforcer) + groupUseCase := biz.NewGroupUseCase(logger, groupRepo, membershipRepo, userRepo, orgInvitationUseCase, auditorUseCase, orgInvitationRepo, authzUseCase, membershipUseCase) + projectUseCase := biz.NewProjectsUseCase(logger, projectsRepo, membershipRepo, auditorUseCase, groupUseCase, membershipUseCase, orgInvitationUseCase, orgInvitationRepo, authzUseCase) testingRepos := &TestingRepos{ Membership: membershipRepo, Referrer: referrerRepo, @@ -211,6 +213,15 @@ func authzConfig() *authz.Config { return &authz.Config{RolesMap: authz.RolesMap} } +func authzUseCaseConfig(conf2 *conf.Bootstrap, enforcer *authz.Enforcer, apiTokenRepo biz.APITokenRepo, logger log.Logger) *biz.AuthzUseCaseConfig { + return &biz.AuthzUseCaseConfig{ + Enforcer: enforcer, + APITokenRepo: apiTokenRepo, + RestrictOrgCreation: conf2.RestrictOrgCreation, + Logger: logger, + } +} + func newJWTConfig(conf2 *conf.Auth) *biz.APITokenJWTConfig { return &biz.APITokenJWTConfig{ SymmetricHmacKey: conf2.GeneratedJwsHmacSecret, From dbec713c3c4b8422e707d8f7044b1a68daef7c2c Mon Sep 17 00:00:00 2001 From: Miguel Martinez Date: Sun, 16 Nov 2025 00:35:39 +0100 Subject: [PATCH 4/5] test: add unit tests for AuthzUseCase.Enforce method Add comprehensive unit tests for the Enforce method in pkg/biz/authz.go covering both API token and regular user authentication flows. Test coverage includes: - API token validation with invalid UUIDs - Token not found scenarios - Database error handling - Policy matching (allowed/denied) - Empty and nil policy handling - Partial policy matches - Multiple policy evaluation Uses testify suite pattern with mocked APITokenRepo and real enforcer instance for integration testing. Signed-off-by: Miguel Martinez --- app/controlplane/pkg/biz/authz_test.go | 305 +++++++++++++++++++++++++ 1 file changed, 305 insertions(+) create mode 100644 app/controlplane/pkg/biz/authz_test.go diff --git a/app/controlplane/pkg/biz/authz_test.go b/app/controlplane/pkg/biz/authz_test.go new file mode 100644 index 000000000..140215a7b --- /dev/null +++ b/app/controlplane/pkg/biz/authz_test.go @@ -0,0 +1,305 @@ +// +// Copyright 2025 The Chainloop Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package biz_test + +import ( + "context" + "errors" + "io" + "testing" + + "github.com/chainloop-dev/chainloop/app/controlplane/pkg/authz" + "github.com/chainloop-dev/chainloop/app/controlplane/pkg/biz" + bizMocks "github.com/chainloop-dev/chainloop/app/controlplane/pkg/biz/mocks" + "github.com/go-kratos/kratos/v2/log" + "github.com/google/uuid" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/suite" +) + +type authzTestSuite struct { + suite.Suite + useCase *biz.AuthzUseCase + apiTokenRepo *bizMocks.APITokenRepo + enforcer *authz.Enforcer + logger log.Logger +} + +func (s *authzTestSuite) SetupTest() { + s.apiTokenRepo = bizMocks.NewAPITokenRepo(s.T()) + s.logger = log.NewStdLogger(io.Discard) + + // Create a real enforcer for testing + var err error + s.enforcer, err = authz.NewEnforcer(&authz.Config{ + RolesMap: authz.RolesMap, + }) + s.Require().NoError(err) + + s.useCase = biz.NewAuthzUseCase(&biz.AuthzUseCaseConfig{ + Enforcer: s.enforcer, + APITokenRepo: s.apiTokenRepo, + RestrictOrgCreation: false, + Logger: s.logger, + }) +} + +func TestAuthzUseCase(t *testing.T) { + suite.Run(t, new(authzTestSuite)) +} + +func (s *authzTestSuite) TestEnforce_RegularUser_APITokenSubjectReturnsError() { + // The enforcer itself should reject API token subjects + subject := "api-token:some-id" + policy := &authz.Policy{ + Resource: authz.ResourceWorkflow, + Action: authz.ActionRead, + } + + // The enforcer.Enforce() method directly rejects API token subjects + ok, err := s.enforcer.Enforce(subject, policy) + + s.Error(err) + s.False(ok) + s.Contains(err.Error(), "API token subjects not supported") +} + +func (s *authzTestSuite) TestEnforce_APIToken() { + assert := assert.New(s.T()) + + s.Run("InvalidUUID", func() { + ctx := context.Background() + subject := "api-token:invalid-uuid" + policy := &authz.Policy{ + Resource: authz.ResourceWorkflow, + Action: authz.ActionRead, + } + + ok, err := s.useCase.Enforce(ctx, subject, policy) + + assert.Error(err) + assert.False(ok) + }) + + s.Run("TokenNotFound", func() { + ctx := context.Background() + tokenID := uuid.New() + subject := "api-token:" + tokenID.String() + policy := &authz.Policy{ + Resource: authz.ResourceWorkflow, + Action: authz.ActionRead, + } + + s.apiTokenRepo.On("FindByID", ctx, tokenID).Return(nil, nil) + + ok, err := s.useCase.Enforce(ctx, subject, policy) + + assert.Error(err) + assert.True(biz.IsNotFound(err)) + assert.False(ok) + }) + + s.Run("RepoError", func() { + ctx := context.Background() + tokenID := uuid.New() + subject := "api-token:" + tokenID.String() + policy := &authz.Policy{ + Resource: authz.ResourceWorkflow, + Action: authz.ActionRead, + } + + expectedErr := errors.New("database error") + s.apiTokenRepo.On("FindByID", ctx, tokenID).Return(nil, expectedErr) + + ok, err := s.useCase.Enforce(ctx, subject, policy) + + assert.Error(err) + assert.Equal(expectedErr, err) + assert.False(ok) + }) + + s.Run("PolicyAllowed", func() { + ctx := context.Background() + tokenID := uuid.New() + subject := "api-token:" + tokenID.String() + policy := &authz.Policy{ + Resource: authz.ResourceWorkflow, + Action: authz.ActionRead, + } + + token := &biz.APIToken{ + ID: tokenID, + Policies: []*authz.Policy{ + {Resource: authz.ResourceWorkflow, Action: authz.ActionRead}, + {Resource: authz.ResourceWorkflowRun, Action: authz.ActionList}, + }, + } + + s.apiTokenRepo.On("FindByID", ctx, tokenID).Return(token, nil) + + ok, err := s.useCase.Enforce(ctx, subject, policy) + + assert.NoError(err) + assert.True(ok) + }) + + s.Run("PolicyDenied", func() { + ctx := context.Background() + tokenID := uuid.New() + subject := "api-token:" + tokenID.String() + policy := &authz.Policy{ + Resource: authz.ResourceWorkflow, + Action: authz.ActionDelete, + } + + token := &biz.APIToken{ + ID: tokenID, + Policies: []*authz.Policy{ + {Resource: authz.ResourceWorkflow, Action: authz.ActionRead}, + {Resource: authz.ResourceWorkflow, Action: authz.ActionList}, + }, + } + + s.apiTokenRepo.On("FindByID", ctx, tokenID).Return(token, nil) + + ok, err := s.useCase.Enforce(ctx, subject, policy) + + assert.NoError(err) + assert.False(ok) + }) + + s.Run("EmptyPolicies", func() { + ctx := context.Background() + tokenID := uuid.New() + subject := "api-token:" + tokenID.String() + policy := &authz.Policy{ + Resource: authz.ResourceWorkflow, + Action: authz.ActionRead, + } + + token := &biz.APIToken{ + ID: tokenID, + Policies: []*authz.Policy{}, + } + + s.apiTokenRepo.On("FindByID", ctx, tokenID).Return(token, nil) + + ok, err := s.useCase.Enforce(ctx, subject, policy) + + assert.NoError(err) + assert.False(ok) + }) + + s.Run("NilPolicies", func() { + ctx := context.Background() + tokenID := uuid.New() + subject := "api-token:" + tokenID.String() + policy := &authz.Policy{ + Resource: authz.ResourceWorkflow, + Action: authz.ActionRead, + } + + token := &biz.APIToken{ + ID: tokenID, + Policies: nil, + } + + s.apiTokenRepo.On("FindByID", ctx, tokenID).Return(token, nil) + + ok, err := s.useCase.Enforce(ctx, subject, policy) + + assert.NoError(err) + assert.False(ok) + }) + + s.Run("PartialMatchResource", func() { + ctx := context.Background() + tokenID := uuid.New() + subject := "api-token:" + tokenID.String() + policy := &authz.Policy{ + Resource: authz.ResourceWorkflow, + Action: authz.ActionRead, + } + + // Token has matching resource but different action + token := &biz.APIToken{ + ID: tokenID, + Policies: []*authz.Policy{ + {Resource: authz.ResourceWorkflow, Action: authz.ActionCreate}, + }, + } + + s.apiTokenRepo.On("FindByID", ctx, tokenID).Return(token, nil) + + ok, err := s.useCase.Enforce(ctx, subject, policy) + + assert.NoError(err) + assert.False(ok) + }) + + s.Run("PartialMatchAction", func() { + ctx := context.Background() + tokenID := uuid.New() + subject := "api-token:" + tokenID.String() + policy := &authz.Policy{ + Resource: authz.ResourceWorkflow, + Action: authz.ActionRead, + } + + // Token has matching action but different resource + token := &biz.APIToken{ + ID: tokenID, + Policies: []*authz.Policy{ + {Resource: authz.ResourceWorkflowRun, Action: authz.ActionRead}, + }, + } + + s.apiTokenRepo.On("FindByID", ctx, tokenID).Return(token, nil) + + ok, err := s.useCase.Enforce(ctx, subject, policy) + + assert.NoError(err) + assert.False(ok) + }) + + s.Run("MultiplePoliciesWithMatch", func() { + ctx := context.Background() + tokenID := uuid.New() + subject := "api-token:" + tokenID.String() + policy := &authz.Policy{ + Resource: authz.ResourceWorkflowContract, + Action: authz.ActionUpdate, + } + + // Token has multiple policies, one of them matches + token := &biz.APIToken{ + ID: tokenID, + Policies: []*authz.Policy{ + {Resource: authz.ResourceWorkflow, Action: authz.ActionRead}, + {Resource: authz.ResourceWorkflow, Action: authz.ActionCreate}, + {Resource: authz.ResourceWorkflowContract, Action: authz.ActionUpdate}, + {Resource: authz.ResourceCASArtifact, Action: authz.ActionRead}, + }, + } + + s.apiTokenRepo.On("FindByID", ctx, tokenID).Return(token, nil) + + ok, err := s.useCase.Enforce(ctx, subject, policy) + + assert.NoError(err) + assert.True(ok) + }) +} From 5bd74116a25819ce1b0c5bafb98f68f0c99ddcda Mon Sep 17 00:00:00 2001 From: Miguel Martinez Date: Mon, 17 Nov 2025 10:43:23 +0100 Subject: [PATCH 5/5] rename vble Signed-off-by: Miguel Martinez --- app/controlplane/cmd/wire.go | 6 +++--- app/controlplane/cmd/wire_gen.go | 8 ++++---- app/controlplane/configs/config.devel.yaml | 8 ++++---- app/controlplane/pkg/authz/authz_test.go | 4 ++-- app/controlplane/pkg/authz/enforcer.go | 12 ++++++------ app/controlplane/pkg/biz/authz.go | 11 +++++------ app/controlplane/pkg/biz/authz_test.go | 6 +++--- app/controlplane/pkg/biz/testhelpers/database.go | 2 +- app/controlplane/pkg/biz/testhelpers/wire.go | 6 +++--- app/controlplane/pkg/biz/testhelpers/wire_gen.go | 10 +++++----- 10 files changed, 36 insertions(+), 37 deletions(-) diff --git a/app/controlplane/cmd/wire.go b/app/controlplane/cmd/wire.go index a52478a8a..c3ac9787c 100644 --- a/app/controlplane/cmd/wire.go +++ b/app/controlplane/cmd/wire.go @@ -53,7 +53,7 @@ func wireApp(*conf.Bootstrap, credentials.ReaderWriter, log.Logger, sdk.Availabl wire.FieldsOf(new(*conf.Bootstrap), "Server", "Auth", "Data", "CasServer", "ReferrerSharedIndex", "Onboarding", "PrometheusIntegration", "PolicyProviders", "NatsServer", "FederatedAuthentication"), wire.FieldsOf(new(*conf.Data), "Database"), dispatcher.New, - authz.NewEnforcer, + authz.NewCasbinEnforcer, policies.NewRegistry, newApp, newProtoValidator, @@ -75,9 +75,9 @@ func authzConfig() *authz.Config { return &authz.Config{RolesMap: authz.RolesMap} } -func authzUseCaseConfig(conf *conf.Bootstrap, enforcer *authz.Enforcer, apiTokenRepo biz.APITokenRepo, logger log.Logger) *biz.AuthzUseCaseConfig { +func authzUseCaseConfig(conf *conf.Bootstrap, casbinEnforcer *authz.CasbinEnforcer, apiTokenRepo biz.APITokenRepo, logger log.Logger) *biz.AuthzUseCaseConfig { return &biz.AuthzUseCaseConfig{ - Enforcer: enforcer, + CasbinEnforcer: casbinEnforcer, APITokenRepo: apiTokenRepo, RestrictOrgCreation: conf.RestrictOrgCreation, Logger: logger, diff --git a/app/controlplane/cmd/wire_gen.go b/app/controlplane/cmd/wire_gen.go index d0166e823..436700e5e 100644 --- a/app/controlplane/cmd/wire_gen.go +++ b/app/controlplane/cmd/wire_gen.go @@ -31,7 +31,7 @@ import ( func wireApp(bootstrap *conf.Bootstrap, readerWriter credentials.ReaderWriter, logger log.Logger, availablePlugins sdk.AvailablePlugins) (*app, func(), error) { config := authzConfig() - enforcer, err := authz.NewEnforcer(config) + casbinEnforcer, err := authz.NewCasbinEnforcer(config) if err != nil { return nil, nil, err } @@ -43,7 +43,7 @@ func wireApp(bootstrap *conf.Bootstrap, readerWriter credentials.ReaderWriter, l return nil, nil, err } apiTokenRepo := data.NewAPITokenRepo(dataData, logger) - bizAuthzUseCaseConfig := authzUseCaseConfig(bootstrap, enforcer, apiTokenRepo, logger) + bizAuthzUseCaseConfig := authzUseCaseConfig(bootstrap, casbinEnforcer, apiTokenRepo, logger) authzUseCase := biz.NewAuthzUseCase(bizAuthzUseCaseConfig) userRepo := data.NewUserRepo(dataData, logger) groupRepo := data.NewGroupRepo(dataData, logger) @@ -324,9 +324,9 @@ func authzConfig() *authz.Config { return &authz.Config{RolesMap: authz.RolesMap} } -func authzUseCaseConfig(conf2 *conf.Bootstrap, enforcer *authz.Enforcer, apiTokenRepo biz.APITokenRepo, logger log.Logger) *biz.AuthzUseCaseConfig { +func authzUseCaseConfig(conf2 *conf.Bootstrap, casbinEnforcer *authz.CasbinEnforcer, apiTokenRepo biz.APITokenRepo, logger log.Logger) *biz.AuthzUseCaseConfig { return &biz.AuthzUseCaseConfig{ - Enforcer: enforcer, + CasbinEnforcer: casbinEnforcer, APITokenRepo: apiTokenRepo, RestrictOrgCreation: conf2.RestrictOrgCreation, Logger: logger, diff --git a/app/controlplane/configs/config.devel.yaml b/app/controlplane/configs/config.devel.yaml index e7acbe0ec..b52c4d6b4 100644 --- a/app/controlplane/configs/config.devel.yaml +++ b/app/controlplane/configs/config.devel.yaml @@ -98,10 +98,10 @@ prometheus_integration: - org_name: "development" # Policy providers configuration -# policy_providers: -# - name: chainloop -# default: true -# url: http://localhost:8002/v1 +policy_providers: + - name: chainloop + default: true + url: http://localhost:8002/v1 enable_profiler: true # federated_authentication: diff --git a/app/controlplane/pkg/authz/authz_test.go b/app/controlplane/pkg/authz/authz_test.go index 76b32701a..d9a2104bb 100644 --- a/app/controlplane/pkg/authz/authz_test.go +++ b/app/controlplane/pkg/authz/authz_test.go @@ -129,13 +129,13 @@ func TestDoSync(t *testing.T) { assert.Equal(t, "delete", got[0][2]) } -func testEnforcer(t *testing.T) (*Enforcer, io.Closer) { +func testEnforcer(t *testing.T) (*CasbinEnforcer, io.Closer) { f, err := os.CreateTemp(t.TempDir(), "policy*.csv") if err != nil { require.FailNow(t, err.Error()) } - enforcer, err := NewEnforcer(&Config{}) + enforcer, err := NewCasbinEnforcer(&Config{}) require.NoError(t, err) return enforcer, f } diff --git a/app/controlplane/pkg/authz/enforcer.go b/app/controlplane/pkg/authz/enforcer.go index 33b4ff9be..7323a285f 100644 --- a/app/controlplane/pkg/authz/enforcer.go +++ b/app/controlplane/pkg/authz/enforcer.go @@ -41,13 +41,13 @@ type Config struct { RolesMap map[Role][]*Policy } -type Enforcer struct { +type CasbinEnforcer struct { *casbin.Enforcer config *Config } -func (e *Enforcer) Enforce(sub string, p *Policy) (bool, error) { +func (e *CasbinEnforcer) Enforce(sub string, p *Policy) (bool, error) { // This enforcer does not support API token subjects // this is due to the fact that API tokens are not stored in casbin yet // To use them, make sure you use the AuthzUseCase.Enforce method instead @@ -58,9 +58,9 @@ func (e *Enforcer) Enforce(sub string, p *Policy) (bool, error) { return e.Enforcer.Enforce(sub, p.Resource, p.Action) } -// NewEnforcer creates a new casbin authorization enforcer with in-memory storage. +// NewCasbinEnforcer creates a new casbin authorization enforcer with in-memory storage. // Only static role policies from RolesMap are loaded -func NewEnforcer(config *Config) (*Enforcer, error) { +func NewCasbinEnforcer(config *Config) (*CasbinEnforcer, error) { // load model defined in model.conf m, err := model.NewModelFromString(string(modelFile)) if err != nil { @@ -73,7 +73,7 @@ func NewEnforcer(config *Config) (*Enforcer, error) { return nil, fmt.Errorf("failed to create enforcer: %w", err) } - e := &Enforcer{enforcer, config} + e := &CasbinEnforcer{enforcer, config} // Initialize the enforcer with the roles map if err := syncRBACRoles(e, config); err != nil { @@ -83,7 +83,7 @@ func NewEnforcer(config *Config) (*Enforcer, error) { return e, nil } -func syncRBACRoles(e *Enforcer, c *Config) error { +func syncRBACRoles(e *CasbinEnforcer, c *Config) error { // allow to override config during sync conf := c if conf == nil { diff --git a/app/controlplane/pkg/biz/authz.go b/app/controlplane/pkg/biz/authz.go index e9b3c2d5d..5fa21bfe8 100644 --- a/app/controlplane/pkg/biz/authz.go +++ b/app/controlplane/pkg/biz/authz.go @@ -25,15 +25,14 @@ import ( ) type AuthzUseCase struct { - log *log.Helper - // actual CASBIN enforcer - enforcer *authz.Enforcer + log *log.Helper + casbinEnforcer *authz.CasbinEnforcer apiTokenRepo APITokenRepo RestrictOrgCreation bool } type AuthzUseCaseConfig struct { - Enforcer *authz.Enforcer + CasbinEnforcer *authz.CasbinEnforcer APITokenRepo APITokenRepo RestrictOrgCreation bool Logger log.Logger @@ -43,7 +42,7 @@ func NewAuthzUseCase(config *AuthzUseCaseConfig) *AuthzUseCase { return &AuthzUseCase{ log: log.NewHelper(log.With(config.Logger, "component", "biz/authz")), apiTokenRepo: config.APITokenRepo, - enforcer: config.Enforcer, + casbinEnforcer: config.CasbinEnforcer, RestrictOrgCreation: config.RestrictOrgCreation, } } @@ -82,5 +81,5 @@ func (e *AuthzUseCase) Enforce(ctx context.Context, sub string, p *authz.Policy) return false, nil } - return e.enforcer.Enforce(sub, p) + return e.casbinEnforcer.Enforce(sub, p) } diff --git a/app/controlplane/pkg/biz/authz_test.go b/app/controlplane/pkg/biz/authz_test.go index 140215a7b..c26d2cffd 100644 --- a/app/controlplane/pkg/biz/authz_test.go +++ b/app/controlplane/pkg/biz/authz_test.go @@ -34,7 +34,7 @@ type authzTestSuite struct { suite.Suite useCase *biz.AuthzUseCase apiTokenRepo *bizMocks.APITokenRepo - enforcer *authz.Enforcer + enforcer *authz.CasbinEnforcer logger log.Logger } @@ -44,13 +44,13 @@ func (s *authzTestSuite) SetupTest() { // Create a real enforcer for testing var err error - s.enforcer, err = authz.NewEnforcer(&authz.Config{ + s.enforcer, err = authz.NewCasbinEnforcer(&authz.Config{ RolesMap: authz.RolesMap, }) s.Require().NoError(err) s.useCase = biz.NewAuthzUseCase(&biz.AuthzUseCaseConfig{ - Enforcer: s.enforcer, + CasbinEnforcer: s.enforcer, APITokenRepo: s.apiTokenRepo, RestrictOrgCreation: false, Logger: s.logger, diff --git a/app/controlplane/pkg/biz/testhelpers/database.go b/app/controlplane/pkg/biz/testhelpers/database.go index da16ce209..26324354f 100644 --- a/app/controlplane/pkg/biz/testhelpers/database.go +++ b/app/controlplane/pkg/biz/testhelpers/database.go @@ -71,7 +71,7 @@ type TestingUseCases struct { OrgInvitation *biz.OrgInvitationUseCase Referrer *biz.ReferrerUseCase APIToken *biz.APITokenUseCase - Enforcer *authz.Enforcer + Enforcer *authz.CasbinEnforcer AttestationState *biz.AttestationStateUseCase ProjectVersion *biz.ProjectVersionUseCase Project *biz.ProjectUseCase diff --git a/app/controlplane/pkg/biz/testhelpers/wire.go b/app/controlplane/pkg/biz/testhelpers/wire.go index 1e88cefe9..2d124363b 100644 --- a/app/controlplane/pkg/biz/testhelpers/wire.go +++ b/app/controlplane/pkg/biz/testhelpers/wire.go @@ -54,7 +54,7 @@ func WireTestData(*TestDatabase, *testing.T, log.Logger, credentials.ReaderWrite NewPromSpec, NewPolicyProviderConfig, policies.NewRegistry, - authz.NewEnforcer, + authz.NewCasbinEnforcer, newNatsConnection, auditor.NewAuditLogPublisher, NewCASBackendConfig, @@ -72,9 +72,9 @@ func authzConfig() *authz.Config { return &authz.Config{RolesMap: authz.RolesMap} } -func authzUseCaseConfig(conf *conf.Bootstrap, enforcer *authz.Enforcer, apiTokenRepo biz.APITokenRepo, logger log.Logger) *biz.AuthzUseCaseConfig { +func authzUseCaseConfig(conf *conf.Bootstrap, casbinEnforcer *authz.CasbinEnforcer, apiTokenRepo biz.APITokenRepo, logger log.Logger) *biz.AuthzUseCaseConfig { return &biz.AuthzUseCaseConfig{ - Enforcer: enforcer, + CasbinEnforcer: casbinEnforcer, APITokenRepo: apiTokenRepo, RestrictOrgCreation: conf.RestrictOrgCreation, Logger: logger, diff --git a/app/controlplane/pkg/biz/testhelpers/wire_gen.go b/app/controlplane/pkg/biz/testhelpers/wire_gen.go index 8a57c38b5..b6f839fcf 100644 --- a/app/controlplane/pkg/biz/testhelpers/wire_gen.go +++ b/app/controlplane/pkg/biz/testhelpers/wire_gen.go @@ -139,12 +139,12 @@ func WireTestData(testDatabase *TestDatabase, t *testing.T, logger log.Logger, r apiTokenRepo := data.NewAPITokenRepo(dataData, logger) apiTokenJWTConfig := newJWTConfig(auth) config := authzConfig() - enforcer, err := authz.NewEnforcer(config) + casbinEnforcer, err := authz.NewCasbinEnforcer(config) if err != nil { cleanup() return nil, nil, err } - bizAuthzUseCaseConfig := authzUseCaseConfig(bootstrap, enforcer, apiTokenRepo, logger) + bizAuthzUseCaseConfig := authzUseCaseConfig(bootstrap, casbinEnforcer, apiTokenRepo, logger) authzUseCase := biz.NewAuthzUseCase(bizAuthzUseCaseConfig) apiTokenUseCase, err := biz.NewAPITokenUseCase(apiTokenRepo, apiTokenJWTConfig, authzUseCase, organizationUseCase, auditorUseCase, logger) if err != nil { @@ -190,7 +190,7 @@ func WireTestData(testDatabase *TestDatabase, t *testing.T, logger log.Logger, r OrgInvitation: orgInvitationUseCase, Referrer: referrerUseCase, APIToken: apiTokenUseCase, - Enforcer: enforcer, + Enforcer: casbinEnforcer, AttestationState: attestationStateUseCase, ProjectVersion: projectVersionUseCase, Project: projectUseCase, @@ -213,9 +213,9 @@ func authzConfig() *authz.Config { return &authz.Config{RolesMap: authz.RolesMap} } -func authzUseCaseConfig(conf2 *conf.Bootstrap, enforcer *authz.Enforcer, apiTokenRepo biz.APITokenRepo, logger log.Logger) *biz.AuthzUseCaseConfig { +func authzUseCaseConfig(conf2 *conf.Bootstrap, casbinEnforcer *authz.CasbinEnforcer, apiTokenRepo biz.APITokenRepo, logger log.Logger) *biz.AuthzUseCaseConfig { return &biz.AuthzUseCaseConfig{ - Enforcer: enforcer, + CasbinEnforcer: casbinEnforcer, APITokenRepo: apiTokenRepo, RestrictOrgCreation: conf2.RestrictOrgCreation, Logger: logger,