diff --git a/components/control-plane/cmd/hypershell-controller/main.go b/components/control-plane/cmd/hypershell-controller/main.go index ec33cd122..611c2f89f 100644 --- a/components/control-plane/cmd/hypershell-controller/main.go +++ b/components/control-plane/cmd/hypershell-controller/main.go @@ -526,7 +526,7 @@ func main() { supervise("CIProvider watch", func(ctx context.Context) error { return watcher.WatchCIProviders(ctx, conn, ciProviderReconciler, cfg.WatchIdleTimeout) }) - ciTrustReconciler := reconciler.NewCITrustReconciler(kcClient, conn) + ciTrustReconciler := reconciler.NewCITrustReconciler(kcClient, conn, clusterID) supervise("CITrust watch", func(ctx context.Context) error { return watcher.WatchCITrusts(ctx, conn, ciTrustReconciler, cfg.WatchIdleTimeout) }) diff --git a/components/control-plane/internal/keycloak/ci_federation.go b/components/control-plane/internal/keycloak/ci_federation.go new file mode 100644 index 000000000..43ebc7613 --- /dev/null +++ b/components/control-plane/internal/keycloak/ci_federation.go @@ -0,0 +1,558 @@ +package keycloak + +import ( + "bytes" + "context" + "encoding/json" + "fmt" + "io" + "log" + "net/http" + "net/url" + "strings" +) + +// CI federation realizes keyless CI (GitHub Actions / GitLab CI) against a +// gateway through the Keycloak 26.x external-to-internal token-exchange v2 +// mechanism (TOKEN_EXCHANGE_EXTERNAL_INTERNAL_V2). The v1 token-exchange +// permission API (.../management/permissions) is DISABLED on the production +// fleet Keycloak and returns 501, so none of the code here calls it; it drives +// the v2 admin-REST surface instead (ci-trust-ext-api.spec.md, Keycloak 26.x +// external-to-internal token-exchange v2 requirement). +// +// A CITrust is realized as three artifacts: +// +// (a) a per-trust scoped claim-to-group IdP mapper on the (platform +// provisioned, adopted) CIProvider IdP. It admits only brokered tokens +// whose claims match the compiled subject predicate (repository / +// repository_owner, plus project_path / environment / ref / ref_protected +// when the subject sets them) and lands the matched identity in the trust's +// OWN per-trust group (not a shared per-owner group). This is the trust +// boundary (PLAT-CITR-04): audience pinning alone is insufficient because +// any repository can mint a token carrying the pinned audience, so the +// repository/owner claim match is the real gate. A token that does not +// match lands in no group, holds no role, and is rejected default-deny. +// +// (b) a token-exchange authorization enabling the public (keyless) federation +// client to exchange a brokered token from the IdP onto the gateway client +// (the exchange audience) via external-to-internal v2. +// +// (c) a gateway RBAC binding mapping the gateway client role onto the trust's +// OWN per-trust group, so a matched identity holds exactly the role its own +// CITrust grants on its own gateway. The group is keyed on the CITrust id, +// which uniquely identifies one (repository, gateway, role) tuple, so no two +// distinct trusts ever aggregate their roles onto a shared group: a +// low-privilege repo cannot inherit a sibling repo's higher role, and a role +// granted for one gateway never leaks onto another gateway's client +// (cross-repo / cross-gateway privilege-escalation boundary). +// +// LIVE-VERIFY: the exact v2 admin-REST shape for (a) the per-repo subject +// constraint and (b) the per-requester external-to-internal authorization is +// flagged in the issue (#819) as needing empirical confirmation against a live +// Keycloak 26.6. The calls below follow the documented admin REST API (IdP +// advanced-claim group mapper; client standard-token-exchange attribute; group +// client-role mapping) and are each idempotent and individually verifiable. + +const ( + // ciTrustMapperPrefix names the per-trust IdP mapper deterministically from + // the CITrust id, so repeated reconciles converge to the same mapper and a + // delete targets exactly this trust without touching siblings. + ciTrustMapperPrefix = "citrust-" + + // advancedGroupIdpMapper is the Keycloak IdP mapper type that joins a + // brokered identity to a group only when every configured claim matches. + advancedGroupIdpMapper = "oidc-advanced-group-idp-mapper" + + // standardTokenExchangeEnabledAttribute is the Keycloak 26.x client + // attribute that opts a client in as a token-exchange audience under token + // exchange v2. LIVE-VERIFY against 26.6. + standardTokenExchangeEnabledAttribute = "standard.token.exchange.enabled" +) + +// CISubjectPredicate is the compiled claim predicate a brokered CI token must +// satisfy. Empty fields are not constrained. Repository/RepositoryOwner are the +// trust boundary for forge providers that carry them. +type CISubjectPredicate struct { + Repository string + RepositoryOwner string + ProjectPath string + Environment string + Ref string + RefProtected bool +} + +// CIFederationSpec is the desired Keycloak state for one CITrust. +type CIFederationSpec struct { + // TrustID is the CITrust resource id; it names the per-trust IdP mapper. + TrustID string + // Forge selects the token claim names ("github", "gitlab", "forgejo"). + Forge string + // IdpAlias is the adopted CIProvider IdP alias (the exchange subject_issuer), + // e.g. "github-actions". This reconciler never creates or deletes the IdP; it + // is owned by the gitops realm configuration (PLAT-CITR-01). + IdpAlias string + // FederationClientID is the shared public (keyless) exchange requester, e.g. + // "ci-federation". + FederationClientID string + // GatewayClientID is the gateway's Keycloak client ({name}-{id}); the + // exchange audience. + GatewayClientID string + // Workspace is the derived forge-namespaced workspace label, e.g. "gh-acme". + // It is informational only (reported in CITrust status and the workflow + // hint); it is NOT the Keycloak group the role is bound to. The role grant is + // isolated to a per-trust group (see GroupName) so sibling trusts under the + // same owner never aggregate their roles. + Workspace string + // Role is the gateway client role bound to this trust's per-trust group. + Role string + // Subject is the compiled claim predicate. + Subject CISubjectPredicate +} + +// GroupName is the Keycloak group a matched identity lands in and the group the +// gateway role is bound to. It is keyed on the CITrust id, which uniquely +// identifies one (repository, gateway, role) tuple, so each trust's role grant +// is isolated to its own subject and its own gateway. A shared per-owner group +// would let a low-privilege repo inherit a sibling repo's higher role and leak +// roles across gateways; keying on the trust id closes that boundary. The +// Workspace label prefixes the name only for legibility in the Keycloak admin +// UI; it never widens the scope. +func (s CIFederationSpec) GroupName() string { + return fmt.Sprintf("%s-citrust-%s", s.Workspace, s.TrustID) +} + +type idpMapper struct { + ID string `json:"id,omitempty"` + Name string `json:"name"` + IdentityProviderAlias string `json:"identityProviderAlias"` + IdentityProviderMapper string `json:"identityProviderMapper"` + Config map[string]string `json:"config"` +} + +type keycloakGroup struct { + ID string `json:"id,omitempty"` + Name string `json:"name"` +} + +// EnsureCIFederation idempotently realizes artifacts (a), (b), and (c) for a +// CITrust. It is re-entrant: repeated calls converge to the same Keycloak state +// without duplicating artifacts (idempotent-reconcile requirement). A failed read of Keycloak state +// is surfaced as an error (treated as unknown, not absent) so the caller does +// not act on a transient admin-API failure (STD-RCON-08). +func (c *Client) EnsureCIFederation(ctx context.Context, spec CIFederationSpec) error { + if err := spec.validate(); err != nil { + return err + } + + // The IdP must already exist (adopted, not created here). A missing IdP is a + // configuration error, not something to paper over by creating a parallel one. + exists, err := c.identityProviderExists(ctx, spec.IdpAlias) + if err != nil { + return fmt.Errorf("resolve CIProvider IdP %q: %w", spec.IdpAlias, err) + } + if !exists { + return fmt.Errorf("CIProvider IdP %q not found; it must be provisioned by the gitops realm configuration before a CITrust can adopt it", spec.IdpAlias) + } + + // Resolve the gateway client once; both the RBAC binding (c) and the + // exchange authorization (b) target it, so a single lookup serves both. + gatewayUUID, err := c.getClientUUID(ctx, spec.GatewayClientID) + if err != nil { + return fmt.Errorf("resolve gateway client %q: %w", spec.GatewayClientID, err) + } + if gatewayUUID == "" { + return &ClientNotFoundError{ClientID: spec.GatewayClientID} + } + + // (a) subject->workspace claim mapper on the IdP. + if err := c.ensureCITrustIdpMapper(ctx, spec); err != nil { + return fmt.Errorf("ensure CI trust subject mapper: %w", err) + } + + // (c) workspace group + gateway role binding. + if err := c.ensureWorkspaceRoleBinding(ctx, spec, gatewayUUID); err != nil { + return fmt.Errorf("ensure workspace role binding: %w", err) + } + + // (b) token-exchange authorization onto the gateway client. + if err := c.ensureGatewayExchangeAuthorization(ctx, spec, gatewayUUID); err != nil { + return fmt.Errorf("ensure gateway token-exchange authorization: %w", err) + } + + return nil +} + +// DeleteCIFederation revokes a CITrust. It removes the per-trust subject mapper +// (a), which is the authoritative revocation: the deleted subject can no longer +// land in the trust's group, so it holds no role and is rejected default-deny +// (the CITrust-delete-revokes-exchange requirement). The per-trust group and its +// role binding (c) and the gateway client's exchange opt-in (b) are left in +// place: the empty group grants nothing without a matching subject mapper, and +// the exchange opt-in is shared by every trust on the gateway client. +// +// Tolerant delete: an empty idp_alias or trust id means there is nothing this +// reconciler can resolve to revoke (e.g. the CIProvider was already torn down, so +// its adopted alias is unknowable), and an already-gone mapper is a no-op. Since +// the WatchCITrusts stream does not resync deletes, a hard error here would wedge +// the delete forever, so these are treated as already-revoked rather than failed. +func (c *Client) DeleteCIFederation(ctx context.Context, spec CIFederationSpec) error { + if spec.IdpAlias == "" || spec.TrustID == "" { + log.Printf("INFO keycloak: CI federation delete for trust %q has no idp_alias to resolve; nothing to revoke", spec.TrustID) + return nil + } + if err := c.deleteCITrustIdpMapper(ctx, spec.IdpAlias, spec.TrustID); err != nil { + return fmt.Errorf("delete CI trust subject mapper: %w", err) + } + return nil +} + +func (s CIFederationSpec) validate() error { + switch { + case s.TrustID == "": + return fmt.Errorf("CI federation requires a trust id") + case s.IdpAlias == "": + return fmt.Errorf("CI federation requires an idp_alias") + case s.GatewayClientID == "": + return fmt.Errorf("CI federation requires a gateway client id") + case s.Workspace == "": + return fmt.Errorf("CI federation requires a workspace") + case s.Role == "": + return fmt.Errorf("CI federation requires a role") + } + if len(subjectClaims(s.Forge, s.Subject)) == 0 { + return fmt.Errorf("CI federation requires at least one subject claim constraint (the trust boundary); refusing to authorize an unconstrained exchange") + } + // The identity claim (repository/owner for github/forgejo, project_path for + // gitlab) is the real trust boundary: a trust constrained only by ref or + // environment would admit ANY repository building that ref. Require it + // explicitly so a ref/environment-only subject is rejected, not quietly + // widened to every repo. + switch s.Forge { + case "gitlab": + if s.Subject.ProjectPath == "" { + return fmt.Errorf("gitlab CI federation requires a project_path subject (the trust boundary); a ref/environment-only trust is refused") + } + default: // github, forgejo + if s.Subject.Repository == "" && s.Subject.RepositoryOwner == "" { + return fmt.Errorf("%s CI federation requires a repository or repository_owner subject (the trust boundary); a ref/environment-only trust is refused", s.Forge) + } + } + return nil +} + +// subjectClaims compiles the forge token claims a brokered token must carry, +// as the {key,value} pairs the advanced-claim IdP mapper matches. GitHub and +// Forgejo expose repository / repository_owner; GitLab exposes project_path. +// Only set fields are constrained. +func subjectClaims(forge string, s CISubjectPredicate) []map[string]string { + claims := make([]map[string]string, 0, 5) + add := func(key, value string) { + if value != "" { + claims = append(claims, map[string]string{"key": key, "value": value}) + } + } + switch forge { + case "gitlab": + add("project_path", s.ProjectPath) + add("ref", s.Ref) + add("environment", s.Environment) + if s.RefProtected { + add("ref_protected", "true") + } + default: // github, forgejo + add("repository", s.Repository) + add("repository_owner", s.RepositoryOwner) + add("ref", s.Ref) + add("environment", s.Environment) + if s.RefProtected { + add("ref_protected", "true") + } + } + return claims +} + +func (c *Client) ensureCITrustIdpMapper(ctx context.Context, spec CIFederationSpec) error { + name := ciTrustMapperPrefix + spec.TrustID + + claims, err := json.Marshal(subjectClaims(spec.Forge, spec.Subject)) + if err != nil { + return fmt.Errorf("marshal subject claims: %w", err) + } + + desired := idpMapper{ + Name: name, + IdentityProviderAlias: spec.IdpAlias, + IdentityProviderMapper: advancedGroupIdpMapper, + Config: map[string]string{ + "claims": string(claims), + "are.claim.values.regex": "false", + // Land the matched identity in this trust's OWN per-trust group, not a + // shared per-owner workspace group, so the role binding (c) cannot leak + // across sibling repos or gateways. + "group": "/" + spec.GroupName(), + "syncMode": "FORCE", + }, + } + + existing, err := c.findIdpMapper(ctx, spec.IdpAlias, name) + if err != nil { + return err + } + base := fmt.Sprintf("/admin/realms/%s/identity-provider/instances/%s/mappers", + c.realm, url.PathEscape(spec.IdpAlias)) + if existing != nil { + // PUT the desired representation over the existing mapper so a changed + // subject predicate or workspace is corrected in place (Keycloak requires + // the id in both the path and the body). + desired.ID = existing.ID + body, _ := json.Marshal(desired) + if _, err := c.doRequest(ctx, http.MethodPut, base+"/"+url.PathEscape(existing.ID), body); err != nil { + return fmt.Errorf("update IdP mapper %q: %w", name, err) + } + return nil + } + body, _ := json.Marshal(desired) + if _, err := c.doRequest(ctx, http.MethodPost, base, body); err != nil { + return fmt.Errorf("create IdP mapper %q: %w", name, err) + } + return nil +} + +func (c *Client) deleteCITrustIdpMapper(ctx context.Context, idpAlias, trustID string) error { + name := ciTrustMapperPrefix + trustID + existing, err := c.findIdpMapper(ctx, idpAlias, name) + if err != nil { + return err + } + if existing == nil { + log.Printf("INFO keycloak: CI trust mapper %q not found on IdP %q, nothing to delete", name, idpAlias) + return nil + } + path := fmt.Sprintf("/admin/realms/%s/identity-provider/instances/%s/mappers/%s", + c.realm, url.PathEscape(idpAlias), url.PathEscape(existing.ID)) + if _, err := c.doRequest(ctx, http.MethodDelete, path, nil); err != nil { + return fmt.Errorf("delete IdP mapper %q: %w", name, err) + } + return nil +} + +func (c *Client) findIdpMapper(ctx context.Context, idpAlias, name string) (*idpMapper, error) { + path := fmt.Sprintf("/admin/realms/%s/identity-provider/instances/%s/mappers", + c.realm, url.PathEscape(idpAlias)) + body, err := c.doRequest(ctx, http.MethodGet, path, nil) + if err != nil { + return nil, fmt.Errorf("list IdP mappers on %q: %w", idpAlias, err) + } + var mappers []idpMapper + if len(bytes.TrimSpace(body)) > 0 { + if err := json.Unmarshal(body, &mappers); err != nil { + return nil, fmt.Errorf("parse IdP mappers on %q: %w", idpAlias, err) + } + } + for i := range mappers { + if mappers[i].Name == name { + return &mappers[i], nil + } + } + return nil, nil +} + +func (c *Client) identityProviderExists(ctx context.Context, idpAlias string) (bool, error) { + path := fmt.Sprintf("/admin/realms/%s/identity-provider/instances/%s", + c.realm, url.PathEscape(idpAlias)) + resp, err := c.doRequestRaw(ctx, http.MethodGet, path, nil) + if err != nil { + return false, err + } + defer func() { _ = resp.Body.Close() }() + switch resp.StatusCode { + case http.StatusOK: + return true, nil + case http.StatusNotFound: + return false, nil + default: + return false, fmt.Errorf("get IdP %q returned %d", idpAlias, resp.StatusCode) + } +} + +// ensureWorkspaceRoleBinding realizes artifact (c): the trust's OWN per-trust +// group exists and carries the gateway client role. Idempotent: an existing +// group and an already-present role mapping are no-ops. The group is keyed on the +// CITrust id (GroupName), so the role is granted only to the identity this +// trust's mapper admits, on this trust's gateway - never aggregated onto a shared +// per-owner group. gatewayUUID is the already-resolved Keycloak UUID of the +// gateway client. +func (c *Client) ensureWorkspaceRoleBinding(ctx context.Context, spec CIFederationSpec, gatewayUUID string) error { + group := spec.GroupName() + groupID, err := c.ensureGroup(ctx, group) + if err != nil { + return err + } + + roleUUID, err := c.getFederationClientRoleUUID(ctx, gatewayUUID, spec.GatewayClientID, spec.Role) + if err != nil { + return fmt.Errorf("resolve role %q on gateway client %q: %w", spec.Role, spec.GatewayClientID, err) + } + if roleUUID == "" { + // A 200 with no id means the role record is not usable yet (provisioning + // race); surface the retryable not-found path rather than a terminal error. + return &ClientNotFoundError{ClientID: spec.GatewayClientID} + } + + roles := []keycloakRole{{ID: roleUUID, Name: spec.Role}} + body, _ := json.Marshal(roles) + // POSTing an already-present group role mapping is a Keycloak no-op, so this + // converges without a pre-read. + path := fmt.Sprintf("/admin/realms/%s/groups/%s/role-mappings/clients/%s", + c.realm, url.PathEscape(groupID), url.PathEscape(gatewayUUID)) + if _, err := c.doRequest(ctx, http.MethodPost, path, body); err != nil { + return fmt.Errorf("bind role %q to per-trust group %q on gateway client %q: %w", + spec.Role, group, spec.GatewayClientID, err) + } + return nil +} + +// getFederationClientRoleUUID resolves a gateway client role UUID, mapping a 404 +// to the retryable ClientNotFoundError path. The client can exist before its +// roles are provisioned (a reconcile race), and doRequest collapses every status +// >= 400 - including 404 - into a generic error, which would surface the race as +// a terminal "reconcile keycloak federation" failure. Inspecting the raw status +// here lets the caller reschedule via errors.As(*ClientNotFoundError). +func (c *Client) getFederationClientRoleUUID(ctx context.Context, clientUUID, clientID, roleName string) (string, error) { + path := fmt.Sprintf("/admin/realms/%s/clients/%s/roles/%s", + c.realm, url.PathEscape(clientUUID), url.PathEscape(roleName)) + resp, err := c.doRequestRaw(ctx, http.MethodGet, path, nil) + if err != nil { + return "", err + } + defer func() { _ = resp.Body.Close() }() + if resp.StatusCode == http.StatusNotFound { + return "", &ClientNotFoundError{ClientID: clientID} + } + respBody, _ := io.ReadAll(resp.Body) + if resp.StatusCode >= 400 { + return "", fmt.Errorf("get role %q on client %s returned %d: %s", roleName, clientID, resp.StatusCode, string(respBody)) + } + var role keycloakRole + if err := json.Unmarshal(respBody, &role); err != nil { + return "", fmt.Errorf("parse role %q: %w", roleName, err) + } + return role.ID, nil +} + +// ensureGroup returns the id of a top-level realm group with the given name, +// creating it if absent. A 409 on create means a concurrent reconcile won the +// race, so it re-reads rather than failing. +func (c *Client) ensureGroup(ctx context.Context, name string) (string, error) { + if id, err := c.findGroup(ctx, name); err != nil { + return "", err + } else if id != "" { + return id, nil + } + payload, _ := json.Marshal(keycloakGroup{Name: name}) + path := fmt.Sprintf("/admin/realms/%s/groups", c.realm) + resp, err := c.doRequestRaw(ctx, http.MethodPost, path, payload) + if err != nil { + return "", fmt.Errorf("create group %q: %w", name, err) + } + defer func() { _ = resp.Body.Close() }() + if resp.StatusCode == http.StatusConflict { + id, err := c.findGroup(ctx, name) + if err != nil { + return "", err + } + if id == "" { + return "", fmt.Errorf("create group %q returned 409 but the group was not found", name) + } + return id, nil + } + if resp.StatusCode != http.StatusCreated && resp.StatusCode != http.StatusOK { + return "", fmt.Errorf("create group %q returned %d", name, resp.StatusCode) + } + id, err := c.findGroup(ctx, name) + if err != nil { + return "", err + } + if id == "" { + return "", fmt.Errorf("group %q not found immediately after create", name) + } + return id, nil +} + +func (c *Client) findGroup(ctx context.Context, name string) (string, error) { + path := fmt.Sprintf("/admin/realms/%s/groups?search=%s&exact=true", c.realm, url.QueryEscape(name)) + body, err := c.doRequest(ctx, http.MethodGet, path, nil) + if err != nil { + return "", fmt.Errorf("search group %q: %w", name, err) + } + var groups []keycloakGroup + if len(bytes.TrimSpace(body)) > 0 { + if err := json.Unmarshal(body, &groups); err != nil { + return "", fmt.Errorf("parse group search for %q: %w", name, err) + } + } + for _, g := range groups { + if g.Name == name { + return g.ID, nil + } + } + return "", nil +} + +// ensureGatewayExchangeAuthorization realizes artifact (b): it opts the gateway +// client in as a token-exchange audience so the public federation client may +// exchange a brokered IdP token onto it (external-to-internal v2). It GET-merges +// the attribute onto the existing client representation so unrelated settings are +// preserved, and is a no-op when already enabled. +// +// LIVE-VERIFY: the precise 26.6 enablement for external-to-internal v2 with a +// public requester is flagged in #819. This sets the documented +// standard.token.exchange.enabled client attribute; the per-requester scoping of +// the exchange needs confirmation against a live Keycloak. +func (c *Client) ensureGatewayExchangeAuthorization(ctx context.Context, spec CIFederationSpec, gatewayUUID string) error { + path := fmt.Sprintf("/admin/realms/%s/clients/%s", c.realm, url.PathEscape(gatewayUUID)) + respBody, err := c.doRequest(ctx, http.MethodGet, path, nil) + if err != nil { + return fmt.Errorf("get gateway client %q: %w", spec.GatewayClientID, err) + } + var representation map[string]json.RawMessage + if err := json.Unmarshal(respBody, &representation); err != nil { + return fmt.Errorf("parse gateway client %q: %w", spec.GatewayClientID, err) + } + + attributes := make(map[string]string) + if raw, ok := representation["attributes"]; ok && + len(raw) > 0 && !bytes.Equal(bytes.TrimSpace(raw), []byte("null")) { + if err := json.Unmarshal(raw, &attributes); err != nil { + return fmt.Errorf("parse attributes for gateway client %q: %w", spec.GatewayClientID, err) + } + } + if attributes[standardTokenExchangeEnabledAttribute] == "true" { + return nil + } + attributes[standardTokenExchangeEnabledAttribute] = "true" + + rawAttributes, err := json.Marshal(attributes) + if err != nil { + return fmt.Errorf("marshal attributes for gateway client %q: %w", spec.GatewayClientID, err) + } + representation["attributes"] = rawAttributes + body, err := json.Marshal(representation) + if err != nil { + return fmt.Errorf("marshal gateway client %q: %w", spec.GatewayClientID, err) + } + if _, err := c.doRequest(ctx, http.MethodPut, path, body); err != nil { + return fmt.Errorf("enable token exchange on gateway client %q: %w", spec.GatewayClientID, err) + } + return nil +} + +// OwnerFromRepository returns the owner segment of an "owner/repo" repository +// string (the GitHub/Forgejo repository_owner claim), or the input unchanged +// when it carries no slash. +func OwnerFromRepository(repository string) string { + if idx := strings.Index(repository, "/"); idx >= 0 { + return repository[:idx] + } + return repository +} diff --git a/components/control-plane/internal/keycloak/ci_federation_test.go b/components/control-plane/internal/keycloak/ci_federation_test.go new file mode 100644 index 000000000..29c565c6c --- /dev/null +++ b/components/control-plane/internal/keycloak/ci_federation_test.go @@ -0,0 +1,666 @@ +package keycloak + +import ( + "encoding/json" + "errors" + "fmt" + "io" + "net/http" + "net/http/httptest" + "strings" + "sync" + "testing" +) + +const ( + testIdpAlias = "github-actions" + testFederationID = "ci-federation" + testWorkspace = "gh-acme" + testGatewayRoleID = "role-openshell-user-uuid" +) + +// fakeCIKeycloak is a stateful httptest fake of the Keycloak admin REST surface +// the CI federation code drives. It records every write so tests can assert the +// exact artifacts created, and it serves reads from its own state so a second +// Ensure pass observes what the first created (idempotency). +type fakeCIKeycloak struct { + mu sync.Mutex + + idpExists bool + // idpMappers maps mapper name -> representation, keyed as Keycloak would. + idpMappers map[string]idpMapper + nextID int + + // groups maps group name -> group id, as Keycloak would after create. + groups map[string]string + + // gatewayClientExists controls whether the gateway clientId resolves. + gatewayClientExists bool + // gatewayAttrs is the current gateway client attribute map (PUT target). + gatewayAttrs map[string]string + // roleExists controls whether the role resolves on the gateway client. + roleExists bool + + // groupRoleMappings records POSTed role bindings keyed by group id, so a test + // can assert which group a role landed on (cross-trust isolation). + groupRoleMappings map[string][][]keycloakRole + + requests []string // method+path trail, for ordering assertions +} + +func newFakeCIKeycloak(t *testing.T) (*httptest.Server, *fakeCIKeycloak) { + t.Helper() + state := &fakeCIKeycloak{ + idpExists: true, + idpMappers: map[string]idpMapper{}, + groups: map[string]string{}, + gatewayClientExists: true, + gatewayAttrs: map[string]string{}, + roleExists: true, + groupRoleMappings: map[string][][]keycloakRole{}, + } + + mux := http.NewServeMux() + + mux.HandleFunc(fmt.Sprintf("/realms/%s/protocol/openid-connect/token", testRealm), func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(map[string]any{"access_token": "fake-token", "expires_in": 300}) + }) + + // IdP instance existence. + mux.HandleFunc(fmt.Sprintf("/admin/realms/%s/identity-provider/instances/%s", testRealm, testIdpAlias), func(w http.ResponseWriter, r *http.Request) { + state.mu.Lock() + defer state.mu.Unlock() + state.requests = append(state.requests, r.Method+" idp") + if !state.idpExists { + http.Error(w, "not found", http.StatusNotFound) + return + } + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(map[string]any{"alias": testIdpAlias}) + }) + + // IdP mappers collection (GET list, POST create). + mappersBase := fmt.Sprintf("/admin/realms/%s/identity-provider/instances/%s/mappers", testRealm, testIdpAlias) + mux.HandleFunc(mappersBase, func(w http.ResponseWriter, r *http.Request) { + state.mu.Lock() + defer state.mu.Unlock() + switch r.Method { + case http.MethodGet: + state.requests = append(state.requests, "GET mappers") + out := make([]idpMapper, 0, len(state.idpMappers)) + for _, m := range state.idpMappers { + out = append(out, m) + } + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(out) + case http.MethodPost: + state.requests = append(state.requests, "POST mappers") + var m idpMapper + _ = json.NewDecoder(r.Body).Decode(&m) + state.nextID++ + m.ID = fmt.Sprintf("mapper-%d", state.nextID) + state.idpMappers[m.Name] = m + w.WriteHeader(http.StatusCreated) + default: + http.Error(w, "method not allowed", http.StatusMethodNotAllowed) + } + }) + + // IdP mapper by id (PUT update, DELETE). + mux.HandleFunc(mappersBase+"/", func(w http.ResponseWriter, r *http.Request) { + state.mu.Lock() + defer state.mu.Unlock() + id := strings.TrimPrefix(r.URL.Path, mappersBase+"/") + switch r.Method { + case http.MethodPut: + state.requests = append(state.requests, "PUT mapper") + var m idpMapper + _ = json.NewDecoder(r.Body).Decode(&m) + state.idpMappers[m.Name] = m + w.WriteHeader(http.StatusNoContent) + case http.MethodDelete: + state.requests = append(state.requests, "DELETE mapper") + for name, m := range state.idpMappers { + if m.ID == id { + delete(state.idpMappers, name) + } + } + w.WriteHeader(http.StatusNoContent) + default: + http.Error(w, "method not allowed", http.StatusMethodNotAllowed) + } + }) + + // Groups collection (GET search, POST create). Group ids are tracked by name + // so a test can observe which per-trust group a role is bound to. + groupsBase := fmt.Sprintf("/admin/realms/%s/groups", testRealm) + mux.HandleFunc(groupsBase, func(w http.ResponseWriter, r *http.Request) { + state.mu.Lock() + defer state.mu.Unlock() + switch r.Method { + case http.MethodGet: + state.requests = append(state.requests, "GET groups") + w.Header().Set("Content-Type", "application/json") + search := r.URL.Query().Get("search") + if id, ok := state.groups[search]; ok { + _ = json.NewEncoder(w).Encode([]keycloakGroup{{ID: id, Name: search}}) + return + } + _ = json.NewEncoder(w).Encode([]keycloakGroup{}) + case http.MethodPost: + state.requests = append(state.requests, "POST groups") + var g keycloakGroup + _ = json.NewDecoder(r.Body).Decode(&g) + state.nextID++ + state.groups[g.Name] = fmt.Sprintf("group-%d", state.nextID) + w.WriteHeader(http.StatusCreated) + default: + http.Error(w, "method not allowed", http.StatusMethodNotAllowed) + } + }) + + // Clients collection -- resolve gateway clientId -> uuid. + mux.HandleFunc(fmt.Sprintf("/admin/realms/%s/clients", testRealm), func(w http.ResponseWriter, r *http.Request) { + state.mu.Lock() + defer state.mu.Unlock() + w.Header().Set("Content-Type", "application/json") + if r.URL.Query().Get("clientId") == testGatewayClientID && state.gatewayClientExists { + _ = json.NewEncoder(w).Encode([]keycloakClient{{ID: testGatewayUUID, ClientID: testGatewayClientID}}) + return + } + _ = json.NewEncoder(w).Encode([]keycloakClient{}) + }) + + // Gateway client role resolution. + mux.HandleFunc(fmt.Sprintf("/admin/realms/%s/clients/%s/roles/", testRealm, testGatewayUUID), func(w http.ResponseWriter, r *http.Request) { + state.mu.Lock() + defer state.mu.Unlock() + if !state.roleExists { + http.Error(w, "not found", http.StatusNotFound) + return + } + roleName := strings.TrimPrefix(r.URL.Path, fmt.Sprintf("/admin/realms/%s/clients/%s/roles/", testRealm, testGatewayUUID)) + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(keycloakRole{ID: testGatewayRoleID, Name: roleName}) + }) + + // Gateway client representation GET/PUT (exchange authorization attribute). + mux.HandleFunc(fmt.Sprintf("/admin/realms/%s/clients/%s", testRealm, testGatewayUUID), func(w http.ResponseWriter, r *http.Request) { + state.mu.Lock() + defer state.mu.Unlock() + switch r.Method { + case http.MethodGet: + state.requests = append(state.requests, "GET gateway client") + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(map[string]any{ + "id": testGatewayUUID, + "clientId": testGatewayClientID, + "attributes": state.gatewayAttrs, + }) + case http.MethodPut: + state.requests = append(state.requests, "PUT gateway client") + var rep map[string]json.RawMessage + _ = json.NewDecoder(r.Body).Decode(&rep) + if raw, ok := rep["attributes"]; ok { + _ = json.Unmarshal(raw, &state.gatewayAttrs) + } + w.WriteHeader(http.StatusNoContent) + default: + http.Error(w, "method not allowed", http.StatusMethodNotAllowed) + } + }) + + // Group role-mappings onto the gateway client. Registered as a prefix handler + // so any per-trust group id is accepted; the group id is parsed from the path + // and the binding recorded against it (cross-trust isolation assertions). + groupsPrefix := groupsBase + "/" + mux.HandleFunc(groupsPrefix, func(w http.ResponseWriter, r *http.Request) { + state.mu.Lock() + defer state.mu.Unlock() + rest := strings.TrimPrefix(r.URL.Path, groupsPrefix) + marker := "/role-mappings/clients/" + idx := strings.Index(rest, marker) + if idx < 0 || r.Method != http.MethodPost { + http.Error(w, "not found", http.StatusNotFound) + return + } + groupID := rest[:idx] + state.requests = append(state.requests, "POST group role-mapping") + body, _ := io.ReadAll(r.Body) + var roles []keycloakRole + _ = json.Unmarshal(body, &roles) + state.groupRoleMappings[groupID] = append(state.groupRoleMappings[groupID], roles) + w.WriteHeader(http.StatusNoContent) + }) + + server := httptest.NewServer(mux) + t.Cleanup(server.Close) + return server, state +} + +func ciTestClient(server *httptest.Server) *Client { + return NewClient(server.URL, testRealm, testAdminClientID, testAdminSecret) +} + +func githubSpec() CIFederationSpec { + return CIFederationSpec{ + TrustID: "trust-123", + Forge: "github", + IdpAlias: testIdpAlias, + FederationClientID: testFederationID, + GatewayClientID: testGatewayClientID, + Workspace: testWorkspace, + Role: "openshell-user", + Subject: CISubjectPredicate{ + Repository: "acme/app", + RepositoryOwner: "acme", + }, + } +} + +func TestEnsureCIFederation_CreatesAllArtifacts(t *testing.T) { + server, state := newFakeCIKeycloak(t) + client := ciTestClient(server) + + if err := client.EnsureCIFederation(t.Context(), githubSpec()); err != nil { + t.Fatalf("EnsureCIFederation: %v", err) + } + + // (a) per-trust subject mapper created with the compiled claim predicate. + mapper, ok := state.idpMappers["citrust-trust-123"] + if !ok { + t.Fatalf("subject IdP mapper not created; mappers=%v", state.idpMappers) + } + if mapper.IdentityProviderMapper != advancedGroupIdpMapper { + t.Errorf("mapper type = %q, want %q", mapper.IdentityProviderMapper, advancedGroupIdpMapper) + } + // The mapper lands the identity in this trust's OWN per-trust group, not the + // shared per-owner workspace label. + wantGroup := "/" + githubSpec().GroupName() + if got := mapper.Config["group"]; got != wantGroup { + t.Errorf("mapper group = %q, want per-trust group %q", got, wantGroup) + } + if mapper.Config["group"] == "/"+testWorkspace { + t.Errorf("mapper must not target the shared per-owner group %q", testWorkspace) + } + var claims []map[string]string + if err := json.Unmarshal([]byte(mapper.Config["claims"]), &claims); err != nil { + t.Fatalf("parse claims: %v", err) + } + assertClaim(t, claims, "repository", "acme/app") + assertClaim(t, claims, "repository_owner", "acme") + + // (c) gateway role bound onto exactly one per-trust group. + if len(state.groupRoleMappings) != 1 { + t.Fatalf("role bound to %d groups, want exactly one per-trust group; got %v", len(state.groupRoleMappings), state.groupRoleMappings) + } + for _, bindings := range state.groupRoleMappings { + if len(bindings) != 1 || len(bindings[0]) != 1 || bindings[0][0].Name != "openshell-user" { + t.Errorf("group role bindings = %v, want one openshell-user binding", bindings) + } + } + + // (b) gateway client opted in as a token-exchange audience. + if state.gatewayAttrs[standardTokenExchangeEnabledAttribute] != "true" { + t.Errorf("gateway exchange attribute = %q, want true", state.gatewayAttrs[standardTokenExchangeEnabledAttribute]) + } +} + +func TestEnsureCIFederation_Idempotent(t *testing.T) { + server, state := newFakeCIKeycloak(t) + client := ciTestClient(server) + + for pass := 1; pass <= 2; pass++ { + if err := client.EnsureCIFederation(t.Context(), githubSpec()); err != nil { + t.Fatalf("EnsureCIFederation pass %d: %v", pass, err) + } + } + + // Exactly one mapper, created once then updated in place (no duplicate). + if len(state.idpMappers) != 1 { + t.Errorf("idpMappers = %d, want 1 after two passes", len(state.idpMappers)) + } + posts, puts := 0, 0 + for _, req := range state.requests { + switch req { + case "POST mappers": + posts++ + case "PUT mapper": + puts++ + } + } + if posts != 1 { + t.Errorf("mapper POSTs = %d, want 1 (create once)", posts) + } + if puts != 1 { + t.Errorf("mapper PUTs = %d, want 1 (update on second pass)", puts) + } + // The group is created once; the second pass finds it. + groupPosts := 0 + for _, req := range state.requests { + if req == "POST groups" { + groupPosts++ + } + } + if groupPosts != 1 { + t.Errorf("group POSTs = %d, want 1", groupPosts) + } + // The gateway client is PUT only on the first pass; the second finds the + // attribute already set and skips the write. + gwPuts := 0 + for _, req := range state.requests { + if req == "PUT gateway client" { + gwPuts++ + } + } + if gwPuts != 1 { + t.Errorf("gateway client PUTs = %d, want 1 (no rewrite when already enabled)", gwPuts) + } +} + +func TestEnsureCIFederation_MissingIdPFailsClosed(t *testing.T) { + server, state := newFakeCIKeycloak(t) + state.idpExists = false + client := ciTestClient(server) + + err := client.EnsureCIFederation(t.Context(), githubSpec()) + if err == nil { + t.Fatal("expected error when the CIProvider IdP is absent") + } + if !strings.Contains(err.Error(), "not found") { + t.Errorf("error = %v, want it to name the missing IdP", err) + } + if len(state.idpMappers) != 0 { + t.Errorf("no mapper should be created when the IdP is absent; got %v", state.idpMappers) + } +} + +func TestEnsureCIFederation_MissingGatewayClientIsRetryable(t *testing.T) { + server, state := newFakeCIKeycloak(t) + state.gatewayClientExists = false + client := ciTestClient(server) + + err := client.EnsureCIFederation(t.Context(), githubSpec()) + if err == nil { + t.Fatal("expected error when the gateway client is not provisioned") + } + var notFound *ClientNotFoundError + if !errors.As(err, ¬Found) { + t.Errorf("error = %v, want a ClientNotFoundError so the caller retries", err) + } +} + +func TestEnsureCIFederation_RejectsUnconstrainedSubject(t *testing.T) { + server, _ := newFakeCIKeycloak(t) + client := ciTestClient(server) + + spec := githubSpec() + spec.Subject = CISubjectPredicate{} // no claims -> no trust boundary + if err := client.EnsureCIFederation(t.Context(), spec); err == nil { + t.Fatal("expected an unconstrained subject (no claims) to be rejected") + } +} + +func TestEnsureCIFederation_GitLabUsesProjectPathClaim(t *testing.T) { + server, state := newFakeCIKeycloak(t) + client := ciTestClient(server) + + spec := githubSpec() + spec.Forge = "gitlab" + spec.Subject = CISubjectPredicate{ProjectPath: "acme/app"} + if err := client.EnsureCIFederation(t.Context(), spec); err != nil { + t.Fatalf("EnsureCIFederation: %v", err) + } + mapper := state.idpMappers["citrust-trust-123"] + var claims []map[string]string + _ = json.Unmarshal([]byte(mapper.Config["claims"]), &claims) + assertClaim(t, claims, "project_path", "acme/app") + for _, c := range claims { + if c["key"] == "repository" { + t.Errorf("gitlab mapper must not carry a repository claim; got %v", claims) + } + } +} + +func TestEnsureCIFederation_TighterClaimsIncluded(t *testing.T) { + server, state := newFakeCIKeycloak(t) + client := ciTestClient(server) + + spec := githubSpec() + spec.Subject = CISubjectPredicate{ + Repository: "acme/app", + RepositoryOwner: "acme", + Environment: "production", + Ref: "refs/heads/main", + RefProtected: true, + } + if err := client.EnsureCIFederation(t.Context(), spec); err != nil { + t.Fatalf("EnsureCIFederation: %v", err) + } + mapper := state.idpMappers["citrust-trust-123"] + var claims []map[string]string + _ = json.Unmarshal([]byte(mapper.Config["claims"]), &claims) + assertClaim(t, claims, "environment", "production") + assertClaim(t, claims, "ref", "refs/heads/main") + assertClaim(t, claims, "ref_protected", "true") +} + +func TestDeleteCIFederation_RemovesSubjectMapper(t *testing.T) { + server, state := newFakeCIKeycloak(t) + client := ciTestClient(server) + + if err := client.EnsureCIFederation(t.Context(), githubSpec()); err != nil { + t.Fatalf("EnsureCIFederation: %v", err) + } + if _, ok := state.idpMappers["citrust-trust-123"]; !ok { + t.Fatal("precondition: mapper should exist") + } + + if err := client.DeleteCIFederation(t.Context(), githubSpec()); err != nil { + t.Fatalf("DeleteCIFederation: %v", err) + } + if _, ok := state.idpMappers["citrust-trust-123"]; ok { + t.Error("subject mapper should be removed on delete") + } + // The per-trust group binding is left in place (inert without the mapper); the + // mapper removal is the authoritative revocation. + if len(state.groupRoleMappings) == 0 { + t.Error("per-trust group binding should survive the mapper delete") + } +} + +func TestDeleteCIFederation_MissingMapperIsNoOp(t *testing.T) { + server, _ := newFakeCIKeycloak(t) + client := ciTestClient(server) + + if err := client.DeleteCIFederation(t.Context(), githubSpec()); err != nil { + t.Fatalf("DeleteCIFederation on absent mapper should be a no-op, got %v", err) + } +} + +// TestEnsureCIFederation_SameOwnerDifferentReposDoNotCrossInherit is the +// regression lock for finding 1: two CITrusts under the same owner but for +// different repositories with different roles must each land their role on their +// OWN per-trust group, so the lower-privileged repo never inherits the sibling's +// higher role. +func TestEnsureCIFederation_SameOwnerDifferentReposDoNotCrossInherit(t *testing.T) { + server, state := newFakeCIKeycloak(t) + client := ciTestClient(server) + + specUser := githubSpec() + specUser.TrustID = "trust-user" + specUser.Subject = CISubjectPredicate{Repository: "acme/low", RepositoryOwner: "acme"} + specUser.Role = "openshell-user" + + specAdmin := githubSpec() + specAdmin.TrustID = "trust-admin" + specAdmin.Subject = CISubjectPredicate{Repository: "acme/high", RepositoryOwner: "acme"} + specAdmin.Role = "openshell-admin" + + for _, s := range []CIFederationSpec{specUser, specAdmin} { + if err := client.EnsureCIFederation(t.Context(), s); err != nil { + t.Fatalf("EnsureCIFederation(%s): %v", s.TrustID, err) + } + } + + // Each trust owns a distinct group. + if specUser.GroupName() == specAdmin.GroupName() { + t.Fatalf("both trusts share group %q; per-trust isolation lost", specUser.GroupName()) + } + + assertOnlyRole := func(spec CIFederationSpec, wantRole string) { + t.Helper() + groupID, ok := state.groups[spec.GroupName()] + if !ok { + t.Fatalf("group %q never created", spec.GroupName()) + } + bindings := state.groupRoleMappings[groupID] + for _, roles := range bindings { + for _, role := range roles { + if role.Name != wantRole { + t.Errorf("group %q carries role %q, want only %q (cross-repo inheritance)", spec.GroupName(), role.Name, wantRole) + } + } + } + if len(bindings) == 0 { + t.Errorf("group %q got no role binding", spec.GroupName()) + } + } + assertOnlyRole(specUser, "openshell-user") + assertOnlyRole(specAdmin, "openshell-admin") + + // Each per-trust mapper admits only its own repository. + assertRepoClaim := func(trustID, repo string) { + t.Helper() + var claims []map[string]string + _ = json.Unmarshal([]byte(state.idpMappers["citrust-"+trustID].Config["claims"]), &claims) + assertClaim(t, claims, "repository", repo) + } + assertRepoClaim("trust-user", "acme/low") + assertRepoClaim("trust-admin", "acme/high") +} + +// TestGroupName_PerTrustAndPerGateway locks the structural isolation invariant: +// the Keycloak group is a function of the trust id, so distinct trusts (distinct +// repos, or the same repo on a different gateway - each is a separate CITrust with +// its own id) never share a group, and the role grant cannot leak across repos or +// gateways. +func TestGroupName_PerTrustAndPerGateway(t *testing.T) { + base := githubSpec() + + sameOwnerRepoA := base + sameOwnerRepoA.TrustID = "trust-a" + sameOwnerRepoB := base + sameOwnerRepoB.TrustID = "trust-b" + if sameOwnerRepoA.GroupName() == sameOwnerRepoB.GroupName() { + t.Errorf("distinct trusts share a group: %q", sameOwnerRepoA.GroupName()) + } + + // Same repository/owner, but a second gateway is modeled as a second CITrust + // with its own id; the groups must still differ so a role on gateway A's + // client never binds onto a group gateway B's trust also uses. + gatewayA := base + gatewayA.TrustID = "trust-gw-a" + gatewayA.GatewayClientID = "prod-gw-a" + gatewayB := base + gatewayB.TrustID = "trust-gw-b" + gatewayB.GatewayClientID = "prod-gw-b" + if gatewayA.GroupName() == gatewayB.GroupName() { + t.Errorf("cross-gateway trusts share a group: %q", gatewayA.GroupName()) + } +} + +// TestEnsureCIFederation_RejectsIdentityClaimlessSubject locks finding 2: a +// github/forgejo trust constrained only by ref or environment (no repository or +// owner) is rejected, and likewise a gitlab trust without project_path. +func TestEnsureCIFederation_RejectsIdentityClaimlessSubject(t *testing.T) { + cases := []struct { + name string + forge string + subj CISubjectPredicate + }{ + {"github ref only", "github", CISubjectPredicate{Ref: "refs/heads/main"}}, + {"github environment only", "github", CISubjectPredicate{Environment: "production"}}, + {"forgejo ref only", "forgejo", CISubjectPredicate{Ref: "refs/heads/main"}}, + {"gitlab ref only", "gitlab", CISubjectPredicate{Ref: "refs/heads/main"}}, + {"gitlab environment only", "gitlab", CISubjectPredicate{Environment: "production"}}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + server, state := newFakeCIKeycloak(t) + client := ciTestClient(server) + spec := githubSpec() + spec.Forge = tc.forge + spec.Subject = tc.subj + if err := client.EnsureCIFederation(t.Context(), spec); err == nil { + t.Fatal("expected a trust without a repository/owner (or project_path) identity claim to be rejected") + } + if len(state.idpMappers) != 0 { + t.Errorf("no mapper should be created for a rejected trust; got %v", state.idpMappers) + } + }) + } +} + +// TestEnsureCIFederation_GitLabRefProtectedIncluded locks finding 3: a GitLab +// trust with RefProtected set carries the ref_protected claim. +func TestEnsureCIFederation_GitLabRefProtectedIncluded(t *testing.T) { + server, state := newFakeCIKeycloak(t) + client := ciTestClient(server) + + spec := githubSpec() + spec.Forge = "gitlab" + spec.Subject = CISubjectPredicate{ProjectPath: "acme/app", RefProtected: true} + if err := client.EnsureCIFederation(t.Context(), spec); err != nil { + t.Fatalf("EnsureCIFederation: %v", err) + } + var claims []map[string]string + _ = json.Unmarshal([]byte(state.idpMappers["citrust-trust-123"].Config["claims"]), &claims) + assertClaim(t, claims, "ref_protected", "true") +} + +// TestEnsureCIFederation_MissingRoleIsRetryable locks finding 5: when the gateway +// client exists but its role 404s (provisioning race), the error is a retryable +// ClientNotFoundError, not a terminal failure. +func TestEnsureCIFederation_MissingRoleIsRetryable(t *testing.T) { + server, state := newFakeCIKeycloak(t) + state.roleExists = false + client := ciTestClient(server) + + err := client.EnsureCIFederation(t.Context(), githubSpec()) + if err == nil { + t.Fatal("expected an error when the gateway client role is not provisioned") + } + var notFound *ClientNotFoundError + if !errors.As(err, ¬Found) { + t.Errorf("error = %v, want a ClientNotFoundError so the caller retries", err) + } +} + +// TestDeleteCIFederation_EmptyIdPAliasIsNoOp locks finding 4: a delete with no +// resolvable idp_alias (the CIProvider is already gone) is a tolerant no-op, not +// a hard error that would wedge the delete given WatchCITrusts does not resync. +func TestDeleteCIFederation_EmptyIdPAliasIsNoOp(t *testing.T) { + server, _ := newFakeCIKeycloak(t) + client := ciTestClient(server) + + spec := githubSpec() + spec.IdpAlias = "" + if err := client.DeleteCIFederation(t.Context(), spec); err != nil { + t.Fatalf("delete with empty idp_alias should be a no-op, got %v", err) + } +} + +func assertClaim(t *testing.T, claims []map[string]string, key, value string) { + t.Helper() + for _, c := range claims { + if c["key"] == key { + if c["value"] != value { + t.Errorf("claim %q = %q, want %q", key, c["value"], value) + } + return + } + } + t.Errorf("claim %q not present in %v", key, claims) +} diff --git a/components/control-plane/internal/reconciler/ci_trust_reconciler.go b/components/control-plane/internal/reconciler/ci_trust_reconciler.go index 1eb6c1fa4..2c038fd65 100644 --- a/components/control-plane/internal/reconciler/ci_trust_reconciler.go +++ b/components/control-plane/internal/reconciler/ci_trust_reconciler.go @@ -2,6 +2,7 @@ package reconciler import ( "context" + "errors" "fmt" "log" "strings" @@ -12,27 +13,56 @@ import ( "google.golang.org/grpc" ) -// CITrustReconciler reconciles CITrust resources into Keycloak claim mappers, -// token-exchange permissions, and gateway RBAC bindings. Each CITrust declares -// that CI workloads matching a subject predicate may federate into a specific -// gateway with a given role. +// defaultCIFederationClientID is the shared, public (keyless) token-exchange +// requester the gitops realm configuration provisions (hypershell-gitops#1636). +// The CI run's OIDC token is the only credential; the client holds no secret. +const defaultCIFederationClientID = "ci-federation" + +// ciFederationKeycloak is the Keycloak surface the CITrust reconciler needs. It +// is an interface so the reconciler can be unit-tested against a fake; the +// production implementation is *keycloak.Client. +type ciFederationKeycloak interface { + EnsureCIFederation(ctx context.Context, spec keycloak.CIFederationSpec) error + DeleteCIFederation(ctx context.Context, spec keycloak.CIFederationSpec) error + Issuer() string +} + +// CITrustReconciler reconciles CITrust resources into Keycloak 26.x +// external-to-internal token-exchange v2 artifacts: a per-trust scoped +// claim-to-workspace IdP mapper (the subject trust boundary), a token-exchange +// authorization onto the gateway client, and a gateway RBAC binding on the +// derived workspace. Each CITrust declares that CI workloads matching a subject +// predicate may federate into a specific gateway with a given role, keyless. // -// On upsert: derive the workspace name from the subject (forge-namespaced -// isolation), ensure the Keycloak claim mapper and token-exchange permission, -// bind the gateway RBAC role, then write back status (exchange_endpoint, -// workspace, workflow_hint). On delete: remove the Keycloak artifacts and RBAC -// binding. +// On upsert: resolve the CIProvider (forge + adopted IdP alias) and the gateway +// (Keycloak client + owning cluster), derive the forge-namespaced workspace, +// realize the Keycloak artifacts, then write back status (exchange_endpoint, +// workspace, workflow_hint). On delete: revoke the trust. The reconcile is +// cluster-scoped: only the control plane whose cluster hosts the gateway acts, +// mirroring the RoleBinding reconciler. type CITrustReconciler struct { - keycloakClient *keycloak.Client - ciTrustSvc pb.CITrustServiceClient - ciProviderSvc pb.CIProviderServiceClient + keycloakClient ciFederationKeycloak + ciTrustSvc pb.CITrustServiceClient + ciProviderSvc pb.CIProviderServiceClient + gatewaySvc pb.GatewayServiceClient + federationClientID string + clusterID string } -func NewCITrustReconciler(kc *keycloak.Client, conn *grpc.ClientConn) *CITrustReconciler { +func NewCITrustReconciler(kc *keycloak.Client, conn *grpc.ClientConn, clusterID string) *CITrustReconciler { + var client ciFederationKeycloak + // A typed-nil *keycloak.Client stored in an interface is non-nil, which would + // defeat the nil guard in Handle; only wrap a real client. + if kc != nil { + client = kc + } return &CITrustReconciler{ - keycloakClient: kc, - ciTrustSvc: pb.NewCITrustServiceClient(conn), - ciProviderSvc: pb.NewCIProviderServiceClient(conn), + keycloakClient: client, + ciTrustSvc: pb.NewCITrustServiceClient(conn), + ciProviderSvc: pb.NewCIProviderServiceClient(conn), + gatewaySvc: pb.NewGatewayServiceClient(conn), + federationClientID: defaultCIFederationClientID, + clusterID: clusterID, } } @@ -44,11 +74,7 @@ func NewCITrustReconciler(kc *keycloak.Client, conn *grpc.ClientConn) *CITrustRe func deriveWorkspace(forge, subjectRepository, subjectProjectPath string) string { switch forge { case "github": - owner := subjectRepository - if idx := strings.Index(subjectRepository, "/"); idx >= 0 { - owner = subjectRepository[:idx] - } - return "gh-" + strings.ToLower(owner) + return "gh-" + strings.ToLower(keycloak.OwnerFromRepository(subjectRepository)) case "gitlab": ns := subjectProjectPath if idx := strings.Index(subjectProjectPath, "/"); idx >= 0 { @@ -56,11 +82,7 @@ func deriveWorkspace(forge, subjectRepository, subjectProjectPath string) string } return "gl-" + strings.ToLower(ns) case "forgejo": - owner := subjectRepository - if idx := strings.Index(subjectRepository, "/"); idx >= 0 { - owner = subjectRepository[:idx] - } - return "fj-" + strings.ToLower(owner) + return "fj-" + strings.ToLower(keycloak.OwnerFromRepository(subjectRepository)) default: return "ci-" + strings.ToLower(forge) } @@ -73,6 +95,11 @@ func (r *CITrustReconciler) Handle(ctx context.Context, event watcher.Event[*pb. } id := event.ResourceID + if r.keycloakClient == nil { + log.Printf("DEBUG CITrust %s: keycloak not configured, skipping", id) + return nil + } + switch event.Type { case watcher.EventDeleted: return r.handleDelete(ctx, id, trust) @@ -82,47 +109,158 @@ func (r *CITrustReconciler) Handle(ctx context.Context, event watcher.Event[*pb. } func (r *CITrustReconciler) handleUpsert(ctx context.Context, id string, trust *pb.CITrust) error { - providerResp, err := r.ciProviderSvc.GetCIProvider(ctx, &pb.GetCIProviderRequest{ - Id: trust.GetCiProviderId(), - }) + provider, err := r.resolveProvider(ctx, trust) if err != nil { - return fmt.Errorf("fetching CI provider %s for trust %s: %w", trust.GetCiProviderId(), id, err) + return err } - provider := providerResp.GetCiProvider() - - workspace := deriveWorkspace( - provider.GetForge(), - trust.GetSubjectRepository(), - trust.GetSubjectProjectPath(), - ) - // TODO(PLAT-DATA-25): reconcile Keycloak artifacts: - // 1. Create/update claim mapper on the provider's IdP for the subject predicate - // 2. Enable token-exchange permission from the CI client to the gateway client - // 3. Bind the gateway RBAC role (trust.GetRole()) to the workspace identity - log.Printf("INFO CITrust %s: would reconcile Keycloak artifacts for gateway=%s workspace=%s role=%s (Keycloak integration pending)", - id, trust.GetGatewayId(), workspace, trust.GetRole()) - - statusWorkspace := workspace - statusWorkflowHint := fmt.Sprintf("openshell token-exchange --workspace %s --gateway %s", workspace, trust.GetGatewayId()) - _, err = r.ciTrustSvc.UpdateCITrust(ctx, &pb.UpdateCITrustRequest{ - Id: id, - StatusWorkspace: &statusWorkspace, - StatusWorkflowHint: &statusWorkflowHint, - }) + gatewayClientID, gatewayCluster, err := r.resolveGatewayClientID(ctx, trust.GetGatewayId()) if err != nil { - return fmt.Errorf("updating CI trust %s status: %w", id, err) + if isNotFoundRPC(err) { + // The gateway is not yet visible (provisioning race) or already gone. + // The exchange cannot be authorized onto a client that does not exist; + // return the error so it is logged and the trust is re-reconciled when + // the watch next reseeds current state (reconnect / idle timeout). + return fmt.Errorf("CITrust %s: gateway %s not found: %w", id, trust.GetGatewayId(), err) + } + return fmt.Errorf("CITrust %s: resolve gateway %s: %w", id, trust.GetGatewayId(), err) + } + if gatewayCluster != r.clusterID { + log.Printf("DEBUG CITrust %s: gateway %s belongs to cluster %s, not this cluster (%s); skipping", id, trust.GetGatewayId(), gatewayCluster, r.clusterID) + return nil } - log.Printf("INFO CITrust %s reconciled: workspace=%s", id, workspace) + spec := r.federationSpec(id, provider, trust, gatewayClientID) + + if err := r.keycloakClient.EnsureCIFederation(ctx, spec); err != nil { + var notFound *keycloak.ClientNotFoundError + if errors.As(err, ¬Found) { + // The gateway's Keycloak client has not been provisioned yet. Return the + // error so it is logged and retried on the next watch reseed, rather than + // failing terminally. + return fmt.Errorf("CITrust %s: gateway keycloak client not ready: %w", id, err) + } + return fmt.Errorf("CITrust %s: reconcile keycloak federation: %w", id, err) + } + + exchangeEndpoint := r.keycloakClient.Issuer() + "/protocol/openid-connect/token" + workflowHint := workflowHint(provider.GetForge(), exchangeEndpoint, provider.GetAudience(), r.federationClientID, spec.IdpAlias, gatewayClientID, spec.Workspace) + if _, err := r.ciTrustSvc.UpdateCITrust(ctx, &pb.UpdateCITrustRequest{ + Id: id, + StatusExchangeEndpoint: &exchangeEndpoint, + StatusWorkspace: &spec.Workspace, + StatusWorkflowHint: &workflowHint, + }); err != nil { + return fmt.Errorf("CITrust %s: update status: %w", id, err) + } + + log.Printf("INFO CITrust %s reconciled: workspace=%s gateway_client=%s idp=%s role=%s", id, spec.Workspace, gatewayClientID, spec.IdpAlias, trust.GetRole()) return nil } func (r *CITrustReconciler) handleDelete(ctx context.Context, id string, trust *pb.CITrust) error { - // TODO(PLAT-DATA-25): delete Keycloak claim mapper, token-exchange - // permission, and gateway RBAC binding for this trust - log.Printf("INFO CITrust %s: would delete Keycloak artifacts for gateway=%s (Keycloak integration pending)", - id, trust.GetGatewayId()) + provider, err := r.resolveProvider(ctx, trust) + if err != nil { + if isNotFoundRPC(err) { + // Referential integrity (PLAT-CITR-05.01) keeps a CIProvider alive while + // a CITrust references it, so a missing provider on delete means the + // provider (and its adopted IdP context) is already torn down. The IdP + // alias that targets the per-trust mapper is then unknowable, so there is + // nothing left for this reconciler to revoke. + log.Printf("WARN CITrust %s: CI provider %s not found on delete; cannot resolve IdP alias to revoke", id, trust.GetCiProviderId()) + return nil + } + return err + } + + // Determine ownership where possible. If the gateway is already gone (cascade + // delete), the per-trust IdP mapper cleanup is still safe and idempotent, so + // proceed; if the gateway resolves to another cluster, this control plane does + // not own it and must not touch the shared realm on its behalf. + gatewayClientID := "" + if clientID, gatewayCluster, resolveErr := r.resolveGatewayClientID(ctx, trust.GetGatewayId()); resolveErr != nil { + if !isNotFoundRPC(resolveErr) { + return fmt.Errorf("CITrust %s: resolve gateway %s: %w", id, trust.GetGatewayId(), resolveErr) + } + log.Printf("DEBUG CITrust %s: gateway %s no longer exists; revoking shared IdP mapper", id, trust.GetGatewayId()) + } else { + if gatewayCluster != r.clusterID { + log.Printf("DEBUG CITrust %s: gateway %s belongs to cluster %s, not this cluster (%s); skipping", id, trust.GetGatewayId(), gatewayCluster, r.clusterID) + return nil + } + gatewayClientID = clientID + } + spec := r.federationSpec(id, provider, trust, gatewayClientID) + if err := r.keycloakClient.DeleteCIFederation(ctx, spec); err != nil { + return fmt.Errorf("CITrust %s: revoke keycloak federation: %w", id, err) + } + + log.Printf("INFO CITrust %s revoked: workspace=%s idp=%s", id, spec.Workspace, spec.IdpAlias) return nil } + +// resolveProvider fetches the CITrust's CIProvider and returns it. The IdP alias +// the exchange uses is the provider's realized status.idp_alias (the alias +// Keycloak actually holds, per PLAT-CITR-01 adoption); see federationSpec. +func (r *CITrustReconciler) resolveProvider(ctx context.Context, trust *pb.CITrust) (*pb.CIProvider, error) { + resp, err := r.ciProviderSvc.GetCIProvider(ctx, &pb.GetCIProviderRequest{Id: trust.GetCiProviderId()}) + if err != nil { + return nil, fmt.Errorf("fetch CI provider %s: %w", trust.GetCiProviderId(), err) + } + provider := resp.GetCiProvider() + if provider == nil { + return nil, fmt.Errorf("CI provider %s not found", trust.GetCiProviderId()) + } + return provider, nil +} + +// federationSpec compiles the Keycloak desired state for a CITrust. The IdP alias +// is the provider's realized status.idp_alias; the control plane adopts the +// platform-provisioned IdP and never creates a parallel one (PLAT-CITR-01), so a +// provider whose alias is not yet realized yields an empty alias and +// EnsureCIFederation fails closed. +func (r *CITrustReconciler) federationSpec(id string, provider *pb.CIProvider, trust *pb.CITrust, gatewayClientID string) keycloak.CIFederationSpec { + workspace := deriveWorkspace(provider.GetForge(), trust.GetSubjectRepository(), trust.GetSubjectProjectPath()) + return keycloak.CIFederationSpec{ + TrustID: id, + Forge: provider.GetForge(), + IdpAlias: provider.GetStatusIdpAlias(), + FederationClientID: r.federationClientID, + GatewayClientID: gatewayClientID, + Workspace: workspace, + Role: trust.GetRole(), + Subject: keycloak.CISubjectPredicate{ + Repository: trust.GetSubjectRepository(), + RepositoryOwner: keycloak.OwnerFromRepository(trust.GetSubjectRepository()), + ProjectPath: trust.GetSubjectProjectPath(), + Environment: trust.GetSubjectEnvironment(), + Ref: trust.GetSubjectRef(), + RefProtected: trust.GetSubjectRefProtected(), + }, + } +} + +// resolveGatewayClientID looks up the gateway by id and returns the Keycloak +// client id in the {name}-{id} format the Keycloak provisioning spec specifies, +// plus the cluster the gateway is assigned to so the caller can tell whether this +// control plane owns it. It mirrors the RoleBinding reconciler's resolution. +func (r *CITrustReconciler) resolveGatewayClientID(ctx context.Context, gatewayID string) (string, string, error) { + resp, err := r.gatewaySvc.GetGateway(ctx, &pb.GetGatewayRequest{Id: gatewayID}) + if err != nil { + return "", "", fmt.Errorf("get gateway %s: %w", gatewayID, err) + } + gw := resp.GetGateway() + return fmt.Sprintf("%s-%s", gw.GetName(), gatewayID), gw.GetClusterId(), nil +} + +// workflowHint renders a runnable, secret-free CI snippet hint for the provider's +// forge that mints the OIDC token with the pinned audience and exchanges it at +// the cluster Keycloak (PLAT-CITR-06). +func workflowHint(forge, exchangeEndpoint, audience, federationClientID, idpAlias, gatewayClientID, workspace string) string { + return fmt.Sprintf( + "forge=%s workspace=%s: mint an OIDC token with aud=%s, then POST grant_type=urn:ietf:params:oauth:grant-type:token-exchange "+ + "client_id=%s subject_issuer=%s subject_token_type=urn:ietf:params:oauth:token-type:jwt audience=%s to %s", + forge, workspace, audience, federationClientID, idpAlias, gatewayClientID, exchangeEndpoint, + ) +} diff --git a/components/control-plane/internal/reconciler/ci_trust_reconciler_test.go b/components/control-plane/internal/reconciler/ci_trust_reconciler_test.go new file mode 100644 index 000000000..af64ca0d9 --- /dev/null +++ b/components/control-plane/internal/reconciler/ci_trust_reconciler_test.go @@ -0,0 +1,367 @@ +package reconciler + +import ( + "context" + "testing" + + pb "github.com/openshift-online/hypershell/components/api-server/pkg/api/grpc/hypershell/v1" + "github.com/openshift-online/hypershell/components/control-plane/internal/keycloak" + "github.com/openshift-online/hypershell/components/control-plane/internal/watcher" + "google.golang.org/grpc" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" +) + +const testClusterID = "cluster-local" + +// fakeCIKeycloak records the federation specs it is asked to realize/revoke and +// can be told to fail, so reconciler behavior is tested without a live Keycloak. +type fakeCIKeycloak struct { + ensured []keycloak.CIFederationSpec + deleted []keycloak.CIFederationSpec + ensureErr error + deleteErr error +} + +func (f *fakeCIKeycloak) EnsureCIFederation(_ context.Context, spec keycloak.CIFederationSpec) error { + f.ensured = append(f.ensured, spec) + return f.ensureErr +} + +func (f *fakeCIKeycloak) DeleteCIFederation(_ context.Context, spec keycloak.CIFederationSpec) error { + f.deleted = append(f.deleted, spec) + return f.deleteErr +} + +func (f *fakeCIKeycloak) Issuer() string { return "https://keycloak.example.com/realms/hypershell" } + +// Minimal gRPC service fakes: embed the generated client interface (nil method +// set) and override only the calls the reconciler makes. +type fakeCIProviderSvc struct { + pb.CIProviderServiceClient + provider *pb.CIProvider + notFound bool +} + +func (f *fakeCIProviderSvc) GetCIProvider(_ context.Context, _ *pb.GetCIProviderRequest, _ ...grpc.CallOption) (*pb.GetCIProviderResponse, error) { + if f.notFound { + return nil, status.Error(codes.NotFound, "ci provider not found") + } + return &pb.GetCIProviderResponse{CiProvider: f.provider}, nil +} + +type fakeCITrustSvc struct { + pb.CITrustServiceClient + updated *pb.UpdateCITrustRequest +} + +func (f *fakeCITrustSvc) UpdateCITrust(_ context.Context, req *pb.UpdateCITrustRequest, _ ...grpc.CallOption) (*pb.UpdateCITrustResponse, error) { + f.updated = req + return &pb.UpdateCITrustResponse{}, nil +} + +type fakeGatewaySvc struct { + pb.GatewayServiceClient + gateway *pb.Gateway + notFound bool +} + +func (f *fakeGatewaySvc) GetGateway(_ context.Context, _ *pb.GetGatewayRequest, _ ...grpc.CallOption) (*pb.GetGatewayResponse, error) { + if f.notFound { + return nil, status.Error(codes.NotFound, "gateway not found") + } + return &pb.GetGatewayResponse{Gateway: f.gateway}, nil +} + +func strptr(s string) *string { return &s } + +func newCITrustTestReconciler(kc ciFederationKeycloak, provider *pb.CIProvider, gw *fakeGatewaySvc, trustSvc *fakeCITrustSvc) *CITrustReconciler { + return &CITrustReconciler{ + keycloakClient: kc, + ciTrustSvc: trustSvc, + ciProviderSvc: &fakeCIProviderSvc{provider: provider}, + gatewaySvc: gw, + federationClientID: defaultCIFederationClientID, + clusterID: testClusterID, + } +} + +func githubProvider() *pb.CIProvider { + return &pb.CIProvider{ + Metadata: &pb.ObjectReference{Id: "prov-1"}, + Forge: "github", + Issuer: "https://token.actions.githubusercontent.com", + Audience: "openshell-fleet", + StatusIdpAlias: strptr("github-actions"), + } +} + +func githubTrust() *pb.CITrust { + return &pb.CITrust{ + Metadata: &pb.ObjectReference{Id: "trust-1"}, + CiProviderId: "prov-1", + GatewayId: "gw-1", + SubjectRepository: strptr("acme/app"), + Role: "openshell-user", + } +} + +func localGateway() *fakeGatewaySvc { + return &fakeGatewaySvc{gateway: &pb.Gateway{ + Metadata: &pb.ObjectReference{Id: "gw-1"}, + Name: "prod", + ClusterId: testClusterID, + }} +} + +func TestCITrustUpsert_RealizesFederationAndWritesStatus(t *testing.T) { + kc := &fakeCIKeycloak{} + trustSvc := &fakeCITrustSvc{} + r := newCITrustTestReconciler(kc, githubProvider(), localGateway(), trustSvc) + + err := r.Handle(t.Context(), watcher.Event[*pb.CITrust]{ + Type: watcher.EventCreated, + ResourceID: "trust-1", + Resource: githubTrust(), + }) + if err != nil { + t.Fatalf("Handle upsert: %v", err) + } + + if len(kc.ensured) != 1 { + t.Fatalf("EnsureCIFederation calls = %d, want 1", len(kc.ensured)) + } + spec := kc.ensured[0] + if spec.IdpAlias != "github-actions" { + t.Errorf("IdpAlias = %q, want the adopted github-actions alias", spec.IdpAlias) + } + if spec.GatewayClientID != "prod-gw-1" { + t.Errorf("GatewayClientID = %q, want prod-gw-1 ({name}-{id})", spec.GatewayClientID) + } + if spec.Workspace != "gh-acme" { + t.Errorf("Workspace = %q, want gh-acme", spec.Workspace) + } + if spec.Subject.RepositoryOwner != "acme" { + t.Errorf("RepositoryOwner = %q, want acme", spec.Subject.RepositoryOwner) + } + + if trustSvc.updated == nil { + t.Fatal("status was not written") + } + if got := trustSvc.updated.GetStatusWorkspace(); got != "gh-acme" { + t.Errorf("status.workspace = %q, want gh-acme", got) + } + if got := trustSvc.updated.GetStatusExchangeEndpoint(); got != "https://keycloak.example.com/realms/hypershell/protocol/openid-connect/token" { + t.Errorf("status.exchange_endpoint = %q", got) + } + if trustSvc.updated.GetStatusWorkflowHint() == "" { + t.Error("status.workflow_hint should be set") + } +} + +func TestCITrustUpsert_SkipsForeignCluster(t *testing.T) { + kc := &fakeCIKeycloak{} + trustSvc := &fakeCITrustSvc{} + gw := &fakeGatewaySvc{gateway: &pb.Gateway{ + Metadata: &pb.ObjectReference{Id: "gw-1"}, + Name: "prod", + ClusterId: "some-other-cluster", + }} + r := newCITrustTestReconciler(kc, githubProvider(), gw, trustSvc) + + err := r.Handle(t.Context(), watcher.Event[*pb.CITrust]{ + Type: watcher.EventUpdated, + ResourceID: "trust-1", + Resource: githubTrust(), + }) + if err != nil { + t.Fatalf("Handle: %v", err) + } + if len(kc.ensured) != 0 { + t.Errorf("foreign-cluster gateway must not be reconciled; got %d ensure calls", len(kc.ensured)) + } + if trustSvc.updated != nil { + t.Error("foreign-cluster gateway must not get a status write") + } +} + +func TestCITrustUpsert_GatewayNotFoundIsRetryable(t *testing.T) { + kc := &fakeCIKeycloak{} + gw := &fakeGatewaySvc{notFound: true} + r := newCITrustTestReconciler(kc, githubProvider(), gw, &fakeCITrustSvc{}) + + err := r.Handle(t.Context(), watcher.Event[*pb.CITrust]{ + Type: watcher.EventCreated, + ResourceID: "trust-1", + Resource: githubTrust(), + }) + if err == nil { + t.Fatal("expected an error so the resync retries when the gateway is absent") + } + if len(kc.ensured) != 0 { + t.Error("no federation should be realized when the gateway is absent") + } +} + +func TestCITrustUpsert_KeycloakClientNotReadyIsRetryable(t *testing.T) { + kc := &fakeCIKeycloak{ensureErr: &keycloak.ClientNotFoundError{ClientID: "prod-gw-1"}} + r := newCITrustTestReconciler(kc, githubProvider(), localGateway(), &fakeCITrustSvc{}) + + err := r.Handle(t.Context(), watcher.Event[*pb.CITrust]{ + Type: watcher.EventCreated, + ResourceID: "trust-1", + Resource: githubTrust(), + }) + if err == nil { + t.Fatal("expected a retryable error when the gateway keycloak client is not ready") + } +} + +func TestCITrustDelete_RevokesFederation(t *testing.T) { + kc := &fakeCIKeycloak{} + r := newCITrustTestReconciler(kc, githubProvider(), localGateway(), &fakeCITrustSvc{}) + + err := r.Handle(t.Context(), watcher.Event[*pb.CITrust]{ + Type: watcher.EventDeleted, + ResourceID: "trust-1", + Resource: githubTrust(), + }) + if err != nil { + t.Fatalf("Handle delete: %v", err) + } + if len(kc.deleted) != 1 { + t.Fatalf("DeleteCIFederation calls = %d, want 1", len(kc.deleted)) + } + if kc.deleted[0].TrustID != "trust-1" || kc.deleted[0].IdpAlias != "github-actions" { + t.Errorf("delete spec = %+v, want trust-1 on github-actions", kc.deleted[0]) + } +} + +func TestCITrustDelete_GoneGatewayStillRevokes(t *testing.T) { + kc := &fakeCIKeycloak{} + gw := &fakeGatewaySvc{notFound: true} + r := newCITrustTestReconciler(kc, githubProvider(), gw, &fakeCITrustSvc{}) + + err := r.Handle(t.Context(), watcher.Event[*pb.CITrust]{ + Type: watcher.EventDeleted, + ResourceID: "trust-1", + Resource: githubTrust(), + }) + if err != nil { + t.Fatalf("Handle delete: %v", err) + } + if len(kc.deleted) != 1 { + t.Errorf("a cascade-deleted gateway must still revoke the shared IdP mapper; got %d", len(kc.deleted)) + } +} + +func TestCITrustDelete_MissingProviderIsNoOp(t *testing.T) { + kc := &fakeCIKeycloak{} + r := &CITrustReconciler{ + keycloakClient: kc, + ciTrustSvc: &fakeCITrustSvc{}, + ciProviderSvc: &fakeCIProviderSvc{notFound: true}, + gatewaySvc: localGateway(), + federationClientID: defaultCIFederationClientID, + clusterID: testClusterID, + } + + err := r.Handle(t.Context(), watcher.Event[*pb.CITrust]{ + Type: watcher.EventDeleted, + ResourceID: "trust-1", + Resource: githubTrust(), + }) + if err != nil { + t.Fatalf("delete with a gone CIProvider should be a no-op, got %v", err) + } + if len(kc.deleted) != 0 { + t.Error("no revoke should be attempted when the provider (and its IdP alias) is gone") + } +} + +func TestCITrustHandle_NilKeycloakSkips(t *testing.T) { + r := NewCITrustReconciler(nil, nil, testClusterID) + err := r.Handle(t.Context(), watcher.Event[*pb.CITrust]{ + Type: watcher.EventCreated, + ResourceID: "trust-1", + Resource: githubTrust(), + }) + if err != nil { + t.Fatalf("Handle with nil keycloak should be a no-op, got %v", err) + } +} + +// TestCITrustUpsert_CrossGatewayAndCrossRepoIsolation proves the end-to-end +// wiring preserves per-trust isolation (finding 1): two CITrusts under the same +// owner - one for a different repo, one on a different gateway - each resolve to a +// distinct Keycloak group, so neither can inherit the other's role. The shared +// per-owner workspace label is reported in status for legibility, but it is never +// the group the role binds to. +func TestCITrustUpsert_CrossGatewayAndCrossRepoIsolation(t *testing.T) { + ensure := func(t *testing.T, trustID, repo, gatewayID, gatewayName, cluster, role string) keycloak.CIFederationSpec { + t.Helper() + kc := &fakeCIKeycloak{} + gw := &fakeGatewaySvc{gateway: &pb.Gateway{ + Metadata: &pb.ObjectReference{Id: gatewayID}, + Name: gatewayName, + ClusterId: cluster, + }} + r := newCITrustTestReconciler(kc, githubProvider(), gw, &fakeCITrustSvc{}) + trust := &pb.CITrust{ + Metadata: &pb.ObjectReference{Id: trustID}, + CiProviderId: "prov-1", + GatewayId: gatewayID, + SubjectRepository: strptr(repo), + Role: role, + } + if err := r.Handle(t.Context(), watcher.Event[*pb.CITrust]{ + Type: watcher.EventCreated, + ResourceID: trustID, + Resource: trust, + }); err != nil { + t.Fatalf("Handle(%s): %v", trustID, err) + } + if len(kc.ensured) != 1 { + t.Fatalf("ensure calls for %s = %d, want 1", trustID, len(kc.ensured)) + } + return kc.ensured[0] + } + + // Same owner, different repositories, same gateway. + low := ensure(t, "trust-low", "acme/low", "gw-1", "prod", testClusterID, "openshell-user") + high := ensure(t, "trust-high", "acme/high", "gw-1", "prod", testClusterID, "openshell-admin") + // Same owner and repository, but a different gateway (distinct CITrust). + gwA := ensure(t, "trust-gw-a", "acme/app", "gw-a", "edge-a", testClusterID, "openshell-user") + gwB := ensure(t, "trust-gw-b", "acme/app", "gw-b", "edge-b", testClusterID, "openshell-user") + + // All share the per-owner workspace label (informational). + for _, s := range []keycloak.CIFederationSpec{low, high, gwA, gwB} { + if s.Workspace != "gh-acme" { + t.Errorf("workspace label = %q, want gh-acme", s.Workspace) + } + } + // But every trust binds its role onto a distinct group. + groups := map[string]string{} + for _, s := range []keycloak.CIFederationSpec{low, high, gwA, gwB} { + if prev, dup := groups[s.GroupName()]; dup { + t.Errorf("trust %s and %s share group %q; isolation lost", prev, s.TrustID, s.GroupName()) + } + groups[s.GroupName()] = s.TrustID + } +} + +func TestDeriveWorkspace(t *testing.T) { + cases := []struct { + forge, repo, project, want string + }{ + {"github", "Acme/App", "", "gh-acme"}, + {"gitlab", "", "Acme/group/app", "gl-acme"}, + {"forgejo", "Acme/App", "", "fj-acme"}, + {"other", "", "", "ci-other"}, + } + for _, tc := range cases { + if got := deriveWorkspace(tc.forge, tc.repo, tc.project); got != tc.want { + t.Errorf("deriveWorkspace(%q,%q,%q) = %q, want %q", tc.forge, tc.repo, tc.project, got, tc.want) + } + } +} diff --git a/components/control-plane/internal/reconciler/role_binding_reconciler.go b/components/control-plane/internal/reconciler/role_binding_reconciler.go index e6b6449bb..d68180f50 100644 --- a/components/control-plane/internal/reconciler/role_binding_reconciler.go +++ b/components/control-plane/internal/reconciler/role_binding_reconciler.go @@ -111,7 +111,7 @@ func (r *RoleBindingReconciler) Handle(ctx context.Context, event watcher.Event[ kcClientID, gatewayClusterID, err := r.resolveKeycloakClientID(ctx, *rb.GatewayId) if err != nil { - if isGatewayNotFound(err) { + if isNotFoundRPC(err) { // The binding outlived its gateway: the gateway was deleted while the // stream was down, or the periodic resync's ListRoleBindings surfaced a // binding whose gateway is already gone. There is no per-gateway Keycloak @@ -220,10 +220,10 @@ func unionKcRoles(items []*pb.RoleBinding, excludeID string) map[string]bool { return desired } -// isGatewayNotFound reports whether err (possibly wrapped) is a gRPC NotFound, +// isNotFoundRPC reports whether err (possibly wrapped) is a gRPC NotFound. // which GetGateway returns when the binding's gateway no longer exists. It walks // the wrap chain because resolveKeycloakClientID wraps the RPC error with %w. -func isGatewayNotFound(err error) bool { +func isNotFoundRPC(err error) bool { for err != nil { if st, ok := status.FromError(err); ok && st.Code() == codes.NotFound { return true diff --git a/specs/platform/ci-trust-ext-api.spec.md b/specs/platform/ci-trust-ext-api.spec.md index 56f7fb829..fd4902f25 100644 --- a/specs/platform/ci-trust-ext-api.spec.md +++ b/specs/platform/ci-trust-ext-api.spec.md @@ -83,18 +83,33 @@ conflict and SHALL be rejected with 409. For a Ready `CITrust`, the control plane SHALL realize three things and nothing secret: -1. A **scoped claim-to-workspace mapping** on the referenced `CIProvider` IdP that - accepts only tokens whose claims match the compiled `subject` predicate and - stamps the derived workspace (`gh-` for GitHub, `gl-` for - GitLab) into the issued token. +1. A **scoped claim-to-group mapping** on the referenced `CIProvider` IdP that + accepts only tokens whose claims match the compiled `subject` predicate -- which + SHALL include the repository/owner identity claim for GitHub/Forgejo + (`repository`/`repository_owner`) or `project_path` for GitLab, since a + ref/environment-only predicate is not a trust boundary -- and lands the matched + identity in a **per-trust group** isolated to that one `CITrust` (its subject + and its gateway). The group SHALL NOT be a shared per-owner group: aggregating + distinct trusts' roles onto one `gh-` group would let a low-privilege + repository inherit a sibling repository's higher role and leak roles across + gateways. 2. A **token-exchange permission** from the shared federation client onto the target gateway's audience, so the exchange grant can mint a gateway-scoped token. -3. A **gateway RBAC binding** granting `role` in the derived workspace on the - referenced `Gateway`. +3. A **gateway RBAC binding** granting `role` to that per-trust group on the + referenced `Gateway`, so a matched identity holds exactly the role its own + `CITrust` grants on its own gateway and nothing more. + +The derived workspace label (`gh-` for GitHub, `gl-` for GitLab) +remains the forge-namespaced tenancy namespace reported in `CITrust` status and +used for quota; it is a display/quota namespace, not the RBAC group the role binds +to. The control plane SHALL render no raw credential, PEM, or `Secret` for a -`CITrust`. Deleting a `CITrust` SHALL remove all three realized artifacts. +`CITrust`. Deleting a `CITrust` SHALL remove the per-trust subject mapper, which +is the authoritative revocation (the deleted subject can no longer enter its group +and so holds no role); the inert per-trust group and the gateway's shared +exchange opt-in MAY be left in place. #### Scenario: PLAT-CITR-02.01 -- Forge-Namespaced Workspace Derivation @@ -104,6 +119,15 @@ The control plane SHALL render no raw credential, PEM, or `Secret` for a - THEN the first SHALL map to workspace `gh-acme` and the second to `gl-acme` - AND the two SHALL be isolated workspaces with independent RBAC +#### Scenario: PLAT-CITR-02.02 -- Same-Owner Repositories Do Not Cross-Inherit + +- GIVEN a `CITrust` for `subject.repository = acme/low` granting `openshell-user` on `gw-1` +- AND a second `CITrust` for `subject.repository = acme/high` granting `openshell-admin` on `gw-1` +- WHEN both are reconciled +- THEN each SHALL bind its role to its own per-trust group, not a shared `gh-acme` group +- AND a token from `acme/low` SHALL hold only `openshell-user` (it SHALL NOT inherit `openshell-admin`) +- AND a `CITrust` role granted on one gateway SHALL NOT bind onto another gateway's client + --- ### Requirement: PLAT-CITR-03 -- Gateway Ownership and Role Bound