Merge pull request #2334 from alphagov/add-my-apps-page

Add a page listing the current user's applications

app/controllers/account/applications_controller.rb

@@ -0,0 +1,12 @@
+class Account::ApplicationsController < ApplicationController
+  layout "admin_layout"
+
+  before_action :authenticate_user!
+
+  def index
+    authorize :account_applications
+
+    @applications_with_signin = Doorkeeper::Application.can_signin(current_user)
+    @applications_without_signin = Doorkeeper::Application.not_retired.without_signin_permission_for(current_user)
+  end
+end

app/controllers/accounts_controller.rb

@@ -0,0 +1,13 @@
+class AccountsController < ApplicationController
+  before_action :authenticate_user!
+
+  def show
+    authorize :account_page
+
+    if policy(current_user).edit?
+      redirect_to edit_user_path(current_user)
+    else
+      redirect_to edit_email_or_password_user_path(current_user)
+    end
+  end
+end

app/helpers/application_helper.rb

@@ -15,15 +15,6 @@ def nav_link(text, link)
     end
   end
 
-  def user_link_target
-    # The page the current user's name in the header should link them to
-    if policy(current_user).edit?
-      edit_user_path(current_user)
-    else
-      edit_email_or_password_user_path(current_user)
-    end
-  end
-
   SENSITIVE_QUERY_PARAMETERS = %w[reset_password_token invitation_token].freeze
 
   def sensitive_query_parameters?

app/helpers/navigation_items_helper.rb

@@ -24,7 +24,7 @@ def navigation_items
       end
     end
 
-    items << { text: current_user.name, href: user_link_target }
+    items << { text: current_user.name, href: account_path }
     items << { text: "Sign out", href: destroy_user_session_path }
 
     items

app/models/doorkeeper/application.rb

@@ -7,17 +7,28 @@ class ::Doorkeeper::Application < ActiveRecord::Base
 
   default_scope { order("oauth_applications.name") }
   scope :support_push_updates, -> { where(supports_push_updates: true) }
+  scope :not_retired, -> { where(retired: false) }
   scope :can_signin,
         lambda { |user|
-          joins(supported_permissions: :user_application_permissions)
-            .where("user_application_permissions.user_id" => user.id)
-            .where("supported_permissions.name" => SupportedPermission::SIGNIN_NAME)
-            .where(retired: false)
+          with_signin_permission_for(user)
+            .not_retired
         }
   scope :with_signin_delegatable,
         lambda {
           joins(:supported_permissions)
-            .where(supported_permissions: { name: SupportedPermission::SIGNIN_NAME, delegatable: true })
+            .merge(SupportedPermission.signin)
+            .merge(SupportedPermission.delegatable)
+        }
+  scope :with_signin_permission_for,
+        lambda { |user|
+          joins(supported_permissions: :user_application_permissions)
+            .where(user_application_permissions: { user: })
+            .merge(SupportedPermission.signin)
+        }
+  scope :without_signin_permission_for,
+        lambda { |user|
+          excluded_app_ids = with_signin_permission_for(user).map(&:id)
+          where.not(id: excluded_app_ids)
         }
 
   after_create :create_signin_supported_permission
@@ -36,7 +47,7 @@ def supported_permission_strings(user = nil)
   end
 
   def signin_permission
-    supported_permissions.find_by(name: SupportedPermission::SIGNIN_NAME)
+    supported_permissions.signin.first
   end
 
   def sorted_supported_permissions_grantable_from_ui
@@ -78,7 +89,7 @@ def substituted_uri(uri)
   end
 
   def create_signin_supported_permission
-    supported_permissions.create!(name: SupportedPermission::SIGNIN_NAME, delegatable: true)
+    supported_permissions.delegatable.signin.create!
   end
 
   def create_user_update_supported_permission

app/models/supported_permission.rb

@@ -11,6 +11,7 @@ class SupportedPermission < ApplicationRecord
   scope :delegatable, -> { where(delegatable: true) }
   scope :grantable_from_ui, -> { where(grantable_from_ui: true) }
   scope :default, -> { where(default: true) }
+  scope :signin, -> { where(name: SIGNIN_NAME) }
 
   def signin?
     name.try(:downcase) == SIGNIN_NAME

app/policies/account_applications_policy.rb

@@ -0,0 +1,5 @@
+class AccountApplicationsPolicy < BasePolicy
+  def index?
+    current_user.govuk_admin?
+  end
+end

app/policies/account_page_policy.rb

@@ -0,0 +1,5 @@
+class AccountPagePolicy < BasePolicy
+  def show?
+    current_user.present?
+  end
+end

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

@@ -0,0 +1,53 @@
+<% content_for :title, "GOV.UK apps" %>
+
+<% content_for :breadcrumbs,
+   render("govuk_publishing_components/components/breadcrumbs", {
+     collapse_on_mobile: true,
+     breadcrumbs: [
+       {
+         title: "Dashboard",
+         url: root_path,
+       },
+       {
+         title: "GOV.UK apps",
+       }
+     ]
+   })
+%>
+
+<table class="govuk-table">
+  <caption class="govuk-table__caption govuk-table__caption--m">Apps you have access to</caption>
+  <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>
+    </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>
+      </tr>
+    <% end %>
+  </tbody>
+</table>
+
+<h2 class="govuk-heading-m" id="other-apps-table-heading">Apps you don't have access to</h2>
+
+<table class="govuk-table" aria-labelledby="other-apps-table-heading">
+  <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>
+    </tr>
+  </thead>
+  <tbody class="govuk-table__body">
+    <% @applications_without_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>
+      </tr>
+    <% end %>
+  </tbody>
+</table>

app/views/layouts/application.html.erb

@@ -31,7 +31,7 @@
   <% end %>
 
   <% content_for :navbar_right do %>
-    <%= link_to current_user.name, user_link_target %>
+    <%= link_to current_user.name, account_path %>
     &bull; <%= link_to 'Sign out', destroy_user_session_path %>
   <% end %>
 

config/routes.rb

@@ -49,6 +49,11 @@
   end
   resource :user, only: [:show]
 
+  resource :account, only: [:show]
+  namespace :account do
+    resources :applications, only: [:index]
+  end
+
   resources :batch_invitations, only: %i[new create show]
   resources :bulk_grant_permission_sets, only: %i[new create show]
   resources :organisations, only: %i[index edit update]

test/factories/users.rb

@@ -1,5 +1,5 @@
 FactoryBot.define do
-  factory :user do
+  factory :user, aliases: [:normal_user] do
     transient do
       with_permissions { {} }
       with_signin_permissions_for { [] }

test/integration/account_applications_test.rb

@@ -0,0 +1,63 @@
+require "test_helper"
+
+class AccountApplicationsTest < ActionDispatch::IntegrationTest
+  context "#index" do
+    setup do
+      @application = create(:application, name: "app-name", description: "app-description")
+      @retired_application = create(:application, retired: true, name: "retired-app-name")
+      @user = FactoryBot.create(:admin_user)
+    end
+
+    should "not be accessible to signed out users" do
+      visit account_applications_path
+
+      assert_current_url new_user_session_path
+    end
+
+    should "list the applications the user has access to" do
+      @user.grant_application_signin_permission(@application)
+
+      visit new_user_session_path
+      signin_with @user
+
+      visit account_applications_path
+
+      table = find("table caption[text()='Apps you have access to']").ancestor("table")
+      assert table.has_content?("app-name")
+      assert table.has_content?("app-description")
+    end
+
+    should "not list retired applications the user has access to" do
+      @user.grant_application_signin_permission(@retired_application)
+
+      visit new_user_session_path
+      signin_with @user
+
+      visit account_applications_path
+
+      assert_not page.has_content?("retired-app-name")
+    end
+
+    should "list the applications the user does not have access to" do
+      visit new_user_session_path
+      signin_with @user
+
+      visit account_applications_path
+
+      heading = find("h2", text: "Apps you don't have access to")
+      table = find("table[aria-labelledby='#{heading['id']}']")
+
+      assert table.has_content?("app-name")
+      assert table.has_content?("app-description")
+    end
+
+    should "not list retired applications the user does not have access to" do
+      visit new_user_session_path
+      signin_with @user
+
+      visit account_applications_path
+
+      assert_not page.has_content?("retired-app-name")
+    end
+  end
+end

test/integration/account_test.rb

@@ -0,0 +1,33 @@
+require "test_helper"
+
+class AccountTest < ActionDispatch::IntegrationTest
+  context "#show" do
+    should "not be accessible to signed out users" do
+      visit account_path
+
+      assert_current_url new_user_session_path
+    end
+
+    should "redirect to user's edit page for admin users" do
+      user = FactoryBot.create(:admin_user)
+
+      visit new_user_session_path
+      signin_with user
+
+      visit account_path
+
+      assert_current_url edit_user_path(user)
+    end
+
+    should "redirect to edit email or password page for normal users" do
+      user = FactoryBot.create(:user)
+
+      visit new_user_session_path
+      signin_with user
+
+      visit account_path
+
+      assert_current_url edit_email_or_password_user_path(user)
+    end
+  end
+end

test/models/doorkeeper_application_test.rb

@@ -150,5 +150,67 @@ class ::Doorkeeper::ApplicationTest < ActiveSupport::TestCase
 
       assert_empty Doorkeeper::Application.with_signin_delegatable
     end
+
+    context ".not_retired" do
+      setup do
+        @app = create(:application)
+      end
+
+      should "include apps that have not been retired" do
+        @app.update!(retired: false)
+        assert_equal [@app], Doorkeeper::Application.not_retired
+      end
+
+      should "exclude apps that have been retired" do
+        @app.update!(retired: true)
+        assert_equal [], Doorkeeper::Application.not_retired
+      end
+    end
+
+    context ".with_signin_permission_for" do
+      setup do
+        @user = create(:user)
+        @app = create(:application)
+      end
+
+      should "include applications the user has the signin permission for" do
+        @user.grant_application_signin_permission(@app)
+
+        assert_equal [@app], Doorkeeper::Application.with_signin_permission_for(@user)
+      end
+
+      should "exclude applications the user does not have the signin permission for" do
+        create(:supported_permission, application: @app, name: "not-signin")
+
+        @user.grant_application_permission(@app, %w[not-signin])
+
+        assert_equal [], Doorkeeper::Application.with_signin_permission_for(@user)
+      end
+    end
+
+    context ".without_signin_permission_for" do
+      setup do
+        @user = create(:user)
+        @app = create(:application)
+      end
+
+      should "exclude applications the user has the signin permission for" do
+        @user.grant_application_signin_permission(@app)
+
+        assert_equal [], Doorkeeper::Application.without_signin_permission_for(@user)
+      end
+
+      should "include applications the user does not have the signin permission for" do
+        create(:supported_permission, application: @app, name: "not-signin")
+
+        @user.grant_application_permission(@app, %w[not-signin])
+
+        assert_equal [@app], Doorkeeper::Application.without_signin_permission_for(@user)
+      end
+
+      should "include applications the user doesn't have any permissions for" do
+        assert_equal [@app], Doorkeeper::Application.without_signin_permission_for(@user)
+      end
+    end
   end
 end

test/models/supported_permission_test.rb

@@ -56,4 +56,11 @@ class SupportedPermissionTest < ActiveSupport::TestCase
     assert_not default_permissions.include? permission_three
     assert_not default_permissions.include? application_one.signin_permission
   end
+
+  test ".signin returns all signin permissions" do
+    app1 = create(:application, with_supported_permissions: %w[app1-permission])
+    app2 = create(:application, with_supported_permissions: %w[app2-permission])
+
+    assert_same_elements [app1.signin_permission, app2.signin_permission], SupportedPermission.signin
+  end
 end