Sutty/trabajo-afectivo
5
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

Fixes #3111 - admin.user token permission doesn't allow fetching details of specific user

Browse source
This commit is contained in:
Ryan Lue 2020-08-14 13:12:41 +02:00 committed by Thorsten Eckel
parent 900d8fcd44
commit dde7208ebc
6 changed files with 213 additions and 119 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
app/controllers/user_access_token_controller.rb
View file

@ -32,7 +32,7 @@ curl http://localhost/api/v1/user_access_token -v -u #{login}:#{password}
.order(updated_at: :desc, label: :asc)
base_query = Permission.order(:name).where(active: true)
permission_names = current_user.permissions.keys
permission_names = current_user.permissions.pluck(:name)
ancestor_names = permission_names.flat_map { |name| Permission.with_parents(name) }.uniq -
permission_names
descendant_names = permission_names.map { |name| "#{name}.%" }

40
app/models/concerns/can_be_authorized.rb Normal file
View file

@ -0,0 +1,40 @@
module CanBeAuthorized
extend ActiveSupport::Concern
=begin
true or false for permission
user = User.find(123)
user.permissions?('permission.key') # access to certain permission.key
user.permissions?(['permission.key1', 'permission.key2']) # access to permission.key1 or permission.key2
user.permissions?('permission') # access to all sub keys
user.permissions?('permission.*') # access if one sub key access exists
returns
true|false
=end
def permissions?(auth_query)
verbatim, wildcards = acceptable_permissions_for(auth_query)
permissions.where(name: verbatim).then do |base_query|
wildcards.reduce(base_query) do |query, name|
query.or(permissions.where('permissions.name LIKE ?', name.sub('.*', '.%')))
end
end.exists?
end
private
def acceptable_permissions_for(auth_query)
Array(auth_query)
.reject { |name| Permission.lookup(name: name)&.active == false } # See "chain-of-ancestry quirk" in spec file
.flat_map { |name| Permission.with_parents(name) }.uniq
.partition { |name| name.end_with?('.*') }.reverse
end
end

20
app/models/token.rb
View file

@ -1,6 +1,8 @@
# Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/
class Token < ApplicationModel
include CanBeAuthorized
before_create :generate_token
belongs_to :user, optional: true
store :preferences
@ -99,15 +101,17 @@ cleanup old token
true
end
def permissions?(permissions)
return false if !user.permissions?(permissions)
return false if preferences[:permission].blank?
def permissions
Permission.where(
name: Array(preferences[:permission]),
active: true,
)
end
Array(permissions).any? do |parentless|
Permission.with_parents(parentless).any? do |permission|
preferences[:permission].include?(permission)
end
end
def permissions?(names)
return false if !user.permissions?(names)
super(names)
end
private

69
app/models/user.rb
View file

@ -2,6 +2,7 @@
require_dependency 'karma/user'
class User < ApplicationModel
include CanBeAuthorized
include CanBeImported
include HasActivityStreamLog
include ChecksClientNotification
@ -22,6 +23,7 @@ class User < ApplicationModel
has_many :tokens, after_add: :cache_update, after_remove: :cache_update
has_many :authorizations, after_add: :cache_update, after_remove: :cache_update
belongs_to :organization, inverse_of: :members, optional: true
has_many :permissions, -> { where(roles: { active: true }, active: true) }, through: :roles
before_validation :check_name, :check_email, :check_login, :ensure_uniq_email, :ensure_password, :ensure_roles, :ensure_identifier
before_validation :check_mail_delivery_failed, on: :update
@ -42,6 +44,8 @@ class User < ApplicationModel
:image_source,
:preferences
association_attributes_ignored :permissions
history_attributes_ignored :password,
:last_login,
:image,
@ -377,69 +381,6 @@ returns
=begin
get all permissions of user
user = User.find(123)
user.permissions
returns
{
'permission.key' => true,
# ...
}
=end
def permissions
::Permission.joins(roles: :users)
.where(users: { id: id }, roles: { active: true }, active: true)
.pluck(:name, :preferences)
.each_with_object({}) do |(name, preferences), hash|
hash[name] = true if preferences['selectable'] != false
end
end
=begin
true or false for permission
user = User.find(123)
user.permissions?('permission.key') # access to certain permission.key
user.permissions?(['permission.key1', 'permission.key2']) # access to permission.key1 or permission.key2
user.permissions?('permission') # access to all sub keys
user.permissions?('permission.*') # access if one sub key access exists
returns
true|false
=end
def permissions?(names)
base_query = ::Permission.joins(roles: :users)
.where(users: { id: id })
.where(roles: { active: true })
.where(active: true)
permission_names = Array(names).reject { |name| ::Permission.lookup(name: name)&.active == false }
verbatim_names = permission_names.flat_map { |name| ::Permission.with_parents(name) }.uniq
wildcard_names = permission_names.select { |name| name.end_with?('.*') }
.map { |name| name.sub('.*', '.%') }
permissions = base_query.where(name: verbatim_names)
wildcard_names.each do |name|
permissions = permissions.or(base_query.where('permissions.name LIKE ?', name))
end
permissions.exists?
end
=begin
returns all accessable permission ids of user
user = User.find(123)
@ -454,7 +395,7 @@ returns
def permissions_with_child_ids
where = ''
where_bind = [true]
permissions.each_key do |permission_name|
permissions.pluck(:name).each do |permission_name|
where += ' OR ' if where != ''
where += 'permissions.name = ? OR permissions.name LIKE ?'
where_bind.push permission_name

129
spec/models/token_spec.rb
View file

@ -127,6 +127,135 @@ RSpec.describe Token, type: :model do
end
end
describe '#permissions?' do
subject(:token) do
create(:token, user: user, preferences: { permission: [permission_name] })
end
let(:user) { create(:user, roles: [role]) }
let(:role) { create(:role, permissions: [permission]) }
let(:permission) { create(:permission, name: permission_name) }
context 'with privileges for a root permission (e.g., "foo", not "foo.bar")' do
let(:permission_name) { 'foo' }
context 'when given that exact permission' do
it 'returns true' do
expect(token.permissions?('foo')).to be(true)
end
end
context 'when given a sub-permission (i.e., child permission)' do
let(:subpermission) { create(:permission, name: 'foo.bar') }
context 'that exists' do
before { subpermission }
it 'returns true' do
expect(token.permissions?('foo.bar')).to be(true)
end
end
context 'that is inactive' do
before { subpermission.update(active: false) }
it 'returns false' do
expect(token.permissions?('foo.bar')).to be(false)
end
end
context 'that does not exist' do
it 'returns true' do
expect(token.permissions?('foo.bar')).to be(true)
end
end
end
context 'when given a glob' do
context 'matching that permission' do
it 'returns true' do
expect(token.permissions?('foo.*')).to be(true)
end
end
context 'NOT matching that permission' do
it 'returns false' do
expect(token.permissions?('bar.*')).to be(false)
end
end
end
end
context 'with privileges for a sub-permission (e.g., "foo.bar", not "foo")' do
let(:permission_name) { 'foo.bar' }
context 'when given that exact sub-permission' do
it 'returns true' do
expect(token.permissions?('foo.bar')).to be(true)
end
context 'but the permission is inactive' do
before { permission.update(active: false) }
it 'returns false' do
expect(token.permissions?('foo.bar')).to be(false)
end
end
end
context 'when given a sibling sub-permission' do
let(:sibling_permission) { create(:permission, name: 'foo.baz') }
context 'that exists' do
before { sibling_permission }
it 'returns false' do
expect(token.permissions?('foo.baz')).to be(false)
end
end
context 'that does not exist' do
it 'returns false' do
expect(token.permissions?('foo.baz')).to be(false)
end
end
end
context 'when given the parent permission' do
it 'returns false' do
expect(token.permissions?('foo')).to be(false)
end
end
context 'when given a glob' do
context 'matching that sub-permission' do
it 'returns true' do
expect(token.permissions?('foo.*')).to be(true)
end
context 'but the permission is inactive' do
before { permission.update(active: false) }
context 'and user.permissions?(...) doesn’t fail' do
let(:role) { create(:role, permissions: [parent_permission]) }
let(:parent_permission) { create(:permission, name: permission_name.split('.').first) }
it 'returns false' do
expect(token.permissions?('foo.*')).to be(false)
end
end
end
end
context 'NOT matching that sub-permission' do
it 'returns false' do
expect(token.permissions?('bar.*')).to be(false)
end
end
end
end
end
describe 'Attributes:' do
describe '#persistent' do
context 'when not set on creation' do

72
spec/models/user_spec.rb
View file

@ -374,44 +374,24 @@ RSpec.describe User, type: :model do
let(:role) { user.roles.first }
let(:permissions) { role.permissions }
it 'returns a hash of <permissions> => true' do
expect(user.permissions).to eq(permissions.each.with_object({}) { |p, hash| hash[p.name] = true })
end
shared_examples 'for omissions' do
it 'omits them from the returned hash' do
expect(user.permissions.keys).not_to include(*omitted_permissions.map(&:name))
end
end
context 'for permissions that do not belong to this user' do
let(:omitted_permissions) { Permission.all - permissions }
include_examples 'for omissions'
it 'is a simple association getter' do
expect(user.permissions).to match_array(permissions)
end
context 'for inactive permissions' do
before { omitted_permissions.each { |p| p.update(active: false) } }
before { permissions.first.update(active: false) }
let!(:omitted_permissions) { permissions.first(1) }
include_examples 'for omissions'
it 'omits them from the returned hash' do
expect(user.permissions).not_to include(permissions.first)
end
end
context 'for permissions on inactive roles' do
before { role.update(active: false) }
let(:omitted_permissions) { permissions }
include_examples 'for omissions'
end
context 'for permissions with !preferences["selectable"]' do
before { omitted_permissions.each { |p| p.update(preferences: { selectable: false }) } }
let(:omitted_permissions) { permissions.first(1) }
include_examples 'for omissions'
it 'omits them from the returned hash' do
expect(user.permissions).not_to include(*role.permissions)
end
end
end
@ -430,28 +410,28 @@ RSpec.describe User, type: :model do
end
end
context 'when given a sub-permission (i.e., child permission)' do
let(:subpermission) { create(:permission, name: 'foo.bar') }
context 'when given an active sub-permission' do
before { create(:permission, name: 'foo.bar') }
context 'that exists' do
before { subpermission }
it 'returns true' do
expect(user.permissions?('foo.bar')).to be(true)
end
end
it 'returns true' do
expect(user.permissions?('foo.bar')).to be(true)
describe 'chain-of-ancestry quirk' do
context 'when given an inactive sub-permission' do
before { create(:permission, name: 'foo.bar.baz', active: false) }
it 'returns false, even with active ancestors' do
expect(user.permissions?('foo.bar.baz')).to be(false)
end
end
context 'that is inactive' do
before { subpermission.update(active: false) }
context 'when given a sub-permission that does not exist' do
before { create(:permission, name: 'foo.bar', active: false) }
it 'returns false' do
expect(user.permissions?('foo.bar')).to be(false)
end
end
context 'that does not exist' do
it 'returns true' do
expect(user.permissions?('foo.bar')).to be(true)
it 'can return true, even with inactive ancestors' do
expect(user.permissions?('foo.bar.baz')).to be(true)
end
end
end
@ -522,7 +502,7 @@ RSpec.describe User, type: :model do
before { permission.update(active: false) }
it 'returns false' do
expect(user.permissions?('foo.bar')).to be(false)
expect(user.permissions?('foo.*')).to be(false)
end
end
end