Use project member abilities more extensively

app/controllers/groups/group_members_controller.rb

@@ -3,8 +3,7 @@ class Groups::GroupMembersController < Groups::ApplicationController
 
   # Authorize
   before_action :authorize_read_group!
-  before_action :authorize_admin_group!, except: [:index, :leave]
-  before_action :authorize_admin_group_member!, only: [:create, :resend_invite]
+  before_action :authorize_admin_group_member!, except: [:index, :leave]
 
   def index
     @project = @group.projects.find(params[:project_id]) if params[:project_id]
@@ -17,7 +16,8 @@ class Groups::GroupMembersController < Groups::ApplicationController
     end
 
     @members = @members.order('access_level DESC').page(params[:page]).per(50)
-    @group_member = GroupMember.new
+
+    @group_member = @group.group_members.new
   end
 
   def create
@@ -27,25 +27,24 @@ class Groups::GroupMembersController < Groups::ApplicationController
   end
 
   def update
-    @member = @group.group_members.find(params[:id])
+    @group_member = @group.group_members.find(params[:id])
 
-    return render_403 unless can?(current_user, :update_group_member, @member)
+    return render_403 unless can?(current_user, :update_group_member, @group_member)
 
-    @member.update_attributes(member_params)
+    @group_member.update_attributes(member_params)
   end
 
   def destroy
     @group_member = @group.group_members.find(params[:id])
 
-    if can?(current_user, :destroy_group_member, @group_member)  # May fail if last owner.
+    return render_403 unless can?(current_user, :destroy_group_member, @group_member)
+
     @group_member.destroy
+
     respond_to do |format|
       format.html { redirect_to group_group_members_path(@group), notice: 'User was successfully removed from group.' }
       format.js { render nothing: true }
     end
-    else
-      return render_403
-    end
   end
 
   def resend_invite
@@ -63,10 +62,11 @@ class Groups::GroupMembersController < Groups::ApplicationController
   end
 
   def leave
-    @group_member = @group.group_members.where(user_id: current_user.id).first
+    @group_member = @group.group_members.find_by(user_id: current_user)
 
     if can?(current_user, :destroy_group_member, @group_member)
       @group_member.destroy
+
       redirect_to(dashboard_groups_path, notice: "You left #{group.name} group.")
     else
       if @group.last_owner?(current_user)

app/controllers/projects/project_members_controller.rb

 class Projects::ProjectMembersController < Projects::ApplicationController
   # Authorize
-  before_action :authorize_admin_project!, except: :leave
+  before_action :authorize_admin_project_member!, except: :leave
 
   def index
     @project_members = @project.project_members
@@ -29,10 +29,6 @@ class Projects::ProjectMembersController < Projects::ApplicationController
     @project_member = @project.project_members.new
   end
 
-  def new
-    @project_member = @project.project_members.new
-  end
-
   def create
     @project.team.add_users(params[:user_ids].split(','), params[:access_level], current_user)
 
@@ -41,11 +37,17 @@ class Projects::ProjectMembersController < Projects::ApplicationController
 
   def update
     @project_member = @project.project_members.find(params[:id])
+
+    return render_403 unless can?(current_user, :update_project_member, @project_member)
+
     @project_member.update_attributes(member_params)
   end
 
   def destroy
     @project_member = @project.project_members.find(params[:id])
+
+    return render_403 unless can?(current_user, :destroy_project_member, @project_member)
+
     @project_member.destroy
 
     respond_to do |format|
@@ -71,17 +73,23 @@ class Projects::ProjectMembersController < Projects::ApplicationController
   end
 
   def leave
-    if @project.namespace == current_user.namespace
-      message = 'You can not leave your own project. Transfer or delete the project.'
-      return redirect_back_or_default(default: { action: 'index' }, options: { alert: message })
-    end
+    @project_member = @project.project_members.find_by(user_id: current_user)
 
-    @project.project_members.find_by(user_id: current_user).destroy
+    if can?(current_user, :destroy_project_member, @project_member)
+      @project_member.destroy
 
       respond_to do |format|
-      format.html { redirect_to dashboard_projects_path }
+        format.html { redirect_to dashboard_projects_path, notice: "You left the project." }
         format.js { render nothing: true }
       end
+    else
+      if current_user == @project.owner
+        message = 'You can not leave your own project. Transfer or delete the project.'
+        redirect_back_or_default(default: { action: 'index' }, options: { alert: message })
+      else
+        render_403
+      end
+    end
   end
 
   def apply_import

app/views/groups/group_members/_group_member.html.haml

 - user = member.user
 - return unless user || member.invite?
-- show_roles = true if show_roles.nil?
 
 %li{class: "#{dom_class(member)} js-toggle-container", id: dom_id(member)}
   %span{class: ("list-item-name" if show_controls)}
@@ -25,11 +24,11 @@
           = link_to member.created_by.name, user_path(member.created_by)
         = time_ago_with_tooltip(member.created_at)
 
-      - if show_controls && can?(current_user, :admin_group_member, member)
+      - if show_controls && can?(current_user, :admin_group_member, @group)
         = link_to resend_invite_group_group_member_path(@group, member), method: :post, class: "btn-xs btn", title: 'Resend invite' do
           Resend invite
 
-  - if show_roles
+  - if should_user_see_group_roles?(current_user, @group)
     %span.pull-right
       %strong= member.human_access
       - if show_controls
@@ -37,6 +36,7 @@
           = button_tag class: "btn-xs btn js-toggle-button",
                        title: 'Edit access level', type: 'button' do
             %i.fa.fa-pencil-square-o
+
         - if can?(current_user, :destroy_group_member, member)
            
           - if current_user == user

app/views/groups/group_members/index.html.haml

 - page_title "Members"
 - header_title group_title(@group, "Members", group_group_members_path(@group))
-- show_roles = should_user_see_group_roles?(current_user, @group)
-
-- if show_roles
+- if should_user_see_group_roles?(current_user, @group)
   %p.light
     Members of group have access to all group projects.
     Read more about permissions
@@ -32,7 +30,7 @@
       (#{@members.total_count})
   %ul.well-list
     - @members.each do |member|
-      = render 'groups/group_members/group_member', member: member, show_roles: show_roles, show_controls: true
+      = render 'groups/group_members/group_member', member: member, show_controls: true
 
 = paginate @members, theme: 'gitlab'
 

app/views/projects/project_members/_project_member.html.haml

@@ -24,18 +24,19 @@
           = link_to member.created_by.name, user_path(member.created_by)
         = time_ago_with_tooltip(member.created_at)
 
-      - if current_user_can_admin_project
+      - if can?(current_user, :admin_project_member, @project)
         = link_to resend_invite_namespace_project_project_member_path(@project.namespace, @project, member), method: :post, class: "btn-xs btn", title: 'Resend invite' do
           Resend invite
 
-  - if current_user_can_admin_project
-    - unless @project.personal? && user == current_user
+  - if can?(current_user, :admin_project_member, @project)
     .pull-right
       %strong= member.human_access
+      - if can?(current_user, :update_project_member, member)
         = button_tag class: "btn-xs btn js-toggle-button",
                      title: 'Edit access level', type: 'button' do
           %i.fa.fa-pencil-square-o
 
+      - if can?(current_user, :destroy_project_member, member)
          
         - if current_user == user
           = link_to leave_namespace_project_project_members_path(@project.namespace, @project), data: { confirm: leave_project_message(@project) }, method: :delete, class: "btn-xs btn btn-remove", title: 'Leave project' do

app/views/projects/project_members/_team.html.haml

-- can_admin_project = can?(current_user, :admin_project, @project)
-
 .panel.panel-default.prepend-top-20
   .panel-heading
     %strong #{@project.name}
@@ -8,4 +6,4 @@
       (#{members.count})
   %ul.well-list
     - members.each do |project_member|
-      = render 'project_member', member: project_member, current_user_can_admin_project: can_admin_project
+      = render 'project_member', member: project_member

app/views/projects/project_members/update.js.haml

-- can_admin_project = can?(current_user, :admin_project, @project)
 :plain
-  $("##{dom_id(@project_member)}").replaceWith('#{escape_javascript(render("project_member", member: @project_member, current_user_can_admin_project: can_admin_project))}');
+  $("##{dom_id(@project_member)}").replaceWith('#{escape_javascript(render("project_member", member: @project_member))}');