diff --git a/app/controllers/authorisations_controller.rb b/app/controllers/authorisations_controller.rb index d86b79648..8bb8ea829 100644 --- a/app/controllers/authorisations_controller.rb +++ b/app/controllers/authorisations_controller.rb @@ -17,7 +17,7 @@ def create authorisation.application_id = params[:doorkeeper_access_token][:application_id] if authorisation.save - @api_user.grant_application_permission(authorisation.application, "signin") + @api_user.grant_application_signin_permission(authorisation.application) EventLog.record_event(@api_user, EventLog::ACCESS_TOKEN_GENERATED, initiator: current_user, application: authorisation.application, ip_address: user_ip_address) flash[:authorisation] = { application_name: authorisation.application.name, token: authorisation.token } else diff --git a/app/models/doorkeeper/application.rb b/app/models/doorkeeper/application.rb index 2b69fc913..fcebfcebb 100644 --- a/app/models/doorkeeper/application.rb +++ b/app/models/doorkeeper/application.rb @@ -11,13 +11,13 @@ class ::Doorkeeper::Application < ActiveRecord::Base lambda { |user| joins(supported_permissions: :user_application_permissions) .where("user_application_permissions.user_id" => user.id) - .where("supported_permissions.name" => "signin") + .where("supported_permissions.name" => SupportedPermission::SIGNIN_NAME) .where(retired: false) } scope :with_signin_delegatable, lambda { joins(:supported_permissions) - .where(supported_permissions: { name: "signin", delegatable: true }) + .where(supported_permissions: { name: SupportedPermission::SIGNIN_NAME, delegatable: true }) } after_create :create_signin_supported_permission @@ -36,7 +36,7 @@ def supported_permission_strings(user = nil) end def signin_permission - supported_permissions.find_by(name: "signin") + supported_permissions.find_by(name: SupportedPermission::SIGNIN_NAME) end def sorted_supported_permissions_grantable_from_ui @@ -78,7 +78,7 @@ def substituted_uri(uri) end def create_signin_supported_permission - supported_permissions.create!(name: "signin", delegatable: true) + supported_permissions.create!(name: SupportedPermission::SIGNIN_NAME, delegatable: true) end def create_user_update_supported_permission diff --git a/app/models/supported_permission.rb b/app/models/supported_permission.rb index eea6c2e7d..188bbc3d4 100644 --- a/app/models/supported_permission.rb +++ b/app/models/supported_permission.rb @@ -1,4 +1,6 @@ class SupportedPermission < ApplicationRecord + SIGNIN_NAME = "signin".freeze + belongs_to :application, class_name: "Doorkeeper::Application" has_many :user_application_permissions, dependent: :destroy, inverse_of: :supported_permission @@ -11,7 +13,7 @@ class SupportedPermission < ApplicationRecord scope :default, -> { where(default: true) } def signin? - name.try(:downcase) == "signin" + name.try(:downcase) == SIGNIN_NAME end private @@ -19,7 +21,7 @@ def signin? def signin_permission_name_not_changed return if new_record? || !name_changed? - if name_change.first.casecmp("signin").zero? + if name_change.first.casecmp(SIGNIN_NAME).zero? errors.add(:name, "of permission #{name_change.first} can't be changed") end end diff --git a/app/models/user.rb b/app/models/user.rb index 92ec0be16..063bbcec9 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -167,6 +167,10 @@ def revoke_all_authorisations authorisations.where(revoked_at: nil).find_each(&:revoke) end + def grant_application_signin_permission(application) + grant_application_permission(application, SupportedPermission::SIGNIN_NAME) + end + def grant_application_permission(application, supported_permission_name) grant_application_permissions(application, [supported_permission_name]).first end diff --git a/app/views/shared/_user_permissions.html.erb b/app/views/shared/_user_permissions.html.erb index f63e2eb03..873e44cf5 100644 --- a/app/views/shared/_user_permissions.html.erb +++ b/app/views/shared/_user_permissions.html.erb @@ -31,19 +31,19 @@ # API Users will always have a "signin" permission for apps for which they have access token. # The hidden field ensures it is not lost. %> - <%= hidden_field_tag supported_permission_field_name, application.signin_permission.id, id: "#{supported_permission_field_prefix}_signin" %> + <%= hidden_field_tag supported_permission_field_name, application.signin_permission.id, id: "#{supported_permission_field_prefix}_#{SupportedPermission::SIGNIN_NAME}" %> <% else %> - <%= label_tag "#{supported_permission_field_prefix}_signin", "Has access to #{application.name}?", class: "rm" %> + <%= label_tag "#{supported_permission_field_prefix}_#{SupportedPermission::SIGNIN_NAME}", "Has access to #{application.name}?", class: "rm" %> <%= check_box_tag supported_permission_field_name, application.signin_permission.id, user_object.has_access_to?(application), - id: "#{supported_permission_field_prefix}_signin" %> + id: "#{supported_permission_field_prefix}_#{SupportedPermission::SIGNIN_NAME}" %> <% end %> <%= label_tag "#{supported_permission_field_prefix}_ids", "Permissions for #{application.name}", class: "rm" %> <% supported_permissions_options = application.supported_permissions.grantable_from_ui .inject({}) {|h, per| h.merge(per.name => per.id) } - supported_permissions_options.delete('signin') %> + supported_permissions_options.delete(SupportedPermission::SIGNIN_NAME) %> <%= select_tag supported_permission_field_name, options_for_select(supported_permissions_options, user_object.permission_ids_for(application) - [application.signin_permission.id]), diff --git a/app/views/users/_app_permissions.html.erb b/app/views/users/_app_permissions.html.erb index 9de20ac1f..ecc98413a 100644 --- a/app/views/users/_app_permissions.html.erb +++ b/app/views/users/_app_permissions.html.erb @@ -22,11 +22,11 @@ <% unless user_object.api_user? %> - <%= permission_names.include?('signin') ? 'Yes' : 'No' %> + <%= permission_names.include?(SupportedPermission::SIGNIN_NAME) ? 'Yes' : 'No' %> <% end %> - <%= permission_names.reject{|p| p == 'signin'}.to_sentence %> + <%= permission_names.reject{|p| p == SupportedPermission::SIGNIN_NAME}.to_sentence %> <% end %> diff --git a/lib/numbers/metrics.rb b/lib/numbers/metrics.rb index b5574b0a7..b352d15c8 100644 --- a/lib/numbers/metrics.rb +++ b/lib/numbers/metrics.rb @@ -65,7 +65,7 @@ def to_a private def has_signin_permissions?(permission) - permission.permissions.include?("signin") + permission.permissions.include?(SupportedPermission::SIGNIN_NAME) end def count_values(map) diff --git a/lib/signin_permission_granter.rb b/lib/signin_permission_granter.rb index b7ef5f3b1..bbc99cea3 100644 --- a/lib/signin_permission_granter.rb +++ b/lib/signin_permission_granter.rb @@ -5,7 +5,7 @@ def self.call(users:, application:) next if user.has_access_to?(application) puts "-- Adding signin permission for #{application.name}" - user.grant_application_permission(application, "signin") + user.grant_application_signin_permission(application) if application.supports_push_updates? PermissionUpdater.perform_later(user.uid, application.id) diff --git a/lib/sso_push_credential.rb b/lib/sso_push_credential.rb index b940cf300..5e6452e84 100644 --- a/lib/sso_push_credential.rb +++ b/lib/sso_push_credential.rb @@ -1,10 +1,11 @@ class SSOPushCredential - PERMISSIONS = %w[signin user_update_permission].freeze + PERMISSIONS = %w[user_update_permission].freeze USER_NAME = "Signon API Client (permission and suspension updater)".freeze USER_EMAIL = "signon+permissions@alphagov.co.uk".freeze class << self def credentials(application) + user.grant_application_signin_permission(application) user.grant_application_permissions(application, PERMISSIONS) user.authorisations diff --git a/lib/user_creator.rb b/lib/user_creator.rb index f8ecbfcc4..ccd12280a 100644 --- a/lib/user_creator.rb +++ b/lib/user_creator.rb @@ -35,7 +35,7 @@ def extract_applications_from_names def grant_requested_signin_permissions applications.each do |application| - user.grant_application_permission(application, "signin") + user.grant_application_signin_permission(application) end end diff --git a/test/controllers/authorisations_controller_test.rb b/test/controllers/authorisations_controller_test.rb index 645e3ffe7..d62d6b56a 100644 --- a/test/controllers/authorisations_controller_test.rb +++ b/test/controllers/authorisations_controller_test.rb @@ -58,11 +58,11 @@ class AuthorisationsControllerTest < ActionController::TestCase end should "not duplicate 'signin' permission for the authorised application if it already exists" do - @api_user.grant_application_permission(@application, "signin") + @api_user.grant_application_signin_permission(@application) post :create, params: { api_user_id: @api_user.id, doorkeeper_access_token: { application_id: @application.id } } - assert_equal %w[signin], @api_user.permissions_for(@application) + assert_equal [SupportedPermission::SIGNIN_NAME], @api_user.permissions_for(@application) end end end diff --git a/test/controllers/root_controller_test.rb b/test/controllers/root_controller_test.rb index 72f31aa0c..b8229e380 100644 --- a/test/controllers/root_controller_test.rb +++ b/test/controllers/root_controller_test.rb @@ -26,7 +26,7 @@ def setup test "Your applications should include apps you have permission to signin to" do exclusive_app = create(:application, name: "Exclusive app") everybody_app = create(:application, name: "Everybody app") - user = create(:user, with_permissions: { exclusive_app => [], everybody_app => %w[signin] }) + user = create(:user, with_permissions: { exclusive_app => [], everybody_app => [SupportedPermission::SIGNIN_NAME] }) sign_in user diff --git a/test/controllers/users_controller_test.rb b/test/controllers/users_controller_test.rb index 722aae526..82f45e744 100644 --- a/test/controllers/users_controller_test.rb +++ b/test/controllers/users_controller_test.rb @@ -141,7 +141,7 @@ def change_user_password(user_factory, new_password) should "fetching json profile with a valid oauth token should succeed" do user = create(:user) - user.grant_application_permission(@application, "signin") + user.grant_application_signin_permission(@application) token = create(:access_token, application: @application, resource_owner_id: user.id) @request.env["HTTP_AUTHORIZATION"] = "Bearer #{token.token}" @@ -156,7 +156,7 @@ def change_user_password(user_factory, new_password) # For now. Once gds-sso is updated everywhere, this will 401. user = create(:user) - user.grant_application_permission(@application, "signin") + user.grant_application_signin_permission(@application) token = create(:access_token, application: @application, resource_owner_id: user.id) @request.env["HTTP_AUTHORIZATION"] = "Bearer #{token.token}" @@ -200,7 +200,7 @@ def change_user_password(user_factory, new_password) @request.env["HTTP_AUTHORIZATION"] = "Bearer #{token.token}" get :show, params: { client_id: @application.uid, format: :json } json = JSON.parse(response.body) - assert_equal(%w[signin], json["user"]["permissions"]) + assert_equal([SupportedPermission::SIGNIN_NAME], json["user"]["permissions"]) end should "fetching json profile should include only permissions for the relevant app" do @@ -212,12 +212,12 @@ def change_user_password(user_factory, new_password) @request.env["HTTP_AUTHORIZATION"] = "Bearer #{token.token}" get :show, params: { client_id: @application.uid, format: :json } json = JSON.parse(response.body) - assert_equal(%w[signin], json["user"]["permissions"]) + assert_equal([SupportedPermission::SIGNIN_NAME], json["user"]["permissions"]) end should "fetching json profile should update last_synced_at for the relevant app" do user = create(:user) - user.grant_application_permission(@application, "signin") + user.grant_application_signin_permission(@application) token = create(:access_token, application: @application, resource_owner_id: user.id) @request.env["HTTP_AUTHORIZATION"] = "Bearer #{token.token}" @@ -469,13 +469,13 @@ def change_user_password(user_factory, new_password) end should "can give permissions to all applications" do - delegatable_app = create(:application, with_delegatable_supported_permissions: %w[signin]) - non_delegatable_app = create(:application, with_supported_permissions: %w[signin]) - delegatable_no_access_to_app = create(:application, with_delegatable_supported_permissions: %w[signin]) - non_delegatable_no_access_to_app = create(:application, with_supported_permissions: %w[signin]) + 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_permission(delegatable_app, "signin") - @user.grant_application_permission(non_delegatable_app, "signin") + @user.grant_application_signin_permission(delegatable_app) + @user.grant_application_signin_permission(non_delegatable_app) user = create(:user_in_organisation) @@ -507,10 +507,10 @@ def change_user_password(user_factory, new_password) 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: %w[signin]) - non_delegatable_app = create(:application, with_supported_permissions: %w[signin]) - delegatable_no_access_to_app = create(:application, with_delegatable_supported_permissions: %w[signin]) - non_delegatable_no_access_to_app = create(:application, with_supported_permissions: %w[signin]) + 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 = create(:organisation_admin_user, with_signin_permissions_for: [delegatable_app, non_delegatable_app]) @@ -533,10 +533,10 @@ def change_user_password(user_factory, new_password) end should "be able to see all permissions to applications for a user" do - delegatable_app = create(:application, with_delegatable_supported_permissions: %w[signin Editor]) - non_delegatable_app = create(:application, with_supported_permissions: ["signin", "GDS Admin"]) - delegatable_no_access_to_app = create(:application, with_delegatable_supported_permissions: ["signin", "GDS Editor"]) - non_delegatable_no_access_to_app = create(:application, with_supported_permissions: ["signin", "Import CSVs"]) + 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 = create(:organisation_admin_user, with_signin_permissions_for: [delegatable_app, non_delegatable_app]) @@ -546,9 +546,9 @@ def change_user_password(user_factory, new_password) :user_in_organisation, organisation: organisation_admin.organisation, with_permissions: { delegatable_app => %w[Editor], - non_delegatable_app => ["signin", "GDS Admin"], - delegatable_no_access_to_app => ["signin", "GDS Editor"], - non_delegatable_no_access_to_app => ["signin", "Import CSVs"] }, + 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 :edit, params: { id: user.id } @@ -590,10 +590,10 @@ def change_user_password(user_factory, new_password) 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: %w[signin]) - non_delegatable_app = create(:application, with_supported_permissions: %w[signin]) - delegatable_no_access_to_app = create(:application, with_delegatable_supported_permissions: %w[signin]) - non_delegatable_no_access_to_app = create(:application, with_supported_permissions: %w[signin]) + 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_org_admin = create(:super_organisation_admin_user, with_signin_permissions_for: [delegatable_app, non_delegatable_app]) @@ -616,10 +616,10 @@ def change_user_password(user_factory, new_password) end should "be able to see all permissions to applications for a user" do - delegatable_app = create(:application, with_delegatable_supported_permissions: %w[signin Editor]) - non_delegatable_app = create(:application, with_supported_permissions: ["signin", "GDS Admin"]) - delegatable_no_access_to_app = create(:application, with_delegatable_supported_permissions: ["signin", "GDS Editor"]) - non_delegatable_no_access_to_app = create(:application, with_supported_permissions: ["signin", "Import CSVs"]) + 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_org_admin = create(:super_organisation_admin_user, with_signin_permissions_for: [delegatable_app, non_delegatable_app]) @@ -629,9 +629,9 @@ def change_user_password(user_factory, new_password) :user_in_organisation, organisation: super_org_admin.organisation, with_permissions: { delegatable_app => %w[Editor], - non_delegatable_app => ["signin", "GDS Admin"], - delegatable_no_access_to_app => ["signin", "GDS Editor"], - non_delegatable_no_access_to_app => ["signin", "Import CSVs"] }, + 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 :edit, params: { id: user.id } @@ -660,10 +660,10 @@ def change_user_password(user_factory, new_password) context "superadmin" do should "not be able to see all permissions to applications for a user" do - delegatable_app = create(:application, with_delegatable_supported_permissions: %w[signin Editor]) - non_delegatable_app = create(:application, with_supported_permissions: ["signin", "GDS Admin"]) - delegatable_no_access_to_app = create(:application, with_delegatable_supported_permissions: ["signin", "GDS Editor"]) - non_delegatable_no_access_to_app = create(:application, with_supported_permissions: ["signin", "Import CSVs"]) + 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 = create(:superadmin_user, with_signin_permissions_for: [delegatable_app, non_delegatable_app]) @@ -673,9 +673,9 @@ def change_user_password(user_factory, new_password) :user_in_organisation, organisation: superadmin.organisation, with_permissions: { delegatable_app => %w[Editor], - non_delegatable_app => ["signin", "GDS Admin"], - delegatable_no_access_to_app => ["signin", "GDS Editor"], - non_delegatable_no_access_to_app => ["signin", "Import CSVs"] }, + 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 :edit, params: { id: user.id } @@ -842,7 +842,7 @@ def change_user_password(user_factory, new_password) end should "remove all applications access for a user" do - @another_user.grant_application_permission(@application, "signin") + @another_user.grant_application_signin_permission(@application) put :update, params: { id: @another_user.id, user: {} } diff --git a/test/factories/oauth_applications.rb b/test/factories/oauth_applications.rb index 1ff8dfa6e..2e1bbb89b 100644 --- a/test/factories/oauth_applications.rb +++ b/test/factories/oauth_applications.rb @@ -16,19 +16,19 @@ evaluator.with_supported_permissions.each do |permission_name| # we create signin in an after_create on application. # this line takes care of tests creating signin in order to look complete or modify delegatable on it. - app.signin_permission.update(delegatable: false) && next if permission_name == "signin" + app.signin_permission.update(delegatable: false) && next if permission_name == SupportedPermission::SIGNIN_NAME create(:supported_permission, application_id: app.id, name: permission_name) end evaluator.with_supported_permissions_not_grantable_from_ui.each do |permission_name| - next if permission_name == "signin" + next if permission_name == SupportedPermission::SIGNIN_NAME create(:supported_permission, application_id: app.id, name: permission_name, grantable_from_ui: false) end evaluator.with_delegatable_supported_permissions.each do |permission_name| - next if permission_name == "signin" + next if permission_name == SupportedPermission::SIGNIN_NAME create(:delegatable_supported_permission, application_id: app.id, name: permission_name) end diff --git a/test/factories/users.rb b/test/factories/users.rb index 8824fd223..3f9f2a5ea 100644 --- a/test/factories/users.rb +++ b/test/factories/users.rb @@ -29,7 +29,7 @@ else app_or_name end - user.grant_application_permission(app, "signin") + user.grant_application_signin_permission(app) end end end diff --git a/test/helpers/users_helper_test.rb b/test/helpers/users_helper_test.rb index 5d37825c7..7036071ed 100644 --- a/test/helpers/users_helper_test.rb +++ b/test/helpers/users_helper_test.rb @@ -4,7 +4,7 @@ class UsersHelperTest < ActionView::TestCase test "sync_needed? should work with user permissions not synced yet" do application = create(:application) user = create(:user) - user.grant_application_permission(application, "signin") + user.grant_application_signin_permission(application) assert_nothing_raised { sync_needed?(user.application_permissions) } end diff --git a/test/integration/api_authentication_test.rb b/test/integration/api_authentication_test.rb index 7521d2927..8526da4eb 100644 --- a/test/integration/api_authentication_test.rb +++ b/test/integration/api_authentication_test.rb @@ -11,7 +11,7 @@ def access_user_endpoint(token = nil, params = {}) setup do @app1 = create(:application, name: "MyApp", with_supported_permissions: %w[write]) - @user = create(:user, with_permissions: { @app1 => %w[signin write] }) + @user = create(:user, with_permissions: { @app1 => [SupportedPermission::SIGNIN_NAME, "write"] }) @user.authorisations.create!(application_id: @app1.id) end diff --git a/test/integration/batch_inviting_users_test.rb b/test/integration/batch_inviting_users_test.rb index 6c7a4fbc9..fea5ca3ea 100644 --- a/test/integration/batch_inviting_users_test.rb +++ b/test/integration/batch_inviting_users_test.rb @@ -48,7 +48,7 @@ class BatchInvitingUsersTest < ActionDispatch::IntegrationTest context "for organisation admins" do setup do @user = create(:organisation_admin_user) - @user.grant_application_permission(@application, %w[signin]) + @user.grant_application_signin_permission(@application) end should "not allow batch inviting users" do @@ -63,7 +63,7 @@ class BatchInvitingUsersTest < ActionDispatch::IntegrationTest context "for super organisation admins" do setup do @user = create(:super_organisation_admin_user) - @user.grant_application_permission(@application, %w[signin]) + @user.grant_application_signin_permission(@application) end should "not allow batch inviting users" do @@ -77,7 +77,7 @@ class BatchInvitingUsersTest < ActionDispatch::IntegrationTest should "ensure that batch invited users get default permissions even when not checked in UI" do create(:supported_permission, application: @application, name: "reader", default: true) - support_app = create(:application, name: "support", with_supported_permissions: %w[signin]) + support_app = create(:application, name: "support", with_supported_permissions: [SupportedPermission::SIGNIN_NAME]) support_app.signin_permission.update!(default: true) user = create(:admin_user) diff --git a/test/integration/bulk_granting_permisions_test.rb b/test/integration/bulk_granting_permisions_test.rb index a764e5c5b..bb6ed85e0 100644 --- a/test/integration/bulk_granting_permisions_test.rb +++ b/test/integration/bulk_granting_permisions_test.rb @@ -9,7 +9,7 @@ class BulkGrantingPermissionsTest < ActionDispatch::IntegrationTest @admins = create_list(:admin_user, 2) @superadmins = create_list(:superadmin_user, 2) - @application = create(:application, with_supported_permissions: %w[signin]) + @application = create(:application, with_supported_permissions: [SupportedPermission::SIGNIN_NAME]) end should "superadmin user can grant multiple permissions to all users in one go" do @@ -74,7 +74,7 @@ def perform_bulk_grant_as_user(acting_user, application) [acting_user, @users, @org_admins, @admins, @superadmins].flatten.each do |user| user.reload - assert_equal %w[signin], user.permissions_for(application) + assert_equal [SupportedPermission::SIGNIN_NAME], user.permissions_for(application) end end end diff --git a/test/integration/granting_permissions_test.rb b/test/integration/granting_permissions_test.rb index 95e2dabb1..465064e44 100644 --- a/test/integration/granting_permissions_test.rb +++ b/test/integration/granting_permissions_test.rb @@ -99,8 +99,8 @@ class GrantingPermissionsTest < ActionDispatch::IntegrationTest end should "support granting signin permissions to delegatable apps that the super organisation admin has access to" do - app = create(:application, name: "MyApp", with_delegatable_supported_permissions: %w[signin]) - @super_org_admin.grant_application_permission(app, "signin") + app = create(:application, name: "MyApp", with_delegatable_supported_permissions: [SupportedPermission::SIGNIN_NAME]) + @super_org_admin.grant_application_signin_permission(app) visit edit_user_path(@user) check "Has access to MyApp?" @@ -113,14 +113,14 @@ class GrantingPermissionsTest < ActionDispatch::IntegrationTest app = create(:application, name: "MyApp") signin_permission = app.signin_permission signin_permission.update!(delegatable: false) - @super_org_admin.grant_application_permission(app, "signin") + @super_org_admin.grant_application_signin_permission(app) visit edit_user_path(@user) assert page.has_no_field? "Has access to MyApp?" end should "not support granting signin permissions to apps that the super organisation admin doesn't have access to" do - create(:application, name: "MyApp", with_delegatable_supported_permissions: %w[signin]) + create(:application, name: "MyApp", with_delegatable_supported_permissions: [SupportedPermission::SIGNIN_NAME]) visit edit_user_path(@user) assert page.has_no_field? "Has access to MyApp?" @@ -128,7 +128,7 @@ class GrantingPermissionsTest < ActionDispatch::IntegrationTest should "not remove permissions the user has that the super organisation admin does not have" do app = create(:application, name: "MyApp") - @user.grant_application_permission(app, "signin") + @user.grant_application_signin_permission(app) visit edit_user_path(@user) click_button "Update User" @@ -139,8 +139,8 @@ class GrantingPermissionsTest < ActionDispatch::IntegrationTest 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_permission(app, "signin") - @user.grant_application_permission(app, "signin") + @super_org_admin.grant_application_signin_permission(app) + @user.grant_application_signin_permission(app) visit edit_user_path(@user) click_button "Update User" @@ -154,7 +154,7 @@ class GrantingPermissionsTest < ActionDispatch::IntegrationTest name: "MyApp", with_supported_permissions: %w[pre-existing adding never], ) - @super_org_admin.grant_application_permission(app, "signin") + @super_org_admin.grant_application_signin_permission(app) @user.grant_application_permission(app, "pre-existing") visit edit_user_path(@user) @@ -168,7 +168,7 @@ class GrantingPermissionsTest < ActionDispatch::IntegrationTest should "not be able to grant permissions that are not grantable_from_ui" do app = create(:application, name: "MyApp", with_supported_permissions_not_grantable_from_ui: %w[user_update_permission]) - @super_org_admin.grant_application_permission(app, "signin") + @super_org_admin.grant_application_signin_permission(app) visit edit_user_path(@user) assert page.has_no_select?("Permissions for MyApp", options: %w[user_update_permission]) @@ -185,8 +185,8 @@ class GrantingPermissionsTest < ActionDispatch::IntegrationTest end should "support granting signin permissions to delegatable apps that the organisation admin has access to" do - app = create(:application, name: "MyApp", with_delegatable_supported_permissions: %w[signin]) - @organisation_admin.grant_application_permission(app, "signin") + app = create(:application, name: "MyApp", with_delegatable_supported_permissions: [SupportedPermission::SIGNIN_NAME]) + @organisation_admin.grant_application_signin_permission(app) visit edit_user_path(@user) check "Has access to MyApp?" @@ -199,14 +199,14 @@ class GrantingPermissionsTest < ActionDispatch::IntegrationTest app = create(:application, name: "MyApp") signin_permission = app.signin_permission signin_permission.update!(delegatable: false) - @organisation_admin.grant_application_permission(app, "signin") + @organisation_admin.grant_application_signin_permission(app) visit edit_user_path(@user) assert page.has_no_field? "Has access to MyApp?" end should "not support granting signin permissions to apps that the organisation admin doesn't have access to" do - create(:application, name: "MyApp", with_delegatable_supported_permissions: %w[signin]) + create(:application, name: "MyApp", with_delegatable_supported_permissions: [SupportedPermission::SIGNIN_NAME]) visit edit_user_path(@user) assert page.has_no_field? "Has access to MyApp?" @@ -214,7 +214,7 @@ class GrantingPermissionsTest < ActionDispatch::IntegrationTest should "not remove permissions the user has that the organisation admin does not have" do app = create(:application, name: "MyApp") - @user.grant_application_permission(app, "signin") + @user.grant_application_signin_permission(app) visit edit_user_path(@user) click_button "Update User" @@ -225,8 +225,8 @@ class GrantingPermissionsTest < ActionDispatch::IntegrationTest 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_permission(app, "signin") - @user.grant_application_permission(app, "signin") + @organisation_admin.grant_application_signin_permission(app) + @user.grant_application_signin_permission(app) visit edit_user_path(@user) click_button "Update User" @@ -240,7 +240,7 @@ class GrantingPermissionsTest < ActionDispatch::IntegrationTest name: "MyApp", with_supported_permissions: %w[pre-existing adding never], ) - @organisation_admin.grant_application_permission(app, "signin") + @organisation_admin.grant_application_signin_permission(app) @user.grant_application_permission(app, "pre-existing") visit edit_user_path(@user) @@ -254,7 +254,7 @@ class GrantingPermissionsTest < ActionDispatch::IntegrationTest should "not be able to grant permissions that are not grantable_from_ui" do app = create(:application, name: "MyApp", with_supported_permissions_not_grantable_from_ui: %w[user_update_permission]) - @organisation_admin.grant_application_permission(app, "signin") + @organisation_admin.grant_application_signin_permission(app) visit edit_user_path(@user) assert page.has_no_select?("Permissions for MyApp", options: %w[user_update_permission]) diff --git a/test/integration/manage_api_users_test.rb b/test/integration/manage_api_users_test.rb index bcaa65890..3f7524553 100644 --- a/test/integration/manage_api_users_test.rb +++ b/test/integration/manage_api_users_test.rb @@ -34,7 +34,7 @@ class ManageApiUsersTest < ActionDispatch::IntegrationTest end should "be able to authorise application access and manage permissions for an API user which should get recorded in event log" do - create(:application, name: "Whitehall", with_supported_permissions: ["Managing Editor", "signin"]) + create(:application, name: "Whitehall", with_supported_permissions: ["Managing Editor", SupportedPermission::SIGNIN_NAME]) click_link @api_user.name click_link "Add application token" diff --git a/test/integration/superadmin_application_users_test.rb b/test/integration/superadmin_application_users_test.rb index 448fb1106..19b9f1144 100644 --- a/test/integration/superadmin_application_users_test.rb +++ b/test/integration/superadmin_application_users_test.rb @@ -16,7 +16,7 @@ class SuperAdminApplicationUsersTest < ActionDispatch::IntegrationTest # Create a user that's authorized to use our app user = create(:user, name: "My Test User") - user.grant_application_permission(@application, "signin") + user.grant_application_signin_permission(@application) click_link @application.name diff --git a/test/jobs/permission_updater_test.rb b/test/jobs/permission_updater_test.rb index f15cdd180..8b4c34247 100644 --- a/test/jobs/permission_updater_test.rb +++ b/test/jobs/permission_updater_test.rb @@ -9,7 +9,7 @@ def users_url(application) setup do @user = create(:user) @application = create(:application, redirect_uri: "https://app.com/callback", supports_push_updates: true) - @signin_permission = @user.grant_application_permission(@application, "signin") + @signin_permission = @user.grant_application_signin_permission(@application) @other_permission = @user.grant_application_permission(@application, "user_update_permission") end diff --git a/test/lib/numbers/numbers_csv_test.rb b/test/lib/numbers/numbers_csv_test.rb index 934100606..4999baad0 100644 --- a/test/lib/numbers/numbers_csv_test.rb +++ b/test/lib/numbers/numbers_csv_test.rb @@ -49,7 +49,7 @@ def numbers_csv test "csv contains counts by application access" do app = create(:application, name: "WhiteCloud") - User.first.grant_application_permission(app, "signin") + User.first.grant_application_signin_permission(app) Numbers::NumbersCsv.generate @@ -110,10 +110,10 @@ def numbers_csv another_app = create(:application, name: "Another app") users = create_list(:user, 4) - users[0].grant_application_permission(@licensing, "signin") + users[0].grant_application_signin_permission(@licensing) - users[1].grant_application_permission(@licensing, "signin") - users[1].grant_application_permission(another_app, "signin") + users[1].grant_application_signin_permission(@licensing) + users[1].grant_application_signin_permission(another_app) users[2].grant_application_permission(@licensing, "another perm") diff --git a/test/lib/signin_permission_granter_test.rb b/test/lib/signin_permission_granter_test.rb index 954f2f0ff..e647b6eb2 100644 --- a/test/lib/signin_permission_granter_test.rb +++ b/test/lib/signin_permission_granter_test.rb @@ -11,8 +11,8 @@ class SigninPermissionGranterTest < ActiveSupport::TestCase first_user = create(:user) second_user = create(:user) user_with_permission = create(:user, with_signin_permissions_for: [@application_supporting_push_updates]) - first_user.expects(:grant_application_permission).with(@application_supporting_push_updates, "signin").once - second_user.expects(:grant_application_permission).with(@application_supporting_push_updates, "signin").once + first_user.expects(:grant_application_signin_permission).with(@application_supporting_push_updates).once + second_user.expects(:grant_application_signin_permission).with(@application_supporting_push_updates).once PermissionUpdater.expects(:perform_later).with(anything, @application_supporting_push_updates.id).twice user_with_permission.expects(:grant_application_permission).never @@ -23,7 +23,7 @@ class SigninPermissionGranterTest < ActiveSupport::TestCase should "not send a push update to an application with updated users if the application does not support it" do user = create(:user) - user.expects(:grant_application_permission).with(@application_not_supporting_push_updates, "signin").once + user.expects(:grant_application_signin_permission).with(@application_not_supporting_push_updates).once PermissionUpdater.expects(:perform_later).with(user.id, @application_not_supporting_push_updates.id).never SigninPermissionGranter.call(users: [user], application: @application_not_supporting_push_updates) diff --git a/test/lib/sso_push_credential_test.rb b/test/lib/sso_push_credential_test.rb index 12c9f4000..869ccdec4 100644 --- a/test/lib/sso_push_credential_test.rb +++ b/test/lib/sso_push_credential_test.rb @@ -23,17 +23,18 @@ class SSOPushCredentialTest < ActiveSupport::TestCase SSOPushCredential.credentials(@application) assert_equal 2, @user.application_permissions.count - assert_same_elements %w[signin user_update_permission], @user.permissions_for(@application) + assert_same_elements [SupportedPermission::SIGNIN_NAME, "user_update_permission"], @user.permissions_for(@application) end should "not create new application permissions if both already exist" do - @user.grant_application_permissions(@application, %w[user_update_permission signin]) + @user.grant_application_signin_permission(@application) + @user.grant_application_permissions(@application, %w[user_update_permission]) assert_equal 2, @user.application_permissions.count SSOPushCredential.credentials(@application) assert_equal 2, @user.application_permissions.count - assert_same_elements %w[user_update_permission signin], @user.permissions_for(@application) + assert_same_elements ["user_update_permission", SupportedPermission::SIGNIN_NAME], @user.permissions_for(@application) end end diff --git a/test/lib/user_creator_test.rb b/test/lib/user_creator_test.rb index 7287da38e..f710b41a5 100644 --- a/test/lib/user_creator_test.rb +++ b/test/lib/user_creator_test.rb @@ -2,7 +2,7 @@ class UserCreatorTest < ActiveSupport::TestCase test "it creates a new user with the supplied name and email" do - FactoryBot.create(:application, name: "app-o-tron", with_supported_permissions: %w[signin]) + FactoryBot.create(:application, name: "app-o-tron", with_supported_permissions: [SupportedPermission::SIGNIN_NAME]) user_creator = UserCreator.new("Alicia", "alicia@example.com", "app-o-tron") user_creator.create_user! @@ -13,7 +13,7 @@ class UserCreatorTest < ActiveSupport::TestCase end test "invites the new user, so they must validate their email before they can signin" do - FactoryBot.create(:application, name: "app-o-tron", with_supported_permissions: %w[signin]) + FactoryBot.create(:application, name: "app-o-tron", with_supported_permissions: [SupportedPermission::SIGNIN_NAME]) user_creator = UserCreator.new("Alicia", "alicia@example.com", "app-o-tron") user_creator.create_user! @@ -22,8 +22,8 @@ class UserCreatorTest < ActiveSupport::TestCase end test 'it grants "signin" permission to each application supplied' do - app_o_tron = FactoryBot.create(:application, name: "app-o-tron", with_supported_permissions: %w[signin]) - app_erator = FactoryBot.create(:application, name: "app-erator", with_supported_permissions: %w[signin]) + app_o_tron = FactoryBot.create(:application, name: "app-o-tron", with_supported_permissions: [SupportedPermission::SIGNIN_NAME]) + app_erator = FactoryBot.create(:application, name: "app-erator", with_supported_permissions: [SupportedPermission::SIGNIN_NAME]) user_creator = UserCreator.new("Alicia", "alicia@example.com", "app-o-tron,app-erator") user_creator.create_user! @@ -33,8 +33,8 @@ class UserCreatorTest < ActiveSupport::TestCase end test "it grants all default permissions, even if not signin" do - app_o_tron = FactoryBot.create(:application, name: "app-o-tron", with_supported_permissions: %w[signin]) - app_erator = FactoryBot.create(:application, name: "app-erator", with_supported_permissions: %w[signin fall]) + app_o_tron = FactoryBot.create(:application, name: "app-o-tron", with_supported_permissions: [SupportedPermission::SIGNIN_NAME]) + app_erator = FactoryBot.create(:application, name: "app-erator", with_supported_permissions: [SupportedPermission::SIGNIN_NAME, "fall"]) create(:supported_permission, application: app_o_tron, name: "bounce", default: true) app_erator.signin_permission.update!(default: true) user_creator = UserCreator.new("Alicia", "alicia@example.com", "") @@ -43,6 +43,6 @@ class UserCreatorTest < ActiveSupport::TestCase created_user = user_creator.user assert_equal %w[bounce], created_user.permissions_for(app_o_tron) - assert_equal %w[signin], created_user.permissions_for(app_erator) + assert_equal [SupportedPermission::SIGNIN_NAME], created_user.permissions_for(app_erator) end end diff --git a/test/lib/user_permission_migrator_test.rb b/test/lib/user_permission_migrator_test.rb index f75b27350..f976e4b76 100644 --- a/test/lib/user_permission_migrator_test.rb +++ b/test/lib/user_permission_migrator_test.rb @@ -15,11 +15,11 @@ class UserPermissionMigrationTest < ActiveSupport::TestCase create(:supported_permission, application: @unrelated_application, name: "gds_editor") create(:supported_permission, application: @unrelated_application, name: "editor") - @gds_editor = create(:user, with_permissions: { "Specialist Publisher" => %w[editor gds_editor signin] }) - @editor = create(:user, with_permissions: { "Specialist Publisher" => %w[editor signin] }) - @writer = create(:user, with_permissions: { "Specialist Publisher" => %w[signin] }) + @gds_editor = create(:user, with_permissions: { "Specialist Publisher" => ["editor", "gds_editor", SupportedPermission::SIGNIN_NAME] }) + @editor = create(:user, with_permissions: { "Specialist Publisher" => ["editor", SupportedPermission::SIGNIN_NAME] }) + @writer = create(:user, with_permissions: { "Specialist Publisher" => [SupportedPermission::SIGNIN_NAME] }) @user_without_access = create(:user) - @user_with_unrelated_access = create(:user, with_permissions: { "unrelated application" => %w[editor gds_editor signin] }) + @user_with_unrelated_access = create(:user, with_permissions: { "unrelated application" => ["editor", "gds_editor", SupportedPermission::SIGNIN_NAME] }) end should "copy permissions over for all users of an application to another application" do @@ -28,15 +28,15 @@ class UserPermissionMigrationTest < ActiveSupport::TestCase target: "Manuals Publisher", ) - assert_equal %w[editor gds_editor signin], @gds_editor.permissions_for(@manuals_publisher) - assert_equal %w[editor signin], @editor.permissions_for(@manuals_publisher) - assert_equal %w[signin], @writer.permissions_for(@manuals_publisher) + assert_equal ["editor", "gds_editor", SupportedPermission::SIGNIN_NAME], @gds_editor.permissions_for(@manuals_publisher) + assert_equal ["editor", SupportedPermission::SIGNIN_NAME], @editor.permissions_for(@manuals_publisher) + assert_equal [SupportedPermission::SIGNIN_NAME], @writer.permissions_for(@manuals_publisher) assert_equal %w[], @user_without_access.permissions_for(@manuals_publisher) assert_equal %w[], @user_with_unrelated_access.permissions_for(@manuals_publisher) - assert_equal %w[editor gds_editor signin], @gds_editor.permissions_for(@specialist_publisher) - assert_equal %w[editor signin], @editor.permissions_for(@specialist_publisher) - assert_equal %w[signin], @writer.permissions_for(@specialist_publisher) + assert_equal ["editor", "gds_editor", SupportedPermission::SIGNIN_NAME], @gds_editor.permissions_for(@specialist_publisher) + assert_equal ["editor", SupportedPermission::SIGNIN_NAME], @editor.permissions_for(@specialist_publisher) + assert_equal [SupportedPermission::SIGNIN_NAME], @writer.permissions_for(@specialist_publisher) assert_equal %w[], @user_without_access.permissions_for(@specialist_publisher) assert_equal %w[], @user_with_unrelated_access.permissions_for(@specialist_publisher) end diff --git a/test/lib/user_permissions_exporter_test.rb b/test/lib/user_permissions_exporter_test.rb index c230430f4..9aa85aa3a 100644 --- a/test/lib/user_permissions_exporter_test.rb +++ b/test/lib/user_permissions_exporter_test.rb @@ -23,9 +23,12 @@ def setup def test_export_one_application foo_app = create(:application, name: "Foo", with_supported_permissions: %w[administer add_vinegar do_some_stuff cook]) - @bill.grant_application_permissions(foo_app, %w[signin cook]) - @anne.grant_application_permissions(foo_app, %w[signin administer add_vinegar]) - @mary.grant_application_permissions(foo_app, %w[signin do_some_stuff]) + @bill.grant_application_signin_permission(foo_app) + @bill.grant_application_permissions(foo_app, %w[cook]) + @anne.grant_application_signin_permission(foo_app) + @anne.grant_application_permissions(foo_app, %w[administer add_vinegar]) + @mary.grant_application_signin_permission(foo_app) + @mary.grant_application_permissions(foo_app, %w[do_some_stuff]) UserPermissionsExporter.new(@tmpfile.path).export(%w[Foo]) @@ -42,12 +45,17 @@ def test_export_multiple_applications bar_app = create(:application, name: "Bar", with_supported_permissions: %w[administer]) baz_app = create(:application, name: "Baz") - @bill.grant_application_permissions(foo_app, %w[signin cook]) + @bill.grant_application_signin_permission(foo_app) + @bill.grant_application_permissions(foo_app, %w[cook]) @bill.grant_application_permissions(baz_app, []) - @anne.grant_application_permissions(foo_app, %w[signin administer add_vinegar]) - @anne.grant_application_permissions(bar_app, %w[signin administer]) - @mary.grant_application_permissions(foo_app, %w[signin do_some_stuff]) - @mary.grant_application_permissions(bar_app, %w[signin administer]) + @anne.grant_application_signin_permission(foo_app) + @anne.grant_application_permissions(foo_app, %w[administer add_vinegar]) + @anne.grant_application_signin_permission(bar_app) + @anne.grant_application_permissions(bar_app, %w[administer]) + @mary.grant_application_signin_permission(foo_app) + @mary.grant_application_permissions(foo_app, %w[do_some_stuff]) + @mary.grant_application_signin_permission(bar_app) + @mary.grant_application_permissions(bar_app, %w[administer]) UserPermissionsExporter.new(@tmpfile.path).export(%w[Foo Bar Baz]) diff --git a/test/models/batch_invitation_test.rb b/test/models/batch_invitation_test.rb index 6bd2ba8cf..08a4dfa2f 100644 --- a/test/models/batch_invitation_test.rb +++ b/test/models/batch_invitation_test.rb @@ -33,7 +33,7 @@ class BatchInvitationTest < ActiveSupport::TestCase user = User.find_by(email: "a@m.com") assert_not_nil user assert_equal "A", user.name - assert_equal %w[signin], user.permissions_for(@app) + assert_equal [SupportedPermission::SIGNIN_NAME], user.permissions_for(@app) end should "trigger an invitation email" do @@ -70,14 +70,15 @@ class BatchInvitationTest < ActiveSupport::TestCase app = create(:application) another_app = create(:application) create(:supported_permission, application_id: another_app.id, name: "foo") - @user.grant_application_permissions(another_app, %w[signin foo]) + @user.grant_application_signin_permission(another_app) + @user.grant_application_permissions(another_app, %w[foo]) @bi.supported_permission_ids = [another_app.signin_permission.id] @bi.save! @bi.perform assert_empty @user.permissions_for(app) - assert_same_elements %w[signin foo], @user.permissions_for(another_app) + assert_same_elements [SupportedPermission::SIGNIN_NAME, "foo"], @user.permissions_for(another_app) end end diff --git a/test/models/bulk_grant_permission_set_test.rb b/test/models/bulk_grant_permission_set_test.rb index 2b3141b6c..4496f8ba3 100644 --- a/test/models/bulk_grant_permission_set_test.rb +++ b/test/models/bulk_grant_permission_set_test.rb @@ -56,10 +56,10 @@ class BulkGrantPermissionSetTest < ActiveSupport::TestCase @permission_set.perform - assert_equal %w[signin], user.permissions_for(@app) - assert_equal %w[signin], admin_user.permissions_for(@app) - assert_equal %w[signin], superadmin_user.permissions_for(@app) - assert_equal %w[signin], orgadmin_user.permissions_for(@app) + assert_equal [SupportedPermission::SIGNIN_NAME], user.permissions_for(@app) + assert_equal [SupportedPermission::SIGNIN_NAME], admin_user.permissions_for(@app) + assert_equal [SupportedPermission::SIGNIN_NAME], superadmin_user.permissions_for(@app) + assert_equal [SupportedPermission::SIGNIN_NAME], orgadmin_user.permissions_for(@app) end should "record the total number of users to be processed" do @@ -86,11 +86,11 @@ class BulkGrantPermissionSetTest < ActiveSupport::TestCase end should "not fail if a user already has one of the supplied permissions" do - user = create(:user, with_permissions: { @app => %w[signin] }) + user = create(:user, with_permissions: { @app => [SupportedPermission::SIGNIN_NAME] }) @permission_set.perform - assert_equal %w[signin], user.permissions_for(@app) + assert_equal [SupportedPermission::SIGNIN_NAME], user.permissions_for(@app) end should "not remove permissions a user has that are not part of the supplied ones for the bulk grant set" do @@ -101,7 +101,7 @@ class BulkGrantPermissionSetTest < ActiveSupport::TestCase @permission_set.perform assert_equal %w[editor], user.permissions_for(other_app) - assert_equal %w[signin admin].sort, user.permissions_for(@app).sort + assert_equal [SupportedPermission::SIGNIN_NAME, "admin"].sort, user.permissions_for(@app).sort end should "mark it as failed if there is an error during processing and pass the error on for the worker to record the error details" do @@ -125,7 +125,7 @@ class BulkGrantPermissionSetTest < ActiveSupport::TestCase context "recording events against users" do setup do - @app2 = create(:application, with_supported_permissions: %w[signin editor]) + @app2 = create(:application, with_supported_permissions: [SupportedPermission::SIGNIN_NAME, "editor"]) @permission_set.supported_permissions += @app2.supported_permissions @permission_set.save! end @@ -161,7 +161,7 @@ class BulkGrantPermissionSetTest < ActiveSupport::TestCase end should "not create an event log for the app if the user already has all the permissions we are trying to grant for that app" do - user = create(:user, with_permissions: { @app => %w[signin] }) + user = create(:user, with_permissions: { @app => [SupportedPermission::SIGNIN_NAME] }) user.event_logs.destroy_all @permission_set.perform @@ -173,7 +173,7 @@ class BulkGrantPermissionSetTest < ActiveSupport::TestCase end should "not create any event logs if the user already has all the permissions we are trying to grant" do - user = create(:user, with_permissions: { @app => %w[signin], @app2 => %w[signin editor] }) + user = create(:user, with_permissions: { @app => [SupportedPermission::SIGNIN_NAME], @app2 => [SupportedPermission::SIGNIN_NAME, "editor"] }) user.event_logs.destroy_all @permission_set.perform diff --git a/test/models/doorkeeper_application_test.rb b/test/models/doorkeeper_application_test.rb index df688858b..01d54f87b 100644 --- a/test/models/doorkeeper_application_test.rb +++ b/test/models/doorkeeper_application_test.rb @@ -29,7 +29,7 @@ class ::Doorkeeper::ApplicationTest < ActiveSupport::TestCase user = create(:user) app = create(:application, with_supported_permissions: %w[write]) - assert_equal %w[signin write], app.supported_permission_strings(user) + assert_equal [SupportedPermission::SIGNIN_NAME, "write"], app.supported_permission_strings(user) end should "only show permissions that super organisation admins themselves have" do @@ -119,7 +119,7 @@ class ::Doorkeeper::ApplicationTest < ActiveSupport::TestCase should "return applications that the user can signin into" do user = create(:user) application = create(:application) - user.grant_application_permission(application, "signin") + user.grant_application_signin_permission(application) assert_includes Doorkeeper::Application.can_signin(user), application end @@ -127,7 +127,7 @@ class ::Doorkeeper::ApplicationTest < ActiveSupport::TestCase should "not return applications that are retired" do user = create(:user) application = create(:application, retired: true) - user.grant_application_permission(application, "signin") + user.grant_application_signin_permission(application) assert_empty Doorkeeper::Application.can_signin(user) end @@ -140,13 +140,13 @@ class ::Doorkeeper::ApplicationTest < ActiveSupport::TestCase end should "return applications that support delegation of signin permission" do - application = create(:application, with_delegatable_supported_permissions: %w[signin]) + application = create(:application, with_delegatable_supported_permissions: [SupportedPermission::SIGNIN_NAME]) assert_includes Doorkeeper::Application.with_signin_delegatable, application end should "not return applications that don't support delegation of signin permission" do - create(:application, with_supported_permissions: %w[signin]) + create(:application, with_supported_permissions: [SupportedPermission::SIGNIN_NAME]) assert_empty Doorkeeper::Application.with_signin_delegatable end diff --git a/test/models/user_o_auth_presenter_test.rb b/test/models/user_o_auth_presenter_test.rb index 515d0b657..db722ac40 100644 --- a/test/models/user_o_auth_presenter_test.rb +++ b/test/models/user_o_auth_presenter_test.rb @@ -8,7 +8,8 @@ class UserOAuthPresenterTest < ActiveSupport::TestCase should "generate JSON" do user = create(:user) justice_league = create(:organisation, slug: "justice-league") - user.grant_application_permissions(@application, %w[signin managing_editor]) + user.grant_application_signin_permission(@application) + user.grant_application_permissions(@application, %w[managing_editor]) user.organisation = justice_league expected_user_attributes = { @@ -21,7 +22,7 @@ class UserOAuthPresenterTest < ActiveSupport::TestCase } user_representation = UserOAuthPresenter.new(user, @application).as_hash[:user] - assert_same_elements %w[signin managing_editor], user_representation.delete(:permissions) + assert_same_elements [SupportedPermission::SIGNIN_NAME, "managing_editor"], user_representation.delete(:permissions) assert_equal expected_user_attributes, user_representation end @@ -34,7 +35,8 @@ class UserOAuthPresenterTest < ActiveSupport::TestCase should "mark suspended users disabled" do suspended_user = create(:suspended_user) - suspended_user.grant_application_permissions(@application, %w[signin managing_editor]) + suspended_user.grant_application_signin_permission(@application) + suspended_user.grant_application_permissions(@application, %w[managing_editor]) presenter = UserOAuthPresenter.new(suspended_user, @application) assert presenter.as_hash[:user][:disabled] @@ -42,7 +44,8 @@ class UserOAuthPresenterTest < ActiveSupport::TestCase should "exclude permissions if user is suspended" do suspended_user = create(:suspended_user) - suspended_user.grant_application_permissions(@application, %w[signin managing_editor]) + suspended_user.grant_application_signin_permission(@application) + suspended_user.grant_application_permissions(@application, %w[managing_editor]) presenter = UserOAuthPresenter.new(suspended_user, @application) assert_empty presenter.as_hash[:user][:permissions] diff --git a/test/models/user_test.rb b/test/models/user_test.rb index acf6441e3..8670684f3 100644 --- a/test/models/user_test.rb +++ b/test/models/user_test.rb @@ -549,6 +549,17 @@ def setup assert_equal old_encrypted_password, u.encrypted_password, "Changed password" end + test "can grant signin permission to allow user to access the app" do + app = create(:application) + user = create(:user) + + assert_not user.has_access_to?(app) + + user.grant_application_signin_permission(app) + + assert user.has_access_to?(app) + end + test "can grant permissions to users and return the created permission" do app = create(:application, name: "my_app", with_supported_permissions: ["Create publications", "Delete publications"]) user = create(:user) @@ -563,20 +574,20 @@ def setup app = create(:application, name: "my_app") user = create(:user) - user.grant_application_permission(app, "signin") - user.grant_application_permission(app, "signin") + user.grant_application_signin_permission(app) + user.grant_application_signin_permission(app) - assert_user_has_permissions %w[signin], app, user + assert_user_has_permissions [SupportedPermission::SIGNIN_NAME], app, user end test "returns multiple permissions in name order" do app = create(:application, name: "my_app", with_supported_permissions: %w[edit]) user = create(:user) - user.grant_application_permission(app, "signin") + user.grant_application_signin_permission(app) user.grant_application_permission(app, "edit") - assert_user_has_permissions %w[edit signin], app, user + assert_user_has_permissions ["edit", SupportedPermission::SIGNIN_NAME], app, user end test "inviting a user sets confirmed_at" do diff --git a/test/policies/supported_permission_policy_scope_test.rb b/test/policies/supported_permission_policy_scope_test.rb index 033cee649..bb73f2f12 100644 --- a/test/policies/supported_permission_policy_scope_test.rb +++ b/test/policies/supported_permission_policy_scope_test.rb @@ -71,8 +71,8 @@ class SupportedPermissionPolicyScopeTest < ActiveSupport::TestCase context "super organisation admins" do setup do user = create(:super_organisation_admin_user).tap do |u| - u.grant_application_permission(@app_one, "signin") - u.grant_application_permission(@app_two, "signin") + u.grant_application_signin_permission(@app_one) + u.grant_application_signin_permission(@app_two) end @resolved_scope = SupportedPermissionPolicy::Scope.new(user, SupportedPermission.all).resolve @@ -104,8 +104,8 @@ class SupportedPermissionPolicyScopeTest < ActiveSupport::TestCase context "organisation admins" do setup do user = create(:organisation_admin_user).tap do |u| - u.grant_application_permission(@app_one, "signin") - u.grant_application_permission(@app_two, "signin") + u.grant_application_signin_permission(@app_one) + u.grant_application_signin_permission(@app_two) end @resolved_scope = SupportedPermissionPolicy::Scope.new(user, SupportedPermission.all).resolve diff --git a/test/policies/user_permission_manageable_application_policy_scope_test.rb b/test/policies/user_permission_manageable_application_policy_scope_test.rb index bba059b8c..a986b4a7e 100644 --- a/test/policies/user_permission_manageable_application_policy_scope_test.rb +++ b/test/policies/user_permission_manageable_application_policy_scope_test.rb @@ -35,8 +35,8 @@ class UserPermissionManageableApplicationPolicyScopeTest < ActiveSupport::TestCa context "super organisation admins" do setup do user = create(:super_organisation_admin_user).tap do |u| - u.grant_application_permission(@app_one, "signin") - u.grant_application_permission(@app_two, "signin") + u.grant_application_signin_permission(@app_one) + u.grant_application_signin_permission(@app_two) end @resolved_scope = UserPermissionManageableApplicationPolicy::Scope.new(user).resolve @@ -59,8 +59,8 @@ class UserPermissionManageableApplicationPolicyScopeTest < ActiveSupport::TestCa context "for organisation admins" do setup do user = create(:organisation_admin_user).tap do |u| - u.grant_application_permission(@app_one, "signin") - u.grant_application_permission(@app_two, "signin") + u.grant_application_signin_permission(@app_one) + u.grant_application_signin_permission(@app_two) end @resolved_scope = UserPermissionManageableApplicationPolicy::Scope.new(user).resolve diff --git a/test/services/user_update_test.rb b/test/services/user_update_test.rb index d50143cbb..99485c8b6 100644 --- a/test/services/user_update_test.rb +++ b/test/services/user_update_test.rb @@ -18,7 +18,7 @@ class UserUpdateTest < ActionView::TestCase parsed_ip_address = IPAddr.new(ip_address, Socket::AF_INET).to_s affected_user = create(:user) - app = create(:application, name: "App", with_supported_permissions: ["Editor", "signin", "Something Else"]) + app = create(:application, name: "App", with_supported_permissions: ["Editor", SupportedPermission::SIGNIN_NAME, "Something Else"]) affected_user.grant_application_permission(app, "Something Else") perms = app.supported_permissions.first(2).map(&:id)