From fc75580a3ce7c0d2ddd8aea759c46232d12a95ac Mon Sep 17 00:00:00 2001 From: Paul Chavard Date: Thu, 27 Jun 2019 16:26:07 +0200 Subject: [PATCH] Start using pundit --- Gemfile | 1 + Gemfile.lock | 3 + app/controllers/application_controller.rb | 15 +++-- app/controllers/champs/carte_controller.rb | 10 +--- .../champs/repetition_controller.rb | 5 +- app/models/gestionnaire.rb | 4 ++ app/policies/application_policy.rb | 49 ++++++++++++++++ app/policies/champ_policy.rb | 24 ++++++++ app/policies/type_de_champ_policy.rb | 13 +++++ spec/policies/champ_policy_spec.rb | 56 +++++++++++++++++++ spec/policies/type_de_champ_policy_spec.rb | 23 ++++++++ 11 files changed, 187 insertions(+), 16 deletions(-) create mode 100644 app/policies/application_policy.rb create mode 100644 app/policies/champ_policy.rb create mode 100644 app/policies/type_de_champ_policy.rb create mode 100644 spec/policies/champ_policy_spec.rb create mode 100644 spec/policies/type_de_champ_policy_spec.rb diff --git a/Gemfile b/Gemfile index 7ffa37c1c..4bc7bd8ca 100644 --- a/Gemfile +++ b/Gemfile @@ -47,6 +47,7 @@ gem 'prawn' # PDF Generation gem 'prawn_rails' gem 'premailer-rails' gem 'puma' # Use Puma as the app server +gem 'pundit' gem 'rack-mini-profiler' gem 'rails' gem 'rails-i18n' # Locales par défaut diff --git a/Gemfile.lock b/Gemfile.lock index b925daa7a..8b597a428 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -431,6 +431,8 @@ GEM pry (~> 0.10) public_suffix (3.0.3) puma (3.12.0) + pundit (2.0.1) + activesupport (>= 3.0.0) rack (2.0.6) rack-mini-profiler (1.0.1) rack (>= 1.2.0) @@ -749,6 +751,7 @@ DEPENDENCIES premailer-rails pry-byebug puma + pundit rack-mini-profiler rails rails-controller-testing diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 9b58e571d..f405328a9 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -1,5 +1,6 @@ class ApplicationController < ActionController::Base include TrustedDeviceConcern + include Pundit MAINTENANCE_MESSAGE = 'Le site est actuellement en maintenance. Il sera à nouveau disponible dans un court instant.' @@ -41,12 +42,18 @@ class ApplicationController < ActionController::Base logged_user.present? end - def logged_user_ids - logged_users.map(&:id) - end - helper_method :logged_in? + def pundit_user + if administrateur_signed_in? + current_administrateur + elsif gestionnaire_signed_in? + current_gestionnaire + else + current_user + end + end + protected def authenticate_logged_user! diff --git a/app/controllers/champs/carte_controller.rb b/app/controllers/champs/carte_controller.rb index d214f30d0..1410a9cde 100644 --- a/app/controllers/champs/carte_controller.rb +++ b/app/controllers/champs/carte_controller.rb @@ -14,15 +14,9 @@ class Champs::CarteController < ApplicationController end @champ = if params[:champ_id].present? - Champ - .joins(:dossier) - .where(dossiers: { user_id: logged_user_ids }) - .find(params[:champ_id]) + policy_scope(Champ).find(params[:champ_id]) else - TypeDeChamp - .joins(:procedure) - .where(procedures: { administrateur_id: logged_user_ids }) - .find(params[:type_de_champ_id]).champ.build + policy_scope(TypeDeChamp).find(params[:type_de_champ_id]).champ.build end geo_areas = [] diff --git a/app/controllers/champs/repetition_controller.rb b/app/controllers/champs/repetition_controller.rb index c5d8b12ea..ab0596abb 100644 --- a/app/controllers/champs/repetition_controller.rb +++ b/app/controllers/champs/repetition_controller.rb @@ -2,10 +2,7 @@ class Champs::RepetitionController < ApplicationController before_action :authenticate_logged_user! def show - @champ = Champ - .joins(:dossier) - .where(dossiers: { user_id: logged_user_ids }) - .find(params[:champ_id]) + @champ = policy_scope(Champ).find(params[:champ_id]) @position = params[:position] row = (@champ.champs.empty? ? 0 : @champ.champs.last.row) + 1 diff --git a/app/models/gestionnaire.rb b/app/models/gestionnaire.rb index 1c94f5070..ea633c7e8 100644 --- a/app/models/gestionnaire.rb +++ b/app/models/gestionnaire.rb @@ -225,6 +225,10 @@ class Gestionnaire < ApplicationRecord end end + def user + User.find_by(email: email) + end + private def annotations_hash(demande, annotations_privees, avis, messagerie) diff --git a/app/policies/application_policy.rb b/app/policies/application_policy.rb new file mode 100644 index 000000000..eefe976c6 --- /dev/null +++ b/app/policies/application_policy.rb @@ -0,0 +1,49 @@ +class ApplicationPolicy + attr_reader :user, :record + + def initialize(user, record) + @user = user + @record = record + end + + def index? + false + end + + def show? + false + end + + def create? + false + end + + def new? + create? + end + + def update? + false + end + + def edit? + update? + end + + def destroy? + false + end + + class Scope + attr_reader :user, :scope + + def initialize(user, scope) + @user = user + @scope = scope + end + + def resolve + scope.all + end + end +end diff --git a/app/policies/champ_policy.rb b/app/policies/champ_policy.rb new file mode 100644 index 000000000..601fa4529 --- /dev/null +++ b/app/policies/champ_policy.rb @@ -0,0 +1,24 @@ +class ChampPolicy < ApplicationPolicy + class Scope < Scope + def resolve + if user.is_a?(User) + scope + .joins(:dossier) + .where({ dossiers: { user_id: user.id } }) + elsif user.is_a?(Gestionnaire) + scope_with_join = scope.joins(dossier: :follows) + scope_with_left_join = scope.left_joins(dossier: :follows) + + if user.user + scope_with_left_join + .where({ dossiers: { user_id: user.user.id } }) + .or(scope_with_left_join.where(dossiers: { follows: { gestionnaire_id: user.id } })) + else + scope_with_join.where(dossiers: { follows: { gestionnaire_id: user.id } }) + end + else + scope.none + end + end + end +end diff --git a/app/policies/type_de_champ_policy.rb b/app/policies/type_de_champ_policy.rb new file mode 100644 index 000000000..3fab51906 --- /dev/null +++ b/app/policies/type_de_champ_policy.rb @@ -0,0 +1,13 @@ +class TypeDeChampPolicy < ApplicationPolicy + class Scope < Scope + def resolve + if user.is_a?(Administrateur) + scope + .joins(procedure: [:administrateurs]) + .where({ administrateurs: { id: user.id } }) + else + scope.none + end + end + end +end diff --git a/spec/policies/champ_policy_spec.rb b/spec/policies/champ_policy_spec.rb new file mode 100644 index 000000000..2e06f464c --- /dev/null +++ b/spec/policies/champ_policy_spec.rb @@ -0,0 +1,56 @@ +require 'spec_helper' + +describe ChampPolicy do + let(:user) { create(:user) } + let(:dossier) { create(:dossier, user: user) } + let!(:champ) { create(:champ_text, dossier: dossier) } + + let(:pundit_user) { user } + subject { Pundit.policy_scope(pundit_user, Champ) } + + context 'when the user has only user rights' do + context 'cannot access champs for other dossiers' do + let(:pundit_user) { create(:user) } + + it { expect(subject.find_by(id: champ.id)).to eq(nil) } + end + + context 'can access champs for its own dossiers' do + it { + expect(subject.find(champ.id)).to eq(champ) + } + end + end + + context 'when the user has only gestionnaire rights' do + context 'can access champs for dossiers it follows' do + let(:dossier) { create(:dossier, :followed) } + let(:pundit_user) { dossier.followers_gestionnaires.first } + + it { expect(subject.find(champ.id)).to eq(champ) } + end + end + + context 'when the user has user and gestionnaire rights' do + let(:pundit_user) { dossier.followers_gestionnaires.first } + let(:dossier) { create(:dossier, :followed) } + + let(:user) { create(:user, email: pundit_user.email) } + let(:dossier2) { create(:dossier, user: user) } + let!(:champ_2) { create(:champ_text, dossier: dossier2) } + + context 'can access champs for dossiers it follows' do + it do + expect(pundit_user.user).to eq(user) + expect(subject.find(champ.id)).to eq(champ) + end + end + + context 'can access champs for its own dossiers' do + it do + expect(pundit_user.user).to eq(user) + expect(subject.find(champ_2.id)).to eq(champ_2) + end + end + end +end diff --git a/spec/policies/type_de_champ_policy_spec.rb b/spec/policies/type_de_champ_policy_spec.rb new file mode 100644 index 000000000..ca3e7e635 --- /dev/null +++ b/spec/policies/type_de_champ_policy_spec.rb @@ -0,0 +1,23 @@ +require 'spec_helper' + +describe TypeDeChampPolicy do + let(:procedure) { create(:procedure) } + let!(:type_de_champ) { create(:type_de_champ_text, procedure: procedure) } + + let(:pundit_user) { create(:user) } + subject { Pundit.policy_scope(pundit_user, TypeDeChamp) } + + context 'when the user has only user rights' do + it 'can not access' do + expect(subject.find_by(id: type_de_champ.id)).to eq(nil) + end + end + + context 'when the user has administrateur rights' do + let(:pundit_user) { procedure.administrateurs.first } + + it 'can access' do + expect(subject.find(type_de_champ.id)).to eq(type_de_champ) + end + end +end