Allow GOV.UK Admins to remove their access to apps

Users are required to confirm the removal of access to avoid the problem
that exists in the current `/users/<id>/edit` page where Publishing
Managers can accidentally remove their access to an application but then
have to open a support request to get that access back again. NOTE.  The
functionality in this PR is currently restricted to GOV.UK Admins but
will be made available to Publishing Managers soon.

I experimented with using the button component for the "Remove access"
button but couldn't work out how to pass the visually hidden span into
it.

app/controllers/account/signin_permissions_controller.rb

@@ -1,4 +1,6 @@
 class Account::SigninPermissionsController < ApplicationController
+  layout "admin_layout"
+
   before_action :authenticate_user!
 
   def create
@@ -8,4 +10,18 @@ def create
     current_user.grant_application_signin_permission(application)
     redirect_to account_applications_path
   end
+
+  def delete
+    authorize :account_applications, :remove_signin_permission?
+
+    @application = Doorkeeper::Application.find(params[:application_id])
+  end
+
+  def destroy
+    authorize :account_applications, :remove_signin_permission?
+
+    application = Doorkeeper::Application.find(params[:application_id])
+    current_user.remove_application_signin_permission(application)
+    redirect_to account_applications_path
+  end
 end

app/models/user.rb

@@ -202,6 +202,11 @@ def grant_application_permissions(application, supported_permission_names)
     end
   end
 
+  def remove_application_signin_permission(application)
+    signin_permission = application_permissions.where(supported_permission: application.signin_permission)
+    application_permissions.destroy(signin_permission)
+  end
+
   def grant_permission(supported_permission)
     application_permissions.where(supported_permission_id: supported_permission.id).first_or_create!
   end

app/policies/account_applications_policy.rb

@@ -4,4 +4,5 @@ def index?
   end
 
   alias_method :grant_signin_permission?, :index?
+  alias_method :remove_signin_permission?, :index?
 end

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

@@ -20,14 +20,22 @@
   <thead class="govuk-table__head">
     <tr class="govuk-table__row">
       <th scope="col" class="govuk-table__header govuk-!-width-one-quarter">Name</th>
-      <th scope="col" class="govuk-table__header govuk-!-width-three-quarters">Description</th>
+      <th scope="col" class="govuk-table__header govuk-!-width-two-quarters">Description</th>
+      <th scope="col" class="govuk-table__header govuk-!-width-one-quarter"></th>
     </tr>
   </thead>
   <tbody class="govuk-table__body">
     <% @applications_with_signin.each do |application| %>
       <tr class="govuk-table__row">
         <td class="govuk-table__cell"><%= application.name %></td>
         <td class="govuk-table__cell"><%= application.description %></td>
+        <td class="govuk-table__cell">
+          <%= link_to delete_account_application_signin_permission_path(application),
+                      class: "govuk-button govuk-!-margin-0",
+                      data: { module: "govuk-button" } do %>
+              Remove access<span class="govuk-visually-hidden"> to <%= application.name %></span>
+          <% end %>
+        </td>
       </tr>
     <% end %>
   </tbody>

app/views/account/signin_permissions/delete.html.erb

@@ -0,0 +1,32 @@
+<% content_for :title, "Remove access to #{@application.name}" %>
+
+<% content_for :breadcrumbs,
+   render("govuk_publishing_components/components/breadcrumbs", {
+     collapse_on_mobile: true,
+     breadcrumbs: [
+       {
+         title: "Dashboard",
+         url: root_path,
+       },
+       {
+         title: "GOV.UK apps",
+         url: account_applications_path,
+       },
+       {
+         title: "Remove access",
+       },
+     ]
+   })
+%>
+
+<p class="govuk-body">Are you sure you want to remove access to <%= @application.name %>?</p>
+
+<%= form_with url: account_application_signin_permission_path(@application), method: :delete do |form| %>
+  <div class="govuk-button-group">
+    <%= render "govuk_publishing_components/components/button", {
+      text: "Confirm",
+    } %>
+
+    <%= link_to "Cancel", account_applications_path, class: "govuk-link govuk-link--no-visited-state" %>
+  </div>
+<% end %>

config/routes.rb

@@ -52,7 +52,9 @@
   resource :account, only: [:show]
   namespace :account do
     resources :applications, only: [:index] do
-      resource :signin_permission, only: [:create]
+      resource :signin_permission, only: %i[create destroy] do
+        get :delete
+      end
     end
   end
 

test/controllers/account/signin_permissions_controller_test.rb

@@ -8,4 +8,20 @@ class Account::SigninPermissionsControllerTest < ActionController::TestCase
 
     assert_redirected_to "/users/sign_in"
   end
+
+  should "prevent unauthenticated users from accessing delete" do
+    application = create(:application, name: "app-name", description: "app-description")
+
+    get :delete, params: { application_id: application.id }
+
+    assert_redirected_to "/users/sign_in"
+  end
+
+  should "prevent unauthenticated users from accessing destroy" do
+    application = create(:application, name: "app-name", description: "app-description")
+
+    delete :destroy, params: { application_id: application.id }
+
+    assert_redirected_to "/users/sign_in"
+  end
 end

test/integration/account_applications_test.rb

@@ -79,4 +79,26 @@ class AccountApplicationsTest < ActionDispatch::IntegrationTest
       assert table.has_content?("app-name")
     end
   end
+
+  context "removing access to apps" do
+    setup do
+      application = create(:application, name: "app-name", description: "app-description")
+      @user = FactoryBot.create(:admin_user)
+      @user.grant_application_signin_permission(application)
+    end
+
+    should "allow admins to remove their access to apps" do
+      visit new_user_session_path
+      signin_with @user
+
+      visit account_applications_path
+
+      click_on "Remove access to app-name"
+      click_on "Confirm"
+
+      heading = find("h2", text: "Apps you don't have access to")
+      table = find("table[aria-labelledby='#{heading['id']}']")
+      assert table.has_content?("app-name")
+    end
+  end
 end

test/models/user_test.rb

@@ -510,6 +510,58 @@ def setup
     assert user.has_access_to?(app)
   end
 
+  context "#remove_application_signin_permission" do
+    setup do
+      @app = create(:application)
+      @user = create(:user)
+    end
+
+    context "when the user has the signin permission for the app" do
+      setup do
+        @user.grant_application_signin_permission(@app)
+      end
+
+      should "remove signin permission" do
+        assert @user.has_access_to?(@app)
+
+        @user.remove_application_signin_permission(@app)
+        assert_not @user.has_access_to?(@app)
+      end
+
+      should "be idempotent" do
+        @user.remove_application_signin_permission(@app)
+        @user.remove_application_signin_permission(@app)
+      end
+    end
+
+    context "when the user does not have the signin permission for the app" do
+      setup do
+        assert_not @user.has_access_to?(@app)
+      end
+
+      should "be a noop" do
+        @user.remove_application_signin_permission(@app)
+
+        assert_not @user.has_access_to?(@app)
+      end
+    end
+
+    context "when the app does not have the signin permission" do
+      setup do
+        @app.supported_permissions.delete_all
+
+        assert_nil @app.signin_permission
+        assert_not @user.has_access_to?(@app)
+      end
+
+      should "be a noop if the app doesn't have the signin permission" do
+        @user.remove_application_signin_permission(@app)
+
+        assert_not @user.has_access_to?(@app)
+      end
+    end
+  end
+
   test "can grant permissions to users and return the created permission" do
     app = create(:application, name: "my_app", with_supported_permissions: ["Create publications", "Delete publications"])
     user = create(:user)

test/policies/account_applications_policy_test.rb

@@ -55,4 +55,30 @@ class AccountApplicationsPolicyTest < ActiveSupport::TestCase
       end
     end
   end
+
+  context "accessing remove_signin_permission?" do
+    %i[superadmin admin].each do |user_role|
+      context "for #{user_role} users" do
+        setup do
+          @current_user = FactoryBot.build(:"#{user_role}_user")
+        end
+
+        should "be permitted" do
+          assert permit?(@current_user, nil, :remove_signin_permission)
+        end
+      end
+    end
+
+    %i[super_organisation_admin organisation_admin normal].each do |user_role|
+      context "for #{user_role} users" do
+        setup do
+          @current_user = FactoryBot.build(:"#{user_role}_user")
+        end
+
+        should "be forbidden" do
+          assert forbid?(@current_user, nil, :remove_signin_permission)
+        end
+      end
+    end
+  end
 end