-
Notifications
You must be signed in to change notification settings - Fork 50
[WIP] Expose Insights permissions to Scalprum apps via ForemanContext #1137
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: develop
Are you sure you want to change the base?
Changes from all commits
794be24
6e44be4
4ea0a43
03d327e
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 |
|---|---|---|
|
|
@@ -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}/ | ||
| # | 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_has_permission?(user, 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 | ||
|
|
@@ -116,5 +224,29 @@ 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] | ||
|
Comment on lines
+232
to
+239
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. 🚨 suggestion (security): Relying on first regex match in SCOPED_REQUESTS makes permission resolution order-dependent. Because Suggested implementation: # Returns the required permission for the given path and HTTP method
# Resolves overlapping patterns by choosing the most specific matcher,
# defined as the one with the longest regex source that matches the path.
# This avoids permission changes caused by reordering SCOPED_REQUESTS.
# @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 # 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)
# Collect all matching patterns
matching_patterns = SCOPED_REQUESTS.select { |pattern| pattern[:test].match?(path) }
return nil if matching_patterns.empty?
# Choose the most specific pattern: longest regex source wins.
# This makes overlapping patterns deterministic and independent of array order.
request_pattern = matching_patterns.max_by { |pattern| pattern[:test].source.length }
permissions = request_pattern[:permissions]
return nil unless permissions
permissions[http_method]
end |
||
| end | ||
|
|
||
| # Checks if the user has the specified permission | ||
| # @param user [User] The user to check | ||
| # @param permission [Symbol] The permission to check for | ||
| # @return [Boolean] true if user has permission, false otherwise | ||
| def user_has_permission?(user, permission) | ||
| return false unless user | ||
|
|
||
| user.can?(permission) | ||
| 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.
issue (bug_risk): Use a more specific or HTTP-aware exception type for authorization failures.
Raising a generic
Foreman::Exceptionhere means onlyInsightsCloud::UiRequestsControllercurrently maps this to a 403; any other caller that doesn’t rescue it will return a 500 instead. Either use a more specific/HTTP-aware exception (e.g., one that implies 403) or handle the mapping to HTTP status at this layer so all callers get consistent authorization behavior.