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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 67 additions & 0 deletions app/helpers/foreman_rh_cloud_permissions_helper.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
module ForemanRhCloudPermissionsHelper
# Mapping from Foreman permissions to Insights permissions
# Based on the permission mapping table:
#
# Foreman Permission | Insights Permissions | Paths
# ---------------------|---------------------------------------------------|----------------------------------
# view_vulnerability | inventory:hosts:read | GET /api/inventory/v1/hosts(/*)
# view_vulnerability | vulnerability:vulnerability_results:read | GET /api/vulnerability/v1/*
# | vulnerability:system.opt_out:read | POST /api/vulnerability/v1/vulnerabilities/cves
# | vulnerability:report_and_export:read |
# | vulnerability:advanced_report:read |
# edit_vulnerability | vulnerability:system.cve.status:write | PATCH /api/vulnerability/v1/status
# edit_vulnerability | vulnerability:cve.business_risk_and_status:write | PATCH /api/vulnerability/v1/cves/status
# | | PATCH /api/vulnerability/v1/cves/business_risk
# edit_vulnerability | vulnerability:system.opt_out:write | PATCH /api/vulnerability/v1/systems/opt_out
#
# view_advisor | advisor:recommendation-results:read | GET /api/insights/v1/*
# | advisor:exports:read |
# edit_advisor | advisor:disable-recommendations:write | POST /api/insights/v1/ack/
# | | DELETE /api/insights/v1/ack/{rule_id}/
# | | POST /api/insights/v1/hostack/
# | | DELETE /api/insights/v1/hostack/{id}/
# | | POST /api/insights/v1/rule/{rule_id}/unack_hosts/
#
RH_CLOUD_TO_INSIGHTS_PERMISSIONS = {
view_vulnerability: [
'inventory:hosts:read',
'vulnerability:vulnerability_results:read',
'vulnerability:system.opt_out:read',
'vulnerability:report_and_export:read',
'vulnerability:advanced_report:read',
],
edit_vulnerability: [
'vulnerability:system.cve.status:write',
'vulnerability:cve.business_risk_and_status:write',
'vulnerability:system.opt_out:write',
],
view_advisor: [
'advisor:recommendation-results:read',
'advisor:exports:read',
],
edit_advisor: [
'advisor:disable-recommendations:write',
],
}.freeze

# Returns user permissions in Chrome API format for Insights applications
# @return [Array<Hash>] Array of permission objects with :permission and :resourceDefinitions keys
def insights_user_permissions
return [] unless User.current

permissions = []

RH_CLOUD_TO_INSIGHTS_PERMISSIONS.each do |foreman_permission, insights_permissions|
next unless User.current.can?(foreman_permission)

insights_permissions.each do |insights_permission|
permissions << {
permission: insights_permission,
resourceDefinitions: [],
}
end
end

permissions
end
end
126 changes: 124 additions & 2 deletions app/services/foreman_rh_cloud/insights_api_forwarder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,125 @@ module ForemanRhCloud
class InsightsApiForwarder
include ForemanRhCloud::CertAuth

# Permission mapping for API paths:
#
# Foreman Permission | Paths
# ---------------------|--------------------------------------------------
# view_vulnerability | GET /api/inventory/v1/hosts(/*)
# view_vulnerability | GET /api/vulnerability/v1/*
# | POST /api/vulnerability/v1/vulnerabilities/cves
# edit_vulnerability | PATCH /api/vulnerability/v1/status
# edit_vulnerability | PATCH /api/vulnerability/v1/cves/status
# | PATCH /api/vulnerability/v1/cves/business_risk
# edit_vulnerability | PATCH /api/vulnerability/v1/systems/opt_out
#
# view_advisor | GET /api/insights/v1/*
# edit_advisor | POST /api/insights/v1/ack/
# | DELETE /api/insights/v1/ack/{rule_id}/
Comment on lines +12 to +21
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚨 issue (security): Permission table comments for vulnerability GET endpoints don't match the actual enforcement logic.

The header states that view_vulnerability is required for all GET /api/vulnerability/v1/*, but SCOPED_REQUESTS only applies permissions to specific POST/PATCH routes and otherwise only tags these requests. As a result, vulnerability GET endpoints (e.g. CVE list, reports) are not actually gated by required_permission_for('view_vulnerability'). Either add an explicit pattern to enforce view_vulnerability on these GET routes (similar to view_advisor for GET /api/insights/v1/.*) or update the comment to describe the narrower enforcement that currently exists.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do need to make sure these tables match reality (I think this will be easier with AI). Or another option is to remove them, but I do think they are nice to have..

# | POST /api/insights/v1/hostack/
# | DELETE /api/insights/v1/hostack/{id}/
# | POST /api/insights/v1/rule/{rule_id}/unack_hosts/
#
SCOPED_REQUESTS = [
{ test: %r{api/vulnerability/v1/vulnerabilities/cves}, tag_name: :tags },
# Inventory hosts - requires view_vulnerability for GET
{
test: %r{api/inventory/v1/hosts(/.*)?$},
tag_name: :tags,
permissions: {
'GET' => :view_vulnerability,
},
},
# Vulnerability CVEs list - POST requires view_vulnerability (for filtering)
{
test: %r{api/vulnerability/v1/vulnerabilities/cves},
tag_name: :tags,
permissions: {
'POST' => :view_vulnerability,
},
},
# Vulnerability status - PATCH requires edit_vulnerability
{
test: %r{api/vulnerability/v1/status},
tag_name: :tags,
permissions: {
'PATCH' => :edit_vulnerability,
},
},
# CVE status - PATCH requires edit_vulnerability
{
test: %r{api/vulnerability/v1/cves/status},
tag_name: :tags,
permissions: {
'PATCH' => :edit_vulnerability,
},
},
# CVE business risk - PATCH requires edit_vulnerability
{
test: %r{api/vulnerability/v1/cves/business_risk},
tag_name: :tags,
permissions: {
'PATCH' => :edit_vulnerability,
},
},
# Systems opt out - PATCH requires edit_vulnerability
{
test: %r{api/vulnerability/v1/systems/opt_out},
tag_name: :tags,
permissions: {
'PATCH' => :edit_vulnerability,
},
},
# Other vulnerability endpoints (no specific permission enforcement, just tagging)
{ test: %r{api/vulnerability/v1/dashbar}, tag_name: :tags },
{ test: %r{api/vulnerability/v1/cves/[^/]+/affected_systems}, tag_name: :tags },
{ test: %r{api/vulnerability/v1/systems/[^/]+/cves}, tag_name: :tags },
{ test: %r{api/insights/.*}, tag_name: :tags },
# Advisor ack endpoints - POST/DELETE require edit_advisor
{
test: %r{api/insights/v1/ack(/[^/]*)?$},
tag_name: :tags,
permissions: {
'POST' => :edit_advisor,
'DELETE' => :edit_advisor,
},
},
# Advisor hostack endpoints - POST/DELETE require edit_advisor
{
test: %r{api/insights/v1/hostack(/[^/]*)?$},
tag_name: :tags,
permissions: {
'POST' => :edit_advisor,
'DELETE' => :edit_advisor,
},
},
# Advisor rule unack_hosts - POST requires edit_advisor
{
test: %r{api/insights/v1/rule/[^/]+/unack_hosts},
tag_name: :tags,
permissions: {
'POST' => :edit_advisor,
},
},
# Other Advisor/Insights endpoints - GET requires view_advisor
{
test: %r{api/insights/v1/.*},
tag_name: :tags,
permissions: {
'GET' => :view_advisor,
},
},
# Other API endpoints (tagging only, no permission enforcement)
{ test: %r{api/inventory/.*}, tag_name: :tags },
{ test: %r{api/tasks/.*}, tag_name: :tags },
].freeze

def forward_request(original_request, path, controller_name, user, organization, location)
# Check permissions before forwarding
permission = required_permission_for(path, original_request.request_method)
if permission && !user&.can?(permission)
logger.warn("User #{user.login} lacks permission #{permission} for #{original_request.request_method} #{path}")
raise ::Foreman::Exception.new(N_("You do not have permission to perform this action"))
end

TagsAuth.new(user, organization, location, logger).update_tag if scope_request?(original_request, path)

forward_params = prepare_forward_params(original_request, path, user: user, organization: organization, location: location).to_a
Expand Down Expand Up @@ -116,5 +224,19 @@ def http_user_agent(original_request)
def logger
Foreman::Logging.logger('app')
end

# Returns the required permission for the given path and HTTP method
# @param path [String] The request path
# @param http_method [String] The HTTP method (GET, POST, etc.)
# @return [Symbol, nil] The required permission symbol or nil if no permission required
def required_permission_for(path, http_method)
request_pattern = SCOPED_REQUESTS.find { |pattern| pattern[:test].match?(path) }
return nil unless request_pattern

permissions = request_pattern[:permissions]
return nil unless permissions

permissions[http_method]
end
end
end
30 changes: 29 additions & 1 deletion lib/foreman_rh_cloud/plugin.rb
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,37 @@ def self.register
:control_organization_insights,
'insights_cloud/settings': [:set_org_parameter]
)
# Insights Vulnerability permissions
permission(
:view_vulnerability,
{},
:resource_type => 'ForemanRhCloud'
)
permission(
:edit_vulnerability,
{},
:resource_type => 'ForemanRhCloud'
)
# Insights Advisor permissions
permission(
:view_advisor,
{},
:resource_type => 'ForemanRhCloud'
)
permission(
:edit_advisor,
{},
:resource_type => 'ForemanRhCloud'
)
end

plugin_permissions = [:view_foreman_rh_cloud, :generate_foreman_rh_cloud, :view_insights_hits, :dispatch_cloud_requests, :control_organization_insights]
# Core RH Cloud permissions for inventory upload and sync
rh_cloud_permissions = [:view_foreman_rh_cloud, :generate_foreman_rh_cloud, :view_insights_hits, :dispatch_cloud_requests, :control_organization_insights]

# Insights application permissions (Vulnerability, Advisor)
insights_permissions = [:view_vulnerability, :edit_vulnerability, :view_advisor, :edit_advisor]

plugin_permissions = rh_cloud_permissions + insights_permissions

role 'ForemanRhCloud', plugin_permissions, 'Role granting permissions to view the hosts inventory,
generate a report, upload it to the cloud and download it locally'
Expand Down
Loading
Loading