diff --git a/internal/keycloak/adapter.go b/internal/keycloak/adapter.go index 756f8c2..c87c3c6 100644 --- a/internal/keycloak/adapter.go +++ b/internal/keycloak/adapter.go @@ -75,11 +75,12 @@ type Adapter interface { SyncClaims(ctx context.Context, userID string, c Claims) error // Memberships resolves the tenants user userID (the Keycloak user id, - // i.e. the JWT `sub`) belongs to, as Keycloak records them. The realm - // has no Organizations yet, so this reads the user's attribute - // projection — zero or one memberships. When the realm migrates to - // Organizations this becomes an org-membership query and callers keep - // working unchanged. Returns ErrUserNotFound for an unknown user id. + // i.e. the JWT `sub`) belongs to, as Keycloak records them. Since the + // realm enabled Organizations (2026-09-01) this is an org-membership + // query: one membership per enabled org the user belongs to (alias = + // tenant slug, org attribute tenant_id = registry UUID). The legacy + // user-attribute projection no longer grants membership on its own. + // Returns ErrUserNotFound for an unknown user id. Memberships(ctx context.Context, userID string) ([]Claims, error) // Health pings the admin endpoint. Used by readyz and the cluster cold- diff --git a/internal/keycloak/users.go b/internal/keycloak/users.go index 23184b8..de0cf5c 100644 --- a/internal/keycloak/users.go +++ b/internal/keycloak/users.go @@ -18,7 +18,33 @@ type userRepresentation struct { Attributes map[string][]string `json:"attributes"` } -// Memberships implements Adapter against GET /admin/realms/{realm}/users/{id}. +// memberOrgRepresentation is the slice of OrganizationRepresentation the +// membership query needs: the alias IS the tenant slug and the org +// attribute "tenant_id" carries the registry tenant UUID (both written by +// CreateOrgAndInvite, or provisioned by the realm admin for pre-existing +// tenants). +type memberOrgRepresentation struct { + ID string `json:"id"` + Alias string `json:"alias"` + Enabled bool `json:"enabled"` + Attributes map[string][]string `json:"attributes"` +} + +// Memberships implements Adapter with Keycloak Organizations as the +// authoritative membership source (prod shape, realm orgs enabled +// 2026-09-01): +// +// 1. GET /users/{id} — preserves ErrUserNotFound semantics and supplies +// the per-user claim attributes (org_roles / products / plan / +// tenant_status) that SyncClaims maintains. +// 2. GET /organizations/members/{id}/organizations — the memberships. +// +// One Claims entry per ENABLED organization: TenantID = org attribute +// "tenant_id" (registry UUID), TenantSlug = org alias. A disabled org +// grants no membership. A user in no organization has zero memberships — +// the legacy user-attribute tenant projection is NO LONGER consulted for +// membership, so a stale tenant_id/tenant_slug user attribute cannot +// grant access the org model has revoked. func (a *HTTPAdapter) Memberships(ctx context.Context, userID string) ([]Claims, error) { var u userRepresentation resp, err := a.adminCall(ctx, http.MethodGet, "/users/"+userID, nil, &u) @@ -33,25 +59,34 @@ func (a *HTTPAdapter) Memberships(ctx context.Context, userID string) ([]Claims, _ = resp.Body.Close() return nil, fmt.Errorf("keycloak get user: %d", resp.StatusCode) } - return claimsFromAttributes(u.Attributes), nil -} -// claimsFromAttributes builds the membership list from a user's attribute -// projection. A user with no tenant_id and no tenant_slug attribute simply -// has no memberships — that is a valid state, not an error. -func claimsFromAttributes(attrs map[string][]string) []Claims { - c := Claims{ - TenantID: attrValue(attrs, "tenant_id"), - TenantSlug: attrValue(attrs, "tenant_slug"), - OrgRoles: attrValues(attrs, "org_roles"), - Products: attrValues(attrs, "products"), - Plan: attrValue(attrs, "plan"), - TenantStatus: attrValue(attrs, "tenant_status"), + var orgs []memberOrgRepresentation + resp, err = a.adminCall( + ctx, http.MethodGet, "/organizations/members/"+userID+"/organizations", nil, &orgs, + ) + if err != nil { + return nil, err } - if c.TenantID == "" && c.TenantSlug == "" { - return []Claims{} + if resp.StatusCode/100 != 2 { + _ = resp.Body.Close() + return nil, fmt.Errorf("keycloak member organizations: %d", resp.StatusCode) } - return []Claims{c} + + claims := []Claims{} + for _, org := range orgs { + if !org.Enabled { + continue + } + claims = append(claims, Claims{ + TenantID: attrValue(org.Attributes, "tenant_id"), + TenantSlug: org.Alias, + OrgRoles: attrValues(u.Attributes, "org_roles"), + Products: attrValues(u.Attributes, "products"), + Plan: attrValue(u.Attributes, "plan"), + TenantStatus: attrValue(u.Attributes, "tenant_status"), + }) + } + return claims, nil } func attrValue(attrs map[string][]string, key string) string { diff --git a/internal/keycloak/users_test.go b/internal/keycloak/users_test.go index 9921ac1..0148829 100644 --- a/internal/keycloak/users_test.go +++ b/internal/keycloak/users_test.go @@ -9,9 +9,13 @@ import ( "testing" ) -// stubUsersKC is a users-endpoint-only KC look-alike; stubKC (client_test.go) -// covers the org/invite paths and doesn't register GET /users/{id}. -func stubUsersKC(t *testing.T, users map[string]userRepresentation) *httptest.Server { +// stubUsersKC is a users+organizations KC look-alike; stubKC (client_test.go) +// covers the org-create/invite paths and doesn't register these reads. +func stubUsersKC( + t *testing.T, + users map[string]userRepresentation, + memberOrgs map[string][]memberOrgRepresentation, +) *httptest.Server { t.Helper() mux := http.NewServeMux() mux.HandleFunc("/realms/test-realm/protocol/openid-connect/token", func(w http.ResponseWriter, _ *http.Request) { @@ -28,6 +32,14 @@ func stubUsersKC(t *testing.T, users map[string]userRepresentation) *httptest.Se w.Header().Set("Content-Type", "application/json") _ = json.NewEncoder(w).Encode(u) }) + mux.HandleFunc("GET /admin/realms/test-realm/organizations/members/{id}/organizations", func(w http.ResponseWriter, r *http.Request) { + orgs, ok := memberOrgs[r.PathValue("id")] + if !ok { + orgs = []memberOrgRepresentation{} + } + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(orgs) + }) srv := httptest.NewServer(mux) t.Cleanup(srv.Close) return srv @@ -40,10 +52,11 @@ func usersAdapter(srv *httptest.Server) *HTTPAdapter { } func TestHTTPAdapter_Memberships(t *testing.T) { - srv := stubUsersKC(t, map[string]userRepresentation{ + users := map[string]userRepresentation{ "u-1": {ID: "u-1", Username: "test@breakpilot.com", Enabled: true, Attributes: map[string][]string{ - "tenant_id": {"acme-001"}, - "tenant_slug": {"acme"}, + // legacy projection attrs — MUST NOT grant membership on their own + "tenant_id": {"stale-legacy-001"}, + "tenant_slug": {"stale"}, "tenant_status": {"active"}, "plan": {"Scale"}, "org_roles": {"IT_ADMIN", "FINANCE"}, @@ -51,10 +64,30 @@ func TestHTTPAdapter_Memberships(t *testing.T) { "products": {"compliance##certifai"}, }}, "u-2": {ID: "u-2", Username: "bare@breakpilot.com", Enabled: true}, - }) + "u-3": {ID: "u-3", Username: "multi@breakpilot.com", Enabled: true}, + "u-4": {ID: "u-4", Username: "attrs-only@breakpilot.com", Enabled: true, Attributes: map[string][]string{ + "tenant_id": {"acme-001"}, + "tenant_slug": {"acme"}, + }}, + } + memberOrgs := map[string][]memberOrgRepresentation{ + "u-1": {{ + ID: "org-1", Alias: "acme", Enabled: true, + Attributes: map[string][]string{"tenant_id": {"2f875d6a-1d94-433a-b2ec-8529451a2d89"}}, + }}, + "u-3": { + {ID: "org-1", Alias: "acme", Enabled: true, + Attributes: map[string][]string{"tenant_id": {"2f875d6a-1d94-433a-b2ec-8529451a2d89"}}}, + {ID: "org-2", Alias: "globex", Enabled: true, + Attributes: map[string][]string{"tenant_id": {"7c3f2b10-0000-4000-8000-000000000042"}}}, + {ID: "org-3", Alias: "disabled-co", Enabled: false, + Attributes: map[string][]string{"tenant_id": {"9e9e9e9e-0000-4000-8000-000000000099"}}}, + }, + } + srv := stubUsersKC(t, users, memberOrgs) a := usersAdapter(srv) - t.Run("attribute projection becomes one membership", func(t *testing.T) { + t.Run("org membership becomes the claim, org attrs are authoritative", func(t *testing.T) { got, err := a.Memberships(context.Background(), "u-1") if err != nil { t.Fatalf("memberships: %v", err) @@ -63,8 +96,14 @@ func TestHTTPAdapter_Memberships(t *testing.T) { t.Fatalf("want 1 membership, got %d", len(got)) } c := got[0] - if c.TenantID != "acme-001" || c.TenantSlug != "acme" || c.Plan != "Scale" || c.TenantStatus != "active" { - t.Errorf("scalar claims wrong: %+v", c) + // tenant identity comes from the ORG (alias + tenant_id attribute), + // never from the user's legacy projection attributes + if c.TenantID != "2f875d6a-1d94-433a-b2ec-8529451a2d89" || c.TenantSlug != "acme" { + t.Errorf("org identity wrong: %+v", c) + } + // per-user claim attrs still ride along + if c.Plan != "Scale" || c.TenantStatus != "active" { + t.Errorf("user claim attrs wrong: %+v", c) } if len(c.OrgRoles) != 2 || c.OrgRoles[0] != "IT_ADMIN" || c.OrgRoles[1] != "FINANCE" { t.Errorf("org_roles wrong: %v", c.OrgRoles) @@ -74,7 +113,7 @@ func TestHTTPAdapter_Memberships(t *testing.T) { } }) - t.Run("user without tenant attributes has zero memberships", func(t *testing.T) { + t.Run("user in no org has zero memberships", func(t *testing.T) { got, err := a.Memberships(context.Background(), "u-2") if err != nil { t.Fatalf("memberships: %v", err) @@ -84,6 +123,29 @@ func TestHTTPAdapter_Memberships(t *testing.T) { } }) + t.Run("legacy tenant attributes alone grant NO membership", func(t *testing.T) { + got, err := a.Memberships(context.Background(), "u-4") + if err != nil { + t.Fatalf("memberships: %v", err) + } + if len(got) != 0 { + t.Fatalf("attribute projection must not grant membership, got %+v", got) + } + }) + + t.Run("multiple orgs give multiple memberships, disabled org skipped", func(t *testing.T) { + got, err := a.Memberships(context.Background(), "u-3") + if err != nil { + t.Fatalf("memberships: %v", err) + } + if len(got) != 2 { + t.Fatalf("want 2 memberships (disabled org skipped), got %d", len(got)) + } + if got[0].TenantSlug != "acme" || got[1].TenantSlug != "globex" { + t.Errorf("slugs wrong: %+v", got) + } + }) + t.Run("unknown user is ErrUserNotFound", func(t *testing.T) { _, err := a.Memberships(context.Background(), "nope") if !errors.Is(err, ErrUserNotFound) { @@ -92,15 +154,24 @@ func TestHTTPAdapter_Memberships(t *testing.T) { }) } -func TestMock_Memberships(t *testing.T) { - m := NewMock() - if _, err := m.Memberships(context.Background(), "ghost"); !errors.Is(err, ErrUserNotFound) { - t.Fatalf("want ErrUserNotFound, got %v", err) - } - want := Claims{TenantSlug: "acme", OrgRoles: []string{"IT_ADMIN"}} - m.Claims["u-1"] = want - got, err := m.Memberships(context.Background(), "u-1") - if err != nil || len(got) != 1 || got[0].TenantSlug != "acme" { - t.Fatalf("got %+v err %v", got, err) +func TestHTTPAdapter_Memberships_OrgQueryFailure(t *testing.T) { + mux := http.NewServeMux() + mux.HandleFunc("/realms/test-realm/protocol/openid-connect/token", func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(map[string]any{"access_token": "t", "expires_in": 60}) + }) + mux.HandleFunc("GET /admin/realms/test-realm/users/{id}", func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(userRepresentation{ID: "u-1", Enabled: true}) + }) + mux.HandleFunc("GET /admin/realms/test-realm/organizations/members/{id}/organizations", func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusInternalServerError) + }) + srv := httptest.NewServer(mux) + t.Cleanup(srv.Close) + + _, err := usersAdapter(srv).Memberships(context.Background(), "u-1") + if err == nil { + t.Fatal("want error when the org query fails, got nil") } }