diff --git a/internal/tests/mock_plugin_services.go b/internal/tests/mock_plugin_services.go index 6f6c0ed0..883a97c8 100644 --- a/internal/tests/mock_plugin_services.go +++ b/internal/tests/mock_plugin_services.go @@ -49,3 +49,11 @@ func (m *MockOrganizationService) ExistsByID(ctx context.Context, organizationID args := m.Called(ctx, organizationID) return args.Bool(0), args.Error(1) } + +func (m *MockOrganizationService) GetUserPermissionsInOrganization(ctx context.Context, userID string, organizationID string) ([]string, error) { + args := m.Called(ctx, userID, organizationID) + if args.Get(0) == nil { + return nil, args.Error(1) + } + return args.Get(0).([]string), args.Error(1) +} diff --git a/plugins/access-control/services/access_control_service.go b/plugins/access-control/services/access_control_service.go index b46c93e4..100292a0 100644 --- a/plugins/access-control/services/access_control_service.go +++ b/plugins/access-control/services/access_control_service.go @@ -2,7 +2,6 @@ package services import ( "context" - "time" coreerrors "github.com/Authula/authula/core/errors" "github.com/Authula/authula/plugins/access-control/repositories" @@ -30,34 +29,38 @@ func (s *AccessControlService) RoleExists(ctx context.Context, roleName string) return role != nil && role.ID != "", nil } -func (s *AccessControlService) ValidateRoleAssignment(ctx context.Context, roleName string, assignerUserID *string) (bool, error) { +func (s *AccessControlService) GetRolePermissionsByName(ctx context.Context, roleName string) ([]string, error) { role, err := s.rolesService.GetRoleByName(ctx, nil, roleName) if err != nil { - return false, err + return nil, err } if role == nil || role.ID == "" { - return false, coreerrors.ErrNotFound - } - - if assignerUserID == nil || *assignerUserID == "" { - return false, nil + return nil, coreerrors.ErrNotFound } - assignerRoles, err := s.userRolesService.GetUserRoles(ctx, nil, *assignerUserID) + details, err := s.rolesService.GetRoleByID(ctx, nil, role.ID) if err != nil { - return false, err + return nil, err } - highestWeight, activeCount := determineHighestActiveRoleWeight(assignerRoles, time.Now().UTC()) - if activeCount == 0 { - return false, coreerrors.ErrForbidden + permissions := make([]string, 0, len(details.Permissions)) + for _, permission := range details.Permissions { + permissions = append(permissions, permission.PermissionKey) } - if role.Weight > highestWeight { - return false, coreerrors.ErrForbidden + return permissions, nil +} + +func (s *AccessControlService) GetRoleWeightByName(ctx context.Context, roleName string) (int, error) { + role, err := s.rolesService.GetRoleByName(ctx, nil, roleName) + if err != nil { + return 0, err + } + if role == nil || role.ID == "" { + return 0, coreerrors.ErrNotFound } - return true, nil + return role.Weight, nil } func (s *AccessControlService) ValidatePermissionKeys(ctx context.Context, permissionKeys []string) error { diff --git a/plugins/access-control/services/access_control_service_test.go b/plugins/access-control/services/access_control_service_test.go index 7ebe5f64..862b9c6c 100644 --- a/plugins/access-control/services/access_control_service_test.go +++ b/plugins/access-control/services/access_control_service_test.go @@ -2,8 +2,8 @@ package services import ( "context" + "slices" "testing" - "time" "github.com/stretchr/testify/mock" @@ -142,66 +142,125 @@ func TestAccessControlServiceValidatePermissionKeys(t *testing.T) { } } -func TestAccessControlServiceValidateRoleAssignment(t *testing.T) { +func TestAccessControlServiceGetRolePermissionsByName(t *testing.T) { t.Parallel() - assignerUserID := func() *string { value := "assigner-user-1"; return &value }() - tests := []struct { - name string - roleName string - assigner *string - setup func(*accesscontroltests.MockRolesRepository, *accesscontroltests.MockUserRolesRepository) - wantErr error - wantOK bool + name string + roleName string + setup func(*accesscontroltests.MockRolesRepository, *accesscontroltests.MockRolePermissionsRepository) + wantErr error + wantPerms []string }{ { name: "role not found", roleName: "missing", - setup: func(rolesRepo *accesscontroltests.MockRolesRepository, userRolesRepo *accesscontroltests.MockUserRolesRepository) { - rolesRepo.On("GetRoleByName", mock.Anything, "missing").Return((*types.Role)(nil), nil).Once() + setup: func(rolesRepo *accesscontroltests.MockRolesRepository, rolePermRepo *accesscontroltests.MockRolePermissionsRepository) { + rolesRepo.On("GetRoleByName", mock.Anything, "missing").Return((*types.Role)(nil), coreerrors.ErrNotFound).Once() + }, + wantErr: coreerrors.ErrNotFound, + }, + { + name: "nil role treated as not found", + roleName: "ghost", + setup: func(rolesRepo *accesscontroltests.MockRolesRepository, rolePermRepo *accesscontroltests.MockRolePermissionsRepository) { + rolesRepo.On("GetRoleByName", mock.Anything, "ghost").Return((*types.Role)(nil), nil).Once() }, wantErr: coreerrors.ErrNotFound, }, { - name: "nil assigner is not allowed", + name: "role permissions resolved", roleName: "editor", - assigner: nil, - setup: func(rolesRepo *accesscontroltests.MockRolesRepository, userRolesRepo *accesscontroltests.MockUserRolesRepository) { + setup: func(rolesRepo *accesscontroltests.MockRolesRepository, rolePermRepo *accesscontroltests.MockRolePermissionsRepository) { rolesRepo.On("GetRoleByName", mock.Anything, "editor").Return(&types.Role{ID: "role-1", Name: "editor", Weight: 10}, nil).Once() + rolesRepo.On("GetRoleByID", mock.Anything, "role-1").Return(&types.Role{ID: "role-1", Name: "editor", Weight: 10}, nil).Once() + rolePermRepo.On("GetRolePermissions", mock.Anything, "role-1").Return([]types.UserPermissionInfo{ + {PermissionKey: "organizations:members:list"}, + {PermissionKey: "organizations:members:read"}, + }, nil).Once() }, - wantOK: false, + wantPerms: []string{"organizations:members:list", "organizations:members:read"}, }, { - name: "forbidden when assigner has no active roles", + name: "repository error propagates", roleName: "editor", - assigner: assignerUserID, - setup: func(rolesRepo *accesscontroltests.MockRolesRepository, userRolesRepo *accesscontroltests.MockUserRolesRepository) { + setup: func(rolesRepo *accesscontroltests.MockRolesRepository, rolePermRepo *accesscontroltests.MockRolePermissionsRepository) { rolesRepo.On("GetRoleByName", mock.Anything, "editor").Return(&types.Role{ID: "role-1", Name: "editor", Weight: 10}, nil).Once() - userRolesRepo.On("GetUserRoles", mock.Anything, "assigner-user-1").Return([]types.UserRoleInfo{{RoleID: "role-old", RoleName: "old", RoleWeight: 100, ExpiresAt: func() *time.Time { value := time.Now().UTC().Add(-time.Hour); return &value }()}}, nil).Once() + rolesRepo.On("GetRoleByID", mock.Anything, "role-1").Return(&types.Role{ID: "role-1", Name: "editor", Weight: 10}, nil).Once() + rolePermRepo.On("GetRolePermissions", mock.Anything, "role-1").Return(([]types.UserPermissionInfo)(nil), coreerrors.ErrForbidden).Once() }, wantErr: coreerrors.ErrForbidden, }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + rolesRepo := &accesscontroltests.MockRolesRepository{} + rolePermRepo := &accesscontroltests.MockRolePermissionsRepository{} + if tc.setup != nil { + tc.setup(rolesRepo, rolePermRepo) + } + + service := NewAccessControlService(NewRolesService(rolesRepo, rolePermRepo, nil), NewUserRolesService(nil, nil), nil) + perms, err := service.GetRolePermissionsByName(context.Background(), tc.roleName) + if err != tc.wantErr { + t.Fatalf("expected err %v, got %v", tc.wantErr, err) + } + if tc.wantErr != nil { + if perms != nil { + t.Fatalf("expected nil permissions, got %v", perms) + } + } else if !slices.Equal(perms, tc.wantPerms) { + t.Fatalf("expected permissions %v, got %v", tc.wantPerms, perms) + } + + rolesRepo.AssertExpectations(t) + rolePermRepo.AssertExpectations(t) + }) + } +} + +func TestAccessControlServiceGetRoleWeightByName(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + roleName string + setup func(*accesscontroltests.MockRolesRepository) + wantErr error + wantWeight int + }{ { - name: "expired roles are ignored", - roleName: "editor", - assigner: assignerUserID, - setup: func(rolesRepo *accesscontroltests.MockRolesRepository, userRolesRepo *accesscontroltests.MockUserRolesRepository) { - rolesRepo.On("GetRoleByName", mock.Anything, "editor").Return(&types.Role{ID: "role-1", Name: "editor", Weight: 20}, nil).Once() - userRolesRepo.On("GetUserRoles", mock.Anything, "assigner-user-1").Return([]types.UserRoleInfo{ - {RoleID: "role-expired", RoleName: "expired", RoleWeight: 100, ExpiresAt: func() *time.Time { value := time.Now().UTC().Add(-time.Hour); return &value }()}, - {RoleID: "role-active", RoleName: "active", RoleWeight: 30}, - }, nil).Once() + name: "role not found", + roleName: "missing", + setup: func(rolesRepo *accesscontroltests.MockRolesRepository) { + rolesRepo.On("GetRoleByName", mock.Anything, "missing").Return((*types.Role)(nil), coreerrors.ErrNotFound).Once() }, - wantOK: true, + wantErr: coreerrors.ErrNotFound, + }, + { + name: "nil role treated as not found", + roleName: "ghost", + setup: func(rolesRepo *accesscontroltests.MockRolesRepository) { + rolesRepo.On("GetRoleByName", mock.Anything, "ghost").Return((*types.Role)(nil), nil).Once() + }, + wantErr: coreerrors.ErrNotFound, }, { - name: "forbidden when target exceeds assigner weight", + name: "role weight resolved", roleName: "admin", - assigner: assignerUserID, - setup: func(rolesRepo *accesscontroltests.MockRolesRepository, userRolesRepo *accesscontroltests.MockUserRolesRepository) { + setup: func(rolesRepo *accesscontroltests.MockRolesRepository) { rolesRepo.On("GetRoleByName", mock.Anything, "admin").Return(&types.Role{ID: "role-2", Name: "admin", Weight: 80}, nil).Once() - userRolesRepo.On("GetUserRoles", mock.Anything, "assigner-user-1").Return([]types.UserRoleInfo{{RoleID: "role-member", RoleName: "member", RoleWeight: 10}}, nil).Once() + }, + wantWeight: 80, + }, + { + name: "repository error propagates", + roleName: "admin", + setup: func(rolesRepo *accesscontroltests.MockRolesRepository) { + rolesRepo.On("GetRoleByName", mock.Anything, "admin").Return((*types.Role)(nil), coreerrors.ErrForbidden).Once() }, wantErr: coreerrors.ErrForbidden, }, @@ -212,26 +271,24 @@ func TestAccessControlServiceValidateRoleAssignment(t *testing.T) { t.Parallel() rolesRepo := &accesscontroltests.MockRolesRepository{} - userRolesRepo := &accesscontroltests.MockUserRolesRepository{} if tc.setup != nil { - tc.setup(rolesRepo, userRolesRepo) + tc.setup(rolesRepo) } - service := NewAccessControlService(NewRolesService(rolesRepo, nil, userRolesRepo), NewUserRolesService(userRolesRepo, rolesRepo), nil) - ok, err := service.ValidateRoleAssignment(context.Background(), tc.roleName, tc.assigner) + service := NewAccessControlService(NewRolesService(rolesRepo, nil, nil), NewUserRolesService(nil, nil), nil) + weight, err := service.GetRoleWeightByName(context.Background(), tc.roleName) if err != tc.wantErr { t.Fatalf("expected err %v, got %v", tc.wantErr, err) } if tc.wantErr != nil { - if ok { - t.Fatalf("expected false, got true") + if weight != 0 { + t.Fatalf("expected 0 weight, got %d", weight) } - } else if ok != tc.wantOK { - t.Fatalf("unexpected result %v", ok) + } else if weight != tc.wantWeight { + t.Fatalf("expected weight %d, got %d", tc.wantWeight, weight) } rolesRepo.AssertExpectations(t) - userRolesRepo.AssertExpectations(t) }) } } diff --git a/plugins/api-key/plugin.go b/plugins/api-key/plugin.go index 0ef5ef39..8e897221 100644 --- a/plugins/api-key/plugin.go +++ b/plugins/api-key/plugin.go @@ -87,7 +87,7 @@ func (p *ApiKeyPlugin) Init(ctx *models.PluginContext) error { p.rateLimiterService = rateLimiterService } - var organizationService rootservices.OrganizationService + var organizationService apiservices.OrganizationLookupService if p.config.AllowOrgKeys { orgSvc, ok := ctx.ServiceRegistry.Get(models.ServiceOrganization.String()).(orgplugins.OrganizationLookupService) if !ok { diff --git a/plugins/api-key/services/api_key_service.go b/plugins/api-key/services/api_key_service.go index 760fad54..44039120 100644 --- a/plugins/api-key/services/api_key_service.go +++ b/plugins/api-key/services/api_key_service.go @@ -21,7 +21,7 @@ type apiKeyService struct { tokenService rootservices.TokenService accessControlService rootservices.AccessControlService rateLimiterService rootservices.RateLimiterService - organizationService rootservices.OrganizationService + organizationService OrganizationLookupService apiKeyRepo repositories.ApiKeyRepository } @@ -36,7 +36,7 @@ func NewApiKeyService( tokenService rootservices.TokenService, accessControlService rootservices.AccessControlService, rateLimiterService rootservices.RateLimiterService, - organizationService rootservices.OrganizationService, + organizationService OrganizationLookupService, apiKeyRepo repositories.ApiKeyRepository, ) ApiKeyService { return &apiKeyService{ @@ -54,7 +54,7 @@ func NewApiKeyService( } func (s *apiKeyService) Create(ctx context.Context, actor *models.Actor, req types.CreateApiKeyRequest) (*types.CreateApiKeyResponse, error) { - if err := s.authorizeCreate(actor, req); err != nil { + if err := s.authorizeCreate(ctx, actor, req); err != nil { return nil, err } @@ -169,7 +169,7 @@ func (s *apiKeyService) Create(ctx context.Context, actor *models.Actor, req typ }, nil } -func (s *apiKeyService) authorizeCreate(actor *models.Actor, req types.CreateApiKeyRequest) error { +func (s *apiKeyService) authorizeCreate(ctx context.Context, actor *models.Actor, req types.CreateApiKeyRequest) error { switch req.OwnerType { case types.OwnerTypeUser: if req.OwnerID != "" && req.OwnerID != actor.ID { @@ -185,7 +185,11 @@ func (s *apiKeyService) authorizeCreate(actor *models.Actor, req types.CreateApi if s.organizationService == nil { return fmt.Errorf("%w: organization service is not available", coreerrors.ErrUnprocessableEntity) } - if err := s.validatePermissionsSubset(actor.Scopes, req.Permissions); err != nil { + perms, err := s.organizationService.GetUserPermissionsInOrganization(ctx, actor.ID, req.OwnerID) + if err != nil { + return err + } + if err := s.validatePermissionsSubset(perms, req.Permissions); err != nil { return err } } @@ -270,7 +274,14 @@ func (s *apiKeyService) Update(ctx context.Context, actor *models.Actor, id stri } case types.OwnerTypeOrganization: if len(req.Permissions) > 0 { - if err := s.validatePermissionsSubset(actor.Scopes, req.Permissions); err != nil { + if s.organizationService == nil { + return nil, fmt.Errorf("%w: organization service is not available", coreerrors.ErrUnprocessableEntity) + } + perms, err := s.organizationService.GetUserPermissionsInOrganization(ctx, actor.ID, apiKey.OwnerID) + if err != nil { + return nil, err + } + if err := s.validatePermissionsSubset(perms, req.Permissions); err != nil { return nil, err } } diff --git a/plugins/api-key/services/api_key_service_test.go b/plugins/api-key/services/api_key_service_test.go index ddd2d850..6f4faddc 100644 --- a/plugins/api-key/services/api_key_service_test.go +++ b/plugins/api-key/services/api_key_service_test.go @@ -113,10 +113,23 @@ func TestApiKeyServiceCreate(t *testing.T) { }, }, { - name: "org_create_privilege_escalation_rejected", - actor: &models.Actor{ID: userID, Type: models.ActorUser, Scopes: []string{"org:api-key:create"}}, - config: types.ApiKeyPluginConfig{AllowOrgKeys: true}, - req: types.CreateApiKeyRequest{Name: "Key", OwnerType: types.OwnerTypeOrganization, OwnerID: orgID, Permissions: []string{"admin"}}, + name: "org_create_privilege_escalation_rejected", + actor: &models.Actor{ID: userID, Type: models.ActorUser, Scopes: []string{"org:api-key:create"}}, + config: types.ApiKeyPluginConfig{AllowOrgKeys: true}, + req: types.CreateApiKeyRequest{Name: "Key", OwnerType: types.OwnerTypeOrganization, OwnerID: orgID, Permissions: []string{"admin"}}, + setup: func(f *apiKeyServiceFixture) { + f.mockOrgService.On("GetUserPermissionsInOrganization", mock.Anything, userID, orgID).Return([]string{"org:api-key:create"}, nil).Once() + }, + wantErr: coreerrors.ErrForbidden, + }, + { + name: "org_create_member_required", + actor: &models.Actor{ID: userID, Type: models.ActorUser, Scopes: []string{"org:api-key:create"}}, + config: types.ApiKeyPluginConfig{AllowOrgKeys: true}, + req: types.CreateApiKeyRequest{Name: "Key", OwnerType: types.OwnerTypeOrganization, OwnerID: orgID, Permissions: []string{"read"}}, + setup: func(f *apiKeyServiceFixture) { + f.mockOrgService.On("GetUserPermissionsInOrganization", mock.Anything, userID, orgID).Return(([]string)(nil), coreerrors.ErrForbidden).Once() + }, wantErr: coreerrors.ErrForbidden, }, { @@ -125,6 +138,7 @@ func TestApiKeyServiceCreate(t *testing.T) { config: types.ApiKeyPluginConfig{AllowOrgKeys: true}, req: types.CreateApiKeyRequest{Name: "Key", OwnerType: types.OwnerTypeOrganization, OwnerID: orgID, Permissions: []string{"read"}}, setup: func(f *apiKeyServiceFixture) { + f.mockOrgService.On("GetUserPermissionsInOrganization", mock.Anything, userID, orgID).Return([]string{"org:api-key:create", "read"}, nil).Once() f.mockAccessControlService.On("ValidatePermissionKeys", mock.Anything, []string{"read"}).Return(nil).Once() f.mockOrgService.On("ExistsByID", mock.Anything, orgID).Return(true, nil).Once() f.mockTokenService.On("Generate").Return(rawApiKey, nil).Once() @@ -158,6 +172,7 @@ func TestApiKeyServiceCreate(t *testing.T) { config: types.ApiKeyPluginConfig{AllowOrgKeys: true}, req: types.CreateApiKeyRequest{Name: "Key", OwnerType: types.OwnerTypeOrganization, OwnerID: orgID}, setup: func(f *apiKeyServiceFixture) { + f.mockOrgService.On("GetUserPermissionsInOrganization", mock.Anything, userID, orgID).Return([]string{"org:api-key:create"}, nil).Once() f.mockOrgService.On("ExistsByID", mock.Anything, orgID).Return(false, nil).Once() }, wantErr: coreerrors.ErrNotFound, @@ -424,6 +439,7 @@ func TestApiKeyServiceUpdate(t *testing.T) { name: "org_update_allowed", actor: &models.Actor{ID: userID, Type: models.ActorUser, Scopes: []string{"org:api-key:update", "read", "write"}}, setup: func(f *apiKeyServiceFixture) { + f.mockOrgService.On("GetUserPermissionsInOrganization", mock.Anything, userID, orgID).Return([]string{"org:api-key:update", "read", "write"}, nil).Once() f.mockAccessControlService.On("ValidatePermissionKeys", mock.Anything, permissions).Return(nil).Once() f.mockApiKeyRepo.On("GetByID", mock.Anything, "api-key-1").Return(&types.ApiKey{ID: "api-key-1", Name: "old", Enabled: true, OwnerType: types.OwnerTypeOrganization, OwnerID: orgID}, nil).Once() f.mockApiKeyRepo.On("Update", mock.Anything, mock.MatchedBy(func(apiKey *types.ApiKey) bool { diff --git a/plugins/api-key/services/interfaces.go b/plugins/api-key/services/interfaces.go index 03747779..79ff2177 100644 --- a/plugins/api-key/services/interfaces.go +++ b/plugins/api-key/services/interfaces.go @@ -20,3 +20,8 @@ type ApiKeyService interface { Verify(ctx context.Context, req types.VerifyApiKeyRequest) (*types.VerifyApiKeyResult, error) ValidatePermissionKeys(ctx context.Context, permissionKeys []string) error } + +type OrganizationLookupService interface { + ExistsByID(ctx context.Context, organizationID string) (bool, error) + GetUserPermissionsInOrganization(ctx context.Context, userID string, organizationID string) ([]string, error) +} diff --git a/plugins/api-key/tests/mocks.go b/plugins/api-key/tests/mocks.go index 7e7b4544..72eca9f0 100644 --- a/plugins/api-key/tests/mocks.go +++ b/plugins/api-key/tests/mocks.go @@ -24,9 +24,17 @@ func (m *MockAccessControlService) RoleExists(ctx context.Context, roleName stri return args.Bool(0), args.Error(1) } -func (m *MockAccessControlService) ValidateRoleAssignment(ctx context.Context, roleName string, assignerUserID *string) (bool, error) { - args := m.Called(ctx, roleName, assignerUserID) - return args.Bool(0), args.Error(1) +func (m *MockAccessControlService) GetRolePermissionsByName(ctx context.Context, roleName string) ([]string, error) { + args := m.Called(ctx, roleName) + if args.Get(0) == nil { + return nil, args.Error(1) + } + return args.Get(0).([]string), args.Error(1) +} + +func (m *MockAccessControlService) GetRoleWeightByName(ctx context.Context, roleName string) (int, error) { + args := m.Called(ctx, roleName) + return args.Int(0), args.Error(1) } func (m *MockAccessControlService) ValidatePermissionKeys(ctx context.Context, permissionKeys []string) error { diff --git a/plugins/organizations/api.go b/plugins/organizations/api.go index f6fa70c2..eead1884 100644 --- a/plugins/organizations/api.go +++ b/plugins/organizations/api.go @@ -3,18 +3,25 @@ package organizations import ( "context" + coreerrors "github.com/Authula/authula/core/errors" "github.com/Authula/authula/models" + "github.com/Authula/authula/plugins/organizations/repositories" "github.com/Authula/authula/plugins/organizations/types" "github.com/Authula/authula/plugins/organizations/usecases" + rootservices "github.com/Authula/authula/services" ) type API struct { - useCases *usecases.UseCases + useCases *usecases.UseCases + memberRepo repositories.OrganizationMemberRepository + accessControlService rootservices.AccessControlService } func BuildAPI(plugin *OrganizationsPlugin) *API { return &API{ - useCases: plugin.useCases, + useCases: plugin.useCases, + memberRepo: plugin.memberRepo, + accessControlService: plugin.accessControlService, } } @@ -24,6 +31,22 @@ func (a *API) ExistsByID(ctx context.Context, organizationID string) (bool, erro return a.useCases.ExistsByID(ctx, organizationID) } +func (a *API) GetUserPermissionsInOrganization(ctx context.Context, userID string, organizationID string) ([]string, error) { + if userID == "" || organizationID == "" { + return nil, coreerrors.ErrUnauthorized + } + + member, err := a.memberRepo.GetByOrganizationIDAndUserID(ctx, organizationID, userID) + if err != nil { + return nil, err + } + if member == nil { + return nil, coreerrors.ErrForbidden + } + + return a.accessControlService.GetRolePermissionsByName(ctx, member.Role) +} + func (a *API) CreateOrganization(ctx context.Context, actor *models.Actor, request types.CreateOrganizationRequest) (*types.Organization, error) { return a.useCases.CreateOrganization(ctx, actor, request) } diff --git a/plugins/organizations/handlers/handler_test_helpers.go b/plugins/organizations/handlers/handler_test_helpers.go index 0b8fb8fc..bda1a130 100644 --- a/plugins/organizations/handlers/handler_test_helpers.go +++ b/plugins/organizations/handlers/handler_test_helpers.go @@ -9,6 +9,7 @@ import ( "github.com/Authula/authula/models" orgservices "github.com/Authula/authula/plugins/organizations/services" orgtests "github.com/Authula/authula/plugins/organizations/tests" + orgtypes "github.com/Authula/authula/plugins/organizations/types" orgusecases "github.com/Authula/authula/plugins/organizations/usecases" ) @@ -29,6 +30,18 @@ func defaultMockUserService() *internaltests.MockUserService { return svc } +func defaultServiceUtils() *orgservices.ServiceUtils { + orgRepo := &orgtests.MockOrganizationRepository{} + memberRepo := &orgtests.MockOrganizationMemberRepository{} + orgRepo.On("GetByID", mock.Anything, mock.Anything).Return(&orgtypes.Organization{ID: "org-1", OwnerID: "user-1"}, nil).Maybe() + memberRepo.On("GetByOrganizationIDAndUserID", mock.Anything, mock.Anything, mock.Anything).Return(&orgtypes.OrganizationMember{ID: "mem-1", OrganizationID: "org-1", UserID: "user-1", Role: "admin"}, nil).Maybe() + return orgservices.NewServiceUtils(orgRepo, memberRepo, nil, nil) +} + +func defaultAccessControlService() *orgtests.AccessControlServiceStub { + return orgtests.NewAccessControlServiceStub() +} + func newOrgUseCases(svc orgservices.OrganizationService) *orgusecases.UseCases { return orgusecases.NewUseCases( svc, @@ -39,6 +52,8 @@ func newOrgUseCases(svc orgservices.OrganizationService) *orgusecases.UseCases { defaultMockUserService(), &models.Config{}, &noopAuthorizer{}, + defaultServiceUtils(), + defaultAccessControlService(), ) } @@ -52,6 +67,8 @@ func newInvitationUseCases(orgSvc orgservices.OrganizationService, svc orgservic defaultMockUserService(), &models.Config{}, &noopAuthorizer{}, + defaultServiceUtils(), + defaultAccessControlService(), ) } @@ -65,6 +82,8 @@ func newMemberUseCases(svc orgservices.OrganizationMemberService) *orgusecases.U defaultMockUserService(), &models.Config{}, &noopAuthorizer{}, + defaultServiceUtils(), + defaultAccessControlService(), ) } @@ -78,6 +97,8 @@ func newTeamUseCases(svc orgservices.OrganizationTeamService) *orgusecases.UseCa defaultMockUserService(), &models.Config{}, &noopAuthorizer{}, + defaultServiceUtils(), + defaultAccessControlService(), ) } @@ -91,5 +112,7 @@ func newTeamMemberUseCases(svc orgservices.OrganizationTeamMemberService) *orgus defaultMockUserService(), &models.Config{}, &noopAuthorizer{}, + defaultServiceUtils(), + defaultAccessControlService(), ) } diff --git a/plugins/organizations/handlers/organization_handlers.go b/plugins/organizations/handlers/organization_handlers.go index 2bdea9a3..e4de1d9c 100644 --- a/plugins/organizations/handlers/organization_handlers.go +++ b/plugins/organizations/handlers/organization_handlers.go @@ -38,12 +38,6 @@ func (h *CreateOrganizationHandler) Handle() http.HandlerFunc { return } - reqCtx.Values[models.ContextAccessControlAssignRole.String()] = &models.AccessControlAssignRoleContext{ - UserID: actor.ID, - RoleName: request.Role, - AssignerUserID: nil, - } - reqCtx.SetJSONResponse(http.StatusCreated, organization) } } diff --git a/plugins/organizations/handlers/organization_handlers_test.go b/plugins/organizations/handlers/organization_handlers_test.go index 6df18e20..dfaadbb4 100644 --- a/plugins/organizations/handlers/organization_handlers_test.go +++ b/plugins/organizations/handlers/organization_handlers_test.go @@ -139,13 +139,6 @@ func TestCreateOrganizationHandler(t *testing.T) { assert.Equal(t, "user-1", org.OwnerID) assert.Equal(t, "Acme Inc", org.Name) assert.Equal(t, "acme-inc", org.Slug) - assignRoleValue, ok := reqCtx.Values[models.ContextAccessControlAssignRole.String()] - require.True(t, ok) - assignRoleCtx, ok := assignRoleValue.(*models.AccessControlAssignRoleContext) - require.True(t, ok) - assert.Equal(t, "user-1", assignRoleCtx.UserID) - assert.Equal(t, "member", assignRoleCtx.RoleName) - assert.Nil(t, assignRoleCtx.AssignerUserID) }, }, } diff --git a/plugins/organizations/handlers/organization_invitation_handlers.go b/plugins/organizations/handlers/organization_invitation_handlers.go index f7d23a9d..d87dac8f 100644 --- a/plugins/organizations/handlers/organization_invitation_handlers.go +++ b/plugins/organizations/handlers/organization_invitation_handlers.go @@ -129,12 +129,6 @@ func (h *AcceptOrganizationInvitationHandler) Handle() http.HandlerFunc { return } - reqCtx.Values[models.ContextAccessControlAssignRole.String()] = &models.AccessControlAssignRoleContext{ - UserID: actor.ID, - RoleName: invitation.Role, - AssignerUserID: &invitation.InviterID, - } - redirectURL := r.URL.Query().Get("redirect_url") if redirectURL != "" { validatedURL, err := util.IsTrustedCallbackURL(redirectURL, h.TrustedOrigins) diff --git a/plugins/organizations/handlers/organization_invitation_handlers_test.go b/plugins/organizations/handlers/organization_invitation_handlers_test.go index da53002d..2d098d16 100644 --- a/plugins/organizations/handlers/organization_invitation_handlers_test.go +++ b/plugins/organizations/handlers/organization_invitation_handlers_test.go @@ -351,13 +351,6 @@ func TestAcceptOrganizationInvitationHandler(t *testing.T) { invitation := internaltests.DecodeResponseJSON[orgtypes.OrganizationInvitation](t, reqCtx) assert.Equal(t, "inv-1", invitation.ID) assert.Equal(t, orgtypes.OrganizationInvitationStatusAccepted, invitation.Status) - assignRoleValue, ok := reqCtx.Values[models.ContextAccessControlAssignRole.String()] - require.True(t, ok) - assignRoleCtx, ok := assignRoleValue.(*models.AccessControlAssignRoleContext) - require.True(t, ok) - assert.Equal(t, "user-1", assignRoleCtx.UserID) - assert.Equal(t, "member", assignRoleCtx.RoleName) - assert.Equal(t, "user-2", *assignRoleCtx.AssignerUserID) }, }, }) diff --git a/plugins/organizations/handlers/organization_member_handlers.go b/plugins/organizations/handlers/organization_member_handlers.go index 8ba7c4c6..d75889eb 100644 --- a/plugins/organizations/handlers/organization_member_handlers.go +++ b/plugins/organizations/handlers/organization_member_handlers.go @@ -40,12 +40,6 @@ func (h *AddOrganizationMemberHandler) Handle() http.HandlerFunc { return } - reqCtx.Values[models.ContextAccessControlAssignRole.String()] = &models.AccessControlAssignRoleContext{ - UserID: request.UserID, - RoleName: request.Role, - AssignerUserID: &actor.ID, - } - reqCtx.SetJSONResponse(http.StatusCreated, member) } } @@ -148,12 +142,6 @@ func (h *UpdateOrganizationMemberHandler) Handle() http.HandlerFunc { return } - reqCtx.Values[models.ContextAccessControlAssignRole.String()] = &models.AccessControlAssignRoleContext{ - UserID: member.UserID, - RoleName: request.Role, - AssignerUserID: &actor.ID, - } - reqCtx.SetJSONResponse(http.StatusOK, member) } } diff --git a/plugins/organizations/handlers/organization_member_handlers_test.go b/plugins/organizations/handlers/organization_member_handlers_test.go index 22e6fa9d..67e86478 100644 --- a/plugins/organizations/handlers/organization_member_handlers_test.go +++ b/plugins/organizations/handlers/organization_member_handlers_test.go @@ -148,13 +148,6 @@ func TestAddOrganizationMemberHandler(t *testing.T) { assert.Equal(t, "mem-1", member.ID) assert.Equal(t, "org-1", member.OrganizationID) assert.Equal(t, "user-2", member.UserID) - assignRoleValue, ok := reqCtx.Values[models.ContextAccessControlAssignRole.String()] - assert.True(t, ok) - assignRoleCtx, ok := assignRoleValue.(*models.AccessControlAssignRoleContext) - assert.True(t, ok) - assert.Equal(t, "user-2", assignRoleCtx.UserID) - assert.Equal(t, "member", assignRoleCtx.RoleName) - assert.Equal(t, "user-1", *assignRoleCtx.AssignerUserID) }, }, }) @@ -329,13 +322,6 @@ func TestUpdateOrganizationMemberHandler(t *testing.T) { checkResponse: func(t *testing.T, reqCtx *models.RequestContext) { member := internaltests.DecodeResponseJSON[orgtypes.OrganizationMember](t, reqCtx) assert.Equal(t, "admin", member.Role) - assignRoleValue, ok := reqCtx.Values[models.ContextAccessControlAssignRole.String()] - assert.True(t, ok) - assignRoleCtx, ok := assignRoleValue.(*models.AccessControlAssignRoleContext) - assert.True(t, ok) - assert.Equal(t, "user-2", assignRoleCtx.UserID) - assert.Equal(t, "admin", assignRoleCtx.RoleName) - assert.Equal(t, "user-1", *assignRoleCtx.AssignerUserID) }, }, }) diff --git a/plugins/organizations/plugin.go b/plugins/organizations/plugin.go index be802c23..f3307397 100644 --- a/plugins/organizations/plugin.go +++ b/plugins/organizations/plugin.go @@ -105,7 +105,7 @@ func (p *OrganizationsPlugin) Init(ctx *models.PluginContext) error { p.teamMemberService = services.NewOrganizationTeamMemberService(p.organizationRepo, p.memberRepo, p.teamRepo, p.teamMemberRepo, p.serviceUtils, p.hooksExecutor) authorizer := rootservices.NewDefaultAuthorizer() - p.useCases = usecases.NewUseCases(p.organizationService, p.invitationService, p.memberService, p.teamService, p.teamMemberService, userService, p.globalConfig, authorizer) + p.useCases = usecases.NewUseCases(p.organizationService, p.invitationService, p.memberService, p.teamService, p.teamMemberService, userService, p.globalConfig, authorizer, p.serviceUtils, accessControlService) p.Api = BuildAPI(p) diff --git a/plugins/organizations/routes.go b/plugins/organizations/routes.go index 4d8dbee5..2c01b23a 100644 --- a/plugins/organizations/routes.go +++ b/plugins/organizations/routes.go @@ -88,6 +88,7 @@ func Routes(plugin *OrganizationsPlugin) []models.Route { Path: "/organizations/{organization_id}/invitations", Middleware: []func(http.Handler) http.Handler{ middleware.RequireAuthenticated(), + middleware.RequireActor(models.ActorUser), }, Handler: createInvitationHandler.Handle(), }, @@ -137,6 +138,7 @@ func Routes(plugin *OrganizationsPlugin) []models.Route { Path: "/organizations/{organization_id}/members", Middleware: []func(http.Handler) http.Handler{ middleware.RequireAuthenticated(), + middleware.RequireActor(models.ActorUser), }, Handler: addMemberHandler.Handle(), }, @@ -169,6 +171,7 @@ func Routes(plugin *OrganizationsPlugin) []models.Route { Path: "/organizations/{organization_id}/members/{member_id}", Middleware: []func(http.Handler) http.Handler{ middleware.RequireAuthenticated(), + middleware.RequireActor(models.ActorUser), }, Handler: updateMemberHandler.Handle(), }, diff --git a/plugins/organizations/service.go b/plugins/organizations/service.go index 4ff4a258..8603e60c 100644 --- a/plugins/organizations/service.go +++ b/plugins/organizations/service.go @@ -4,4 +4,5 @@ import "context" type OrganizationLookupService interface { ExistsByID(ctx context.Context, organizationID string) (bool, error) + GetUserPermissionsInOrganization(ctx context.Context, userID string, organizationID string) ([]string, error) } diff --git a/plugins/organizations/services/organization_invitation_service.go b/plugins/organizations/services/organization_invitation_service.go index ca6f1e5f..dfd60839 100644 --- a/plugins/organizations/services/organization_invitation_service.go +++ b/plugins/organizations/services/organization_invitation_service.go @@ -88,29 +88,23 @@ func (s *organizationInvitationService) CreateOrganizationInvitation(ctx context } actorID := actor.ID - organization, _, err := s.serviceUtils.authorizeOrganizationAccess(ctx, actor, organizationID) + organization, actorMember, err := s.serviceUtils.AuthorizeOrganizationAccess(ctx, actor, organizationID) if err != nil { return nil, err } + if actor.Type == models.ActorMachine { + return nil, coreerrors.ErrForbidden + } + role := request.Role if role == "" { return nil, coreerrors.ErrUnprocessableEntity } - validatedRoleAssignment, err := s.accessControlService.ValidateRoleAssignment(ctx, role, &actorID) - if err != nil { - if err.Error() == coreerrors.ErrForbidden.Error() { - return nil, coreerrors.ErrForbidden - } - if err.Error() == coreerrors.ErrNotFound.Error() { - return nil, coreerrors.ErrUnprocessableEntity - } + if err := authorizeRoleWeight(ctx, s.accessControlService, actorMember, role); err != nil { return nil, err } - if !validatedRoleAssignment { - return nil, coreerrors.ErrUnprocessableEntity - } expiresAt := time.Now().UTC().Add(s.pluginConfig.InvitationExpiresIn) if !expiresAt.After(time.Now().UTC()) { @@ -271,7 +265,7 @@ func (s *organizationInvitationService) GetAllOrganizationInvitations(ctx contex return nil, coreerrors.ErrUnauthorized } - if _, _, err := s.serviceUtils.authorizeOrganizationAccess(ctx, actor, organizationID); err != nil { + if _, _, err := s.serviceUtils.AuthorizeOrganizationAccess(ctx, actor, organizationID); err != nil { return nil, err } @@ -288,7 +282,7 @@ func (s *organizationInvitationService) GetOrganizationInvitation(ctx context.Co return nil, coreerrors.ErrUnauthorized } - if _, _, err := s.serviceUtils.authorizeOrganizationAccess(ctx, actor, organizationID); err != nil { + if _, _, err := s.serviceUtils.AuthorizeOrganizationAccess(ctx, actor, organizationID); err != nil { return nil, err } @@ -353,7 +347,7 @@ func (s *organizationInvitationService) RevokeOrganizationInvitation(ctx context return nil, coreerrors.ErrUnauthorized } - if _, _, err := s.serviceUtils.authorizeOrganizationAccess(ctx, actor, organizationID); err != nil { + if _, _, err := s.serviceUtils.AuthorizeOrganizationAccess(ctx, actor, organizationID); err != nil { return nil, err } diff --git a/plugins/organizations/services/organization_invitation_service_test.go b/plugins/organizations/services/organization_invitation_service_test.go index 1585cc35..f0e87723 100644 --- a/plugins/organizations/services/organization_invitation_service_test.go +++ b/plugins/organizations/services/organization_invitation_service_test.go @@ -152,31 +152,7 @@ func TestOrganizationInvitationService_CreateOrganizationInvitation(t *testing.T setup: func(orgRepo *orgtests.MockOrganizationRepository, invRepo *orgtests.MockOrganizationInvitationRepository, memberRepo *orgtests.MockOrganizationMemberRepository, hooks *orgtests.MockOrganizationInvitationHooks) { orgRepo.On("GetByID", mock.Anything, "org-1").Return(&types.Organization{ID: "org-1", OwnerID: "user-1"}, nil).Once() }, - expectErr: coreerrors.ErrUnprocessableEntity, - }, - { - name: "higher role is forbidden", - actorUserID: "user-1", - organizationID: "org-1", - invitationExpiresIn: 36 * time.Hour, - request: types.CreateOrganizationInvitationRequest{Email: "user@example.com", Role: "manager"}, - accessControlService: orgtests.NewAccessControlServiceStubWithWeights(nil, map[string]int{"user-1": 10}), - setup: func(orgRepo *orgtests.MockOrganizationRepository, invRepo *orgtests.MockOrganizationInvitationRepository, memberRepo *orgtests.MockOrganizationMemberRepository, hooks *orgtests.MockOrganizationInvitationHooks) { - orgRepo.On("GetByID", mock.Anything, "org-1").Return(&types.Organization{ID: "org-1", OwnerID: "owner-1"}, nil).Once() - }, - expectErr: coreerrors.ErrForbidden, - }, - { - name: "access control forbidden is normalized", - actorUserID: "user-1", - organizationID: "org-1", - invitationExpiresIn: 36 * time.Hour, - request: types.CreateOrganizationInvitationRequest{Email: "user@example.com", Role: "manager"}, - accessControlService: &orgtests.AccessControlServiceStub{Err: errors.New("forbidden")}, - setup: func(orgRepo *orgtests.MockOrganizationRepository, invRepo *orgtests.MockOrganizationInvitationRepository, memberRepo *orgtests.MockOrganizationMemberRepository, hooks *orgtests.MockOrganizationInvitationHooks) { - orgRepo.On("GetByID", mock.Anything, "org-1").Return(&types.Organization{ID: "org-1", OwnerID: "owner-1"}, nil).Once() - }, - expectErr: coreerrors.ErrForbidden, + expectErr: coreerrors.ErrBadRequest, }, { name: "forbidden for non owner", @@ -185,6 +161,7 @@ func TestOrganizationInvitationService_CreateOrganizationInvitation(t *testing.T request: types.CreateOrganizationInvitationRequest{Email: "user@example.com", Role: "member"}, setup: func(orgRepo *orgtests.MockOrganizationRepository, invRepo *orgtests.MockOrganizationInvitationRepository, memberRepo *orgtests.MockOrganizationMemberRepository, hooks *orgtests.MockOrganizationInvitationHooks) { orgRepo.On("GetByID", mock.Anything, "org-1").Return(&types.Organization{ID: "org-1", OwnerID: "owner-1"}, nil).Once() + memberRepo.On("GetByOrganizationIDAndUserID", mock.Anything, "org-1", "user-1").Return(nil, nil).Once() }, expectErr: coreerrors.ErrForbidden, }, @@ -566,6 +543,7 @@ func TestOrganizationInvitationService_CreateOrganizationInvitation(t *testing.T if tt.setup != nil { tt.setup(orgRepo, orgInvitationRepo, memberRepo, orgInvitationHooks) } + expectActorMember(memberRepo, tt.organizationID, tt.actorUserID) userSvc := &internaltests.MockUserService{} if tt.userSetup != nil { tt.userSetup(userSvc) @@ -722,6 +700,7 @@ func TestOrganizationInvitationService_GetOrganizationInvitation(t *testing.T) { if tt.setup != nil { tt.setup(orgRepo, invRepo, memberRepo) } + expectActorMember(memberRepo, tt.organizationID, tt.actorUserID) svc := newTestOrganizationInvitationService(&orgtests.MockOrganizationInvitationTxRunner{}, pluginConfig, &internaltests.MockUserService{}, orgtests.NewAccessControlServiceStub(), orgRepo, invRepo, memberRepo) invitation, err := svc.GetOrganizationInvitation(context.Background(), orgtests.Actor(tt.actorUserID), tt.organizationID, tt.invitationID) @@ -841,6 +820,7 @@ func TestOrganizationInvitationService_GetAllOrganizationInvitations(t *testing. if tt.setup != nil { tt.setup(orgRepo, invRepo, memberRepo) } + expectActorMember(memberRepo, tt.organizationID, tt.actorUserID) svc := newTestOrganizationInvitationService(&orgtests.MockOrganizationInvitationTxRunner{}, pluginConfig, &internaltests.MockUserService{}, orgtests.NewAccessControlServiceStub(), orgRepo, invRepo, memberRepo) invitations, err := svc.GetAllOrganizationInvitations(context.Background(), orgtests.Actor(tt.actorUserID), tt.organizationID) @@ -949,6 +929,7 @@ func TestOrganizationInvitationService_RevokeOrganizationInvitation(t *testing.T if tt.setup != nil { tt.setup(orgRepo, invRepo, memberRepo, hooks) } + expectActorMember(memberRepo, tt.organizationID, tt.actorUserID) svc := newTestOrganizationInvitationService(&orgtests.MockOrganizationInvitationTxRunner{}, pluginConfig, &internaltests.MockUserService{}, orgtests.NewAccessControlServiceStub(), orgRepo, invRepo, memberRepo) invitation, err := svc.RevokeOrganizationInvitation(context.Background(), orgtests.Actor(tt.actorUserID), tt.organizationID, tt.invitationID) @@ -1463,3 +1444,20 @@ func TestOrganizationInvitationService_RejectOrganizationInvitation(t *testing.T }) } } + +func TestOrganizationInvitationService_MachineActorForbidden(t *testing.T) { + t.Parallel() + + actor := &models.Actor{ID: "key-1", Type: models.ActorMachine, Claims: map[string]any{"organization_id": "org-1"}} + + orgRepo := &orgtests.MockOrganizationRepository{} + invRepo := &orgtests.MockOrganizationInvitationRepository{} + memberRepo := &orgtests.MockOrganizationMemberRepository{} + orgRepo.On("GetByID", mock.Anything, "org-1").Return(&types.Organization{ID: "org-1", OwnerID: "user-1"}, nil).Once() + + pluginConfig := &types.OrganizationsPluginConfig{Enabled: true, InvitationExpiresIn: time.Hour} + svc := newTestOrganizationInvitationService(&orgtests.MockOrganizationInvitationTxRunner{}, pluginConfig, &internaltests.MockUserService{}, orgtests.NewAccessControlServiceStub(), orgRepo, invRepo, memberRepo) + _, err := svc.CreateOrganizationInvitation(context.Background(), actor, "org-1", types.CreateOrganizationInvitationRequest{Email: "user@example.com", Role: "member"}, "") + require.ErrorIs(t, err, coreerrors.ErrForbidden) + orgRepo.AssertExpectations(t) +} diff --git a/plugins/organizations/services/organization_member_service.go b/plugins/organizations/services/organization_member_service.go index 086211ab..5ce125db 100644 --- a/plugins/organizations/services/organization_member_service.go +++ b/plugins/organizations/services/organization_member_service.go @@ -3,6 +3,7 @@ package services import ( "context" "database/sql" + "errors" "github.com/uptrace/bun" @@ -39,10 +40,15 @@ func NewOrganizationMemberService(userService rootservices.UserService, accessCo } func (s *organizationMemberService) AddMember(ctx context.Context, actor *models.Actor, organizationID string, request types.AddOrganizationMemberRequest) (*types.OrganizationMember, error) { - if _, _, err := s.serviceUtils.authorizeOrganizationAccess(ctx, actor, organizationID); err != nil { + _, actorMember, err := s.serviceUtils.AuthorizeOrganizationAccess(ctx, actor, organizationID) + if err != nil { return nil, err } + if actor.Type == models.ActorMachine { + return nil, coreerrors.ErrForbidden + } + userID := request.UserID if userID == "" { return nil, coreerrors.ErrUnprocessableEntity @@ -67,20 +73,9 @@ func (s *organizationMemberService) AddMember(ctx context.Context, actor *models return nil, coreerrors.ErrConflict } - actorID := actor.ID - validatedRoleAssignment, err := s.accessControlService.ValidateRoleAssignment(ctx, role, &actorID) - if err != nil { - if err.Error() == coreerrors.ErrForbidden.Error() { - return nil, coreerrors.ErrForbidden - } - if err.Error() == coreerrors.ErrNotFound.Error() { - return nil, coreerrors.ErrBadRequest - } + if err := authorizeRoleWeight(ctx, s.accessControlService, actorMember, role); err != nil { return nil, err } - if !validatedRoleAssignment { - return nil, coreerrors.ErrBadRequest - } member := &types.OrganizationMember{ ID: util.GenerateUUID(), @@ -124,7 +119,7 @@ func (s *organizationMemberService) AddMember(ctx context.Context, actor *models } func (s *organizationMemberService) GetAllMembers(ctx context.Context, actor *models.Actor, organizationID string, page int, limit int) ([]types.OrganizationMemberResponse, error) { - if _, _, err := s.serviceUtils.authorizeOrganizationAccess(ctx, actor, organizationID); err != nil { + if _, _, err := s.serviceUtils.AuthorizeOrganizationAccess(ctx, actor, organizationID); err != nil { return nil, err } @@ -132,7 +127,7 @@ func (s *organizationMemberService) GetAllMembers(ctx context.Context, actor *mo } func (s *organizationMemberService) GetMember(ctx context.Context, actor *models.Actor, organizationID string, memberID string) (*types.OrganizationMemberResponse, error) { - if _, _, err := s.serviceUtils.authorizeOrganizationAccess(ctx, actor, organizationID); err != nil { + if _, _, err := s.serviceUtils.AuthorizeOrganizationAccess(ctx, actor, organizationID); err != nil { return nil, err } @@ -152,7 +147,7 @@ func (s *organizationMemberService) GetMember(ctx context.Context, actor *models } func (s *organizationMemberService) GetMemberByUserID(ctx context.Context, actor *models.Actor, organizationID string, userID string) (*types.OrganizationMemberResponse, error) { - if _, _, err := s.serviceUtils.authorizeOrganizationAccess(ctx, actor, organizationID); err != nil { + if _, _, err := s.serviceUtils.AuthorizeOrganizationAccess(ctx, actor, organizationID); err != nil { return nil, err } @@ -172,11 +167,15 @@ func (s *organizationMemberService) GetMemberByUserID(ctx context.Context, actor } func (s *organizationMemberService) UpdateMember(ctx context.Context, actor *models.Actor, organizationID string, memberID string, request types.UpdateOrganizationMemberRequest) (*types.OrganizationMember, error) { - _, actorMember, err := s.serviceUtils.authorizeOrganizationAccess(ctx, actor, organizationID) + _, actorMember, err := s.serviceUtils.AuthorizeOrganizationAccess(ctx, actor, organizationID) if err != nil { return nil, err } + if actor.Type == models.ActorMachine { + return nil, coreerrors.ErrForbidden + } + member, err := s.orgMemberRepo.GetByID(ctx, memberID) if err != nil { return nil, err @@ -190,20 +189,9 @@ func (s *organizationMemberService) UpdateMember(ctx context.Context, actor *mod return nil, coreerrors.ErrBadRequest } - actorID := actor.ID - validatedRoleAssignment, err := s.accessControlService.ValidateRoleAssignment(ctx, role, &actorID) - if err != nil { - if err.Error() == coreerrors.ErrForbidden.Error() { - return nil, coreerrors.ErrForbidden - } - if err.Error() == coreerrors.ErrNotFound.Error() { - return nil, coreerrors.ErrBadRequest - } + if err := authorizeRoleWeight(ctx, s.accessControlService, actorMember, role); err != nil { return nil, err } - if !validatedRoleAssignment { - return nil, coreerrors.ErrBadRequest - } if actorMember != nil && actorMember.UserID == member.UserID { return nil, coreerrors.ErrForbidden @@ -232,7 +220,8 @@ func (s *organizationMemberService) UpdateMember(ctx context.Context, actor *mod } func (s *organizationMemberService) RemoveMember(ctx context.Context, actor *models.Actor, organizationID string, memberID string) error { - if _, _, err := s.serviceUtils.authorizeOrganizationAccess(ctx, actor, organizationID); err != nil { + organization, actorMember, err := s.serviceUtils.AuthorizeOrganizationAccess(ctx, actor, organizationID) + if err != nil { return err } @@ -244,6 +233,14 @@ func (s *organizationMemberService) RemoveMember(ctx context.Context, actor *mod return coreerrors.ErrNotFound } + if err := authorizeRemoveMemberWeight(ctx, s.accessControlService, actorMember, member); err != nil { + return err + } + + if member.UserID == organization.OwnerID && actor.ID != organization.OwnerID { + return coreerrors.ErrForbidden + } + if s.hooks != nil { if err := s.hooks.BeforeDeleteOrganizationMember(ctx, actor, member); err != nil { return err @@ -263,6 +260,33 @@ func (s *organizationMemberService) RemoveMember(ctx context.Context, actor *mod return nil } +func authorizeRemoveMemberWeight(ctx context.Context, accessControl rootservices.AccessControlService, actorMember, targetMember *types.OrganizationMember) error { + if actorMember == nil { + return coreerrors.ErrForbidden + } + + actorWeight, err := accessControl.GetRoleWeightByName(ctx, actorMember.Role) + if err != nil { + if errors.Is(err, coreerrors.ErrNotFound) { + return coreerrors.ErrForbidden + } + return err + } + + targetWeight, err := accessControl.GetRoleWeightByName(ctx, targetMember.Role) + if err != nil { + if errors.Is(err, coreerrors.ErrNotFound) { + return coreerrors.ErrForbidden + } + return err + } + + if targetWeight > actorWeight { + return coreerrors.ErrForbidden + } + return nil +} + func ensureOrganizationMembersLimit(ctx context.Context, memberRepo repositories.OrganizationMemberRepository, organizationID string, membersLimit *int) error { if membersLimit == nil || *membersLimit <= 0 { return nil diff --git a/plugins/organizations/services/organization_member_service_test.go b/plugins/organizations/services/organization_member_service_test.go index dc4f544a..cfd86842 100644 --- a/plugins/organizations/services/organization_member_service_test.go +++ b/plugins/organizations/services/organization_member_service_test.go @@ -23,6 +23,13 @@ func newTestOrganizationMemberService(userSvc *internaltests.MockUserService, ac return NewOrganizationMemberService(userSvc, accessControlService, orgRepo, memberRepo, membersLimit, &orgtests.MockTxRunner{}, serviceUtils) } +func expectActorMember(memberRepo *orgtests.MockOrganizationMemberRepository, organizationID, actorUserID string) { + if actorUserID == "" || organizationID == "" { + return + } + memberRepo.On("GetByOrganizationIDAndUserID", mock.Anything, organizationID, actorUserID).Return(&types.OrganizationMember{ID: "mem-actor", OrganizationID: organizationID, UserID: actorUserID, Role: "admin"}, nil).Maybe() +} + func TestOrganizationMemberService_AddMember(t *testing.T) { t.Parallel() @@ -174,20 +181,6 @@ func TestOrganizationMemberService_AddMember(t *testing.T) { }, expectErr: coreerrors.ErrForbidden, }, - { - name: "access control forbidden is normalized", - actorUserID: "user-2", - organizationID: "org-1", - request: types.AddOrganizationMemberRequest{UserID: "user-3", Role: "manager"}, - accessControlService: &orgtests.AccessControlServiceStub{Err: errors.New("forbidden")}, - setup: func(orgRepo *orgtests.MockOrganizationRepository, memberRepo *orgtests.MockOrganizationMemberRepository, userSvc *internaltests.MockUserService, hooks *orgtests.MockOrganizationMemberHooks) { - orgRepo.On("GetByID", mock.Anything, "org-1").Return(&types.Organization{ID: "org-1", OwnerID: "owner-1"}, nil).Once() - memberRepo.On("GetByOrganizationIDAndUserID", mock.Anything, "org-1", "user-2").Return(&types.OrganizationMember{ID: "mem-1", OrganizationID: "org-1", UserID: "user-2", Role: "member"}, nil).Once() - userSvc.On("GetByID", mock.Anything, "user-3").Return(&models.User{ID: "user-3", Email: "user3@example.com"}, nil).Once() - memberRepo.On("GetByOrganizationIDAndUserID", mock.Anything, "org-1", "user-3").Return(nil, nil).Once() - }, - expectErr: coreerrors.ErrForbidden, - }, { name: "user lookup error", actorUserID: "user-1", @@ -276,6 +269,7 @@ func TestOrganizationMemberService_AddMember(t *testing.T) { if tt.setup != nil { tt.setup(orgRepo, memberRepo, userSvc, hooks) } + expectActorMember(memberRepo, tt.organizationID, tt.actorUserID) accessControlService := tt.accessControlService if accessControlService == nil { @@ -379,6 +373,7 @@ func TestOrganizationMemberService_GetAllMembers(t *testing.T) { if tt.setup != nil { tt.setup(orgRepo, memberRepo) } + expectActorMember(memberRepo, tt.organizationID, tt.actorUserID) svc := newTestOrganizationMemberService(userService, orgtests.NewAccessControlServiceStub(), orgRepo, memberRepo, nil) members, err := svc.GetAllMembers(context.Background(), orgtests.Actor(tt.actorUserID), tt.organizationID, 1, 10) @@ -520,6 +515,7 @@ func TestOrganizationMemberService_GetMember(t *testing.T) { if tt.setup != nil { tt.setup(orgRepo, memberRepo) } + expectActorMember(memberRepo, tt.organizationID, tt.actorUserID) svc := newTestOrganizationMemberService(userService, orgtests.NewAccessControlServiceStub(), orgRepo, memberRepo, nil) member, err := svc.GetMember(context.Background(), orgtests.Actor(tt.actorUserID), tt.organizationID, tt.memberID) @@ -640,6 +636,7 @@ func TestOrganizationMemberService_GetMemberByUserID(t *testing.T) { if tt.setup != nil { tt.setup(orgRepo, memberRepo) } + expectActorMember(memberRepo, tt.organizationID, tt.actorUserID) svc := newTestOrganizationMemberService(userService, orgtests.NewAccessControlServiceStub(), orgRepo, memberRepo, nil) member, err := svc.GetMemberByUserID(context.Background(), orgtests.Actor(tt.actorUserID), tt.organizationID, tt.userID) @@ -785,20 +782,6 @@ func TestOrganizationMemberService_UpdateMember(t *testing.T) { }, expectErr: coreerrors.ErrForbidden, }, - { - name: "access control forbidden is normalized", - actorUserID: "user-2", - organizationID: "org-1", - memberID: "mem-1", - request: types.UpdateOrganizationMemberRequest{Role: "manager"}, - accessControlService: &orgtests.AccessControlServiceStub{Err: errors.New("forbidden")}, - setup: func(orgRepo *orgtests.MockOrganizationRepository, memberRepo *orgtests.MockOrganizationMemberRepository, hooks *orgtests.MockOrganizationMemberHooks) { - orgRepo.On("GetByID", mock.Anything, "org-1").Return(&types.Organization{ID: "org-1", OwnerID: "owner-1"}, nil).Once() - memberRepo.On("GetByOrganizationIDAndUserID", mock.Anything, "org-1", "user-2").Return(&types.OrganizationMember{ID: "mem-actor", OrganizationID: "org-1", UserID: "user-2", Role: "member"}, nil).Once() - memberRepo.On("GetByID", mock.Anything, "mem-1").Return(&types.OrganizationMember{ID: "mem-1", OrganizationID: "org-1", UserID: "user-3", Role: "member"}, nil).Once() - }, - expectErr: coreerrors.ErrForbidden, - }, { name: "update error", actorUserID: "user-1", @@ -842,6 +825,7 @@ func TestOrganizationMemberService_UpdateMember(t *testing.T) { if tt.setup != nil { tt.setup(orgRepo, memberRepo, hooks) } + expectActorMember(memberRepo, tt.organizationID, tt.actorUserID) accessControlService := tt.accessControlService if accessControlService == nil { @@ -985,6 +969,7 @@ func TestOrganizationMemberService_RemoveMember(t *testing.T) { if tt.setup != nil { tt.setup(orgRepo, memberRepo, hooks) } + expectActorMember(memberRepo, tt.organizationID, tt.actorUserID) svc := newTestOrganizationMemberService(userService, orgtests.NewAccessControlServiceStub(), orgRepo, memberRepo, nil) err := svc.RemoveMember(context.Background(), orgtests.Actor(tt.actorUserID), tt.organizationID, tt.memberID) @@ -1005,3 +990,88 @@ func TestOrganizationMemberService_RemoveMember(t *testing.T) { }) } } + +func TestOrganizationMemberService_MachineActorForbidden(t *testing.T) { + t.Parallel() + + actor := &models.Actor{ID: "key-1", Type: models.ActorMachine, Claims: map[string]any{"organization_id": "org-1"}} + + t.Run("add member", func(t *testing.T) { + t.Parallel() + + orgRepo := &orgtests.MockOrganizationRepository{} + memberRepo := &orgtests.MockOrganizationMemberRepository{} + orgRepo.On("GetByID", mock.Anything, "org-1").Return(&types.Organization{ID: "org-1", OwnerID: "user-1"}, nil).Once() + + svc := newTestOrganizationMemberService(&internaltests.MockUserService{}, orgtests.NewAccessControlServiceStub(), orgRepo, memberRepo, nil) + _, err := svc.AddMember(context.Background(), actor, "org-1", types.AddOrganizationMemberRequest{UserID: "user-2", Role: "member"}) + require.ErrorIs(t, err, coreerrors.ErrForbidden) + orgRepo.AssertExpectations(t) + }) + + t.Run("update member", func(t *testing.T) { + t.Parallel() + + orgRepo := &orgtests.MockOrganizationRepository{} + memberRepo := &orgtests.MockOrganizationMemberRepository{} + orgRepo.On("GetByID", mock.Anything, "org-1").Return(&types.Organization{ID: "org-1", OwnerID: "user-1"}, nil).Once() + + svc := newTestOrganizationMemberService(&internaltests.MockUserService{}, orgtests.NewAccessControlServiceStub(), orgRepo, memberRepo, nil) + _, err := svc.UpdateMember(context.Background(), actor, "org-1", "mem-1", types.UpdateOrganizationMemberRequest{Role: "admin"}) + require.ErrorIs(t, err, coreerrors.ErrForbidden) + orgRepo.AssertExpectations(t) + }) +} + +func TestOrganizationMemberService_RemoveMemberGuards(t *testing.T) { + t.Parallel() + + t.Run("cannot remove member with heavier role", func(t *testing.T) { + t.Parallel() + + orgRepo := &orgtests.MockOrganizationRepository{} + memberRepo := &orgtests.MockOrganizationMemberRepository{} + orgRepo.On("GetByID", mock.Anything, "org-1").Return(&types.Organization{ID: "org-1", OwnerID: "user-1"}, nil).Once() + memberRepo.On("GetByOrganizationIDAndUserID", mock.Anything, "org-1", "user-1").Return(&types.OrganizationMember{ID: "mem-actor", OrganizationID: "org-1", UserID: "user-1", Role: "member"}, nil).Once() + memberRepo.On("GetByID", mock.Anything, "mem-1").Return(&types.OrganizationMember{ID: "mem-1", OrganizationID: "org-1", UserID: "user-2", Role: "admin"}, nil).Once() + + svc := newTestOrganizationMemberService(&internaltests.MockUserService{}, orgtests.NewAccessControlServiceStub(), orgRepo, memberRepo, nil) + err := svc.RemoveMember(context.Background(), orgtests.Actor("user-1"), "org-1", "mem-1") + require.ErrorIs(t, err, coreerrors.ErrForbidden) + orgRepo.AssertExpectations(t) + memberRepo.AssertExpectations(t) + }) + + t.Run("cannot remove owner unless actor is the owner", func(t *testing.T) { + t.Parallel() + + orgRepo := &orgtests.MockOrganizationRepository{} + memberRepo := &orgtests.MockOrganizationMemberRepository{} + orgRepo.On("GetByID", mock.Anything, "org-1").Return(&types.Organization{ID: "org-1", OwnerID: "user-1"}, nil).Once() + memberRepo.On("GetByOrganizationIDAndUserID", mock.Anything, "org-1", "user-2").Return(&types.OrganizationMember{ID: "mem-actor", OrganizationID: "org-1", UserID: "user-2", Role: "admin"}, nil).Once() + memberRepo.On("GetByID", mock.Anything, "mem-1").Return(&types.OrganizationMember{ID: "mem-1", OrganizationID: "org-1", UserID: "user-1", Role: "admin"}, nil).Once() + + svc := newTestOrganizationMemberService(&internaltests.MockUserService{}, orgtests.NewAccessControlServiceStub(), orgRepo, memberRepo, nil) + err := svc.RemoveMember(context.Background(), orgtests.Actor("user-2"), "org-1", "mem-1") + require.ErrorIs(t, err, coreerrors.ErrForbidden) + orgRepo.AssertExpectations(t) + memberRepo.AssertExpectations(t) + }) + + t.Run("owner can remove a member", func(t *testing.T) { + t.Parallel() + + orgRepo := &orgtests.MockOrganizationRepository{} + memberRepo := &orgtests.MockOrganizationMemberRepository{} + orgRepo.On("GetByID", mock.Anything, "org-1").Return(&types.Organization{ID: "org-1", OwnerID: "user-1"}, nil).Once() + memberRepo.On("GetByOrganizationIDAndUserID", mock.Anything, "org-1", "user-1").Return(&types.OrganizationMember{ID: "mem-actor", OrganizationID: "org-1", UserID: "user-1", Role: "admin"}, nil).Once() + memberRepo.On("GetByID", mock.Anything, "mem-1").Return(&types.OrganizationMember{ID: "mem-1", OrganizationID: "org-1", UserID: "user-2", Role: "member"}, nil).Once() + memberRepo.On("Delete", mock.Anything, "mem-1").Return(nil).Once() + + svc := newTestOrganizationMemberService(&internaltests.MockUserService{}, orgtests.NewAccessControlServiceStub(), orgRepo, memberRepo, nil) + err := svc.RemoveMember(context.Background(), orgtests.Actor("user-1"), "org-1", "mem-1") + require.NoError(t, err) + orgRepo.AssertExpectations(t) + memberRepo.AssertExpectations(t) + }) +} diff --git a/plugins/organizations/services/organization_service.go b/plugins/organizations/services/organization_service.go index 54ad6039..fb90c7b2 100644 --- a/plugins/organizations/services/organization_service.go +++ b/plugins/organizations/services/organization_service.go @@ -261,7 +261,7 @@ func (s *organizationService) GetAllOrganizationsByOwner(ctx context.Context, ac } func (s *organizationService) GetOrganizationByID(ctx context.Context, actor *models.Actor, organizationID string) (*types.Organization, error) { - organization, _, err := s.serviceUtils.authorizeOrganizationAccess(ctx, actor, organizationID) + organization, _, err := s.serviceUtils.AuthorizeOrganizationAccess(ctx, actor, organizationID) if err != nil { return nil, err } @@ -270,7 +270,7 @@ func (s *organizationService) GetOrganizationByID(ctx context.Context, actor *mo } func (s *organizationService) UpdateOrganization(ctx context.Context, actor *models.Actor, organizationID string, request types.UpdateOrganizationRequest) (*types.Organization, error) { - organization, _, err := s.serviceUtils.authorizeOrganizationAccess(ctx, actor, organizationID) + organization, _, err := s.serviceUtils.AuthorizeOrganizationAccess(ctx, actor, organizationID) if err != nil { return nil, err } diff --git a/plugins/organizations/services/organization_service_test.go b/plugins/organizations/services/organization_service_test.go index a8e1c70b..8d2b17a5 100644 --- a/plugins/organizations/services/organization_service_test.go +++ b/plugins/organizations/services/organization_service_test.go @@ -244,6 +244,7 @@ func TestOrganizationService_GetOrganizationByID(t *testing.T) { organizationID: "org-1", setup: func(repo *orgtests.MockOrganizationRepository, memberRepo *orgtests.MockOrganizationMemberRepository) { repo.On("GetByID", mock.Anything, "org-1").Return(&types.Organization{ID: "org-1", OwnerID: "user-1"}, nil).Once() + memberRepo.On("GetByOrganizationIDAndUserID", mock.Anything, "org-1", "user-1").Return(&types.OrganizationMember{ID: "mem-1", OrganizationID: "org-1", UserID: "user-1", Role: "member"}, nil).Once() }, }, } @@ -328,6 +329,7 @@ func TestOrganizationService_UpdateOrganization(t *testing.T) { request: types.UpdateOrganizationRequest{Name: new("Acme Platform")}, setup: func(repo *orgtests.MockOrganizationRepository, memberRepo *orgtests.MockOrganizationMemberRepository, hooks *orgtests.MockOrganizationHooks, serviceUtils *ServiceUtils) { repo.On("GetByID", mock.Anything, "org-1").Return(&types.Organization{ID: "org-1", OwnerID: "user-1", Name: "Acme", Slug: "acme"}, nil).Once() + memberRepo.On("GetByOrganizationIDAndUserID", mock.Anything, "org-1", "user-1").Return(&types.OrganizationMember{ID: "mem-1", OrganizationID: "org-1", UserID: "user-1", Role: "member"}, nil).Once() repo.On("Update", mock.Anything, mock.MatchedBy(func(org *types.Organization) bool { return org != nil && org.ID == "org-1" && org.Name == "Acme Platform" && org.Slug == "acme" })).Return(&types.Organization{ID: "org-1", OwnerID: "user-1", Name: "Acme Platform", Slug: "acme"}, nil).Once() diff --git a/plugins/organizations/services/organization_team_member_service.go b/plugins/organizations/services/organization_team_member_service.go index 1dd86275..08ecf8eb 100644 --- a/plugins/organizations/services/organization_team_member_service.go +++ b/plugins/organizations/services/organization_team_member_service.go @@ -40,7 +40,7 @@ func (s *organizationTeamMemberService) AddTeamMember(ctx context.Context, actor return nil, coreerrors.ErrUnprocessableEntity } - if _, _, err := s.serviceUtils.authorizeOrganizationAccess(ctx, actor, organizationID); err != nil { + if _, _, err := s.serviceUtils.AuthorizeOrganizationAccess(ctx, actor, organizationID); err != nil { return nil, err } @@ -97,7 +97,7 @@ func (s *organizationTeamMemberService) AddTeamMember(ctx context.Context, actor } func (s *organizationTeamMemberService) GetAllTeamMembers(ctx context.Context, actor *models.Actor, organizationID string, teamID string, page int, limit int) ([]types.OrganizationTeamMemberResponse, error) { - if _, _, err := s.serviceUtils.authorizeOrganizationAccess(ctx, actor, organizationID); err != nil { + if _, _, err := s.serviceUtils.AuthorizeOrganizationAccess(ctx, actor, organizationID); err != nil { return nil, err } @@ -117,7 +117,7 @@ func (s *organizationTeamMemberService) GetAllTeamMembers(ctx context.Context, a } func (s *organizationTeamMemberService) GetTeamMember(ctx context.Context, actor *models.Actor, organizationID string, teamID string, memberID string) (*types.OrganizationTeamMemberResponse, error) { - if _, _, err := s.serviceUtils.authorizeOrganizationAccess(ctx, actor, organizationID); err != nil { + if _, _, err := s.serviceUtils.AuthorizeOrganizationAccess(ctx, actor, organizationID); err != nil { return nil, err } @@ -158,7 +158,7 @@ func (s *organizationTeamMemberService) GetTeamMember(ctx context.Context, actor } func (s *organizationTeamMemberService) RemoveTeamMember(ctx context.Context, actor *models.Actor, organizationID string, teamID string, memberID string) error { - if _, _, err := s.serviceUtils.authorizeOrganizationAccess(ctx, actor, organizationID); err != nil { + if _, _, err := s.serviceUtils.AuthorizeOrganizationAccess(ctx, actor, organizationID); err != nil { return err } diff --git a/plugins/organizations/services/organization_team_member_service_test.go b/plugins/organizations/services/organization_team_member_service_test.go index 34c9e1e2..e3346769 100644 --- a/plugins/organizations/services/organization_team_member_service_test.go +++ b/plugins/organizations/services/organization_team_member_service_test.go @@ -46,7 +46,7 @@ func TestOrganizationTeamService_GetAllTeamMembers(t *testing.T) { teamID: "team-1", setup: func(orgRepo *orgtests.MockOrganizationRepository, teamRepo *orgtests.MockOrganizationTeamRepository, memberRepo *orgtests.MockOrganizationMemberRepository, teamMemberRepo *orgtests.MockOrganizationTeamMemberRepository) { orgRepo.On("GetByID", mock.Anything, "org-1").Return(&types.Organization{ID: "org-1", OwnerID: "user-1"}, nil).Once() - memberRepo.On("GetByOrganizationIDAndUserID", mock.Anything, "org-1", "user-1").Return(&types.OrganizationMember{ID: "owner-member-1", OrganizationID: "org-1", UserID: "user-1", Role: "owner"}, nil).Once() + memberRepo.On("GetByOrganizationIDAndUserID", mock.Anything, "org-1", "user-1").Return(&types.OrganizationMember{ID: "owner-member-1", OrganizationID: "org-1", UserID: "user-1", Role: "owner"}, nil).Twice() teamRepo.On("GetByID", mock.Anything, "team-1").Return(&types.OrganizationTeam{ID: "team-1", OrganizationID: "org-1"}, nil).Twice() teamMemberRepo.On("GetByTeamIDAndMemberID", mock.Anything, "team-1", "owner-member-1").Return(&types.OrganizationTeamMember{ID: "owner-tm-1", TeamID: "team-1", MemberID: "owner-member-1"}, nil).Once() teamMemberRepo.On("GetAllByTeamIDWithMemberAndUser", mock.Anything, "team-1", 1, 10).Return([]types.OrganizationTeamMemberResponse{{ID: "tm-1", TeamID: "team-1"}}, nil).Once() @@ -106,6 +106,7 @@ func TestOrganizationTeamService_GetAllTeamMembers(t *testing.T) { teamID: "team-1", setup: func(orgRepo *orgtests.MockOrganizationRepository, teamRepo *orgtests.MockOrganizationTeamRepository, memberRepo *orgtests.MockOrganizationMemberRepository, teamMemberRepo *orgtests.MockOrganizationTeamMemberRepository) { orgRepo.On("GetByID", mock.Anything, "org-1").Return(&types.Organization{ID: "org-1", OwnerID: "user-1"}, nil).Once() + memberRepo.On("GetByOrganizationIDAndUserID", mock.Anything, "org-1", "user-1").Return(&types.OrganizationMember{ID: "owner-member-1", OrganizationID: "org-1", UserID: "user-1", Role: "owner"}, nil).Once() teamRepo.On("GetByID", mock.Anything, "team-1").Return((*types.OrganizationTeam)(nil), repoErr).Once() }, expectErr: repoErr, @@ -118,6 +119,7 @@ func TestOrganizationTeamService_GetAllTeamMembers(t *testing.T) { teamID: "team-1", setup: func(orgRepo *orgtests.MockOrganizationRepository, teamRepo *orgtests.MockOrganizationTeamRepository, memberRepo *orgtests.MockOrganizationMemberRepository, teamMemberRepo *orgtests.MockOrganizationTeamMemberRepository) { orgRepo.On("GetByID", mock.Anything, "org-1").Return(&types.Organization{ID: "org-1", OwnerID: "user-1"}, nil).Once() + memberRepo.On("GetByOrganizationIDAndUserID", mock.Anything, "org-1", "user-1").Return(&types.OrganizationMember{ID: "owner-member-1", OrganizationID: "org-1", UserID: "user-1", Role: "owner"}, nil).Once() teamRepo.On("GetByID", mock.Anything, "team-1").Return(nil, nil).Once() }, expectErr: coreerrors.ErrNotFound, @@ -130,6 +132,7 @@ func TestOrganizationTeamService_GetAllTeamMembers(t *testing.T) { teamID: "team-1", setup: func(orgRepo *orgtests.MockOrganizationRepository, teamRepo *orgtests.MockOrganizationTeamRepository, memberRepo *orgtests.MockOrganizationMemberRepository, teamMemberRepo *orgtests.MockOrganizationTeamMemberRepository) { orgRepo.On("GetByID", mock.Anything, "org-1").Return(&types.Organization{ID: "org-1", OwnerID: "user-1"}, nil).Once() + memberRepo.On("GetByOrganizationIDAndUserID", mock.Anything, "org-1", "user-1").Return(&types.OrganizationMember{ID: "owner-member-1", OrganizationID: "org-1", UserID: "user-1", Role: "owner"}, nil).Once() teamRepo.On("GetByID", mock.Anything, "team-1").Return(&types.OrganizationTeam{ID: "team-1", OrganizationID: "org-2"}, nil).Once() }, expectErr: coreerrors.ErrNotFound, @@ -142,7 +145,7 @@ func TestOrganizationTeamService_GetAllTeamMembers(t *testing.T) { teamID: "team-1", setup: func(orgRepo *orgtests.MockOrganizationRepository, teamRepo *orgtests.MockOrganizationTeamRepository, memberRepo *orgtests.MockOrganizationMemberRepository, teamMemberRepo *orgtests.MockOrganizationTeamMemberRepository) { orgRepo.On("GetByID", mock.Anything, "org-1").Return(&types.Organization{ID: "org-1", OwnerID: "user-1"}, nil).Once() - memberRepo.On("GetByOrganizationIDAndUserID", mock.Anything, "org-1", "user-1").Return(&types.OrganizationMember{ID: "owner-member-1", OrganizationID: "org-1", UserID: "user-1", Role: "owner"}, nil).Once() + memberRepo.On("GetByOrganizationIDAndUserID", mock.Anything, "org-1", "user-1").Return(&types.OrganizationMember{ID: "owner-member-1", OrganizationID: "org-1", UserID: "user-1", Role: "owner"}, nil).Twice() teamRepo.On("GetByID", mock.Anything, "team-1").Return(&types.OrganizationTeam{ID: "team-1", OrganizationID: "org-1"}, nil).Twice() teamMemberRepo.On("GetByTeamIDAndMemberID", mock.Anything, "team-1", "owner-member-1").Return(&types.OrganizationTeamMember{ID: "owner-tm-1", TeamID: "team-1", MemberID: "owner-member-1"}, nil).Once() teamMemberRepo.On("GetAllByTeamIDWithMemberAndUser", mock.Anything, "team-1", 1, 10).Return(([]types.OrganizationTeamMemberResponse)(nil), repoErr).Once() @@ -209,7 +212,7 @@ func TestOrganizationTeamService_AddTeamMember(t *testing.T) { request: types.AddOrganizationTeamMemberRequest{MemberID: "member-1"}, setup: func(orgRepo *orgtests.MockOrganizationRepository, teamRepo *orgtests.MockOrganizationTeamRepository, memberRepo *orgtests.MockOrganizationMemberRepository, teamMemberRepo *orgtests.MockOrganizationTeamMemberRepository, hooks *orgtests.MockOrganizationTeamMemberHooks, serviceUtils *ServiceUtils) { orgRepo.On("GetByID", mock.Anything, "org-1").Return(&types.Organization{ID: "org-1", OwnerID: "user-1"}, nil).Once() - memberRepo.On("GetByOrganizationIDAndUserID", mock.Anything, "org-1", "user-1").Return(&types.OrganizationMember{ID: "owner-member-1", OrganizationID: "org-1", UserID: "user-1", Role: "owner"}, nil).Once() + memberRepo.On("GetByOrganizationIDAndUserID", mock.Anything, "org-1", "user-1").Return(&types.OrganizationMember{ID: "owner-member-1", OrganizationID: "org-1", UserID: "user-1", Role: "owner"}, nil).Twice() teamRepo.On("GetByID", mock.Anything, "team-1").Return(&types.OrganizationTeam{ID: "team-1", OrganizationID: "org-1", Name: "Platform", Slug: "platform"}, nil).Twice() teamMemberRepo.On("GetByTeamIDAndMemberID", mock.Anything, "team-1", "owner-member-1").Return(&types.OrganizationTeamMember{ID: "owner-tm-1", TeamID: "team-1", MemberID: "owner-member-1"}, nil).Once() memberRepo.On("GetByID", mock.Anything, "member-1").Return(&types.OrganizationMember{ID: "member-1", OrganizationID: "org-1", UserID: "user-2", Role: "member"}, nil).Once() @@ -292,6 +295,7 @@ func TestOrganizationTeamService_AddTeamMember(t *testing.T) { request: types.AddOrganizationTeamMemberRequest{MemberID: "member-1"}, setup: func(orgRepo *orgtests.MockOrganizationRepository, teamRepo *orgtests.MockOrganizationTeamRepository, memberRepo *orgtests.MockOrganizationMemberRepository, teamMemberRepo *orgtests.MockOrganizationTeamMemberRepository, hooks *orgtests.MockOrganizationTeamMemberHooks, serviceUtils *ServiceUtils) { orgRepo.On("GetByID", mock.Anything, "org-1").Return(&types.Organization{ID: "org-1", OwnerID: "user-1"}, nil).Once() + memberRepo.On("GetByOrganizationIDAndUserID", mock.Anything, "org-1", "user-1").Return(&types.OrganizationMember{ID: "owner-member-1", OrganizationID: "org-1", UserID: "user-1", Role: "owner"}, nil).Once() teamRepo.On("GetByID", mock.Anything, "team-1").Return((*types.OrganizationTeam)(nil), repoErr).Once() }, expectErr: repoErr, @@ -305,6 +309,7 @@ func TestOrganizationTeamService_AddTeamMember(t *testing.T) { request: types.AddOrganizationTeamMemberRequest{MemberID: "member-1"}, setup: func(orgRepo *orgtests.MockOrganizationRepository, teamRepo *orgtests.MockOrganizationTeamRepository, memberRepo *orgtests.MockOrganizationMemberRepository, teamMemberRepo *orgtests.MockOrganizationTeamMemberRepository, hooks *orgtests.MockOrganizationTeamMemberHooks, serviceUtils *ServiceUtils) { orgRepo.On("GetByID", mock.Anything, "org-1").Return(&types.Organization{ID: "org-1", OwnerID: "user-1"}, nil).Once() + memberRepo.On("GetByOrganizationIDAndUserID", mock.Anything, "org-1", "user-1").Return(&types.OrganizationMember{ID: "owner-member-1", OrganizationID: "org-1", UserID: "user-1", Role: "owner"}, nil).Once() teamRepo.On("GetByID", mock.Anything, "team-1").Return(nil, nil).Once() }, expectErr: coreerrors.ErrNotFound, @@ -318,6 +323,7 @@ func TestOrganizationTeamService_AddTeamMember(t *testing.T) { request: types.AddOrganizationTeamMemberRequest{MemberID: "member-1"}, setup: func(orgRepo *orgtests.MockOrganizationRepository, teamRepo *orgtests.MockOrganizationTeamRepository, memberRepo *orgtests.MockOrganizationMemberRepository, teamMemberRepo *orgtests.MockOrganizationTeamMemberRepository, hooks *orgtests.MockOrganizationTeamMemberHooks, serviceUtils *ServiceUtils) { orgRepo.On("GetByID", mock.Anything, "org-1").Return(&types.Organization{ID: "org-1", OwnerID: "user-1"}, nil).Once() + memberRepo.On("GetByOrganizationIDAndUserID", mock.Anything, "org-1", "user-1").Return(&types.OrganizationMember{ID: "owner-member-1", OrganizationID: "org-1", UserID: "user-1", Role: "owner"}, nil).Once() teamRepo.On("GetByID", mock.Anything, "team-1").Return(&types.OrganizationTeam{ID: "team-1", OrganizationID: "org-2", Name: "Platform", Slug: "platform"}, nil).Once() }, expectErr: coreerrors.ErrNotFound, @@ -331,7 +337,7 @@ func TestOrganizationTeamService_AddTeamMember(t *testing.T) { request: types.AddOrganizationTeamMemberRequest{MemberID: "member-1"}, setup: func(orgRepo *orgtests.MockOrganizationRepository, teamRepo *orgtests.MockOrganizationTeamRepository, memberRepo *orgtests.MockOrganizationMemberRepository, teamMemberRepo *orgtests.MockOrganizationTeamMemberRepository, hooks *orgtests.MockOrganizationTeamMemberHooks, serviceUtils *ServiceUtils) { orgRepo.On("GetByID", mock.Anything, "org-1").Return(&types.Organization{ID: "org-1", OwnerID: "user-1"}, nil).Once() - memberRepo.On("GetByOrganizationIDAndUserID", mock.Anything, "org-1", "user-1").Return(&types.OrganizationMember{ID: "owner-member-1", OrganizationID: "org-1", UserID: "user-1", Role: "owner"}, nil).Once() + memberRepo.On("GetByOrganizationIDAndUserID", mock.Anything, "org-1", "user-1").Return(&types.OrganizationMember{ID: "owner-member-1", OrganizationID: "org-1", UserID: "user-1", Role: "owner"}, nil).Twice() teamRepo.On("GetByID", mock.Anything, "team-1").Return(&types.OrganizationTeam{ID: "team-1", OrganizationID: "org-1", Name: "Platform", Slug: "platform"}, nil).Twice() teamMemberRepo.On("GetByTeamIDAndMemberID", mock.Anything, "team-1", "owner-member-1").Return(&types.OrganizationTeamMember{ID: "owner-tm-1", TeamID: "team-1", MemberID: "owner-member-1"}, nil).Once() memberRepo.On("GetByID", mock.Anything, "member-1").Return(&types.OrganizationMember{ID: "member-1", OrganizationID: "org-2", UserID: "user-2", Role: "member"}, nil).Once() @@ -356,7 +362,7 @@ func TestOrganizationTeamService_AddTeamMember(t *testing.T) { request: types.AddOrganizationTeamMemberRequest{MemberID: "member-1"}, setup: func(orgRepo *orgtests.MockOrganizationRepository, teamRepo *orgtests.MockOrganizationTeamRepository, memberRepo *orgtests.MockOrganizationMemberRepository, teamMemberRepo *orgtests.MockOrganizationTeamMemberRepository, hooks *orgtests.MockOrganizationTeamMemberHooks, serviceUtils *ServiceUtils) { orgRepo.On("GetByID", mock.Anything, "org-1").Return(&types.Organization{ID: "org-1", OwnerID: "user-1"}, nil).Once() - memberRepo.On("GetByOrganizationIDAndUserID", mock.Anything, "org-1", "user-1").Return(&types.OrganizationMember{ID: "owner-member-1", OrganizationID: "org-1", UserID: "user-1", Role: "owner"}, nil).Once() + memberRepo.On("GetByOrganizationIDAndUserID", mock.Anything, "org-1", "user-1").Return(&types.OrganizationMember{ID: "owner-member-1", OrganizationID: "org-1", UserID: "user-1", Role: "owner"}, nil).Twice() teamRepo.On("GetByID", mock.Anything, "team-1").Return(&types.OrganizationTeam{ID: "team-1", OrganizationID: "org-1", Name: "Platform", Slug: "platform"}, nil).Twice() teamMemberRepo.On("GetByTeamIDAndMemberID", mock.Anything, "team-1", "owner-member-1").Return(&types.OrganizationTeamMember{ID: "owner-tm-1", TeamID: "team-1", MemberID: "owner-member-1"}, nil).Once() memberRepo.On("GetByID", mock.Anything, "member-1").Return((*types.OrganizationMember)(nil), repoErr).Once() @@ -372,7 +378,7 @@ func TestOrganizationTeamService_AddTeamMember(t *testing.T) { request: types.AddOrganizationTeamMemberRequest{MemberID: "member-1"}, setup: func(orgRepo *orgtests.MockOrganizationRepository, teamRepo *orgtests.MockOrganizationTeamRepository, memberRepo *orgtests.MockOrganizationMemberRepository, teamMemberRepo *orgtests.MockOrganizationTeamMemberRepository, hooks *orgtests.MockOrganizationTeamMemberHooks, serviceUtils *ServiceUtils) { orgRepo.On("GetByID", mock.Anything, "org-1").Return(&types.Organization{ID: "org-1", OwnerID: "user-1"}, nil).Once() - memberRepo.On("GetByOrganizationIDAndUserID", mock.Anything, "org-1", "user-1").Return(&types.OrganizationMember{ID: "owner-member-1", OrganizationID: "org-1", UserID: "user-1", Role: "owner"}, nil).Once() + memberRepo.On("GetByOrganizationIDAndUserID", mock.Anything, "org-1", "user-1").Return(&types.OrganizationMember{ID: "owner-member-1", OrganizationID: "org-1", UserID: "user-1", Role: "owner"}, nil).Twice() teamRepo.On("GetByID", mock.Anything, "team-1").Return(&types.OrganizationTeam{ID: "team-1", OrganizationID: "org-1", Name: "Platform", Slug: "platform"}, nil).Twice() teamMemberRepo.On("GetByTeamIDAndMemberID", mock.Anything, "team-1", "owner-member-1").Return(&types.OrganizationTeamMember{ID: "owner-tm-1", TeamID: "team-1", MemberID: "owner-member-1"}, nil).Once() memberRepo.On("GetByID", mock.Anything, "member-1").Return(nil, nil).Once() @@ -388,7 +394,7 @@ func TestOrganizationTeamService_AddTeamMember(t *testing.T) { request: types.AddOrganizationTeamMemberRequest{MemberID: "member-1"}, setup: func(orgRepo *orgtests.MockOrganizationRepository, teamRepo *orgtests.MockOrganizationTeamRepository, memberRepo *orgtests.MockOrganizationMemberRepository, teamMemberRepo *orgtests.MockOrganizationTeamMemberRepository, hooks *orgtests.MockOrganizationTeamMemberHooks, serviceUtils *ServiceUtils) { orgRepo.On("GetByID", mock.Anything, "org-1").Return(&types.Organization{ID: "org-1", OwnerID: "user-1"}, nil).Once() - memberRepo.On("GetByOrganizationIDAndUserID", mock.Anything, "org-1", "user-1").Return(&types.OrganizationMember{ID: "owner-member-1", OrganizationID: "org-1", UserID: "user-1", Role: "owner"}, nil).Once() + memberRepo.On("GetByOrganizationIDAndUserID", mock.Anything, "org-1", "user-1").Return(&types.OrganizationMember{ID: "owner-member-1", OrganizationID: "org-1", UserID: "user-1", Role: "owner"}, nil).Twice() teamRepo.On("GetByID", mock.Anything, "team-1").Return(&types.OrganizationTeam{ID: "team-1", OrganizationID: "org-1", Name: "Platform", Slug: "platform"}, nil).Twice() teamMemberRepo.On("GetByTeamIDAndMemberID", mock.Anything, "team-1", "owner-member-1").Return(&types.OrganizationTeamMember{ID: "owner-tm-1", TeamID: "team-1", MemberID: "owner-member-1"}, nil).Once() memberRepo.On("GetByID", mock.Anything, "member-1").Return(&types.OrganizationMember{ID: "member-1", OrganizationID: "org-1", UserID: "user-2", Role: "member"}, nil).Once() @@ -405,7 +411,7 @@ func TestOrganizationTeamService_AddTeamMember(t *testing.T) { request: types.AddOrganizationTeamMemberRequest{MemberID: "member-1"}, setup: func(orgRepo *orgtests.MockOrganizationRepository, teamRepo *orgtests.MockOrganizationTeamRepository, memberRepo *orgtests.MockOrganizationMemberRepository, teamMemberRepo *orgtests.MockOrganizationTeamMemberRepository, hooks *orgtests.MockOrganizationTeamMemberHooks, serviceUtils *ServiceUtils) { orgRepo.On("GetByID", mock.Anything, "org-1").Return(&types.Organization{ID: "org-1", OwnerID: "user-1"}, nil).Once() - memberRepo.On("GetByOrganizationIDAndUserID", mock.Anything, "org-1", "user-1").Return(&types.OrganizationMember{ID: "owner-member-1", OrganizationID: "org-1", UserID: "user-1", Role: "owner"}, nil).Once() + memberRepo.On("GetByOrganizationIDAndUserID", mock.Anything, "org-1", "user-1").Return(&types.OrganizationMember{ID: "owner-member-1", OrganizationID: "org-1", UserID: "user-1", Role: "owner"}, nil).Twice() teamRepo.On("GetByID", mock.Anything, "team-1").Return(&types.OrganizationTeam{ID: "team-1", OrganizationID: "org-1", Name: "Platform", Slug: "platform"}, nil).Twice() teamMemberRepo.On("GetByTeamIDAndMemberID", mock.Anything, "team-1", "owner-member-1").Return(&types.OrganizationTeamMember{ID: "owner-tm-1", TeamID: "team-1", MemberID: "owner-member-1"}, nil).Once() memberRepo.On("GetByID", mock.Anything, "member-1").Return(&types.OrganizationMember{ID: "member-1", OrganizationID: "org-1", UserID: "user-2", Role: "member"}, nil).Once() @@ -422,7 +428,7 @@ func TestOrganizationTeamService_AddTeamMember(t *testing.T) { request: types.AddOrganizationTeamMemberRequest{MemberID: "member-1"}, setup: func(orgRepo *orgtests.MockOrganizationRepository, teamRepo *orgtests.MockOrganizationTeamRepository, memberRepo *orgtests.MockOrganizationMemberRepository, teamMemberRepo *orgtests.MockOrganizationTeamMemberRepository, hooks *orgtests.MockOrganizationTeamMemberHooks, serviceUtils *ServiceUtils) { orgRepo.On("GetByID", mock.Anything, "org-1").Return(&types.Organization{ID: "org-1", OwnerID: "user-1"}, nil).Once() - memberRepo.On("GetByOrganizationIDAndUserID", mock.Anything, "org-1", "user-1").Return(&types.OrganizationMember{ID: "owner-member-1", OrganizationID: "org-1", UserID: "user-1", Role: "owner"}, nil).Once() + memberRepo.On("GetByOrganizationIDAndUserID", mock.Anything, "org-1", "user-1").Return(&types.OrganizationMember{ID: "owner-member-1", OrganizationID: "org-1", UserID: "user-1", Role: "owner"}, nil).Twice() teamRepo.On("GetByID", mock.Anything, "team-1").Return(&types.OrganizationTeam{ID: "team-1", OrganizationID: "org-1", Name: "Platform", Slug: "platform"}, nil).Twice() teamMemberRepo.On("GetByTeamIDAndMemberID", mock.Anything, "team-1", "owner-member-1").Return(&types.OrganizationTeamMember{ID: "owner-tm-1", TeamID: "team-1", MemberID: "owner-member-1"}, nil).Once() memberRepo.On("GetByID", mock.Anything, "member-1").Return(&types.OrganizationMember{ID: "member-1", OrganizationID: "org-1", UserID: "user-2", Role: "member"}, nil).Once() @@ -502,7 +508,7 @@ func TestOrganizationTeamService_GetTeamMember(t *testing.T) { memberID: "member-1", setup: func(orgRepo *orgtests.MockOrganizationRepository, teamRepo *orgtests.MockOrganizationTeamRepository, memberRepo *orgtests.MockOrganizationMemberRepository, teamMemberRepo *orgtests.MockOrganizationTeamMemberRepository) { orgRepo.On("GetByID", mock.Anything, "org-1").Return(&types.Organization{ID: "org-1", OwnerID: "user-1"}, nil).Once() - memberRepo.On("GetByOrganizationIDAndUserID", mock.Anything, "org-1", "user-1").Return(&types.OrganizationMember{ID: "owner-member-1", OrganizationID: "org-1", UserID: "user-1", Role: "owner"}, nil).Once() + memberRepo.On("GetByOrganizationIDAndUserID", mock.Anything, "org-1", "user-1").Return(&types.OrganizationMember{ID: "owner-member-1", OrganizationID: "org-1", UserID: "user-1", Role: "owner"}, nil).Twice() teamRepo.On("GetByID", mock.Anything, "team-1").Return(&types.OrganizationTeam{ID: "team-1", OrganizationID: "org-1"}, nil).Twice() teamMemberRepo.On("GetByTeamIDAndMemberID", mock.Anything, "team-1", "owner-member-1").Return(&types.OrganizationTeamMember{ID: "owner-tm-1", TeamID: "team-1", MemberID: "owner-member-1"}, nil).Once() memberRepo.On("GetByIDWithUser", mock.Anything, "member-1").Return(&types.OrganizationMemberResponse{ID: "member-1", OrganizationID: "org-1", Role: "member"}, nil).Once() @@ -569,6 +575,7 @@ func TestOrganizationTeamService_GetTeamMember(t *testing.T) { memberID: "member-1", setup: func(orgRepo *orgtests.MockOrganizationRepository, teamRepo *orgtests.MockOrganizationTeamRepository, memberRepo *orgtests.MockOrganizationMemberRepository, teamMemberRepo *orgtests.MockOrganizationTeamMemberRepository) { orgRepo.On("GetByID", mock.Anything, "org-1").Return(&types.Organization{ID: "org-1", OwnerID: "user-1"}, nil).Once() + memberRepo.On("GetByOrganizationIDAndUserID", mock.Anything, "org-1", "user-1").Return(&types.OrganizationMember{ID: "owner-member-1", OrganizationID: "org-1", UserID: "user-1", Role: "owner"}, nil).Once() teamRepo.On("GetByID", mock.Anything, "team-1").Return((*types.OrganizationTeam)(nil), repoErr).Once() }, expectErr: repoErr, @@ -582,6 +589,7 @@ func TestOrganizationTeamService_GetTeamMember(t *testing.T) { memberID: "member-1", setup: func(orgRepo *orgtests.MockOrganizationRepository, teamRepo *orgtests.MockOrganizationTeamRepository, memberRepo *orgtests.MockOrganizationMemberRepository, teamMemberRepo *orgtests.MockOrganizationTeamMemberRepository) { orgRepo.On("GetByID", mock.Anything, "org-1").Return(&types.Organization{ID: "org-1", OwnerID: "user-1"}, nil).Once() + memberRepo.On("GetByOrganizationIDAndUserID", mock.Anything, "org-1", "user-1").Return(&types.OrganizationMember{ID: "owner-member-1", OrganizationID: "org-1", UserID: "user-1", Role: "owner"}, nil).Once() teamRepo.On("GetByID", mock.Anything, "team-1").Return(nil, nil).Once() }, expectErr: coreerrors.ErrNotFound, @@ -595,6 +603,7 @@ func TestOrganizationTeamService_GetTeamMember(t *testing.T) { memberID: "member-1", setup: func(orgRepo *orgtests.MockOrganizationRepository, teamRepo *orgtests.MockOrganizationTeamRepository, memberRepo *orgtests.MockOrganizationMemberRepository, teamMemberRepo *orgtests.MockOrganizationTeamMemberRepository) { orgRepo.On("GetByID", mock.Anything, "org-1").Return(&types.Organization{ID: "org-1", OwnerID: "user-1"}, nil).Once() + memberRepo.On("GetByOrganizationIDAndUserID", mock.Anything, "org-1", "user-1").Return(&types.OrganizationMember{ID: "owner-member-1", OrganizationID: "org-1", UserID: "user-1", Role: "owner"}, nil).Once() teamRepo.On("GetByID", mock.Anything, "team-1").Return(&types.OrganizationTeam{ID: "team-1", OrganizationID: "org-2"}, nil).Once() }, expectErr: coreerrors.ErrNotFound, @@ -608,7 +617,7 @@ func TestOrganizationTeamService_GetTeamMember(t *testing.T) { memberID: "member-1", setup: func(orgRepo *orgtests.MockOrganizationRepository, teamRepo *orgtests.MockOrganizationTeamRepository, memberRepo *orgtests.MockOrganizationMemberRepository, teamMemberRepo *orgtests.MockOrganizationTeamMemberRepository) { orgRepo.On("GetByID", mock.Anything, "org-1").Return(&types.Organization{ID: "org-1", OwnerID: "user-1"}, nil).Once() - memberRepo.On("GetByOrganizationIDAndUserID", mock.Anything, "org-1", "user-1").Return(&types.OrganizationMember{ID: "owner-member-1", OrganizationID: "org-1", UserID: "user-1", Role: "owner"}, nil).Once() + memberRepo.On("GetByOrganizationIDAndUserID", mock.Anything, "org-1", "user-1").Return(&types.OrganizationMember{ID: "owner-member-1", OrganizationID: "org-1", UserID: "user-1", Role: "owner"}, nil).Twice() teamRepo.On("GetByID", mock.Anything, "team-1").Return(&types.OrganizationTeam{ID: "team-1", OrganizationID: "org-1"}, nil).Twice() teamMemberRepo.On("GetByTeamIDAndMemberID", mock.Anything, "team-1", "owner-member-1").Return(&types.OrganizationTeamMember{ID: "owner-tm-1", TeamID: "team-1", MemberID: "owner-member-1"}, nil).Once() memberRepo.On("GetByIDWithUser", mock.Anything, "member-1").Return(&types.OrganizationMemberResponse{ID: "member-1", OrganizationID: "org-1", Role: "member"}, nil).Once() @@ -625,7 +634,7 @@ func TestOrganizationTeamService_GetTeamMember(t *testing.T) { memberID: "member-1", setup: func(orgRepo *orgtests.MockOrganizationRepository, teamRepo *orgtests.MockOrganizationTeamRepository, memberRepo *orgtests.MockOrganizationMemberRepository, teamMemberRepo *orgtests.MockOrganizationTeamMemberRepository) { orgRepo.On("GetByID", mock.Anything, "org-1").Return(&types.Organization{ID: "org-1", OwnerID: "user-1"}, nil).Once() - memberRepo.On("GetByOrganizationIDAndUserID", mock.Anything, "org-1", "user-1").Return(&types.OrganizationMember{ID: "owner-member-1", OrganizationID: "org-1", UserID: "user-1", Role: "owner"}, nil).Once() + memberRepo.On("GetByOrganizationIDAndUserID", mock.Anything, "org-1", "user-1").Return(&types.OrganizationMember{ID: "owner-member-1", OrganizationID: "org-1", UserID: "user-1", Role: "owner"}, nil).Twice() teamRepo.On("GetByID", mock.Anything, "team-1").Return(&types.OrganizationTeam{ID: "team-1", OrganizationID: "org-1"}, nil).Twice() teamMemberRepo.On("GetByTeamIDAndMemberID", mock.Anything, "team-1", "owner-member-1").Return(&types.OrganizationTeamMember{ID: "owner-tm-1", TeamID: "team-1", MemberID: "owner-member-1"}, nil).Once() memberRepo.On("GetByIDWithUser", mock.Anything, "member-1").Return(nil, nil).Once() @@ -641,7 +650,7 @@ func TestOrganizationTeamService_GetTeamMember(t *testing.T) { memberID: "", setup: func(orgRepo *orgtests.MockOrganizationRepository, teamRepo *orgtests.MockOrganizationTeamRepository, memberRepo *orgtests.MockOrganizationMemberRepository, teamMemberRepo *orgtests.MockOrganizationTeamMemberRepository) { orgRepo.On("GetByID", mock.Anything, "org-1").Return(&types.Organization{ID: "org-1", OwnerID: "user-1"}, nil).Once() - memberRepo.On("GetByOrganizationIDAndUserID", mock.Anything, "org-1", "user-1").Return(&types.OrganizationMember{ID: "owner-member-1", OrganizationID: "org-1", UserID: "user-1", Role: "owner"}, nil).Once() + memberRepo.On("GetByOrganizationIDAndUserID", mock.Anything, "org-1", "user-1").Return(&types.OrganizationMember{ID: "owner-member-1", OrganizationID: "org-1", UserID: "user-1", Role: "owner"}, nil).Twice() teamRepo.On("GetByID", mock.Anything, "team-1").Return(&types.OrganizationTeam{ID: "team-1", OrganizationID: "org-1"}, nil).Twice() teamMemberRepo.On("GetByTeamIDAndMemberID", mock.Anything, "team-1", "owner-member-1").Return(&types.OrganizationTeamMember{ID: "owner-tm-1", TeamID: "team-1", MemberID: "owner-member-1"}, nil).Once() memberRepo.On("GetByIDWithUser", mock.Anything, "").Return(nil, nil).Once() @@ -710,7 +719,7 @@ func TestOrganizationTeamService_RemoveTeamMember(t *testing.T) { memberID: "member-1", setup: func(orgRepo *orgtests.MockOrganizationRepository, teamRepo *orgtests.MockOrganizationTeamRepository, memberRepo *orgtests.MockOrganizationMemberRepository, teamMemberRepo *orgtests.MockOrganizationTeamMemberRepository) { orgRepo.On("GetByID", mock.Anything, "org-1").Return(&types.Organization{ID: "org-1", OwnerID: "user-1"}, nil).Once() - memberRepo.On("GetByOrganizationIDAndUserID", mock.Anything, "org-1", "user-1").Return(&types.OrganizationMember{ID: "owner-member-1", OrganizationID: "org-1", UserID: "user-1", Role: "owner"}, nil).Once() + memberRepo.On("GetByOrganizationIDAndUserID", mock.Anything, "org-1", "user-1").Return(&types.OrganizationMember{ID: "owner-member-1", OrganizationID: "org-1", UserID: "user-1", Role: "owner"}, nil).Twice() teamRepo.On("GetByID", mock.Anything, "team-1").Return(&types.OrganizationTeam{ID: "team-1", OrganizationID: "org-1"}, nil).Twice() teamMemberRepo.On("GetByTeamIDAndMemberID", mock.Anything, "team-1", "owner-member-1").Return(&types.OrganizationTeamMember{ID: "owner-tm-1", TeamID: "team-1", MemberID: "owner-member-1"}, nil).Once() memberRepo.On("GetByID", mock.Anything, "member-1").Return(&types.OrganizationMember{ID: "member-1", OrganizationID: "org-1", UserID: "user-2", Role: "member"}, nil).Once() @@ -777,6 +786,7 @@ func TestOrganizationTeamService_RemoveTeamMember(t *testing.T) { memberID: "member-1", setup: func(orgRepo *orgtests.MockOrganizationRepository, teamRepo *orgtests.MockOrganizationTeamRepository, memberRepo *orgtests.MockOrganizationMemberRepository, teamMemberRepo *orgtests.MockOrganizationTeamMemberRepository) { orgRepo.On("GetByID", mock.Anything, "org-1").Return(&types.Organization{ID: "org-1", OwnerID: "user-1"}, nil).Once() + memberRepo.On("GetByOrganizationIDAndUserID", mock.Anything, "org-1", "user-1").Return(&types.OrganizationMember{ID: "owner-member-1", OrganizationID: "org-1", UserID: "user-1", Role: "owner"}, nil).Once() teamRepo.On("GetByID", mock.Anything, "team-1").Return((*types.OrganizationTeam)(nil), repoErr).Once() }, expectErr: repoErr, @@ -790,6 +800,7 @@ func TestOrganizationTeamService_RemoveTeamMember(t *testing.T) { memberID: "member-1", setup: func(orgRepo *orgtests.MockOrganizationRepository, teamRepo *orgtests.MockOrganizationTeamRepository, memberRepo *orgtests.MockOrganizationMemberRepository, teamMemberRepo *orgtests.MockOrganizationTeamMemberRepository) { orgRepo.On("GetByID", mock.Anything, "org-1").Return(&types.Organization{ID: "org-1", OwnerID: "user-1"}, nil).Once() + memberRepo.On("GetByOrganizationIDAndUserID", mock.Anything, "org-1", "user-1").Return(&types.OrganizationMember{ID: "owner-member-1", OrganizationID: "org-1", UserID: "user-1", Role: "owner"}, nil).Once() teamRepo.On("GetByID", mock.Anything, "team-1").Return(nil, nil).Once() }, expectErr: coreerrors.ErrNotFound, @@ -803,6 +814,7 @@ func TestOrganizationTeamService_RemoveTeamMember(t *testing.T) { memberID: "member-1", setup: func(orgRepo *orgtests.MockOrganizationRepository, teamRepo *orgtests.MockOrganizationTeamRepository, memberRepo *orgtests.MockOrganizationMemberRepository, teamMemberRepo *orgtests.MockOrganizationTeamMemberRepository) { orgRepo.On("GetByID", mock.Anything, "org-1").Return(&types.Organization{ID: "org-1", OwnerID: "user-1"}, nil).Once() + memberRepo.On("GetByOrganizationIDAndUserID", mock.Anything, "org-1", "user-1").Return(&types.OrganizationMember{ID: "owner-member-1", OrganizationID: "org-1", UserID: "user-1", Role: "owner"}, nil).Once() teamRepo.On("GetByID", mock.Anything, "team-1").Return(&types.OrganizationTeam{ID: "team-1", OrganizationID: "org-2"}, nil).Once() }, expectErr: coreerrors.ErrNotFound, @@ -816,7 +828,7 @@ func TestOrganizationTeamService_RemoveTeamMember(t *testing.T) { memberID: "member-1", setup: func(orgRepo *orgtests.MockOrganizationRepository, teamRepo *orgtests.MockOrganizationTeamRepository, memberRepo *orgtests.MockOrganizationMemberRepository, teamMemberRepo *orgtests.MockOrganizationTeamMemberRepository) { orgRepo.On("GetByID", mock.Anything, "org-1").Return(&types.Organization{ID: "org-1", OwnerID: "user-1"}, nil).Once() - memberRepo.On("GetByOrganizationIDAndUserID", mock.Anything, "org-1", "user-1").Return(&types.OrganizationMember{ID: "owner-member-1", OrganizationID: "org-1", UserID: "user-1", Role: "owner"}, nil).Once() + memberRepo.On("GetByOrganizationIDAndUserID", mock.Anything, "org-1", "user-1").Return(&types.OrganizationMember{ID: "owner-member-1", OrganizationID: "org-1", UserID: "user-1", Role: "owner"}, nil).Twice() teamRepo.On("GetByID", mock.Anything, "team-1").Return(&types.OrganizationTeam{ID: "team-1", OrganizationID: "org-1"}, nil).Twice() teamMemberRepo.On("GetByTeamIDAndMemberID", mock.Anything, "team-1", "owner-member-1").Return(&types.OrganizationTeamMember{ID: "owner-tm-1", TeamID: "team-1", MemberID: "owner-member-1"}, nil).Once() memberRepo.On("GetByID", mock.Anything, "member-1").Return(&types.OrganizationMember{ID: "member-1", OrganizationID: "org-1", UserID: "user-2", Role: "member"}, nil).Once() @@ -833,7 +845,7 @@ func TestOrganizationTeamService_RemoveTeamMember(t *testing.T) { memberID: "member-1", setup: func(orgRepo *orgtests.MockOrganizationRepository, teamRepo *orgtests.MockOrganizationTeamRepository, memberRepo *orgtests.MockOrganizationMemberRepository, teamMemberRepo *orgtests.MockOrganizationTeamMemberRepository) { orgRepo.On("GetByID", mock.Anything, "org-1").Return(&types.Organization{ID: "org-1", OwnerID: "user-1"}, nil).Once() - memberRepo.On("GetByOrganizationIDAndUserID", mock.Anything, "org-1", "user-1").Return(&types.OrganizationMember{ID: "owner-member-1", OrganizationID: "org-1", UserID: "user-1", Role: "owner"}, nil).Once() + memberRepo.On("GetByOrganizationIDAndUserID", mock.Anything, "org-1", "user-1").Return(&types.OrganizationMember{ID: "owner-member-1", OrganizationID: "org-1", UserID: "user-1", Role: "owner"}, nil).Twice() teamRepo.On("GetByID", mock.Anything, "team-1").Return(&types.OrganizationTeam{ID: "team-1", OrganizationID: "org-1"}, nil).Twice() teamMemberRepo.On("GetByTeamIDAndMemberID", mock.Anything, "team-1", "owner-member-1").Return(&types.OrganizationTeamMember{ID: "owner-tm-1", TeamID: "team-1", MemberID: "owner-member-1"}, nil).Once() memberRepo.On("GetByID", mock.Anything, "member-1").Return(nil, nil).Once() @@ -849,7 +861,7 @@ func TestOrganizationTeamService_RemoveTeamMember(t *testing.T) { memberID: "member-1", setup: func(orgRepo *orgtests.MockOrganizationRepository, teamRepo *orgtests.MockOrganizationTeamRepository, memberRepo *orgtests.MockOrganizationMemberRepository, teamMemberRepo *orgtests.MockOrganizationTeamMemberRepository) { orgRepo.On("GetByID", mock.Anything, "org-1").Return(&types.Organization{ID: "org-1", OwnerID: "user-1"}, nil).Once() - memberRepo.On("GetByOrganizationIDAndUserID", mock.Anything, "org-1", "user-1").Return(&types.OrganizationMember{ID: "owner-member-1", OrganizationID: "org-1", UserID: "user-1", Role: "owner"}, nil).Once() + memberRepo.On("GetByOrganizationIDAndUserID", mock.Anything, "org-1", "user-1").Return(&types.OrganizationMember{ID: "owner-member-1", OrganizationID: "org-1", UserID: "user-1", Role: "owner"}, nil).Twice() teamRepo.On("GetByID", mock.Anything, "team-1").Return(&types.OrganizationTeam{ID: "team-1", OrganizationID: "org-1"}, nil).Twice() teamMemberRepo.On("GetByTeamIDAndMemberID", mock.Anything, "team-1", "owner-member-1").Return(&types.OrganizationTeamMember{ID: "owner-tm-1", TeamID: "team-1", MemberID: "owner-member-1"}, nil).Once() memberRepo.On("GetByID", mock.Anything, "member-1").Return(&types.OrganizationMember{ID: "member-1", OrganizationID: "org-1", UserID: "user-2", Role: "member"}, nil).Once() diff --git a/plugins/organizations/services/organization_team_service.go b/plugins/organizations/services/organization_team_service.go index 3e74b734..41714b07 100644 --- a/plugins/organizations/services/organization_team_service.go +++ b/plugins/organizations/services/organization_team_service.go @@ -44,7 +44,7 @@ func NewOrganizationTeamService( } func (s *organizationTeamService) CreateTeam(ctx context.Context, actor *models.Actor, organizationID string, request types.CreateOrganizationTeamRequest) (*types.OrganizationTeam, error) { - organization, actorMember, err := s.serviceUtils.authorizeOrganizationAccess(ctx, actor, organizationID) + organization, actorMember, err := s.serviceUtils.AuthorizeOrganizationAccess(ctx, actor, organizationID) if err != nil { return nil, err } @@ -150,7 +150,7 @@ func (s *organizationTeamService) CreateTeam(ctx context.Context, actor *models. } func (s *organizationTeamService) GetAllTeams(ctx context.Context, actor *models.Actor, organizationID string) ([]types.OrganizationTeam, error) { - if _, _, err := s.serviceUtils.authorizeOrganizationAccess(ctx, actor, organizationID); err != nil { + if _, _, err := s.serviceUtils.AuthorizeOrganizationAccess(ctx, actor, organizationID); err != nil { return nil, err } @@ -158,7 +158,7 @@ func (s *organizationTeamService) GetAllTeams(ctx context.Context, actor *models } func (s *organizationTeamService) GetTeam(ctx context.Context, actor *models.Actor, organizationID string, teamID string) (*types.OrganizationTeam, error) { - if _, _, err := s.serviceUtils.authorizeOrganizationAccess(ctx, actor, organizationID); err != nil { + if _, _, err := s.serviceUtils.AuthorizeOrganizationAccess(ctx, actor, organizationID); err != nil { return nil, err } @@ -178,7 +178,7 @@ func (s *organizationTeamService) GetTeam(ctx context.Context, actor *models.Act } func (s *organizationTeamService) UpdateTeam(ctx context.Context, actor *models.Actor, organizationID string, teamID string, request types.UpdateOrganizationTeamRequest) (*types.OrganizationTeam, error) { - if _, _, err := s.serviceUtils.authorizeOrganizationAccess(ctx, actor, organizationID); err != nil { + if _, _, err := s.serviceUtils.AuthorizeOrganizationAccess(ctx, actor, organizationID); err != nil { return nil, err } @@ -245,7 +245,7 @@ func (s *organizationTeamService) UpdateTeam(ctx context.Context, actor *models. } func (s *organizationTeamService) DeleteTeam(ctx context.Context, actor *models.Actor, organizationID string, teamID string) error { - if _, _, err := s.serviceUtils.authorizeOrganizationAccess(ctx, actor, organizationID); err != nil { + if _, _, err := s.serviceUtils.AuthorizeOrganizationAccess(ctx, actor, organizationID); err != nil { return err } diff --git a/plugins/organizations/services/organization_team_service_test.go b/plugins/organizations/services/organization_team_service_test.go index 1a494328..99a4ed19 100644 --- a/plugins/organizations/services/organization_team_service_test.go +++ b/plugins/organizations/services/organization_team_service_test.go @@ -138,6 +138,7 @@ func TestOrganizationTeamService_CreateTeam(t *testing.T) { request: types.CreateOrganizationTeamRequest{Name: ""}, setup: func(orgRepo *orgtests.MockOrganizationRepository, teamRepo *orgtests.MockOrganizationTeamRepository, memberRepo *orgtests.MockOrganizationMemberRepository, teamMemberRepo *orgtests.MockOrganizationTeamMemberRepository, hooks *orgtests.MockOrganizationTeamHooks, serviceUtils *ServiceUtils) { orgRepo.On("GetByID", mock.Anything, "org-1").Return(&types.Organization{ID: "org-1", OwnerID: "user-1"}, nil).Once() + memberRepo.On("GetByOrganizationIDAndUserID", mock.Anything, "org-1", "user-1").Return(&types.OrganizationMember{ID: "mem-1", OrganizationID: "org-1", UserID: "user-1", Role: "owner"}, nil).Once() }, expectErr: coreerrors.ErrBadRequest, }, @@ -148,6 +149,7 @@ func TestOrganizationTeamService_CreateTeam(t *testing.T) { request: types.CreateOrganizationTeamRequest{Name: "!!!"}, setup: func(orgRepo *orgtests.MockOrganizationRepository, teamRepo *orgtests.MockOrganizationTeamRepository, memberRepo *orgtests.MockOrganizationMemberRepository, teamMemberRepo *orgtests.MockOrganizationTeamMemberRepository, hooks *orgtests.MockOrganizationTeamHooks, serviceUtils *ServiceUtils) { orgRepo.On("GetByID", mock.Anything, "org-1").Return(&types.Organization{ID: "org-1", OwnerID: "user-1"}, nil).Once() + memberRepo.On("GetByOrganizationIDAndUserID", mock.Anything, "org-1", "user-1").Return(&types.OrganizationMember{ID: "mem-1", OrganizationID: "org-1", UserID: "user-1", Role: "owner"}, nil).Once() }, expectErr: coreerrors.ErrBadRequest, }, @@ -158,6 +160,7 @@ func TestOrganizationTeamService_CreateTeam(t *testing.T) { request: types.CreateOrganizationTeamRequest{Name: "Platform"}, setup: func(orgRepo *orgtests.MockOrganizationRepository, teamRepo *orgtests.MockOrganizationTeamRepository, memberRepo *orgtests.MockOrganizationMemberRepository, teamMemberRepo *orgtests.MockOrganizationTeamMemberRepository, hooks *orgtests.MockOrganizationTeamHooks, serviceUtils *ServiceUtils) { orgRepo.On("GetByID", mock.Anything, "org-1").Return(&types.Organization{ID: "org-1", OwnerID: "user-1"}, nil).Once() + memberRepo.On("GetByOrganizationIDAndUserID", mock.Anything, "org-1", "user-1").Return(&types.OrganizationMember{ID: "mem-1", OrganizationID: "org-1", UserID: "user-1", Role: "owner"}, nil).Once() teamRepo.On("GetByOrganizationIDAndSlug", mock.Anything, "org-1", "platform").Return((*types.OrganizationTeam)(nil), repoErr).Once() }, expectErr: repoErr, @@ -169,6 +172,7 @@ func TestOrganizationTeamService_CreateTeam(t *testing.T) { request: types.CreateOrganizationTeamRequest{Name: "Platform"}, setup: func(orgRepo *orgtests.MockOrganizationRepository, teamRepo *orgtests.MockOrganizationTeamRepository, memberRepo *orgtests.MockOrganizationMemberRepository, teamMemberRepo *orgtests.MockOrganizationTeamMemberRepository, hooks *orgtests.MockOrganizationTeamHooks, serviceUtils *ServiceUtils) { orgRepo.On("GetByID", mock.Anything, "org-1").Return(&types.Organization{ID: "org-1", OwnerID: "user-1"}, nil).Once() + memberRepo.On("GetByOrganizationIDAndUserID", mock.Anything, "org-1", "user-1").Return(&types.OrganizationMember{ID: "mem-1", OrganizationID: "org-1", UserID: "user-1", Role: "owner"}, nil).Once() teamRepo.On("GetByOrganizationIDAndSlug", mock.Anything, "org-1", "platform").Return(&types.OrganizationTeam{ID: "team-2"}, nil).Once() }, expectErr: coreerrors.ErrConflict, @@ -180,6 +184,7 @@ func TestOrganizationTeamService_CreateTeam(t *testing.T) { request: types.CreateOrganizationTeamRequest{Name: "Platform"}, setup: func(orgRepo *orgtests.MockOrganizationRepository, teamRepo *orgtests.MockOrganizationTeamRepository, memberRepo *orgtests.MockOrganizationMemberRepository, teamMemberRepo *orgtests.MockOrganizationTeamMemberRepository, hooks *orgtests.MockOrganizationTeamHooks, serviceUtils *ServiceUtils) { orgRepo.On("GetByID", mock.Anything, "org-1").Return(&types.Organization{ID: "org-1", OwnerID: "user-1"}, nil).Once() + memberRepo.On("GetByOrganizationIDAndUserID", mock.Anything, "org-1", "user-1").Return(&types.OrganizationMember{ID: "mem-1", OrganizationID: "org-1", UserID: "user-1", Role: "owner"}, nil).Once() teamRepo.On("GetByOrganizationIDAndSlug", mock.Anything, "org-1", "platform").Return(nil, nil).Once() teamRepo.On("Create", mock.Anything, mock.MatchedBy(func(team *types.OrganizationTeam) bool { return team != nil && team.OrganizationID == "org-1" && team.Name == "Platform" && team.Slug == "platform" @@ -249,6 +254,7 @@ func TestOrganizationTeamService_GetAllTeams(t *testing.T) { organizationID: "org-1", setup: func(orgRepo *orgtests.MockOrganizationRepository, teamRepo *orgtests.MockOrganizationTeamRepository, memberRepo *orgtests.MockOrganizationMemberRepository) { orgRepo.On("GetByID", mock.Anything, "org-1").Return(&types.Organization{ID: "org-1", OwnerID: "user-1"}, nil).Once() + memberRepo.On("GetByOrganizationIDAndUserID", mock.Anything, "org-1", "user-1").Return(&types.OrganizationMember{ID: "mem-1", OrganizationID: "org-1", UserID: "user-1", Role: "owner"}, nil).Once() teamRepo.On("GetAllByOrganizationID", mock.Anything, "org-1").Return([]types.OrganizationTeam{{ID: "team-1", OrganizationID: "org-1", Name: "Platform", Slug: "platform"}}, nil).Once() }, expectLen: 1, @@ -308,6 +314,7 @@ func TestOrganizationTeamService_GetAllTeams(t *testing.T) { organizationID: "org-1", setup: func(orgRepo *orgtests.MockOrganizationRepository, teamRepo *orgtests.MockOrganizationTeamRepository, memberRepo *orgtests.MockOrganizationMemberRepository) { orgRepo.On("GetByID", mock.Anything, "org-1").Return(&types.Organization{ID: "org-1", OwnerID: "user-1"}, nil).Once() + memberRepo.On("GetByOrganizationIDAndUserID", mock.Anything, "org-1", "user-1").Return(&types.OrganizationMember{ID: "mem-1", OrganizationID: "org-1", UserID: "user-1", Role: "owner"}, nil).Once() teamRepo.On("GetAllByOrganizationID", mock.Anything, "org-1").Return(([]types.OrganizationTeam)(nil), repoErr).Once() }, expectErr: repoErr, @@ -358,7 +365,7 @@ func TestOrganizationTeamService_GetTeam(t *testing.T) { setup: func(orgRepo *orgtests.MockOrganizationRepository, teamRepo *orgtests.MockOrganizationTeamRepository, memberRepo *orgtests.MockOrganizationMemberRepository, teamMemberRepo *orgtests.MockOrganizationTeamMemberRepository) { orgRepo.On("GetByID", mock.Anything, "org-1").Return(&types.Organization{ID: "org-1", OwnerID: "user-1"}, nil).Once() teamRepo.On("GetByID", mock.Anything, "team-1").Return(&types.OrganizationTeam{ID: "team-1", OrganizationID: "org-1", Name: "Platform", Slug: "platform"}, nil).Twice() - memberRepo.On("GetByOrganizationIDAndUserID", mock.Anything, "org-1", "user-1").Return(&types.OrganizationMember{ID: "mem-1", OrganizationID: "org-1", UserID: "user-1", Role: "member"}, nil).Once() + memberRepo.On("GetByOrganizationIDAndUserID", mock.Anything, "org-1", "user-1").Return(&types.OrganizationMember{ID: "mem-1", OrganizationID: "org-1", UserID: "user-1", Role: "member"}, nil).Twice() teamMemberRepo.On("GetByTeamIDAndMemberID", mock.Anything, "team-1", "mem-1").Return(&types.OrganizationTeamMember{ID: "team-member-1", TeamID: "team-1", MemberID: "mem-1"}, nil).Once() }, expectTeamID: "team-1", @@ -458,7 +465,7 @@ func TestOrganizationTeamService_UpdateTeam(t *testing.T) { teamRepo.On("Update", mock.Anything, mock.MatchedBy(func(team *types.OrganizationTeam) bool { return team != nil && team.ID == "team-1" && team.Name == "Platform Revamp" && team.Slug == "platform" })).Return(&types.OrganizationTeam{ID: "team-1", OrganizationID: "org-1", Name: "Platform Revamp", Slug: "platform"}, nil).Once() - memberRepo.On("GetByOrganizationIDAndUserID", mock.Anything, "org-1", "user-1").Return(&types.OrganizationMember{ID: "mem-1", OrganizationID: "org-1", UserID: "user-1", Role: "owner"}, nil).Once() + memberRepo.On("GetByOrganizationIDAndUserID", mock.Anything, "org-1", "user-1").Return(&types.OrganizationMember{ID: "mem-1", OrganizationID: "org-1", UserID: "user-1", Role: "owner"}, nil).Twice() teamMemberRepo.On("GetByTeamIDAndMemberID", mock.Anything, "team-1", "mem-1").Return(&types.OrganizationTeamMember{ID: "team-member-1", TeamID: "team-1", MemberID: "mem-1"}, nil).Once() }, }, @@ -535,7 +542,7 @@ func TestOrganizationTeamService_UpdateTeam(t *testing.T) { orgRepo.On("GetByID", mock.Anything, "org-1").Return(&types.Organization{ID: "org-1", OwnerID: "user-1"}, nil).Once() teamRepo.On("GetByID", mock.Anything, "team-1").Return(&types.OrganizationTeam{ID: "team-1", OrganizationID: "org-1", Name: "Platform", Slug: "platform"}, nil).Once() teamRepo.On("GetByID", mock.Anything, "team-1").Return((*types.OrganizationTeam)(nil), repoErr).Once() - memberRepo.On("GetByOrganizationIDAndUserID", mock.Anything, "org-1", "user-1").Return(&types.OrganizationMember{ID: "mem-1", OrganizationID: "org-1", UserID: "user-1", Role: "owner"}, nil).Once() + memberRepo.On("GetByOrganizationIDAndUserID", mock.Anything, "org-1", "user-1").Return(&types.OrganizationMember{ID: "mem-1", OrganizationID: "org-1", UserID: "user-1", Role: "owner"}, nil).Twice() teamMemberRepo.On("GetByTeamIDAndMemberID", mock.Anything, "team-1", "mem-1").Return(&types.OrganizationTeamMember{ID: "team-member-1", TeamID: "team-1", MemberID: "mem-1"}, nil).Once() }, expectErr: repoErr, @@ -550,7 +557,7 @@ func TestOrganizationTeamService_UpdateTeam(t *testing.T) { orgRepo.On("GetByID", mock.Anything, "org-1").Return(&types.Organization{ID: "org-1", OwnerID: "user-1"}, nil).Once() teamRepo.On("GetByID", mock.Anything, "team-1").Return(&types.OrganizationTeam{ID: "team-1", OrganizationID: "org-1", Name: "Platform", Slug: "platform"}, nil).Once() teamRepo.On("GetByID", mock.Anything, "team-1").Return(nil, nil).Once() - memberRepo.On("GetByOrganizationIDAndUserID", mock.Anything, "org-1", "user-1").Return(&types.OrganizationMember{ID: "mem-1", OrganizationID: "org-1", UserID: "user-1", Role: "owner"}, nil).Once() + memberRepo.On("GetByOrganizationIDAndUserID", mock.Anything, "org-1", "user-1").Return(&types.OrganizationMember{ID: "mem-1", OrganizationID: "org-1", UserID: "user-1", Role: "owner"}, nil).Twice() teamMemberRepo.On("GetByTeamIDAndMemberID", mock.Anything, "team-1", "mem-1").Return(&types.OrganizationTeamMember{ID: "team-member-1", TeamID: "team-1", MemberID: "mem-1"}, nil).Once() }, expectErr: coreerrors.ErrNotFound, @@ -565,7 +572,7 @@ func TestOrganizationTeamService_UpdateTeam(t *testing.T) { orgRepo.On("GetByID", mock.Anything, "org-1").Return(&types.Organization{ID: "org-1", OwnerID: "user-1"}, nil).Once() teamRepo.On("GetByID", mock.Anything, "team-1").Return(&types.OrganizationTeam{ID: "team-1", OrganizationID: "org-1", Name: "Platform", Slug: "platform"}, nil).Once() teamRepo.On("GetByID", mock.Anything, "team-1").Return(&types.OrganizationTeam{ID: "team-1", OrganizationID: "org-2", Name: "Platform", Slug: "platform"}, nil).Once() - memberRepo.On("GetByOrganizationIDAndUserID", mock.Anything, "org-1", "user-1").Return(&types.OrganizationMember{ID: "mem-1", OrganizationID: "org-1", UserID: "user-1", Role: "owner"}, nil).Once() + memberRepo.On("GetByOrganizationIDAndUserID", mock.Anything, "org-1", "user-1").Return(&types.OrganizationMember{ID: "mem-1", OrganizationID: "org-1", UserID: "user-1", Role: "owner"}, nil).Twice() teamMemberRepo.On("GetByTeamIDAndMemberID", mock.Anything, "team-1", "mem-1").Return(&types.OrganizationTeamMember{ID: "team-member-1", TeamID: "team-1", MemberID: "mem-1"}, nil).Once() }, expectErr: coreerrors.ErrNotFound, @@ -579,7 +586,7 @@ func TestOrganizationTeamService_UpdateTeam(t *testing.T) { setup: func(orgRepo *orgtests.MockOrganizationRepository, teamRepo *orgtests.MockOrganizationTeamRepository, memberRepo *orgtests.MockOrganizationMemberRepository, hooks *orgtests.MockOrganizationTeamHooks, teamMemberRepo *orgtests.MockOrganizationTeamMemberRepository) { orgRepo.On("GetByID", mock.Anything, "org-1").Return(&types.Organization{ID: "org-1", OwnerID: "user-1"}, nil).Once() teamRepo.On("GetByID", mock.Anything, "team-1").Return(&types.OrganizationTeam{ID: "team-1", OrganizationID: "org-1", Name: "Platform", Slug: "platform"}, nil).Twice() - memberRepo.On("GetByOrganizationIDAndUserID", mock.Anything, "org-1", "user-1").Return(&types.OrganizationMember{ID: "mem-1", OrganizationID: "org-1", UserID: "user-1", Role: "owner"}, nil).Once() + memberRepo.On("GetByOrganizationIDAndUserID", mock.Anything, "org-1", "user-1").Return(&types.OrganizationMember{ID: "mem-1", OrganizationID: "org-1", UserID: "user-1", Role: "owner"}, nil).Twice() teamMemberRepo.On("GetByTeamIDAndMemberID", mock.Anything, "team-1", "mem-1").Return(&types.OrganizationTeamMember{ID: "team-member-1", TeamID: "team-1", MemberID: "mem-1"}, nil).Once() }, expectErr: coreerrors.ErrBadRequest, @@ -593,7 +600,7 @@ func TestOrganizationTeamService_UpdateTeam(t *testing.T) { setup: func(orgRepo *orgtests.MockOrganizationRepository, teamRepo *orgtests.MockOrganizationTeamRepository, memberRepo *orgtests.MockOrganizationMemberRepository, hooks *orgtests.MockOrganizationTeamHooks, teamMemberRepo *orgtests.MockOrganizationTeamMemberRepository) { orgRepo.On("GetByID", mock.Anything, "org-1").Return(&types.Organization{ID: "org-1", OwnerID: "user-1"}, nil).Once() teamRepo.On("GetByID", mock.Anything, "team-1").Return(&types.OrganizationTeam{ID: "team-1", OrganizationID: "org-1", Name: "Platform", Slug: "platform"}, nil).Twice() - memberRepo.On("GetByOrganizationIDAndUserID", mock.Anything, "org-1", "user-1").Return(&types.OrganizationMember{ID: "mem-1", OrganizationID: "org-1", UserID: "user-1", Role: "owner"}, nil).Once() + memberRepo.On("GetByOrganizationIDAndUserID", mock.Anything, "org-1", "user-1").Return(&types.OrganizationMember{ID: "mem-1", OrganizationID: "org-1", UserID: "user-1", Role: "owner"}, nil).Twice() teamMemberRepo.On("GetByTeamIDAndMemberID", mock.Anything, "team-1", "mem-1").Return(&types.OrganizationTeamMember{ID: "team-member-1", TeamID: "team-1", MemberID: "mem-1"}, nil).Once() }, expectErr: coreerrors.ErrBadRequest, @@ -608,7 +615,7 @@ func TestOrganizationTeamService_UpdateTeam(t *testing.T) { orgRepo.On("GetByID", mock.Anything, "org-1").Return(&types.Organization{ID: "org-1", OwnerID: "user-1"}, nil).Once() teamRepo.On("GetByID", mock.Anything, "team-1").Return(&types.OrganizationTeam{ID: "team-1", OrganizationID: "org-1", Name: "Platform", Slug: "platform"}, nil).Twice() teamRepo.On("GetByOrganizationIDAndSlug", mock.Anything, "org-1", "platform").Return((*types.OrganizationTeam)(nil), repoErr).Once() - memberRepo.On("GetByOrganizationIDAndUserID", mock.Anything, "org-1", "user-1").Return(&types.OrganizationMember{ID: "mem-1", OrganizationID: "org-1", UserID: "user-1", Role: "owner"}, nil).Once() + memberRepo.On("GetByOrganizationIDAndUserID", mock.Anything, "org-1", "user-1").Return(&types.OrganizationMember{ID: "mem-1", OrganizationID: "org-1", UserID: "user-1", Role: "owner"}, nil).Twice() teamMemberRepo.On("GetByTeamIDAndMemberID", mock.Anything, "team-1", "mem-1").Return(&types.OrganizationTeamMember{ID: "team-member-1", TeamID: "team-1", MemberID: "mem-1"}, nil).Once() }, expectErr: repoErr, @@ -623,7 +630,7 @@ func TestOrganizationTeamService_UpdateTeam(t *testing.T) { orgRepo.On("GetByID", mock.Anything, "org-1").Return(&types.Organization{ID: "org-1", OwnerID: "user-1"}, nil).Once() teamRepo.On("GetByID", mock.Anything, "team-1").Return(&types.OrganizationTeam{ID: "team-1", OrganizationID: "org-1", Name: "Platform", Slug: "platform"}, nil).Twice() teamRepo.On("GetByOrganizationIDAndSlug", mock.Anything, "org-1", "platform").Return(&types.OrganizationTeam{ID: "team-2"}, nil).Once() - memberRepo.On("GetByOrganizationIDAndUserID", mock.Anything, "org-1", "user-1").Return(&types.OrganizationMember{ID: "mem-1", OrganizationID: "org-1", UserID: "user-1", Role: "owner"}, nil).Once() + memberRepo.On("GetByOrganizationIDAndUserID", mock.Anything, "org-1", "user-1").Return(&types.OrganizationMember{ID: "mem-1", OrganizationID: "org-1", UserID: "user-1", Role: "owner"}, nil).Twice() teamMemberRepo.On("GetByTeamIDAndMemberID", mock.Anything, "team-1", "mem-1").Return(&types.OrganizationTeamMember{ID: "team-member-1", TeamID: "team-1", MemberID: "mem-1"}, nil).Once() }, expectErr: coreerrors.ErrConflict, @@ -641,7 +648,7 @@ func TestOrganizationTeamService_UpdateTeam(t *testing.T) { teamRepo.On("Update", mock.Anything, mock.MatchedBy(func(team *types.OrganizationTeam) bool { return team != nil && team.ID == "team-1" && team.Name == "Platform Revamp" && team.Slug == "platform" })).Return((*types.OrganizationTeam)(nil), updateErr).Once() - memberRepo.On("GetByOrganizationIDAndUserID", mock.Anything, "org-1", "user-1").Return(&types.OrganizationMember{ID: "mem-1", OrganizationID: "org-1", UserID: "user-1", Role: "owner"}, nil).Once() + memberRepo.On("GetByOrganizationIDAndUserID", mock.Anything, "org-1", "user-1").Return(&types.OrganizationMember{ID: "mem-1", OrganizationID: "org-1", UserID: "user-1", Role: "owner"}, nil).Twice() teamMemberRepo.On("GetByTeamIDAndMemberID", mock.Anything, "team-1", "mem-1").Return(&types.OrganizationTeamMember{ID: "team-member-1", TeamID: "team-1", MemberID: "mem-1"}, nil).Once() }, expectErr: updateErr, @@ -698,7 +705,7 @@ func TestOrganizationTeamService_DeleteTeam(t *testing.T) { orgRepo.On("GetByID", mock.Anything, "org-1").Return(&types.Organization{ID: "org-1", OwnerID: "user-1"}, nil).Once() teamRepo.On("GetByID", mock.Anything, "team-1").Return(&types.OrganizationTeam{ID: "team-1", OrganizationID: "org-1", Name: "Platform", Slug: "platform"}, nil).Twice() teamRepo.On("Delete", mock.Anything, "team-1").Return(nil).Once() - memberRepo.On("GetByOrganizationIDAndUserID", mock.Anything, "org-1", "user-1").Return(&types.OrganizationMember{ID: "mem-1", OrganizationID: "org-1", UserID: "user-1", Role: "owner"}, nil).Once() + memberRepo.On("GetByOrganizationIDAndUserID", mock.Anything, "org-1", "user-1").Return(&types.OrganizationMember{ID: "mem-1", OrganizationID: "org-1", UserID: "user-1", Role: "owner"}, nil).Twice() teamMemberRepo.On("GetByTeamIDAndMemberID", mock.Anything, "team-1", "mem-1").Return(&types.OrganizationTeamMember{ID: "team-member-1", TeamID: "team-1", MemberID: "mem-1"}, nil).Once() }, }, @@ -767,7 +774,7 @@ func TestOrganizationTeamService_DeleteTeam(t *testing.T) { orgRepo.On("GetByID", mock.Anything, "org-1").Return(&types.Organization{ID: "org-1", OwnerID: "user-1"}, nil).Once() teamRepo.On("GetByID", mock.Anything, "team-1").Return(&types.OrganizationTeam{ID: "team-1", OrganizationID: "org-1", Name: "Platform", Slug: "platform"}, nil).Once() teamRepo.On("GetByID", mock.Anything, "team-1").Return((*types.OrganizationTeam)(nil), repoErr).Once() - memberRepo.On("GetByOrganizationIDAndUserID", mock.Anything, "org-1", "user-1").Return(&types.OrganizationMember{ID: "mem-1", OrganizationID: "org-1", UserID: "user-1", Role: "owner"}, nil).Once() + memberRepo.On("GetByOrganizationIDAndUserID", mock.Anything, "org-1", "user-1").Return(&types.OrganizationMember{ID: "mem-1", OrganizationID: "org-1", UserID: "user-1", Role: "owner"}, nil).Twice() teamMemberRepo.On("GetByTeamIDAndMemberID", mock.Anything, "team-1", "mem-1").Return(&types.OrganizationTeamMember{ID: "team-member-1", TeamID: "team-1", MemberID: "mem-1"}, nil).Once() }, expectErr: repoErr, @@ -781,7 +788,7 @@ func TestOrganizationTeamService_DeleteTeam(t *testing.T) { orgRepo.On("GetByID", mock.Anything, "org-1").Return(&types.Organization{ID: "org-1", OwnerID: "user-1"}, nil).Once() teamRepo.On("GetByID", mock.Anything, "team-1").Return(&types.OrganizationTeam{ID: "team-1", OrganizationID: "org-1", Name: "Platform", Slug: "platform"}, nil).Once() teamRepo.On("GetByID", mock.Anything, "team-1").Return(nil, nil).Once() - memberRepo.On("GetByOrganizationIDAndUserID", mock.Anything, "org-1", "user-1").Return(&types.OrganizationMember{ID: "mem-1", OrganizationID: "org-1", UserID: "user-1", Role: "owner"}, nil).Once() + memberRepo.On("GetByOrganizationIDAndUserID", mock.Anything, "org-1", "user-1").Return(&types.OrganizationMember{ID: "mem-1", OrganizationID: "org-1", UserID: "user-1", Role: "owner"}, nil).Twice() teamMemberRepo.On("GetByTeamIDAndMemberID", mock.Anything, "team-1", "mem-1").Return(&types.OrganizationTeamMember{ID: "team-member-1", TeamID: "team-1", MemberID: "mem-1"}, nil).Once() }, expectErr: coreerrors.ErrNotFound, @@ -795,7 +802,7 @@ func TestOrganizationTeamService_DeleteTeam(t *testing.T) { orgRepo.On("GetByID", mock.Anything, "org-1").Return(&types.Organization{ID: "org-1", OwnerID: "user-1"}, nil).Once() teamRepo.On("GetByID", mock.Anything, "team-1").Return(&types.OrganizationTeam{ID: "team-1", OrganizationID: "org-1", Name: "Platform", Slug: "platform"}, nil).Once() teamRepo.On("GetByID", mock.Anything, "team-1").Return(&types.OrganizationTeam{ID: "team-1", OrganizationID: "org-2", Name: "Platform", Slug: "platform"}, nil).Once() - memberRepo.On("GetByOrganizationIDAndUserID", mock.Anything, "org-1", "user-1").Return(&types.OrganizationMember{ID: "mem-1", OrganizationID: "org-1", UserID: "user-1", Role: "owner"}, nil).Once() + memberRepo.On("GetByOrganizationIDAndUserID", mock.Anything, "org-1", "user-1").Return(&types.OrganizationMember{ID: "mem-1", OrganizationID: "org-1", UserID: "user-1", Role: "owner"}, nil).Twice() teamMemberRepo.On("GetByTeamIDAndMemberID", mock.Anything, "team-1", "mem-1").Return(&types.OrganizationTeamMember{ID: "team-member-1", TeamID: "team-1", MemberID: "mem-1"}, nil).Once() }, expectErr: coreerrors.ErrNotFound, @@ -809,7 +816,7 @@ func TestOrganizationTeamService_DeleteTeam(t *testing.T) { orgRepo.On("GetByID", mock.Anything, "org-1").Return(&types.Organization{ID: "org-1", OwnerID: "user-1"}, nil).Once() teamRepo.On("GetByID", mock.Anything, "team-1").Return(&types.OrganizationTeam{ID: "team-1", OrganizationID: "org-1", Name: "Platform", Slug: "platform"}, nil).Twice() teamRepo.On("Delete", mock.Anything, "team-1").Return(repoErr).Once() - memberRepo.On("GetByOrganizationIDAndUserID", mock.Anything, "org-1", "user-1").Return(&types.OrganizationMember{ID: "mem-1", OrganizationID: "org-1", UserID: "user-1", Role: "owner"}, nil).Once() + memberRepo.On("GetByOrganizationIDAndUserID", mock.Anything, "org-1", "user-1").Return(&types.OrganizationMember{ID: "mem-1", OrganizationID: "org-1", UserID: "user-1", Role: "owner"}, nil).Twice() teamMemberRepo.On("GetByTeamIDAndMemberID", mock.Anything, "team-1", "mem-1").Return(&types.OrganizationTeamMember{ID: "team-member-1", TeamID: "team-1", MemberID: "mem-1"}, nil).Once() }, expectErr: repoErr, diff --git a/plugins/organizations/services/service_utils.go b/plugins/organizations/services/service_utils.go index 2df0e9cd..cb443cc7 100644 --- a/plugins/organizations/services/service_utils.go +++ b/plugins/organizations/services/service_utils.go @@ -2,11 +2,13 @@ package services import ( "context" + "errors" coreerrors "github.com/Authula/authula/core/errors" "github.com/Authula/authula/models" "github.com/Authula/authula/plugins/organizations/repositories" "github.com/Authula/authula/plugins/organizations/types" + rootservices "github.com/Authula/authula/services" ) type ServiceUtils struct { @@ -37,8 +39,8 @@ func (s *ServiceUtils) authorizeOwner(ctx context.Context, actor *models.Actor, if organization == nil { return nil, coreerrors.ErrNotFound } - if _, ok := actor.GetClaimString("organization_id"); ok { - return organization, nil + if err := verifyOrgClaim(actor, organizationID); err != nil { + return nil, err } if organization.OwnerID != actor.ID { return nil, coreerrors.ErrForbidden @@ -47,7 +49,7 @@ func (s *ServiceUtils) authorizeOwner(ctx context.Context, actor *models.Actor, return organization, nil } -func (s *ServiceUtils) authorizeOrganizationAccess(ctx context.Context, actor *models.Actor, organizationID string) (*types.Organization, *types.OrganizationMember, error) { +func (s *ServiceUtils) AuthorizeOrganizationAccess(ctx context.Context, actor *models.Actor, organizationID string) (*types.Organization, *types.OrganizationMember, error) { if actor == nil || actor.ID == "" || organizationID == "" { return nil, nil, coreerrors.ErrUnauthorized } @@ -59,10 +61,11 @@ func (s *ServiceUtils) authorizeOrganizationAccess(ctx context.Context, actor *m if organization == nil { return nil, nil, coreerrors.ErrNotFound } - if _, ok := actor.GetClaimString("organization_id"); ok { - return organization, nil, nil + if err := verifyOrgClaim(actor, organizationID); err != nil { + return nil, nil, err } - if organization.OwnerID == actor.ID { + + if actor.Type == models.ActorMachine { return organization, nil, nil } @@ -86,7 +89,11 @@ func (s *ServiceUtils) authorizeTeamAccess(ctx context.Context, actor *models.Ac return coreerrors.ErrNotFound } - if _, ok := actor.GetClaimString("organization_id"); ok { + if err := verifyOrgClaim(actor, orgID); err != nil { + return err + } + + if actor.Type == models.ActorMachine { return nil } @@ -107,3 +114,44 @@ func (s *ServiceUtils) authorizeTeamAccess(ctx context.Context, actor *models.Ac } return nil } + +func verifyOrgClaim(actor *models.Actor, organizationID string) error { + claimOrgID, hasClaim := actor.GetClaimString("organization_id") + if !hasClaim || claimOrgID == "" { + if actor.Type == models.ActorMachine { + return coreerrors.ErrForbidden + } + return nil + } + if claimOrgID != organizationID { + return coreerrors.ErrForbidden + } + return nil +} + +func authorizeRoleWeight(ctx context.Context, accessControl rootservices.AccessControlService, actorMember *types.OrganizationMember, targetRole string) error { + if actorMember == nil { + return coreerrors.ErrForbidden + } + + actorWeight, err := accessControl.GetRoleWeightByName(ctx, actorMember.Role) + if err != nil { + if errors.Is(err, coreerrors.ErrNotFound) { + return coreerrors.ErrForbidden + } + return err + } + + targetWeight, err := accessControl.GetRoleWeightByName(ctx, targetRole) + if err != nil { + if errors.Is(err, coreerrors.ErrNotFound) { + return coreerrors.ErrBadRequest + } + return err + } + + if targetWeight > actorWeight { + return coreerrors.ErrForbidden + } + return nil +} diff --git a/plugins/organizations/services/service_utils_test.go b/plugins/organizations/services/service_utils_test.go index 70d880a3..ea8416f8 100644 --- a/plugins/organizations/services/service_utils_test.go +++ b/plugins/organizations/services/service_utils_test.go @@ -106,6 +106,7 @@ func TestServiceUtils_authorizeOrganizationAccess(t *testing.T) { tests := []struct { name string + actor *models.Actor actorUserID string organization string setup func(*orgtests.MockOrganizationRepository, *orgtests.MockOrganizationMemberRepository) @@ -119,8 +120,9 @@ func TestServiceUtils_authorizeOrganizationAccess(t *testing.T) { organization: "org-1", setup: func(orgRepo *orgtests.MockOrganizationRepository, memberRepo *orgtests.MockOrganizationMemberRepository) { orgRepo.On("GetByID", mock.Anything, "org-1").Return(&types.Organization{ID: "org-1", OwnerID: "user-1"}, nil).Once() + memberRepo.On("GetByOrganizationIDAndUserID", mock.Anything, "org-1", "user-1").Return(&types.OrganizationMember{ID: "mem-1", OrganizationID: "org-1", UserID: "user-1", Role: "owner"}, nil).Once() }, - expectOwner: "user-1", + expectMember: "user-1", }, { name: "member access", @@ -132,6 +134,42 @@ func TestServiceUtils_authorizeOrganizationAccess(t *testing.T) { }, expectMember: "user-2", }, + { + name: "machine with matching claim is allowed", + actor: &models.Actor{ID: "key-1", Type: models.ActorMachine, Claims: map[string]any{"organization_id": "org-1"}}, + organization: "org-1", + setup: func(orgRepo *orgtests.MockOrganizationRepository, memberRepo *orgtests.MockOrganizationMemberRepository) { + orgRepo.On("GetByID", mock.Anything, "org-1").Return(&types.Organization{ID: "org-1", OwnerID: "user-1"}, nil).Once() + }, + expectOwner: "user-1", + }, + { + name: "machine without claim is forbidden", + actor: &models.Actor{ID: "key-1", Type: models.ActorMachine}, + organization: "org-1", + setup: func(orgRepo *orgtests.MockOrganizationRepository, memberRepo *orgtests.MockOrganizationMemberRepository) { + orgRepo.On("GetByID", mock.Anything, "org-1").Return(&types.Organization{ID: "org-1", OwnerID: "user-1"}, nil).Once() + }, + expectErr: coreerrors.ErrForbidden, + }, + { + name: "machine with mismatched claim is forbidden", + actor: &models.Actor{ID: "key-1", Type: models.ActorMachine, Claims: map[string]any{"organization_id": "org-2"}}, + organization: "org-1", + setup: func(orgRepo *orgtests.MockOrganizationRepository, memberRepo *orgtests.MockOrganizationMemberRepository) { + orgRepo.On("GetByID", mock.Anything, "org-1").Return(&types.Organization{ID: "org-1", OwnerID: "user-1"}, nil).Once() + }, + expectErr: coreerrors.ErrForbidden, + }, + { + name: "user with mismatched claim is forbidden", + actor: &models.Actor{ID: "user-1", Type: models.ActorUser, Claims: map[string]any{"organization_id": "org-2"}}, + organization: "org-1", + setup: func(orgRepo *orgtests.MockOrganizationRepository, memberRepo *orgtests.MockOrganizationMemberRepository) { + orgRepo.On("GetByID", mock.Anything, "org-1").Return(&types.Organization{ID: "org-1", OwnerID: "user-1"}, nil).Once() + }, + expectErr: coreerrors.ErrForbidden, + }, { name: "unauthorized when inputs are missing", actorUserID: "", @@ -188,7 +226,11 @@ func TestServiceUtils_authorizeOrganizationAccess(t *testing.T) { tt.setup(orgRepo, memberRepo) } - org, member, err := (&ServiceUtils{orgRepo: orgRepo, orgMemberRepo: memberRepo}).authorizeOrganizationAccess(context.Background(), orgtests.Actor(tt.actorUserID), tt.organization) + actor := tt.actor + if actor == nil { + actor = orgtests.Actor(tt.actorUserID) + } + org, member, err := (&ServiceUtils{orgRepo: orgRepo, orgMemberRepo: memberRepo}).AuthorizeOrganizationAccess(context.Background(), actor, tt.organization) if tt.expectErr != nil { require.ErrorIs(t, err, tt.expectErr) require.Nil(t, org) diff --git a/plugins/organizations/tests/access_control_stub.go b/plugins/organizations/tests/access_control_stub.go index 038d3ab7..d519fd89 100644 --- a/plugins/organizations/tests/access_control_stub.go +++ b/plugins/organizations/tests/access_control_stub.go @@ -9,6 +9,7 @@ import ( type AccessControlServiceStub struct { RoleWeights map[string]int + RolePermissions map[string][]string AssignerWeights map[string]int Err error } @@ -33,6 +34,7 @@ func NewAccessControlServiceStubWithWeights(roleWeights, assignerWeights map[str return &AccessControlServiceStub{ RoleWeights: roleWeights, + RolePermissions: map[string][]string{}, AssignerWeights: assignerWeights, } } @@ -51,34 +53,34 @@ func (s *AccessControlServiceStub) RoleExists(ctx context.Context, roleName stri return true, nil } -func (s *AccessControlServiceStub) ValidateRoleAssignment(ctx context.Context, roleName string, assignerUserID *string) (bool, error) { +func (s *AccessControlServiceStub) GetRolePermissionsByName(ctx context.Context, roleName string) ([]string, error) { _ = ctx if s != nil && s.Err != nil { - return false, s.Err + return nil, s.Err } - roleWeight, ok := s.roleWeight(roleName) - if !ok { - return false, nil + if perms, ok := s.RolePermissions[roleName]; ok { + return perms, nil } - if s == nil || len(s.AssignerWeights) == 0 { - return true, nil + if _, ok := s.roleWeight(roleName); ok { + return []string{"*"}, nil } - if assignerUserID == nil || *assignerUserID == "" { - return false, nil - } + return nil, coreerrors.ErrNotFound +} - assignerWeight, ok := s.assignerWeight(*assignerUserID) - if !ok { - return false, nil +func (s *AccessControlServiceStub) GetRoleWeightByName(ctx context.Context, roleName string) (int, error) { + _ = ctx + if s != nil && s.Err != nil { + return 0, s.Err } - if roleWeight > assignerWeight { - return false, coreerrors.ErrForbidden + weight, ok := s.roleWeight(roleName) + if !ok { + return 0, coreerrors.ErrNotFound } - return true, nil + return weight, nil } func (s *AccessControlServiceStub) ValidatePermissionKeys(ctx context.Context, permissionKeys []string) error { @@ -108,11 +110,3 @@ func (s *AccessControlServiceStub) roleWeight(roleName string) (int, bool) { weight, ok := s.RoleWeights[roleName] return weight, ok } - -func (s *AccessControlServiceStub) assignerWeight(userID string) (int, bool) { - if s == nil { - return 0, false - } - weight, ok := s.AssignerWeights[userID] - return weight, ok -} diff --git a/plugins/organizations/usecases/usecases.go b/plugins/organizations/usecases/usecases.go index 75f2d750..3aa48b1c 100644 --- a/plugins/organizations/usecases/usecases.go +++ b/plugins/organizations/usecases/usecases.go @@ -2,6 +2,7 @@ package usecases import ( "context" + "errors" "strings" coreerrors "github.com/Authula/authula/core/errors" @@ -21,6 +22,8 @@ type UseCases struct { userService rootservices.UserService globalConfig *models.Config authorizer rootservices.Authorizer + serviceUtils *orgservices.ServiceUtils + accessControl rootservices.AccessControlService } func NewUseCases( @@ -32,6 +35,8 @@ func NewUseCases( userService rootservices.UserService, globalConfig *models.Config, authorizer rootservices.Authorizer, + serviceUtils *orgservices.ServiceUtils, + accessControl rootservices.AccessControlService, ) *UseCases { return &UseCases{ orgService: orgService, @@ -42,9 +47,51 @@ func NewUseCases( userService: userService, globalConfig: globalConfig, authorizer: authorizer, + serviceUtils: serviceUtils, + accessControl: accessControl, } } +func (u *UseCases) authorizeOrgAccess(ctx context.Context, actor *models.Actor, orgID, requiredScope string) error { + if err := u.authorizer.AuthorizeOrganizationAccess(ctx, actor, orgID); err != nil { + return err + } + _, member, err := u.serviceUtils.AuthorizeOrganizationAccess(ctx, actor, orgID) + if err != nil { + return err + } + if member == nil { + if actor.Type != models.ActorMachine { + return coreerrors.ErrForbidden + } + return u.authorizer.AuthorizeScope(ctx, actor, requiredScope) + } + + perms, err := u.accessControl.GetRolePermissionsByName(ctx, member.Role) + if err != nil { + if errors.Is(err, coreerrors.ErrNotFound) { + return coreerrors.ErrForbidden + } + return err + } + if !hasPermissionKey(perms, requiredScope) { + return coreerrors.ErrInsufficientPermissions + } + return nil +} + +func hasPermissionKey(permissions []string, required string) bool { + for _, permission := range permissions { + if permission == "*" || permission == required { + return true + } + if strings.HasSuffix(permission, "*") && strings.HasPrefix(required, strings.TrimSuffix(permission, "*")) { + return true + } + } + return false +} + // ------------- OrganizationService ------------- func (u *UseCases) CreateOrganization(ctx context.Context, actor *models.Actor, request types.CreateOrganizationRequest) (*types.Organization, error) { @@ -56,30 +103,21 @@ func (u *UseCases) GetAllOrganizationsByOwner(ctx context.Context, actor *models } func (u *UseCases) GetOrganizationByID(ctx context.Context, actor *models.Actor, organizationID string) (*types.Organization, error) { - if err := u.authorizer.AuthorizeOrganizationAccess(ctx, actor, organizationID); err != nil { - return nil, err - } - if err := u.authorizer.AuthorizeScope(ctx, actor, orgconstants.OrganizationsReadPermission); err != nil { + if err := u.authorizeOrgAccess(ctx, actor, organizationID, orgconstants.OrganizationsReadPermission); err != nil { return nil, err } return u.orgService.GetOrganizationByID(ctx, actor, organizationID) } func (u *UseCases) UpdateOrganization(ctx context.Context, actor *models.Actor, organizationID string, request types.UpdateOrganizationRequest) (*types.Organization, error) { - if err := u.authorizer.AuthorizeOrganizationAccess(ctx, actor, organizationID); err != nil { - return nil, err - } - if err := u.authorizer.AuthorizeScope(ctx, actor, orgconstants.OrganizationsUpdatePermission); err != nil { + if err := u.authorizeOrgAccess(ctx, actor, organizationID, orgconstants.OrganizationsUpdatePermission); err != nil { return nil, err } return u.orgService.UpdateOrganization(ctx, actor, organizationID, request) } func (u *UseCases) DeleteOrganization(ctx context.Context, actor *models.Actor, organizationID string) error { - if err := u.authorizer.AuthorizeOrganizationAccess(ctx, actor, organizationID); err != nil { - return err - } - if err := u.authorizer.AuthorizeScope(ctx, actor, orgconstants.OrganizationsDeletePermission); err != nil { + if err := u.authorizeOrgAccess(ctx, actor, organizationID, orgconstants.OrganizationsDeletePermission); err != nil { return err } return u.orgService.DeleteOrganization(ctx, actor, organizationID) @@ -92,20 +130,14 @@ func (u *UseCases) ExistsByID(ctx context.Context, organizationID string) (bool, // ------------- OrganizationInvitationService ------------- func (u *UseCases) CreateOrganizationInvitation(ctx context.Context, actor *models.Actor, organizationID string, request types.CreateOrganizationInvitationRequest, redirectURL string) (*types.OrganizationInvitation, error) { - if err := u.authorizer.AuthorizeOrganizationAccess(ctx, actor, organizationID); err != nil { - return nil, err - } - if err := u.authorizer.AuthorizeScope(ctx, actor, orgconstants.OrganizationsInvitationsCreatePermission); err != nil { + if err := u.authorizeOrgAccess(ctx, actor, organizationID, orgconstants.OrganizationsInvitationsCreatePermission); err != nil { return nil, err } return u.invitationService.CreateOrganizationInvitation(ctx, actor, organizationID, request, redirectURL) } func (u *UseCases) GetAllOrganizationInvitations(ctx context.Context, actor *models.Actor, organizationID string) ([]types.GetOrganizationInvitationResponse, error) { - if err := u.authorizer.AuthorizeOrganizationAccess(ctx, actor, organizationID); err != nil { - return nil, err - } - if err := u.authorizer.AuthorizeScope(ctx, actor, orgconstants.OrganizationsInvitationsListPermission); err != nil { + if err := u.authorizeOrgAccess(ctx, actor, organizationID, orgconstants.OrganizationsInvitationsListPermission); err != nil { return nil, err } @@ -144,10 +176,7 @@ func (u *UseCases) GetOrganizationInvitation(ctx context.Context, actor *models. } func (u *UseCases) RevokeOrganizationInvitation(ctx context.Context, actor *models.Actor, organizationID string, invitationID string) (*types.OrganizationInvitation, error) { - if err := u.authorizer.AuthorizeOrganizationAccess(ctx, actor, organizationID); err != nil { - return nil, err - } - if err := u.authorizer.AuthorizeScope(ctx, actor, orgconstants.OrganizationsInvitationsRevokePermission); err != nil { + if err := u.authorizeOrgAccess(ctx, actor, organizationID, orgconstants.OrganizationsInvitationsRevokePermission); err != nil { return nil, err } return u.invitationService.RevokeOrganizationInvitation(ctx, actor, organizationID, invitationID) @@ -164,60 +193,42 @@ func (u *UseCases) RejectOrganizationInvitation(ctx context.Context, actor *mode // ------------- OrganizationMemberService ------------- func (u *UseCases) AddMember(ctx context.Context, actor *models.Actor, organizationID string, request types.AddOrganizationMemberRequest) (*types.OrganizationMember, error) { - if err := u.authorizer.AuthorizeOrganizationAccess(ctx, actor, organizationID); err != nil { - return nil, err - } - if err := u.authorizer.AuthorizeScope(ctx, actor, orgconstants.OrganizationsMembersAddPermission); err != nil { + if err := u.authorizeOrgAccess(ctx, actor, organizationID, orgconstants.OrganizationsMembersAddPermission); err != nil { return nil, err } return u.memberService.AddMember(ctx, actor, organizationID, request) } func (u *UseCases) GetAllMembers(ctx context.Context, actor *models.Actor, organizationID string, page int, limit int) ([]types.OrganizationMemberResponse, error) { - if err := u.authorizer.AuthorizeOrganizationAccess(ctx, actor, organizationID); err != nil { - return nil, err - } - if err := u.authorizer.AuthorizeScope(ctx, actor, orgconstants.OrganizationsMembersListPermission); err != nil { + if err := u.authorizeOrgAccess(ctx, actor, organizationID, orgconstants.OrganizationsMembersListPermission); err != nil { return nil, err } return u.memberService.GetAllMembers(ctx, actor, organizationID, page, limit) } func (u *UseCases) GetMember(ctx context.Context, actor *models.Actor, organizationID string, memberID string) (*types.OrganizationMemberResponse, error) { - if err := u.authorizer.AuthorizeOrganizationAccess(ctx, actor, organizationID); err != nil { - return nil, err - } - if err := u.authorizer.AuthorizeScope(ctx, actor, orgconstants.OrganizationsMembersReadPermission); err != nil { + if err := u.authorizeOrgAccess(ctx, actor, organizationID, orgconstants.OrganizationsMembersReadPermission); err != nil { return nil, err } return u.memberService.GetMember(ctx, actor, organizationID, memberID) } func (u *UseCases) GetMemberByUserID(ctx context.Context, actor *models.Actor, organizationID string, userID string) (*types.OrganizationMemberResponse, error) { - if err := u.authorizer.AuthorizeOrganizationAccess(ctx, actor, organizationID); err != nil { - return nil, err - } - if err := u.authorizer.AuthorizeScope(ctx, actor, orgconstants.OrganizationsMembersReadPermission); err != nil { + if err := u.authorizeOrgAccess(ctx, actor, organizationID, orgconstants.OrganizationsMembersReadPermission); err != nil { return nil, err } return u.memberService.GetMemberByUserID(ctx, actor, organizationID, userID) } func (u *UseCases) UpdateMember(ctx context.Context, actor *models.Actor, organizationID string, memberID string, request types.UpdateOrganizationMemberRequest) (*types.OrganizationMember, error) { - if err := u.authorizer.AuthorizeOrganizationAccess(ctx, actor, organizationID); err != nil { - return nil, err - } - if err := u.authorizer.AuthorizeScope(ctx, actor, orgconstants.OrganizationsMembersUpdatePermission); err != nil { + if err := u.authorizeOrgAccess(ctx, actor, organizationID, orgconstants.OrganizationsMembersUpdatePermission); err != nil { return nil, err } return u.memberService.UpdateMember(ctx, actor, organizationID, memberID, request) } func (u *UseCases) RemoveMember(ctx context.Context, actor *models.Actor, organizationID string, memberID string) error { - if err := u.authorizer.AuthorizeOrganizationAccess(ctx, actor, organizationID); err != nil { - return err - } - if err := u.authorizer.AuthorizeScope(ctx, actor, orgconstants.OrganizationsMembersRemovePermission); err != nil { + if err := u.authorizeOrgAccess(ctx, actor, organizationID, orgconstants.OrganizationsMembersRemovePermission); err != nil { return err } return u.memberService.RemoveMember(ctx, actor, organizationID, memberID) @@ -226,50 +237,35 @@ func (u *UseCases) RemoveMember(ctx context.Context, actor *models.Actor, organi // ------------- OrganizationTeamService ------------- func (u *UseCases) CreateTeam(ctx context.Context, actor *models.Actor, organizationID string, request types.CreateOrganizationTeamRequest) (*types.OrganizationTeam, error) { - if err := u.authorizer.AuthorizeOrganizationAccess(ctx, actor, organizationID); err != nil { - return nil, err - } - if err := u.authorizer.AuthorizeScope(ctx, actor, orgconstants.OrganizationsTeamsCreatePermission); err != nil { + if err := u.authorizeOrgAccess(ctx, actor, organizationID, orgconstants.OrganizationsTeamsCreatePermission); err != nil { return nil, err } return u.teamService.CreateTeam(ctx, actor, organizationID, request) } func (u *UseCases) GetAllTeams(ctx context.Context, actor *models.Actor, organizationID string) ([]types.OrganizationTeam, error) { - if err := u.authorizer.AuthorizeOrganizationAccess(ctx, actor, organizationID); err != nil { - return nil, err - } - if err := u.authorizer.AuthorizeScope(ctx, actor, orgconstants.OrganizationsTeamsListPermission); err != nil { + if err := u.authorizeOrgAccess(ctx, actor, organizationID, orgconstants.OrganizationsTeamsListPermission); err != nil { return nil, err } return u.teamService.GetAllTeams(ctx, actor, organizationID) } func (u *UseCases) GetTeam(ctx context.Context, actor *models.Actor, organizationID string, teamID string) (*types.OrganizationTeam, error) { - if err := u.authorizer.AuthorizeOrganizationAccess(ctx, actor, organizationID); err != nil { - return nil, err - } - if err := u.authorizer.AuthorizeScope(ctx, actor, orgconstants.OrganizationsTeamsReadPermission); err != nil { + if err := u.authorizeOrgAccess(ctx, actor, organizationID, orgconstants.OrganizationsTeamsReadPermission); err != nil { return nil, err } return u.teamService.GetTeam(ctx, actor, organizationID, teamID) } func (u *UseCases) UpdateTeam(ctx context.Context, actor *models.Actor, organizationID string, teamID string, request types.UpdateOrganizationTeamRequest) (*types.OrganizationTeam, error) { - if err := u.authorizer.AuthorizeOrganizationAccess(ctx, actor, organizationID); err != nil { - return nil, err - } - if err := u.authorizer.AuthorizeScope(ctx, actor, orgconstants.OrganizationsTeamsUpdatePermission); err != nil { + if err := u.authorizeOrgAccess(ctx, actor, organizationID, orgconstants.OrganizationsTeamsUpdatePermission); err != nil { return nil, err } return u.teamService.UpdateTeam(ctx, actor, organizationID, teamID, request) } func (u *UseCases) DeleteTeam(ctx context.Context, actor *models.Actor, organizationID string, teamID string) error { - if err := u.authorizer.AuthorizeOrganizationAccess(ctx, actor, organizationID); err != nil { - return err - } - if err := u.authorizer.AuthorizeScope(ctx, actor, orgconstants.OrganizationsTeamsDeletePermission); err != nil { + if err := u.authorizeOrgAccess(ctx, actor, organizationID, orgconstants.OrganizationsTeamsDeletePermission); err != nil { return err } return u.teamService.DeleteTeam(ctx, actor, organizationID, teamID) @@ -278,40 +274,28 @@ func (u *UseCases) DeleteTeam(ctx context.Context, actor *models.Actor, organiza // ------------- OrganizationTeamMemberService ------------- func (u *UseCases) AddTeamMember(ctx context.Context, actor *models.Actor, organizationID string, teamID string, request types.AddOrganizationTeamMemberRequest) (*types.OrganizationTeamMember, error) { - if err := u.authorizer.AuthorizeOrganizationAccess(ctx, actor, organizationID); err != nil { - return nil, err - } - if err := u.authorizer.AuthorizeScope(ctx, actor, orgconstants.OrganizationsTeamMembersAddPermission); err != nil { + if err := u.authorizeOrgAccess(ctx, actor, organizationID, orgconstants.OrganizationsTeamMembersAddPermission); err != nil { return nil, err } return u.teamMemberService.AddTeamMember(ctx, actor, organizationID, teamID, request) } func (u *UseCases) GetAllTeamMembers(ctx context.Context, actor *models.Actor, organizationID string, teamID string, page int, limit int) ([]types.OrganizationTeamMemberResponse, error) { - if err := u.authorizer.AuthorizeOrganizationAccess(ctx, actor, organizationID); err != nil { - return nil, err - } - if err := u.authorizer.AuthorizeScope(ctx, actor, orgconstants.OrganizationsTeamMembersListPermission); err != nil { + if err := u.authorizeOrgAccess(ctx, actor, organizationID, orgconstants.OrganizationsTeamMembersListPermission); err != nil { return nil, err } return u.teamMemberService.GetAllTeamMembers(ctx, actor, organizationID, teamID, page, limit) } func (u *UseCases) GetTeamMember(ctx context.Context, actor *models.Actor, organizationID string, teamID string, memberID string) (*types.OrganizationTeamMemberResponse, error) { - if err := u.authorizer.AuthorizeOrganizationAccess(ctx, actor, organizationID); err != nil { - return nil, err - } - if err := u.authorizer.AuthorizeScope(ctx, actor, orgconstants.OrganizationsTeamMembersReadPermission); err != nil { + if err := u.authorizeOrgAccess(ctx, actor, organizationID, orgconstants.OrganizationsTeamMembersReadPermission); err != nil { return nil, err } return u.teamMemberService.GetTeamMember(ctx, actor, organizationID, teamID, memberID) } func (u *UseCases) RemoveTeamMember(ctx context.Context, actor *models.Actor, organizationID string, teamID string, memberID string) error { - if err := u.authorizer.AuthorizeOrganizationAccess(ctx, actor, organizationID); err != nil { - return err - } - if err := u.authorizer.AuthorizeScope(ctx, actor, orgconstants.OrganizationsTeamMembersRemovePermission); err != nil { + if err := u.authorizeOrgAccess(ctx, actor, organizationID, orgconstants.OrganizationsTeamMembersRemovePermission); err != nil { return err } return u.teamMemberService.RemoveTeamMember(ctx, actor, organizationID, teamID, memberID) diff --git a/plugins/organizations/usecases/usecases_test.go b/plugins/organizations/usecases/usecases_test.go new file mode 100644 index 00000000..b00bf09c --- /dev/null +++ b/plugins/organizations/usecases/usecases_test.go @@ -0,0 +1,151 @@ +package usecases + +import ( + "context" + "testing" + + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" + + coreerrors "github.com/Authula/authula/core/errors" + "github.com/Authula/authula/models" + orgconstants "github.com/Authula/authula/plugins/organizations/constants" + orgservices "github.com/Authula/authula/plugins/organizations/services" + orgtests "github.com/Authula/authula/plugins/organizations/tests" + "github.com/Authula/authula/plugins/organizations/types" + rootservices "github.com/Authula/authula/services" +) + +func newAuthorizeOrgAccessUseCases(orgRepo *orgtests.MockOrganizationRepository, memberRepo *orgtests.MockOrganizationMemberRepository, accessControl *orgtests.AccessControlServiceStub) *UseCases { + return &UseCases{ + authorizer: rootservices.NewDefaultAuthorizer(), + serviceUtils: orgservices.NewServiceUtils(orgRepo, memberRepo, nil, nil), + accessControl: accessControl, + } +} + +func TestUseCases_authorizeOrgAccess(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + actor *models.Actor + orgID string + requiredScope string + setup func(*orgtests.MockOrganizationRepository, *orgtests.MockOrganizationMemberRepository, *orgtests.AccessControlServiceStub) + expectErr error + }{ + { + name: "admin in org A cannot delete org B when viewer there", + actor: &models.Actor{ID: "user-1", Type: models.ActorUser}, + orgID: "org-b", + requiredScope: orgconstants.OrganizationsDeletePermission, + setup: func(orgRepo *orgtests.MockOrganizationRepository, memberRepo *orgtests.MockOrganizationMemberRepository, accessControl *orgtests.AccessControlServiceStub) { + orgRepo.On("GetByID", mock.Anything, "org-b").Return(&types.Organization{ID: "org-b", OwnerID: "owner-1"}, nil).Once() + memberRepo.On("GetByOrganizationIDAndUserID", mock.Anything, "org-b", "user-1").Return(&types.OrganizationMember{ID: "mem-b", OrganizationID: "org-b", UserID: "user-1", Role: "viewer"}, nil).Once() + accessControl.RolePermissions = map[string][]string{ + "viewer": {"organizations:members:read"}, + "admin": {"organizations:delete"}, + } + }, + expectErr: coreerrors.ErrInsufficientPermissions, + }, + { + name: "admin role grants delete in org A", + actor: &models.Actor{ID: "user-1", Type: models.ActorUser}, + orgID: "org-a", + requiredScope: orgconstants.OrganizationsDeletePermission, + setup: func(orgRepo *orgtests.MockOrganizationRepository, memberRepo *orgtests.MockOrganizationMemberRepository, accessControl *orgtests.AccessControlServiceStub) { + orgRepo.On("GetByID", mock.Anything, "org-a").Return(&types.Organization{ID: "org-a", OwnerID: "user-1"}, nil).Once() + memberRepo.On("GetByOrganizationIDAndUserID", mock.Anything, "org-a", "user-1").Return(&types.OrganizationMember{ID: "mem-a", OrganizationID: "org-a", UserID: "user-1", Role: "admin"}, nil).Once() + accessControl.RolePermissions = map[string][]string{ + "admin": {"organizations:delete"}, + } + }, + }, + { + name: "wildcard role permission grants scope", + actor: &models.Actor{ID: "user-1", Type: models.ActorUser}, + orgID: "org-a", + requiredScope: orgconstants.OrganizationsDeletePermission, + setup: func(orgRepo *orgtests.MockOrganizationRepository, memberRepo *orgtests.MockOrganizationMemberRepository, accessControl *orgtests.AccessControlServiceStub) { + orgRepo.On("GetByID", mock.Anything, "org-a").Return(&types.Organization{ID: "org-a", OwnerID: "user-1"}, nil).Once() + memberRepo.On("GetByOrganizationIDAndUserID", mock.Anything, "org-a", "user-1").Return(&types.OrganizationMember{ID: "mem-a", OrganizationID: "org-a", UserID: "user-1", Role: "admin"}, nil).Once() + accessControl.RolePermissions = map[string][]string{ + "admin": {"organizations:*"}, + } + }, + }, + { + name: "user with no member row is forbidden", + actor: &models.Actor{ID: "user-1", Type: models.ActorUser}, + orgID: "org-a", + requiredScope: orgconstants.OrganizationsDeletePermission, + setup: func(orgRepo *orgtests.MockOrganizationRepository, memberRepo *orgtests.MockOrganizationMemberRepository, accessControl *orgtests.AccessControlServiceStub) { + orgRepo.On("GetByID", mock.Anything, "org-a").Return(&types.Organization{ID: "org-a", OwnerID: "user-1"}, nil).Once() + memberRepo.On("GetByOrganizationIDAndUserID", mock.Anything, "org-a", "user-1").Return((*types.OrganizationMember)(nil), nil).Once() + }, + expectErr: coreerrors.ErrForbidden, + }, + { + name: "machine bound to org A is blocked on org B", + actor: &models.Actor{ID: "key-1", Type: models.ActorMachine, Scopes: []string{"organizations:delete"}, Claims: map[string]any{"organization_id": "org-a"}}, + orgID: "org-b", + requiredScope: orgconstants.OrganizationsDeletePermission, + expectErr: coreerrors.ErrForbidden, + }, + { + name: "machine bound to org A with matching scope is allowed", + actor: &models.Actor{ID: "key-1", Type: models.ActorMachine, Scopes: []string{"organizations:delete"}, Claims: map[string]any{"organization_id": "org-a"}}, + orgID: "org-a", + requiredScope: orgconstants.OrganizationsDeletePermission, + setup: func(orgRepo *orgtests.MockOrganizationRepository, memberRepo *orgtests.MockOrganizationMemberRepository, accessControl *orgtests.AccessControlServiceStub) { + orgRepo.On("GetByID", mock.Anything, "org-a").Return(&types.Organization{ID: "org-a", OwnerID: "user-1"}, nil).Once() + }, + }, + { + name: "machine bound to org A without scope is denied", + actor: &models.Actor{ID: "key-1", Type: models.ActorMachine, Scopes: []string{"organizations:read"}, Claims: map[string]any{"organization_id": "org-a"}}, + orgID: "org-a", + requiredScope: orgconstants.OrganizationsDeletePermission, + setup: func(orgRepo *orgtests.MockOrganizationRepository, memberRepo *orgtests.MockOrganizationMemberRepository, accessControl *orgtests.AccessControlServiceStub) { + orgRepo.On("GetByID", mock.Anything, "org-a").Return(&types.Organization{ID: "org-a", OwnerID: "user-1"}, nil).Once() + }, + expectErr: coreerrors.ErrInsufficientPermissions, + }, + { + name: "dangling member role fails closed", + actor: &models.Actor{ID: "user-1", Type: models.ActorUser}, + orgID: "org-a", + requiredScope: orgconstants.OrganizationsDeletePermission, + setup: func(orgRepo *orgtests.MockOrganizationRepository, memberRepo *orgtests.MockOrganizationMemberRepository, accessControl *orgtests.AccessControlServiceStub) { + orgRepo.On("GetByID", mock.Anything, "org-a").Return(&types.Organization{ID: "org-a", OwnerID: "user-1"}, nil).Once() + memberRepo.On("GetByOrganizationIDAndUserID", mock.Anything, "org-a", "user-1").Return(&types.OrganizationMember{ID: "mem-a", OrganizationID: "org-a", UserID: "user-1", Role: "ghost"}, nil).Once() + }, + expectErr: coreerrors.ErrForbidden, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + orgRepo := &orgtests.MockOrganizationRepository{} + memberRepo := &orgtests.MockOrganizationMemberRepository{} + accessControl := orgtests.NewAccessControlServiceStub() + if tt.setup != nil { + tt.setup(orgRepo, memberRepo, accessControl) + } + + uc := newAuthorizeOrgAccessUseCases(orgRepo, memberRepo, accessControl) + err := uc.authorizeOrgAccess(context.Background(), tt.actor, tt.orgID, tt.requiredScope) + if tt.expectErr != nil { + require.ErrorIs(t, err, tt.expectErr) + } else { + require.NoError(t, err) + } + orgRepo.AssertExpectations(t) + memberRepo.AssertExpectations(t) + }) + } +} diff --git a/services/access_control.go b/services/access_control.go index f62d1504..f4704d45 100644 --- a/services/access_control.go +++ b/services/access_control.go @@ -9,7 +9,8 @@ type PermissionDefinition struct { type AccessControlService interface { RoleExists(ctx context.Context, roleName string) (bool, error) - ValidateRoleAssignment(ctx context.Context, roleName string, assignerUserID *string) (bool, error) + GetRolePermissionsByName(ctx context.Context, roleName string) ([]string, error) + GetRoleWeightByName(ctx context.Context, roleName string) (int, error) ValidatePermissionKeys(ctx context.Context, permissionKeys []string) error EnsurePermissions(ctx context.Context, permissions []PermissionDefinition) error }