Update permission editing on users edit page

app/controllers/account/permissions_controller.rb

@@ -5,21 +5,21 @@ class Account::PermissionsController < ApplicationController
   before_action :set_application
 
   def show
-    authorize [:account, @application], :view_permissions?
+    authorize [:account, UserApplicationPermission]
 
     @permissions = @application
       .sorted_supported_permissions_grantable_from_ui
       .sort_by { |permission| current_user.has_permission?(permission) ? 0 : 1 }
   end
 
   def edit
-    authorize [:account, @application], :edit_permissions?
+    authorize [:account, current_user.signin_permission_for(@application)]
 
     @permissions = @application.sorted_supported_permissions_grantable_from_ui(include_signin: false)
   end
 
   def update
-    authorize [:account, @application], :edit_permissions?
+    authorize [:account, current_user.signin_permission_for(@application)], :edit?
 
     permission_ids_for_other_applications = current_user.supported_permissions.excluding_application(@application).pluck(:id)
     user_update_params = { supported_permission_ids: permission_ids_for_other_applications + params[:application][:supported_permission_ids] }

app/controllers/account/signin_permissions_controller.rb

@@ -4,7 +4,7 @@ class Account::SigninPermissionsController < ApplicationController
   before_action :authenticate_user!
 
   def create
-    authorize [:account, Doorkeeper::Application], :grant_signin_permission?
+    authorize SigninPermission
 
     params = { supported_permission_ids: current_user.supported_permissions.map(&:id) + [application.signin_permission.id] }
     UserUpdate.new(current_user, params, current_user, user_ip_address).call
@@ -13,11 +13,11 @@ def create
   end
 
   def delete
-    authorize [:account, application], :remove_signin_permission?
+    authorize [:account, SigninPermission.new(current_user.signin_permission_for(application))]
   end
 
   def destroy
-    authorize [:account, application], :remove_signin_permission?
+    authorize [:account, SigninPermission.new(current_user.signin_permission_for(application))], :delete?
 
     params = { supported_permission_ids: current_user.supported_permissions.map(&:id) - [application.signin_permission.id] }
     UserUpdate.new(current_user, params, current_user, user_ip_address).call

app/controllers/users/applications_controller.rb

@@ -0,0 +1,22 @@
+class Users::ApplicationsController < ApplicationController
+  layout "admin_layout"
+
+  before_action :authenticate_user!
+  before_action :load_user
+  before_action :authorize_user
+
+  def index
+    @applications_with_signin = Doorkeeper::Application.not_api_only.can_signin(@user)
+    @applications_without_signin = Doorkeeper::Application.not_api_only.without_signin_permission_for(@user)
+  end
+
+  private
+
+  def load_user
+    @user = User.find(params[:user_id])
+  end
+
+  def authorize_user
+    authorize @user
+  end
+end

app/controllers/users/permissions_controller.rb

@@ -0,0 +1,42 @@
+class Users::PermissionsController < ApplicationController
+  layout "admin_layout"
+
+  before_action :authenticate_user!
+  before_action :load_user
+  before_action :set_application
+
+  def show
+    @permissions = @application
+      .sorted_supported_permissions_grantable_from_ui
+      .sort_by { |permission| @user.has_permission?(permission) ? 0 : 1 }
+
+    authorize @user.signin_permission_for(@application)
+  end
+
+  def edit
+    @permissions = @application.sorted_supported_permissions_grantable_from_ui(include_signin: false)
+
+    authorize @user.signin_permission_for(@application)
+  end
+
+  def update
+    authorize @user.signin_permission_for(@application)
+
+    permission_ids_for_other_applications = @user.supported_permissions.excluding_application(@application).pluck(:id)
+    user_update_params = { supported_permission_ids: permission_ids_for_other_applications + params[:application][:supported_permission_ids] }
+    UserUpdate.new(@user, user_update_params, current_user, user_ip_address).call
+
+    flash[:application_id] = @application.id
+    redirect_to user_applications_path(@user)
+  end
+
+  private
+
+  def load_user
+    @user = User.find(params[:user_id])
+  end
+
+  def set_application
+    @application = Doorkeeper::Application.not_api_only.find(params[:application_id])
+  end
+end

app/controllers/users/signin_permissions_controller.rb

@@ -0,0 +1,39 @@
+class Users::SigninPermissionsController < ApplicationController
+  layout "admin_layout"
+
+  before_action :authenticate_user!
+  before_action :load_user
+  before_action :set_application
+
+  def create
+    authorize SigninPermission
+
+    params = { supported_permission_ids: @user.supported_permissions.map(&:id) + [@application.signin_permission.id] }
+    UserUpdate.new(@user, params, current_user, user_ip_address).call
+
+    redirect_to user_applications_path(@user)
+  end
+
+  def delete
+    authorize @user.signin_permission_for(@application)
+  end
+
+  def destroy
+    authorize @user.signin_permission_for(@application)
+
+    params = { supported_permission_ids: @user.supported_permissions.map(&:id) - [@application.signin_permission.id] }
+    UserUpdate.new(@user, params, current_user, user_ip_address).call
+
+    redirect_to user_applications_path(@user)
+  end
+
+private
+
+  def load_user
+    @user = User.find(params[:user_id])
+  end
+
+  def set_application
+    @application ||= Doorkeeper::Application.find(params[:application_id])
+  end
+end

app/helpers/account_applications_helper.rb

@@ -1,12 +1,16 @@
 module AccountApplicationsHelper
-  def message_for_success(application_id)
+  def message_for_success(application_id, user: current_user)
     application = Doorkeeper::Application.find_by(id: application_id)
     return nil unless application
 
-    additional_permissions = current_user.permissions_for(application).reject { |permission| permission == SupportedPermission::SIGNIN_NAME }
+    additional_permissions = user.permissions_for(application).reject { |permission| permission == SupportedPermission::SIGNIN_NAME }
 
     if additional_permissions.any?
-      paragraph = tag.p("You now have the following permissions for #{application.name}:", class: "govuk-body")
+      if user == current_user
+        paragraph = tag.p("You now have the following permissions for #{application.name}:", class: "govuk-body")
+      else
+        paragraph = tag.p("#{user.name} now has the following permissions for #{application.name}:", class: "govuk-body")
+      end
       list = tag.ul(class: "govuk-list govuk-list--bullet")
       additional_permissions.map { |permission| list << tag.li(permission) }
     else

app/models/signin_permission.rb

@@ -0,0 +1,7 @@
+class SigninPermission
+  attr_reader :user_application_permission
+
+  def initialize(user_application_permission)
+    @user_application_permission = user_application_permission
+  end
+end

app/models/user.rb

@@ -173,10 +173,14 @@ def permission_ids_for(application)
     application_permissions.select { |ap| ap.application_id == application.id }.map(&:supported_permission_id)
   end
 
-  def has_access_to?(application)
+  def signin_permission_for(application)
     application_permissions.detect { |permission| permission.supported_permission_id == application.signin_permission.id }
   end
 
+  def has_access_to?(application)
+    signin_permission_for(application).present?
+  end
+
   def has_permission?(supported_permission)
     if persisted?
       supported_permissions.exists?(supported_permission.id)

app/policies/account/application_policy.rb

@@ -4,18 +4,4 @@ def index?
   end
 
   alias_method :show?, :index?
-  alias_method :view_permissions?, :index?
-
-  def grant_signin_permission?
-    current_user.govuk_admin?
-  end
-
-  def remove_signin_permission?
-    current_user.has_access_to?(record) &&
-      (
-        current_user.govuk_admin? ||
-        current_user.publishing_manager? && record.signin_permission.delegatable?
-      )
-  end
-  alias_method :edit_permissions?, :remove_signin_permission?
 end

app/policies/account/user_application_permission_policy.rb

@@ -0,0 +1,16 @@
+class Account::UserApplicationPermissionPolicy < BasePolicy
+  def show?
+    Pundit.policy(current_user, user).edit?
+  end
+
+  def edit?
+    return false unless show?
+    return true if current_user.govuk_admin?
+
+    current_user.has_access_to?(application) && application.signin_permission.delegatable?
+  end
+
+  private
+
+  delegate :user, :application, to: :record, allow_nil: true
+end

app/policies/signin_permission_policy.rb

@@ -0,0 +1,13 @@
+class SigninPermissionPolicy < BasePolicy
+  def create?
+    current_user.govuk_admin?
+  end
+
+  def delete?
+    Pundit.policy(current_user, [:account, user_application_permission]).edit?
+  end
+
+  private
+
+  delegate :user_application_permission, to: :record
+end

app/policies/user_application_permission_policy.rb

@@ -0,0 +1,52 @@
+class UserApplicationPermissionPolicy < BasePolicy
+  def index?
+    Pundit.policy(current_user, user).edit?
+  end
+
+  def remove_signin_permission?
+    user.has_access_to?(application) &&
+      (
+        current_user.govuk_admin? ||
+        current_user.publishing_manager? && application.signin_permission.delegatable?
+      )
+  end
+
+  def edit_permissions?
+    remove_signin_permission?
+  end
+
+  def view_permissions?
+    Pundit.policy(current_user, user).edit? &&
+      user.has_access_to?(application)
+  end
+
+  def delete?
+    edit_permissions?
+  end
+
+  def destroy?
+    delete?
+  end
+
+  def show?
+    view_permissions?
+  end
+
+  def edit?
+    edit_permissions?
+  end
+
+  def update?
+    edit_permissions?
+  end
+
+  private
+
+  def user
+    record.user
+  end
+
+  def application
+    record.application
+  end
+end

app/views/account/applications/index.html.erb

@@ -38,20 +38,20 @@
         <td class="govuk-table__cell"><%= application.name %></td>
         <td class="govuk-table__cell"><%= application.description %></td>
         <td class="govuk-table__cell govuk-!-text-align-right">
-          <% if policy([:account, application]).edit_permissions? %>
+          <% if policy([:account, current_user.signin_permission_for(application)]).edit? %>
             <% unless application.sorted_supported_permissions_grantable_from_ui(include_signin: false).empty? %>
               <%= link_to edit_account_application_permissions_path(application), class: "govuk-link" do %>
                 Update permissions<span class="govuk-visually-hidden"> for <%= application.name %></span>
               <% end %>
             <% end %>
-          <% elsif policy([:account, application]).view_permissions? %>
+          <% elsif policy([:account, current_user.signin_permission_for(application)]).show? %>
             <%= link_to account_application_permissions_path(application), class: "govuk-link" do %>
                 View permissions<span class="govuk-visually-hidden"> for <%= application.name %></span>
             <% end %>
           <% end %>
         </td>
         <td class="govuk-table__cell govuk-!-text-align-right">
-          <% if policy([:account, application]).remove_signin_permission? %>
+          <% if policy(SigninPermission.new(current_user.signin_permission_for(application))).delete? %>
             <%= link_to delete_account_application_signin_permission_path(application),
                         class: "govuk-button govuk-button--warning govuk-!-margin-0",
                         data: { module: "govuk-button" } do %>
@@ -80,7 +80,7 @@
         <td class="govuk-table__cell"><%= application.name %></td>
         <td class="govuk-table__cell"><%= application.description %></td>
         <td class="govuk-table__cell govuk-!-text-align-right">
-          <% if policy([:account, Doorkeeper::Application]).grant_signin_permission? %>
+          <% if policy(SigninPermission).create? %>
             <%= button_to account_application_signin_permission_path(application),
                           class: "govuk-button govuk-!-margin-0",
                           data: { module: "govuk-button" } do %>

app/views/users/_form_fields.html.erb

@@ -78,10 +78,12 @@
   </dl>
 <% end %>
 
-<h2 class="add-vertical-margins"> <%= "Editable " if (current_user.publishing_manager? ) %>Permissions</h2>
-<%= render partial: "shared/user_permissions", locals: { user_object: f.object }%>
-
-<% if current_user.publishing_manager? %>
-    <h2 class="add-vertical-margins">All Permissions for this user</h2>
-    <%= render partial: "app_permissions", locals: { user_object: f.object }%>
+<% if policy(User).assign_organisations? %>
+  <p class="form-group">
+    <%= f.label :organisation_id, "Organisation" %><br />
+    <%= f.select :organisation_id, organisation_options(f), organisation_select_options, { class: "chosen-select form-control", 'data-module' => 'chosen' } %>
+  </p>
 <% end %>
+
+<h2 class="add-vertical-margins">Permissions</h2>
+<%= link_to "Manage permissions", user_applications_path(f.object) %>