diff --git a/fulfillment-service/internal/controllers/tenant/tenant_reconciler_function.go b/fulfillment-service/internal/controllers/tenant/tenant_reconciler_function.go index 0a7b3c1933..0f34a52fb9 100644 --- a/fulfillment-service/internal/controllers/tenant/tenant_reconciler_function.go +++ b/fulfillment-service/internal/controllers/tenant/tenant_reconciler_function.go @@ -681,11 +681,6 @@ func (t *task) ensureVaultNamespace(ctx context.Context) error { if t.r.vaultLifecycle == nil { return nil } - // The shared tenant stores platform-managed Secrets used by shared templates, so it needs a - // Vault namespace. The system tenant remains internal-only and must not get one. - if t.tenant.GetMetadata().GetName() == auth.SystemTenant { - return nil - } if t.isConditionTrue(condType) { return nil } diff --git a/fulfillment-service/internal/controllers/tenant/tenant_reconciler_function_test.go b/fulfillment-service/internal/controllers/tenant/tenant_reconciler_function_test.go index 87a1011534..f844a29b08 100644 --- a/fulfillment-service/internal/controllers/tenant/tenant_reconciler_function_test.go +++ b/fulfillment-service/internal/controllers/tenant/tenant_reconciler_function_test.go @@ -2455,37 +2455,6 @@ var _ = Describe("Vault namespace provisioning", func() { Expect(cond.GetStatus()).To(Equal(privatev1.ConditionStatus_CONDITION_STATUS_TRUE)) }) - It("skips vault provisioning for the system tenant", func() { - reconciler := &function{ - logger: logger, - idpManager: idpManager, - vaultLifecycle: mockVaultClient, - } - - tenant := privatev1.Tenant_builder{ - Id: "org-system", - Metadata: privatev1.Metadata_builder{ - Name: auth.SystemTenant, - Finalizers: []string{finalizers.Controller}, - Tenant: auth.SystemTenant, - }.Build(), - Status: privatev1.TenantStatus_builder{ - State: privatev1.TenantState_TENANT_STATE_SYNCED, - IdpTenantName: auth.SystemTenant, - }.Build(), - }.Build() - - mockIDPClient.EXPECT(). - GetTenant(gomock.Any(), auth.SystemTenant). - Return(&idp.Tenant{Name: auth.SystemTenant}, nil) - - t := &task{r: reconciler, tenant: tenant} - err := t.update(ctx) - Expect(err).ToNot(HaveOccurred()) - cond := findCondition(tenant) - Expect(cond).ToNot(BeNil()) - Expect(cond.GetStatus()).To(Equal(privatev1.ConditionStatus_CONDITION_STATUS_FALSE)) - }) }) var _ = Describe("Vault namespace cleanup during deletion", func() { diff --git a/fulfillment-service/internal/servers/private_secrets_server.go b/fulfillment-service/internal/servers/private_secrets_server.go index 4e477260c2..abef64332f 100644 --- a/fulfillment-service/internal/servers/private_secrets_server.go +++ b/fulfillment-service/internal/servers/private_secrets_server.go @@ -128,7 +128,7 @@ func (b *PrivateSecretsServerBuilder) Build() (result *PrivateSecretsServer, err SetTenancyLogic(b.tenancyLogic). SetMetricsRegisterer(b.metricsRegisterer). SetFilterDesc(b.filterDesc). - AddAllowedTenants(auth.SharedTenant). + AddAllowedTenants(auth.SharedTenant, auth.SystemTenant). Build() if err != nil { return @@ -160,7 +160,7 @@ func (s *PrivateSecretsServer) Get(ctx context.Context, } obj := response.GetObject() - if err = s.authorizeSharedSecretManagement(ctx, obj); err != nil { + if err = s.authorizePlatformSecretManagement(ctx, obj); err != nil { return } if s.secretStore != nil && obj.GetBackend() == privatev1.SecretBackend_SECRET_BACKEND_VAULT { @@ -223,16 +223,16 @@ func (s *PrivateSecretsServer) Create(ctx context.Context, return createErr } created := response.GetObject() - if authErr := s.authorizeSharedSecretManagement(opCtx, created); authErr != nil { + if authErr := s.authorizePlatformSecretManagement(opCtx, created); authErr != nil { return authErr } - if created.GetMetadata().GetTenant() == auth.SharedTenant { + if tenant := created.GetMetadata().GetTenant(); tenant == auth.SharedTenant || tenant == auth.SystemTenant { if created.GetBackend() != privatev1.SecretBackend_SECRET_BACKEND_VAULT { - return grpcstatus.Errorf(grpccodes.InvalidArgument, "shared Secrets must use the Vault backend") + return grpcstatus.Errorf(grpccodes.InvalidArgument, "%s Secrets must use the Vault backend", tenant) } if s.secretStore == nil { return grpcstatus.Errorf(grpccodes.FailedPrecondition, - "shared Secrets require a configured Vault backend") + "%s Secrets require a configured Vault backend", tenant) } } if !persistInVault || isDryRun(opCtx) { @@ -272,7 +272,7 @@ func (s *PrivateSecretsServer) Update(ctx context.Context, } existingSecret := getResponse.GetObject() - if err = s.authorizeSharedSecretManagement(ctx, existingSecret); err != nil { + if err = s.authorizePlatformSecretManagement(ctx, existingSecret); err != nil { return } @@ -330,7 +330,7 @@ func (s *PrivateSecretsServer) Delete(ctx context.Context, return } obj := getResponse.GetObject() - if err = s.authorizeSharedSecretManagement(ctx, obj); err != nil { + if err = s.authorizePlatformSecretManagement(ctx, obj); err != nil { return } @@ -354,30 +354,34 @@ func (s *PrivateSecretsServer) Delete(ctx context.Context, return } -// authorizeSharedSecretManagement restricts decrypted reads and mutations of shared Secrets to -// platform administrators and controllers. Both identities have universal tenant scope. Metadata -// remains listable so shared template references can be resolved without exposing credential data. -func (s *PrivateSecretsServer) authorizeSharedSecretManagement(ctx context.Context, secret *privatev1.Secret) error { - if secret == nil || secret.GetMetadata().GetTenant() != auth.SharedTenant { +// authorizePlatformSecretManagement restricts reads and mutations of shared and system Secrets to +// platform administrators and controllers. Shared metadata remains listable for template references; +// system metadata is hidden by tenant visibility filtering. +func (s *PrivateSecretsServer) authorizePlatformSecretManagement(ctx context.Context, secret *privatev1.Secret) error { + if secret == nil { + return nil + } + tenant := secret.GetMetadata().GetTenant() + if tenant != auth.SharedTenant && tenant != auth.SystemTenant { return nil } - allowed, err := s.canManageSharedSecrets(ctx) + allowed, err := s.canManagePlatformSecrets(ctx) if err != nil { return err } if !allowed { return grpcstatus.Errorf( grpccodes.PermissionDenied, - "shared Secrets can only be read or managed by platform administrators and controllers", + "%s Secrets can only be read or managed by platform administrators and controllers", tenant, ) } return nil } -func (s *PrivateSecretsServer) canManageSharedSecrets(ctx context.Context) (bool, error) { +func (s *PrivateSecretsServer) canManagePlatformSecrets(ctx context.Context) (bool, error) { assignable, err := s.tenancyLogic.DetermineAssignableTenants(ctx) if err != nil { - return false, grpcstatus.Errorf(grpccodes.Internal, "failed to determine shared Secret access") + return false, grpcstatus.Errorf(grpccodes.Internal, "failed to determine platform Secret access") } return assignable.Universal(), nil } @@ -398,7 +402,7 @@ func (s *PrivateSecretsServer) Signal(ctx context.Context, if err = s.generic.Get(ctx, getRequest, &getResponse); err != nil { return } - if err = s.authorizeSharedSecretManagement(ctx, getResponse.GetObject()); err != nil { + if err = s.authorizePlatformSecretManagement(ctx, getResponse.GetObject()); err != nil { return } err = s.generic.Signal(ctx, request, &response) diff --git a/fulfillment-service/internal/servers/private_secrets_server_test.go b/fulfillment-service/internal/servers/private_secrets_server_test.go index 0a51918658..f24ee54c50 100644 --- a/fulfillment-service/internal/servers/private_secrets_server_test.go +++ b/fulfillment-service/internal/servers/private_secrets_server_test.go @@ -733,7 +733,7 @@ var _ = Describe("Private secrets server", func() { }) }) - Describe("Shared Secret authorization", func() { + Describe("Platform Secret authorization", func() { newTenantUserServer := func() *PrivateSecretsServer { visibility, err := auth.NewVisibility(). AddVisibleTenants(auth.SharedTenant, testTenant). @@ -791,6 +791,69 @@ var _ = Describe("Private secrets server", func() { Expect(response.GetObject().GetData()).To(HaveKey("key")) }) + It("allows a platform administrator to create and retrieve a system Vault Secret", func() { + mockStore.EXPECT(). + Store(gomock.Any(), auth.SystemTenant, "", "system-admin-secret", gomock.Any()). + Return(nil) + created, err := server.Create(ctx, privatev1.SecretsCreateRequest_builder{ + Object: privatev1.Secret_builder{ + Metadata: privatev1.Metadata_builder{ + Name: "system-admin-secret", + Tenant: auth.SystemTenant, + }.Build(), + Data: map[string][]byte{"key": []byte("value")}, + }.Build(), + }.Build()) + Expect(err).ToNot(HaveOccurred()) + Expect(created.GetObject().GetMetadata().GetTenant()).To(Equal(auth.SystemTenant)) + + mockStore.EXPECT(). + Fetch(gomock.Any(), auth.SystemTenant, "", "system-admin-secret"). + Return(map[string][]byte{"key": []byte("value")}, nil) + response, err := server.Get(ctx, privatev1.SecretsGetRequest_builder{ + Id: created.GetObject().GetId(), + }.Build()) + Expect(err).ToNot(HaveOccurred()) + Expect(response.GetObject().GetData()).To(HaveKeyWithValue("key", []byte("value"))) + }) + + It("hides system Secrets from tenant-scoped identities", func() { + mockStore.EXPECT(). + Store(gomock.Any(), auth.SystemTenant, "", "system-protected-secret", gomock.Any()). + Return(nil) + created, err := server.Create(ctx, privatev1.SecretsCreateRequest_builder{ + Object: privatev1.Secret_builder{ + Metadata: privatev1.Metadata_builder{ + Name: "system-protected-secret", + Tenant: auth.SystemTenant, + }.Build(), + Data: map[string][]byte{"key": []byte("value")}, + }.Build(), + }.Build()) + Expect(err).ToNot(HaveOccurred()) + + restrictedServer := newTenantUserServer() + _, err = restrictedServer.Create(ctx, privatev1.SecretsCreateRequest_builder{ + Object: privatev1.Secret_builder{ + Metadata: privatev1.Metadata_builder{ + Name: "rejected-system-secret", + Tenant: auth.SystemTenant, + }.Build(), + Data: map[string][]byte{"key": []byte("value")}, + }.Build(), + }.Build()) + Expect(status.Code(err)).To(Equal(codes.PermissionDenied)) + + list, err := restrictedServer.List(ctx, privatev1.SecretsListRequest_builder{}.Build()) + Expect(err).ToNot(HaveOccurred()) + Expect(list.GetItems()).To(BeEmpty()) + + _, err = restrictedServer.Get(ctx, privatev1.SecretsGetRequest_builder{ + Id: created.GetObject().GetId(), + }.Build()) + Expect(status.Code(err)).To(Equal(codes.NotFound)) + }) + It("rejects a tenant user's shared Secret create before writing to Vault", func() { restrictedServer := newTenantUserServer() _, err := restrictedServer.Create(ctx, privatev1.SecretsCreateRequest_builder{