Move the policy store services to organization tenancy

Marcos Rocharequested to merge
mc_rocha-policy-store-organization-tenancy into master

What does this MR do and why?

Moves the four Policy Store authoring services from group to organization tenancy. Split out of the list endpoint MR, which carried this alongside the route.

The move is mechanical. All four services took group: only to reduce it to group.organization_id on the next line, in BaseService#find_policy, ListService#execute, and CreateService#policy_attributes. The store has always been keyed by organization, so the group was a lookup vehicle for a value the caller already had. All four rename together because they share a constructor through super, even though only ListService has an endpoint so far. The store gem is untouched, and nothing outside the specs constructs these services yet.

CreateService stops setting namespace_id, which an organization-owned policy has nothing to supply. The gem stopped requiring it in !249164 (merged) (merged). Cross-organization isolation stays a Ruby-side filter in BaseService#find_policy, now comparing organization.id directly, because the store's find(id) and delete(id) take no organization and a policy id is global.

The gate changes shape. Group#policy_store_experiment_active? requires root? and the group's own namespace_settings.policy_store_experiment_enabled, and an organization has neither. The new Organization#policy_store_experiment_active? is therefore the feature flag (against :instance, matching the API's before block and the admin settings partial), the instance setting, and the licence. Two consequences worth naming:

  • The licence check widens from licensed_feature_available?, which resolves the namespace's plan on GitLab.com, to ::License.feature_available?. Organizations have no plan. Self-managed is unaffected, and the namespace-scoped check returns with the ActiveRecord-backed repository.
  • The group UI stays gated more tightly, since Groups::Security::PolicyStoreController and the sidebar menu still call the group method. The two surfaces converge when the group UI moves to the organization endpoints.

How to set up and validate locally

Requires an Ultimate licence. Every step runs on the rails console, apart from the admin setting in step 2.

  1. Enable the security_policies_v2 feature flag
Feature.enable(:security_policies_v2)
  1. As an administrator, go to Admin > Settings > Security and compliance and turn on the policy store experiment
  2. Confirm the new organization gate is satisfied, since it is what the services now read
organization = Organizations::Organization.find(1)
organization.policy_store_experiment_active?   # => true
  1. Create a policy and verify it is keyed by the organization with no namespace
result = Security::SecurityOrchestrationPolicies::PolicyStore::CreateService.new(
  organization: organization,
  params: { name: 'Block deployments on critical findings', trigger_type: 'deployment_requested' }
).execute

puts(result.success? ? result.payload[:policy].to_h : result.message)

organization_id matches step 3 and namespace_id is nil. Before this change the same call took group: and namespace_id came back as the group's id.

  1. Verify the list and find services agree, reading through the same organization
policy_id = Security::SecurityOrchestrationPolicies::PolicyStore::ListService
  .new(organization: organization).execute.payload[:policies].last.id

Security::SecurityOrchestrationPolicies::PolicyStore::FindService
  .new(organization: organization, policy_id: policy_id).execute.payload[:policy].name
# => "Block deployments on critical findings"
  1. Verify a second organization cannot reach that policy by id, which is the only cross-organization guard today
other_organization = Organizations::Organization.create!(name: 'Other', path: 'other-org')

result = Security::SecurityOrchestrationPolicies::PolicyStore::FindService
  .new(organization: other_organization, policy_id: policy_id).execute

puts result.message   # => "Policy was not found"
  1. Turn the instance setting from step 2 back off and verify the services refuse, with the message now naming the organization
Gitlab::CurrentSettings.update!(policy_store_experiment_enabled: false)

result = Security::SecurityOrchestrationPolicies::PolicyStore::ListService
  .new(organization: organization).execute

puts result.reason    # => experiment_not_active
puts result.message   # => "Policy Store experiment is not active for this organization"

References

Edited by Marcos Rocha

Merge request reports