Skip to content

Commit

Permalink
Authorize user with non-dynamic product feature if included in user's…
Browse files Browse the repository at this point in the history
… role

- This was a bug in the logic in ManageIQ#18102
- This fixes test failures in the API repo
  • Loading branch information
gtanzillo committed Nov 8, 2018
1 parent 879bc8f commit d819beb
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 4 deletions.
2 changes: 1 addition & 1 deletion app/models/miq_product_feature.rb
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ def self.root_tenant_identifier?(identifier)
end

def self.current_tenant_identifier(identifier)
identifier && feature_details(identifier) && root_tenant_identifier?(identifier) ? tenant_identifier(identifier, User.current_tenant.id) : identifier
tenant_identifier(identifier, User.current_tenant.id) if identifier && feature_details(identifier) && root_tenant_identifier?(identifier)
end

def self.feature_yaml(path = FIXTURE_PATH)
Expand Down
4 changes: 3 additions & 1 deletion lib/rbac/authorizer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,12 @@ def role_allows?(options = {})

any = options[:any]

identifier = MiqProductFeature.current_tenant_identifier(identifier)
tenant_identifier = MiqProductFeature.current_tenant_identifier(identifier)

auth = if any.present?
user_role_allows_any?(user, :identifiers => (identifiers || [identifier]))
elsif tenant_identifier
[identifier, tenant_identifier].any? { |i| user_role_allows?(user, :identifier => i) }
else
user_role_allows?(user, :identifier => identifier)
end
Expand Down
15 changes: 13 additions & 2 deletions spec/models/miq_user_role_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -81,12 +81,16 @@
let!(:tenant_1) { FactoryGirl.create(:tenant, :parent => root_tenant) }
let!(:tenant_2) { FactoryGirl.create(:tenant, :parent => root_tenant) }

let(:feature) { MiqProductFeature.find_all_by_identifier(["dialog_edit_editor_tenant_#{tenant_2.id}", "rbac_tenant_manage_quotas_tenant_#{tenant_2.id}"]) }
let(:role) { FactoryGirl.create(:miq_user_role, :miq_product_features => feature) }
let(:feature) { MiqProductFeature.find_all_by_identifier(["dialog_edit_editor_tenant_#{tenant_2.id}", "rbac_tenant_manage_quotas_tenant_#{tenant_2.id}"]) }
let(:non_dynamic_feature) { MiqProductFeature.find_all_by_identifier(["dialog_edit_editor", "rbac_tenant_manage_quotas"]) }
let(:role) { FactoryGirl.create(:miq_user_role, :miq_product_features => feature) }
let(:role_no_dynamic) { FactoryGirl.create(:miq_user_role, :miq_product_features => non_dynamic_feature) }
let(:group_tenant_1) { FactoryGirl.create(:miq_group, :miq_user_role => role, :tenant => tenant_1) }
let(:group_tenant_2) { FactoryGirl.create(:miq_group, :miq_user_role => role, :tenant => tenant_2) }
let(:group_3) { FactoryGirl.create(:miq_group, :miq_user_role => role_no_dynamic, :tenant => tenant_2) }
let!(:user_1) { FactoryGirl.create(:user, :userid => "user_1", :miq_groups => [group_tenant_1]) }
let!(:user_2) { FactoryGirl.create(:user, :userid => "user_2", :miq_groups => [group_tenant_2]) }
let!(:user_3) { FactoryGirl.create(:user, :userid => "user_3", :miq_groups => [group_3]) }

it "doesn't authorise user without dynamic product feature" do
User.with_user(user_1) do
Expand All @@ -101,6 +105,13 @@
expect(user_2.role_allows?(:identifier => "rbac_tenant_manage_quotas")).to be_truthy
end
end

it "authorise user with non-dynamic product feature" do
User.with_user(user_3) do
expect(user_3.role_allows?(:identifier => "dialog_edit_editor")).to be_truthy
expect(user_3.role_allows?(:identifier => "rbac_tenant_manage_quotas")).to be_truthy
end
end
end

it "should return the correct answer calling allows? when requested feature is directly assigned or a descendant of a feature in a role" do
Expand Down

0 comments on commit d819beb

Please sign in to comment.