-
Notifications
You must be signed in to change notification settings - Fork 371
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
Merged
Merged
Changes from all commits
Commits
Show all changes
64 commits
Select commit
Hold shift + click to select a range
a08ada1
Add allowed_sources support for mTLS app-to-app routing
rkoster 0d6eb53
Add unit tests for allowed_sources validation
rkoster c04068f
Fix allowed_sources validation to handle symbol keys
rkoster 52a53c4
Rename allowed_sources to mtls_allowed_sources for clarity
rkoster cd5f1cb
Refactor mTLS route options to RFC-0027 compliant flat format
rkoster 76ffb9b
Implement RFC domain-scoped mTLS routing with /v3/access_rules API
rkoster 8c2b6b1
Fix access_rules_controller permissions query
rkoster e7fb1bf
Add automatic Diego sync callbacks to RouteAccessRule
rkoster 16e39d3
Implement include=selector_resource for /v3/access_rules endpoint
rkoster c4a5efc
Add space_guids filtering to /v3/access_rules endpoint
rkoster 0143a26
Implement include=route for /v3/access_rules endpoint
rkoster b8a40e6
Remove name field from access rules, add read-only relationships per …
rkoster d60d4ca
Add metadata support to RouteAccessRule model
rkoster 81db2b3
Fix class loading for RouteAccessRule metadata models
rkoster 1f3d1a5
Add validation to prevent access rules on internal domains per RFC
rkoster 74b62d5
Consolidate access rules migrations, fix RuboCop offenses, and clean …
rkoster 15be0e1
Fix race condition, double join, LIKE injection, N+1 queries, and dom…
rkoster 73cedec
Fix incomplete LIKE metacharacter escaping (CodeQL rb/incomplete-sani…
rkoster a94a19f
Add tests for LIKE metacharacter escaping (backslash, underscore)
rkoster 78206b9
Fix MySQL key length limit in metadata table migration
rkoster b7cbe84
Fix routing_info_spec: remove nonexistent name field from RouteAccess…
rkoster b8776ae
Fix route presenter regression: include options: {} when empty
rkoster 74793a4
Fix CI failures: route presenter options logic and domain V2 serializ…
rkoster b704b9d
Rebrand: access rules → route policies, selector → source
rkoster 4dd1dc8
Fix test failures: complete terminology rebrand in specs
rkoster 884f803
Fix routing_info_spec: use enforce_route_policies field names
rkoster 9a24c3d
Fix domain_create_message_spec: use enforce_route_policies field names
rkoster 04b93b5
Fix route_policies_spec: use 'Source' terminology and 'sources' query…
rkoster cfb84f1
Revert: restore label_selector (was incorrectly renamed to label_source)
rkoster 0377ac1
Add API documentation for Route Policies
rkoster d703ab5
Add include support to route policies show endpoint
rkoster f00e86b
Remove devbox configuration files from branch
rkoster c5593aa
Address philippthun's review feedback
rkoster 065b198
Rename add_mtls_options to add_route_policy_options and refactor stru…
rkoster 3cfd4e2
Limit route policy includes to 'route' and 'source' only
rkoster 2081e5b
Prevent enforce_route_policies on internal and router_group domains
rkoster 9666a15
Fix RuboCop offenses in domain_create_message and route_policies_list…
rkoster f86eda9
Add include=route_policies support on routes endpoints
rkoster 96cf486
Fix race condition in RoutePolicyCreate: lock parent route row
rkoster 42f7d58
Fix missing permission filter in IncludeRoutePolicySourceDecorator
rkoster c5e46cb
Enforce route_policies_scope boundary in RoutePolicyCreate
rkoster 8b54729
Fix org manager visibility in route policies index
rkoster 00f4e59
Split route_policies.source into source_type + source_guid columns
rkoster e443e4c
Guard RoutePolicy source= setter against nil and malformed input
rkoster ab15bd9
Add unit tests for RoutePolicy source virtual getter/setter
rkoster 86f74e6
Add column-level assertions to RoutePolicyCreate spec
rkoster ed12e15
Update route policy filters to use source_type/source_guid columns
rkoster 55eac70
Use source_type/source_guid directly in IncludeRoutePolicySourceDecor…
rkoster 2674f33
Use source_type/source_guid directly in RoutePolicyPresenter
rkoster 23824af
Fix RuboCop offenses
rkoster 74aefaa
Grant org auditors read access to route policies
rkoster fa095ab
Restore .envrc to main branch version
rkoster 54417a0
Add audit events for route policy create/update/delete
rkoster 2bd093c
Add label_selector support to route policies list
rkoster bd357dd
Fix RoutePolicy.create calls using non-hex fake GUIDs via virtual sou…
rkoster 200f3df
Register route_policy audit events in event_types_spec and docs
rkoster 55426bd
Validate sources format in RoutePoliciesListMessage
rkoster 6d6706c
Guard against route deletion race in RoutePolicyCreate
rkoster c88b416
Drop empty source_not_cf_any_with_others method
rkoster c28c42a
Fix source_guids filter description in route policies list docs
rkoster c8ee32e
Fix destroy action: destroy before recording audit event
rkoster 66a48a6
Use can_read_route_policy_from_space? in find_and_authorize_route_for…
rkoster ab112db
Use add_unique_constraint for labels/annotations unique indexes
rkoster 6ae2575
Mark route policies feature as experimental in docs
rkoster File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,47 @@ | ||
| module VCAP::CloudController | ||
| class RoutePolicyCreate | ||
| class Error < StandardError | ||
| end | ||
|
|
||
| def create(route:, message:) | ||
| validate_scope!(route.domain, message.source) | ||
|
|
||
| RoutePolicy.db.transaction do | ||
| # 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 | ||
| # exclusivity validation. Locking the route row (which always exists) | ||
| # ensures they serialize regardless of how many policies currently exist. | ||
| Route.where(id: route.id).for_update.first or raise Error.new("Route '#{route.guid}' not found.") | ||
|
|
||
| existing_policies = RoutePolicy.where(route_id: route.id).all | ||
| validate_source_exclusivity(existing_policies, message.source) | ||
|
|
||
| RoutePolicy.create( | ||
| source: message.source, | ||
| route_id: route.id | ||
| ) | ||
| end | ||
| rescue Sequel::UniqueConstraintViolation | ||
| raise Error.new("A route policy with source '#{message.source}' already exists for this route.") | ||
| end | ||
|
|
||
| private | ||
|
|
||
| def validate_scope!(domain, source) | ||
| return unless domain.route_policies_scope == 'space' | ||
| return unless source.start_with?('cf:org:') | ||
|
|
||
| raise Error.new("Source '#{source}' is not allowed: domain's route_policies_scope is 'space'.") | ||
| end | ||
|
|
||
| def validate_source_exclusivity(locked_policies, source) | ||
| existing_sources = locked_policies.map(&:source) | ||
|
|
||
| # Enforce cf:any exclusivity: if new policy is cf:any, reject if route already has any policies; | ||
| # if route already has a cf:any policy, reject new policies. | ||
| raise Error.new("Cannot add 'cf:any' source when other route policies already exist for this route.") if source == 'cf:any' && existing_sources.any? | ||
| raise Error.new("Cannot add source '#{source}': route already has a 'cf:any' policy.") if existing_sources.include?('cf:any') | ||
| end | ||
| end | ||
| end | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,184 @@ | ||
| require 'messages/route_policy_create_message' | ||
| require 'messages/route_policy_update_message' | ||
| require 'messages/route_policies_list_message' | ||
| require 'messages/route_policy_show_message' | ||
| require 'presenters/v3/route_policy_presenter' | ||
| require 'decorators/include_route_policy_source_decorator' | ||
| require 'decorators/include_route_policy_route_decorator' | ||
| require 'actions/route_policy_create' | ||
| require 'repositories/route_policy_event_repository' | ||
| require 'fetchers/label_selector_query_generator' | ||
|
|
||
| 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) | ||
|
|
||
|
rkoster marked this conversation as resolved.
|
||
| 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 | ||
| message = RoutePolicyShowMessage.from_params(query_params) | ||
| unprocessable!(message.errors.full_messages) unless message.valid? | ||
|
|
||
| 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_route_policy_from_space?(route.space.id, route.space.organization_id) | ||
|
|
||
|
rkoster marked this conversation as resolved.
|
||
| decorators = [] | ||
| decorators << IncludeRoutePolicySourceDecorator if IncludeRoutePolicySourceDecorator.match?(message.include) | ||
| decorators << IncludeRoutePolicyRouteDecorator if IncludeRoutePolicyRouteDecorator.match?(message.include) | ||
|
|
||
| render status: :ok, json: Presenters::V3::RoutePolicyPresenter.new(route_policy, decorators: decorators) | ||
| end | ||
|
|
||
| def create | ||
|
rkoster marked this conversation as resolved.
|
||
| 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::RoutePolicyCreate.new.create(route: route, message: message) | ||
|
|
||
| route_policy_event_repository.record_route_policy_create( | ||
| route_policy, | ||
| user_audit_info, | ||
| { 'source' => message.source, 'route_guid' => message.route_guid } | ||
| ) | ||
|
|
||
| render status: :created, json: Presenters::V3::RoutePolicyPresenter.new(route_policy) | ||
| rescue VCAP::CloudController::RoutePolicyCreate::Error => e | ||
| unprocessable!(e.message) | ||
| end | ||
|
|
||
| def update | ||
| route_policy = VCAP::CloudController::RoutePolicy.find(guid: hashed_params[:guid]) | ||
| resource_not_found!(:route_policy) unless route_policy | ||
|
|
||
| find_and_authorize_route_for_policy(route_policy) | ||
|
|
||
| message = RoutePolicyUpdateMessage.new(hashed_params[:body]) | ||
| unprocessable!(message.errors.full_messages) unless message.valid? | ||
|
|
||
| VCAP::CloudController::MetadataUpdate.update(route_policy, message) | ||
|
|
||
| route_policy_event_repository.record_route_policy_update( | ||
| route_policy.reload, | ||
| user_audit_info, | ||
| message.audit_hash | ||
| ) | ||
|
|
||
| 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 | ||
|
|
||
| find_and_authorize_route_for_policy(route_policy) | ||
|
|
||
| route_policy.destroy | ||
|
|
||
| route_policy_event_repository.record_route_policy_delete( | ||
| route_policy, | ||
| user_audit_info | ||
| ) | ||
| head :no_content | ||
| end | ||
|
|
||
| private | ||
|
|
||
| def route_policy_event_repository | ||
| @route_policy_event_repository ||= Repositories::RoutePolicyEventRepository.new | ||
| end | ||
|
|
||
| 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 find_and_authorize_route_for_policy(route_policy) | ||
| route = route_policy.route | ||
| resource_not_found!(:route_policy) unless route && permission_queryer.can_read_route_policy_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) | ||
| 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 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_route_policies_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) | ||
|
|
||
| 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 | ||
| dataset = dataset.where(Sequel.|(*conditions)) | ||
| end | ||
|
|
||
| dataset = dataset.where(source_guid: message.source_guids) if message.requested?(:source_guids) | ||
|
|
||
| if message.requested?(:label_selector) | ||
| dataset = VCAP::CloudController::LabelSelectorQueryGenerator.add_selector_queries( | ||
| label_klass: VCAP::CloudController::RoutePolicyLabelModel, | ||
| resource_dataset: dataset, | ||
| requirements: message.requirements, | ||
| resource_klass: VCAP::CloudController::RoutePolicy | ||
| ) | ||
| end | ||
|
|
||
| dataset | ||
| end | ||
| end | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| module VCAP::CloudController | ||
| class IncludeRoutePoliciesDecorator | ||
| class << self | ||
| def match?(include) | ||
| include&.any? { |i| %w[route_policies].include?(i) } | ||
| end | ||
|
|
||
| def decorate(hash, routes) | ||
| hash[:included] ||= {} | ||
| route_ids = routes.map(&:id).uniq | ||
| route_policies = RoutePolicy.where(route_id: route_ids). | ||
| eager(:route, :labels, :annotations).all | ||
|
|
||
| hash[:included][:route_policies] = route_policies.map { |rp| Presenters::V3::RoutePolicyPresenter.new(rp).to_hash } | ||
| hash | ||
| end | ||
| end | ||
| end | ||
| end |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| 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 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,71 @@ | ||
| 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. | ||
| # Resources the current user cannot read are also silently absent. | ||
|
|
||
| def self.match?(include_params) | ||
| return false unless include_params | ||
|
|
||
| include_params.include?('source') | ||
| 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| | ||
| next if policy.source_type == 'any' | ||
|
|
||
| case policy.source_type | ||
| when 'app' then app_guids << policy.source_guid | ||
| when 'space' then space_guids << policy.source_guid | ||
| when 'org' then org_guids << policy.source_guid | ||
| end | ||
| end | ||
|
|
||
| permission_queryer = Permissions.new(SecurityContext.current_user) | ||
|
|
||
| # Fetch and present resources, filtering by what the current user can read | ||
| 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) | ||
|
|
||
| hash | ||
| end | ||
|
|
||
| private_class_method def self.fetch_and_present_apps(guids, permission_queryer) | ||
| return [] if guids.empty? | ||
|
|
||
| apps = AppModel.where(guid: guids) | ||
| apps = apps.where(space_guid: permission_queryer.readable_space_guids_query) unless permission_queryer.can_read_globally? | ||
| apps.order(:created_at, :guid). | ||
| eager(Presenters::V3::AppPresenter.associated_resources).all. | ||
| map { |app| Presenters::V3::AppPresenter.new(app).to_hash } | ||
| end | ||
|
|
||
| private_class_method def self.fetch_and_present_spaces(guids, permission_queryer) | ||
| return [] if guids.empty? | ||
|
|
||
| spaces = Space.where(guid: guids) | ||
| spaces = spaces.where(guid: permission_queryer.readable_space_guids_query) unless permission_queryer.can_read_globally? | ||
| spaces.order(:created_at, :guid). | ||
| eager(Presenters::V3::SpacePresenter.associated_resources).all. | ||
| map { |space| Presenters::V3::SpacePresenter.new(space).to_hash } | ||
| end | ||
|
|
||
| private_class_method def self.fetch_and_present_organizations(guids, permission_queryer) | ||
| return [] if guids.empty? | ||
|
|
||
| orgs = Organization.where(guid: guids) | ||
| orgs = orgs.where(guid: permission_queryer.readable_org_guids_query) unless permission_queryer.can_read_globally? | ||
| orgs.order(:created_at, :guid). | ||
| eager(Presenters::V3::OrganizationPresenter.associated_resources).all. | ||
| map { |org| Presenters::V3::OrganizationPresenter.new(org).to_hash } | ||
| end | ||
| end | ||
| end |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.