From 4dac361d3179dd84efc07855152cc0cf300b7cb6 Mon Sep 17 00:00:00 2001 From: Danny Eldridge Date: Thu, 6 Mar 2025 09:26:46 -0800 Subject: [PATCH 1/3] Use SpiceDB for lease-related auth checks --- .../authorization.rb | 138 +++++++++++++++++- 1 file changed, 134 insertions(+), 4 deletions(-) diff --git a/lib/declarative_authorization/authorization.rb b/lib/declarative_authorization/authorization.rb index a1e81e0..03c4995 100644 --- a/lib/declarative_authorization/authorization.rb +++ b/lib/declarative_authorization/authorization.rb @@ -186,23 +186,153 @@ def permit!(privilege, options = {}) attr_validator = AttributeValidator.new(self, user, options[:object], privilege, options[:context]) rules = matching_auth_rules(roles, privileges, options[:context]) - # Test each rule in turn to see whether any one of them is satisfied. + # _____ _ ____ ____ + # / ___| (_) | _ \| _ \ + # \ `--. _ __ _ ___ ___| | | | |_) | + # `--. \ '_ \| |/ __/ _ \ | | | _ < + # /\__/ / |_) | | (_| __/ |_| | |_) | + # \____/| .__/|_|\___\___|____/|____/ + # | | + # |_| + use_spicedb_auth = false + rules.each do |rule| - return true if rule.validate?(attr_validator, options[:skip_attribute_test]) + unless rule.role.to_s.start_with?("leases__", "lease_renewals__") + # Existing behavior for non-lease-related rules + return true if rule.validate?(attr_validator, options[:skip_attribute_test]) + next + end + + use_spicedb_auth = true + + auth_service_class = Rails.application.config.try(:spicedb_authorization_service) + @auth_service = auth_service_class.new + + vhost_id = Core::Company.guid if defined?(Core::Company) + raise StandardError, "Vhost ID not found" if vhost_id.nil? + + puts "\n==== Processing new rule ====" + puts "Rule: #{rule.inspect}" + puts "Role: #{rule.role}" + + permission_to_check = rule.role.to_s.gsub("__", "_") + "_permission" + puts "Permission to check: #{permission_to_check}" + + if rule.attributes.empty? + puts "Rule has no attributes, checking spicedb directly" + + authorized = @auth_service.check_permission( + resource: { type: "vhost", id: vhost_id }, + permission: permission_to_check + ) + puts "Authorized? #{authorized}" + + return true if authorized + else + puts "Rule has #{rule.attributes.count} attributes, examining them:" + + rule.attributes.each_with_index do |attribute, index| + puts "\n -- Attribute ##{index + 1}: #{attribute.inspect}" + + if attribute.instance_variable_defined?('@conditions_hash') + conditions = attribute.instance_variable_get('@conditions_hash') + puts " Conditions hash: #{conditions.inspect}" + else + puts " !! No conditions_hash instance variable found, skipping" + next + end + + next unless conditions.is_a?(Hash) + + puts " Checking conditions against current values:" + condition_matched = false + + if conditions.key?(:granular_permissions) + rule_requires = conditions[:granular_permissions][1] + actual_value = options[:object]&.granular_permissions if options[:object].respond_to?(:granular_permissions) + + puts " Granular permissions - Rule requires: #{rule_requires}, Actual: #{actual_value}" + if rule_requires == actual_value + condition_matched = true + puts " ✓ Granular permissions condition matched!" + else + puts " ✗ Granular permissions condition did not match" + end + else + puts " (No granular_permissions condition to check)" + end + + if conditions.key?(:is_renewal) + rule_requires = conditions[:is_renewal][1] + # If options[:object] has is_renewal, we'll use that value instead of obtaining it from SpiceDB. + # This allows for Blue Moon leases and others to be checked without having to make a SpiceDB call for the value of is_renewal. + if options[:object].respond_to?(:is_renewal) + actual_value = options[:object].is_renewal + else + # Check if the lease document is a renewal. Ideally we implement a method in the auth service that is better + # suited for this kind of check, but for the POC we'll just use lookup_subjects since it does what we need. + puts " Checking SpiceDB for `renewal` relation on lease document uuid: #{options[:object]&.lease_document_uuid.to_s}" + response = @auth_service.lookup_subjects( + resource_type: "lease_document", + resource_id: options[:object]&.lease_document_uuid.to_s, + permission: "renewal", + subject_type: "lease_document", + ) + actual_value = response[:subject_ids].any? + end + + puts " Is renewal - Rule requires: #{rule_requires}, Actual: #{actual_value}" + if rule_requires == actual_value + condition_matched = true + puts " ✓ Is_renewal condition matched!" + else + puts " ✗ Is_renewal condition did not match" + end + else + puts " (No is_renewal condition to check)" + end + + if condition_matched + # For now, we will assume each rule's conditions are OR'd together. This is not the case + # for all rules, but it's the most common case and a good starting point. + puts " At least one matching condition found, checking spicedb" + + authorized = @auth_service.check_permission( + resource: { type: "vhost", id: vhost_id }, + permission: permission_to_check + ) + puts " Authorized? #{authorized}" + + return true if authorized + else + puts " No matching conditions, skipping spicedb check for this attribute" + end + end + end end + source_prefix = use_spicedb_auth ? "SpiceDB authorization failed. " : "" + if options[:bang] if rules.empty? raise NotAuthorized, "No matching rules found for #{privilege} for User with id #{user.try(:id)} " + "(roles #{roles.inspect}, privileges #{privileges.inspect}, " + "context #{options[:context].inspect})." else - raise AttributeAuthorizationError, "#{privilege} not allowed for User with id #{user.try(:id)} on #{(options[:object] || options[:context]).inspect}." + raise AttributeAuthorizationError, "#{source_prefix}#{privilege} not allowed for User with id #{user.try(:id)} " + + "on #{(options[:object] || options[:context]).inspect}." end else false end - end + end + # _____ _ _ _____ + # | ___| | \ | | | _ | + # | |__ | \| | | | | | + # | __| | . ` | | | | | + # | |___ | |\ | \ \_/ / + # \____/ \_| \_/ \___/ + # Calls permit! but doesn't raise authorization errors. If no exception is # raised, permit? returns true and yields to the optional block. From 6d1fd849cc4c3c4328dceaebfe32d12e5673dbcd Mon Sep 17 00:00:00 2001 From: Danny Eldridge Date: Tue, 18 Mar 2025 12:34:34 -0700 Subject: [PATCH 2/3] Handle id_in_scope attribute checks --- .../authorization.rb | 73 ++++++++++++++----- 1 file changed, 56 insertions(+), 17 deletions(-) diff --git a/lib/declarative_authorization/authorization.rb b/lib/declarative_authorization/authorization.rb index 03c4995..cb7d5f0 100644 --- a/lib/declarative_authorization/authorization.rb +++ b/lib/declarative_authorization/authorization.rb @@ -214,6 +214,7 @@ def permit!(privilege, options = {}) puts "\n==== Processing new rule ====" puts "Rule: #{rule.inspect}" puts "Role: #{rule.role}" + puts "Join Operator: #{rule.join_operator}" if rule.respond_to?(:join_operator) permission_to_check = rule.role.to_s.gsub("__", "_") + "_permission" puts "Permission to check: #{permission_to_check}" @@ -230,6 +231,9 @@ def permit!(privilege, options = {}) return true if authorized else puts "Rule has #{rule.attributes.count} attributes, examining them:" + + all_attributes_matched = true + any_attribute_matched = false rule.attributes.each_with_index do |attribute, index| puts "\n -- Attribute ##{index + 1}: #{attribute.inspect}" @@ -245,7 +249,7 @@ def permit!(privilege, options = {}) next unless conditions.is_a?(Hash) puts " Checking conditions against current values:" - condition_matched = false + current_attribute_matched = true if conditions.key?(:granular_permissions) rule_requires = conditions[:granular_permissions][1] @@ -253,16 +257,14 @@ def permit!(privilege, options = {}) puts " Granular permissions - Rule requires: #{rule_requires}, Actual: #{actual_value}" if rule_requires == actual_value - condition_matched = true puts " ✓ Granular permissions condition matched!" else puts " ✗ Granular permissions condition did not match" + current_attribute_matched = false end - else - puts " (No granular_permissions condition to check)" end - if conditions.key?(:is_renewal) + if conditions.key?(:is_renewal) && current_attribute_matched rule_requires = conditions[:is_renewal][1] # If options[:object] has is_renewal, we'll use that value instead of obtaining it from SpiceDB. # This allows for Blue Moon leases and others to be checked without having to make a SpiceDB call for the value of is_renewal. @@ -276,36 +278,73 @@ def permit!(privilege, options = {}) resource_type: "lease_document", resource_id: options[:object]&.lease_document_uuid.to_s, permission: "renewal", - subject_type: "lease_document", + subject_type: "lease_document" ) - actual_value = response[:subject_ids].any? + actual_value = response.any? end puts " Is renewal - Rule requires: #{rule_requires}, Actual: #{actual_value}" if rule_requires == actual_value - condition_matched = true puts " ✓ Is_renewal condition matched!" else puts " ✗ Is_renewal condition did not match" + current_attribute_matched = false + end + end + + if conditions.key?(:id) && current_attribute_matched + condition_type = conditions[:id][0] + condition_proc = conditions[:id][1] + + if condition_type == :id_in_scope && condition_proc.is_a?(Proc) + puts " Checking Occupancy ID in scope condition" + user_has_access_to_occupancy = attribute.validate?(attr_validator) + puts " User has access to occupancy? #{user_has_access_to_occupancy}" + if user_has_access_to_occupancy + puts " ✓ This attribute matched conditions" + else + puts " ✗ This attribute did not match conditions" + current_attribute_matched = false + end end + end + + if current_attribute_matched + any_attribute_matched = true + puts " ✓ All conditions matched for this attribute" else - puts " (No is_renewal condition to check)" + all_attributes_matched = false + puts " ✗ Not all conditions matched for this attribute" end - - if condition_matched - # For now, we will assume each rule's conditions are OR'd together. This is not the case - # for all rules, but it's the most common case and a good starting point. - puts " At least one matching condition found, checking spicedb" - + end + + puts " Finished checking attributes, checking if rule is authorized" + + if rule.respond_to?(:join_operator) && rule.join_operator == :and + puts " Rule uses AND operator, checking if all attributes matched: #{all_attributes_matched}" + if all_attributes_matched + puts " All attributes matched, checking SpiceDB permission" + authorized = @auth_service.check_permission( + resource: { type: "vhost", id: vhost_id }, + permission: permission_to_check + ) + puts " Authorized? #{authorized}" + return true if authorized + else + puts " Not all attributes matched, authorization denied for this rule" + end + else + puts " Rule uses OR operator (default), checking if any attribute matched: #{any_attribute_matched}" + if any_attribute_matched + puts " At least one attribute matched, checking SpiceDB permission" authorized = @auth_service.check_permission( resource: { type: "vhost", id: vhost_id }, permission: permission_to_check ) puts " Authorized? #{authorized}" - return true if authorized else - puts " No matching conditions, skipping spicedb check for this attribute" + puts " No attributes matched, authorization denied for this rule" end end end From 777b4b9c3812f4305d928fcfa5136d1d45210537 Mon Sep 17 00:00:00 2001 From: Danny Eldridge Date: Tue, 18 Mar 2025 12:37:00 -0700 Subject: [PATCH 3/3] fixup! Handle id_in_scope attribute checks --- .../authorization.rb | 34 ++++++++++--------- 1 file changed, 18 insertions(+), 16 deletions(-) diff --git a/lib/declarative_authorization/authorization.rb b/lib/declarative_authorization/authorization.rb index cb7d5f0..bef46b5 100644 --- a/lib/declarative_authorization/authorization.rb +++ b/lib/declarative_authorization/authorization.rb @@ -232,8 +232,7 @@ def permit!(privilege, options = {}) else puts "Rule has #{rule.attributes.count} attributes, examining them:" - all_attributes_matched = true - any_attribute_matched = false + matching_attributes_count = 0 rule.attributes.each_with_index do |attribute, index| puts "\n -- Attribute ##{index + 1}: #{attribute.inspect}" @@ -248,8 +247,9 @@ def permit!(privilege, options = {}) next unless conditions.is_a?(Hash) - puts " Checking conditions against current values:" - current_attribute_matched = true + puts " Checking #{conditions.count} conditions against current values:" + matching_conditions_count = 0 + any_condition_failed = false if conditions.key?(:granular_permissions) rule_requires = conditions[:granular_permissions][1] @@ -258,13 +258,14 @@ def permit!(privilege, options = {}) puts " Granular permissions - Rule requires: #{rule_requires}, Actual: #{actual_value}" if rule_requires == actual_value puts " ✓ Granular permissions condition matched!" + matching_conditions_count += 1 else puts " ✗ Granular permissions condition did not match" - current_attribute_matched = false + any_condition_failed = true end end - if conditions.key?(:is_renewal) && current_attribute_matched + if conditions.key?(:is_renewal) && !any_condition_failed rule_requires = conditions[:is_renewal][1] # If options[:object] has is_renewal, we'll use that value instead of obtaining it from SpiceDB. # This allows for Blue Moon leases and others to be checked without having to make a SpiceDB call for the value of is_renewal. @@ -286,13 +287,14 @@ def permit!(privilege, options = {}) puts " Is renewal - Rule requires: #{rule_requires}, Actual: #{actual_value}" if rule_requires == actual_value puts " ✓ Is_renewal condition matched!" + matching_conditions_count += 1 else puts " ✗ Is_renewal condition did not match" - current_attribute_matched = false + any_condition_failed = true end end - if conditions.key?(:id) && current_attribute_matched + if conditions.key?(:id) && !any_condition_failed condition_type = conditions[:id][0] condition_proc = conditions[:id][1] @@ -302,18 +304,18 @@ def permit!(privilege, options = {}) puts " User has access to occupancy? #{user_has_access_to_occupancy}" if user_has_access_to_occupancy puts " ✓ This attribute matched conditions" + matching_conditions_count += 1 else puts " ✗ This attribute did not match conditions" - current_attribute_matched = false + any_condition_failed = true end end end - if current_attribute_matched - any_attribute_matched = true + if matching_conditions_count == conditions.count puts " ✓ All conditions matched for this attribute" + matching_attributes_count += 1 else - all_attributes_matched = false puts " ✗ Not all conditions matched for this attribute" end end @@ -321,8 +323,8 @@ def permit!(privilege, options = {}) puts " Finished checking attributes, checking if rule is authorized" if rule.respond_to?(:join_operator) && rule.join_operator == :and - puts " Rule uses AND operator, checking if all attributes matched: #{all_attributes_matched}" - if all_attributes_matched + puts " Rule uses AND operator, checking if all attributes matched: #{matching_attributes_count == rule.attributes.count}" + if matching_attributes_count == rule.attributes.count puts " All attributes matched, checking SpiceDB permission" authorized = @auth_service.check_permission( resource: { type: "vhost", id: vhost_id }, @@ -334,8 +336,8 @@ def permit!(privilege, options = {}) puts " Not all attributes matched, authorization denied for this rule" end else - puts " Rule uses OR operator (default), checking if any attribute matched: #{any_attribute_matched}" - if any_attribute_matched + puts " Rule uses OR operator (default), checking if any attribute matched: #{matching_attributes_count > 0}" + if matching_attributes_count > 0 puts " At least one attribute matched, checking SpiceDB permission" authorized = @auth_service.check_permission( resource: { type: "vhost", id: vhost_id },