-
Notifications
You must be signed in to change notification settings - Fork 369
RFC0055 Identity-Aware Routing #4910
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
8076e92
1259e16
fae1fcb
0314a49
514cdb5
f1c3791
cda2efb
e0fc36f
f90941d
248d4f4
059d789
0388e60
039cfe5
0ea1af5
f9441cf
31466c6
dd0688f
553d333
aa5811c
e2b537e
f4a1f72
42c5b2c
273a1db
c0b1b01
79f9b22
97c7e18
a41e9b0
2826e2c
ca18462
1ba2f3f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,66 @@ | ||
| module VCAP::CloudController | ||
| class RoutePolicyAccess < BaseAccess | ||
| # Space Developer of the route's space can manage route policies. | ||
| # No bilateral requirement — destination-controlled auth only. | ||
|
|
||
| def create?(route_policy, _params=nil) | ||
| return true if admin_user? | ||
|
|
||
| route = route_policy.route | ||
| return false unless route | ||
|
|
||
| space = route.space | ||
| context.user_email && context.user.is_a?(User) && | ||
| space.developers.include?(context.user) | ||
| end | ||
|
|
||
| def read?(route_policy) | ||
| return true if admin_user? || admin_read_only_user? || global_auditor? | ||
|
|
||
| route = route_policy.route | ||
| return false unless route | ||
|
|
||
| object_is_visible_to_user?(route_policy, context.user) | ||
| end | ||
|
|
||
| def update?(route_policy, _params=nil) | ||
| create?(route_policy) | ||
| end | ||
|
|
||
| def delete?(route_policy) | ||
| create?(route_policy) | ||
| end | ||
|
|
||
| def index?(_object_class, _params=nil) | ||
| admin_user? || admin_read_only_user? || has_read_scope? || global_auditor? | ||
| end | ||
|
|
||
| def read_with_token?(_) | ||
| admin_user? || admin_read_only_user? || has_read_scope? || global_auditor? | ||
| end | ||
|
|
||
| def create_with_token?(_) | ||
| admin_user? || has_write_scope? | ||
| end | ||
|
|
||
| def read_for_update_with_token?(_) | ||
| admin_user? || has_write_scope? | ||
| end | ||
|
|
||
| def can_remove_related_object_with_token?(*) | ||
| read_for_update_with_token?(*) | ||
| end | ||
|
|
||
| def read_related_object_for_update_with_token?(*) | ||
| read_for_update_with_token?(*) | ||
| end | ||
|
|
||
| def update_with_token?(_) | ||
| admin_user? || has_write_scope? | ||
| end | ||
|
|
||
| def delete_with_token?(_) | ||
| admin_user? || has_write_scope? | ||
| end | ||
| end | ||
| end | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,168 @@ | ||
| require 'messages/route_policy_create_message' | ||
| require 'messages/route_policy_update_message' | ||
| require 'messages/route_policies_list_message' | ||
| require 'presenters/v3/route_policy_presenter' | ||
| require 'decorators/include_route_policy_source_decorator' | ||
| require 'decorators/include_route_policy_route_decorator' | ||
|
|
||
| class RoutePoliciesController < ApplicationController | ||
| def index | ||
| message = RoutePoliciesListMessage.from_params(query_params) | ||
| invalid_param!(message.errors.full_messages) unless message.valid? | ||
|
|
||
| dataset = build_dataset(message) | ||
|
|
||
| decorators = [] | ||
| decorators << IncludeRoutePolicySourceDecorator if IncludeRoutePolicySourceDecorator.match?(message.include) | ||
| decorators << IncludeRoutePolicyRouteDecorator if IncludeRoutePolicyRouteDecorator.match?(message.include) | ||
|
|
||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it includes source and route includes, but it's missing the app space and organization |
||
| render status: :ok, json: Presenters::V3::PaginatedListPresenter.new( | ||
| presenter: Presenters::V3::RoutePolicyPresenter, | ||
| paginated_result: SequelPaginator.new.get_page(dataset, message.try(:pagination_options)), | ||
| path: '/v3/route_policies', | ||
| message: message, | ||
| decorators: decorators | ||
| ) | ||
| end | ||
|
|
||
| def show | ||
| route_policy = VCAP::CloudController::RoutePolicy.find(guid: hashed_params[:guid]) | ||
| resource_not_found!(:route_policy) unless route_policy | ||
|
|
||
| route = route_policy.route | ||
| resource_not_found!(:route_policy) unless route && permission_queryer.can_read_from_space?(route.space.id, route.space.organization_id) | ||
|
|
||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the show has no includes, I would argue that it should also have the same includes as the listing.... source, route, org, space and app |
||
| render status: :ok, json: Presenters::V3::RoutePolicyPresenter.new(route_policy) | ||
| end | ||
|
|
||
| def create | ||
| message = RoutePolicyCreateMessage.new(hashed_params[:body]) | ||
| unprocessable!(message.errors.full_messages) unless message.valid? | ||
|
|
||
| route = find_and_authorize_route(message.route_guid) | ||
| validate_route_domain(route) | ||
|
|
||
| route_policy = VCAP::CloudController::RoutePolicy.db.transaction do | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This block should be extracted into a |
||
| # Lock existing route policies for this route to prevent concurrent inserts | ||
| # from violating cf:any exclusivity or uniqueness constraints | ||
| VCAP::CloudController::RoutePolicy.where(route_id: route.id).for_update.all | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As this returns the locked policies, it would be efficient to use them in the And in |
||
|
|
||
| validate_source_exclusivity(route, message.source) | ||
|
|
||
| policy = VCAP::CloudController::RoutePolicy.new( | ||
| guid: SecureRandom.uuid, | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| source: message.source, | ||
| route_id: route.id, | ||
| created_at: Time.now.utc, | ||
| updated_at: Time.now.utc | ||
| ) | ||
| policy.save | ||
| policy | ||
| end | ||
|
|
||
| render status: :created, json: Presenters::V3::RoutePolicyPresenter.new(route_policy) | ||
| rescue Sequel::UniqueConstraintViolation | ||
| unprocessable!("A route policy with source '#{message.source}' already exists for this route.") | ||
| end | ||
|
|
||
| def update | ||
| route_policy = VCAP::CloudController::RoutePolicy.find(guid: hashed_params[:guid]) | ||
| resource_not_found!(:route_policy) unless route_policy | ||
|
|
||
| route = route_policy.route | ||
| resource_not_found!(:route_policy) unless route && permission_queryer.can_read_from_space?(route.space.id, route.space.organization_id) | ||
| unauthorized! unless permission_queryer.can_write_to_active_space?(route.space.id) | ||
| suspended! unless permission_queryer.is_space_active?(route.space.id) | ||
|
Comment on lines
+73
to
+75
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe |
||
|
|
||
| message = RoutePolicyUpdateMessage.new(hashed_params[:body]) | ||
| unprocessable!(message.errors.full_messages) unless message.valid? | ||
|
|
||
| VCAP::CloudController::MetadataUpdate.update(route_policy, message) | ||
|
|
||
| render status: :ok, json: Presenters::V3::RoutePolicyPresenter.new(route_policy.reload) | ||
| end | ||
|
|
||
| def destroy | ||
| route_policy = VCAP::CloudController::RoutePolicy.find(guid: hashed_params[:guid]) | ||
| resource_not_found!(:route_policy) unless route_policy | ||
|
|
||
| route = route_policy.route | ||
| resource_not_found!(:route_policy) unless route && permission_queryer.can_read_from_space?(route.space.id, route.space.organization_id) | ||
| unauthorized! unless permission_queryer.can_write_to_active_space?(route.space.id) | ||
| suspended! unless permission_queryer.is_space_active?(route.space.id) | ||
|
|
||
| route_policy.destroy | ||
| head :no_content | ||
| end | ||
|
|
||
| private | ||
|
|
||
| def find_and_authorize_route(route_guid) | ||
| route = VCAP::CloudController::Route.find(guid: route_guid) | ||
| resource_not_found!(:route) unless route && permission_queryer.can_read_from_space?(route.space.id, route.space.organization_id) | ||
| unauthorized! unless permission_queryer.can_write_to_active_space?(route.space.id) | ||
| suspended! unless permission_queryer.is_space_active?(route.space.id) | ||
| route | ||
| end | ||
|
|
||
| def validate_route_domain(route) | ||
| if route.domain.internal? | ||
| unprocessable!('Cannot create route policies for routes on internal domains. Internal routes use container-to-container networking and bypass GoRouter.') | ||
| end | ||
| return if route.domain.enforce_route_policies | ||
|
|
||
| unprocessable!("Cannot create route policies for route '#{route.guid}': the route's domain does not have enforce_route_policies enabled.") | ||
| end | ||
|
|
||
| def validate_source_exclusivity(route, source) | ||
| existing_sources = route.route_policies.map(&:source) | ||
|
|
||
| # Enforce cf:any exclusivity: if route already has a cf:any policy, reject new policies; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The order of the comment does not match the code. |
||
| # if new policy is cf:any, reject if route already has any policies. | ||
| unprocessable!("Cannot add 'cf:any' source when other route policies already exist for this route.") if source == 'cf:any' && existing_sources.any? | ||
| unprocessable!("Cannot add source '#{source}': route already has a 'cf:any' policy.") if existing_sources.include?('cf:any') && source != 'cf:any' | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the |
||
|
|
||
| # Uniqueness: source must be unique per route | ||
| unprocessable!("A route policy with source '#{source}' already exists for this route.") if existing_sources.include?(source) | ||
| end | ||
|
|
||
| def build_dataset(message) | ||
| dataset = VCAP::CloudController::RoutePolicy.dataset | ||
|
|
||
| if permission_queryer.can_read_globally? | ||
| readable_route_ids = VCAP::CloudController::Route.select(:id) | ||
| else | ||
| readable_space_ids = permission_queryer.readable_space_scoped_spaces_query.select(:id) | ||
| readable_route_ids = VCAP::CloudController::Route.where(space_id: readable_space_ids).select(:id) | ||
| end | ||
|
|
||
| dataset = dataset.where(route_id: readable_route_ids) | ||
|
|
||
| # Join routes at most once when either route_guids or space_guids is requested | ||
| if message.requested?(:route_guids) || message.requested?(:space_guids) | ||
| dataset = dataset. | ||
| join(:routes, id: :route_id). | ||
| select_all(:route_policies) | ||
|
|
||
| dataset = dataset.where(Sequel[:routes][:guid] => message.route_guids) if message.requested?(:route_guids) | ||
|
|
||
| dataset = dataset.where(Sequel[:routes][:space_id] => VCAP::CloudController::Space.where(guid: message.space_guids).select(:id)) if message.requested?(:space_guids) | ||
| end | ||
|
|
||
| dataset = dataset.where(guid: message.guids) if message.requested?(:guids) | ||
| dataset = dataset.where(source: message.sources) if message.requested?(:sources) | ||
|
|
||
| if message.requested?(:source_guids) | ||
| # Text-match against source string for resource GUIDs | ||
| # Handles cf:app:<guid>, cf:space:<guid>, cf:org:<guid> | ||
| # Escape LIKE metacharacters (\, %, _) in user-provided values | ||
| conditions = message.source_guids.map do |guid| | ||
| escaped_guid = guid.gsub('\\', '\\\\').gsub('%', '\\%').gsub('_', '\\_') | ||
| Sequel.like(:source, "%#{escaped_guid}%") | ||
| end | ||
| dataset = dataset.where(Sequel.|(*conditions)) | ||
| end | ||
|
|
||
| dataset | ||
| end | ||
| end | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| module VCAP::CloudController | ||
| class IncludeRoutePolicyRouteDecorator | ||
| # Handles `?include=route` for GET /v3/route_policies | ||
| # Includes the route resources associated with the route policies | ||
|
|
||
| def self.match?(include_params) | ||
| include_params&.include?('route') | ||
| end | ||
|
|
||
| def self.decorate(hash, route_policies) | ||
| hash[:included] ||= {} | ||
|
|
||
| # Collect all unique route IDs from route policies | ||
| route_ids = route_policies.map(&:route_id).uniq | ||
|
|
||
| # Fetch routes with their associations | ||
| routes = Route.where(id: route_ids). | ||
| order(:created_at, :guid). | ||
| eager(Presenters::V3::RoutePresenter.associated_resources).all | ||
|
|
||
| # Present routes | ||
| hash[:included][:routes] = routes.map { |route| Presenters::V3::RoutePresenter.new(route).to_hash } | ||
|
|
||
| hash | ||
| end | ||
| end | ||
| end |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,75 @@ | ||
| module VCAP::CloudController | ||
| class IncludeRoutePolicySourceDecorator | ||
| # Handles `?include=source` for GET /v3/route_policies | ||
| # Stale/missing resources (source GUIDs that no longer exist) are silently absent. | ||
|
|
||
| SOURCE_REGEX = /\Acf:(app|space|org):([0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12})\z/ | ||
|
|
||
| def self.match?(include_params) | ||
| return false unless include_params | ||
|
|
||
| # Match if any of: source, app, space, organization | ||
| include_params.intersect?(%w[source app space organization]) | ||
| end | ||
|
|
||
| def self.decorate(hash, route_policies) | ||
| hash[:included] ||= {} | ||
|
|
||
| # Collect all GUIDs by type | ||
| app_guids = [] | ||
| space_guids = [] | ||
| org_guids = [] | ||
|
|
||
| route_policies.each do |policy| | ||
| match = SOURCE_REGEX.match(policy.source) | ||
| next unless match | ||
|
|
||
| resource_type = match[1] | ||
| resource_guid = match[2] | ||
|
|
||
| case resource_type | ||
| when 'app' | ||
| app_guids << resource_guid | ||
| when 'space' | ||
| space_guids << resource_guid | ||
| when 'org' | ||
| org_guids << resource_guid | ||
| end | ||
| end | ||
|
|
||
| # Fetch and present resources | ||
| hash[:included][:apps] = fetch_and_present_apps(app_guids.uniq) | ||
| hash[:included][:spaces] = fetch_and_present_spaces(space_guids.uniq) | ||
| hash[:included][:organizations] = fetch_and_present_organizations(org_guids.uniq) | ||
|
|
||
| hash | ||
| end | ||
|
|
||
| private_class_method def self.fetch_and_present_apps(guids) | ||
| return [] if guids.empty? | ||
|
|
||
| apps = AppModel.where(guid: guids). | ||
| order(:created_at, :guid). | ||
| eager(Presenters::V3::AppPresenter.associated_resources).all | ||
| apps.map { |app| Presenters::V3::AppPresenter.new(app).to_hash } | ||
| end | ||
|
|
||
| private_class_method def self.fetch_and_present_spaces(guids) | ||
| return [] if guids.empty? | ||
|
|
||
| spaces = Space.where(guid: guids). | ||
| order(:created_at, :guid). | ||
| eager(Presenters::V3::SpacePresenter.associated_resources).all | ||
| spaces.map { |space| Presenters::V3::SpacePresenter.new(space).to_hash } | ||
| end | ||
|
|
||
| private_class_method def self.fetch_and_present_organizations(guids) | ||
| return [] if guids.empty? | ||
|
|
||
| orgs = Organization.where(guid: guids). | ||
| order(:created_at, :guid). | ||
| eager(Presenters::V3::OrganizationPresenter.associated_resources).all | ||
| orgs.map { |org| Presenters::V3::OrganizationPresenter.new(org).to_hash } | ||
| end | ||
| end | ||
| end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
BaseAccesssubclasses are the v2 permission layer used byRestController- not needed since there is no v2 endpoint for this feature.