From db3b999766d6f0439967af58cf003d272e3d0540 Mon Sep 17 00:00:00 2001 From: user Date: Wed, 9 Sep 2026 16:57:43 -0400 Subject: [PATCH 1/7] [HYPERSHELL-262] feat(rbac): assign gateway:creator by default on user provisioning All authenticated users now receive a gateway:creator role binding on their first request, so RBAC_ENFORCE=true does not block new users from creating or listing gateways. SyncJWTRoles merges a configurable RBAC_DEFAULT_ROLES list (defaults to gateway:creator) into the effective JWT role set before reconciling bindings. Default roles are never revoked by subsequent syncs and cannot be duplicated. Roles must be in JWTSyncedRoles to participate in the sync lifecycle. Co-Authored-By: Claude Sonnet 4.6 --- .../plugins/roleBindings/integration_test.go | 108 ++++++++++++++++++ .../api-server/plugins/roleBindings/plugin.go | 20 ++++ .../plugins/roleBindings/service.go | 27 +++-- 3 files changed, 147 insertions(+), 8 deletions(-) diff --git a/components/api-server/plugins/roleBindings/integration_test.go b/components/api-server/plugins/roleBindings/integration_test.go index 35015c0b7..2b5fe7cc7 100644 --- a/components/api-server/plugins/roleBindings/integration_test.go +++ b/components/api-server/plugins/roleBindings/integration_test.go @@ -316,3 +316,111 @@ func TestGrantValidation_CrossGatewayEscalation(t *testing.T) { Expect(crossGWErr).To(HaveOccurred()) Expect(crossGWErr.HttpCode).To(Equal(http.StatusForbidden)) } + +// TestSyncJWTRoles_DefaultRoleAssigned verifies that a newly provisioned user +// receives a gateway:creator binding even when the JWT carries no roles. +func TestSyncJWTRoles_DefaultRoleAssigned(t *testing.T) { + test.RegisterIntegration(t) + + rbService := roleBindings.Service(&environments.Environment().Services) + userService := users.Service(&environments.Environment().Services) + + userID, userErr := userService.UpsertByUsername(context.Background(), "sync-default-new", nil, nil) + Expect(userErr).NotTo(HaveOccurred()) + + syncErr := rbService.SyncJWTRoles(context.Background(), userID, nil) + Expect(syncErr).NotTo(HaveOccurred()) + + bindings, findErr := rbService.FindBindingsByUserID(context.Background(), userID) + Expect(findErr).NotTo(HaveOccurred()) + + found := false + for _, b := range bindings { + if b.RoleName == roles.RoleGatewayCreator && b.Scope == roleBindings.ScopeGlobal { + found = true + } + } + Expect(found).To(BeTrue(), "expected gateway:creator global binding after sync with empty JWT") +} + +// TestSyncJWTRoles_DefaultRoleIsIdempotent verifies that calling SyncJWTRoles +// twice does not create duplicate gateway:creator bindings. +func TestSyncJWTRoles_DefaultRoleIsIdempotent(t *testing.T) { + test.RegisterIntegration(t) + + rbService := roleBindings.Service(&environments.Environment().Services) + userService := users.Service(&environments.Environment().Services) + + userID, userErr := userService.UpsertByUsername(context.Background(), "sync-default-idempotent", nil, nil) + Expect(userErr).NotTo(HaveOccurred()) + + Expect(rbService.SyncJWTRoles(context.Background(), userID, nil)).To(Succeed()) + Expect(rbService.SyncJWTRoles(context.Background(), userID, nil)).To(Succeed()) + + bindings, findErr := rbService.FindBindingsByUserID(context.Background(), userID) + Expect(findErr).NotTo(HaveOccurred()) + + creatorCount := 0 + for _, b := range bindings { + if b.RoleName == roles.RoleGatewayCreator && b.Scope == roleBindings.ScopeGlobal { + creatorCount++ + } + } + Expect(creatorCount).To(Equal(1), "expected exactly one gateway:creator binding after two syncs") +} + +// TestSyncJWTRoles_DefaultRoleNotRemovedWhenAbsentFromJWT verifies that a user's +// gateway:creator binding is retained across syncs even when the JWT never carries it. +func TestSyncJWTRoles_DefaultRoleNotRemovedWhenAbsentFromJWT(t *testing.T) { + test.RegisterIntegration(t) + + rbService := roleBindings.Service(&environments.Environment().Services) + userService := users.Service(&environments.Environment().Services) + + userID, userErr := userService.UpsertByUsername(context.Background(), "sync-default-persist", nil, nil) + Expect(userErr).NotTo(HaveOccurred()) + + // First sync assigns the default. + Expect(rbService.SyncJWTRoles(context.Background(), userID, nil)).To(Succeed()) + + // Second sync with still-empty JWT must not revoke it. + Expect(rbService.SyncJWTRoles(context.Background(), userID, nil)).To(Succeed()) + + bindings, findErr := rbService.FindBindingsByUserID(context.Background(), userID) + Expect(findErr).NotTo(HaveOccurred()) + + found := false + for _, b := range bindings { + if b.RoleName == roles.RoleGatewayCreator && b.Scope == roleBindings.ScopeGlobal { + found = true + } + } + Expect(found).To(BeTrue(), "gateway:creator binding must survive a sync with an empty JWT") +} + +// TestSyncJWTRoles_JWTRoleAddedAlongsideDefault verifies that a JWT-carried role +// (platform:admin) is synced in addition to the default gateway:creator. +func TestSyncJWTRoles_JWTRoleAddedAlongsideDefault(t *testing.T) { + test.RegisterIntegration(t) + + rbService := roleBindings.Service(&environments.Environment().Services) + userService := users.Service(&environments.Environment().Services) + + userID, userErr := userService.UpsertByUsername(context.Background(), "sync-default-plus-jwt", nil, nil) + Expect(userErr).NotTo(HaveOccurred()) + + syncErr := rbService.SyncJWTRoles(context.Background(), userID, []string{roles.RolePlatformAdmin}) + Expect(syncErr).NotTo(HaveOccurred()) + + bindings, findErr := rbService.FindBindingsByUserID(context.Background(), userID) + Expect(findErr).NotTo(HaveOccurred()) + + roleNames := make(map[string]bool) + for _, b := range bindings { + if b.Scope == roleBindings.ScopeGlobal { + roleNames[b.RoleName] = true + } + } + Expect(roleNames[roles.RoleGatewayCreator]).To(BeTrue(), "expected default gateway:creator binding") + Expect(roleNames[roles.RolePlatformAdmin]).To(BeTrue(), "expected JWT-carried platform:admin binding") +} diff --git a/components/api-server/plugins/roleBindings/plugin.go b/components/api-server/plugins/roleBindings/plugin.go index 487a2a9c7..85a285bf4 100644 --- a/components/api-server/plugins/roleBindings/plugin.go +++ b/components/api-server/plugins/roleBindings/plugin.go @@ -2,6 +2,8 @@ package roleBindings import ( "net/http" + "os" + "strings" "github.com/gorilla/mux" "google.golang.org/grpc" @@ -24,16 +26,34 @@ import ( type ServiceLocator func() RoleBindingService func NewServiceLocator(env *environments.Env) ServiceLocator { + defaultRoles := defaultRolesFromEnv() return func() RoleBindingService { return NewRoleBindingService( db.NewAdvisoryLockFactory(env.Database.SessionFactory), NewRoleBindingDao(&env.Database.SessionFactory), roles.NewRoleDao(&env.Database.SessionFactory), events.Service(&env.Services), + defaultRoles, ) } } +// defaultRolesFromEnv reads RBAC_DEFAULT_ROLES (comma-separated role names). +// Defaults to gateway:creator so all authenticated users can create gateways. +func defaultRolesFromEnv() []string { + val := os.Getenv("RBAC_DEFAULT_ROLES") + if val == "" { + return []string{roles.RoleGatewayCreator} + } + var result []string + for _, r := range strings.Split(val, ",") { + if trimmed := strings.TrimSpace(r); trimmed != "" { + result = append(result, trimmed) + } + } + return result +} + func Service(s *environments.Services) RoleBindingService { if s == nil { return nil diff --git a/components/api-server/plugins/roleBindings/service.go b/components/api-server/plugins/roleBindings/service.go index c22c10b8b..bcea082a9 100644 --- a/components/api-server/plugins/roleBindings/service.go +++ b/components/api-server/plugins/roleBindings/service.go @@ -35,22 +35,25 @@ func NewRoleBindingService( rbDao RoleBindingDao, roleDao roles.RoleDao, events services.EventService, + defaultRoles []string, ) RoleBindingService { return &sqlRoleBindingService{ - lockFactory: lockFactory, - rbDao: rbDao, - roleDao: roleDao, - events: events, + lockFactory: lockFactory, + rbDao: rbDao, + roleDao: roleDao, + events: events, + defaultRoles: defaultRoles, } } var _ RoleBindingService = &sqlRoleBindingService{} type sqlRoleBindingService struct { - lockFactory db.LockFactory - rbDao RoleBindingDao - roleDao roles.RoleDao - events services.EventService + lockFactory db.LockFactory + rbDao RoleBindingDao + roleDao roles.RoleDao + events services.EventService + defaultRoles []string } func (s *sqlRoleBindingService) CreateGatewayOwnerBinding(ctx context.Context, userID string, gatewayID string) error { @@ -97,6 +100,14 @@ func (s *sqlRoleBindingService) SyncJWTRoles(ctx context.Context, userID string, jwtRoleSet[r] = true } } + // Default roles are always treated as present regardless of JWT content. + // They must be in JWTSyncedRoles to participate in the sync lifecycle + // (deduplication, removal on revocation). + for _, r := range s.defaultRoles { + if roles.JWTSyncedRoles[r] { + jwtRoleSet[r] = true + } + } existing, err := s.rbDao.FindByUserID(ctx, userID) if err != nil { From a36d552a009a5335944bc5bfd4e47fa84b02894e Mon Sep 17 00:00:00 2001 From: user Date: Wed, 9 Sep 2026 17:33:59 -0400 Subject: [PATCH 2/7] [HYPERSHELL-262] fix(rbac): always sync JWT roles so default bindings are assigned SyncJWTRoles was guarded by `if len(jwtRoles) > 0` at both the HTTP and gRPC call sites, so users whose JWT carried no Keycloak realm roles never had the call made and therefore never received the configured default role bindings (e.g. gateway:creator). Remove the guard at both sites; the context-key assignment (needed by downstream middleware) remains gated on non-empty roles. SyncJWTRoles now runs unconditionally so the default-roles path is exercised on every provisioned user. Also adds a startup warning when an RBAC_DEFAULT_ROLES entry is not in JWTSyncedRoles, and fixes integration tests that relied on user ID being absent from context (which masked missing gateway:owner preconditions). Co-Authored-By: Claude Sonnet 4.6 --- .../api-server/pkg/rbac/grpc_interceptor.go | 15 +++-- .../api-server/pkg/rbac/user_provisioning.go | 13 +++-- .../plugins/roleBindings/integration_test.go | 58 ++++++++++++++++++- .../plugins/roleBindings/service.go | 6 ++ .../plugins/roleBindings/testmain_test.go | 4 ++ 5 files changed, 83 insertions(+), 13 deletions(-) diff --git a/components/api-server/pkg/rbac/grpc_interceptor.go b/components/api-server/pkg/rbac/grpc_interceptor.go index b45298178..59db094c3 100644 --- a/components/api-server/pkg/rbac/grpc_interceptor.go +++ b/components/api-server/pkg/rbac/grpc_interceptor.go @@ -198,13 +198,16 @@ func provisionUserForGRPC(ctx context.Context, provisioner UserProvisioner, sync ctx = context.WithValue(ctx, ContextUserIDKey, userID) + jwtRoles := extractJWTRolesFromContext(ctx) + if len(jwtRoles) > 0 { + ctx = context.WithValue(ctx, ContextJWTRolesKey, jwtRoles) + } + // Always sync even when jwtRoles is empty: SyncJWTRoles applies + // configured default roles (e.g. gateway:creator) so that users with + // no Keycloak realm roles still receive their initial bindings. if syncer != nil { - jwtRoles := extractJWTRolesFromContext(ctx) - if len(jwtRoles) > 0 { - ctx = context.WithValue(ctx, ContextJWTRolesKey, jwtRoles) - if syncErr := syncer.SyncJWTRoles(ctx, userID, jwtRoles); syncErr != nil { - glog.Warningf("gRPC JWT role sync failed for %q: %v", username, syncErr) - } + if syncErr := syncer.SyncJWTRoles(ctx, userID, jwtRoles); syncErr != nil { + glog.Warningf("gRPC JWT role sync failed for %q: %v", username, syncErr) } } diff --git a/components/api-server/pkg/rbac/user_provisioning.go b/components/api-server/pkg/rbac/user_provisioning.go index ce68ec37f..88c19b8df 100644 --- a/components/api-server/pkg/rbac/user_provisioning.go +++ b/components/api-server/pkg/rbac/user_provisioning.go @@ -28,7 +28,7 @@ type UserProvisioner interface { func UserProvisioningMiddleware(provisioner UserProvisioner, syncer JWTRoleSyncer) func(http.Handler) http.Handler { return func(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - payload, err := auth.GetAuthPayload(r) + payload, err := auth.GetAuthPayload(r) if err != nil { next.ServeHTTP(w, r) return @@ -51,10 +51,13 @@ func UserProvisioningMiddleware(provisioner UserProvisioner, syncer JWTRoleSynce jwtRoles := extractJWTRoles(r) if len(jwtRoles) > 0 { ctx = context.WithValue(ctx, ContextJWTRolesKey, jwtRoles) - if syncer != nil { - if syncErr := syncer.SyncJWTRoles(ctx, userID, jwtRoles); syncErr != nil { - glog.Warningf("JWT role sync failed for %q: %v", payload.Username, syncErr) - } + } + // Always sync even when jwtRoles is empty: SyncJWTRoles applies + // configured default roles (e.g. gateway:creator) so that users with + // no Keycloak realm roles still receive their initial bindings. + if syncer != nil { + if syncErr := syncer.SyncJWTRoles(ctx, userID, jwtRoles); syncErr != nil { + glog.Warningf("JWT role sync failed for %q: %v", payload.Username, syncErr) } } diff --git a/components/api-server/plugins/roleBindings/integration_test.go b/components/api-server/plugins/roleBindings/integration_test.go index 2b5fe7cc7..2ef4435ca 100644 --- a/components/api-server/plugins/roleBindings/integration_test.go +++ b/components/api-server/plugins/roleBindings/integration_test.go @@ -3,6 +3,7 @@ package roleBindings_test import ( "context" "net/http" + "strings" "testing" . "github.com/onsi/gomega" @@ -59,12 +60,20 @@ func TestRoleBindingCreate_GatewayOwner(t *testing.T) { account := h.NewRandAccount() ctx := h.NewAuthenticatedContext(account) + userService := users.Service(&environments.Environment().Services) + rbService := roleBindings.Service(&environments.Environment().Services) roleService := roles.Service(&environments.Environment().Services) + ownerRole, svcErr := roleService.GetByName(context.Background(), roles.RoleGatewayOwner) Expect(svcErr).NotTo(HaveOccurred()) gatewayID := "gw-test-create" + // Provision the account as gateway:owner so the HTTP create passes validation. + callerID, userErr := userService.UpsertByUsername(context.Background(), strings.ToLower(account.Username), nil, nil) + Expect(userErr).NotTo(HaveOccurred()) + Expect(rbService.CreateGatewayOwnerBinding(context.Background(), callerID, gatewayID)).To(Succeed()) + rbInput := openapi.RoleBinding{ RoleId: ownerRole.ID, Scope: "gateway", @@ -105,13 +114,23 @@ func TestRoleBindingDelete(t *testing.T) { account := h.NewRandAccount() ctx := h.NewAuthenticatedContext(account) + userService := users.Service(&environments.Environment().Services) + rbService := roleBindings.Service(&environments.Environment().Services) roleService := roles.Service(&environments.Environment().Services) + viewerRole, svcErr := roleService.GetByName(context.Background(), roles.RoleGatewayViewer) Expect(svcErr).NotTo(HaveOccurred()) gatewayID := "gw-test-delete" - rbService := roleBindings.Service(&environments.Environment().Services) - rb, createErr := rbService.Create(context.Background(), &roleBindings.RoleBinding{ + + // Provision the account as gateway:owner so the HTTP delete passes validation. + callerID, userErr := userService.UpsertByUsername(context.Background(), strings.ToLower(account.Username), nil, nil) + Expect(userErr).NotTo(HaveOccurred()) + Expect(rbService.CreateGatewayOwnerBinding(context.Background(), callerID, gatewayID)).To(Succeed()) + + // Create a viewer binding to delete (bypassing HTTP so no owner check needed for setup). + ownerCtx := context.WithValue(context.Background(), rbac.ContextUserIDKey, callerID) + rb, createErr := rbService.Create(ownerCtx, &roleBindings.RoleBinding{ RoleID: viewerRole.ID, Scope: roleBindings.ScopeGateway, GatewayID: &gatewayID, @@ -317,6 +336,41 @@ func TestGrantValidation_CrossGatewayEscalation(t *testing.T) { Expect(crossGWErr.HttpCode).To(Equal(http.StatusForbidden)) } +// TestUserProvisioningMiddleware_DefaultRoleAssignedWithNoJWTRoles drives the +// real HTTP middleware path with a token that carries no realm_access roles and +// asserts that the user receives a gateway:creator binding. This is the exact +// scenario RBAC_ENFORCE=true must handle: a brand-new user with no Keycloak roles. +func TestUserProvisioningMiddleware_DefaultRoleAssignedWithNoJWTRoles(t *testing.T) { + h, client := test.RegisterIntegration(t) + + // NewRandAccount creates a JWT with no realm_access claims. + account := h.NewRandAccount() + ctx := h.NewAuthenticatedContext(account) + + // GET /roles is RBAC-exempt so it succeeds regardless of bindings, + // but it still runs UserProvisioningMiddleware on the apiV1Router. + _, resp, err := client.DefaultAPI.ListRoles(ctx).Execute() + Expect(err).NotTo(HaveOccurred()) + Expect(resp.StatusCode).To(Equal(http.StatusOK)) + + // The JWT helper lowercases the username; UpsertByUsername is idempotent. + userService := users.Service(&environments.Environment().Services) + rbService := roleBindings.Service(&environments.Environment().Services) + userID, userErr := userService.UpsertByUsername(context.Background(), strings.ToLower(account.Username), nil, nil) + Expect(userErr).NotTo(HaveOccurred()) + + bindings, findErr := rbService.FindBindingsByUserID(context.Background(), userID) + Expect(findErr).NotTo(HaveOccurred()) + + found := false + for _, b := range bindings { + if b.RoleName == roles.RoleGatewayCreator && b.Scope == roleBindings.ScopeGlobal { + found = true + } + } + Expect(found).To(BeTrue(), "expected gateway:creator binding after first authenticated request with no JWT roles") +} + // TestSyncJWTRoles_DefaultRoleAssigned verifies that a newly provisioned user // receives a gateway:creator binding even when the JWT carries no roles. func TestSyncJWTRoles_DefaultRoleAssigned(t *testing.T) { diff --git a/components/api-server/plugins/roleBindings/service.go b/components/api-server/plugins/roleBindings/service.go index bcea082a9..242d6ea48 100644 --- a/components/api-server/plugins/roleBindings/service.go +++ b/components/api-server/plugins/roleBindings/service.go @@ -4,6 +4,7 @@ import ( "context" "fmt" + "github.com/golang/glog" "github.com/openshift-online/hypershell/components/api-server/pkg/rbac" "github.com/openshift-online/hypershell/components/api-server/plugins/roles" "github.com/openshift-online/rh-trex-ai/pkg/api" @@ -37,6 +38,11 @@ func NewRoleBindingService( events services.EventService, defaultRoles []string, ) RoleBindingService { + for _, r := range defaultRoles { + if !roles.JWTSyncedRoles[r] { + glog.Warningf("RBAC_DEFAULT_ROLES: role %q is not in JWTSyncedRoles and will be ignored; add it to JWTSyncedRoles to make it sync-eligible", r) + } + } return &sqlRoleBindingService{ lockFactory: lockFactory, rbDao: rbDao, diff --git a/components/api-server/plugins/roleBindings/testmain_test.go b/components/api-server/plugins/roleBindings/testmain_test.go index 3fbfdc413..d2d824a17 100644 --- a/components/api-server/plugins/roleBindings/testmain_test.go +++ b/components/api-server/plugins/roleBindings/testmain_test.go @@ -9,6 +9,10 @@ import ( "github.com/golang/glog" "github.com/openshift-online/hypershell/components/api-server/test" + + // Register the rbac plugin so that UserProvisioningMiddleware is wired onto + // apiV1Router for HTTP-path integration tests. + _ "github.com/openshift-online/hypershell/components/api-server/plugins/rbac" ) func TestMain(m *testing.M) { From 2d53d04c489b5f750af5437f8cc7d5d22211ce34 Mon Sep 17 00:00:00 2001 From: user Date: Wed, 9 Sep 2026 17:51:42 -0400 Subject: [PATCH 3/7] [HYPERSHELL-262] fix(rbac): restrict default role bootstrap to zero-role JWT users The previous change applied RBAC_DEFAULT_ROLES to every authenticated user on every request. This conflicts with deployments where Keycloak assigns non-creator realm roles (e.g. openshell-user) to users who should be denied gateway:creator: SyncJWTRoles was silently overriding that intent by appending the default regardless of JWT content. Change the default-role logic so that defaults only apply when the JWT carries NO realm roles at all. Users whose Keycloak token already contains role assignments -- even if none are JWT-synced roles -- are managed entirely through Keycloak and receive no default override. This preserves the bootstrap use-case (brand-new users with no Keycloak assignments get gateway:creator so RBAC_ENFORCE=true does not strand them) while restoring the expected E2E posture where Keycloak-role-bearing users without explicit gateway:creator remain blocked. Also fixes gofmt and renames TestSyncJWTRoles_JWTRoleAddedAlongsideDefault to TestSyncJWTRoles_JWTRolesPreventDefaultAssignment to reflect the corrected behavior. Co-Authored-By: Claude Sonnet 4.6 --- .../api-server/pkg/rbac/user_provisioning.go | 2 +- .../plugins/roleBindings/integration_test.go | 12 +++++++----- .../api-server/plugins/roleBindings/service.go | 15 +++++++++------ 3 files changed, 17 insertions(+), 12 deletions(-) diff --git a/components/api-server/pkg/rbac/user_provisioning.go b/components/api-server/pkg/rbac/user_provisioning.go index 88c19b8df..f2416b22c 100644 --- a/components/api-server/pkg/rbac/user_provisioning.go +++ b/components/api-server/pkg/rbac/user_provisioning.go @@ -28,7 +28,7 @@ type UserProvisioner interface { func UserProvisioningMiddleware(provisioner UserProvisioner, syncer JWTRoleSyncer) func(http.Handler) http.Handler { return func(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - payload, err := auth.GetAuthPayload(r) + payload, err := auth.GetAuthPayload(r) if err != nil { next.ServeHTTP(w, r) return diff --git a/components/api-server/plugins/roleBindings/integration_test.go b/components/api-server/plugins/roleBindings/integration_test.go index 2ef4435ca..487133313 100644 --- a/components/api-server/plugins/roleBindings/integration_test.go +++ b/components/api-server/plugins/roleBindings/integration_test.go @@ -452,17 +452,19 @@ func TestSyncJWTRoles_DefaultRoleNotRemovedWhenAbsentFromJWT(t *testing.T) { Expect(found).To(BeTrue(), "gateway:creator binding must survive a sync with an empty JWT") } -// TestSyncJWTRoles_JWTRoleAddedAlongsideDefault verifies that a JWT-carried role -// (platform:admin) is synced in addition to the default gateway:creator. -func TestSyncJWTRoles_JWTRoleAddedAlongsideDefault(t *testing.T) { +// TestSyncJWTRoles_JWTRolesPreventDefaultAssignment verifies that a user whose JWT +// carries realm roles (even non-creator ones) does NOT receive the default +// gateway:creator binding. Only zero-role JWTs trigger the default bootstrap. +func TestSyncJWTRoles_JWTRolesPreventDefaultAssignment(t *testing.T) { test.RegisterIntegration(t) rbService := roleBindings.Service(&environments.Environment().Services) userService := users.Service(&environments.Environment().Services) - userID, userErr := userService.UpsertByUsername(context.Background(), "sync-default-plus-jwt", nil, nil) + userID, userErr := userService.UpsertByUsername(context.Background(), "sync-jwt-no-default", nil, nil) Expect(userErr).NotTo(HaveOccurred()) + // User has platform:admin in their JWT — they have realm roles, so defaults must not apply. syncErr := rbService.SyncJWTRoles(context.Background(), userID, []string{roles.RolePlatformAdmin}) Expect(syncErr).NotTo(HaveOccurred()) @@ -475,6 +477,6 @@ func TestSyncJWTRoles_JWTRoleAddedAlongsideDefault(t *testing.T) { roleNames[b.RoleName] = true } } - Expect(roleNames[roles.RoleGatewayCreator]).To(BeTrue(), "expected default gateway:creator binding") Expect(roleNames[roles.RolePlatformAdmin]).To(BeTrue(), "expected JWT-carried platform:admin binding") + Expect(roleNames[roles.RoleGatewayCreator]).To(BeFalse(), "default gateway:creator must not be assigned when JWT carries realm roles") } diff --git a/components/api-server/plugins/roleBindings/service.go b/components/api-server/plugins/roleBindings/service.go index 242d6ea48..91cfbfba4 100644 --- a/components/api-server/plugins/roleBindings/service.go +++ b/components/api-server/plugins/roleBindings/service.go @@ -106,12 +106,15 @@ func (s *sqlRoleBindingService) SyncJWTRoles(ctx context.Context, userID string, jwtRoleSet[r] = true } } - // Default roles are always treated as present regardless of JWT content. - // They must be in JWTSyncedRoles to participate in the sync lifecycle - // (deduplication, removal on revocation). - for _, r := range s.defaultRoles { - if roles.JWTSyncedRoles[r] { - jwtRoleSet[r] = true + // Default roles bootstrap users whose JWT carries NO realm roles at all. + // Users who already have Keycloak realm-role assignments (even non-synced + // ones) are managed entirely through Keycloak; applying defaults to them + // would grant capabilities that Keycloak intentionally withheld. + if len(jwtRoles) == 0 { + for _, r := range s.defaultRoles { + if roles.JWTSyncedRoles[r] { + jwtRoleSet[r] = true + } } } From 73d87166a070cec89175112d8ed63e2bb1277f62 Mon Sep 17 00:00:00 2001 From: user Date: Wed, 9 Sep 2026 19:24:05 -0400 Subject: [PATCH 4/7] [HYPERSHELL-262] fix(rbac): always merge default roles regardless of JWT role cardinality Remove the `if len(jwtRoles) == 0` guard from SyncJWTRoles so that RBAC_DEFAULT_ROLES are unconditionally merged with whatever the JWT carries. Prior behavior bootstrapped defaults only for users with zero JWT realm roles. This was fragile: an admin who already held platform:admin in Keycloak would never receive gateway:creator via the default binding, forcing manual setup for every new deployment. The new behavior mirrors the intent in user_provisioning.go (which already calls SyncJWTRoles unconditionally): default roles represent the platform's baseline posture and must be present regardless of what other roles the JWT brings. The Keycloak role list and the default-role list may change independently; each request reconciles both. Updates the integration test to assert that a JWT-carried platform:admin binding and the default gateway:creator binding are both present after a single sync. Co-Authored-By: Claude Sonnet 4.6 --- .../plugins/roleBindings/integration_test.go | 13 ++++++------- .../api-server/plugins/roleBindings/service.go | 17 ++++++++--------- 2 files changed, 14 insertions(+), 16 deletions(-) diff --git a/components/api-server/plugins/roleBindings/integration_test.go b/components/api-server/plugins/roleBindings/integration_test.go index 487133313..b75f34d39 100644 --- a/components/api-server/plugins/roleBindings/integration_test.go +++ b/components/api-server/plugins/roleBindings/integration_test.go @@ -452,19 +452,18 @@ func TestSyncJWTRoles_DefaultRoleNotRemovedWhenAbsentFromJWT(t *testing.T) { Expect(found).To(BeTrue(), "gateway:creator binding must survive a sync with an empty JWT") } -// TestSyncJWTRoles_JWTRolesPreventDefaultAssignment verifies that a user whose JWT -// carries realm roles (even non-creator ones) does NOT receive the default -// gateway:creator binding. Only zero-role JWTs trigger the default bootstrap. -func TestSyncJWTRoles_JWTRolesPreventDefaultAssignment(t *testing.T) { +// TestSyncJWTRoles_DefaultAndJWTRolesBothApplied verifies that a JWT-carried role +// (platform:admin) is synced in addition to the default gateway:creator. +// Default roles are always applied regardless of what the JWT carries. +func TestSyncJWTRoles_DefaultAndJWTRolesBothApplied(t *testing.T) { test.RegisterIntegration(t) rbService := roleBindings.Service(&environments.Environment().Services) userService := users.Service(&environments.Environment().Services) - userID, userErr := userService.UpsertByUsername(context.Background(), "sync-jwt-no-default", nil, nil) + userID, userErr := userService.UpsertByUsername(context.Background(), "sync-default-and-jwt", nil, nil) Expect(userErr).NotTo(HaveOccurred()) - // User has platform:admin in their JWT — they have realm roles, so defaults must not apply. syncErr := rbService.SyncJWTRoles(context.Background(), userID, []string{roles.RolePlatformAdmin}) Expect(syncErr).NotTo(HaveOccurred()) @@ -478,5 +477,5 @@ func TestSyncJWTRoles_JWTRolesPreventDefaultAssignment(t *testing.T) { } } Expect(roleNames[roles.RolePlatformAdmin]).To(BeTrue(), "expected JWT-carried platform:admin binding") - Expect(roleNames[roles.RoleGatewayCreator]).To(BeFalse(), "default gateway:creator must not be assigned when JWT carries realm roles") + Expect(roleNames[roles.RoleGatewayCreator]).To(BeTrue(), "expected default gateway:creator binding alongside JWT roles") } diff --git a/components/api-server/plugins/roleBindings/service.go b/components/api-server/plugins/roleBindings/service.go index 91cfbfba4..ce200be7f 100644 --- a/components/api-server/plugins/roleBindings/service.go +++ b/components/api-server/plugins/roleBindings/service.go @@ -106,15 +106,14 @@ func (s *sqlRoleBindingService) SyncJWTRoles(ctx context.Context, userID string, jwtRoleSet[r] = true } } - // Default roles bootstrap users whose JWT carries NO realm roles at all. - // Users who already have Keycloak realm-role assignments (even non-synced - // ones) are managed entirely through Keycloak; applying defaults to them - // would grant capabilities that Keycloak intentionally withheld. - if len(jwtRoles) == 0 { - for _, r := range s.defaultRoles { - if roles.JWTSyncedRoles[r] { - jwtRoleSet[r] = true - } + // Default roles are always merged regardless of what the JWT carries. + // They represent the platform's baseline posture: every authenticated + // principal receives these capabilities unless explicitly disabled via + // RBAC_DEFAULT_ROLES=. Only roles in JWTSyncedRoles participate in the + // sync lifecycle (idempotent add, never revoked by JWT absence). + for _, r := range s.defaultRoles { + if roles.JWTSyncedRoles[r] { + jwtRoleSet[r] = true } } From e144c3c0a82d358d72b3a0d6182c92422cbc3092 Mon Sep 17 00:00:00 2001 From: user Date: Wed, 9 Sep 2026 19:24:17 -0400 Subject: [PATCH 5/7] [HYPERSHELL-262] test(e2e): verify developer and platform-admin receive default gateway:creator E2E assertions (areas 9, 10): - Developer user: gateway create returns 201 (gateway:creator from RBAC_DEFAULT_ROLES) - Platform admin: gateway create returns 201 (gateway:creator from RBAC_DEFAULT_ROLES) Kind driver fixes required to reach these areas locally: - Discover ephemeral kindccm-gw port and front it with an IPv4-only socat forwarder. Docker's IPv6 NAT for the ephemeral port is unreliable (TLS RST); the openshell CLI (Rust/hyper) prefers IPv6 and does not fall back after a TCP-level RST during the TLS handshake. Binding socat to 127.0.0.1 ensures ECONNREFUSED on ::1 so hyper retries on 127.0.0.1. - Return the socat port as the gateway endpoint in discover_gateway_endpoint so the openshell CLI connects on IPv4. - Export OPENSHELL_GATEWAY_INSECURE=true: Kind uses self-signed certs not in the system trust store; curl already uses -sk for all driver calls. - Set gateway_insecure: true in openshell metadata.json for per-gateway and developer configs, driven by the OPENSHELL_GATEWAY_INSECURE env var. Co-Authored-By: Claude Sonnet 4.6 --- tests/e2e/drivers/kind.sh | 66 ++++++++++++++++++++++++++++++++++++-- tests/e2e/e2e-openshell.sh | 66 ++++++++++++++++++++------------------ 2 files changed, 99 insertions(+), 33 deletions(-) diff --git a/tests/e2e/drivers/kind.sh b/tests/e2e/drivers/kind.sh index 5662d9c3d..2a6dc7765 100755 --- a/tests/e2e/drivers/kind.sh +++ b/tests/e2e/drivers/kind.sh @@ -11,8 +11,63 @@ # Kind uses locally issued certificates. Other drivers may override this seam # while reusing the production OIDC and role-assignment behavior below. + +# Kind's self-signed CA is not in the system trust store. Instruct the +# openshell CLI to skip TLS verification for gateway connections. curl already +# uses -sk (insecure) for all driver requests; this extends the same treatment +# to the openshell binary. +export OPENSHELL_GATEWAY_INSECURE=true + +# Force IPv4 and remap *.hypershell.localhost:443 to the cloud-provider-kind +# envoy ephemeral port. Two problems motivate this: +# 1. DNS stub returns both 127.0.0.1 and ::1 for *.localhost; the envoy proxy +# only binds IPv4, so curl must prefer IPv4 (--ipv4). +# 2. Without sudo, iptables cannot redirect port 443 to the ephemeral port +# (typically 32768). curl's --connect-to lets us rewrite the TCP target at +# the connection layer while keeping the SNI as the original hostname, so +# the envoy proxy can route by hostname as normal. +_KINDCCM_PORT="${_KINDCCM_PORT:-}" +_KINDCCM_SOCAT_PID="${_KINDCCM_SOCAT_PID:-}" +_kind_discover_port() { + if [[ -z "${_KINDCCM_PORT}" ]]; then + local proxy_container raw_port + proxy_container=$(${CONTAINER_ENGINE:-docker} ps -q --filter "name=kindccm-gw" 2>/dev/null | head -1) + if [[ -n "$proxy_container" ]]; then + raw_port=$(${CONTAINER_ENGINE:-docker} port "${proxy_container}" 443 2>/dev/null \ + | head -1 | grep -oE '[0-9]+$' || true) + fi + if [[ -n "${raw_port}" && "${raw_port}" != "443" ]]; then + # Docker's IPv6 NAT for the kindccm-gw port is unreliable on some kernels: + # IPv6 TCP connections reach the proxy but TLS immediately EOF. The openshell + # CLI (Rust/hyper) prefers IPv6 and doesn't fall back after a TLS RST, so it + # never succeeds. Work around this by fronting the proxy with a socat forwarder + # bound to 127.0.0.1 only (IPv4). DNS for *.localhost returns both ::1 and + # 127.0.0.1; with the IPv4-only listener, ::1 gets ECONNREFUSED and hyper + # falls back to 127.0.0.1 correctly. + local socat_port + socat_port=$(python3 -c "import socket; s=socket.socket(); s.bind(('127.0.0.1',0)); p=s.getsockname()[1]; s.close(); print(p)") + socat TCP4-LISTEN:"${socat_port}",bind=127.0.0.1,reuseaddr,fork \ + TCP4:127.0.0.1:"${raw_port}" &>/dev/null & + _KINDCCM_SOCAT_PID=$! + _KINDCCM_PORT="${socat_port}" + else + _KINDCCM_PORT="${raw_port}" + fi + fi +} _driver_curl() { - curl -sk "$@" + _kind_discover_port + local connect_args=() + if [[ -n "${_KINDCCM_PORT}" && "${_KINDCCM_PORT}" != "443" ]]; then + connect_args+=( + --connect-to "api.hypershell.localhost:443:127.0.0.1:${_KINDCCM_PORT}" + --connect-to "keycloak.hypershell.localhost:443:127.0.0.1:${_KINDCCM_PORT}" + --connect-to "console.hypershell.localhost:443:127.0.0.1:${_KINDCCM_PORT}" + --connect-to "health.hypershell.localhost:443:127.0.0.1:${_KINDCCM_PORT}" + --connect-to "observability.hypershell.localhost:443:127.0.0.1:${_KINDCCM_PORT}" + ) + fi + curl -sk --ipv4 "${connect_args[@]}" "$@" } # discover_api_host - find the HyperShell API server base URL. @@ -34,9 +89,11 @@ discover_api_host() { local code code=$(_driver_curl --connect-timeout 5 -o /dev/null -w '%{http_code}' \ "${url}/api/hypershell/v1/gateways" 2>/dev/null || true) + # Any HTTP response (401 unauthenticated, 200, 404, ...) proves the route # reaches the API server. "000" means the connection never completed -- route # not programmed, 443->LB mapping down, or the api-server pod not serving. + # _driver_curl handles port remapping transparently when iptables is unavailable. if [[ -z "$code" || "$code" == "000" ]]; then red " API route ${url} is not reachable (no HTTP response)" red " Verify: Gateway Programmed, api-server pod Ready, and 443->LB mapping active" @@ -94,7 +151,12 @@ discover_gateway_endpoint() { -o jsonpath='{range .status.conditions[*]}{.type}={.status}{"\n"}{end}' 2>/dev/null \ | grep -c 'Programmed=True' || true) if [[ "${gw_programmed:-0}" -ge 1 ]]; then - _DISCOVER_GW_ENDPOINT="https://${grpc_host}:443" + _kind_discover_port + if [[ -n "${_KINDCCM_PORT}" && "${_KINDCCM_PORT}" != "443" ]]; then + _DISCOVER_GW_ENDPOINT="https://${grpc_host}:${_KINDCCM_PORT}" + else + _DISCOVER_GW_ENDPOINT="https://${grpc_host}:443" + fi return fi fi diff --git a/tests/e2e/e2e-openshell.sh b/tests/e2e/e2e-openshell.sh index 8feeff738..dc598acb2 100755 --- a/tests/e2e/e2e-openshell.sh +++ b/tests/e2e/e2e-openshell.sh @@ -854,7 +854,8 @@ meta = { 'gateway_port': 0, 'auth_mode': 'oidc', 'oidc_issuer': os.environ['E2E_OIDC_ISSUER'], - 'oidc_client_id': os.environ['OIDC_CLIENT_ID_EFFECTIVE'] + 'oidc_client_id': os.environ['OIDC_CLIENT_ID_EFFECTIVE'], + 'gateway_insecure': bool(os.environ.get('OPENSHELL_GATEWAY_INSECURE', '')) } with open(os.path.join(config_dir, 'metadata.json'), 'w') as f: json.dump(meta, f, indent=2) @@ -1214,7 +1215,8 @@ meta = { 'gateway_port': 0, 'auth_mode': 'oidc', 'oidc_issuer': os.environ['E2E_OIDC_ISSUER'], - 'oidc_client_id': os.environ['DEV_OIDC_CLIENT_ID_EFFECTIVE'] + 'oidc_client_id': os.environ['DEV_OIDC_CLIENT_ID_EFFECTIVE'], + 'gateway_insecure': bool(os.environ.get('OPENSHELL_GATEWAY_INSECURE', '')) } with open(os.path.join(config_dir, 'metadata.json'), 'w') as f: json.dump(meta, f, indent=2) @@ -1351,11 +1353,11 @@ except Exception: fi fi - # ── negative assertion: openshell-user may NOT create a gateway ── - # gateway:viewer lacks the platform-scoped gateway:creator role, so - # POST /gateways MUST be rejected with 403 (rbac-enforcement.spec.md scenario - # "User without creator role cannot create gateways"). SUCCESS here would mean - # RBAC is NOT enforced. + # ── positive assertion: authenticated user receives gateway:creator by default ── + # RBAC_DEFAULT_ROLES defaults to gateway:creator, so every authenticated user + # is a creator. A developer with openshell-user Keycloak roles still gets the + # platform default binding and therefore can create gateways. This verifies + # that the default-role bootstrap fires correctly (HYPERSHELL-262). DEV_GW_CREATE_NAME="e2e-dev-gw-$(date +%s | tail -c5)" DEV_GW_BODY=$(GW_NAME="$DEV_GW_CREATE_NAME" E2E_OIDC_ISSUER="$E2E_OIDC_ISSUER" \ E2E_OIDC_CLIENT_ID="$E2E_OIDC_CLIENT_ID" python3 -c " @@ -1376,8 +1378,8 @@ body = { } print(json.dumps(body)) ") - show_cmd "curl -X POST ${API_HOST}/api/hypershell/v1/gateways (as developer) -> expect 403" - dim " Expecting 403 Forbidden (developer lacks gateway:creator)..." + show_cmd "curl -X POST ${API_HOST}/api/hypershell/v1/gateways (as developer) -> expect 201 (gateway:creator by default)" + dim " Expecting 201 Created (developer receives gateway:creator via RBAC_DEFAULT_ROLES)..." DEV_GW_RESP_FILE=$(mktemp) DEV_GW_STATUS=$(_driver_curl -o "${DEV_GW_RESP_FILE}" -w '%{http_code}' \ @@ -1387,19 +1389,18 @@ print(json.dumps(body)) -d "${DEV_GW_BODY}" 2>/dev/null || true) DEV_GW_RESP=$(sed 's/\x1b\[[0-9;]*m//g' "${DEV_GW_RESP_FILE}" 2>/dev/null | tr '\n' ' ' | tr -s ' ') - if [[ "$DEV_GW_STATUS" == "403" ]]; then - pass "Developer user: gateway create correctly denied (403 Forbidden)" - elif [[ "$DEV_GW_STATUS" =~ ^2 ]]; then - fail_test "Developer user: RBAC not enforced -- non-creator created a gateway (HTTP ${DEV_GW_STATUS})" - # A gateway was wrongly created; the creator auto-owns it, so delete it as the - # developer to avoid leaking test state. - DEV_BAD_GW_ID=$(echo "$DEV_GW_RESP" | python3 -c "import json,sys; print(json.load(sys.stdin).get('id',''))" 2>/dev/null || true) - if [[ -n "$DEV_BAD_GW_ID" ]]; then - _driver_curl -X DELETE "${API_HOST}/api/hypershell/v1/gateways/${DEV_BAD_GW_ID}" \ + if [[ "$DEV_GW_STATUS" =~ ^2 ]]; then + pass "Developer user: gateway create allowed (gateway:creator default binding active)" + DEV_DEFAULT_GW_ID=$(echo "$DEV_GW_RESP" | python3 -c "import json,sys; print(json.load(sys.stdin).get('id',''))" 2>/dev/null || true) + if [[ -n "$DEV_DEFAULT_GW_ID" ]]; then + _driver_curl -X DELETE "${API_HOST}/api/hypershell/v1/gateways/${DEV_DEFAULT_GW_ID}" \ -H "Authorization: Bearer ${DEV_TOKEN}" &>/dev/null || true fi + elif [[ "$DEV_GW_STATUS" == "403" ]]; then + fail_test "Developer user: gateway create blocked -- default gateway:creator binding was not assigned (HTTP 403)" + dim " ${DEV_GW_RESP:0:200}" else - fail_test "Developer user: gateway create did not return 403 (got HTTP ${DEV_GW_STATUS:-none})" + fail_test "Developer user: unexpected HTTP ${DEV_GW_STATUS:-none} on gateway create" dim " ${DEV_GW_RESP:0:200}" fi rm -f "${DEV_GW_RESP_FILE}" 2>/dev/null || true @@ -1506,7 +1507,10 @@ print('true' if has_owner else 'false') fi rm -f "${PADMIN_DELETE_FILE}" 2>/dev/null || true - # ── negative assertion: platform:admin cannot create gateways without gateway:creator ── + # ── positive assertion: platform:admin also receives gateway:creator by default ── + # RBAC_DEFAULT_ROLES applies to all authenticated users including platform:admin. + # They can create gateways via the default binding even without explicit + # gateway:creator in their Keycloak realm roles (HYPERSHELL-262). PADMIN_GW_CREATE_NAME="e2e-padmin-gw-$(date +%s | tail -c5)" PADMIN_GW_BODY=$(GW_NAME="$PADMIN_GW_CREATE_NAME" E2E_OIDC_ISSUER="$E2E_OIDC_ISSUER" \ E2E_OIDC_CLIENT_ID="$E2E_OIDC_CLIENT_ID" python3 -c " @@ -1527,8 +1531,8 @@ body = { } print(json.dumps(body)) ") - show_cmd "curl -X POST ${API_HOST}/api/hypershell/v1/gateways (as platform admin) -> expect 403" - dim " Expecting 403 Forbidden (platform:admin lacks gateway:creator)..." + show_cmd "curl -X POST ${API_HOST}/api/hypershell/v1/gateways (as platform admin) -> expect 201 (gateway:creator by default)" + dim " Expecting 201 Created (platform:admin receives gateway:creator via RBAC_DEFAULT_ROLES)..." PADMIN_CREATE_FILE=$(mktemp) PADMIN_CREATE_STATUS=$(_driver_curl -o "${PADMIN_CREATE_FILE}" -w '%{http_code}' \ @@ -1538,18 +1542,18 @@ print(json.dumps(body)) -d "${PADMIN_GW_BODY}" 2>/dev/null || true) PADMIN_CREATE_RESP=$(cat "${PADMIN_CREATE_FILE}" 2>/dev/null || true) - if [[ "$PADMIN_CREATE_STATUS" == "403" ]]; then - pass "Platform admin: gateway create correctly denied (403 Forbidden)" - elif [[ "$PADMIN_CREATE_STATUS" =~ ^2 ]]; then - fail_test "Platform admin: RBAC not enforced -- platform:admin created gateway without gateway:creator (HTTP ${PADMIN_CREATE_STATUS})" - # Clean up wrongly created gateway - PADMIN_BAD_GW_ID=$(echo "$PADMIN_CREATE_RESP" | python3 -c "import json,sys; print(json.load(sys.stdin).get('id',''))" 2>/dev/null || true) - if [[ -n "$PADMIN_BAD_GW_ID" ]]; then - _driver_curl -X DELETE "${API_HOST}/api/hypershell/v1/gateways/${PADMIN_BAD_GW_ID}" \ + if [[ "$PADMIN_CREATE_STATUS" =~ ^2 ]]; then + pass "Platform admin: gateway create allowed (gateway:creator default binding active)" + PADMIN_DEFAULT_GW_ID=$(echo "$PADMIN_CREATE_RESP" | python3 -c "import json,sys; print(json.load(sys.stdin).get('id',''))" 2>/dev/null || true) + if [[ -n "$PADMIN_DEFAULT_GW_ID" ]]; then + _driver_curl -X DELETE "${API_HOST}/api/hypershell/v1/gateways/${PADMIN_DEFAULT_GW_ID}" \ -H "Authorization: Bearer ${PADMIN_TOKEN}" &>/dev/null || true fi + elif [[ "$PADMIN_CREATE_STATUS" == "403" ]]; then + fail_test "Platform admin: gateway create blocked -- default gateway:creator binding was not assigned (HTTP 403)" + dim " ${PADMIN_CREATE_RESP:0:200}" else - fail_test "Platform admin: gateway create did not return 403 (got HTTP ${PADMIN_CREATE_STATUS:-none})" + fail_test "Platform admin: unexpected HTTP ${PADMIN_CREATE_STATUS:-none} on gateway create" dim " ${PADMIN_CREATE_RESP:0:200}" fi rm -f "${PADMIN_CREATE_FILE}" 2>/dev/null || true From 5d9ea17c64774055f799ff5c4cc1a0a09fdb17a6 Mon Sep 17 00:00:00 2001 From: user Date: Thu, 10 Sep 2026 06:36:59 -0400 Subject: [PATCH 6/7] [HYPERSHELL-262] fix(e2e): scope socat IPv4 forwarder to gateway endpoint only The socat IPv4-only forwarder must not replace _KINDCCM_PORT -- that port is used by _driver_curl for all curl requests (API server, Keycloak, console) which already use --ipv4 and are unaffected by IPv6. Replacing _KINDCCM_PORT broke CI runners that don't have socat: curl would connect to a dead port and get "000" responses, failing API host discovery before any test ran. Split into two variables: - _KINDCCM_PORT: raw kindccm-gw ephemeral port, set once by _kind_discover_port. Used by _driver_curl via --connect-to (curl handles IPv6 via --ipv4). - _KINDCCM_GW_PORT: set by _kind_start_gw_socat, used only for the gateway endpoint URL written into openshell CLI metadata.json. socat is started lazily when discover_gateway_endpoint is called, only if socat is available. Falls back to the raw port if socat is absent (IPv6 may fail for the CLI but all other test areas complete). Co-Authored-By: Claude Sonnet 4.6 --- tests/e2e/drivers/kind.sh | 64 +++++++++++++++++++++++++-------------- 1 file changed, 42 insertions(+), 22 deletions(-) diff --git a/tests/e2e/drivers/kind.sh b/tests/e2e/drivers/kind.sh index 2a6dc7765..c88a9d32b 100755 --- a/tests/e2e/drivers/kind.sh +++ b/tests/e2e/drivers/kind.sh @@ -27,34 +27,54 @@ export OPENSHELL_GATEWAY_INSECURE=true # the connection layer while keeping the SNI as the original hostname, so # the envoy proxy can route by hostname as normal. _KINDCCM_PORT="${_KINDCCM_PORT:-}" +# _KINDCCM_GW_PORT: IPv4-only socat port used exclusively for the openshell CLI +# gateway endpoint. curl requests use _KINDCCM_PORT directly (--ipv4 already +# prevents IPv6). The socat forwarder is started lazily by discover_gateway_endpoint. +_KINDCCM_GW_PORT="${_KINDCCM_GW_PORT:-}" _KINDCCM_SOCAT_PID="${_KINDCCM_SOCAT_PID:-}" _kind_discover_port() { if [[ -z "${_KINDCCM_PORT}" ]]; then - local proxy_container raw_port + local proxy_container proxy_container=$(${CONTAINER_ENGINE:-docker} ps -q --filter "name=kindccm-gw" 2>/dev/null | head -1) if [[ -n "$proxy_container" ]]; then - raw_port=$(${CONTAINER_ENGINE:-docker} port "${proxy_container}" 443 2>/dev/null \ + _KINDCCM_PORT=$(${CONTAINER_ENGINE:-docker} port "${proxy_container}" 443 2>/dev/null \ | head -1 | grep -oE '[0-9]+$' || true) fi - if [[ -n "${raw_port}" && "${raw_port}" != "443" ]]; then - # Docker's IPv6 NAT for the kindccm-gw port is unreliable on some kernels: - # IPv6 TCP connections reach the proxy but TLS immediately EOF. The openshell - # CLI (Rust/hyper) prefers IPv6 and doesn't fall back after a TLS RST, so it - # never succeeds. Work around this by fronting the proxy with a socat forwarder - # bound to 127.0.0.1 only (IPv4). DNS for *.localhost returns both ::1 and - # 127.0.0.1; with the IPv4-only listener, ::1 gets ECONNREFUSED and hyper - # falls back to 127.0.0.1 correctly. - local socat_port - socat_port=$(python3 -c "import socket; s=socket.socket(); s.bind(('127.0.0.1',0)); p=s.getsockname()[1]; s.close(); print(p)") - socat TCP4-LISTEN:"${socat_port}",bind=127.0.0.1,reuseaddr,fork \ - TCP4:127.0.0.1:"${raw_port}" &>/dev/null & - _KINDCCM_SOCAT_PID=$! - _KINDCCM_PORT="${socat_port}" - else - _KINDCCM_PORT="${raw_port}" - fi fi } +# _kind_gw_port - return an IPv4-only port for the openshell CLI gateway endpoint. +# The openshell CLI (Rust/hyper) prefers IPv6 for *.gw.localhost and does NOT +# fall back after a TLS RST (Docker's IPv6 NAT is unreliable on some kernels). +# We front the envoy port with a socat listener bound to 127.0.0.1 only: ::1 +# then gets ECONNREFUSED and hyper retries on 127.0.0.1. curl is unaffected +# because it already uses --ipv4. Sets _KINDCCM_GW_PORT. +_kind_start_gw_socat() { + [[ -n "${_KINDCCM_GW_PORT}" ]] && return + _kind_discover_port + local raw_port="${_KINDCCM_PORT}" + # When sudo set up iptables (port 443 redirected), socat isn't needed: + # the openshell CLI can reach port 443 directly on IPv4 and IPv6 doesn't + # matter because port 443 is forwarded by the kernel. + if [[ -z "${raw_port}" || "${raw_port}" == "443" ]]; then + _KINDCCM_GW_PORT="${raw_port:-443}" + return + fi + if ! command -v socat &>/dev/null; then + # socat unavailable; fall back to the raw port and accept that IPv6 may fail. + _KINDCCM_GW_PORT="${raw_port}" + return + fi + local socat_port + socat_port=$(python3 -c "import socket; s=socket.socket(); s.bind(('127.0.0.1',0)); p=s.getsockname()[1]; s.close(); print(p)" 2>/dev/null) + if [[ -z "${socat_port}" ]]; then + _KINDCCM_GW_PORT="${raw_port}" + return + fi + socat TCP4-LISTEN:"${socat_port}",bind=127.0.0.1,reuseaddr,fork \ + TCP4:127.0.0.1:"${raw_port}" &>/dev/null & + _KINDCCM_SOCAT_PID=$! + _KINDCCM_GW_PORT="${socat_port}" +} _driver_curl() { _kind_discover_port local connect_args=() @@ -151,9 +171,9 @@ discover_gateway_endpoint() { -o jsonpath='{range .status.conditions[*]}{.type}={.status}{"\n"}{end}' 2>/dev/null \ | grep -c 'Programmed=True' || true) if [[ "${gw_programmed:-0}" -ge 1 ]]; then - _kind_discover_port - if [[ -n "${_KINDCCM_PORT}" && "${_KINDCCM_PORT}" != "443" ]]; then - _DISCOVER_GW_ENDPOINT="https://${grpc_host}:${_KINDCCM_PORT}" + _kind_start_gw_socat + if [[ -n "${_KINDCCM_GW_PORT}" && "${_KINDCCM_GW_PORT}" != "443" ]]; then + _DISCOVER_GW_ENDPOINT="https://${grpc_host}:${_KINDCCM_GW_PORT}" else _DISCOVER_GW_ENDPOINT="https://${grpc_host}:443" fi From 2e4bc504d882c1f56aa9077f83330f2cb16e6610 Mon Sep 17 00:00:00 2001 From: user Date: Thu, 10 Sep 2026 07:21:19 -0400 Subject: [PATCH 7/7] [HYPERSHELL-262] fix(rbac): fix RBAC_DEFAULT_ROLES opt-out; document default-role model - Fix defaultRolesFromEnv to use os.LookupEnv so that RBAC_DEFAULT_ROLES= (explicit empty string) actually disables the default gateway:creator grant. Previously os.Getenv could not distinguish unset from empty, so operators had no working way to opt out of the default. - Update specs/security/rbac-enforcement.spec.md to document the default-role model: RBAC_DEFAULT_ROLES variable semantics, the always-merge behavior, the Keycloak-only opt-out via RBAC_DEFAULT_ROLES=, and the revocation caveat. Scenarios that previously described the zero-default posture are annotated with the RBAC_DEFAULT_ROLES= precondition so they remain accurate under the new default. Adds a new "Default Role Bootstrap" requirement section with scenarios. Co-Authored-By: Claude Sonnet 4.6 --- .../api-server/plugins/roleBindings/plugin.go | 8 +- specs/security/rbac-enforcement.spec.md | 133 ++++++++++++++---- 2 files changed, 108 insertions(+), 33 deletions(-) diff --git a/components/api-server/plugins/roleBindings/plugin.go b/components/api-server/plugins/roleBindings/plugin.go index 85a285bf4..0378e954e 100644 --- a/components/api-server/plugins/roleBindings/plugin.go +++ b/components/api-server/plugins/roleBindings/plugin.go @@ -40,11 +40,15 @@ func NewServiceLocator(env *environments.Env) ServiceLocator { // defaultRolesFromEnv reads RBAC_DEFAULT_ROLES (comma-separated role names). // Defaults to gateway:creator so all authenticated users can create gateways. +// Set RBAC_DEFAULT_ROLES= (explicit empty) to disable defaults entirely. func defaultRolesFromEnv() []string { - val := os.Getenv("RBAC_DEFAULT_ROLES") - if val == "" { + val, set := os.LookupEnv("RBAC_DEFAULT_ROLES") + if !set { return []string{roles.RoleGatewayCreator} } + if val == "" { + return nil + } var result []string for _, r := range strings.Split(val, ",") { if trimmed := strings.TrimSpace(r); trimmed != "" { diff --git a/specs/security/rbac-enforcement.spec.md b/specs/security/rbac-enforcement.spec.md index 7fd737f91..2b11aebf6 100644 --- a/specs/security/rbac-enforcement.spec.md +++ b/specs/security/rbac-enforcement.spec.md @@ -13,13 +13,17 @@ The HyperShell API server SHALL enforce authorization on all API endpoints (HTTP gRPC) using a four-role model backed by Keycloak as the source of truth for platform-wide roles and a PostgreSQL-backed RoleBinding model for per-gateway grants. -Keycloak is the authority for identity and platform-wide role assignment. A privileged -administrator assigns Keycloak roles (e.g., `gateway:creator`) to users and service -accounts. The API server middleware reads JWT claims, lazily provisions User and -RoleBinding records, and evaluates authorization against the database projection. +Keycloak is the authority for identity and platform-wide role assignment. The API server +middleware reads JWT claims, lazily provisions User and RoleBinding records, and evaluates +authorization against the database projection. -Users start with zero permissions and gain access by receiving a Keycloak role -(`gateway:creator`) or being granted a per-gateway binding (`gateway:owner`, +By default, every authenticated user receives the `gateway:creator` role via the +platform's configured default roles (`RBAC_DEFAULT_ROLES=gateway:creator`). This ensures +users are not stranded when `RBAC_ENFORCE=true` is first enabled. Operators who want +Keycloak to be the sole authority for gateway creation can set `RBAC_DEFAULT_ROLES=` +(explicit empty string) to disable the default grant. + +Users can also gain access by being granted a per-gateway binding (`gateway:owner`, `gateway:viewer`) by an existing gateway owner. --- @@ -148,15 +152,21 @@ requiring a separate sync process. - THEN the middleware creates a User record and a `gateway:creator` RoleBinding - AND user A can create gateways -#### Scenario: Keycloak admin revokes gateway:creator +#### Scenario: Keycloak admin revokes gateway:creator (Keycloak-only mode) -- GIVEN user A previously had `gateway:creator` assigned in Keycloak +- GIVEN `RBAC_DEFAULT_ROLES=` is set (empty) so no defaults are applied +- AND user A previously had `gateway:creator` assigned in Keycloak - WHEN the Keycloak admin removes the role - THEN user A's next API request carries a JWT without `gateway:creator` - AND the middleware removes the corresponding RoleBinding - AND user A can no longer create new gateways - AND existing `gateway:owner` bindings on previously-created gateways are unaffected +Note: when `RBAC_DEFAULT_ROLES=gateway:creator` (the default), `gateway:creator` is +re-applied on every request regardless of JWT content. Keycloak revocation of +`gateway:creator` has no effect in this configuration. Set `RBAC_DEFAULT_ROLES=` to +restore Keycloak revocation semantics. + ### Requirement: Service Account Support Service accounts (e.g., the control plane) are Keycloak clients using the @@ -213,12 +223,16 @@ This binding is created in the same database transaction as the gateway. - AND a `gateway:owner` RoleBinding is created for user A on the new gateway - AND user A can immediately manage the gateway -#### Scenario: User without creator role cannot create gateways +#### Scenario: User without creator role cannot create gateways (Keycloak-only mode) -- GIVEN user A has only `gateway:viewer` on some gateway +- GIVEN `RBAC_DEFAULT_ROLES=` is set (empty) +- AND user A has only `gateway:viewer` on some gateway - WHEN user A calls `POST /api/hypershell/v1/gateways` - THEN the request returns 403 Forbidden +Note: in the default configuration (`RBAC_DEFAULT_ROLES=gateway:creator`), all +authenticated users receive `gateway:creator` and this scenario does not apply. + ### Requirement: Per-Gateway Authorization The authorization middleware SHALL evaluate permissions against the binding's gateway @@ -338,13 +352,17 @@ Future iterations may expand platform:admin permissions to include these resourc - WHEN user A calls `PATCH /api/hypershell/v1/gateways/gw-1` - THEN the response is 403 Forbidden -#### Scenario: Platform admin cannot create gateways without creator role +#### Scenario: Platform admin cannot create gateways without creator role (Keycloak-only mode) -- GIVEN user A has `platform:admin` only -- AND user A does NOT have `gateway:creator` +- GIVEN `RBAC_DEFAULT_ROLES=` is set (empty) +- AND user A has `platform:admin` only (no `gateway:creator`) - WHEN user A calls `POST /api/hypershell/v1/gateways` - THEN the response is 403 Forbidden +Note: in the default configuration (`RBAC_DEFAULT_ROLES=gateway:creator`), all +authenticated users including platform admins receive `gateway:creator` and can +create gateways regardless of their Keycloak role assignments. + #### Scenario: Platform admin cannot grant role bindings - GIVEN user A has `platform:admin` only @@ -428,16 +446,65 @@ Platform administrator actions SHALL be logged with: High-privilege operations (gateway deletion by platform:admin) SHALL be logged at INFO level or higher to ensure visibility in operational monitoring and security audits. +### Requirement: Default Role Bootstrap + +The API server SHALL support a configurable set of default roles applied to every +authenticated user on every request, independent of JWT claim content. This prevents +users from being stranded when `RBAC_ENFORCE=true` is first enabled. + +The default role set is controlled by the `RBAC_DEFAULT_ROLES` environment variable +(comma-separated role names). The default value is `gateway:creator`. + +- If `RBAC_DEFAULT_ROLES` is unset, `gateway:creator` is applied to all users. +- If `RBAC_DEFAULT_ROLES=` is set to an explicit empty string, no defaults are applied. +- Default roles are always merged alongside JWT-carried roles; both sources participate + in the effective role set on every request. +- Only roles present in the `JWTSyncedRoles` set are eligible as default roles. A + startup warning is emitted for any configured default role not in `JWTSyncedRoles`. + +Default role bindings are created with the same idempotent, non-revoking semantics as +JWT-synced bindings: re-applying the same role is a no-op, and the binding persists +even when removed manually (it is re-created on the next authenticated request). + +Note: because defaults are re-applied on every request, Keycloak cannot revoke a role +that is also configured as a default. Operators who need Keycloak-controlled revocation +for `gateway:creator` must set `RBAC_DEFAULT_ROLES=` to opt out of the default grant. + +#### Scenario: New user receives default gateway:creator on first request + +- GIVEN `RBAC_DEFAULT_ROLES=gateway:creator` (default) +- AND a user authenticates for the first time with a JWT carrying no Keycloak realm roles +- WHEN any authenticated API request is processed +- THEN the middleware creates a User record and a `gateway:creator` RoleBinding +- AND the user can create gateways immediately + +#### Scenario: Default role applied alongside JWT-assigned roles + +- GIVEN `RBAC_DEFAULT_ROLES=gateway:creator` (default) +- AND user A has `platform:admin` in their Keycloak JWT +- WHEN user A makes an authenticated request +- THEN the middleware assigns both `platform:admin` (from JWT) and `gateway:creator` (default) +- AND user A has both platform admin access and gateway creation capability + +#### Scenario: Default roles disabled via configuration + +- GIVEN `RBAC_DEFAULT_ROLES=` (explicit empty) +- AND user A has no Keycloak realm roles +- WHEN user A makes an authenticated request +- THEN no default `gateway:creator` binding is created +- AND user A cannot create gateways until a Keycloak admin assigns the role + ### Requirement: Production Rollout RBAC enforcement SHALL be gated behind the `RBAC_ENFORCE` configuration flag. When disabled, all authenticated requests pass. When enabled, all requests are evaluated against bindings. -The first `gateway:creator` and `platform:admin` users are provisioned by assigning the -roles in Keycloak. No database migration or CLI command is needed for bootstrapping users --- only the built-in Role records are seeded via migration; RoleBindings are created -dynamically from JWT claims. +In the default configuration (`RBAC_DEFAULT_ROLES=gateway:creator`), all authenticated +users can create gateways immediately. The first `platform:admin` users are provisioned +by assigning the role in Keycloak. No database migration or CLI command is needed for +bootstrapping users -- only the built-in Role records are seeded via migration; +RoleBindings are created dynamically on every authenticated request. ### Requirement: Database Migration @@ -454,31 +521,35 @@ This migration SHALL run alongside the existing migrations that seed `gateway:cr RoleBindings from JWT claims are synced regardless of whether enforcement is enabled, ensuring bindings exist before enforcement is turned on. -#### Operator Note: Enabling Enforcement Is a Breaking Change +#### Operator Note: Enabling Enforcement and Default-Role Posture The OpenShift overlay (`deploy/openshift/kustomization.yaml`) ships with `RBAC_ENFORCE=true`. Applying it to an existing cluster is a breaking change: from that point every gateway operation requires the caller's token to carry `gateway:creator` (create) or a matching per-gateway RoleBinding (read/write). -On a real cluster the IdP is external SSO (not the bundled Keycloak), so the -following MUST be in place BEFORE -- or atomically with -- the rollout: +In the default configuration (`RBAC_DEFAULT_ROLES=gateway:creator`), all authenticated +users automatically receive `gateway:creator` on every request. This means enabling +`RBAC_ENFORCE=true` without changing `RBAC_DEFAULT_ROLES` does NOT strand users - they +can create gateways immediately without any Keycloak configuration. + +To restrict gateway creation to explicitly-authorized users (Keycloak-only mode): -- The external SSO defines both `gateway:creator` and `platform:admin` realm roles. -- Platform operators who need to create gateways are assigned `gateway:creator`. -- Platform operators who need global view/delete access are assigned `platform:admin`. -- The SSO emits these roles in the `realm_access.roles` claim of issued access tokens - (or the equivalent configurable claim path the API server reads). +1. Set `RBAC_DEFAULT_ROLES=` (explicit empty) in the deployment. +2. Define `gateway:creator` and `platform:admin` realm roles in the external SSO. +3. Assign `gateway:creator` to operators who need to create gateways. +4. Assign `platform:admin` to operators who need global view/delete access. +5. Ensure the SSO emits these roles in the `realm_access.roles` claim. -If enforcement is turned on before the roles are wired, gateway operations fail -immediately after upgrade with no other symptom -- a silent, hard-to-diagnose outage: +Without step 1, steps 2-5 have no effect on gateway creation access (all users already +have it via the default). Without steps 2-5, Keycloak-only mode strands all users: - Gateway create calls return 403 without `gateway:creator` - Gateway reads return 404 without appropriate ownership or `platform:admin` - Gateway deletes return 403 without ownership or `platform:admin` -Coordinate the SSO role mapping and the overlay rollout together, and call out these -SSO prerequisites in the release notes for the version that makes the overlay default -enforce RBAC. +Coordinate the SSO role mapping and the `RBAC_DEFAULT_ROLES` setting together, and +call out these prerequisites in the release notes for the version that makes the overlay +default enforce RBAC. ### Requirement: Integration Test Coverage @@ -511,7 +582,7 @@ Integration tests SHALL exercise RBAC enforcement with the new four-role model. | Service accounts treated identically to users | Control plane gets `gateway:creator` in Keycloak, provisions like any user. No special bypass logic needed. | | Gateway owners can grant co-owners | No hierarchy restriction. Team leads assign `gateway:creator` to team members or invite them as owners/viewers per gateway. Simple mental model. | | Auto-assign `gateway:owner` on creation | Creator automatically owns what they create. No separate grant step needed. | -| `gateway:creator` from Keycloak only | Cannot be self-assigned via the API. A Keycloak admin decides who can create gateways. | +| `gateway:creator` via default roles or Keycloak | By default (`RBAC_DEFAULT_ROLES=gateway:creator`), all authenticated users receive `gateway:creator` on every request. Set `RBAC_DEFAULT_ROLES=` to restrict assignment to Keycloak administrators only. The default cannot be self-assigned via the API; it is applied by the server on the provisioning path. | | Per-gateway bindings stored in DB | Gateway-scoped access requires per-resource granularity that JWT claims cannot provide (you'd need dynamic claim values per gateway ID). | | No resource grouping as a security boundary | The Sector/Fleet grouping was removed. RBAC operates at platform level (creator) and gateway level (owner/viewer); there is no fleet-scoped isolation. | | 404 on unauthorized singleton GETs | Returning 403 confirms the resource exists. 404 prevents ID enumeration. |