diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 69b20db5..3e02de85 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -6,6 +6,8 @@ class UsersController < ApplicationController before_action :build_user, only: %i[new create] authorize_resource :user, only: %i[new create] + before_action(only: :index) { authorize!(:manage, current_account) } + def index @users = if params[:status] == 'archived' diff --git a/app/models/user.rb b/app/models/user.rb index 31039bc3..01c944eb 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -48,7 +48,9 @@ # class User < ApplicationRecord ROLES = [ - ADMIN_ROLE = 'admin' + ADMIN_ROLE = 'admin', + EDITOR_ROLE = 'editor', + VIEWER_ROLE = 'viewer' ].freeze EMAIL_REGEXP = /[^@;,<>\s]+@[^@;,<>\s]+/ @@ -129,6 +131,10 @@ class User < ApplicationRecord end end + def admin? = role == ADMIN_ROLE + def editor? = role == EDITOR_ROLE + def viewer? = role == VIEWER_ROLE + def signed_in_via_sso? provider == 'google_oauth2' && uid.present? end diff --git a/app/views/users/_role_select.html.erb b/app/views/users/_role_select.html.erb index c10a57ad..7a4b62fa 100644 --- a/app/views/users/_role_select.html.erb +++ b/app/views/users/_role_select.html.erb @@ -2,12 +2,12 @@ <%= f.label :role, class: 'label' %> <%= f.select :role, nil, {}, class: 'base-select' do %> - - + + <% end %> diff --git a/lib/ability.rb b/lib/ability.rb index da472f58..b9b2639b 100644 --- a/lib/ability.rb +++ b/lib/ability.rb @@ -4,25 +4,69 @@ class Ability include CanCan::Ability def initialize(user) + return if user.blank? + + grant_personal_abilities(user) + + case user.role + when User::ADMIN_ROLE then grant_admin_abilities(user) + when User::EDITOR_ROLE then grant_editor_abilities(user) + when User::VIEWER_ROLE then grant_viewer_abilities(user) + end + end + + private + + def grant_personal_abilities(user) + can :read, Account, id: user.account_id + can :manage, User, id: user.id + can :manage, UserConfig, user_id: user.id + can :manage, EncryptedUserConfig, user_id: user.id + can :manage, AccessToken, user_id: user.id + end + + def grant_admin_abilities(user) can %i[read create update], Template, Abilities::TemplateConditions.collection(user) do |template| Abilities::TemplateConditions.entity(template, user:, ability: 'manage') end - can :destroy, Template, account_id: user.account_id - can :manage, TemplateFolder, account_id: user.account_id - can :manage, TemplateSharing, template: { account_id: user.account_id } - can :manage, Submission, account_id: user.account_id - can :manage, Submitter, account_id: user.account_id - can :manage, User, account_id: user.account_id - can :manage, EncryptedConfig, account_id: user.account_id - can :manage, EncryptedUserConfig, user_id: user.id - can :manage, AccountConfig, account_id: user.account_id - can :manage, UserConfig, user_id: user.id - can :manage, Account, id: user.account_id - can :manage, AccessToken, user_id: user.id - can :manage, McpToken, user_id: user.id - can :manage, WebhookUrl, account_id: user.account_id + can :destroy, Template, account_id: user.account_id + can :manage, TemplateFolder, account_id: user.account_id + can :manage, TemplateSharing, template: { account_id: user.account_id } + can :manage, Submission, account_id: user.account_id + can :manage, Submitter, account_id: user.account_id + can :manage, User, account_id: user.account_id + can :manage, EncryptedConfig, account_id: user.account_id + can :manage, AccountConfig, account_id: user.account_id + can :manage, Account, id: user.account_id + can :manage, McpToken, user_id: user.id + can :manage, WebhookUrl, account_id: user.account_id can :manage, :mcp end + + def grant_editor_abilities(user) + can %i[read create update], Template, Abilities::TemplateConditions.collection(user) do |template| + Abilities::TemplateConditions.entity(template, user:, ability: 'manage') + end + + can :destroy, Template, account_id: user.account_id + can :manage, TemplateFolder, account_id: user.account_id + can :manage, TemplateSharing, template: { account_id: user.account_id } + can :manage, Submission, account_id: user.account_id + can :manage, Submitter, account_id: user.account_id + can :read, AccountConfig, account_id: user.account_id + end + + def grant_viewer_abilities(user) + can :read, Template, Abilities::TemplateConditions.collection(user, ability: :read) do |template| + Abilities::TemplateConditions.entity(template, user:, ability: 'read') + end + + can :read, TemplateFolder, account_id: user.account_id + can :read, TemplateSharing, template: { account_id: user.account_id } + can :read, Submission, account_id: user.account_id + can :read, Submitter, account_id: user.account_id + can :read, AccountConfig, account_id: user.account_id + end end diff --git a/spec/models/ability_spec.rb b/spec/models/ability_spec.rb new file mode 100644 index 00000000..b673ec07 --- /dev/null +++ b/spec/models/ability_spec.rb @@ -0,0 +1,162 @@ +# frozen_string_literal: true + +require 'rails_helper' +require 'cancan/matchers' + +RSpec.describe Ability do + let(:account) { create(:account) } + let(:other_account) { create(:account) } + + def template_for(account) + Template.new(account_id: account.id) + end + + def template_folder_for(account) + TemplateFolder.new(account_id: account.id) + end + + def submission_for(account) + Submission.new(account_id: account.id) + end + + def submitter_for(account) + Submitter.new(account_id: account.id) + end + + shared_examples 'personal-resource grants' do + it 'manages own User, UserConfig, EncryptedUserConfig, AccessToken' do + expect(ability).to be_able_to(:manage, user) + expect(ability).to be_able_to(:manage, UserConfig.new(user_id: user.id)) + expect(ability).to be_able_to(:manage, EncryptedUserConfig.new(user_id: user.id)) + expect(ability).to be_able_to(:manage, AccessToken.new(user_id: user.id)) + expect(ability).to be_able_to(:read, Account.new.tap { |a| a.id = account.id }) + end + + it "cannot touch another user's User / UserConfig / AccessToken (unless admin)" do + other_user = create(:user, account: account) + expect(ability).not_to be_able_to(:manage, UserConfig.new(user_id: other_user.id)) + expect(ability).not_to be_able_to(:manage, AccessToken.new(user_id: other_user.id)) + + # Admin's full :manage User rule covers same-account users; editor/viewer don't. + if user.role == User::ADMIN_ROLE + expect(ability).to be_able_to(:manage, other_user) + else + expect(ability).not_to be_able_to(:manage, other_user) + end + end + end + + describe 'admin role' do + let(:user) { create(:user, account: account, role: User::ADMIN_ROLE) } + let(:ability) { described_class.new(user) } + + include_examples 'personal-resource grants' + + it 'manages templates, folders, sharings, submissions, submitters in own account' do + expect(ability).to be_able_to(:read, template_for(account)) + expect(ability).to be_able_to(:create, template_for(account)) + expect(ability).to be_able_to(:update, template_for(account)) + expect(ability).to be_able_to(:destroy, template_for(account)) + expect(ability).to be_able_to(:manage, template_folder_for(account)) + expect(ability).to be_able_to(:manage, submission_for(account)) + expect(ability).to be_able_to(:manage, submitter_for(account)) + end + + it 'manages account-wide settings and users' do + expect(ability).to be_able_to(:manage, User.new(account_id: account.id)) + expect(ability).to be_able_to(:manage, Account.new.tap { |a| a.id = account.id }) + expect(ability).to be_able_to(:manage, AccountConfig.new(account_id: account.id)) + expect(ability).to be_able_to(:manage, EncryptedConfig.new(account_id: account.id)) + expect(ability).to be_able_to(:manage, WebhookUrl.new(account_id: account.id)) + expect(ability).to be_able_to(:manage, McpToken.new(user_id: user.id)) + expect(ability).to be_able_to(:manage, :mcp) + end + + it 'cannot touch resources scoped to another account' do + expect(ability).not_to be_able_to(:read, template_for(other_account)) + expect(ability).not_to be_able_to(:destroy, template_for(other_account)) + expect(ability).not_to be_able_to(:manage, submission_for(other_account)) + expect(ability).not_to be_able_to(:manage, User.new(account_id: other_account.id)) + expect(ability).not_to be_able_to(:manage, Account.new.tap { |a| a.id = other_account.id }) + end + end + + describe 'editor role' do + let(:user) { create(:user, account: account, role: User::EDITOR_ROLE) } + let(:ability) { described_class.new(user) } + + include_examples 'personal-resource grants' + + it 'manages templates, folders, sharings, submissions, submitters in own account' do + expect(ability).to be_able_to(:read, template_for(account)) + expect(ability).to be_able_to(:create, template_for(account)) + expect(ability).to be_able_to(:update, template_for(account)) + expect(ability).to be_able_to(:destroy, template_for(account)) + expect(ability).to be_able_to(:manage, template_folder_for(account)) + expect(ability).to be_able_to(:manage, submission_for(account)) + expect(ability).to be_able_to(:manage, submitter_for(account)) + end + + it 'can read AccountConfig but cannot manage account-wide settings' do + expect(ability).to be_able_to(:read, AccountConfig.new(account_id: account.id)) + expect(ability).not_to be_able_to(:manage, AccountConfig.new(account_id: account.id)) + expect(ability).not_to be_able_to(:manage, EncryptedConfig.new(account_id: account.id)) + expect(ability).not_to be_able_to(:manage, User.new(account_id: account.id)) + expect(ability).not_to be_able_to(:manage, Account.new.tap { |a| a.id = account.id }) + expect(ability).not_to be_able_to(:manage, WebhookUrl.new(account_id: account.id)) + expect(ability).not_to be_able_to(:manage, McpToken.new(user_id: user.id)) + expect(ability).not_to be_able_to(:manage, :mcp) + end + + it 'cannot touch resources scoped to another account' do + expect(ability).not_to be_able_to(:read, template_for(other_account)) + expect(ability).not_to be_able_to(:manage, submission_for(other_account)) + end + end + + describe 'viewer role' do + let(:user) { create(:user, account: account, role: User::VIEWER_ROLE) } + let(:ability) { described_class.new(user) } + + include_examples 'personal-resource grants' + + it 'reads templates, folders, sharings, submissions, submitters in own account' do + expect(ability).to be_able_to(:read, template_for(account)) + expect(ability).to be_able_to(:read, template_folder_for(account)) + expect(ability).to be_able_to(:read, submission_for(account)) + expect(ability).to be_able_to(:read, submitter_for(account)) + expect(ability).to be_able_to(:read, AccountConfig.new(account_id: account.id)) + end + + it 'cannot mutate anything in own account beyond personal resources' do + expect(ability).not_to be_able_to(:create, template_for(account)) + expect(ability).not_to be_able_to(:update, template_for(account)) + expect(ability).not_to be_able_to(:destroy, template_for(account)) + expect(ability).not_to be_able_to(:manage, template_folder_for(account)) + expect(ability).not_to be_able_to(:manage, submission_for(account)) + expect(ability).not_to be_able_to(:manage, submitter_for(account)) + expect(ability).not_to be_able_to(:manage, User.new(account_id: account.id)) + expect(ability).not_to be_able_to(:manage, AccountConfig.new(account_id: account.id)) + expect(ability).not_to be_able_to(:manage, EncryptedConfig.new(account_id: account.id)) + expect(ability).not_to be_able_to(:manage, WebhookUrl.new(account_id: account.id)) + expect(ability).not_to be_able_to(:manage, Account.new.tap { |a| a.id = account.id }) + expect(ability).not_to be_able_to(:manage, :mcp) + end + + it 'cannot read resources scoped to another account' do + expect(ability).not_to be_able_to(:read, template_for(other_account)) + expect(ability).not_to be_able_to(:read, submission_for(other_account)) + end + end + + describe 'unknown role' do + let(:user) { create(:user, account: account, role: User::ADMIN_ROLE).tap { |u| u.update_column(:role, 'mystery') } } + let(:ability) { described_class.new(user) } + + it 'still grants personal resources but no role-specific abilities' do + expect(ability).to be_able_to(:manage, UserConfig.new(user_id: user.id)) + expect(ability).not_to be_able_to(:read, template_for(account)) + expect(ability).not_to be_able_to(:manage, submission_for(account)) + end + end +end diff --git a/spec/requests/role_authorization_spec.rb b/spec/requests/role_authorization_spec.rb new file mode 100644 index 00000000..50b52523 --- /dev/null +++ b/spec/requests/role_authorization_spec.rb @@ -0,0 +1,101 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe 'Role-based authorization', type: :request do + let!(:account) { create(:account) } + let!(:admin) { create(:user, account: account, role: User::ADMIN_ROLE, email: 'admin@wabo.cc') } + let!(:editor) { create(:user, account: account, role: User::EDITOR_ROLE, email: 'editor@wabo.cc') } + let!(:viewer) { create(:user, account: account, role: User::VIEWER_ROLE, email: 'viewer@wabo.cc') } + + # `ApplicationController` rescues `CanCan::AccessDenied` only in production/test + # and redirects to `root_path` (see app/controllers/application_controller.rb). + def expect_denied + expect(response).to have_http_status(:found) + expect(response).to redirect_to(root_path) + end + + shared_examples 'an admin-only settings route' do |path_helper| + it "is denied for editors (#{path_helper})" do + sign_in editor + get send(path_helper) + expect_denied + end + + it "is denied for viewers (#{path_helper})" do + sign_in viewer + get send(path_helper) + expect_denied + end + + it "is reachable for admins (#{path_helper})" do + sign_in admin + get send(path_helper) + expect(response).to have_http_status(:ok).or have_http_status(:found) + expect(response).not_to redirect_to(root_path) if response.status == 302 + end + end + + describe 'admin-only settings' do + include_examples 'an admin-only settings route', :settings_users_path + include_examples 'an admin-only settings route', :settings_sso_index_path + include_examples 'an admin-only settings route', :settings_webhooks_path + include_examples 'an admin-only settings route', :settings_esign_path + + # Personalization's GET reads `AccountConfig`, which Editor/Viewer can do + # (so UI chrome renders correctly). Writes are gated by :create AccountConfig, + # which only admins hold. + it 'lets editors view personalization but blocks the POST' do + sign_in editor + get settings_personalization_path + expect(response).to have_http_status(:ok) + + post settings_personalization_path, params: { + account_config: { key: AccountConfig::FORM_COMPLETED_BUTTON_KEY, value: { title: 'Done', url: '' } } + } + expect(response).to redirect_to(root_path) + end + + it 'lets viewers view personalization but blocks the POST' do + sign_in viewer + get settings_personalization_path + expect(response).to have_http_status(:ok) + + post settings_personalization_path, params: { + account_config: { key: AccountConfig::FORM_COMPLETED_BUTTON_KEY, value: { title: 'Done', url: '' } } + } + expect(response).to redirect_to(root_path) + end + end + + describe 'templates and submissions list pages' do + it 'are reachable for editors' do + sign_in editor + get templates_path + expect(response).to have_http_status(:ok) + get submissions_path + expect(response).to have_http_status(:ok) + end + + it 'are reachable for viewers' do + sign_in viewer + get templates_path + expect(response).to have_http_status(:ok) + get submissions_path + expect(response).to have_http_status(:ok) + end + end + + describe 'self-service profile' do + it 'is reachable for editors and viewers' do + sign_in editor + get settings_profile_index_path + expect(response).to have_http_status(:ok) + + sign_out editor + sign_in viewer + get settings_profile_index_path + expect(response).to have_http_status(:ok) + end + end +end