diff --git a/.rubocop.yml b/.rubocop.yml index 449462386..8f11d4415 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -220,7 +220,7 @@ Naming/MethodParameterName: Checks for method parameter names that contain capital letters, end in numbers, or do not meet a minimal length. Enabled: true - AllowedNames: e, id, _, ip + AllowedNames: e, id, _, ip, to Lint/BooleanSymbol: Description: 'Check for `:true` and `:false` symbols.' diff --git a/Gemfile b/Gemfile index 422488559..7accb7feb 100644 --- a/Gemfile +++ b/Gemfile @@ -35,6 +35,9 @@ gem 'argon2' # core - state machine gem 'aasm' +# core - authorization +gem 'pundit' + # performance - Memcached gem 'dalli' @@ -151,6 +154,9 @@ group :development, :test do gem 'shoulda-matchers' gem 'test-unit' + # for testing Pundit authorisation policies in RSpec + gem 'pundit-matchers' + # code coverage gem 'coveralls', require: false gem 'simplecov' diff --git a/Gemfile.lock b/Gemfile.lock index 229fed331..8117f1c72 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -378,6 +378,10 @@ GEM pry (>= 0.9.11) public_suffix (3.0.3) puma (3.12.4) + pundit (2.0.1) + activesupport (>= 3.0.0) + pundit-matchers (1.6.0) + rspec-rails (>= 3.0.0) rack (2.2.2) rack-livereload (0.3.17) rack @@ -627,6 +631,8 @@ DEPENDENCIES pry-rescue pry-stack_explorer puma (~> 3.12) + pundit + pundit-matchers rack-livereload rails (= 5.2.4.1) rails-controller-testing diff --git a/app/assets/javascripts/app/models/user.coffee b/app/assets/javascripts/app/models/user.coffee index 3281a0f02..859b752b1 100644 --- a/app/assets/javascripts/app/models/user.coffee +++ b/app/assets/javascripts/app/models/user.coffee @@ -303,7 +303,7 @@ class App.User extends App.Model Checks if requester has given access level on requested. Possible access levels are: read, update and delete - See backend method User#access? + See backend policy UserPolicy requester = App.User.find(1) requested = App.User.find(3) diff --git a/app/controllers/application_channel_controller.rb b/app/controllers/application_channel_controller.rb deleted file mode 100644 index 2117a3ba1..000000000 --- a/app/controllers/application_channel_controller.rb +++ /dev/null @@ -1,47 +0,0 @@ -class ApplicationChannelController < ApplicationController - # Extending controllers has to define following constants: - # PERMISSION = "admin.channel_xyz" - # AREA = "XYZ::Account" - - def index - render json: channels_data - end - - def show - model_show_render(Channel, params) - end - - def create - model_create_render(Channel, channel_params) - end - - def update - model_update_render(Channel, channel_params) - end - - def enable - channel.update!(active: true) - render json: channel - end - - def disable - channel.update!(active: false) - render json: channel - end - - def destroy - channel.destroy! - render json: {} - end - - private - - def channel - @channel ||= Channel.lookup(id: params[:id]) - end - - def channel_params - params.permit!.to_s - end - -end diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index c0bfab08f..50e3f83b2 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -12,5 +12,5 @@ class ApplicationController < ActionController::Base include ApplicationController::PreventsCsrf include ApplicationController::HasSecureContentSecurityPolicyForDownloads include ApplicationController::LogsHttpAccess - include ApplicationController::ChecksAccess + include ApplicationController::Authorizes end diff --git a/app/controllers/application_controller/authenticates.rb b/app/controllers/application_controller/authenticates.rb index 07873bfd1..b4087434b 100644 --- a/app/controllers/application_controller/authenticates.rb +++ b/app/controllers/application_controller/authenticates.rb @@ -4,6 +4,8 @@ module ApplicationController::Authenticates private def permission_check(key) + ActiveSupport::Deprecation.warn("Method 'permission_check' is deprecated. Use Pundit policy and `authorize!` instead.") + if @_token_auth user = Token.check( action: 'api', @@ -76,6 +78,8 @@ module ApplicationController::Authenticates inactive_user: true, ) if user && auth_param[:permission] + ActiveSupport::Deprecation.warn("Paramter ':permission' is deprecated. Use Pundit policy and `authorize!` instead.") + user = Token.check( action: 'api', name: token_string, @@ -95,6 +99,8 @@ module ApplicationController::Authenticates Time.zone.today >= token.expires_at raise Exceptions::NotAuthorized, 'Not authorized (token expired)!' end + + @_token = token # remember for Pundit authorization / permit! end @_token_auth = token_string # remember for permission_check @@ -152,7 +158,14 @@ module ApplicationController::Authenticates def authentication_check_prerequesits(user, auth_type, auth_param) raise Exceptions::NotAuthorized, 'Maintenance mode enabled!' if in_maintenance_mode?(user) raise Exceptions::NotAuthorized, 'User is inactive!' if !user.active - raise Exceptions::NotAuthorized, 'Not authorized (user)!' if auth_param[:permission] && !user.permissions?(auth_param[:permission]) + + if auth_param[:permission] + ActiveSupport::Deprecation.warn("Parameter ':permission' is deprecated. Use Pundit policy and `authorize!` instead.") + + if !user.permissions?(auth_param[:permission]) + raise Exceptions::NotAuthorized, 'Not authorized (user)!' + end + end current_user_set(user, auth_type) user_device_log(user, auth_type) diff --git a/app/controllers/application_controller/authorizes.rb b/app/controllers/application_controller/authorizes.rb new file mode 100644 index 000000000..7c246ac0d --- /dev/null +++ b/app/controllers/application_controller/authorizes.rb @@ -0,0 +1,61 @@ +module ApplicationController::Authorizes + extend ActiveSupport::Concern + include Pundit + + private + + def authorize!(record = policy_record, query = nil) + authorize(record, query) + end + + def authorized?(record = policy_record, query = nil) + authorize!(record, query) + true + rescue Exceptions::NotAuthorized, Pundit::NotAuthorizedError + false + end + + def policy_record + # check permissions in matching Pundit policy + # Controllers namspace is used (See: https://github.com/varvet/pundit#policy-namespacing) + # [:controllers, self] => Controllers::RolesControllerPolicy + [:controllers, self] + end + + # We need a special UserContext when authorizing in controller context + # because of Token authentication which has it's own permissions + # See: https://github.com/varvet/pundit#additional-context + # We use a Delegator here to have transparent / DuckType access + # to the underlying User instance in the Policy + class UserContext < Delegator + + def initialize(user, token) + @user = user + @token = token + end + + def __getobj__ + @user + end + + def permissions!(permissions) + raise Exceptions::NotAuthorized, 'authentication failed' if !@user + raise Exceptions::NotAuthorized, 'Not authorized (user)!' if !@user.permissions?(permissions) + return if !@token + return if @token.permissions?(permissions) + + raise Exceptions::NotAuthorized, 'Not authorized (token)!' + end + + def permissions?(permissions) + permissions!(permissions) + true + rescue Exceptions::NotAuthorized + false + end + end + + def pundit_user + @pundit_user ||= UserContext.new(current_user, @_token) + end +end diff --git a/app/controllers/application_controller/checks_access.rb b/app/controllers/application_controller/checks_access.rb deleted file mode 100644 index 7d246e541..000000000 --- a/app/controllers/application_controller/checks_access.rb +++ /dev/null @@ -1,9 +0,0 @@ -module ApplicationController::ChecksAccess - extend ActiveSupport::Concern - - private - - def access!(instance, access) - instance.access!(current_user, access) - end -end diff --git a/app/controllers/application_controller/handles_errors.rb b/app/controllers/application_controller/handles_errors.rb index ca8f8bf5d..0f19e2dfd 100644 --- a/app/controllers/application_controller/handles_errors.rb +++ b/app/controllers/application_controller/handles_errors.rb @@ -11,6 +11,7 @@ module ApplicationController::HandlesErrors rescue_from ArgumentError, with: :unprocessable_entity rescue_from Exceptions::UnprocessableEntity, with: :unprocessable_entity rescue_from Exceptions::NotAuthorized, with: :unauthorized + rescue_from Pundit::NotAuthorizedError, with: :pundit_not_authorized_error end def not_found(e) @@ -32,12 +33,22 @@ module ApplicationController::HandlesErrors end def unauthorized(e) + logger.info { e } error = humanize_error(e) response.headers['X-Failure'] = error.fetch(:error_human, error[:error]) respond_to_exception(e, :unauthorized) http_log end + def pundit_not_authorized_error(e) + logger.info { e } + # check if a special authorization_error should be shown in the result payload + # which was raised in one of the policies. Fall back to a simple "Not authorized" + # error to hide actual cause for security reasons. + exeption = e.policy.custom_exception || Exceptions::NotAuthorized.new + unauthorized(exeption) + end + private def respond_to_exception(e, status) diff --git a/app/controllers/application_controller/has_user.rb b/app/controllers/application_controller/has_user.rb index 3003d6847..b4d0429ec 100644 --- a/app/controllers/application_controller/has_user.rb +++ b/app/controllers/application_controller/has_user.rb @@ -95,10 +95,4 @@ module ApplicationController::HasUser session[:user_agent] = request.env['HTTP_USER_AGENT'] end - - def valid_session_with_user - return true if current_user - - raise Exceptions::UnprocessableEntity, 'No session user!' - end end diff --git a/app/controllers/applications_controller.rb b/app/controllers/applications_controller.rb index c1ed90481..13cda2dba 100644 --- a/app/controllers/applications_controller.rb +++ b/app/controllers/applications_controller.rb @@ -1,7 +1,7 @@ # Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/ class ApplicationsController < ApplicationController - prepend_before_action { authentication_check(permission: 'admin.api') } + prepend_before_action { authentication_check && authorize! } def index all = Doorkeeper::Application.all diff --git a/app/controllers/calendar_subscriptions_controller.rb b/app/controllers/calendar_subscriptions_controller.rb index 189e79c30..43fcd808b 100644 --- a/app/controllers/calendar_subscriptions_controller.rb +++ b/app/controllers/calendar_subscriptions_controller.rb @@ -1,6 +1,6 @@ # Copyright (C) 2012-2015 Zammad Foundation, http://zammad-foundation.org/ class CalendarSubscriptionsController < ApplicationController - prepend_before_action { authentication_check( { basic_auth_promt: true, permission: 'user_preferences.calendar' } ) } + prepend_before_action { authentication_check(basic_auth_promt: true) && authorize! } # @path [GET] /calendar_subscriptions # diff --git a/app/controllers/calendars_controller.rb b/app/controllers/calendars_controller.rb index e16e188ad..a1bfa0e3e 100644 --- a/app/controllers/calendars_controller.rb +++ b/app/controllers/calendars_controller.rb @@ -1,8 +1,7 @@ # Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/ class CalendarsController < ApplicationController - prepend_before_action -> { authentication_check(permission: 'admin.calendar') }, only: %i[init index show create update destroy] - prepend_before_action -> { authentication_check(permission: 'admin') }, only: %i[timezones] + prepend_before_action { authentication_check && authorize! } def init assets = {} diff --git a/app/controllers/channels_email_controller.rb b/app/controllers/channels_email_controller.rb index beb06fb66..5416107d2 100644 --- a/app/controllers/channels_email_controller.rb +++ b/app/controllers/channels_email_controller.rb @@ -1,7 +1,7 @@ # Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/ class ChannelsEmailController < ApplicationController - prepend_before_action { authentication_check(permission: 'admin.channel_email') } + prepend_before_action { authentication_check && authorize! } def index system_online_service = Setting.get('system_online_service') diff --git a/app/controllers/channels_facebook_controller.rb b/app/controllers/channels_facebook_controller.rb index 8549fee85..0ab4b675b 100644 --- a/app/controllers/channels_facebook_controller.rb +++ b/app/controllers/channels_facebook_controller.rb @@ -1,7 +1,7 @@ # Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/ class ChannelsFacebookController < ApplicationController - prepend_before_action { authentication_check(permission: 'admin.channel_facebook') } + prepend_before_action { authentication_check && authorize! } def index assets = {} diff --git a/app/controllers/channels_sms_controller.rb b/app/controllers/channels_sms_controller.rb index e9cd354e3..171b9ab5c 100644 --- a/app/controllers/channels_sms_controller.rb +++ b/app/controllers/channels_sms_controller.rb @@ -1,8 +1,5 @@ -class ChannelsSmsController < ApplicationChannelController - PERMISSION = 'admin.channel_sms'.freeze - AREA = ['Sms::Notification'.freeze, 'Sms::Account'.freeze].freeze - - prepend_before_action -> { authentication_check(permission: self.class::PERMISSION) }, except: [:webhook] +class ChannelsSmsController < ApplicationController + prepend_before_action -> { authentication_check && authorize! }, except: [:webhook] skip_before_action :verify_csrf_token, only: [:webhook] def index @@ -15,6 +12,33 @@ class ChannelsSmsController < ApplicationChannelController } end + def show + model_show_render(Channel, params) + end + + def create + model_create_render(Channel, channel_params) + end + + def update + model_update_render(Channel, channel_params) + end + + def enable + channel.update!(active: true) + render json: channel + end + + def disable + channel.update!(active: false) + render json: channel + end + + def destroy + channel.destroy! + render json: {} + end + def test raise 'Missing parameter options.adapter' if params[:options][:adapter].blank? @@ -49,13 +73,17 @@ class ChannelsSmsController < ApplicationChannelController private + def channel + @channel ||= Channel.lookup(id: params[:id]) + end + def test_options params.permit(:recipient, :message) end def channel_params raise 'Missing area params' if params[:area].blank? - if !self.class::AREA.include?(params[:area]) + if ['Sms::Notification', 'Sms::Account'].exclude?(params[:area]) raise "Invalid area '#{params[:area]}'!" end raise 'Missing options params' if params[:options].blank? diff --git a/app/controllers/channels_telegram_controller.rb b/app/controllers/channels_telegram_controller.rb index e2cd6f760..cc1de568d 100644 --- a/app/controllers/channels_telegram_controller.rb +++ b/app/controllers/channels_telegram_controller.rb @@ -1,7 +1,7 @@ # Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/ class ChannelsTelegramController < ApplicationController - prepend_before_action -> { authentication_check(permission: 'admin.channel_telegram') }, except: [:webhook] + prepend_before_action -> { authentication_check && authorize! }, except: [:webhook] skip_before_action :verify_csrf_token, only: [:webhook] def index diff --git a/app/controllers/channels_twitter_controller.rb b/app/controllers/channels_twitter_controller.rb index fdd4d852a..a19e42bd3 100644 --- a/app/controllers/channels_twitter_controller.rb +++ b/app/controllers/channels_twitter_controller.rb @@ -2,7 +2,7 @@ require_dependency 'channel/driver/twitter' class ChannelsTwitterController < ApplicationController - prepend_before_action -> { authentication_check(permission: 'admin.channel_twitter') }, except: %i[webhook_incoming webhook_verify] + prepend_before_action -> { authentication_check && authorize! }, except: %i[webhook_incoming webhook_verify] skip_before_action :verify_csrf_token, only: %i[webhook_incoming webhook_verify] before_action :validate_webhook_signature!, only: :webhook_incoming diff --git a/app/controllers/chat_sessions_controller.rb b/app/controllers/chat_sessions_controller.rb index e6628b304..0597425cf 100644 --- a/app/controllers/chat_sessions_controller.rb +++ b/app/controllers/chat_sessions_controller.rb @@ -1,7 +1,7 @@ # Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/ class ChatSessionsController < ApplicationController - prepend_before_action { authentication_check(permission: 'chat.agent') } + prepend_before_action { authentication_check && authorize! } def show model_show_render(Chat::Session, params) diff --git a/app/controllers/chats_controller.rb b/app/controllers/chats_controller.rb index 8954eb187..0c46cffbf 100644 --- a/app/controllers/chats_controller.rb +++ b/app/controllers/chats_controller.rb @@ -1,7 +1,7 @@ # Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/ class ChatsController < ApplicationController - prepend_before_action { authentication_check(permission: 'admin.channel_chat') } + prepend_before_action { authentication_check && authorize! } def index chat_ids = [] diff --git a/app/controllers/concerns/checks_user_attributes_by_current_user_permission.rb b/app/controllers/concerns/checks_user_attributes_by_current_user_permission.rb index ab8b5e3da..eae06c9cd 100644 --- a/app/controllers/concerns/checks_user_attributes_by_current_user_permission.rb +++ b/app/controllers/concerns/checks_user_attributes_by_current_user_permission.rb @@ -4,12 +4,11 @@ module ChecksUserAttributesByCurrentUserPermission private def check_attributes_by_current_user_permission(params) + authorize! + # admins can do whatever they want return true if current_user.permissions?('admin.user') - # non-agents (customers) can't set anything - raise Exceptions::NotAuthorized if !current_user.permissions?('ticket.agent') - # regular agents are not allowed to set Groups and Roles %w[Role Group].each do |model| diff --git a/app/controllers/cti_controller.rb b/app/controllers/cti_controller.rb index a868bb6d3..b6bce1da6 100644 --- a/app/controllers/cti_controller.rb +++ b/app/controllers/cti_controller.rb @@ -1,7 +1,7 @@ # Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/ class CtiController < ApplicationController - prepend_before_action { authentication_check(permission: 'cti.agent') } + prepend_before_action { authentication_check && authorize! } # list current caller log # GET /api/v1/cti/log diff --git a/app/controllers/email_addresses_controller.rb b/app/controllers/email_addresses_controller.rb index 382ccbd9c..67ed854fc 100644 --- a/app/controllers/email_addresses_controller.rb +++ b/app/controllers/email_addresses_controller.rb @@ -1,7 +1,7 @@ # Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/ class EmailAddressesController < ApplicationController - prepend_before_action :authentication_check + prepend_before_action { authentication_check && authorize! } =begin @@ -46,7 +46,6 @@ curl http://localhost/api/v1/email_addresses.json -v -u #{login}:#{password} =end def index - permission_check(['admin.channel_email', 'ticket.agent']) model_index_render(EmailAddress, params) end @@ -68,7 +67,6 @@ curl http://localhost/api/v1/email_addresses/#{id}.json -v -u #{login}:#{passwor =end def show - permission_check(['admin.channel_email', 'ticket.agent']) model_show_render(EmailAddress, params) end @@ -99,7 +97,6 @@ curl http://localhost/api/v1/email_addresses.json -v -u #{login}:#{password} -H =end def create - permission_check('admin.channel_email') model_create_render(EmailAddress, params) end @@ -130,7 +127,6 @@ curl http://localhost/api/v1/email_addresses/#{id}.json -v -u #{login}:#{passwor =end def update - permission_check('admin.channel_email') model_update_render(EmailAddress, params) end @@ -148,7 +144,6 @@ curl http://localhost/api/v1/email_addresses/#{id}.json -v -u #{login}:#{passwor =end def destroy - permission_check('admin.channel_email') model_destroy_render(EmailAddress, params) end end diff --git a/app/controllers/external_credentials_controller.rb b/app/controllers/external_credentials_controller.rb index 63ab17db1..8f9eb704b 100644 --- a/app/controllers/external_credentials_controller.rb +++ b/app/controllers/external_credentials_controller.rb @@ -1,7 +1,7 @@ # Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/ class ExternalCredentialsController < ApplicationController - prepend_before_action :permission_check + prepend_before_action { authentication_check && authorize! } def index model_index_render(ExternalCredential, params) @@ -53,29 +53,4 @@ class ExternalCredentialsController < ApplicationController def app_url(provider, channel_id) ExternalCredential.app_url(provider, channel_id) end - - def permission_check - if params[:id].present? && ExternalCredential.exists?(params[:id]) - external_credential = ExternalCredential.find(params[:id]) - raise 'No such ExternalCredential!' if !external_credential - - authentication_check(permission: ["admin.channel_#{external_credential.name}"]) - return - end - - if params[:name].present? || params[:provider].present? - if params[:name].present? - name = params[:name].downcase - elsif params[:provider].present? - name = params[:provider].downcase - else - raise 'Missing name/provider!' - end - authentication_check(permission: ["admin.channel_#{name}"]) - return - end - - authentication_check(permission: ['admin']) - end - end diff --git a/app/controllers/first_steps_controller.rb b/app/controllers/first_steps_controller.rb index 4af07aed1..ede641098 100644 --- a/app/controllers/first_steps_controller.rb +++ b/app/controllers/first_steps_controller.rb @@ -3,9 +3,9 @@ class FirstStepsController < ApplicationController prepend_before_action :authentication_check - def index - return if !access? + before_action -> { render json: [] }, if: -> { !authorized? } + def index invite_agents = false #if User.of_role('Agent').count > 2 # invite_agents = true @@ -169,8 +169,6 @@ class FirstStepsController < ApplicationController end def test_ticket - return if !access? - agent = current_user customer = test_customer from = "#{customer.fullname} <#{customer.email}>" @@ -221,13 +219,6 @@ class FirstStepsController < ApplicationController User.find_by(login: 'nicole.braun@zammad.org') end - def access? - return true if current_user.permissions?(['admin', 'ticket.agent']) - - render json: [] - false - end - def check_availability(result) test_ticket_active = true overview = test_overview diff --git a/app/controllers/form_controller.rb b/app/controllers/form_controller.rb index 00e2f33dc..e6d7c5304 100644 --- a/app/controllers/form_controller.rb +++ b/app/controllers/form_controller.rb @@ -1,13 +1,14 @@ # Copyright (C) 2012-2014 Zammad Foundation, http://zammad-foundation.org/ class FormController < ApplicationController + prepend_before_action -> { authorize! }, only: %i[configuration submit] + skip_before_action :verify_csrf_token before_action :cors_preflight_check after_action :set_access_control_headers_execute skip_before_action :user_device_check def configuration - return if !enabled? return if !fingerprint_exists? return if limit_reached? @@ -23,7 +24,7 @@ class FormController < ApplicationController token: token_gen(params[:fingerprint]) } - if params[:test] && current_user && current_user.permissions?('admin.channel_formular') + if authorized?(policy_record, :test?) result[:enabled] = true end @@ -31,7 +32,6 @@ class FormController < ApplicationController end def submit - return if !enabled? return if !fingerprint_exists? return if !token_valid?(params[:token], params[:fingerprint]) return if limit_reached? @@ -144,6 +144,14 @@ class FormController < ApplicationController private + # we don't wann to tell what the cause for the authorization error is + # so we capture the exception and raise an anonymized one + def authorize!(*) + super + rescue Pundit::NotAuthorizedError + raise Exceptions::NotAuthorized + end + def token_gen(fingerprint) crypt = ActiveSupport::MessageEncryptor.new(Setting.get('application_secret')[0, 32]) fingerprint = "#{Base64.strict_encode64(Setting.get('fqdn'))}:#{Time.zone.now.to_i}:#{Base64.strict_encode64(fingerprint)}" @@ -216,12 +224,4 @@ class FormController < ApplicationController Rails.logger.info 'No fingerprint given!' raise Exceptions::NotAuthorized end - - def enabled? - return true if params[:test] && current_user && current_user.permissions?('admin.channel_formular') - return true if Setting.get('form_ticket_create') - - raise Exceptions::NotAuthorized - end - end diff --git a/app/controllers/getting_started_controller.rb b/app/controllers/getting_started_controller.rb index 46ab046df..023ba3aa0 100644 --- a/app/controllers/getting_started_controller.rb +++ b/app/controllers/getting_started_controller.rb @@ -1,5 +1,6 @@ # Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/ class GettingStartedController < ApplicationController + prepend_before_action -> { authorize! }, only: [:base] =begin @@ -105,10 +106,6 @@ curl http://localhost/api/v1/getting_started -v -u #{login}:#{password} end def base - - # check admin permissions - permission_check('admin.wizard') - # validate url messages = {} settings = {} diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb index 7e8291198..de6f587cc 100644 --- a/app/controllers/groups_controller.rb +++ b/app/controllers/groups_controller.rb @@ -1,7 +1,7 @@ # Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/ class GroupsController < ApplicationController - prepend_before_action { authentication_check(permission: 'admin.group') } + prepend_before_action { authentication_check && authorize! } =begin diff --git a/app/controllers/http_logs_controller.rb b/app/controllers/http_logs_controller.rb index 82ef3eb2c..e0163cd1c 100644 --- a/app/controllers/http_logs_controller.rb +++ b/app/controllers/http_logs_controller.rb @@ -1,11 +1,10 @@ # Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/ class HttpLogsController < ApplicationController - prepend_before_action :authentication_check + prepend_before_action { authentication_check && authorize! } # GET /http_logs/:facility def index - permission_check('admin.*') list = if params[:facility] HttpLog.where(facility: params[:facility]).order(created_at: :desc).limit(params[:limit] || 50) else @@ -16,7 +15,6 @@ class HttpLogsController < ApplicationController # POST /http_logs def create - permission_check('admin.*') model_create_render(HttpLog, params) end diff --git a/app/controllers/integration/exchange_controller.rb b/app/controllers/integration/exchange_controller.rb index 68778bed1..a07c9ff71 100644 --- a/app/controllers/integration/exchange_controller.rb +++ b/app/controllers/integration/exchange_controller.rb @@ -3,7 +3,7 @@ class Integration::ExchangeController < ApplicationController include Integration::ImportJobBase - prepend_before_action { authentication_check(permission: 'admin.integration.exchange') } + prepend_before_action { authentication_check && authorize! } def autodiscover answer_with do diff --git a/app/controllers/integration/idoit_controller.rb b/app/controllers/integration/idoit_controller.rb index 3eae7d3aa..c32202f45 100644 --- a/app/controllers/integration/idoit_controller.rb +++ b/app/controllers/integration/idoit_controller.rb @@ -1,9 +1,7 @@ # Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/ class Integration::IdoitController < ApplicationController - prepend_before_action -> { authentication_check(permission: ['agent.integration.idoit', 'admin.integration.idoit']) }, except: %i[verify query update] - prepend_before_action -> { authentication_check(permission: ['admin.integration.idoit']) }, only: [:verify] - prepend_before_action -> { authentication_check(permission: ['ticket.agent']) }, only: %i[query update] + prepend_before_action { authentication_check && authorize! } def verify response = ::Idoit.verify(params[:api_token], params[:endpoint], params[:client_id]) @@ -39,7 +37,7 @@ class Integration::IdoitController < ApplicationController params[:object_ids] ||= [] ticket = Ticket.find(params[:ticket_id]) ticket.with_lock do - access!(ticket, 'read') + authorize!(ticket, :show?) ticket.preferences[:idoit] ||= {} ticket.preferences[:idoit][:object_ids] = Array(params[:object_ids]).uniq ticket.save! diff --git a/app/controllers/integration/ldap_controller.rb b/app/controllers/integration/ldap_controller.rb index f30470b04..988e39f1e 100644 --- a/app/controllers/integration/ldap_controller.rb +++ b/app/controllers/integration/ldap_controller.rb @@ -6,7 +6,7 @@ require_dependency 'ldap/group' class Integration::LdapController < ApplicationController include Integration::ImportJobBase - prepend_before_action { authentication_check(permission: 'admin.integration.ldap') } + prepend_before_action { authentication_check && authorize! } def discover answer_with do diff --git a/app/controllers/jobs_controller.rb b/app/controllers/jobs_controller.rb index b80988503..01a161841 100644 --- a/app/controllers/jobs_controller.rb +++ b/app/controllers/jobs_controller.rb @@ -1,7 +1,7 @@ # Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/ class JobsController < ApplicationController - prepend_before_action { authentication_check(permission: 'admin.scheduler') } + prepend_before_action { authentication_check && authorize! } def index model_index_render(Job, params) diff --git a/app/controllers/knowledge_base/answer/attachments_controller.rb b/app/controllers/knowledge_base/answer/attachments_controller.rb index def885962..e7e658690 100644 --- a/app/controllers/knowledge_base/answer/attachments_controller.rb +++ b/app/controllers/knowledge_base/answer/attachments_controller.rb @@ -2,7 +2,7 @@ class KnowledgeBase::Answer::AttachmentsController < ApplicationController prepend_before_action :authentication_check - prepend_before_action { permission_check('knowledge_base.editor') } + prepend_before_action :authorize! before_action :fetch_answer diff --git a/app/controllers/knowledge_base/answers_controller.rb b/app/controllers/knowledge_base/answers_controller.rb index 827d85bf2..da47c157a 100644 --- a/app/controllers/knowledge_base/answers_controller.rb +++ b/app/controllers/knowledge_base/answers_controller.rb @@ -3,8 +3,6 @@ class KnowledgeBase::AnswersController < KnowledgeBase::BaseController include HasPublishing - before_action :check_reader_access, only: :show - # accessible outside of specific Knowledge Base # /api/v1/knowledge_bases/recent_answers def recent_answers @@ -83,15 +81,4 @@ class KnowledgeBase::AnswersController < KnowledgeBase::BaseController render json: { id: object.id, assets: assets }, status: :created end - private - - def check_reader_access - return if current_user.permissions? 'knowledge_base.editor' - - object = klass.find params[:id] - - return if object.can_be_published_aasm.internal? || object.can_be_published_aasm.published? - - raise ActiveRecord::RecordNotFound - end end diff --git a/app/controllers/knowledge_base/base_controller.rb b/app/controllers/knowledge_base/base_controller.rb index e5a261e38..719f8b729 100644 --- a/app/controllers/knowledge_base/base_controller.rb +++ b/app/controllers/knowledge_base/base_controller.rb @@ -2,8 +2,7 @@ class KnowledgeBase::BaseController < ApplicationController prepend_before_action :authentication_check - before_action :ensure_editor_or_reader - before_action :ensure_editor, only: %i[create update destroy] + before_action :authorize! def show model_show_render(klass, params_for_permission) @@ -21,21 +20,13 @@ class KnowledgeBase::BaseController < ApplicationController model_destroy_render(klass, params_for_permission) end - private - def klass @klass ||= controller_path.classify.constantize end + private + def params_for_permission params.permit klass.agent_allowed_params end - - def ensure_editor - permission_check 'knowledge_base.editor' - end - - def ensure_editor_or_reader - permission_check 'knowledge_base.*' - end end diff --git a/app/controllers/knowledge_base/categories_controller.rb b/app/controllers/knowledge_base/categories_controller.rb index 6dbc44146..177750041 100644 --- a/app/controllers/knowledge_base/categories_controller.rb +++ b/app/controllers/knowledge_base/categories_controller.rb @@ -2,7 +2,6 @@ class KnowledgeBase::CategoriesController < KnowledgeBase::BaseController before_action :load_knowledge_base, only: %i[reorder_root_categories reorder_categories reorder_answers] - before_action :check_reader_access, only: :show # rubocop:disable Rails/LexicallyScopedActionFilter def reorder_root_categories reorder @knowledge_base.categories.root, params[:ordered_ids], KnowledgeBase::Category @@ -46,14 +45,4 @@ class KnowledgeBase::CategoriesController < KnowledgeBase::BaseController @knowledge_base = KnowledgeBase.find params[:knowledge_base_id] @category = @knowledge_base.categories.find params[:id] if params.key? :id end - - def check_reader_access - return if current_user.permissions? 'knowledge_base.editor' - - object = klass.find params[:id] - - return if object.internal_content? - - raise ActiveRecord::RecordNotFound - end end diff --git a/app/controllers/knowledge_base/manage_controller.rb b/app/controllers/knowledge_base/manage_controller.rb index 158e9c03e..16a181f14 100644 --- a/app/controllers/knowledge_base/manage_controller.rb +++ b/app/controllers/knowledge_base/manage_controller.rb @@ -1,9 +1,5 @@ # Copyright (C) 2012-2017 Zammad Foundation, http://zammad-foundation.org/ class KnowledgeBase::ManageController < KnowledgeBase::BaseController - prepend_before_action { permission_check('admin.knowledge_base') } - skip_before_action :ensure_editor_or_reader - skip_before_action :ensure_editor - def init render json: assets end diff --git a/app/controllers/knowledge_bases_controller.rb b/app/controllers/knowledge_bases_controller.rb index 85c4aaf22..e2bead44a 100644 --- a/app/controllers/knowledge_bases_controller.rb +++ b/app/controllers/knowledge_bases_controller.rb @@ -1,20 +1,10 @@ # Copyright (C) 2012-2017 Zammad Foundation, http://zammad-foundation.org/ class KnowledgeBasesController < KnowledgeBase::BaseController - skip_before_action :ensure_editor_or_reader, only: :init - def init render json: assets(params[:answer_translation_content_ids]) end - def create - raise Exceptions::NotAuthorized - end - - def destroy - raise Exceptions::NotAuthorized - end - private def assets(answer_translation_content_ids = nil) diff --git a/app/controllers/monitoring_controller.rb b/app/controllers/monitoring_controller.rb index 3a855beea..9dae0b050 100644 --- a/app/controllers/monitoring_controller.rb +++ b/app/controllers/monitoring_controller.rb @@ -1,7 +1,10 @@ # Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/ class MonitoringController < ApplicationController - prepend_before_action -> { authentication_check(permission: 'admin.monitoring') }, except: %i[health_check status amount_check] + prepend_before_action { authorize! } + prepend_before_action -> { authentication_check }, except: %i[health_check status amount_check] + prepend_before_action -> { authentication_check_only }, only: %i[health_check status amount_check] + skip_before_action :verify_csrf_token =begin @@ -27,8 +30,6 @@ curl http://localhost/api/v1/monitoring/health_check?token=XXX =end def health_check - token_or_permission_check - issues = [] actions = Set.new @@ -210,8 +211,6 @@ curl http://localhost/api/v1/monitoring/status?token=XXX =end def status - token_or_permission_check - last_login = nil last_login_user = User.where('last_login IS NOT NULL').order(last_login: :desc).limit(1).first if last_login_user @@ -298,8 +297,6 @@ curl http://localhost/api/v1/monitoring/amount_check?token=XXX&periode=1h =end def amount_check - token_or_permission_check - raise Exceptions::UnprocessableEntity, 'periode is missing!' if params[:periode].blank? scale = params[:periode][-1, 1] @@ -370,7 +367,6 @@ curl http://localhost/api/v1/monitoring/amount_check?token=XXX&periode=1h end def token - access_check token = SecureRandom.urlsafe_base64(40) Setting.set('monitoring_token', token) @@ -381,27 +377,8 @@ curl http://localhost/api/v1/monitoring/amount_check?token=XXX&periode=1h end def restart_failed_jobs - access_check - Scheduler.restart_failed_jobs render json: {}, status: :ok end - - private - - def token_or_permission_check - user = authentication_check_only(permission: 'admin.monitoring') - return if user - return if Setting.get('monitoring_token') == params[:token] - - raise Exceptions::NotAuthorized - end - - def access_check - return if Permission.find_by(name: 'admin.monitoring', active: true) - - raise Exceptions::NotAuthorized - end - end diff --git a/app/controllers/object_manager_attributes_controller.rb b/app/controllers/object_manager_attributes_controller.rb index 3a4904dec..d377d0cfb 100644 --- a/app/controllers/object_manager_attributes_controller.rb +++ b/app/controllers/object_manager_attributes_controller.rb @@ -1,7 +1,7 @@ # Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/ class ObjectManagerAttributesController < ApplicationController - prepend_before_action { authentication_check(permission: 'admin.object') } + prepend_before_action { authentication_check && authorize! } # GET /object_manager_attributes_list def list diff --git a/app/controllers/online_notifications_controller.rb b/app/controllers/online_notifications_controller.rb index f60977218..dc1d53fb5 100644 --- a/app/controllers/online_notifications_controller.rb +++ b/app/controllers/online_notifications_controller.rb @@ -1,6 +1,7 @@ # Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/ class OnlineNotificationsController < ApplicationController + prepend_before_action -> { authorize! }, only: %i[show update destroy] prepend_before_action :authentication_check =begin @@ -102,8 +103,6 @@ curl http://localhost/api/v1/online_notifications/#{id} -v -u #{login}:#{passwor =end def show - return if !access? - model_show_render(OnlineNotification, params) end @@ -131,8 +130,6 @@ curl http://localhost/api/v1/online_notifications -v -u #{login}:#{password} -H =end def update - return if !access? - model_update_render(OnlineNotification, params) end @@ -150,8 +147,6 @@ curl http://localhost/api/v1/online_notifications/{id}.json -v -u #{login}:#{pas =end def destroy - return if !access? - model_destroy_render(OnlineNotification, params) end @@ -180,14 +175,4 @@ curl http://localhost/api/v1/online_notifications/mark_all_as_read -v -u #{login end render json: {}, status: :ok end - - private - - def access? - notification = OnlineNotification.find(params[:id]) - return true if notification.user_id == current_user.id - - raise Exceptions::NotAuthorized - end - end diff --git a/app/controllers/organizations_controller.rb b/app/controllers/organizations_controller.rb index 08271edba..2474bc0d0 100644 --- a/app/controllers/organizations_controller.rb +++ b/app/controllers/organizations_controller.rb @@ -1,7 +1,8 @@ # Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/ class OrganizationsController < ApplicationController - prepend_before_action :authentication_check + prepend_before_action -> { authorize! }, except: %i[index show] + prepend_before_action { authentication_check } =begin @@ -59,15 +60,7 @@ curl http://localhost/api/v1/organizations -v -u #{login}:#{password} per_page = 500 end - # only allow customer to fetch his own organization - organizations = [] - if !current_user.permissions?(['admin.organization', 'ticket.agent']) - if current_user.organization_id - organizations = Organization.where(id: current_user.organization_id).order(id: :asc).offset(offset).limit(per_page) - end - else - organizations = Organization.all.order(id: :asc).offset(offset).limit(per_page) - end + organizations = policy_scope(Organization).order(id: :asc).offset(offset).limit(per_page) if response_expand? list = [] @@ -117,14 +110,17 @@ curl http://localhost/api/v1/organizations/#{id} -v -u #{login}:#{password} =end def show + begin + authorize! + rescue Pundit::NotAuthorizedError + # we have a special case here where Users that have no + # organization can request any organization_id but get + # an empty response. However, users with an organization_id + # get that error + raise if current_user.organization_id - # only allow customer to fetch his own organization - if !current_user.permissions?(['admin.organization', 'ticket.agent']) - if !current_user.organization_id - render json: {} - return - end - raise Exceptions::NotAuthorized if params[:id].to_i != current_user.organization_id + render json: {} + return end if response_expand? @@ -168,7 +164,6 @@ curl http://localhost/api/v1/organizations -v -u #{login}:#{password} -H "Conten =end def create - permission_check(['admin.organization', 'ticket.agent']) model_create_render(Organization, params) end @@ -199,7 +194,6 @@ curl http://localhost/api/v1/organizations -v -u #{login}:#{password} -H "Conten =end def update - permission_check(['admin.organization', 'ticket.agent']) model_update_render(Organization, params) end @@ -217,15 +211,12 @@ curl http://localhost/api/v1/organization/{id} -v -u #{login}:#{password} -H "Co =end def destroy - permission_check(['admin.organization', 'ticket.agent']) model_references_check(Organization, params) model_destroy_render(Organization, params) end # GET /api/v1/organizations/search def search - raise Exceptions::NotAuthorized if !current_user.permissions?(['admin.organization', 'ticket.agent']) - per_page = params[:per_page] || params[:limit] || 100 per_page = per_page.to_i if per_page > 500 @@ -301,8 +292,6 @@ curl http://localhost/api/v1/organization/{id} -v -u #{login}:#{password} -H "Co # GET /api/v1/organizations/history/1 def history - raise Exceptions::NotAuthorized if !current_user.permissions?(['admin.organization', 'ticket.agent']) - # get organization data organization = Organization.find(params[:id]) @@ -319,7 +308,6 @@ curl http://localhost/api/v1/organization/{id} -v -u #{login}:#{password} -H "Co # @response_message 200 File download. # @response_message 401 Invalid session. def import_example - permission_check('admin.organization') send_data( Organization.csv_example, filename: 'organization-example.csv', @@ -338,7 +326,6 @@ curl http://localhost/api/v1/organization/{id} -v -u #{login}:#{password} -H "Co # @response_message 201 Import started. # @response_message 401 Invalid session. def import_start - permission_check('admin.user') string = params[:data] if string.blank? && params[:file].present? string = params[:file].read.force_encoding('utf-8') diff --git a/app/controllers/overviews_controller.rb b/app/controllers/overviews_controller.rb index 2a717bf5b..26b1dca44 100644 --- a/app/controllers/overviews_controller.rb +++ b/app/controllers/overviews_controller.rb @@ -1,7 +1,7 @@ # Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/ class OverviewsController < ApplicationController - prepend_before_action { authentication_check(permission: 'admin.overview') } + prepend_before_action { authentication_check && authorize! } =begin diff --git a/app/controllers/packages_controller.rb b/app/controllers/packages_controller.rb index 1c5e0f38f..fc298a7bc 100644 --- a/app/controllers/packages_controller.rb +++ b/app/controllers/packages_controller.rb @@ -1,7 +1,7 @@ # Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/ class PackagesController < ApplicationController - prepend_before_action { authentication_check(permission: 'admin.package') } + prepend_before_action { authentication_check && authorize! } # GET /api/v1/packages def index diff --git a/app/controllers/postmaster_filters_controller.rb b/app/controllers/postmaster_filters_controller.rb index cb46f7c26..34f84ed8e 100644 --- a/app/controllers/postmaster_filters_controller.rb +++ b/app/controllers/postmaster_filters_controller.rb @@ -1,7 +1,7 @@ # Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/ class PostmasterFiltersController < ApplicationController - prepend_before_action { authentication_check(permission: 'admin.channel_email') } + prepend_before_action { authentication_check && authorize! } =begin diff --git a/app/controllers/proxy_controller.rb b/app/controllers/proxy_controller.rb index 934f70f3a..c53091128 100644 --- a/app/controllers/proxy_controller.rb +++ b/app/controllers/proxy_controller.rb @@ -1,7 +1,7 @@ # Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/ class ProxyController < ApplicationController - prepend_before_action { authentication_check(permission: 'admin.system') } + prepend_before_action { authentication_check && authorize! } # POST /api/v1/proxy def test diff --git a/app/controllers/report_profiles_controller.rb b/app/controllers/report_profiles_controller.rb index d39851568..eb3bdff22 100644 --- a/app/controllers/report_profiles_controller.rb +++ b/app/controllers/report_profiles_controller.rb @@ -1,5 +1,5 @@ class ReportProfilesController < ApplicationController - prepend_before_action { authentication_check(permission: 'admin.report_profile') } + prepend_before_action { authentication_check && authorize! } =begin diff --git a/app/controllers/reports_controller.rb b/app/controllers/reports_controller.rb index 62fd2350a..346a7cbd7 100644 --- a/app/controllers/reports_controller.rb +++ b/app/controllers/reports_controller.rb @@ -1,6 +1,6 @@ # Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/ class ReportsController < ApplicationController - prepend_before_action { authentication_check(permission: 'report') } + prepend_before_action { authentication_check && authorize! } # GET /api/reports/config def reporting_config diff --git a/app/controllers/roles_controller.rb b/app/controllers/roles_controller.rb index 1d6ca533b..4da6aedd9 100644 --- a/app/controllers/roles_controller.rb +++ b/app/controllers/roles_controller.rb @@ -1,7 +1,7 @@ # Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/ class RolesController < ApplicationController - prepend_before_action { authentication_check(permission: 'admin.role') } + prepend_before_action { authentication_check && authorize! } =begin diff --git a/app/controllers/search_controller.rb b/app/controllers/search_controller.rb index 16d87d1d8..bad76eb12 100644 --- a/app/controllers/search_controller.rb +++ b/app/controllers/search_controller.rb @@ -7,10 +7,6 @@ class SearchController < ApplicationController # GET|POST /api/v1/search/:objects def search_generic - - # enable search only for users with valid session - raise Exceptions::NotAuthorized if !current_user - # get params query = params[:query] if query.respond_to?(:permit!) diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index f01cc8a77..d298f2b0e 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -1,7 +1,7 @@ # Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/ class SessionsController < ApplicationController - prepend_before_action :authentication_check, only: %i[switch_to_user list delete] + prepend_before_action -> { authentication_check && authorize! }, only: %i[switch_to_user list delete] skip_before_action :verify_csrf_token, only: %i[show destroy create_omniauth failure_omniauth] skip_before_action :user_device_check, only: %i[create_sso] @@ -97,8 +97,6 @@ class SessionsController < ApplicationController # "switch" to user def switch_to_user - permission_check(['admin.session', 'admin.user']) - # check user if !params[:id] render( @@ -176,7 +174,6 @@ class SessionsController < ApplicationController end def list - permission_check('admin.session') assets = {} sessions_clean = [] SessionHelper.list.each do |session| @@ -197,7 +194,6 @@ class SessionsController < ApplicationController end def delete - permission_check('admin.session') SessionHelper.destroy(params[:id]) render json: {} end diff --git a/app/controllers/settings_controller.rb b/app/controllers/settings_controller.rb index 676e216c5..fdb09dd87 100644 --- a/app/controllers/settings_controller.rb +++ b/app/controllers/settings_controller.rb @@ -1,13 +1,13 @@ # Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/ class SettingsController < ApplicationController - prepend_before_action { authentication_check(permission: 'admin.*') } + prepend_before_action { authentication_check && authorize! } # GET /settings def index list = [] Setting.all.each do |setting| - next if setting.preferences[:permission] && !current_user.permissions?(setting.preferences[:permission]) + next if !authorized?(setting, :show?) list.push setting end @@ -16,7 +16,6 @@ class SettingsController < ApplicationController # GET /settings/1 def show - check_access('read') model_show_render(Setting, params) end @@ -27,14 +26,12 @@ class SettingsController < ApplicationController # PUT /settings/1 def update - check_access('write') clean_params = keep_certain_attributes model_update_render(Setting, clean_params) end # PUT /settings/image/:id def update_image - check_access('write') clean_params = keep_certain_attributes if !clean_params[:logo] @@ -105,20 +102,4 @@ class SettingsController < ApplicationController end params end - - def check_access(type) - setting = Setting.lookup(id: params[:id]) - - if setting.preferences[:permission] && !current_user.permissions?(setting.preferences[:permission]) - raise Exceptions::NotAuthorized, "Not authorized (required #{setting.preferences[:permission].inspect})" - end - - if type == 'write' - return true if !Setting.get('system_online_service') - if setting.preferences && setting.preferences[:online_service_disable] - raise Exceptions::NotAuthorized, 'Not authorized (service disabled)' - end - end - true - end end diff --git a/app/controllers/signatures_controller.rb b/app/controllers/signatures_controller.rb index 4cf1d770c..8431a0bb2 100644 --- a/app/controllers/signatures_controller.rb +++ b/app/controllers/signatures_controller.rb @@ -1,7 +1,7 @@ # Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/ class SignaturesController < ApplicationController - prepend_before_action :authentication_check + prepend_before_action { authentication_check && authorize! } =begin @@ -47,7 +47,6 @@ curl http://localhost/api/v1/signatures.json -v -u #{login}:#{password} =end def index - permission_check(['admin.channel_email', 'ticket.agent']) model_index_render(Signature, params) end @@ -69,7 +68,6 @@ curl http://localhost/api/v1/signatures/#{id}.json -v -u #{login}:#{password} =end def show - permission_check(['admin.channel_email', 'ticket.agent']) model_show_render(Signature, params) end @@ -98,7 +96,6 @@ curl http://localhost/api/v1/signatures.json -v -u #{login}:#{password} -H "Cont =end def create - permission_check(['admin.channel_email']) model_create_render(Signature, params) end @@ -127,7 +124,6 @@ curl http://localhost/api/v1/signatures.json -v -u #{login}:#{password} -H "Cont =end def update - permission_check(['admin.channel_email']) model_update_render(Signature, params) end @@ -142,7 +138,6 @@ Test: =end def destroy - permission_check(['admin.channel_email']) model_destroy_render(Signature, params) end end diff --git a/app/controllers/slas_controller.rb b/app/controllers/slas_controller.rb index a0ac5f602..4ae47b141 100644 --- a/app/controllers/slas_controller.rb +++ b/app/controllers/slas_controller.rb @@ -1,7 +1,7 @@ # Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/ class SlasController < ApplicationController - prepend_before_action { authentication_check(permission: 'admin.sla') } + prepend_before_action { authentication_check && authorize! } =begin diff --git a/app/controllers/tags_controller.rb b/app/controllers/tags_controller.rb index 66810265f..064fe1026 100644 --- a/app/controllers/tags_controller.rb +++ b/app/controllers/tags_controller.rb @@ -1,7 +1,8 @@ # Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/ class TagsController < ApplicationController - prepend_before_action :authentication_check + prepend_before_action -> { authorize! }, only: %i[admin_list admin_create admin_rename admin_delete] + prepend_before_action { authentication_check } # GET /api/v1/tag_search?term=abc def search @@ -60,7 +61,6 @@ class TagsController < ApplicationController # GET /api/v1/tag_list def admin_list - permission_check('admin.tag') list = Tag::Item.order(name: :asc).limit(params[:limit] || 1000) results = [] list.each do |item| @@ -76,14 +76,12 @@ class TagsController < ApplicationController # POST /api/v1/tag_list def admin_create - permission_check('admin.tag') Tag::Item.lookup_by_name_and_create(params[:name]) render json: {} end # PUT /api/v1/tag_list/:id def admin_rename - permission_check('admin.tag') Tag::Item.rename( id: params[:id], name: params[:name], @@ -93,7 +91,6 @@ class TagsController < ApplicationController # DELETE /api/v1/tag_list/:id def admin_delete - permission_check('admin.tag') Tag::Item.remove(params[:id]) render json: {} end diff --git a/app/controllers/taskbar_controller.rb b/app/controllers/taskbar_controller.rb index 31af46b2c..ba8157f63 100644 --- a/app/controllers/taskbar_controller.rb +++ b/app/controllers/taskbar_controller.rb @@ -1,45 +1,35 @@ # Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/ class TaskbarController < ApplicationController + prepend_before_action -> { authorize! }, only: %i[show update destroy] prepend_before_action :authentication_check + before_action :set_task_user_param, only: %i[create update] + def index current_user_tasks = Taskbar.where(user_id: current_user.id) model_index_render_result(current_user_tasks) end def show - taskbar = Taskbar.find(params[:id]) - access_to_taskbar(taskbar) model_create_render(Taskbar, params) end def create - task_user(params) model_create_render(Taskbar, params) end def update - taskbar = Taskbar.find(params[:id]) - access_to_taskbar(taskbar) - task_user(params) model_update_render(Taskbar, params) end def destroy - taskbar = Taskbar.find(params[:id]) - access_to_taskbar(taskbar) model_destroy_render(Taskbar, params) end private - def access_to_taskbar(taskbar) - raise Exceptions::UnprocessableEntity, 'Not allowed to access this task.' if taskbar.user_id != current_user.id - end - - def task_user(params) + def set_task_user_param params[:user_id] = current_user.id end - end diff --git a/app/controllers/templates_controller.rb b/app/controllers/templates_controller.rb index 8f8abce87..950cfce7e 100644 --- a/app/controllers/templates_controller.rb +++ b/app/controllers/templates_controller.rb @@ -1,7 +1,7 @@ # Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/ class TemplatesController < ApplicationController - prepend_before_action :authentication_check + prepend_before_action { authentication_check && authorize! } =begin @@ -47,7 +47,6 @@ curl http://localhost/api/v1/templates.json -v -u #{login}:#{password} =end def index - permission_check(['admin.template', 'ticket.agent']) model_index_render(Template, params) end @@ -69,7 +68,6 @@ curl http://localhost/api/v1/templates/#{id}.json -v -u #{login}:#{password} =end def show - permission_check(['admin.template', 'ticket.agent']) model_show_render(Template, params) end @@ -97,7 +95,6 @@ curl http://localhost/api/v1/templates.json -v -u #{login}:#{password} -H "Conte =end def create - permission_check(['admin.template', 'ticket.agent']) model_create_render(Template, params) end @@ -125,7 +122,6 @@ curl http://localhost/api/v1/templates.json -v -u #{login}:#{password} -H "Conte =end def update - permission_check(['admin.template', 'ticket.agent']) model_update_render(Template, params) end @@ -143,7 +139,6 @@ curl http://localhost/api/v1/templates.json -v -u #{login}:#{password} -H "Conte =end def destroy - permission_check(['admin.template', 'ticket.agent']) model_destroy_render(Template, params) end end diff --git a/app/controllers/text_modules_controller.rb b/app/controllers/text_modules_controller.rb index 0fd3f6f4a..9748a76ae 100644 --- a/app/controllers/text_modules_controller.rb +++ b/app/controllers/text_modules_controller.rb @@ -1,7 +1,7 @@ # Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/ class TextModulesController < ApplicationController - prepend_before_action :authentication_check + prepend_before_action { authentication_check && authorize! } =begin @@ -49,7 +49,6 @@ curl http://localhost/api/v1/text_modules.json -v -u #{login}:#{password} =end def index - permission_check(['admin.text_module', 'ticket.agent']) model_index_render(TextModule, params) end @@ -71,7 +70,6 @@ curl http://localhost/api/v1/text_modules/#{id}.json -v -u #{login}:#{password} =end def show - permission_check(['admin.text_module', 'ticket.agent']) model_show_render(TextModule, params) end @@ -101,7 +99,6 @@ curl http://localhost/api/v1/text_modules.json -v -u #{login}:#{password} -H "Co =end def create - permission_check('admin.text_module') model_create_render(TextModule, params) end @@ -131,7 +128,6 @@ curl http://localhost/api/v1/text_modules.json -v -u #{login}:#{password} -H "Co =end def update - permission_check('admin.text_module') model_update_render(TextModule, params) end @@ -149,7 +145,6 @@ curl http://localhost/api/v1/text_modules.json -v -u #{login}:#{password} -H "Co =end def destroy - permission_check('admin.text_module') model_destroy_render(TextModule, params) end @@ -162,7 +157,6 @@ curl http://localhost/api/v1/text_modules.json -v -u #{login}:#{password} -H "Co # @response_message 200 File download. # @response_message 401 Invalid session. def import_example - permission_check('admin.text_module') csv_string = TextModule.csv_example( col_sep: params[:col_sep] || ',', ) @@ -185,7 +179,6 @@ curl http://localhost/api/v1/text_modules.json -v -u #{login}:#{password} -H "Co # @response_message 201 Import started. # @response_message 401 Invalid session. def import_start - permission_check('admin.text_module') string = params[:data] if string.blank? && params[:file].present? string = params[:file].read.force_encoding('utf-8') diff --git a/app/controllers/ticket_articles_controller.rb b/app/controllers/ticket_articles_controller.rb index 52e72a472..797953be1 100644 --- a/app/controllers/ticket_articles_controller.rb +++ b/app/controllers/ticket_articles_controller.rb @@ -4,18 +4,18 @@ class TicketArticlesController < ApplicationController include CreatesTicketArticles include ClonesTicketArticleAttachments + prepend_before_action -> { authorize! }, only: %i[index import_example import_start] prepend_before_action :authentication_check # GET /articles def index - permission_check('admin') model_index_render(Ticket::Article, params) end # GET /articles/1 def show article = Ticket::Article.find(params[:id]) - access!(article, 'read') + authorize!(article) if response_expand? result = article.attributes_with_association_names @@ -35,15 +35,13 @@ class TicketArticlesController < ApplicationController # GET /ticket_articles/by_ticket/1 def index_by_ticket ticket = Ticket.find(params[:id]) - access!(ticket, 'read') + authorize!(ticket, :show?) articles = [] if response_expand? ticket.articles.each do |article| - - # ignore internal article if customer is requesting - next if article.internal == true && current_user.permissions?('ticket.customer') + next if !authorized?(article, :show?) result = article.attributes_with_association_names articles.push result @@ -57,9 +55,7 @@ class TicketArticlesController < ApplicationController assets = {} record_ids = [] ticket.articles.each do |article| - - # ignore internal article if customer is requesting - next if article.internal == true && current_user.permissions?('ticket.customer') + next if !authorized?(article, :show?) record_ids.push article.id assets = article.assets({}) @@ -72,9 +68,7 @@ class TicketArticlesController < ApplicationController end ticket.articles.each do |article| - - # ignore internal article if customer is requesting - next if article.internal == true && current_user.permissions?('ticket.customer') + next if !authorized?(article, :show?) articles.push article.attributes_with_association_names end @@ -84,7 +78,7 @@ class TicketArticlesController < ApplicationController # POST /articles def create ticket = Ticket.find(params[:ticket_id]) - access!(ticket, 'create') + authorize!(ticket) article = article_create(ticket, params) if response_expand? @@ -105,11 +99,7 @@ class TicketArticlesController < ApplicationController # PUT /articles/1 def update article = Ticket::Article.find(params[:id]) - access!(article, 'change') - - if !current_user.permissions?('ticket.agent') && !current_user.permissions?('admin') - raise Exceptions::NotAuthorized, 'Not authorized (ticket.agent or admin permission required)!' - end + authorize!(article) clean_params = Ticket::Article.association_name_to_id_convert(params) clean_params = Ticket::Article.param_cleanup(clean_params, true) @@ -137,34 +127,15 @@ class TicketArticlesController < ApplicationController # DELETE /api/v1/ticket_articles/:id def destroy article = Ticket::Article.find(params[:id]) - access!(article, 'delete') - - if current_user.permissions?('admin') - article.destroy! - render json: {}, status: :ok - return - end - - article_deletable = - current_user.permissions?('ticket.agent') && - article.created_by_id == current_user.id && - !article.type.communication? - - raise Exceptions::NotAuthorized, 'Not authorized (admin permission required)!' if !article_deletable - - if article_deletable && article.created_at >= 10.minutes.ago - article.destroy! - render json: {}, status: :ok - return - end - - raise Exceptions::NotAuthorized, 'Articles can only be deleted within 10 minutes after creation.' + authorize!(article) + article.destroy! + render json: {}, status: :ok end # POST /ticket_attachment_upload_clone_by_article def ticket_attachment_upload_clone_by_article article = Ticket::Article.find(params[:article_id]) - access!(article.ticket, 'read') + authorize!(article.ticket, :show?) render json: { attachments: article_attachments_clone(article), @@ -174,7 +145,7 @@ class TicketArticlesController < ApplicationController # GET /ticket_attachment/:ticket_id/:article_id/:id def attachment ticket = Ticket.lookup(id: params[:ticket_id]) - access!(ticket, 'read') + authorize!(ticket, :show?) article = Ticket::Article.find(params[:article_id]) if ticket.id != article.ticket_id @@ -185,7 +156,7 @@ class TicketArticlesController < ApplicationController end ticket = article.ticket - access!(ticket, 'read') + authorize!(ticket, :show?) end list = article.attachments || [] @@ -226,7 +197,7 @@ class TicketArticlesController < ApplicationController # GET /ticket_article_plain/1 def article_plain article = Ticket::Article.find(params[:id]) - access!(article, 'read') + authorize!(article, :show?) file = article.as_raw @@ -250,7 +221,6 @@ class TicketArticlesController < ApplicationController # @response_message 200 File download. # @response_message 401 Invalid session. def import_example - permission_check('admin') csv_string = Ticket::Article.csv_example( col_sep: ',', ) @@ -273,7 +243,6 @@ class TicketArticlesController < ApplicationController # @response_message 201 Import started. # @response_message 401 Invalid session. def import_start - permission_check('admin') if Setting.get('import_mode') != true raise 'Only can import tickets if system is in import mode.' end diff --git a/app/controllers/ticket_priorities_controller.rb b/app/controllers/ticket_priorities_controller.rb index 7cc9381ab..fd2c90091 100644 --- a/app/controllers/ticket_priorities_controller.rb +++ b/app/controllers/ticket_priorities_controller.rb @@ -1,35 +1,30 @@ # Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/ class TicketPrioritiesController < ApplicationController - prepend_before_action :authentication_check + prepend_before_action { authentication_check && authorize! } # GET /ticket_priorities def index - permission_check(['admin.object', 'ticket.agent', 'ticket.customer']) model_index_render(Ticket::Priority, params) end # GET /ticket_priorities/1 def show - permission_check(['admin.object', 'ticket.agent', 'ticket.customer']) model_show_render(Ticket::Priority, params) end # POST /ticket_priorities def create - permission_check('admin.object') model_create_render(Ticket::Priority, params) end # PUT /ticket_priorities/1 def update - permission_check('admin.object') model_update_render(Ticket::Priority, params) end # DELETE /ticket_priorities/1 def destroy - permission_check('admin.object') model_references_check(Ticket::Priority, params) model_destroy_render(Ticket::Priority, params) end diff --git a/app/controllers/ticket_states_controller.rb b/app/controllers/ticket_states_controller.rb index 4db94415b..e3d35b350 100644 --- a/app/controllers/ticket_states_controller.rb +++ b/app/controllers/ticket_states_controller.rb @@ -1,37 +1,31 @@ # Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/ class TicketStatesController < ApplicationController - prepend_before_action :authentication_check + prepend_before_action { authentication_check && authorize! } # GET /ticket_states def index - permission_check(['admin.object', 'ticket.agent', 'ticket.customer']) model_index_render(Ticket::State, params) end # GET /ticket_states/1 def show - permission_check(['admin.object', 'ticket.agent', 'ticket.customer']) model_show_render(Ticket::State, params) end # POST /ticket_states def create - permission_check('admin.object') model_create_render(Ticket::State, params) end # PUT /ticket_states/1 def update - permission_check('admin.object') model_update_render(Ticket::State, params) end # DELETE /ticket_states/1 def destroy - permission_check('admin.object') - return if model_references_check(Ticket::State, params) - + model_references_check(Ticket::State, params) model_destroy_render(Ticket::State, params) end end diff --git a/app/controllers/tickets_controller.rb b/app/controllers/tickets_controller.rb index 134c42613..4e814acaf 100644 --- a/app/controllers/tickets_controller.rb +++ b/app/controllers/tickets_controller.rb @@ -6,8 +6,8 @@ class TicketsController < ApplicationController include ChecksUserAttributesByCurrentUserPermission include TicketStats + prepend_before_action -> { authorize! }, only: %i[selector import_example import_start] prepend_before_action :authentication_check - before_action :follow_up_possible_check, only: :update # GET /api/v1/tickets def index @@ -55,7 +55,7 @@ class TicketsController < ApplicationController # GET /api/v1/tickets/1 def show ticket = Ticket.find(params[:id]) - access!(ticket, 'read') + authorize!(ticket) if response_expand? result = ticket.attributes_with_association_names @@ -221,7 +221,8 @@ class TicketsController < ApplicationController # PUT /api/v1/tickets/1 def update ticket = Ticket.find(params[:id]) - access!(ticket, 'change') + authorize!(ticket, :follow_up?) + authorize!(ticket) clean_params = Ticket.association_name_to_id_convert(params) clean_params = Ticket.param_cleanup(clean_params, true) @@ -269,9 +270,7 @@ class TicketsController < ApplicationController # DELETE /api/v1/tickets/1 def destroy ticket = Ticket.find(params[:id]) - access!(ticket, 'delete') - - raise Exceptions::NotAuthorized, 'Not authorized (admin permission required)!' if !current_user.permissions?('admin') + authorize!(ticket) ticket.destroy! @@ -296,7 +295,7 @@ class TicketsController < ApplicationController # get ticket data ticket = Ticket.find(params[:id]) - access!(ticket, 'read') + authorize!(ticket, :show?) # get history of ticket render json: ticket.history_get(true) @@ -385,7 +384,7 @@ class TicketsController < ApplicationController } return end - access!(ticket_master, 'change') + authorize!(ticket_master, :update?) # check slave ticket ticket_slave = Ticket.find_by(id: params[:slave_ticket_id]) @@ -396,7 +395,7 @@ class TicketsController < ApplicationController } return end - access!(ticket_slave, 'change') + authorize!(ticket_slave, :update?) # merge ticket ticket_slave.merge_to( @@ -415,11 +414,11 @@ class TicketsController < ApplicationController # GET /api/v1/ticket_split def ticket_split ticket = Ticket.find(params[:ticket_id]) - access!(ticket, 'read') + authorize!(ticket, :show?) assets = ticket.assets({}) article = Ticket::Article.find(params[:article_id]) - access!(article.ticket, 'read') + authorize!(article.ticket, :show?) assets = article.assets(assets) render json: { @@ -497,8 +496,6 @@ class TicketsController < ApplicationController # GET /api/v1/tickets/selector def selector - permission_check('admin.*') - ticket_count, tickets = Ticket.selectors(params[:condition], limit: 6, execution_time: true) assets = {} @@ -627,7 +624,6 @@ class TicketsController < ApplicationController # @response_message 200 File download. # @response_message 401 Invalid session. def import_example - permission_check('admin') csv_string = Ticket.csv_example( col_sep: ',', ) @@ -650,7 +646,6 @@ class TicketsController < ApplicationController # @response_message 201 Import started. # @response_message 401 Invalid session. def import_start - permission_check('admin') if Setting.get('import_mode') != true raise 'Only can import tickets if system is in import mode.' end @@ -673,16 +668,6 @@ class TicketsController < ApplicationController private - def follow_up_possible_check - ticket = Ticket.find(params[:id]) - - return true if current_user.permissions?('ticket.agent') # agents can always reopen tickets, regardless of group configuration - return true if ticket.group.follow_up_possible != 'new_ticket' # check if the setting for follow_up_possible is disabled - return true if ticket.state.name != 'closed' # check if the ticket state is already closed - - raise Exceptions::UnprocessableEntity, 'Cannot follow-up on a closed ticket. Please create a new ticket.' - end - def ticket_all(ticket) # get attributes to update @@ -698,9 +683,7 @@ class TicketsController < ApplicationController # get related users article_ids = [] ticket.articles.each do |article| - - # ignore internal article if customer is requesting - next if article.internal == true && current_user.permissions?('ticket.customer') + next if !authorized?(article, :show?) article_ids.push article.id assets = article.assets(assets) diff --git a/app/controllers/time_accountings_controller.rb b/app/controllers/time_accountings_controller.rb index f410c593b..260279512 100644 --- a/app/controllers/time_accountings_controller.rb +++ b/app/controllers/time_accountings_controller.rb @@ -1,7 +1,7 @@ # Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/ class TimeAccountingsController < ApplicationController - prepend_before_action { authentication_check(permission: 'admin.time_accounting') } + prepend_before_action { authentication_check && authorize! } def by_ticket diff --git a/app/controllers/translations_controller.rb b/app/controllers/translations_controller.rb index 7a295d4a6..6495ba263 100644 --- a/app/controllers/translations_controller.rb +++ b/app/controllers/translations_controller.rb @@ -1,7 +1,7 @@ # Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/ class TranslationsController < ApplicationController - prepend_before_action :authentication_check, except: [:lang] + prepend_before_action -> { authentication_check && authorize! }, except: [:lang] # GET /translations/lang/:locale def lang @@ -10,7 +10,6 @@ class TranslationsController < ApplicationController # PUT /translations/push def push - permission_check('admin.translation') start = Time.zone.now Translation.push(params[:locale]) if start > Time.zone.now - 4.seconds @@ -21,51 +20,43 @@ class TranslationsController < ApplicationController # POST /translations/sync/:locale def sync - permission_check('admin.translation') Translation.load(params[:locale]) render json: { message: 'ok' }, status: :ok end # POST /translations/reset def reset - permission_check('admin.translation') Translation.reset(params[:locale]) render json: { message: 'ok' }, status: :ok end # GET /translations/admin/lang/:locale def admin - permission_check('admin.translation') render json: Translation.lang(params[:locale], true) end # GET /translations def index - permission_check('admin.translation') model_index_render(Translation, params) end # GET /translations/1 def show - permission_check('admin.translation') model_show_render(Translation, params) end # POST /translations def create - permission_check('admin.translation') model_create_render(Translation, params) end # PUT /translations/1 def update - permission_check('admin.translation') model_update_render(Translation, params) end # DELETE /translations/1 def destroy - permission_check('admin.translation') model_destroy_render(Translation, params) end end diff --git a/app/controllers/triggers_controller.rb b/app/controllers/triggers_controller.rb index f87b252c5..90cd01246 100644 --- a/app/controllers/triggers_controller.rb +++ b/app/controllers/triggers_controller.rb @@ -1,7 +1,7 @@ # Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/ class TriggersController < ApplicationController - prepend_before_action { authentication_check(permission: 'admin.trigger') } + prepend_before_action { authentication_check && authorize! } def index model_index_render(Trigger, params) diff --git a/app/controllers/user_access_token_controller.rb b/app/controllers/user_access_token_controller.rb index 91c252c02..224716014 100644 --- a/app/controllers/user_access_token_controller.rb +++ b/app/controllers/user_access_token_controller.rb @@ -1,7 +1,7 @@ # Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/ class UserAccessTokenController < ApplicationController - prepend_before_action { authentication_check(permission: 'user_preferences.access_token') } + prepend_before_action { authentication_check && authorize! } =begin diff --git a/app/controllers/user_devices_controller.rb b/app/controllers/user_devices_controller.rb index 11169348c..e1740cd79 100644 --- a/app/controllers/user_devices_controller.rb +++ b/app/controllers/user_devices_controller.rb @@ -1,7 +1,7 @@ # Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/ class UserDevicesController < ApplicationController - prepend_before_action { authentication_check(permission: 'user_preferences.device') } + prepend_before_action { authentication_check && authorize! } def index devices = UserDevice.where(user_id: current_user.id).order(updated_at: :desc, name: :asc) diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 257df6ab9..45c3080fb 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -3,6 +3,7 @@ class UsersController < ApplicationController include ChecksUserAttributesByCurrentUserPermission + prepend_before_action -> { authorize! }, only: %i[import_example import_start search history] prepend_before_action :authentication_check, except: %i[create password_reset_send password_reset_verify image] prepend_before_action :authentication_check_only, only: [:create] @@ -27,12 +28,7 @@ class UsersController < ApplicationController per_page = 500 end - # only allow customer to fetch him self - users = if !current_user.permissions?(['admin.user', 'ticket.agent']) - User.where(id: current_user.id).order(id: :asc).offset(offset).limit(per_page) - else - User.all.order(id: :asc).offset(offset).limit(per_page) - end + users = policy_scope(User).order(id: :asc).offset(offset).limit(per_page) if response_expand? list = [] @@ -78,7 +74,7 @@ class UsersController < ApplicationController # @response_message 401 Invalid session. def show user = User.find(params[:id]) - access!(user, 'read') + authorize!(user) if response_expand? result = user.attributes_with_association_names @@ -263,7 +259,7 @@ class UsersController < ApplicationController # @response_message 401 Invalid session. def update user = User.find(params[:id]) - access!(user, 'change') + authorize!(user) # permission check check_attributes_by_current_user_permission(params) @@ -312,7 +308,7 @@ class UsersController < ApplicationController # @response_message 401 Invalid session. def destroy user = User.find(params[:id]) - access!(user, 'delete') + authorize!(user) model_references_check(User, params) model_destroy_render(User, params) @@ -367,8 +363,6 @@ class UsersController < ApplicationController # @response_message 200 [Array] A list of User records matching the search term. # @response_message 401 Invalid session. def search - raise Exceptions::NotAuthorized if !current_user.permissions?(['ticket.agent', 'admin.user']) - per_page = params[:per_page] || params[:limit] || 100 per_page = per_page.to_i if per_page > 500 @@ -472,8 +466,6 @@ class UsersController < ApplicationController # @response_message 200 [History] The History records of the requested User record. # @response_message 401 Invalid session. def history - raise Exceptions::NotAuthorized if !current_user.permissions?(['admin.user', 'ticket.agent']) - # get user data user = User.find(params[:id]) @@ -775,8 +767,6 @@ curl http://localhost/api/v1/users/preferences -v -u #{login}:#{password} -H "Co =end def preferences - raise Exceptions::UnprocessableEntity, 'No current user!' if !current_user - preferences_params = params.except(:controller, :action) if preferences_params.present? @@ -816,8 +806,6 @@ curl http://localhost/api/v1/users/out_of_office -v -u #{login}:#{password} -H " =end def out_of_office - raise Exceptions::UnprocessableEntity, 'No current user!' if !current_user - user = User.find(current_user.id) user.with_lock do user.assign_attributes( @@ -854,8 +842,6 @@ curl http://localhost/api/v1/users/account -v -u #{login}:#{password} -H "Conten =end def account_remove - raise Exceptions::UnprocessableEntity, 'No current user!' if !current_user - # provider + uid to remove raise Exceptions::UnprocessableEntity, 'provider needed!' if !params[:provider] raise Exceptions::UnprocessableEntity, 'uid needed!' if !params[:uid] @@ -934,8 +920,6 @@ curl http://localhost/api/v1/users/avatar -v -u #{login}:#{password} -H "Content =end def avatar_new - return if !valid_session_with_user - # get & validate image file_full = StaticAssets.data_url_attributes(params[:avatar_full]) file_resize = StaticAssets.data_url_attributes(params[:avatar_resize]) @@ -963,8 +947,6 @@ curl http://localhost/api/v1/users/avatar -v -u #{login}:#{password} -H "Content end def avatar_set_default - return if !valid_session_with_user - # get & validate image raise Exceptions::UnprocessableEntity, 'No id of avatar!' if !params[:id] @@ -979,8 +961,6 @@ curl http://localhost/api/v1/users/avatar -v -u #{login}:#{password} -H "Content end def avatar_destroy - return if !valid_session_with_user - # get & validate image raise Exceptions::UnprocessableEntity, 'No id of avatar!' if !params[:id] @@ -996,8 +976,6 @@ curl http://localhost/api/v1/users/avatar -v -u #{login}:#{password} -H "Content end def avatar_list - return if !valid_session_with_user - # list of avatars result = Avatar.list('User', current_user.id) render json: { avatars: result }, status: :ok @@ -1012,7 +990,6 @@ curl http://localhost/api/v1/users/avatar -v -u #{login}:#{password} -H "Content # @response_message 200 File download. # @response_message 401 Invalid session. def import_example - permission_check('admin.user') send_data( User.csv_example, filename: 'user-example.csv', @@ -1031,7 +1008,6 @@ curl http://localhost/api/v1/users/avatar -v -u #{login}:#{password} -H "Content # @response_message 201 Import started. # @response_message 401 Invalid session. def import_start - permission_check('admin.user') string = params[:data] if string.blank? && params[:file].present? string = params[:file].read.force_encoding('utf-8') diff --git a/app/controllers/version_controller.rb b/app/controllers/version_controller.rb index 639a4e72d..dc5d87988 100644 --- a/app/controllers/version_controller.rb +++ b/app/controllers/version_controller.rb @@ -1,7 +1,8 @@ # Copyright (C) 2012-2017 Zammad Foundation, http://zammad-foundation.org/ class VersionController < ApplicationController - prepend_before_action { authentication_check(permission: 'admin.version') } + + prepend_before_action { authentication_check && authorize! } # GET /api/v1/version def index diff --git a/app/models/organization.rb b/app/models/organization.rb index 50354bc20..8e560b28f 100644 --- a/app/models/organization.rb +++ b/app/models/organization.rb @@ -10,7 +10,6 @@ class Organization < ApplicationModel include ChecksHtmlSanitized include HasObjectManagerAttributesValidation - include Organization::ChecksAccess include Organization::Assets include Organization::Search include Organization::SearchIndex diff --git a/app/models/organization/checks_access.rb b/app/models/organization/checks_access.rb deleted file mode 100644 index b1b9e30c7..000000000 --- a/app/models/organization/checks_access.rb +++ /dev/null @@ -1,51 +0,0 @@ -# Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/ -class Organization - module ChecksAccess - extend ActiveSupport::Concern - - # Checks the given access of a given user for an organization. - # - # @param [User] The user that will be checked for given access. - # @param [String] The access that should get checked. - # - # @example - # organization.access?(user, 'read') - # #=> true - # - # @return [Boolean] - def access?(user, access) - - # check customer - if user.permissions?('ticket.customer') - - # access ok if its own organization - return false if access != 'read' - return false if !user.organization_id - - return id == user.organization_id - end - - # check agent - return true if user.permissions?('admin') - return true if user.permissions?('ticket.agent') - - false - end - - # Checks the given access of a given user for an organization and fails with an exception. - # - # @param (see Organization#access?) - # - # @example - # organization.access!(user, 'read') - # - # @raise [NotAuthorized] Gets raised if given user doesn't have the given access. - # - # @return [nil] - def access!(user, access) - return if access?(user, access) - - raise Exceptions::NotAuthorized - end - end -end diff --git a/app/models/recent_view.rb b/app/models/recent_view.rb index fbc18082d..02db62002 100644 --- a/app/models/recent_view.rb +++ b/app/models/recent_view.rb @@ -76,12 +76,11 @@ class RecentView < ApplicationModel end def self.access(object, o_id, user) - object.to_s - .constantize - .try(:lookup, { id: o_id }) - .try(:access?, user, 'read') - rescue NameError - false + record = object.to_s + .safe_constantize + .try(:lookup, { id: o_id }) + + Pundit.policy(user, record).try(:show?) end =begin diff --git a/app/models/ticket.rb b/app/models/ticket.rb index f2c8b3095..98bfd001e 100644 --- a/app/models/ticket.rb +++ b/app/models/ticket.rb @@ -13,7 +13,6 @@ class Ticket < ApplicationModel include HasOnlineNotifications include HasKarmaActivityLog include HasLinks - include Ticket::ChecksAccess include HasObjectManagerAttributesValidation include Ticket::Escalation diff --git a/app/models/ticket/article.rb b/app/models/ticket/article.rb index f12cc0a1b..9b2cb2538 100644 --- a/app/models/ticket/article.rb +++ b/app/models/ticket/article.rb @@ -8,7 +8,6 @@ class Ticket::Article < ApplicationModel include CanCsvImport include HasObjectManagerAttributesValidation - include Ticket::Article::ChecksAccess include Ticket::Article::Assets belongs_to :ticket, optional: true diff --git a/app/models/ticket/article/checks_access.rb b/app/models/ticket/article/checks_access.rb deleted file mode 100644 index ac2bb2c30..000000000 --- a/app/models/ticket/article/checks_access.rb +++ /dev/null @@ -1,43 +0,0 @@ -# Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/ -class Ticket - class Article - module ChecksAccess - extend ActiveSupport::Concern - - # Checks the given access of a given user for a ticket article. - # - # @param [User] The user that will be checked for given access. - # @param [String] The access that should get checked. - # - # @example - # article.access?(user, 'read') - # #=> true - # - # @return [Boolean] - def access?(user, access) - if user.permissions?('ticket.customer') - return false if internal == true - end - - ticket = Ticket.lookup(id: ticket_id) - ticket.access?(user, access) - end - - # Checks the given access of a given user for a ticket article and fails with an exception. - # - # @param (see Ticket::Article#access?) - # - # @example - # article.access!(user, 'read') - # - # @raise [NotAuthorized] Gets raised if given user doesn't have the given access. - # - # @return [nil] - def access!(user, access) - return if access?(user, access) - - raise Exceptions::NotAuthorized - end - end - end -end diff --git a/app/models/ticket/checks_access.rb b/app/models/ticket/checks_access.rb deleted file mode 100644 index 45eadf1e1..000000000 --- a/app/models/ticket/checks_access.rb +++ /dev/null @@ -1,57 +0,0 @@ -# Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/ -class Ticket - module ChecksAccess - extend ActiveSupport::Concern - - # Checks the given access of a given user for a ticket. - # - # @param [User] The user that will be checked for given access. - # @param [String] The access that should get checked. - # - # @example - # ticket.access?(user, 'read') - # #=> true - # - # @return [Boolean] - def access?(user, access) - - # check customer - if user.permissions?('ticket.customer') - - # access ok if its own ticket - return true if customer_id == user.id - - # check organization ticket access - return false if organization_id.blank? - return false if user.organization_id.blank? - return false if organization_id != user.organization_id - - return organization.shared? - end - - # check agent - - # access if requestor is owner - return true if owner_id == user.id - - # access if requestor is in group - user.group_access?(group.id, access) - end - - # Checks the given access of a given user for a ticket and fails with an exception. - # - # @param (see Ticket#access?) - # - # @example - # ticket.access!(user, 'read') - # - # @raise [NotAuthorized] Gets raised if given user doesn't have the given access. - # - # @return [nil] - def access!(user, access) - return if access?(user, access) - - raise Exceptions::NotAuthorized - end - end -end diff --git a/app/models/token.rb b/app/models/token.rb index d5e5d3f8f..48e41ef60 100644 --- a/app/models/token.rb +++ b/app/models/token.rb @@ -79,27 +79,7 @@ returns # add permission check if data[:permission] - return if !user.permissions?(data[:permission]) - return if !token.preferences[:permission] - - local_permissions = data[:permission] - if data[:permission].class != Array - local_permissions = [data[:permission]] - end - match = false - local_permissions.each do |local_permission| - local_permissions = Permission.with_parents(local_permission) - local_permissions.each do |local_permission_name| - next if !token.preferences[:permission].include?(local_permission_name) - - match = true - break - end - next if !match - - break - end - return if !match + return if !token.permissions?(data[:permission]) end # return token user @@ -119,6 +99,17 @@ cleanup old token true end + def permissions?(permissions) + return false if !user.permissions?(permissions) + return false if preferences[:permission].blank? + + Array(permissions).any? do |parentless| + Permission.with_parents(parentless).any? do |permission| + preferences[:permission].include?(permission) + end + end + end + private def generate_token diff --git a/app/models/user.rb b/app/models/user.rb index c2cb3f6b8..c9f2e6d2f 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -14,7 +14,6 @@ class User < ApplicationModel include HasObjectManagerAttributesValidation include HasTicketCreateScreenImpact include User::HasTicketCreateScreenImpact - include User::ChecksAccess include User::Assets include User::Search include User::SearchIndex @@ -421,14 +420,10 @@ returns =end def permissions?(key) - keys = key - if key.class == String - keys = [key] - end - keys.each do |local_key| + Array(key).each do |local_key| list = [] if local_key.match?(/\.\*$/) - local_key.sub!('.*', '.%') + local_key = local_key.sub('.*', '.%') permissions = ::Permission.with_parents(local_key) list = ::Permission.select('preferences').joins(:roles).where('roles.id IN (?) AND roles.active = ? AND (permissions.name IN (?) OR permissions.name LIKE ?) AND permissions.active = ?', role_ids, true, permissions, local_key, true).pluck(:preferences) else diff --git a/app/models/user/checks_access.rb b/app/models/user/checks_access.rb deleted file mode 100644 index c0d111452..000000000 --- a/app/models/user/checks_access.rb +++ /dev/null @@ -1,74 +0,0 @@ -# Copyright (C) 2012-2016 Zammad Foundation, http://zammad-foundation.org/ -class User - module ChecksAccess - extend ActiveSupport::Concern - - # Checks the given access of a given user for another user. - # - # @param [User] The user that will be checked for given access. - # @param [String] The access that should get checked. - # - # @example - # user.access?(user, 'read') - # #=> true - # - # @return [Boolean] - def access?(requester, access) - # full admins can do whatever they want - return true if requester.permissions?('admin') - - send("#{access}able_by?".to_sym, requester) - end - - # Checks the given access of a given user for another user and fails with an exception. - # - # @param (see User#access?) - # - # @example - # user.access!(user, 'read') - # - # @raise [NotAuthorized] Gets raised if given user doesn't have the given access. - # - # @return [nil] - def access!(user, access) - return if access?(user, access) - - raise Exceptions::NotAuthorized - end - - private - - def readable_by?(requester) - return true if own_account?(requester) - return true if requester.permissions?('admin.*') - return true if requester.permissions?('ticket.agent') - # check same organization for customers - return false if !requester.permissions?('ticket.customer') - - same_organization?(requester) - end - - def changeable_by?(requester) - return true if requester.permissions?('admin.user') - # allow agents to change customers - return false if !requester.permissions?('ticket.agent') - - permissions?('ticket.customer') - end - - def deleteable_by?(requester) - requester.permissions?('admin.user') - end - - def own_account?(requester) - id == requester.id - end - - def same_organization?(requester) - return false if organization_id.blank? - return false if requester.organization_id.blank? - - organization_id == requester.organization_id - end - end -end diff --git a/app/policies/application_policy.rb b/app/policies/application_policy.rb new file mode 100644 index 000000000..0ca619226 --- /dev/null +++ b/app/policies/application_policy.rb @@ -0,0 +1,19 @@ +class ApplicationPolicy + include PunditPolicy + + attr_reader :record + + def initialize_context(record) + @record = record + end + + class Scope + include PunditPolicy + + attr_reader :scope + + def initialize_context(scope) + @scope = scope + end + end +end diff --git a/app/policies/controllers/application_controller_policy.rb b/app/policies/controllers/application_controller_policy.rb new file mode 100644 index 000000000..378699a6c --- /dev/null +++ b/app/policies/controllers/application_controller_policy.rb @@ -0,0 +1,33 @@ +class Controllers::ApplicationControllerPolicy < ApplicationPolicy + class_attribute(:action_permissions_map, default: {}) + + def self.inherited(subclass) + subclass.action_permissions_map = action_permissions_map.deep_dup + end + + def self.default_permit!(permissions) + action_permissions_map.default = permissions + end + + def self.permit!(actions, to:) + Array(actions).each do |action| + action_permissions_map[:"#{action}?"] = to + end + end + + def method_missing(missing_method, *) + case (permission = action_permissions_map[missing_method]) + when String, Array + user.permissions!(permission) || true + when Proc + user.permissions!(instance_exec(&permission)) || true + else + super + end + end + + def respond_to_missing?(missing_method) + action_permissions_map[missing_method] || super + end + +end diff --git a/app/policies/controllers/applications_controller_policy.rb b/app/policies/controllers/applications_controller_policy.rb new file mode 100644 index 000000000..fe886042c --- /dev/null +++ b/app/policies/controllers/applications_controller_policy.rb @@ -0,0 +1,3 @@ +class Controllers::ApplicationsControllerPolicy < Controllers::ApplicationControllerPolicy + default_permit!('admin.api') +end diff --git a/app/policies/controllers/calendar_subscriptions_controller_policy.rb b/app/policies/controllers/calendar_subscriptions_controller_policy.rb new file mode 100644 index 000000000..02248f2f2 --- /dev/null +++ b/app/policies/controllers/calendar_subscriptions_controller_policy.rb @@ -0,0 +1,3 @@ +class Controllers::CalendarSubscriptionsControllerPolicy < Controllers::ApplicationControllerPolicy + default_permit!('user_preferences.calendar') +end diff --git a/app/policies/controllers/calendars_controller_policy.rb b/app/policies/controllers/calendars_controller_policy.rb new file mode 100644 index 000000000..f3ecfa28b --- /dev/null +++ b/app/policies/controllers/calendars_controller_policy.rb @@ -0,0 +1,4 @@ +class Controllers::CalendarsControllerPolicy < Controllers::ApplicationControllerPolicy + permit! :timezones, to: 'admin' + default_permit!('admin.calendar') +end diff --git a/app/policies/controllers/channels_email_controller_policy.rb b/app/policies/controllers/channels_email_controller_policy.rb new file mode 100644 index 000000000..611c5a725 --- /dev/null +++ b/app/policies/controllers/channels_email_controller_policy.rb @@ -0,0 +1,3 @@ +class Controllers::ChannelsEmailControllerPolicy < Controllers::ApplicationControllerPolicy + default_permit!('admin.channel_email') +end diff --git a/app/policies/controllers/channels_facebook_controller_policy.rb b/app/policies/controllers/channels_facebook_controller_policy.rb new file mode 100644 index 000000000..14bfe7d48 --- /dev/null +++ b/app/policies/controllers/channels_facebook_controller_policy.rb @@ -0,0 +1,3 @@ +class Controllers::ChannelsFacebookControllerPolicy < Controllers::ApplicationControllerPolicy + default_permit!('admin.channel_facebook') +end diff --git a/app/policies/controllers/channels_sms_controller_policy.rb b/app/policies/controllers/channels_sms_controller_policy.rb new file mode 100644 index 000000000..b8ea771cc --- /dev/null +++ b/app/policies/controllers/channels_sms_controller_policy.rb @@ -0,0 +1,3 @@ +class Controllers::ChannelsSmsControllerPolicy < Controllers::ApplicationControllerPolicy + default_permit!('admin.channel_sms') +end diff --git a/app/policies/controllers/channels_telegram_controller_policy.rb b/app/policies/controllers/channels_telegram_controller_policy.rb new file mode 100644 index 000000000..9d47e5362 --- /dev/null +++ b/app/policies/controllers/channels_telegram_controller_policy.rb @@ -0,0 +1,3 @@ +class Controllers::ChannelsTelegramControllerPolicy < Controllers::ApplicationControllerPolicy + default_permit!('admin.channel_telegram') +end diff --git a/app/policies/controllers/channels_twitter_controller_policy.rb b/app/policies/controllers/channels_twitter_controller_policy.rb new file mode 100644 index 000000000..1858401b5 --- /dev/null +++ b/app/policies/controllers/channels_twitter_controller_policy.rb @@ -0,0 +1,3 @@ +class Controllers::ChannelsTwitterControllerPolicy < Controllers::ApplicationControllerPolicy + default_permit!('admin.channel_twitter') +end diff --git a/app/policies/controllers/chat_sessions_controller_policy.rb b/app/policies/controllers/chat_sessions_controller_policy.rb new file mode 100644 index 000000000..b1bc3d003 --- /dev/null +++ b/app/policies/controllers/chat_sessions_controller_policy.rb @@ -0,0 +1,3 @@ +class Controllers::ChatSessionsControllerPolicy < Controllers::ApplicationControllerPolicy + default_permit!('chat.agent') +end diff --git a/app/policies/controllers/chats_controller_policy.rb b/app/policies/controllers/chats_controller_policy.rb new file mode 100644 index 000000000..733cfd427 --- /dev/null +++ b/app/policies/controllers/chats_controller_policy.rb @@ -0,0 +1,3 @@ +class Controllers::ChatsControllerPolicy < Controllers::ApplicationControllerPolicy + default_permit!('admin.channel_chat') +end diff --git a/app/policies/controllers/cti_controller_policy.rb b/app/policies/controllers/cti_controller_policy.rb new file mode 100644 index 000000000..c9e4bd590 --- /dev/null +++ b/app/policies/controllers/cti_controller_policy.rb @@ -0,0 +1,3 @@ +class Controllers::CtiControllerPolicy < Controllers::ApplicationControllerPolicy + default_permit!('cti.agent') +end diff --git a/app/policies/controllers/email_addresses_controller_policy.rb b/app/policies/controllers/email_addresses_controller_policy.rb new file mode 100644 index 000000000..21c05b9ff --- /dev/null +++ b/app/policies/controllers/email_addresses_controller_policy.rb @@ -0,0 +1,4 @@ +class Controllers::EmailAddressesControllerPolicy < Controllers::ApplicationControllerPolicy + permit! %i[index show], to: ['ticket.agent', 'admin.channel_email'] + default_permit!('admin.channel_email') +end diff --git a/app/policies/controllers/external_credentials_controller_policy.rb b/app/policies/controllers/external_credentials_controller_policy.rb new file mode 100644 index 000000000..22a931142 --- /dev/null +++ b/app/policies/controllers/external_credentials_controller_policy.rb @@ -0,0 +1,16 @@ +class Controllers::ExternalCredentialsControllerPolicy < Controllers::ApplicationControllerPolicy + permit! :index, to: 'admin' + default_permit! -> { "admin.channel_#{provider_name}" } + + private + + def provider_name + @provider_name ||= begin + if record.params[:id].present? + ExternalCredential.find(record.params[:id]).name + else + record.params[:provider] || record.params[:name] + end + end + end +end diff --git a/app/policies/controllers/first_steps_controller_policy.rb b/app/policies/controllers/first_steps_controller_policy.rb new file mode 100644 index 000000000..aa58f67de --- /dev/null +++ b/app/policies/controllers/first_steps_controller_policy.rb @@ -0,0 +1,3 @@ +class Controllers::FirstStepsControllerPolicy < Controllers::ApplicationControllerPolicy + permit! %i[index test_ticket], to: ['ticket.agent', 'admin'] +end diff --git a/app/policies/controllers/form_controller_policy.rb b/app/policies/controllers/form_controller_policy.rb new file mode 100644 index 000000000..05d25bb23 --- /dev/null +++ b/app/policies/controllers/form_controller_policy.rb @@ -0,0 +1,28 @@ +class Controllers::FormControllerPolicy < Controllers::ApplicationControllerPolicy + + def configuration? + authorized? + end + + def submit? + authorized? + end + + def test? + record.params[:test] && user&.permissions?('admin.channel_formular') + end + + private + + def authorized? + test? || enabled? + end + + def user_required? + false + end + + def enabled? + Setting.get('form_ticket_create') + end +end diff --git a/app/policies/controllers/getting_started_controller_policy.rb b/app/policies/controllers/getting_started_controller_policy.rb new file mode 100644 index 000000000..33cdc84cb --- /dev/null +++ b/app/policies/controllers/getting_started_controller_policy.rb @@ -0,0 +1,3 @@ +class Controllers::GettingStartedControllerPolicy < Controllers::ApplicationControllerPolicy + permit! :base, to: 'admin.wizard' +end diff --git a/app/policies/controllers/groups_controller_policy.rb b/app/policies/controllers/groups_controller_policy.rb new file mode 100644 index 000000000..5011889b1 --- /dev/null +++ b/app/policies/controllers/groups_controller_policy.rb @@ -0,0 +1,3 @@ +class Controllers::GroupsControllerPolicy < Controllers::ApplicationControllerPolicy + default_permit!('admin.group') +end diff --git a/app/policies/controllers/http_logs_controller_policy.rb b/app/policies/controllers/http_logs_controller_policy.rb new file mode 100644 index 000000000..b398ce2d2 --- /dev/null +++ b/app/policies/controllers/http_logs_controller_policy.rb @@ -0,0 +1,3 @@ +class Controllers::HttpLogsControllerPolicy < Controllers::ApplicationControllerPolicy + default_permit!('admin.*') +end diff --git a/app/policies/controllers/integration/exchange_controller_policy.rb b/app/policies/controllers/integration/exchange_controller_policy.rb new file mode 100644 index 000000000..96c13f9a6 --- /dev/null +++ b/app/policies/controllers/integration/exchange_controller_policy.rb @@ -0,0 +1,3 @@ +class Controllers::Integration::ExchangeControllerPolicy < Controllers::ApplicationControllerPolicy + default_permit!('admin.integration.exchange') +end diff --git a/app/policies/controllers/integration/idoit_controller_policy.rb b/app/policies/controllers/integration/idoit_controller_policy.rb new file mode 100644 index 000000000..6fffd4595 --- /dev/null +++ b/app/policies/controllers/integration/idoit_controller_policy.rb @@ -0,0 +1,5 @@ +class Controllers::Integration::IdoitControllerPolicy < Controllers::ApplicationControllerPolicy + permit! %i[query update], to: 'ticket.agent' + permit! :verify, to: 'admin.integration.idoit' + default_permit!(['agent.integration.idoit', 'admin.integration.idoit']) +end diff --git a/app/policies/controllers/integration/ldap_controller_policy.rb b/app/policies/controllers/integration/ldap_controller_policy.rb new file mode 100644 index 000000000..1164b0ace --- /dev/null +++ b/app/policies/controllers/integration/ldap_controller_policy.rb @@ -0,0 +1,3 @@ +class Controllers::Integration::LdapControllerPolicy < Controllers::ApplicationControllerPolicy + default_permit!('admin.integration.ldap') +end diff --git a/app/policies/controllers/jobs_controller_policy.rb b/app/policies/controllers/jobs_controller_policy.rb new file mode 100644 index 000000000..099d112fe --- /dev/null +++ b/app/policies/controllers/jobs_controller_policy.rb @@ -0,0 +1,3 @@ +class Controllers::JobsControllerPolicy < Controllers::ApplicationControllerPolicy + default_permit!('admin.scheduler') +end diff --git a/app/policies/controllers/knowledge_base/answer/attachments_controller_policy.rb b/app/policies/controllers/knowledge_base/answer/attachments_controller_policy.rb new file mode 100644 index 000000000..edb808b64 --- /dev/null +++ b/app/policies/controllers/knowledge_base/answer/attachments_controller_policy.rb @@ -0,0 +1,3 @@ +class Controllers::KnowledgeBase::Answer::AttachmentsControllerPolicy < Controllers::ApplicationControllerPolicy + default_permit!('knowledge_base.editor') +end diff --git a/app/policies/controllers/knowledge_base/answers_controller_policy.rb b/app/policies/controllers/knowledge_base/answers_controller_policy.rb new file mode 100644 index 000000000..6d792b1a0 --- /dev/null +++ b/app/policies/controllers/knowledge_base/answers_controller_policy.rb @@ -0,0 +1,8 @@ +class Controllers::KnowledgeBase::AnswersControllerPolicy < Controllers::KnowledgeBase::BaseControllerPolicy + def show? + return true if user.permissions?('knowledge_base.editor') + + object = record.klass.find(record.params[:id]) + object.can_be_published_aasm.internal? || object.can_be_published_aasm.published? + end +end diff --git a/app/policies/controllers/knowledge_base/base_controller_policy.rb b/app/policies/controllers/knowledge_base/base_controller_policy.rb new file mode 100644 index 000000000..3285c46cf --- /dev/null +++ b/app/policies/controllers/knowledge_base/base_controller_policy.rb @@ -0,0 +1,4 @@ +class Controllers::KnowledgeBase::BaseControllerPolicy < Controllers::ApplicationControllerPolicy + default_permit!('knowledge_base.*') + permit! %i[create update destroy], to: 'knowledge_base.editor' +end diff --git a/app/policies/controllers/knowledge_base/categories_controller_policy.rb b/app/policies/controllers/knowledge_base/categories_controller_policy.rb new file mode 100644 index 000000000..f329b4bea --- /dev/null +++ b/app/policies/controllers/knowledge_base/categories_controller_policy.rb @@ -0,0 +1,7 @@ +class Controllers::KnowledgeBase::CategoriesControllerPolicy < Controllers::KnowledgeBase::BaseControllerPolicy + def show? + return if user.permissions?('knowledge_base.editor') + + record.klass.find(params[:id]).internal_content? + end +end diff --git a/app/policies/controllers/knowledge_base/manage_controller_policy.rb b/app/policies/controllers/knowledge_base/manage_controller_policy.rb new file mode 100644 index 000000000..4cdd355b7 --- /dev/null +++ b/app/policies/controllers/knowledge_base/manage_controller_policy.rb @@ -0,0 +1,3 @@ +class Controllers::KnowledgeBase::ManageControllerPolicy < Controllers::KnowledgeBase::BaseControllerPolicy + default_permit!('admin.knowledge_base') +end diff --git a/app/policies/controllers/knowledge_bases_controller_policy.rb b/app/policies/controllers/knowledge_bases_controller_policy.rb new file mode 100644 index 000000000..8786bfbb3 --- /dev/null +++ b/app/policies/controllers/knowledge_bases_controller_policy.rb @@ -0,0 +1,13 @@ +class Controllers::KnowledgeBasesControllerPolicy < Controllers::KnowledgeBase::BaseControllerPolicy + def init? + true + end + + def create? + false + end + + def destroy? + false + end +end diff --git a/app/policies/controllers/monitoring_controller_policy.rb b/app/policies/controllers/monitoring_controller_policy.rb new file mode 100644 index 000000000..71fea988f --- /dev/null +++ b/app/policies/controllers/monitoring_controller_policy.rb @@ -0,0 +1,56 @@ +class Controllers::MonitoringControllerPolicy < Controllers::ApplicationControllerPolicy + + def health_check? + token_or_permission? + end + + def status? + token_or_permission? + end + + def amount_check? + token_or_permission? + end + + def token? + permission_and_permission_active? + end + + def restart_failed_jobs? + permission_and_permission_active? + end + + private + + def user_required? + false + end + + def token_or_permission? + return true if user.present? && monitoring_admin! + return true if valid_token_param? + + not_authorized + end + + def permission_and_permission_active? + user_required! + monitoring_admin! + return true if permission_active? + + not_authorized + end + + def valid_token_param? + Setting.get('monitoring_token') == record.params[:token] + end + + def permission_active? + Permission.exists?(name: 'admin.monitoring', active: true) + end + + def monitoring_admin! + user.permissions!('admin.monitoring') + true + end +end diff --git a/app/policies/controllers/object_manager_attributes_controller_policy.rb b/app/policies/controllers/object_manager_attributes_controller_policy.rb new file mode 100644 index 000000000..e94674b2c --- /dev/null +++ b/app/policies/controllers/object_manager_attributes_controller_policy.rb @@ -0,0 +1,3 @@ +class Controllers::ObjectManagerAttributesControllerPolicy < Controllers::ApplicationControllerPolicy + default_permit!('admin.object') +end diff --git a/app/policies/controllers/online_notifications_controller_policy.rb b/app/policies/controllers/online_notifications_controller_policy.rb new file mode 100644 index 000000000..bac27a174 --- /dev/null +++ b/app/policies/controllers/online_notifications_controller_policy.rb @@ -0,0 +1,21 @@ +class Controllers::OnlineNotificationsControllerPolicy < Controllers::ApplicationControllerPolicy + + def show? + own? + end + + def update? + own? + end + + def destroy? + own? + end + + private + + def own? + notification = OnlineNotification.find(record.params[:id]) + notification.user_id == user.id + end +end diff --git a/app/policies/controllers/organizations_controller_policy.rb b/app/policies/controllers/organizations_controller_policy.rb new file mode 100644 index 000000000..2530cb618 --- /dev/null +++ b/app/policies/controllers/organizations_controller_policy.rb @@ -0,0 +1,11 @@ +class Controllers::OrganizationsControllerPolicy < Controllers::ApplicationControllerPolicy + permit! :import_example, to: 'admin.organization' + permit! :import_start, to: 'admin.user' + permit! %i[create update destroy search history], to: ['ticket.agent', 'admin.organization'] + + def show? + return true if user.permissions?(['ticket.agent', 'admin.organization']) + + record.params[:id].to_i == user.organization_id + end +end diff --git a/app/policies/controllers/overviews_controller_policy.rb b/app/policies/controllers/overviews_controller_policy.rb new file mode 100644 index 000000000..ace9642e0 --- /dev/null +++ b/app/policies/controllers/overviews_controller_policy.rb @@ -0,0 +1,3 @@ +class Controllers::OverviewsControllerPolicy < Controllers::ApplicationControllerPolicy + default_permit!('admin.overview') +end diff --git a/app/policies/controllers/packages_controller_policy.rb b/app/policies/controllers/packages_controller_policy.rb new file mode 100644 index 000000000..5810aa9e0 --- /dev/null +++ b/app/policies/controllers/packages_controller_policy.rb @@ -0,0 +1,3 @@ +class Controllers::PackagesControllerPolicy < Controllers::ApplicationControllerPolicy + default_permit!('admin.package') +end diff --git a/app/policies/controllers/postmaster_filters_controller_policy.rb b/app/policies/controllers/postmaster_filters_controller_policy.rb new file mode 100644 index 000000000..2f62933ee --- /dev/null +++ b/app/policies/controllers/postmaster_filters_controller_policy.rb @@ -0,0 +1,3 @@ +class Controllers::PostmasterFiltersControllerPolicy < Controllers::ApplicationControllerPolicy + default_permit!('admin.channel_email') +end diff --git a/app/policies/controllers/proxy_controller_policy.rb b/app/policies/controllers/proxy_controller_policy.rb new file mode 100644 index 000000000..a387644be --- /dev/null +++ b/app/policies/controllers/proxy_controller_policy.rb @@ -0,0 +1,3 @@ +class Controllers::ProxyControllerPolicy < Controllers::ApplicationControllerPolicy + default_permit!('admin.system') +end diff --git a/app/policies/controllers/report_profiles_controller_policy.rb b/app/policies/controllers/report_profiles_controller_policy.rb new file mode 100644 index 000000000..19454e0d3 --- /dev/null +++ b/app/policies/controllers/report_profiles_controller_policy.rb @@ -0,0 +1,3 @@ +class Controllers::ReportProfilesControllerPolicy < Controllers::ApplicationControllerPolicy + default_permit!('admin.report_profile') +end diff --git a/app/policies/controllers/reports_controller_policy.rb b/app/policies/controllers/reports_controller_policy.rb new file mode 100644 index 000000000..7da95dcce --- /dev/null +++ b/app/policies/controllers/reports_controller_policy.rb @@ -0,0 +1,3 @@ +class Controllers::ReportsControllerPolicy < Controllers::ApplicationControllerPolicy + default_permit!('report') +end diff --git a/app/policies/controllers/roles_controller_policy.rb b/app/policies/controllers/roles_controller_policy.rb new file mode 100644 index 000000000..1fc8705b7 --- /dev/null +++ b/app/policies/controllers/roles_controller_policy.rb @@ -0,0 +1,3 @@ +class Controllers::RolesControllerPolicy < Controllers::ApplicationControllerPolicy + default_permit!('admin.role') +end diff --git a/app/policies/controllers/sessions_controller_policy.rb b/app/policies/controllers/sessions_controller_policy.rb new file mode 100644 index 000000000..43d53c637 --- /dev/null +++ b/app/policies/controllers/sessions_controller_policy.rb @@ -0,0 +1,4 @@ +class Controllers::SessionsControllerPolicy < Controllers::ApplicationControllerPolicy + permit! :switch_to_user, to: ['admin.session', 'admin.user'] + permit! %i[list delete], to: 'admin.session' +end diff --git a/app/policies/controllers/settings_controller_policy.rb b/app/policies/controllers/settings_controller_policy.rb new file mode 100644 index 000000000..20a1c2cc3 --- /dev/null +++ b/app/policies/controllers/settings_controller_policy.rb @@ -0,0 +1,43 @@ +class Controllers::SettingsControllerPolicy < Controllers::ApplicationControllerPolicy + default_permit!('admin.*') + + def show? + user.permissions!('admin.*') + authorized_for_setting?(:show?) + end + + def update? + updateable? + end + + def update_image? + updateable? + end + + private + + def setting + @setting ||= Setting.lookup(id: record.params[:id]) + end + + def authorized_for_setting?(query) + Pundit.authorize(user, setting, query) + true + rescue Pundit::NotAuthorizedError + not_authorized("required #{setting.preferences[:permission].inspect}") + end + + def updateable? + return false if !user.permissions?('admin.*') + return false if !authorized_for_setting?(:update?) + + service_enabled? + end + + def service_enabled? + return true if !Setting.get('system_online_service') + return true if !setting.preferences[:online_service_disable] + + not_authorized('service disabled') + end +end diff --git a/app/policies/controllers/signatures_controller_policy.rb b/app/policies/controllers/signatures_controller_policy.rb new file mode 100644 index 000000000..bd51c81a8 --- /dev/null +++ b/app/policies/controllers/signatures_controller_policy.rb @@ -0,0 +1,4 @@ +class Controllers::SignaturesControllerPolicy < Controllers::ApplicationControllerPolicy + permit! %i[index show], to: ['ticket.agent', 'admin.channel_email'] + permit! %i[create update destroy], to: 'admin.channel_email' +end diff --git a/app/policies/controllers/slas_controller_policy.rb b/app/policies/controllers/slas_controller_policy.rb new file mode 100644 index 000000000..40ad48c1c --- /dev/null +++ b/app/policies/controllers/slas_controller_policy.rb @@ -0,0 +1,3 @@ +class Controllers::SlasControllerPolicy < Controllers::ApplicationControllerPolicy + default_permit!('admin.sla') +end diff --git a/app/policies/controllers/tags_controller_policy.rb b/app/policies/controllers/tags_controller_policy.rb new file mode 100644 index 000000000..4dbc38108 --- /dev/null +++ b/app/policies/controllers/tags_controller_policy.rb @@ -0,0 +1,3 @@ +class Controllers::TagsControllerPolicy < Controllers::ApplicationControllerPolicy + default_permit!('admin.tag') +end diff --git a/app/policies/controllers/taskbar_controller_policy.rb b/app/policies/controllers/taskbar_controller_policy.rb new file mode 100644 index 000000000..b44b92e8a --- /dev/null +++ b/app/policies/controllers/taskbar_controller_policy.rb @@ -0,0 +1,25 @@ +class Controllers::TaskbarControllerPolicy < Controllers::ApplicationControllerPolicy + + def show? + own? + end + + def update? + own? + end + + def destroy? + own? + end + + private + + def own? + taskbar = Taskbar.find(record.params[:id]) + return true if taskbar.user_id == user.id + + # current implementation requires this exception type + # should be replaced by unified way + raise Exceptions::UnprocessableEntity, 'Not allowed to access this task.' + end +end diff --git a/app/policies/controllers/templates_controller_policy.rb b/app/policies/controllers/templates_controller_policy.rb new file mode 100644 index 000000000..c45eb764f --- /dev/null +++ b/app/policies/controllers/templates_controller_policy.rb @@ -0,0 +1,3 @@ +class Controllers::TemplatesControllerPolicy < Controllers::ApplicationControllerPolicy + default_permit!(['ticket.agent', 'admin.template']) +end diff --git a/app/policies/controllers/text_modules_controller_policy.rb b/app/policies/controllers/text_modules_controller_policy.rb new file mode 100644 index 000000000..527894d74 --- /dev/null +++ b/app/policies/controllers/text_modules_controller_policy.rb @@ -0,0 +1,4 @@ +class Controllers::TextModulesControllerPolicy < Controllers::ApplicationControllerPolicy + permit! %i[index show], to: ['ticket.agent', 'admin.text_module'] + permit! %i[create update destroy import_example import_start], to: 'admin.text_module' +end diff --git a/app/policies/controllers/ticket_articles_controller_policy.rb b/app/policies/controllers/ticket_articles_controller_policy.rb new file mode 100644 index 000000000..88ef7da39 --- /dev/null +++ b/app/policies/controllers/ticket_articles_controller_policy.rb @@ -0,0 +1,3 @@ +class Controllers::TicketArticlesControllerPolicy < Controllers::ApplicationControllerPolicy + default_permit!('admin') +end diff --git a/app/policies/controllers/ticket_priorities_controller_policy.rb b/app/policies/controllers/ticket_priorities_controller_policy.rb new file mode 100644 index 000000000..ebc8b6505 --- /dev/null +++ b/app/policies/controllers/ticket_priorities_controller_policy.rb @@ -0,0 +1,4 @@ +class Controllers::TicketPrioritiesControllerPolicy < Controllers::ApplicationControllerPolicy + permit! %i[index show], to: ['ticket.agent', 'admin.object', 'ticket.customer'] + permit! %i[create update destroy], to: 'admin.object' +end diff --git a/app/policies/controllers/ticket_states_controller_policy.rb b/app/policies/controllers/ticket_states_controller_policy.rb new file mode 100644 index 000000000..667646cf5 --- /dev/null +++ b/app/policies/controllers/ticket_states_controller_policy.rb @@ -0,0 +1,4 @@ +class Controllers::TicketStatesControllerPolicy < Controllers::ApplicationControllerPolicy + permit! %i[index show], to: ['ticket.agent', 'admin.object', 'ticket.customer'] + permit! %i[create update destroy], to: 'admin.object' +end diff --git a/app/policies/controllers/tickets_controller_policy.rb b/app/policies/controllers/tickets_controller_policy.rb new file mode 100644 index 000000000..2aadda786 --- /dev/null +++ b/app/policies/controllers/tickets_controller_policy.rb @@ -0,0 +1,5 @@ +class Controllers::TicketsControllerPolicy < Controllers::ApplicationControllerPolicy + permit! %i[import_example import_start], to: 'admin' + permit! :selector, to: 'admin.*' + permit! :create, to: ['ticket.agent', 'ticket.customer'] +end diff --git a/app/policies/controllers/time_accountings_controller_policy.rb b/app/policies/controllers/time_accountings_controller_policy.rb new file mode 100644 index 000000000..d91de4fa3 --- /dev/null +++ b/app/policies/controllers/time_accountings_controller_policy.rb @@ -0,0 +1,3 @@ +class Controllers::TimeAccountingsControllerPolicy < Controllers::ApplicationControllerPolicy + default_permit!('admin.time_accounting') +end diff --git a/app/policies/controllers/translations_controller_policy.rb b/app/policies/controllers/translations_controller_policy.rb new file mode 100644 index 000000000..d81bacb1f --- /dev/null +++ b/app/policies/controllers/translations_controller_policy.rb @@ -0,0 +1,3 @@ +class Controllers::TranslationsControllerPolicy < Controllers::ApplicationControllerPolicy + default_permit!('admin.translation') +end diff --git a/app/policies/controllers/triggers_controller_policy.rb b/app/policies/controllers/triggers_controller_policy.rb new file mode 100644 index 000000000..0d9c21fc0 --- /dev/null +++ b/app/policies/controllers/triggers_controller_policy.rb @@ -0,0 +1,3 @@ +class Controllers::TriggersControllerPolicy < Controllers::ApplicationControllerPolicy + default_permit!('admin.trigger') +end diff --git a/app/policies/controllers/user_access_token_controller_policy.rb b/app/policies/controllers/user_access_token_controller_policy.rb new file mode 100644 index 000000000..620a9296f --- /dev/null +++ b/app/policies/controllers/user_access_token_controller_policy.rb @@ -0,0 +1,3 @@ +class Controllers::UserAccessTokenControllerPolicy < Controllers::ApplicationControllerPolicy + default_permit!('user_preferences.access_token') +end diff --git a/app/policies/controllers/user_devices_controller_policy.rb b/app/policies/controllers/user_devices_controller_policy.rb new file mode 100644 index 000000000..ec9355c93 --- /dev/null +++ b/app/policies/controllers/user_devices_controller_policy.rb @@ -0,0 +1,3 @@ +class Controllers::UserDevicesControllerPolicy < Controllers::ApplicationControllerPolicy + default_permit!('user_preferences.device') +end diff --git a/app/policies/controllers/users_controller_policy.rb b/app/policies/controllers/users_controller_policy.rb new file mode 100644 index 000000000..a49338ea9 --- /dev/null +++ b/app/policies/controllers/users_controller_policy.rb @@ -0,0 +1,4 @@ +class Controllers::UsersControllerPolicy < Controllers::ApplicationControllerPolicy + permit! %i[import_example import_start], to: 'admin.user' + permit! %i[search history create update], to: ['ticket.agent', 'admin.user'] +end diff --git a/app/policies/controllers/version_controller_policy.rb b/app/policies/controllers/version_controller_policy.rb new file mode 100644 index 000000000..ea12eed76 --- /dev/null +++ b/app/policies/controllers/version_controller_policy.rb @@ -0,0 +1,3 @@ +class Controllers::VersionControllerPolicy < Controllers::ApplicationControllerPolicy + default_permit!('admin.version') +end diff --git a/app/policies/organization_policy.rb b/app/policies/organization_policy.rb new file mode 100644 index 000000000..8b52fa13a --- /dev/null +++ b/app/policies/organization_policy.rb @@ -0,0 +1,28 @@ +class OrganizationPolicy < ApplicationPolicy + + def show? + return true if user.permissions?(['admin', 'ticket.agent']) + return false if !user.permissions?('ticket.customer') + + record.id == user.organization_id + end + + def update? + return false if user.permissions?('ticket.customer') + + user.permissions?(['admin', 'ticket.agent']) + end + + class Scope < ApplicationPolicy::Scope + + def resolve + if user.permissions?(['ticket.agent', 'admin.organization']) + scope.all + elsif user.organization_id + scope.where(id: user.organization_id) + else + scope.none + end + end + end +end diff --git a/app/policies/pundit_policy.rb b/app/policies/pundit_policy.rb new file mode 100644 index 000000000..7ec65ce91 --- /dev/null +++ b/app/policies/pundit_policy.rb @@ -0,0 +1,32 @@ +module PunditPolicy + + attr_reader :user, :custom_exception + + def initialize(user, context) + @user = user + user_required! if user_required? + + initialize_context(context) + end + + def user_required? + true + end + + def user_required! + return if user + + raise Exceptions::NotAuthorized, 'authentication failed' + end + + private + + def not_authorized(details = nil) + if details + details = "Not authorized (#{details})!" + end + @custom_exception = Exceptions::NotAuthorized.new(details) + false + end + +end diff --git a/app/policies/setting_policy.rb b/app/policies/setting_policy.rb new file mode 100644 index 000000000..ca964f435 --- /dev/null +++ b/app/policies/setting_policy.rb @@ -0,0 +1,18 @@ +class SettingPolicy < ApplicationPolicy + + def show? + permitted? + end + + def update? + permitted? + end + + private + + def permitted? + return true if !record.preferences[:permission] + + user.permissions?(record.preferences[:permission]) + end +end diff --git a/app/policies/ticket/article_policy.rb b/app/policies/ticket/article_policy.rb new file mode 100644 index 000000000..d80c195e9 --- /dev/null +++ b/app/policies/ticket/article_policy.rb @@ -0,0 +1,52 @@ +class Ticket::ArticlePolicy < ApplicationPolicy + + def show? + access?(__method__) + end + + def create? + access?(__method__) + end + + def update? + return false if !access?(__method__) + return true if user.permissions?(['ticket.agent', 'admin']) + + not_authorized('ticket.agent or admin permission required') + end + + def destroy? + return true if user.permissions?('admin') + return false if !access?(__method__) + # don't let edge case exceptions raised in the TicketPolicy stop + # other possible positive authorization checks + rescue Pundit::NotAuthorizedError + # agents can destroy articles of type 'note' + # which were created by themselves within the last 10 minutes + return missing_admin_permission if !user.permissions?('ticket.agent') + return missing_admin_permission if record.created_by_id != user.id + return missing_admin_permission if record.type.communication? + return too_old_to_undo if record.created_at <= 10.minutes.ago + + true + end + + private + + def access?(query) + if record.internal == true && user.permissions?('ticket.customer') + return false + end + + ticket = Ticket.lookup(id: record.ticket_id) + Pundit.authorize(user, ticket, query) + end + + def missing_admin_permission + not_authorized('admin permission required') + end + + def too_old_to_undo + not_authorized('articles more than 10 minutes old may not be deleted') + end +end diff --git a/app/policies/ticket_policy.rb b/app/policies/ticket_policy.rb new file mode 100644 index 000000000..ab78f8718 --- /dev/null +++ b/app/policies/ticket_policy.rb @@ -0,0 +1,63 @@ +class TicketPolicy < ApplicationPolicy + + def show? + access?('read') + end + + def create? + access?('create') + end + + def update? + access?('change') + end + + def destroy? + return true if user.permissions?('admin') + + # This might look like a bug is actually just defining + # what exception is being raised and shown to the user. + return false if !access?('delete') + + not_authorized('admin permission required') + end + + def full? + access?('full') + end + + def follow_up? + return true if user.permissions?('ticket.agent') # agents can always reopen tickets, regardless of group configuration + return true if record.group.follow_up_possible != 'new_ticket' # check if the setting for follow_up_possible is disabled + return true if record.state.name != 'closed' # check if the ticket state is already closed + + raise Exceptions::UnprocessableEntity, 'Cannot follow-up on a closed ticket. Please create a new ticket.' + end + + private + + def access?(access) + + # check customer + if user.permissions?('ticket.customer') + + # access ok if its own ticket + return true if record.customer_id == user.id + + # check organization ticket access + return false if record.organization_id.blank? + return false if user.organization_id.blank? + return false if record.organization_id != user.organization_id + + return record.organization.shared? + end + + # check agent + + # access if requester is owner + return true if record.owner_id == user.id + + # access if requester is in group + user.group_access?(record.group.id, access) + end +end diff --git a/app/policies/user_policy.rb b/app/policies/user_policy.rb new file mode 100644 index 000000000..17f476f9f --- /dev/null +++ b/app/policies/user_policy.rb @@ -0,0 +1,49 @@ +class UserPolicy < ApplicationPolicy + + def show? + return true if user.permissions?('admin.*') + return true if own_account? + return true if user.permissions?('ticket.agent') + # check same organization for customers + return false if !user.permissions?('ticket.customer') + + same_organization? + end + + def update? + return true if user.permissions?('admin.user') + # forbid non-agents to change users + return false if !user.permissions?('ticket.agent') + + # allow agents to change customers + record.permissions?('ticket.customer') + end + + def destroy? + user.permissions?('admin.user') + end + + private + + def own_account? + record.id == user.id + end + + def same_organization? + return false if record.organization_id.blank? + return false if user.organization_id.blank? + + record.organization_id == user.organization_id + end + + class Scope < ApplicationPolicy::Scope + + def resolve + if user.permissions?(['ticket.agent', 'admin.user']) + scope.all + else + scope.where(id: user.id) + end + end + end +end diff --git a/spec/models/ticket_spec.rb b/spec/models/ticket_spec.rb index e6606d63d..176aac82b 100644 --- a/spec/models/ticket_spec.rb +++ b/spec/models/ticket_spec.rb @@ -301,63 +301,6 @@ RSpec.describe Ticket, type: :model do end end - describe '#access?' do - context 'when given ticket’s owner' do - it 'returns true for both "read" and "full" privileges' do - expect(ticket.access?(ticket.owner, 'read')).to be(true) - expect(ticket.access?(ticket.owner, 'full')).to be(true) - end - end - - context 'when given the ticket’s customer' do - it 'returns true for both "read" and "full" privileges' do - expect(ticket.access?(ticket.customer, 'read')).to be(true) - expect(ticket.access?(ticket.customer, 'full')).to be(true) - end - end - - context 'when given a user that is neither owner nor customer' do - let(:user) { create(:agent_user) } - - it 'returns false for both "read" and "full" privileges' do - expect(ticket.access?(user, 'read')).to be(false) - expect(ticket.access?(user, 'full')).to be(false) - end - - context 'but the user is an agent with full access to ticket’s group' do - before { user.group_names_access_map = { ticket.group.name => 'full' } } - - it 'returns true for both "read" and "full" privileges' do - expect(ticket.access?(user, 'read')).to be(true) - expect(ticket.access?(user, 'full')).to be(true) - end - end - - context 'but the user is a customer from the same organization as ticket’s customer' do - subject(:ticket) { create(:ticket, customer: customer) } - - let(:customer) { create(:customer_user, organization: create(:organization)) } - let(:colleague) { create(:customer_user, organization: customer.organization) } - - context 'and organization.shared is true (default)' do - it 'returns true for both "read" and "full" privileges' do - expect(ticket.access?(colleague, 'read')).to be(true) - expect(ticket.access?(colleague, 'full')).to be(true) - end - end - - context 'but organization.shared is false' do - before { customer.organization.update(shared: false) } - - it 'returns false for both "read" and "full" privileges' do - expect(ticket.access?(colleague, 'read')).to be(false) - expect(ticket.access?(colleague, 'full')).to be(false) - end - end - end - end - end - describe '#subject_build' do context 'with default "ticket_hook_position" setting ("right")' do it 'returns the given string followed by a ticket reference (of the form "[Ticket#123]")' do diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 154c83f15..7c94508a1 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -369,139 +369,6 @@ RSpec.describe User, type: :model do end end - describe '#access?' do - context 'when an admin' do - subject(:user) { create(:user, roles: [partial_admin_role]) } - - context 'with "admin.user" privileges' do - let(:partial_admin_role) do - create(:role).tap { |role| role.permission_grant('admin.user') } - end - - context 'wants to read, change, or delete any user' do - it 'returns true' do - expect(admin.access?(user, 'read')).to be(true) - expect(admin.access?(user, 'change')).to be(true) - expect(admin.access?(user, 'delete')).to be(true) - expect(agent.access?(user, 'read')).to be(true) - expect(agent.access?(user, 'change')).to be(true) - expect(agent.access?(user, 'delete')).to be(true) - expect(customer.access?(user, 'read')).to be(true) - expect(customer.access?(user, 'change')).to be(true) - expect(customer.access?(user, 'delete')).to be(true) - expect(user.access?(user, 'read')).to be(true) - expect(user.access?(user, 'change')).to be(true) - expect(user.access?(user, 'delete')).to be(true) - end - end - end - - context 'without "admin.user" privileges' do - let(:partial_admin_role) do - create(:role).tap { |role| role.permission_grant('admin.tag') } - end - - context 'wants to read any user' do - it 'returns true' do - expect(admin.access?(user, 'read')).to be(true) - expect(agent.access?(user, 'read')).to be(true) - expect(customer.access?(user, 'read')).to be(true) - expect(user.access?(user, 'read')).to be(true) - end - end - - context 'wants to change or delete any user' do - it 'returns false' do - expect(admin.access?(user, 'change')).to be(false) - expect(admin.access?(user, 'delete')).to be(false) - expect(agent.access?(user, 'change')).to be(false) - expect(agent.access?(user, 'delete')).to be(false) - expect(customer.access?(user, 'change')).to be(false) - expect(customer.access?(user, 'delete')).to be(false) - expect(user.access?(user, 'change')).to be(false) - expect(user.access?(user, 'delete')).to be(false) - end - end - end - end - - context 'when an agent' do - subject(:user) { create(:agent_user) } - - context 'wants to read any user' do - it 'returns true' do - expect(admin.access?(user, 'read')).to be(true) - expect(agent.access?(user, 'read')).to be(true) - expect(customer.access?(user, 'read')).to be(true) - expect(user.access?(user, 'read')).to be(true) - end - end - - context 'wants to change' do - context 'any admin or agent' do - it 'returns false' do - expect(admin.access?(user, 'change')).to be(false) - expect(agent.access?(user, 'change')).to be(false) - expect(user.access?(user, 'change')).to be(false) - end - end - - context 'any customer' do - it 'returns true' do - expect(customer.access?(user, 'change')).to be(true) - end - end - end - - context 'wants to delete any user' do - it 'returns false' do - expect(admin.access?(user, 'delete')).to be(false) - expect(agent.access?(user, 'delete')).to be(false) - expect(customer.access?(user, 'delete')).to be(false) - expect(user.access?(user, 'delete')).to be(false) - end - end - end - - context 'when a customer' do - subject(:user) { create(:customer_user, :with_org) } - - let(:colleague) { create(:customer_user, organization: user.organization) } - - context 'wants to read' do - context 'any admin, agent, or customer from a different organization' do - it 'returns false' do - expect(admin.access?(user, 'read')).to be(false) - expect(agent.access?(user, 'read')).to be(false) - expect(customer.access?(user, 'read')).to be(false) - end - end - - context 'any customer from the same organization' do - it 'returns true' do - expect(user.access?(user, 'read')).to be(true) - expect(colleague.access?(user, 'read')).to be(true) - end - end - end - - context 'wants to change or delete any user' do - it 'returns false' do - expect(admin.access?(user, 'change')).to be(false) - expect(admin.access?(user, 'delete')).to be(false) - expect(agent.access?(user, 'change')).to be(false) - expect(agent.access?(user, 'delete')).to be(false) - expect(customer.access?(user, 'change')).to be(false) - expect(customer.access?(user, 'delete')).to be(false) - expect(colleague.access?(user, 'change')).to be(false) - expect(colleague.access?(user, 'delete')).to be(false) - expect(user.access?(user, 'change')).to be(false) - expect(user.access?(user, 'delete')).to be(false) - end - end - end - end - describe '#permissions?' do subject(:user) { create(:user, roles: [role]) } diff --git a/spec/policies/ticket_policy_spec.rb b/spec/policies/ticket_policy_spec.rb new file mode 100644 index 000000000..19cd1b886 --- /dev/null +++ b/spec/policies/ticket_policy_spec.rb @@ -0,0 +1,42 @@ +require 'rails_helper' + +describe TicketPolicy do + subject { described_class.new(user, record) } + + let(:record) { create(:ticket) } + + context 'when given ticket’s owner' do + let(:user) { record.owner } + + it { is_expected.to permit_actions(%i[show full]) } + end + + context 'when given a user that is neither owner nor customer' do + let(:user) { create(:agent_user) } + + it { is_expected.not_to permit_actions(%i[show full]) } + + context 'but the user is an agent with full access to ticket’s group' do + before { user.group_names_access_map = { record.group.name => 'full' } } + + it { is_expected.to permit_actions(%i[show full]) } + end + + context 'but the user is a customer from the same organization as ticket’s customer' do + let(:record) { create(:ticket, customer: customer) } + let(:customer) { create(:customer_user, organization: create(:organization)) } + let(:user) { create(:customer_user, organization: customer.organization) } + + context 'and organization.shared is true (default)' do + + it { is_expected.to permit_actions(%i[show full]) } + end + + context 'but organization.shared is false' do + before { customer.organization.update(shared: false) } + + it { is_expected.not_to permit_actions(%i[show full]) } + end + end + end +end diff --git a/spec/policies/user_policy_spec.rb b/spec/policies/user_policy_spec.rb new file mode 100644 index 000000000..8b3a076eb --- /dev/null +++ b/spec/policies/user_policy_spec.rb @@ -0,0 +1,171 @@ +require 'rails_helper' + +describe UserPolicy do + subject { described_class.new(user, record) } + + context 'when user is an admin' do + let(:user) { create(:user, roles: [partial_admin_role]) } + + context 'with "admin.user" privileges' do + let(:partial_admin_role) do + create(:role).tap { |role| role.permission_grant('admin.user') } + end + + context 'wants to read, change, or delete any user' do + + context 'when record is an admin user' do + let(:record) { create(:admin_user) } + + it { is_expected.to permit_actions(%i[show update destroy]) } + end + + context 'when record is an agent user' do + let(:record) { create(:agent_user) } + + it { is_expected.to permit_actions(%i[show update destroy]) } + end + + context 'when record is a customer user' do + let(:record) { create(:customer_user) } + + it { is_expected.to permit_actions(%i[show update destroy]) } + end + + context 'when record is any user' do + let(:record) { create(:user) } + + it { is_expected.to permit_actions(%i[show update destroy]) } + end + + context 'when record is the same user' do + let(:record) { user } + + it { is_expected.to permit_actions(%i[show update destroy]) } + end + end + end + + context 'without "admin.user" privileges' do + let(:partial_admin_role) do + create(:role).tap { |role| role.permission_grant('admin.tag') } + end + + context 'when record is an admin user' do + let(:record) { create(:admin_user) } + + it { is_expected.to permit_action(:show) } + it { is_expected.not_to permit_actions(%i[update destroy]) } + end + + context 'when record is an agent user' do + let(:record) { create(:agent_user) } + + it { is_expected.to permit_action(:show) } + it { is_expected.not_to permit_actions(%i[update destroy]) } + end + + context 'when record is a customer user' do + let(:record) { create(:customer_user) } + + it { is_expected.to permit_action(:show) } + it { is_expected.not_to permit_actions(%i[update destroy]) } + end + + context 'when record is any user' do + let(:record) { create(:user) } + + it { is_expected.to permit_action(:show) } + it { is_expected.not_to permit_actions(%i[update destroy]) } + end + + context 'when record is the same user' do + let(:record) { user } + + it { is_expected.to permit_action(:show) } + it { is_expected.not_to permit_actions(%i[update destroy]) } + end + end + end + + context 'when user is an agent' do + let(:user) { create(:agent_user) } + + context 'when record is an admin user' do + let(:record) { create(:admin_user) } + + it { is_expected.to permit_action(:show) } + it { is_expected.not_to permit_actions(%i[update destroy]) } + end + + context 'when record is an agent user' do + let(:record) { create(:agent_user) } + + it { is_expected.to permit_action(:show) } + it { is_expected.not_to permit_actions(%i[update destroy]) } + end + + context 'when record is a customer user' do + let(:record) { create(:customer_user) } + + it { is_expected.to permit_actions(%i[show update]) } + it { is_expected.not_to permit_action(:destroy) } + end + + context 'when record is any user' do + let(:record) { create(:user) } + + it { is_expected.to permit_action(:show) } + it { is_expected.not_to permit_actions(%i[update destroy]) } + end + + context 'when record is the same user' do + let(:record) { user } + + it { is_expected.to permit_action(:show) } + it { is_expected.not_to permit_actions(%i[update destroy]) } + end + end + + context 'when user is a customer' do + let(:user) { create(:customer_user) } + + context 'when record is an admin user' do + let(:record) { create(:admin_user) } + + it { is_expected.not_to permit_actions(%i[show update destroy]) } + end + + context 'when record is an agent user' do + let(:record) { create(:agent_user) } + + it { is_expected.not_to permit_actions(%i[show update destroy]) } + end + + context 'when record is a customer user' do + let(:record) { create(:customer_user) } + + it { is_expected.not_to permit_actions(%i[show update destroy]) } + end + + context 'when record is any user' do + let(:record) { create(:user) } + + it { is_expected.not_to permit_actions(%i[show update destroy]) } + end + + context 'when record is a colleague' do + let(:user) { create(:customer_user, :with_org) } + let(:record) { create(:customer_user, organization: user.organization) } + + it { is_expected.to permit_action(:show) } + it { is_expected.not_to permit_actions(%i[update destroy]) } + end + + context 'when record is the same user' do + let(:record) { user } + + it { is_expected.to permit_action(:show) } + it { is_expected.not_to permit_actions(%i[update destroy]) } + end + end +end diff --git a/spec/requests/settings_spec.rb b/spec/requests/settings_spec.rb index 42dc26026..cd2f86bd5 100644 --- a/spec/requests/settings_spec.rb +++ b/spec/requests/settings_spec.rb @@ -137,7 +137,7 @@ RSpec.describe 'Settings', type: :request do setting = Setting.find_by(name: 'product_name') get "/api/v1/settings/#{setting.id}", params: {}, as: :json expect(response).to have_http_status(:unauthorized) - expect(json_response['error']).to eq('Not authorized (required ["admin.branding"])') + expect(json_response['error']).to eq('Not authorized (required ["admin.branding"])!') setting = Setting.find_by(name: 'api_token_access') get "/api/v1/settings/#{setting.id}", params: {}, as: :json @@ -157,7 +157,7 @@ RSpec.describe 'Settings', type: :request do } put "/api/v1/settings/#{setting.id}", params: params, as: :json expect(response).to have_http_status(:unauthorized) - expect(json_response['error']).to eq('Not authorized (required ["admin.branding"])') + expect(json_response['error']).to eq('Not authorized (required ["admin.branding"])!') # update setting = Setting.find_by(name: 'api_token_access') diff --git a/spec/support/pundit_matchers.rb b/spec/support/pundit_matchers.rb new file mode 100644 index 000000000..0e4f4fcdf --- /dev/null +++ b/spec/support/pundit_matchers.rb @@ -0,0 +1 @@ +require 'pundit/matchers'