Skip to content
Merged
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
14 changes: 1 addition & 13 deletions app/models/concerns/foreman_puppet/extensions/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,19 +9,7 @@ module User

module PatchedMethods
def visible_environments
authorized_scope = ForemanPuppet::Environment.unscoped.authorized(:view_environments)
authorized_scope = authorized_scope
.joins(:taxable_taxonomies)
.where('taxable_taxonomies.taxonomy_id' => taxonomy_ids[:organizations] + taxonomy_ids[:locations])
result = authorized_scope.distinct.pluck(:name)
if ::User.current.admin?
# Admin users can also see Environments that do not have any organization or location, even when
# organizations and locations are enabled.
untaxed_env_ids = TaxableTaxonomy.where(taxable_type: 'ForemanPuppet::Environment').distinct.select(:taxable_id)
untaxed_environments = ForemanPuppet::Environment.unscoped.where.not(id: untaxed_env_ids).pluck(:name)
result += untaxed_environments
end
result
ForemanPuppet::Environment.authorized(:view_environments).pluck(:name)
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This currently requires theforeman/foreman#10898 . An alternative would be this suggestion

Suggested change
ForemanPuppet::Environment.authorized(:view_environments).pluck(:name)
ForemanPuppet::Environment.authorized(:view_environments).distinct.pluck(:name)

end
end
end
Expand Down
2 changes: 1 addition & 1 deletion lib/foreman_puppet/register.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
Foreman::Plugin.register :foreman_puppet do
requires_foreman '>= 3.13'
requires_foreman '>= 3.19'
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for fixing! Let's keep in mind to do a major version bump during the next release.

register_gettext

# Add Global JS file for extending foreman-core components and routes
Expand Down
6 changes: 5 additions & 1 deletion test/models/foreman_puppet/user_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,12 @@ class UserTest < ActiveSupport::TestCase
end

test 'should show the list of environments visible as non-admin user' do
# Non-admin user only sees environments in a taxonomy at least
# Non-admin user only sees environments in the same taxonomies at least
setup_user 'view', 'environments'
# The user is in the same location as the environment, but in a
# different org. The org mismatch would make the environment invisible
# to the user.
User.current.organizations << environment.organizations.first
assert_equal [environment.name], ::User.current.visible_environments
end
end
Expand Down
Loading