Skip to content

Commit

Permalink
Merge pull request #221 from solver-it-sro/GO-401/deny_self_user_dele…
Browse files Browse the repository at this point in the history
…tion
  • Loading branch information
stage-rl authored Dec 4, 2023
2 parents 467dea4 + 2fc4777 commit 8592c14
Show file tree
Hide file tree
Showing 10 changed files with 59 additions and 49 deletions.
1 change: 1 addition & 0 deletions app/components/admin/groups/group_form_component.html.erb
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
<%= tag.turbo_frame id: "modal" do %>
<%= render Common::AlertComponent.new %>
<div class="fixed inset-0 z-40 p-2" role="dialog" aria-modal="true">
<div class="fixed inset-0 transition-opacity bg-gray-400 bg-opacity-75" aria-hidden="true"></div>
<div class="relative flex items-center justify-center h-full">
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
<div class="w-16 h-16 justify-center items-center flex">
<img class="w-16 h-16 rounded-full" src="https://via.placeholder.com/64x64" />
<img class="w-16 h-16 rounded-full" src="https://via.placeholder.com/64x64">
</div>
<div class="grow shrink basis-0 flex-col justify-start items-start gap-1 inline-flex">
<div class="text-center text-gray-900 text-lg font-medium leading-loose"><%= @user.name %></div>
<div class="text-center text-gray-500 text-base font-normal leading-normal"><%= @user.email %></div>
</div>
<%= button_to admin_tenant_group_group_membership_path(Current.tenant, @group_membership.group, @group_membership), method: :delete do %>
<%= render Common::DeleteButtonComponent.new %>
<% if Pundit.policy(Current.user, [:admin, @group_membership]).destroy? %>
<%= button_to admin_tenant_group_group_membership_path(Current.tenant, @group_membership.group, @group_membership), method: :delete do %>
<%= render Common::DeleteButtonComponent.new %>
<% end %>
<% end %>
1 change: 1 addition & 0 deletions app/components/admin/users/users_list_component.html.erb
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
<%= render Common::AlertComponent.new %>
<div class="w-full p-4 flex-col justify-start items-start gap-4 inline-flex">
<div class="self-stretch bg-white rounded-md border border-gray-200 flex-col justify-start items-start flex">
<div class="self-stretch p-6 border-b border-gray-200 justify-start items-center gap-4 inline-flex">
Expand Down
6 changes: 4 additions & 2 deletions app/components/admin/users/users_list_row_component.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,10 @@
<%= link_to edit_admin_tenant_user_path(@user.tenant, @user) do %>
<%= render Common::EditButtonComponent.new %>
<% end %>
<%= button_to admin_tenant_user_path(@user.tenant, @user), method: :delete do %>
<%= render Common::DeleteButtonComponent.new %>
<% if Pundit.policy(Current.user, [:admin, @user]).destroy? %>
<%= button_to admin_tenant_user_path(@user.tenant, [:admin, @user]), method: :delete do %>
<%= render Common::DeleteButtonComponent.new %>
<% end %>
<% end %>
</div>
</div>
2 changes: 1 addition & 1 deletion app/components/common/alert_component.html.erb
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<div class="fixed top-20 inset-x-0 m-4 z-30">
<div class="fixed top-20 inset-x-0 m-4 z-50">
<div class="flex flex-col items-end space-y-4">
<% flash.each do |type, msg| %>
<div data-controller="dismissible-alert" class="max-w-sm w-full pointer-events-auto rounded-md <%= type == 'notice' ? "bg-green-50" : "bg-red-50" %> p-4">
Expand Down
14 changes: 9 additions & 5 deletions app/controllers/admin/group_memberships_controller.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
class Admin::GroupMembershipsController < ApplicationController
before_action :set_group_membership, only: %i[destroy]
# TODO - rediscuss the whole concept of SITE_ADMIN vs TENANT admin responsibilities and functionality
# TODO: - rediscuss the whole concept of SITE_ADMIN vs TENANT admin responsibilities and functionality

def create
# TODO: Takto mi teoreticky moze vzniknut neopravneny membership, lebo nekontrolujem tenanta. Ako spravit tak, aby som nezacal pisat exlpicitne rucne kontroly?
Expand All @@ -17,15 +17,19 @@ def create

def destroy
authorize([:admin, @group_membership])
@group_membership.destroy
redirect_to edit_members_admin_tenant_group_path(Current.tenant, @group_membership.group),
notice: 'Group was membership was successfully deleted'
if @group_membership.destroy
redirect_to edit_members_admin_tenant_group_path(Current.tenant, @group_membership.group),
notice: 'Group was membership was successfully deleted'
else
flash[:alert] = @group_membership.errors.full_messages[0]
redirect_to edit_members_admin_tenant_group_path(Current.tenant, @group_membership.group)
end
end

private

def set_group_membership
@group_membership = policy_scope([:admin,GroupMembership]).find(params[:id])
@group_membership = policy_scope([:admin, GroupMembership]).find(params[:id])
end

def group_membership_params
Expand Down
8 changes: 6 additions & 2 deletions app/controllers/admin/users_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,12 @@ def update

def destroy
authorize([:admin, @user])
@user.destroy
redirect_to admin_tenant_users_url(Current.tenant), notice: 'User was successfully destroyed'
if @user.destroy
redirect_to admin_tenant_users_url(Current.tenant), notice: 'User was successfully destroyed'
else
flash[:alert] = @user.errors.full_messages[0]
redirect_to admin_tenant_users_url(Current.tenant)
end
end

private
Expand Down
2 changes: 1 addition & 1 deletion app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ class User < ApplicationRecord

has_many :group_memberships, dependent: :destroy
has_many :groups, through: :group_memberships
has_many :own_tags, class_name: 'Tag', foreign_key: 'user_id', inverse_of: :owner, dependent: :nullify
has_many :own_tags, class_name: 'Tag', inverse_of: :owner, dependent: :nullify
has_many :message_drafts, foreign_key: :author_id
has_many :automation_rules, class_name: 'Automation::Rule'
has_many :filters, foreign_key: :author_id
Expand Down
30 changes: 6 additions & 24 deletions app/policies/admin/group_membership_policy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,41 +13,23 @@ def resolve
if @user.site_admin?
scope.all
else
scope.includes(:user, :group).where(user: {tenant_id: Current.tenant.id}, group: {tenant_id: Current.tenant.id})
scope.includes(:user, :group).where(user: { tenant_id: Current.tenant.id }, group: { tenant_id: Current.tenant.id })
end
end
end

def index
@user.site_admin? || @user.admin?
end

def show?
@user.site_admin? || @user.admin?
end

def create?
return false if !@user.site_admin? && !@user.admin?
return false unless @user.site_admin? || @user.admin?
return false unless @group_membership.group.tenant == Current.tenant
return false unless @group_membership.user.tenant == Current.tenant

true
end

def new?
create?
end

def update?
@user.site_admin? || @user.admin?
end

def edit?
update?
end

def destroy?
@user.site_admin? || @user.admin?
return false unless @user.site_admin? || @user.admin?
return true unless @group_membership.user == @user && @group_membership.group.type == 'AdminGroup'

false
end
end

36 changes: 25 additions & 11 deletions app/policies/admin/user_policy.rb
Original file line number Diff line number Diff line change
@@ -1,48 +1,62 @@
# frozen_string_literal: true

class Admin::UserPolicy < ApplicationPolicy
attr_reader :user
def initialize(actor, user_to_authorize)
@actor = actor
@user_to_authorize = user_to_authorize
end

def initialize(user_logged_in, user_to_authorize)
@user = user_logged_in
def user
@actor
end

class Scope < Scope
def initialize(actor, scope)
@actor = actor
@scope = scope
end

def user
@actor
end

def resolve
if @user.site_admin?
if @actor.site_admin?
scope.all
else
scope.where(tenant_id: @user.tenant_id)
scope.where(tenant_id: @actor.tenant_id)
end
end
end

def index?
@user.site_admin? || @user.admin?
@actor.site_admin? || @actor.admin?
end

def show?
@user.site_admin? || @user.admin?
@actor.site_admin? || @actor.admin?
end

def create?
@user.site_admin? || @user.admin?
@actor.site_admin? || @actor.admin?
end

def new?
create?
end

def update?
@user.site_admin? || @user.admin?
@actor.site_admin? || @actor.admin?
end

def edit?
update?
end

def destroy?
@user.site_admin? || @user.admin?
end
return false unless @actor.site_admin? || @actor.admin?
return false if @user_to_authorize == @actor

true
end
end

0 comments on commit 8592c14

Please sign in to comment.