Skip to content

Commit

Permalink
Merge pull request #3081 from alphagov/1292-delegatable-permissions-n…
Browse files Browse the repository at this point in the history
…otice

Show notice when there are non-delegatable perms
yndajas authored Aug 13, 2024

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
2 parents cfc2314 + 2a8436c commit 44519de
Showing 10 changed files with 158 additions and 0 deletions.
2 changes: 2 additions & 0 deletions app/controllers/account/permissions_controller.rb
Original file line number Diff line number Diff line change
@@ -3,6 +3,8 @@ class Account::PermissionsController < ApplicationController
before_action :set_application
before_action :set_permissions, only: %i[edit update]

include ApplicationPermissionsHelper

def show
authorize [:account, @application], :view_permissions?

2 changes: 2 additions & 0 deletions app/controllers/users/permissions_controller.rb
Original file line number Diff line number Diff line change
@@ -4,6 +4,8 @@ class Users::PermissionsController < ApplicationController
before_action :set_application
before_action :set_permissions, only: %i[edit update]

include ApplicationPermissionsHelper

def show
authorize [{ application: @application, user: @user }], :view_permissions?, policy_class: Users::ApplicationPolicy

21 changes: 21 additions & 0 deletions app/helpers/application_permissions_helper.rb
Original file line number Diff line number Diff line change
@@ -22,4 +22,25 @@ def message_for_success(application_id, user = current_user)

paragraph + list
end

def notice_about_non_delegatable_permissions(current_user, application, other_grantee = nil)
return nil if current_user.govuk_admin?
return nil unless application.has_non_delegatable_non_signin_permissions_grantable_from_ui?

link = if other_grantee
link_to(
"view all the permissions #{other_grantee.name} has for #{application.name}",
user_application_permissions_path(other_grantee, application),
class: "govuk-link",
)
else
link_to(
"view all the permissions you have for #{application.name}",
account_application_permissions_path(application),
class: "govuk-link",
)
end

"Below, you will only see permissions that you are authorised to manage. You can also #{link}.".html_safe
end
end
4 changes: 4 additions & 0 deletions app/models/doorkeeper/application.rb
Original file line number Diff line number Diff line change
@@ -67,6 +67,10 @@ def has_delegatable_non_signin_permissions_grantable_from_ui?
(supported_permissions.delegatable.grantable_from_ui - [signin_permission]).any?
end

def has_non_delegatable_non_signin_permissions_grantable_from_ui?
(supported_permissions.grantable_from_ui.where(delegatable: false) - [signin_permission]).any?
end

def url_without_path
parsed_url = URI.parse(redirect_uri)
"#{parsed_url.scheme}://#{parsed_url.host}:#{parsed_url.port}"
8 changes: 8 additions & 0 deletions app/views/account/permissions/edit.html.erb
Original file line number Diff line number Diff line change
@@ -29,6 +29,14 @@
<% end %>
<% end %>

<% if notice = notice_about_non_delegatable_permissions(current_user, @application, @user) %>
<div class="govuk-grid-row">
<div class="govuk-grid-column-two-thirds">
<%= render "govuk_publishing_components/components/inset_text", { text: notice } %>
</div>
</div>
<% end %>

<%= render "shared/permissions_forms", {
assigned_permissions: @assigned_permissions,
unassigned_permission_options: @unassigned_permission_options,
8 changes: 8 additions & 0 deletions app/views/users/permissions/edit.html.erb
Original file line number Diff line number Diff line change
@@ -34,6 +34,14 @@
<% end %>
<% end %>

<% if notice = notice_about_non_delegatable_permissions(current_user, @application, @user) %>
<div class="govuk-grid-row">
<div class="govuk-grid-column-two-thirds">
<%= render "govuk_publishing_components/components/inset_text", { text: notice } %>
</div>
</div>
<% end %>

<%= render "shared/permissions_forms", {
assigned_permissions: @assigned_permissions,
unassigned_permission_options: @unassigned_permission_options,
57 changes: 57 additions & 0 deletions test/helpers/application_permissions_helper_test.rb
Original file line number Diff line number Diff line change
@@ -57,4 +57,61 @@ class ApplicationPermissionsHelperTest < ActionView::TestCase
end
end
end

context "#notice_about_non_delegatable_permissions" do
context "when the current user is a GOV.UK admin" do
setup do
@current_user = create(:user)
@current_user.expects(:govuk_admin?).returns(true)
end

should "return nil" do
assert_nil notice_about_non_delegatable_permissions(@current_user, create(:application))
end
end

context "when the current user is not a GOV.UK admin" do
setup do
@current_user = create(:user, name: "Current User")
@current_user.expects(:govuk_admin?).returns(false)
@application = create(:application, name: "My First App")
end

context "when the app has no non-delegatable non-signin permissions grantable from the UI" do
setup { @application.expects(:has_non_delegatable_non_signin_permissions_grantable_from_ui?).returns(false) }

should "return nil" do
assert_nil notice_about_non_delegatable_permissions(@current_user, @application)
end
end

context "when the app has some non-delegatable non-signin permissions grantable from the UI" do
setup { @application.expects(:has_non_delegatable_non_signin_permissions_grantable_from_ui?).returns(true) }

context "without another user passed in as the grantee" do
should "infer the grantee as the current user and return a notice with a link to the account application permissions path" do
link = "<a class=\"govuk-link\" href=\"#{account_application_permissions_path(@application)}\">view all the permissions you have for My First App</a>"

assert_equal(
"Below, you will only see permissions that you are authorised to manage. You can also #{link}.",
notice_about_non_delegatable_permissions(@current_user, @application),
)
end
end

context "with another user passed in as the grantee" do
should "return a notice with a link to the user application permissions path for the given grantee" do
grantee = create(:user, name: "Another User")

link = "<a class=\"govuk-link\" href=\"#{user_application_permissions_path(grantee, @application)}\">view all the permissions Another User has for My First App</a>"

assert_equal(
"Below, you will only see permissions that you are authorised to manage. You can also #{link}.",
notice_about_non_delegatable_permissions(@current_user, @application, grantee),
)
end
end
end
end
end
end
2 changes: 2 additions & 0 deletions test/integration/account_applications_test.rb
Original file line number Diff line number Diff line change
@@ -252,6 +252,8 @@ class AccountApplicationsTest < ActionDispatch::IntegrationTest

assert page.has_field?("delegatable_perm")
assert page.has_no_field?("non_delegatable_perm")

assert_selector ".govuk-inset-text", text: "Below, you will only see permissions that you are authorised to manage. You can also view all the permissions you have for app-name."
end

should "not be able to grant permissions that are not grantable_from_ui" do
4 changes: 4 additions & 0 deletions test/integration/granting_permissions_test.rb
Original file line number Diff line number Diff line change
@@ -225,6 +225,8 @@ class GrantingPermissionsTest < ActionDispatch::IntegrationTest

assert page.has_field?("delegatable_perm")
assert page.has_no_field?("non_delegatable_perm")

assert_selector ".govuk-inset-text", text: "Below, you will only see permissions that you are authorised to manage. You can also view all the permissions #{@user.name} has for MyApp."
end

should "not be able to grant permissions that are not grantable_from_ui" do
@@ -322,6 +324,8 @@ class GrantingPermissionsTest < ActionDispatch::IntegrationTest

assert page.has_field?("delegatable_perm")
assert page.has_no_field?("non_delegatable_perm")

assert_selector ".govuk-inset-text", text: "Below, you will only see permissions that you are authorised to manage. You can also view all the permissions #{@user.name} has for MyApp."
end

should "not be able to grant permissions that are not grantable_from_ui" do
50 changes: 50 additions & 0 deletions test/models/doorkeeper/application_test.rb
Original file line number Diff line number Diff line change
@@ -263,6 +263,56 @@ class Doorkeeper::ApplicationTest < ActiveSupport::TestCase
end
end

context "has_non_delegatable_non_signin_permissions_grantable_from_ui?" do
should "return false if no permissions are non-delegatable" do
app = create(
:application,
with_delegatable_supported_permissions: %w[delegtable],
with_delegatable_supported_permissions_not_grantable_from_ui: %w[delegatable-non-grantable],
with_non_delegatable_supported_permissions: [],
with_non_delegatable_supported_permissions_not_grantable_from_ui: [],
)

assert_not app.has_non_delegatable_non_signin_permissions_grantable_from_ui?
end

should "return false if no permissions are grantable from the UI" do
app = create(
:application,
with_delegatable_supported_permissions: [],
with_delegatable_supported_permissions_not_grantable_from_ui: %w[non-grantable],
with_non_delegatable_supported_permissions: [],
with_non_delegatable_supported_permissions_not_grantable_from_ui: %w[non-delegatable-non-grantable],
)

assert_not app.has_non_delegatable_non_signin_permissions_grantable_from_ui?
end

should "return false if only the signin permission is non-delegatable and grantable from the UI" do
app = create(
:application,
with_delegatable_supported_permissions: %w[delegatable],
with_delegatable_supported_permissions_not_grantable_from_ui: %w[non-grantable],
with_non_delegatable_supported_permissions: [SupportedPermission::SIGNIN_NAME],
with_non_delegatable_supported_permissions_not_grantable_from_ui: %w[non-delegatable-non-grantable],
)

assert_not app.has_non_delegatable_non_signin_permissions_grantable_from_ui?
end

should "return true if there are non-delegatable non-signin permissions grantable from the UI" do
app = create(
:application,
with_delegatable_supported_permissions: %w[delegatable],
with_delegatable_supported_permissions_not_grantable_from_ui: %w[non-grantable],
with_non_delegatable_supported_permissions: %w[yay!],
with_non_delegatable_supported_permissions_not_grantable_from_ui: %w[non-delegatable-non-grantable],
)

assert app.has_non_delegatable_non_signin_permissions_grantable_from_ui?
end
end

context ".all (default scope)" do
setup do
@app = create(:application)

0 comments on commit 44519de

Please sign in to comment.