From a72bd0de6471c96285c760bc75a0e83dafdde666 Mon Sep 17 00:00:00 2001 From: Miguel Martinez Trivino Date: Fri, 4 Sep 2026 18:44:25 +0200 Subject: [PATCH] fix(controlplane): enforce project-scoped RBAC on AttestationService/GetContract AttestationService/GetContract resolved the workflow org-scoped without checking the caller's rights on the requested project, letting a project-scoped API token read another project's workflow metadata and full contract schema through the attestation JWT path, which the authz middleware skips. Apply the same project authorization the sibling Init/Store/Cancel handlers already perform, via userHasPermissionOnProject with PolicyWorkflowRead, after the workflow is resolved. Export usercontext.WithRobotAccount so the service-layer integration tests can build the same context the attestation middlewares produce. Assisted-by: OpenCode Signed-off-by: Miguel Martinez Trivino Chainloop-Trace-Sessions: ses_f92bd9943ffeCzB41EHYLejTZR --- .../internal/service/attestation.go | 5 + .../service/attestation_integration_test.go | 180 ++++++++++++++++++ .../usercontext/apitoken_middleware.go | 2 +- .../usercontext/currentuser_middleware.go | 2 +- .../usercontext/federated_middleware.go | 2 +- .../usercontext/robotaccount_middleware.go | 5 +- 6 files changed, 191 insertions(+), 5 deletions(-) create mode 100644 app/controlplane/internal/service/attestation_integration_test.go diff --git a/app/controlplane/internal/service/attestation.go b/app/controlplane/internal/service/attestation.go index b6dd1e128..90ce3efd6 100644 --- a/app/controlplane/internal/service/attestation.go +++ b/app/controlplane/internal/service/attestation.go @@ -131,6 +131,11 @@ func (s *AttestationService) GetContract(ctx context.Context, req *cpAPI.Attesta return nil, handleUseCaseErr(err, s.log) } + // Apply RBAC on the project + if _, err = s.userHasPermissionOnProject(ctx, robotAccount.OrgID, &cpAPI.IdentityReference{Name: &req.ProjectName}, authz.PolicyWorkflowRead); err != nil { + return nil, err + } + // Find contract revision contractVersion, err := s.workflowContractUseCase.Describe(ctx, wf.OrgID.String(), wf.ContractID.String(), int(req.ContractRevision), biz.WithoutReferences()) if err != nil { diff --git a/app/controlplane/internal/service/attestation_integration_test.go b/app/controlplane/internal/service/attestation_integration_test.go new file mode 100644 index 000000000..d57866c06 --- /dev/null +++ b/app/controlplane/internal/service/attestation_integration_test.go @@ -0,0 +1,180 @@ +// +// Copyright 2026 The Chainloop Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package service + +import ( + "context" + "testing" + + pb "github.com/chainloop-dev/chainloop/app/controlplane/api/controlplane/v1" + "github.com/chainloop-dev/chainloop/app/controlplane/internal/usercontext" + "github.com/chainloop-dev/chainloop/app/controlplane/internal/usercontext/attjwtmiddleware" + "github.com/chainloop-dev/chainloop/app/controlplane/pkg/authz" + "github.com/chainloop-dev/chainloop/app/controlplane/pkg/biz" + "github.com/chainloop-dev/chainloop/app/controlplane/pkg/biz/testhelpers" + "github.com/chainloop-dev/chainloop/app/controlplane/pkg/usercontext/entities" + kerrors "github.com/go-kratos/kratos/v2/errors" + "github.com/stretchr/testify/suite" +) + +// Regression tests for PFM-6717: a project-scoped API token must not be able to retrieve +// another project's workflow metadata and contract schema through AttestationService/GetContract. +// Attestation endpoints are skipped by the authz middleware, so the handler is the only +// authorization point. +type getContractRBACIntegrationSuite struct { + testhelpers.UseCasesEachTestSuite + org *biz.Organization + projectA, projectB *biz.Project + workflowA, workflowB *biz.Workflow + projectToken *biz.APIToken + workflowToken *biz.APIToken + orgToken *biz.APIToken + svc *AttestationService +} + +func (s *getContractRBACIntegrationSuite) SetupTest() { + s.TestingUseCases = testhelpers.NewTestingUseCases(s.T()) + + ctx := context.Background() + var err error + + s.org, err = s.Organization.Create(ctx, "get-contract-rbac-org") + s.Require().NoError(err) + + s.projectA, err = s.Project.Create(ctx, s.org.ID, "project-a") + s.Require().NoError(err) + s.projectB, err = s.Project.Create(ctx, s.org.ID, "project-b") + s.Require().NoError(err) + + s.workflowA, err = s.Workflow.Create(ctx, &biz.WorkflowCreateOpts{Name: "workflow-a", OrgID: s.org.ID, Project: "project-a"}) + s.Require().NoError(err) + s.workflowB, err = s.Workflow.Create(ctx, &biz.WorkflowCreateOpts{Name: "workflow-b", OrgID: s.org.ID, Project: "project-b"}) + s.Require().NoError(err) + + // A token confined to project A, one pinned to workflow A, and an org-wide token. + s.projectToken, err = s.APIToken.Create(ctx, "token-project-a", nil, nil, &s.org.ID, biz.APITokenWithProject(s.projectA)) + s.Require().NoError(err) + s.workflowToken, err = s.APIToken.Create(ctx, "token-workflow-a", nil, nil, &s.org.ID, biz.APITokenWithProject(s.projectA), biz.APITokenWithWorkflow(s.workflowA)) + s.Require().NoError(err) + s.orgToken, err = s.APIToken.Create(ctx, "token-org", nil, nil, &s.org.ID) + s.Require().NoError(err) + + authzUC := biz.NewAuthzUseCase(&biz.AuthzUseCaseConfig{ + CasbinEnforcer: s.Enforcer, + APITokenRepo: s.Repos.APITokenRepo, + Logger: s.L, + }) + + s.svc = NewAttestationService(&NewAttestationServiceOpts{ + WorkflowRunUC: s.WorkflowRun, + WorkflowUC: s.Workflow, + WorkflowContractUC: s.WorkflowContract, + OrgUC: s.Organization, + ProjectUC: s.Project, + ProjectVersionUC: s.ProjectVersion, + Opts: []NewOpt{WithProjectUseCase(s.Project), WithEnforcer(authzUC)}, + }) +} + +func (s *getContractRBACIntegrationSuite) TestProjectScopedToken() { + s.Run("cannot read another project's workflow and contract", func() { + _, err := s.svc.GetContract(s.ctxForToken(s.projectToken), &pb.AttestationServiceGetContractRequest{ + ProjectName: s.projectB.Name, + WorkflowName: s.workflowB.Name, + }) + s.Require().Error(err) + s.True(kerrors.IsForbidden(err), "expected forbidden, got %v", err) + }) + + s.Run("can read its own project's workflow and contract", func() { + resp, err := s.svc.GetContract(s.ctxForToken(s.projectToken), &pb.AttestationServiceGetContractRequest{ + ProjectName: s.projectA.Name, + WorkflowName: s.workflowA.Name, + }) + s.Require().NoError(err) + s.Equal(s.workflowA.Name, resp.GetResult().GetWorkflow().GetName()) + s.NotNil(resp.GetResult().GetContract()) + s.Equal(s.projectA.Name, resp.GetResult().GetWorkflow().GetProject()) + }) +} + +func (s *getContractRBACIntegrationSuite) TestWorkflowScopedToken() { + // Workflow-scoped tokens are already confined by findWorkflowFromTokenOrNameOrRunID; + // this guards against regressions on top of the project-level check. + s.Run("cannot read another project's workflow and contract", func() { + _, err := s.svc.GetContract(s.ctxForToken(s.workflowToken), &pb.AttestationServiceGetContractRequest{ + ProjectName: s.projectB.Name, + WorkflowName: s.workflowB.Name, + }) + s.Require().Error(err) + s.True(kerrors.IsForbidden(err), "expected forbidden, got %v", err) + }) + + s.Run("can read its own workflow and contract", func() { + resp, err := s.svc.GetContract(s.ctxForToken(s.workflowToken), &pb.AttestationServiceGetContractRequest{ + ProjectName: s.projectA.Name, + WorkflowName: s.workflowA.Name, + }) + s.Require().NoError(err) + s.Equal(s.workflowA.Name, resp.GetResult().GetWorkflow().GetName()) + s.NotNil(resp.GetResult().GetContract()) + }) +} + +func (s *getContractRBACIntegrationSuite) TestOrgScopedToken() { + // Backward compatibility: org-wide tokens keep reaching every workflow in the organization. + for _, wf := range []*biz.Workflow{s.workflowA, s.workflowB} { + resp, err := s.svc.GetContract(s.ctxForToken(s.orgToken), &pb.AttestationServiceGetContractRequest{ + ProjectName: wf.Project, + WorkflowName: wf.Name, + }) + s.Require().NoError(err) + s.Equal(wf.Name, resp.GetResult().GetWorkflow().GetName()) + s.NotNil(resp.GetResult().GetContract()) + } + + // The workflow lookup itself requires an explicit project name, for every caller. + _, err := s.svc.GetContract(s.ctxForToken(s.orgToken), &pb.AttestationServiceGetContractRequest{ + WorkflowName: s.workflowA.Name, + }) + s.Require().Error(err) + s.True(kerrors.IsBadRequest(err), "expected bad request, got %v", err) +} + +// ctxForToken builds a context equivalent to the one the attestation middlewares produce for +// an API-token authenticated call: current org, current API token, the api-token authz +// subject and the robot account the attestation handlers read the organization from. +func (s *getContractRBACIntegrationSuite) ctxForToken(token *biz.APIToken) context.Context { + ctx := entities.WithCurrentOrg(context.Background(), &entities.Org{ID: s.org.ID, Name: s.org.Name}) + ctx = entities.WithCurrentAPIToken(ctx, &entities.APIToken{ + ID: token.ID.String(), + Name: token.Name, + ProjectID: token.ProjectID, + ProjectName: token.ProjectName, + WorkflowID: token.WorkflowID, + WorkflowName: token.WorkflowName, + }) + ctx = usercontext.WithAuthzSubject(ctx, (&authz.SubjectAPIToken{ID: token.ID.String()}).String()) + + return usercontext.WithRobotAccount(ctx, &usercontext.RobotAccount{ + OrgID: s.org.ID, + ProviderKey: attjwtmiddleware.APITokenProviderKey, + }) +} + +func TestGetContractRBACIntegration(t *testing.T) { + suite.Run(t, new(getContractRBACIntegrationSuite)) +} diff --git a/app/controlplane/internal/usercontext/apitoken_middleware.go b/app/controlplane/internal/usercontext/apitoken_middleware.go index 6e75ece12..d4389b847 100644 --- a/app/controlplane/internal/usercontext/apitoken_middleware.go +++ b/app/controlplane/internal/usercontext/apitoken_middleware.go @@ -165,7 +165,7 @@ func setRobotAccountFromAPIToken(ctx context.Context, apiTokenUC *biz.APITokenUs return nil, errors.New("API token revoked") } - ctx = withRobotAccount(ctx, &RobotAccount{OrgID: token.OrganizationID.String(), ProviderKey: attjwtmiddleware.APITokenProviderKey}) + ctx = WithRobotAccount(ctx, &RobotAccount{OrgID: token.OrganizationID.String(), ProviderKey: attjwtmiddleware.APITokenProviderKey}) return ctx, nil } diff --git a/app/controlplane/internal/usercontext/currentuser_middleware.go b/app/controlplane/internal/usercontext/currentuser_middleware.go index 5e214f831..876ae15b0 100644 --- a/app/controlplane/internal/usercontext/currentuser_middleware.go +++ b/app/controlplane/internal/usercontext/currentuser_middleware.go @@ -139,7 +139,7 @@ func WithAttestationContextFromUser(userUC *biz.UserUseCase, orgUC *biz.Organiza return nil, fmt.Errorf("your user doesn't have permissions to perform attestations in this organization, role=%s, orgID=%s", subject, org.ID) } - ctx = withRobotAccount(ctx, &RobotAccount{OrgID: org.ID, ProviderKey: attjwtmiddleware.UserTokenProviderKey}) + ctx = WithRobotAccount(ctx, &RobotAccount{OrgID: org.ID, ProviderKey: attjwtmiddleware.UserTokenProviderKey}) logger.Infow("msg", "[authN] processed credentials", "type", attjwtmiddleware.UserTokenProviderKey) return handler(ctx, req) diff --git a/app/controlplane/internal/usercontext/federated_middleware.go b/app/controlplane/internal/usercontext/federated_middleware.go index a60db4c33..a9c145b1a 100644 --- a/app/controlplane/internal/usercontext/federated_middleware.go +++ b/app/controlplane/internal/usercontext/federated_middleware.go @@ -51,7 +51,7 @@ func WithAttestationContextFromFederatedInfo(orgUC *biz.OrganizationUseCase, log orgID := (*claims)["orgId"].(string) - ctx = withRobotAccount(ctx, &RobotAccount{OrgID: orgID, ProviderKey: attjwtmiddleware.FederatedProviderKey}) + ctx = WithRobotAccount(ctx, &RobotAccount{OrgID: orgID, ProviderKey: attjwtmiddleware.FederatedProviderKey}) // Find the associated organization org, err := orgUC.FindByID(ctx, orgID) if err != nil { diff --git a/app/controlplane/internal/usercontext/robotaccount_middleware.go b/app/controlplane/internal/usercontext/robotaccount_middleware.go index e547aa9a3..9a4f8158b 100644 --- a/app/controlplane/internal/usercontext/robotaccount_middleware.go +++ b/app/controlplane/internal/usercontext/robotaccount_middleware.go @@ -32,7 +32,8 @@ type RobotAccount struct { ID, WorkflowID, OrgID, ProviderKey string } -func withRobotAccount(ctx context.Context, acc *RobotAccount) context.Context { +// Set the current robot account in the context +func WithRobotAccount(ctx context.Context, acc *RobotAccount) context.Context { return context.WithValue(ctx, currentRobotAccountCtxKey{}, acc) } @@ -121,7 +122,7 @@ func WithAttestationContextFromRobotAccount(robotAccountUseCase *biz.RobotAccoun } // Set the robot account in the context - ctx = withRobotAccount(ctx, &RobotAccount{ + ctx = WithRobotAccount(ctx, &RobotAccount{ ID: account.ID.String(), WorkflowID: workflowID, OrgID: orgID, ProviderKey: authInfo.ProviderKey, })