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..f2416b22c 100644 --- a/components/api-server/pkg/rbac/user_provisioning.go +++ b/components/api-server/pkg/rbac/user_provisioning.go @@ -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 35015c0b7..b75f34d39 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, @@ -316,3 +335,147 @@ func TestGrantValidation_CrossGatewayEscalation(t *testing.T) { Expect(crossGWErr).To(HaveOccurred()) 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) { + 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_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-default-and-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.RolePlatformAdmin]).To(BeTrue(), "expected JWT-carried platform:admin binding") + Expect(roleNames[roles.RoleGatewayCreator]).To(BeTrue(), "expected default gateway:creator binding alongside JWT roles") +} diff --git a/components/api-server/plugins/roleBindings/plugin.go b/components/api-server/plugins/roleBindings/plugin.go index 487a2a9c7..0378e954e 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,38 @@ 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. +// Set RBAC_DEFAULT_ROLES= (explicit empty) to disable defaults entirely. +func defaultRolesFromEnv() []string { + 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 != "" { + 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..ce200be7f 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" @@ -35,22 +36,30 @@ func NewRoleBindingService( rbDao RoleBindingDao, roleDao roles.RoleDao, 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, - 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 +106,16 @@ func (s *sqlRoleBindingService) SyncJWTRoles(ctx context.Context, userID string, 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 + } + } existing, err := s.rbDao.FindByUserID(ctx, userID) if err != nil { 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) { 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. | diff --git a/tests/e2e/drivers/kind.sh b/tests/e2e/drivers/kind.sh index 5662d9c3d..c88a9d32b 100755 --- a/tests/e2e/drivers/kind.sh +++ b/tests/e2e/drivers/kind.sh @@ -11,8 +11,83 @@ # 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_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 + proxy_container=$(${CONTAINER_ENGINE:-docker} ps -q --filter "name=kindccm-gw" 2>/dev/null | head -1) + if [[ -n "$proxy_container" ]]; then + _KINDCCM_PORT=$(${CONTAINER_ENGINE:-docker} port "${proxy_container}" 443 2>/dev/null \ + | head -1 | grep -oE '[0-9]+$' || true) + 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() { - 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 +109,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 +171,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_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 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