Skip to content

Commit

Permalink
Merge pull request #14092 from opf/replace-allowed-to-in-application-…
Browse files Browse the repository at this point in the history
…controller

Permission Rework [18/18]: Use new permission check methods in ApplicationController and action authorization
  • Loading branch information
ulferts authored Nov 7, 2023
2 parents b078a88 + f7fb74d commit 53bab1f
Show file tree
Hide file tree
Showing 4 changed files with 95 additions and 14 deletions.
90 changes: 82 additions & 8 deletions app/controllers/application_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -231,15 +231,77 @@ def deny_access(not_found: false)
end
end

def authorize_in_project(ctrl = params[:controller], action = params[:action])
authorization_project = @project || @projects
allowed = User.current.allowed_in_project?({ controller: ctrl, action: }, authorization_project)

unless allowed
if @project&.archived?
render_403 message: :notice_not_authorized_archived_project
else
deny_access
end
end

allowed
end

def authorize_globally(ctrl = params[:controller], action = params[:action])
allowed = User.current.allowed_globally?({ controller: ctrl, action: })

deny_access unless allowed

allowed
end

def authorize_in_any_project(ctrl = params[:controller], action = params[:action])
allowed = User.current.allowed_in_any_project?({ controller: ctrl, action: })

deny_access unless allowed

allowed
end

def authorize_in_model(ctrl = params[:controller], action = params[:action])
model = instance_variable_get("@#{model_object.to_s.underscore}")

allowed = User.current.allowed_in_entity?({ controller: ctrl, action: }, model, model_object)

unless allowed
if model.respond_to?(:project) && model.project&.archived?
render_403 message: :notice_not_authorized_archived_project
else
deny_access
end
end

allowed
end

def authorize_in_any_model(ctrl = params[:controller], action = params[:action])
allowed = if @project
User.current.allowed_in_any_entity?({ controller: ctrl, action: }, model_object, in_project: @project)
else
User.current.allowed_in_any_entity?({ controller: ctrl, action: }, model_object)
end

deny_access unless allowed

allowed
end

# Authorize the user for the requested controller action.
# To be used in before_action hooks
def authorize(ctrl = params[:controller], action = params[:action])
# OpenProject::Deprecation.deprecate_method(ApplicationController, :authorize)
do_authorize({ controller: ctrl, action: }, global: false)
end

# Authorize the user for the requested controller action outside a project
# To be used in before_action hooks
def authorize_global
# OpenProject::Deprecation.deprecate_method(ApplicationController, :authorize_global)

action = { controller: params[:controller], action: params[:action] }
do_authorize(action, global: true)
end
Expand All @@ -249,9 +311,21 @@ def authorize_global
# Action can be:
# * a parameter-like Hash (eg. { controller: '/projects', action: 'edit' })
# * a permission Symbol (eg. :edit_project)
def do_authorize(action, global: false)
context = @project || @projects
is_authorized = User.current.allowed_to?(action, context, global:)
def do_authorize(action, global: false) # rubocop:disable Metrics/PerceivedComplexity
# OpenProject::Deprecation.deprecate_method(ApplicationController, :do_authorize)
# context = @project || @projects
# old_authorized = User.current.allowed_to?(action, context, global:)

is_authorized = begin
if global
User.current.allowed_based_on_permission_context?(action)
else
User.current.allowed_based_on_permission_context?(action, project: @project || @projects,
entity: @work_package || @work_packages)
end
rescue Authorization::UnknownPermissionError
false
end

unless is_authorized
if @project&.archived?
Expand Down Expand Up @@ -289,14 +363,14 @@ def find_optional_project

def find_optional_project_and_raise_error
@project = Project.find(params[:project_id]) if params[:project_id].present?
allowed = User.current.allowed_to?({ controller: params[:controller], action: params[:action] },
@project, global: @project.nil?)
allowed = User.current.allowed_based_on_permission_context?({ controller: params[:controller], action: params[:action] },
project: @project)
allowed ? true : deny_access
end

# Finds and sets @project based on @object.project
def find_project_from_association
render_404 unless @object.present?
render_404 if @object.blank?

@project = @object.project
rescue ActiveRecord::RecordNotFound
Expand All @@ -307,7 +381,7 @@ def find_model_object(object_id = :id)
model = self.class._model_object
if model
@object = model.find(params[object_id])
instance_variable_set('@' + controller_name.singularize, @object) if @object
instance_variable_set("@#{controller_name.singularize}", @object) if @object
end
rescue ActiveRecord::RecordNotFound
render_404
Expand All @@ -318,7 +392,7 @@ def find_model_object_and_project(object_id = :id)
model_object = self.class._model_object
instance = model_object.find(params[object_id])
@project = instance.project
instance_variable_set('@' + model_object.to_s.underscore, instance)
instance_variable_set("@#{model_object.to_s.underscore}", instance)
else
@project = Project.find(params[:project_id])
end
Expand Down
10 changes: 7 additions & 3 deletions app/models/users/permission_checks.rb
Original file line number Diff line number Diff line change
Expand Up @@ -103,13 +103,17 @@ def allowed_based_on_permission_context?(permission, project: nil, entity: nil)
permissions.any? do |perm|
if perm.global?
allowed_globally?(perm)
elsif perm.work_package? && entity.is_a?(WorkPackage)
elsif perm.work_package? && (entity.is_a?(WorkPackage) || (entity.is_a?(Array) && entity.all? { |e| e.is_a?(WorkPackage) }))
allowed_in_work_package?(perm, entity)
elsif perm.work_package? && entity.blank? && project.blank?
allowed_in_any_work_package?(perm)
elsif perm.work_package? && project && entity.blank?
allowed_in_any_work_package?(perm, in_project: project)
elsif perm.project? && project
allowed_in_project?(perm, project)
elsif perm.project? && entity && entity.respond_to?(:project)
allowed_in_project?(perm, entity.project)
elsif perm.project? && entity.nil? && project.nil?
elsif perm.project? && entity.blank? && project.blank?
allowed_in_any_project?(perm)
else
false
Expand All @@ -119,7 +123,7 @@ def allowed_based_on_permission_context?(permission, project: nil, entity: nil)

# Old allowed_to? interface. Marked as deprecated, should be removed at some point ... Guessing 14.0?
def allowed_to?(action, context, global: false)
# OpenProject::Deprecation.deprecate_method(User, :allowed_to?)
OpenProject::Deprecation.deprecate_method(User, :allowed_to?)
user_allowed_service.call(action, context, global:)
end

Expand Down
7 changes: 5 additions & 2 deletions app/services/authorization/user_permissible_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ def allowed_in_project?(permission, projects_to_check)

def allowed_in_any_project?(permission)
perms = contextual_permissions(permission, :project)
return false unless authorizable_user?
return true if admin_and_all_granted_to_admin?(perms)

Project.allowed_to(user, perms).exists?
Expand All @@ -48,15 +49,17 @@ def allowed_in_entity?(permission, entities_to_check, entity_class)

def allowed_in_any_entity?(permission, entity_class, in_project: nil)
perms = contextual_permissions(permission, context_name(entity_class))
return false unless authorizable_user?
return true if admin_and_all_granted_to_admin?(perms)

# entity_class.allowed_to will also check whether the user has the permission via a membership in the project.
# TODO: ^--- thats' actually not the case weirdly...
allowed_scope = entity_class.allowed_to(user, perms)

if in_project
allowed_scope.exists?(project: in_project)
allowed_in_single_project?(perms, in_project) || allowed_scope.exists?(project: in_project)
else
allowed_scope.exists?
allowed_in_any_project?(perms) || allowed_scope.exists?
end
end

Expand Down
2 changes: 1 addition & 1 deletion config/initializers/permissions.rb
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@

map.permission :manage_user,
{
users: %i[index show edit update],
users: %i[index show edit update change_status change_status_info],
'users/memberships': %i[create update destroy],
admin: %i[index]
},
Expand Down

0 comments on commit 53bab1f

Please sign in to comment.