From a20b6b73a2c08986b7a660467194bb0a6da0424c Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Mon, 7 Oct 2019 18:06:55 +0200 Subject: [PATCH 1/2] policies: rename "Scope" to "ApplicationScope" For clarity. --- app/policies/application_policy.rb | 2 +- app/policies/champ_policy.rb | 2 +- app/policies/type_de_champ_policy.rb | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/policies/application_policy.rb b/app/policies/application_policy.rb index 91d5d2e01..dfebff858 100644 --- a/app/policies/application_policy.rb +++ b/app/policies/application_policy.rb @@ -34,7 +34,7 @@ class ApplicationPolicy false end - class Scope + class ApplicationScope attr_reader :user, :instructeur, :administrateur, :scope def initialize(account, scope) diff --git a/app/policies/champ_policy.rb b/app/policies/champ_policy.rb index 26bd9a01a..13a44e8c8 100644 --- a/app/policies/champ_policy.rb +++ b/app/policies/champ_policy.rb @@ -1,5 +1,5 @@ class ChampPolicy < ApplicationPolicy - class Scope < Scope + class Scope < ApplicationScope def resolve if user.present? scope diff --git a/app/policies/type_de_champ_policy.rb b/app/policies/type_de_champ_policy.rb index 3f8390014..98bd57ec6 100644 --- a/app/policies/type_de_champ_policy.rb +++ b/app/policies/type_de_champ_policy.rb @@ -1,5 +1,5 @@ class TypeDeChampPolicy < ApplicationPolicy - class Scope < Scope + class Scope < ApplicationScope def resolve if administrateur.present? scope From c2e82e4145808388ffdd33f1ab065687a544bba8 Mon Sep 17 00:00:00 2001 From: Pierre de La Morinerie Date: Tue, 8 Oct 2019 12:28:26 +0000 Subject: [PATCH 2/2] champ_policy: allow instructeurs to access private annotations Fix #4388 --- app/policies/champ_policy.rb | 24 +++++++--- spec/policies/champ_policy_spec.rb | 75 +++++++++++++++++++++++++----- 2 files changed, 81 insertions(+), 18 deletions(-) diff --git a/app/policies/champ_policy.rb b/app/policies/champ_policy.rb index 13a44e8c8..81d2eb11b 100644 --- a/app/policies/champ_policy.rb +++ b/app/policies/champ_policy.rb @@ -1,13 +1,25 @@ class ChampPolicy < ApplicationPolicy class Scope < ApplicationScope def resolve - if user.present? - scope - .joins(:dossier) - .where({ dossiers: { user_id: user.id } }) - else - scope.none + if user.blank? + return scope.none end + + # Users can access public champs on their own dossiers. + resolved_scope = scope + .left_outer_joins(dossier: { groupe_instructeur: [:instructeurs] }) + .where('dossiers.user_id': user.id, private: false) + + if instructeur.present? + # Additionnaly, instructeurs can access private champs + # on dossiers they are allowed to instruct. + instructeur_clause = scope + .left_outer_joins(dossier: { groupe_instructeur: [:instructeurs] }) + .where('instructeurs.id': instructeur.id, private: true) + resolved_scope = resolved_scope.or(instructeur_clause) + end + + resolved_scope end end end diff --git a/spec/policies/champ_policy_spec.rb b/spec/policies/champ_policy_spec.rb index 4dec03f29..441133bb7 100644 --- a/spec/policies/champ_policy_spec.rb +++ b/spec/policies/champ_policy_spec.rb @@ -1,25 +1,76 @@ require 'spec_helper' describe ChampPolicy do - let(:user) { create(:user) } - let(:dossier) { create(:dossier, user: user) } - let!(:champ) { create(:champ_text, dossier: dossier) } + let(:champ) { create(:champ_text, private: private, dossier: dossier) } + let(:dossier) { create(:dossier, user: dossier_owner) } + let(:dossier_owner) { create(:user) } - let(:account) { { user: user } } + let(:signed_in_user) { create(:user) } + let(:account) { { user: signed_in_user } } subject { Pundit.policy_scope(account, Champ) } - context 'when the user has only user rights' do - context 'cannot access champs for other dossiers' do - let(:account) { { user: create(:user) } } + shared_examples_for 'they can access a public champ' do + let(:private) { false } + it { expect(subject.find_by(id: champ.id)).to eq(champ) } + end - it { expect(subject.find_by(id: champ.id)).to eq(nil) } + shared_examples_for 'they can’t access a public champ' do + let(:private) { false } + it { expect(subject.find_by(id: champ.id)).to eq(nil) } + end + + shared_examples_for 'they can access a private champ' do + let(:private) { true } + it { expect(subject.find_by(id: champ.id)).to eq(champ) } + end + + shared_examples_for 'they can’t access a private champ' do + let(:private) { true } + it { expect(subject.find_by(id: champ.id)).to eq(nil) } + end + + context 'when an user only has user rights' do + context 'as the dossier owner' do + let(:signed_in_user) { dossier_owner } + + it_behaves_like 'they can access a public champ' + it_behaves_like 'they can’t access a private champ' end - context 'can access champs for its own dossiers' do - it { - expect(subject.find(champ.id)).to eq(champ) - } + context 'as another user' do + let(:signed_in_user) { create(:user) } + + it_behaves_like 'they can’t access a public champ' + it_behaves_like 'they can’t access a private champ' + end + end + + context 'when the user also has instruction rights' do + let(:instructeur) { create(:instructeur, email: signed_in_user.email, password: signed_in_user.password) } + let(:account) { { user: signed_in_user, instructeur: instructeur } } + + context 'as the dossier instructeur and owner' do + let(:signed_in_user) { dossier_owner } + before { instructeur.assign_to_procedure(dossier.procedure) } + + it_behaves_like 'they can access a public champ' + it_behaves_like 'they can access a private champ' + end + + context 'as the dossier instructeur (but not owner)' do + let(:signed_in_user) { create(:user) } + before { instructeur.assign_to_procedure(dossier.procedure) } + + it_behaves_like 'they can’t access a public champ' + it_behaves_like 'they can access a private champ' + end + + context 'as an instructeur not assigned to the procedure' do + let(:signed_in_user) { create(:user) } + + it_behaves_like 'they can’t access a public champ' + it_behaves_like 'they can’t access a private champ' end end end