diff --git a/app/actions/app_assign_service_account.rb b/app/actions/app_assign_service_account.rb new file mode 100644 index 00000000000..7f5d1e2b913 --- /dev/null +++ b/app/actions/app_assign_service_account.rb @@ -0,0 +1,78 @@ +require 'repositories/service_account_event_repository' + +module VCAP::CloudController + class AppAssignServiceAccount + class Error < StandardError; end + class Unauthorized < Error; end + class Conflict < Error; end + + def initialize(permissions, actor: nil) + @permissions = permissions + @actor = actor + end + + def assign(app, account, provision: false) + job = nil + app.db.transaction do + app.lock! + raise Unauthorized.new('not authorized to update the app') unless @permissions.can_write_to_active_space?(app.space.id) + + validate_binding_change!(app, account) + + if account + account.lock! + raise Conflict.new('organization service account prefix is unset') unless account.organization.service_account_prefix + raise Conflict.new('account must be entitled to the app space in the same organization') unless currently_entitled?(account, app.space) + + validate_ready!(account, provision) + raise Conflict.new('another service account is already assigned; unbind first') if app.service_account_guid && app.service_account_guid != account.guid + + job = provision_account(account) unless account.status == 'ready' + end + + unless app.service_account_guid == account&.guid + previous_guid = app.service_account_guid + app.update(service_account: account) + record_assignment(app, account, previous_guid) if @actor + end + end + provision ? job : app + end + + private + + def currently_entitled?(account, space) + account.organization_id == space.organization_id && account.db[:service_account_space_entitlements]. + where(service_account_guid: account.guid, space_guid: space.guid).for_update.first + end + + def validate_binding_change!(app, account) + return unless app.service_account_guid && app.service_account_guid != account&.guid && app.service_bindings_dataset.exclude(service_account_guid: nil).any? + + raise Conflict.new('remove service bindings before changing the service account') + end + + def record_assignment(app, account, previous_guid) + Repositories::ServiceAccountEventRepository.record( + app, account ? 'assign' : 'unassign', @actor, + service_account_guid: account&.guid || previous_guid + ) + end + + def validate_ready!(account, provision) + raise Conflict.new('service account is not ready or enabled') unless account.enabled && (account.status == 'ready' || provision) + + if PollableJobModel.where(resource_guid: account.guid, resource_type: 'service_account', state: %w[PROCESSING POLLING]).exclude(operation: 'service_account.provision').any? + raise Conflict.new('service account lifecycle operation is in progress') + end + end + + def provision_account(account) + job = PollableJobModel.where(resource_guid: account.guid, operation: 'service_account.provision', state: %w[PROCESSING POLLING]).for_update.first + return job if job + + account.update(status: 'reconciling') + Jobs::Enqueuer.new(queue: Jobs::Queues.generic).enqueue_pollable(Jobs::V3::ServiceAccountProvision.new(account.guid)) + end + end +end diff --git a/app/actions/organization_create.rb b/app/actions/organization_create.rb index 0c0736610ef..d3db1e9518f 100644 --- a/app/actions/organization_create.rb +++ b/app/actions/organization_create.rb @@ -14,6 +14,7 @@ def create(message) Organization.db.transaction do org = VCAP::CloudController::Organization.create( name: message.name, + service_account_prefix: message.service_account_prefix, status: message.suspended ? Organization::SUSPENDED : Organization::ACTIVE ) @@ -29,6 +30,7 @@ def create(message) private def validation_error!(error, message) + error!("Service account prefix '#{message.service_account_prefix}' is already reserved.") if error.errors.on(:service_account_prefix)&.include?(:unique) error!("Organization '#{message.name}' already exists.") if error.errors.on(:name)&.include?(:unique) error!(error.message) end diff --git a/app/actions/organization_delete.rb b/app/actions/organization_delete.rb index 76327654ed1..f9a351040ff 100644 --- a/app/actions/organization_delete.rb +++ b/app/actions/organization_delete.rb @@ -1,9 +1,12 @@ require 'actions/space_delete' require 'actions/label_delete' require 'actions/annotation_delete' +require 'actions/organization_service_account_cleanup' module VCAP::CloudController class OrganizationDelete + include OrganizationServiceAccountCleanup + def initialize(space_deleter, user_audit_info) @space_deleter = space_deleter @user_audit_info = user_audit_info @@ -11,12 +14,19 @@ def initialize(space_deleter, user_audit_info) def delete(org_dataset) org_dataset.each do |org| + shared_errors = shared_domain_errors(org) + return shared_errors unless shared_errors.empty? + + mark_deleting_for_account_cleanup(org) errs = @space_deleter.delete(org.spaces_dataset) unless errs.empty? error_message = errs.map(&:message).join("\n\n") return [CloudController::Errors::ApiError.new_from_details('OrganizationDeletionFailed', org.name, error_message)] end + account_errors = delete_service_accounts(org) + return [CloudController::Errors::ApiError.new_from_details('OrganizationDeletionFailed', org.name, account_errors.map(&:message).join("\n\n"))] unless account_errors.empty? + domains_to_unshare = [] org.private_domains.each do |private_domain| if private_domain.owning_organization == org diff --git a/app/actions/organization_quotas_create.rb b/app/actions/organization_quotas_create.rb index 95c47189bbb..32cb9a1c666 100644 --- a/app/actions/organization_quotas_create.rb +++ b/app/actions/organization_quotas_create.rb @@ -16,6 +16,7 @@ def create(message) QuotaDefinition.db.transaction do org_quota = VCAP::CloudController::QuotaDefinition.create( name: message.name, + total_service_accounts: message.total_service_accounts || QuotaDefinition::UNLIMITED, # Apps memory_limit: message.total_memory_in_mb || QuotaDefinition::DEFAULT_MEMORY_LIMIT, diff --git a/app/actions/organization_quotas_update.rb b/app/actions/organization_quotas_update.rb index 1ead4b47209..1f8bb4f785f 100644 --- a/app/actions/organization_quotas_update.rb +++ b/app/actions/organization_quotas_update.rb @@ -18,6 +18,7 @@ def self.update(quota, message, user_audit_info) quota.lock! quota.name = message.name if message.requested? :name + quota.total_service_accounts = message.total_service_accounts || QuotaDefinition::UNLIMITED if message.service_accounts_limits_message.requested?(:total_service_accounts) quota.memory_limit = memory_limit(message) if message.apps_limits_message.requested? :total_memory_in_mb quota.instance_memory_limit = instance_memory_limit(message) if message.apps_limits_message.requested? :per_process_memory_in_mb diff --git a/app/actions/organization_service_account_cleanup.rb b/app/actions/organization_service_account_cleanup.rb new file mode 100644 index 00000000000..c3d96f5d222 --- /dev/null +++ b/app/actions/organization_service_account_cleanup.rb @@ -0,0 +1,56 @@ +require 'actions/service_account_provision' +require 'repositories/service_account_event_repository' + +module VCAP::CloudController + module OrganizationServiceAccountCleanup + private + + def shared_domain_errors(org) + return [] unless ServiceAccountModel.where(organization_id: org.id).any? + + domains = org.owned_private_domains_dataset.all.select(&:shared_with_any_orgs?) + return [] if domains.empty? + + details = domains.map { |domain| "Domain '#{domain.name}' is shared with other organizations. Unshare before deleting." }.join("\n") + [CloudController::Errors::ApiError.new_from_details('OrganizationDeletionFailed', org.name, details)] + end + + def mark_deleting_for_account_cleanup(org) + return unless ServiceAccountModel.where(organization_id: org.id).any? + + org.db.transaction do + org.lock! + org.update(status: Organization::DELETING) + end + end + + def delete_service_accounts(org) + accounts = ServiceAccountModel.where(organization_id: org.id).order(:guid).all + return [] if accounts.empty? + + accounts.each_with_object([]) do |account, errors| + locally_deleted = false + account.db.transaction do + account.lock! + if PollableJobModel.where(resource_guid: account.guid, resource_type: 'service_account', state: %w[PROCESSING POLLING]).any? + raise ServiceAccountProvision::Conflict.new('service account operation is already in progress') + end + + if account.status == 'reserved' && !account.in_use? + Repositories::ServiceAccountEventRepository.record(account, 'delete', @user_audit_info, { recursive: true }) + account.destroy + locally_deleted = true + else + account.update(enabled: false, status: 'deleting') + end + end + next if locally_deleted + + Repositories::ServiceAccountEventRepository.record(account, 'delete', @user_audit_info, { recursive: true }) + CloudController::DependencyLocator.instance.service_account_provisioner.deprovision(account, delete: true) + rescue StandardError => e + errors << e + end + end + end +end diff --git a/app/actions/organization_update.rb b/app/actions/organization_update.rb index 2c814cde6bd..30a1892eba3 100644 --- a/app/actions/organization_update.rb +++ b/app/actions/organization_update.rb @@ -10,6 +10,7 @@ def initialize(user_audit_info) def update(org, message) org.db.transaction do org.lock! + org.service_account_prefix = message.service_account_prefix if message.requested?(:service_account_prefix) org.name = message.name if message.requested?(:name) LabelsUpdate.update(org, message.labels, OrganizationLabelModel) AnnotationsUpdate.update(org, message.annotations, OrganizationAnnotationModel) @@ -29,6 +30,9 @@ def update(org, message) end def validation_error!(error, message) + prefix_errors = error.errors.on(:service_account_prefix) + error!("Service account prefix '#{message.service_account_prefix}' is already reserved.") if prefix_errors&.include?(:unique) + error!('Service account prefix is immutable once allocated.') if prefix_errors&.include?(:immutable) error!("Organization name '#{message.name}' is already taken.") if error.errors.on(:name)&.include?(:unique) error!(error.message) end diff --git a/app/actions/process_restart.rb b/app/actions/process_restart.rb index 6a405e2647a..ba2f213bc07 100644 --- a/app/actions/process_restart.rb +++ b/app/actions/process_restart.rb @@ -14,7 +14,7 @@ def restart(process:, config:, stop_in_runtime:, revision: nil) runners(config).runner_for_process(process).stop end - process.update(state: ProcessModel::STARTED, revision: revision_to_set) + process.update(state: ProcessModel::STARTED, revision: revision_to_set, service_account_guid: process.app.service_account_guid, service_account_snapshot: true) runners(config).runner_for_process(process).start end end diff --git a/app/actions/route_policy_create.rb b/app/actions/route_policy_create.rb index b2905c15b46..567f9e1ba81 100644 --- a/app/actions/route_policy_create.rb +++ b/app/actions/route_policy_create.rb @@ -13,6 +13,7 @@ def create(route:, message:) validate_scope!(route.domain, message.source) route_policy = RoutePolicy.db.transaction do + lock_account_source!(message.source) # Lock the parent route row to serialize concurrent creates. # SELECT ... FOR UPDATE on an empty policies table acquires no row locks, # so two concurrent transactions can both read [] and both pass cf:any @@ -45,6 +46,17 @@ def create(route:, message:) private + def lock_account_source!(source) + match = source.match(RoutePolicy::SERVICE_ACCOUNT_SOURCE_REGEX) + return unless match + + account = ServiceAccountModel.first(identity_prefix: match[1], name: match[2]) + raise Error.new('Service account not found') unless account + + account.lock! + raise Error.new('Service account is being deleted') if account.status == 'deleting' + end + def validate_scope!(domain, source) return unless domain.route_policies_scope == 'space' return unless source.start_with?('cf:org:') diff --git a/app/actions/service_account_creation_budget.rb b/app/actions/service_account_creation_budget.rb new file mode 100644 index 00000000000..8eb6877c087 --- /dev/null +++ b/app/actions/service_account_creation_budget.rb @@ -0,0 +1,27 @@ +module VCAP::CloudController + class ServiceAccountCreationBudget + WINDOW = 7.days + + def self.consume(principal, exempt:) + return yield if exempt + + # Lock the authenticated principal, not a space: requests on different API + # instances and in different spaces must share one atomic budget. + principal.lock! + now = Time.now.utc + creations = ServiceAccountModel.db[:service_account_creations] + limit = Config.config.get(:service_account_creation_limit) || -1 + # MySQL permission/authentication reads may have established an older + # repeatable-read snapshot before the principal lock was acquired. + used = creations.where(principal_guid: principal.guid).where { created_at > now - WINDOW }.select(:principal_guid).for_update.all.size + raise CloudController::Errors::V3::ApiError.new_from_details('ServiceAccountCreationLimitExceeded') if limit.between?(0, used) + + result = yield + # Independent of accounts, tombstones and audit-event retention. Failed + # creations roll this back; successful deletions never refund the budget. + creations.insert(principal_guid: principal.guid, created_at: now) # rubocop:disable Rails/SkipsModelValidations -- Transactional creation ledger, not an ActiveRecord model. + creations.where(principal_guid: principal.guid).where { created_at <= now - WINDOW }.delete + result + end + end +end diff --git a/app/actions/service_account_provision.rb b/app/actions/service_account_provision.rb new file mode 100644 index 00000000000..bb2ccc34001 --- /dev/null +++ b/app/actions/service_account_provision.rb @@ -0,0 +1,126 @@ +module VCAP::CloudController + class ServiceAccountProvision + class Conflict < StandardError; end + + # identity_ca_ref names an operator-defined UAA trust bundle containing the instance identity CA. + # CA rotation changes the bundle's contents in UAA; registered clients never change. + def initialize(clients, identity_ca_ref:, allowed_resources_ref: nil) + raise ArgumentError.new('identity_ca_ref must name a UAA trust bundle') if identity_ca_ref.blank? + + @clients = clients + @identity_ca_ref = identity_ca_ref + @allowed_resources_ref = allowed_resources_ref + end + + def provision(account) + failure = nil + account.db.transaction(savepoint: true) do + account.lock! + raise Conflict.new('service account is disabled') unless account.enabled + + begin + account.db.transaction(savepoint: true) do + principal = User.first(guid: account.client_id) + raise Conflict.new('principal identity collision') if principal && !principal.is_oauth_client + + desired = registration(account) + existing = existing_client(account.client_id) + if existing + raise Conflict.new('client identity collision') unless matching_registration?(existing, desired) + else + @clients.add(:client, desired) + end + + account.update(status: 'ready') + end + rescue StandardError => e + failure = e + account.reload.update(status: 'failed') + end + end + raise failure if failure + + account + end + + def deprovision(account, delete: false) + failure = nil + account.db.transaction(savepoint: true) do + account.lock! + raise Conflict.new('service account was enabled again') if account.enabled + raise Conflict.new('service account is still assigned or in use') if delete && account.in_use? + + begin + account.db.transaction(savepoint: true) do + existing = existing_client(account.client_id) + if existing + raise Conflict.new('client identity collision') unless matching_registration?(existing, registration(account)) + + @clients.delete(:client, account.client_id) + end + if delete + account.destroy + else + account.update(status: 'disabled') + end + end + rescue StandardError => e + failure = e + account.reload.update(status: 'failed') + end + end + raise failure if failure + end + + private + + def matching_registration?(existing, desired) + tls_keys = existing.keys.grep(/\Atls[-_]/) + return false unless tls_keys.sort == desired.keys.grep(/\Atls[-_]/).sort + + desired.all? do |key, value| + actual = existing[key] + actual = [] if key == 'scope' && !existing.key?(key) + actual = [] if key == 'scope' && actual == ['uaa.none'] + if string_array?(value) + actual.is_a?(Array) && actual.all? { |entry| entry.is_a?(String) } && actual.sort == value.sort + else + actual == value + end + end + end + + def string_array?(value) + value.is_a?(Array) && value.all? { |entry| entry.is_a?(String) } + end + + def existing_client(client_id) + @clients.get(:client, client_id) + rescue CF::UAA::NotFound + nil + end + + def registration(account) + profile = { + 'client_id' => account.client_id, + 'authorized_grant_types' => ['client_credentials'], + 'authorities' => %w[cloud_controller.read cloud_controller.write], + 'scope' => [], + 'access_token_validity' => 300, + 'tls-client-auth-ca-ref' => @identity_ca_ref, + 'tls_client_auth_san_dns' => account.certificate_dns_san, + 'tls_client_certificate_bound_access_tokens' => false, + 'tls-client-auth-claim-mappings' => [ + { 'field' => 'subject_cn', 'claim' => 'cf_instance_guid' }, + { 'field' => 'subject_ou', 'pattern' => '^app:(.+)$', 'claim' => 'app_guid' }, + { 'field' => 'subject_ou', 'pattern' => '^space:(.+)$', 'claim' => 'space_guid' }, + { 'field' => 'subject_ou', 'pattern' => '^organization:(.+)$', 'claim' => 'org_guid' } + ], + 'tls-client-auth-required-claims' => { 'org_guid' => account.organization.guid }, + 'cf_service_account_guid' => account.guid + } + profile['tls-client-auth-allowed-resources-ref'] = @allowed_resources_ref if @allowed_resources_ref + profile + end + end +end diff --git a/app/actions/space_delete.rb b/app/actions/space_delete.rb index 6d73dd46b4c..927d8654d58 100644 --- a/app/actions/space_delete.rb +++ b/app/actions/space_delete.rb @@ -1,5 +1,6 @@ require 'actions/v3/service_instance_delete' require 'jobs/v3/delete_service_instance_job' +require 'repositories/service_account_event_repository' module VCAP::CloudController class SpaceDelete @@ -76,7 +77,7 @@ def unshare_service_instances(space_model) end def delete_apps(space_model) - AppDelete.new(@user_audit_info).delete(space_model.app_models) + AppDelete.new(@user_audit_info).delete(space_model.app_models_dataset.all) end def delete_service_brokers(space_model) diff --git a/app/actions/task_create.rb b/app/actions/task_create.rb index 2eddace7bad..d2662cdb367 100644 --- a/app/actions/task_create.rb +++ b/app/actions/task_create.rb @@ -19,11 +19,16 @@ def create(app, message, user_audit_info, droplet: nil) task = nil TaskModel.db.transaction do app.lock! + account = app.service_account + account&.lock! + raise InvalidTask.new('service account must be entitled to the app space') if account && !account.entitled_to?(app.space) template_process = process_from_template(message) task = TaskModel.create( name: use_requested_name_or_generate_name(message), app: app, + service_account_guid: app.service_account_guid, + service_account_snapshot: true, state: TaskModel::PENDING_STATE, droplet: droplet, command: command(message, template_process), diff --git a/app/actions/v2/organization_delete.rb b/app/actions/v2/organization_delete.rb index 7d463fa17a1..eae8147feba 100644 --- a/app/actions/v2/organization_delete.rb +++ b/app/actions/v2/organization_delete.rb @@ -1,22 +1,35 @@ require 'actions/space_delete' require 'actions/label_delete' require 'actions/annotation_delete' +require 'actions/organization_service_account_cleanup' module VCAP::CloudController module V2 class OrganizationDelete - def initialize(space_deleter) + include OrganizationServiceAccountCleanup + + def initialize(space_deleter, user_audit_info) @space_deleter = space_deleter + @user_audit_info = user_audit_info end def delete(org_dataset) org_dataset.each do |org| + shared_errors = shared_domain_errors(org) + return shared_errors unless shared_errors.empty? + + mark_deleting_for_account_cleanup(org) errs = @space_deleter.delete(org.spaces_dataset) unless errs.empty? error_message = errs.map(&:message).join("\n\n") return [CloudController::Errors::ApiError.new_from_details('OrganizationDeletionFailed', org.name, error_message)] end + account_errors = delete_service_accounts(org) + unless account_errors.empty? + return [CloudController::Errors::ApiError.new_from_details('OrganizationDeletionFailed', org.name, account_errors.map(&:message).join("\n\n"))] + end + Organization.db.transaction do org.destroy end diff --git a/app/actions/v2/services/service_binding_create.rb b/app/actions/v2/services/service_binding_create.rb index cdb06c2807d..5e80448b5ab 100644 --- a/app/actions/v2/services/service_binding_create.rb +++ b/app/actions/v2/services/service_binding_create.rb @@ -24,6 +24,27 @@ def initialize(user_audit_info, manifest_triggered: false) end def create(app, service_instance, message, volume_mount_services_enabled, accepts_incomplete) + broker_error = nil + binding = ServiceBinding.db.transaction(savepoint: true) do + app.lock! if service_instance.is_a?(ManagedServiceInstance) + create_binding(app, service_instance, message, volume_mount_services_enabled, accepts_incomplete) + rescue VCAP::Services::ServiceBrokers::V2::Errors::HttpClientTimeout, + VCAP::Services::ServiceBrokers::V2::Errors::ServiceBrokerBadResponse, + VCAP::Services::ServiceBrokers::V2::Errors::ServiceBrokerInvalidVolumeMounts, + VCAP::Services::ServiceBrokers::V2::Errors::ServiceBrokerInvalidSyslogDrainUrl, + VCAP::Services::ServiceBrokers::V2::Errors::ServiceBrokerResponseMalformed => e + # The broker client enqueues orphan mitigation before raising. Preserve + # those jobs; no binding has been persisted for these broker failures. + broker_error = e + end + raise broker_error if broker_error + + binding + end + + private + + def create_binding(app, service_instance, message, volume_mount_services_enabled, accepts_incomplete) raise ServiceInstanceNotBindable unless service_instance.bindable? raise VolumeMountServiceDisabled if service_instance.volume_service? && !volume_mount_services_enabled raise SpaceMismatch unless bindable_in_space?(service_instance, app.space) @@ -40,6 +61,8 @@ def create(app, service_instance, message, volume_mount_services_enabled, accept ) raise InvalidServiceBinding.new(binding.errors.full_messages.join(' ')) unless binding.valid? + binding.capture_service_account_identity! + client = VCAP::Services::ServiceClientProvider.provide(instance: service_instance) binding_result = request_binding_from_broker(client, binding, message.parameters, accepts_incomplete) @@ -68,8 +91,6 @@ def create(app, service_instance, message, volume_mount_services_enabled, accept binding end - private - def request_binding_from_broker(client, service_binding, parameters, accepts_incomplete) client.bind(service_binding, arbitrary_parameters: parameters, accepts_incomplete: accepts_incomplete) end diff --git a/app/collection_transformers/username_populator.rb b/app/collection_transformers/username_populator.rb index 866baf3e145..f3a90adde7c 100644 --- a/app/collection_transformers/username_populator.rb +++ b/app/collection_transformers/username_populator.rb @@ -8,9 +8,12 @@ def initialize(uaa_client) def transform(users, _opts={}) users = Array(users) - user_ids = users.collect(&:guid) - username_mapping = uaa_client.usernames_for_ids(user_ids) - users.each { |user| user.username = username_mapping[user.guid] } + client_ids = users.select(&:is_oauth_client).map(&:guid) + accounts = ServiceAccountModel.for_client_ids(client_ids).all + account_names = accounts.to_h { |account| [account.client_id, account.name] } + human_users = users.reject { |user| user.is_oauth_client && account_names.key?(user.guid) } + username_mapping = human_users.empty? ? {} : uaa_client.usernames_for_ids(human_users.collect(&:guid)) + users.each { |user| user.username = user.is_oauth_client && account_names.key?(user.guid) ? account_names[user.guid] : username_mapping[user.guid] } end end end diff --git a/app/controllers/runtime/organizations_controller.rb b/app/controllers/runtime/organizations_controller.rb index 571aa054d5d..20e59b6ab0b 100644 --- a/app/controllers/runtime/organizations_controller.rb +++ b/app/controllers/runtime/organizations_controller.rb @@ -268,7 +268,7 @@ def delete(guid) raise CloudController::Errors::ApiError.new_from_details('AssociationNotEmpty', 'spaces', Organization.table_name) if org.spaces_dataset.any? && !recursive_delete? space_delete_action = SpaceDelete.new(UserAuditInfo.from_context(SecurityContext), @services_event_repository) - delete_action = V2::OrganizationDelete.new(space_delete_action) + delete_action = V2::OrganizationDelete.new(space_delete_action, UserAuditInfo.from_context(SecurityContext)) deletion_job = VCAP::CloudController::Jobs::DeleteActionJob.new(Organization, guid, delete_action) response = run_or_enqueue_deletion_job(deletion_job) diff --git a/app/controllers/v3/app_service_accounts_controller.rb b/app/controllers/v3/app_service_accounts_controller.rb new file mode 100644 index 00000000000..d12dd167d93 --- /dev/null +++ b/app/controllers/v3/app_service_accounts_controller.rb @@ -0,0 +1,43 @@ +require 'actions/app_assign_service_account' +require 'messages/app_service_account_update_message' +require 'fetchers/app_fetcher' +require 'jobs/v3/service_account_provision' + +class AppServiceAccountsController < ApplicationController + def show + app, = fetch_readable_app + render status: :ok, json: relationship(app) + end + + def update + app, space = fetch_readable_app + unauthorized! unless permission_queryer.can_write_to_active_space?(space.id) + require_writable_space!(space) + message = AppServiceAccountUpdateMessage.new(hashed_params[:body]) + unprocessable!(message.errors.full_messages) unless message.valid? + + account = ServiceAccountModel.where(guid: message.account_guid).first if message.account_guid + resource_not_found!(:service_account) if message.account_guid && !account + job = AppAssignServiceAccount.new(permission_queryer, actor: user_audit_info).assign(app, account, provision: Config.config.get(:service_accounts_enabled) == true) + add_warning_headers(["Restart #{app.name} for the service-account assignment change to take effect."]) + return head :accepted, 'Location' => url_builder.build_url(path: "/v3/jobs/#{job.guid}") if job.is_a?(PollableJobModel) + + render status: :ok, json: relationship(app) + rescue AppAssignServiceAccount::Unauthorized + unauthorized! + rescue AppAssignServiceAccount::Conflict => e + raise CloudController::Errors::V3::ApiError.new_from_details('ServiceAccountAssignmentConflict', e.message) + end + + private + + def fetch_readable_app + app, space = AppFetcher.new.fetch(hashed_params[:app_guid]) + resource_not_found!(:app) unless app && permission_queryer.can_read_from_space?(space.id, space.organization_id) + [app, space] + end + + def relationship(app) + { data: app.service_account_guid ? { guid: app.service_account_guid } : nil } + end +end diff --git a/app/controllers/v3/organizations_controller.rb b/app/controllers/v3/organizations_controller.rb index 162ea64ab6e..652a0f5d236 100644 --- a/app/controllers/v3/organizations_controller.rb +++ b/app/controllers/v3/organizations_controller.rb @@ -52,6 +52,7 @@ def create unprocessable!(message.errors.full_messages) unless message.valid? unauthorized! if suspended_by_non_admin?(message) + unauthorized! if message.requested?(:service_account_prefix) && !permission_queryer.can_write_globally? org = nil @@ -82,6 +83,7 @@ def update unprocessable!(message.errors.full_messages) unless message.valid? unauthorized! if suspended_by_non_admin?(message) + unauthorized! if message.requested?(:service_account_prefix) && !permission_queryer.can_write_globally? org = OrganizationUpdate.new(user_audit_info).update(org, message) diff --git a/app/controllers/v3/route_policies_controller.rb b/app/controllers/v3/route_policies_controller.rb index 0fe23c7b67f..fc9bbbeb726 100755 --- a/app/controllers/v3/route_policies_controller.rb +++ b/app/controllers/v3/route_policies_controller.rb @@ -53,6 +53,7 @@ def create route = find_and_authorize_route(message.route_guid) validate_route_domain(route) + validate_account_source!(message.source) route_policy = VCAP::CloudController::RoutePolicyCreate.new(user_audit_info).create(route: route, message: message) @@ -88,6 +89,17 @@ def destroy private + def validate_account_source!(source) + match = source.match(RoutePolicy::SERVICE_ACCOUNT_SOURCE_REGEX) + return unless match + + account = ServiceAccountModel.first(identity_prefix: match[1], name: match[2]) + readable = account && (permission_queryer.can_read_globally? || + account.organization.managers_dataset.where(users__id: current_user.id).any? || + account.entitled_spaces.any? { |space| permission_queryer.can_read_from_space?(space.id, space.organization_id) }) + unprocessable!('Service account not found or not readable') unless readable + end + def find_route_policy_with_route_and_space(guid) VCAP::CloudController::RoutePolicy.eager_graph(route: :space).where(Sequel[:route_policies][:guid] => guid).all[0] end @@ -142,14 +154,7 @@ def build_dataset(message) dataset = dataset.where(guid: message.guids) if message.requested?(:guids) if message.requested?(:sources) - conditions = message.sources.map do |src| - if src == 'cf:any' - Sequel.&(source_type: 'any', source_guid: '') - else - m = src.match(/\Acf:(app|space|org):([0-9a-f-]+)\z/) - Sequel.&(source_type: m[1], source_guid: m[2]) - end - end + conditions = message.sources.map { |source| source_condition(source) } dataset = dataset.where(Sequel.|(*conditions)) end @@ -166,4 +171,16 @@ def build_dataset(message) dataset end + + def source_condition(source) + return Sequel.&(source_type: 'any', source_guid: '') if source == 'cf:any' + + if (identity = source.match(RoutePolicy::SERVICE_ACCOUNT_SOURCE_REGEX)) + account = ServiceAccountModel.first(identity_prefix: identity[1], name: identity[2]) + Sequel.&(source_type: 'svc', source_guid: account&.guid) + else + match = source.match(/\Acf:(app|space|org):([0-9a-f-]+)\z/) + Sequel.&(source_type: match[1], source_guid: match[2]) + end + end end diff --git a/app/controllers/v3/service_accounts_controller.rb b/app/controllers/v3/service_accounts_controller.rb new file mode 100644 index 00000000000..d49bfac1d8f --- /dev/null +++ b/app/controllers/v3/service_accounts_controller.rb @@ -0,0 +1,253 @@ +require 'messages/service_account_create_message' +require 'presenters/v3/service_account_presenter' +require 'messages/service_account_update_message' +require 'messages/service_accounts_list_message' +require 'messages/apps_list_message' +require 'fetchers/app_list_fetcher' +require 'fetchers/base_list_fetcher' +require 'presenters/v3/app_presenter' +require 'repositories/service_account_event_repository' +require 'actions/service_account_creation_budget' + +class ServiceAccountsController < ApplicationController + def index + message = ServiceAccountsListMessage.from_params(query_params) + invalid_param!(message.errors.full_messages) unless message.valid? + dataset = ServiceAccountModel.eager(:organization, :entitled_spaces, :labels, :annotations) + unless permission_queryer.can_read_globally? + entitled = ServiceAccountModel.db[:service_account_space_entitlements].where(space_guid: permission_queryer.readable_space_guids_query).select(:service_account_guid) + managed_orgs = current_user.managed_organizations_dataset.select(:id) + dataset = dataset.where(Sequel.|({ guid: entitled }, { organization_id: managed_orgs })) + end + dataset = dataset.where(name: message.names) if message.requested?(:names) + if message.requested?(:space_guids) + dataset = dataset.where(guid: ServiceAccountModel.db[:service_account_space_entitlements].where(space_guid: message.space_guids).select(:service_account_guid)) + end + dataset = dataset.where(organization_id: Organization.where(guid: message.organization_guids).select(:id)) if message.requested?(:organization_guids) + dataset = BaseListFetcher.filter(message, dataset, ServiceAccountModel) + render_list(dataset, message, Presenters::V3::ServiceAccountPresenter, '/v3/service_accounts') + end + + def apps + account = readable_account + message = AppsListMessage.from_params(query_params) + invalid_param!(message.errors.full_messages) unless message.valid? + spaces = account.entitled_spaces_dataset + spaces = spaces.where(guid: permission_queryer.readable_space_guids_query) unless permission_queryer.can_read_globally? + spaces = spaces.select_map(:guid) + dataset = AppListFetcher.fetch(message, spaces).where(service_account_guid: account.guid) + render_list(dataset, message, Presenters::V3::AppPresenter, "/v3/service_accounts/#{account.guid}/apps") + end + + def show + account = readable_account + + render status: :ok, json: Presenters::V3::ServiceAccountPresenter.new(account) + end + + def create + message = ServiceAccountCreateMessage.new(hashed_params[:body]) + unprocessable!(message.errors.full_messages) unless message.valid? + + space = Space.where(guid: message.space_guid).first + unprocessable!('Space not found') unless space && permission_queryer.can_read_from_space?(space.id, space.organization_id) + reuse = authorize_creation!(space, message) + require_writable_space!(space) + unprocessable!('Organization service account prefix is unset') unless space.organization.service_account_prefix + + account = nil + previous_guid = nil + ServiceAccountModel.db.transaction do + space.organization.lock! + unprocessable!('Organization is being deleted') if space.organization.deleting? + limit = space.organization.quota_definition.total_service_accounts + unprocessable!('Organization service account quota exceeded') if limit != -1 && ServiceAccountModel.where(organization_id: space.organization_id).count >= limit + ServiceAccountCreationBudget.consume(current_user, exempt: permission_queryer.can_write_globally?) do + previous_guid = release_reservation_for_reuse(space, message) if reuse + account = ServiceAccountModel.create(name: message.name, space: space) + apply_metadata(account, message) + Repositories::ServiceAccountEventRepository.record(account, 'create', user_audit_info, reuse_name: reuse, previous_account_guid: previous_guid) + end + end + add_warning_headers(['Reusing this identity can inherit external grants and accept still-valid old certificates.']) if reuse + render status: :created, json: Presenters::V3::ServiceAccountPresenter.new(account) + rescue Sequel::ValidationFailed => e + raise CloudController::Errors::V3::ApiError.new_from_details('ServiceAccountNameReserved') if e.message.include?('is already reserved') + + unprocessable!(e.message) + end + + def update + account = readable_account + authorize_account_management!(account) + message = ServiceAccountUpdateMessage.new(hashed_params[:body]) + unprocessable!(message.errors.full_messages) unless message.valid? + job = nil + account.db.transaction do + account.lock! + authorize_account_management!(account) + apply_metadata(account, message) + if message.requested?(:enabled) + require_provisioning! + reject_active_operation!(account) + account.update(enabled: message.enabled, status: 'reconciling') + job = enqueue_lifecycle(account, message.enabled ? 'provision' : 'disable') + end + Repositories::ServiceAccountEventRepository.record(account, 'update', user_audit_info, message.audit_hash) + end + return head :accepted, 'Location' => url_builder.build_url(path: "/v3/jobs/#{job.guid}") if job + + render status: :ok, json: Presenters::V3::ServiceAccountPresenter.new(account.reload) + end + + def destroy + account = readable_account + authorize_account_management!(account) + require_provisioning! + job = nil + account.db.transaction do + account.lock! + authorize_account_management!(account) + lifecycle_conflict!('service account is still assigned or in use') if account.in_use? + reject_active_operation!(account) + account.update(enabled: false, status: 'deleting') + job = enqueue_lifecycle(account, 'delete') + Repositories::ServiceAccountEventRepository.record(account, 'delete', user_audit_info) + end + head :accepted, 'Location' => url_builder.build_url(path: "/v3/jobs/#{job.guid}") + end + + private + + def release_reservation_for_reuse(space, message) + lifecycle_conflict!('live account name is already in use') if ServiceAccountModel.where(organization_id: space.organization_id, name: message.name).any? + history = ServiceAccountModel.db[:service_account_name_deletions]. + where(organization_guid: space.organization.guid, name: message.name).order(Sequel.desc(:deleted_at)).first + ServiceAccountModel.db[:service_account_names].where(organization_id: space.organization_id, name: message.name).delete + history&.fetch(:account_guid) + end + + def authorize_creation!(space, message) + reuse = message.reuse_name == true + manager = permission_queryer.can_write_globally? || space.organization.managers_dataset.where(users__id: current_user.id).any? + unauthorized! if reuse && !manager + unauthorized! unless permission_queryer.can_write_to_active_space?(space.id) || (reuse && manager) + reuse + end + + def org_manager?(account) + permission_queryer.can_write_globally? || account.organization.managers_dataset.where(users__id: current_user.id).any? + end + + def developer?(space) + space.developers_dataset.where(users__id: current_user.id).any? + end + + def authorize_account_management!(account) + unauthorized! unless org_manager?(account) || account.entitled_spaces_dataset.all.any? { |space| developer?(space) && space.active? } + require_writable_org!(account.organization) + end + + public + + def spaces + account = readable_account + render status: :ok, json: { data: account.entitled_spaces.map { |space| { guid: space.guid } } } + end + + def entitle + account = readable_account + unprocessable!('Organization service account prefix is unset') unless account.organization.service_account_prefix + data = entitlement_guids + account.db.transaction do + account.lock! + lifecycle_conflict!('service account is being deleted') if account.status == 'deleting' + require_writable_org!(account.organization) + data.each do |guid| + space = Space.first(guid: guid) + unprocessable!('Space must belong to the account organization') unless space && space.organization_id == account.organization_id + unprocessable!('Space is being deleted') if space.deleting? + unauthorized! unless can_entitle?(account, space) + require_writable_space!(space) + account.add_entitled_space(space) unless account.entitled_spaces_dataset.where(spaces__guid: space.guid).any? + Repositories::ServiceAccountEventRepository.record(account, 'entitle', user_audit_info, space_guid: space.guid) + end + end + spaces + end + + def unentitle + account = readable_account + account.db.transaction do + account.lock! + require_writable_org!(account.organization) + space = account.entitled_spaces_dataset.where(spaces__guid: hashed_params[:space_guid]).first + resource_not_found!(:space) unless space + unauthorized! unless org_manager?(account) || developer?(space) + require_writable_space!(space) + lifecycle_conflict!('cannot revoke the last entitlement; delete the account instead') if !org_manager?(account) && account.entitled_spaces_dataset.one? + lifecycle_conflict!('service account is still assigned or in use in this space') if account.in_use_in_space?(space) + account.remove_entitled_space(space) + Repositories::ServiceAccountEventRepository.record(account, 'unentitle', user_audit_info, space_guid: space.guid) + end + render status: :ok, json: { data: account.entitled_spaces_dataset.all.map { |space| { guid: space.guid } } } + end + + private + + def can_entitle?(account, space) + org_manager?(account) || (developer?(space) && account.entitled_spaces_dataset.all.any? { |source| source.active? && developer?(source) }) + end + + def entitlement_guids + data = hashed_params[:body]['data'] || hashed_params[:body][:data] + unprocessable!('Data must be an array of space GUIDs') unless data.is_a?(Array) + data.map do |entry| + unprocessable!('Data must be an array of space GUIDs') unless entry.is_a?(Hash) && (entry['guid'] || entry[:guid]).is_a?(String) + entry['guid'] || entry[:guid] + end + end + + def readable?(account) + permission_queryer.can_read_globally? || org_manager?(account) || + account.entitled_spaces.any? { |space| permission_queryer.can_read_from_space?(space.id, space.organization_id) } + end + + def require_provisioning! + lifecycle_conflict!('service accounts are not enabled') unless Config.config.get(:service_accounts_enabled) == true + end + + def reject_active_operation!(account) + return unless PollableJobModel.where(resource_guid: account.guid, resource_type: 'service_account', state: %w[PROCESSING POLLING]).any? + + lifecycle_conflict!('service account operation is already in progress') + end + + def lifecycle_conflict!(detail) + raise CloudController::Errors::V3::ApiError.new_from_details('ServiceAccountAssignmentConflict', detail) + end + + def enqueue_lifecycle(account, operation) + Jobs::Enqueuer.new(queue: Jobs::Queues.generic).enqueue_pollable(Jobs::V3::ServiceAccountProvision.new(account.guid, operation: operation)) + end + + def readable_account + account = ServiceAccountModel.first(guid: hashed_params[:guid]) + resource_not_found!(:service_account) unless account && readable?(account) + account + end + + def apply_metadata(account, message) + account.update(description: message.description || '') if message.requested?(:description) + LabelsUpdate.update(account, message.labels, ServiceAccountLabelModel) + AnnotationsUpdate.update(account, message.annotations, ServiceAccountAnnotationModel) + end + + def render_list(dataset, message, presenter, path) + render status: :ok, json: Presenters::V3::PaginatedListPresenter.new( + presenter: presenter, + paginated_result: SequelPaginator.new.get_page(dataset, message.pagination_options), + path: path, message: message + ) + end +end diff --git a/app/decorators/include_route_policy_source_decorator.rb b/app/decorators/include_route_policy_source_decorator.rb index c89d1e72be9..3f7400ff67a 100644 --- a/app/decorators/include_route_policy_source_decorator.rb +++ b/app/decorators/include_route_policy_source_decorator.rb @@ -31,12 +31,22 @@ def decorate(hash, route_policies) hash[:included][:apps] = fetch_and_present_apps(app_guids.uniq, permission_queryer) hash[:included][:spaces] = fetch_and_present_spaces(space_guids.uniq, permission_queryer) hash[:included][:organizations] = fetch_and_present_organizations(org_guids.uniq, permission_queryer) + account_guids = route_policies.select { |policy| policy.source_type == 'svc' }.map(&:source_guid) + hash[:included][:service_accounts] = fetch_and_present_accounts(account_guids, permission_queryer) unless account_guids.empty? hash end private + def fetch_and_present_accounts(guids, permissions) + accounts = ServiceAccountModel.where(guid: guids).all.select do |account| + permissions.can_read_globally? || account.organization.managers_dataset.where(users__id: SecurityContext.current_user.id).any? || + account.entitled_spaces.any? { |space| permissions.can_read_from_space?(space.id, space.organization_id) } + end + accounts.map { |account| Presenters::V3::ServiceAccountPresenter.new(account).to_hash } + end + def fetch_and_present_apps(guids, permission_queryer) return [] if guids.empty? diff --git a/app/jobs/v3/service_account_provision.rb b/app/jobs/v3/service_account_provision.rb new file mode 100644 index 00000000000..756ad02fad8 --- /dev/null +++ b/app/jobs/v3/service_account_provision.rb @@ -0,0 +1,48 @@ +require 'jobs/cc_job' +require 'actions/service_account_provision' + +module VCAP::CloudController + module Jobs + module V3 + class ServiceAccountProvision < CCJob + attr_reader :resource_guid + + def initialize(account_guid, operation: 'provision') + @resource_guid = account_guid + @operation = operation + end + + def perform + account = ServiceAccountModel.first(guid: resource_guid) + raise CloudController::Errors::ApiError.new_from_details('ResourceNotFound', 'The service account could not be found') unless account + + provisioner = CloudController::DependencyLocator.instance.service_account_provisioner + case @operation + when 'provision' + provisioner.provision(account) + when 'disable', 'delete' + provisioner.deprovision(account, delete: @operation == 'delete') + else + raise ArgumentError.new('unsupported service account operation') + end + end + + def max_attempts + 3 + end + + def resource_type + 'service_account' + end + + def display_name + "service_account.#{@operation}" + end + + def job_name_in_configuration + :service_account_provision + end + end + end + end +end diff --git a/app/messages/app_service_account_update_message.rb b/app/messages/app_service_account_update_message.rb new file mode 100644 index 00000000000..03b952c5660 --- /dev/null +++ b/app/messages/app_service_account_update_message.rb @@ -0,0 +1,24 @@ +require 'messages/base_message' + +module VCAP::CloudController + class AppServiceAccountUpdateMessage < BaseMessage + register_allowed_keys [:data] + validates_with NoAdditionalKeysValidator + validate :relationship_data + + def account_guid + HashUtils.dig(data, :guid) + end + + def relationship_data + unless requested?(:data) + errors.add(:data, 'must be provided') + return + end + return if data.nil? + return if data.is_a?(Hash) && data.keys == [:guid] && account_guid.is_a?(String) && account_guid.present? + + errors.add(:data, 'must be null or an object containing only a nonempty guid string') + end + end +end diff --git a/app/messages/organization_quotas_update_message.rb b/app/messages/organization_quotas_update_message.rb index d111bf733a3..df499883af8 100644 --- a/app/messages/organization_quotas_update_message.rb +++ b/app/messages/organization_quotas_update_message.rb @@ -12,7 +12,7 @@ def self.key_requested?(key) proc { |a| a.requested?(key) } end - register_allowed_keys %i[name apps services routes domains] + register_allowed_keys %i[name apps services routes domains service_accounts] validates_with NoAdditionalKeysValidator validates :name, @@ -24,6 +24,20 @@ def self.key_requested?(key) validate :services_validator, if: key_requested?(:services) validate :routes_validator, if: key_requested?(:routes) validate :domains_validator, if: key_requested?(:domains) + validate :service_accounts_validator, if: key_requested?(:service_accounts) + + def service_accounts_limits_message + @service_accounts_limits_message ||= ServiceAccountsLimitsMessage.new(service_accounts&.deep_symbolize_keys) + end + + delegate :total_service_accounts, to: :service_accounts_limits_message + + def service_accounts_validator + return unless validates_hash(service_accounts, :service_accounts) + return if service_accounts_limits_message.valid? + + service_accounts_limits_message.errors.full_messages.each { |message| errors.add(:service_accounts, message:) } + end # Apps validations delegate :total_memory_in_mb, :per_process_memory_in_mb, :total_instances, :per_app_tasks, :log_rate_limit_in_bytes_per_second, to: :apps_limits_message @@ -107,4 +121,10 @@ class DomainsLimitsMessage < BaseMessage numericality: { only_integer: true, greater_than_or_equal_to: 0 }, allow_nil: true end + + class ServiceAccountsLimitsMessage < BaseMessage + register_allowed_keys [:total_service_accounts] + validates_with NoAdditionalKeysValidator + validates :total_service_accounts, numericality: { only_integer: true, greater_than_or_equal_to: 0 }, allow_nil: true + end end diff --git a/app/messages/organization_update_message.rb b/app/messages/organization_update_message.rb index 26e82b4b0f2..b79f76726e2 100644 --- a/app/messages/organization_update_message.rb +++ b/app/messages/organization_update_message.rb @@ -2,7 +2,7 @@ module VCAP::CloudController class OrganizationUpdateMessage < MetadataBaseMessage - register_allowed_keys %i[name suspended] + register_allowed_keys %i[name suspended service_account_prefix] validates_with NoAdditionalKeysValidator @@ -15,5 +15,11 @@ class OrganizationUpdateMessage < MetadataBaseMessage validates :suspended, boolean: true, allow_nil: true + + validates :service_account_prefix, + string: true, + format: { with: ->(_) { Organization::SERVICE_ACCOUNT_PREFIX_REGEX }, + message: 'must be a lowercase RFC 1123 label of 3-63 characters' }, + allow_nil: true end end diff --git a/app/messages/route_policies_list_message.rb b/app/messages/route_policies_list_message.rb index 6e380806aaf..96fe95fc9ee 100644 --- a/app/messages/route_policies_list_message.rb +++ b/app/messages/route_policies_list_message.rb @@ -30,7 +30,7 @@ def self.from_params(params) def sources_format_valid return unless sources.is_a?(Array) - invalid = sources.reject { |s| s.is_a?(String) && RoutePolicy::SOURCE_REGEX.match?(s) } + invalid = sources.reject { |s| s.is_a?(String) && (RoutePolicy::SOURCE_REGEX.match?(s) || RoutePolicy::SERVICE_ACCOUNT_SOURCE_REGEX.match?(s)) } return if invalid.empty? errors.add(:sources, "contains invalid source format: #{invalid.join(', ')}") diff --git a/app/messages/route_policy_create_message.rb b/app/messages/route_policy_create_message.rb index fa7edb57c94..0dba5cfbcbe 100644 --- a/app/messages/route_policy_create_message.rb +++ b/app/messages/route_policy_create_message.rb @@ -26,6 +26,7 @@ def relationships_message def source_format_valid return unless source.is_a?(String) return if RoutePolicy::SOURCE_REGEX.match?(source) + return if RoutePolicy::SERVICE_ACCOUNT_SOURCE_REGEX.match?(source) errors.add(:source, "must be in format 'cf:app:', 'cf:space:', 'cf:org:', or 'cf:any'") end diff --git a/app/messages/service_account_create_message.rb b/app/messages/service_account_create_message.rb new file mode 100644 index 00000000000..ecfc42e9f2f --- /dev/null +++ b/app/messages/service_account_create_message.rb @@ -0,0 +1,28 @@ +require 'messages/metadata_base_message' + +module VCAP::CloudController + class ServiceAccountCreateMessage < MetadataBaseMessage + register_allowed_keys %i[name relationships description reuse_name] + validates :reuse_name, boolean: true, allow_nil: true + validates :description, string: true, allow_nil: true, length: { maximum: 250 } + validates_with NoAdditionalKeysValidator, RelationshipValidator + validates :name, presence: true, string: true, + format: { with: ->(_) { ServiceAccountModel::NAME_PATTERN } }, length: { in: 3..63 } + + delegate :space_guid, to: :relationships_message + + def relationships_message + @relationships_message ||= Relationships.new(relationships.deep_symbolize_keys) + end + + class Relationships < BaseMessage + register_allowed_keys [:space] + validates_with NoAdditionalKeysValidator + validates :space, presence: true, to_one_relationship: true + + def space_guid + HashUtils.dig(space, :data, :guid) + end + end + end +end diff --git a/app/messages/service_account_update_message.rb b/app/messages/service_account_update_message.rb new file mode 100644 index 00000000000..6bc199effe0 --- /dev/null +++ b/app/messages/service_account_update_message.rb @@ -0,0 +1,10 @@ +require 'messages/metadata_base_message' + +module VCAP::CloudController + class ServiceAccountUpdateMessage < MetadataBaseMessage + register_allowed_keys %i[description enabled] + validates :enabled, boolean: true, if: -> { requested?(:enabled) } + validates_with NoAdditionalKeysValidator + validates :description, string: true, allow_nil: true, length: { maximum: 250 } + end +end diff --git a/app/messages/service_accounts_list_message.rb b/app/messages/service_accounts_list_message.rb new file mode 100644 index 00000000000..ef4e83f9471 --- /dev/null +++ b/app/messages/service_accounts_list_message.rb @@ -0,0 +1,17 @@ +require 'messages/list_message' + +module VCAP::CloudController + class ServiceAccountsListMessage < ListMessage + register_allowed_keys %i[names space_guids organization_guids] + validates_with NoAdditionalParamsValidator + validates :names, :space_guids, :organization_guids, array: true, allow_nil: true + + def self.from_params(params) + super(params, %w[names space_guids organization_guids]) + end + + def valid_order_by_values + super + [:name] + end + end +end diff --git a/app/models.rb b/app/models.rb index 7944a73f04b..f839d668447 100644 --- a/app/models.rb +++ b/app/models.rb @@ -13,6 +13,7 @@ require 'models/runtime/docker_lifecycle_data_model' require 'models/runtime/task_model' require 'models/runtime/isolation_segment_model' +require 'models/runtime/service_account_model' require 'models/runtime/pollable_job_model' require 'models/runtime/job_warning_model' require 'models/runtime/security_group' @@ -89,6 +90,8 @@ require 'models/runtime/domain_label_model' require 'models/runtime/droplet_label_model' require 'models/runtime/isolation_segment_label_model' +require 'models/runtime/service_account_label_model' +require 'models/runtime/service_account_annotation_model' require 'models/runtime/organization_label_model' require 'models/runtime/package_label_model' require 'models/runtime/process_label_model' diff --git a/app/models/runtime/app_model.rb b/app/models/runtime/app_model.rb index cdefb16b438..26a362518eb 100644 --- a/app/models/runtime/app_model.rb +++ b/app/models/runtime/app_model.rb @@ -17,6 +17,7 @@ class AppModel < Sequel::Model(:apps) one_to_many :tasks, class: 'VCAP::CloudController::TaskModel', key: :app_guid, primary_key: :guid many_to_one :space, class: 'VCAP::CloudController::Space', key: :space_guid, primary_key: :guid, without_guid_generation: true + many_to_one :service_account, class: 'VCAP::CloudController::ServiceAccountModel', key: :service_account_guid, primary_key: :guid, without_guid_generation: true one_through_one :organization, join_table: Space.table_name, left_key: :guid, left_primary_key: :space_guid, right_primary_key: :id, right_key: :organization_id one_to_many :processes, class: 'VCAP::CloudController::ProcessModel', key: :app_guid, primary_key: :guid do |dataset| @@ -94,6 +95,8 @@ def validate validate_environment_variables validate_droplet_is_staged + errors.add(:service_account, 'must be entitled to the app space in the same organization') if service_account && !service_account.entitled_to?(space) + validates_includes Lifecycles::TYPES, :lifecycle_type end diff --git a/app/models/runtime/organization.rb b/app/models/runtime/organization.rb index 88111564116..4fc96c3b64e 100644 --- a/app/models/runtime/organization.rb +++ b/app/models/runtime/organization.rb @@ -7,6 +7,7 @@ class Organization < Sequel::Model ORG_NAME_REGEX = /\A[[:alnum:][:punct:][:print:]]+\Z/ ORG_STATUS_VALUES = VALID_STATUSES + SERVICE_ACCOUNT_PREFIX_REGEX = /\A[a-z0-9][a-z0-9-]{1,61}[a-z0-9]\z/ one_to_many :spaces @@ -205,12 +206,21 @@ def before_save validate_quota + # Unique insertion serializes allocation; reservations survive org deletion. + # rubocop:disable Rails/SkipsModelValidations -- Atomic Sequel dataset reservation, not an ActiveRecord model. + db[:service_account_prefixes].insert(prefix: service_account_prefix) if service_account_prefix && (new? || column_changed?(:service_account_prefix)) + # rubocop:enable Rails/SkipsModelValidations + super end def around_save yield rescue Sequel::UniqueConstraintViolation => e + if e.message.include?('service_account_prefixes_prefix_unique') || e.message.include?('organizations_service_account_prefix_unique') + errors.add(:service_account_prefix, :unique) + raise validation_failed_error + end raise e unless e.message.include?('organizations_name_index') errors.add(:name, :unique) @@ -221,6 +231,8 @@ def validate validates_presence :name validates_format ORG_NAME_REGEX, :name validates_includes ORG_STATUS_VALUES, :status, allow_missing: true + validates_format SERVICE_ACCOUNT_PREFIX_REGEX, :service_account_prefix, allow_nil: true + errors.add(:service_account_prefix, :immutable) if !new? && column_changed?(:service_account_prefix) && initial_value(:service_account_prefix) return unless column_changed?(:default_isolation_segment_guid) diff --git a/app/models/runtime/process_model.rb b/app/models/runtime/process_model.rb index eb031eda821..be92ac3e725 100644 --- a/app/models/runtime/process_model.rb +++ b/app/models/runtime/process_model.rb @@ -304,10 +304,24 @@ def before_validation end def before_save + if being_started? + account = app.service_account + account&.lock! + if account && !account.entitled_to?(app.space) + errors.add(:service_account, 'must be entitled to the app space') + raise validation_failed_error + end + self.service_account_guid = app.service_account_guid + self.service_account_snapshot = true + end set_new_version if version_needs_to_be_updated? super end + def runtime_service_account_guid + service_account_snapshot ? service_account_guid : nil + end + # rubocop:disable Metrics/CyclomaticComplexity def version_needs_to_be_updated? # change version if: diff --git a/app/models/runtime/route_policy.rb b/app/models/runtime/route_policy.rb index 87f63a70ebb..eb8eb332091 100644 --- a/app/models/runtime/route_policy.rb +++ b/app/models/runtime/route_policy.rb @@ -1,6 +1,7 @@ module VCAP::CloudController class RoutePolicy < Sequel::Model(:route_policies) SOURCE_REGEX = /\A(cf:(app|space|org):[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}|cf:any)\z/ + SERVICE_ACCOUNT_SOURCE_REGEX = /\Acf:svc:([a-z0-9][a-z0-9-]{1,61}[a-z0-9]):([a-z0-9][a-z0-9-]{1,61}[a-z0-9])\z/ many_to_one :route, class: 'VCAP::CloudController::Route', @@ -14,13 +15,21 @@ class RoutePolicy < Sequel::Model(:route_policies) add_association_dependencies annotations: :destroy def source + if source_type == 'svc' + account = ServiceAccountModel.first(guid: source_guid) + return "cf:svc:#{account.identity_prefix}:#{account.name}" if account + end source_guid.to_s.empty? ? 'cf:any' : "cf:#{source_type}:#{source_guid}" end def source=(val) return if val.nil? - if val == 'cf:any' + if (identity = val.match(SERVICE_ACCOUNT_SOURCE_REGEX)) + account = ServiceAccountModel.first(identity_prefix: identity[1], name: identity[2]) + self.source_type = 'svc' + self.source_guid = account&.guid + elsif val == 'cf:any' self.source_type = 'any' self.source_guid = '' else @@ -33,6 +42,7 @@ def source=(val) def validate validates_presence :source_type validates_presence :route_id + errors.add(:source, 'service account not found') if source_type == 'svc' && source_guid.nil? validate_cf_any_exclusivity end diff --git a/app/models/runtime/service_account_annotation_model.rb b/app/models/runtime/service_account_annotation_model.rb new file mode 100644 index 00000000000..234db2f44bf --- /dev/null +++ b/app/models/runtime/service_account_annotation_model.rb @@ -0,0 +1,6 @@ +module VCAP::CloudController + class ServiceAccountAnnotationModel < Sequel::Model(:service_account_annotations) + many_to_one :service_account, class: 'VCAP::CloudController::ServiceAccountModel', primary_key: :guid, key: :resource_guid, without_guid_generation: true + include MetadataModelMixin + end +end diff --git a/app/models/runtime/service_account_label_model.rb b/app/models/runtime/service_account_label_model.rb new file mode 100644 index 00000000000..15b2e7270e6 --- /dev/null +++ b/app/models/runtime/service_account_label_model.rb @@ -0,0 +1,6 @@ +module VCAP::CloudController + class ServiceAccountLabelModel < Sequel::Model(:service_account_labels) + many_to_one :service_account, class: 'VCAP::CloudController::ServiceAccountModel', primary_key: :guid, key: :resource_guid, without_guid_generation: true + include MetadataModelMixin + end +end diff --git a/app/models/runtime/service_account_model.rb b/app/models/runtime/service_account_model.rb new file mode 100644 index 00000000000..ab018c3dc04 --- /dev/null +++ b/app/models/runtime/service_account_model.rb @@ -0,0 +1,131 @@ +module VCAP::CloudController + class ServiceAccountModel < Sequel::Model(:service_accounts) + NAME_PATTERN = /\A[a-z0-9][a-z0-9-]{1,61}[a-z0-9]\z/ + + def self.for_client_ids(client_ids) + identities = client_ids.filter_map do |client_id| + namespace, type, prefix, name, extra = client_id.split(':', -1) + next unless namespace == 'cf' && type == 'service-account' && extra.nil? + next unless NAME_PATTERN.match?(prefix.to_s) && NAME_PATTERN.match?(name.to_s) + + { identity_prefix: prefix, name: name } + end + identities.empty? ? where(false) : where(Sequel.|(*identities)) + end + + # Creation input for the initial entitlement; never persisted as ownership. + attr_accessor :space + + many_to_one :organization + many_to_many :entitled_spaces, + class: 'VCAP::CloudController::Space', + join_table: :service_account_space_entitlements, + left_key: :service_account_guid, left_primary_key: :guid, + right_key: :space_guid, right_primary_key: :guid, + before_add: proc { |account, space| account.validate_entitled_space!(space) } + one_to_many :apps, class: 'VCAP::CloudController::AppModel', key: :service_account_guid, primary_key: :guid + one_to_many :labels, class: 'VCAP::CloudController::ServiceAccountLabelModel', key: :resource_guid, primary_key: :guid + one_to_many :annotations, class: 'VCAP::CloudController::ServiceAccountAnnotationModel', key: :resource_guid, primary_key: :guid + add_association_dependencies labels: :destroy, annotations: :destroy + + def before_validation + self.organization = space.organization if new? && organization_id.nil? && space + self.identity_prefix = organization.service_account_prefix if new? && identity_prefix.nil? && organization + super + end + + def validate + super + validates_presence :space if new? + validates_presence :organization + validate_identity_prefix + errors.add(:organization, 'must own the initial space') if new? && space && organization_id != space.organization_id + errors.add(:organization, 'is immutable') if !new? && column_changed?(:organization_id) + validate_identity_immutability + validates_format NAME_PATTERN, :name + errors.add(:name, 'is immutable') if !new? && column_changed?(:name) + end + + def validate_identity_immutability + errors.add(:identity_prefix, 'is immutable') if !new? && column_changed?(:identity_prefix) + end + + def validate_identity_prefix + validates_presence :identity_prefix + return unless organization + + errors.add(:organization, 'service account prefix is unset') unless organization.service_account_prefix + errors.add(:identity_prefix, 'must match the owning organization prefix') if new? && identity_prefix != organization.service_account_prefix + end + + def around_create + db.transaction(savepoint: true) do + organization.lock! + quota = organization.quota_definition + limit = quota.total_service_accounts + if limit != -1 && self.class.where(organization_id: organization_id).count >= limit + errors.add(:organization, 'service account quota exceeded') + raise validation_failed_error + end + # Keep this reservation permanently, even when the account is destroyed. + # The unique constraint, rather than a preflight query, serializes claims. + db[:service_account_names].insert(name: name, organization_id: organization_id) # rubocop:disable Rails/SkipsModelValidations -- Atomic namespace reservation, not an ActiveRecord model. + yield + end + rescue Sequel::UniqueConstraintViolation + errors.add(:name, 'is already reserved') + raise validation_failed_error + end + + def after_create + super + add_entitled_space(space) + end + + def client_id + "cf:service-account:#{identity_prefix}:#{name}" + end + + def in_use? + apps_dataset.any? || ServiceBinding.where(service_account_guid: guid).any? || + ProcessModel.where(service_account_guid: guid, state: ProcessModel::STARTED).any? || + TaskModel.where(service_account_guid: guid, state: [TaskModel::PENDING_STATE, TaskModel::RUNNING_STATE, TaskModel::CANCELING_STATE]).any? + end + + def in_use_in_space?(space) + app_guids = AppModel.where(space_guid: space.guid).select(:guid) + apps_dataset.where(space_guid: space.guid).any? || ServiceBinding.where(app_guid: app_guids, service_account_guid: guid).any? || + ProcessModel.where(app_guid: app_guids, service_account_guid: guid, state: ProcessModel::STARTED).any? || + TaskModel.where(app_guid: app_guids, service_account_guid: guid, state: [TaskModel::PENDING_STATE, TaskModel::RUNNING_STATE, TaskModel::CANCELING_STATE]).any? + end + + def entitled_to?(space) + organization_id == space.organization_id && entitled_spaces_dataset.where(spaces__guid: space.guid).any? + end + + def validate_entitled_space!(space) + return if organization_id == space.organization_id + + errors.add(:organization, 'must own every entitled space') + raise validation_failed_error + end + + def certificate_dns_san + "#{name}.#{identity_prefix}.svc.identity" + end + + def before_destroy + RoutePolicy.where(source_type: 'svc', source_guid: guid).all.each do |policy| + policy.destroy + policy.notify_diego + end + db[:service_account_names].where(name: name, organization_id: organization_id).update(deleted_at: Sequel::CURRENT_TIMESTAMP) + # Durable history is independent of audit-event retention and reservation reuse. + # rubocop:disable Rails/SkipsModelValidations -- Sequel dataset history, not an ActiveRecord model. + history = { account_guid: guid, organization_guid: organization.guid, name: name, identity_prefix: identity_prefix, deleted_at: Sequel::CURRENT_TIMESTAMP } + db[:service_account_name_deletions].insert(history) + # rubocop:enable Rails/SkipsModelValidations + super + end + end +end diff --git a/app/models/runtime/task_model.rb b/app/models/runtime/task_model.rb index bad22c00eef..7cf82b686bc 100644 --- a/app/models/runtime/task_model.rb +++ b/app/models/runtime/task_model.rb @@ -28,6 +28,10 @@ class TaskModel < Sequel::Model(:tasks) set_field_as_encrypted :environment_variables, column: :encrypted_environment_variables serializes_via_json :environment_variables + def runtime_service_account_guid + service_account_snapshot ? service_account_guid : nil + end + def after_update super diff --git a/app/models/services/service_binding.rb b/app/models/services/service_binding.rb index bd38acb9ed0..d0ad68809c5 100644 --- a/app/models/services/service_binding.rb +++ b/app/models/services/service_binding.rb @@ -98,10 +98,26 @@ def before_update end def before_create + capture_service_account_identity! unless @service_account_identity_captured encode_syslog_drain_url_commas super end + def capture_service_account_identity! + if app && service_instance.is_a?(ManagedServiceInstance) + app.lock! + account = app.service_account + account&.lock! + if account && !(account.organization_id == app.space.organization_id && + account.db[:service_account_space_entitlements].where(service_account_guid: account.guid, space_guid: app.space_guid).for_update.first) + errors.add(:service_account, 'must be entitled to the app space') + raise validation_failed_error + end + self.service_account_guid = account&.guid + end + @service_account_identity_captured = true + end + def encode_syslog_drain_url_commas return unless syslog_drain_url diff --git a/app/presenters/v3/organization_presenter.rb b/app/presenters/v3/organization_presenter.rb index 65ce25b12ae..fa7ded1c36d 100644 --- a/app/presenters/v3/organization_presenter.rb +++ b/app/presenters/v3/organization_presenter.rb @@ -18,6 +18,7 @@ def to_hash created_at: organization.created_at, updated_at: organization.updated_at, name: organization.name, + service_account_prefix: organization.service_account_prefix, suspended: organization.suspended?, relationships: { quota: { diff --git a/app/presenters/v3/organization_quota_presenter.rb b/app/presenters/v3/organization_quota_presenter.rb index be8c131889b..00f66b96176 100644 --- a/app/presenters/v3/organization_quota_presenter.rb +++ b/app/presenters/v3/organization_quota_presenter.rb @@ -21,6 +21,7 @@ def to_hash created_at: organization_quota.created_at, updated_at: organization_quota.updated_at, name: organization_quota.name, + service_accounts: { total_service_accounts: convert_unlimited_to_nil(organization_quota.total_service_accounts) }, apps: { total_memory_in_mb: convert_unlimited_to_nil(organization_quota.memory_limit), per_process_memory_in_mb: convert_unlimited_to_nil(organization_quota.instance_memory_limit), diff --git a/app/presenters/v3/route_policy_presenter.rb b/app/presenters/v3/route_policy_presenter.rb index ddbe02283d8..5e26266d41f 100755 --- a/app/presenters/v3/route_policy_presenter.rb +++ b/app/presenters/v3/route_policy_presenter.rb @@ -36,12 +36,14 @@ def route_policy end def build_relationships - { + relationships = { route: { data: { guid: route_policy.route.guid } }, app: { data: route_policy.source_type == 'app' ? { guid: route_policy.source_guid } : nil }, space: { data: route_policy.source_type == 'space' ? { guid: route_policy.source_guid } : nil }, organization: { data: route_policy.source_type == 'org' ? { guid: route_policy.source_guid } : nil } } + relationships[:service_account] = { data: { guid: route_policy.source_guid } } if route_policy.source_type == 'svc' + relationships end def build_links @@ -61,6 +63,8 @@ def build_links links[:space] = { href: url_builder.build_url(path: "/v3/spaces/#{route_policy.source_guid}") } when 'org' links[:organization] = { href: url_builder.build_url(path: "/v3/organizations/#{route_policy.source_guid}") } + when 'svc' + links[:service_account] = { href: url_builder.build_url(path: "/v3/service_accounts/#{route_policy.source_guid}") } end links diff --git a/app/presenters/v3/service_account_presenter.rb b/app/presenters/v3/service_account_presenter.rb new file mode 100644 index 00000000000..d31711fd7c0 --- /dev/null +++ b/app/presenters/v3/service_account_presenter.rb @@ -0,0 +1,30 @@ +require 'presenters/v3/base_presenter' +require 'presenters/mixins/metadata_presentation_helpers' + +module VCAP::CloudController + module Presenters + module V3 + class ServiceAccountPresenter < BasePresenter + include Mixins::MetadataPresentationHelpers + + def to_hash + { + guid: @resource.guid, + name: @resource.name, + description: @resource.description, + metadata: { labels: hashified_labels(@resource.labels), annotations: hashified_annotations(@resource.annotations) }, + created_at: @resource.created_at, + updated_at: @resource.updated_at, + client_id: @resource.client_id, + certificate_dns_san: @resource.certificate_dns_san, + enabled: @resource.enabled, + status: @resource.status, + relationships: { organization: { data: { guid: @resource.organization.guid } }, + spaces: { data: @resource.entitled_spaces.map { |space| { guid: space.guid } } } }, + links: { self: { href: url_builder.build_url(path: "/v3/service_accounts/#{@resource.guid}") } } + } + end + end + end + end +end diff --git a/app/repositories/service_account_event_repository.rb b/app/repositories/service_account_event_repository.rb new file mode 100644 index 00000000000..415fe9e4635 --- /dev/null +++ b/app/repositories/service_account_event_repository.rb @@ -0,0 +1,22 @@ +module VCAP::CloudController + module Repositories + class ServiceAccountEventRepository + def self.record(resource, operation, actor, metadata={}) + Event.create( + type: "audit.#{resource.is_a?(AppModel) ? 'app.service_account' : 'service_account'}.#{operation}", + space_guid: resource.space&.guid || '', + organization_guid: resource.is_a?(AppModel) ? resource.space.organization.guid : resource.organization.guid, + actee: resource.guid, + actee_type: resource.is_a?(AppModel) ? 'app' : 'service_account', + actee_name: resource.name, + actor: actor.user_guid, + actor_type: 'user', + actor_name: actor.user_email, + actor_username: actor.user_name, + timestamp: Sequel::CURRENT_TIMESTAMP, + metadata: metadata + ) + end + end + end +end diff --git a/config/routes.rb b/config/routes.rb index 28526281d86..996f9c0cab4 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -5,6 +5,18 @@ post '/admin/actions/clear_buildpack_cache', to: 'admin_actions#clear_buildpack_cache' # apps + post '/service_accounts', to: 'service_accounts#create' + get '/service_accounts', to: 'service_accounts#index' + patch '/service_accounts/:guid', to: 'service_accounts#update' + delete '/service_accounts/:guid', to: 'service_accounts#destroy' + get '/service_accounts/:guid/apps', to: 'service_accounts#apps' + get '/service_accounts/:guid/relationships/spaces', to: 'service_accounts#spaces' + post '/service_accounts/:guid/relationships/spaces', to: 'service_accounts#entitle' + delete '/service_accounts/:guid/relationships/spaces/:space_guid', to: 'service_accounts#unentitle' + get '/service_accounts/:guid', to: 'service_accounts#show' + get '/apps/:app_guid/relationships/service_account', to: 'app_service_accounts#show' + patch '/apps/:app_guid/relationships/service_account', to: 'app_service_accounts#update' + get '/apps', to: 'apps_v3#index' post '/apps', to: 'apps_v3#create' get '/apps/:guid', to: 'apps_v3#show' diff --git a/db/migrations/20261005120000_create_service_accounts.rb b/db/migrations/20261005120000_create_service_accounts.rb new file mode 100644 index 00000000000..82b221520e6 --- /dev/null +++ b/db/migrations/20261005120000_create_service_accounts.rb @@ -0,0 +1,28 @@ +Sequel.migration do + up do + organization_id_type = schema(:organizations).to_h.fetch(:id).fetch(:db_type) + create_table :service_account_names do + primary_key :id, name: :service_account_names_pkey + String :name, size: 63, null: false + column :organization_id, organization_id_type, null: false + DateTime :deleted_at + index %i[organization_id name], unique: true, name: :service_account_names_org_name_unique + end + + create_table :service_accounts do + VCAP::Migration.common(self) + String :name, size: 63, null: false + column :organization_id, organization_id_type, null: false + foreign_key [:organization_id], :organizations, name: :fk_service_accounts_organization + String :identity_prefix, size: 63, null: false + index %i[organization_id name], unique: true, name: :service_accounts_org_name_unique + TrueClass :enabled, null: false, default: true + String :status, size: 32, null: false, default: 'reserved' + end + end + + down do + drop_table :service_accounts + drop_table :service_account_names + end +end diff --git a/db/migrations/20261005120100_add_app_service_account.rb b/db/migrations/20261005120100_add_app_service_account.rb new file mode 100644 index 00000000000..791fdca4444 --- /dev/null +++ b/db/migrations/20261005120100_add_app_service_account.rb @@ -0,0 +1,40 @@ +Sequel.migration do + no_transaction + + up do + options = { size: 255 } + if database_type == :mysql + parent = fetch("SHOW FULL COLUMNS FROM service_accounts LIKE 'guid'").first + options[:collate] = parent[:Collation] || parent[:collation] + end + existing_column = schema(:apps).any? { |name, _| name == :service_account_guid } + existing_foreign_key = foreign_key_list(:apps).any? { |key| key[:name] == :fk_apps_service_account_guid } + alter_table :apps do + if existing_column && !existing_foreign_key + set_column_type :service_account_guid, String, **options, size: 255 + elsif !existing_column + add_column :service_account_guid, String, **options, size: 255 + end + add_foreign_key [:service_account_guid], :service_accounts, key: :guid, name: :fk_apps_service_account_guid unless existing_foreign_key + end + VCAP::Migration.with_concurrent_timeout(self) do + if database_type == :postgres + index_valid = fetch("SELECT indisvalid FROM pg_index WHERE indexrelid = to_regclass('apps_service_account_guid_index')").get(:indisvalid) + drop_index :apps, :service_account_guid, name: :apps_service_account_guid_index, concurrently: true if index_valid == false + add_index :apps, :service_account_guid, name: :apps_service_account_guid_index, concurrently: true unless index_valid + elsif !indexes(:apps).key?(:apps_service_account_guid_index) + add_index :apps, :service_account_guid, name: :apps_service_account_guid_index, concurrently: false + end + end + end + + down do + alter_table(:apps) { drop_foreign_key [:service_account_guid], name: :fk_apps_service_account_guid } + VCAP::Migration.with_concurrent_timeout(self) do + drop_index :apps, :service_account_guid, name: :apps_service_account_guid_index, concurrently: database_type == :postgres + end + alter_table :apps do + drop_column :service_account_guid + end + end +end diff --git a/db/migrations/20261005120200_add_service_account_metadata.rb b/db/migrations/20261005120200_add_service_account_metadata.rb new file mode 100644 index 00000000000..31f06b1f531 --- /dev/null +++ b/db/migrations/20261005120200_add_service_account_metadata.rb @@ -0,0 +1,16 @@ +Sequel.migration do + change do + alter_table(:service_accounts) do + add_column :description, String, size: 250, null: false, default: '' + end + create_table(:service_account_labels) do + VCAP::Migration.common(self) + VCAP::Migration.labels_common(self, :service_account_labels, :service_accounts) + end + create_table(:service_account_annotations) do + VCAP::Migration.common(self) + VCAP::Migration.annotations_common(self, :service_account_annotations, :service_accounts) + end + rename_column :service_account_annotations, :key, :key_name + end +end diff --git a/db/migrations/20261005120300_add_task_service_account_snapshot.rb b/db/migrations/20261005120300_add_task_service_account_snapshot.rb new file mode 100644 index 00000000000..cdcdf66ba30 --- /dev/null +++ b/db/migrations/20261005120300_add_task_service_account_snapshot.rb @@ -0,0 +1,8 @@ +Sequel.migration do + change do + alter_table(:tasks) do + add_column :service_account_guid, String, size: 255 + add_column :service_account_snapshot, TrueClass, null: false, default: false + end + end +end diff --git a/db/migrations/20261005120400_add_process_service_account_snapshot.rb b/db/migrations/20261005120400_add_process_service_account_snapshot.rb new file mode 100644 index 00000000000..65f3a6c124d --- /dev/null +++ b/db/migrations/20261005120400_add_process_service_account_snapshot.rb @@ -0,0 +1,8 @@ +Sequel.migration do + change do + alter_table(:processes) do + add_column :service_account_guid, String, size: 255 + add_column :service_account_snapshot, TrueClass, null: false, default: false + end + end +end diff --git a/db/migrations/20261007120000_create_service_account_creations.rb b/db/migrations/20261007120000_create_service_account_creations.rb new file mode 100644 index 00000000000..5367cc8a40d --- /dev/null +++ b/db/migrations/20261007120000_create_service_account_creations.rb @@ -0,0 +1,10 @@ +Sequel.migration do + change do + create_table :service_account_creations do + primary_key :id, name: :service_account_creations_pkey + String :principal_guid, size: 255, null: false + DateTime :created_at, null: false + index %i[principal_guid created_at], name: :service_account_creations_principal_time + end + end +end diff --git a/db/migrations/20261109120000_add_organization_service_account_prefixes.rb b/db/migrations/20261109120000_add_organization_service_account_prefixes.rb new file mode 100644 index 00000000000..23caadd9026 --- /dev/null +++ b/db/migrations/20261109120000_add_organization_service_account_prefixes.rb @@ -0,0 +1,16 @@ +Sequel.migration do + change do + # Reservations deliberately outlive their organizations. + create_table :service_account_prefixes do + primary_key :id, name: :service_account_prefixes_pkey + String :prefix, size: 63, null: false + index :prefix, unique: true, name: :service_account_prefixes_prefix_unique + end + + alter_table :organizations do + add_column :service_account_prefix, String, size: 63 + # The new column is null for all existing organizations. + add_index :service_account_prefix, unique: true, name: :organizations_service_account_prefix_unique, concurrently: false + end + end +end diff --git a/db/migrations/20261109121000_create_service_account_space_entitlements.rb b/db/migrations/20261109121000_create_service_account_space_entitlements.rb new file mode 100644 index 00000000000..febbaa5f9ef --- /dev/null +++ b/db/migrations/20261109121000_create_service_account_space_entitlements.rb @@ -0,0 +1,25 @@ +Sequel.migration do + up do + account_guid_options = {} + space_guid_options = {} + if database_type == :mysql + account_column = fetch("SHOW FULL COLUMNS FROM service_accounts LIKE 'guid'").first + space_column = fetch("SHOW FULL COLUMNS FROM spaces LIKE 'guid'").first + account_guid_options[:collate] = account_column[:Collation] || account_column[:collation] + space_guid_options[:collate] = space_column[:Collation] || space_column[:collation] + end + + create_table :service_account_space_entitlements do + String :service_account_guid, **account_guid_options, size: 255, null: false + String :space_guid, **space_guid_options, size: 255, null: false + primary_key %i[service_account_guid space_guid], name: :service_account_space_entitlements_pkey + index %i[space_guid service_account_guid], name: :service_account_entitlements_space_account_index, concurrently: false + foreign_key [:service_account_guid], :service_accounts, key: :guid, name: :fk_sa_entitlements_account, on_delete: :cascade + foreign_key [:space_guid], :spaces, key: :guid, name: :fk_sa_entitlements_space, on_delete: :cascade + end + end + + down do + drop_table :service_account_space_entitlements + end +end diff --git a/db/migrations/20261109124000_add_org_service_account_quota.rb b/db/migrations/20261109124000_add_org_service_account_quota.rb new file mode 100644 index 00000000000..5b826aab8cb --- /dev/null +++ b/db/migrations/20261109124000_add_org_service_account_quota.rb @@ -0,0 +1,5 @@ +Sequel.migration do + change do + alter_table(:quota_definitions) { add_column :total_service_accounts, Integer, null: false, default: -1 } + end +end diff --git a/db/migrations/20261109125000_create_service_account_deletion_history.rb b/db/migrations/20261109125000_create_service_account_deletion_history.rb new file mode 100644 index 00000000000..9567faf72d8 --- /dev/null +++ b/db/migrations/20261109125000_create_service_account_deletion_history.rb @@ -0,0 +1,14 @@ +Sequel.migration do + change do + create_table :service_account_name_deletions do + primary_key :id, name: :service_account_name_deletions_pkey + String :account_guid, size: 255, null: false + String :organization_guid, size: 255, null: false + String :name, size: 63, null: false + String :identity_prefix, size: 63, null: false + DateTime :deleted_at, null: false + index :account_guid, unique: true, name: :service_account_deletions_account_unique + index :deleted_at, name: :service_account_deletions_time_index + end + end +end diff --git a/db/migrations/20261109131000_snapshot_service_binding_account.rb b/db/migrations/20261109131000_snapshot_service_binding_account.rb new file mode 100644 index 00000000000..7098fe61ac1 --- /dev/null +++ b/db/migrations/20261109131000_snapshot_service_binding_account.rb @@ -0,0 +1,5 @@ +Sequel.migration do + change do + alter_table(:service_bindings) { add_column :service_account_guid, String, size: 255 } + end +end diff --git a/db/migrations/20261112120000_add_service_account_reference_indexes.rb b/db/migrations/20261112120000_add_service_account_reference_indexes.rb new file mode 100644 index 00000000000..9f5c2d186d0 --- /dev/null +++ b/db/migrations/20261112120000_add_service_account_reference_indexes.rb @@ -0,0 +1,37 @@ +Sequel.migration do + no_transaction + + up do + { + service_bindings: [:service_bindings_service_account_guid_index, [:service_account_guid]], + processes: [:processes_service_account_guid_state_index, %i[service_account_guid state]], + tasks: [:tasks_service_account_guid_state_index, %i[service_account_guid state]] + }.each do |table, (name, columns)| + VCAP::Migration.with_concurrent_timeout(self) do + if database_type == :postgres + index_valid = fetch('SELECT indisvalid FROM pg_index WHERE indexrelid = to_regclass(?)', name.to_s).get(:indisvalid) + drop_index table, columns, name: name, concurrently: true if index_valid == false + add_index table, columns, name: name, concurrently: true unless index_valid + elsif !indexes(table).key?(name) + add_index table, columns, name: name, concurrently: false + end + end + end + end + + down do + { + service_bindings: :service_bindings_service_account_guid_index, + processes: :processes_service_account_guid_state_index, + tasks: :tasks_service_account_guid_state_index + }.each do |table, name| + VCAP::Migration.with_concurrent_timeout(self) do + if database_type == :postgres + drop_index table, nil, name: name, concurrently: true, if_exists: true + elsif indexes(table).key?(name) + drop_index table, nil, name: name, concurrently: false + end + end + end + end +end diff --git a/docs/v3/source/includes/api_resources/_organizations.erb b/docs/v3/source/includes/api_resources/_organizations.erb index e2664fb7a4d..6105b4be137 100644 --- a/docs/v3/source/includes/api_resources/_organizations.erb +++ b/docs/v3/source/includes/api_resources/_organizations.erb @@ -4,6 +4,7 @@ "created_at": "2017-02-01T01:33:58Z", "updated_at": "2017-02-01T01:33:58Z", "name": "my-organization", + "service_account_prefix": null, "suspended": false, "relationships": { "quota": { @@ -53,6 +54,7 @@ "created_at": "2017-02-01T01:33:58Z", "updated_at": "2017-02-01T01:33:58Z", "name": "org1", + "service_account_prefix": null, "suspended": false, "relationships": { "quota": { @@ -85,6 +87,7 @@ "created_at": "2017-02-02T00:14:30Z", "updated_at": "2017-02-02T00:14:30Z", "name": "org2", + "service_account_prefix": null, "suspended": false, "relationships": { "quota": { @@ -136,6 +139,7 @@ "created_at": "2017-02-01T01:33:58Z", "updated_at": "2017-02-01T01:33:58Z", "name": "org1", + "service_account_prefix": null, "suspended": false, "links": { "self": { @@ -168,6 +172,7 @@ "created_at": "2017-02-02T00:14:30Z", "updated_at": "2017-02-02T00:14:30Z", "name": "org2", + "service_account_prefix": null, "suspended": false, "relationships": { "quota": { diff --git a/docs/v3/source/includes/api_resources/_service_accounts.md b/docs/v3/source/includes/api_resources/_service_accounts.md new file mode 100644 index 00000000000..dc722e3c806 --- /dev/null +++ b/docs/v3/source/includes/api_resources/_service_accounts.md @@ -0,0 +1,146 @@ +# Service accounts (experimental) + +Service accounts belong to an immutable owning organization. Apps in explicitly +entitled spaces of that org can share an account; each app has zero or one desired +account. Assignment does not grant Cloud Controller roles, cloud grants or route +permissions. + +## Names and identity + +An Admin allocates `service_account_prefix` on the V3 organization at creation or +once later. The prefix is foundation-unique, independent of the display name, +immutable, and reserved after org deletion. No prefix is generated automatically. +Prefixes and account names are lowercase RFC 1123 labels, 3–63 characters. + +For prefix `acme-payments` and name `payments-worker`: + +| Identifier | Value | +| --- | --- | +| OAuth client/subject | `cf:service-account:acme-payments:payments-worker` | +| Certificate DNS SAN | `payments-worker.acme-payments.svc.identity` | +| Route source | `cf:svc:acme-payments:payments-worker` | + +Names are unique within the org, including permanent deletion reservations. +Deleting records the deletion timestamp separately from account creation/audit +retention. Historical dates remain unknown. Org Managers/Admins may explicitly +create with `reuse_name: true`: fresh UUID and initial entitlement, no restored +bindings, roles or grants. Live collisions and quotas still apply. The reused +subject/SAN may inherit external grants or accept still-valid old certificates. + +## APIs and permissions + +Creation uses the target space to choose the org and grant the initial entitlement: + +```json +{ + "name": "payments-worker", + "relationships": { "space": { "data": { "guid": "SPACE_GUID" } } } +} +``` + +Optional fields: `description`, `metadata`, `reuse_name`. Identity/state and owning +org are platform controlled. Account representations expose `organization` and +`spaces` relationships. + +| Method | Path | Behavior | +| --- | --- | --- | +| POST | `/v3/service_accounts` | Space Developer/Admin creates a reserved account; requires org prefix | +| GET | `/v3/service_accounts` | Entitled-space readers and Org Managers; `names`, `space_guids`, `organization_guids` filters | +| GET | `/v3/service_accounts/:guid` | Read account; Org Managers also see zero-entitlement accounts | +| GET | `/v3/service_accounts/:guid/apps` | Read assigned apps visible to caller | +| PATCH | `/v3/service_accounts/:guid` | Metadata or async enable/disable | +| DELETE | `/v3/service_accounts/:guid` | Async unused-account cleanup | +| GET | `/v3/service_accounts/:guid/relationships/spaces` | List entitled space GUIDs | +| POST | `/v3/service_accounts/:guid/relationships/spaces` | Add `{ "data": [{ "guid": "SPACE_GUID" }] }` | +| DELETE | `/v3/service_accounts/:guid/relationships/spaces/:space_guid` | Revoke entitlement | +| GET/PATCH | `/v3/apps/:app_guid/relationships/service_account` | Read/assign `{ "data": { "guid": "ACCOUNT_GUID" } }`, clear with `null` | + +Lifecycle management: Org Manager/Admin or Developer in an entitled active space. +Entitlement addition: Org Manager/Admin, or Developer of both an entitled source +space and destination. Cross-org entitlements are rejected. Developers may revoke +their space unless it is the last entitlement; Org Managers/Admins can revoke the +last. Desired assignments, active process/task snapshots and identity-bearing +service bindings block revocation. App assignment requires existing app-write +permission, same-org entitlement and allocated org prefix. + +Org quota `service_accounts.total_service_accounts` is a nonnegative integer or +`null` for unlimited. It counts existing accounts including disabled/reserved; +bindings/deleted names do not consume capacity. Creation checks serialize by org. +The independent optional rolling seven-day creation budget spans the foundation +per authenticated principal; Admins are exempt. Failures do not consume budget, +and deletion does not refund it. `service_account_creation_limit: -1` is unlimited. + +## Provisioning, attribution and lifecycle + +First bind queues UAA provisioning and returns 202 with a pollable job; ready +account binds return 200. Wait before launching; bind/unbind never restarts apps. +Instances retain launch identity through renewal; tasks capture desired assignment; +staging has no account identity. Independent instance credentials remain intact. + +The managed client is secretless, client_credentials-only, bearer policy explicitly +false for certificate binding, TTL 300, named CA bundle reference, exact SAN and +required owning-org claim. CN/OU mappings provide instance/app/space/org attribution. +Binding does not create a CAPI user. CAPI users and their explicit resource roles +are independently managed; the existing CAPI authentication flow can lazily create +a roleless OAuth user on its first API request. OAuth authorities alone do not +grant resource access. Any registration difference is a collision, not adopted/reconciled. + +Disable removes the managed registration asynchronously, stopping new token +issuance when effective. Existing JWTs, certificate routes and external sessions +remain valid until their own expiry/revocation. Enable recreates the registration +and retains resource roles. Delete requires no desired/active/binding references. +Space deletion removes workloads/entitlement but retains the org account and +authentication. Org deletion cleans workloads, managed UAA clients and accounts +retryably; failed cleanup retains a deleting org. Service-account deletion does +not delete independently managed CAPI users or their roles; ordinary space/org +deletion still removes roles on the deleted resources. Names/prefixes persist. + +This intentionally diverges from RFC Appendix D's automatic roleless-principal +creation on first bind: service-account provisioning and retirement manage UAA +clients, not CAPI user lifecycles. Existing users/roles can therefore apply to the +same client ID after explicit identity reuse; remove those grants independently. + +## Broker and routes + +App service bindings capture the assigned account; ready/entitled identity is +sent as platform `context.service_account {issuer,subject}`. Existing bindings +without an account snapshot and service keys carry no account identity. Remove +identity-bearing bindings before reassigning. Broker implementations own shared +grant lifetime/reference counting and cleanup across entitled spaces. + +Qualified account route rules resolve to UUID references, are readable-source +authorized, and are removed on account deletion. Domain org/space restrictions +remain caller restrictions enforced by routing. The source relationship and +`include=source` return readable account data. Disabling tokens does not revoke +certificate-only route access. + +## Rollout, migration and diagnostics + +BOSH `cc.service_accounts` properties: + +- `enabled`: default false; renders `service_accounts_enabled` for API, worker, + clock and deployment updater. Enable only after compatible UAA/BBS/Diego and + routing components are deployed. There is no numeric contract-version setting. +- `token_endpoint`: HTTPS UAA mTLS endpoint, no credentials/query/fragment. +- Worker-only `management_client_id`, `management_client_secret`, `identity_ca_ref`. + Inline CA certificates are unsupported. Rotate named bundle contents in UAA. +- Worker-only optional `allowed_resources_ref`: requires UAA named-resource-policy + support (#4125). Leave unset until supported; CAPI does not copy audience lists. + +Accounts use only prefix-qualified identities and org ownership. The schema does +not include migration paths for earlier unpublished proposal models. Deletion +timestamps are retained for future operational queries; no statistics collection +or automatic reservation release is implemented. + +Remaining external acceptance: qualified Diego/router rollout, UAA resource-policy +support, min(300s, remaining certificate lifetime) issuance, management namespace +confinement, provider audience interoperability and live upgrade/renewal/rotation. +Operator contract gates are explicit assertions, not automatic cell negotiation. + +Ship qualified account-source support in routing-release before releasing the +CAPI producer changes, and pin/deploy that routing version first. Existing routers +skip unknown source types and deny when no recognized source matches; existing +recognized rules retain their OR semantics. No extra router-version setting is +required. cf-deployment's global `update.serial: false` permits parallel instance +group updates and does not guarantee router-before-CAPI completion within one +deployment. Verify the routing-first release/deployment sequence explicitly. diff --git a/docs/v3/source/includes/resources/organizations/_create.md.erb b/docs/v3/source/includes/resources/organizations/_create.md.erb index ecfd933f6b3..6ebff90fcab 100644 --- a/docs/v3/source/includes/resources/organizations/_create.md.erb +++ b/docs/v3/source/includes/resources/organizations/_create.md.erb @@ -38,6 +38,7 @@ Name | Type | Description Name | Type | Description ---- | ---- | ----------- **suspended** | _boolean_ | Whether an organization is suspended or not +**service_account_prefix** | _string_ or _null_ | Admin only; reserve a foundation-unique service-account namespace. Must be a lowercase RFC 1123 label of 3–63 characters, beginning and ending with a letter or digit. Defaults to `null`; no prefix is generated. Once allocated, the prefix cannot be renamed, cleared or reassigned, and remains reserved after org deletion. **metadata.labels** | [_label object_](#labels) | Labels applied to the organization **metadata.annotations** | [_annotation object_](#annotations) | Annotations applied to the organization diff --git a/docs/v3/source/includes/resources/organizations/_object.md.erb b/docs/v3/source/includes/resources/organizations/_object.md.erb index 45defad851c..e54406763f6 100644 --- a/docs/v3/source/includes/resources/organizations/_object.md.erb +++ b/docs/v3/source/includes/resources/organizations/_object.md.erb @@ -13,6 +13,7 @@ Name | Type | Description **created_at** | _[timestamp](#timestamps)_ | The time with zone when the object was created **updated_at** | _[timestamp](#timestamps)_ | The time with zone when the object was last updated **name** | _string_ | Name of the organization +**service_account_prefix** | _string_ or _null_ | Immutable service-account namespace, unique within the foundation and allocated by an Admin. Independent of the organization name and GUID; `null` until allocated. Reservations survive organization deletion. **suspended** | _boolean_ | Whether an organization is suspended or not; non-admins will be blocked from creating, updating, or deleting resources in a suspended organization **relationships.quota** | [_to-one relationship_](#to-one-relationships) | The quota applied to the organization **metadata.labels** | [_label object_](#labels) | Labels applied to the organization diff --git a/docs/v3/source/includes/resources/organizations/_update.md.erb b/docs/v3/source/includes/resources/organizations/_update.md.erb index 9e6f08cf059..acdfe3b95f3 100644 --- a/docs/v3/source/includes/resources/organizations/_update.md.erb +++ b/docs/v3/source/includes/resources/organizations/_update.md.erb @@ -33,6 +33,7 @@ Name | Type | Description ---- | ---- | ----------- **name** | _string_ | Organization name | **suspended** | _boolean_ | Whether an organization is suspended or not +**service_account_prefix** | _string_ or _null_ | Admin only; allocate a prefix when currently unset. Must be an available lowercase RFC 1123 label of 3–63 characters with alphanumeric boundaries. Repeating the current value succeeds; changing or clearing an allocated prefix fails. Org rename does not change it; org deletion does not release it. **metadata.labels** | [_label object_](#labels) | Labels applied to the organization **metadata.annotations** | [_annotation object_](#annotations) | Annotations applied to the organization @@ -41,4 +42,4 @@ Name | Type | Description Role | Notes ---- | ----- Admin | -Org Manager | Cannot change the `suspended` field +Org Manager | Cannot change the `suspended` field or supply `service_account_prefix` diff --git a/docs/v3/source/index.html.md b/docs/v3/source/index.html.md index efb673f35ec..9af499384d7 100644 --- a/docs/v3/source/index.html.md +++ b/docs/v3/source/index.html.md @@ -29,6 +29,7 @@ includes: - api_resources/routes - api_resources/security_groups - api_resources/service_brokers + - api_resources/service_accounts - api_resources/service_offerings - api_resources/service_plans - api_resources/service_plan_visibility diff --git a/errors/v3.yml b/errors/v3.yml index 35a7cc6dd35..703a21d3b55 100644 --- a/errors/v3.yml +++ b/errors/v3.yml @@ -3,6 +3,21 @@ http_code: 422 message: "%s" +10017: + name: ServiceAccountNameReserved + http_code: 409 + message: "Service account name is already reserved" + +10018: + name: ServiceAccountAssignmentConflict + http_code: 409 + message: "%s" + +10019: + name: ServiceAccountCreationLimitExceeded + http_code: 429 + message: "Service account creation budget exhausted for this principal; retry when creations leave the rolling seven-day window" + 270010: name: ServiceBrokerNotRemovable http_code: 422 diff --git a/lib/cloud_controller/backends/staging_environment_builder.rb b/lib/cloud_controller/backends/staging_environment_builder.rb index 9395c5d3c5c..e22c393d790 100644 --- a/lib/cloud_controller/backends/staging_environment_builder.rb +++ b/lib/cloud_controller/backends/staging_environment_builder.rb @@ -27,7 +27,8 @@ def build(app, space, lifecycle, memory_limit, staging_disk_in_mb, vars_from_mes 'MEMORY_LIMIT' => "#{memory_limit}m" } ). - merge(SystemEnvPresenter.new(app).system_env.stringify_keys) + merge(SystemEnvPresenter.new(app).system_env.stringify_keys). + except('VCAP_SERVICE_ACCOUNT', :VCAP_SERVICE_ACCOUNT) end end end diff --git a/lib/cloud_controller/config_schemas/api_schema.rb b/lib/cloud_controller/config_schemas/api_schema.rb index acb517879da..e2af80fc37e 100644 --- a/lib/cloud_controller/config_schemas/api_schema.rb +++ b/lib/cloud_controller/config_schemas/api_schema.rb @@ -30,6 +30,14 @@ class ApiSchema < VCAP::Config optional(:custom) => Hash }, optional(:custom_root_links) => Array, + optional(:service_accounts_enabled) => bool, + optional(:service_account_creation_limit) => Integer, + optional(:service_account_token_endpoint) => String, + optional(:service_account_provisioning) => { + client_id: String, + client_secret: String, + identity_ca_ref: String + }, system_domain: String, optional(:system_domain_organization) => enum(String, NilClass), diff --git a/lib/cloud_controller/config_schemas/clock_schema.rb b/lib/cloud_controller/config_schemas/clock_schema.rb index 79c8de01bee..a0205ff2c6f 100644 --- a/lib/cloud_controller/config_schemas/clock_schema.rb +++ b/lib/cloud_controller/config_schemas/clock_schema.rb @@ -6,6 +6,8 @@ class ClockSchema < VCAP::Config # rubocop:disable Metrics/BlockLength define_schema do { + optional(:service_accounts_enabled) => bool, + optional(:service_account_token_endpoint) => String, external_port: Integer, external_domain: String, tls_port: Integer, diff --git a/lib/cloud_controller/config_schemas/deployment_updater_schema.rb b/lib/cloud_controller/config_schemas/deployment_updater_schema.rb index 11814cea8f0..1ac89aa044a 100644 --- a/lib/cloud_controller/config_schemas/deployment_updater_schema.rb +++ b/lib/cloud_controller/config_schemas/deployment_updater_schema.rb @@ -6,6 +6,8 @@ class DeploymentUpdaterSchema < VCAP::Config # rubocop:disable Metrics/BlockLength define_schema do { + optional(:service_accounts_enabled) => bool, + optional(:service_account_token_endpoint) => String, logging: { level: String, # debug, info, etc. diff --git a/lib/cloud_controller/config_schemas/worker_schema.rb b/lib/cloud_controller/config_schemas/worker_schema.rb index c60807a5f75..8a47b2960e3 100644 --- a/lib/cloud_controller/config_schemas/worker_schema.rb +++ b/lib/cloud_controller/config_schemas/worker_schema.rb @@ -29,6 +29,15 @@ class WorkerSchema < VCAP::Config client_timeout: Integer }, + optional(:service_account_provisioning) => { + client_id: String, + client_secret: String, + identity_ca_ref: String, + optional(:allowed_resources_ref) => String + }, + optional(:service_accounts_enabled) => bool, + optional(:service_account_token_endpoint) => String, + logging: { level: String, # debug, info, etc. file: String # Log file to use diff --git a/lib/cloud_controller/dependency_locator.rb b/lib/cloud_controller/dependency_locator.rb index 1f1ebf5aaaf..06f21b7e14f 100644 --- a/lib/cloud_controller/dependency_locator.rb +++ b/lib/cloud_controller/dependency_locator.rb @@ -26,6 +26,9 @@ require 'cloud_controller/metrics/prometheus_updater' require 'statsd/instrument' require 'cloud_controller/execution_context' +require 'cloud_controller/uaa/service_account_client' +require 'cloud_controller/uaa/issuer_discovery' +require 'actions/service_account_provision' module CloudController class DependencyLocator @@ -305,6 +308,30 @@ def uaa_username_lookup_client ) end + def service_account_provisioner + return @dependencies[:service_account_provisioner] if @dependencies[:service_account_provisioner] + + settings = config.get(:service_account_provisioning) + configured = ->(key) { settings[key].is_a?(String) && settings[key].present? } + # Inline CA certificates are not supported: clients reference a UAA trust bundle. + unless settings.is_a?(Hash) && %i[client_id client_secret identity_ca_ref].all?(&configured) && !settings.key?(:identity_ca) + raise 'service account provisioning is not configured' + end + + clients = ServiceAccountClient.new( + uaa_target: config.get(:uaa, :internal_url), + client_id: settings[:client_id], + secret: settings[:client_secret], + ca_file: config.get(:uaa, :ca_file) + ) + register(:service_account_provisioner, + ServiceAccountProvision.new(clients, identity_ca_ref: settings[:identity_ca_ref], allowed_resources_ref: settings[:allowed_resources_ref])) + end + + def uaa_issuer_discovery + @dependencies[:uaa_issuer_discovery] ||= IssuerDiscovery.new(config.get(:uaa)) + end + def uaa_shadow_user_creation_client client = config.get(:uaa, :clients)&.find { |client_config| client_config['name'] == 'cloud_controller_shadow_user_creation' } diff --git a/lib/cloud_controller/diego/app_recipe_builder.rb b/lib/cloud_controller/diego/app_recipe_builder.rb index cedf097dd06..1a628165124 100644 --- a/lib/cloud_controller/diego/app_recipe_builder.rb +++ b/lib/cloud_controller/diego/app_recipe_builder.rb @@ -11,6 +11,7 @@ require 'credhub/config_helpers' require 'models/helpers/health_check_types' require 'cloud_controller/diego/main_lrp_action_builder' +require 'cloud_controller/diego/service_account_identity' module VCAP::CloudController module Diego @@ -99,9 +100,7 @@ def app_lrp_arguments check_definition: generate_healthcheck_definition(desired_lrp_builder), routes: ::Diego::Bbs::Models::ProtoRoutes.new(routes:), max_pids: @config.get(:diego, :pid_limit), - certificate_properties: ::Diego::Bbs::Models::CertificateProperties.new( - organizational_unit: ["organization:#{process.organization.guid}", "space:#{process.space.guid}", "app:#{process.app_guid}"] - ), + certificate_properties: ServiceAccountIdentity.new(process.app, config, account_guid: process.runtime_service_account_guid).certificate_properties, **lrp_image_credentials, volume_mounted_files: ServiceBindingFilesBuilder.build(process) }.compact diff --git a/lib/cloud_controller/diego/environment.rb b/lib/cloud_controller/diego/environment.rb index d5789782432..d30ff19a5af 100644 --- a/lib/cloud_controller/diego/environment.rb +++ b/lib/cloud_controller/diego/environment.rb @@ -1,6 +1,7 @@ require 'presenters/system_environment/system_env_presenter' require 'cloud_controller/diego/normal_env_hash_to_diego_env_array_philosopher' require_relative '../../vcap/vars_builder' +require 'cloud_controller/diego/service_account_identity' module VCAP::CloudController module Diego @@ -43,6 +44,9 @@ def common_json_and_merge(&blk) merge(blk.call). merge(SystemEnvPresenter.new(process).system_env) + diego_env = diego_env.except('VCAP_SERVICE_ACCOUNT', :VCAP_SERVICE_ACCOUNT). + merge(ServiceAccountIdentity.new(process.app, Config.config, account_guid: process.runtime_service_account_guid).environment) + diego_env = diego_env.merge(DATABASE_URL: process.database_uri) if process.database_uri NormalEnvHashToDiegoEnvArrayPhilosopher.muse(diego_env) diff --git a/lib/cloud_controller/diego/processes_sync.rb b/lib/cloud_controller/diego/processes_sync.rb index eadc8d20b00..8d6847f539d 100644 --- a/lib/cloud_controller/diego/processes_sync.rb +++ b/lib/cloud_controller/diego/processes_sync.rb @@ -1,4 +1,5 @@ require 'utils/workpool' +require 'cloud_controller/diego/service_account_identity' module VCAP::CloudController module Diego @@ -69,7 +70,7 @@ def process_workpool_exceptions(exceptions) invalid_lrps = 0 exceptions.each do |e| error_name = e.is_a?(CloudController::Errors::ApiError) ? e.name : e.class.name - if error_name == 'RunnerInvalidRequest' + if error_name == 'RunnerInvalidRequest' || e.is_a?(ServiceAccountIdentity::NotReady) logger.info('synced-invalid-desired-lrps', error: error_name, error_message: e.message) invalid_lrps += 1 elsif error_name == 'RunnerError' && e.message['the requested resource already exists'] diff --git a/lib/cloud_controller/diego/service_account_identity.rb b/lib/cloud_controller/diego/service_account_identity.rb new file mode 100644 index 00000000000..afabff5eda8 --- /dev/null +++ b/lib/cloud_controller/diego/service_account_identity.rb @@ -0,0 +1,61 @@ +require 'uri' + +module VCAP::CloudController + module Diego + class ServiceAccountIdentity + class NotReady < CloudController::Errors::ApiError; end + + def initialize(app, config, account_guid: app.service_account_guid) + @app = app + @config = config + @account_guid = account_guid + end + + def certificate_properties + account = ready_account + attributes = { organizational_unit: ["organization:#{@app.organization_guid}", "space:#{@app.space_guid}", "app:#{@app.guid}"] } + attributes[:service_account] = ::Diego::Bbs::Models::ServiceAccount.new(name: account.name, prefix: account.identity_prefix) if account + ::Diego::Bbs::Models::CertificateProperties.new(attributes) + end + + def environment + account = ready_account + return {} unless account + + endpoint = @config.get(:service_account_token_endpoint) + raise CloudController::Errors::ApiError.new_from_details('UnprocessableEntity', 'Service account token endpoint is not configured') unless valid_endpoint?(endpoint) + + { 'VCAP_SERVICE_ACCOUNT' => { + guid: account.guid, name: account.name, client_id: account.client_id, + certificate_dns_san: account.certificate_dns_san, token_endpoint: endpoint + } } + end + + private + + def valid_endpoint?(endpoint) + return false unless endpoint.is_a?(String) + + uri = URI.parse(endpoint) + uri.is_a?(URI::HTTPS) && uri.host.present? && uri.userinfo.nil? && uri.query.nil? && uri.fragment.nil? + rescue URI::InvalidURIError + false + end + + def ready_account + return unless @account_guid + + unless @config.get(:service_accounts_enabled) == true + raise CloudController::Errors::ApiError.new_from_details('UnprocessableEntity', 'Service account runtime is not enabled') + end + + account = ServiceAccountModel.first(guid: @account_guid) + unless account && account.enabled && account.status == 'ready' && account.entitled_to?(@app.space) + raise NotReady.new_from_details('UnprocessableEntity', 'Service account is not ready for runtime credentials') + end + + account + end + end + end +end diff --git a/lib/cloud_controller/diego/task_environment.rb b/lib/cloud_controller/diego/task_environment.rb index dad26816613..129f7ec0055 100644 --- a/lib/cloud_controller/diego/task_environment.rb +++ b/lib/cloud_controller/diego/task_environment.rb @@ -1,4 +1,5 @@ require 'credhub/config_helpers' +require 'cloud_controller/diego/service_account_identity' module VCAP::CloudController module Diego @@ -21,6 +22,9 @@ def build merge('VCAP_APPLICATION' => vcap_application, 'MEMORY_LIMIT' => "#{task.memory_in_mb}m"). merge(SystemEnvPresenter.new(app).system_env.stringify_keys) + task_env = task_env.except('VCAP_SERVICE_ACCOUNT', :VCAP_SERVICE_ACCOUNT). + merge(ServiceAccountIdentity.new(app, Config.config, account_guid: task.runtime_service_account_guid).environment) + task_env = task_env.merge('VCAP_PLATFORM_OPTIONS' => credhub_url) if credhub_url.present? && cred_interpolation_enabled? task_env = task_env.merge('LANG' => DEFAULT_LANG) if [BuildpackLifecycleDataModel::LIFECYCLE_TYPE, CNBLifecycleDataModel::LIFECYCLE_TYPE].include?(app.lifecycle_type) diff --git a/lib/cloud_controller/diego/task_recipe_builder.rb b/lib/cloud_controller/diego/task_recipe_builder.rb index 4e55e677a09..785b9d22aab 100644 --- a/lib/cloud_controller/diego/task_recipe_builder.rb +++ b/lib/cloud_controller/diego/task_recipe_builder.rb @@ -7,6 +7,7 @@ require 'cloud_controller/diego/task_completion_callback_generator' require 'cloud_controller/diego/task_cpu_weight_calculator' require 'cloud_controller/diego/service_binding_files_builder' +require 'cloud_controller/diego/service_account_identity' module VCAP::CloudController module Diego @@ -46,13 +47,7 @@ def build_app_task(config, task) root_fs: task_action_builder.stack, environment_variables: task_action_builder.task_environment_variables, placement_tags: [VCAP::CloudController::IsolationSegmentSelector.for_space(task.space)].compact, - certificate_properties: ::Diego::Bbs::Models::CertificateProperties.new( - organizational_unit: [ - "organization:#{task.app.organization.guid}", - "space:#{task.app.space_guid}", - "app:#{task.app_guid}" - ] - ), + certificate_properties: ServiceAccountIdentity.new(task.app, config, account_guid: task.runtime_service_account_guid).certificate_properties, **task_image_credentials(task), volume_mounted_files: ServiceBindingFilesBuilder.build(task.app) }.compact) diff --git a/lib/cloud_controller/uaa/issuer_discovery.rb b/lib/cloud_controller/uaa/issuer_discovery.rb new file mode 100644 index 00000000000..d15a2786ce8 --- /dev/null +++ b/lib/cloud_controller/uaa/issuer_discovery.rb @@ -0,0 +1,31 @@ +require 'httpclient' + +module VCAP::CloudController + class IssuerDiscovery + CACHE_TTL = 300 + + def initialize(config) + @config = config + @lock = Mutex.new + end + + def issuer + @lock.synchronize do + return @issuer if @expires_at && Time.now < @expires_at + + client = HTTPClient.new(base_url: @config[:internal_url]) + client.ssl_config.verify_mode = OpenSSL::SSL::VERIFY_PEER + client.ssl_config.set_trust_ca(@config[:ca_file]) if @config[:ca_file].present? + response = client.get('.well-known/openid-configuration') + raise "Could not retrieve issuer information from UAA: #{response.status}" unless response.status == 200 + + value = Oj.strict_load(response.body).fetch('issuer') + raise 'UAA issuer discovery returned an empty issuer' unless value.is_a?(String) && value.present? + + @issuer = value + @expires_at = Time.now + CACHE_TTL + @issuer + end + end + end +end diff --git a/lib/cloud_controller/uaa/service_account_client.rb b/lib/cloud_controller/uaa/service_account_client.rb new file mode 100644 index 00000000000..81646f6cc0a --- /dev/null +++ b/lib/cloud_controller/uaa/service_account_client.rb @@ -0,0 +1,23 @@ +require 'cloud_controller/uaa/uaa_client' + +module VCAP::CloudController + class ServiceAccountClient < UaaClient + def get(type, id) + raise ArgumentError.new('only client resources are supported') unless type == :client + + super + end + + def add(type, registration) + raise ArgumentError.new('only client resources are supported') unless type == :client + + with_cache_retry { scim.add(type, registration) } + end + + def delete(type, id) + raise ArgumentError.new('only client resources are supported') unless type == :client + + with_cache_retry { scim.delete(type, id) } + end + end +end diff --git a/lib/diego/bbs/models/certificate_properties_pb.rb b/lib/diego/bbs/models/certificate_properties_pb.rb index 003d022b902..3a327cdd67d 100644 --- a/lib/diego/bbs/models/certificate_properties_pb.rb +++ b/lib/diego/bbs/models/certificate_properties_pb.rb @@ -5,15 +5,16 @@ require 'google/protobuf' -descriptor_data = "\n\x1c\x63\x65rtificate_properties.proto\x12\x10\x64iego.bbs.models\"4\n\x15\x43\x65rtificateProperties\x12\x1b\n\x13organizational_unit\x18\x01 \x03(\tb\x06proto3" +descriptor_data = "\n\x1c\x63\x65rtificate_properties.proto\x12\x10\x64iego.bbs.models\"o\n\x15\x43\x65rtificateProperties\x12\x1b\n\x13organizational_unit\x18\x01 \x03(\t\x12\x39\n\x0fservice_account\x18\x02 \x01(\x0b\x32 .diego.bbs.models.ServiceAccount\".\n\x0eServiceAccount\x12\x0c\n\x04name\x18\x01 \x01(\t\x12\x0e\n\x06prefix\x18\x02 \x01(\tb\x06proto3" -pool = Google::Protobuf::DescriptorPool.generated_pool +pool = ::Google::Protobuf::DescriptorPool.generated_pool pool.add_serialized_file(descriptor_data) module Diego module Bbs module Models CertificateProperties = ::Google::Protobuf::DescriptorPool.generated_pool.lookup("diego.bbs.models.CertificateProperties").msgclass + ServiceAccount = ::Google::Protobuf::DescriptorPool.generated_pool.lookup("diego.bbs.models.ServiceAccount").msgclass end end end diff --git a/lib/services/service_brokers/v2/client.rb b/lib/services/service_brokers/v2/client.rb index e80cf656810..9ad8e55fd66 100644 --- a/lib/services/service_brokers/v2/client.rb +++ b/lib/services/service_brokers/v2/client.rb @@ -118,6 +118,8 @@ def bind(binding, arbitrary_parameters: {}, accepts_incomplete: false, user_guid body = body.compact body[:parameters] = arbitrary_parameters if arbitrary_parameters.present? + add_service_account_context(body, binding) + begin response = @http_client.put(path, body, user_guid:) rescue Errors::HttpClientTimeout => e @@ -367,6 +369,20 @@ def context_hash(service_instance) } end + def add_service_account_context(body, binding) + return unless binding.respond_to?(:service_account_guid) && binding.service_account_guid + + account = VCAP::CloudController::ServiceAccountModel.first(guid: binding.service_account_guid) + raise VCAP::CloudController::ServiceAccountProvision::Conflict.new('captured service account not found') unless account + + unless account.enabled && account.status == 'ready' && account.entitled_to?(binding.app.space) + raise VCAP::CloudController::ServiceAccountProvision::Conflict.new('service account is not ready for a broker binding') + end + + issuer = CloudController::DependencyLocator.instance.uaa_issuer_discovery.issuer + body[:context][:service_account] = { issuer: issuer, subject: account.client_id } + end + def async_response?(response) response.code == 202 end diff --git a/spec/isolated_specs/processes_sync_spec.rb b/spec/isolated_specs/processes_sync_spec.rb index 2ed21bbb9bf..d50ee977552 100644 --- a/spec/isolated_specs/processes_sync_spec.rb +++ b/spec/isolated_specs/processes_sync_spec.rb @@ -31,6 +31,48 @@ module Diego expect(bbs_apps_client).to have_received(:bump_freshness).once end + context 'when a missing LRP belongs to a disabled service account' do + let(:config) { Config.new({ service_accounts_enabled: true }) } + let(:bbs_transport) { double(:bbs_transport) } + let(:bbs_apps_client) { BbsAppsClient.new(bbs_transport, config) } + let!(:blocked_process) { create(:process_model, :diego_runnable, app: create(:app_model, space: create(:space, :with_service_account_prefix))) } + let!(:healthy_process) { create(:process_model, :diego_runnable) } + + before do + TestConfig.override(service_accounts_enabled: true, service_account_token_endpoint: 'https://uaa.example.test/oauth/mtls/token') + CloudController::DependencyLocator.instance.register(:bbs_apps_client, bbs_apps_client) + account = ServiceAccountModel.create(name: 'blocked-worker', space: blocked_process.space, status: 'ready') + blocked_process.app.update(service_account: account) + blocked_process.update(service_account_guid: account.guid, service_account_snapshot: true) + account.update(enabled: false) + allow(bbs_transport).to receive_messages( + desired_lrp_scheduling_infos: ::Diego::Bbs::Models::DesiredLRPSchedulingInfosResponse.new, + desire_lrp: ::Diego::Bbs::Models::DesiredLRPResponse.new, + upsert_domain: ::Diego::Bbs::Models::UpsertDomainResponse.new + ) + allow(bbs_apps_client).to receive(:desire_app).and_call_original + allow(bbs_apps_client).to receive(:bump_freshness).and_call_original + allow(AppRecipeBuilder).to receive(:new) do |config:, process:| + recipe = double(:identity_recipe) + allow(recipe).to receive(:build_app_lrp) do + properties = ServiceAccountIdentity.new(process.app, config, account_guid: process.runtime_service_account_guid).certificate_properties + ::Diego::Bbs::Models::DesiredLRP.new(process_guid: ProcessGuid.from_process(process), certificate_properties: properties) + end + recipe + end + end + + it 'refuses its new credentials while reconciling healthy apps and refreshing the shared domain', isolation: :truncation do + subject.sync + + expect(bbs_transport).to have_received(:desire_lrp).once.with( + have_attributes(process_guid: ProcessGuid.from_process(healthy_process)) + ) + expect(bbs_transport).to have_received(:upsert_domain).with(domain: APP_LRP_DOMAIN, ttl: APP_LRP_DOMAIN_TTL) + expect(statsd_updater).to have_received(:update_synced_invalid_lrps).with(1) + end + end + context 'when bbs and CC are in sync' do let(:scheduling_infos) { [good_lrp_scheduling_info] } let(:good_lrp_scheduling_info) do diff --git a/spec/migrations/20261005120100_add_app_service_account_spec.rb b/spec/migrations/20261005120100_add_app_service_account_spec.rb new file mode 100644 index 00000000000..091084926b9 --- /dev/null +++ b/spec/migrations/20261005120100_add_app_service_account_spec.rb @@ -0,0 +1,42 @@ +require 'spec_helper' + +RSpec.describe 'app service account relationship migration recovery', isolation: :truncation, type: :migration do + it 'resumes after index creation failure and preserves an already valid index' do + db = Sequel::Model.db + load File.join(DBMigrator::SEQUEL_MIGRATIONS, '20261005120100_add_app_service_account.rb') + migration = Sequel::Migration.descendants.last + migration.apply(db, :down) + allow(db).to receive(:add_index).and_call_original + allow(db).to receive(:add_index).with(:apps, :service_account_guid, anything).and_raise(Sequel::DatabaseError, 'index creation interrupted') + + expect { migration.apply(db, :up) }.to raise_error(Sequel::DatabaseError, 'index creation interrupted') + expect(db.schema(:apps).to_h).to have_key(:service_account_guid) + expect(db.foreign_key_list(:apps)).to include(hash_including(name: :fk_apps_service_account_guid)) + allow(db).to receive(:add_index).and_call_original + + expect { migration.apply(db, :up) }.not_to raise_error + expect(db.indexes(:apps)).to have_key(:apps_service_account_guid_index) + expect(db).not_to receive(:add_index) + expect(db).not_to receive(:drop_index) + expect { migration.apply(db, :up) }.not_to raise_error + ensure + allow(db).to receive(:add_index).and_call_original + allow(db).to receive(:drop_index).and_call_original + migration.apply(db, :up) unless db.indexes(:apps).key?(:apps_service_account_guid_index) + end + + it 'rebuilds an invalid PostgreSQL concurrent index' do + db = Sequel::Model.db + skip 'PostgreSQL concurrent index validity' unless db.database_type == :postgres + + load File.join(DBMigrator::SEQUEL_MIGRATIONS, '20261005120100_add_app_service_account.rb') + migration = Sequel::Migration.descendants.last + db.run("UPDATE pg_index SET indisvalid = false WHERE indexrelid = 'apps_service_account_guid_index'::regclass") + + migration.apply(db, :up) + + expect(db.fetch("SELECT indisvalid FROM pg_index WHERE indexrelid = 'apps_service_account_guid_index'::regclass").get(:indisvalid)).to be(true) + ensure + db.run("UPDATE pg_index SET indisvalid = true WHERE indexrelid = 'apps_service_account_guid_index'::regclass") if db&.database_type == :postgres + end +end diff --git a/spec/migrations/20261112120000_add_service_account_reference_indexes_spec.rb b/spec/migrations/20261112120000_add_service_account_reference_indexes_spec.rb new file mode 100644 index 00000000000..1cfcb18cf73 --- /dev/null +++ b/spec/migrations/20261112120000_add_service_account_reference_indexes_spec.rb @@ -0,0 +1,67 @@ +require 'spec_helper' +require 'migrations/helpers/migration_shared_context' + +RSpec.describe 'service account reference indexes', isolation: :truncation, type: :migration do + include_context 'migration' do + let(:migration_filename) { '20261112120000_add_service_account_reference_indexes.rb' } + end + + it 'adds account-first indexes, supports retry after partial creation and removes them on rollback' do + expected_indexes = { + service_bindings: [:service_bindings_service_account_guid_index, [:service_account_guid]], + processes: [:processes_service_account_guid_state_index, %i[service_account_guid state]], + tasks: [:tasks_service_account_guid_state_index, %i[service_account_guid state]] + } + expected_indexes.each do |table, (name, _columns)| + expect(db.indexes(table)).not_to have_key(name) + end + + Sequel::Migrator.run(db, migrations_path, target: current_migration_index, allow_missing_migration_files: true) + expected_indexes.each do |table, (name, columns)| + expect(db.indexes(table)[name]).to include(columns: columns, unique: false) + end + + # Simulate an interrupted run with the first index already installed. + load File.join(migrations_path, migration_filename) + migration = Sequel::Migration.descendants.last + %i[processes tasks].each do |table| + name, columns = expected_indexes.fetch(table) + db.drop_index(table, columns, name: name, concurrently: db.database_type == :postgres) + end + expect { migration.apply(db, :up) }.not_to raise_error + expected_indexes.each do |table, (name, columns)| + expect(db.indexes(table)[name]).to include(columns: columns) + end + + Sequel::Migrator.run(db, migrations_path, target: current_migration_index - 1, allow_missing_migration_files: true) + expected_indexes.each do |table, (name, _columns)| + expect(db.indexes(table)).not_to have_key(name) + end + expect { migration.apply(db, :down) }.not_to raise_error + end + + it 'repairs invalid PostgreSQL indexes while preserving valid indexes on subsequent retries' do + skip 'PostgreSQL concurrent index validity' unless db.database_type == :postgres + + names = %w[service_bindings_service_account_guid_index processes_service_account_guid_state_index tasks_service_account_guid_state_index] + Sequel::Migrator.run(db, migrations_path, target: current_migration_index, allow_missing_migration_files: true) + load File.join(migrations_path, migration_filename) + migration = Sequel::Migration.descendants.last + names.each do |name| + db.run("UPDATE pg_index SET indisvalid = false WHERE indexrelid = '#{name}'::regclass") + end + + migration.apply(db, :up) + + names.each do |name| + expect(db.fetch("SELECT indisvalid FROM pg_index WHERE indexrelid = '#{name}'::regclass").get(:indisvalid)).to be(true) + end + expect(db).not_to receive(:add_index) + expect(db).not_to receive(:drop_index) + migration.apply(db, :up) + ensure + names&.each do |name| + db.run("UPDATE pg_index SET indisvalid = true WHERE indexrelid = '#{name}'::regclass") + end + end +end diff --git a/spec/migrations/service_account_schema_spec.rb b/spec/migrations/service_account_schema_spec.rb new file mode 100644 index 00000000000..c798dce9fb8 --- /dev/null +++ b/spec/migrations/service_account_schema_spec.rb @@ -0,0 +1,19 @@ +require 'spec_helper' + +RSpec.describe 'Service account schema', isolation: :truncation do + it 'allows reverting empty account assignments and restoring their foreign key' do + db = Sequel::Model.db + expect(db[:service_accounts].count).to eq(0) + expect { Sequel::Migrator.run(db, DBMigrator::SEQUEL_MIGRATIONS, target: 20_261_005_120_000) }.not_to raise_error + Sequel::Migrator.run(db, DBMigrator::SEQUEL_MIGRATIONS) + expect(db.foreign_key_list(:apps)).to include(hash_including(columns: [:service_account_guid], table: :service_accounts)) + columns = db.schema(:service_accounts).to_h + expect(columns).not_to have_key(:space_guid) + expect(columns.fetch(:organization_id)[:allow_null]).to be(false) + expect(columns.fetch(:identity_prefix)[:allow_null]).to be(false) + expect(db.schema(:service_account_names).to_h).to have_key(:deleted_at) + expect(db.indexes(:service_account_space_entitlements).values).to include( + hash_including(columns: %i[space_guid service_account_guid]) + ) + end +end diff --git a/spec/request/app_service_accounts_spec.rb b/spec/request/app_service_accounts_spec.rb new file mode 100644 index 00000000000..eb34eb22ba8 --- /dev/null +++ b/spec/request/app_service_accounts_spec.rb @@ -0,0 +1,202 @@ +require 'spec_helper' + +RSpec.describe 'App service account relationship' do + let(:space) { create(:space, organization: create(:organization, service_account_prefix: 'assignment-team')) } + let(:user) { create(:user) } + let(:app_model) { create(:app_model, space: space, desired_state: 'STARTED') } + let(:account) { VCAP::CloudController::ServiceAccountModel.create(name: 'payments-worker', space: space, status: 'ready') } + let(:path) { "/v3/apps/#{app_model.guid}/relationships/service_account" } + + def headers(role) + set_user_with_header_as_role(role: role, org: space.organization, space: space, user: user) + end + + it 'lets a developer bind a ready same-space account without restarting the app' do + patch path, { data: { guid: account.guid } }.to_json, headers('space_developer') + expect(last_response.status).to eq(200) + expect(Oj.load(last_response.body)['data']).to eq('guid' => account.guid) + expect(last_response.headers['X-Cf-Warnings']).to include('Restart') + expect(app_model.reload.service_account).to eq(account) + expect(app_model.desired_state).to eq('STARTED') + event = VCAP::CloudController::Event.first(type: 'audit.app.service_account.assign', actee: app_model.guid) + expect(event).not_to be_nil + expect(event.metadata['service_account_guid']).to eq(account.guid) + end + + it 'shows an empty relationship for an unbound app' do + get path, nil, headers('space_auditor') + expect(last_response.status).to eq(200) + expect(Oj.load(last_response.body)['data']).to be_nil + end + + it 'denies an auditor assignment permission' do + patch path, { data: { guid: account.guid } }.to_json, headers('space_auditor') + expect(last_response.status).to eq(403) + expect(app_model.reload.service_account).to be_nil + end + + it 'denies a cross-space account even for an administrator' do + other = VCAP::CloudController::ServiceAccountModel.create(name: 'reporting-reader', space: create(:space, organization: space.organization), status: 'ready') + patch path, { data: { guid: other.guid } }.to_json, headers('admin') + expect(last_response.status).to eq(409) + expect(app_model.reload.service_account).to be_nil + end + + it 'requires unbinding before replacing an account' do + app_model.update(service_account: account) + other = VCAP::CloudController::ServiceAccountModel.create(name: 'reporting-reader', space: space, status: 'ready') + patch path, { data: { guid: other.guid } }.to_json, headers('space_developer') + expect(last_response.status).to eq(409) + expect(app_model.reload.service_account).to eq(account) + end + + it 'supports explicit unbinding and returns restart guidance' do + app_model.update(service_account: account) + patch path, { data: nil }.to_json, headers('space_developer') + expect(last_response.status).to eq(200) + expect(app_model.reload.service_account).to be_nil + expect(last_response.headers['X-Cf-Warnings']).to include('Restart') + expect(VCAP::CloudController::Event.where(type: 'audit.app.service_account.unassign', actee: app_model.guid).count).to eq(1) + end + + it 'rejects assignment while the space is suspended' do + space.update(status: VCAP::CloudController::Space::SUSPENDED) + patch path, { data: { guid: account.guid } }.to_json, headers('space_developer') + expect(last_response.status).to eq(403) + end + + it 'rejects unprovisioned accounts rather than claiming successful binding' do + account.update(status: 'reserved') + patch path, { data: { guid: account.guid } }.to_json, headers('space_developer') + expect(last_response.status).to eq(409) + end + + context 'when asynchronous provisioning is enabled' do + let(:clients) { instance_double(CF::UAA::Scim) } + + before do + TestConfig.override(service_accounts_enabled: true) + CloudController::DependencyLocator.instance.register( + :service_account_provisioner, + VCAP::CloudController::ServiceAccountProvision.new(clients, identity_ca_ref: 'diego-instance-identity') + ) + account.update(status: 'reserved') + allow(clients).to receive(:get).and_raise(CF::UAA::NotFound) + allow(clients).to receive(:add) + end + + it 'accepts desired binding and exposes one pollable provisioning job without restarting' do + patch path, { data: { guid: account.guid } }.to_json, headers('space_developer') + expect(last_response.status).to eq(202) + location = last_response.headers['Location'] + expect(location).to include('/v3/jobs/') + expect(app_model.reload.service_account_guid).to eq(account.guid) + expect(app_model.desired_state).to eq('STARTED') + expect(account.reload.status).to eq('reconciling') + expect(VCAP::CloudController::User.first(guid: account.client_id)).to be_nil + expect(clients).not_to have_received(:add) + + get URI(location).path, nil, headers('space_developer') + expect(last_response.status).to eq(200) + expect(Oj.load(last_response.body)).to include('state' => 'PROCESSING', 'operation' => 'service_account.provision') + expect(Delayed::Worker.new.work_off).to eq([1, 0]) + expect(account.reload.status).to eq('ready') + get URI(location).path, nil, headers('space_developer') + expect(Oj.load(last_response.body)['state']).to eq('COMPLETE') + expect(VCAP::CloudController::User.first(guid: account.client_id)).to be_nil + end + + it 'reuses the active job for repeated binding and another app sharing the account' do + auth = headers('space_developer') + patch path, { data: { guid: account.guid } }.to_json, auth + location = last_response.headers['Location'] + patch path, { data: { guid: account.guid } }.to_json, auth + expect(last_response.status).to eq(202) + expect(last_response.headers['Location']).to eq(location) + other_app = create(:app_model, space: space) + patch "/v3/apps/#{other_app.guid}/relationships/service_account", { data: { guid: account.guid } }.to_json, auth + expect(last_response.headers['Location']).to eq(location) + expect(Delayed::Job.count).to eq(1) + end + + it 'allows unbinding while queued and does not restore the assignment when provisioning finishes' do + auth = headers('space_developer') + patch path, { data: { guid: account.guid } }.to_json, auth + patch path, { data: nil }.to_json, auth + expect(last_response.status).to eq(200) + Delayed::Worker.new.work_off + expect(app_model.reload.service_account_guid).to be_nil + expect(account.reload.status).to eq('ready') + end + + %w[space_auditor space_manager].each do |role| + it "does not queue provisioning for #{role}" do + patch path, { data: { guid: account.guid } }.to_json, headers(role) + expect(last_response.status).to eq(403) + expect(Delayed::Job.count).to eq(0) + expect(account.reload.status).to eq('reserved') + end + end + + it 'does not queue provisioning for a disabled account' do + account.update(enabled: false) + patch path, { data: { guid: account.guid } }.to_json, headers('space_developer') + expect(last_response.status).to eq(409) + expect(Delayed::Job.count).to eq(0) + end + + it 'does not queue a cross-space account even for an administrator' do + other = VCAP::CloudController::ServiceAccountModel.create(name: 'other-worker', space: create(:space, :with_service_account_prefix)) + patch path, { data: { guid: other.guid } }.to_json, headers('admin') + expect(last_response.status).to eq(409) + expect(Delayed::Job.count).to eq(0) + end + + it 'does not bind an account while a delete operation is retrying' do + account.update(status: 'failed') + VCAP::CloudController::PollableJobModel.create( + delayed_job_guid: SecureRandom.uuid, operation: 'service_account.delete', + resource_guid: account.guid, resource_type: 'service_account', state: 'PROCESSING' + ) + patch path, { data: { guid: account.guid } }.to_json, headers('space_developer') + expect(last_response.status).to eq(409) + expect(app_model.reload.service_account_guid).to be_nil + end + + it 'rolls back desired binding and state if enqueueing fails' do + error = CloudController::Errors::ApiError.new_from_details('ServerError') + allow_any_instance_of(VCAP::CloudController::Jobs::Enqueuer).to receive(:enqueue_pollable).and_raise(error) + patch path, { data: { guid: account.guid } }.to_json, headers('space_developer') + expect(last_response.status).to eq(500) + expect(app_model.reload.service_account_guid).to be_nil + expect(account.reload.status).to eq('reserved') + end + + it 'retries a failed bind through the same pollable job and then reports completion' do + allow(clients).to receive(:add).and_raise(CF::UAA::BadTarget, 'unavailable') + patch path, { data: { guid: account.guid } }.to_json, headers('space_developer') + expect(last_response.status).to eq(202) + location = last_response.headers['Location'] + expect(Delayed::Worker.new.work_off).to eq([0, 1]) + expect(account.reload.status).to eq('failed') + allow(clients).to receive(:add) + Delayed::Job.first.update(run_at: Time.now.utc - 1) + expect(Delayed::Worker.new.work_off).to eq([1, 0]) + get URI(location).path, nil, headers('space_developer') + expect(Oj.load(last_response.body)['state']).to eq('COMPLETE') + end + end + + [{}, { data: {} }, { data: { guid: nil } }, { data: { guid: 'x', name: 'injected' } }, { data: [] }].each do |body| + it "rejects malformed relationship body #{body.inspect}" do + patch path, body.to_json, headers('space_developer') + expect(last_response.status).to eq(422) + end + end + + it 'hides apps outside the caller space' do + other_app = create(:app_model, space: create(:space, organization: space.organization)) + get "/v3/apps/#{other_app.guid}/relationships/service_account", nil, headers('space_developer') + expect(last_response.status).to eq(404) + end +end diff --git a/spec/request/apps_spec.rb b/spec/request/apps_spec.rb index 1fb50217fc8..e5e1092e956 100644 --- a/spec/request/apps_spec.rb +++ b/spec/request/apps_spec.rb @@ -1320,6 +1320,7 @@ 'created_at' => iso8601, 'updated_at' => iso8601, 'name' => org1.name, + 'service_account_prefix' => org1.service_account_prefix, 'metadata' => { 'labels' => {}, 'annotations' => {} @@ -1346,6 +1347,7 @@ 'created_at' => iso8601, 'updated_at' => iso8601, 'name' => org2.name, + 'service_account_prefix' => org2.service_account_prefix, 'suspended' => false, 'metadata' => { 'labels' => {}, @@ -1631,6 +1633,7 @@ 'created_at' => iso8601, 'updated_at' => iso8601, 'name' => org.name, + 'service_account_prefix' => org.service_account_prefix, 'metadata' => { 'labels' => {}, 'annotations' => {} diff --git a/spec/request/organization_quotas_spec.rb b/spec/request/organization_quotas_spec.rb index 06d6e3165c1..c5646f9a4a6 100644 --- a/spec/request/organization_quotas_spec.rb +++ b/spec/request/organization_quotas_spec.rb @@ -31,6 +31,7 @@ module VCAP::CloudController created_at: iso8601, updated_at: iso8601, name: params[:name], + service_accounts: { total_service_accounts: nil }, apps: { total_memory_in_mb: nil, per_process_memory_in_mb: nil, @@ -114,6 +115,7 @@ module VCAP::CloudController created_at: iso8601, updated_at: iso8601, name: 'org1', + service_accounts: { total_service_accounts: nil }, apps: { total_memory_in_mb: 5120, per_process_memory_in_mb: 1024, @@ -344,6 +346,7 @@ module VCAP::CloudController created_at: iso8601, updated_at: iso8601, name: params[:name], + service_accounts: { total_service_accounts: nil }, apps: { total_memory_in_mb: 5120, per_process_memory_in_mb: 1024, @@ -622,6 +625,7 @@ def generate_org_quota_single_response(list_of_orgs) created_at: iso8601, updated_at: iso8601, name: organization_quota.name, + service_accounts: { total_service_accounts: nil }, apps: { total_memory_in_mb: 20_480, per_process_memory_in_mb: nil, @@ -671,6 +675,7 @@ def generate_default_org_quota_response(global_read) created_at: iso8601, updated_at: iso8601, name: default_quota.name, + service_accounts: { total_service_accounts: nil }, apps: { total_memory_in_mb: 10_240, per_process_memory_in_mb: nil, diff --git a/spec/request/organization_service_account_prefix_spec.rb b/spec/request/organization_service_account_prefix_spec.rb new file mode 100644 index 00000000000..132a877e34a --- /dev/null +++ b/spec/request/organization_service_account_prefix_spec.rb @@ -0,0 +1,153 @@ +require 'spec_helper' + +RSpec.describe 'Organization service account prefixes' do + let(:user) { create(:user) } + let(:org) { create(:organization) } + let(:admin_header) { admin_headers_for(user) } + + def create_org(name, prefix) + post '/v3/organizations', { name: name, service_account_prefix: prefix }.to_json, admin_header + end + + def allocate_prefix(prefix, auth=admin_header) + patch "/v3/organizations/#{org.guid}", { service_account_prefix: prefix }.to_json, auth + end + + it 'allows an admin to allocate a prefix at creation and records it in the audit event' do + create_org('Payments', 'acme-payments') + + expect(last_response.status).to eq(201) + expect(parsed_response['service_account_prefix']).to eq('acme-payments') + created_guid = parsed_response['guid'] + event = VCAP::CloudController::Event.first(type: 'audit.organization.create', actee: created_guid) + expect(event.metadata.dig('request', 'service_account_prefix')).to eq('acme-payments') + + get "/v3/organizations/#{created_guid}", nil, admin_header + expect(parsed_response['service_account_prefix']).to eq('acme-payments') + get '/v3/organizations', nil, admin_header + expect(parsed_response['resources'].find { |resource| resource['guid'] == created_guid }['service_account_prefix']).to eq('acme-payments') + end + + it 'leaves ordinary org creation available without generating a prefix' do + post '/v3/organizations', { name: 'Ordinary Org' }.to_json, admin_header + + expect(last_response.status).to eq(201) + expect(parsed_response).to include('service_account_prefix' => nil) + end + + it 'accepts an explicitly unset prefix on creation' do + create_org('Ordinary Org', nil) + + expect(last_response.status).to eq(201) + expect(parsed_response).to include('service_account_prefix' => nil) + end + + it 'allocates once on an existing org and preserves the prefix across rename and metadata updates' do + allocate_prefix('acme-payments') + expect(last_response.status).to eq(200) + expect(parsed_response['service_account_prefix']).to eq('acme-payments') + event = VCAP::CloudController::Event.first(type: 'audit.organization.update', actee: org.guid) + expect(event.metadata.dig('request', 'service_account_prefix')).to eq('acme-payments') + + allocate_prefix('acme-payments') + expect(last_response.status).to eq(200) + patch "/v3/organizations/#{org.guid}", { name: 'Renamed', metadata: { labels: { team: 'payments' } } }.to_json, admin_header + expect(last_response.status).to eq(200) + expect(parsed_response).to include('name' => 'Renamed', 'service_account_prefix' => 'acme-payments') + end + + ['different-prefix', nil].each do |replacement| + it "rejects changing an allocated prefix to #{replacement.inspect} atomically" do + allocate_prefix('acme-payments') + expect(last_response.status).to eq(200) + + patch "/v3/organizations/#{org.guid}", { name: 'Must not change', service_account_prefix: replacement }.to_json, admin_header + + expect(last_response.status).to eq(422) + expect(parsed_response.dig('errors', 0, 'detail')).to include('immutable') + expect(org.reload.name).not_to eq('Must not change') + get "/v3/organizations/#{org.guid}", nil, admin_header + expect(parsed_response['service_account_prefix']).to eq('acme-payments') + end + end + + it 'rejects a prefix already allocated to another org on creation or update' do + create_org('First Org', 'acme-payments') + expect(last_response.status).to eq(201) + + expect { create_org('Second Org', 'acme-payments') }.not_to change(VCAP::CloudController::Organization, :count) + expect(last_response.status).to eq(422) + expect(parsed_response.dig('errors', 0, 'detail')).to include('already reserved') + allocate_prefix('acme-payments') + expect(last_response.status).to eq(422) + allocate_prefix('available-prefix') + expect(last_response.status).to eq(200) + end + + it 'retains a prefix reservation after org deletion' do + allocate_prefix('acme-payments') + expect(last_response.status).to eq(200) + expect(VCAP::CloudController::Organization.db[:service_account_prefixes].where(prefix: 'acme-payments').count).to eq(1) + delete "/v3/organizations/#{org.guid}", nil, admin_header + expect(last_response.status).to eq(202) + execute_all_jobs(expected_successes: 1, expected_failures: 0) + expect(VCAP::CloudController::Organization.first(guid: org.guid)).to be_nil + expect(VCAP::CloudController::Organization.db[:service_account_prefixes].where(prefix: 'acme-payments').count).to eq(1) + + create_org('Replacement Org', 'acme-payments') + expect(last_response.status).to eq(422), last_response.body + expect(parsed_response.dig('errors', 0, 'detail')).to include('already reserved') + end + + ['abc', 'a-1', '123', 'a' * 63].each do |prefix| + it "accepts the RFC label #{prefix.inspect}" do + create_org('Valid Org', prefix) + + expect(last_response.status).to eq(201) + expect(parsed_response['service_account_prefix']).to eq(prefix) + end + end + + ['', 'ab', 'a' * 64, 'Upper', '-abc', 'abc-', 'a.b', 'a_b', 'abc def', "abc\n", 'café', 123, false, {}].each do |prefix| + it "rejects the invalid prefix #{prefix.inspect} on create and update" do + create_org('Invalid Org', prefix) + expect(last_response.status).to eq(422) + expect(parsed_response.dig('errors', 0, 'detail')).to match(/Service account prefix/i) + + allocate_prefix(prefix) + expect(last_response.status).to eq(422) + expect(parsed_response.dig('errors', 0, 'detail')).to match(/Service account prefix/i) + end + end + + it 'does not let an org manager allocate a prefix or combine it with another update' do + org.add_user(user) + org.add_manager(user) + auth = headers_for(user) + + patch "/v3/organizations/#{org.guid}", { name: 'Must not change', service_account_prefix: 'acme-payments' }.to_json, auth + + expect(last_response.status).to eq(403) + expect(org.reload.name).not_to eq('Must not change') + allocate_prefix('acme-payments') + expect(last_response.status).to eq(200) + allocate_prefix('acme-payments', auth) + expect(last_response.status).to eq(403) + end + + it 'does not let self-service org creators allocate a prefix' do + create(:feature_flag, name: 'user_org_creation', enabled: true) + uaa_client = instance_double(VCAP::CloudController::UaaClient) + allow(CloudController::DependencyLocator.instance).to receive(:uaa_username_lookup_client).and_return(uaa_client) + allow(uaa_client).to receive(:usernames_for_ids).with([user.guid]).and_return({ user.guid => 'creator' }) + + expect do + post '/v3/organizations', { name: 'Self Service', service_account_prefix: 'acme-payments' }.to_json, headers_for(user) + end.not_to change(VCAP::CloudController::Organization, :count) + expect(last_response.status).to eq(403) + + post '/v3/organizations', { name: 'Self Service' }.to_json, headers_for(user) + expect(last_response.status).to eq(201) + expect(parsed_response).to include('service_account_prefix' => nil) + end +end diff --git a/spec/request/organizations_spec.rb b/spec/request/organizations_spec.rb index 0c914aafa5a..ff63c5402af 100644 --- a/spec/request/organizations_spec.rb +++ b/spec/request/organizations_spec.rb @@ -68,6 +68,7 @@ module VCAP::CloudController 'labels' => { 'freaky' => 'friday' }, 'annotations' => { 'make' => 'subaru', 'model' => 'xv crosstrek', 'color' => 'orange' } }, + 'service_account_prefix' => nil, 'suspended' => false } ) @@ -98,6 +99,7 @@ module VCAP::CloudController }, 'metadata' => { 'labels' => {}, 'annotations' => {} }, 'relationships' => { 'quota' => { 'data' => { 'guid' => created_org.quota_definition.guid } } }, + 'service_account_prefix' => nil, 'suspended' => true } ) @@ -133,6 +135,7 @@ module VCAP::CloudController 'labels' => { 'freaky' => 'friday' }, 'annotations' => { 'make' => 'subaru', 'model' => 'xv crosstrek', 'color' => 'orange' } }, + 'service_account_prefix' => nil, 'suspended' => false } ) @@ -284,6 +287,7 @@ module VCAP::CloudController 'labels' => {}, 'annotations' => {} }, + 'service_account_prefix' => nil, 'suspended' => false }, { @@ -304,6 +308,7 @@ module VCAP::CloudController 'labels' => {}, 'annotations' => {} }, + 'service_account_prefix' => nil, 'suspended' => false } ] @@ -409,6 +414,7 @@ module VCAP::CloudController 'labels' => {}, 'annotations' => {} }, + 'service_account_prefix' => nil, 'suspended' => false }, { @@ -429,6 +435,7 @@ module VCAP::CloudController 'labels' => {}, 'annotations' => {} }, + 'service_account_prefix' => nil, 'suspended' => false } ] @@ -1157,6 +1164,7 @@ module VCAP::CloudController 'labels' => {}, 'annotations' => {} }, + 'service_account_prefix' => nil, 'suspended' => false } end @@ -1216,6 +1224,7 @@ module VCAP::CloudController 'labels' => { 'freaky' => 'thursday' }, 'annotations' => { 'quality' => 'p sus' } }, + 'service_account_prefix' => nil, 'suspended' => false } @@ -1266,6 +1275,7 @@ module VCAP::CloudController 'created_at' => iso8601, 'updated_at' => iso8601, 'metadata' => { 'labels' => {}, 'annotations' => {} }, + 'service_account_prefix' => nil, 'suspended' => true } @@ -1311,6 +1321,7 @@ module VCAP::CloudController 'labels' => { 'animal' => 'horse' }, 'annotations' => {} }, + 'service_account_prefix' => nil, 'suspended' => false } diff --git a/spec/request/roles_spec.rb b/spec/request/roles_spec.rb index 50c09e41144..5f155b502c3 100644 --- a/spec/request/roles_spec.rb +++ b/spec/request/roles_spec.rb @@ -1410,6 +1410,7 @@ def make_space_role_for_current_user(type) created_at: iso8601, updated_at: iso8601, name: org.name, + service_account_prefix: org.service_account_prefix, suspended: false, relationships: { quota: { @@ -1540,6 +1541,7 @@ def make_space_role_for_current_user(type) created_at: iso8601, updated_at: iso8601, name: another_org.name, + service_account_prefix: another_org.service_account_prefix, suspended: false, relationships: { quota: { @@ -1748,6 +1750,7 @@ def make_space_role_for_current_user(type) created_at: iso8601, updated_at: iso8601, name: org.name, + service_account_prefix: org.service_account_prefix, suspended: false, relationships: { quota: { diff --git a/spec/request/service_account_entitlements_spec.rb b/spec/request/service_account_entitlements_spec.rb new file mode 100644 index 00000000000..9316d25229a --- /dev/null +++ b/spec/request/service_account_entitlements_spec.rb @@ -0,0 +1,188 @@ +require 'spec_helper' + +module VCAP::CloudController + RSpec.describe 'Service account space entitlements' do + let(:org) { create(:organization, service_account_prefix: 'test-team') } + let(:space) { create(:space, organization: org) } + let(:target) { create(:space, organization: org) } + let(:user) { create(:user) } + let(:account) { ServiceAccountModel.create(name: 'shared-worker', space: space, status: 'ready') } + let(:path) { "/v3/service_accounts/#{account.guid}/relationships/spaces" } + + def auth(role) + set_user_with_header_as_role(role: role, org: org, space: space, user: user) + end + + def entitle(destination, headers) + post path, { data: [{ guid: destination.guid }] }.to_json, headers + end + + it 'allows a developer of both spaces to entitle and bind a second space' do + headers = auth('space_developer') + target.add_developer(user) + entitle(target, headers) + expect(last_response.status).to eq(200) + expect(account.reload.entitled_spaces).to contain_exactly(space, target) + app = create(:app_model, space: target) + patch "/v3/apps/#{app.guid}/relationships/service_account", { data: { guid: account.guid } }.to_json, headers + expect(last_response.status).to eq(200) + expect(app.reload.service_account_guid).to eq(account.guid) + end + + it 'requires development access to both an entitled source and destination space' do + headers = auth('space_developer') + entitle(target, headers) + expect(last_response.status).to eq(403) + expect(account.reload.entitled_spaces).to contain_exactly(space) + end + + it 'rejects cross-org entitlements even for admins' do + entitle(create(:space), auth('admin')) + expect(last_response.status).to eq(422) + expect(account.reload.entitled_spaces).to contain_exactly(space) + end + + it 'shows org managers zero-entitlement accounts and filters by organization' do + headers = auth('org_manager') + account.remove_all_entitled_spaces + get "/v3/service_accounts/#{account.guid}", nil, headers + expect(last_response.status).to eq(200) + expect(parsed_response.dig('relationships', 'organization', 'data', 'guid')).to eq(org.guid) + get "/v3/service_accounts?organization_guids=#{org.guid}", nil, headers + expect(last_response.status).to eq(200) + expect(parsed_response['resources'].pluck('guid')).to eq([account.guid]) + end + + it 'blocks revocation while desired or active workload references use the account' do + headers = auth('org_manager') + entitle(target, headers) + expect(last_response.status).to eq(200) + app = create(:app_model, space: target, service_account: account) + delete "#{path}/#{target.guid}", nil, headers + expect(last_response.status).to eq(409) + app.update(service_account: nil) + process = create(:process_model, app: app, state: ProcessModel::STARTED, service_account_guid: account.guid, service_account_snapshot: true) + process.update(service_account_guid: account.guid, service_account_snapshot: true) + delete "#{path}/#{target.guid}", nil, headers + expect(last_response.status).to eq(409) + process.destroy + delete "#{path}/#{target.guid}", nil, headers + expect(last_response.status).to eq(200) + end + + it 'does not let a developer revoke the last entitlement but permits org managers' do + delete "#{path}/#{space.guid}", nil, auth('space_developer') + expect(last_response.status).to eq(409) + delete "#{path}/#{space.guid}", nil, auth('org_manager') + expect(last_response.status).to eq(200) + expect(account.reload.entitled_spaces).to be_empty + end + + it 'allows a developer to create an account with an initial entitlement' do + body = { name: 'new-worker', relationships: { space: { data: { guid: space.guid } } } } + post '/v3/service_accounts', body.to_json, auth('space_developer') + expect(last_response.status).to eq(201) + expect(ServiceAccountModel.first(guid: parsed_response['guid']).entitled_spaces).to contain_exactly(space) + end + + it 'does not let a developer in a suspended entitled space manage the account' do + headers = auth('space_developer') + account + space.update(status: Space::SUSPENDED) + patch "/v3/service_accounts/#{account.guid}", { description: 'blocked' }.to_json, headers + expect(last_response.status).to eq(403) + end + + it 'refuses adding an entitlement to a deleting account' do + account.update(status: 'deleting', enabled: false) + entitle(target, auth('org_manager')) + expect(last_response.status).to eq(409) + end + + it 'rechecks management authority after acquiring the account lock' do + headers = auth('space_developer') + account + allow_any_instance_of(ServiceAccountModel).to receive(:lock!).and_wrap_original do |method, *args| + result = method.call(*args) + method.receiver.remove_all_entitled_spaces + result + end + patch "/v3/service_accounts/#{account.guid}", { description: 'raced update' }.to_json, headers + expect(last_response.status).to eq(403) + expect(account.reload.description).to eq('') + end + + it 'does not let a developer bootstrap authority by ordering a multi-space entitlement request' do + headers = auth('space_developer') + target.add_developer(user) + account.remove_all_entitled_spaces + post path, { data: [{ guid: target.guid }, { guid: space.guid }] }.to_json, headers + expect(last_response.status).to eq(404) + end + + it 'does not entitle a space whose deletion has begun' do + target.update(status: Space::DELETING) + entitle(target, auth('org_manager')) + expect(last_response.status).to eq(422) + expect(account.reload.entitled_spaces).to contain_exactly(space) + end + + it 'authorizes entitlement expansion using current source access under the account lock' do + headers = auth('space_developer') + target.add_developer(user) + account + allow_any_instance_of(ServiceAccountModel).to receive(:lock!).and_wrap_original do |method, *args| + result = method.call(*args) + method.receiver.remove_all_entitled_spaces + result + end + entitle(target, headers) + expect(last_response.status).to eq(403) + end + + it 'returns success when revoking the callers last readable entitlement while another space remains' do + headers = auth('admin') + entitle(target, headers) + expect(last_response.status).to eq(200) + delete "#{path}/#{space.guid}", nil, auth('space_developer') + expect(last_response.status).to eq(200) + expect(account.reload.entitled_spaces).to contain_exactly(target) + end + + [Space::SUSPENDED, Space::DELETING].each do |state| + it "prevents a developer from revoking an entitlement in a #{state} space" do + headers = auth('space_developer') + account.add_entitled_space(target) + space.update(status: state) + + delete "#{path}/#{space.guid}", nil, headers + + expect(last_response.status).to eq(state == Space::DELETING ? 422 : 403) + expect(account.reload.entitled_spaces).to contain_exactly(space, target) + end + + %w[org_manager admin].each do |role| + it "allows an #{role} to revoke an unused entitlement in a #{state} space" do + headers = auth(role) + account.add_entitled_space(target) + space.update(status: state) + + delete "#{path}/#{space.guid}", nil, headers + + expect(last_response.status).to eq(200) + expect(account.reload.entitled_spaces).to contain_exactly(target) + end + end + end + + it 'does not use a suspended source space to authorize entitlement expansion' do + headers = auth('space_developer') + target.add_developer(user) + account + space.update(status: Space::SUSPENDED) + entitle(target, headers) + expect(last_response.status).to eq(403) + expect(account.reload.entitled_spaces).to contain_exactly(space) + end + end +end diff --git a/spec/request/service_account_namespaces_spec.rb b/spec/request/service_account_namespaces_spec.rb new file mode 100644 index 00000000000..452154fdaf4 --- /dev/null +++ b/spec/request/service_account_namespaces_spec.rb @@ -0,0 +1,90 @@ +require 'spec_helper' + +module VCAP::CloudController + RSpec.describe 'Service account org namespaces' do + let(:user) { create(:user) } + let(:org) { create(:organization, service_account_prefix: 'acme-payments') } + let(:space) { create(:space, organization: org) } + let(:body) { { name: 'payments-worker', relationships: { space: { data: { guid: space.guid } } } } } + + def auth(role='space_developer') + set_user_with_header_as_role(role: role, org: org, space: space, user: user) + end + + it 'derives a qualified subject and SAN and permits the same name in another org' do + post '/v3/service_accounts', body.to_json, auth + expect(last_response.status).to eq(201) + expect(parsed_response['client_id']).to eq('cf:service-account:acme-payments:payments-worker') + expect(parsed_response['certificate_dns_san']).to eq('payments-worker.acme-payments.svc.identity') + other = create(:space, organization: create(:organization, service_account_prefix: 'other-team')) + post '/v3/service_accounts', body.merge(relationships: { space: { data: { guid: other.guid } } }).to_json, admin_headers_for(user) + expect(last_response.status).to eq(201) + end + + it 'rejects creation when the org prefix is unset' do + other = create(:space) + post '/v3/service_accounts', body.merge(relationships: { space: { data: { guid: other.guid } } }).to_json, admin_headers_for(user) + expect(last_response.status).to eq(422) + expect(parsed_response.dig('errors', 0, 'detail')).to include('prefix') + end + + it 'timestamps deletion and lets an org manager explicitly reuse the name with a new UUID and audit' do + post '/v3/service_accounts', body.to_json, auth + expect(last_response.status).to eq(201) + old_guid = parsed_response['guid'] + ServiceAccountModel.first(guid: old_guid).destroy + reservation = ServiceAccountModel.db[:service_account_names].first(name: 'payments-worker', organization_id: org.id) + expect(reservation[:deleted_at]).to be_a(Time) + post '/v3/service_accounts', body.to_json, auth + expect(last_response.status).to eq(409) + post '/v3/service_accounts', body.merge(reuse_name: true).to_json, auth + expect(last_response.status).to eq(403) + post '/v3/service_accounts', body.merge(reuse_name: true).to_json, auth('org_manager') + expect(last_response.status).to eq(201) + expect(parsed_response['guid']).not_to eq(old_guid) + expect(parsed_response['status']).to eq('reserved') + expect(Event.first(type: 'audit.service_account.create', actee: parsed_response['guid']).metadata).to include('reuse_name' => true) + end + + it 'keeps deletion history when a tombstone is explicitly reused' do + post '/v3/service_accounts', body.to_json, auth + original = ServiceAccountModel.first(guid: parsed_response['guid']) + original.destroy + timestamp = ServiceAccountModel.db[:service_account_names].first(name: body[:name], organization_id: org.id)[:deleted_at] + post '/v3/service_accounts', body.merge(reuse_name: true).to_json, auth('org_manager') + expect(last_response.status).to eq(201) + expect(ServiceAccountModel.db[:service_account_name_deletions].first(account_guid: original.guid)).to include(deleted_at: timestamp) + end + + it 'refuses a previously fetched actor after its last entitlement is revoked' do + headers = auth + post '/v3/service_accounts', body.to_json, headers + account = ServiceAccountModel.first(guid: parsed_response['guid']) + account.remove_all_entitled_spaces + patch "/v3/service_accounts/#{account.guid}", { description: 'forbidden' }.to_json, headers + expect(last_response.status).to eq(404) + end + + it 'keeps reuse warnings and the previous account GUID in durable audit evidence' do + post '/v3/service_accounts', body.to_json, auth + old_guid = parsed_response['guid'] + ServiceAccountModel.first(guid: old_guid).destroy + post '/v3/service_accounts', body.merge(reuse_name: true).to_json, auth('org_manager') + expect(last_response.status).to eq(201) + expect(last_response.headers['X-Cf-Warnings']).to match(/external|certificate/i) + expect(Event.first(type: 'audit.service_account.create', actee: parsed_response['guid']).metadata['previous_account_guid']).to eq(old_guid) + end + + it 'refuses creation if the org starts deleting before its lock is acquired' do + headers = auth + allow_any_instance_of(Organization).to receive(:lock!).and_wrap_original do |method, *args| + result = method.call(*args) + method.receiver.update(status: Organization::DELETING) + result + end + post '/v3/service_accounts', body.to_json, headers + expect(last_response.status).to eq(422) + expect(ServiceAccountModel.count).to eq(0) + end + end +end diff --git a/spec/request/service_account_quotas_spec.rb b/spec/request/service_account_quotas_spec.rb new file mode 100644 index 00000000000..0b58aa07f84 --- /dev/null +++ b/spec/request/service_account_quotas_spec.rb @@ -0,0 +1,42 @@ +require 'spec_helper' + +module VCAP::CloudController + RSpec.describe 'Service account org quotas' do + let(:user) { create(:user) } + let(:org) { create(:organization, service_account_prefix: 'quota-team') } + let(:space) { create(:space, organization: org) } + let(:headers) { admin_headers_for(user) } + + it 'persists quota limits, counts disabled accounts, and frees capacity on deletion without freeing names' do + post '/v3/organization_quotas', { name: 'account-quota', service_accounts: { total_service_accounts: 1 } }.to_json, headers + expect(last_response.status).to eq(201) + expect(parsed_response['service_accounts']).to eq('total_service_accounts' => 1) + quota = QuotaDefinition.first(guid: parsed_response['guid']) + org.update(quota_definition: quota) + body = { name: 'first-worker', relationships: { space: { data: { guid: space.guid } } } } + post '/v3/service_accounts', body.to_json, headers + expect(last_response.status).to eq(201) + account = ServiceAccountModel.first(guid: parsed_response['guid']) + account.update(enabled: false) + post '/v3/service_accounts', body.merge(name: 'second-worker').to_json, headers + expect(last_response.status).to eq(422) + expect(parsed_response.dig('errors', 0, 'detail')).to include('quota') + account.destroy + post '/v3/service_accounts', body.merge(name: 'second-worker').to_json, headers + expect(last_response.status).to eq(201) + post '/v3/service_accounts', body.to_json, headers + expect(last_response.status).not_to eq(201) + patch "/v3/organization_quotas/#{quota.guid}", { service_accounts: { total_service_accounts: nil } }.to_json, headers + expect(last_response.status).to eq(200) + expect(parsed_response['service_accounts']).to eq('total_service_accounts' => nil) + end + + [-1, 'one', 1.5].each do |limit| + it "rejects invalid limit #{limit.inspect}" do + post '/v3/organization_quotas', { name: 'invalid-quota', service_accounts: { total_service_accounts: limit } }.to_json, headers + expect(last_response.status).to eq(422) + expect(parsed_response.dig('errors', 0, 'detail')).to include('Service accounts') + end + end + end +end diff --git a/spec/request/service_account_roles_spec.rb b/spec/request/service_account_roles_spec.rb new file mode 100644 index 00000000000..7d930e2770f --- /dev/null +++ b/spec/request/service_account_roles_spec.rb @@ -0,0 +1,53 @@ +require 'spec_helper' + +RSpec.describe 'Explicit service account resource roles' do + let(:space) { create(:space, organization: create(:organization, service_account_prefix: 'roles-team')) } + let(:admin) { create(:user) } + let(:account) { VCAP::CloudController::ServiceAccountModel.create(name: 'payments-worker', space: space) } + let(:clients) { instance_double(CF::UAA::Scim) } + let(:principal) { VCAP::CloudController::User.first(guid: account.client_id) } + + before do + allow(clients).to receive(:get).and_raise(CF::UAA::NotFound) + allow(clients).to receive(:add) + VCAP::CloudController::ServiceAccountProvision.new(clients, identity_ca_ref: 'diego-instance-identity').provision(account) + create(:user, guid: account.client_id, is_oauth_client: true) + lookup = instance_double(VCAP::CloudController::UaaClient, usernames_for_ids: {}) + expect(lookup).not_to receive(:usernames_for_ids).with([principal.guid]) + allow(CloudController::DependencyLocator.instance).to receive(:uaa_username_lookup_client).and_return(lookup) + end + + def grant(type, resource, guid) + post '/v3/roles', { + type: type, + relationships: { user: { data: { guid: principal.guid } }, resource => { data: { guid: guid } } } + }.to_json, admin_headers_for(admin) + end + + it 'requires explicit organization membership and then a space role before account tokens can write apps' do + grant('space_developer', :space, space.guid) + expect(last_response.status).to eq(422) + grant('organization_user', :organization, space.organization.guid) + expect(last_response.status).to eq(201) + + app_body = { name: 'account-created', relationships: { space: { data: { guid: space.guid } } } } + post '/v3/apps', app_body.to_json, headers_for(principal, client: true) + expect(last_response.status).to eq(422) + + grant('space_developer', :space, space.guid) + expect(last_response.status).to eq(201) + post '/v3/apps', app_body.to_json, headers_for(principal, client: true) + expect(last_response.status).to eq(201) + + post '/v3/service_accounts', { name: 'privilege-escalation', relationships: { space: { data: { guid: space.guid } } } }.to_json, headers_for(principal, client: true) + expect(last_response.status).to eq(201) + end + + it 'keeps account tokens denied in unrelated spaces after an explicit role grant' do + grant('organization_user', :organization, space.organization.guid) + grant('space_developer', :space, space.guid) + other = create(:space, organization: space.organization) + post '/v3/apps', { name: 'unauthorized', relationships: { space: { data: { guid: other.guid } } } }.to_json, headers_for(principal, client: true) + expect(last_response.status).to eq(422) + end +end diff --git a/spec/request/service_account_route_policies_spec.rb b/spec/request/service_account_route_policies_spec.rb new file mode 100644 index 00000000000..c9aa8366838 --- /dev/null +++ b/spec/request/service_account_route_policies_spec.rb @@ -0,0 +1,67 @@ +require 'spec_helper' + +module VCAP::CloudController + RSpec.describe 'Service account route sources' do + it 'refuses a grant to an account whose deletion has begun' do + org = create(:organization, service_account_prefix: 'route-team') + space = create(:space, organization: org) + ServiceAccountModel.create(name: 'route-worker', space: space, status: 'deleting', enabled: false) + domain = create(:private_domain, owning_organization: org, enforce_route_policies: true) + route = create(:route, space: space, domain: domain) + body = { source: 'cf:svc:route-team:route-worker', relationships: { route: { data: { guid: route.guid } } } } + post '/v3/route_policies', body.to_json, admin_headers_for(create(:user)) + expect(last_response.status).to eq(422) + end + + it 'creates account rules without an extra router-version configuration' do + org = create(:organization, service_account_prefix: 'route-team') + space = create(:space, organization: org) + ServiceAccountModel.create(name: 'route-worker', space: space) + domain = create(:private_domain, owning_organization: org, enforce_route_policies: true) + route = create(:route, space: space, domain: domain) + body = { source: 'cf:svc:route-team:route-worker', relationships: { route: { data: { guid: route.guid } } } } + post '/v3/route_policies', body.to_json, admin_headers_for(create(:user)) + expect(last_response.status).to eq(201) + expect(parsed_response['source']).to eq(body[:source]) + end + + it 'rejects granting an unreadable account and filters qualified sources without parser failures' do + org = create(:organization, service_account_prefix: 'route-team') + space = create(:space, organization: org) + hidden_space = create(:space, organization: create(:organization, service_account_prefix: 'hidden-team')) + ServiceAccountModel.create(name: 'hidden-worker', space: hidden_space) + domain = create(:private_domain, owning_organization: org, enforce_route_policies: true) + route = create(:route, space: space, domain: domain) + user = create(:user) + headers = set_user_with_header_as_role(role: 'space_developer', org: org, space: space, user: user) + body = { source: 'cf:svc:hidden-team:hidden-worker', relationships: { route: { data: { guid: route.guid } } } } + post '/v3/route_policies', body.to_json, headers + expect(last_response.status).to eq(422) + get '/v3/route_policies?sources=cf:svc:route-team:missing-worker', nil, admin_headers_for(user) + expect(last_response.status).to eq(200) + expect(parsed_response['resources']).to eq([]) + end + + it 'resolves a qualified source to an immutable account reference and removes grants on deletion' do + org = create(:organization, service_account_prefix: 'route-team') + space = create(:space, organization: org) + account = ServiceAccountModel.create(name: 'route-worker', space: space) + domain = create(:private_domain, owning_organization: org, enforce_route_policies: true, route_policies_scope: 'space') + route = create(:route, space: space, domain: domain) + headers = admin_headers_for(create(:user)) + body = { source: 'cf:svc:route-team:route-worker', relationships: { route: { data: { guid: route.guid } } } } + + post '/v3/route_policies', body.to_json, headers + + expect(last_response.status).to eq(201) + expect(parsed_response['source']).to eq(body[:source]) + policy = RoutePolicy.first(guid: parsed_response['guid']) + expect(policy.source_guid).to eq(account.guid) + get "/v3/route_policies/#{policy.guid}?include=source", nil, headers + expect(parsed_response.dig('relationships', 'service_account', 'data', 'guid')).to eq(account.guid) + expect(parsed_response.dig('included', 'service_accounts').pluck('guid')).to eq([account.guid]) + account.destroy + expect(RoutePolicy.first(guid: policy.guid)).to be_nil + end + end +end diff --git a/spec/request/service_accounts_spec.rb b/spec/request/service_accounts_spec.rb new file mode 100644 index 00000000000..a255f0b5fd0 --- /dev/null +++ b/spec/request/service_accounts_spec.rb @@ -0,0 +1,430 @@ +require 'spec_helper' + +RSpec.describe 'Service accounts' do + let(:org) { create(:organization, service_account_prefix: 'test-team') } + let(:space) { create(:space, organization: org) } + let(:user) { create(:user) } + let(:body) { { name: 'payments-worker', relationships: { space: { data: { guid: space.guid } } } } } + + def headers(role) + set_user_with_header_as_role(role: role, org: org, space: space, user: user) + end + + it 'allows a space developer to reserve an identity without provisioning a client' do + post '/v3/service_accounts', body.to_json, headers('space_developer') + + expect(last_response.status).to eq(201) + result = Oj.load(last_response.body) + expect(result).to include('name' => 'payments-worker', 'status' => 'reserved', 'enabled' => true, + 'client_id' => 'cf:service-account:test-team:payments-worker', 'certificate_dns_san' => 'payments-worker.test-team.svc.identity') + expect(result.dig('relationships', 'organization', 'data', 'guid')).to eq(org.guid) + expect(result.dig('relationships', 'spaces', 'data')).to eq([{ 'guid' => space.guid }]) + event = VCAP::CloudController::Event.first(type: 'audit.service_account.create', actee: result['guid']) + expect(event).not_to be_nil + expect(event.actor).to eq(user.guid) + expect(event.space_guid).to eq(space.guid) + end + + it 'allows a platform administrator to create an account' do + post '/v3/service_accounts', body.to_json, headers('admin') + expect(last_response.status).to eq(201) + end + + context 'weekly creation budget' do + before { TestConfig.override(service_account_creation_limit: 1) } + + it 'limits a principal across spaces without refunding deleted accounts' do + auth = headers('space_developer') + post '/v3/service_accounts', body.to_json, auth + expect(last_response.status).to eq(201) + VCAP::CloudController::ServiceAccountModel.first.destroy + other_space = create(:space, organization: org) + other_space.add_developer(user) + other_body = { name: 'other-worker', relationships: { space: { data: { guid: other_space.guid } } } } + + post '/v3/service_accounts', other_body.to_json, auth + + expect(last_response.status).to eq(429) + expect(Oj.load(last_response.body).dig('errors', 0, 'title')).to eq('CF-ServiceAccountCreationLimitExceeded') + expect(VCAP::CloudController::ServiceAccountModel.count).to eq(0) + end + + it 'uses a rolling seven-day window' do + auth = headers('space_developer') + Timecop.freeze(Time.utc(2026, 10, 1)) do + post '/v3/service_accounts', body.to_json, auth + expect(last_response.status).to eq(201) + end + Timecop.freeze(Time.utc(2026, 10, 7, 23, 59, 59)) do + post '/v3/service_accounts', body.merge(name: 'second-worker').to_json, auth + expect(last_response.status).to eq(429) + end + Timecop.freeze(Time.utc(2026, 10, 8)) do + post '/v3/service_accounts', body.merge(name: 'second-worker').to_json, auth + expect(last_response.status).to eq(201) + end + end + + it 'does not consume budget on a failed creation' do + auth = headers('space_developer') + VCAP::CloudController::ServiceAccountModel.create(name: body[:name], space: space) + post '/v3/service_accounts', body.to_json, auth + expect(last_response.status).to eq(409) + post '/v3/service_accounts', body.merge(name: 'second-worker').to_json, auth + expect(last_response.status).to eq(201) + post '/v3/service_accounts', body.merge(name: 'third-worker').to_json, auth + expect(last_response.status).to eq(429) + end + + it 'does not share budget between principals' do + post '/v3/service_accounts', body.to_json, headers('space_developer') + expect(last_response.status).to eq(201) + other_user = create(:user) + auth = set_user_with_header_as_role(role: 'space_developer', org: org, space: space, user: other_user) + post '/v3/service_accounts', body.merge(name: 'second-worker').to_json, auth + expect(last_response.status).to eq(201) + end + + it 'supports explicit unlimited configuration' do + TestConfig.override(service_account_creation_limit: -1) + auth = headers('space_developer') + %w[first-worker second-worker].each do |name| + post '/v3/service_accounts', body.merge(name: name).to_json, auth + expect(last_response.status).to eq(201) + end + end + + it 'exempts platform admins even when the configured budget is zero' do + TestConfig.override(service_account_creation_limit: 0) + auth = headers('admin') + %w[first-worker second-worker].each do |name| + post '/v3/service_accounts', body.merge(name: name).to_json, auth + expect(last_response.status).to eq(201) + end + post '/v3/service_accounts', body.to_json, headers('space_developer') + expect(last_response.status).to eq(429) + end + + it 'applies to non-admin automation clients and exempts admin automation' do + user.update(is_oauth_client: true) + org.add_user(user) + space.add_developer(user) + coder = CF::UAA::TokenCoder.new(audience_ids: TestConfig.config[:uaa][:resource_id], skey: TestConfig.config[:uaa][:symmetric_secret], pkey: nil) + token = { client_id: user.guid, scope: %w[cloud_controller.read cloud_controller.write], jti: 'automation', iss: UAAIssuer::ISSUER } + auth = base_json_headers('HTTP_AUTHORIZATION' => "bearer #{coder.encode(token)}") + post '/v3/service_accounts', body.to_json, auth + expect(last_response.status).to eq(201) + post '/v3/service_accounts', body.merge(name: 'second-worker').to_json, auth + expect(last_response.status).to eq(429) + token[:scope] << 'cloud_controller.admin' + admin_auth = base_json_headers('HTTP_AUTHORIZATION' => "bearer #{coder.encode(token)}") + post '/v3/service_accounts', body.merge(name: 'second-worker').to_json, admin_auth + expect(last_response.status).to eq(201) + end + end + + it 'allows a space developer account-creation authority' do + post '/v3/service_accounts', body.to_json, headers('space_developer') + expect(last_response.status).to eq(201) + expect(VCAP::CloudController::ServiceAccountModel.count).to eq(1) + end + + it 'rejects invalid names without normalizing them' do + post '/v3/service_accounts', body.merge(name: 'Payments').to_json, headers('admin') + expect(last_response.status).to eq(422) + end + + it 'rejects caller-supplied identity and provisioning fields' do + post '/v3/service_accounts', body.merge(client_id: 'attacker', status: 'ready').to_json, headers('admin') + expect(last_response.status).to eq(422) + end + + it 'rejects malformed relationship bodies' do + post '/v3/service_accounts', { name: 'payments-worker', relationships: nil }.to_json, headers('admin') + expect(last_response.status).to eq(422) + end + + it 'returns conflict for a permanently reserved name' do + account = VCAP::CloudController::ServiceAccountModel.create(name: 'payments-worker', space: space) + account.destroy + post '/v3/service_accounts', body.to_json, headers('admin') + expect(last_response.status).to eq(409) + end + + it 'denies creation in a suspended organization' do + org.update(status: 'suspended') + post '/v3/service_accounts', body.to_json, headers('space_developer') + expect(last_response.status).to eq(403) + end + + it 'allows an entitled-space member to read the account' do + account = VCAP::CloudController::ServiceAccountModel.create(name: 'payments-worker', space: space) + get "/v3/service_accounts/#{account.guid}", nil, headers('space_developer') + + expect(last_response.status).to eq(200) + expect(Oj.load(last_response.body)['guid']).to eq(account.guid) + end + + it 'hides accounts from developers in another space of the same organization' do + account = VCAP::CloudController::ServiceAccountModel.create(name: 'payments-worker', space: create(:space, organization: org)) + get "/v3/service_accounts/#{account.guid}", nil, headers('space_developer') + expect(last_response.status).to eq(404) + end + + it 'rejects unauthenticated access' do + post '/v3/service_accounts', body.to_json, base_json_headers + expect(last_response.status).to eq(401) + end + + it 'rejects an organization ownership relationship' do + post '/v3/service_accounts', { name: 'payments-worker', relationships: { organization: { data: { guid: org.guid } } } }.to_json, headers('admin') + expect(last_response.status).to eq(422) + end + + it 'denies creation in a suspended space' do + space.update(status: VCAP::CloudController::Space::SUSPENDED) + post '/v3/service_accounts', body.to_json, headers('space_developer') + expect(last_response.status).to eq(403) + end + + it 'creates and updates description and metadata using normal merge/remove semantics' do + auth = headers('space_developer') + post '/v3/service_accounts', body.merge(description: 'Batch payments', metadata: { labels: { team: 'payments' }, annotations: { note: 'original' } }).to_json, auth + expect(last_response.status).to eq(201) + result = Oj.load(last_response.body) + expect(result['description']).to eq('Batch payments') + patch "/v3/service_accounts/#{result['guid']}", { description: 'Revised', metadata: { labels: { team: nil, owner: 'finance' } } }.to_json, auth + expect(last_response.status).to eq(200) + result = Oj.load(last_response.body) + expect(result['description']).to eq('Revised') + expect(result['metadata']).to eq('labels' => { 'owner' => 'finance' }, 'annotations' => { 'note' => 'original' }) + expect(VCAP::CloudController::Event.where(type: 'audit.service_account.update', actee: result['guid']).count).to eq(1) + end + + it 'lists only readable accounts with pagination and space/name filters' do + own = VCAP::CloudController::ServiceAccountModel.create(name: 'payments-worker', space: space) + VCAP::CloudController::ServiceAccountModel.create(name: 'hidden-worker', space: create(:space, :with_service_account_prefix)) + get '/v3/service_accounts?per_page=1&names=payments-worker', nil, headers('space_developer') + expect(last_response.status).to eq(200) + result = Oj.load(last_response.body) + expect(result.dig('pagination', 'total_results')).to eq(1) + expect(result['resources'].pluck('guid')).to eq([own.guid]) + get "/v3/service_accounts?space_guids=#{space.guid}", nil, headers('admin') + expect(Oj.load(last_response.body)['resources'].pluck('guid')).to eq([own.guid]) + end + + it 'lists assigned apps only for account readers' do + own = VCAP::CloudController::ServiceAccountModel.create(name: 'payments-worker', space: space) + app = create(:app_model, space: space, service_account: own) + get "/v3/service_accounts/#{own.guid}/apps", nil, headers('space_auditor') + expect(last_response.status).to eq(200) + expect(Oj.load(last_response.body)['resources'].pluck('guid')).to eq([app.guid]) + get "/v3/service_accounts/#{own.guid}/apps?names=missing-app", nil, headers('space_auditor') + expect(Oj.load(last_response.body)['resources']).to eq([]) + end + + context 'common list filters' do + let!(:earlier_account) { VCAP::CloudController::ServiceAccountModel.create(name: 'earlier-worker', space: space) } + let!(:later_account) { VCAP::CloudController::ServiceAccountModel.create(name: 'later-worker', space: space) } + + before do + earlier_account.this.update(created_at: Time.utc(2026, 1, 1), updated_at: Time.utc(2026, 1, 1)) + later_account.this.update(created_at: Time.utc(2026, 1, 3), updated_at: Time.utc(2026, 1, 3)) + end + + it 'filters by GUID before pagination' do + get "/v3/service_accounts?guids=#{later_account.guid}&per_page=1", nil, headers('space_developer') + + expect(last_response.status).to eq(200) + result = Oj.load(last_response.body) + expect(result['resources'].pluck('guid')).to eq([later_account.guid]) + expect(result.dig('pagination', 'total_results')).to eq(1) + end + + it 'filters creation timestamps' do + get '/v3/service_accounts?created_ats[lt]=2026-01-02T00:00:00Z', nil, headers('space_developer') + + expect(last_response.status).to eq(200) + expect(Oj.load(last_response.body)['resources'].pluck('guid')).to eq([earlier_account.guid]) + end + + it 'filters update timestamps independently of creation time' do + earlier_account.this.update(updated_at: Time.utc(2026, 1, 4)) + + get '/v3/service_accounts?updated_ats[gte]=2026-01-04T00:00:00Z', nil, headers('space_developer') + + expect(last_response.status).to eq(200) + expect(Oj.load(last_response.body)['resources'].pluck('guid')).to eq([earlier_account.guid]) + end + + it 'intersects GUIDs with visibility and account-specific filters' do + hidden = VCAP::CloudController::ServiceAccountModel.create(name: 'hidden-worker', space: create(:space, :with_service_account_prefix)) + query = "guids=#{earlier_account.guid},#{hidden.guid}&organization_guids=#{org.guid}&space_guids=#{space.guid}&names=earlier-worker" + + get "/v3/service_accounts?#{query}", nil, headers('space_developer') + + expect(last_response.status).to eq(200) + expect(Oj.load(last_response.body)['resources'].pluck('guid')).to eq([earlier_account.guid]) + + get "/v3/service_accounts?guids=#{hidden.guid}", nil, headers('space_developer') + + expect(last_response.status).to eq(200) + expect(Oj.load(last_response.body)['resources']).to be_empty + end + end + + it 'loads presentation associations once per page rather than once per account' do + auth = headers('admin') + accounts = Array.new(3) do |index| + entitled_space = create(:space, :with_service_account_prefix) + account = VCAP::CloudController::ServiceAccountModel.create(name: "worker-#{index}", space: entitled_space) + VCAP::CloudController::ServiceAccountLabelModel.create(resource_guid: account.guid, key_name: 'team', value: "team-#{index}") + VCAP::CloudController::ServiceAccountAnnotationModel.create(resource_guid: account.guid, key_name: 'note', value: "note-#{index}") + [account, entitled_space] + end + + [1, 3].each do |per_page| + %w[organizations spaces service_account_labels service_account_annotations].each do |table| + expect do + get "/v3/service_accounts?per_page=#{per_page}", nil, auth + end.to have_queried_db_times(/SELECT .* FROM .#{table}. /i, 1) + end + + expect(last_response.status).to eq(200) + resources = Oj.load(last_response.body)['resources'] + expect(resources.size).to eq(per_page) + resources.each do |resource| + account, entitled_space = accounts.find { |entry| entry.first.guid == resource['guid'] } + expect(resource.dig('relationships', 'organization', 'data', 'guid')).to eq(entitled_space.organization.guid) + expect(resource.dig('relationships', 'spaces', 'data')).to eq([{ 'guid' => entitled_space.guid }]) + index = account.name.delete_prefix('worker-') + expected_metadata = { + 'labels' => { 'team' => "team-#{index}" }, + 'annotations' => { 'note' => "note-#{index}" } + } + expect(resource['metadata']).to eq(expected_metadata) + end + end + end + + it 'allows developer updates and hides unreadable resources' do + own = VCAP::CloudController::ServiceAccountModel.create(name: 'payments-worker', space: space) + patch "/v3/service_accounts/#{own.guid}", { description: 'unauthorized' }.to_json, headers('space_developer') + expect(last_response.status).to eq(200) + hidden = VCAP::CloudController::ServiceAccountModel.create(name: 'hidden-worker', space: create(:space, :with_service_account_prefix)) + patch "/v3/service_accounts/#{hidden.guid}", { description: 'unauthorized' }.to_json, headers('space_manager') + expect(last_response.status).to eq(404) + end + + %i[name relationships status client_id certificate_dns_san].each do |key| + it "rejects mutation of platform-owned #{key}" do + own = VCAP::CloudController::ServiceAccountModel.create(name: 'payments-worker', space: space) + patch "/v3/service_accounts/#{own.guid}", { key => 'injected' }.to_json, headers('space_developer') + expect(last_response.status).to eq(422) + end + end + + it 'rejects invalid metadata and list parameters' do + post '/v3/service_accounts', body.merge(metadata: { labels: { 'invalid/key/key' => 'value' } }).to_json, headers('admin') + expect(last_response.status).to eq(422) + get '/v3/service_accounts?unknown=value', nil, headers('admin') + expect(last_response.status).to eq(400) + end + + context 'account lifecycle' do + let(:account) { VCAP::CloudController::ServiceAccountModel.create(name: 'payments-worker', space: space) } + let(:clients) { instance_double(CF::UAA::Scim) } + let(:provisioner) { VCAP::CloudController::ServiceAccountProvision.new(clients, identity_ca_ref: 'diego-instance-identity') } + let(:account_path) { "/v3/service_accounts/#{account.guid}" } + + before do + TestConfig.override(service_accounts_enabled: true) + CloudController::DependencyLocator.instance.register(:service_account_provisioner, provisioner) + allow(clients).to receive(:get).and_raise(CF::UAA::NotFound) + allow(clients).to receive(:add) + allow(clients).to receive(:delete) + end + + it 'disables new authentication and enables again while retaining explicit principal roles' do + provisioner.provision(account) + principal = create(:user, guid: account.client_id, is_oauth_client: true) + org.add_user(principal) + space.add_developer(principal) + registration = provisioner.send(:registration, account) + allow(clients).to receive(:get).and_return(registration) + auth = headers('space_developer') + + patch account_path, { enabled: false }.to_json, auth + expect(last_response.status).to eq(202) + expect(account.reload.enabled).to be(false) + expect(Delayed::Worker.new.work_off).to eq([1, 0]) + expect(clients).to have_received(:delete).with(:client, account.client_id).once + expect(account.reload.status).to eq('disabled') + expect(principal.reload.spaces).to include(space) + + allow(clients).to receive(:get).and_raise(CF::UAA::NotFound) + patch account_path, { enabled: true }.to_json, auth + expect(last_response.status).to eq(202) + expect(Delayed::Worker.new.work_off).to eq([1, 0]) + expect(account.reload.status).to eq('ready') + expect(principal.reload.spaces).to include(space) + end + + it 'deletes an unused account asynchronously and keeps its name permanently reserved' do + provisioner.provision(account) + allow(clients).to receive(:get).and_return(provisioner.send(:registration, account)) + guid = account.guid + auth = headers('space_developer') + delete account_path, nil, auth + expect(last_response.status).to eq(202) + expect(account.reload.status).to eq('deleting') + expect(account.enabled).to be(false) + expect(Delayed::Worker.new.work_off).to eq([1, 0]) + expect(VCAP::CloudController::ServiceAccountModel.first(guid: guid)).to be_nil + expect(VCAP::CloudController::User.first(guid: account.client_id)).to be_nil + post '/v3/service_accounts', body.to_json, auth + expect(last_response.status).to eq(409) + end + + it 'rejects deleting an account still assigned to an app' do + create(:app_model, space: space, service_account: account) + delete account_path, nil, headers('space_developer') + expect(last_response.status).to eq(409) + expect(account.reload.status).to eq('reserved') + expect(Delayed::Job.count).to eq(0) + end + + it 'denies auditor deletion and rejects non-boolean enabled values' do + delete account_path, nil, headers('space_auditor') + expect(last_response.status).to eq(403) + patch account_path, { enabled: 'false' }.to_json, headers('space_developer') + expect(last_response.status).to eq(422) + end + + it 'refuses to delete an unmanaged colliding client' do + allow(clients).to receive(:get).and_return('client_id' => account.client_id) + delete account_path, nil, headers('space_developer') + expect(last_response.status).to eq(202) + expect(Delayed::Worker.new.work_off).to eq([0, 1]) + expect(clients).not_to have_received(:delete) + expect(account.reload.status).to eq('failed') + end + + it 'rejects deletion while an unbound app still has the account in a running launch snapshot' do + app = create(:app_model, space: space) + process = create(:process_model, app: app, state: 'STARTED') + process.update(service_account_guid: account.guid, service_account_snapshot: true) + delete account_path, nil, headers('space_developer') + expect(last_response.status).to eq(409) + expect(Delayed::Job.count).to eq(0) + end + + it 'rejects deletion while an active task holds the account snapshot' do + app = create(:app_model, space: space) + create(:task_model, app: app, service_account_guid: account.guid, service_account_snapshot: true, state: 'RUNNING') + delete account_path, nil, headers('space_developer') + expect(last_response.status).to eq(409) + expect(Delayed::Job.count).to eq(0) + end + end +end diff --git a/spec/request/spaces_spec.rb b/spec/request/spaces_spec.rb index c2c4a8aa503..a1c9f2020fe 100644 --- a/spec/request/spaces_spec.rb +++ b/spec/request/spaces_spec.rb @@ -160,6 +160,7 @@ 'created_at' => iso8601, 'updated_at' => iso8601, 'name' => org.name, + 'service_account_prefix' => org.service_account_prefix, 'suspended' => false, 'metadata' => { 'labels' => {}, @@ -414,6 +415,7 @@ 'created_at' => iso8601, 'updated_at' => iso8601, 'name' => org1.name, + 'service_account_prefix' => org1.service_account_prefix, 'suspended' => false, 'metadata' => { 'labels' => {}, @@ -440,6 +442,7 @@ 'created_at' => iso8601, 'updated_at' => iso8601, 'name' => org2.name, + 'service_account_prefix' => org2.service_account_prefix, 'suspended' => false, 'metadata' => { 'labels' => {}, diff --git a/spec/support/factory_definitions/space.rb b/spec/support/factory_definitions/space.rb index 8525f624c25..cf6bfcd43e1 100644 --- a/spec/support/factory_definitions/space.rb +++ b/spec/support/factory_definitions/space.rb @@ -2,5 +2,9 @@ factory :space, class: 'VCAP::CloudController::Space' do name { generate(:name) } association :organization + + trait :with_service_account_prefix do + organization { association(:organization, service_account_prefix: "workload-#{generate(:name)}") } + end end end diff --git a/spec/unit/actions/app_assign_service_account_spec.rb b/spec/unit/actions/app_assign_service_account_spec.rb new file mode 100644 index 00000000000..44ad4910cf1 --- /dev/null +++ b/spec/unit/actions/app_assign_service_account_spec.rb @@ -0,0 +1,117 @@ +require 'spec_helper' +require 'actions/app_assign_service_account' if File.exist?('app/actions/app_assign_service_account.rb') + +module VCAP::CloudController + RSpec.describe 'Entitled-space service account assignment' do + let(:space) { create(:space, organization: create(:organization, service_account_prefix: 'assignment-team')) } + let(:app) { create(:app_model, space: space) } + let(:account) { VCAP::CloudController.const_get(:ServiceAccountModel).create(name: 'payments-worker', space: space, status: 'ready') } + let(:permissions) { instance_double(Permissions, can_write_to_active_space?: true) } + let(:action) { VCAP::CloudController.const_get(:AppAssignServiceAccount).new(permissions) } + + it 'allows an app writer to use an account in an entitled space' do + action.assign(app, account) + expect(app.reload.service_account).to eq(account) + expect(app.desired_state).to eq('STOPPED') + end + + it 'denies callers without app-write permission' do + allow(permissions).to receive(:can_write_to_active_space?).and_return(false) + expect { action.assign(app, account) }.to raise_error(AppAssignServiceAccount::Unauthorized) + expect(app.reload.service_account).to be_nil + end + + it 'rejects an account in another space of the same organization' do + other = ServiceAccountModel.create(name: 'reporting-reader', space: create(:space, organization: space.organization), status: 'ready') + expect { action.assign(app, other) }.to raise_error(AppAssignServiceAccount::Conflict, /entitled/) + end + + it 'is idempotent but requires explicit unbinding before replacement' do + action.assign(app, account) + action.assign(app, account) + other = ServiceAccountModel.create(name: 'reporting-reader', space: space, status: 'ready') + expect { action.assign(app, other) }.to raise_error(AppAssignServiceAccount::Conflict, /already assigned/) + expect(app.reload.service_account).to eq(account) + end + + it 'allows an app writer to unbind' do + app.update(service_account: account) + action.assign(app, nil) + expect(app.reload.service_account).to be_nil + end + + it 'rejects disabled and unprovisioned accounts' do + account.update(enabled: false) + expect { action.assign(app, account) }.to raise_error(AppAssignServiceAccount::Conflict, /not ready/) + account.update(enabled: true, status: 'reserved') + expect { action.assign(app, account) }.to raise_error(AppAssignServiceAccount::Conflict, /not ready/) + end + + it 'rejects assignment when entitlement revocation commits after its permission snapshot', isolation: :truncation do + app_guid = app.guid + account_guid = account.guid + writer = create(:user) + space.organization.add_user(writer) + space.add_developer(writer) + permission_queryer = Permissions.new(writer) + permission_checked = Queue.new + allow(permission_queryer).to receive(:can_write_to_active_space?).and_wrap_original do |method, *args| + result = method.call(*args) + permission_checked << true + result + end + + assignment = nil + account.db.transaction do + account.lock! + assignment = Thread.new do + AppAssignServiceAccount.new(permission_queryer).assign(AppModel.first(guid: app_guid), ServiceAccountModel.first(guid: account_guid)) + rescue AppAssignServiceAccount::Conflict => e + e + end + Timeout.timeout(10) { permission_checked.pop } + account.remove_entitled_space(space) + end + + result = Timeout.timeout(10) { assignment.value } + expect(result).to be_a(AppAssignServiceAccount::Conflict) + expect(result.message).to include('entitled') + expect(app.reload.service_account).to be_nil + expect(account.reload.entitled_spaces).to be_empty + ensure + assignment&.kill if assignment&.alive? + assignment&.join + end + + it 'serializes concurrent first binds to two apps into a single provisioning job', isolation: :truncation do + account.update(status: 'reserved') + other_app = create(:app_model, space: space) + ids = [app.guid, other_app.guid] + writer = create(:user) + space.organization.add_user(writer) + space.add_developer(writer) + permission_queryer = Permissions.new(writer) + permission_checks = Queue.new + continue_assignments = Queue.new + allow(permission_queryer).to receive(:can_write_to_active_space?).and_wrap_original do |method, *args| + result = method.call(*args) + # Both transactions must read before either locks the shared account. On MySQL, + # this establishes repeatable-read snapshots that cannot see the other job. + permission_checks << true + continue_assignments.pop + result + end + threads = ids.map do |guid| + Thread.new do + AppAssignServiceAccount.new(permission_queryer).assign(AppModel.first(guid: guid), ServiceAccountModel.first(guid: account.guid), provision: true) + end + end + Timeout.timeout(10) { ids.size.times { permission_checks.pop } } + ids.size.times { continue_assignments << true } + jobs = threads.map(&:value) + expect(jobs.map(&:guid).uniq.size).to eq(1) + expect(PollableJobModel.where(resource_guid: account.guid).count).to eq(1) + expect(AppModel.where(guid: ids).select_map(:service_account_guid)).to eq([account.guid, account.guid]) + end + end +end diff --git a/spec/unit/actions/organization_delete_spec.rb b/spec/unit/actions/organization_delete_spec.rb index 3d6329a71e8..9afe3c07d75 100644 --- a/spec/unit/actions/organization_delete_spec.rb +++ b/spec/unit/actions/organization_delete_spec.rb @@ -73,6 +73,42 @@ module VCAP::CloudController end describe 'recursive deletion' do + context 'service accounts' do + let!(:account) do + space.organization.update(service_account_prefix: 'org-delete-team') + ServiceAccountModel.create(name: 'org-worker', space: space) + end + let(:clients) { instance_double(CF::UAA::Scim) } + let(:provisioner) { ServiceAccountProvision.new(clients, identity_ca_ref: 'diego-instance-identity') } + + before do + TestConfig.override(service_accounts_enabled: true) + CloudController::DependencyLocator.instance.register(:service_account_provisioner, provisioner) + allow(clients).to receive(:get).and_raise(CF::UAA::NotFound) + allow(clients).to receive(:delete) + end + + it 'cascades account deletion through spaces and retains the name tombstone' do + app.update(service_account: account) + expect(org_delete.delete(org_dataset)).to be_empty + expect(ServiceAccountModel.first(guid: account.guid)).to be_nil + expect(Organization.first(guid: org_1.guid)).to be_nil + expect(ServiceAccountModel.db[:service_account_names].where(name: account.name).count).to eq(1) + end + + it 'keeps the organization until account cleanup succeeds' do + account.update(status: 'ready') + allow(clients).to receive(:get).and_raise('UAA unavailable') + errors = org_delete.delete(org_dataset) + expect(errors.map(&:message).join).to include('UAA unavailable') + expect(Organization.first(guid: org_1.guid)).not_to be_nil + expect(Space.first(guid: space.guid)).to be_nil + allow(clients).to receive(:get).and_raise(CF::UAA::NotFound) + expect(org_delete.delete(org_dataset)).to be_empty + expect(Organization.first(guid: org_1.guid)).to be_nil + end + end + it 'deletes any spaces in the org' do expect do org_delete.delete(org_dataset) diff --git a/spec/unit/actions/organization_service_account_prefix_spec.rb b/spec/unit/actions/organization_service_account_prefix_spec.rb new file mode 100644 index 00000000000..3c4aea24168 --- /dev/null +++ b/spec/unit/actions/organization_service_account_prefix_spec.rb @@ -0,0 +1,47 @@ +require 'spec_helper' +require 'actions/organization_create' +require 'actions/organization_update' + +module VCAP::CloudController + RSpec.describe 'Atomic organization service account prefix allocation' do + let(:user_audit_info) { UserAuditInfo.new(user_guid: create(:user).guid, user_email: 'admin@example.com') } + + it 'allows exactly one concurrent creator to reserve a prefix', isolation: :truncation do + audit_info = user_audit_info + gate = Queue.new + threads = 2.times.map do |index| + Thread.new do + gate.pop + message = OrganizationCreateMessage.new(name: "org-#{index}", service_account_prefix: 'shared-prefix') + OrganizationCreate.new(user_audit_info: audit_info).create(message) + :created + rescue OrganizationCreate::Error => e + e.message + end + end + threads.size.times { gate << true } + + expect(threads.map(&:value)).to contain_exactly(:created, "Service account prefix 'shared-prefix' is already reserved.") + expect(Organization.where(name: %w[org-0 org-1]).count).to eq(1) + end + + it 'serializes competing allocations on the same initially unset org', isolation: :truncation do + org = create(:organization) + audit_info = user_audit_info + gate = Queue.new + threads = %w[first-prefix second-prefix].map do |prefix| + Thread.new do + stale_org = Organization.first(guid: org.guid) + gate.pop + OrganizationUpdate.new(audit_info).update(stale_org, OrganizationUpdateMessage.new(service_account_prefix: prefix)) + :updated + rescue OrganizationUpdate::Error => e + e.message + end + end + threads.size.times { gate << true } + + expect(threads.map(&:value)).to contain_exactly(:updated, 'Service account prefix is immutable once allocated.') + end + end +end diff --git a/spec/unit/actions/process_restart_spec.rb b/spec/unit/actions/process_restart_spec.rb index 2f659ed34ee..43fb3a7286b 100644 --- a/spec/unit/actions/process_restart_spec.rb +++ b/spec/unit/actions/process_restart_spec.rb @@ -49,6 +49,26 @@ module VCAP::CloudController end context 'when the process is STARTED' do + it 'does not grant a newly bound account to a pre-feature launch before restart' do + app.space.organization.update(service_account_prefix: 'process-team') + account = ServiceAccountModel.create(name: 'payments-worker', space: app.space, status: 'ready') + app.update(service_account: account) + process.this.update(service_account_snapshot: false, service_account_guid: nil) + expect(process.reload.runtime_service_account_guid).to be_nil + end + + it 'captures account identity only at restart and preserves it after delayed assignment changes' do + app.space.organization.update(service_account_prefix: 'process-team') + account = ServiceAccountModel.create(name: 'payments-worker', space: app.space, status: 'ready') + app.update(service_account: account) + ProcessRestart.restart(process: process, config: config, stop_in_runtime: true) + expect(process.reload.runtime_service_account_guid).to eq(account.guid) + app.update(service_account: nil) + expect(process.reload.runtime_service_account_guid).to eq(account.guid) + ProcessRestart.restart(process: process, config: config, stop_in_runtime: true) + expect(process.reload.runtime_service_account_guid).to be_nil + end + it 'keeps process state as STARTED' do ProcessRestart.restart(process: process, config: config, stop_in_runtime: true) expect(process.reload.state).to eq('STARTED') diff --git a/spec/unit/actions/service_account_binding_guard_spec.rb b/spec/unit/actions/service_account_binding_guard_spec.rb new file mode 100644 index 00000000000..78a5eff5dc2 --- /dev/null +++ b/spec/unit/actions/service_account_binding_guard_spec.rb @@ -0,0 +1,76 @@ +require 'spec_helper' +require 'actions/app_assign_service_account' + +module VCAP::CloudController + RSpec.describe 'Service account identity binding guard' do + it 'keeps user-provided bindings credential-only and allows account unassignment' do + space = create(:space, :with_service_account_prefix) + account = ServiceAccountModel.create(name: 'binding-worker', space: space, status: 'ready') + app = create(:app_model, space: space, service_account: account) + credentials = { 'host' => 'database.example.test', 'username' => 'worker' } + binding = create(:service_binding, app: app, + service_instance: create(:user_provided_service_instance, space: space), credentials: credentials) + + expect(binding.reload.service_account_guid).to be_nil + AppAssignServiceAccount.new(double(can_write_to_active_space?: true)).assign(app, nil) + + expect(app.reload.service_account_guid).to be_nil + expect(account.in_use?).to be(false) + expect(account.in_use_in_space?(space)).to be(false) + expect(binding.reload.credentials).to eq(credentials) + end + + it 'does not require account entitlement to create a credential-only user-provided binding' do + space = create(:space, :with_service_account_prefix) + account = ServiceAccountModel.create(name: 'binding-worker', space: space, status: 'ready') + app = create(:app_model, space: space, service_account: account) + account.remove_all_entitled_spaces + + binding = create(:service_binding, app: app, service_instance: create(:user_provided_service_instance, space: space)) + + expect(binding.reload.service_account_guid).to be_nil + end + + it 'rejects binding snapshots whose account entitlement was revoked before persistence' do + space = create(:space, :with_service_account_prefix) + account = ServiceAccountModel.create(name: 'binding-worker', space: space, status: 'ready') + app = create(:app_model, space: space, service_account: account) + account.remove_all_entitled_spaces + expect do + create(:service_binding, app: app, service_instance: create(:managed_service_instance, space: space)) + end.to raise_error(Sequel::ValidationFailed, /entitled/) + end + + it 'counts identity-bearing bindings as account lifecycle references' do + space = create(:space, :with_service_account_prefix) + account = ServiceAccountModel.create(name: 'binding-worker', space: space, status: 'ready') + app = create(:app_model, space: space, service_account: account) + create(:service_binding, app: app, service_instance: create(:managed_service_instance, space: space)) + app.update(service_account: nil) + expect(account.in_use?).to be(true) + expect(account.in_use_in_space?(space)).to be(true) + end + + it 'allows removing an assignment when the binding predates account assignment' do + space = create(:space, organization: create(:organization, service_account_prefix: 'binding-team')) + app = create(:app_model, space: space) + create(:service_binding, app: app, service_instance: create(:managed_service_instance, space: space)) + account = ServiceAccountModel.create(name: 'binding-worker', space: space, status: 'ready') + permissions = double(can_write_to_active_space?: true) + action = AppAssignServiceAccount.new(permissions) + action.assign(app, account) + expect { action.assign(app, nil) }.not_to raise_error + end + + it 'refuses to unassign an account while a broker binding depends on the identity' do + space = create(:space, :with_service_account_prefix) + account = ServiceAccountModel.create(name: 'binding-worker', space: space, status: 'ready') + app = create(:app_model, space: space, service_account: account) + create(:service_binding, app: app, service_instance: create(:managed_service_instance, space: space)) + permissions = double(can_write_to_active_space?: true) + + expect { AppAssignServiceAccount.new(permissions).assign(app, nil) }.to raise_error(AppAssignServiceAccount::Conflict, /bindings/) + expect(app.reload.service_account_guid).to eq(account.guid) + end + end +end diff --git a/spec/unit/actions/service_account_creation_budget_spec.rb b/spec/unit/actions/service_account_creation_budget_spec.rb new file mode 100644 index 00000000000..6e0b81230ef --- /dev/null +++ b/spec/unit/actions/service_account_creation_budget_spec.rb @@ -0,0 +1,39 @@ +require 'spec_helper' +require 'actions/service_account_creation_budget' + +module VCAP::CloudController + RSpec.describe ServiceAccountCreationBudget do + it 'serializes concurrent creations by the same principal across spaces', isolation: :truncation do + TestConfig.override(service_account_creation_limit: 1) + principal = create(:user) + spaces = create_list(:space, 2, :with_service_account_prefix) + gate = Queue.new + snapshots = Queue.new + consume_budget = Queue.new + threads = spaces.each_with_index.map do |space, index| + Thread.new do + gate.pop + ServiceAccountModel.db.transaction do + authenticated_principal = User.first(guid: principal.guid) + snapshots << true + consume_budget.pop + described_class.consume(authenticated_principal, exempt: false) do + ServiceAccountModel.create(name: "worker-#{index}", space: space) + end + end + :created + rescue CloudController::Errors::V3::ApiError => e + e.name + end + end + threads.size.times { gate << true } + Timeout.timeout(10) { threads.size.times { snapshots.pop } } + threads.size.times { consume_budget << true } + + expect(threads.map(&:value)).to contain_exactly(:created, 'ServiceAccountCreationLimitExceeded') + expect(ServiceAccountModel.count).to eq(1) + expect(ServiceAccountModel.db[:service_account_creations].count).to eq(1) + expect(ServiceAccountModel.db[:service_account_names].count).to eq(1) + end + end +end diff --git a/spec/unit/actions/service_account_org_lifecycle_spec.rb b/spec/unit/actions/service_account_org_lifecycle_spec.rb new file mode 100644 index 00000000000..15a4e111511 --- /dev/null +++ b/spec/unit/actions/service_account_org_lifecycle_spec.rb @@ -0,0 +1,103 @@ +require 'spec_helper' +require 'actions/organization_delete' + +module VCAP::CloudController + RSpec.describe 'Org-owned service account lifecycle' do + let(:space) { create(:space, organization: create(:organization, service_account_prefix: 'lifecycle-team')) } + let(:org) { space.organization } + let(:account) { ServiceAccountModel.create(name: 'shared-worker', space: space) } + let(:actor) { UserAuditInfo.new(user_guid: create(:user).guid, user_email: 'manager@example.com') } + let(:space_delete) { SpaceDelete.new(actor, Repositories::ServiceEventRepository.new(actor)) } + let(:org_delete) { OrganizationDelete.new(space_delete, actor) } + let(:clients) { instance_double(CF::UAA::Scim) } + let(:provisioner) { ServiceAccountProvision.new(clients, identity_ca_ref: 'diego-instance-identity') } + + before do + TestConfig.override(kubernetes: {}) + CloudController::DependencyLocator.instance.register(:service_account_provisioner, provisioner) + allow(clients).to receive(:get).and_raise(CF::UAA::NotFound) + allow(clients).to receive(:add) + allow(clients).to receive(:delete) + end + + it 'keeps the enabled account, principal and grants when its last space is deleted' do + provisioner.provision(account) + principal = create(:user, guid: account.client_id, is_oauth_client: true) + org.add_user(principal) + app = create(:app_model, space: space, service_account: account) + + expect(space_delete.delete([space])).to be_empty + + expect(account.reload.organization).to eq(org) + expect(account.enabled).to be(true) + expect(account.status).to eq('ready') + expect(account.entitled_spaces).to be_empty + expect(User.first(guid: principal.guid)).not_to be_nil + expect(org.users_dataset.where(users__id: principal.id).any?).to be(true) + expect(AppModel.first(guid: app.guid)).to be_nil + expect(clients).not_to have_received(:delete) + end + + it 'cleans up a zero-entitlement account on org deletion, retaining the parent after failure for retry' do + provisioner.provision(account) + expect(space_delete.delete([space])).to be_empty + allow(clients).to receive(:get).and_raise('UAA unavailable') + + expect(org_delete.delete([org]).map(&:message).join).to include('UAA unavailable') + expect(Organization.first(id: org.id)).not_to be_nil + expect(org.reload.status).to eq(Organization::DELETING) + expect(account.reload.status).to eq('failed') + + allow(clients).to receive(:get).and_raise(CF::UAA::NotFound) + expect(org_delete.delete([org])).to be_empty + expect(ServiceAccountModel.first(guid: account.guid)).to be_nil + expect(Organization.first(id: org.id)).to be_nil + end + + it 'deletes an unused reservation without provisioning configuration and retains its deletion history' do + guid = account.guid + org_guid = org.guid + locator = CloudController::DependencyLocator.instance + expect(locator).not_to receive(:service_account_provisioner) + + expect(org_delete.delete([org])).to be_empty + + expect(ServiceAccountModel.first(guid: guid)).to be_nil + expect(Organization.first(id: org.id)).to be_nil + expect(ServiceAccountModel.db[:service_account_names].where(name: account.name, organization_id: org.id).first[:deleted_at]).not_to be_nil + expect(ServiceAccountModel.db[:service_account_name_deletions].where(account_guid: guid, organization_guid: org_guid)).not_to be_empty + expect(Event.where(type: 'audit.service_account.delete', actee: guid)).not_to be_empty + expect(clients).not_to have_received(:get) + end + + it 'retains a failed account and its org when provisioning configuration is unavailable' do + account.update(status: 'failed') + allow(CloudController::DependencyLocator.instance).to receive(:service_account_provisioner).and_raise('service account provisioning is not configured') + + expect(org_delete.delete([org]).map(&:message).join).to include('service account provisioning is not configured') + + expect(ServiceAccountModel.first(guid: account.guid)).not_to be_nil + expect(org.reload.status).to eq(Organization::DELETING) + end + + it 'does not locally delete a reservation with an active lifecycle job' do + job = Jobs::Enqueuer.new(queue: Jobs::Queues.generic).enqueue_pollable(Jobs::V3::ServiceAccountProvision.new(account.guid)) + expect(CloudController::DependencyLocator.instance).not_to receive(:service_account_provisioner) + + expect(org_delete.delete([org]).map(&:message).join).to include('operation is already in progress') + + expect(account.reload.status).to eq('reserved') + expect(PollableJobModel.first(guid: job.guid)).not_to be_nil + expect(org.reload.status).to eq(Organization::DELETING) + end + + it 'does not bypass shared-domain deletion guards when accounts mark an org deleting' do + account + domain = create(:private_domain, owning_organization: org) + domain.add_shared_organization(create(:organization)) + expect(org_delete.delete([org]).map(&:message).join).to include('shared') + expect(Organization.first(id: org.id)).not_to be_nil + expect(PrivateDomain.first(guid: domain.guid)).not_to be_nil + end + end +end diff --git a/spec/unit/actions/service_account_provision_spec.rb b/spec/unit/actions/service_account_provision_spec.rb new file mode 100644 index 00000000000..82686a0b0ea --- /dev/null +++ b/spec/unit/actions/service_account_provision_spec.rb @@ -0,0 +1,265 @@ +require 'spec_helper' +require 'actions/service_account_provision' if File.exist?('app/actions/service_account_provision.rb') + +module VCAP::CloudController + RSpec.describe 'Service account provisioning' do + let(:account) { ServiceAccountModel.create(name: 'payments-worker', space: create(:space, :with_service_account_prefix)) } + let(:clients) { instance_double(CF::UAA::Scim) } + let(:action) { VCAP::CloudController.const_get(:ServiceAccountProvision).new(clients, identity_ca_ref: 'diego-instance-identity') } + let(:registration) do + { + 'client_id' => account.client_id, + 'authorized_grant_types' => ['client_credentials'], + 'authorities' => %w[cloud_controller.read cloud_controller.write], + 'scope' => [], + 'access_token_validity' => 300, + 'tls-client-auth-ca-ref' => 'diego-instance-identity', + 'tls_client_auth_san_dns' => account.certificate_dns_san, + 'tls_client_certificate_bound_access_tokens' => false, + 'tls-client-auth-required-claims' => { 'org_guid' => account.organization.guid }, + 'tls-client-auth-claim-mappings' => [ + { 'field' => 'subject_cn', 'claim' => 'cf_instance_guid' }, + { 'field' => 'subject_ou', 'pattern' => '^app:(.+)$', 'claim' => 'app_guid' }, + { 'field' => 'subject_ou', 'pattern' => '^space:(.+)$', 'claim' => 'space_guid' }, + { 'field' => 'subject_ou', 'pattern' => '^organization:(.+)$', 'claim' => 'org_guid' } + ], + 'cf_service_account_guid' => account.guid + } + end + + it 'creates one canonical client without creating a CAPI user before marking ready' do + allow(clients).to receive(:get).with(:client, account.client_id).and_raise(CF::UAA::NotFound) + expect(clients).to receive(:add).with(:client, registration) + action.provision(account) + + expect(User.first(guid: account.client_id)).to be_nil + expect(account.reload.status).to eq('ready') + end + + it 'reuses a matching managed registration after a partial failure or retry' do + allow(clients).to receive(:get).with(:client, account.client_id).and_return(registration) + expect(clients).not_to receive(:add) + action.provision(account) + action.provision(account) + expect(User.where(guid: account.client_id).count).to eq(0) + end + + it 'accepts UAA normalized inert scope during disable reconciliation' do + account.update(enabled: false) + allow(clients).to receive(:get).and_return(registration.merge('scope' => ['uaa.none'])) + expect(clients).to receive(:delete).with(:client, account.client_id) + action.deprovision(account) + expect(account.reload.status).to eq('disabled') + end + + it 'does not adopt an existing unmanaged client even if its SAN matches' do + allow(clients).to receive(:get).and_return(registration.except('cf_service_account_guid')) + expect(clients).not_to receive(:add) + expect { action.provision(account) }.to raise_error(/client identity collision/) + expect(account.reload.status).to eq('failed') + expect(User.first(guid: account.client_id)).to be_nil + end + + it 'refuses a managed registration with altered trust policy' do + allow(clients).to receive(:get).and_return(registration.merge('tls_client_auth_san_dns' => 'other.svc.identity')) + expect { action.provision(account) }.to raise_error(/client identity collision/) + expect(account.reload.status).to eq('failed') + end + + it 'leaves a visible failed state on UAA failure and can retry' do + allow(clients).to receive(:get).and_raise(CF::UAA::NotFound) + allow(clients).to receive(:add).and_raise(StandardError, 'unavailable') + expect { action.provision(account) }.to raise_error('unavailable') + expect(account.reload.status).to eq('failed') + expect(User.first(guid: account.client_id)).to be_nil + allow(clients).to receive(:add).and_return(registration) + action.provision(account) + expect(account.reload.status).to eq('ready') + end + + it 'refuses to repurpose an existing human principal' do + create(:user, guid: account.client_id, is_oauth_client: false) + expect(clients).not_to receive(:get) + expect { action.provision(account) }.to raise_error(/principal identity collision/) + expect(account.reload.status).to eq('failed') + end + + it 'leaves independently managed CAPI users and their roles untouched during provisioning' do + principal = create(:user, guid: account.client_id, is_oauth_client: true) + space = create(:space) + space.organization.add_user(principal) + space.add_developer(principal) + allow(clients).to receive(:get).and_raise(CF::UAA::NotFound) + allow(clients).to receive(:add) + + action.provision(account) + + expect(principal.reload.spaces).to include(space) + expect(account.reload.status).to eq('ready') + end + + it 'does not delete an existing OAuth principal when deleting a never-provisioned account' do + principal = create(:user, guid: account.client_id, is_oauth_client: true) + space = create(:space) + space.organization.add_user(principal) + space.add_developer(principal) + account.update(enabled: false) + allow(clients).to receive(:get).and_raise(CF::UAA::NotFound) + + action.deprovision(account, delete: true) + + expect(ServiceAccountModel.first(guid: account.guid)).to be_nil + expect(principal.reload.spaces).to include(space) + end + + it 'does not create or modify a CAPI principal when re-enabling a previously provisioned account' do + allow(clients).to receive(:get).and_raise(CF::UAA::NotFound) + allow(clients).to receive(:add) + action.provision(account) + principal = create(:user, guid: account.client_id, is_oauth_client: true, active: true) + space = create(:space) + space.organization.add_user(principal) + space.add_developer(principal) + account.update(enabled: false) + action.deprovision(account) + expect(principal.reload.active).to be(true) + expect(principal.spaces).to include(space) + principal.destroy + account.update(enabled: true) + + action.provision(account) + + expect(User.first(guid: account.client_id)).to be_nil + expect(account.reload.status).to eq('ready') + end + + it 'retains the independently managed CAPI user and its roles on account deletion' do + allow(clients).to receive(:get).and_raise(CF::UAA::NotFound) + allow(clients).to receive(:add) + action.provision(account) + principal = create(:user, guid: account.client_id, is_oauth_client: true) + space = create(:space) + space.organization.add_user(principal) + space.add_developer(principal) + account.update(enabled: false) + + action.deprovision(account, delete: true) + + expect(User.first(guid: principal.guid).id).to eq(principal.id) + expect(principal.reload.spaces).to include(space) + end + + it 'preserves explicit roles through failed re-enable and retry' do + allow(clients).to receive(:get).and_raise(CF::UAA::NotFound) + allow(clients).to receive(:add) + action.provision(account) + principal = create(:user, guid: account.client_id, is_oauth_client: true) + space = create(:space) + space.organization.add_user(principal) + space.add_developer(principal) + account.update(enabled: false) + action.deprovision(account) + account.update(enabled: true) + allow(clients).to receive(:add).and_raise('UAA unavailable') + + expect { action.provision(account) }.to raise_error('UAA unavailable') + expect(principal.reload.spaces).to include(space) + allow(clients).to receive(:add) + action.provision(account) + + expect(User.first(guid: account.client_id).id).to eq(principal.id) + expect(principal.reload.spaces).to include(space) + expect(account.reload.status).to eq('ready') + end + + it 'does not provision disabled accounts' do + account.update(enabled: false) + expect(clients).not_to receive(:get) + expect { action.provision(account) }.to raise_error(/disabled/) + expect(account.reload.status).to eq('reserved') + end + + it 'never registers inline CA certificates' do + allow(clients).to receive(:get).and_raise(CF::UAA::NotFound) + expect(clients).to receive(:add) do |_, payload| + expect(payload).not_to have_key('tls-client-auth-ca') + expect(payload['tls-client-auth-ca-ref']).to eq('diego-instance-identity') + end + action.provision(account) + end + + [ + { 'tls-client-auth-ca-ref' => 'previous-bundle' }, + { 'tls-client-auth-ca-ref' => nil, 'tls-client-auth-ca' => 'inline-ca' } + ].each do |altered_trust| + it "refuses its own marked client with a different trust anchor (#{altered_trust.compact.keys.first})" do + allow(clients).to receive(:get).and_return(registration.merge(altered_trust).compact) + expect(clients).not_to receive(:add) + expect { action.provision(account) }.to raise_error(/client identity collision/) + expect(account.reload.status).to eq('failed') + end + end + + it 'only supports UAA trust bundle references' do + expect { ServiceAccountProvision.new(clients, identity_ca: 'inline-ca') }.to raise_error(ArgumentError) + end + + [nil, '', ' '].each do |reference| + it "requires a nonblank trust bundle reference (#{reference.inspect})" do + expect { ServiceAccountProvision.new(clients, identity_ca_ref: reference) }. + to raise_error(ArgumentError, /identity_ca_ref must name a UAA trust bundle/) + end + end + + context 'with the real UAA client library' do + let(:uaa_url) { 'https://service-account-uaa.example.test' } + let(:clients) { CF::UAA::Scim.new(uaa_url, 'bearer test-token') } + let(:client_url) { "#{uaa_url}/oauth/clients/#{account.client_id}" } + let(:response_headers) { { 'content-type' => 'application/json' } } + + it 'serializes the canonical secretless registration over HTTP' do + WebMock::API.stub_request(:get, client_url).to_return(status: 404) + creation = WebMock::API.stub_request(:post, "#{uaa_url}/oauth/clients"). + with(headers: { 'Authorization' => 'bearer test-token' }) { |request| Oj.load(request.body) == registration }. + to_return(status: 201, headers: response_headers, body: registration.to_json) + + action.provision(account) + + expect(creation).to have_been_requested.once + expect(account.reload.status).to eq('ready') + end + + it 'reuses a client with omitted empty scopes and reordered authorities on repeated reconciliation' do + response = registration.except('scope').merge('authorities' => registration['authorities'].reverse) + WebMock::API.stub_request(:get, client_url). + to_return(headers: response_headers, body: response.to_json) + + action.provision(account) + action.provision(account) + + expect(account.reload.status).to eq('ready') + expect(User.where(guid: account.client_id).count).to eq(0) + expect(WebMock::API.a_request(:post, "#{uaa_url}/oauth/clients")).not_to have_been_made + end + + [ + { 'scope' => ['clients.admin'] }, + { 'authorities' => %w[cloud_controller.read cloud_controller.write cloud_controller.admin] }, + { 'tls-client-auth-sub-template' => 'admin' }, + { 'tls-client-auth-aud-templates' => ['other-api'] }, + { 'tls_client_auth_subject_dn' => 'CN=other' } + ].each do |altered_policy| + it "refuses an existing client with altered #{altered_policy.keys.first}" do + WebMock::API.stub_request(:get, client_url). + to_return(headers: response_headers, body: registration.merge(altered_policy).to_json) + + expect { action.provision(account) }.to raise_error(ServiceAccountProvision::Conflict, 'client identity collision') + + expect(account.reload.status).to eq('failed') + expect(User.first(guid: account.client_id)).to be_nil + expect(WebMock::API.a_request(:post, "#{uaa_url}/oauth/clients")).not_to have_been_made + end + end + end + end +end diff --git a/spec/unit/actions/service_account_quota_concurrency_spec.rb b/spec/unit/actions/service_account_quota_concurrency_spec.rb new file mode 100644 index 00000000000..58248e3ccf8 --- /dev/null +++ b/spec/unit/actions/service_account_quota_concurrency_spec.rb @@ -0,0 +1,33 @@ +require 'spec_helper' + +module VCAP::CloudController + RSpec.describe 'Service account quota concurrency' do + it 'checks the current quota rather than an association cached before quota lowering' do + org = create(:organization, service_account_prefix: 'race-team') + space = create(:space, organization: org) + cached_quota = org.quota_definition + account = ServiceAccountModel.new(name: 'worker-app', space: space, organization: org) + QuotaDefinition.where(id: cached_quota.id).update(total_service_accounts: 0) + expect { account.save }.to raise_error(Sequel::ValidationFailed, /quota/) + end + + it 'serializes direct account creations within an org', isolation: :truncation do + org = create(:organization, service_account_prefix: 'race-team') + org.quota_definition.update(total_service_accounts: 1) + space = create(:space, organization: org) + gate = Queue.new + threads = 2.times.map do |index| + Thread.new do + gate.pop + ServiceAccountModel.create(name: "worker-#{index}", space: Space.first(guid: space.guid)) + :created + rescue Sequel::ValidationFailed + :quota_exceeded + end + end + threads.size.times { gate << true } + expect(threads.map(&:value)).to contain_exactly(:created, :quota_exceeded) + expect(ServiceAccountModel.where(organization_id: org.id).count).to eq(1) + end + end +end diff --git a/spec/unit/actions/service_account_registration_profile_spec.rb b/spec/unit/actions/service_account_registration_profile_spec.rb new file mode 100644 index 00000000000..931c8ffa35f --- /dev/null +++ b/spec/unit/actions/service_account_registration_profile_spec.rb @@ -0,0 +1,38 @@ +require 'spec_helper' +require 'actions/service_account_provision' + +module VCAP::CloudController + RSpec.describe 'Service account UAA attribution profile' do + it 'registers the operator named resource policy without copying audience lists' do + org = create(:organization, service_account_prefix: 'policy-team') + account = ServiceAccountModel.create(name: 'policy-worker', space: create(:space, organization: org)) + clients = instance_double(CF::UAA::Scim) + allow(clients).to receive(:get).and_raise(CF::UAA::NotFound) + allow(clients).to receive(:add) + ServiceAccountProvision.new(clients, identity_ca_ref: 'diego-instance-identity', allowed_resources_ref: 'cf-workload-federation').provision(account) + expect(clients).to have_received(:add).with(:client, hash_including('tls-client-auth-allowed-resources-ref' => 'cf-workload-federation')) + end + + it 'registers an explicitly bearer account restricted to its owning org with caller attribution' do + org = create(:organization, service_account_prefix: 'payments-team') + account = ServiceAccountModel.create(name: 'payments-worker', space: create(:space, organization: org)) + clients = instance_double(CF::UAA::Scim) + allow(clients).to receive(:get).and_raise(CF::UAA::NotFound) + allow(clients).to receive(:add) + + ServiceAccountProvision.new(clients, identity_ca_ref: 'diego-instance-identity').provision(account) + + profile = { + 'tls_client_certificate_bound_access_tokens' => false, + 'tls-client-auth-required-claims' => { 'org_guid' => org.guid }, + 'tls-client-auth-claim-mappings' => [ + { 'field' => 'subject_cn', 'claim' => 'cf_instance_guid' }, + { 'field' => 'subject_ou', 'pattern' => '^app:(.+)$', 'claim' => 'app_guid' }, + { 'field' => 'subject_ou', 'pattern' => '^space:(.+)$', 'claim' => 'space_guid' }, + { 'field' => 'subject_ou', 'pattern' => '^organization:(.+)$', 'claim' => 'org_guid' } + ] + } + expect(clients).to have_received(:add).with(:client, hash_including(profile)) + end + end +end diff --git a/spec/unit/actions/service_credential_binding_app_create_spec.rb b/spec/unit/actions/service_credential_binding_app_create_spec.rb index 15c9507b7b3..0847efaeeec 100644 --- a/spec/unit/actions/service_credential_binding_app_create_spec.rb +++ b/spec/unit/actions/service_credential_binding_app_create_spec.rb @@ -178,7 +178,7 @@ module V3 # We mock that a second binding is created after the first one acquires a lock and expect an `UnprocessableCreate` error. allow(app).to receive(:lock!).and_wrap_original do |m, *args, &block| m.call(*args, &block) - create(:service_binding, service_instance:, app:) + create(:service_binding, service_instance: service_instance, app: app.class.find(guid: app.guid)) end expect do diff --git a/spec/unit/actions/space_delete_spec.rb b/spec/unit/actions/space_delete_spec.rb index f09e6c59980..5f7f2d2745f 100644 --- a/spec/unit/actions/space_delete_spec.rb +++ b/spec/unit/actions/space_delete_spec.rb @@ -27,6 +27,91 @@ module VCAP::CloudController expect { space.refresh }.to raise_error Sequel::Error, 'Record not found' end + context 'owned service accounts' do + let!(:account) do + space.organization.update(service_account_prefix: 'space-delete-team') + ServiceAccountModel.create(name: 'payments-worker', space: space) + end + let(:clients) { instance_double(CF::UAA::Scim) } + let(:provisioner) { ServiceAccountProvision.new(clients, identity_ca_ref: 'diego-instance-identity') } + + before do + TestConfig.override(service_accounts_enabled: true) + CloudController::DependencyLocator.instance.register(:service_account_provisioner, provisioner) + allow(clients).to receive(:get).and_raise(CF::UAA::NotFound) + allow(clients).to receive(:add) + allow(clients).to receive(:delete) + end + + it 'deletes assigned workloads while retaining the account and principal' do + provisioner.provision(account) + principal = create(:user, guid: account.client_id, is_oauth_client: true) + space.organization.add_user(principal) + space.add_auditor(principal) + app.update(service_account: account) + process = create(:process_model, app: app, state: ProcessModel::STARTED, service_account_guid: account.guid, service_account_snapshot: true) + task = create(:task_model, app: app, state: TaskModel::RUNNING_STATE, service_account_guid: account.guid, service_account_snapshot: true) + allow(clients).to receive(:get).and_return(provisioner.send(:registration, account)) + allow(clients).to receive(:delete) do + expect(AppModel.first(guid: app.guid)).to be_nil + expect(ProcessModel.first(guid: process.guid)).to be_nil + expect(TaskModel.first(guid: task.guid)).to be_nil + expect(Space.first(guid: space.guid)).not_to be_nil + end + + expect(space_delete.delete([space])).to be_empty + + expect(clients).not_to have_received(:delete) + expect(account.reload.status).to eq('ready') + expect(account.entitled_spaces).to be_empty + expect(User.first(guid: principal.guid)).not_to be_nil + expect(Space.first(guid: space.guid)).to be_nil + expect(ServiceAccountModel.db[:service_account_names].where(name: account.name).count).to eq(1) + expect(Event.where(type: 'audit.service_account.delete', actee: account.guid).count).to eq(0) + end + + it 'deletes the space independently of UAA availability' do + provisioner.provision(account) + app.update(service_account: account) + allow(clients).to receive(:get).and_return(provisioner.send(:registration, account)) + allow(clients).to receive(:delete).and_raise('UAA unavailable') + + errors = space_delete.delete([space]) + + expect(errors).to be_empty + expect(account.reload.enabled).to be(true) + expect(account.status).to eq('ready') + expect(Space.first(guid: space.guid)).to be_nil + end + + it 'retains account reconciliation while deleting the space workloads' do + PollableJobModel.create(resource_guid: account.guid, resource_type: 'service_account', operation: 'service_account.provision', state: 'POLLING') + errors = space_delete.delete([space]) + expect(errors).to be_empty + expect(AppModel.first(guid: app.guid)).to be_nil + expect(Space.first(guid: space.guid)).to be_nil + expect(ServiceAccountModel.first(guid: account.guid)).not_to be_nil + expect(clients).not_to have_received(:delete) + end + + it 'does not adopt or remove an unrelated UAA client' do + allow(clients).to receive(:get).and_return('client_id' => account.client_id, 'cf_service_account_guid' => 'someone-else') + errors = space_delete.delete([space]) + expect(errors).to be_empty + expect(ServiceAccountModel.first(guid: account.guid)).not_to be_nil + expect(Space.first(guid: space.guid)).to be_nil + expect(clients).not_to have_received(:delete) + end + + it 'retains the account and space if workload deletion fails' do + allow_any_instance_of(AppDelete).to receive(:delete).and_raise('workload cleanup failed') + expect { space_delete.delete([space]) }.to raise_error('workload cleanup failed') + expect(ServiceAccountModel.first(guid: account.guid)).not_to be_nil + expect(Space.first(guid: space.guid)).not_to be_nil + expect(clients).not_to have_received(:delete) + end + end + it 'creates audit events for recursive app deletion and space deletion' do space_delete.delete([space]) expect(VCAP::CloudController::Event.count).to eq(2) diff --git a/spec/unit/actions/task_create_spec.rb b/spec/unit/actions/task_create_spec.rb index aa6e4542aa6..cf1b19c2b2d 100644 --- a/spec/unit/actions/task_create_spec.rb +++ b/spec/unit/actions/task_create_spec.rb @@ -72,6 +72,24 @@ module VCAP::CloudController expect(TaskModel.count).to eq(1) end + it 'refuses a task when the account entitlement disappears before launch' do + space.organization.update(service_account_prefix: 'task-team') + account = ServiceAccountModel.create(name: 'task-worker', space: space, status: 'ready') + app.update(service_account: account) + account.remove_all_entitled_spaces + expect_any_instance_of(ServiceAccountModel).to receive(:lock!).and_call_original + expect { task_create_action.create(app, message, user_audit_info) }.to raise_error(TaskCreate::InvalidTask, /entitled/) + end + + it 'snapshots the app account on task creation so later unbind cannot change task identity' do + app.space.organization.update(service_account_prefix: 'task-team') + account = ServiceAccountModel.create(name: 'payments-worker', space: app.space, status: 'ready') + app.update(service_account: account) + task = task_create_action.create(app, message, user_audit_info) + app.update(service_account: nil) + expect(task.reload.service_account_guid).to eq(account.guid) + end + it "sets the task state to 'RUNNING'" do task = task_create_action.create(app, message, user_audit_info) diff --git a/spec/unit/actions/v2/organization_delete_spec.rb b/spec/unit/actions/v2/organization_delete_spec.rb index fd670b44885..1e0a9dfdc70 100644 --- a/spec/unit/actions/v2/organization_delete_spec.rb +++ b/spec/unit/actions/v2/organization_delete_spec.rb @@ -9,7 +9,7 @@ module V2 let(:user_audit_info) { UserAuditInfo.new(user_guid: user.guid, user_email: user_email) } let(:space_delete) { SpaceDelete.new(user_audit_info, services_event_repository) } - subject(:org_delete) { V2::OrganizationDelete.new(space_delete) } + subject(:org_delete) { V2::OrganizationDelete.new(space_delete, user_audit_info) } describe '#delete' do let!(:org_1) { create(:organization) } let!(:org_2) { create(:organization) } diff --git a/spec/unit/actions/v2/services/service_binding_create_spec.rb b/spec/unit/actions/v2/services/service_binding_create_spec.rb index 9315282ab8c..34da920de89 100644 --- a/spec/unit/actions/v2/services/service_binding_create_spec.rb +++ b/spec/unit/actions/v2/services/service_binding_create_spec.rb @@ -42,6 +42,87 @@ module VCAP::CloudController allow(client).to receive(:bind).and_return({ async: false, binding: binding_params, operation: nil }) end + context 'with a service account' do + let(:app) { create(:app_model, space: create(:space, :with_service_account_prefix)) } + let(:account) { ServiceAccountModel.create(name: 'broker-worker', space: app.space, status: 'ready') } + + before { app.update(service_account: account) } + + it 'captures the account before the broker request and persists the same identity' do + expect(client).to receive(:bind) do |binding, **| + expect(binding.service_account_guid).to eq(account.guid) + expect(binding).to be_new + expect(ServiceBinding.db.in_transaction?).to be(true) + { async: false, binding: {}, operation: nil } + end + + binding = service_binding_create.create(app, service_instance, message, volume_mount_services_enabled, accepts_incomplete) + + expect(binding.reload.service_account_guid).to eq(account.guid) + end + + it 'rejects a revoked entitlement before contacting the broker' do + account.remove_all_entitled_spaces + + expect do + service_binding_create.create(app, service_instance, message, volume_mount_services_enabled, accepts_incomplete) + end.to raise_error(Sequel::ValidationFailed, /entitled/) + expect(client).not_to have_received(:bind) + expect(ServiceBinding.count).to eq(0) + end + + it 'serializes concurrent unassignment until the identity-bearing binding is persisted', isolation: :truncation do + app_guid = app.guid + broker_called = Queue.new + finish_broker = Queue.new + writer_started = Queue.new + allow(client).to receive(:bind) do |binding, **| + broker_called << binding.service_account_guid + finish_broker.pop + { async: false, binding: {}, operation: nil } + end + instance = service_instance + binding_message = message + creator = Thread.new do + service_binding_create.create(AppModel.first(guid: app_guid), instance, binding_message, true, false) + end + expect(Timeout.timeout(10) { broker_called.pop }).to eq(account.guid) + writer = Thread.new do + writer_started << true + AppAssignServiceAccount.new(double(can_write_to_active_space?: true)).assign(AppModel.first(guid: app_guid), nil) + rescue AppAssignServiceAccount::Conflict => e + e + end + Timeout.timeout(10) { writer_started.pop } + finish_broker << true + + binding = Timeout.timeout(10) { creator.value } + expect(Timeout.timeout(10) { writer.value }).to be_a(AppAssignServiceAccount::Conflict) + expect(binding.reload.service_account_guid).to eq(account.guid) + expect(app.reload.service_account_guid).to eq(account.guid) + ensure + finish_broker << true if finish_broker + [creator, writer].compact.each do |thread| + thread.kill if thread.alive? + thread.join + end + end + + it 'does not recapture a later assignment after the broker has received the original identity' do + original_guid = account.guid + expect(client).to receive(:bind) do |binding, **| + expect(binding.service_account_guid).to eq(original_guid) + # A stale caller object must not replace the identity captured under lock. + binding.app.service_account = nil + { async: false, binding: {}, operation: nil } + end + + binding = service_binding_create.create(app, service_instance, message, volume_mount_services_enabled, accepts_incomplete) + + expect(binding.reload.service_account_guid).to eq(original_guid) + end + end + context 'when the service broker does not include any additional binding parameters' do it 'creates a plain old Service Binding' do service_binding = service_binding_create.create(app, service_instance, message, volume_mount_services_enabled, accepts_incomplete) diff --git a/spec/unit/collection_transformers/username_populator_spec.rb b/spec/unit/collection_transformers/username_populator_spec.rb index ea845fb9db4..d47f8debf83 100644 --- a/spec/unit/collection_transformers/username_populator_spec.rb +++ b/spec/unit/collection_transformers/username_populator_spec.rb @@ -28,6 +28,20 @@ module VCAP::CloudController username_populator.transform(user1) expect(user1.username).to eq('Username1') end + + it 'delegates qualified account lookup to the model while retaining UAA lookup for other users' do + account = ServiceAccountModel.create(name: 'payments-worker', space: create(:space, :with_service_account_prefix)) + account_user = User.new(guid: account.client_id, is_oauth_client: true) + other_client = User.new(guid: 'other-client', is_oauth_client: true) + expect(ServiceAccountModel).to receive(:for_client_ids).with([account.client_id, other_client.guid]).and_call_original + expect(uaa_client).to receive(:usernames_for_ids).with(%w[1 other-client]).and_return('1' => 'Username1') + + username_populator.transform([account_user, user1, other_client]) + + expect(account_user.username).to eq(account.name) + expect(user1.username).to eq('Username1') + expect(other_client.username).to be_nil + end end end end diff --git a/spec/unit/controllers/runtime/organizations_controller_spec.rb b/spec/unit/controllers/runtime/organizations_controller_spec.rb index 25e9eaf626f..6d01a947ca2 100644 --- a/spec/unit/controllers/runtime/organizations_controller_spec.rb +++ b/spec/unit/controllers/runtime/organizations_controller_spec.rb @@ -1389,6 +1389,66 @@ def decoded_guids end context 'with recursive=true' do + context 'with service accounts' do + let(:org) { create(:organization, service_account_prefix: 'v2-delete-team') } + let(:space) { create(:space, organization: org) } + let!(:account) { ServiceAccountModel.create(name: 'worker-app', space: space) } + let(:clients) { instance_double(CF::UAA::Scim) } + let(:provisioner) { ServiceAccountProvision.new(clients, identity_ca_ref: 'diego-instance-identity') } + + before do + allow(clients).to receive(:get).and_raise(CF::UAA::NotFound) + allow(clients).to receive(:add) + allow(clients).to receive(:delete) + CloudController::DependencyLocator.instance.register(:service_account_provisioner, provisioner) + end + + it 'cleans an unused reservation without resolving UAA provisioning' do + expect(CloudController::DependencyLocator.instance).not_to receive(:service_account_provisioner) + + delete "/v2/organizations/#{org.guid}?recursive=true" + + expect(last_response).to have_status_code(204) + expect(ServiceAccountModel.first(guid: account.guid)).to be_nil + expect(Organization.first(guid: org.guid)).to be_nil + expect(ServiceAccountModel.db[:service_account_name_deletions].where(account_guid: account.guid)).not_to be_empty + end + + it 'removes the managed client before deleting the account and organization' do + provisioner.provision(account) + allow(clients).to receive(:get).and_return(provisioner.send(:registration, account)) + + delete "/v2/organizations/#{org.guid}?recursive=true" + + expect(last_response).to have_status_code(204) + expect(clients).to have_received(:delete).with(:client, account.client_id) + expect(ServiceAccountModel.first(guid: account.guid)).to be_nil + expect(Organization.first(guid: org.guid)).to be_nil + event = Event.first(type: 'audit.service_account.delete', actee: account.guid) + expect(event.actor).to eq(SecurityContext.current_user.guid) + end + + it 'retains a deleting organization after UAA failure and permits cleanup retry' do + provisioner.provision(account) + allow(clients).to receive(:get).and_raise('UAA unavailable') + + delete "/v2/organizations/#{org.guid}?recursive=true" + + expect(last_response.status).to be >= 400 + expect(org.reload.status).to eq(Organization::DELETING) + expect(Space.first(guid: space.guid)).to be_nil + expect(account.reload.status).to eq('failed') + allow(clients).to receive(:get).and_return(provisioner.send(:registration, account)) + + delete "/v2/organizations/#{org.guid}?recursive=true" + + expect(last_response).to have_status_code(204) + expect(clients).to have_received(:delete).with(:client, account.client_id) + expect(ServiceAccountModel.first(guid: account.guid)).to be_nil + expect(Organization.first(guid: org.guid)).to be_nil + end + end + it 'deletes the org and all of its spaces' do space_1 = create(:space, organization: org) space_2 = create(:space, organization: org) diff --git a/spec/unit/jobs/v3/service_account_provision_spec.rb b/spec/unit/jobs/v3/service_account_provision_spec.rb new file mode 100644 index 00000000000..80add2b6405 --- /dev/null +++ b/spec/unit/jobs/v3/service_account_provision_spec.rb @@ -0,0 +1,71 @@ +require 'spec_helper' +require 'actions/service_account_provision' +require 'jobs/v3/service_account_provision' if File.exist?('app/jobs/v3/service_account_provision.rb') + +module VCAP::CloudController + RSpec.describe 'Service account provisioning job', job_context: :worker do + let(:account) { ServiceAccountModel.create(name: 'payments-worker', space: create(:space, :with_service_account_prefix)) } + let(:clients) { instance_double(CF::UAA::Scim) } + let(:provisioner) { ServiceAccountProvision.new(clients, identity_ca_ref: 'diego-instance-identity') } + let(:locator) { CloudController::DependencyLocator.instance } + let(:job) { Jobs::V3.const_get(:ServiceAccountProvision).new(account.guid) } + + before do + locator.register(:service_account_provisioner, provisioner) + allow(clients).to receive(:get).and_raise(CF::UAA::NotFound) + allow(clients).to receive(:add) + end + + it 'is serializable without credentials and resolves provisioning dependencies in the worker' do + serialized = YAML.dump(job) + expect(serialized).not_to include('worker-only-ca') + expect(serialized).not_to include('CF::UAA::Scim') + restored = YAML.unsafe_load(serialized) + + restored.perform + + expect(account.reload.status).to eq('ready') + expect(User.first(guid: account.client_id)).to be_nil + expect(restored.resource_guid).to eq(account.guid) + expect(restored.resource_type).to eq('service_account') + expect(restored.display_name).to eq('service_account.provision') + expect(restored).to be_a_valid_job + expect(restored.max_attempts).to eq(3) + end + + it 'propagates transient errors for retry and reconciles the persisted failed account on the next attempt' do + allow(clients).to receive(:add).and_raise(CF::UAA::BadTarget, 'unavailable') + expect { job.perform }.to raise_error(CF::UAA::BadTarget) + expect(account.reload.status).to eq('failed') + expect(User.first(guid: account.client_id)).to be_nil + + allow(clients).to receive(:add) + job.perform + + expect(account.reload.status).to eq('ready') + end + + it 'does not contact UAA or create a principal if the account was disabled while queued' do + queued = job + account.update(enabled: false) + expect(clients).not_to receive(:get) + expect { queued.perform }.to raise_error(ServiceAccountProvision::Conflict, /disabled/) + expect(User.first(guid: account.client_id)).to be_nil + end + + it 'does not contact UAA if the account was deleted while queued' do + queued = job + account.destroy + expect(clients).not_to receive(:get) + expect { queued.perform }.to raise_error(CloudController::Errors::ApiError, /could not be found/) + end + + it 'fails closed when worker provisioning has not been configured' do + locator.register(:service_account_provisioner, nil) + expect(clients).not_to receive(:get) + expect { job.perform }.to raise_error(/service account provisioning is not configured/) + expect(account.reload.status).to eq('reserved') + expect(User.first(guid: account.client_id)).to be_nil + end + end +end diff --git a/spec/unit/lib/cloud_controller/backends/staging_environment_builder_spec.rb b/spec/unit/lib/cloud_controller/backends/staging_environment_builder_spec.rb index 421d6867c6f..740b18ceeb2 100644 --- a/spec/unit/lib/cloud_controller/backends/staging_environment_builder_spec.rb +++ b/spec/unit/lib/cloud_controller/backends/staging_environment_builder_spec.rb @@ -60,6 +60,14 @@ module VCAP::CloudController }) end + it 'never exposes account discovery in staging even when supplied by a staging request' do + app.space.organization.update(service_account_prefix: 'staging-team') + account = ServiceAccountModel.create(name: 'payments-worker', space: app.space, status: 'ready') + app.update(service_account: account) + result = builder.build(app, space, lifecycle, memory_limit, staging_disk_in_mb, { 'VCAP_SERVICE_ACCOUNT' => 'injected' }) + expect(result).not_to have_key('VCAP_SERVICE_ACCOUNT') + end + context 'when the app has a route associated with it' do it 'includes the uris as part of vcap_application' do route1 = create(:route, space:) diff --git a/spec/unit/lib/cloud_controller/deployment_updater/actions/finalize_spec.rb b/spec/unit/lib/cloud_controller/deployment_updater/actions/finalize_spec.rb index 72a5c6e9652..0c317117bf8 100644 --- a/spec/unit/lib/cloud_controller/deployment_updater/actions/finalize_spec.rb +++ b/spec/unit/lib/cloud_controller/deployment_updater/actions/finalize_spec.rb @@ -94,6 +94,31 @@ module VCAP::CloudController expect(old_worker_process.reload.command).to eq('new_worker_command') end + it 'restarts account-bound workers using deployment-updater identity configuration' do + app.space.organization.update(service_account_prefix: 'updater-team') + account = ServiceAccountModel.create(name: 'worker-app', space: app.space, status: 'ready') + app.update(service_account: account) + settings = TestConfig.config.merge( + service_accounts_enabled: true, + service_account_token_endpoint: 'https://uaa.example.test/oauth/mtls/token' + ) + updater_config = Config.new(settings, context: :deployment_updater) + allow(Config).to receive(:config).and_return(updater_config) + allow(ProcessRestart).to receive(:restart).and_call_original + runner = double(stop: nil) + allow_any_instance_of(Runners).to receive(:runner_for_process).and_return(runner) + expect(runner).to receive(:start).twice do + identity = Diego::ServiceAccountIdentity.new(app, updater_config) + expect(identity.certificate_properties.service_account.prefix).to eq('updater-team') + expect(identity.environment.fetch('VCAP_SERVICE_ACCOUNT')[:token_endpoint]).to eq('https://uaa.example.test/oauth/mtls/token') + end + + finalize_action.call + + expect(deployment.reload.state).to eq(DeploymentModel::DEPLOYED_STATE) + expect(old_worker_process.reload.runtime_service_account_guid).to eq(account.guid) + end + it 'puts the deployment into its finished DEPLOYED_STATE' do subject.call deployment.reload diff --git a/spec/unit/lib/cloud_controller/diego/app_recipe_builder_spec.rb b/spec/unit/lib/cloud_controller/diego/app_recipe_builder_spec.rb index 15b650d2fa1..c7bbe4a8df2 100644 --- a/spec/unit/lib/cloud_controller/diego/app_recipe_builder_spec.rb +++ b/spec/unit/lib/cloud_controller/diego/app_recipe_builder_spec.rb @@ -356,6 +356,21 @@ module Diego it_behaves_like 'creating a desired lrp' + context 'with a launched service account' do + let(:config) do + Config.new({ service_accounts_enabled: true, + diego: { use_privileged_containers_for_running: false, lifecycle_bundles: { 'potato-stack' => 'some-uri' }, pid_limit: 100 } }) + end + + it 'keeps the launch identity in rebuilt LRPs after desired unbind' do + space.organization.update(service_account_prefix: 'recipe-team') + account = ServiceAccountModel.create(name: 'payments-worker', space: space, status: 'ready') + process.update(service_account_guid: account.guid, service_account_snapshot: true) + app_model.update(service_account: nil) + expect(builder.build_app_lrp.certificate_properties.service_account.name).to eq(account.name) + end + end + it 'creates a desired lrp with buildpack specific properties' do lrp = builder.build_app_lrp expect(lrp.environment_variables).to contain_exactly(::Diego::Bbs::Models::EnvironmentVariable.new(name: 'foo', value: 'bar')) diff --git a/spec/unit/lib/cloud_controller/diego/environment_spec.rb b/spec/unit/lib/cloud_controller/diego/environment_spec.rb index 93642fabd9b..236521bad96 100644 --- a/spec/unit/lib/cloud_controller/diego/environment_spec.rb +++ b/spec/unit/lib/cloud_controller/diego/environment_spec.rb @@ -39,6 +39,23 @@ module VCAP::CloudController::Diego ]) end + it 'overrides injected account discovery with the ready platform account' do + process.space.organization.update(service_account_prefix: 'environment-team') + TestConfig.override(service_accounts_enabled: true, + service_account_token_endpoint: 'https://uaa.example.test/oauth/token/mtls') + account = VCAP::CloudController::ServiceAccountModel.create(name: 'payments-worker', space: process.space, status: 'ready') + process.app.update(service_account: account) + process.update(service_account_guid: account.guid, service_account_snapshot: true) + environment['VCAP_SERVICE_ACCOUNT'] = 'injected' + value = Environment.new(process).as_json.find { |entry| entry['name'] == 'VCAP_SERVICE_ACCOUNT' }['value'] + expect(Oj.load(value)).to include('name' => account.name, 'client_id' => account.client_id) + end + + it 'removes injected account discovery from unbound apps' do + environment['VCAP_SERVICE_ACCOUNT'] = 'injected' + expect(Environment.new(process).as_json.pluck('name')).not_to include('VCAP_SERVICE_ACCOUNT') + end + context 'when the user specifies their own MEMORY_LIMIT' do it 'uses the system provided MEMORY_LIMIT' do environment['MEMORY_LIMIT'] = 'i-should-not-be-usedMB' diff --git a/spec/unit/lib/cloud_controller/diego/qualified_service_account_identity_spec.rb b/spec/unit/lib/cloud_controller/diego/qualified_service_account_identity_spec.rb new file mode 100644 index 00000000000..b38a2d9f9e8 --- /dev/null +++ b/spec/unit/lib/cloud_controller/diego/qualified_service_account_identity_spec.rb @@ -0,0 +1,29 @@ +require 'spec_helper' +require 'cloud_controller/diego/service_account_identity' + +module VCAP::CloudController::Diego + RSpec.describe 'Qualified runtime account identity' do + it 'encodes the immutable prefix for an entitled app in another space' do + org = create(:organization, service_account_prefix: 'payments-team') + source = create(:space, organization: org) + target = create(:space, organization: org) + account = VCAP::CloudController::ServiceAccountModel.create(name: 'worker-app', space: source, status: 'ready') + account.add_entitled_space(target) + app = create(:app_model, space: target, service_account: account) + TestConfig.override(service_accounts_enabled: true) + + properties = ServiceAccountIdentity.new(app, TestConfig.config_instance).certificate_properties + expect(properties.service_account.to_h).to include(name: 'worker-app', prefix: 'payments-team') + end + + it 'fails closed until service accounts are enabled' do + org = create(:organization, service_account_prefix: 'payments-team') + space = create(:space, organization: org) + account = VCAP::CloudController::ServiceAccountModel.create(name: 'worker-app', space: space, status: 'ready') + app = create(:app_model, space: space, service_account: account) + TestConfig.override(service_accounts_enabled: false) + expect { ServiceAccountIdentity.new(app, TestConfig.config_instance).certificate_properties }. + to raise_error(CloudController::Errors::ApiError, /not enabled/) + end + end +end diff --git a/spec/unit/lib/cloud_controller/diego/service_account_identity_spec.rb b/spec/unit/lib/cloud_controller/diego/service_account_identity_spec.rb new file mode 100644 index 00000000000..8722dfe821c --- /dev/null +++ b/spec/unit/lib/cloud_controller/diego/service_account_identity_spec.rb @@ -0,0 +1,59 @@ +require 'spec_helper' +require 'cloud_controller/diego/service_account_identity' if File.exist?('lib/cloud_controller/diego/service_account_identity.rb') + +module VCAP::CloudController::Diego + RSpec.describe 'Service account runtime identity' do + let(:app) { create(:app_model, space: create(:space, :with_service_account_prefix)) } + let(:account) { VCAP::CloudController::ServiceAccountModel.create(name: 'payments-worker', space: app.space, status: 'ready') } + let(:identity) { VCAP::CloudController::Diego.const_get(:ServiceAccountIdentity).new(app, TestConfig.config_instance) } + + before do + TestConfig.override(service_accounts_enabled: true, + service_account_token_endpoint: 'https://uaa.example.test/oauth/token/mtls') + app.update(service_account: account) + end + + it 'serializes a typed launch identity while preserving organizational units' do + properties = identity.certificate_properties + decoded = ::Diego::Bbs::Models::CertificateProperties.decode(properties.to_proto) + expect(decoded.service_account.name).to eq(account.name) + expect(decoded.organizational_unit).to eq(["organization:#{app.organization_guid}", "space:#{app.space_guid}", "app:#{app.guid}"]) + app.update(service_account: nil) + expect(decoded.service_account.name).to eq(account.name) + end + + it 'exposes non-secret discovery information' do + metadata = { + guid: account.guid, name: account.name, client_id: account.client_id, certificate_dns_san: account.certificate_dns_san, + token_endpoint: 'https://uaa.example.test/oauth/token/mtls' + } + expect(identity.environment).to eq('VCAP_SERVICE_ACCOUNT' => metadata) + end + + %w[reserved reconciling failed disabled deleting].each do |state| + it "rejects new runtime credentials in #{state} state" do + account.update(status: state) + expect { identity.certificate_properties }.to raise_error(CloudController::Errors::ApiError, /not ready/) + end + end + + it 'rejects a disabled ready account and a mixed-version disabled runtime' do + account.update(enabled: false) + expect { identity.certificate_properties }.to raise_error(CloudController::Errors::ApiError, /not ready/) + account.update(enabled: true) + TestConfig.config[:service_accounts_enabled] = false + expect { identity.certificate_properties }.to raise_error(CloudController::Errors::ApiError, /runtime is not enabled/) + end + + it 'does not give unbound apps account SANs or metadata' do + app.update(service_account: nil) + expect(identity.certificate_properties.to_h).not_to have_key(:service_account) + expect(identity.environment).to eq({}) + end + + it 'rejects token discovery endpoints containing embedded credentials' do + TestConfig.config[:service_account_token_endpoint] = 'https://user:secret@uaa.example.test/oauth/token/mtls' + expect { identity.environment }.to raise_error(CloudController::Errors::ApiError, /endpoint/) + end + end +end diff --git a/spec/unit/lib/cloud_controller/diego/task_environment_spec.rb b/spec/unit/lib/cloud_controller/diego/task_environment_spec.rb index 48bc645d0ba..47cb0343204 100644 --- a/spec/unit/lib/cloud_controller/diego/task_environment_spec.rb +++ b/spec/unit/lib/cloud_controller/diego/task_environment_spec.rb @@ -42,6 +42,17 @@ module VCAP::CloudController::Diego TestConfig.config[:default_app_disk_in_mb] = staging_disk_in_mb end + it 'inherits the ready platform account discovery and overrides injected values' do + app.space.organization.update(service_account_prefix: 'task-environment-team') + TestConfig.override(service_accounts_enabled: true, + service_account_token_endpoint: 'https://uaa.example.test/oauth/token/mtls') + account = VCAP::CloudController::ServiceAccountModel.create(name: 'payments-worker', space: app.space, status: 'ready') + app.update(service_account: account) + task.update(service_account_guid: account.guid, service_account_snapshot: true) + env = TaskEnvironment.new(app, task, space, { 'VCAP_SERVICE_ACCOUNT' => 'injected' }).build + expect(env['VCAP_SERVICE_ACCOUNT']).to include(name: account.name, client_id: account.client_id) + end + it 'returns the correct environment hash for a v3 app' do constructed_envs = TaskEnvironment.new(app, task, space).build diff --git a/spec/unit/lib/cloud_controller/diego/task_recipe_builder_spec.rb b/spec/unit/lib/cloud_controller/diego/task_recipe_builder_spec.rb index 1047b185402..d9ca99d9832 100644 --- a/spec/unit/lib/cloud_controller/diego/task_recipe_builder_spec.rb +++ b/spec/unit/lib/cloud_controller/diego/task_recipe_builder_spec.rb @@ -564,6 +564,15 @@ module Diego allow(TaskCpuWeightCalculator).to receive(:new).with(memory_in_mb: task.memory_in_mb).and_return(calculator) end + it 'propagates a ready service account as typed task certificate properties' do + app.space.organization.update(service_account_prefix: 'task-recipe-team') + config.config_hash[:service_accounts_enabled] = true + account = ServiceAccountModel.create(name: 'payments-worker', space: app.space, status: 'ready') + app.update(service_account: account) + task.update(service_account_guid: account.guid, service_account_snapshot: true) + expect(task_recipe_builder.build_app_task(config, task).certificate_properties.service_account.name).to eq(account.name) + end + it 'constructs a TaskDefinition with app task instructions' do result = task_recipe_builder.build_app_task(config, task) expected_callback_url = "https://#{internal_service_hostname}:#{tls_port}/internal/v4/tasks/#{task.guid}/completed" diff --git a/spec/unit/lib/cloud_controller/service_account_provisioner_configuration_spec.rb b/spec/unit/lib/cloud_controller/service_account_provisioner_configuration_spec.rb new file mode 100644 index 00000000000..aaca14a7c87 --- /dev/null +++ b/spec/unit/lib/cloud_controller/service_account_provisioner_configuration_spec.rb @@ -0,0 +1,96 @@ +require 'spec_helper' +require 'cloud_controller/dependency_locator' + +module VCAP::CloudController + RSpec.describe 'Configured service account provisioner', job_context: :worker do + let(:locator) { CloudController::DependencyLocator.instance } + let(:settings) { { client_id: 'account-manager', client_secret: 'manager-secret', identity_ca_ref: 'diego-instance-identity' } } + let(:account) { ServiceAccountModel.create(name: 'payments-worker', space: create(:space, :with_service_account_prefix)) } + let(:uaa_url) { 'https://uaa.service.cf.internal' } + let(:headers) { { 'content-type' => 'application/json' } } + let(:worker_config) { Config.read_file('config/cloud_controller.yml').merge(service_account_provisioning: settings) } + + before do + UaaTokenCache.clear! + TestConfig.override(service_account_provisioning: settings) + end + + it 'accepts complete worker configuration' do + expect { ConfigSchemas::WorkerSchema.validate(worker_config) }.not_to raise_error + end + + it 'validates provisioning configuration for API-hosted local workers too' do + api_config = Config.read_file('config/cloud_controller.yml').merge(service_account_provisioning: settings.except(:client_id)) + expect { ConfigSchemas::ApiSchema.validate(api_config) }.to raise_error(Membrane::SchemaValidationError, /client_id => Missing key/) + end + + %i[client_id client_secret identity_ca_ref].each do |key| + it "rejects worker configuration missing #{key}" do + worker_config[:service_account_provisioning] = settings.except(key) + expect { ConfigSchemas::WorkerSchema.validate(worker_config) }.to raise_error(Membrane::SchemaValidationError, /#{key} => Missing key/) + end + + it "fails closed before authentication with blank #{key}" do + TestConfig.override(service_account_provisioning: settings.merge(key => ' ')) + expect { locator.service_account_provisioner }.to raise_error(/service account provisioning is not configured/) + end + end + + it 'accepts the reference in API configuration for local workers' do + api_config = Config.read_file('config/cloud_controller.yml').merge(service_account_provisioning: settings) + expect { ConfigSchemas::ApiSchema.validate(api_config) }.not_to raise_error + end + + it 'fails closed when inline CA certificates are configured' do + TestConfig.override(service_account_provisioning: settings.merge(identity_ca: 'instance-identity-ca')) + expect { locator.service_account_provisioner }.to raise_error(/service account provisioning is not configured/) + end + + it 'uses only the configured management credentials and trust bundle reference' do + token = WebMock::API.stub_request(:post, "#{uaa_url}/oauth/token"). + with(basic_auth: %w[account-manager manager-secret], body: { 'grant_type' => 'client_credentials' }). + to_return(headers: headers, body: { access_token: 'manager-token', token_type: 'bearer', expires_in: 300 }.to_json) + WebMock::API.stub_request(:get, "#{uaa_url}/oauth/clients/#{account.client_id}"). + with(headers: { 'Authorization' => 'bearer manager-token' }).to_return(status: 404) + creation = WebMock::API.stub_request(:post, "#{uaa_url}/oauth/clients"). + with(headers: { 'Authorization' => 'bearer manager-token' }) do |request| + payload = Oj.load(request.body) + payload['client_id'] == account.client_id && + payload['tls-client-auth-ca-ref'] == settings[:identity_ca_ref] && !payload.key?('tls-client-auth-ca') && + payload['tls_client_auth_san_dns'] == account.certificate_dns_san && + !payload.key?('client_secret') + end.to_return(status: 201, headers: headers, body: { client_id: account.client_id }.to_json) + + provisioner = locator.service_account_provisioner + expect(locator.service_account_provisioner).to equal(provisioner) + provisioner.provision(account) + + expect(account.reload.status).to eq('ready') + expect(token).to have_been_requested.at_least_once + expect(creation).to have_been_requested.once + end + + it 'refreshes an invalid cached management token before retrying client creation' do + UaaTokenCache.set_token(settings[:client_id], 'bearer expired-token') + WebMock::API.stub_request(:get, "#{uaa_url}/oauth/clients/#{account.client_id}").to_return(status: 404) + WebMock::API.stub_request(:post, "#{uaa_url}/oauth/clients"). + with(headers: { 'Authorization' => 'bearer expired-token' }). + to_return(status: 401, headers: headers, body: { error: 'invalid_token' }.to_json) + WebMock::API.stub_request(:post, "#{uaa_url}/oauth/token"). + to_return(headers: headers, body: { access_token: 'refreshed-token', token_type: 'bearer', expires_in: 300 }.to_json) + creation = WebMock::API.stub_request(:post, "#{uaa_url}/oauth/clients"). + with(headers: { 'Authorization' => 'bearer refreshed-token' }). + to_return(status: 201, headers: headers, body: { client_id: account.client_id }.to_json) + + locator.service_account_provisioner.provision(account) + + expect(account.reload.status).to eq('ready') + expect(creation).to have_been_requested.once + end + + it 'fails closed without configuration and does not fall back to another UAA client' do + TestConfig.override(service_account_provisioning: nil) + expect { locator.service_account_provisioner }.to raise_error(/service account provisioning is not configured/) + end + end +end diff --git a/spec/unit/lib/cloud_controller/uaa/issuer_discovery_spec.rb b/spec/unit/lib/cloud_controller/uaa/issuer_discovery_spec.rb new file mode 100644 index 00000000000..11bdf420ba4 --- /dev/null +++ b/spec/unit/lib/cloud_controller/uaa/issuer_discovery_spec.rb @@ -0,0 +1,62 @@ +require 'spec_helper' + +module VCAP::CloudController + RSpec.describe 'Shared UAA issuer discovery' do + let(:locator) { CloudController::DependencyLocator.instance } + let(:discovery_url) { 'https://uaa.service.cf.internal/.well-known/openid-configuration' } + + it 'reuses successful discovery across callers until the cache expires' do + request = stub_request(:get, discovery_url).to_return(body: { issuer: 'https://uaa.example.test/oauth/token' }.to_json) + + Timecop.freeze do + first = locator.uaa_issuer_discovery + expect(first.issuer).to eq('https://uaa.example.test/oauth/token') + expect(locator.uaa_issuer_discovery.issuer).to eq('https://uaa.example.test/oauth/token') + expect(request).to have_been_requested.once + Timecop.travel(Time.now + 301) + expect(first.issuer).to eq('https://uaa.example.test/oauth/token') + expect(request).to have_been_requested.twice + end + end + + it 'does not cache unsuccessful discovery and permits retry' do + request = stub_request(:get, discovery_url).to_return(status: 503).then.to_return(body: { issuer: 'https://uaa.example.test/oauth/token' }.to_json) + + expect { locator.uaa_issuer_discovery.issuer }.to raise_error(/issuer/) + expect(locator.uaa_issuer_discovery.issuer).to eq('https://uaa.example.test/oauth/token') + expect(request).to have_been_requested.twice + end + + it 'shares one successful lookup among concurrent callers' do + request = stub_request(:get, discovery_url).to_return(body: { issuer: 'https://uaa.example.test/oauth/token' }.to_json) + discovery = locator.uaa_issuer_discovery + gate = Queue.new + threads = Array.new(4) do + Thread.new do + gate.pop + discovery.issuer + end + end + threads.size.times { gate << true } + + expect(threads.map(&:value)).to eq(Array.new(4, 'https://uaa.example.test/oauth/token')) + expect(request).to have_been_requested.once + end + + it 'treats Ruby object markers as ordinary JSON data' do + stub_request(:get, discovery_url).to_return(body: '{"^o":"Object","issuer":"https://uaa.example.test/oauth/token"}') + + expect(locator.uaa_issuer_discovery.issuer).to eq('https://uaa.example.test/oauth/token') + end + + it 'fails closed instead of using an expired issuer when refresh fails' do + stub_request(:get, discovery_url).to_return(body: { issuer: 'https://uaa.example.test/oauth/token' }.to_json).then.to_return(status: 503) + + Timecop.freeze do + expect(locator.uaa_issuer_discovery.issuer).to eq('https://uaa.example.test/oauth/token') + Timecop.travel(Time.now + 301) + expect { locator.uaa_issuer_discovery.issuer }.to raise_error(/issuer/) + end + end + end +end diff --git a/spec/unit/lib/services/service_brokers/v2/client_spec.rb b/spec/unit/lib/services/service_brokers/v2/client_spec.rb index 780848c10d4..12cd62fcdfa 100644 --- a/spec/unit/lib/services/service_brokers/v2/client_spec.rb +++ b/spec/unit/lib/services/service_brokers/v2/client_spec.rb @@ -1438,6 +1438,39 @@ module VCAP::Services::ServiceBrokers::V2 allow(http_client).to receive(:put).and_return(response) end + it 'hands the ready assigned account issuer and subject to the broker in platform context' do + app.space.organization.update(service_account_prefix: 'broker-team') + account = VCAP::CloudController::ServiceAccountModel.create(name: 'broker-worker', space: app.space, status: 'ready') + app.update(service_account: account) + stub_request(:get, 'https://uaa.service.cf.internal/.well-known/openid-configuration').to_return(body: { issuer: UAAIssuer::ISSUER }.to_json) + + client.bind(binding) + + context = hash_including(service_account: { issuer: UAAIssuer::ISSUER, subject: account.client_id }) + expect(http_client).to have_received(:put).with(anything, hash_including(context: context), anything) + end + + it 'uses shared issuer discovery across broker client instances' do + app.space.organization.update(service_account_prefix: 'broker-team') + account = VCAP::CloudController::ServiceAccountModel.create(name: 'broker-worker', space: app.space, status: 'ready') + app.update(service_account: account) + discovery = double(:issuer_discovery, issuer: 'https://shared-issuer.example.test/oauth/token') + allow(CloudController::DependencyLocator.instance).to receive(:uaa_issuer_discovery).and_return(discovery) + + client.bind(binding) + described_class.new(client_attrs).bind(binding) + + expect(discovery).to have_received(:issuer).twice + expected_context = hash_including(service_account: { issuer: 'https://shared-issuer.example.test/oauth/token', subject: account.client_id }) + expect(http_client).to have_received(:put).with(anything, hash_including(context: expected_context), anything).twice + end + + it 'fails closed rather than sending a credential-only bind when its captured account is missing' do + binding.update(service_account_guid: SecureRandom.uuid) + expect { client.bind(binding) }.to raise_error(VCAP::CloudController::ServiceAccountProvision::Conflict, /not found/) + expect(http_client).not_to have_received(:put) + end + it 'makes a put request with correct path' do client.bind(binding) diff --git a/spec/unit/models/runtime/service_account_assignment_spec.rb b/spec/unit/models/runtime/service_account_assignment_spec.rb new file mode 100644 index 00000000000..9ff9864f801 --- /dev/null +++ b/spec/unit/models/runtime/service_account_assignment_spec.rb @@ -0,0 +1,46 @@ +require 'spec_helper' + +module VCAP::CloudController + RSpec.describe 'Entitled service account app assignments' do + let(:space) { create(:space, organization: create(:organization, service_account_prefix: 'assignment-team')) } + let(:app) { create(:app_model, space: space) } + let(:account) { ServiceAccountModel.create(name: 'payments-worker', space: space) } + + it 'leaves existing apps unbound by default' do + expect(app.service_account).to be_nil + end + + it 'allows multiple apps in an entitled space to share one account' do + other_app = create(:app_model, space: space) + app.update(service_account: account) + other_app.update(service_account: account) + expect(account.apps).to contain_exactly(app, other_app) + end + + it 'supports unbinding' do + app.update(service_account: account) + app.update(service_account: nil) + expect(app.reload.service_account).to be_nil + end + + it 'rejects assignment without an entitlement in the same organization' do + other_space = create(:space, organization: space.organization) + other_account = ServiceAccountModel.create(name: 'reporting-reader', space: other_space) + expect { app.update(service_account: other_account) }.to raise_error(Sequel::ValidationFailed, /entitled to the app space in the same organization/) + expect(app.reload.service_account).to be_nil + end + + it 'rejects moving a bound app to an unentitled space' do + app.update(service_account: account) + expect do + app.update(space: create(:space, organization: space.organization)) + end.to raise_error(Sequel::ValidationFailed, /entitled to the app space in the same organization/) + expect(app.reload.space).to eq(space) + end + + it 'prevents deleting a referenced account' do + app.update(service_account: account) + expect { account.db.transaction(savepoint: true) { account.destroy } }.to raise_error(Sequel::ForeignKeyConstraintViolation) + end + end +end diff --git a/spec/unit/models/runtime/service_account_model_spec.rb b/spec/unit/models/runtime/service_account_model_spec.rb new file mode 100644 index 00000000000..03fbd62b93d --- /dev/null +++ b/spec/unit/models/runtime/service_account_model_spec.rb @@ -0,0 +1,95 @@ +require 'spec_helper' + +module VCAP::CloudController + RSpec.describe 'Service account registry' do + let(:space) { create(:space, organization: create(:organization, service_account_prefix: 'registry-team')) } + let(:model) { VCAP::CloudController.const_get(:ServiceAccountModel) } + + def reserve(name, owner=space) + model.create(name: name, space: owner) + end + + it 'reserves an org-owned qualified identity without provisioning authentication' do + account = reserve('payments-worker') + + expect(account.guid).not_to be_empty + expect(account.organization).to eq(space.organization) + expect(account.entitled_spaces).to contain_exactly(space) + expect(account.client_id).to eq('cf:service-account:registry-team:payments-worker') + expect(account.certificate_dns_san).to eq('payments-worker.registry-team.svc.identity') + expect(account.enabled).to be(true) + expect(account.status).to eq('reserved') + end + + it 'accepts canonical labels at both length boundaries' do + expect(reserve('a-b').name).to eq('a-b') + expect(reserve('a' * 63).name).to eq('a' * 63) + end + + ['', 'ab', 'a' * 64, '-worker', 'worker-', 'Worker', 'work_er', + 'work.er', "worker\n", 'wörker', 'work%65r', 'work:er', '*worker'].each do |name| + it "rejects noncanonical label #{name.inspect}" do + expect { reserve(name) }.to raise_error(Sequel::ValidationFailed, /name/) + end + end + + it 'enforces org-local uniqueness across spaces in the database' do + account = reserve('payments-worker') + other_owner = create(:space, organization: space.organization) + + expect { reserve(account.name, other_owner) }.to raise_error(Sequel::ValidationFailed, /name/) + expect do + model.db.transaction(savepoint: true) do + # Deliberately bypass model validation to exercise the database constraint. + model.dataset.insert(account.values.except(:id, :guid).merge(guid: SecureRandom.uuid)) # rubocop:disable Rails/SkipsModelValidations + end + end.to raise_error(Sequel::UniqueConstraintViolation) + end + + it 'does not permit changing the identity name' do + account = reserve('payments-worker') + + expect { account.update(name: 'reporting-reader') }.to raise_error(Sequel::ValidationFailed, /immutable/) + expect(account.reload.name).to eq('payments-worker') + end + + it 'retains the name after deletion within the owning org' do + account = reserve('payments-worker') + account.destroy + + expect(model.where(guid: account.guid).first).to be_nil + expect { reserve('payments-worker', create(:space, organization: space.organization)) }.to raise_error(Sequel::ValidationFailed, /name/) + end + + it 'rejects accounts without an allocated organization prefix' do + expect { reserve('worker-app', create(:space)) }.to raise_error(Sequel::ValidationFailed, /prefix/) + end + + it 'rejects a prefix that does not belong to the owning organization' do + expect do + model.create(name: 'worker-app', space: space, identity_prefix: 'another-team') + end.to raise_error(Sequel::ValidationFailed, /prefix/) + end + + describe '.for_client_ids' do + it 'selects exact qualified identities without matching other prefix and name combinations' do + other_space = create(:space, organization: create(:organization, service_account_prefix: 'another-team')) + requested = [reserve('payments-worker'), reserve('reporting-reader', other_space)] + reserve('reporting-reader') + reserve('payments-worker', other_space) + + expect(model.for_client_ids(requested.map(&:client_id)).all).to match_array(requested) + end + + it 'returns no accounts for empty, unrelated or malformed identities' do + reserve('payments-worker') + invalid_ids = ['payments-worker', 'cf:service-account:payments-worker', + 'cf:service-account:registry-team:payments-worker:extra', + "cf:service-account:registry-team:payments-worker\n", 'other-client'] + + expect(model.for_client_ids([]).all).to be_empty + expect(model.for_client_ids(invalid_ids).all).to be_empty + end + end + end +end diff --git a/spec/unit/models/runtime/service_account_ownership_spec.rb b/spec/unit/models/runtime/service_account_ownership_spec.rb new file mode 100644 index 00000000000..35b2417fa58 --- /dev/null +++ b/spec/unit/models/runtime/service_account_ownership_spec.rb @@ -0,0 +1,62 @@ +require 'spec_helper' + +module VCAP::CloudController + RSpec.describe 'Service account organization ownership' do + let(:space) { create(:space, organization: create(:organization, service_account_prefix: 'ownership-team')) } + + it 'persists the immutable org owner and initial space entitlement at creation' do + account = ServiceAccountModel.create(name: 'payments-worker', space: space) + + expect(account.reload.organization).to eq(space.organization) + expect(account.entitled_spaces).to contain_exactly(space) + expect(account.client_id).to eq('cf:service-account:ownership-team:payments-worker') + expect(account.certificate_dns_san).to eq('payments-worker.ownership-team.svc.identity') + end + + it 'does not duplicate the initial entitlement when metadata or state changes' do + account = ServiceAccountModel.create(name: 'payments-worker', space: space) + account.update(description: 'Batch processing', enabled: false) + + expect(account.reload.entitled_spaces).to contain_exactly(space) + end + + it 'rejects changing the owning organization' do + account = ServiceAccountModel.create(name: 'payments-worker', space: space) + other_org = create(:organization) + + expect { account.update(organization: other_org) }.to raise_error(Sequel::ValidationFailed, /organization.*immutable/) + expect(account.reload.organization).to eq(space.organization) + end + + it 'rejects creating an account whose initial space belongs to another org' do + other_org = create(:organization) + + expect do + ServiceAccountModel.create(name: 'payments-worker', space: space, organization: other_org) + end.to raise_error(Sequel::ValidationFailed, /organization/) + expect(ServiceAccountModel.count).to eq(0) + end + + it 'removes entitlement rows when the account is deleted without deleting the space or org' do + account = ServiceAccountModel.create(name: 'payments-worker', space: space) + account.destroy + + expect(ServiceAccountModel.db[:service_account_space_entitlements].where(service_account_guid: account.guid).count).to eq(0) + expect(Space.first(guid: space.guid)).not_to be_nil + expect(Organization.first(id: space.organization_id)).not_to be_nil + end + + it 'refuses a cross-org entitlement even when the model association is used directly' do + account = ServiceAccountModel.create(name: 'payments-worker', space: space) + expect { account.add_entitled_space(create(:space)) }.to raise_error(Sequel::ValidationFailed, /organization/) + end + + it 'refuses starting a process after its desired account entitlement disappears' do + account = ServiceAccountModel.create(name: 'payments-worker', space: space, status: 'ready') + app = create(:app_model, space: space, service_account: account) + process = create(:process_model, app: app, state: ProcessModel::STOPPED) + account.remove_all_entitled_spaces + expect { process.update(state: ProcessModel::STARTED) }.to raise_error(Sequel::ValidationFailed, /entitled/) + end + end +end diff --git a/spec/unit/service_account_migration_spec.rb b/spec/unit/service_account_migration_spec.rb new file mode 100644 index 00000000000..755fae9d489 --- /dev/null +++ b/spec/unit/service_account_migration_spec.rb @@ -0,0 +1,14 @@ +require 'spec_helper' + +RSpec.describe 'service account app relationship migration' do + it 'matches MySQL parent collation when retrying a partially applied migration' do + db = Sequel.mock(host: :mysql, fetch: [{ Field: 'guid', Collation: 'utf8mb3_general_ci' }]) + allow(db).to receive(:schema).with(:apps).and_return([[:service_account_guid, { type: :string }]]) + allow(db).to receive(:foreign_key_list).with(:apps).and_return([]) + allow(db).to receive(:indexes).with(:apps).and_return({}) + load File.expand_path('../../db/migrations/20261005120100_add_app_service_account.rb', __dir__) + migration = Sequel::Migration.descendants.last + migration.apply(db, :up) + expect(db.sqls.join).to include('COLLATE utf8mb3_general_ci') + end +end