| Name | +Description | +Permissions | +Remove access | +
|---|---|---|---|
| <%= application.name %> | +<%= application.description %> | ++ <% if policy(UserApplicationPermission.for(@user, application)).edit? %> + <% unless application.sorted_supported_permissions_grantable_from_ui(include_signin: false).empty? %> + <%= link_to edit_user_application_permissions_path(@user, application), class: "govuk-link" do %> + Update permissions for <%= application.name %> + <% end %> + <% end %> + <% else %> + <%= link_to user_application_permissions_path(@user, application), class: "govuk-link" do %> + View permissions for <%= application.name %> + <% end %> + <% end %> + | ++ <% if policy(UserApplicationPermission.for(@user, application)).delete? %> + <%= link_to delete_user_application_signin_permission_path(@user, application), + class: "govuk-button govuk-button--warning govuk-!-margin-0", + data: { module: "govuk-button" } do %> + Remove access to <%= application.name %> + <% end %> + <% end %> + | +
| Name | +Description | +Grant access | +
|---|---|---|
| <%= application.name %> | +<%= application.description %> | ++ <% if policy(UserApplicationPermission.for(@user, application)).create? %> + <%= button_to user_application_signin_permission_path(@user, application), + class: "govuk-button govuk-!-margin-0", + data: { module: "govuk-button" } do %> + Grant access to <%= application.name %> + <% end %> + <% end %> + | +
| Name | +Has this permission? | +
|---|---|
| <%= permission.name %> | ++ <% if @user.has_permission?(permission) %> + + Yes + + <% else %> + + No + + <% end %> + | +
Are you sure you want to remove <%= @user.name %>'s access to <%= @application.name %>?
+ +<%= form_with url: user_application_signin_permission_path(@user, @application), method: :delete do |form| %> + +<% end %> diff --git a/config/routes.rb b/config/routes.rb index f6337c61a..f6afe7ea8 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -37,7 +37,6 @@ resources :users, except: [:show] do member do - get :manage_permissions get :event_logs get :require_2sv end @@ -52,6 +51,12 @@ resource :two_step_verification_mandation, only: %i[edit update], controller: "users/two_step_verification_mandations" resource :invitation_resend, only: %i[edit update], controller: "users/invitation_resends" resource :unlocking, only: %i[edit update], controller: "users/unlockings" + resources :applications, only: %i[index show], controller: "users/applications" do + resource :permissions, only: %i[show edit update], controller: "users/permissions" + resource :signin_permission, only: %i[create destroy], controller: "users/signin_permissions" do + get :delete + end + end end get "user", to: "oauth_users#show" diff --git a/test/controllers/users/applications_controller_test.rb b/test/controllers/users/applications_controller_test.rb new file mode 100644 index 000000000..1e2f50995 --- /dev/null +++ b/test/controllers/users/applications_controller_test.rb @@ -0,0 +1,266 @@ +require "test_helper" + +class Users::ApplicationsControllerTest < ActionController::TestCase + context "#show" do + setup do + @application = create(:application) + end + + should "prevent unauthenticated users" do + user = create(:user) + + get :show, params: { user_id: user, id: @application.id } + + assert_redirected_to "/users/sign_in" + end + + should "prevent unauthorised users" do + user = create(:user) + + current_user = create(:admin_user) + sign_in current_user + + stub_policy current_user, user, edit?: false + + get :show, params: { user_id: user, id: @application.id } + + assert_not_authorised + end + + should "redirect authorised users to /accounts/applications" do + user = create(:user) + + current_user = create(:admin_user) + sign_in current_user + + stub_policy current_user, user, edit?: true + + get :show, params: { user_id: user, id: @application.id } + + assert_redirected_to user_applications_path(user) + end + end + + context "#index" do + should "prevent unauthenticated users" do + user = create(:user) + + get :index, params: { user_id: user } + + assert_redirected_to "/users/sign_in" + end + + should "prevent unauthorised users" do + user = create(:user) + + current_user = create(:admin_user) + sign_in current_user + + stub_policy current_user, user, edit?: false + + get :index, params: { user_id: user } + + assert_not_authorised + end + + should "display the applications the user has access to" do + user = create(:user) + application = create(:application, name: "app-name") + user.grant_application_signin_permission(application) + + current_user = create(:admin_user) + sign_in current_user + + stub_policy current_user, user, edit?: true + stub_policy_for_navigation_links current_user + + get :index, params: { user_id: user } + + assert_select "table:has( > caption[text()='Apps #{user.name} has access to'])" do + assert_select "tr td", text: "app-name" + end + end + + should "display the applications the user does not have access to" do + user = create(:user) + create(:application, name: "app-name") + + current_user = create(:admin_user) + sign_in current_user + + stub_policy current_user, user, edit?: true + stub_policy_for_navigation_links current_user + + get :index, params: { user_id: user } + + heading_id = css_select("h2:contains('Apps #{user.name} does not have access to')").attribute("id").value + assert_select "table[aria-labelledby='#{heading_id}']" do + assert_select "tr td", text: "app-name" + end + end + + should "display a button allowing the user to grant access to applications if they're authorised to grant access" do + user = create(:user) + application = create(:application, name: "app-name") + + current_user = create(:admin_user) + sign_in current_user + + stub_policy current_user, user, edit?: true + stub_policy_for_navigation_links current_user + + get :index, params: { user_id: user } + + assert_select "form[action='#{user_application_signin_permission_path(user, application)}']" do + assert_select "button", "Grant access to app-name" + end + end + + should "not display a button allowing the user to grant access to applications if they're not authorised to grant access" do + user = create(:user) + create(:application, name: "app-name") + + current_user = create(:admin_user) + sign_in current_user + + stub_policy current_user, user, edit?: false + stub_policy_for_navigation_links current_user + + get :index, params: { user_id: user } + + assert_select "button", { text: "Grant access to app-name", count: 0 } + end + + should "display a button allowing the user to remove access to applications if they're authorised to remove access" do + user = create(:user) + application = create(:application, name: "app-name") + user.grant_application_signin_permission(application) + + current_user = create(:admin_user) + sign_in current_user + + permission = stub_user_application_permission(user, application) + stub_policy current_user, user, edit?: true + stub_policy current_user, permission, delete?: true + stub_policy_for_navigation_links current_user + + get :index, params: { user_id: user } + + assert_select "a[href='#{delete_user_application_signin_permission_path(user, application)}']", text: "Remove access to app-name" + end + + should "not display a button allowing the user to remove access to applications if they're not authorised to remove access" do + user = create(:user) + application = create(:application, name: "app-name") + user.grant_application_signin_permission(application) + + current_user = create(:admin_user) + sign_in current_user + + permission = stub_user_application_permission(user, application) + stub_policy current_user, user, edit?: true + stub_policy current_user, permission, delete?: false + stub_policy_for_navigation_links current_user + + get :index, params: { user_id: user } + + assert_select "a[href='#{delete_user_application_signin_permission_path(user, application)}']", count: 0 + end + + should "not display a link to edit permissions if the user is authorised to edit permissions but the app only has the signin permission" do + user = create(:user) + application = create(:application, name: "app-name") + user.grant_application_signin_permission(application) + + current_user = create(:admin_user) + sign_in current_user + + permission = stub_user_application_permission(user, application) + stub_policy current_user, user, edit?: true + stub_policy current_user, permission, edit?: true + stub_policy_for_navigation_links current_user + + get :index, params: { user_id: user } + + assert_select "a[href='#{edit_user_application_permissions_path(user, application)}']", count: 0 + end + + should "display a link to edit permissions if the user is authorised to edit permissions" do + user = create(:user) + application = create(:application, name: "app-name", with_supported_permissions: %w[foo]) + user.grant_application_signin_permission(application) + + current_user = create(:admin_user) + sign_in current_user + + permission = stub_user_application_permission(user, application) + stub_policy current_user, user, edit?: true + stub_policy current_user, permission, edit?: true + stub_policy_for_navigation_links current_user + + get :index, params: { user_id: user } + + assert_select "a[href='#{edit_user_application_permissions_path(user, application)}']", text: "Update permissions for app-name" + end + + should "display a link to view permissions if the user is not authorised to edit permissions" do + user = create(:user) + application = create(:application, name: "app-name") + user.grant_application_signin_permission(application) + + current_user = create(:admin_user) + sign_in current_user + + permission = stub_user_application_permission(user, application) + stub_policy current_user, user, edit?: true + stub_policy current_user, permission, edit?: false + stub_policy_for_navigation_links current_user + + get :index, params: { user_id: user } + + assert_select "a[href='#{user_application_permissions_path(user, application)}']", text: "View permissions for app-name" + end + + should "not display a retired application" do + user = create(:user) + create(:application, name: "retired-app-name", retired: true) + + current_user = create(:admin_user) + sign_in current_user + + stub_policy current_user, user, edit?: true + stub_policy_for_navigation_links current_user + + get :index, params: { user_id: user } + + assert_select "tr td", text: "retired-app-name", count: 0 + end + + should "not display an API-only application" do + user = create(:user) + create(:application, name: "api-only-app-name", api_only: true) + + current_user = create(:admin_user) + sign_in current_user + + stub_policy current_user, user, edit?: true + stub_policy_for_navigation_links current_user + + get :index, params: { user_id: user } + + assert_select "tr td", text: "api-only-app-name", count: 0 + end + end + +private + + def stub_policy_for_navigation_links(current_user) + stub_policy(current_user, User, index?: true) + end + + def stub_user_application_permission(user, application) + permission = UserApplicationPermission.new + UserApplicationPermission.stubs(:for).with(user, application).returns(permission) + permission + end +end diff --git a/test/controllers/users/permissions_controller_test.rb b/test/controllers/users/permissions_controller_test.rb new file mode 100644 index 000000000..e43e2942a --- /dev/null +++ b/test/controllers/users/permissions_controller_test.rb @@ -0,0 +1,339 @@ +require "test_helper" + +class Users::PermissionsControllerTest < ActionController::TestCase + context "#show" do + should "prevent unauthenticated users" do + application = create(:application) + user = create(:user) + + get :show, params: { user_id: user, 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(:user, with_signin_permissions_for: [application]) + + current_user = create(:admin_user) + sign_in current_user + + stub_policy_for_navigation_links current_user + stub_policy current_user, user, edit?: true + + get :show, params: { user_id: user, application_id: application.id } + + assert_select "td", text: "perm-1" + assert_select "td", text: "perm-2", count: 0 + end + + should "exclude applications that the user doesn't have access to" do + sign_in create(:admin_user) + + application = create(:application) + user = create(:user) + + assert_raises(ActiveRecord::RecordNotFound) do + get :show, params: { user_id: user, application_id: application.id } + end + end + + should "exclude retired applications" do + sign_in create(:admin_user) + + application = create(:application, retired: true) + user = create(:user) + + assert_raises(ActiveRecord::RecordNotFound) do + get :show, params: { user_id: user, application_id: application.id } + end + end + + should "exclude API-only applications" do + sign_in create(:admin_user) + + application = create(:application, api_only: true) + user = create(:user) + + assert_raises(ActiveRecord::RecordNotFound) do + get :show, params: { user_id: user, 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(:user, + with_signin_permissions_for: [application], + with_permissions: { application => %w[aaa ttt] }) + + current_user = create(:admin_user) + sign_in current_user + + stub_policy_for_navigation_links current_user + stub_policy current_user, user, edit?: true + + get :show, params: { user_id: user, application_id: application.id } + + assert_equal %w[signin aaa ttt bbb uuu], assigns(:permissions).map(&:name) + end + + should "prevent unauthorised users" do + application = create(:application) + user = create(:user) + user.grant_application_signin_permission(application) + + current_user = create(:admin_user) + sign_in current_user + + stub_policy current_user, user, edit?: false + + get :show, params: { user_id: user, application_id: application.id } + + assert_not_authorised + end + end + + context "#edit" do + should "prevent unauthenticated users" do + application = create(:application) + user = create(:user) + + get :edit, params: { user_id: user, application_id: application.id } + + assert_redirected_to "/users/sign_in" + end + + should "prevent unauthorized users" do + application = create(:application) + user = create(:user) + user.grant_application_signin_permission(application) + + current_user = create(:admin_user) + sign_in current_user + + permission = stub_user_application_permission(user, application) + stub_policy current_user, permission, edit?: false + + get :edit, params: { user_id: user, application_id: application.id } + + assert_not_authorised + end + + should "display checkboxes for the grantable permissions" do + application = create(:application) + perm1 = create(:supported_permission, application:, name: "perm-1") + perm2 = create(:supported_permission, application:, name: "perm-2") + perm3 = create(:supported_permission, application:, name: "perm-3", grantable_from_ui: false) + user = create(:user, with_permissions: { application => %w[perm-1] }) + user.grant_application_signin_permission(application) + + current_user = create(:admin_user) + sign_in current_user + + permission = stub_user_application_permission(user, application) + stub_policy current_user, permission, edit?: true + + get :edit, params: { user_id: user.id, application_id: application.id } + + assert_select "input[type='checkbox'][checked='checked'][name='application[supported_permission_ids][]'][value='#{perm1.id}']" + assert_select "input[type='checkbox'][name='application[supported_permission_ids][]'][value='#{perm2.id}']" + assert_select "input[type='checkbox'][name='application[supported_permission_ids][]'][value='#{perm3.id}']", count: 0 + assert_select "input[type='checkbox'][name='application[supported_permission_ids][]'][value='#{application.signin_permission.id}']", count: 0 + end + + should "include a hidden field for the signin permission so that it is not removed" do + application = create(:application, with_supported_permissions: ["perm-1", SupportedPermission::SIGNIN_NAME]) + user = create(:user, with_permissions: { application => %w[perm-1] }) + user.grant_application_signin_permission(application) + + current_user = create(:admin_user) + sign_in current_user + + permission = stub_user_application_permission(user, application) + stub_policy current_user, permission, edit?: true + + get :edit, params: { user_id: user.id, application_id: application.id } + + assert_select "input[type='hidden'][value='#{application.signin_permission.id}']" + end + + should "exclude retired applications" do + sign_in create(:admin_user) + + application = create(:application, retired: true) + user = create(:user) + + assert_raises(ActiveRecord::RecordNotFound) do + get :edit, params: { user_id: user, application_id: application.id } + end + end + + should "exclude API-only applications" do + sign_in create(:admin_user) + + application = create(:application, api_only: true) + user = create(:user) + + assert_raises(ActiveRecord::RecordNotFound) do + get :edit, params: { user_id: user, application_id: application.id } + end + end + + should "raise an exception if the signin permission cannot be found" do + application = create(:application) + user = create(:user) + + current_user = create(:admin_user) + sign_in current_user + + assert_raises(ActiveRecord::RecordNotFound) do + get :edit, params: { user_id: user, application_id: application.id } + end + end + end + + context "#update" do + should "prevent unauthenticated users" do + application = create(:application) + user = create(:user) + + patch :update, params: { user_id: user, application_id: application.id } + + assert_redirected_to "/users/sign_in" + end + + should "replace the users permissions with new ones" do + application = create(:application, with_supported_permissions: %w[new old]) + user = create(:user, with_permissions: { application => %w[old] }) + user.grant_application_signin_permission(application) + + current_user = create(:admin_user) + sign_in current_user + + permission = stub_user_application_permission(user, application) + stub_policy current_user, permission, update?: true + + new_permission = application.supported_permissions.find_by(name: "new") + + expected_params = { supported_permission_ids: [new_permission.id] } + user_update = stub("user-update").responds_like_instance_of(UserUpdate) + user_update.expects(:call) + UserUpdate.stubs(:new).with(user, expected_params, current_user, anything).returns(user_update) + + patch :update, params: { user_id: user, application_id: application.id, application: { supported_permission_ids: [new_permission.id] } } + end + + should "redirect once the permissions have been updated" do + application = create(:application, with_supported_permissions: %w[new old]) + user = create(:user, with_permissions: { application => %w[old] }) + user.grant_application_signin_permission(application) + + current_user = create(:admin_user) + sign_in current_user + + permission = stub_user_application_permission(user, application) + stub_policy current_user, permission, update?: true + + new_permission = application.supported_permissions.find_by(name: "new") + + patch :update, params: { user_id: user, application_id: application.id, application: { supported_permission_ids: [new_permission.id] } } + + assert_redirected_to user_applications_path(user) + end + + should "retain permissions for other apps" do + other_application = create(:application, with_supported_permissions: %w[other]) + application = create(:application, with_supported_permissions: %w[new old]) + user = create(:user, with_permissions: { application => %w[old], other_application => %w[other] }) + user.grant_application_signin_permission(application) + + current_user = create(:admin_user) + sign_in current_user + + permission = stub_user_application_permission(user, application) + stub_policy current_user, permission, update?: true + + new_permission = application.supported_permissions.find_by(name: "new") + other_permission = other_application.supported_permissions.find_by(name: "other") + + expected_params = { supported_permission_ids: [other_permission.id, new_permission.id] } + user_update = stub("user-update").responds_like_instance_of(UserUpdate) + user_update.expects(:call) + UserUpdate.stubs(:new).with(user, expected_params, current_user, anything).returns(user_update) + + patch :update, params: { user_id: user, application_id: application.id, application: { supported_permission_ids: [new_permission.id] } } + end + + should "assign the application id to the application_id flash" do + application = create(:application, with_supported_permissions: %w[new old]) + user = create(:user, with_permissions: { application => %w[old] }) + user.grant_application_signin_permission(application) + + current_user = create(:admin_user) + sign_in current_user + + permission = stub_user_application_permission(user, application) + stub_policy current_user, permission, update?: true + + new_permission = application.supported_permissions.find_by(name: "new") + + patch :update, params: { user_id: user, application_id: application.id, application: { supported_permission_ids: [new_permission.id] } } + + assert_equal application.id, flash[:application_id] + end + + should "raise an exception if the user cannot be found" do + application = create(:application) + + current_user = create(:admin_user) + sign_in current_user + + assert_raises(ActiveRecord::RecordNotFound) do + patch :update, params: { user_id: "unknown-id", application_id: application.id, application: { supported_permission_ids: %w[id] } } + end + end + + should "raise an exception if the signin permission cannot be found" do + application = create(:application) + user = create(:user) + + current_user = create(:admin_user) + sign_in current_user + + assert_raises(ActiveRecord::RecordNotFound) do + patch :update, params: { user_id: user, application_id: application.id, application: { supported_permission_ids: %w[id] } } + end + end + + should "prevent unauthorised users" do + application = create(:application) + user = create(:user) + user.grant_application_signin_permission(application) + + current_user = create(:admin_user) + sign_in current_user + + permission = stub_user_application_permission(user, application) + stub_policy current_user, permission, update?: false + + patch :update, params: { user_id: user, application_id: application.id, application: { supported_permission_ids: [] } } + + assert_not_authorised + end + end + +private + + def stub_policy_for_navigation_links(current_user) + stub_policy(current_user, User, index?: true) + end + + def stub_user_application_permission(user, application) + permission = UserApplicationPermission.new + UserApplicationPermission.stubs(:for).with(user, application).returns(permission) + permission + end +end diff --git a/test/controllers/users/signin_permissions_controller_test.rb b/test/controllers/users/signin_permissions_controller_test.rb new file mode 100644 index 000000000..8e10637ee --- /dev/null +++ b/test/controllers/users/signin_permissions_controller_test.rb @@ -0,0 +1,302 @@ +require "test_helper" + +class Users::SigninPermissionsControllerTest < ActionController::TestCase + context "#create" do + should "call the UserUpdate service to grant the user access to the application" do + current_user = create(:admin_user) + sign_in current_user + + user = create(:user) + application = create(:application) + + permission = stub_user_application_permission(user, application) + stub_policy current_user, permission, create?: true + + expected_params = { supported_permission_ids: [application.signin_permission.id] } + user_update = stub("user-update").responds_like_instance_of(UserUpdate) + user_update.expects(:call) + UserUpdate.stubs(:new).with(user, expected_params, current_user, anything).returns(user_update) + + post :create, params: { user_id: user, application_id: application.id } + end + + should "redirect to the edit user path" do + current_user = create(:admin_user) + sign_in current_user + + user = create(:user) + application = create(:application) + + permission = stub_user_application_permission(user, application) + stub_policy current_user, permission, create?: true + + post :create, params: { user_id: user, application_id: application.id } + + assert_redirected_to user_applications_path(user) + end + + should "prevent unauthenticated users" do + user = create(:user) + application = create(:application) + + post :create, params: { user_id: user, application_id: application.id } + + assert_redirected_to "/users/sign_in" + end + + should "not authorize access if UserApplicationPermissionPolicy#create? returns false" do + current_user = create(:admin_user) + sign_in current_user + + user = create(:user) + application = create(:application) + + permission = stub_user_application_permission(user, application) + stub_policy current_user, permission, create?: false + + post :create, params: { user_id: user, application_id: application.id } + + assert_not_authorised + end + + should "raise exception is user isn't found" do + sign_in create(:admin_user) + + application = create(:application) + + assert_raises(ActiveRecord::RecordNotFound) do + post :create, params: { user_id: "non-existent-user-id", application_id: application.id } + end + end + + should "exclude retired applications" do + current_user = create(:admin_user) + sign_in current_user + + user = create(:user) + application = create(:application, retired: true) + + assert_raises(ActiveRecord::RecordNotFound) do + post :create, params: { user_id: user, application_id: application.id } + end + end + + should "exclude API-only applications" do + current_user = create(:admin_user) + sign_in current_user + + user = create(:user) + application = create(:application, api_only: true) + + assert_raises(ActiveRecord::RecordNotFound) do + post :create, params: { user_id: user, application_id: application.id } + end + end + end + + context "#delete" do + should "have a button to confirm deletion of the user when current_user is authorized to delete" do + current_user = create(:admin_user) + sign_in current_user + + user = create(:user) + application = create(:application) + user.grant_application_signin_permission(application) + + permission = stub_user_application_permission(user, application) + stub_policy current_user, permission, delete?: true + + get :delete, params: { user_id: user, application_id: application.id } + + assert_template :delete + assert_select "form[action='#{user_application_signin_permission_path(user, application)}']" do + assert_select "button[type='submit']", text: "Confirm" + assert_select "a[href='#{edit_user_path(user)}']", text: "Cancel" + end + end + + should "prevent unauthenticated users" do + user = create(:user) + application = create(:application) + + get :delete, params: { user_id: user, application_id: application.id } + + assert_redirected_to "/users/sign_in" + end + + should "not authorize access if UserApplicationPermissionPolicy#delete? returns false" do + current_user = create(:admin_user) + sign_in current_user + + user = create(:user) + application = create(:application) + user.grant_application_signin_permission(application) + + permission = stub_user_application_permission(user, application) + stub_policy current_user, permission, delete?: false + + get :delete, params: { user_id: user, application_id: application.id } + + assert_not_authorised + end + + should "raise exception if user isn't found" do + sign_in create(:admin_user) + + application = create(:application) + + assert_raises(ActiveRecord::RecordNotFound) do + get :delete, params: { user_id: "non-existent-user-id", application_id: application.id } + end + end + + should "raise exception if user doesn't have the signin permission for the app" do + sign_in create(:admin_user) + + user = create(:user) + application = create(:application) + + assert_raises(ActiveRecord::RecordNotFound) do + get :delete, params: { user_id: user, application_id: application.id } + end + end + + should "exclude retired applications" do + sign_in create(:admin_user) + + user = create(:user) + application = create(:application, retired: true) + + assert_raises(ActiveRecord::RecordNotFound) do + get :delete, params: { user_id: user, application_id: application.id } + end + end + + should "exclude API-only applications" do + sign_in create(:admin_user) + + user = create(:user) + application = create(:application, api_only: true) + user.grant_application_signin_permission(application) + + assert_raises(ActiveRecord::RecordNotFound) do + get :delete, params: { user_id: user, application_id: application.id } + end + end + end + + context "#destroy" do + should "call the UserUpdate service to remove the user's access to the application" do + current_user = create(:admin_user) + sign_in current_user + + user = create(:user) + application = create(:application) + user.grant_application_signin_permission(application) + + permission = stub_user_application_permission(user, application) + stub_policy current_user, permission, destroy?: true + + expected_params = { supported_permission_ids: [] } + user_update = stub("user-update").responds_like_instance_of(UserUpdate) + user_update.expects(:call) + UserUpdate.stubs(:new).with(user, expected_params, current_user, anything).returns(user_update) + + delete :destroy, params: { user_id: user, application_id: application.id } + end + + should "redirect to the edit user path" do + current_user = create(:admin_user) + sign_in current_user + + user = create(:user) + application = create(:application) + user.grant_application_signin_permission(application) + + permission = stub_user_application_permission(user, application) + stub_policy current_user, permission, destroy?: true + + delete :destroy, params: { user_id: user, application_id: application.id } + + assert_redirected_to user_applications_path(user) + end + + should "prevent unauthenticated users" do + user = create(:user) + application = create(:application) + + delete :destroy, params: { user_id: user, application_id: application.id } + + assert_redirected_to "/users/sign_in" + end + + should "not authorize access if UserApplicationPermissionPolicy#destroy? returns false" do + current_user = create(:admin_user) + sign_in current_user + + user = create(:user) + application = create(:application) + user.grant_application_signin_permission(application) + + permission = stub_user_application_permission(user, application) + stub_policy current_user, permission, destroy?: false + + delete :destroy, params: { user_id: user, application_id: application.id } + + assert_not_authorised + end + + should "raise exception is user isn't found" do + sign_in create(:admin_user) + + application = create(:application) + + assert_raises(ActiveRecord::RecordNotFound) do + delete :destroy, params: { user_id: "non-existent-user-id", application_id: application.id } + end + end + + should "raise exception is user doesn't have the signin permission for the app" do + sign_in create(:admin_user) + + user = create(:user) + application = create(:application) + + assert_raises(ActiveRecord::RecordNotFound) do + delete :destroy, params: { user_id: user, application_id: application.id } + end + end + + should "exclude retired applications" do + sign_in create(:admin_user) + + user = create(:user) + application = create(:application, retired: true) + user.grant_application_signin_permission(application) + + assert_raises(ActiveRecord::RecordNotFound) do + delete :destroy, params: { user_id: user, application_id: application.id } + end + end + + should "exclude API-only applications" do + sign_in create(:admin_user) + + user = create(:user) + application = create(:application, api_only: true) + user.grant_application_signin_permission(application) + + assert_raises(ActiveRecord::RecordNotFound) do + delete :destroy, params: { user_id: user, application_id: application.id } + end + end + end + +private + + def stub_user_application_permission(user, application) + permission = UserApplicationPermission.new + UserApplicationPermission.stubs(:for).with(user, application).returns(permission) + permission + end +end diff --git a/test/controllers/users_controller_test.rb b/test/controllers/users_controller_test.rb index 18e225b33..3214fb231 100644 --- a/test/controllers/users_controller_test.rb +++ b/test/controllers/users_controller_test.rb @@ -249,6 +249,20 @@ class UsersControllerTest < ActionController::TestCase end context "GET edit" do + should "display link to manage applications" do + user = create(:user) + + current_user = create(:user) + sign_in current_user + + stub_policy_for_navigation_links current_user + stub_policy current_user, user, edit?: true + + get :edit, params: { id: user } + + assert_select "a[href='#{user_applications_path(user)}']" + end + context "signed in as Admin user" do setup do @user = create(:admin_user, email: "admin@gov.uk") @@ -398,285 +412,6 @@ class UsersControllerTest < ActionController::TestCase end end - context "GET manage_permissions" do - context "signed in as Admin user" do - setup do - @user = create(:admin_user, email: "admin@gov.uk") - sign_in @user - end - - context "for the currently logged in user" do - should "redirect to the account page" do - get :manage_permissions, params: { id: @user.id } - assert_redirected_to account_path - end - end - - should "not be able to manage permissions for superadmins" do - superadmin = create(:superadmin_user) - - get :manage_permissions, params: { id: superadmin.id } - - assert_not_authorised - end - - should "can give permissions to all applications" do - delegatable_app = create(:application, with_delegatable_supported_permissions: [SupportedPermission::SIGNIN_NAME]) - non_delegatable_app = create(:application, with_supported_permissions: [SupportedPermission::SIGNIN_NAME]) - delegatable_no_access_to_app = create(:application, with_delegatable_supported_permissions: [SupportedPermission::SIGNIN_NAME]) - non_delegatable_no_access_to_app = create(:application, with_supported_permissions: [SupportedPermission::SIGNIN_NAME]) - - @user.grant_application_signin_permission(delegatable_app) - @user.grant_application_signin_permission(non_delegatable_app) - - user = create(:user_in_organisation) - - get :manage_permissions, params: { id: user.id } - - assert_select ".container" do - # can give permissions to a delegatable app - assert_select "td", count: 1, text: delegatable_app.name - # can give permissions to a non delegatable app - assert_select "td", count: 1, text: non_delegatable_app.name - # can give permissions to a delegatable app the admin doesn't have access to - assert_select "td", count: 1, text: delegatable_no_access_to_app.name - # can give permissions to a non-delegatable app the admin doesn't have access to - assert_select "td", count: 1, text: non_delegatable_no_access_to_app.name - end - end - end - - context "signed in as Organisation Admin user" do - setup do - @organisation_admin = create(:organisation_admin_user) - sign_in @organisation_admin - end - - should "be able to give permissions only to applications they themselves have access to and that also have delegatable signin permissions" do - delegatable_app = create(:application, with_delegatable_supported_permissions: [SupportedPermission::SIGNIN_NAME]) - non_delegatable_app = create(:application, with_supported_permissions: [SupportedPermission::SIGNIN_NAME]) - delegatable_no_access_to_app = create(:application, with_delegatable_supported_permissions: [SupportedPermission::SIGNIN_NAME]) - non_delegatable_no_access_to_app = create(:application, with_supported_permissions: [SupportedPermission::SIGNIN_NAME]) - - @organisation_admin.grant_application_signin_permission(delegatable_app) - @organisation_admin.grant_application_signin_permission(non_delegatable_app) - - user = create(:user_in_organisation, organisation: @organisation_admin.organisation) - - get :manage_permissions, params: { id: user.id } - - assert_select "#editable-permissions" do - # can give access to a delegatable app they have access to - assert_select "td", count: 1, text: delegatable_app.name - # can not give access to a non-delegatable app they have access to - assert_select "td", count: 0, text: non_delegatable_app.name - # can not give access to a delegatable app they do not have access to - assert_select "td", count: 0, text: delegatable_no_access_to_app.name - # can not give access to a non delegatable app they do not have access to - assert_select "td", count: 0, text: non_delegatable_no_access_to_app.name - end - end - - should "be able to see all permissions to applications for a user" do - delegatable_app = create(:application, with_delegatable_supported_permissions: [SupportedPermission::SIGNIN_NAME, "Editor"]) - non_delegatable_app = create(:application, with_supported_permissions: [SupportedPermission::SIGNIN_NAME, "GDS Admin"]) - delegatable_no_access_to_app = create(:application, with_delegatable_supported_permissions: [SupportedPermission::SIGNIN_NAME, "GDS Editor"]) - non_delegatable_no_access_to_app = create(:application, with_supported_permissions: [SupportedPermission::SIGNIN_NAME, "Import CSVs"]) - - @organisation_admin.grant_application_signin_permission(delegatable_app) - @organisation_admin.grant_application_signin_permission(non_delegatable_app) - - user = create( - :user_in_organisation, - organisation: @organisation_admin.organisation, - with_permissions: { delegatable_app => %w[Editor], - non_delegatable_app => [SupportedPermission::SIGNIN_NAME, "GDS Admin"], - delegatable_no_access_to_app => [SupportedPermission::SIGNIN_NAME, "GDS Editor"], - non_delegatable_no_access_to_app => [SupportedPermission::SIGNIN_NAME, "Import CSVs"] }, - ) - - get :manage_permissions, params: { id: user.id } - - assert_select "h2", "All Permissions for this user" - assert_select "#all-permissions" do - # can see permissions for a delegatable app the user has access to - assert_select "td", count: 1, text: delegatable_app.name - # can see permissions to a non-delegatable app the user has access to - assert_select "td", count: 1, text: non_delegatable_app.name - # can see permissions to a delegatable app the user has access to - assert_select "td", count: 1, text: delegatable_no_access_to_app.name - # can see permissions to a non delegatable app the user has access to - assert_select "td", count: 1, text: non_delegatable_no_access_to_app.name - # can see that user has signin permission for three of the four applications in "Has access?" column - assert_select "td", count: 3, text: "Yes" - assert_select "td", count: 1, text: "No" - # can see role - assert_select "td", count: 1, text: "Editor" - assert_select "td", count: 1, text: "GDS Admin" - assert_select "td", count: 1, text: "GDS Editor" - assert_select "td", count: 1, text: "Import CSVs" - end - end - end - - context "signed in as Super Organisation Admin user" do - setup do - @super_organisation_admin = create(:super_organisation_admin_user) - sign_in @super_organisation_admin - end - - should "be able to give permissions only to applications they themselves have access to and that also have delegatable signin permissions" do - delegatable_app = create(:application, with_delegatable_supported_permissions: [SupportedPermission::SIGNIN_NAME]) - non_delegatable_app = create(:application, with_supported_permissions: [SupportedPermission::SIGNIN_NAME]) - delegatable_no_access_to_app = create(:application, with_delegatable_supported_permissions: [SupportedPermission::SIGNIN_NAME]) - non_delegatable_no_access_to_app = create(:application, with_supported_permissions: [SupportedPermission::SIGNIN_NAME]) - - @super_organisation_admin.grant_application_signin_permission(delegatable_app) - @super_organisation_admin.grant_application_signin_permission(non_delegatable_app) - - user = create(:user_in_organisation, organisation: @super_organisation_admin.organisation) - - get :manage_permissions, params: { id: user.id } - - assert_select "#editable-permissions" do - # can give access to a delegatable app they have access to - assert_select "td", count: 1, text: delegatable_app.name - # can not give access to a non-delegatable app they have access to - assert_select "td", count: 0, text: non_delegatable_app.name - # can not give access to a delegatable app they do not have access to - assert_select "td", count: 0, text: delegatable_no_access_to_app.name - # can not give access to a non delegatable app they do not have access to - assert_select "td", count: 0, text: non_delegatable_no_access_to_app.name - end - end - - should "be able to see all permissions to applications for a user" do - delegatable_app = create(:application, with_delegatable_supported_permissions: [SupportedPermission::SIGNIN_NAME, "Editor"]) - non_delegatable_app = create(:application, with_supported_permissions: [SupportedPermission::SIGNIN_NAME, "GDS Admin"]) - delegatable_no_access_to_app = create(:application, with_delegatable_supported_permissions: [SupportedPermission::SIGNIN_NAME, "GDS Editor"]) - non_delegatable_no_access_to_app = create(:application, with_supported_permissions: [SupportedPermission::SIGNIN_NAME, "Import CSVs"]) - - @super_organisation_admin.grant_application_signin_permission(delegatable_app) - @super_organisation_admin.grant_application_signin_permission(non_delegatable_app) - - user = create( - :user_in_organisation, - organisation: @super_organisation_admin.organisation, - with_permissions: { delegatable_app => %w[Editor], - non_delegatable_app => [SupportedPermission::SIGNIN_NAME, "GDS Admin"], - delegatable_no_access_to_app => [SupportedPermission::SIGNIN_NAME, "GDS Editor"], - non_delegatable_no_access_to_app => [SupportedPermission::SIGNIN_NAME, "Import CSVs"] }, - ) - - get :manage_permissions, params: { id: user.id } - - assert_select "h2", "All Permissions for this user" - assert_select "#all-permissions" do - # can see permissions for a delegatable app the user has access to - assert_select "td", count: 1, text: delegatable_app.name - # can see permissions to a non-delegatable app the user has access to - assert_select "td", count: 1, text: non_delegatable_app.name - # can see permissions to a delegatable app the user has access to - assert_select "td", count: 1, text: delegatable_no_access_to_app.name - # can see permissions to a non delegatable app the user has access to - assert_select "td", count: 1, text: non_delegatable_no_access_to_app.name - # can see that user has signin permission for three of the four applications in "Has access?" column - assert_select "td", count: 3, text: "Yes" - assert_select "td", count: 1, text: "No" - # can see role - assert_select "td", count: 1, text: "Editor" - assert_select "td", count: 1, text: "GDS Admin" - assert_select "td", count: 1, text: "GDS Editor" - assert_select "td", count: 1, text: "Import CSVs" - end - end - - should "not be able to see permissions to retired applications for a user" do - retired_app = create(:application, retired: true) - - user = create(:user_in_organisation, organisation: @super_organisation_admin.organisation) - create(:user_application_permission, application: retired_app, user:) - - get :manage_permissions, params: { id: user.id } - - assert_select "h2", "All Permissions for this user" - assert_select "#all-permissions" do - assert_select "td", text: retired_app.name, count: 0 - end - end - - should "not be able to see permissions to API-only applications for a user" do - api_only_app = create(:application, api_only: true) - - user = create(:user_in_organisation, organisation: @super_organisation_admin.organisation) - create(:user_application_permission, application: api_only_app, user:) - - get :manage_permissions, params: { id: user.id } - - assert_select "h2", "All Permissions for this user" - assert_select "#all-permissions" do - assert_select "td", text: api_only_app.name, count: 0 - end - end - end - - context "signed in as Superadmin user" do - setup do - @superadmin = create(:superadmin_user) - sign_in @superadmin - end - - should "not be able to see all permissions to applications for a user" do - delegatable_app = create(:application, with_delegatable_supported_permissions: [SupportedPermission::SIGNIN_NAME, "Editor"]) - non_delegatable_app = create(:application, with_supported_permissions: [SupportedPermission::SIGNIN_NAME, "GDS Admin"]) - delegatable_no_access_to_app = create(:application, with_delegatable_supported_permissions: [SupportedPermission::SIGNIN_NAME, "GDS Editor"]) - non_delegatable_no_access_to_app = create(:application, with_supported_permissions: [SupportedPermission::SIGNIN_NAME, "Import CSVs"]) - - @superadmin.grant_application_signin_permission(delegatable_app) - @superadmin.grant_application_signin_permission(non_delegatable_app) - - user = create( - :user_in_organisation, - organisation: @superadmin.organisation, - with_permissions: { delegatable_app => %w[Editor], - non_delegatable_app => [SupportedPermission::SIGNIN_NAME, "GDS Admin"], - delegatable_no_access_to_app => [SupportedPermission::SIGNIN_NAME, "GDS Editor"], - non_delegatable_no_access_to_app => [SupportedPermission::SIGNIN_NAME, "Import CSVs"] }, - ) - - get :manage_permissions, params: { id: user.id } - - assert_select "h2", count: 0, text: "All Permissions for this user" - assert_select "#all-permissions", count: 0 - end - end - - context "signed in as Normal user" do - setup do - @user = create(:user, email: "normal@gov.uk") - sign_in @user - end - - context "when current user tries to manage their own permissions" do - should "redirect to the account page" do - get :manage_permissions, params: { id: @user } - - assert_redirected_to account_path - end - end - - context "when current user tries to manage another user's permissions" do - should "redirect to the dashboard and explain user does not have permission" do - another_user = create(:user) - - get :manage_permissions, params: { id: another_user } - - assert_not_authorised - end - end - end - end - context "PUT update" do context "signed in as Admin user" do setup do @@ -696,74 +431,14 @@ class UsersControllerTest < ActionController::TestCase another_user = create(:user) PermissionUpdater.expects(:perform_on).with(another_user).once - put :update, params: { id: another_user.id, user: {} } - end - - context "update application access" do - setup do - sign_in create(:admin_user) - @application = create(:application) - @another_user = create(:user) - end - - should "remove all applications access for a user" do - @another_user.grant_application_signin_permission(@application) - - put :update, params: { id: @another_user.id, user: {} } - - assert_empty @another_user.reload.application_permissions - end - - should "add application access for a user" do - put( - :update, - params: { - id: @another_user.id, - user: { - supported_permission_ids: [ - @application.supported_permissions.first.id, - ], - }, - }, - ) - - assert_equal 1, @another_user.reload.application_permissions.count - end + put :update, params: { id: another_user.id, user: { require_2sv: true } } end end + end - context "signed in as Organisation Admin user" do - setup do - @organisation_admin = create(:organisation_admin_user) - sign_in(@organisation_admin) - end - - should "redisplay the form if save fails" do - organisation = @organisation_admin.organisation - organisation_admin_for_same_organisation = create(:organisation_admin_user, organisation:) - UserUpdate.stubs(:new).returns(stub("UserUpdate", call: false)) - - put :update, params: { id: organisation_admin_for_same_organisation.id, user: {} } - - assert_select "form#edit_user_#{organisation_admin_for_same_organisation.id}" - end - end - - context "signed in as Super Organisation Admin user" do - setup do - @super_organisation_admin = create(:super_organisation_admin_user) - sign_in(@super_organisation_admin) - end - - should "redisplay the form if save fails" do - organisation = @super_organisation_admin.organisation - super_organisation_admin_for_same_organisation = create(:super_organisation_admin_user, organisation:) - UserUpdate.stubs(:new).returns(stub("UserUpdate", call: false)) - - put :update, params: { id: super_organisation_admin_for_same_organisation.id, user: {} } +private - assert_select "form#edit_user_#{super_organisation_admin_for_same_organisation.id}" - end - end + def stub_policy_for_navigation_links(current_user) + stub_policy(current_user, User, index?: true) end end diff --git a/test/helpers/account_applications_helper_test.rb b/test/helpers/application_permissions_helper_test.rb similarity index 59% rename from test/helpers/account_applications_helper_test.rb rename to test/helpers/application_permissions_helper_test.rb index e55f9105f..40751ff05 100644 --- a/test/helpers/account_applications_helper_test.rb +++ b/test/helpers/application_permissions_helper_test.rb @@ -1,6 +1,6 @@ require "test_helper" -class AccountApplicationsHelperTest < ActionView::TestCase +class ApplicationPermissionsHelperTest < ActionView::TestCase context "#message_for_success" do setup do @application = create(:application, name: "Whitehall", with_supported_permissions: ["Permission 1"]) @@ -36,5 +36,25 @@ class AccountApplicationsHelperTest < ActionView::TestCase assert_includes message_for_success(@application.id), "You can access Whitehall but you do not have any additional permissions." end end + + context "when the user isn't the current user" do + setup do + @user = create(:user, name: "user-name", with_permissions: { @application => ["Permission 1", SupportedPermission::SIGNIN_NAME] }) + end + + should "include the application name in the message" do + assert_includes message_for_success(@application.id, @user), "user-name now has the following permissions for Whitehall" + end + end + + context "when the user isn't the current user and the user has no additional permissions" do + setup do + @user = create(:user, name: "user-name", with_permissions: { @application => [SupportedPermission::SIGNIN_NAME] }) + end + + should "indicate that the user has no additional permissions" do + assert_includes message_for_success(@application.id, @user), "user-name can access Whitehall but does not have any additional permissions." + end + end end end diff --git a/test/integration/granting_permissions_test.rb b/test/integration/granting_permissions_test.rb index 676529046..b8b399ad8 100644 --- a/test/integration/granting_permissions_test.rb +++ b/test/integration/granting_permissions_test.rb @@ -15,8 +15,7 @@ class GrantingPermissionsTest < ActionDispatch::IntegrationTest visit edit_user_path(@user) click_link "Manage permissions" - check "Has access to MyApp?" - click_button "Update User" + click_button "Grant access to MyApp" assert @user.has_access_to?(app) end @@ -27,12 +26,14 @@ class GrantingPermissionsTest < ActionDispatch::IntegrationTest name: "MyApp", with_supported_permissions: %w[pre-existing adding never], ) + @user.grant_application_signin_permission(app) @user.grant_application_permission(app, "pre-existing") visit edit_user_path(@user) click_link "Manage permissions" - select "adding", from: "Permissions for MyApp" - click_button "Update User" + click_link "Update permissions for MyApp" + check "adding" + click_button "Update permissions" assert_includes @user.permissions_for(app), "pre-existing" assert_includes @user.permissions_for(app), "adding" @@ -40,11 +41,15 @@ class GrantingPermissionsTest < ActionDispatch::IntegrationTest end should "not be able to grant permissions that are not grantable_from_ui" do - create(:application, name: "MyApp", with_supported_permissions_not_grantable_from_ui: %w[user_update_permission]) + app = create(:application, name: "MyApp", with_supported_permissions: %w[perm], with_supported_permissions_not_grantable_from_ui: %w[user_update_permission]) + @user.grant_application_signin_permission(app) visit edit_user_path(@user) click_link "Manage permissions" - assert page.has_no_select?("Permissions for MyApp", options: %w[user_update_permission]) + click_link "Update permissions for MyApp" + + assert page.has_field?("perm") + assert page.has_no_field?("user_update_permission") end end @@ -62,8 +67,7 @@ class GrantingPermissionsTest < ActionDispatch::IntegrationTest visit edit_user_path(@user) click_link "Manage permissions" - check "Has access to MyApp?" - click_button "Update User" + click_button "Grant access to MyApp" assert @user.has_access_to?(app) end @@ -74,12 +78,14 @@ class GrantingPermissionsTest < ActionDispatch::IntegrationTest name: "MyApp", with_supported_permissions: %w[pre-existing adding never], ) + @user.grant_application_signin_permission(app) @user.grant_application_permission(app, "pre-existing") visit edit_user_path(@user) click_link "Manage permissions" - select "adding", from: "Permissions for MyApp" - click_button "Update User" + click_link "Update permissions for MyApp" + check "adding" + click_button "Update permissions" assert_includes @user.permissions_for(app), "pre-existing" assert_includes @user.permissions_for(app), "adding" @@ -87,11 +93,15 @@ class GrantingPermissionsTest < ActionDispatch::IntegrationTest end should "not be able to grant permissions that are not grantable_from_ui" do - create(:application, name: "MyApp", with_supported_permissions_not_grantable_from_ui: %w[user_update_permission]) + app = create(:application, name: "MyApp", with_supported_permissions: %w[perm], with_supported_permissions_not_grantable_from_ui: %w[user_update_permission]) + @user.grant_application_signin_permission(app) visit edit_user_path(@user) click_link "Manage permissions" - assert page.has_no_select?("Permissions for MyApp", options: %w[user_update_permission]) + click_link "Update permissions for MyApp" + + assert page.has_field?("perm") + assert page.has_no_field?("user_update_permission") end end @@ -110,8 +120,7 @@ class GrantingPermissionsTest < ActionDispatch::IntegrationTest visit edit_user_path(@user) click_link "Manage permissions" - check "Has access to MyApp?" - click_button "Update User" + click_button "Grant access to MyApp" assert @user.reload.has_access_to?(app) end @@ -124,7 +133,8 @@ class GrantingPermissionsTest < ActionDispatch::IntegrationTest visit edit_user_path(@user) click_link "Manage permissions" - assert page.has_no_field? "Has access to MyApp?" + + assert page.has_no_button? "Grant access to MyApp?" end should "not support granting signin permissions to apps that the super organisation admin doesn't have access to" do @@ -132,31 +142,8 @@ class GrantingPermissionsTest < ActionDispatch::IntegrationTest visit edit_user_path(@user) click_link "Manage permissions" - assert page.has_no_field? "Has access to MyApp?" - end - - should "not remove permissions the user has that the super organisation admin does not have" do - app = create(:application, name: "MyApp") - @user.grant_application_signin_permission(app) - - visit edit_user_path(@user) - click_link "Manage permissions" - click_button "Update User" - - assert @user.reload.has_access_to?(app) - end - - should "not remove permissions the user has that the super organisation admin cannot delegate" do - app = create(:application, name: "MyApp") - app.signin_permission.update!(delegatable: false) - @super_org_admin.grant_application_signin_permission(app) - @user.grant_application_signin_permission(app) - - visit edit_user_path(@user) - click_link "Manage permissions" - click_button "Update User" - assert @user.reload.has_access_to?(app) + assert page.has_no_button? "Grant access to MyApp?" end should "support granting app-specific permissions" do @@ -166,12 +153,14 @@ class GrantingPermissionsTest < ActionDispatch::IntegrationTest with_supported_permissions: %w[pre-existing adding never], ) @super_org_admin.grant_application_signin_permission(app) + @user.grant_application_signin_permission(app) @user.grant_application_permission(app, "pre-existing") visit edit_user_path(@user) click_link "Manage permissions" - select "adding", from: "Permissions for MyApp" - click_button "Update User" + click_link "Update permissions for MyApp" + check "adding" + click_button "Update permissions" assert_includes @user.permissions_for(app), "pre-existing" assert_includes @user.permissions_for(app), "adding" @@ -203,8 +192,7 @@ class GrantingPermissionsTest < ActionDispatch::IntegrationTest visit edit_user_path(@user) click_link "Manage permissions" - check "Has access to MyApp?" - click_button "Update User" + click_button "Grant access to MyApp" assert @user.reload.has_access_to?(app) end @@ -228,30 +216,6 @@ class GrantingPermissionsTest < ActionDispatch::IntegrationTest assert page.has_no_field? "Has access to MyApp?" end - should "not remove permissions the user has that the organisation admin does not have" do - app = create(:application, name: "MyApp") - @user.grant_application_signin_permission(app) - - visit edit_user_path(@user) - click_link "Manage permissions" - click_button "Update User" - - assert @user.reload.has_access_to?(app) - end - - should "not remove permissions the user has that the organisation admin cannot delegate" do - app = create(:application, name: "MyApp") - app.signin_permission.update!(delegatable: false) - @organisation_admin.grant_application_signin_permission(app) - @user.grant_application_signin_permission(app) - - visit edit_user_path(@user) - click_link "Manage permissions" - click_button "Update User" - - assert @user.reload.has_access_to?(app) - end - should "support granting app-specific permissions" do app = create( :application, @@ -259,12 +223,14 @@ class GrantingPermissionsTest < ActionDispatch::IntegrationTest with_supported_permissions: %w[pre-existing adding never], ) @organisation_admin.grant_application_signin_permission(app) + @user.grant_application_signin_permission(app) @user.grant_application_permission(app, "pre-existing") visit edit_user_path(@user) click_link "Manage permissions" - select "adding", from: "Permissions for MyApp" - click_button "Update User" + click_link "Update permissions for MyApp" + check "adding" + click_button "Update permissions" assert_includes @user.permissions_for(app), "pre-existing" assert_includes @user.permissions_for(app), "adding" diff --git a/test/integration/user_applications_test.rb b/test/integration/user_applications_test.rb new file mode 100644 index 000000000..ce8f1d335 --- /dev/null +++ b/test/integration/user_applications_test.rb @@ -0,0 +1,72 @@ +require "test_helper" + +class UserApplicationsTest < ActionDispatch::IntegrationTest + should "allow admins to grant users access to apps" do + user = create(:user, name: "user-name") + create(:application, name: "app-name") + + admin_user = create(:admin_user) + visit new_user_session_path + signin_with admin_user + + visit user_applications_path(user) + + heading = find("h2", text: "Apps user-name does not have access to") + table = find("table[aria-labelledby='#{heading['id']}']") + assert table.has_content?("app-name") + + click_on "Grant access to app-name" + + table = find("table caption[text()='Apps user-name has access to']").ancestor("table") + assert table.has_content?("app-name") + end + + should "allow admins to remove users' access to apps" do + user = create(:user, name: "user-name") + application = create(:application, name: "app-name") + user.grant_application_signin_permission(application) + + admin_user = create(:admin_user) + visit new_user_session_path + signin_with admin_user + + visit user_applications_path(user) + + table = find("table caption[text()='Apps user-name has access to']").ancestor("table") + assert table.has_content?("app-name") + + click_on "Remove access to app-name" + click_on "Confirm" + + heading = find("h2", text: "Apps user-name does not have access to") + table = find("table[aria-labelledby='#{heading['id']}']") + assert table.has_content?("app-name") + end + + should "allow admins to update users' permissions for apps" do + application = create(:application, name: "app-name", with_supported_permissions: %w[perm1 perm2]) + application.signin_permission.update!(delegatable: true) + + user = create(:admin_user) + user.grant_application_signin_permission(application) + user.grant_application_permission(application, "perm1") + + admin_user = create(:admin_user) + visit new_user_session_path + signin_with admin_user + + visit user_applications_path(user) + + click_on "Update permissions for app-name" + + assert page.has_checked_field?("perm1") + assert page.has_unchecked_field?("perm2") + + check "perm2" + click_button "Update permissions" + + success_flash = find("div[role='alert']") + assert success_flash.has_content?("perm1") + assert success_flash.has_content?("perm2") + end +end diff --git a/test/policies/user_application_permission_policy_test.rb b/test/policies/user_application_permission_policy_test.rb new file mode 100644 index 000000000..395b83898 --- /dev/null +++ b/test/policies/user_application_permission_policy_test.rb @@ -0,0 +1,97 @@ +require "test_helper" +require "support/policy_helpers" + +class UserApplicationPermissionPolicyTest < ActiveSupport::TestCase + include PolicyHelpers + include PunditHelpers + + %i[create destroy delete update edit].each do |policy_method| + context "#{policy_method}?" do + setup do + @user = create(:user) + @application = create(:application) + @signin_permission = @user.grant_application_signin_permission(@application) + end + + should "be allowed for superadmins" do + current_user = build(:superadmin_user) + + stub_policy(current_user, @user, edit?: true) + + assert permit?(current_user, @signin_permission, policy_method) + end + + should "be allowed for admins" do + current_user = build(:admin_user) + + stub_policy(current_user, @user, edit?: true) + + assert permit?(current_user, @signin_permission, policy_method) + end + + should "be allowed for super org admins when they have access to the application" do + current_user = build(:super_organisation_admin_user) + current_user.grant_application_signin_permission(@application) + + stub_policy(current_user, @user, edit?: true) + + assert permit?(current_user, @signin_permission, policy_method) + end + + should "not be allowed for super org admins when they don't have access to the application" do + current_user = build(:super_organisation_admin_user) + + stub_policy(current_user, @user, edit?: true) + + assert forbid?(current_user, @signin_permission, policy_method) + end + + should "be allowed for org admins when they have access to the application" do + current_user = build(:organisation_admin_user) + current_user.grant_application_signin_permission(@application) + + stub_policy(current_user, @user, edit?: true) + + assert permit?(current_user, @signin_permission, policy_method) + end + + should "not be allowed for org admins when they don't have access to the application" do + current_user = build(:organisation_admin_user) + + stub_policy(current_user, @user, edit?: true) + + assert forbid?(current_user, @signin_permission, policy_method) + end + + context "when the signin permission is not delegatable" do + setup do + @application.signin_permission.update!(delegatable: false) + end + + should "not be allowed for super org admins" do + current_user = build(:super_organisation_admin_user) + + stub_policy(current_user, @user, edit?: true) + + assert forbid?(current_user, @signin_permission, policy_method) + end + + should "not be allowed for org admins" do + current_user = build(:organisation_admin_user) + + stub_policy(current_user, @user, edit?: true) + + assert forbid?(current_user, @signin_permission, policy_method) + end + end + + should "not be allowed if current user is not allowed to edit the target user" do + current_user = build(:user) + + stub_policy(current_user, @user, edit?: false) + + assert forbid?(current_user, @signin_permission, policy_method) + end + end + end +end diff --git a/test/policies/user_policy_test.rb b/test/policies/user_policy_test.rb index aab5dc5bf..abd3f12d9 100644 --- a/test/policies/user_policy_test.rb +++ b/test/policies/user_policy_test.rb @@ -13,7 +13,7 @@ class UserPolicyTest < ActiveSupport::TestCase end primary_management_actions = %i[new create assign_organisation] - user_management_actions = %i[edit update unlock suspension cancel_email_change resend_email_change event_logs reset_2sv mandate_2sv resend_invitation manage_permissions] + user_management_actions = %i[edit update unlock suspension cancel_email_change resend_email_change event_logs reset_2sv mandate_2sv resend_invitation] superadmin_actions = %i[assign_role] two_step_verification_exemption_actions = %i[exempt_from_two_step_verification] diff --git a/test/support/pundit_helpers.rb b/test/support/pundit_helpers.rb new file mode 100644 index 000000000..da29348da --- /dev/null +++ b/test/support/pundit_helpers.rb @@ -0,0 +1,7 @@ +module PunditHelpers + def stub_policy(current_user, record, method_and_return_value) + policy_class = Pundit::PolicyFinder.new(record).policy + policy = stub_everything("policy", method_and_return_value).responds_like_instance_of(policy_class) + policy_class.stubs(:new).with(current_user, record).returns(policy) + end +end diff --git a/test/test_helper.rb b/test/test_helper.rb index 85f39aa11..ae577cf55 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -27,10 +27,12 @@ class ActiveSupport::TestCase WebMock.disable_net_connect!(allow_localhost: true) require "support/confirmation_token_helpers" +require "support/pundit_helpers" class ActionController::TestCase include Devise::Test::ControllerHelpers include ConfirmationTokenHelpers + include PunditHelpers def sign_in(user, passed_mfa: true) warden.stubs(authenticate!: user)