From 2f1843151d2f565efbafb36da02ebe9da7af3b0d Mon Sep 17 00:00:00 2001 From: Alex Turchyn Date: Sun, 17 Sep 2023 00:45:57 +0300 Subject: [PATCH] add cancan for authorization --- Gemfile | 1 + Gemfile.lock | 2 + app/controllers/accounts_controller.rb | 7 +++ app/controllers/api/api_base_controller.rb | 9 ++++ app/controllers/api/attachments_controller.rb | 1 + app/controllers/api/submissions_controller.rb | 14 +++-- .../api/submitter_email_clicks_controller.rb | 1 + .../api/submitter_form_views_controller.rb | 1 + app/controllers/api/templates_controller.rb | 8 +-- .../api/templates_documents_controller.rb | 4 +- app/controllers/api_settings_controller.rb | 4 +- app/controllers/application_controller.rb | 10 ++++ .../console_redirect_controller.rb | 1 + app/controllers/dashboard_controller.rb | 26 ++++++++-- app/controllers/email_settings_controller.rb | 2 + app/controllers/esign_settings_controller.rb | 32 ++++++------ app/controllers/mfa_setup_controller.rb | 4 ++ app/controllers/newsletters_controller.rb | 2 + .../personalization_settings_controller.rb | 6 ++- app/controllers/profile_controller.rb | 4 ++ .../send_submission_email_controller.rb | 1 + app/controllers/setup_controller.rb | 1 + app/controllers/sms_settings_controller.rb | 11 ++++ app/controllers/start_form_controller.rb | 1 + .../storage_settings_controller.rb | 2 + app/controllers/submissions_controller.rb | 24 +++++---- .../submissions_debug_controller.rb | 1 + .../submissions_download_controller.rb | 1 + .../submissions_export_controller.rb | 17 +++--- app/controllers/submit_form_controller.rb | 1 + .../submitters_send_email_controller.rb | 16 +++--- .../templates_archived_controller.rb | 8 +-- ...mplates_archived_submissions_controller.rb | 11 ++-- app/controllers/templates_controller.rb | 16 +++--- .../templates_restore_controller.rb | 8 +-- .../templates_uploads_controller.rb | 14 ++--- app/controllers/users_controller.rb | 33 ++++++------ .../verify_pdf_signature_controller.rb | 2 + .../webhook_settings_controller.rb | 16 +++--- app/views/accounts/show.html.erb | 8 +-- app/views/dashboard/index.html.erb | 10 ++-- app/views/esign_settings/new.html.erb | 2 +- app/views/esign_settings/show.html.erb | 12 +++-- app/views/shared/_settings_nav.html.erb | 52 ++++++++++++------- app/views/submissions/show.html.erb | 6 +-- app/views/templates/_submission.html.erb | 2 +- app/views/templates/_template.html.erb | 14 ++--- app/views/templates/_title.html.erb | 14 ++--- app/views/templates/show.html.erb | 12 +++-- .../{show.html.erb => index.html.erb} | 0 app/views/users/_form.html.erb | 3 ++ app/views/users/_role_select.html.erb | 13 +++++ app/views/users/index.html.erb | 20 ++++--- config/routes.rb | 2 +- lib/ability.rb | 16 ++++++ lib/templates/clone_attachments.rb | 2 +- spec/system/dashboard_spec.rb | 2 + spec/system/team_settings_spec.rb | 10 ++-- 58 files changed, 340 insertions(+), 183 deletions(-) rename app/views/templates_archived_submissions/{show.html.erb => index.html.erb} (100%) create mode 100644 app/views/users/_role_select.html.erb create mode 100644 lib/ability.rb diff --git a/Gemfile b/Gemfile index 0b5badcc..471e5b9b 100644 --- a/Gemfile +++ b/Gemfile @@ -7,6 +7,7 @@ ruby '3.2.2' gem 'aws-sdk-s3', require: false gem 'azure-storage-blob', require: false gem 'bootsnap', require: false +gem 'cancancan' gem 'devise' gem 'devise-two-factor' gem 'dotenv', require: false diff --git a/Gemfile.lock b/Gemfile.lock index b102b054..8deb63e3 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -114,6 +114,7 @@ GEM bullet (7.0.7) activesupport (>= 3.0.0) uniform_notifier (~> 1.11) + cancancan (3.5.0) capybara (3.39.2) addressable matrix @@ -566,6 +567,7 @@ DEPENDENCIES better_html bootsnap bullet + cancancan capybara cuprite debug diff --git a/app/controllers/accounts_controller.rb b/app/controllers/accounts_controller.rb index 5c568870..16d7bcc0 100644 --- a/app/controllers/accounts_controller.rb +++ b/app/controllers/accounts_controller.rb @@ -9,6 +9,9 @@ class AccountsController < ApplicationController 'de-DE' => 'German (Germany)' }.freeze + before_action :load_account + authorize_resource :account + def show; end def update @@ -35,6 +38,10 @@ class AccountsController < ApplicationController private + def load_account + @account = current_account + end + def account_params params.require(:account).permit(:name, :timezone, :locale) end diff --git a/app/controllers/api/api_base_controller.rb b/app/controllers/api/api_base_controller.rb index 1b49bfa9..57e3d216 100644 --- a/app/controllers/api/api_base_controller.rb +++ b/app/controllers/api/api_base_controller.rb @@ -5,6 +5,15 @@ module Api include ActiveStorage::SetCurrent before_action :authenticate_user! + check_authorization + + if Rails.env.production? + rescue_from CanCan::AccessDenied do |e| + Rollbar.error(e) if defined?(Rollbar) + + render json: { error: e.message }, status: :forbidden + end + end private diff --git a/app/controllers/api/attachments_controller.rb b/app/controllers/api/attachments_controller.rb index d61867f2..7ff46fe7 100644 --- a/app/controllers/api/attachments_controller.rb +++ b/app/controllers/api/attachments_controller.rb @@ -3,6 +3,7 @@ module Api class AttachmentsController < ApiBaseController skip_before_action :authenticate_user! + skip_authorization_check def create submitter = Submitter.find_by!(slug: params[:submitter_slug]) diff --git a/app/controllers/api/submissions_controller.rb b/app/controllers/api/submissions_controller.rb index 9f34507d..c3000285 100644 --- a/app/controllers/api/submissions_controller.rb +++ b/app/controllers/api/submissions_controller.rb @@ -5,21 +5,25 @@ module Api UnknownFieldName = Class.new(StandardError) UnknownSubmitterName = Class.new(StandardError) - def create - template = current_account.templates.find(params[:template_id]) + load_and_authorize_resource :template + + before_action do + authorize!(:create, Submission) + end + def create submissions = if (emails = (params[:emails] || params[:email]).presence) - Submissions.create_from_emails(template:, + Submissions.create_from_emails(template: @template, user: current_user, source: :api, mark_as_sent: params[:send_email] != 'false', emails:) else - submissions_attrs = normalize_submissions_params!(submissions_params[:submission], template) + submissions_attrs = normalize_submissions_params!(submissions_params[:submission], @template) Submissions.create_from_submitters( - template:, + template: @template, user: current_user, source: :api, mark_as_sent: params[:send_email] != 'false', diff --git a/app/controllers/api/submitter_email_clicks_controller.rb b/app/controllers/api/submitter_email_clicks_controller.rb index a9d09a28..87221680 100644 --- a/app/controllers/api/submitter_email_clicks_controller.rb +++ b/app/controllers/api/submitter_email_clicks_controller.rb @@ -3,6 +3,7 @@ module Api class SubmitterEmailClicksController < ApiBaseController skip_before_action :authenticate_user! + skip_authorization_check def create submitter = Submitter.find_by!(slug: params[:submitter_slug]) diff --git a/app/controllers/api/submitter_form_views_controller.rb b/app/controllers/api/submitter_form_views_controller.rb index b5ac5440..93c668f1 100644 --- a/app/controllers/api/submitter_form_views_controller.rb +++ b/app/controllers/api/submitter_form_views_controller.rb @@ -3,6 +3,7 @@ module Api class SubmitterFormViewsController < ApiBaseController skip_before_action :authenticate_user! + skip_authorization_check def create submitter = Submitter.find_by!(slug: params[:submitter_slug]) diff --git a/app/controllers/api/templates_controller.rb b/app/controllers/api/templates_controller.rb index dcbb0ee9..ef4a1e33 100644 --- a/app/controllers/api/templates_controller.rb +++ b/app/controllers/api/templates_controller.rb @@ -2,10 +2,10 @@ module Api class TemplatesController < ApiBaseController - before_action :load_template, only: %i[show update] + load_and_authorize_resource :template def index - render json: current_account.templates + render json: @templates end def show @@ -28,9 +28,5 @@ module Api fields: [[:uuid, :submitter_uuid, :name, :type, :required, { options: [], areas: [%i[x y w h cell_w attachment_uuid page]] }]]) end - - def load_template - @template = current_account.templates.find(params[:id]) - end end end diff --git a/app/controllers/api/templates_documents_controller.rb b/app/controllers/api/templates_documents_controller.rb index db5ae398..1657e70e 100644 --- a/app/controllers/api/templates_documents_controller.rb +++ b/app/controllers/api/templates_documents_controller.rb @@ -2,11 +2,11 @@ module Api class TemplatesDocumentsController < ApiBaseController + load_and_authorize_resource :template + def create return head :unprocessable_entity if params[:blobs].blank? && params[:files].blank? - @template = current_account.templates.find(params[:template_id]) - documents = Templates::CreateAttachments.call(@template, params) schema = documents.map do |doc| diff --git a/app/controllers/api_settings_controller.rb b/app/controllers/api_settings_controller.rb index 40e57ba2..4013dd42 100644 --- a/app/controllers/api_settings_controller.rb +++ b/app/controllers/api_settings_controller.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true class ApiSettingsController < ApplicationController - def index; end + def index + authorize!(:read, current_user.access_token) + end end diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 3d0f96b9..e860fe4f 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -4,6 +4,8 @@ class ApplicationController < ActionController::Base include ActiveStorage::SetCurrent include Pagy::Backend + check_authorization unless: :devise_controller? + before_action :sign_in_for_demo, if: -> { Docuseal.demo? } before_action :maybe_redirect_to_setup, unless: :signed_in? before_action :authenticate_user!, unless: :devise_controller? @@ -16,6 +18,14 @@ class ApplicationController < ActionController::Base redirect_to request.path end + if Rails.env.production? + rescue_from CanCan::AccessDenied do |e| + Rollbar.error(e) if defined?(Rollbar) + + redirect_back(fallback_location: root_path, alert: e.message) + end + end + def default_url_options Docuseal.default_url_options end diff --git a/app/controllers/console_redirect_controller.rb b/app/controllers/console_redirect_controller.rb index 5dd00789..5c4b037d 100644 --- a/app/controllers/console_redirect_controller.rb +++ b/app/controllers/console_redirect_controller.rb @@ -2,6 +2,7 @@ class ConsoleRedirectController < ApplicationController skip_before_action :authenticate_user! + skip_authorization_check def index return redirect_to(new_user_session_path({ redir: params[:redir] }.compact)) if current_user.blank? diff --git a/app/controllers/dashboard_controller.rb b/app/controllers/dashboard_controller.rb index bbf54813..fe7c1c5b 100644 --- a/app/controllers/dashboard_controller.rb +++ b/app/controllers/dashboard_controller.rb @@ -3,13 +3,29 @@ class DashboardController < ApplicationController skip_before_action :authenticate_user!, only: %i[index] + before_action :maybe_redirect_product_url + before_action :maybe_render_landing + + load_and_authorize_resource :template, parent: false + def index - return redirect_to Docuseal::PRODUCT_URL, allow_other_host: true if Docuseal.multitenant? && !signed_in? - return render 'pages/landing' unless signed_in? + @templates = @templates.active.preload(:author).order(id: :desc) + @templates = Templates.search(@templates, params[:q]) + + @pagy, @templates = pagy(@templates, items: 12) + end + + private + + def maybe_redirect_product_url + return if !Docuseal.multitenant? || signed_in? + + redirect_to Docuseal::PRODUCT_URL, allow_other_host: true + end - templates = current_account.templates.active.preload(:author).order(id: :desc) - templates = Templates.search(templates, params[:q]) + def maybe_render_landing + return if signed_in? - @pagy, @templates = pagy(templates, items: 12) + render 'pages/landing' end end diff --git a/app/controllers/email_settings_controller.rb b/app/controllers/email_settings_controller.rb index f5700872..2c39053c 100644 --- a/app/controllers/email_settings_controller.rb +++ b/app/controllers/email_settings_controller.rb @@ -2,6 +2,8 @@ class EmailSettingsController < ApplicationController before_action :load_encrypted_config + authorize_resource :encrypted_config, only: :index + authorize_resource :encrypted_config, parent: false, only: :create def index; end diff --git a/app/controllers/esign_settings_controller.rb b/app/controllers/esign_settings_controller.rb index ee3743d5..1b87df45 100644 --- a/app/controllers/esign_settings_controller.rb +++ b/app/controllers/esign_settings_controller.rb @@ -11,9 +11,12 @@ class EsignSettingsController < ApplicationController end end + before_action :load_encrypted_config + authorize_resource :encrypted_config, parent: false, only: %i[new create] + authorize_resource :encrypted_config, only: %i[update destroy show] + def show - cert_data = EncryptedConfig.find_by(account: current_account, - key: EncryptedConfig::ESIGN_CERTS_KEY)&.value || {} + cert_data = @encrypted_config.value || {} default_pkcs = GenerateCertificate.load_pkcs(cert_data) if cert_data['cert'].present? @@ -42,10 +45,7 @@ class EsignSettingsController < ApplicationController def create @cert_record = CertFormRecord.new(**cert_params) - cert_configs = EncryptedConfig.find_or_initialize_by(account: current_account, - key: EncryptedConfig::ESIGN_CERTS_KEY) - - if (cert_configs.value && cert_configs.value['custom']&.any? { |e| e['name'] == @cert_record.name }) || + if (@encrypted_config.value && @encrypted_config.value['custom']&.any? { |e| e['name'] == @cert_record.name }) || @cert_record.name == DEFAULT_CERT_NAME @cert_record.errors.add(:name, 'already exists') @@ -54,7 +54,7 @@ class EsignSettingsController < ApplicationController status: :unprocessable_entity end - save_new_cert!(cert_configs, @cert_record) + save_new_cert!(@encrypted_config, @cert_record) redirect_to settings_esign_path, notice: 'Certificate has been successfully added!' rescue OpenSSL::PKCS12::PKCS12Error @@ -64,29 +64,31 @@ class EsignSettingsController < ApplicationController end def update - cert_configs = EncryptedConfig.find_by(account: current_account, key: EncryptedConfig::ESIGN_CERTS_KEY) + @encrypted_config.value['custom'].each { |e| e['status'] = 'validate' } - cert_configs.value['custom'].each { |e| e['status'] = 'validate' } - custom_cert_data = cert_configs.value['custom'].find { |e| e['name'] == params[:name] } + custom_cert_data = @encrypted_config.value['custom'].find { |e| e['name'] == params[:name] } custom_cert_data['status'] = 'default' if custom_cert_data - cert_configs.save! + @encrypted_config.save! redirect_to settings_esign_path, notice: 'Default certificate has been selected' end def destroy - cert_configs = EncryptedConfig.find_by(account: current_account, key: EncryptedConfig::ESIGN_CERTS_KEY) + @encrypted_config.value['custom'].reject! { |e| e['name'] == params[:name] } - cert_configs.value['custom'].reject! { |e| e['name'] == params[:name] } - - cert_configs.save! + @encrypted_config.save! redirect_to settings_esign_path, notice: 'Certificate has been removed' end private + def load_encrypted_config + @encrypted_config = EncryptedConfig.find_or_initialize_by(account: current_account, + key: EncryptedConfig::ESIGN_CERTS_KEY) + end + def save_new_cert!(cert_configs, cert_record) pkcs = OpenSSL::PKCS12.new(cert_record.file.read, cert_record.password) diff --git a/app/controllers/mfa_setup_controller.rb b/app/controllers/mfa_setup_controller.rb index 79b0e895..32cea983 100644 --- a/app/controllers/mfa_setup_controller.rb +++ b/app/controllers/mfa_setup_controller.rb @@ -1,6 +1,10 @@ # frozen_string_literal: true class MfaSetupController < ApplicationController + before_action do + authorize!(:update, current_user) + end + def new current_user.otp_secret ||= User.generate_otp_secret diff --git a/app/controllers/newsletters_controller.rb b/app/controllers/newsletters_controller.rb index 37cc8645..ddd305c5 100644 --- a/app/controllers/newsletters_controller.rb +++ b/app/controllers/newsletters_controller.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true class NewslettersController < ApplicationController + skip_authorization_check + def show; end def update diff --git a/app/controllers/personalization_settings_controller.rb b/app/controllers/personalization_settings_controller.rb index 5c1b272e..e5c08bf5 100644 --- a/app/controllers/personalization_settings_controller.rb +++ b/app/controllers/personalization_settings_controller.rb @@ -1,12 +1,16 @@ # frozen_string_literal: true class PersonalizationSettingsController < ApplicationController - def show; end + def show + authorize!(:read, AccountConfig) + end def create account_config = current_account.account_configs.find_or_initialize_by(key: encrypted_config_params[:key]) + authorize!(:create, account_config) + account_config.update!(encrypted_config_params) redirect_back(fallback_location: settings_personalization_path, notice: 'Settings have been saved.') diff --git a/app/controllers/profile_controller.rb b/app/controllers/profile_controller.rb index 62e047a1..e61a312d 100644 --- a/app/controllers/profile_controller.rb +++ b/app/controllers/profile_controller.rb @@ -1,6 +1,10 @@ # frozen_string_literal: true class ProfileController < ApplicationController + before_action do + authorize!(:manage, current_user) + end + def index; end def update_contact diff --git a/app/controllers/send_submission_email_controller.rb b/app/controllers/send_submission_email_controller.rb index cd61045a..28e2a364 100644 --- a/app/controllers/send_submission_email_controller.rb +++ b/app/controllers/send_submission_email_controller.rb @@ -5,6 +5,7 @@ class SendSubmissionEmailController < ApplicationController skip_before_action :authenticate_user! skip_before_action :verify_authenticity_token + skip_authorization_check def success; end diff --git a/app/controllers/setup_controller.rb b/app/controllers/setup_controller.rb index ca45ccf9..7d386888 100644 --- a/app/controllers/setup_controller.rb +++ b/app/controllers/setup_controller.rb @@ -3,6 +3,7 @@ class SetupController < ApplicationController skip_before_action :maybe_redirect_to_setup skip_before_action :authenticate_user! + skip_authorization_check before_action :redirect_to_root_if_signed, if: :signed_in? before_action :ensure_first_user_not_created! diff --git a/app/controllers/sms_settings_controller.rb b/app/controllers/sms_settings_controller.rb index 1f48fd30..96168605 100644 --- a/app/controllers/sms_settings_controller.rb +++ b/app/controllers/sms_settings_controller.rb @@ -1,5 +1,16 @@ # frozen_string_literal: true class SmsSettingsController < ApplicationController + before_action :load_encrypted_config + authorize_resource :encrypted_config, only: :index + authorize_resource :encrypted_config, parent: false, except: :index + def index; end + + private + + def load_encrypted_config + @encrypted_config = + EncryptedConfig.find_or_initialize_by(account: current_account, key: 'sms_configs') + end end diff --git a/app/controllers/start_form_controller.rb b/app/controllers/start_form_controller.rb index 285e2c79..5e472485 100644 --- a/app/controllers/start_form_controller.rb +++ b/app/controllers/start_form_controller.rb @@ -4,6 +4,7 @@ class StartFormController < ApplicationController layout 'form' skip_before_action :authenticate_user! + skip_authorization_check before_action :load_template diff --git a/app/controllers/storage_settings_controller.rb b/app/controllers/storage_settings_controller.rb index 83f5cdb0..c4478a2d 100644 --- a/app/controllers/storage_settings_controller.rb +++ b/app/controllers/storage_settings_controller.rb @@ -2,6 +2,8 @@ class StorageSettingsController < ApplicationController before_action :load_encrypted_config + authorize_resource :encrypted_config, only: :index + authorize_resource :encrypted_config, parent: false, only: :create def index; end diff --git a/app/controllers/submissions_controller.rb b/app/controllers/submissions_controller.rb index 854908d5..311efb19 100644 --- a/app/controllers/submissions_controller.rb +++ b/app/controllers/submissions_controller.rb @@ -2,19 +2,26 @@ class SubmissionsController < ApplicationController before_action :load_template, only: %i[new create] + authorize_resource :template, only: %i[new create] + + load_and_authorize_resource :submission, only: %i[show destroy] def show - @submission = - Submission.joins(:template).where(template: { account_id: current_account.id }) - .preload(:template, template_schema_documents: [:blob, { preview_images_attachments: :blob }]) - .find(params[:id]) + ActiveRecord::Associations::Preloader.new( + records: [@submission], + associations: [:template, { template_schema_documents: [:blob, { preview_images_attachments: :blob }] }] + ).call render :show, layout: 'plain' end - def new; end + def new + authorize!(:new, Submission) + end def create + authorize!(:create, Submission) + submissions = if params[:emails].present? Submissions.create_from_emails(template: @template, @@ -37,12 +44,9 @@ class SubmissionsController < ApplicationController end def destroy - submission = Submission.joins(:template).where(template: { account_id: current_account.id }) - .find(params[:id]) - - submission.update!(deleted_at: Time.current) + @submission.update!(deleted_at: Time.current) - redirect_back(fallback_location: template_path(submission.template), notice: 'Submission has been archived') + redirect_back(fallback_location: template_path(@submission.template), notice: 'Submission has been archived') end private diff --git a/app/controllers/submissions_debug_controller.rb b/app/controllers/submissions_debug_controller.rb index 2b3bab97..e411db15 100644 --- a/app/controllers/submissions_debug_controller.rb +++ b/app/controllers/submissions_debug_controller.rb @@ -4,6 +4,7 @@ class SubmissionsDebugController < ApplicationController layout 'plain' skip_before_action :authenticate_user! + skip_authorization_check def index @submitter = Submitter.preload({ attachments_attachments: :blob }, diff --git a/app/controllers/submissions_download_controller.rb b/app/controllers/submissions_download_controller.rb index 04ffcf56..96a6cd18 100644 --- a/app/controllers/submissions_download_controller.rb +++ b/app/controllers/submissions_download_controller.rb @@ -2,6 +2,7 @@ class SubmissionsDownloadController < ApplicationController skip_before_action :authenticate_user! + skip_authorization_check def index submitter = Submitter.find_by(slug: params[:submitter_slug]) diff --git a/app/controllers/submissions_export_controller.rb b/app/controllers/submissions_export_controller.rb index 33045b1d..e929c12d 100644 --- a/app/controllers/submissions_export_controller.rb +++ b/app/controllers/submissions_export_controller.rb @@ -1,13 +1,14 @@ # frozen_string_literal: true class SubmissionsExportController < ApplicationController - before_action :load_template + load_and_authorize_resource :template + load_and_authorize_resource :submission, through: :template, parent: false, only: :index def index - submissions = @template.submissions.active - .preload(submitters: { documents_attachments: :blob, - attachments_attachments: :blob }) - .order(id: :asc) + submissions = @submissions.active + .preload(submitters: { documents_attachments: :blob, + attachments_attachments: :blob }) + .order(id: :asc) if params[:format] == 'csv' send_data Submissions::GenerateExportFiles.call(submissions, format: params[:format]), @@ -19,10 +20,4 @@ class SubmissionsExportController < ApplicationController end def new; end - - private - - def load_template - @template = current_account.templates.find(params[:template_id]) - end end diff --git a/app/controllers/submit_form_controller.rb b/app/controllers/submit_form_controller.rb index 8485a843..cf27e8f1 100644 --- a/app/controllers/submit_form_controller.rb +++ b/app/controllers/submit_form_controller.rb @@ -4,6 +4,7 @@ class SubmitFormController < ApplicationController layout 'form' skip_before_action :authenticate_user! + skip_authorization_check def show @submitter = diff --git a/app/controllers/submitters_send_email_controller.rb b/app/controllers/submitters_send_email_controller.rb index 3dfc1067..daeb777e 100644 --- a/app/controllers/submitters_send_email_controller.rb +++ b/app/controllers/submitters_send_email_controller.rb @@ -1,18 +1,16 @@ # frozen_string_literal: true class SubmittersSendEmailController < ApplicationController - def create - submitter = Submitter.joins(:template) - .where(template: { account_id: current_account.id }) - .find_by!(slug: params[:submitter_slug]) + load_and_authorize_resource :submitter, id_param: :submitter_slug, find_by: :slug - SubmitterMailer.invitation_email(submitter).deliver_later! + def create + SubmitterMailer.invitation_email(@submitter).deliver_later! - SubmissionEvent.create!(submitter:, event_type: 'send_email') + SubmissionEvent.create!(submitter: @submitter, event_type: 'send_email') - submitter.sent_at ||= Time.current - submitter.save! + @submitter.sent_at ||= Time.current + @submitter.save! - redirect_back(fallback_location: submission_path(submitter.submission), notice: 'Email has been sent') + redirect_back(fallback_location: submission_path(@submitter.submission), notice: 'Email has been sent') end end diff --git a/app/controllers/templates_archived_controller.rb b/app/controllers/templates_archived_controller.rb index a4fccc0b..f598b817 100644 --- a/app/controllers/templates_archived_controller.rb +++ b/app/controllers/templates_archived_controller.rb @@ -1,10 +1,12 @@ # frozen_string_literal: true class TemplatesArchivedController < ApplicationController + load_and_authorize_resource :template, parent: false + def index - templates = current_account.templates.where.not(deleted_at: nil).preload(:author).order(id: :desc) - templates = Templates.search(templates, params[:q]) + @templates = @templates.where.not(deleted_at: nil).preload(:author).order(id: :desc) + @templates = Templates.search(@templates, params[:q]) - @pagy, @templates = pagy(templates, items: 12) + @pagy, @templates = pagy(@templates, items: 12) end end diff --git a/app/controllers/templates_archived_submissions_controller.rb b/app/controllers/templates_archived_submissions_controller.rb index 2ea5938f..6d5f82b7 100644 --- a/app/controllers/templates_archived_submissions_controller.rb +++ b/app/controllers/templates_archived_submissions_controller.rb @@ -1,13 +1,14 @@ # frozen_string_literal: true class TemplatesArchivedSubmissionsController < ApplicationController - def show - @template = current_account.templates.find(params[:template_id]) + load_and_authorize_resource :template + load_and_authorize_resource :submission, through: :template, parent: false - submissions = @template.submissions.where.not(deleted_at: nil) - submissions = Submissions.search(submissions, params[:q]) + def index + @submissions = @submissions.where.not(deleted_at: nil) + @submissions = Submissions.search(@submissions, params[:q]) - @pagy, @submissions = pagy(submissions.preload(:submitters).order(id: :desc)) + @pagy, @submissions = pagy(@submissions.preload(:submitters).order(id: :desc)) rescue ActiveRecord::RecordNotFound redirect_to root_path end diff --git a/app/controllers/templates_controller.rb b/app/controllers/templates_controller.rb index 98fd7e99..d9a9bfc8 100644 --- a/app/controllers/templates_controller.rb +++ b/app/controllers/templates_controller.rb @@ -1,10 +1,11 @@ # frozen_string_literal: true class TemplatesController < ApplicationController + load_and_authorize_resource :template + before_action :load_base_template, only: %i[new create] def show - @template = current_account.templates.find(params[:id]) submissions = @template.submissions submissions = submissions.active if @template.deleted_at.blank? submissions = Submissions.search(submissions, params[:q]) @@ -15,19 +16,19 @@ class TemplatesController < ApplicationController end def new - @template = current_account.templates.new @template.name = "#{@base_template.name} (Clone)" if @base_template end def edit - @template = current_account.templates.preload(schema_documents: { preview_images_attachments: :blob }) - .find(params[:id]) + ActiveRecord::Associations::Preloader.new( + records: [@template], + associations: [schema_documents: { preview_images_attachments: :blob }] + ).call render :edit, layout: 'plain' end def create - @template = current_account.templates.new(template_params) @template.author = current_user @template.assign_attributes(@base_template.slice(:fields, :schema, :submitters)) if @base_template @@ -41,7 +42,6 @@ class TemplatesController < ApplicationController end def destroy - @template = current_account.templates.find(params[:id]) @template.update!(deleted_at: Time.current) redirect_back(fallback_location: root_path, notice: 'Template has been archived.') @@ -56,8 +56,6 @@ class TemplatesController < ApplicationController def load_base_template return if params[:base_template_id].blank? - @base_template = current_account.templates - .preload(documents_attachments: :preview_images_attachments) - .find_by(id: params[:base_template_id]) + @base_template = current_account.templates.find_by(id: params[:base_template_id]) end end diff --git a/app/controllers/templates_restore_controller.rb b/app/controllers/templates_restore_controller.rb index d691bed2..954743ce 100644 --- a/app/controllers/templates_restore_controller.rb +++ b/app/controllers/templates_restore_controller.rb @@ -1,11 +1,11 @@ # frozen_string_literal: true class TemplatesRestoreController < ApplicationController - def create - template = current_account.templates.find(params[:template_id]) + load_and_authorize_resource :template - template.update!(deleted_at: nil) + def create + @template.update!(deleted_at: nil) - redirect_to template_path(template), notice: 'Template has been unarchived' + redirect_to template_path(@template), notice: 'Template has been unarchived' end end diff --git a/app/controllers/templates_uploads_controller.rb b/app/controllers/templates_uploads_controller.rb index ffe5a148..fd57f577 100644 --- a/app/controllers/templates_uploads_controller.rb +++ b/app/controllers/templates_uploads_controller.rb @@ -1,18 +1,20 @@ # frozen_string_literal: true class TemplatesUploadsController < ApplicationController + load_and_authorize_resource :template, parent: false + def create - template = current_account.templates.new(author: current_user) - template.name = File.basename(params[:files].first.original_filename, '.*') + @template.author = current_user + @template.name = File.basename(params[:files].first.original_filename, '.*') - template.save! + @template.save! - documents = Templates::CreateAttachments.call(template, params) + documents = Templates::CreateAttachments.call(@template, params) schema = documents.map { |doc| { attachment_uuid: doc.uuid, name: doc.filename.base } } - template.update!(schema:) + @template.update!(schema:) - redirect_to edit_template_path(template) + redirect_to edit_template_path(@template) end end diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 989d90b4..7cb672ce 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -1,26 +1,20 @@ # frozen_string_literal: true class UsersController < ApplicationController - before_action :load_user, only: %i[edit update destroy] + load_and_authorize_resource :user, only: %i[index edit new update destroy] + + before_action :build_user, only: :create + authorize_resource :user, only: :create def index - @pagy, @users = pagy(current_account.users.active.order(id: :desc)) + @pagy, @users = pagy(@users.active.order(id: :desc)) end - def new - @user = current_account.users.new - end + def new; end def edit; end def create - @user = current_account.users.find_by(email: user_params[:email])&.tap do |user| - user.assign_attributes(user_params) - user.deleted_at = nil - end - - @user ||= current_account.users.new(user_params) - if @user.save UserMailer.invitation_email(@user).deliver_later! @@ -33,7 +27,7 @@ class UsersController < ApplicationController def update return redirect_to settings_users_path, notice: 'Unable to update user.' if Docuseal.demo? - if @user.update(user_params.compact_blank) + if @user.update(user_params.compact_blank.except(current_user == @user ? :role : nil)) redirect_to settings_users_path, notice: 'User has been updated' else render turbo_stream: turbo_stream.replace(:modal, template: 'users/edit'), status: :unprocessable_entity @@ -52,11 +46,18 @@ class UsersController < ApplicationController private - def load_user - @user = current_account.users.find(params[:id]) + def build_user + @user = current_account.users.find_by(email: user_params[:email])&.tap do |user| + user.assign_attributes(user_params) + user.deleted_at = nil + end + + @user ||= current_account.users.new(user_params) + + @user end def user_params - params.require(:user).permit(:email, :first_name, :last_name, :password) + params.require(:user).permit(:email, :first_name, :last_name, :password, :role) end end diff --git a/app/controllers/verify_pdf_signature_controller.rb b/app/controllers/verify_pdf_signature_controller.rb index 90d2dc52..49d9e44a 100644 --- a/app/controllers/verify_pdf_signature_controller.rb +++ b/app/controllers/verify_pdf_signature_controller.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true class VerifyPdfSignatureController < ApplicationController + skip_authorization_check + def create pdfs = params[:files].map do |file| diff --git a/app/controllers/webhook_settings_controller.rb b/app/controllers/webhook_settings_controller.rb index cb36d44d..2382dad3 100644 --- a/app/controllers/webhook_settings_controller.rb +++ b/app/controllers/webhook_settings_controller.rb @@ -1,15 +1,12 @@ # frozen_string_literal: true class WebhookSettingsController < ApplicationController - def show - @encrypted_config = - current_account.encrypted_configs.find_or_initialize_by(key: EncryptedConfig::WEBHOOK_URL_KEY) - end + before_action :load_encrypted_config + authorize_resource :encrypted_config, parent: false - def create - @encrypted_config = - current_account.encrypted_configs.find_or_initialize_by(key: EncryptedConfig::WEBHOOK_URL_KEY) + def show; end + def create @encrypted_config.update!(encrypted_config_params) redirect_back(fallback_location: settings_webhooks_path, notice: 'Webhook URL has been saved.') @@ -25,6 +22,11 @@ class WebhookSettingsController < ApplicationController private + def load_encrypted_config + @encrypted_config = + current_account.encrypted_configs.find_or_initialize_by(key: EncryptedConfig::WEBHOOK_URL_KEY) + end + def encrypted_config_params params.require(:encrypted_config).permit(:value) end diff --git a/app/views/accounts/show.html.erb b/app/views/accounts/show.html.erb index bb97c819..96282853 100644 --- a/app/views/accounts/show.html.erb +++ b/app/views/accounts/show.html.erb @@ -29,9 +29,11 @@ <% end %> <% end %> -
- <%= f.button button_title(title: 'Update', disabled_with: 'Updating'), class: 'base-button' %> -
+ <% if can?(:update, current_account) %> +
+ <%= f.button button_title(title: 'Update', disabled_with: 'Updating'), class: 'base-button' %> +
+ <% end %> <% end %>
diff --git a/app/views/dashboard/index.html.erb b/app/views/dashboard/index.html.erb index c74730eb..4887abba 100644 --- a/app/views/dashboard/index.html.erb +++ b/app/views/dashboard/index.html.erb @@ -6,10 +6,12 @@ <% if params[:q].present? || @pagy.pages > 1 %> <%= render 'shared/search_input' %> <% end %> - <%= render 'templates/upload_button' %> - <%= link_to new_template_path, class: 'btn btn-primary text-base btn-md gap-2', data: { turbo_frame: :modal } do %> - <%= svg_icon('plus', class: 'w-6 h-6 stroke-2') %> - + <% if can?(:create, ::Template) %> + <%= render 'templates/upload_button' %> + <%= link_to new_template_path, class: 'btn btn-primary text-base btn-md gap-2', data: { turbo_frame: :modal } do %> + <%= svg_icon('plus', class: 'w-6 h-6 stroke-2') %> + + <% end %> <% end %> diff --git a/app/views/esign_settings/new.html.erb b/app/views/esign_settings/new.html.erb index 45c36046..a31729f2 100644 --- a/app/views/esign_settings/new.html.erb +++ b/app/views/esign_settings/new.html.erb @@ -14,7 +14,7 @@
<%= f.label :password, 'Password (optional)', class: 'label' %> - <%= f.text_field :password, class: 'base-input' %> + <%= f.password_field :password, class: 'base-input' %>
diff --git a/app/views/esign_settings/show.html.erb b/app/views/esign_settings/show.html.erb index 4b9a9476..43e6782e 100644 --- a/app/views/esign_settings/show.html.erb +++ b/app/views/esign_settings/show.html.erb @@ -40,9 +40,11 @@

Signing Certificates

- <%= link_to new_settings_esign_path, class: 'btn btn-primary btn-md', data: { turbo_frame: 'modal' } do %> - <%= svg_icon('plus', class: 'w-6 h-6') %> - Upload Cert + <% if can?(:create, @encrypted_config) %> + <%= link_to new_settings_esign_path, class: 'btn btn-primary btn-md', data: { turbo_frame: 'modal' } do %> + <%= svg_icon('plus', class: 'w-6 h-6') %> + Upload Cert + <% end %> <% end %>
<%= render 'alert' %> @@ -77,14 +79,14 @@ <%= item['status'] %> - <% else %> + <% elsif can?(:update, @encrypted_config) %> <%= button_to settings_esign_path, method: :put, params: { name: item['name'] }, class: 'btn btn-outline btn-neutral btn-xs whitespace-nowrap', title: 'Delete', data: { turbo_confirm: 'Are you sure?' } do %> Make Default <% end %> <% end %> - <% if item['name'] != EsignSettingsController::DEFAULT_CERT_NAME && item['status'] != 'default' %> + <% if item['name'] != EsignSettingsController::DEFAULT_CERT_NAME && item['status'] != 'default' && can?(:destroy, @encrypted_config) %> <%= button_to settings_esign_path, params: { name: item['name'] }, method: :delete, class: 'btn btn-outline btn-error btn-xs', title: 'Delete', data: { turbo_confirm: 'Are you sure?' } do %> Remove <% end %> diff --git a/app/views/shared/_settings_nav.html.erb b/app/views/shared/_settings_nav.html.erb index b4aad85c..15d3ec4a 100644 --- a/app/views/shared/_settings_nav.html.erb +++ b/app/views/shared/_settings_nav.html.erb @@ -10,33 +10,49 @@ <%= link_to 'Account', settings_account_path, class: 'text-base hover:bg-base-300' %> <% unless Docuseal.multitenant? %> + <% if can?(:read, EncryptedConfig.new(key: EncryptedConfig::EMAIL_SMTP_KEY, account: current_account)) %> +
  • + <%= link_to 'Email', settings_email_index_path, class: 'text-base hover:bg-base-300' %> +
  • + <% end %> + <% if can?(:read, EncryptedConfig.new(key: EncryptedConfig::FILES_STORAGE_KEY, account: current_account)) %> +
  • + <%= link_to 'Storage', settings_storage_index_path, class: 'text-base hover:bg-base-300' %> +
  • + <% end %> + <% if can?(:read, EncryptedConfig.new(key: 'submitter_invitation_sms', account: current_account)) %> +
  • + <%= link_to 'SMS', settings_sms_path, class: 'text-base hover:bg-base-300' %> +
  • + <% end %> + <% end %> + <% if can?(:read, EncryptedConfig.new(key: EncryptedConfig::ESIGN_CERTS_KEY, account: current_account)) %>
  • - <%= link_to 'Email', settings_email_index_path, class: 'text-base hover:bg-base-300' %> -
  • -
  • - <%= link_to 'Storage', settings_storage_index_path, class: 'text-base hover:bg-base-300' %> + <%= link_to 'E-Signature', settings_esign_path, class: 'text-base hover:bg-base-300' %>
  • + <% end %> + <% if can?(:read, User) %>
  • - <%= link_to 'SMS', settings_sms_path, class: 'text-base hover:bg-base-300' %> + <%= link_to 'Team', settings_users_path, class: 'text-base hover:bg-base-300' %>
  • <% end %> -
  • - <%= link_to 'E-Signature', settings_esign_path, class: 'text-base hover:bg-base-300' %> -
  • -
  • - <%= link_to 'Team', settings_users_path, class: 'text-base hover:bg-base-300' %> -
  • <% if Docuseal.demo? || !Docuseal.multitenant? %> + <% if can?(:read, AccessToken) %> +
  • + <%= link_to 'API', settings_api_index_path, class: 'text-base hover:bg-base-300' %> +
  • + <% end %> + <% if can?(:read, EncryptedConfig.new(key: EncryptedConfig::WEBHOOK_URL_KEY, account: current_account)) %> +
  • + <%= link_to 'Webhooks', settings_webhooks_path, class: 'text-base hover:bg-base-300' %> +
  • + <% end %> + <% end %> + <% if can?(:read, AccountConfig) %>
  • - <%= link_to 'API', settings_api_index_path, class: 'text-base hover:bg-base-300' %> -
  • -
  • - <%= link_to 'Webhooks', settings_webhooks_path, class: 'text-base hover:bg-base-300' %> + <%= link_to 'Personalization', settings_personalization_path, class: 'text-base hover:bg-base-300' %>
  • <% end %> -
  • - <%= link_to 'Personalization', settings_personalization_path, class: 'text-base hover:bg-base-300' %> -
  • <% unless Docuseal.demo? %>
  • <%= link_to Docuseal.multitenant? ? console_redirect_index_path : Docuseal::CONSOLE_URL, class: 'text-base hover:bg-base-300', data: { prefetch: false } do %> diff --git a/app/views/submissions/show.html.erb b/app/views/submissions/show.html.erb index e13283f2..dfb42499 100644 --- a/app/views/submissions/show.html.erb +++ b/app/views/submissions/show.html.erb @@ -108,15 +108,15 @@ <%= submitter&.completed_at? ? l(submitter.completed_at.in_time_zone(current_account.timezone), format: :long, locale: current_account.locale) : 'Not completed yet' %> - <% if submitter && submitter.email && !submitter.completed_at %> + <% if submitter && submitter.email && !submitter.completed_at && can?(:update, submitter) %>
    <%= button_to button_title(title: submitter.sent_at? ? 'Re-send Email' : 'Send Email', disabled_with: 'Sending'), submitter_send_email_index_path(submitter_slug: submitter.slug), class: 'btn btn-sm btn-primary w-full' %>
    <% end %> - <% if submitter && submitter.phone && !submitter.completed_at %> + <% if submitter && submitter.phone && !submitter.completed_at && can?(:update, submitter) %> <%= render 'send_sms_button', submitter: %> <% end %> - <% if submitter && !submitter.completed_at? %> + <% if submitter && !submitter.completed_at? && can?(:create, submitter) %> diff --git a/app/views/templates/_template.html.erb b/app/views/templates/_template.html.erb index 1665f630..fb40d738 100644 --- a/app/views/templates/_template.html.erb +++ b/app/views/templates/_template.html.erb @@ -15,20 +15,22 @@
    <%= f.button button_title, class: 'base-button' %> diff --git a/app/views/users/_role_select.html.erb b/app/views/users/_role_select.html.erb new file mode 100644 index 00000000..0f598125 --- /dev/null +++ b/app/views/users/_role_select.html.erb @@ -0,0 +1,13 @@ +
    + <%= f.label :role, class: 'label' %> + <%= f.select :role, nil, {}, class: 'base-select' do %> + + + + <% end %> + "> + <%= svg_icon('info_circle', class: 'w-4 h-4 inline align-text-bottom') %> + Unlock more user roles with DocuSeal Enterprise. + Learn More + +
    diff --git a/app/views/users/index.html.erb b/app/views/users/index.html.erb index ba06d97f..318fb2eb 100644 --- a/app/views/users/index.html.erb +++ b/app/views/users/index.html.erb @@ -3,9 +3,11 @@

    Team

    - <%= link_to new_user_path, class: 'btn btn-primary btn-md gap-2', data: { turbo_frame: 'modal' } do %> - <%= svg_icon('plus', class: 'w-6 h-6') %> - New User + <% if can?(:create, User.new(account: current_account)) %> + <%= link_to new_user_path, class: 'btn btn-primary btn-md gap-2', data: { turbo_frame: 'modal' } do %> + <%= svg_icon('plus', class: 'w-6 h-6') %> + New User + <% end %> <% end %>
    @@ -46,11 +48,15 @@ <%= user.last_sign_in_at ? l(user.last_sign_in_at.in_time_zone(current_account.timezone), format: :short, locale: current_account.locale) : '-' %> - <%= link_to edit_user_path(user), class: 'btn btn-outline btn-xs', title: 'Edit', data: { turbo_frame: 'modal' } do %> - Edit + <% if can?(:update, user) %> + <%= link_to edit_user_path(user), class: 'btn btn-outline btn-xs', title: 'Edit', data: { turbo_frame: 'modal' } do %> + Edit + <% end %> <% end %> - <%= button_to user_path(user), method: :delete, class: 'btn btn-outline btn-error btn-xs', title: 'Delete', data: { turbo_confirm: 'Are you sure?' } do %> - Remove + <% if can?(:destroy, user) && user != current_user %> + <%= button_to user_path(user), method: :delete, class: 'btn btn-outline btn-error btn-xs', title: 'Delete', data: { turbo_confirm: 'Are you sure?' } do %> + Remove + <% end %> <% end %> diff --git a/config/routes.rb b/config/routes.rb index 7380f432..de9beea5 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -52,7 +52,7 @@ Rails.application.routes.draw do resources :templates_archived, only: %i[index], path: 'archived' resources :templates, only: %i[new create edit show destroy] do resources :restore, only: %i[create], controller: 'templates_restore' - resource :archived, only: %i[show], controller: 'templates_archived_submissions' + resources :archived, only: %i[index], controller: 'templates_archived_submissions' resources :submissions, only: %i[new create] resources :submissions_export, only: %i[index new] end diff --git a/lib/ability.rb b/lib/ability.rb new file mode 100644 index 00000000..eeb4acdc --- /dev/null +++ b/lib/ability.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +class Ability + include CanCan::Ability + + def initialize(user) + can :manage, Template, account_id: user.account_id + can :manage, Submission, template: { account_id: user.account_id } + can :manage, Submitter, template: { 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, AccessToken, user_id: user.id + end +end diff --git a/lib/templates/clone_attachments.rb b/lib/templates/clone_attachments.rb index 81948e57..14927a1c 100644 --- a/lib/templates/clone_attachments.rb +++ b/lib/templates/clone_attachments.rb @@ -5,7 +5,7 @@ module Templates module_function def call(template:, original_template:) - original_template.documents.each do |document| + original_template.documents.preload(:preview_images_attachments).each do |document| new_document = ActiveStorage::Attachment.create!( uuid: document.uuid, blob_id: document.blob_id, diff --git a/spec/system/dashboard_spec.rb b/spec/system/dashboard_spec.rb index 2632cbaa..9ca98d3f 100644 --- a/spec/system/dashboard_spec.rb +++ b/spec/system/dashboard_spec.rb @@ -23,6 +23,7 @@ RSpec.describe 'Dashboard Page' do context 'when there are templates' do let!(:authors) { create_list(:user, 5, account:) } let!(:templates) { authors.map { |author| create(:template, account:, author:) } } + let!(:other_template) { create(:template) } before do visit root_path @@ -35,6 +36,7 @@ RSpec.describe 'Dashboard Page' do end expect(page).to have_content('Templates') + expect(page).not_to have_content(other_template.name) expect(page).to have_link('Create', href: new_template_path) end diff --git a/spec/system/team_settings_spec.rb b/spec/system/team_settings_spec.rb index 090e746f..86769ed4 100644 --- a/spec/system/team_settings_spec.rb +++ b/spec/system/team_settings_spec.rb @@ -12,6 +12,7 @@ RSpec.describe 'Team Settings' do context 'when multiple users' do let!(:users) { create_list(:user, 2, account:) } + let!(:other_user) { create(:user) } before do visit settings_users_path @@ -22,6 +23,7 @@ RSpec.describe 'Team Settings' do users.each do |user| expect(page).to have_content(user.full_name) expect(page).to have_content(user.email) + expect(page).not_to have_content(other_user.email) end end end @@ -84,13 +86,7 @@ RSpec.describe 'Team Settings' do end it 'does not allow to remove the current user' do - expect do - accept_confirm('Are you sure?') do - first(:button, 'Delete').click - end - end.not_to(change { User.admins.count }) - - expect(page).to have_content('Unable to remove user') + expect(page).not_to have_content('User has been removed') end end end