Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
40 changes: 22 additions & 18 deletions fulfillment-service/internal/servers/private_secrets_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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
}

Expand All @@ -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
}
Expand All @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand Down Expand Up @@ -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{
Expand Down
Loading