From 16452b4a3aa6af4b0aa30ef217b7ef231ebb656c Mon Sep 17 00:00:00 2001 From: Javier Rodriguez Date: Mon, 5 Oct 2026 16:23:07 +0200 Subject: [PATCH 01/17] feat(controlplane): sign the scope type and scope id in API token claims Mutation check (each condition disabled in turn, the package tests fail): - agreesWith instance-admin check: TestGenerateJWT/the_instance-admin_claim_on_an_organization_scope, TestSignedScope - agreesWith workflow-needs-project: TestSignedScope/a_workflow_claim_without_a_project_claim - agreesWith instance names no org/project: TestSignedScope/an_instance_scope_naming_an_organization - agreesWith organization: TestGenerateJWT/an_organization_scope_naming_another_organization, TestSignedScope - agreesWith project: TestGenerateJWT/a_project_scope_naming_another_project, TestSignedScope - agreesWith product: TestSignedScope/a_product_scope_with_a_project_claim - namedScope ScopeID != "" check: TestSignedScope/an_instance_scope_naming_a_resource - SignedScope call in GenerateJWT: TestGenerateJWT inconsistent-scope cases Assisted-by: Claude Code Signed-off-by: Javier Rodriguez Chainloop-Trace-Sessions: 796bcbff-1898-452d-aaeb-b5311bffdfa6 --- app/controlplane/pkg/jwt/apitoken/apitoken.go | 151 +++++++++++- .../pkg/jwt/apitoken/apitoken_test.go | 223 +++++++++++++----- 2 files changed, 318 insertions(+), 56 deletions(-) diff --git a/app/controlplane/pkg/jwt/apitoken/apitoken.go b/app/controlplane/pkg/jwt/apitoken/apitoken.go index 26ca5b948..430aeea40 100644 --- a/app/controlplane/pkg/jwt/apitoken/apitoken.go +++ b/app/controlplane/pkg/jwt/apitoken/apitoken.go @@ -16,9 +16,12 @@ package apitoken import ( + "encoding/json" "errors" + "fmt" "time" + "github.com/chainloop-dev/chainloop/app/controlplane/pkg/authz" "github.com/golang-jwt/jwt/v5" "github.com/google/uuid" ) @@ -77,7 +80,13 @@ type GenerateJWTOptions struct { WorkflowID *uuid.UUID WorkflowName *string ExpiresAt *time.Time - Scope *string + // Scope is the legacy instance-admin claim. The platform and control planes up to v1.112 + // read it, so instance tokens keep carrying it. + Scope *string + // ScopeType and ScopeID are what the token is scoped to, as its row records them. ScopeType + // is required. ScopeID is unset only for an instance token. + ScopeType *authz.ResourceType + ScopeID *uuid.UUID } // GenerateJWT creates a new JWT token for the given organization and keyID @@ -126,6 +135,22 @@ func (ra *Builder) GenerateJWT(opts *GenerateJWTOptions) (string, error) { claims.WorkflowName = *opts.WorkflowName } + if opts.ScopeID != nil && opts.ScopeType == nil { + return "", errors.New("scopeType is required when scopeID is set") + } + + if opts.ScopeType != nil { + claims.ScopeType = string(*opts.ScopeType) + if opts.ScopeID != nil { + claims.ScopeID = opts.ScopeID.String() + } + + // Never sign a token whose claims contradict themselves + if _, _, err := claims.SignedScope(); err != nil { + return "", fmt.Errorf("inconsistent token scope: %w", err) + } + } + // optional expiration value, i.e 30 days if opts.ExpiresAt != nil { claims.ExpiresAt = jwt.NewNumericDate(*opts.ExpiresAt) @@ -144,5 +169,129 @@ type CustomClaims struct { WorkflowID string `json:"workflow_id,omitempty"` WorkflowName string `json:"workflow_name,omitempty"` Scope string `json:"scope,omitempty"` + // ScopeType and ScopeID bind the token to what it was granted. A token minted before they + // existed carries neither, and SignedScope derives its scope from the claims it does carry. + ScopeType string `json:"scope_type,omitempty"` + ScopeID string `json:"scope_id,omitempty"` jwt.RegisteredClaims } + +// SignedScope returns the scope the claims bind the token to, and refuses claims that contradict +// themselves. A token minted with the scope_type and scope_id claims gets those. An older token +// gets the scope its other claims imply, by the rule the scope backfill migration applied to its +// row. A product token minted before the scope claims implies only its organization. +func (c *CustomClaims) SignedScope() (authz.ResourceType, *uuid.UUID, error) { + kind, id, err := c.namedScope() + if err != nil { + return "", nil, err + } + + if err := c.agreesWith(kind, id); err != nil { + return "", nil, err + } + + return kind, id, nil +} + +// namedScope is the scope the scope_type and scope_id claims name, else the one the claims of an +// older token imply. +func (c *CustomClaims) namedScope() (authz.ResourceType, *uuid.UUID, error) { + if c.ScopeType == "" { + return c.legacyScope() + } + + kind := authz.ResourceType(c.ScopeType) + switch kind { + case authz.ResourceTypeInstance: + if c.ScopeID != "" { + return "", nil, errors.New("an instance scope names no resource") + } + + return kind, nil, nil + case authz.ResourceTypeOrganization, authz.ResourceTypeProject, authz.ResourceTypeProduct: + id, err := uuid.Parse(c.ScopeID) + if err != nil { + return "", nil, fmt.Errorf("invalid scope_id claim: %w", err) + } + + return kind, &id, nil + default: + return "", nil, fmt.Errorf("unknown scope_type claim %q", c.ScopeType) + } +} + +// legacyScope is the scope the claims of a token minted before the scope_type claim imply: the +// instance for the instance-admin claim, else its project, else its organization. +func (c *CustomClaims) legacyScope() (authz.ResourceType, *uuid.UUID, error) { + var kind authz.ResourceType + var raw string + switch { + case c.Scope == authz.ScopeInstanceAdmin: + return authz.ResourceTypeInstance, nil, nil + case c.ProjectID != "": + kind, raw = authz.ResourceTypeProject, c.ProjectID + case c.OrgID != "": + kind, raw = authz.ResourceTypeOrganization, c.OrgID + default: + return "", nil, errors.New("the claims name no scope") + } + + id, err := uuid.Parse(raw) + if err != nil { + return "", nil, fmt.Errorf("invalid %s claim: %w", kind, err) + } + + return kind, &id, nil +} + +// agreesWith checks that the other claims fit the scope, so that every reader of the token, the +// platform's included, reads the same scope off it. Only an instance token carries the +// instance-admin claim, and it names no organization. An organization token names its own +// organization, a project token the project it is scoped to, and a workflow comes with its +// project. +func (c *CustomClaims) agreesWith(kind authz.ResourceType, id *uuid.UUID) error { + if (c.Scope == authz.ScopeInstanceAdmin) != (kind == authz.ResourceTypeInstance) { + return errors.New("the instance-admin claim does not agree with the scope") + } + + if c.WorkflowID != "" && c.ProjectID == "" { + return errors.New("a workflow claim needs a project claim") + } + + switch kind { + case authz.ResourceTypeInstance: + if c.OrgID != "" || c.ProjectID != "" { + return errors.New("an instance scope names no organization or project") + } + case authz.ResourceTypeOrganization: + if c.OrgID != id.String() || c.ProjectID != "" { + return errors.New("an organization scope names its own organization and no project") + } + case authz.ResourceTypeProject: + if c.OrgID == "" || c.ProjectID != id.String() { + return errors.New("a project scope names its organization and its own project") + } + case authz.ResourceTypeProduct: + if c.OrgID == "" || c.ProjectID != "" { + return errors.New("a product scope names its organization and no project") + } + } + + return nil +} + +// ClaimsFromMap reads API-token claims that were parsed generically, as the API entry point +// receives them. A claim of the wrong type is an error, not an absent claim. +func ClaimsFromMap(m jwt.MapClaims) (*CustomClaims, error) { + raw, err := json.Marshal(m) + if err != nil { + return nil, fmt.Errorf("encoding claims: %w", err) + } + + claims := &CustomClaims{} + if err := json.Unmarshal(raw, claims); err != nil { + return nil, fmt.Errorf("decoding claims: %w", err) + } + + return claims, nil +} diff --git a/app/controlplane/pkg/jwt/apitoken/apitoken_test.go b/app/controlplane/pkg/jwt/apitoken/apitoken_test.go index f026a9940..dbd24b8b4 100644 --- a/app/controlplane/pkg/jwt/apitoken/apitoken_test.go +++ b/app/controlplane/pkg/jwt/apitoken/apitoken_test.go @@ -19,6 +19,7 @@ import ( "testing" "time" + "github.com/chainloop-dev/chainloop/app/controlplane/pkg/authz" "github.com/golang-jwt/jwt/v5" "github.com/google/uuid" "github.com/stretchr/testify/assert" @@ -70,83 +71,84 @@ func TestNewBuilder(t *testing.T) { func TestGenerateJWT(t *testing.T) { const hmacSecret = "my-secret" + org := uuid.MustParse("123e4567-e89b-12d3-a456-426614174000") + project := uuid.MustParse("223e4567-e89b-12d3-a456-426614174000") + workflow := uuid.MustParse("323e4567-e89b-12d3-a456-426614174000") + product := uuid.MustParse("423e4567-e89b-12d3-a456-426614174000") + keyID := uuid.MustParse("523e4567-e89b-12d3-a456-426614174000") + orgScope, projectScope := authz.ResourceTypeOrganization, authz.ResourceTypeProject + productScope, instanceScope := authz.ResourceTypeProduct, authz.ResourceTypeInstance + testCases := []struct { name string opts *GenerateJWTOptions wantErr bool }{ { - name: "no project", - opts: &GenerateJWTOptions{ - OrgID: toPtr(uuid.MustParse("123e4567-e89b-12d3-a456-426614174000")), - OrgName: toPtr("org-name"), - KeyName: "key-name", - KeyID: uuid.MustParse("123e4567-e89b-12d3-a456-426614174000"), - ExpiresAt: toPtr(time.Now().Add(1 * time.Hour)), - }, + name: "organization token", + opts: &GenerateJWTOptions{OrgID: &org, OrgName: toPtr("org-name"), KeyName: "key-name", KeyID: keyID, + ExpiresAt: toPtr(time.Now().Add(1 * time.Hour)), ScopeType: &orgScope, ScopeID: &org}, }, { name: "no expiration", - opts: &GenerateJWTOptions{ - OrgID: toPtr(uuid.MustParse("123e4567-e89b-12d3-a456-426614174000")), - OrgName: toPtr("org-name"), - KeyName: "key-name", - KeyID: uuid.MustParse("123e4567-e89b-12d3-a456-426614174000"), - }, + opts: &GenerateJWTOptions{OrgID: &org, OrgName: toPtr("org-name"), KeyName: "key-name", KeyID: keyID, + ScopeType: &orgScope, ScopeID: &org}, }, { - name: "with project", - opts: &GenerateJWTOptions{ - OrgID: toPtr(uuid.MustParse("123e4567-e89b-12d3-a456-426614174000")), - OrgName: toPtr("org-name"), - KeyName: "key-name", - KeyID: uuid.MustParse("123e4567-e89b-12d3-a456-426614174000"), - ProjectID: toPtr(uuid.MustParse("123e4567-e89b-12d3-a456-426614174000")), - ProjectName: toPtr("project-name"), - ExpiresAt: toPtr(time.Now().Add(1 * time.Hour)), - }, + name: "project token", + opts: &GenerateJWTOptions{OrgID: &org, OrgName: toPtr("org-name"), KeyName: "key-name", KeyID: keyID, + ProjectID: &project, ProjectName: toPtr("project-name"), ExpiresAt: toPtr(time.Now().Add(1 * time.Hour)), + ScopeType: &projectScope, ScopeID: &project}, }, { - name: "with workflow", - opts: &GenerateJWTOptions{ - OrgID: toPtr(uuid.MustParse("123e4567-e89b-12d3-a456-426614174000")), - OrgName: toPtr("org-name"), - KeyName: "key-name", - KeyID: uuid.MustParse("123e4567-e89b-12d3-a456-426614174000"), - ProjectID: toPtr(uuid.MustParse("223e4567-e89b-12d3-a456-426614174000")), - ProjectName: toPtr("project-name"), - WorkflowID: toPtr(uuid.MustParse("323e4567-e89b-12d3-a456-426614174000")), - WorkflowName: toPtr("workflow-name"), - ExpiresAt: toPtr(time.Now().Add(1 * time.Hour)), - }, + name: "workflow-pinned token", + opts: &GenerateJWTOptions{OrgID: &org, OrgName: toPtr("org-name"), KeyName: "key-name", KeyID: keyID, + ProjectID: &project, ProjectName: toPtr("project-name"), WorkflowID: &workflow, WorkflowName: toPtr("workflow-name"), + ExpiresAt: toPtr(time.Now().Add(1 * time.Hour)), ScopeType: &projectScope, ScopeID: &project}, }, { - name: "instance token - no orgID or orgName", - opts: &GenerateJWTOptions{ - KeyName: "key-name", - KeyID: uuid.MustParse("123e4567-e89b-12d3-a456-426614174000"), - ExpiresAt: toPtr(time.Now().Add(1 * time.Hour)), - Scope: toPtr("INSTANCE_ADMIN"), - }, + name: "product token", + opts: &GenerateJWTOptions{OrgID: &org, OrgName: toPtr("org-name"), KeyName: "key-name", KeyID: keyID, + ScopeType: &productScope, ScopeID: &product}, + }, + { + name: "instance token keeps the instance-admin claim", + opts: &GenerateJWTOptions{KeyName: "key-name", KeyID: keyID, ExpiresAt: toPtr(time.Now().Add(1 * time.Hour)), + Scope: toPtr("INSTANCE_ADMIN"), ScopeType: &instanceScope}, }, { name: "missing keyID", - opts: &GenerateJWTOptions{ - OrgID: toPtr(uuid.MustParse("123e4567-e89b-12d3-a456-426614174000")), - OrgName: toPtr("org-name"), - KeyName: "key-name", - ExpiresAt: toPtr(time.Now().Add(1 * time.Hour)), - }, + opts: &GenerateJWTOptions{OrgID: &org, OrgName: toPtr("org-name"), KeyName: "key-name", + ScopeType: &orgScope, ScopeID: &org}, wantErr: true, }, { name: "missing keyName", - opts: &GenerateJWTOptions{ - OrgID: toPtr(uuid.MustParse("123e4567-e89b-12d3-a456-426614174000")), - OrgName: toPtr("org-name"), - KeyID: uuid.MustParse("123e4567-e89b-12d3-a456-426614174000"), - ExpiresAt: toPtr(time.Now().Add(1 * time.Hour)), - }, + opts: &GenerateJWTOptions{OrgID: &org, OrgName: toPtr("org-name"), KeyID: keyID, + ScopeType: &orgScope, ScopeID: &org}, + wantErr: true, + }, + { + name: "a scope id without a scope type", + opts: &GenerateJWTOptions{OrgID: &org, OrgName: toPtr("org-name"), KeyName: "key-name", KeyID: keyID, ScopeID: &org}, + wantErr: true, + }, + { + name: "an organization scope naming another organization", + opts: &GenerateJWTOptions{OrgID: &org, OrgName: toPtr("org-name"), KeyName: "key-name", KeyID: keyID, + ScopeType: &orgScope, ScopeID: &product}, + wantErr: true, + }, + { + name: "the instance-admin claim on an organization scope", + opts: &GenerateJWTOptions{OrgID: &org, OrgName: toPtr("org-name"), KeyName: "key-name", KeyID: keyID, + Scope: toPtr("INSTANCE_ADMIN"), ScopeType: &orgScope, ScopeID: &org}, + wantErr: true, + }, + { + name: "a project scope naming another project", + opts: &GenerateJWTOptions{OrgID: &org, OrgName: toPtr("org-name"), KeyName: "key-name", KeyID: keyID, + ProjectID: &project, ProjectName: toPtr("project-name"), ScopeType: &projectScope, ScopeID: &product}, wantErr: true, }, } @@ -209,6 +211,13 @@ func TestGenerateJWT(t *testing.T) { assert.Empty(t, claims.Scope) } + assert.Equal(t, string(*tc.opts.ScopeType), claims.ScopeType) + if tc.opts.ScopeID != nil { + assert.Equal(t, tc.opts.ScopeID.String(), claims.ScopeID) + } else { + assert.Empty(t, claims.ScopeID) + } + if tc.opts.ExpiresAt != nil { assert.True(t, claims.ExpiresAt.After(time.Now())) } else { @@ -218,6 +227,110 @@ func TestGenerateJWT(t *testing.T) { } } +func TestSignedScope(t *testing.T) { + org, project, product, other := uuid.New(), uuid.New(), uuid.New(), uuid.New() + workflow := uuid.New() + + testCases := []struct { + name string + claims CustomClaims + wantKind authz.ResourceType + wantID *uuid.UUID + wantErr bool + }{ + {name: "signed organization scope", claims: CustomClaims{OrgID: org.String(), ScopeType: "organization", ScopeID: org.String()}, wantKind: authz.ResourceTypeOrganization, wantID: &org}, + {name: "signed project scope", claims: CustomClaims{OrgID: org.String(), ProjectID: project.String(), ScopeType: "project", ScopeID: project.String()}, wantKind: authz.ResourceTypeProject, wantID: &project}, + {name: "signed workflow-pinned scope", claims: CustomClaims{OrgID: org.String(), ProjectID: project.String(), WorkflowID: workflow.String(), ScopeType: "project", ScopeID: project.String()}, wantKind: authz.ResourceTypeProject, wantID: &project}, + {name: "signed product scope", claims: CustomClaims{OrgID: org.String(), ScopeType: "product", ScopeID: product.String()}, wantKind: authz.ResourceTypeProduct, wantID: &product}, + {name: "signed instance scope", claims: CustomClaims{Scope: authz.ScopeInstanceAdmin, ScopeType: "instance"}, wantKind: authz.ResourceTypeInstance}, + {name: "legacy instance-admin token", claims: CustomClaims{Scope: authz.ScopeInstanceAdmin}, wantKind: authz.ResourceTypeInstance}, + {name: "legacy project token", claims: CustomClaims{OrgID: org.String(), ProjectID: project.String()}, wantKind: authz.ResourceTypeProject, wantID: &project}, + {name: "legacy workflow-pinned token", claims: CustomClaims{OrgID: org.String(), ProjectID: project.String(), WorkflowID: workflow.String()}, wantKind: authz.ResourceTypeProject, wantID: &project}, + {name: "legacy organization token", claims: CustomClaims{OrgID: org.String()}, wantKind: authz.ResourceTypeOrganization, wantID: &org}, + + // Malformed scope claims + {name: "an instance scope naming a resource", claims: CustomClaims{Scope: authz.ScopeInstanceAdmin, ScopeType: "instance", ScopeID: org.String()}, wantErr: true}, + {name: "a resource scope without an id", claims: CustomClaims{OrgID: org.String(), ProjectID: project.String(), ScopeType: "project"}, wantErr: true}, + {name: "a scope id that is not a uuid", claims: CustomClaims{OrgID: org.String(), ProjectID: project.String(), ScopeType: "project", ScopeID: "nope"}, wantErr: true}, + {name: "a scope type no token has", claims: CustomClaims{OrgID: org.String(), ScopeType: "group", ScopeID: org.String()}, wantErr: true}, + {name: "legacy claims naming nothing", claims: CustomClaims{}, wantErr: true}, + {name: "a legacy project claim that is not a uuid", claims: CustomClaims{OrgID: org.String(), ProjectID: "nope"}, wantErr: true}, + + // Claims that contradict themselves: every reader must see the same scope + {name: "the instance-admin claim on an organization scope", claims: CustomClaims{OrgID: org.String(), Scope: authz.ScopeInstanceAdmin, ScopeType: "organization", ScopeID: org.String()}, wantErr: true}, + {name: "an instance scope without the instance-admin claim", claims: CustomClaims{ScopeType: "instance"}, wantErr: true}, + {name: "an instance scope naming an organization", claims: CustomClaims{OrgID: org.String(), Scope: authz.ScopeInstanceAdmin, ScopeType: "instance"}, wantErr: true}, + {name: "a legacy instance-admin claim naming an organization", claims: CustomClaims{OrgID: org.String(), Scope: authz.ScopeInstanceAdmin}, wantErr: true}, + {name: "an organization scope naming another organization", claims: CustomClaims{OrgID: org.String(), ScopeType: "organization", ScopeID: other.String()}, wantErr: true}, + {name: "an organization scope with a project claim", claims: CustomClaims{OrgID: org.String(), ProjectID: project.String(), ScopeType: "organization", ScopeID: org.String()}, wantErr: true}, + {name: "a project scope naming another project", claims: CustomClaims{OrgID: org.String(), ProjectID: project.String(), ScopeType: "project", ScopeID: other.String()}, wantErr: true}, + {name: "a project scope without an organization", claims: CustomClaims{ProjectID: project.String(), ScopeType: "project", ScopeID: project.String()}, wantErr: true}, + {name: "a product scope with a project claim", claims: CustomClaims{OrgID: org.String(), ProjectID: project.String(), ScopeType: "product", ScopeID: product.String()}, wantErr: true}, + {name: "a product scope without an organization", claims: CustomClaims{ScopeType: "product", ScopeID: product.String()}, wantErr: true}, + {name: "a workflow claim without a project claim", claims: CustomClaims{OrgID: org.String(), WorkflowID: workflow.String(), ScopeType: "organization", ScopeID: org.String()}, wantErr: true}, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + kind, id, err := tc.claims.SignedScope() + if tc.wantErr { + require.Error(t, err) + return + } + + require.NoError(t, err) + assert.Equal(t, tc.wantKind, kind) + assert.Equal(t, tc.wantID, id) + }) + } +} + +func TestClaimsFromMap(t *testing.T) { + testCases := []struct { + name string + in jwt.MapClaims + want *CustomClaims + wantErr bool + }{ + { + name: "every claim is read", + in: jwt.MapClaims{ + "jti": "id", "aud": []any{Audience}, "org_id": "o", "org_name": "on", "token_name": "t", + "project_id": "p", "workflow_id": "w", "scope": authz.ScopeInstanceAdmin, "scope_type": "project", "scope_id": "s", + }, + want: &CustomClaims{ + OrgID: "o", OrgName: "on", KeyName: "t", ProjectID: "p", WorkflowID: "w", + Scope: authz.ScopeInstanceAdmin, ScopeType: "project", ScopeID: "s", + RegisteredClaims: jwt.RegisteredClaims{ID: "id", Audience: jwt.ClaimStrings{Audience}}, + }, + }, + { + name: "a single audience string is read", + in: jwt.MapClaims{"jti": "id", "aud": Audience}, + want: &CustomClaims{RegisteredClaims: jwt.RegisteredClaims{ID: "id", Audience: jwt.ClaimStrings{Audience}}}, + }, + { + // The API entry point used to drop such a claim silently and skip its cross-check + name: "a claim of the wrong type is refused", + in: jwt.MapClaims{"jti": "id", "project_id": 42}, + wantErr: true, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + got, err := ClaimsFromMap(tc.in) + if tc.wantErr { + require.Error(t, err) + return + } + + require.NoError(t, err) + assert.Equal(t, tc.want, got) + }) + } +} + func toPtr[T any](t T) *T { return &t } From c14d9691f695e16191f9ad30b7f9ffafd8222493 Mon Sep 17 00:00:00 2001 From: Javier Rodriguez Date: Mon, 5 Oct 2026 16:24:35 +0200 Subject: [PATCH 02/17] test(controlplane): use constants in the API token claim tests Assisted-by: Claude Code Signed-off-by: Javier Rodriguez Chainloop-Trace-Sessions: 796bcbff-1898-452d-aaeb-b5311bffdfa6 --- .../pkg/jwt/apitoken/apitoken_test.go | 71 ++++++++++--------- 1 file changed, 38 insertions(+), 33 deletions(-) diff --git a/app/controlplane/pkg/jwt/apitoken/apitoken_test.go b/app/controlplane/pkg/jwt/apitoken/apitoken_test.go index dbd24b8b4..a45cf7f60 100644 --- a/app/controlplane/pkg/jwt/apitoken/apitoken_test.go +++ b/app/controlplane/pkg/jwt/apitoken/apitoken_test.go @@ -69,6 +69,11 @@ func TestNewBuilder(t *testing.T) { } } +const ( + testKeyName = "key-name" + claimJTI = "jti" +) + func TestGenerateJWT(t *testing.T) { const hmacSecret = "my-secret" org := uuid.MustParse("123e4567-e89b-12d3-a456-426614174000") @@ -86,39 +91,39 @@ func TestGenerateJWT(t *testing.T) { }{ { name: "organization token", - opts: &GenerateJWTOptions{OrgID: &org, OrgName: toPtr("org-name"), KeyName: "key-name", KeyID: keyID, + opts: &GenerateJWTOptions{OrgID: &org, OrgName: toPtr("org-name"), KeyName: testKeyName, KeyID: keyID, ExpiresAt: toPtr(time.Now().Add(1 * time.Hour)), ScopeType: &orgScope, ScopeID: &org}, }, { name: "no expiration", - opts: &GenerateJWTOptions{OrgID: &org, OrgName: toPtr("org-name"), KeyName: "key-name", KeyID: keyID, + opts: &GenerateJWTOptions{OrgID: &org, OrgName: toPtr("org-name"), KeyName: testKeyName, KeyID: keyID, ScopeType: &orgScope, ScopeID: &org}, }, { name: "project token", - opts: &GenerateJWTOptions{OrgID: &org, OrgName: toPtr("org-name"), KeyName: "key-name", KeyID: keyID, + opts: &GenerateJWTOptions{OrgID: &org, OrgName: toPtr("org-name"), KeyName: testKeyName, KeyID: keyID, ProjectID: &project, ProjectName: toPtr("project-name"), ExpiresAt: toPtr(time.Now().Add(1 * time.Hour)), ScopeType: &projectScope, ScopeID: &project}, }, { name: "workflow-pinned token", - opts: &GenerateJWTOptions{OrgID: &org, OrgName: toPtr("org-name"), KeyName: "key-name", KeyID: keyID, + opts: &GenerateJWTOptions{OrgID: &org, OrgName: toPtr("org-name"), KeyName: testKeyName, KeyID: keyID, ProjectID: &project, ProjectName: toPtr("project-name"), WorkflowID: &workflow, WorkflowName: toPtr("workflow-name"), ExpiresAt: toPtr(time.Now().Add(1 * time.Hour)), ScopeType: &projectScope, ScopeID: &project}, }, { name: "product token", - opts: &GenerateJWTOptions{OrgID: &org, OrgName: toPtr("org-name"), KeyName: "key-name", KeyID: keyID, + opts: &GenerateJWTOptions{OrgID: &org, OrgName: toPtr("org-name"), KeyName: testKeyName, KeyID: keyID, ScopeType: &productScope, ScopeID: &product}, }, { name: "instance token keeps the instance-admin claim", - opts: &GenerateJWTOptions{KeyName: "key-name", KeyID: keyID, ExpiresAt: toPtr(time.Now().Add(1 * time.Hour)), + opts: &GenerateJWTOptions{KeyName: testKeyName, KeyID: keyID, ExpiresAt: toPtr(time.Now().Add(1 * time.Hour)), Scope: toPtr("INSTANCE_ADMIN"), ScopeType: &instanceScope}, }, { name: "missing keyID", - opts: &GenerateJWTOptions{OrgID: &org, OrgName: toPtr("org-name"), KeyName: "key-name", + opts: &GenerateJWTOptions{OrgID: &org, OrgName: toPtr("org-name"), KeyName: testKeyName, ScopeType: &orgScope, ScopeID: &org}, wantErr: true, }, @@ -130,24 +135,24 @@ func TestGenerateJWT(t *testing.T) { }, { name: "a scope id without a scope type", - opts: &GenerateJWTOptions{OrgID: &org, OrgName: toPtr("org-name"), KeyName: "key-name", KeyID: keyID, ScopeID: &org}, + opts: &GenerateJWTOptions{OrgID: &org, OrgName: toPtr("org-name"), KeyName: testKeyName, KeyID: keyID, ScopeID: &org}, wantErr: true, }, { name: "an organization scope naming another organization", - opts: &GenerateJWTOptions{OrgID: &org, OrgName: toPtr("org-name"), KeyName: "key-name", KeyID: keyID, + opts: &GenerateJWTOptions{OrgID: &org, OrgName: toPtr("org-name"), KeyName: testKeyName, KeyID: keyID, ScopeType: &orgScope, ScopeID: &product}, wantErr: true, }, { name: "the instance-admin claim on an organization scope", - opts: &GenerateJWTOptions{OrgID: &org, OrgName: toPtr("org-name"), KeyName: "key-name", KeyID: keyID, + opts: &GenerateJWTOptions{OrgID: &org, OrgName: toPtr("org-name"), KeyName: testKeyName, KeyID: keyID, Scope: toPtr("INSTANCE_ADMIN"), ScopeType: &orgScope, ScopeID: &org}, wantErr: true, }, { name: "a project scope naming another project", - opts: &GenerateJWTOptions{OrgID: &org, OrgName: toPtr("org-name"), KeyName: "key-name", KeyID: keyID, + opts: &GenerateJWTOptions{OrgID: &org, OrgName: toPtr("org-name"), KeyName: testKeyName, KeyID: keyID, ProjectID: &project, ProjectName: toPtr("project-name"), ScopeType: &projectScope, ScopeID: &product}, wantErr: true, }, @@ -238,36 +243,36 @@ func TestSignedScope(t *testing.T) { wantID *uuid.UUID wantErr bool }{ - {name: "signed organization scope", claims: CustomClaims{OrgID: org.String(), ScopeType: "organization", ScopeID: org.String()}, wantKind: authz.ResourceTypeOrganization, wantID: &org}, - {name: "signed project scope", claims: CustomClaims{OrgID: org.String(), ProjectID: project.String(), ScopeType: "project", ScopeID: project.String()}, wantKind: authz.ResourceTypeProject, wantID: &project}, - {name: "signed workflow-pinned scope", claims: CustomClaims{OrgID: org.String(), ProjectID: project.String(), WorkflowID: workflow.String(), ScopeType: "project", ScopeID: project.String()}, wantKind: authz.ResourceTypeProject, wantID: &project}, - {name: "signed product scope", claims: CustomClaims{OrgID: org.String(), ScopeType: "product", ScopeID: product.String()}, wantKind: authz.ResourceTypeProduct, wantID: &product}, - {name: "signed instance scope", claims: CustomClaims{Scope: authz.ScopeInstanceAdmin, ScopeType: "instance"}, wantKind: authz.ResourceTypeInstance}, + {name: "signed organization scope", claims: CustomClaims{OrgID: org.String(), ScopeType: string(authz.ResourceTypeOrganization), ScopeID: org.String()}, wantKind: authz.ResourceTypeOrganization, wantID: &org}, + {name: "signed project scope", claims: CustomClaims{OrgID: org.String(), ProjectID: project.String(), ScopeType: string(authz.ResourceTypeProject), ScopeID: project.String()}, wantKind: authz.ResourceTypeProject, wantID: &project}, + {name: "signed workflow-pinned scope", claims: CustomClaims{OrgID: org.String(), ProjectID: project.String(), WorkflowID: workflow.String(), ScopeType: string(authz.ResourceTypeProject), ScopeID: project.String()}, wantKind: authz.ResourceTypeProject, wantID: &project}, + {name: "signed product scope", claims: CustomClaims{OrgID: org.String(), ScopeType: string(authz.ResourceTypeProduct), ScopeID: product.String()}, wantKind: authz.ResourceTypeProduct, wantID: &product}, + {name: "signed instance scope", claims: CustomClaims{Scope: authz.ScopeInstanceAdmin, ScopeType: string(authz.ResourceTypeInstance)}, wantKind: authz.ResourceTypeInstance}, {name: "legacy instance-admin token", claims: CustomClaims{Scope: authz.ScopeInstanceAdmin}, wantKind: authz.ResourceTypeInstance}, {name: "legacy project token", claims: CustomClaims{OrgID: org.String(), ProjectID: project.String()}, wantKind: authz.ResourceTypeProject, wantID: &project}, {name: "legacy workflow-pinned token", claims: CustomClaims{OrgID: org.String(), ProjectID: project.String(), WorkflowID: workflow.String()}, wantKind: authz.ResourceTypeProject, wantID: &project}, {name: "legacy organization token", claims: CustomClaims{OrgID: org.String()}, wantKind: authz.ResourceTypeOrganization, wantID: &org}, // Malformed scope claims - {name: "an instance scope naming a resource", claims: CustomClaims{Scope: authz.ScopeInstanceAdmin, ScopeType: "instance", ScopeID: org.String()}, wantErr: true}, - {name: "a resource scope without an id", claims: CustomClaims{OrgID: org.String(), ProjectID: project.String(), ScopeType: "project"}, wantErr: true}, - {name: "a scope id that is not a uuid", claims: CustomClaims{OrgID: org.String(), ProjectID: project.String(), ScopeType: "project", ScopeID: "nope"}, wantErr: true}, + {name: "an instance scope naming a resource", claims: CustomClaims{Scope: authz.ScopeInstanceAdmin, ScopeType: string(authz.ResourceTypeInstance), ScopeID: org.String()}, wantErr: true}, + {name: "a resource scope without an id", claims: CustomClaims{OrgID: org.String(), ProjectID: project.String(), ScopeType: string(authz.ResourceTypeProject)}, wantErr: true}, + {name: "a scope id that is not a uuid", claims: CustomClaims{OrgID: org.String(), ProjectID: project.String(), ScopeType: string(authz.ResourceTypeProject), ScopeID: "nope"}, wantErr: true}, {name: "a scope type no token has", claims: CustomClaims{OrgID: org.String(), ScopeType: "group", ScopeID: org.String()}, wantErr: true}, {name: "legacy claims naming nothing", claims: CustomClaims{}, wantErr: true}, {name: "a legacy project claim that is not a uuid", claims: CustomClaims{OrgID: org.String(), ProjectID: "nope"}, wantErr: true}, // Claims that contradict themselves: every reader must see the same scope - {name: "the instance-admin claim on an organization scope", claims: CustomClaims{OrgID: org.String(), Scope: authz.ScopeInstanceAdmin, ScopeType: "organization", ScopeID: org.String()}, wantErr: true}, - {name: "an instance scope without the instance-admin claim", claims: CustomClaims{ScopeType: "instance"}, wantErr: true}, - {name: "an instance scope naming an organization", claims: CustomClaims{OrgID: org.String(), Scope: authz.ScopeInstanceAdmin, ScopeType: "instance"}, wantErr: true}, + {name: "the instance-admin claim on an organization scope", claims: CustomClaims{OrgID: org.String(), Scope: authz.ScopeInstanceAdmin, ScopeType: string(authz.ResourceTypeOrganization), ScopeID: org.String()}, wantErr: true}, + {name: "an instance scope without the instance-admin claim", claims: CustomClaims{ScopeType: string(authz.ResourceTypeInstance)}, wantErr: true}, + {name: "an instance scope naming an organization", claims: CustomClaims{OrgID: org.String(), Scope: authz.ScopeInstanceAdmin, ScopeType: string(authz.ResourceTypeInstance)}, wantErr: true}, {name: "a legacy instance-admin claim naming an organization", claims: CustomClaims{OrgID: org.String(), Scope: authz.ScopeInstanceAdmin}, wantErr: true}, - {name: "an organization scope naming another organization", claims: CustomClaims{OrgID: org.String(), ScopeType: "organization", ScopeID: other.String()}, wantErr: true}, - {name: "an organization scope with a project claim", claims: CustomClaims{OrgID: org.String(), ProjectID: project.String(), ScopeType: "organization", ScopeID: org.String()}, wantErr: true}, - {name: "a project scope naming another project", claims: CustomClaims{OrgID: org.String(), ProjectID: project.String(), ScopeType: "project", ScopeID: other.String()}, wantErr: true}, - {name: "a project scope without an organization", claims: CustomClaims{ProjectID: project.String(), ScopeType: "project", ScopeID: project.String()}, wantErr: true}, - {name: "a product scope with a project claim", claims: CustomClaims{OrgID: org.String(), ProjectID: project.String(), ScopeType: "product", ScopeID: product.String()}, wantErr: true}, - {name: "a product scope without an organization", claims: CustomClaims{ScopeType: "product", ScopeID: product.String()}, wantErr: true}, - {name: "a workflow claim without a project claim", claims: CustomClaims{OrgID: org.String(), WorkflowID: workflow.String(), ScopeType: "organization", ScopeID: org.String()}, wantErr: true}, + {name: "an organization scope naming another organization", claims: CustomClaims{OrgID: org.String(), ScopeType: string(authz.ResourceTypeOrganization), ScopeID: other.String()}, wantErr: true}, + {name: "an organization scope with a project claim", claims: CustomClaims{OrgID: org.String(), ProjectID: project.String(), ScopeType: string(authz.ResourceTypeOrganization), ScopeID: org.String()}, wantErr: true}, + {name: "a project scope naming another project", claims: CustomClaims{OrgID: org.String(), ProjectID: project.String(), ScopeType: string(authz.ResourceTypeProject), ScopeID: other.String()}, wantErr: true}, + {name: "a project scope without an organization", claims: CustomClaims{ProjectID: project.String(), ScopeType: string(authz.ResourceTypeProject), ScopeID: project.String()}, wantErr: true}, + {name: "a product scope with a project claim", claims: CustomClaims{OrgID: org.String(), ProjectID: project.String(), ScopeType: string(authz.ResourceTypeProduct), ScopeID: product.String()}, wantErr: true}, + {name: "a product scope without an organization", claims: CustomClaims{ScopeType: string(authz.ResourceTypeProduct), ScopeID: product.String()}, wantErr: true}, + {name: "a workflow claim without a project claim", claims: CustomClaims{OrgID: org.String(), WorkflowID: workflow.String(), ScopeType: string(authz.ResourceTypeOrganization), ScopeID: org.String()}, wantErr: true}, } for _, tc := range testCases { @@ -295,24 +300,24 @@ func TestClaimsFromMap(t *testing.T) { { name: "every claim is read", in: jwt.MapClaims{ - "jti": "id", "aud": []any{Audience}, "org_id": "o", "org_name": "on", "token_name": "t", + claimJTI: "id", "aud": []any{Audience}, "org_id": "o", "org_name": "on", "token_name": "t", "project_id": "p", "workflow_id": "w", "scope": authz.ScopeInstanceAdmin, "scope_type": "project", "scope_id": "s", }, want: &CustomClaims{ OrgID: "o", OrgName: "on", KeyName: "t", ProjectID: "p", WorkflowID: "w", - Scope: authz.ScopeInstanceAdmin, ScopeType: "project", ScopeID: "s", + Scope: authz.ScopeInstanceAdmin, ScopeType: string(authz.ResourceTypeProject), ScopeID: "s", RegisteredClaims: jwt.RegisteredClaims{ID: "id", Audience: jwt.ClaimStrings{Audience}}, }, }, { name: "a single audience string is read", - in: jwt.MapClaims{"jti": "id", "aud": Audience}, + in: jwt.MapClaims{claimJTI: "id", "aud": Audience}, want: &CustomClaims{RegisteredClaims: jwt.RegisteredClaims{ID: "id", Audience: jwt.ClaimStrings{Audience}}}, }, { // The API entry point used to drop such a claim silently and skip its cross-check name: "a claim of the wrong type is refused", - in: jwt.MapClaims{"jti": "id", "project_id": 42}, + in: jwt.MapClaims{claimJTI: "id", "project_id": 42}, wantErr: true, }, } From 7c39a3ac97ddb121f75991f9c4f4c65f2d8ab97f Mon Sep 17 00:00:00 2001 From: Javier Rodriguez Date: Mon, 5 Oct 2026 16:29:20 +0200 Subject: [PATCH 03/17] feat(controlplane): verify an API token's row against its signed claims Mutation check: disabling each guard in VerifyClaims fails at least one case. nil claims: 'no claims'; no scope: 'a row recording no scope'; legacy product: 'a product token minted before the scope claims'; scope comparison: the 'widened'/'made an instance row'/'moved to another ...' cases; organization: 'a product row moved to another organization'; project: 'the project claim disagrees with the row's project column'; workflow: 'a workflow claim on a row with no workflow'; SignedScope error: 'malformed scope claims' and 'an instance-admin claim on an organization row'. Dropping only the scope id comparison fails the 'moved to another project/product/organization' cases. Gap: flipping sameScopeID's nil branch to return true is not caught by any case (one-nil/one-set ids with equal kinds are unexercised). Assisted-by: Claude Code Signed-off-by: Javier Rodriguez Chainloop-Trace-Sessions: 796bcbff-1898-452d-aaeb-b5311bffdfa6 --- app/controlplane/pkg/biz/apitoken.go | 63 +++++++++++ .../pkg/biz/apitoken_verify_claims_test.go | 106 ++++++++++++++++++ 2 files changed, 169 insertions(+) create mode 100644 app/controlplane/pkg/biz/apitoken_verify_claims_test.go diff --git a/app/controlplane/pkg/biz/apitoken.go b/app/controlplane/pkg/biz/apitoken.go index 58b8b0481..6b3a723d4 100644 --- a/app/controlplane/pkg/biz/apitoken.go +++ b/app/controlplane/pkg/biz/apitoken.go @@ -18,6 +18,7 @@ package biz import ( "bytes" "context" + "errors" "fmt" "slices" "time" @@ -34,6 +35,11 @@ import ( "github.com/google/uuid" ) +// ErrAPITokenClaimsMismatch is a token whose row disagrees with the claims it was signed with, +// or whose claims are malformed. Something wrote the row wrongly, so it is a security event, not +// an expired or legacy credential. +var ErrAPITokenClaimsMismatch = errors.New("API token claims do not match its row") + var apiTokenTracer = otelx.Tracer("chainloop-controlplane", "biz/apitoken") type APITokenJWTConfig struct { @@ -161,6 +167,63 @@ func (t *APIToken) IsOrgScoped() bool { return t.scopeView().IsOrgScoped() } +// VerifyClaims checks the token's row against the claims its JWT was signed with. The claims bind +// what the token was granted: its scope and its organization, and its project and workflow when +// they name one. The row may only confirm them, so a row that disagrees refuses the token +// instead of widening or moving it. A product token minted before the scope claims existed is +// refused: its claims name only its organization. +func (t *APIToken) VerifyClaims(claims *apitoken.CustomClaims) error { + if claims == nil { + return errors.New("API token has no claims") + } + + if t.Scope == nil { + return errors.New("API token records no scope") + } + + if claims.ScopeType == "" && t.IsProductScoped() { + return errors.New("API token was minted before its scope was signed, create a new one") + } + + kind, id, err := claims.SignedScope() + if err != nil { + return fmt.Errorf("API token scope claims: %w: %w", err, ErrAPITokenClaimsMismatch) + } + + if *t.Scope != kind || !sameScopeID(t.ScopeID, id) { + return fmt.Errorf("API token scope mismatch: %w", ErrAPITokenClaimsMismatch) + } + + // An instance token has no organization, and its org_id claim is empty + orgID := "" + if t.OrganizationID != uuid.Nil { + orgID = t.OrganizationID.String() + } + + if claims.OrgID != orgID { + return fmt.Errorf("API token organization mismatch: %w", ErrAPITokenClaimsMismatch) + } + + if claims.ProjectID != "" && (t.ProjectID == nil || t.ProjectID.String() != claims.ProjectID) { + return fmt.Errorf("API token project mismatch: %w", ErrAPITokenClaimsMismatch) + } + + if claims.WorkflowID != "" && (t.WorkflowID == nil || t.WorkflowID.String() != claims.WorkflowID) { + return fmt.Errorf("API token workflow mismatch: %w", ErrAPITokenClaimsMismatch) + } + + return nil +} + +// sameScopeID reports whether two optional scope ids are equal, both unset included. +func sameScopeID(a, b *uuid.UUID) bool { + if a == nil || b == nil { + return a == b + } + + return *a == *b +} + // APITokenCreateOpts is everything the repository persists for a new token. type APITokenCreateOpts struct { Name string diff --git a/app/controlplane/pkg/biz/apitoken_verify_claims_test.go b/app/controlplane/pkg/biz/apitoken_verify_claims_test.go new file mode 100644 index 000000000..4e0d87d53 --- /dev/null +++ b/app/controlplane/pkg/biz/apitoken_verify_claims_test.go @@ -0,0 +1,106 @@ +// +// 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 biz + +import ( + "errors" + "testing" + + "github.com/chainloop-dev/chainloop/app/controlplane/pkg/authz" + "github.com/chainloop-dev/chainloop/app/controlplane/pkg/jwt/apitoken" + "github.com/google/uuid" + "github.com/stretchr/testify/assert" +) + +// errScopeMismatch is the message of a row whose scope differs from the signed one. +const errScopeMismatch = "scope mismatch" + +// The claims bind what a token was granted; its row may only confirm them. Whatever wrote a row +// that disagrees, the token is refused rather than widened or moved. +func TestAPITokenVerifyClaims(t *testing.T) { + org, otherOrg := uuid.New(), uuid.New() + project, otherProject := uuid.New(), uuid.New() + product, otherProduct := uuid.New(), uuid.New() + workflow := uuid.New() + + orgKind, projectKind := authz.ResourceTypeOrganization, authz.ResourceTypeProject + productKind, instanceKind := authz.ResourceTypeProduct, authz.ResourceTypeInstance + + orgRow := &APIToken{OrganizationID: org, Scope: &orgKind, ScopeID: &org} + projectRow := &APIToken{OrganizationID: org, ProjectID: &project, Scope: &projectKind, ScopeID: &project} + workflowRow := &APIToken{OrganizationID: org, ProjectID: &project, WorkflowID: &workflow, Scope: &projectKind, ScopeID: &project} + productRow := &APIToken{OrganizationID: org, Scope: &productKind, ScopeID: &product, ProjectIDs: []uuid.UUID{project}} + instanceRow := &APIToken{Scope: &instanceKind} + + signedOrg := apitoken.CustomClaims{OrgID: org.String(), ScopeType: string(authz.ResourceTypeOrganization), ScopeID: org.String()} + signedProject := apitoken.CustomClaims{OrgID: org.String(), ProjectID: project.String(), ScopeType: string(authz.ResourceTypeProject), ScopeID: project.String()} + signedWorkflow := apitoken.CustomClaims{OrgID: org.String(), ProjectID: project.String(), WorkflowID: workflow.String(), ScopeType: string(authz.ResourceTypeProject), ScopeID: project.String()} + signedProduct := apitoken.CustomClaims{OrgID: org.String(), ScopeType: string(authz.ResourceTypeProduct), ScopeID: product.String()} + legacyOrg := apitoken.CustomClaims{OrgID: org.String()} + legacyProject := apitoken.CustomClaims{OrgID: org.String(), ProjectID: project.String()} + + testCases := []struct { + name string + row *APIToken + claims apitoken.CustomClaims + wantErr string + // mismatch marks a refusal that means the row was written wrongly: it must wrap + // ErrAPITokenClaimsMismatch so the middleware logs it as a security event + mismatch bool + }{ + {name: "signed organization token", row: orgRow, claims: signedOrg}, + {name: "signed project token", row: projectRow, claims: signedProject}, + {name: "signed workflow-pinned token", row: workflowRow, claims: signedWorkflow}, + {name: "signed product token", row: productRow, claims: signedProduct}, + {name: "signed instance token", row: instanceRow, claims: apitoken.CustomClaims{Scope: authz.ScopeInstanceAdmin, ScopeType: string(authz.ResourceTypeInstance)}}, + {name: "legacy organization token", row: orgRow, claims: legacyOrg}, + {name: "legacy project token", row: projectRow, claims: legacyProject}, + {name: "legacy workflow-pinned token", row: workflowRow, claims: apitoken.CustomClaims{OrgID: org.String(), ProjectID: project.String(), WorkflowID: workflow.String()}}, + {name: "legacy instance token", row: instanceRow, claims: apitoken.CustomClaims{Scope: authz.ScopeInstanceAdmin}}, + + {name: "a row recording no scope", row: &APIToken{OrganizationID: org}, claims: legacyOrg, wantErr: "records no scope"}, + {name: "a product token minted before the scope claims", row: productRow, claims: legacyOrg, wantErr: "create a new one"}, + {name: "malformed scope claims", row: orgRow, claims: apitoken.CustomClaims{OrgID: org.String(), ScopeType: string(authz.ResourceTypeOrganization)}, wantErr: "scope claims", mismatch: true}, + {name: "an instance-admin claim on an organization row", row: orgRow, claims: apitoken.CustomClaims{OrgID: org.String(), Scope: authz.ScopeInstanceAdmin}, wantErr: "scope claims", mismatch: true}, + {name: "a project row widened to its organization, signed claims", row: &APIToken{OrganizationID: org, ProjectID: &project, Scope: &orgKind, ScopeID: &org}, claims: signedProject, wantErr: errScopeMismatch, mismatch: true}, + {name: "a project row widened to its organization, legacy claims", row: &APIToken{OrganizationID: org, ProjectID: &project, Scope: &orgKind, ScopeID: &org}, claims: legacyProject, wantErr: errScopeMismatch, mismatch: true}, + {name: "an organization row made an instance row, signed claims", row: &APIToken{OrganizationID: org, Scope: &instanceKind}, claims: signedOrg, wantErr: errScopeMismatch, mismatch: true}, + {name: "an organization row made an instance row, legacy claims", row: &APIToken{OrganizationID: org, Scope: &instanceKind}, claims: legacyOrg, wantErr: errScopeMismatch, mismatch: true}, + {name: "a project row moved to another project", row: &APIToken{OrganizationID: org, ProjectID: &project, Scope: &projectKind, ScopeID: &otherProject}, claims: signedProject, wantErr: errScopeMismatch, mismatch: true}, + {name: "a product row moved to another product", row: &APIToken{OrganizationID: org, Scope: &productKind, ScopeID: &otherProduct}, claims: signedProduct, wantErr: errScopeMismatch, mismatch: true}, + {name: "an organization row moved to another organization", row: &APIToken{OrganizationID: otherOrg, Scope: &orgKind, ScopeID: &otherOrg}, claims: legacyOrg, wantErr: errScopeMismatch, mismatch: true}, + {name: "a product row moved to another organization", row: &APIToken{OrganizationID: otherOrg, Scope: &productKind, ScopeID: &product}, claims: signedProduct, wantErr: "organization mismatch", mismatch: true}, + {name: "the project claim disagrees with the row's project column", row: &APIToken{OrganizationID: org, ProjectID: &otherProject, Scope: &projectKind, ScopeID: &project}, claims: signedProject, wantErr: "project mismatch", mismatch: true}, + {name: "a workflow claim on a row with no workflow", row: projectRow, claims: signedWorkflow, wantErr: "workflow mismatch", mismatch: true}, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + err := tc.row.VerifyClaims(&tc.claims) + if tc.wantErr == "" { + assert.NoError(t, err) + return + } + + assert.ErrorContains(t, err, tc.wantErr) + assert.Equal(t, tc.mismatch, errors.Is(err, ErrAPITokenClaimsMismatch)) + }) + } + + t.Run("no claims", func(t *testing.T) { + assert.ErrorContains(t, orgRow.VerifyClaims(nil), "no claims") + }) +} From 7632e6bddc3d0345902ac92db3f592c24060e053 Mon Sep 17 00:00:00 2001 From: Javier Rodriguez Date: Mon, 5 Oct 2026 16:30:06 +0200 Subject: [PATCH 04/17] test(controlplane): cover a row that records no scope id Assisted-by: Claude Code Signed-off-by: Javier Rodriguez Chainloop-Trace-Sessions: 796bcbff-1898-452d-aaeb-b5311bffdfa6 --- app/controlplane/pkg/biz/apitoken_verify_claims_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/app/controlplane/pkg/biz/apitoken_verify_claims_test.go b/app/controlplane/pkg/biz/apitoken_verify_claims_test.go index 4e0d87d53..001d42166 100644 --- a/app/controlplane/pkg/biz/apitoken_verify_claims_test.go +++ b/app/controlplane/pkg/biz/apitoken_verify_claims_test.go @@ -77,6 +77,7 @@ func TestAPITokenVerifyClaims(t *testing.T) { {name: "an instance-admin claim on an organization row", row: orgRow, claims: apitoken.CustomClaims{OrgID: org.String(), Scope: authz.ScopeInstanceAdmin}, wantErr: "scope claims", mismatch: true}, {name: "a project row widened to its organization, signed claims", row: &APIToken{OrganizationID: org, ProjectID: &project, Scope: &orgKind, ScopeID: &org}, claims: signedProject, wantErr: errScopeMismatch, mismatch: true}, {name: "a project row widened to its organization, legacy claims", row: &APIToken{OrganizationID: org, ProjectID: &project, Scope: &orgKind, ScopeID: &org}, claims: legacyProject, wantErr: errScopeMismatch, mismatch: true}, + {name: "an organization row recording no scope id", row: &APIToken{OrganizationID: org, Scope: &orgKind}, claims: signedOrg, wantErr: errScopeMismatch, mismatch: true}, {name: "an organization row made an instance row, signed claims", row: &APIToken{OrganizationID: org, Scope: &instanceKind}, claims: signedOrg, wantErr: errScopeMismatch, mismatch: true}, {name: "an organization row made an instance row, legacy claims", row: &APIToken{OrganizationID: org, Scope: &instanceKind}, claims: legacyOrg, wantErr: errScopeMismatch, mismatch: true}, {name: "a project row moved to another project", row: &APIToken{OrganizationID: org, ProjectID: &project, Scope: &projectKind, ScopeID: &otherProject}, claims: signedProject, wantErr: errScopeMismatch, mismatch: true}, From e33e037615bb9b07fe188216a26cf3782fc30f90 Mon Sep 17 00:00:00 2001 From: Javier Rodriguez Date: Mon, 5 Oct 2026 16:38:58 +0200 Subject: [PATCH 05/17] feat(controlplane): sign the token scope on API token creation and regeneration Assisted-by: Claude Code Signed-off-by: Javier Rodriguez Chainloop-Trace-Sessions: 796bcbff-1898-452d-aaeb-b5311bffdfa6 --- .../apitoken_middleware_integration_test.go | 15 +-- app/controlplane/pkg/biz/apitoken.go | 24 ++++- .../pkg/biz/apitoken_integration_test.go | 97 ++++++++++++++++--- app/controlplane/pkg/jwt/apitoken/apitoken.go | 20 ++-- .../pkg/jwt/apitoken/apitoken_test.go | 5 + 5 files changed, 121 insertions(+), 40 deletions(-) diff --git a/app/controlplane/internal/usercontext/apitoken_middleware_integration_test.go b/app/controlplane/internal/usercontext/apitoken_middleware_integration_test.go index 26587c33e..7d96ed3f1 100644 --- a/app/controlplane/internal/usercontext/apitoken_middleware_integration_test.go +++ b/app/controlplane/internal/usercontext/apitoken_middleware_integration_test.go @@ -33,9 +33,8 @@ import ( "github.com/stretchr/testify/require" ) -// The token the service layer sees carries exactly the scope its row records. A row recording -// none, such as one a control plane from before the scope columns wrote after the backfill ran, -// is confined to nothing, and with no organization either it is refused. +// The token the service layer sees carries exactly the scope its row records, once the row has +// confirmed the claims the token was signed with. func TestAPITokenMiddlewareCarriesTheRowScope(t *testing.T) { if !testhelpers.IntegrationTestsEnabled() { t.Skip() @@ -80,16 +79,6 @@ func TestAPITokenMiddlewareCarriesTheRowScope(t *testing.T) { opts: biz.APITokenCreateOpts{Scope: biz.ToPtr(authz.ResourceTypeInstance)}, wantScope: biz.ToPtr(authz.ResourceTypeInstance), wantInstanceScoped: true, }, - { - name: "an organization row recording no scope is confined to nothing", - opts: biz.APITokenCreateOpts{OrganizationID: &orgID}, - wantReach: []uuid.UUID{}, - }, - { - name: "a row with neither an organization nor a scope is refused", - opts: biz.APITokenCreateOpts{}, - wantErr: true, - }, } for _, tc := range testCases { diff --git a/app/controlplane/pkg/biz/apitoken.go b/app/controlplane/pkg/biz/apitoken.go index 6b3a723d4..11c0bcd61 100644 --- a/app/controlplane/pkg/biz/apitoken.go +++ b/app/controlplane/pkg/biz/apitoken.go @@ -544,6 +544,9 @@ func (uc *APITokenUseCase) Create(ctx context.Context, name string, description KeyID: token.ID, KeyName: name, ExpiresAt: expiresAt, + // Signed so that the row can only confirm what the token was granted + ScopeType: scope, + ScopeID: scopeID, } // Set org info if available or instance-level token scope @@ -585,7 +588,9 @@ func (uc *APITokenUseCase) Create(ctx context.Context, name string, description return token, nil } -// RegenerateJWT will regenerate a new JWT for the given token. Use with caution, since old JWTs are not invalidated. +// RegenerateJWT will regenerate a new JWT for the given token. Use with caution, since old JWTs are +// not invalidated. The new JWT signs the scope the row records, so the caller must know the row is +// the token it means to re-sign: a row whose shape is coherent but wrong would be signed as it is. func (uc *APITokenUseCase) RegenerateJWT(ctx context.Context, tokenID uuid.UUID, expiresIn time.Duration) (*APIToken, error) { ctx, span := otelx.Start(ctx, apiTokenTracer, "APITokenUseCase.RegenerateJWT") defer span.End() @@ -601,10 +606,27 @@ func (uc *APITokenUseCase) RegenerateJWT(ctx context.Context, tokenID uuid.UUID, return nil, fmt.Errorf("finding token: %w", err) } + // Regenerating must never sign a row that records no scope, or one that disagrees with the + // token it is on. + if token.Scope == nil { + return nil, NewErrValidationStr("the token records no scope") + } + + var rowOrgID *uuid.UUID + if token.OrganizationID != uuid.Nil { + rowOrgID = &token.OrganizationID + } + + if err := ValidateTokenShape(token.Scope, token.ScopeID, rowOrgID, token.ProjectID, token.ProjectIDs); err != nil { + return nil, err + } + generationOpts := &apitoken.GenerateJWTOptions{ KeyID: token.ID, KeyName: token.Name, ExpiresAt: &expiresAt, + ScopeType: token.Scope, + ScopeID: token.ScopeID, } // Check if this is an org-scoped or instance-level token diff --git a/app/controlplane/pkg/biz/apitoken_integration_test.go b/app/controlplane/pkg/biz/apitoken_integration_test.go index 016a71345..13c8b850f 100644 --- a/app/controlplane/pkg/biz/apitoken_integration_test.go +++ b/app/controlplane/pkg/biz/apitoken_integration_test.go @@ -24,6 +24,8 @@ import ( "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/data/ent" + "github.com/chainloop-dev/chainloop/app/controlplane/pkg/jwt/apitoken" "github.com/golang-jwt/jwt/v5" "github.com/google/uuid" @@ -1250,14 +1252,17 @@ func (s *apiTokenTestSuite) TestCreateAProductTokenWithItsProjects() { } } -// A product token's JWT names no product, on creation or regeneration: the row decides what the -// token is confined to. -func (s *apiTokenTestSuite) TestGeneratedJWTCarriesNoProductClaim() { +// Every JWT signs the scope its row records, on creation and on regeneration, so that the row can +// only confirm it. A product token's JWT names its product through that scope alone. +func (s *apiTokenTestSuite) TestGeneratedJWTSignsTheTokenScope() { ctx := context.Background() productID := uuid.New() - claimsOf := func(raw string) jwt.MapClaims { - claims := jwt.MapClaims{} + wf, err := s.Workflow.Create(ctx, &biz.WorkflowCreateOpts{Name: randomName(), OrgID: s.org.ID, Project: s.p1.Name}) + s.Require().NoError(err) + + claimsOf := func(raw string) *apitoken.CustomClaims { + claims := &apitoken.CustomClaims{} info, err := jwt.ParseWithClaims(raw, claims, func(_ *jwt.Token) (interface{}, error) { return []byte("test"), nil }) @@ -1267,18 +1272,80 @@ func (s *apiTokenTestSuite) TestGeneratedJWTCarriesNoProductClaim() { return claims } - token, err := s.APIToken.Create(ctx, randomName(), nil, toPtrDuration(24*time.Hour), &s.org.ID, - biz.APITokenWithScope(authz.ResourceTypeProduct, &productID), biz.APITokenWithProjectIDs(nil)) - s.Require().NoError(err) + testCases := []struct { + name string + org *string + opts []biz.APITokenCreateOpt + wantScopeType authz.ResourceType + wantScopeID string + // wantScope is the legacy "scope" claim + wantScope string + }{ + {name: "organization", org: &s.org.ID, wantScopeType: authz.ResourceTypeOrganization, wantScopeID: s.org.ID}, + {name: string(authz.ResourceTypeProject), org: &s.org.ID, opts: []biz.APITokenCreateOpt{biz.APITokenWithProject(s.p1)}, wantScopeType: authz.ResourceTypeProject, wantScopeID: s.p1.ID.String()}, + {name: "workflow-pinned", org: &s.org.ID, opts: []biz.APITokenCreateOpt{biz.APITokenWithProject(s.p1), biz.APITokenWithWorkflow(wf)}, wantScopeType: authz.ResourceTypeProject, wantScopeID: s.p1.ID.String()}, + {name: "product", org: &s.org.ID, opts: []biz.APITokenCreateOpt{biz.APITokenWithScope(authz.ResourceTypeProduct, &productID), biz.APITokenWithProjectIDs(nil)}, wantScopeType: authz.ResourceTypeProduct, wantScopeID: productID.String()}, + {name: "instance", wantScopeType: authz.ResourceTypeInstance, wantScope: authz.ScopeInstanceAdmin}, + } - regenerated, err := s.APIToken.RegenerateJWT(ctx, token.ID, 48*time.Hour) - s.Require().NoError(err) + for _, tc := range testCases { + s.Run(tc.name, func() { + created, err := s.APIToken.Create(ctx, randomName(), nil, toPtrDuration(24*time.Hour), tc.org, tc.opts...) + s.Require().NoError(err) + regenerated, err := s.APIToken.RegenerateJWT(ctx, created.ID, 48*time.Hour) + s.Require().NoError(err) + stored, err := s.Repos.APITokenRepo.FindByID(ctx, created.ID) + s.Require().NoError(err) - for minted, raw := range map[string]string{"created": token.JWT, "regenerated": regenerated.JWT} { - claims := claimsOf(raw) - s.NotContains(claims, "product_id", minted) - s.Equal(token.ID.String(), claims["jti"], minted) - s.Equal(s.org.ID, claims["org_id"], minted) + for minted, raw := range map[string]string{"created": created.JWT, "regenerated": regenerated.JWT} { + claims := claimsOf(raw) + s.Equal(string(tc.wantScopeType), claims.ScopeType, minted) + s.Equal(tc.wantScopeID, claims.ScopeID, minted) + s.Equal(tc.wantScope, claims.Scope, minted) + s.NoError(stored.VerifyClaims(claims), minted) + + // What changes during a token's life never goes into the JWT + payload := jwt.MapClaims{} + _, _, err := jwt.NewParser().ParseUnverified(raw, payload) + s.Require().NoError(err) + s.NotContains(payload, "project_ids", minted) + s.NotContains(payload, "policies", minted) + s.NotContains(payload, "product_id", minted) + } + }) + } +} + +// Regenerating signs the scope the row records. A row that records none, or one that disagrees +// with the token it is on, is refused instead of being signed. +func (s *apiTokenTestSuite) TestRegenerateJWTRefusesARowItCannotSign() { + ctx := context.Background() + orgUUID := uuid.MustParse(s.org.ID) + + testCases := []struct { + name string + row func(*ent.APITokenCreate) *ent.APITokenCreate + }{ + {name: "a row recording no scope", row: func(c *ent.APITokenCreate) *ent.APITokenCreate { + return c.SetOrganizationID(orgUUID) + }}, + {name: "an instance scope on an organization's token", row: func(c *ent.APITokenCreate) *ent.APITokenCreate { + return c.SetOrganizationID(orgUUID).SetScope(authz.ResourceTypeInstance) + }}, + {name: "an organization scope on a project token", row: func(c *ent.APITokenCreate) *ent.APITokenCreate { + return c.SetOrganizationID(orgUUID).SetProjectID(s.p1.ID).SetScope(authz.ResourceTypeOrganization).SetScopeID(orgUUID) + }}, + } + + for _, tc := range testCases { + s.Run(tc.name, func() { + row, err := tc.row(s.Data.DB.APIToken.Create().SetName(randomName())).Save(ctx) + s.Require().NoError(err) + + _, err = s.APIToken.RegenerateJWT(ctx, row.ID, time.Hour) + s.Require().Error(err) + s.True(biz.IsErrValidation(err), "got %v", err) + }) } } diff --git a/app/controlplane/pkg/jwt/apitoken/apitoken.go b/app/controlplane/pkg/jwt/apitoken/apitoken.go index 430aeea40..b9974e782 100644 --- a/app/controlplane/pkg/jwt/apitoken/apitoken.go +++ b/app/controlplane/pkg/jwt/apitoken/apitoken.go @@ -135,20 +135,18 @@ func (ra *Builder) GenerateJWT(opts *GenerateJWTOptions) (string, error) { claims.WorkflowName = *opts.WorkflowName } - if opts.ScopeID != nil && opts.ScopeType == nil { - return "", errors.New("scopeType is required when scopeID is set") + if opts.ScopeType == nil { + return "", errors.New("scopeType is required") } - if opts.ScopeType != nil { - claims.ScopeType = string(*opts.ScopeType) - if opts.ScopeID != nil { - claims.ScopeID = opts.ScopeID.String() - } + claims.ScopeType = string(*opts.ScopeType) + if opts.ScopeID != nil { + claims.ScopeID = opts.ScopeID.String() + } - // Never sign a token whose claims contradict themselves - if _, _, err := claims.SignedScope(); err != nil { - return "", fmt.Errorf("inconsistent token scope: %w", err) - } + // Never sign a token whose claims contradict themselves + if _, _, err := claims.SignedScope(); err != nil { + return "", fmt.Errorf("inconsistent token scope: %w", err) } // optional expiration value, i.e 30 days diff --git a/app/controlplane/pkg/jwt/apitoken/apitoken_test.go b/app/controlplane/pkg/jwt/apitoken/apitoken_test.go index a45cf7f60..33fd140a9 100644 --- a/app/controlplane/pkg/jwt/apitoken/apitoken_test.go +++ b/app/controlplane/pkg/jwt/apitoken/apitoken_test.go @@ -138,6 +138,11 @@ func TestGenerateJWT(t *testing.T) { opts: &GenerateJWTOptions{OrgID: &org, OrgName: toPtr("org-name"), KeyName: testKeyName, KeyID: keyID, ScopeID: &org}, wantErr: true, }, + { + name: "missing scope type", + opts: &GenerateJWTOptions{OrgID: &org, OrgName: toPtr("org-name"), KeyName: testKeyName, KeyID: keyID}, + wantErr: true, + }, { name: "an organization scope naming another organization", opts: &GenerateJWTOptions{OrgID: &org, OrgName: toPtr("org-name"), KeyName: testKeyName, KeyID: keyID, From 050cc7db69acf525b9d6ea152c2e0012f5ab03f5 Mon Sep 17 00:00:00 2001 From: Javier Rodriguez Date: Mon, 5 Oct 2026 16:42:28 +0200 Subject: [PATCH 06/17] feat(controlplane): refuse an API token whose row disagrees with its signed claims Assisted-by: Claude Code Signed-off-by: Javier Rodriguez Chainloop-Trace-Sessions: 796bcbff-1898-452d-aaeb-b5311bffdfa6 --- .../usercontext/apitoken_middleware.go | 48 ++- .../apitoken_middleware_integration_test.go | 316 ++++++++++++++++++ .../usercontext/apitoken_middleware_test.go | 142 ++++++-- 3 files changed, 441 insertions(+), 65 deletions(-) diff --git a/app/controlplane/internal/usercontext/apitoken_middleware.go b/app/controlplane/internal/usercontext/apitoken_middleware.go index 425c072e6..2f096bf6d 100644 --- a/app/controlplane/internal/usercontext/apitoken_middleware.go +++ b/app/controlplane/internal/usercontext/apitoken_middleware.go @@ -71,23 +71,18 @@ func WithCurrentAPITokenAndOrgMiddleware(apiTokenUC *biz.APITokenUseCase, orgUC // We've received an API-token if claimsHaveAudience(genericClaims, apitoken.Audience) { - var err error - tokenID, ok := genericClaims["jti"].(string) - if !ok || tokenID == "" { + claims, err := apitoken.ClaimsFromMap(genericClaims) + if err != nil || claims.ID == "" { return nil, errors.New("error mapping the API-token claims") } - // Project ID is optional - projectID, _ := genericClaims["project_id"].(string) - - workflowID, _ := genericClaims["workflow_id"].(string) - - ctx, err = setCurrentOrgAndAPIToken(ctx, apiTokenUC, orgUC, tokenID, projectID, workflowID) + ctx, err = setCurrentOrgAndAPIToken(ctx, apiTokenUC, orgUC, claims, logger) if err != nil { return nil, fmt.Errorf("error setting current org and user: %w", err) } - logger.Infow("msg", "[authN] processed credentials", "id", tokenID, "type", "API-token", "projectID", projectID) + // legacy_claims counts the tokens minted before the scope claims, to plan their sunset + logger.Infow("msg", "[authN] processed credentials", "id", claims.ID, "type", "API-token", "projectID", claims.ProjectID, "legacy_claims", claims.ScopeType == "") } return handler(ctx, req) @@ -131,12 +126,12 @@ func WithAttestationContextFromAPIToken(apiTokenUC *biz.APITokenUseCase, orgUC * return nil, fmt.Errorf("error extracting organization from APIToken: %w", err) } - ctx, err = setCurrentOrgAndAPIToken(ctx, apiTokenUC, orgUC, tokenID, claims.ProjectID, claims.WorkflowID) + ctx, err = setCurrentOrgAndAPIToken(ctx, apiTokenUC, orgUC, claims, logger) if err != nil { return nil, fmt.Errorf("error setting current org and user: %w", err) } - logger.Infow("msg", "[authN] processed credentials", "id", tokenID, "type", "API-token") + logger.Infow("msg", "[authN] processed credentials", "id", tokenID, "type", "API-token", "legacy_claims", claims.ScopeType == "") return handler(ctx, req) } @@ -167,34 +162,31 @@ func setRobotAccountFromAPIToken(ctx context.Context, apiTokenUC *biz.APITokenUs return ctx, nil } -// Set the current organization and API-Token in the context. The project and workflow claims are -// cross-checked against the token row, never an authorization input: the row decides what the -// token reaches. -func setCurrentOrgAndAPIToken(ctx context.Context, apiTokenUC *biz.APITokenUseCase, orgUC *biz.OrganizationUseCase, tokenID, projectIDInClaim, workflowIDInClaim string) (context.Context, error) { - if tokenID == "" { +// Set the current organization and API-Token in the context. The row must agree with the claims +// the token was signed with: they bind its scope, organization, project and workflow, and the row +// only confirms them. What the row adds, its policies, its project list and its revocation, is +// read from the row. +func setCurrentOrgAndAPIToken(ctx context.Context, apiTokenUC *biz.APITokenUseCase, orgUC *biz.OrganizationUseCase, claims *apitoken.CustomClaims, logger *log.Helper) (context.Context, error) { + if claims == nil || claims.ID == "" { return nil, errors.New("error retrieving the key ID from the API token") } // Check that the token exists and is not revoked - token, err := apiTokenUC.FindByID(ctx, tokenID) + token, err := apiTokenUC.FindByID(ctx, claims.ID) if err != nil { return nil, fmt.Errorf("error retrieving the API token: %w", err) } else if token == nil { return nil, errors.New("API token not found") } - // Make sure that the projectID that comes in the token claim matches the one in the DB - if projectIDInClaim != "" { - if token.ProjectID == nil || token.ProjectID.String() != projectIDInClaim { - return nil, errors.New("API token project mismatch") + if err := token.VerifyClaims(claims); err != nil { + // A row should never disagree with its signed claims: something wrote it wrongly. The raw + // JWT is never logged. + if errors.Is(err, biz.ErrAPITokenClaimsMismatch) { + logger.Errorw("msg", "[authN] API token row disagrees with its signed claims", "id", claims.ID, "error", err) } - } - // Same defense in depth for the workflow claim - if workflowIDInClaim != "" { - if token.WorkflowID == nil || token.WorkflowID.String() != workflowIDInClaim { - return nil, errors.New("API token workflow mismatch") - } + return nil, err } // Note: Expiration time does not need to be checked because that's done at the JWT diff --git a/app/controlplane/internal/usercontext/apitoken_middleware_integration_test.go b/app/controlplane/internal/usercontext/apitoken_middleware_integration_test.go index 7d96ed3f1..e9623c4e3 100644 --- a/app/controlplane/internal/usercontext/apitoken_middleware_integration_test.go +++ b/app/controlplane/internal/usercontext/apitoken_middleware_integration_test.go @@ -18,15 +18,21 @@ package usercontext import ( "context" "io" + "os" "testing" "time" + "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/data/ent" + "github.com/chainloop-dev/chainloop/app/controlplane/pkg/jwt/apitoken" "github.com/chainloop-dev/chainloop/app/controlplane/pkg/usercontext/entities" "github.com/go-kratos/kratos/v2/log" + "github.com/go-kratos/kratos/v2/middleware" jwtmiddleware "github.com/go-kratos/kratos/v2/middleware/auth/jwt" + "github.com/go-kratos/kratos/v2/transport" "github.com/golang-jwt/jwt/v5" "github.com/google/uuid" "github.com/stretchr/testify/assert" @@ -120,3 +126,313 @@ func TestAPITokenMiddlewareCarriesTheRowScope(t *testing.T) { }) } } + +const scopeBackfillMigration = "../../pkg/data/ent/migrate/migrations/20260930160057.sql" + +// authenticated is what an entry point put in the context, or why it refused the token. +type authenticated struct { + token *entities.APIToken + org *entities.Org + err error +} + +// authenticateAtBothEntryPoints sends signed through the API and the attestation entry points, +// naming orgName in the organization header. +func authenticateAtBothEntryPoints(t *testing.T, tu *testhelpers.TestingUseCases, signed, orgName string) map[string]authenticated { + t.Helper() + const orgHeader = "Chainloop-Organization" + logger := log.NewHelper(log.NewStdLogger(io.Discard)) + + capture := func(out *authenticated) middleware.Handler { + return func(ctx context.Context, _ interface{}) (interface{}, error) { + out.token, out.org = entities.CurrentAPIToken(ctx), entities.CurrentOrg(ctx) + return nil, nil + } + } + + var api, attestation authenticated + + claims := jwt.MapClaims{} + _, _, err := jwt.NewParser().ParseUnverified(signed, claims) + require.NoError(t, err) + ctx := transport.NewServerContext(context.Background(), &fakeTransport{header: headerCarrier{orgHeader: {orgName}}}) + _, api.err = WithCurrentAPITokenAndOrgMiddleware(tu.APIToken, tu.Organization, logger)(capture(&api))(jwtmiddleware.NewContext(ctx, claims), nil) + + ctx = transport.NewServerContext(context.Background(), &fakeTransport{header: headerCarrier{ + authorizationHeader: {"Bearer " + signed}, + orgHeader: {orgName}, + }}) + _, attestation.err = middleware.Chain( + attjwtmiddleware.WithJWTMulti(log.NewStdLogger(io.Discard), attjwtmiddleware.NewAPITokenProvider("test")), + WithAttestationContextFromAPIToken(tu.APIToken, tu.Organization, logger), + )(capture(&attestation))(ctx, nil) + + return map[string]authenticated{entryAPI: api, entryAttestation: attestation} +} + +// signLegacy signs claims the way a control plane from before the scope claims did, with the +// testhelpers' signing key. +func signLegacy(t *testing.T, tokenID uuid.UUID, claims jwt.MapClaims) string { + t.Helper() + all := jwt.MapClaims{"token_name": "legacy", claimJTI: tokenID.String(), "iss": "cp.chainloop", claimAud: []string{apitoken.Audience}} + for k, v := range claims { + all[k] = v + } + + signed, err := jwt.NewWithClaims(apitoken.SigningMethod, all).SignedString([]byte("test")) + require.NoError(t, err) + + return signed +} + +// Organization, project, workflow-pinned and instance tokens minted before the scope claims keep +// working, with the scope they always had: their rows went through the scope backfill and their +// claims imply the same scope. A product token minted before the scope claims, and a row written +// with no scope after the backfill ran, are refused. +func TestAPITokenMiddlewareAcceptsTokensMintedBeforeTheScopeClaims(t *testing.T) { + if !testhelpers.IntegrationTestsEnabled() { + t.Skip() + } + + tu := testhelpers.NewTestingUseCases(t) + defer tu.DB.Close(t) + ctx := context.Background() + + org, err := tu.Organization.CreateWithRandomName(ctx) + require.NoError(t, err) + orgID := uuid.MustParse(org.ID) + headerOrg, err := tu.Organization.CreateWithRandomName(ctx) + require.NoError(t, err) + headerOrgID := uuid.MustParse(headerOrg.ID) + project, err := tu.Project.Create(ctx, org.ID, "legacy") + require.NoError(t, err) + other, err := tu.Project.Create(ctx, org.ID, "other") + require.NoError(t, err) + workflow, err := tu.Workflow.Create(ctx, &biz.WorkflowCreateOpts{OrgID: org.ID, Name: "legacy", Project: project.Name}) + require.NoError(t, err) + product := uuid.New() + + orgClaims := jwt.MapClaims{claimOrgID: org.ID, claimOrgName: org.Name} + projectClaims := jwt.MapClaims{claimOrgID: org.ID, claimOrgName: org.Name, claimProjectID: project.ID.String(), "project_name": project.Name} + instanceClaims := jwt.MapClaims{claimOrgID: "", claimOrgName: "", "scope": authz.ScopeInstanceAdmin} + + testCases := []struct { + name string + // row writes the columns the control plane that minted the token wrote + row func(*ent.APITokenCreate) *ent.APITokenCreate + claims jwt.MapClaims + header string + // afterBackfill writes the row once the backfill has run, as a control plane from before + // the scope columns does during a rolling upgrade or after a rollback + afterBackfill bool + + wantErr string + wantScope authz.ResourceType + wantScopeID *uuid.UUID + wantOrg *uuid.UUID + wantReachAll bool + wantReach []uuid.UUID + }{ + { + name: "oldest organization token, no org_name claim", + row: func(c *ent.APITokenCreate) *ent.APITokenCreate { return c.SetOrganizationID(orgID) }, + claims: jwt.MapClaims{claimOrgID: org.ID}, + wantScope: authz.ResourceTypeOrganization, wantScopeID: &orgID, wantOrg: &orgID, wantReachAll: true, + }, + { + name: "organization token ignores the organization header", + row: func(c *ent.APITokenCreate) *ent.APITokenCreate { return c.SetOrganizationID(orgID) }, + claims: orgClaims, header: headerOrg.Name, + wantScope: authz.ResourceTypeOrganization, wantScopeID: &orgID, wantOrg: &orgID, wantReachAll: true, + }, + { + name: "project token", + row: func(c *ent.APITokenCreate) *ent.APITokenCreate { + return c.SetOrganizationID(orgID).SetProjectID(project.ID) + }, + claims: projectClaims, + wantScope: authz.ResourceTypeProject, wantScopeID: &project.ID, wantOrg: &orgID, wantReach: []uuid.UUID{project.ID}, + }, + { + name: "workflow-pinned token", + row: func(c *ent.APITokenCreate) *ent.APITokenCreate { + return c.SetOrganizationID(orgID).SetProjectID(project.ID).SetWorkflowID(workflow.ID) + }, + claims: jwt.MapClaims{claimOrgID: org.ID, claimOrgName: org.Name, claimProjectID: project.ID.String(), "project_name": project.Name, + "workflow_id": workflow.ID.String(), "workflow_name": workflow.Name}, + wantScope: authz.ResourceTypeProject, wantScopeID: &project.ID, wantOrg: &orgID, wantReach: []uuid.UUID{project.ID}, + }, + { + name: "instance token takes the organization header", + row: func(c *ent.APITokenCreate) *ent.APITokenCreate { return c }, + claims: instanceClaims, header: headerOrg.Name, + wantScope: authz.ResourceTypeInstance, wantOrg: &headerOrgID, wantReachAll: true, + }, + { + name: "instance token without the header has no organization", + row: func(c *ent.APITokenCreate) *ent.APITokenCreate { return c }, + claims: instanceClaims, + wantScope: authz.ResourceTypeInstance, wantReachAll: true, + }, + { + name: "product token minted before the scope claims", + row: func(c *ent.APITokenCreate) *ent.APITokenCreate { + return c.SetOrganizationID(orgID).SetScope(authz.ResourceTypeProduct).SetScopeID(product).SetProjectIds([]uuid.UUID{project.ID}) + }, + claims: orgClaims, + wantErr: "create a new one", + }, + { + name: "organization row written with no scope after the backfill", + row: func(c *ent.APITokenCreate) *ent.APITokenCreate { return c.SetOrganizationID(orgID) }, + claims: orgClaims, + afterBackfill: true, + wantErr: errRecordsNoScope, + }, + { + name: "instance row written with no scope after the backfill", + row: func(c *ent.APITokenCreate) *ent.APITokenCreate { return c }, + claims: instanceClaims, + afterBackfill: true, + wantErr: errRecordsNoScope, + }, + } + + create := func(i int) uuid.UUID { + row, err := testCases[i].row(tu.Data.DB.APIToken.Create().SetName("legacy-" + uuid.NewString())).Save(ctx) + require.NoError(t, err) + return row.ID + } + + ids := make([]uuid.UUID, len(testCases)) + for i, tc := range testCases { + if !tc.afterBackfill { + ids[i] = create(i) + } + } + + backfill, err := os.ReadFile(scopeBackfillMigration) + require.NoError(t, err) + _, err = tu.Data.SQLDB.ExecContext(ctx, string(backfill)) + require.NoError(t, err) + + for i, tc := range testCases { + if tc.afterBackfill { + ids[i] = create(i) + } + } + + for i, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + for entry, got := range authenticateAtBothEntryPoints(t, tu, signLegacy(t, ids[i], tc.claims), tc.header) { + if tc.wantErr != "" { + assert.ErrorContains(t, got.err, tc.wantErr, entry) + assert.NotErrorIs(t, got.err, biz.ErrAPITokenClaimsMismatch, entry) + continue + } + + require.NoError(t, got.err, entry) + require.NotNil(t, got.token, entry) + require.NotNil(t, got.token.Scope, entry) + assert.Equal(t, tc.wantScope, *got.token.Scope, entry) + assert.Equal(t, tc.wantScopeID, got.token.ScopeID, entry) + if tc.wantOrg == nil { + assert.Nil(t, got.org, entry) + } else { + require.NotNil(t, got.org, entry) + assert.Equal(t, tc.wantOrg.String(), got.org.ID, entry) + } + if tc.wantReachAll { + assert.Nil(t, got.token.ReachableProjects(), entry) + assert.True(t, got.token.ReachesProject(other.ID), entry) + } else { + assert.Equal(t, tc.wantReach, got.token.ReachableProjects(), entry) + assert.False(t, got.token.ReachesProject(other.ID), entry) + } + } + }) + } +} + +// A row that disagrees with the claims its token was signed with refuses the token, at both entry +// points, as a security event. Whatever wrote the row, it cannot widen the token, move it to +// another resource or organization, or make it an instance token. +func TestAPITokenMiddlewareRefusesARowThatDisagreesWithItsClaims(t *testing.T) { + if !testhelpers.IntegrationTestsEnabled() { + t.Skip() + } + + tu := testhelpers.NewTestingUseCases(t) + defer tu.DB.Close(t) + ctx := context.Background() + + org, err := tu.Organization.CreateWithRandomName(ctx) + require.NoError(t, err) + otherOrg, err := tu.Organization.CreateWithRandomName(ctx) + require.NoError(t, err) + orgID, otherOrgID := uuid.MustParse(org.ID), uuid.MustParse(otherOrg.ID) + project, err := tu.Project.Create(ctx, org.ID, "signed") + require.NoError(t, err) + other, err := tu.Project.Create(ctx, org.ID, "other") + require.NoError(t, err) + product, otherProduct := uuid.New(), uuid.New() + + orgToken := []biz.APITokenCreateOpt{} + projectToken := []biz.APITokenCreateOpt{biz.APITokenWithProject(project)} + productToken := []biz.APITokenCreateOpt{biz.APITokenWithScope(authz.ResourceTypeProduct, &product), biz.APITokenWithProjectIDs([]uuid.UUID{project.ID})} + + testCases := []struct { + name string + opts []biz.APITokenCreateOpt + // alter rewrites the row after the token was minted, nil leaves it as minted + alter func(*ent.APITokenUpdateOne) *ent.APITokenUpdateOne + header string + wantErr string + }{ + {name: "an organization token as minted", opts: orgToken}, + {name: "a project token as minted", opts: projectToken}, + {name: "a product token as minted", opts: productToken}, + {name: "a project token widened to its organization", opts: projectToken, wantErr: errScopeMismatch, + alter: func(u *ent.APITokenUpdateOne) *ent.APITokenUpdateOne { + return u.SetScope(authz.ResourceTypeOrganization).SetScopeID(orgID) + }}, + {name: "an organization token made an instance token", opts: orgToken, header: otherOrg.Name, wantErr: errScopeMismatch, + alter: func(u *ent.APITokenUpdateOne) *ent.APITokenUpdateOne { + return u.SetScope(authz.ResourceTypeInstance).ClearScopeID() + }}, + {name: "a project token moved to another project", opts: projectToken, wantErr: errScopeMismatch, + alter: func(u *ent.APITokenUpdateOne) *ent.APITokenUpdateOne { return u.SetScopeID(other.ID) }}, + {name: "an organization token moved to another organization", opts: orgToken, wantErr: errScopeMismatch, + alter: func(u *ent.APITokenUpdateOne) *ent.APITokenUpdateOne { + return u.SetOrganizationID(otherOrgID).SetScopeID(otherOrgID) + }}, + {name: "a product token moved to another product", opts: productToken, wantErr: errScopeMismatch, + alter: func(u *ent.APITokenUpdateOne) *ent.APITokenUpdateOne { return u.SetScopeID(otherProduct) }}, + {name: "a product token moved to another organization", opts: productToken, wantErr: "organization mismatch", + alter: func(u *ent.APITokenUpdateOne) *ent.APITokenUpdateOne { return u.SetOrganizationID(otherOrgID) }}, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + token, err := tu.APIToken.Create(ctx, "signed-"+uuid.NewString()[:8], nil, nil, &org.ID, tc.opts...) + require.NoError(t, err) + if tc.alter != nil { + require.NoError(t, tc.alter(tu.Data.DB.APIToken.UpdateOneID(token.ID)).Exec(ctx)) + } + + for entry, got := range authenticateAtBothEntryPoints(t, tu, token.JWT, tc.header) { + if tc.wantErr != "" { + assert.ErrorContains(t, got.err, tc.wantErr, entry) + assert.ErrorIs(t, got.err, biz.ErrAPITokenClaimsMismatch, entry) + assert.Nil(t, got.token, entry) + continue + } + + require.NoError(t, got.err, entry) + require.NotNil(t, got.token, entry) + assert.Equal(t, token.Scope, got.token.Scope, entry) + assert.Equal(t, token.ScopeID, got.token.ScopeID, entry) + } + }) + } +} diff --git a/app/controlplane/internal/usercontext/apitoken_middleware_test.go b/app/controlplane/internal/usercontext/apitoken_middleware_test.go index 946dfff9d..0983e2d24 100644 --- a/app/controlplane/internal/usercontext/apitoken_middleware_test.go +++ b/app/controlplane/internal/usercontext/apitoken_middleware_test.go @@ -39,6 +39,20 @@ import ( "github.com/stretchr/testify/require" ) +// Claim names and error substrings the tests of both entry points share. +const ( + claimAud = "aud" + claimJTI = "jti" + claimOrgID = "org_id" + claimProjectID = "project_id" + claimScopeType = "scope_type" + errScopeMismatch = "scope mismatch" + errRecordsNoScope = "records no scope" + entryAPI = "API" + entryAttestation = "attestation" + claimOrgName = "org_name" +) + // authorizationHeader carries the bearer token the attestation entry point reads. const authorizationHeader = "Authorization" @@ -53,6 +67,8 @@ type middlewareTestCase struct { workflowIDClaim string // tokenWorkflowID, if set, is the workflow_id stored on the DB row tokenWorkflowID *uuid.UUID + // extraClaims are added to the JWT claims + extraClaims jwt.MapClaims // the middleware logic got skipped skipped bool wantErr bool @@ -63,6 +79,7 @@ func TestWithCurrentAPITokenAndOrgMiddleware(t *testing.T) { logger := log.NewHelper(log.NewStdLogger(io.Discard)) matchingWorkflowID := uuid.New() otherWorkflowID := uuid.New() + projectID := uuid.New() testCases := []middlewareTestCase{ { name: "invalid audience", // in this case it gets ignored @@ -134,12 +151,29 @@ func TestWithCurrentAPITokenAndOrgMiddleware(t *testing.T) { wantErr: true, wantErrContains: "workflow mismatch", }, + { + name: "scope claims disagree with the DB row", + receivedToken: true, + audience: apitoken.Audience, + tokenExists: true, + extraClaims: jwt.MapClaims{claimScopeType: "product", "scope_id": uuid.NewString()}, + wantErr: true, + wantErrContains: errScopeMismatch, + }, + { + name: "a claim of the wrong type is refused", + receivedToken: true, + audience: apitoken.Audience, + extraClaims: jwt.MapClaims{claimProjectID: 42}, + wantErr: true, + wantErrContains: "mapping the API-token claims", + }, } for _, tc := range testCases { wantOrgID := uuid.New() wantOrg := &biz.Organization{ID: wantOrgID.String()} - wantToken := &biz.APIToken{ID: uuid.New(), OrganizationID: wantOrgID} + wantToken := &biz.APIToken{ID: uuid.New(), OrganizationID: wantOrgID, Scope: biz.ToPtr(authz.ResourceTypeOrganization), ScopeID: &wantOrgID} t.Run(tc.name, func(t *testing.T) { apiTokenRepo := mocks.NewAPITokenRepo(t) @@ -152,11 +186,20 @@ func TestWithCurrentAPITokenAndOrgMiddleware(t *testing.T) { ctx := context.Background() if tc.receivedToken { c := jwt.MapClaims{ - "aud": tc.audience, - "jti": wantToken.ID.String(), + claimAud: tc.audience, + claimJTI: wantToken.ID.String(), + claimOrgID: wantOrgID.String(), } if tc.workflowIDClaim != "" { + // A workflow claim always comes with its project c["workflow_id"] = tc.workflowIDClaim + c[claimProjectID] = projectID.String() + wantToken.ProjectID = &projectID + wantToken.Scope = biz.ToPtr(authz.ResourceTypeProject) + wantToken.ScopeID = &projectID + } + for k, v := range tc.extraClaims { + c[k] = v } ctx = jwtmiddleware.NewContext(ctx, c) @@ -214,43 +257,50 @@ func toTimePtr(t time.Time) *time.Time { return &t } -// The resource scope must reach the service layer from the database row, never from a claim: -// the row is the authorization input, the claim is only ever a cross-check. +// The scope reaches the service layer from the database row, once the row has confirmed the +// claims the token was signed with. func TestWithCurrentAPITokenAndOrgMiddlewareCarriesScope(t *testing.T) { logger := log.NewHelper(log.NewStdLogger(io.Discard)) productID := uuid.New() projectA := uuid.New() + orgID := uuid.New() testCases := []struct { name string // scope as stored on the token row + rowOrg uuid.UUID rowScope *authz.ResourceType rowScopeID *uuid.UUID rowProjectIDs []uuid.UUID - // instanceAdminClaim signs the JWT with the instance-admin "scope" claim - instanceAdminClaim bool + // claims the token was signed with, besides aud and jti + claims jwt.MapClaims }{ { name: "a product-scoped token", + rowOrg: orgID, rowScope: biz.ToPtr(authz.ResourceTypeProduct), rowScopeID: &productID, rowProjectIDs: []uuid.UUID{projectA}, + claims: jwt.MapClaims{claimOrgID: orgID.String(), claimScopeType: "product", "scope_id": productID.String()}, }, { - name: "an unscoped token carries no scope", + name: "an organization token with legacy claims", + rowOrg: orgID, + rowScope: biz.ToPtr(authz.ResourceTypeOrganization), + rowScopeID: &orgID, + claims: jwt.MapClaims{claimOrgID: orgID.String()}, }, { - // The instance-admin claim never becomes the token's resource scope. - name: "an instance-admin claim carries no scope", - instanceAdminClaim: true, + name: "an instance token", + rowScope: biz.ToPtr(authz.ResourceTypeInstance), + claims: jwt.MapClaims{claimOrgID: "", "scope": authz.ScopeInstanceAdmin, claimScopeType: "instance"}, }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - orgID := uuid.New() token := &biz.APIToken{ - ID: uuid.New(), Name: "ci", OrganizationID: orgID, + ID: uuid.New(), Name: "ci", OrganizationID: tc.rowOrg, Scope: tc.rowScope, ScopeID: tc.rowScopeID, ProjectIDs: tc.rowProjectIDs, } @@ -263,9 +313,9 @@ func TestWithCurrentAPITokenAndOrgMiddlewareCarriesScope(t *testing.T) { require.NoError(t, err) orgUC := biz.NewOrganizationUseCase(orgRepo, nil, nil, nil, nil, nil, nil) - claims := jwt.MapClaims{"aud": apitoken.Audience, "jti": token.ID.String()} - if tc.instanceAdminClaim { - claims["scope"] = authz.ScopeInstanceAdmin + claims := jwt.MapClaims{claimAud: apitoken.Audience, claimJTI: token.ID.String()} + for k, v := range tc.claims { + claims[k] = v } ctx := jwtmiddleware.NewContext(context.Background(), claims) @@ -292,9 +342,9 @@ type preProductClaimRemoval struct { ProductID string `json:"product_id,omitempty"` } -// A JWT minted before the product_id claim was dropped may still carry one. The row decides what -// a token is confined to, so both entry points accept such a JWT and ignore the claim, whatever -// product it names. +// A JWT minted before the product_id claim was dropped may still carry one. Both entry points +// ignore it, whatever product it names: the signed scope claims, confirmed by the row, decide +// what the token is confined to. func TestAPITokenMiddlewaresIgnoreAProductClaim(t *testing.T) { const signingKey = "test" logger := log.NewHelper(log.NewStdLogger(io.Discard)) @@ -306,18 +356,22 @@ func TestAPITokenMiddlewaresIgnoreAProductClaim(t *testing.T) { // rowScope and rowScopeID are the scope stored on the token row rowScope *authz.ResourceType rowScopeID *uuid.UUID + // signScope signs the row's scope into the scope claims + signScope bool // productClaim is the product_id claim the JWT carries productClaim uuid.UUID + wantErr string }{ - {name: "the claim names the row's product", rowScope: &product, rowScopeID: &rowProduct, productClaim: rowProduct}, - {name: "the claim names another product", rowScope: &product, rowScopeID: &rowProduct, productClaim: otherProduct}, + {name: "the claim names the row's product", rowScope: &product, rowScopeID: &rowProduct, signScope: true, productClaim: rowProduct}, + {name: "the claim names another product", rowScope: &product, rowScopeID: &rowProduct, signScope: true, productClaim: otherProduct}, {name: "the claim is on an organization-scoped row", rowScope: &organization, rowScopeID: &orgID, productClaim: orgID}, - {name: "the claim is on a row that records no scope", productClaim: otherProduct}, + {name: "a product token minted before the scope claims", rowScope: &product, rowScopeID: &rowProduct, productClaim: rowProduct, wantErr: "create a new one"}, + {name: "the claim is on a row that records no scope", productClaim: otherProduct, wantErr: errRecordsNoScope}, } type entryPoint func(apiTokenUC *biz.APITokenUseCase, orgUC *biz.OrganizationUseCase, signed string, handler middleware.Handler) error entryPoints := map[string]entryPoint{ - "API": func(apiTokenUC *biz.APITokenUseCase, orgUC *biz.OrganizationUseCase, signed string, handler middleware.Handler) error { + entryAPI: func(apiTokenUC *biz.APITokenUseCase, orgUC *biz.OrganizationUseCase, signed string, handler middleware.Handler) error { claims := jwt.MapClaims{} if _, _, err := jwt.NewParser().ParseUnverified(signed, claims); err != nil { return err @@ -326,7 +380,7 @@ func TestAPITokenMiddlewaresIgnoreAProductClaim(t *testing.T) { _, err := WithCurrentAPITokenAndOrgMiddleware(apiTokenUC, orgUC, logger)(handler)(jwtmiddleware.NewContext(context.Background(), claims), nil) return err }, - "attestation": func(apiTokenUC *biz.APITokenUseCase, orgUC *biz.OrganizationUseCase, signed string, handler middleware.Handler) error { + entryAttestation: func(apiTokenUC *biz.APITokenUseCase, orgUC *biz.OrganizationUseCase, signed string, handler middleware.Handler) error { ctx := transport.NewServerContext(context.Background(), &fakeTransport{header: headerCarrier{authorizationHeader: {"Bearer " + signed}}}) _, err := middleware.Chain( attjwtmiddleware.WithJWTMulti(log.NewStdLogger(io.Discard), attjwtmiddleware.NewAPITokenProvider(signingKey)), @@ -344,18 +398,23 @@ func TestAPITokenMiddlewaresIgnoreAProductClaim(t *testing.T) { apiTokenRepo := mocks.NewAPITokenRepo(t) apiTokenRepo.On("FindByID", mock.Anything, token.ID).Return(token, nil) orgRepo := mocks.NewOrganizationRepo(t) - orgRepo.On("FindByID", mock.Anything, orgID).Return(&biz.Organization{ID: orgID.String()}, nil) + orgRepo.On("FindByID", mock.Anything, orgID).Maybe().Return(&biz.Organization{ID: orgID.String()}, nil) apiTokenUC, err := biz.NewAPITokenUseCase(apiTokenRepo, &biz.APITokenJWTConfig{SymmetricHmacKey: signingKey}, nil, nil, nil, nil) require.NoError(t, err) orgUC := biz.NewOrganizationUseCase(orgRepo, nil, nil, nil, nil, nil, nil) + jwtClaims := apitoken.CustomClaims{ + OrgID: orgID.String(), OrgName: "acme", KeyName: token.Name, + RegisteredClaims: jwt.RegisteredClaims{ID: token.ID.String(), Issuer: "test", Audience: jwt.ClaimStrings{apitoken.Audience}}, + } + if tc.signScope { + jwtClaims.ScopeType, jwtClaims.ScopeID = string(*tc.rowScope), tc.rowScopeID.String() + } + signed, err := jwt.NewWithClaims(apitoken.SigningMethod, preProductClaimRemoval{ - CustomClaims: apitoken.CustomClaims{ - OrgID: orgID.String(), OrgName: "acme", KeyName: token.Name, - RegisteredClaims: jwt.RegisteredClaims{ID: token.ID.String(), Issuer: "test", Audience: jwt.ClaimStrings{apitoken.Audience}}, - }, - ProductID: tc.productClaim.String(), + CustomClaims: jwtClaims, + ProductID: tc.productClaim.String(), }).SignedString([]byte(signingKey)) require.NoError(t, err) @@ -364,6 +423,10 @@ func TestAPITokenMiddlewaresIgnoreAProductClaim(t *testing.T) { got = entities.CurrentAPIToken(ctx) return nil, nil }) + if tc.wantErr != "" { + require.ErrorContains(t, err, tc.wantErr) + return + } require.NoError(t, err) require.NotNil(t, got) @@ -375,7 +438,7 @@ func TestAPITokenMiddlewaresIgnoreAProductClaim(t *testing.T) { } // A token's row decides whether it takes the instance-admin path, which reads the organization -// from the request header rather than from the token row; the JWT "scope" claim is ignored. Both +// from the request header rather than from the token row; the JWT's claims must name the same scope. Both // entry points agree. func TestAPITokenMiddlewaresResolveInstanceAdminTokens(t *testing.T) { const ( @@ -395,18 +458,19 @@ func TestAPITokenMiddlewaresResolveInstanceAdminTokens(t *testing.T) { // header is the organization named in the request header header string wantOrg *biz.Organization + wantErr string }{ {name: "an instance-admin token takes the organization in the header", scopeClaim: authz.ScopeInstanceAdmin, header: headerOrg.Name, wantOrg: headerOrg}, {name: "an instance-admin token without the header has no organization", scopeClaim: authz.ScopeInstanceAdmin}, {name: "an organization token takes its row's organization", rowOrg: rowOrg, header: headerOrg.Name, wantOrg: rowOrg}, - // The row decides, not the claim. - {name: "an instance token without the claim is instance-admin", header: headerOrg.Name, wantOrg: headerOrg}, - {name: "an organization token carrying the claim takes its row's organization", scopeClaim: authz.ScopeInstanceAdmin, rowOrg: rowOrg, header: headerOrg.Name, wantOrg: rowOrg}, + // The signed claims and the row must agree, and the claims must agree with themselves. + {name: "an instance row whose JWT names no scope is refused", header: headerOrg.Name, wantErr: "scope claims"}, + {name: "an organization row whose JWT carries the instance-admin claim is refused", scopeClaim: authz.ScopeInstanceAdmin, rowOrg: rowOrg, header: headerOrg.Name, wantErr: "scope claims"}, } type entryPoint func(apiTokenUC *biz.APITokenUseCase, orgUC *biz.OrganizationUseCase, signed, header string, handler middleware.Handler) error entryPoints := map[string]entryPoint{ - "API": func(apiTokenUC *biz.APITokenUseCase, orgUC *biz.OrganizationUseCase, signed, header string, handler middleware.Handler) error { + entryAPI: func(apiTokenUC *biz.APITokenUseCase, orgUC *biz.OrganizationUseCase, signed, header string, handler middleware.Handler) error { claims := jwt.MapClaims{} if _, _, err := jwt.NewParser().ParseUnverified(signed, claims); err != nil { return err @@ -416,7 +480,7 @@ func TestAPITokenMiddlewaresResolveInstanceAdminTokens(t *testing.T) { _, err := WithCurrentAPITokenAndOrgMiddleware(apiTokenUC, orgUC, logger)(handler)(jwtmiddleware.NewContext(ctx, claims), nil) return err }, - "attestation": func(apiTokenUC *biz.APITokenUseCase, orgUC *biz.OrganizationUseCase, signed, header string, handler middleware.Handler) error { + entryAttestation: func(apiTokenUC *biz.APITokenUseCase, orgUC *biz.OrganizationUseCase, signed, header string, handler middleware.Handler) error { ctx := transport.NewServerContext(context.Background(), &fakeTransport{header: headerCarrier{ authorizationHeader: {"Bearer " + signed}, orgHeader: {header}, @@ -446,7 +510,7 @@ func TestAPITokenMiddlewaresResolveInstanceAdminTokens(t *testing.T) { token.OrganizationID = uuid.MustParse(tc.rowOrg.ID) rowOrgID = &token.OrganizationID jwtClaims.OrgID, jwtClaims.OrgName = tc.rowOrg.ID, tc.rowOrg.Name - orgRepo.On("FindByID", mock.Anything, token.OrganizationID).Return(tc.rowOrg, nil) + orgRepo.On("FindByID", mock.Anything, token.OrganizationID).Maybe().Return(tc.rowOrg, nil) } // The row records its scope: its organization's, or the instance's when it has none if rowOrgID != nil { @@ -473,6 +537,10 @@ func TestAPITokenMiddlewaresResolveInstanceAdminTokens(t *testing.T) { gotOrg, gotToken, gotSubject = entities.CurrentOrg(ctx), entities.CurrentAPIToken(ctx), CurrentAuthzSubject(ctx) return nil, nil }) + if tc.wantErr != "" { + require.ErrorContains(t, err, tc.wantErr) + return + } require.NoError(t, err) require.NotNil(t, gotToken) From 0493b9332943d9f9e6e0bce5d91b54050d299874 Mon Sep 17 00:00:00 2001 From: Javier Rodriguez Date: Mon, 5 Oct 2026 17:10:09 +0200 Subject: [PATCH 07/17] fix(controlplane): guard API token revocation and log claim mismatches Assisted-by: Claude Code Signed-off-by: Javier Rodriguez Chainloop-Trace-Sessions: 796bcbff-1898-452d-aaeb-b5311bffdfa6 --- .../usercontext/apitoken_middleware.go | 11 +++- .../apitoken_middleware_integration_test.go | 8 +++ .../usercontext/apitoken_middleware_test.go | 62 +++++++++++++++---- app/controlplane/pkg/biz/apitoken.go | 13 +++- .../pkg/biz/apitoken_verify_claims_test.go | 6 ++ 5 files changed, 85 insertions(+), 15 deletions(-) diff --git a/app/controlplane/internal/usercontext/apitoken_middleware.go b/app/controlplane/internal/usercontext/apitoken_middleware.go index 2f096bf6d..5de1a2b41 100644 --- a/app/controlplane/internal/usercontext/apitoken_middleware.go +++ b/app/controlplane/internal/usercontext/apitoken_middleware.go @@ -72,7 +72,16 @@ func WithCurrentAPITokenAndOrgMiddleware(apiTokenUC *biz.APITokenUseCase, orgUC // We've received an API-token if claimsHaveAudience(genericClaims, apitoken.Audience) { claims, err := apitoken.ClaimsFromMap(genericClaims) - if err != nil || claims.ID == "" { + if err != nil { + // A claim of the wrong type is not something this control plane signs. The + // raw token and the claims map are never logged. + id, _ := genericClaims["jti"].(string) + logger.Errorw("msg", "[authN] API token claims do not decode", "id", id, "error", err) + + return nil, errors.New("error mapping the API-token claims") + } + + if claims.ID == "" { return nil, errors.New("error mapping the API-token claims") } diff --git a/app/controlplane/internal/usercontext/apitoken_middleware_integration_test.go b/app/controlplane/internal/usercontext/apitoken_middleware_integration_test.go index e9623c4e3..cb20bf5e3 100644 --- a/app/controlplane/internal/usercontext/apitoken_middleware_integration_test.go +++ b/app/controlplane/internal/usercontext/apitoken_middleware_integration_test.go @@ -282,6 +282,14 @@ func TestAPITokenMiddlewareAcceptsTokensMintedBeforeTheScopeClaims(t *testing.T) claims: orgClaims, wantErr: "create a new one", }, + { + name: "revoked organization token", + row: func(c *ent.APITokenCreate) *ent.APITokenCreate { + return c.SetOrganizationID(orgID).SetRevokedAt(time.Now()) + }, + claims: orgClaims, + wantErr: "revoked", + }, { name: "organization row written with no scope after the backfill", row: func(c *ent.APITokenCreate) *ent.APITokenCreate { return c.SetOrganizationID(orgID) }, diff --git a/app/controlplane/internal/usercontext/apitoken_middleware_test.go b/app/controlplane/internal/usercontext/apitoken_middleware_test.go index 0983e2d24..b430b08e3 100644 --- a/app/controlplane/internal/usercontext/apitoken_middleware_test.go +++ b/app/controlplane/internal/usercontext/apitoken_middleware_test.go @@ -16,6 +16,7 @@ package usercontext import ( + "bytes" "context" "fmt" "io" @@ -51,6 +52,7 @@ const ( entryAPI = "API" entryAttestation = "attestation" claimOrgName = "org_name" + claimScopeID = "scope_id" ) // authorizationHeader carries the bearer token the attestation entry point reads. @@ -96,12 +98,13 @@ func TestWithCurrentAPITokenAndOrgMiddleware(t *testing.T) { wantErr: false, }, { - name: "token revoked", - receivedToken: true, - audience: apitoken.Audience, - tokenExists: true, - tokenRevoked: true, - wantErr: true, + name: "token revoked", + receivedToken: true, + audience: apitoken.Audience, + tokenExists: true, + tokenRevoked: true, + wantErr: true, + wantErrContains: "revoked", }, { name: "token does not exist", @@ -111,11 +114,12 @@ func TestWithCurrentAPITokenAndOrgMiddleware(t *testing.T) { wantErr: true, }, { - name: "org does not exist", - receivedToken: true, - audience: apitoken.Audience, - tokenExists: true, - wantErr: true, + name: "org does not exist", + receivedToken: true, + audience: apitoken.Audience, + tokenExists: true, + wantErr: true, + wantErrContains: "organization not found", }, { name: "no token received", @@ -156,7 +160,7 @@ func TestWithCurrentAPITokenAndOrgMiddleware(t *testing.T) { receivedToken: true, audience: apitoken.Audience, tokenExists: true, - extraClaims: jwt.MapClaims{claimScopeType: "product", "scope_id": uuid.NewString()}, + extraClaims: jwt.MapClaims{claimScopeType: "product", claimScopeID: uuid.NewString()}, wantErr: true, wantErrContains: errScopeMismatch, }, @@ -281,7 +285,7 @@ func TestWithCurrentAPITokenAndOrgMiddlewareCarriesScope(t *testing.T) { rowScope: biz.ToPtr(authz.ResourceTypeProduct), rowScopeID: &productID, rowProjectIDs: []uuid.UUID{projectA}, - claims: jwt.MapClaims{claimOrgID: orgID.String(), claimScopeType: "product", "scope_id": productID.String()}, + claims: jwt.MapClaims{claimOrgID: orgID.String(), claimScopeType: "product", claimScopeID: productID.String()}, }, { name: "an organization token with legacy claims", @@ -335,6 +339,38 @@ func TestWithCurrentAPITokenAndOrgMiddlewareCarriesScope(t *testing.T) { } } +// A row that disagrees with its signed claims is logged as a security event, with the token id +// and never the raw token. +func TestWithCurrentAPITokenAndOrgMiddlewareLogsAClaimsMismatch(t *testing.T) { + const signedToken = "raw.signed.token" + orgID := uuid.New() + token := &biz.APIToken{ID: uuid.New(), OrganizationID: orgID, Scope: biz.ToPtr(authz.ResourceTypeOrganization), ScopeID: &orgID} + + apiTokenRepo := mocks.NewAPITokenRepo(t) + apiTokenRepo.On("FindByID", mock.Anything, token.ID).Return(token, nil) + orgRepo := mocks.NewOrganizationRepo(t) + apiTokenUC, err := biz.NewAPITokenUseCase(apiTokenRepo, &biz.APITokenJWTConfig{SymmetricHmacKey: "test"}, nil, nil, nil, nil) + require.NoError(t, err) + orgUC := biz.NewOrganizationUseCase(orgRepo, nil, nil, nil, nil, nil, nil) + + var buf bytes.Buffer + logger := log.NewHelper(log.NewStdLogger(&buf)) + + claims := jwt.MapClaims{ + claimAud: apitoken.Audience, claimJTI: token.ID.String(), claimOrgID: orgID.String(), + claimScopeType: string(authz.ResourceTypeProduct), claimScopeID: uuid.NewString(), + "raw": signedToken, + } + + _, err = WithCurrentAPITokenAndOrgMiddleware(apiTokenUC, orgUC, logger)( + func(context.Context, interface{}) (interface{}, error) { return nil, nil })(jwtmiddleware.NewContext(context.Background(), claims), nil) + + require.Error(t, err) + assert.Contains(t, buf.String(), "disagrees with its signed claims") + assert.Contains(t, buf.String(), token.ID.String()) + assert.NotContains(t, buf.String(), signedToken) +} + // preProductClaimRemoval are the claims a product token was signed with while the control plane // still minted a product_id claim. type preProductClaimRemoval struct { diff --git a/app/controlplane/pkg/biz/apitoken.go b/app/controlplane/pkg/biz/apitoken.go index 11c0bcd61..3c5b36a69 100644 --- a/app/controlplane/pkg/biz/apitoken.go +++ b/app/controlplane/pkg/biz/apitoken.go @@ -171,13 +171,24 @@ func (t *APIToken) IsOrgScoped() bool { // what the token was granted: its scope and its organization, and its project and workflow when // they name one. The row may only confirm them, so a row that disagrees refuses the token // instead of widening or moving it. A product token minted before the scope claims existed is -// refused: its claims name only its organization. +// refused: its claims name only its organization. A row recording no scope under claims that +// sign one has had its scope cleared, which is a mismatch. func (t *APIToken) VerifyClaims(claims *apitoken.CustomClaims) error { + if t == nil { + return errors.New("API token not found") + } + if claims == nil { return errors.New("API token has no claims") } if t.Scope == nil { + // Claims naming a scope mean the row was minted with one: it was cleared afterwards. Legacy + // claims name none, and a row that predates the scope columns records none. + if claims.ScopeType != "" { + return fmt.Errorf("API token records no scope: %w", ErrAPITokenClaimsMismatch) + } + return errors.New("API token records no scope") } diff --git a/app/controlplane/pkg/biz/apitoken_verify_claims_test.go b/app/controlplane/pkg/biz/apitoken_verify_claims_test.go index 001d42166..e19d11746 100644 --- a/app/controlplane/pkg/biz/apitoken_verify_claims_test.go +++ b/app/controlplane/pkg/biz/apitoken_verify_claims_test.go @@ -72,6 +72,7 @@ func TestAPITokenVerifyClaims(t *testing.T) { {name: "legacy instance token", row: instanceRow, claims: apitoken.CustomClaims{Scope: authz.ScopeInstanceAdmin}}, {name: "a row recording no scope", row: &APIToken{OrganizationID: org}, claims: legacyOrg, wantErr: "records no scope"}, + {name: "a signed token whose row's scope was cleared", row: &APIToken{OrganizationID: org}, claims: signedOrg, wantErr: "records no scope", mismatch: true}, {name: "a product token minted before the scope claims", row: productRow, claims: legacyOrg, wantErr: "create a new one"}, {name: "malformed scope claims", row: orgRow, claims: apitoken.CustomClaims{OrgID: org.String(), ScopeType: string(authz.ResourceTypeOrganization)}, wantErr: "scope claims", mismatch: true}, {name: "an instance-admin claim on an organization row", row: orgRow, claims: apitoken.CustomClaims{OrgID: org.String(), Scope: authz.ScopeInstanceAdmin}, wantErr: "scope claims", mismatch: true}, @@ -101,6 +102,11 @@ func TestAPITokenVerifyClaims(t *testing.T) { }) } + t.Run("no token", func(t *testing.T) { + var missing *APIToken + assert.ErrorContains(t, missing.VerifyClaims(&legacyOrg), "not found") + }) + t.Run("no claims", func(t *testing.T) { assert.ErrorContains(t, orgRow.VerifyClaims(nil), "no claims") }) From ded42fbcd81c2ce55761698e4d5fad22ff8b4606 Mon Sep 17 00:00:00 2001 From: Javier Rodriguez Date: Mon, 5 Oct 2026 17:13:39 +0200 Subject: [PATCH 08/17] docs(controlplane): say the API token scope is confirmed by its signed claims Assisted-by: Claude Code Signed-off-by: Javier Rodriguez Chainloop-Trace-Sessions: 796bcbff-1898-452d-aaeb-b5311bffdfa6 --- .../internal/usercontext/apitoken_middleware.go | 13 +++++++------ .../pkg/usercontext/entities/apitoken.go | 4 ++-- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/app/controlplane/internal/usercontext/apitoken_middleware.go b/app/controlplane/internal/usercontext/apitoken_middleware.go index 5de1a2b41..03d945112 100644 --- a/app/controlplane/internal/usercontext/apitoken_middleware.go +++ b/app/controlplane/internal/usercontext/apitoken_middleware.go @@ -235,6 +235,8 @@ func setCurrentOrgAndAPIToken(ctx context.Context, apiTokenUC *biz.APITokenUseCa ctx = entities.WithCurrentOrg(ctx, &entities.Org{Name: org.Name, ID: org.ID, CreatedAt: org.CreatedAt, Suspended: org.Suspended}) } + // Every value here is read from the row. VerifyClaims has confirmed the scope, project and + // workflow against the signed claims; the policies, project list and system flag are row-only. ctx = entities.WithCurrentAPIToken(ctx, &entities.APIToken{ ID: token.ID.String(), Name: token.Name, @@ -244,12 +246,11 @@ func setCurrentOrgAndAPIToken(ctx context.Context, apiTokenUC *biz.APITokenUseCa ProjectName: token.ProjectName, WorkflowID: token.WorkflowID, WorkflowName: token.WorkflowName, - // Every value here comes from token.*, i.e. the database row - Scope: token.Scope, - ScopeID: token.ScopeID, - ProjectIDs: token.ProjectIDs, - Policies: token.Policies, - IsSystem: token.IsSystem, + Scope: token.Scope, + ScopeID: token.ScopeID, + ProjectIDs: token.ProjectIDs, + Policies: token.Policies, + IsSystem: token.IsSystem, }) // Set the authorization subject that will be used to check the policies diff --git a/app/controlplane/pkg/usercontext/entities/apitoken.go b/app/controlplane/pkg/usercontext/entities/apitoken.go index d2fe1854d..7636ba7b8 100644 --- a/app/controlplane/pkg/usercontext/entities/apitoken.go +++ b/app/controlplane/pkg/usercontext/entities/apitoken.go @@ -36,8 +36,8 @@ type APIToken struct { WorkflowName *string // ACL policies for this token. Used for authorization checks. Policies []*authz.Policy - // Scope and ScopeID name what the token is scoped to. They are loaded from the row, never - // from a claim. Every row records them; a token recording none is confined to nothing. + // Scope and ScopeID name what the token is scoped to. They are read from the row once the row + // has been checked against the token's signed claims; a row recording none is refused. Scope *authz.ResourceType ScopeID *uuid.UUID // ProjectIDs are the projects a product token reaches, loaded from the row. From 0192d45f847ea335f9d0c988722bb63052f43e38 Mon Sep 17 00:00:00 2001 From: Javier Rodriguez Date: Mon, 5 Oct 2026 17:43:05 +0200 Subject: [PATCH 09/17] refactor(controlplane): build the attestation robot account from the verified API token Load and verify the token row once on the attestation path, add CustomClaims.HasScopeClaims, and share one entry-point harness across the middleware tests. Assisted-by: Claude Code Signed-off-by: Javier Rodriguez Chainloop-Trace-Sessions: 796bcbff-1898-452d-aaeb-b5311bffdfa6 --- .../usercontext/apitoken_middleware.go | 70 ++++------ .../apitoken_middleware_integration_test.go | 82 +++++------- .../usercontext/apitoken_middleware_test.go | 123 ++++++++---------- app/controlplane/pkg/biz/apitoken.go | 9 +- .../pkg/biz/apitoken_integration_test.go | 17 +-- .../pkg/biz/apitoken_verify_claims_test.go | 9 +- app/controlplane/pkg/jwt/apitoken/apitoken.go | 11 +- 7 files changed, 137 insertions(+), 184 deletions(-) diff --git a/app/controlplane/internal/usercontext/apitoken_middleware.go b/app/controlplane/internal/usercontext/apitoken_middleware.go index 03d945112..01e8d1c3c 100644 --- a/app/controlplane/internal/usercontext/apitoken_middleware.go +++ b/app/controlplane/internal/usercontext/apitoken_middleware.go @@ -85,13 +85,13 @@ func WithCurrentAPITokenAndOrgMiddleware(apiTokenUC *biz.APITokenUseCase, orgUC return nil, errors.New("error mapping the API-token claims") } - ctx, err = setCurrentOrgAndAPIToken(ctx, apiTokenUC, orgUC, claims, logger) + ctx, _, err = setCurrentOrgAndAPIToken(ctx, apiTokenUC, orgUC, claims, logger) if err != nil { return nil, fmt.Errorf("error setting current org and user: %w", err) } // legacy_claims counts the tokens minted before the scope claims, to plan their sunset - logger.Infow("msg", "[authN] processed credentials", "id", claims.ID, "type", "API-token", "projectID", claims.ProjectID, "legacy_claims", claims.ScopeType == "") + logger.Infow("msg", "[authN] processed credentials", "id", claims.ID, "type", "API-token", "projectID", claims.ProjectID, "legacy_claims", !claims.HasScopeClaims()) } return handler(ctx, req) @@ -130,62 +130,36 @@ func WithAttestationContextFromAPIToken(apiTokenUC *biz.APITokenUseCase, orgUC * return nil, errors.New("error mapping the API-token claims") } - ctx, err := setRobotAccountFromAPIToken(ctx, apiTokenUC, tokenID) - if err != nil { - return nil, fmt.Errorf("error extracting organization from APIToken: %w", err) - } - - ctx, err = setCurrentOrgAndAPIToken(ctx, apiTokenUC, orgUC, claims, logger) + ctx, token, err := setCurrentOrgAndAPIToken(ctx, apiTokenUC, orgUC, claims, logger) if err != nil { return nil, fmt.Errorf("error setting current org and user: %w", err) } - logger.Infow("msg", "[authN] processed credentials", "id", tokenID, "type", "API-token", "legacy_claims", claims.ScopeType == "") + // The robot account comes from the row the claims have just confirmed + ctx = WithRobotAccount(ctx, &RobotAccount{OrgID: token.OrganizationID.String(), ProviderKey: attjwtmiddleware.APITokenProviderKey}) + + logger.Infow("msg", "[authN] processed credentials", "id", tokenID, "type", "API-token", "legacy_claims", !claims.HasScopeClaims()) return handler(ctx, req) } } } -func setRobotAccountFromAPIToken(ctx context.Context, apiTokenUC *biz.APITokenUseCase, tokenID string) (context.Context, error) { - if tokenID == "" { - return nil, errors.New("error retrieving the key ID from the API token") - } - - // Check that the token exists and is not revoked - token, err := apiTokenUC.FindByID(ctx, tokenID) - if err != nil { - return nil, fmt.Errorf("error retrieving the API token: %w", err) - } else if token == nil { - return nil, errors.New("API token not found") - } - - // Note: Expiration time does not need to be checked because that's done at the JWT - // verification layer, which happens before this middleware is called - if token.RevokedAt != nil { - return nil, errors.New("API token revoked") - } - - ctx = WithRobotAccount(ctx, &RobotAccount{OrgID: token.OrganizationID.String(), ProviderKey: attjwtmiddleware.APITokenProviderKey}) - - return ctx, nil -} - -// Set the current organization and API-Token in the context. The row must agree with the claims -// the token was signed with: they bind its scope, organization, project and workflow, and the row -// only confirms them. What the row adds, its policies, its project list and its revocation, is -// read from the row. -func setCurrentOrgAndAPIToken(ctx context.Context, apiTokenUC *biz.APITokenUseCase, orgUC *biz.OrganizationUseCase, claims *apitoken.CustomClaims, logger *log.Helper) (context.Context, error) { +// Set the current organization and API-Token in the context, and return the token's row. The row +// must agree with the claims the token was signed with: they bind its scope, organization, +// project and workflow, and the row only confirms them. What the row adds, its policies, its +// project list and its revocation, is read from the row. +func setCurrentOrgAndAPIToken(ctx context.Context, apiTokenUC *biz.APITokenUseCase, orgUC *biz.OrganizationUseCase, claims *apitoken.CustomClaims, logger *log.Helper) (context.Context, *biz.APIToken, error) { if claims == nil || claims.ID == "" { - return nil, errors.New("error retrieving the key ID from the API token") + return nil, nil, errors.New("error retrieving the key ID from the API token") } // Check that the token exists and is not revoked token, err := apiTokenUC.FindByID(ctx, claims.ID) if err != nil { - return nil, fmt.Errorf("error retrieving the API token: %w", err) + return nil, nil, fmt.Errorf("error retrieving the API token: %w", err) } else if token == nil { - return nil, errors.New("API token not found") + return nil, nil, errors.New("API token not found") } if err := token.VerifyClaims(claims); err != nil { @@ -195,13 +169,13 @@ func setCurrentOrgAndAPIToken(ctx context.Context, apiTokenUC *biz.APITokenUseCa logger.Errorw("msg", "[authN] API token row disagrees with its signed claims", "id", claims.ID, "error", err) } - return nil, err + return nil, nil, err } // Note: Expiration time does not need to be checked because that's done at the JWT // verification layer, which happens before this middleware is called if token.RevokedAt != nil { - return nil, errors.New("API token revoked") + return nil, nil, errors.New("API token revoked") } // Handle instance admin tokens @@ -213,9 +187,9 @@ func setCurrentOrgAndAPIToken(ctx context.Context, apiTokenUC *biz.APITokenUseCa // Load organization from header org, err := orgUC.FindByName(ctx, orgName) if err != nil { - return nil, fmt.Errorf("error retrieving the organization: %w", err) + return nil, nil, fmt.Errorf("error retrieving the organization: %w", err) } else if org == nil { - return nil, errors.New("organization not found") + return nil, nil, errors.New("organization not found") } ctx = entities.WithCurrentOrg(ctx, &entities.Org{Name: org.Name, ID: org.ID, CreatedAt: org.CreatedAt, Suspended: org.Suspended}) @@ -226,9 +200,9 @@ func setCurrentOrgAndAPIToken(ctx context.Context, apiTokenUC *biz.APITokenUseCa } else { org, err := orgUC.FindByID(ctx, token.OrganizationID.String()) if err != nil { - return nil, fmt.Errorf("error retrieving the organization: %w", err) + return nil, nil, fmt.Errorf("error retrieving the organization: %w", err) } else if org == nil { - return nil, errors.New("organization not found") + return nil, nil, errors.New("organization not found") } // Set the current organization in the context @@ -257,7 +231,7 @@ func setCurrentOrgAndAPIToken(ctx context.Context, apiTokenUC *biz.APITokenUseCa subjectAPIToken := authz.SubjectAPIToken{ID: token.ID.String()} ctx = WithAuthzSubject(ctx, subjectAPIToken.String()) - return ctx, nil + return ctx, token, nil } func WithAPITokenUsageUpdater(apiTokenUC *biz.APITokenUseCase, logger *log.Helper) middleware.Middleware { diff --git a/app/controlplane/internal/usercontext/apitoken_middleware_integration_test.go b/app/controlplane/internal/usercontext/apitoken_middleware_integration_test.go index cb20bf5e3..3f1ea19a2 100644 --- a/app/controlplane/internal/usercontext/apitoken_middleware_integration_test.go +++ b/app/controlplane/internal/usercontext/apitoken_middleware_integration_test.go @@ -22,7 +22,6 @@ import ( "testing" "time" - "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" @@ -30,9 +29,7 @@ import ( "github.com/chainloop-dev/chainloop/app/controlplane/pkg/jwt/apitoken" "github.com/chainloop-dev/chainloop/app/controlplane/pkg/usercontext/entities" "github.com/go-kratos/kratos/v2/log" - "github.com/go-kratos/kratos/v2/middleware" jwtmiddleware "github.com/go-kratos/kratos/v2/middleware/auth/jwt" - "github.com/go-kratos/kratos/v2/transport" "github.com/golang-jwt/jwt/v5" "github.com/google/uuid" "github.com/stretchr/testify/assert" @@ -140,46 +137,30 @@ type authenticated struct { // naming orgName in the organization header. func authenticateAtBothEntryPoints(t *testing.T, tu *testhelpers.TestingUseCases, signed, orgName string) map[string]authenticated { t.Helper() - const orgHeader = "Chainloop-Organization" - logger := log.NewHelper(log.NewStdLogger(io.Discard)) - capture := func(out *authenticated) middleware.Handler { - return func(ctx context.Context, _ interface{}) (interface{}, error) { - out.token, out.org = entities.CurrentAPIToken(ctx), entities.CurrentOrg(ctx) + results := make(map[string]authenticated, len(apiTokenEntryPoints)) + for entry, run := range apiTokenEntryPoints { + var got authenticated + got.err = run(tu.APIToken, tu.Organization, signed, orgName, func(ctx context.Context, _ interface{}) (interface{}, error) { + got.token, got.org = entities.CurrentAPIToken(ctx), entities.CurrentOrg(ctx) return nil, nil - } + }) + results[entry] = got } - var api, attestation authenticated - - claims := jwt.MapClaims{} - _, _, err := jwt.NewParser().ParseUnverified(signed, claims) - require.NoError(t, err) - ctx := transport.NewServerContext(context.Background(), &fakeTransport{header: headerCarrier{orgHeader: {orgName}}}) - _, api.err = WithCurrentAPITokenAndOrgMiddleware(tu.APIToken, tu.Organization, logger)(capture(&api))(jwtmiddleware.NewContext(ctx, claims), nil) - - ctx = transport.NewServerContext(context.Background(), &fakeTransport{header: headerCarrier{ - authorizationHeader: {"Bearer " + signed}, - orgHeader: {orgName}, - }}) - _, attestation.err = middleware.Chain( - attjwtmiddleware.WithJWTMulti(log.NewStdLogger(io.Discard), attjwtmiddleware.NewAPITokenProvider("test")), - WithAttestationContextFromAPIToken(tu.APIToken, tu.Organization, logger), - )(capture(&attestation))(ctx, nil) - - return map[string]authenticated{entryAPI: api, entryAttestation: attestation} + return results } // signLegacy signs claims the way a control plane from before the scope claims did, with the // testhelpers' signing key. func signLegacy(t *testing.T, tokenID uuid.UUID, claims jwt.MapClaims) string { t.Helper() - all := jwt.MapClaims{"token_name": "legacy", claimJTI: tokenID.String(), "iss": "cp.chainloop", claimAud: []string{apitoken.Audience}} + all := jwt.MapClaims{"token_name": "legacy", claimJTI: tokenID.String(), "iss": testIssuer, claimAud: []string{apitoken.Audience}} for k, v := range claims { all[k] = v } - signed, err := jwt.NewWithClaims(apitoken.SigningMethod, all).SignedString([]byte("test")) + signed, err := jwt.NewWithClaims(apitoken.SigningMethod, all).SignedString([]byte(testSigningKey)) require.NoError(t, err) return signed @@ -218,7 +199,8 @@ func TestAPITokenMiddlewareAcceptsTokensMintedBeforeTheScopeClaims(t *testing.T) testCases := []struct { name string - // row writes the columns the control plane that minted the token wrote + // row writes the columns the control plane that minted the token wrote. nil writes none, + // as for an instance token row func(*ent.APITokenCreate) *ent.APITokenCreate claims jwt.MapClaims header string @@ -226,24 +208,24 @@ func TestAPITokenMiddlewareAcceptsTokensMintedBeforeTheScopeClaims(t *testing.T) // the scope columns does during a rolling upgrade or after a rollback afterBackfill bool - wantErr string - wantScope authz.ResourceType - wantScopeID *uuid.UUID - wantOrg *uuid.UUID - wantReachAll bool - wantReach []uuid.UUID + wantErr string + wantScope authz.ResourceType + wantScopeID *uuid.UUID + wantOrg *uuid.UUID + // wantReach is what the token is confined to, nil for every project of its organization + wantReach []uuid.UUID }{ { name: "oldest organization token, no org_name claim", row: func(c *ent.APITokenCreate) *ent.APITokenCreate { return c.SetOrganizationID(orgID) }, claims: jwt.MapClaims{claimOrgID: org.ID}, - wantScope: authz.ResourceTypeOrganization, wantScopeID: &orgID, wantOrg: &orgID, wantReachAll: true, + wantScope: authz.ResourceTypeOrganization, wantScopeID: &orgID, wantOrg: &orgID, }, { name: "organization token ignores the organization header", row: func(c *ent.APITokenCreate) *ent.APITokenCreate { return c.SetOrganizationID(orgID) }, claims: orgClaims, header: headerOrg.Name, - wantScope: authz.ResourceTypeOrganization, wantScopeID: &orgID, wantOrg: &orgID, wantReachAll: true, + wantScope: authz.ResourceTypeOrganization, wantScopeID: &orgID, wantOrg: &orgID, }, { name: "project token", @@ -264,15 +246,13 @@ func TestAPITokenMiddlewareAcceptsTokensMintedBeforeTheScopeClaims(t *testing.T) }, { name: "instance token takes the organization header", - row: func(c *ent.APITokenCreate) *ent.APITokenCreate { return c }, claims: instanceClaims, header: headerOrg.Name, - wantScope: authz.ResourceTypeInstance, wantOrg: &headerOrgID, wantReachAll: true, + wantScope: authz.ResourceTypeInstance, wantOrg: &headerOrgID, }, { name: "instance token without the header has no organization", - row: func(c *ent.APITokenCreate) *ent.APITokenCreate { return c }, claims: instanceClaims, - wantScope: authz.ResourceTypeInstance, wantReachAll: true, + wantScope: authz.ResourceTypeInstance, }, { name: "product token minted before the scope claims", @@ -299,23 +279,27 @@ func TestAPITokenMiddlewareAcceptsTokensMintedBeforeTheScopeClaims(t *testing.T) }, { name: "instance row written with no scope after the backfill", - row: func(c *ent.APITokenCreate) *ent.APITokenCreate { return c }, claims: instanceClaims, afterBackfill: true, wantErr: errRecordsNoScope, }, } - create := func(i int) uuid.UUID { - row, err := testCases[i].row(tu.Data.DB.APIToken.Create().SetName("legacy-" + uuid.NewString())).Save(ctx) + create := func(row func(*ent.APITokenCreate) *ent.APITokenCreate) uuid.UUID { + c := tu.Data.DB.APIToken.Create().SetName("legacy-" + uuid.NewString()) + if row != nil { + c = row(c) + } + + saved, err := c.Save(ctx) require.NoError(t, err) - return row.ID + return saved.ID } ids := make([]uuid.UUID, len(testCases)) for i, tc := range testCases { if !tc.afterBackfill { - ids[i] = create(i) + ids[i] = create(tc.row) } } @@ -326,7 +310,7 @@ func TestAPITokenMiddlewareAcceptsTokensMintedBeforeTheScopeClaims(t *testing.T) for i, tc := range testCases { if tc.afterBackfill { - ids[i] = create(i) + ids[i] = create(tc.row) } } @@ -350,7 +334,7 @@ func TestAPITokenMiddlewareAcceptsTokensMintedBeforeTheScopeClaims(t *testing.T) require.NotNil(t, got.org, entry) assert.Equal(t, tc.wantOrg.String(), got.org.ID, entry) } - if tc.wantReachAll { + if tc.wantReach == nil { assert.Nil(t, got.token.ReachableProjects(), entry) assert.True(t, got.token.ReachesProject(other.ID), entry) } else { diff --git a/app/controlplane/internal/usercontext/apitoken_middleware_test.go b/app/controlplane/internal/usercontext/apitoken_middleware_test.go index b430b08e3..31d05b96a 100644 --- a/app/controlplane/internal/usercontext/apitoken_middleware_test.go +++ b/app/controlplane/internal/usercontext/apitoken_middleware_test.go @@ -40,23 +40,59 @@ import ( "github.com/stretchr/testify/require" ) -// Claim names and error substrings the tests of both entry points share. +// Claim names, error substrings and entry point names the tests of both entry points share. const ( claimAud = "aud" claimJTI = "jti" claimOrgID = "org_id" + claimOrgName = "org_name" claimProjectID = "project_id" + claimScopeID = "scope_id" claimScopeType = "scope_type" errScopeMismatch = "scope mismatch" errRecordsNoScope = "records no scope" entryAPI = "API" entryAttestation = "attestation" - claimOrgName = "org_name" - claimScopeID = "scope_id" ) -// authorizationHeader carries the bearer token the attestation entry point reads. -const authorizationHeader = "Authorization" +const ( + // authorizationHeader carries the bearer token the attestation entry point reads. + authorizationHeader = "Authorization" + // orgHeader names the organization an instance token acts in. + orgHeader = "Chainloop-Organization" + // testSigningKey signs the tokens the tests build. The testhelpers sign with it too. + testSigningKey = "test" + // testIssuer is the issuer the tests' tokens name. Nothing checks it. + testIssuer = "cp.chainloop" +) + +// apiTokenEntryPoints run a signed API token through each entry point, naming orgName in the +// organization header. handler runs only when the entry point accepts the token. +var apiTokenEntryPoints = map[string]func(apiTokenUC *biz.APITokenUseCase, orgUC *biz.OrganizationUseCase, signed, orgName string, handler middleware.Handler) error{ + entryAPI: func(apiTokenUC *biz.APITokenUseCase, orgUC *biz.OrganizationUseCase, signed, orgName string, handler middleware.Handler) error { + claims := jwt.MapClaims{} + if _, _, err := jwt.NewParser().ParseUnverified(signed, claims); err != nil { + return err + } + + ctx := transport.NewServerContext(context.Background(), &fakeTransport{header: headerCarrier{orgHeader: {orgName}}}) + logger := log.NewHelper(log.NewStdLogger(io.Discard)) + _, err := WithCurrentAPITokenAndOrgMiddleware(apiTokenUC, orgUC, logger)(handler)(jwtmiddleware.NewContext(ctx, claims), nil) + return err + }, + entryAttestation: func(apiTokenUC *biz.APITokenUseCase, orgUC *biz.OrganizationUseCase, signed, orgName string, handler middleware.Handler) error { + ctx := transport.NewServerContext(context.Background(), &fakeTransport{header: headerCarrier{ + authorizationHeader: {"Bearer " + signed}, + orgHeader: {orgName}, + }}) + logger := log.NewHelper(log.NewStdLogger(io.Discard)) + _, err := middleware.Chain( + attjwtmiddleware.WithJWTMulti(log.NewStdLogger(io.Discard), attjwtmiddleware.NewAPITokenProvider(testSigningKey)), + WithAttestationContextFromAPIToken(apiTokenUC, orgUC, logger), + )(handler)(ctx, nil) + return err + }, +} type middlewareTestCase struct { name string @@ -182,7 +218,7 @@ func TestWithCurrentAPITokenAndOrgMiddleware(t *testing.T) { t.Run(tc.name, func(t *testing.T) { apiTokenRepo := mocks.NewAPITokenRepo(t) orgRepo := mocks.NewOrganizationRepo(t) - apiTokenUC, err := biz.NewAPITokenUseCase(apiTokenRepo, &biz.APITokenJWTConfig{SymmetricHmacKey: "test"}, nil, nil, nil, nil) + apiTokenUC, err := biz.NewAPITokenUseCase(apiTokenRepo, &biz.APITokenJWTConfig{SymmetricHmacKey: testSigningKey}, nil, nil, nil, nil) require.NoError(t, err) orgUC := biz.NewOrganizationUseCase(orgRepo, nil, nil, nil, nil, nil, nil) require.NoError(t, err) @@ -313,7 +349,7 @@ func TestWithCurrentAPITokenAndOrgMiddlewareCarriesScope(t *testing.T) { orgRepo := mocks.NewOrganizationRepo(t) orgRepo.On("FindByID", mock.Anything, orgID).Maybe().Return(&biz.Organization{ID: orgID.String()}, nil) - apiTokenUC, err := biz.NewAPITokenUseCase(apiTokenRepo, &biz.APITokenJWTConfig{SymmetricHmacKey: "test"}, nil, nil, nil, nil) + apiTokenUC, err := biz.NewAPITokenUseCase(apiTokenRepo, &biz.APITokenJWTConfig{SymmetricHmacKey: testSigningKey}, nil, nil, nil, nil) require.NoError(t, err) orgUC := biz.NewOrganizationUseCase(orgRepo, nil, nil, nil, nil, nil, nil) @@ -349,7 +385,7 @@ func TestWithCurrentAPITokenAndOrgMiddlewareLogsAClaimsMismatch(t *testing.T) { apiTokenRepo := mocks.NewAPITokenRepo(t) apiTokenRepo.On("FindByID", mock.Anything, token.ID).Return(token, nil) orgRepo := mocks.NewOrganizationRepo(t) - apiTokenUC, err := biz.NewAPITokenUseCase(apiTokenRepo, &biz.APITokenJWTConfig{SymmetricHmacKey: "test"}, nil, nil, nil, nil) + apiTokenUC, err := biz.NewAPITokenUseCase(apiTokenRepo, &biz.APITokenJWTConfig{SymmetricHmacKey: testSigningKey}, nil, nil, nil, nil) require.NoError(t, err) orgUC := biz.NewOrganizationUseCase(orgRepo, nil, nil, nil, nil, nil, nil) @@ -382,8 +418,6 @@ type preProductClaimRemoval struct { // ignore it, whatever product it names: the signed scope claims, confirmed by the row, decide // what the token is confined to. func TestAPITokenMiddlewaresIgnoreAProductClaim(t *testing.T) { - const signingKey = "test" - logger := log.NewHelper(log.NewStdLogger(io.Discard)) rowProduct, otherProduct, orgID := uuid.New(), uuid.New(), uuid.New() product, organization := authz.ResourceTypeProduct, authz.ResourceTypeOrganization @@ -405,29 +439,8 @@ func TestAPITokenMiddlewaresIgnoreAProductClaim(t *testing.T) { {name: "the claim is on a row that records no scope", productClaim: otherProduct, wantErr: errRecordsNoScope}, } - type entryPoint func(apiTokenUC *biz.APITokenUseCase, orgUC *biz.OrganizationUseCase, signed string, handler middleware.Handler) error - entryPoints := map[string]entryPoint{ - entryAPI: func(apiTokenUC *biz.APITokenUseCase, orgUC *biz.OrganizationUseCase, signed string, handler middleware.Handler) error { - claims := jwt.MapClaims{} - if _, _, err := jwt.NewParser().ParseUnverified(signed, claims); err != nil { - return err - } - - _, err := WithCurrentAPITokenAndOrgMiddleware(apiTokenUC, orgUC, logger)(handler)(jwtmiddleware.NewContext(context.Background(), claims), nil) - return err - }, - entryAttestation: func(apiTokenUC *biz.APITokenUseCase, orgUC *biz.OrganizationUseCase, signed string, handler middleware.Handler) error { - ctx := transport.NewServerContext(context.Background(), &fakeTransport{header: headerCarrier{authorizationHeader: {"Bearer " + signed}}}) - _, err := middleware.Chain( - attjwtmiddleware.WithJWTMulti(log.NewStdLogger(io.Discard), attjwtmiddleware.NewAPITokenProvider(signingKey)), - WithAttestationContextFromAPIToken(apiTokenUC, orgUC, logger), - )(handler)(ctx, nil) - return err - }, - } - for _, tc := range testCases { - for entry, run := range entryPoints { + for entry, run := range apiTokenEntryPoints { t.Run(entry+"/"+tc.name, func(t *testing.T) { token := &biz.APIToken{ID: uuid.New(), Name: "ci", OrganizationID: orgID, Scope: tc.rowScope, ScopeID: tc.rowScopeID} @@ -436,13 +449,13 @@ func TestAPITokenMiddlewaresIgnoreAProductClaim(t *testing.T) { orgRepo := mocks.NewOrganizationRepo(t) orgRepo.On("FindByID", mock.Anything, orgID).Maybe().Return(&biz.Organization{ID: orgID.String()}, nil) - apiTokenUC, err := biz.NewAPITokenUseCase(apiTokenRepo, &biz.APITokenJWTConfig{SymmetricHmacKey: signingKey}, nil, nil, nil, nil) + apiTokenUC, err := biz.NewAPITokenUseCase(apiTokenRepo, &biz.APITokenJWTConfig{SymmetricHmacKey: testSigningKey}, nil, nil, nil, nil) require.NoError(t, err) orgUC := biz.NewOrganizationUseCase(orgRepo, nil, nil, nil, nil, nil, nil) jwtClaims := apitoken.CustomClaims{ OrgID: orgID.String(), OrgName: "acme", KeyName: token.Name, - RegisteredClaims: jwt.RegisteredClaims{ID: token.ID.String(), Issuer: "test", Audience: jwt.ClaimStrings{apitoken.Audience}}, + RegisteredClaims: jwt.RegisteredClaims{ID: token.ID.String(), Issuer: testIssuer, Audience: jwt.ClaimStrings{apitoken.Audience}}, } if tc.signScope { jwtClaims.ScopeType, jwtClaims.ScopeID = string(*tc.rowScope), tc.rowScopeID.String() @@ -451,11 +464,11 @@ func TestAPITokenMiddlewaresIgnoreAProductClaim(t *testing.T) { signed, err := jwt.NewWithClaims(apitoken.SigningMethod, preProductClaimRemoval{ CustomClaims: jwtClaims, ProductID: tc.productClaim.String(), - }).SignedString([]byte(signingKey)) + }).SignedString([]byte(testSigningKey)) require.NoError(t, err) var got *entities.APIToken - err = run(apiTokenUC, orgUC, signed, func(ctx context.Context, _ interface{}) (interface{}, error) { + err = run(apiTokenUC, orgUC, signed, "", func(ctx context.Context, _ interface{}) (interface{}, error) { got = entities.CurrentAPIToken(ctx) return nil, nil }) @@ -477,11 +490,6 @@ func TestAPITokenMiddlewaresIgnoreAProductClaim(t *testing.T) { // from the request header rather than from the token row; the JWT's claims must name the same scope. Both // entry points agree. func TestAPITokenMiddlewaresResolveInstanceAdminTokens(t *testing.T) { - const ( - signingKey = "test" - orgHeader = "Chainloop-Organization" - ) - logger := log.NewHelper(log.NewStdLogger(io.Discard)) rowOrg := &biz.Organization{ID: uuid.NewString(), Name: "row-org"} headerOrg := &biz.Organization{ID: uuid.NewString(), Name: "header-org"} @@ -504,39 +512,14 @@ func TestAPITokenMiddlewaresResolveInstanceAdminTokens(t *testing.T) { {name: "an organization row whose JWT carries the instance-admin claim is refused", scopeClaim: authz.ScopeInstanceAdmin, rowOrg: rowOrg, header: headerOrg.Name, wantErr: "scope claims"}, } - type entryPoint func(apiTokenUC *biz.APITokenUseCase, orgUC *biz.OrganizationUseCase, signed, header string, handler middleware.Handler) error - entryPoints := map[string]entryPoint{ - entryAPI: func(apiTokenUC *biz.APITokenUseCase, orgUC *biz.OrganizationUseCase, signed, header string, handler middleware.Handler) error { - claims := jwt.MapClaims{} - if _, _, err := jwt.NewParser().ParseUnverified(signed, claims); err != nil { - return err - } - - ctx := transport.NewServerContext(context.Background(), &fakeTransport{header: headerCarrier{orgHeader: {header}}}) - _, err := WithCurrentAPITokenAndOrgMiddleware(apiTokenUC, orgUC, logger)(handler)(jwtmiddleware.NewContext(ctx, claims), nil) - return err - }, - entryAttestation: func(apiTokenUC *biz.APITokenUseCase, orgUC *biz.OrganizationUseCase, signed, header string, handler middleware.Handler) error { - ctx := transport.NewServerContext(context.Background(), &fakeTransport{header: headerCarrier{ - authorizationHeader: {"Bearer " + signed}, - orgHeader: {header}, - }}) - _, err := middleware.Chain( - attjwtmiddleware.WithJWTMulti(log.NewStdLogger(io.Discard), attjwtmiddleware.NewAPITokenProvider(signingKey)), - WithAttestationContextFromAPIToken(apiTokenUC, orgUC, logger), - )(handler)(ctx, nil) - return err - }, - } - for _, tc := range testCases { - for entry, run := range entryPoints { + for entry, run := range apiTokenEntryPoints { t.Run(entry+"/"+tc.name, func(t *testing.T) { token := &biz.APIToken{ID: uuid.New(), Name: "ci"} jwtClaims := apitoken.CustomClaims{ KeyName: token.Name, Scope: tc.scopeClaim, - RegisteredClaims: jwt.RegisteredClaims{ID: token.ID.String(), Issuer: "test", Audience: jwt.ClaimStrings{apitoken.Audience}}, + RegisteredClaims: jwt.RegisteredClaims{ID: token.ID.String(), Issuer: testIssuer, Audience: jwt.ClaimStrings{apitoken.Audience}}, } apiTokenRepo := mocks.NewAPITokenRepo(t) @@ -559,11 +542,11 @@ func TestAPITokenMiddlewaresResolveInstanceAdminTokens(t *testing.T) { } apiTokenRepo.On("FindByID", mock.Anything, token.ID).Return(token, nil) - apiTokenUC, err := biz.NewAPITokenUseCase(apiTokenRepo, &biz.APITokenJWTConfig{SymmetricHmacKey: signingKey}, nil, nil, nil, nil) + apiTokenUC, err := biz.NewAPITokenUseCase(apiTokenRepo, &biz.APITokenJWTConfig{SymmetricHmacKey: testSigningKey}, nil, nil, nil, nil) require.NoError(t, err) orgUC := biz.NewOrganizationUseCase(orgRepo, nil, nil, nil, nil, nil, nil) - signed, err := jwt.NewWithClaims(apitoken.SigningMethod, jwtClaims).SignedString([]byte(signingKey)) + signed, err := jwt.NewWithClaims(apitoken.SigningMethod, jwtClaims).SignedString([]byte(testSigningKey)) require.NoError(t, err) var gotOrg *entities.Org diff --git a/app/controlplane/pkg/biz/apitoken.go b/app/controlplane/pkg/biz/apitoken.go index 3c5b36a69..53938353c 100644 --- a/app/controlplane/pkg/biz/apitoken.go +++ b/app/controlplane/pkg/biz/apitoken.go @@ -185,14 +185,15 @@ func (t *APIToken) VerifyClaims(claims *apitoken.CustomClaims) error { if t.Scope == nil { // Claims naming a scope mean the row was minted with one: it was cleared afterwards. Legacy // claims name none, and a row that predates the scope columns records none. - if claims.ScopeType != "" { - return fmt.Errorf("API token records no scope: %w", ErrAPITokenClaimsMismatch) + err := errors.New("API token records no scope") + if claims.HasScopeClaims() { + err = fmt.Errorf("%w: %w", err, ErrAPITokenClaimsMismatch) } - return errors.New("API token records no scope") + return err } - if claims.ScopeType == "" && t.IsProductScoped() { + if !claims.HasScopeClaims() && t.IsProductScoped() { return errors.New("API token was minted before its scope was signed, create a new one") } diff --git a/app/controlplane/pkg/biz/apitoken_integration_test.go b/app/controlplane/pkg/biz/apitoken_integration_test.go index 13c8b850f..7853e9c70 100644 --- a/app/controlplane/pkg/biz/apitoken_integration_test.go +++ b/app/controlplane/pkg/biz/apitoken_integration_test.go @@ -1261,15 +1261,19 @@ func (s *apiTokenTestSuite) TestGeneratedJWTSignsTheTokenScope() { wf, err := s.Workflow.Create(ctx, &biz.WorkflowCreateOpts{Name: randomName(), OrgID: s.org.ID, Project: s.p1.Name}) s.Require().NoError(err) - claimsOf := func(raw string) *apitoken.CustomClaims { - claims := &apitoken.CustomClaims{} - info, err := jwt.ParseWithClaims(raw, claims, func(_ *jwt.Token) (interface{}, error) { + // claimsOf verifies raw and returns its payload, as parsed and as the typed claims + claimsOf := func(raw string) (jwt.MapClaims, *apitoken.CustomClaims) { + payload := jwt.MapClaims{} + info, err := jwt.ParseWithClaims(raw, payload, func(_ *jwt.Token) (interface{}, error) { return []byte("test"), nil }) s.Require().NoError(err) s.True(info.Valid) - return claims + claims, err := apitoken.ClaimsFromMap(payload) + s.Require().NoError(err) + + return payload, claims } testCases := []struct { @@ -1298,16 +1302,13 @@ func (s *apiTokenTestSuite) TestGeneratedJWTSignsTheTokenScope() { s.Require().NoError(err) for minted, raw := range map[string]string{"created": created.JWT, "regenerated": regenerated.JWT} { - claims := claimsOf(raw) + payload, claims := claimsOf(raw) s.Equal(string(tc.wantScopeType), claims.ScopeType, minted) s.Equal(tc.wantScopeID, claims.ScopeID, minted) s.Equal(tc.wantScope, claims.Scope, minted) s.NoError(stored.VerifyClaims(claims), minted) // What changes during a token's life never goes into the JWT - payload := jwt.MapClaims{} - _, _, err := jwt.NewParser().ParseUnverified(raw, payload) - s.Require().NoError(err) s.NotContains(payload, "project_ids", minted) s.NotContains(payload, "policies", minted) s.NotContains(payload, "product_id", minted) diff --git a/app/controlplane/pkg/biz/apitoken_verify_claims_test.go b/app/controlplane/pkg/biz/apitoken_verify_claims_test.go index e19d11746..31d3bf149 100644 --- a/app/controlplane/pkg/biz/apitoken_verify_claims_test.go +++ b/app/controlplane/pkg/biz/apitoken_verify_claims_test.go @@ -51,6 +51,9 @@ func TestAPITokenVerifyClaims(t *testing.T) { signedProduct := apitoken.CustomClaims{OrgID: org.String(), ScopeType: string(authz.ResourceTypeProduct), ScopeID: product.String()} legacyOrg := apitoken.CustomClaims{OrgID: org.String()} legacyProject := apitoken.CustomClaims{OrgID: org.String(), ProjectID: project.String()} + legacyWorkflow := apitoken.CustomClaims{OrgID: org.String(), ProjectID: project.String(), WorkflowID: workflow.String()} + // widenedProjectRow is a project token's row rewritten to its whole organization + widenedProjectRow := &APIToken{OrganizationID: org, ProjectID: &project, Scope: &orgKind, ScopeID: &org} testCases := []struct { name string @@ -68,7 +71,7 @@ func TestAPITokenVerifyClaims(t *testing.T) { {name: "signed instance token", row: instanceRow, claims: apitoken.CustomClaims{Scope: authz.ScopeInstanceAdmin, ScopeType: string(authz.ResourceTypeInstance)}}, {name: "legacy organization token", row: orgRow, claims: legacyOrg}, {name: "legacy project token", row: projectRow, claims: legacyProject}, - {name: "legacy workflow-pinned token", row: workflowRow, claims: apitoken.CustomClaims{OrgID: org.String(), ProjectID: project.String(), WorkflowID: workflow.String()}}, + {name: "legacy workflow-pinned token", row: workflowRow, claims: legacyWorkflow}, {name: "legacy instance token", row: instanceRow, claims: apitoken.CustomClaims{Scope: authz.ScopeInstanceAdmin}}, {name: "a row recording no scope", row: &APIToken{OrganizationID: org}, claims: legacyOrg, wantErr: "records no scope"}, @@ -76,8 +79,8 @@ func TestAPITokenVerifyClaims(t *testing.T) { {name: "a product token minted before the scope claims", row: productRow, claims: legacyOrg, wantErr: "create a new one"}, {name: "malformed scope claims", row: orgRow, claims: apitoken.CustomClaims{OrgID: org.String(), ScopeType: string(authz.ResourceTypeOrganization)}, wantErr: "scope claims", mismatch: true}, {name: "an instance-admin claim on an organization row", row: orgRow, claims: apitoken.CustomClaims{OrgID: org.String(), Scope: authz.ScopeInstanceAdmin}, wantErr: "scope claims", mismatch: true}, - {name: "a project row widened to its organization, signed claims", row: &APIToken{OrganizationID: org, ProjectID: &project, Scope: &orgKind, ScopeID: &org}, claims: signedProject, wantErr: errScopeMismatch, mismatch: true}, - {name: "a project row widened to its organization, legacy claims", row: &APIToken{OrganizationID: org, ProjectID: &project, Scope: &orgKind, ScopeID: &org}, claims: legacyProject, wantErr: errScopeMismatch, mismatch: true}, + {name: "a project row widened to its organization, signed claims", row: widenedProjectRow, claims: signedProject, wantErr: errScopeMismatch, mismatch: true}, + {name: "a project row widened to its organization, legacy claims", row: widenedProjectRow, claims: legacyProject, wantErr: errScopeMismatch, mismatch: true}, {name: "an organization row recording no scope id", row: &APIToken{OrganizationID: org, Scope: &orgKind}, claims: signedOrg, wantErr: errScopeMismatch, mismatch: true}, {name: "an organization row made an instance row, signed claims", row: &APIToken{OrganizationID: org, Scope: &instanceKind}, claims: signedOrg, wantErr: errScopeMismatch, mismatch: true}, {name: "an organization row made an instance row, legacy claims", row: &APIToken{OrganizationID: org, Scope: &instanceKind}, claims: legacyOrg, wantErr: errScopeMismatch, mismatch: true}, diff --git a/app/controlplane/pkg/jwt/apitoken/apitoken.go b/app/controlplane/pkg/jwt/apitoken/apitoken.go index b9974e782..e6879f66e 100644 --- a/app/controlplane/pkg/jwt/apitoken/apitoken.go +++ b/app/controlplane/pkg/jwt/apitoken/apitoken.go @@ -174,6 +174,12 @@ type CustomClaims struct { jwt.RegisteredClaims } +// HasScopeClaims reports whether the token was signed with the scope_type claim. A token without +// it was minted before the scope claims existed. +func (c *CustomClaims) HasScopeClaims() bool { + return c.ScopeType != "" +} + // SignedScope returns the scope the claims bind the token to, and refuses claims that contradict // themselves. A token minted with the scope_type and scope_id claims gets those. An older token // gets the scope its other claims imply, by the rule the scope backfill migration applied to its @@ -194,7 +200,7 @@ func (c *CustomClaims) SignedScope() (authz.ResourceType, *uuid.UUID, error) { // namedScope is the scope the scope_type and scope_id claims name, else the one the claims of an // older token imply. func (c *CustomClaims) namedScope() (authz.ResourceType, *uuid.UUID, error) { - if c.ScopeType == "" { + if !c.HasScopeClaims() { return c.legacyScope() } @@ -219,7 +225,8 @@ func (c *CustomClaims) namedScope() (authz.ResourceType, *uuid.UUID, error) { } // legacyScope is the scope the claims of a token minted before the scope_type claim imply: the -// instance for the instance-admin claim, else its project, else its organization. +// instance for the instance-admin claim, else its project, else its organization. It is the rule +// biz.newTokenScope and the scope backfill migration apply to the row, so the two agree. func (c *CustomClaims) legacyScope() (authz.ResourceType, *uuid.UUID, error) { var kind authz.ResourceType var raw string From 2519b51ef0179099f9320be04cf46cd5667f4a6e Mon Sep 17 00:00:00 2001 From: Javier Rodriguez Date: Mon, 5 Oct 2026 17:47:04 +0200 Subject: [PATCH 10/17] docs(controlplane): explain the API token claim checks in plainer words Assisted-by: Claude Code Signed-off-by: Javier Rodriguez Chainloop-Trace-Sessions: 796bcbff-1898-452d-aaeb-b5311bffdfa6 --- .../usercontext/apitoken_middleware.go | 8 ++--- app/controlplane/pkg/biz/apitoken.go | 27 ++++++++++------- app/controlplane/pkg/jwt/apitoken/apitoken.go | 29 +++++++++++-------- 3 files changed, 37 insertions(+), 27 deletions(-) diff --git a/app/controlplane/internal/usercontext/apitoken_middleware.go b/app/controlplane/internal/usercontext/apitoken_middleware.go index 01e8d1c3c..efe4edb51 100644 --- a/app/controlplane/internal/usercontext/apitoken_middleware.go +++ b/app/controlplane/internal/usercontext/apitoken_middleware.go @@ -145,10 +145,10 @@ func WithAttestationContextFromAPIToken(apiTokenUC *biz.APITokenUseCase, orgUC * } } -// Set the current organization and API-Token in the context, and return the token's row. The row -// must agree with the claims the token was signed with: they bind its scope, organization, -// project and workflow, and the row only confirms them. What the row adds, its policies, its -// project list and its revocation, is read from the row. +// setCurrentOrgAndAPIToken loads the token's row, checks it against the claims the token was +// signed with, and puts the organization and the token in the context. It returns the row. +// The claims fix the token's scope, organization, project and workflow, and the row must match +// them. The policies, the product project list and revocation come from the row alone. func setCurrentOrgAndAPIToken(ctx context.Context, apiTokenUC *biz.APITokenUseCase, orgUC *biz.OrganizationUseCase, claims *apitoken.CustomClaims, logger *log.Helper) (context.Context, *biz.APIToken, error) { if claims == nil || claims.ID == "" { return nil, nil, errors.New("error retrieving the key ID from the API token") diff --git a/app/controlplane/pkg/biz/apitoken.go b/app/controlplane/pkg/biz/apitoken.go index 53938353c..de53e24b1 100644 --- a/app/controlplane/pkg/biz/apitoken.go +++ b/app/controlplane/pkg/biz/apitoken.go @@ -35,9 +35,9 @@ import ( "github.com/google/uuid" ) -// ErrAPITokenClaimsMismatch is a token whose row disagrees with the claims it was signed with, -// or whose claims are malformed. Something wrote the row wrongly, so it is a security event, not -// an expired or legacy credential. +// ErrAPITokenClaimsMismatch marks a token whose row disagrees with the claims it was signed with, +// or whose claims are malformed. It means something wrote the row wrongly, so it is a security +// event, not an expired or old credential. var ErrAPITokenClaimsMismatch = errors.New("API token claims do not match its row") var apiTokenTracer = otelx.Tracer("chainloop-controlplane", "biz/apitoken") @@ -167,12 +167,13 @@ func (t *APIToken) IsOrgScoped() bool { return t.scopeView().IsOrgScoped() } -// VerifyClaims checks the token's row against the claims its JWT was signed with. The claims bind -// what the token was granted: its scope and its organization, and its project and workflow when -// they name one. The row may only confirm them, so a row that disagrees refuses the token -// instead of widening or moving it. A product token minted before the scope claims existed is -// refused: its claims name only its organization. A row recording no scope under claims that -// sign one has had its scope cleared, which is a mismatch. +// VerifyClaims checks that the token's row matches the claims its JWT was signed with: the same +// scope and organization, and the same project and workflow when the claims name them. Any +// difference refuses the token, so a wrong row can never widen the token or move it elsewhere. +// +// A mismatch wraps ErrAPITokenClaimsMismatch. Two expected states of older tokens are refused +// without it: a row that records no scope when the claims carry none either, and a product token +// minted before the scope claims existed, which has to be created again. func (t *APIToken) VerifyClaims(claims *apitoken.CustomClaims) error { if t == nil { return errors.New("API token not found") @@ -193,6 +194,9 @@ func (t *APIToken) VerifyClaims(claims *apitoken.CustomClaims) error { return err } + // This runs before SignedScope on purpose. An older product token's claims name only its + // organization, so SignedScope would read it as an organization scope, and the comparison + // below would log it as a security event rather than ask for a new token. if !claims.HasScopeClaims() && t.IsProductScoped() { return errors.New("API token was minted before its scope was signed, create a new one") } @@ -601,8 +605,9 @@ func (uc *APITokenUseCase) Create(ctx context.Context, name string, description } // RegenerateJWT will regenerate a new JWT for the given token. Use with caution, since old JWTs are -// not invalidated. The new JWT signs the scope the row records, so the caller must know the row is -// the token it means to re-sign: a row whose shape is coherent but wrong would be signed as it is. +// not invalidated. The new JWT signs the scope the row records. A row with no scope, or with a +// scope that contradicts its other columns, is refused. A row that is consistent but wrong would +// still be signed, so callers must be sure it is the token they mean. func (uc *APITokenUseCase) RegenerateJWT(ctx context.Context, tokenID uuid.UUID, expiresIn time.Duration) (*APIToken, error) { ctx, span := otelx.Start(ctx, apiTokenTracer, "APITokenUseCase.RegenerateJWT") defer span.End() diff --git a/app/controlplane/pkg/jwt/apitoken/apitoken.go b/app/controlplane/pkg/jwt/apitoken/apitoken.go index e6879f66e..4fefb3d90 100644 --- a/app/controlplane/pkg/jwt/apitoken/apitoken.go +++ b/app/controlplane/pkg/jwt/apitoken/apitoken.go @@ -166,9 +166,11 @@ type CustomClaims struct { ProjectName string `json:"project_name,omitempty"` WorkflowID string `json:"workflow_id,omitempty"` WorkflowName string `json:"workflow_name,omitempty"` - Scope string `json:"scope,omitempty"` - // ScopeType and ScopeID bind the token to what it was granted. A token minted before they - // existed carries neither, and SignedScope derives its scope from the claims it does carry. + // Scope is the older instance-admin claim ("INSTANCE_ADMIN"), set only on instance tokens. + // Despite its name it is not the token's scope: that is ScopeType and ScopeID. + Scope string `json:"scope,omitempty"` + // ScopeType and ScopeID say what the token was granted. A token minted before they existed + // carries neither, and SignedScope works its scope out from the claims it does carry. ScopeType string `json:"scope_type,omitempty"` ScopeID string `json:"scope_id,omitempty"` jwt.RegisteredClaims @@ -180,10 +182,10 @@ func (c *CustomClaims) HasScopeClaims() bool { return c.ScopeType != "" } -// SignedScope returns the scope the claims bind the token to, and refuses claims that contradict -// themselves. A token minted with the scope_type and scope_id claims gets those. An older token -// gets the scope its other claims imply, by the rule the scope backfill migration applied to its -// row. A product token minted before the scope claims implies only its organization. +// SignedScope returns the scope the claims bind the token to. For a token minted with the +// scope_type and scope_id claims, that is what they say. For an older token, it is the scope its +// other claims imply (see legacyScope), so an older product token comes out as its organization. +// Claims that contradict themselves are an error. func (c *CustomClaims) SignedScope() (authz.ResourceType, *uuid.UUID, error) { kind, id, err := c.namedScope() if err != nil { @@ -249,11 +251,14 @@ func (c *CustomClaims) legacyScope() (authz.ResourceType, *uuid.UUID, error) { return kind, &id, nil } -// agreesWith checks that the other claims fit the scope, so that every reader of the token, the -// platform's included, reads the same scope off it. Only an instance token carries the -// instance-admin claim, and it names no organization. An organization token names its own -// organization, a project token the project it is scoped to, and a workflow comes with its -// project. +// agreesWith checks that the rest of the claims fit the scope, so that the control plane and the +// platform read the same scope from the token: +// - only an instance token carries the instance-admin claim, and it names no organization or +// project; +// - an organization token names its own organization and no project; +// - a project token names its organization and the project it is scoped to; +// - a product token names its organization and no project; +// - a workflow claim always comes with a project claim. func (c *CustomClaims) agreesWith(kind authz.ResourceType, id *uuid.UUID) error { if (c.Scope == authz.ScopeInstanceAdmin) != (kind == authz.ResourceTypeInstance) { return errors.New("the instance-admin claim does not agree with the scope") From e9113528491f5bf6c812fead2ed446cc9edfcf99 Mon Sep 17 00:00:00 2001 From: Javier Rodriguez Date: Mon, 5 Oct 2026 17:56:32 +0200 Subject: [PATCH 11/17] docs(controlplane): rewrite the API token claim comments in plain English Assisted-by: Claude Code Signed-off-by: Javier Rodriguez Chainloop-Trace-Sessions: 796bcbff-1898-452d-aaeb-b5311bffdfa6 --- .../usercontext/apitoken_middleware.go | 26 ++++++----- .../apitoken_middleware_integration_test.go | 30 ++++++------ .../usercontext/apitoken_middleware_test.go | 34 +++++++------- app/controlplane/pkg/biz/apitoken.go | 46 ++++++++++--------- .../pkg/biz/apitoken_integration_test.go | 11 +++-- .../pkg/biz/apitoken_verify_claims_test.go | 8 ++-- app/controlplane/pkg/jwt/apitoken/apitoken.go | 37 +++++++-------- .../pkg/usercontext/entities/apitoken.go | 4 +- 8 files changed, 102 insertions(+), 94 deletions(-) diff --git a/app/controlplane/internal/usercontext/apitoken_middleware.go b/app/controlplane/internal/usercontext/apitoken_middleware.go index efe4edb51..2ca3f1e88 100644 --- a/app/controlplane/internal/usercontext/apitoken_middleware.go +++ b/app/controlplane/internal/usercontext/apitoken_middleware.go @@ -73,8 +73,8 @@ func WithCurrentAPITokenAndOrgMiddleware(apiTokenUC *biz.APITokenUseCase, orgUC if claimsHaveAudience(genericClaims, apitoken.Audience) { claims, err := apitoken.ClaimsFromMap(genericClaims) if err != nil { - // A claim of the wrong type is not something this control plane signs. The - // raw token and the claims map are never logged. + // This control plane never signs a claim of the wrong type. The log line never + // includes the raw token or the claims map. id, _ := genericClaims["jti"].(string) logger.Errorw("msg", "[authN] API token claims do not decode", "id", id, "error", err) @@ -90,7 +90,8 @@ func WithCurrentAPITokenAndOrgMiddleware(apiTokenUC *biz.APITokenUseCase, orgUC return nil, fmt.Errorf("error setting current org and user: %w", err) } - // legacy_claims counts the tokens minted before the scope claims, to plan their sunset + // legacy_claims marks the tokens minted before the scope claims, to plan the end of + // their support logger.Infow("msg", "[authN] processed credentials", "id", claims.ID, "type", "API-token", "projectID", claims.ProjectID, "legacy_claims", !claims.HasScopeClaims()) } @@ -135,7 +136,7 @@ func WithAttestationContextFromAPIToken(apiTokenUC *biz.APITokenUseCase, orgUC * return nil, fmt.Errorf("error setting current org and user: %w", err) } - // The robot account comes from the row the claims have just confirmed + // The robot account comes from the row that VerifyClaims checked. ctx = WithRobotAccount(ctx, &RobotAccount{OrgID: token.OrganizationID.String(), ProviderKey: attjwtmiddleware.APITokenProviderKey}) logger.Infow("msg", "[authN] processed credentials", "id", tokenID, "type", "API-token", "legacy_claims", !claims.HasScopeClaims()) @@ -145,10 +146,10 @@ func WithAttestationContextFromAPIToken(apiTokenUC *biz.APITokenUseCase, orgUC * } } -// setCurrentOrgAndAPIToken loads the token's row, checks it against the claims the token was -// signed with, and puts the organization and the token in the context. It returns the row. -// The claims fix the token's scope, organization, project and workflow, and the row must match -// them. The policies, the product project list and revocation come from the row alone. +// setCurrentOrgAndAPIToken loads the token's row and checks it against the signed claims. Then it +// puts the organization and the token in the context, and returns the row. The claims fix the +// token's scope, organization, project and workflow. The row must match them. The policies, the +// product project list and the revocation come only from the row. func setCurrentOrgAndAPIToken(ctx context.Context, apiTokenUC *biz.APITokenUseCase, orgUC *biz.OrganizationUseCase, claims *apitoken.CustomClaims, logger *log.Helper) (context.Context, *biz.APIToken, error) { if claims == nil || claims.ID == "" { return nil, nil, errors.New("error retrieving the key ID from the API token") @@ -163,8 +164,8 @@ func setCurrentOrgAndAPIToken(ctx context.Context, apiTokenUC *biz.APITokenUseCa } if err := token.VerifyClaims(claims); err != nil { - // A row should never disagree with its signed claims: something wrote it wrongly. The raw - // JWT is never logged. + // A row should never disagree with its signed claims. If it does, something wrote the row + // incorrectly. The log line never includes the raw JWT. if errors.Is(err, biz.ErrAPITokenClaimsMismatch) { logger.Errorw("msg", "[authN] API token row disagrees with its signed claims", "id", claims.ID, "error", err) } @@ -209,8 +210,9 @@ func setCurrentOrgAndAPIToken(ctx context.Context, apiTokenUC *biz.APITokenUseCa ctx = entities.WithCurrentOrg(ctx, &entities.Org{Name: org.Name, ID: org.ID, CreatedAt: org.CreatedAt, Suspended: org.Suspended}) } - // Every value here is read from the row. VerifyClaims has confirmed the scope, project and - // workflow against the signed claims; the policies, project list and system flag are row-only. + // Every value here comes from the row. VerifyClaims checked the scope, project and workflow + // against the signed claims. The policies, the project list and the system flag come only from + // the row. ctx = entities.WithCurrentAPIToken(ctx, &entities.APIToken{ ID: token.ID.String(), Name: token.Name, diff --git a/app/controlplane/internal/usercontext/apitoken_middleware_integration_test.go b/app/controlplane/internal/usercontext/apitoken_middleware_integration_test.go index 3f1ea19a2..ac58055b7 100644 --- a/app/controlplane/internal/usercontext/apitoken_middleware_integration_test.go +++ b/app/controlplane/internal/usercontext/apitoken_middleware_integration_test.go @@ -36,8 +36,8 @@ import ( "github.com/stretchr/testify/require" ) -// The token the service layer sees carries exactly the scope its row records, once the row has -// confirmed the claims the token was signed with. +// After the row matches the signed claims, the service layer gets a token with exactly the scope +// that the row records. func TestAPITokenMiddlewareCarriesTheRowScope(t *testing.T) { if !testhelpers.IntegrationTestsEnabled() { t.Skip() @@ -167,9 +167,10 @@ func signLegacy(t *testing.T, tokenID uuid.UUID, claims jwt.MapClaims) string { } // Organization, project, workflow-pinned and instance tokens minted before the scope claims keep -// working, with the scope they always had: their rows went through the scope backfill and their -// claims imply the same scope. A product token minted before the scope claims, and a row written -// with no scope after the backfill ran, are refused. +// working, with the scope they always had. The scope backfill gave their rows that scope, and their +// claims imply the same scope. Both entry points refuse two cases: +// - a product token minted before the scope claims. +// - a row that a control plane wrote with no scope after the backfill ran. func TestAPITokenMiddlewareAcceptsTokensMintedBeforeTheScopeClaims(t *testing.T) { if !testhelpers.IntegrationTestsEnabled() { t.Skip() @@ -199,20 +200,21 @@ func TestAPITokenMiddlewareAcceptsTokensMintedBeforeTheScopeClaims(t *testing.T) testCases := []struct { name string - // row writes the columns the control plane that minted the token wrote. nil writes none, - // as for an instance token + // row sets the columns that the minting control plane wrote. A nil row sets no columns, as + // for an instance token. row func(*ent.APITokenCreate) *ent.APITokenCreate claims jwt.MapClaims header string - // afterBackfill writes the row once the backfill has run, as a control plane from before - // the scope columns does during a rolling upgrade or after a rollback + // afterBackfill writes the row after the backfill runs. A control plane from before the + // scope columns does this during a rolling upgrade or after a rollback. afterBackfill bool wantErr string wantScope authz.ResourceType wantScopeID *uuid.UUID wantOrg *uuid.UUID - // wantReach is what the token is confined to, nil for every project of its organization + // wantReach lists the projects that the token reaches. nil means every project of its + // organization. wantReach []uuid.UUID }{ { @@ -346,9 +348,9 @@ func TestAPITokenMiddlewareAcceptsTokensMintedBeforeTheScopeClaims(t *testing.T) } } -// A row that disagrees with the claims its token was signed with refuses the token, at both entry -// points, as a security event. Whatever wrote the row, it cannot widen the token, move it to -// another resource or organization, or make it an instance token. +// Both entry points refuse a token whose row disagrees with its signed claims, and treat it as a +// security event. A wrong row cannot widen the token, move it to another resource or organization, +// or make it an instance token. func TestAPITokenMiddlewareRefusesARowThatDisagreesWithItsClaims(t *testing.T) { if !testhelpers.IntegrationTestsEnabled() { t.Skip() @@ -376,7 +378,7 @@ func TestAPITokenMiddlewareRefusesARowThatDisagreesWithItsClaims(t *testing.T) { testCases := []struct { name string opts []biz.APITokenCreateOpt - // alter rewrites the row after the token was minted, nil leaves it as minted + // alter changes the row after the token is minted. nil leaves the row as it was minted. alter func(*ent.APITokenUpdateOne) *ent.APITokenUpdateOne header string wantErr string diff --git a/app/controlplane/internal/usercontext/apitoken_middleware_test.go b/app/controlplane/internal/usercontext/apitoken_middleware_test.go index 31d05b96a..8b6dc345b 100644 --- a/app/controlplane/internal/usercontext/apitoken_middleware_test.go +++ b/app/controlplane/internal/usercontext/apitoken_middleware_test.go @@ -40,7 +40,7 @@ import ( "github.com/stretchr/testify/require" ) -// Claim names, error substrings and entry point names the tests of both entry points share. +// Claim names, error substrings and entry point names that the tests of both entry points share. const ( claimAud = "aud" claimJTI = "jti" @@ -60,14 +60,15 @@ const ( authorizationHeader = "Authorization" // orgHeader names the organization an instance token acts in. orgHeader = "Chainloop-Organization" - // testSigningKey signs the tokens the tests build. The testhelpers sign with it too. + // testSigningKey is the key that signs the test tokens. The testhelpers use the same key. testSigningKey = "test" - // testIssuer is the issuer the tests' tokens name. Nothing checks it. + // testIssuer is the issuer in the test tokens. Nothing checks it. testIssuer = "cp.chainloop" ) -// apiTokenEntryPoints run a signed API token through each entry point, naming orgName in the -// organization header. handler runs only when the entry point accepts the token. +// apiTokenEntryPoints holds one function per entry point. Each function runs a signed API token +// through its entry point, with orgName in the organization header. handler runs only when the +// entry point accepts the token. var apiTokenEntryPoints = map[string]func(apiTokenUC *biz.APITokenUseCase, orgUC *biz.OrganizationUseCase, signed, orgName string, handler middleware.Handler) error{ entryAPI: func(apiTokenUC *biz.APITokenUseCase, orgUC *biz.OrganizationUseCase, signed, orgName string, handler middleware.Handler) error { claims := jwt.MapClaims{} @@ -105,7 +106,7 @@ type middlewareTestCase struct { workflowIDClaim string // tokenWorkflowID, if set, is the workflow_id stored on the DB row tokenWorkflowID *uuid.UUID - // extraClaims are added to the JWT claims + // extraClaims adds claims to the JWT. extraClaims jwt.MapClaims // the middleware logic got skipped skipped bool @@ -297,8 +298,7 @@ func toTimePtr(t time.Time) *time.Time { return &t } -// The scope reaches the service layer from the database row, once the row has confirmed the -// claims the token was signed with. +// After the row matches the signed claims, the service layer gets the scope from the database row. func TestWithCurrentAPITokenAndOrgMiddlewareCarriesScope(t *testing.T) { logger := log.NewHelper(log.NewStdLogger(io.Discard)) productID := uuid.New() @@ -312,7 +312,7 @@ func TestWithCurrentAPITokenAndOrgMiddlewareCarriesScope(t *testing.T) { rowScope *authz.ResourceType rowScopeID *uuid.UUID rowProjectIDs []uuid.UUID - // claims the token was signed with, besides aud and jti + // claims are the signed claims other than aud and jti. claims jwt.MapClaims }{ { @@ -375,8 +375,8 @@ func TestWithCurrentAPITokenAndOrgMiddlewareCarriesScope(t *testing.T) { } } -// A row that disagrees with its signed claims is logged as a security event, with the token id -// and never the raw token. +// The middleware logs a row that disagrees with its signed claims as a security event. The log line +// includes the token ID and never the raw token. func TestWithCurrentAPITokenAndOrgMiddlewareLogsAClaimsMismatch(t *testing.T) { const signedToken = "raw.signed.token" orgID := uuid.New() @@ -414,9 +414,9 @@ type preProductClaimRemoval struct { ProductID string `json:"product_id,omitempty"` } -// A JWT minted before the product_id claim was dropped may still carry one. Both entry points -// ignore it, whatever product it names: the signed scope claims, confirmed by the row, decide -// what the token is confined to. +// A JWT minted before the product_id claim was removed may still carry one. Both entry points +// ignore that claim, whatever product it names. The signed scope claims decide what the token +// reaches, and the row must match them. func TestAPITokenMiddlewaresIgnoreAProductClaim(t *testing.T) { rowProduct, otherProduct, orgID := uuid.New(), uuid.New(), uuid.New() product, organization := authz.ResourceTypeProduct, authz.ResourceTypeOrganization @@ -486,9 +486,9 @@ func TestAPITokenMiddlewaresIgnoreAProductClaim(t *testing.T) { } } -// A token's row decides whether it takes the instance-admin path, which reads the organization -// from the request header rather than from the token row; the JWT's claims must name the same scope. Both -// entry points agree. +// The token's row decides whether the token takes the instance-admin path. That path reads the +// organization from the request header, not from the token row. The JWT's claims must name the same +// scope. Both entry points behave the same. func TestAPITokenMiddlewaresResolveInstanceAdminTokens(t *testing.T) { rowOrg := &biz.Organization{ID: uuid.NewString(), Name: "row-org"} headerOrg := &biz.Organization{ID: uuid.NewString(), Name: "header-org"} diff --git a/app/controlplane/pkg/biz/apitoken.go b/app/controlplane/pkg/biz/apitoken.go index de53e24b1..d01923497 100644 --- a/app/controlplane/pkg/biz/apitoken.go +++ b/app/controlplane/pkg/biz/apitoken.go @@ -35,9 +35,9 @@ import ( "github.com/google/uuid" ) -// ErrAPITokenClaimsMismatch marks a token whose row disagrees with the claims it was signed with, -// or whose claims are malformed. It means something wrote the row wrongly, so it is a security -// event, not an expired or old credential. +// ErrAPITokenClaimsMismatch marks a token whose row disagrees with its signed claims, or whose +// claims are malformed. Something wrote the row or the claims incorrectly. It is a security event, +// not an expired or old credential. var ErrAPITokenClaimsMismatch = errors.New("API token claims do not match its row") var apiTokenTracer = otelx.Tracer("chainloop-controlplane", "biz/apitoken") @@ -167,13 +167,15 @@ func (t *APIToken) IsOrgScoped() bool { return t.scopeView().IsOrgScoped() } -// VerifyClaims checks that the token's row matches the claims its JWT was signed with: the same -// scope and organization, and the same project and workflow when the claims name them. Any -// difference refuses the token, so a wrong row can never widen the token or move it elsewhere. +// VerifyClaims checks that the token's row matches the signed claims. The row must have the same +// scope and organization. It must also have the same project and workflow when the claims name +// them. Any difference refuses the token, so a wrong row can never widen the token or move it +// elsewhere. // -// A mismatch wraps ErrAPITokenClaimsMismatch. Two expected states of older tokens are refused -// without it: a row that records no scope when the claims carry none either, and a product token -// minted before the scope claims existed, which has to be created again. +// A mismatch error wraps ErrAPITokenClaimsMismatch. Two expected states of older tokens get an +// error without it: +// - a row that records no scope, when the claims name no scope either. +// - a product token minted before the scope claims existed. Its owner must create a new token. func (t *APIToken) VerifyClaims(claims *apitoken.CustomClaims) error { if t == nil { return errors.New("API token not found") @@ -184,8 +186,9 @@ func (t *APIToken) VerifyClaims(claims *apitoken.CustomClaims) error { } if t.Scope == nil { - // Claims naming a scope mean the row was minted with one: it was cleared afterwards. Legacy - // claims name none, and a row that predates the scope columns records none. + // Claims that name a scope show that the token had a scope when it was minted. So something + // cleared the row's scope later. Legacy claims name no scope, and a row from before the + // scope columns records no scope. err := errors.New("API token records no scope") if claims.HasScopeClaims() { err = fmt.Errorf("%w: %w", err, ErrAPITokenClaimsMismatch) @@ -194,9 +197,9 @@ func (t *APIToken) VerifyClaims(claims *apitoken.CustomClaims) error { return err } - // This runs before SignedScope on purpose. An older product token's claims name only its - // organization, so SignedScope would read it as an organization scope, and the comparison - // below would log it as a security event rather than ask for a new token. + // This check runs before SignedScope on purpose. An older product token's claims name only its + // organization. SignedScope reads such claims as an organization scope. The comparison below + // would then log a security event instead of asking for a new token. if !claims.HasScopeClaims() && t.IsProductScoped() { return errors.New("API token was minted before its scope was signed, create a new one") } @@ -231,7 +234,7 @@ func (t *APIToken) VerifyClaims(claims *apitoken.CustomClaims) error { return nil } -// sameScopeID reports whether two optional scope ids are equal, both unset included. +// sameScopeID reports whether two optional scope ids are equal. Two unset ids are equal. func sameScopeID(a, b *uuid.UUID) bool { if a == nil || b == nil { return a == b @@ -560,7 +563,7 @@ func (uc *APITokenUseCase) Create(ctx context.Context, name string, description KeyID: token.ID, KeyName: name, ExpiresAt: expiresAt, - // Signed so that the row can only confirm what the token was granted + // The JWT signs the scope. The row must then match it. ScopeType: scope, ScopeID: scopeID, } @@ -604,10 +607,10 @@ func (uc *APITokenUseCase) Create(ctx context.Context, name string, description return token, nil } -// RegenerateJWT will regenerate a new JWT for the given token. Use with caution, since old JWTs are -// not invalidated. The new JWT signs the scope the row records. A row with no scope, or with a -// scope that contradicts its other columns, is refused. A row that is consistent but wrong would -// still be signed, so callers must be sure it is the token they mean. +// RegenerateJWT signs a new JWT for the given token. Use it with caution: it does not invalidate +// old JWTs. The new JWT signs the scope that the row records. RegenerateJWT refuses a row with no +// scope, or with a scope that contradicts its other columns. It still signs a row that is +// consistent but wrong, so callers must be sure that it is the token they mean. func (uc *APITokenUseCase) RegenerateJWT(ctx context.Context, tokenID uuid.UUID, expiresIn time.Duration) (*APIToken, error) { ctx, span := otelx.Start(ctx, apiTokenTracer, "APITokenUseCase.RegenerateJWT") defer span.End() @@ -623,8 +626,7 @@ func (uc *APITokenUseCase) RegenerateJWT(ctx context.Context, tokenID uuid.UUID, return nil, fmt.Errorf("finding token: %w", err) } - // Regenerating must never sign a row that records no scope, or one that disagrees with the - // token it is on. + // Never sign a row that records no scope, or a row whose scope contradicts its other columns. if token.Scope == nil { return nil, NewErrValidationStr("the token records no scope") } diff --git a/app/controlplane/pkg/biz/apitoken_integration_test.go b/app/controlplane/pkg/biz/apitoken_integration_test.go index 7853e9c70..8505fde79 100644 --- a/app/controlplane/pkg/biz/apitoken_integration_test.go +++ b/app/controlplane/pkg/biz/apitoken_integration_test.go @@ -1252,8 +1252,9 @@ func (s *apiTokenTestSuite) TestCreateAProductTokenWithItsProjects() { } } -// Every JWT signs the scope its row records, on creation and on regeneration, so that the row can -// only confirm it. A product token's JWT names its product through that scope alone. +// Every JWT signs the scope that its row records, when the token is created and when it is +// regenerated. The row can then only match that scope. A product token's JWT names its product only +// through that scope. func (s *apiTokenTestSuite) TestGeneratedJWTSignsTheTokenScope() { ctx := context.Background() productID := uuid.New() @@ -1261,7 +1262,7 @@ func (s *apiTokenTestSuite) TestGeneratedJWTSignsTheTokenScope() { wf, err := s.Workflow.Create(ctx, &biz.WorkflowCreateOpts{Name: randomName(), OrgID: s.org.ID, Project: s.p1.Name}) s.Require().NoError(err) - // claimsOf verifies raw and returns its payload, as parsed and as the typed claims + // claimsOf verifies raw. It returns the payload twice: as parsed, and as the typed claims. claimsOf := func(raw string) (jwt.MapClaims, *apitoken.CustomClaims) { payload := jwt.MapClaims{} info, err := jwt.ParseWithClaims(raw, payload, func(_ *jwt.Token) (interface{}, error) { @@ -1317,8 +1318,8 @@ func (s *apiTokenTestSuite) TestGeneratedJWTSignsTheTokenScope() { } } -// Regenerating signs the scope the row records. A row that records none, or one that disagrees -// with the token it is on, is refused instead of being signed. +// RegenerateJWT signs the scope that the row records. It refuses, and does not sign, a row that +// records no scope or that contradicts its other columns. func (s *apiTokenTestSuite) TestRegenerateJWTRefusesARowItCannotSign() { ctx := context.Background() orgUUID := uuid.MustParse(s.org.ID) diff --git a/app/controlplane/pkg/biz/apitoken_verify_claims_test.go b/app/controlplane/pkg/biz/apitoken_verify_claims_test.go index 31d3bf149..fb7ffe2e4 100644 --- a/app/controlplane/pkg/biz/apitoken_verify_claims_test.go +++ b/app/controlplane/pkg/biz/apitoken_verify_claims_test.go @@ -28,8 +28,8 @@ import ( // errScopeMismatch is the message of a row whose scope differs from the signed one. const errScopeMismatch = "scope mismatch" -// The claims bind what a token was granted; its row may only confirm them. Whatever wrote a row -// that disagrees, the token is refused rather than widened or moved. +// The signed claims fix what a token was granted, and its row may only match them. If a row +// disagrees, VerifyClaims refuses the token. It never widens or moves the token. func TestAPITokenVerifyClaims(t *testing.T) { org, otherOrg := uuid.New(), uuid.New() project, otherProject := uuid.New(), uuid.New() @@ -60,8 +60,8 @@ func TestAPITokenVerifyClaims(t *testing.T) { row *APIToken claims apitoken.CustomClaims wantErr string - // mismatch marks a refusal that means the row was written wrongly: it must wrap - // ErrAPITokenClaimsMismatch so the middleware logs it as a security event + // mismatch marks a refusal caused by a wrong row. That error must wrap + // ErrAPITokenClaimsMismatch, so that the middleware logs it as a security event. mismatch bool }{ {name: "signed organization token", row: orgRow, claims: signedOrg}, diff --git a/app/controlplane/pkg/jwt/apitoken/apitoken.go b/app/controlplane/pkg/jwt/apitoken/apitoken.go index 4fefb3d90..e616741df 100644 --- a/app/controlplane/pkg/jwt/apitoken/apitoken.go +++ b/app/controlplane/pkg/jwt/apitoken/apitoken.go @@ -83,8 +83,8 @@ type GenerateJWTOptions struct { // Scope is the legacy instance-admin claim. The platform and control planes up to v1.112 // read it, so instance tokens keep carrying it. Scope *string - // ScopeType and ScopeID are what the token is scoped to, as its row records them. ScopeType - // is required. ScopeID is unset only for an instance token. + // ScopeType and ScopeID name the token's scope, as its row records it. ScopeType is required. + // ScopeID is unset only for an instance token. ScopeType *authz.ResourceType ScopeID *uuid.UUID } @@ -170,7 +170,7 @@ type CustomClaims struct { // Despite its name it is not the token's scope: that is ScopeType and ScopeID. Scope string `json:"scope,omitempty"` // ScopeType and ScopeID say what the token was granted. A token minted before they existed - // carries neither, and SignedScope works its scope out from the claims it does carry. + // carries neither, and SignedScope derives its scope from the claims that it does carry. ScopeType string `json:"scope_type,omitempty"` ScopeID string `json:"scope_id,omitempty"` jwt.RegisteredClaims @@ -182,10 +182,10 @@ func (c *CustomClaims) HasScopeClaims() bool { return c.ScopeType != "" } -// SignedScope returns the scope the claims bind the token to. For a token minted with the -// scope_type and scope_id claims, that is what they say. For an older token, it is the scope its -// other claims imply (see legacyScope), so an older product token comes out as its organization. -// Claims that contradict themselves are an error. +// SignedScope returns the scope that the claims bind the token to. For a token with the scope_type +// and scope_id claims, that scope is what they name. For an older token, it is the scope that its +// other claims imply (see legacyScope). An older product token therefore gets its organization as +// its scope. SignedScope returns an error for claims that contradict themselves. func (c *CustomClaims) SignedScope() (authz.ResourceType, *uuid.UUID, error) { kind, id, err := c.namedScope() if err != nil { @@ -226,9 +226,10 @@ func (c *CustomClaims) namedScope() (authz.ResourceType, *uuid.UUID, error) { } } -// legacyScope is the scope the claims of a token minted before the scope_type claim imply: the -// instance for the instance-admin claim, else its project, else its organization. It is the rule -// biz.newTokenScope and the scope backfill migration apply to the row, so the two agree. +// legacyScope returns the scope that the claims of an older token imply. An older token is a token +// minted before the scope_type claim existed. Its scope is the instance for the instance-admin +// claim, else its project, else its organization. biz.newTokenScope and the scope backfill +// migration apply the same rule to the row, so the two agree. func (c *CustomClaims) legacyScope() (authz.ResourceType, *uuid.UUID, error) { var kind authz.ResourceType var raw string @@ -251,14 +252,14 @@ func (c *CustomClaims) legacyScope() (authz.ResourceType, *uuid.UUID, error) { return kind, &id, nil } -// agreesWith checks that the rest of the claims fit the scope, so that the control plane and the -// platform read the same scope from the token: -// - only an instance token carries the instance-admin claim, and it names no organization or -// project; -// - an organization token names its own organization and no project; -// - a project token names its organization and the project it is scoped to; -// - a product token names its organization and no project; -// - a workflow claim always comes with a project claim. +// agreesWith checks that the other claims fit the scope. This makes the control plane and the +// platform read the same scope from the token. The rules are: +// - Only an instance token carries the instance-admin claim. It names no organization and no +// project. +// - An organization token names its own organization and no project. +// - A project token names its organization and the project of its scope. +// - A product token names its organization and no project. +// - A workflow claim always comes with a project claim. func (c *CustomClaims) agreesWith(kind authz.ResourceType, id *uuid.UUID) error { if (c.Scope == authz.ScopeInstanceAdmin) != (kind == authz.ResourceTypeInstance) { return errors.New("the instance-admin claim does not agree with the scope") diff --git a/app/controlplane/pkg/usercontext/entities/apitoken.go b/app/controlplane/pkg/usercontext/entities/apitoken.go index 7636ba7b8..93ac5471a 100644 --- a/app/controlplane/pkg/usercontext/entities/apitoken.go +++ b/app/controlplane/pkg/usercontext/entities/apitoken.go @@ -36,8 +36,8 @@ type APIToken struct { WorkflowName *string // ACL policies for this token. Used for authorization checks. Policies []*authz.Policy - // Scope and ScopeID name what the token is scoped to. They are read from the row once the row - // has been checked against the token's signed claims; a row recording none is refused. + // Scope and ScopeID name the token's scope. They come from the row, after the middleware checks + // the row against the signed claims. The middleware refuses a row that records no scope. Scope *authz.ResourceType ScopeID *uuid.UUID // ProjectIDs are the projects a product token reaches, loaded from the row. From d6c3b17dd4c91e7faa77ecdf6d439cda8b3dec08 Mon Sep 17 00:00:00 2001 From: Javier Rodriguez Date: Tue, 6 Oct 2026 09:38:56 +0200 Subject: [PATCH 12/17] fix(controlplane): compare the API token project and workflow with the row both ways GenerateJWT refuses an empty scope type. Before, an empty value passed the check, and the builder signed a token without the scope claims. VerifyClaims refuses a row with a project or a workflow that the signed claims do not name. Before, it compared these columns only when the claims named them. Every writer signs the project and workflow claims when the row records them, so older tokens still pass. Assisted-by: Claude Code Signed-off-by: Javier Rodriguez Chainloop-Trace-Sessions: 796bcbff-1898-452d-aaeb-b5311bffdfa6 --- app/controlplane/pkg/biz/apitoken.go | 20 ++++++++++++++----- .../pkg/biz/apitoken_verify_claims_test.go | 13 +++++++++--- app/controlplane/pkg/jwt/apitoken/apitoken.go | 2 +- .../pkg/jwt/apitoken/apitoken_test.go | 7 +++++++ 4 files changed, 33 insertions(+), 9 deletions(-) diff --git a/app/controlplane/pkg/biz/apitoken.go b/app/controlplane/pkg/biz/apitoken.go index d01923497..c1b679607 100644 --- a/app/controlplane/pkg/biz/apitoken.go +++ b/app/controlplane/pkg/biz/apitoken.go @@ -168,9 +168,9 @@ func (t *APIToken) IsOrgScoped() bool { } // VerifyClaims checks that the token's row matches the signed claims. The row must have the same -// scope and organization. It must also have the same project and workflow when the claims name -// them. Any difference refuses the token, so a wrong row can never widen the token or move it -// elsewhere. +// scope, organization, project and workflow. A project or workflow that only the row or only the +// claims name is also a difference. Any difference refuses the token, so a wrong row can never +// widen the token or move it elsewhere. // // A mismatch error wraps ErrAPITokenClaimsMismatch. Two expected states of older tokens get an // error without it: @@ -223,17 +223,27 @@ func (t *APIToken) VerifyClaims(claims *apitoken.CustomClaims) error { return fmt.Errorf("API token organization mismatch: %w", ErrAPITokenClaimsMismatch) } - if claims.ProjectID != "" && (t.ProjectID == nil || t.ProjectID.String() != claims.ProjectID) { + if !sameClaimedID(t.ProjectID, claims.ProjectID) { return fmt.Errorf("API token project mismatch: %w", ErrAPITokenClaimsMismatch) } - if claims.WorkflowID != "" && (t.WorkflowID == nil || t.WorkflowID.String() != claims.WorkflowID) { + if !sameClaimedID(t.WorkflowID, claims.WorkflowID) { return fmt.Errorf("API token workflow mismatch: %w", ErrAPITokenClaimsMismatch) } return nil } +// sameClaimedID reports whether an optional id of the row equals its claim. An unset id equals +// an empty claim. +func sameClaimedID(row *uuid.UUID, claim string) bool { + if row == nil { + return claim == "" + } + + return row.String() == claim +} + // sameScopeID reports whether two optional scope ids are equal. Two unset ids are equal. func sameScopeID(a, b *uuid.UUID) bool { if a == nil || b == nil { diff --git a/app/controlplane/pkg/biz/apitoken_verify_claims_test.go b/app/controlplane/pkg/biz/apitoken_verify_claims_test.go index fb7ffe2e4..e92ab9ba3 100644 --- a/app/controlplane/pkg/biz/apitoken_verify_claims_test.go +++ b/app/controlplane/pkg/biz/apitoken_verify_claims_test.go @@ -25,8 +25,12 @@ import ( "github.com/stretchr/testify/assert" ) -// errScopeMismatch is the message of a row whose scope differs from the signed one. -const errScopeMismatch = "scope mismatch" +const ( + // errScopeMismatch is the message of a row whose scope differs from the signed one. + errScopeMismatch = "scope mismatch" + // errWorkflowMismatch is the message of a row whose workflow differs from the signed one. + errWorkflowMismatch = "workflow mismatch" +) // The signed claims fix what a token was granted, and its row may only match them. If a row // disagrees, VerifyClaims refuses the token. It never widens or moves the token. @@ -89,7 +93,10 @@ func TestAPITokenVerifyClaims(t *testing.T) { {name: "an organization row moved to another organization", row: &APIToken{OrganizationID: otherOrg, Scope: &orgKind, ScopeID: &otherOrg}, claims: legacyOrg, wantErr: errScopeMismatch, mismatch: true}, {name: "a product row moved to another organization", row: &APIToken{OrganizationID: otherOrg, Scope: &productKind, ScopeID: &product}, claims: signedProduct, wantErr: "organization mismatch", mismatch: true}, {name: "the project claim disagrees with the row's project column", row: &APIToken{OrganizationID: org, ProjectID: &otherProject, Scope: &projectKind, ScopeID: &project}, claims: signedProject, wantErr: "project mismatch", mismatch: true}, - {name: "a workflow claim on a row with no workflow", row: projectRow, claims: signedWorkflow, wantErr: "workflow mismatch", mismatch: true}, + {name: "a workflow claim on a row with no workflow", row: projectRow, claims: signedWorkflow, wantErr: errWorkflowMismatch, mismatch: true}, + {name: "a workflow on the row that the signed claims do not name", row: workflowRow, claims: signedProject, wantErr: errWorkflowMismatch, mismatch: true}, + {name: "a workflow on the row that the legacy claims do not name", row: workflowRow, claims: legacyProject, wantErr: errWorkflowMismatch, mismatch: true}, + {name: "a project on an organization row that the claims do not name", row: &APIToken{OrganizationID: org, ProjectID: &project, Scope: &orgKind, ScopeID: &org}, claims: signedOrg, wantErr: "project mismatch", mismatch: true}, } for _, tc := range testCases { diff --git a/app/controlplane/pkg/jwt/apitoken/apitoken.go b/app/controlplane/pkg/jwt/apitoken/apitoken.go index e616741df..b5f75d492 100644 --- a/app/controlplane/pkg/jwt/apitoken/apitoken.go +++ b/app/controlplane/pkg/jwt/apitoken/apitoken.go @@ -135,7 +135,7 @@ func (ra *Builder) GenerateJWT(opts *GenerateJWTOptions) (string, error) { claims.WorkflowName = *opts.WorkflowName } - if opts.ScopeType == nil { + if opts.ScopeType == nil || *opts.ScopeType == "" { return "", errors.New("scopeType is required") } diff --git a/app/controlplane/pkg/jwt/apitoken/apitoken_test.go b/app/controlplane/pkg/jwt/apitoken/apitoken_test.go index 33fd140a9..12ff3455b 100644 --- a/app/controlplane/pkg/jwt/apitoken/apitoken_test.go +++ b/app/controlplane/pkg/jwt/apitoken/apitoken_test.go @@ -143,6 +143,13 @@ func TestGenerateJWT(t *testing.T) { opts: &GenerateJWTOptions{OrgID: &org, OrgName: toPtr("org-name"), KeyName: testKeyName, KeyID: keyID}, wantErr: true, }, + { + // An empty scope type would sign a token without the scope claims, like an older token + name: "an empty scope type", + opts: &GenerateJWTOptions{OrgID: &org, OrgName: toPtr("org-name"), KeyName: testKeyName, KeyID: keyID, + ScopeType: toPtr(authz.ResourceType("")), ScopeID: &org}, + wantErr: true, + }, { name: "an organization scope naming another organization", opts: &GenerateJWTOptions{OrgID: &org, OrgName: toPtr("org-name"), KeyName: testKeyName, KeyID: keyID, From 38ebab4a7ae08f29e632253e88c2220280d4cb42 Mon Sep 17 00:00:00 2001 From: Javier Rodriguez Date: Tue, 6 Oct 2026 10:28:18 +0200 Subject: [PATCH 13/17] refactor(controlplane): name the claims scope getter GetScope and keep mismatch details in the log Rename CustomClaims.SignedScope to GetScope. The scope is not signed yet when GenerateJWT reads it. The name follows the golang-jwt getters on claims, and the Scope field already takes Scope(). A caller whose token row disagrees with its claims now gets only "API token could not be verified". The reason stays in the security log, so the response does not show a forger which check failed, and it does not name the token row. Assisted-by: Claude Code Signed-off-by: Javier Rodriguez Chainloop-Trace-Sessions: 796bcbff-1898-452d-aaeb-b5311bffdfa6 --- .../usercontext/apitoken_middleware.go | 4 +++- .../apitoken_middleware_integration_test.go | 12 +++++----- .../usercontext/apitoken_middleware_test.go | 22 +++++++++++-------- app/controlplane/pkg/biz/apitoken.go | 10 ++++----- app/controlplane/pkg/jwt/apitoken/apitoken.go | 10 ++++----- .../pkg/jwt/apitoken/apitoken_test.go | 4 ++-- 6 files changed, 34 insertions(+), 28 deletions(-) diff --git a/app/controlplane/internal/usercontext/apitoken_middleware.go b/app/controlplane/internal/usercontext/apitoken_middleware.go index 2ca3f1e88..1a809bb9d 100644 --- a/app/controlplane/internal/usercontext/apitoken_middleware.go +++ b/app/controlplane/internal/usercontext/apitoken_middleware.go @@ -165,9 +165,11 @@ func setCurrentOrgAndAPIToken(ctx context.Context, apiTokenUC *biz.APITokenUseCa if err := token.VerifyClaims(claims); err != nil { // A row should never disagree with its signed claims. If it does, something wrote the row - // incorrectly. The log line never includes the raw JWT. + // incorrectly. The log line gives the reason and never includes the raw JWT. The caller + // learns only that the token could not be verified. if errors.Is(err, biz.ErrAPITokenClaimsMismatch) { logger.Errorw("msg", "[authN] API token row disagrees with its signed claims", "id", claims.ID, "error", err) + return nil, nil, biz.ErrAPITokenClaimsMismatch } return nil, nil, err diff --git a/app/controlplane/internal/usercontext/apitoken_middleware_integration_test.go b/app/controlplane/internal/usercontext/apitoken_middleware_integration_test.go index ac58055b7..05243c851 100644 --- a/app/controlplane/internal/usercontext/apitoken_middleware_integration_test.go +++ b/app/controlplane/internal/usercontext/apitoken_middleware_integration_test.go @@ -386,23 +386,23 @@ func TestAPITokenMiddlewareRefusesARowThatDisagreesWithItsClaims(t *testing.T) { {name: "an organization token as minted", opts: orgToken}, {name: "a project token as minted", opts: projectToken}, {name: "a product token as minted", opts: productToken}, - {name: "a project token widened to its organization", opts: projectToken, wantErr: errScopeMismatch, + {name: "a project token widened to its organization", opts: projectToken, wantErr: errNotVerified, alter: func(u *ent.APITokenUpdateOne) *ent.APITokenUpdateOne { return u.SetScope(authz.ResourceTypeOrganization).SetScopeID(orgID) }}, - {name: "an organization token made an instance token", opts: orgToken, header: otherOrg.Name, wantErr: errScopeMismatch, + {name: "an organization token made an instance token", opts: orgToken, header: otherOrg.Name, wantErr: errNotVerified, alter: func(u *ent.APITokenUpdateOne) *ent.APITokenUpdateOne { return u.SetScope(authz.ResourceTypeInstance).ClearScopeID() }}, - {name: "a project token moved to another project", opts: projectToken, wantErr: errScopeMismatch, + {name: "a project token moved to another project", opts: projectToken, wantErr: errNotVerified, alter: func(u *ent.APITokenUpdateOne) *ent.APITokenUpdateOne { return u.SetScopeID(other.ID) }}, - {name: "an organization token moved to another organization", opts: orgToken, wantErr: errScopeMismatch, + {name: "an organization token moved to another organization", opts: orgToken, wantErr: errNotVerified, alter: func(u *ent.APITokenUpdateOne) *ent.APITokenUpdateOne { return u.SetOrganizationID(otherOrgID).SetScopeID(otherOrgID) }}, - {name: "a product token moved to another product", opts: productToken, wantErr: errScopeMismatch, + {name: "a product token moved to another product", opts: productToken, wantErr: errNotVerified, alter: func(u *ent.APITokenUpdateOne) *ent.APITokenUpdateOne { return u.SetScopeID(otherProduct) }}, - {name: "a product token moved to another organization", opts: productToken, wantErr: "organization mismatch", + {name: "a product token moved to another organization", opts: productToken, wantErr: errNotVerified, alter: func(u *ent.APITokenUpdateOne) *ent.APITokenUpdateOne { return u.SetOrganizationID(otherOrgID) }}, } diff --git a/app/controlplane/internal/usercontext/apitoken_middleware_test.go b/app/controlplane/internal/usercontext/apitoken_middleware_test.go index 8b6dc345b..da470557e 100644 --- a/app/controlplane/internal/usercontext/apitoken_middleware_test.go +++ b/app/controlplane/internal/usercontext/apitoken_middleware_test.go @@ -49,10 +49,11 @@ const ( claimProjectID = "project_id" claimScopeID = "scope_id" claimScopeType = "scope_type" - errScopeMismatch = "scope mismatch" errRecordsNoScope = "records no scope" - entryAPI = "API" - entryAttestation = "attestation" + // errNotVerified is all that a caller learns about a token whose row disagrees with its claims + errNotVerified = "API token could not be verified" + entryAPI = "API" + entryAttestation = "attestation" ) const ( @@ -181,7 +182,7 @@ func TestWithCurrentAPITokenAndOrgMiddleware(t *testing.T) { workflowIDClaim: matchingWorkflowID.String(), tokenWorkflowID: &otherWorkflowID, wantErr: true, - wantErrContains: "workflow mismatch", + wantErrContains: errNotVerified, }, { name: "workflow claim present but DB row has none", @@ -190,7 +191,7 @@ func TestWithCurrentAPITokenAndOrgMiddleware(t *testing.T) { tokenExists: true, workflowIDClaim: matchingWorkflowID.String(), wantErr: true, - wantErrContains: "workflow mismatch", + wantErrContains: errNotVerified, }, { name: "scope claims disagree with the DB row", @@ -199,7 +200,7 @@ func TestWithCurrentAPITokenAndOrgMiddleware(t *testing.T) { tokenExists: true, extraClaims: jwt.MapClaims{claimScopeType: "product", claimScopeID: uuid.NewString()}, wantErr: true, - wantErrContains: errScopeMismatch, + wantErrContains: errNotVerified, }, { name: "a claim of the wrong type is refused", @@ -401,8 +402,11 @@ func TestWithCurrentAPITokenAndOrgMiddlewareLogsAClaimsMismatch(t *testing.T) { _, err = WithCurrentAPITokenAndOrgMiddleware(apiTokenUC, orgUC, logger)( func(context.Context, interface{}) (interface{}, error) { return nil, nil })(jwtmiddleware.NewContext(context.Background(), claims), nil) - require.Error(t, err) + // The caller learns only that the token could not be verified. The log line has the reason. + require.ErrorIs(t, err, biz.ErrAPITokenClaimsMismatch) + assert.NotContains(t, err.Error(), "scope mismatch") assert.Contains(t, buf.String(), "disagrees with its signed claims") + assert.Contains(t, buf.String(), "scope mismatch") assert.Contains(t, buf.String(), token.ID.String()) assert.NotContains(t, buf.String(), signedToken) } @@ -508,8 +512,8 @@ func TestAPITokenMiddlewaresResolveInstanceAdminTokens(t *testing.T) { {name: "an instance-admin token without the header has no organization", scopeClaim: authz.ScopeInstanceAdmin}, {name: "an organization token takes its row's organization", rowOrg: rowOrg, header: headerOrg.Name, wantOrg: rowOrg}, // The signed claims and the row must agree, and the claims must agree with themselves. - {name: "an instance row whose JWT names no scope is refused", header: headerOrg.Name, wantErr: "scope claims"}, - {name: "an organization row whose JWT carries the instance-admin claim is refused", scopeClaim: authz.ScopeInstanceAdmin, rowOrg: rowOrg, header: headerOrg.Name, wantErr: "scope claims"}, + {name: "an instance row whose JWT names no scope is refused", header: headerOrg.Name, wantErr: errNotVerified}, + {name: "an organization row whose JWT carries the instance-admin claim is refused", scopeClaim: authz.ScopeInstanceAdmin, rowOrg: rowOrg, header: headerOrg.Name, wantErr: errNotVerified}, } for _, tc := range testCases { diff --git a/app/controlplane/pkg/biz/apitoken.go b/app/controlplane/pkg/biz/apitoken.go index c1b679607..0abe1369f 100644 --- a/app/controlplane/pkg/biz/apitoken.go +++ b/app/controlplane/pkg/biz/apitoken.go @@ -37,8 +37,8 @@ import ( // ErrAPITokenClaimsMismatch marks a token whose row disagrees with its signed claims, or whose // claims are malformed. Something wrote the row or the claims incorrectly. It is a security event, -// not an expired or old credential. -var ErrAPITokenClaimsMismatch = errors.New("API token claims do not match its row") +// not an expired or old credential. The caller sees only this message, so it names no detail. +var ErrAPITokenClaimsMismatch = errors.New("API token could not be verified") var apiTokenTracer = otelx.Tracer("chainloop-controlplane", "biz/apitoken") @@ -197,14 +197,14 @@ func (t *APIToken) VerifyClaims(claims *apitoken.CustomClaims) error { return err } - // This check runs before SignedScope on purpose. An older product token's claims name only its - // organization. SignedScope reads such claims as an organization scope. The comparison below + // This check runs before GetScope on purpose. An older product token's claims name only its + // organization. GetScope reads such claims as an organization scope. The comparison below // would then log a security event instead of asking for a new token. if !claims.HasScopeClaims() && t.IsProductScoped() { return errors.New("API token was minted before its scope was signed, create a new one") } - kind, id, err := claims.SignedScope() + kind, id, err := claims.GetScope() if err != nil { return fmt.Errorf("API token scope claims: %w: %w", err, ErrAPITokenClaimsMismatch) } diff --git a/app/controlplane/pkg/jwt/apitoken/apitoken.go b/app/controlplane/pkg/jwt/apitoken/apitoken.go index b5f75d492..05ea344fc 100644 --- a/app/controlplane/pkg/jwt/apitoken/apitoken.go +++ b/app/controlplane/pkg/jwt/apitoken/apitoken.go @@ -145,7 +145,7 @@ func (ra *Builder) GenerateJWT(opts *GenerateJWTOptions) (string, error) { } // Never sign a token whose claims contradict themselves - if _, _, err := claims.SignedScope(); err != nil { + if _, _, err := claims.GetScope(); err != nil { return "", fmt.Errorf("inconsistent token scope: %w", err) } @@ -170,7 +170,7 @@ type CustomClaims struct { // Despite its name it is not the token's scope: that is ScopeType and ScopeID. Scope string `json:"scope,omitempty"` // ScopeType and ScopeID say what the token was granted. A token minted before they existed - // carries neither, and SignedScope derives its scope from the claims that it does carry. + // carries neither, and GetScope derives its scope from the claims that it does carry. ScopeType string `json:"scope_type,omitempty"` ScopeID string `json:"scope_id,omitempty"` jwt.RegisteredClaims @@ -182,11 +182,11 @@ func (c *CustomClaims) HasScopeClaims() bool { return c.ScopeType != "" } -// SignedScope returns the scope that the claims bind the token to. For a token with the scope_type +// GetScope returns the scope that the claims bind the token to. For a token with the scope_type // and scope_id claims, that scope is what they name. For an older token, it is the scope that its // other claims imply (see legacyScope). An older product token therefore gets its organization as -// its scope. SignedScope returns an error for claims that contradict themselves. -func (c *CustomClaims) SignedScope() (authz.ResourceType, *uuid.UUID, error) { +// its scope. GetScope returns an error for claims that contradict themselves. +func (c *CustomClaims) GetScope() (authz.ResourceType, *uuid.UUID, error) { kind, id, err := c.namedScope() if err != nil { return "", nil, err diff --git a/app/controlplane/pkg/jwt/apitoken/apitoken_test.go b/app/controlplane/pkg/jwt/apitoken/apitoken_test.go index 12ff3455b..9fb56758e 100644 --- a/app/controlplane/pkg/jwt/apitoken/apitoken_test.go +++ b/app/controlplane/pkg/jwt/apitoken/apitoken_test.go @@ -244,7 +244,7 @@ func TestGenerateJWT(t *testing.T) { } } -func TestSignedScope(t *testing.T) { +func TestGetScope(t *testing.T) { org, project, product, other := uuid.New(), uuid.New(), uuid.New(), uuid.New() workflow := uuid.New() @@ -289,7 +289,7 @@ func TestSignedScope(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - kind, id, err := tc.claims.SignedScope() + kind, id, err := tc.claims.GetScope() if tc.wantErr { require.Error(t, err) return From 279f3bdaace976621e1904f5159bc29921a9875a Mon Sep 17 00:00:00 2001 From: Javier Rodriguez Date: Tue, 6 Oct 2026 10:37:36 +0200 Subject: [PATCH 14/17] test(controlplane): compare the whole error that a mismatched API token gets The tests compared only part of the error. A workflow or organization reason added to it would still have passed. They now compare the whole text that both entry points return. Assisted-by: Claude Code Signed-off-by: Javier Rodriguez Chainloop-Trace-Sessions: 796bcbff-1898-452d-aaeb-b5311bffdfa6 --- .../apitoken_middleware_integration_test.go | 14 +++++++------- .../usercontext/apitoken_middleware_test.go | 11 +++++++---- 2 files changed, 14 insertions(+), 11 deletions(-) diff --git a/app/controlplane/internal/usercontext/apitoken_middleware_integration_test.go b/app/controlplane/internal/usercontext/apitoken_middleware_integration_test.go index 05243c851..9045b51f4 100644 --- a/app/controlplane/internal/usercontext/apitoken_middleware_integration_test.go +++ b/app/controlplane/internal/usercontext/apitoken_middleware_integration_test.go @@ -386,23 +386,23 @@ func TestAPITokenMiddlewareRefusesARowThatDisagreesWithItsClaims(t *testing.T) { {name: "an organization token as minted", opts: orgToken}, {name: "a project token as minted", opts: projectToken}, {name: "a product token as minted", opts: productToken}, - {name: "a project token widened to its organization", opts: projectToken, wantErr: errNotVerified, + {name: "a project token widened to its organization", opts: projectToken, wantErr: errNotVerifiedAtEntry, alter: func(u *ent.APITokenUpdateOne) *ent.APITokenUpdateOne { return u.SetScope(authz.ResourceTypeOrganization).SetScopeID(orgID) }}, - {name: "an organization token made an instance token", opts: orgToken, header: otherOrg.Name, wantErr: errNotVerified, + {name: "an organization token made an instance token", opts: orgToken, header: otherOrg.Name, wantErr: errNotVerifiedAtEntry, alter: func(u *ent.APITokenUpdateOne) *ent.APITokenUpdateOne { return u.SetScope(authz.ResourceTypeInstance).ClearScopeID() }}, - {name: "a project token moved to another project", opts: projectToken, wantErr: errNotVerified, + {name: "a project token moved to another project", opts: projectToken, wantErr: errNotVerifiedAtEntry, alter: func(u *ent.APITokenUpdateOne) *ent.APITokenUpdateOne { return u.SetScopeID(other.ID) }}, - {name: "an organization token moved to another organization", opts: orgToken, wantErr: errNotVerified, + {name: "an organization token moved to another organization", opts: orgToken, wantErr: errNotVerifiedAtEntry, alter: func(u *ent.APITokenUpdateOne) *ent.APITokenUpdateOne { return u.SetOrganizationID(otherOrgID).SetScopeID(otherOrgID) }}, - {name: "a product token moved to another product", opts: productToken, wantErr: errNotVerified, + {name: "a product token moved to another product", opts: productToken, wantErr: errNotVerifiedAtEntry, alter: func(u *ent.APITokenUpdateOne) *ent.APITokenUpdateOne { return u.SetScopeID(otherProduct) }}, - {name: "a product token moved to another organization", opts: productToken, wantErr: errNotVerified, + {name: "a product token moved to another organization", opts: productToken, wantErr: errNotVerifiedAtEntry, alter: func(u *ent.APITokenUpdateOne) *ent.APITokenUpdateOne { return u.SetOrganizationID(otherOrgID) }}, } @@ -416,7 +416,7 @@ func TestAPITokenMiddlewareRefusesARowThatDisagreesWithItsClaims(t *testing.T) { for entry, got := range authenticateAtBothEntryPoints(t, tu, token.JWT, tc.header) { if tc.wantErr != "" { - assert.ErrorContains(t, got.err, tc.wantErr, entry) + assert.EqualError(t, got.err, tc.wantErr, entry) assert.ErrorIs(t, got.err, biz.ErrAPITokenClaimsMismatch, entry) assert.Nil(t, got.token, entry) continue diff --git a/app/controlplane/internal/usercontext/apitoken_middleware_test.go b/app/controlplane/internal/usercontext/apitoken_middleware_test.go index da470557e..930411dc8 100644 --- a/app/controlplane/internal/usercontext/apitoken_middleware_test.go +++ b/app/controlplane/internal/usercontext/apitoken_middleware_test.go @@ -51,9 +51,12 @@ const ( claimScopeType = "scope_type" errRecordsNoScope = "records no scope" // errNotVerified is all that a caller learns about a token whose row disagrees with its claims - errNotVerified = "API token could not be verified" - entryAPI = "API" - entryAttestation = "attestation" + errNotVerified = "API token could not be verified" + // errNotVerifiedAtEntry is the whole error that both entry points return for such a token. The + // tests compare the whole text, so that no reason can leak into it. + errNotVerifiedAtEntry = "error setting current org and user: " + errNotVerified + entryAPI = "API" + entryAttestation = "attestation" ) const ( @@ -404,7 +407,7 @@ func TestWithCurrentAPITokenAndOrgMiddlewareLogsAClaimsMismatch(t *testing.T) { // The caller learns only that the token could not be verified. The log line has the reason. require.ErrorIs(t, err, biz.ErrAPITokenClaimsMismatch) - assert.NotContains(t, err.Error(), "scope mismatch") + require.EqualError(t, err, errNotVerifiedAtEntry) assert.Contains(t, buf.String(), "disagrees with its signed claims") assert.Contains(t, buf.String(), "scope mismatch") assert.Contains(t, buf.String(), token.ID.String()) From 06e140f6da45db705c8e95c9deca79919def27a7 Mon Sep 17 00:00:00 2001 From: Javier Rodriguez Date: Tue, 6 Oct 2026 11:05:07 +0200 Subject: [PATCH 15/17] refactor(controlplane): sign the API token scope kind in the scope claim The scope claim now has the scope kind of every token: instance, organization, project or product. The scope_type claim goes away. Before this change, only instance tokens had a scope claim, with the value INSTANCE_ADMIN. Control planes from v1.113 do not read the claim, so a second claim for the kind was not necessary. Older tokens keep working as before. An older instance token keeps its INSTANCE_ADMIN value, and GetScope reads it as the instance scope. The token builder refuses that value for a new token. Assisted-by: Claude Code Signed-off-by: Javier Rodriguez Chainloop-Trace-Sessions: 796bcbff-1898-452d-aaeb-b5311bffdfa6 --- .../usercontext/apitoken_middleware_test.go | 19 ++-- app/controlplane/pkg/authz/authz.go | 3 +- app/controlplane/pkg/biz/apitoken.go | 16 ++-- .../pkg/biz/apitoken_integration_test.go | 25 +++--- .../pkg/biz/apitoken_verify_claims_test.go | 12 +-- app/controlplane/pkg/jwt/apitoken/apitoken.go | 68 +++++++------- .../pkg/jwt/apitoken/apitoken_test.go | 90 +++++++++---------- 7 files changed, 110 insertions(+), 123 deletions(-) diff --git a/app/controlplane/internal/usercontext/apitoken_middleware_test.go b/app/controlplane/internal/usercontext/apitoken_middleware_test.go index 930411dc8..20655fd63 100644 --- a/app/controlplane/internal/usercontext/apitoken_middleware_test.go +++ b/app/controlplane/internal/usercontext/apitoken_middleware_test.go @@ -48,7 +48,7 @@ const ( claimOrgName = "org_name" claimProjectID = "project_id" claimScopeID = "scope_id" - claimScopeType = "scope_type" + claimScope = "scope" errRecordsNoScope = "records no scope" // errNotVerified is all that a caller learns about a token whose row disagrees with its claims errNotVerified = "API token could not be verified" @@ -201,7 +201,7 @@ func TestWithCurrentAPITokenAndOrgMiddleware(t *testing.T) { receivedToken: true, audience: apitoken.Audience, tokenExists: true, - extraClaims: jwt.MapClaims{claimScopeType: "product", claimScopeID: uuid.NewString()}, + extraClaims: jwt.MapClaims{claimScope: "product", claimScopeID: uuid.NewString()}, wantErr: true, wantErrContains: errNotVerified, }, @@ -325,7 +325,7 @@ func TestWithCurrentAPITokenAndOrgMiddlewareCarriesScope(t *testing.T) { rowScope: biz.ToPtr(authz.ResourceTypeProduct), rowScopeID: &productID, rowProjectIDs: []uuid.UUID{projectA}, - claims: jwt.MapClaims{claimOrgID: orgID.String(), claimScopeType: "product", claimScopeID: productID.String()}, + claims: jwt.MapClaims{claimOrgID: orgID.String(), claimScope: "product", claimScopeID: productID.String()}, }, { name: "an organization token with legacy claims", @@ -337,7 +337,7 @@ func TestWithCurrentAPITokenAndOrgMiddlewareCarriesScope(t *testing.T) { { name: "an instance token", rowScope: biz.ToPtr(authz.ResourceTypeInstance), - claims: jwt.MapClaims{claimOrgID: "", "scope": authz.ScopeInstanceAdmin, claimScopeType: "instance"}, + claims: jwt.MapClaims{claimOrgID: "", claimScope: string(authz.ResourceTypeInstance)}, }, } @@ -398,7 +398,7 @@ func TestWithCurrentAPITokenAndOrgMiddlewareLogsAClaimsMismatch(t *testing.T) { claims := jwt.MapClaims{ claimAud: apitoken.Audience, claimJTI: token.ID.String(), claimOrgID: orgID.String(), - claimScopeType: string(authz.ResourceTypeProduct), claimScopeID: uuid.NewString(), + claimScope: string(authz.ResourceTypeProduct), claimScopeID: uuid.NewString(), "raw": signedToken, } @@ -465,7 +465,7 @@ func TestAPITokenMiddlewaresIgnoreAProductClaim(t *testing.T) { RegisteredClaims: jwt.RegisteredClaims{ID: token.ID.String(), Issuer: testIssuer, Audience: jwt.ClaimStrings{apitoken.Audience}}, } if tc.signScope { - jwtClaims.ScopeType, jwtClaims.ScopeID = string(*tc.rowScope), tc.rowScopeID.String() + jwtClaims.Scope, jwtClaims.ScopeID = string(*tc.rowScope), tc.rowScopeID.String() } signed, err := jwt.NewWithClaims(apitoken.SigningMethod, preProductClaimRemoval{ @@ -511,12 +511,15 @@ func TestAPITokenMiddlewaresResolveInstanceAdminTokens(t *testing.T) { wantOrg *biz.Organization wantErr string }{ - {name: "an instance-admin token takes the organization in the header", scopeClaim: authz.ScopeInstanceAdmin, header: headerOrg.Name, wantOrg: headerOrg}, - {name: "an instance-admin token without the header has no organization", scopeClaim: authz.ScopeInstanceAdmin}, + {name: "an instance token takes the organization in the header", scopeClaim: string(authz.ResourceTypeInstance), header: headerOrg.Name, wantOrg: headerOrg}, + {name: "an instance token without the header has no organization", scopeClaim: string(authz.ResourceTypeInstance)}, + {name: "an older instance-admin token takes the organization in the header", scopeClaim: authz.ScopeInstanceAdmin, header: headerOrg.Name, wantOrg: headerOrg}, + {name: "an older instance-admin token without the header has no organization", scopeClaim: authz.ScopeInstanceAdmin}, {name: "an organization token takes its row's organization", rowOrg: rowOrg, header: headerOrg.Name, wantOrg: rowOrg}, // The signed claims and the row must agree, and the claims must agree with themselves. {name: "an instance row whose JWT names no scope is refused", header: headerOrg.Name, wantErr: errNotVerified}, {name: "an organization row whose JWT carries the instance-admin claim is refused", scopeClaim: authz.ScopeInstanceAdmin, rowOrg: rowOrg, header: headerOrg.Name, wantErr: errNotVerified}, + {name: "an organization row whose JWT names the instance scope is refused", scopeClaim: string(authz.ResourceTypeInstance), rowOrg: rowOrg, header: headerOrg.Name, wantErr: errNotVerified}, } for _, tc := range testCases { diff --git a/app/controlplane/pkg/authz/authz.go b/app/controlplane/pkg/authz/authz.go index 471338e60..89ab52bf3 100644 --- a/app/controlplane/pkg/authz/authz.go +++ b/app/controlplane/pkg/authz/authz.go @@ -103,7 +103,8 @@ const ( RoleProductViewer Role = "role:product:viewer" RoleProductAdmin Role = "role:product:admin" - // Scope for instance admin tokens + // ScopeInstanceAdmin is the scope claim of an instance token minted before the control plane + // signed the scope kind. A newer instance token has ResourceTypeInstance in that claim. ScopeInstanceAdmin = "INSTANCE_ADMIN" ) diff --git a/app/controlplane/pkg/biz/apitoken.go b/app/controlplane/pkg/biz/apitoken.go index 0abe1369f..a106e506e 100644 --- a/app/controlplane/pkg/biz/apitoken.go +++ b/app/controlplane/pkg/biz/apitoken.go @@ -574,16 +574,14 @@ func (uc *APITokenUseCase) Create(ctx context.Context, name string, description KeyName: name, ExpiresAt: expiresAt, // The JWT signs the scope. The row must then match it. - ScopeType: scope, - ScopeID: scopeID, + Scope: scope, + ScopeID: scopeID, } - // Set org info if available or instance-level token scope + // An instance-level token has no organization if org != nil { generationOpts.OrgID = &token.OrganizationID generationOpts.OrgName = &org.Name - } else { - generationOpts.Scope = ToPtr(authz.ScopeInstanceAdmin) } if projectID != nil { @@ -654,22 +652,18 @@ func (uc *APITokenUseCase) RegenerateJWT(ctx context.Context, tokenID uuid.UUID, KeyID: token.ID, KeyName: token.Name, ExpiresAt: &expiresAt, - ScopeType: token.Scope, + Scope: token.Scope, ScopeID: token.ScopeID, } - // Check if this is an org-scoped or instance-level token + // An instance-level token has no organization if token.OrganizationID != uuid.Nil { - // Org-scoped token org, err := uc.orgUseCase.FindByID(ctx, token.OrganizationID.String()) if err != nil { return nil, fmt.Errorf("finding organization: %w", err) } generationOpts.OrgID = &token.OrganizationID generationOpts.OrgName = &org.Name - } else { - // Instance-level token - generationOpts.Scope = ToPtr(authz.ScopeInstanceAdmin) } // Preserve project / workflow scope claims that the row carries. diff --git a/app/controlplane/pkg/biz/apitoken_integration_test.go b/app/controlplane/pkg/biz/apitoken_integration_test.go index 8505fde79..bf4331d34 100644 --- a/app/controlplane/pkg/biz/apitoken_integration_test.go +++ b/app/controlplane/pkg/biz/apitoken_integration_test.go @@ -1278,19 +1278,17 @@ func (s *apiTokenTestSuite) TestGeneratedJWTSignsTheTokenScope() { } testCases := []struct { - name string - org *string - opts []biz.APITokenCreateOpt - wantScopeType authz.ResourceType - wantScopeID string - // wantScope is the legacy "scope" claim - wantScope string + name string + org *string + opts []biz.APITokenCreateOpt + wantScope authz.ResourceType + wantScopeID string }{ - {name: "organization", org: &s.org.ID, wantScopeType: authz.ResourceTypeOrganization, wantScopeID: s.org.ID}, - {name: string(authz.ResourceTypeProject), org: &s.org.ID, opts: []biz.APITokenCreateOpt{biz.APITokenWithProject(s.p1)}, wantScopeType: authz.ResourceTypeProject, wantScopeID: s.p1.ID.String()}, - {name: "workflow-pinned", org: &s.org.ID, opts: []biz.APITokenCreateOpt{biz.APITokenWithProject(s.p1), biz.APITokenWithWorkflow(wf)}, wantScopeType: authz.ResourceTypeProject, wantScopeID: s.p1.ID.String()}, - {name: "product", org: &s.org.ID, opts: []biz.APITokenCreateOpt{biz.APITokenWithScope(authz.ResourceTypeProduct, &productID), biz.APITokenWithProjectIDs(nil)}, wantScopeType: authz.ResourceTypeProduct, wantScopeID: productID.String()}, - {name: "instance", wantScopeType: authz.ResourceTypeInstance, wantScope: authz.ScopeInstanceAdmin}, + {name: "organization", org: &s.org.ID, wantScope: authz.ResourceTypeOrganization, wantScopeID: s.org.ID}, + {name: string(authz.ResourceTypeProject), org: &s.org.ID, opts: []biz.APITokenCreateOpt{biz.APITokenWithProject(s.p1)}, wantScope: authz.ResourceTypeProject, wantScopeID: s.p1.ID.String()}, + {name: "workflow-pinned", org: &s.org.ID, opts: []biz.APITokenCreateOpt{biz.APITokenWithProject(s.p1), biz.APITokenWithWorkflow(wf)}, wantScope: authz.ResourceTypeProject, wantScopeID: s.p1.ID.String()}, + {name: "product", org: &s.org.ID, opts: []biz.APITokenCreateOpt{biz.APITokenWithScope(authz.ResourceTypeProduct, &productID), biz.APITokenWithProjectIDs(nil)}, wantScope: authz.ResourceTypeProduct, wantScopeID: productID.String()}, + {name: "instance", wantScope: authz.ResourceTypeInstance}, } for _, tc := range testCases { @@ -1304,9 +1302,8 @@ func (s *apiTokenTestSuite) TestGeneratedJWTSignsTheTokenScope() { for minted, raw := range map[string]string{"created": created.JWT, "regenerated": regenerated.JWT} { payload, claims := claimsOf(raw) - s.Equal(string(tc.wantScopeType), claims.ScopeType, minted) + s.Equal(string(tc.wantScope), claims.Scope, minted) s.Equal(tc.wantScopeID, claims.ScopeID, minted) - s.Equal(tc.wantScope, claims.Scope, minted) s.NoError(stored.VerifyClaims(claims), minted) // What changes during a token's life never goes into the JWT diff --git a/app/controlplane/pkg/biz/apitoken_verify_claims_test.go b/app/controlplane/pkg/biz/apitoken_verify_claims_test.go index e92ab9ba3..da856c667 100644 --- a/app/controlplane/pkg/biz/apitoken_verify_claims_test.go +++ b/app/controlplane/pkg/biz/apitoken_verify_claims_test.go @@ -49,10 +49,10 @@ func TestAPITokenVerifyClaims(t *testing.T) { productRow := &APIToken{OrganizationID: org, Scope: &productKind, ScopeID: &product, ProjectIDs: []uuid.UUID{project}} instanceRow := &APIToken{Scope: &instanceKind} - signedOrg := apitoken.CustomClaims{OrgID: org.String(), ScopeType: string(authz.ResourceTypeOrganization), ScopeID: org.String()} - signedProject := apitoken.CustomClaims{OrgID: org.String(), ProjectID: project.String(), ScopeType: string(authz.ResourceTypeProject), ScopeID: project.String()} - signedWorkflow := apitoken.CustomClaims{OrgID: org.String(), ProjectID: project.String(), WorkflowID: workflow.String(), ScopeType: string(authz.ResourceTypeProject), ScopeID: project.String()} - signedProduct := apitoken.CustomClaims{OrgID: org.String(), ScopeType: string(authz.ResourceTypeProduct), ScopeID: product.String()} + signedOrg := apitoken.CustomClaims{OrgID: org.String(), Scope: string(authz.ResourceTypeOrganization), ScopeID: org.String()} + signedProject := apitoken.CustomClaims{OrgID: org.String(), ProjectID: project.String(), Scope: string(authz.ResourceTypeProject), ScopeID: project.String()} + signedWorkflow := apitoken.CustomClaims{OrgID: org.String(), ProjectID: project.String(), WorkflowID: workflow.String(), Scope: string(authz.ResourceTypeProject), ScopeID: project.String()} + signedProduct := apitoken.CustomClaims{OrgID: org.String(), Scope: string(authz.ResourceTypeProduct), ScopeID: product.String()} legacyOrg := apitoken.CustomClaims{OrgID: org.String()} legacyProject := apitoken.CustomClaims{OrgID: org.String(), ProjectID: project.String()} legacyWorkflow := apitoken.CustomClaims{OrgID: org.String(), ProjectID: project.String(), WorkflowID: workflow.String()} @@ -72,7 +72,7 @@ func TestAPITokenVerifyClaims(t *testing.T) { {name: "signed project token", row: projectRow, claims: signedProject}, {name: "signed workflow-pinned token", row: workflowRow, claims: signedWorkflow}, {name: "signed product token", row: productRow, claims: signedProduct}, - {name: "signed instance token", row: instanceRow, claims: apitoken.CustomClaims{Scope: authz.ScopeInstanceAdmin, ScopeType: string(authz.ResourceTypeInstance)}}, + {name: "signed instance token", row: instanceRow, claims: apitoken.CustomClaims{Scope: string(authz.ResourceTypeInstance)}}, {name: "legacy organization token", row: orgRow, claims: legacyOrg}, {name: "legacy project token", row: projectRow, claims: legacyProject}, {name: "legacy workflow-pinned token", row: workflowRow, claims: legacyWorkflow}, @@ -81,7 +81,7 @@ func TestAPITokenVerifyClaims(t *testing.T) { {name: "a row recording no scope", row: &APIToken{OrganizationID: org}, claims: legacyOrg, wantErr: "records no scope"}, {name: "a signed token whose row's scope was cleared", row: &APIToken{OrganizationID: org}, claims: signedOrg, wantErr: "records no scope", mismatch: true}, {name: "a product token minted before the scope claims", row: productRow, claims: legacyOrg, wantErr: "create a new one"}, - {name: "malformed scope claims", row: orgRow, claims: apitoken.CustomClaims{OrgID: org.String(), ScopeType: string(authz.ResourceTypeOrganization)}, wantErr: "scope claims", mismatch: true}, + {name: "malformed scope claims", row: orgRow, claims: apitoken.CustomClaims{OrgID: org.String(), Scope: string(authz.ResourceTypeOrganization)}, wantErr: "scope claims", mismatch: true}, {name: "an instance-admin claim on an organization row", row: orgRow, claims: apitoken.CustomClaims{OrgID: org.String(), Scope: authz.ScopeInstanceAdmin}, wantErr: "scope claims", mismatch: true}, {name: "a project row widened to its organization, signed claims", row: widenedProjectRow, claims: signedProject, wantErr: errScopeMismatch, mismatch: true}, {name: "a project row widened to its organization, legacy claims", row: widenedProjectRow, claims: legacyProject, wantErr: errScopeMismatch, mismatch: true}, diff --git a/app/controlplane/pkg/jwt/apitoken/apitoken.go b/app/controlplane/pkg/jwt/apitoken/apitoken.go index 05ea344fc..2545e8a74 100644 --- a/app/controlplane/pkg/jwt/apitoken/apitoken.go +++ b/app/controlplane/pkg/jwt/apitoken/apitoken.go @@ -80,13 +80,10 @@ type GenerateJWTOptions struct { WorkflowID *uuid.UUID WorkflowName *string ExpiresAt *time.Time - // Scope is the legacy instance-admin claim. The platform and control planes up to v1.112 - // read it, so instance tokens keep carrying it. - Scope *string - // ScopeType and ScopeID name the token's scope, as its row records it. ScopeType is required. + // Scope and ScopeID name the token's scope, as its row records it. Scope is required. // ScopeID is unset only for an instance token. - ScopeType *authz.ResourceType - ScopeID *uuid.UUID + Scope *authz.ResourceType + ScopeID *uuid.UUID } // GenerateJWT creates a new JWT token for the given organization and keyID @@ -118,10 +115,6 @@ func (ra *Builder) GenerateJWT(opts *GenerateJWTOptions) (string, error) { claims.OrgName = *opts.OrgName } - if opts.Scope != nil { - claims.Scope = *opts.Scope - } - if opts.ProjectID != nil { claims.ProjectID = opts.ProjectID.String() claims.ProjectName = *opts.ProjectName @@ -135,15 +128,20 @@ func (ra *Builder) GenerateJWT(opts *GenerateJWTOptions) (string, error) { claims.WorkflowName = *opts.WorkflowName } - if opts.ScopeType == nil || *opts.ScopeType == "" { - return "", errors.New("scopeType is required") + if opts.Scope == nil || *opts.Scope == "" { + return "", errors.New("scope is required") } - claims.ScopeType = string(*opts.ScopeType) + claims.Scope = string(*opts.Scope) if opts.ScopeID != nil { claims.ScopeID = opts.ScopeID.String() } + // The older instance-admin value would read as a token minted before the scope was signed + if !claims.HasScopeClaims() { + return "", fmt.Errorf("invalid scope %q", claims.Scope) + } + // Never sign a token whose claims contradict themselves if _, _, err := claims.GetScope(); err != nil { return "", fmt.Errorf("inconsistent token scope: %w", err) @@ -166,24 +164,25 @@ type CustomClaims struct { ProjectName string `json:"project_name,omitempty"` WorkflowID string `json:"workflow_id,omitempty"` WorkflowName string `json:"workflow_name,omitempty"` - // Scope is the older instance-admin claim ("INSTANCE_ADMIN"), set only on instance tokens. - // Despite its name it is not the token's scope: that is ScopeType and ScopeID. - Scope string `json:"scope,omitempty"` - // ScopeType and ScopeID say what the token was granted. A token minted before they existed - // carries neither, and GetScope derives its scope from the claims that it does carry. - ScopeType string `json:"scope_type,omitempty"` - ScopeID string `json:"scope_id,omitempty"` + // Scope and ScopeID say what the token was granted. Scope is the kind: instance, organization, + // project or product. ScopeID names the resource, and an instance scope names none. + // + // A token minted before the control plane signed its scope has no scope claim. An older + // instance token has the value "INSTANCE_ADMIN" in it instead. GetScope derives the scope of + // such a token from the claims that it does carry. + Scope string `json:"scope,omitempty"` + ScopeID string `json:"scope_id,omitempty"` jwt.RegisteredClaims } -// HasScopeClaims reports whether the token was signed with the scope_type claim. A token without -// it was minted before the scope claims existed. +// HasScopeClaims reports whether the token was signed with its scope kind in the scope claim. A +// token without it was minted before the control plane signed the scope. func (c *CustomClaims) HasScopeClaims() bool { - return c.ScopeType != "" + return c.Scope != "" && c.Scope != authz.ScopeInstanceAdmin } -// GetScope returns the scope that the claims bind the token to. For a token with the scope_type -// and scope_id claims, that scope is what they name. For an older token, it is the scope that its +// GetScope returns the scope that the claims bind the token to. For a token with the scope and +// scope_id claims, that scope is what they name. For an older token, it is the scope that its // other claims imply (see legacyScope). An older product token therefore gets its organization as // its scope. GetScope returns an error for claims that contradict themselves. func (c *CustomClaims) GetScope() (authz.ResourceType, *uuid.UUID, error) { @@ -199,14 +198,14 @@ func (c *CustomClaims) GetScope() (authz.ResourceType, *uuid.UUID, error) { return kind, id, nil } -// namedScope is the scope the scope_type and scope_id claims name, else the one the claims of an -// older token imply. +// namedScope is the scope the scope and scope_id claims name, else the one the claims of an older +// token imply. func (c *CustomClaims) namedScope() (authz.ResourceType, *uuid.UUID, error) { if !c.HasScopeClaims() { return c.legacyScope() } - kind := authz.ResourceType(c.ScopeType) + kind := authz.ResourceType(c.Scope) switch kind { case authz.ResourceTypeInstance: if c.ScopeID != "" { @@ -222,13 +221,13 @@ func (c *CustomClaims) namedScope() (authz.ResourceType, *uuid.UUID, error) { return kind, &id, nil default: - return "", nil, fmt.Errorf("unknown scope_type claim %q", c.ScopeType) + return "", nil, fmt.Errorf("unknown scope claim %q", c.Scope) } } // legacyScope returns the scope that the claims of an older token imply. An older token is a token -// minted before the scope_type claim existed. Its scope is the instance for the instance-admin -// claim, else its project, else its organization. biz.newTokenScope and the scope backfill +// minted before the control plane signed the scope. Its scope is the instance for the +// instance-admin value, else its project, else its organization. biz.newTokenScope and the scope backfill // migration apply the same rule to the row, so the two agree. func (c *CustomClaims) legacyScope() (authz.ResourceType, *uuid.UUID, error) { var kind authz.ResourceType @@ -254,17 +253,12 @@ func (c *CustomClaims) legacyScope() (authz.ResourceType, *uuid.UUID, error) { // agreesWith checks that the other claims fit the scope. This makes the control plane and the // platform read the same scope from the token. The rules are: -// - Only an instance token carries the instance-admin claim. It names no organization and no -// project. +// - An instance token names no organization and no project. // - An organization token names its own organization and no project. // - A project token names its organization and the project of its scope. // - A product token names its organization and no project. // - A workflow claim always comes with a project claim. func (c *CustomClaims) agreesWith(kind authz.ResourceType, id *uuid.UUID) error { - if (c.Scope == authz.ScopeInstanceAdmin) != (kind == authz.ResourceTypeInstance) { - return errors.New("the instance-admin claim does not agree with the scope") - } - if c.WorkflowID != "" && c.ProjectID == "" { return errors.New("a workflow claim needs a project claim") } diff --git a/app/controlplane/pkg/jwt/apitoken/apitoken_test.go b/app/controlplane/pkg/jwt/apitoken/apitoken_test.go index 9fb56758e..66a542c8b 100644 --- a/app/controlplane/pkg/jwt/apitoken/apitoken_test.go +++ b/app/controlplane/pkg/jwt/apitoken/apitoken_test.go @@ -92,80 +92,86 @@ func TestGenerateJWT(t *testing.T) { { name: "organization token", opts: &GenerateJWTOptions{OrgID: &org, OrgName: toPtr("org-name"), KeyName: testKeyName, KeyID: keyID, - ExpiresAt: toPtr(time.Now().Add(1 * time.Hour)), ScopeType: &orgScope, ScopeID: &org}, + ExpiresAt: toPtr(time.Now().Add(1 * time.Hour)), Scope: &orgScope, ScopeID: &org}, }, { name: "no expiration", opts: &GenerateJWTOptions{OrgID: &org, OrgName: toPtr("org-name"), KeyName: testKeyName, KeyID: keyID, - ScopeType: &orgScope, ScopeID: &org}, + Scope: &orgScope, ScopeID: &org}, }, { name: "project token", opts: &GenerateJWTOptions{OrgID: &org, OrgName: toPtr("org-name"), KeyName: testKeyName, KeyID: keyID, ProjectID: &project, ProjectName: toPtr("project-name"), ExpiresAt: toPtr(time.Now().Add(1 * time.Hour)), - ScopeType: &projectScope, ScopeID: &project}, + Scope: &projectScope, ScopeID: &project}, }, { name: "workflow-pinned token", opts: &GenerateJWTOptions{OrgID: &org, OrgName: toPtr("org-name"), KeyName: testKeyName, KeyID: keyID, ProjectID: &project, ProjectName: toPtr("project-name"), WorkflowID: &workflow, WorkflowName: toPtr("workflow-name"), - ExpiresAt: toPtr(time.Now().Add(1 * time.Hour)), ScopeType: &projectScope, ScopeID: &project}, + ExpiresAt: toPtr(time.Now().Add(1 * time.Hour)), Scope: &projectScope, ScopeID: &project}, }, { name: "product token", opts: &GenerateJWTOptions{OrgID: &org, OrgName: toPtr("org-name"), KeyName: testKeyName, KeyID: keyID, - ScopeType: &productScope, ScopeID: &product}, + Scope: &productScope, ScopeID: &product}, }, { - name: "instance token keeps the instance-admin claim", + name: "instance token", opts: &GenerateJWTOptions{KeyName: testKeyName, KeyID: keyID, ExpiresAt: toPtr(time.Now().Add(1 * time.Hour)), - Scope: toPtr("INSTANCE_ADMIN"), ScopeType: &instanceScope}, + Scope: &instanceScope}, }, { name: "missing keyID", opts: &GenerateJWTOptions{OrgID: &org, OrgName: toPtr("org-name"), KeyName: testKeyName, - ScopeType: &orgScope, ScopeID: &org}, + Scope: &orgScope, ScopeID: &org}, wantErr: true, }, { name: "missing keyName", opts: &GenerateJWTOptions{OrgID: &org, OrgName: toPtr("org-name"), KeyID: keyID, - ScopeType: &orgScope, ScopeID: &org}, + Scope: &orgScope, ScopeID: &org}, wantErr: true, }, { - name: "a scope id without a scope type", + name: "a scope id without a scope", opts: &GenerateJWTOptions{OrgID: &org, OrgName: toPtr("org-name"), KeyName: testKeyName, KeyID: keyID, ScopeID: &org}, wantErr: true, }, { - name: "missing scope type", + name: "missing scope", opts: &GenerateJWTOptions{OrgID: &org, OrgName: toPtr("org-name"), KeyName: testKeyName, KeyID: keyID}, wantErr: true, }, { - // An empty scope type would sign a token without the scope claims, like an older token - name: "an empty scope type", + // An empty scope would sign a token without the scope claims, like an older token + name: "an empty scope", opts: &GenerateJWTOptions{OrgID: &org, OrgName: toPtr("org-name"), KeyName: testKeyName, KeyID: keyID, - ScopeType: toPtr(authz.ResourceType("")), ScopeID: &org}, + Scope: toPtr(authz.ResourceType("")), ScopeID: &org}, wantErr: true, }, { name: "an organization scope naming another organization", opts: &GenerateJWTOptions{OrgID: &org, OrgName: toPtr("org-name"), KeyName: testKeyName, KeyID: keyID, - ScopeType: &orgScope, ScopeID: &product}, + Scope: &orgScope, ScopeID: &product}, wantErr: true, }, { - name: "the instance-admin claim on an organization scope", + // The older instance-admin value would read as a token minted before the scope was signed + name: "the older instance-admin value as the scope", + opts: &GenerateJWTOptions{KeyName: testKeyName, KeyID: keyID, Scope: toPtr(authz.ResourceType(authz.ScopeInstanceAdmin))}, + wantErr: true, + }, + { + name: "an instance scope naming an organization", opts: &GenerateJWTOptions{OrgID: &org, OrgName: toPtr("org-name"), KeyName: testKeyName, KeyID: keyID, - Scope: toPtr("INSTANCE_ADMIN"), ScopeType: &orgScope, ScopeID: &org}, + Scope: &instanceScope}, wantErr: true, }, { name: "a project scope naming another project", opts: &GenerateJWTOptions{OrgID: &org, OrgName: toPtr("org-name"), KeyName: testKeyName, KeyID: keyID, - ProjectID: &project, ProjectName: toPtr("project-name"), ScopeType: &projectScope, ScopeID: &product}, + ProjectID: &project, ProjectName: toPtr("project-name"), Scope: &projectScope, ScopeID: &product}, wantErr: true, }, } @@ -222,13 +228,7 @@ func TestGenerateJWT(t *testing.T) { assert.Empty(t, claims.WorkflowName) } - if tc.opts.Scope != nil { - assert.Equal(t, *tc.opts.Scope, claims.Scope) - } else { - assert.Empty(t, claims.Scope) - } - - assert.Equal(t, string(*tc.opts.ScopeType), claims.ScopeType) + assert.Equal(t, string(*tc.opts.Scope), claims.Scope) if tc.opts.ScopeID != nil { assert.Equal(t, tc.opts.ScopeID.String(), claims.ScopeID) } else { @@ -255,36 +255,34 @@ func TestGetScope(t *testing.T) { wantID *uuid.UUID wantErr bool }{ - {name: "signed organization scope", claims: CustomClaims{OrgID: org.String(), ScopeType: string(authz.ResourceTypeOrganization), ScopeID: org.String()}, wantKind: authz.ResourceTypeOrganization, wantID: &org}, - {name: "signed project scope", claims: CustomClaims{OrgID: org.String(), ProjectID: project.String(), ScopeType: string(authz.ResourceTypeProject), ScopeID: project.String()}, wantKind: authz.ResourceTypeProject, wantID: &project}, - {name: "signed workflow-pinned scope", claims: CustomClaims{OrgID: org.String(), ProjectID: project.String(), WorkflowID: workflow.String(), ScopeType: string(authz.ResourceTypeProject), ScopeID: project.String()}, wantKind: authz.ResourceTypeProject, wantID: &project}, - {name: "signed product scope", claims: CustomClaims{OrgID: org.String(), ScopeType: string(authz.ResourceTypeProduct), ScopeID: product.String()}, wantKind: authz.ResourceTypeProduct, wantID: &product}, - {name: "signed instance scope", claims: CustomClaims{Scope: authz.ScopeInstanceAdmin, ScopeType: string(authz.ResourceTypeInstance)}, wantKind: authz.ResourceTypeInstance}, + {name: "signed organization scope", claims: CustomClaims{OrgID: org.String(), Scope: string(authz.ResourceTypeOrganization), ScopeID: org.String()}, wantKind: authz.ResourceTypeOrganization, wantID: &org}, + {name: "signed project scope", claims: CustomClaims{OrgID: org.String(), ProjectID: project.String(), Scope: string(authz.ResourceTypeProject), ScopeID: project.String()}, wantKind: authz.ResourceTypeProject, wantID: &project}, + {name: "signed workflow-pinned scope", claims: CustomClaims{OrgID: org.String(), ProjectID: project.String(), WorkflowID: workflow.String(), Scope: string(authz.ResourceTypeProject), ScopeID: project.String()}, wantKind: authz.ResourceTypeProject, wantID: &project}, + {name: "signed product scope", claims: CustomClaims{OrgID: org.String(), Scope: string(authz.ResourceTypeProduct), ScopeID: product.String()}, wantKind: authz.ResourceTypeProduct, wantID: &product}, + {name: "signed instance scope", claims: CustomClaims{Scope: string(authz.ResourceTypeInstance)}, wantKind: authz.ResourceTypeInstance}, {name: "legacy instance-admin token", claims: CustomClaims{Scope: authz.ScopeInstanceAdmin}, wantKind: authz.ResourceTypeInstance}, {name: "legacy project token", claims: CustomClaims{OrgID: org.String(), ProjectID: project.String()}, wantKind: authz.ResourceTypeProject, wantID: &project}, {name: "legacy workflow-pinned token", claims: CustomClaims{OrgID: org.String(), ProjectID: project.String(), WorkflowID: workflow.String()}, wantKind: authz.ResourceTypeProject, wantID: &project}, {name: "legacy organization token", claims: CustomClaims{OrgID: org.String()}, wantKind: authz.ResourceTypeOrganization, wantID: &org}, // Malformed scope claims - {name: "an instance scope naming a resource", claims: CustomClaims{Scope: authz.ScopeInstanceAdmin, ScopeType: string(authz.ResourceTypeInstance), ScopeID: org.String()}, wantErr: true}, - {name: "a resource scope without an id", claims: CustomClaims{OrgID: org.String(), ProjectID: project.String(), ScopeType: string(authz.ResourceTypeProject)}, wantErr: true}, - {name: "a scope id that is not a uuid", claims: CustomClaims{OrgID: org.String(), ProjectID: project.String(), ScopeType: string(authz.ResourceTypeProject), ScopeID: "nope"}, wantErr: true}, - {name: "a scope type no token has", claims: CustomClaims{OrgID: org.String(), ScopeType: "group", ScopeID: org.String()}, wantErr: true}, + {name: "an instance scope naming a resource", claims: CustomClaims{Scope: string(authz.ResourceTypeInstance), ScopeID: org.String()}, wantErr: true}, + {name: "a resource scope without an id", claims: CustomClaims{OrgID: org.String(), ProjectID: project.String(), Scope: string(authz.ResourceTypeProject)}, wantErr: true}, + {name: "a scope id that is not a uuid", claims: CustomClaims{OrgID: org.String(), ProjectID: project.String(), Scope: string(authz.ResourceTypeProject), ScopeID: "nope"}, wantErr: true}, + {name: "a scope no token has", claims: CustomClaims{OrgID: org.String(), Scope: "group", ScopeID: org.String()}, wantErr: true}, {name: "legacy claims naming nothing", claims: CustomClaims{}, wantErr: true}, {name: "a legacy project claim that is not a uuid", claims: CustomClaims{OrgID: org.String(), ProjectID: "nope"}, wantErr: true}, // Claims that contradict themselves: every reader must see the same scope - {name: "the instance-admin claim on an organization scope", claims: CustomClaims{OrgID: org.String(), Scope: authz.ScopeInstanceAdmin, ScopeType: string(authz.ResourceTypeOrganization), ScopeID: org.String()}, wantErr: true}, - {name: "an instance scope without the instance-admin claim", claims: CustomClaims{ScopeType: string(authz.ResourceTypeInstance)}, wantErr: true}, - {name: "an instance scope naming an organization", claims: CustomClaims{OrgID: org.String(), Scope: authz.ScopeInstanceAdmin, ScopeType: string(authz.ResourceTypeInstance)}, wantErr: true}, + {name: "an instance scope naming an organization", claims: CustomClaims{OrgID: org.String(), Scope: string(authz.ResourceTypeInstance)}, wantErr: true}, {name: "a legacy instance-admin claim naming an organization", claims: CustomClaims{OrgID: org.String(), Scope: authz.ScopeInstanceAdmin}, wantErr: true}, - {name: "an organization scope naming another organization", claims: CustomClaims{OrgID: org.String(), ScopeType: string(authz.ResourceTypeOrganization), ScopeID: other.String()}, wantErr: true}, - {name: "an organization scope with a project claim", claims: CustomClaims{OrgID: org.String(), ProjectID: project.String(), ScopeType: string(authz.ResourceTypeOrganization), ScopeID: org.String()}, wantErr: true}, - {name: "a project scope naming another project", claims: CustomClaims{OrgID: org.String(), ProjectID: project.String(), ScopeType: string(authz.ResourceTypeProject), ScopeID: other.String()}, wantErr: true}, - {name: "a project scope without an organization", claims: CustomClaims{ProjectID: project.String(), ScopeType: string(authz.ResourceTypeProject), ScopeID: project.String()}, wantErr: true}, - {name: "a product scope with a project claim", claims: CustomClaims{OrgID: org.String(), ProjectID: project.String(), ScopeType: string(authz.ResourceTypeProduct), ScopeID: product.String()}, wantErr: true}, - {name: "a product scope without an organization", claims: CustomClaims{ScopeType: string(authz.ResourceTypeProduct), ScopeID: product.String()}, wantErr: true}, - {name: "a workflow claim without a project claim", claims: CustomClaims{OrgID: org.String(), WorkflowID: workflow.String(), ScopeType: string(authz.ResourceTypeOrganization), ScopeID: org.String()}, wantErr: true}, + {name: "an organization scope naming another organization", claims: CustomClaims{OrgID: org.String(), Scope: string(authz.ResourceTypeOrganization), ScopeID: other.String()}, wantErr: true}, + {name: "an organization scope with a project claim", claims: CustomClaims{OrgID: org.String(), ProjectID: project.String(), Scope: string(authz.ResourceTypeOrganization), ScopeID: org.String()}, wantErr: true}, + {name: "a project scope naming another project", claims: CustomClaims{OrgID: org.String(), ProjectID: project.String(), Scope: string(authz.ResourceTypeProject), ScopeID: other.String()}, wantErr: true}, + {name: "a project scope without an organization", claims: CustomClaims{ProjectID: project.String(), Scope: string(authz.ResourceTypeProject), ScopeID: project.String()}, wantErr: true}, + {name: "a product scope with a project claim", claims: CustomClaims{OrgID: org.String(), ProjectID: project.String(), Scope: string(authz.ResourceTypeProduct), ScopeID: product.String()}, wantErr: true}, + {name: "a product scope without an organization", claims: CustomClaims{Scope: string(authz.ResourceTypeProduct), ScopeID: product.String()}, wantErr: true}, + {name: "a workflow claim without a project claim", claims: CustomClaims{OrgID: org.String(), WorkflowID: workflow.String(), Scope: string(authz.ResourceTypeOrganization), ScopeID: org.String()}, wantErr: true}, } for _, tc := range testCases { @@ -313,11 +311,11 @@ func TestClaimsFromMap(t *testing.T) { name: "every claim is read", in: jwt.MapClaims{ claimJTI: "id", "aud": []any{Audience}, "org_id": "o", "org_name": "on", "token_name": "t", - "project_id": "p", "workflow_id": "w", "scope": authz.ScopeInstanceAdmin, "scope_type": "project", "scope_id": "s", + "project_id": "p", "workflow_id": "w", "scope": "project", "scope_id": "s", }, want: &CustomClaims{ OrgID: "o", OrgName: "on", KeyName: "t", ProjectID: "p", WorkflowID: "w", - Scope: authz.ScopeInstanceAdmin, ScopeType: string(authz.ResourceTypeProject), ScopeID: "s", + Scope: string(authz.ResourceTypeProject), ScopeID: "s", RegisteredClaims: jwt.RegisteredClaims{ID: "id", Audience: jwt.ClaimStrings{Audience}}, }, }, From db244c0fd87ceea98e26f3e7d20533a87e1b237b Mon Sep 17 00:00:00 2001 From: Javier Rodriguez Date: Tue, 6 Oct 2026 12:19:47 +0200 Subject: [PATCH 16/17] refactor(controlplane): read the API token scope from its claims first, then compare the row VerifyClaims now treats the signed claims as the source of truth. It reads the scope from them first, and then compares the row with it: the scope, the organization, the project and the workflow. The special cases for an older product token and for a row that records no scope are gone. Both are now an ordinary difference between the row and the claims. The token builder now checks the scope against the four scope kinds, and its comment on the claims check names an example. Assisted-by: Claude Code Signed-off-by: Javier Rodriguez Chainloop-Trace-Sessions: 796bcbff-1898-452d-aaeb-b5311bffdfa6 --- .../apitoken_middleware_integration_test.go | 23 ++++++++---- .../usercontext/apitoken_middleware_test.go | 19 +++++----- app/controlplane/pkg/biz/apitoken.go | 37 ++++--------------- .../pkg/biz/apitoken_verify_claims_test.go | 7 ++-- app/controlplane/pkg/jwt/apitoken/apitoken.go | 16 ++++---- 5 files changed, 45 insertions(+), 57 deletions(-) diff --git a/app/controlplane/internal/usercontext/apitoken_middleware_integration_test.go b/app/controlplane/internal/usercontext/apitoken_middleware_integration_test.go index 9045b51f4..bc5af79d0 100644 --- a/app/controlplane/internal/usercontext/apitoken_middleware_integration_test.go +++ b/app/controlplane/internal/usercontext/apitoken_middleware_integration_test.go @@ -17,6 +17,7 @@ package usercontext import ( "context" + "errors" "io" "os" "testing" @@ -168,8 +169,9 @@ func signLegacy(t *testing.T, tokenID uuid.UUID, claims jwt.MapClaims) string { // Organization, project, workflow-pinned and instance tokens minted before the scope claims keep // working, with the scope they always had. The scope backfill gave their rows that scope, and their -// claims imply the same scope. Both entry points refuse two cases: -// - a product token minted before the scope claims. +// claims imply the same scope. Both entry points refuse two cases, because the row does not record +// the scope that the claims imply: +// - a product token minted before the scope claims. Its claims imply its organization. // - a row that a control plane wrote with no scope after the backfill ran. func TestAPITokenMiddlewareAcceptsTokensMintedBeforeTheScopeClaims(t *testing.T) { if !testhelpers.IntegrationTestsEnabled() { @@ -209,7 +211,9 @@ func TestAPITokenMiddlewareAcceptsTokensMintedBeforeTheScopeClaims(t *testing.T) // scope columns does this during a rolling upgrade or after a rollback. afterBackfill bool - wantErr string + wantErr string + // mismatch marks a refusal because the row disagrees with the claims + mismatch bool wantScope authz.ResourceType wantScopeID *uuid.UUID wantOrg *uuid.UUID @@ -261,8 +265,9 @@ func TestAPITokenMiddlewareAcceptsTokensMintedBeforeTheScopeClaims(t *testing.T) row: func(c *ent.APITokenCreate) *ent.APITokenCreate { return c.SetOrganizationID(orgID).SetScope(authz.ResourceTypeProduct).SetScopeID(product).SetProjectIds([]uuid.UUID{project.ID}) }, - claims: orgClaims, - wantErr: "create a new one", + claims: orgClaims, + wantErr: errNotVerifiedAtEntry, + mismatch: true, }, { name: "revoked organization token", @@ -277,13 +282,15 @@ func TestAPITokenMiddlewareAcceptsTokensMintedBeforeTheScopeClaims(t *testing.T) row: func(c *ent.APITokenCreate) *ent.APITokenCreate { return c.SetOrganizationID(orgID) }, claims: orgClaims, afterBackfill: true, - wantErr: errRecordsNoScope, + wantErr: errNotVerifiedAtEntry, + mismatch: true, }, { name: "instance row written with no scope after the backfill", claims: instanceClaims, afterBackfill: true, - wantErr: errRecordsNoScope, + wantErr: errNotVerifiedAtEntry, + mismatch: true, }, } @@ -321,7 +328,7 @@ func TestAPITokenMiddlewareAcceptsTokensMintedBeforeTheScopeClaims(t *testing.T) for entry, got := range authenticateAtBothEntryPoints(t, tu, signLegacy(t, ids[i], tc.claims), tc.header) { if tc.wantErr != "" { assert.ErrorContains(t, got.err, tc.wantErr, entry) - assert.NotErrorIs(t, got.err, biz.ErrAPITokenClaimsMismatch, entry) + assert.Equal(t, tc.mismatch, errors.Is(got.err, biz.ErrAPITokenClaimsMismatch), entry) continue } diff --git a/app/controlplane/internal/usercontext/apitoken_middleware_test.go b/app/controlplane/internal/usercontext/apitoken_middleware_test.go index 20655fd63..506807536 100644 --- a/app/controlplane/internal/usercontext/apitoken_middleware_test.go +++ b/app/controlplane/internal/usercontext/apitoken_middleware_test.go @@ -42,14 +42,13 @@ import ( // Claim names, error substrings and entry point names that the tests of both entry points share. const ( - claimAud = "aud" - claimJTI = "jti" - claimOrgID = "org_id" - claimOrgName = "org_name" - claimProjectID = "project_id" - claimScopeID = "scope_id" - claimScope = "scope" - errRecordsNoScope = "records no scope" + claimAud = "aud" + claimJTI = "jti" + claimOrgID = "org_id" + claimOrgName = "org_name" + claimProjectID = "project_id" + claimScopeID = "scope_id" + claimScope = "scope" // errNotVerified is all that a caller learns about a token whose row disagrees with its claims errNotVerified = "API token could not be verified" // errNotVerifiedAtEntry is the whole error that both entry points return for such a token. The @@ -442,8 +441,8 @@ func TestAPITokenMiddlewaresIgnoreAProductClaim(t *testing.T) { {name: "the claim names the row's product", rowScope: &product, rowScopeID: &rowProduct, signScope: true, productClaim: rowProduct}, {name: "the claim names another product", rowScope: &product, rowScopeID: &rowProduct, signScope: true, productClaim: otherProduct}, {name: "the claim is on an organization-scoped row", rowScope: &organization, rowScopeID: &orgID, productClaim: orgID}, - {name: "a product token minted before the scope claims", rowScope: &product, rowScopeID: &rowProduct, productClaim: rowProduct, wantErr: "create a new one"}, - {name: "the claim is on a row that records no scope", productClaim: otherProduct, wantErr: errRecordsNoScope}, + {name: "a product token minted before the scope claims", rowScope: &product, rowScopeID: &rowProduct, productClaim: rowProduct, wantErr: errNotVerified}, + {name: "the claim is on a row that records no scope", productClaim: otherProduct, wantErr: errNotVerified}, } for _, tc := range testCases { diff --git a/app/controlplane/pkg/biz/apitoken.go b/app/controlplane/pkg/biz/apitoken.go index a106e506e..2075a76f9 100644 --- a/app/controlplane/pkg/biz/apitoken.go +++ b/app/controlplane/pkg/biz/apitoken.go @@ -167,15 +167,11 @@ func (t *APIToken) IsOrgScoped() bool { return t.scopeView().IsOrgScoped() } -// VerifyClaims checks that the token's row matches the signed claims. The row must have the same -// scope, organization, project and workflow. A project or workflow that only the row or only the -// claims name is also a difference. Any difference refuses the token, so a wrong row can never -// widen the token or move it elsewhere. -// -// A mismatch error wraps ErrAPITokenClaimsMismatch. Two expected states of older tokens get an -// error without it: -// - a row that records no scope, when the claims name no scope either. -// - a product token minted before the scope claims existed. Its owner must create a new token. +// VerifyClaims checks the token's row against its signed claims. The claims are the source of +// truth. VerifyClaims reads the scope from them first, and then the row must have the same scope, +// organization, project and workflow. A project or workflow that only the row or only the claims +// name is also a difference. Any difference refuses the token, so a wrong row can never widen the +// token or move it elsewhere. Every refusal for a difference wraps ErrAPITokenClaimsMismatch. func (t *APIToken) VerifyClaims(claims *apitoken.CustomClaims) error { if t == nil { return errors.New("API token not found") @@ -185,31 +181,14 @@ func (t *APIToken) VerifyClaims(claims *apitoken.CustomClaims) error { return errors.New("API token has no claims") } - if t.Scope == nil { - // Claims that name a scope show that the token had a scope when it was minted. So something - // cleared the row's scope later. Legacy claims name no scope, and a row from before the - // scope columns records no scope. - err := errors.New("API token records no scope") - if claims.HasScopeClaims() { - err = fmt.Errorf("%w: %w", err, ErrAPITokenClaimsMismatch) - } - - return err - } - - // This check runs before GetScope on purpose. An older product token's claims name only its - // organization. GetScope reads such claims as an organization scope. The comparison below - // would then log a security event instead of asking for a new token. - if !claims.HasScopeClaims() && t.IsProductScoped() { - return errors.New("API token was minted before its scope was signed, create a new one") - } - + // A token minted before the control plane signed its scope gets the scope that its other + // claims imply: INSTANCE_ADMIN, else its project_id, else its org_id. kind, id, err := claims.GetScope() if err != nil { return fmt.Errorf("API token scope claims: %w: %w", err, ErrAPITokenClaimsMismatch) } - if *t.Scope != kind || !sameScopeID(t.ScopeID, id) { + if t.Scope == nil || *t.Scope != kind || !sameScopeID(t.ScopeID, id) { return fmt.Errorf("API token scope mismatch: %w", ErrAPITokenClaimsMismatch) } diff --git a/app/controlplane/pkg/biz/apitoken_verify_claims_test.go b/app/controlplane/pkg/biz/apitoken_verify_claims_test.go index da856c667..103abe265 100644 --- a/app/controlplane/pkg/biz/apitoken_verify_claims_test.go +++ b/app/controlplane/pkg/biz/apitoken_verify_claims_test.go @@ -78,9 +78,10 @@ func TestAPITokenVerifyClaims(t *testing.T) { {name: "legacy workflow-pinned token", row: workflowRow, claims: legacyWorkflow}, {name: "legacy instance token", row: instanceRow, claims: apitoken.CustomClaims{Scope: authz.ScopeInstanceAdmin}}, - {name: "a row recording no scope", row: &APIToken{OrganizationID: org}, claims: legacyOrg, wantErr: "records no scope"}, - {name: "a signed token whose row's scope was cleared", row: &APIToken{OrganizationID: org}, claims: signedOrg, wantErr: "records no scope", mismatch: true}, - {name: "a product token minted before the scope claims", row: productRow, claims: legacyOrg, wantErr: "create a new one"}, + {name: "a row recording no scope", row: &APIToken{OrganizationID: org}, claims: legacyOrg, wantErr: errScopeMismatch, mismatch: true}, + {name: "a signed token whose row's scope was cleared", row: &APIToken{OrganizationID: org}, claims: signedOrg, wantErr: errScopeMismatch, mismatch: true}, + // Its claims imply its organization, which is not the product the row records + {name: "a product token minted before the scope claims", row: productRow, claims: legacyOrg, wantErr: errScopeMismatch, mismatch: true}, {name: "malformed scope claims", row: orgRow, claims: apitoken.CustomClaims{OrgID: org.String(), Scope: string(authz.ResourceTypeOrganization)}, wantErr: "scope claims", mismatch: true}, {name: "an instance-admin claim on an organization row", row: orgRow, claims: apitoken.CustomClaims{OrgID: org.String(), Scope: authz.ScopeInstanceAdmin}, wantErr: "scope claims", mismatch: true}, {name: "a project row widened to its organization, signed claims", row: widenedProjectRow, claims: signedProject, wantErr: errScopeMismatch, mismatch: true}, diff --git a/app/controlplane/pkg/jwt/apitoken/apitoken.go b/app/controlplane/pkg/jwt/apitoken/apitoken.go index 2545e8a74..dc26d1034 100644 --- a/app/controlplane/pkg/jwt/apitoken/apitoken.go +++ b/app/controlplane/pkg/jwt/apitoken/apitoken.go @@ -128,21 +128,23 @@ func (ra *Builder) GenerateJWT(opts *GenerateJWTOptions) (string, error) { claims.WorkflowName = *opts.WorkflowName } - if opts.Scope == nil || *opts.Scope == "" { + if opts.Scope == nil { return "", errors.New("scope is required") } + switch *opts.Scope { + case authz.ResourceTypeInstance, authz.ResourceTypeOrganization, authz.ResourceTypeProject, authz.ResourceTypeProduct: + default: + return "", fmt.Errorf("invalid scope %q", *opts.Scope) + } + claims.Scope = string(*opts.Scope) if opts.ScopeID != nil { claims.ScopeID = opts.ScopeID.String() } - // The older instance-admin value would read as a token minted before the scope was signed - if !claims.HasScopeClaims() { - return "", fmt.Errorf("invalid scope %q", claims.Scope) - } - - // Never sign a token whose claims contradict themselves + // Refuse claims that do not fit the scope, for example a project scope without the claim of + // that project. GetScope runs that check. if _, _, err := claims.GetScope(); err != nil { return "", fmt.Errorf("inconsistent token scope: %w", err) } From 71a4d6b12fe172dbf875f344891463c2e26959c7 Mon Sep 17 00:00:00 2001 From: Javier Rodriguez Date: Tue, 6 Oct 2026 13:01:42 +0200 Subject: [PATCH 17/17] refactor(controlplane): name the claims check validateScope and stop mentioning older product tokens Rename agreesWith to validateScope. Product tokens from before the signed scope are not in use, so the comments and test names no longer describe them as a kind of older token. A product row whose JWT names no scope is still refused, because such claims imply the organization. Assisted-by: Claude Code Signed-off-by: Javier Rodriguez Chainloop-Trace-Sessions: 796bcbff-1898-452d-aaeb-b5311bffdfa6 --- .../apitoken_middleware_integration_test.go | 4 ++-- .../internal/usercontext/apitoken_middleware_test.go | 2 +- .../pkg/biz/apitoken_verify_claims_test.go | 4 ++-- app/controlplane/pkg/jwt/apitoken/apitoken.go | 10 +++++----- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/app/controlplane/internal/usercontext/apitoken_middleware_integration_test.go b/app/controlplane/internal/usercontext/apitoken_middleware_integration_test.go index bc5af79d0..90debb828 100644 --- a/app/controlplane/internal/usercontext/apitoken_middleware_integration_test.go +++ b/app/controlplane/internal/usercontext/apitoken_middleware_integration_test.go @@ -171,7 +171,7 @@ func signLegacy(t *testing.T, tokenID uuid.UUID, claims jwt.MapClaims) string { // working, with the scope they always had. The scope backfill gave their rows that scope, and their // claims imply the same scope. Both entry points refuse two cases, because the row does not record // the scope that the claims imply: -// - a product token minted before the scope claims. Its claims imply its organization. +// - a product row whose JWT names no scope. Such claims imply the organization. // - a row that a control plane wrote with no scope after the backfill ran. func TestAPITokenMiddlewareAcceptsTokensMintedBeforeTheScopeClaims(t *testing.T) { if !testhelpers.IntegrationTestsEnabled() { @@ -261,7 +261,7 @@ func TestAPITokenMiddlewareAcceptsTokensMintedBeforeTheScopeClaims(t *testing.T) wantScope: authz.ResourceTypeInstance, }, { - name: "product token minted before the scope claims", + name: "product row whose JWT names no scope", row: func(c *ent.APITokenCreate) *ent.APITokenCreate { return c.SetOrganizationID(orgID).SetScope(authz.ResourceTypeProduct).SetScopeID(product).SetProjectIds([]uuid.UUID{project.ID}) }, diff --git a/app/controlplane/internal/usercontext/apitoken_middleware_test.go b/app/controlplane/internal/usercontext/apitoken_middleware_test.go index 506807536..7c966be58 100644 --- a/app/controlplane/internal/usercontext/apitoken_middleware_test.go +++ b/app/controlplane/internal/usercontext/apitoken_middleware_test.go @@ -441,7 +441,7 @@ func TestAPITokenMiddlewaresIgnoreAProductClaim(t *testing.T) { {name: "the claim names the row's product", rowScope: &product, rowScopeID: &rowProduct, signScope: true, productClaim: rowProduct}, {name: "the claim names another product", rowScope: &product, rowScopeID: &rowProduct, signScope: true, productClaim: otherProduct}, {name: "the claim is on an organization-scoped row", rowScope: &organization, rowScopeID: &orgID, productClaim: orgID}, - {name: "a product token minted before the scope claims", rowScope: &product, rowScopeID: &rowProduct, productClaim: rowProduct, wantErr: errNotVerified}, + {name: "a product row whose JWT names no scope", rowScope: &product, rowScopeID: &rowProduct, productClaim: rowProduct, wantErr: errNotVerified}, {name: "the claim is on a row that records no scope", productClaim: otherProduct, wantErr: errNotVerified}, } diff --git a/app/controlplane/pkg/biz/apitoken_verify_claims_test.go b/app/controlplane/pkg/biz/apitoken_verify_claims_test.go index 103abe265..eaf553c9e 100644 --- a/app/controlplane/pkg/biz/apitoken_verify_claims_test.go +++ b/app/controlplane/pkg/biz/apitoken_verify_claims_test.go @@ -80,8 +80,8 @@ func TestAPITokenVerifyClaims(t *testing.T) { {name: "a row recording no scope", row: &APIToken{OrganizationID: org}, claims: legacyOrg, wantErr: errScopeMismatch, mismatch: true}, {name: "a signed token whose row's scope was cleared", row: &APIToken{OrganizationID: org}, claims: signedOrg, wantErr: errScopeMismatch, mismatch: true}, - // Its claims imply its organization, which is not the product the row records - {name: "a product token minted before the scope claims", row: productRow, claims: legacyOrg, wantErr: errScopeMismatch, mismatch: true}, + // Claims without a scope name its organization, not the product that the row records + {name: "a product row whose claims name no scope", row: productRow, claims: legacyOrg, wantErr: errScopeMismatch, mismatch: true}, {name: "malformed scope claims", row: orgRow, claims: apitoken.CustomClaims{OrgID: org.String(), Scope: string(authz.ResourceTypeOrganization)}, wantErr: "scope claims", mismatch: true}, {name: "an instance-admin claim on an organization row", row: orgRow, claims: apitoken.CustomClaims{OrgID: org.String(), Scope: authz.ScopeInstanceAdmin}, wantErr: "scope claims", mismatch: true}, {name: "a project row widened to its organization, signed claims", row: widenedProjectRow, claims: signedProject, wantErr: errScopeMismatch, mismatch: true}, diff --git a/app/controlplane/pkg/jwt/apitoken/apitoken.go b/app/controlplane/pkg/jwt/apitoken/apitoken.go index dc26d1034..15738b57c 100644 --- a/app/controlplane/pkg/jwt/apitoken/apitoken.go +++ b/app/controlplane/pkg/jwt/apitoken/apitoken.go @@ -185,15 +185,15 @@ func (c *CustomClaims) HasScopeClaims() bool { // GetScope returns the scope that the claims bind the token to. For a token with the scope and // scope_id claims, that scope is what they name. For an older token, it is the scope that its -// other claims imply (see legacyScope). An older product token therefore gets its organization as -// its scope. GetScope returns an error for claims that contradict themselves. +// other claims imply (see legacyScope). GetScope returns an error for claims that do not fit the +// scope (see validateScope). func (c *CustomClaims) GetScope() (authz.ResourceType, *uuid.UUID, error) { kind, id, err := c.namedScope() if err != nil { return "", nil, err } - if err := c.agreesWith(kind, id); err != nil { + if err := c.validateScope(kind, id); err != nil { return "", nil, err } @@ -253,14 +253,14 @@ func (c *CustomClaims) legacyScope() (authz.ResourceType, *uuid.UUID, error) { return kind, &id, nil } -// agreesWith checks that the other claims fit the scope. This makes the control plane and the +// validateScope checks that the other claims fit the scope. This makes the control plane and the // platform read the same scope from the token. The rules are: // - An instance token names no organization and no project. // - An organization token names its own organization and no project. // - A project token names its organization and the project of its scope. // - A product token names its organization and no project. // - A workflow claim always comes with a project claim. -func (c *CustomClaims) agreesWith(kind authz.ResourceType, id *uuid.UUID) error { +func (c *CustomClaims) validateScope(kind authz.ResourceType, id *uuid.UUID) error { if c.WorkflowID != "" && c.ProjectID == "" { return errors.New("a workflow claim needs a project claim") }