Skip to content

Commit

Permalink
move code to more central context
Browse files Browse the repository at this point in the history
  • Loading branch information
ulferts committed Jun 10, 2024
1 parent 160b8aa commit 5081eb5
Show file tree
Hide file tree
Showing 5 changed files with 116 additions and 46 deletions.
24 changes: 17 additions & 7 deletions app/models/users/permission_checks.rb
Original file line number Diff line number Diff line change
Expand Up @@ -106,14 +106,24 @@ def member_of?(project)
roles_for_project(project).any?(&:member?)
end

# Returns all permissions the user may have for a given context.
# "May" because this method does not check e.g. whether the module
# the permission belongs to is active.
def all_permissions_for(context)
Authorization
.roles(self, context)
.includes(:role_permissions)
.pluck(:permission)
.compact
.map(&:to_sym)
.uniq
if admin?
OpenProject::AccessControl
.permissions
.select { |p| p.permissible_on?(context) && p.grant_to_admin? }
.map(&:name)
else
Authorization
.roles(self, context)
.includes(:role_permissions)
.pluck(:permission)
.compact
.map(&:to_sym)
.uniq
end
end

# Helper method to be used in places where we just throw anything into the permission check and don't know what
Expand Down
20 changes: 1 addition & 19 deletions app/services/authorization/user_permissible_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -63,25 +63,7 @@ def allowed_in_any_entity?(permission, entity_class, in_project: nil) # rubocop:

def cached_permissions(context)
@cached_permissions ||= Hash.new do |hash, context_key|
hash[context_key] = if user.admin?
permissible_key = case context_key
when WorkPackage
:work_package
when Project
:project
when nil
:global
else
raise "Unknown context key: #{context_key}"
end

OpenProject::AccessControl
.permissions
.select { |p| p.permissible_on?(permissible_key) && p.grant_to_admin? }
.map(&:name)
else
user.all_permissions_for(context_key)
end
hash[context_key] = user.all_permissions_for(context_key)
end

@cached_permissions[context]
Expand Down
15 changes: 14 additions & 1 deletion lib/open_project/access_control/permission.rb
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,20 @@ def global?
end

def permissible_on?(context_type)
@permissible_on.include?(context_type)
context_symbol = case context_type
when WorkPackage
:work_package
when Project
:project
when Symbol
context_type
when nil
:global
else
raise "Unknown context: #{context_type}"
end

@permissible_on.include?(context_symbol)
end

def grant_to_admin?
Expand Down
41 changes: 41 additions & 0 deletions spec/lib/open_project/access_control/permission_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,47 @@
end
end

describe "#permissible_on?" do
context "when marked as permissible on work package roles" do
subject(:permission) do
described_class.new(:perm, { cont: [:action] }, permissible_on: :work_package)
end

it { expect(permission).to be_permissible_on(WorkPackage.new) }
it { expect(permission).not_to be_permissible_on(Project.new) }
it { expect(permission).not_to be_permissible_on(nil) }
it { expect(permission).to be_permissible_on(:work_package) }
it { expect(permission).not_to be_permissible_on(:project) }
it { expect(permission).not_to be_permissible_on(:global) }
end

context "when marked as permissible on project roles" do
subject(:permission) do
described_class.new(:perm, { cont: [:action] }, permissible_on: :project)
end

it { expect(permission).not_to be_permissible_on(WorkPackage.new) }
it { expect(permission).to be_permissible_on(Project.new) }
it { expect(permission).not_to be_permissible_on(nil) }
it { expect(permission).not_to be_permissible_on(:work_package) }
it { expect(permission).to be_permissible_on(:project) }
it { expect(permission).not_to be_permissible_on(:global) }
end

context "when marked as permissible on global roles" do
subject(:permission) do
described_class.new(:perm, { cont: [:action] }, permissible_on: :global)
end

it { expect(permission).not_to be_permissible_on(WorkPackage.new) }
it { expect(permission).not_to be_permissible_on(Project.new) }
it { expect(permission).to be_permissible_on(nil) }
it { expect(permission).not_to be_permissible_on(:work_package) }
it { expect(permission).not_to be_permissible_on(:project) }
it { expect(permission).to be_permissible_on(:global) }
end
end

describe "marking it as permissible on multiple role types" do
subject(:permission) do
described_class.new(:perm, { cont: [:action] }, permissible_on: %i[work_package project])
Expand Down
62 changes: 43 additions & 19 deletions spec/models/users/permission_checks_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -177,31 +177,55 @@

let(:public_permissions) { OpenProject::AccessControl.public_permissions.map(&:name) }

subject do
create(:user, global_permissions: [:create_user],
member_with_permissions: { project => %i[view_work_packages edit_work_packages] })
end
context "for a non admin user" do
subject do
create(:user, global_permissions: [:create_user],
member_with_permissions: { project => %i[view_work_packages edit_work_packages] })
end

it "returns all permissions given on the project" do
expect(subject.all_permissions_for(project)).to match_array(%i[view_work_packages edit_work_packages] + public_permissions)
end
it "returns all permissions given on the project" do
expect(subject.all_permissions_for(project))
.to match_array(%i[view_work_packages edit_work_packages] + public_permissions)
end

it "returns non-member permissions given on the project the user is not a member of" do
expect(subject.all_permissions_for(other_project)).to match_array(%i[view_work_packages
manage_members] + public_permissions)
end
it "returns non-member permissions given on the project the user is not a member of" do
expect(subject.all_permissions_for(other_project)).to match_array(%i[view_work_packages
manage_members] + public_permissions)
end

it "returns all global permissions" do
skip "Current implementation of the Authorization.roles query returns ALL permissions the user has, not only global ones. " \
"We should change this in the fututre, thats why this test is already in here."
it "returns all global permissions" do
skip "Current implementation of the Authorization.roles query returns ALL permissions the user has, " \
"not only global ones. We should change this in the future, that's why this test is already in here."

expect(subject.all_permissions_for(nil)).to match_array(%i[create_user])
end

expect(subject.all_permissions_for(nil)).to match_array(%i[create_user])
it "returns all permissions the user has (with project and global permissions)" do
expect(subject.all_permissions_for(nil)).to match_array(%i[create_user
view_work_packages edit_work_packages
manage_members] + public_permissions)
end
end

it "returns all permissions the user has (with project and global permissions)" do
expect(subject.all_permissions_for(nil)).to match_array(%i[create_user
view_work_packages edit_work_packages
manage_members] + public_permissions)
context "for an admin user" do
subject do
create(:admin)
end

it "returns all permissions given on the project" do
expect(subject.all_permissions_for(project))
.to include(:view_work_packages, :edit_work_packages, *public_permissions)
end

it "returns all permissions given globally" do
expect(subject.all_permissions_for(nil))
.to include(:create_user)
end

it "returns all permissions given on a work package" do
expect(subject.all_permissions_for(WorkPackage.new))
.to include(:view_work_packages, :edit_work_packages)
end
end
end

Expand Down

0 comments on commit 5081eb5

Please sign in to comment.