Skip to content

Commit

Permalink
Allowt GOV.UK Admins to view their app permissions
Browse files Browse the repository at this point in the history
This adds the `/account/applications/<id>/permissions` page which lists
all app permissions and whether the current user has that permission or
not.

This new page is currently only accessible to GOV.UK Admins that have
the signin permission to the app.

The list of permissions has "signin" first, followed by:

- other permissions the user has been granted; ordered alphabetically
- other permissions the user has not been granted; ordered alphabetically
  • Loading branch information
chrisroos committed Sep 26, 2023
1 parent ec76d40 commit 99a3927
Show file tree
Hide file tree
Showing 10 changed files with 202 additions and 0 deletions.
15 changes: 15 additions & 0 deletions app/controllers/account/permissions_controller.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
class Account::PermissionsController < ApplicationController
include PermissionsHelper

layout "admin_layout"

before_action :authenticate_user!

def index
@application = Doorkeeper::Application.not_retired.find(params[:application_id])

authorize @current_user, :view_permissions?

@permissions = permissions_for(@application).sort_by { |permission| current_user.has_permission?(permission) ? 0 : 1 }
end
end
4 changes: 4 additions & 0 deletions app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,10 @@ def has_access_to?(application)
application_permissions.detect { |permission| permission.supported_permission_id == application.signin_permission.id }
end

def has_permission?(supported_permission)
supported_permissions.exists?(supported_permission.id)
end

def permissions_synced!(application)
application_permissions.where(application_id: application.id).update_all(last_synced_at: Time.zone.now)
end
Expand Down
1 change: 1 addition & 0 deletions app/policies/user_policy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ def grant_signin_permission?
current_user.govuk_admin?
end
alias_method :remove_signin_permission?, :grant_signin_permission?
alias_method :view_permissions?, :grant_signin_permission?

def edit_email_or_password?
allow_self_only
Expand Down
6 changes: 6 additions & 0 deletions app/views/account/applications/index.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
<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-one-third">Description</th>
<th scope="col" class="govuk-table__header"><span class="govuk-visually-hidden">Permissions</span></th>
<th scope="col" class="govuk-table__header"><span class="govuk-visually-hidden">Remove access</span></th>
</tr>
</thead>
Expand All @@ -29,6 +30,11 @@
<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 govuk-!-text-align-right">
<%= link_to account_application_permissions_path(application) do %>
View permissions<span class="govuk-visually-hidden"> for <%= application.name %></span>
<% end %>
</td>
<td class="govuk-table__cell govuk-!-text-align-right">
<%= link_to delete_account_application_signin_permission_path(application),
class: "govuk-button govuk-button--warning govuk-!-margin-0",
Expand Down
47 changes: 47 additions & 0 deletions app/views/account/permissions/index.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
<% content_for :title, "My permissions for #{@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: "My permissions for #{@application.name}",
},
]
})
%>

<table class="govuk-table">
<thead class="govuk-table__head">
<tr class="govuk-table__row">
<th scope="col" class="govuk-table__header">Name</th>
<th scope="col" class="govuk-table__header">Has this permission?</th>
</tr>
</thead>
<tbody class="govuk-table__body">
<% @permissions.each do |permission| %>
<tr class="govuk-table__row">
<td class="govuk-table__cell"><%= permission.name %></td>
<td class="govuk-table__cell">
<% if current_user.has_permission?(permission) %>
<strong class="govuk-tag govuk-tag--green">
Yes
</strong>
<% else %>
<strong class="govuk-tag govuk-tag--grey">
No
</strong>
<% end %>
</td>
</tr>
<% end %>
</tbody>
</table>
1 change: 1 addition & 0 deletions config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@
resource :account, only: [:show]
namespace :account do
resources :applications, only: %i[show index] do
resources :permissions, only: [:index]
resource :signin_permission, only: %i[create destroy] do
get :delete
end
Expand Down
51 changes: 51 additions & 0 deletions test/controllers/account/permissions_controller_test.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
require "test_helper"

class Account::PermissionsControllerTest < ActionController::TestCase
context "#index" do
should "prevent unauthenticated users" do
application = create(:application)

get :index, params: { application_id: application.id }

assert_redirected_to "/users/sign_in"
end

should "exclude permissions that aren't grantable from the UI" do
application = create(:application,
with_supported_permissions: %w[perm-1],
with_supported_permissions_not_grantable_from_ui: %w[perm-2])
user = create(:admin_user, with_signin_permissions_for: [application])

sign_in user

get :index, params: { application_id: application.id }

assert_select "td", text: "perm-1"
assert_select "td", text: "perm-2", count: 0
end

should "exclude retired applications" do
sign_in create(:admin_user)

application = create(:application, retired: true)

assert_raises(ActiveRecord::RecordNotFound) do
get :index, params: { application_id: application.id }
end
end

should "order permissions by whether the user has access and then alphabetically" do
application = create(:application,
with_supported_permissions: %w[aaa bbb ttt uuu])
user = create(:admin_user,
with_signin_permissions_for: [application],
with_permissions: { application => %w[aaa ttt] })

sign_in user

get :index, params: { application_id: application.id }

assert_equal %w[signin aaa ttt bbb uuu], assigns(:permissions).map(&:name)
end
end
end
27 changes: 27 additions & 0 deletions test/integration/account_applications_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -101,4 +101,31 @@ class AccountApplicationsTest < ActionDispatch::IntegrationTest
assert table.has_content?("app-name")
end
end

context "viewing permissions for an app" do
setup do
application = create(:application, name: "app-name", description: "app-description", with_supported_permissions: %w[perm1 perm2])
@user = create(:admin_user)
@user.grant_application_signin_permission(application)
@user.grant_application_permission(application, "perm1")
end

should "allow admins to view their permissions for apps" do
visit new_user_session_path
signin_with @user

visit account_applications_path

click_on "View permissions for app-name"

signin_permission_row = find("table tr td:nth-child(1)", text: "signin").ancestor("tr")
assert signin_permission_row.has_content?("Yes")

perm1_permission_row = find("table tr td:nth-child(1)", text: "perm1").ancestor("tr")
assert perm1_permission_row.has_content?("Yes")

perm2_permission_row = find("table tr td:nth-child(1)", text: "perm2").ancestor("tr")
assert perm2_permission_row.has_content?("No")
end
end
end
24 changes: 24 additions & 0 deletions test/models/user_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -510,6 +510,30 @@ def setup
assert user.has_access_to?(app)
end

context "#has_permission?" do
setup do
@app = create(:application)
@supported_permission = create(:supported_permission, application: @app)
@user = create(:user)
end

context "when user has the permission" do
setup do
@user.supported_permissions << @supported_permission
end

should "return true" do
assert @user.has_permission?(@supported_permission)
end
end

context "when user does not have the permission" do
should "return false" do
assert_not @user.has_permission?(@supported_permission)
end
end
end

context "#remove_application_signin_permission" do
setup do
@app = create(:application)
Expand Down
26 changes: 26 additions & 0 deletions test/policies/user_policy_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -335,4 +335,30 @@ class UserPolicyTest < ActiveSupport::TestCase
end
end
end

context "#view_permissions?" do
%i[superadmin admin].each do |user_role|
context "for #{user_role} users" do
setup do
@current_user = build(:"#{user_role}_user")
end

should "be permitted" do
assert permit?(@current_user, @current_user, :view_permissions)
end
end
end

%i[super_organisation_admin organisation_admin normal].each do |user_role|
context "for #{user_role} users" do
setup do
@current_user = build(:"#{user_role}_user")
end

should "be forbidden" do
assert forbid?(@current_user, @current_user, :view_permissions)
end
end
end
end
end

0 comments on commit 99a3927

Please sign in to comment.