From 6ac5292de3446c713386479f91a8e5bad4b8c5dd Mon Sep 17 00:00:00 2001 From: Ynda Jas Date: Thu, 27 Jun 2024 17:29:15 +0100 Subject: [PATCH] WIP This contains changes from https://github.com/alphagov/signon/pull/2988 which should go away once that's merged --- app/helpers/application_table_helper.rb | 5 ++++- app/helpers/components/table_helper.rb | 1 + app/models/user_application_permission.rb | 1 + app/policies/account/application_policy.rb | 1 + app/policies/user_application_permission_policy.rb | 8 +++++--- app/views/account/applications/index.html.erb | 8 ++++---- app/views/api_users/applications/index.html.erb | 4 ++-- app/views/components/_table.html.erb | 1 + app/views/users/applications/index.html.erb | 10 +++++----- 9 files changed, 24 insertions(+), 15 deletions(-) diff --git a/app/helpers/application_table_helper.rb b/app/helpers/application_table_helper.rb index 4bc7741726..6dc90a994e 100644 --- a/app/helpers/application_table_helper.rb +++ b/app/helpers/application_table_helper.rb @@ -49,7 +49,10 @@ def account_applications_permissions_link(application) def users_applications_permissions_link(application, user) if policy(UserApplicationPermission.for(user, application)).edit? - update_permissions_link(application, user) + safe_join([ + view_permissions_link(application, user), + update_permissions_link(application, user), + ]) else view_permissions_link(application, user) end diff --git a/app/helpers/components/table_helper.rb b/app/helpers/components/table_helper.rb index 9826014200..fe47237ec7 100644 --- a/app/helpers/components/table_helper.rb +++ b/app/helpers/components/table_helper.rb @@ -50,6 +50,7 @@ def row def header(str, opt = {}) classes = %w[govuk-table__header] + classes << opt[:classes] if opt[:classes] classes << "govuk-table__header--#{opt[:format]}" if opt[:format] classes << "govuk-table__header--active" if opt[:sort_direction] link_classes = %w[app-table__sort-link] diff --git a/app/models/user_application_permission.rb b/app/models/user_application_permission.rb index 6f455e21de..32bdcb02f4 100644 --- a/app/models/user_application_permission.rb +++ b/app/models/user_application_permission.rb @@ -8,6 +8,7 @@ class UserApplicationPermission < ApplicationRecord before_validation :assign_application_id + # can we expand this interface to have an optional permission? def self.for(user, application) new(user:, application:) end diff --git a/app/policies/account/application_policy.rb b/app/policies/account/application_policy.rb index cd39c6d597..128a1028d6 100644 --- a/app/policies/account/application_policy.rb +++ b/app/policies/account/application_policy.rb @@ -8,6 +8,7 @@ def index? def grant_signin_permission? current_user.govuk_admin? + # should this not have `|| current_user.publishing_manager? && record.signin_permission.delegatable?`? end def remove_signin_permission? diff --git a/app/policies/user_application_permission_policy.rb b/app/policies/user_application_permission_policy.rb index 3dc137fcc7..739e232bba 100644 --- a/app/policies/user_application_permission_policy.rb +++ b/app/policies/user_application_permission_policy.rb @@ -1,15 +1,17 @@ class UserApplicationPermissionPolicy < BasePolicy def create? return false unless Pundit.policy(current_user, user).edit? - return true if current_user.govuk_admin? + return false unless current_user.publishing_manager? && current_user.has_access_to?(application) + + permission = supported_permission || application.signin_permission - current_user.publishing_manager? && current_user.has_access_to?(application) && application.signin_permission.delegatable? + current_user.has_permission?(permission) && permission.delegatable? end alias_method :destroy?, :create? alias_method :delete?, :create? alias_method :update?, :create? alias_method :edit?, :create? - delegate :user, :application, to: :record + delegate :user, :application, :supported_permission, to: :record end diff --git a/app/views/account/applications/index.html.erb b/app/views/account/applications/index.html.erb index 75f90dd17d..729f6d9030 100644 --- a/app/views/account/applications/index.html.erb +++ b/app/views/account/applications/index.html.erb @@ -28,8 +28,8 @@ <%= render "components/table", { caption: "Apps you have access to", head: [ - { text: "Name" }, - { text: "Description" }, + { text: "Name", classes: "govuk-!-width-one-quarter" }, + { text: "Description", classes: "govuk-!-width-one-third" }, { text: content_tag(:span, "Actions", class: "govuk-visually-hidden"), visually_hidden: true }, ], rows: @applications_with_signin.map do |application| @@ -50,8 +50,8 @@ <%= render "components/table", { caption: "Apps you don't have access to", head: [ - { text: "Name" }, - { text: "Description" }, + { text: "Name", classes: "govuk-!-width-one-quarter" }, + { text: "Description", classes: "govuk-!-width-one-third" }, { text: content_tag(:span, "Actions", class: "govuk-visually-hidden"), visually_hidden: true } ], rows: @applications_without_signin.map do |application| diff --git a/app/views/api_users/applications/index.html.erb b/app/views/api_users/applications/index.html.erb index 2294038452..44d36ae89d 100644 --- a/app/views/api_users/applications/index.html.erb +++ b/app/views/api_users/applications/index.html.erb @@ -33,8 +33,8 @@ <%= render "components/table", { caption: "Apps #{@api_user.name} has access to", head: [ - { text: "Name" }, - { text: "Description" }, + { text: "Name", classes: "govuk-!-width-one-quarter" }, + { text: "Description", classes: "govuk-!-width-one-third" }, { text: content_tag(:span, "Actions", class: "govuk-visually-hidden"), visually_hidden: true }, ], rows: @applications.map do |application| diff --git a/app/views/components/_table.html.erb b/app/views/components/_table.html.erb index 712a62081b..b95a85abcd 100644 --- a/app/views/components/_table.html.erb +++ b/app/views/components/_table.html.erb @@ -27,6 +27,7 @@ <%= t.head do %> <% head.each_with_index do |item, cellindex| %> <%= t.header item[:text], { + classes: item[:classes], format: item[:format], href: item[:href], data_attributes: item[:data_attributes], diff --git a/app/views/users/applications/index.html.erb b/app/views/users/applications/index.html.erb index d05eeea50a..c4c03f1fa6 100644 --- a/app/views/users/applications/index.html.erb +++ b/app/views/users/applications/index.html.erb @@ -33,8 +33,8 @@ <%= render "components/table", { caption: "Apps #{@user.name} has access to", head: [ - { text: "Name" }, - { text: "Description" }, + { text: "Name", classes: "govuk-!-width-one-quarter" }, + { text: "Description", classes: "govuk-!-width-one-third" }, { text: content_tag(:span, "Actions", class: "govuk-visually-hidden"), visually_hidden: true }, ], rows: @applications_with_signin.map do |application| @@ -54,9 +54,9 @@ <%= render "components/table", { caption: "Apps #{@user.name} does not have access to", head: [ - { text: "Name" }, - { text: "Description" }, - { text: content_tag(:span, "Actions", class: "govuk-visually-hidden"), visually_hidden: true } + { text: "Name", classes: "govuk-!-width-one-quarter" }, + { text: "Description", classes: "govuk-!-width-one-third" }, + { text: content_tag(:span, "Actions", class: "govuk-visually-hidden"), visually_hidden: true }, ], rows: @applications_without_signin.map do |application| [