From d1544bc4aeda20620d2dfff5c3bee815731f8ff0 Mon Sep 17 00:00:00 2001 From: Martin Date: Thu, 21 Jul 2022 16:52:31 +0200 Subject: [PATCH] feat(Administrateurs::ProcedureAdministrateurs): ensure admin from super admin can not add administrateur --- .../administrateur_controller.rb | 10 ++-- .../administrateurs/archives_controller.rb | 1 - .../procedure_administrateurs_controller.rb | 4 +- .../instructeurs/instructeur_controller.rb | 10 ++-- .../_add_admin_form.html.haml | 6 +-- .../create.turbo_stream.haml | 2 +- .../procedure_administrateurs/index.html.haml | 5 +- ...ocedure_administrateurs_controller_spec.rb | 12 ++++- .../procedures_controller_spec.rb | 10 +++- spec/factories/administrateurs_procedure.rb | 6 +++ .../procedure_administrateurs_spec.rb | 49 +++++++++++++++++++ .../administrateurs/procedure_update_spec.rb | 14 ------ 12 files changed, 95 insertions(+), 34 deletions(-) create mode 100644 spec/factories/administrateurs_procedure.rb create mode 100644 spec/system/administrateurs/procedure_administrateurs_spec.rb diff --git a/app/controllers/administrateurs/administrateur_controller.rb b/app/controllers/administrateurs/administrateur_controller.rb index 9f182bc6c..53ef7a77b 100644 --- a/app/controllers/administrateurs/administrateur_controller.rb +++ b/app/controllers/administrateurs/administrateur_controller.rb @@ -11,15 +11,15 @@ module Administrateurs redirect_to admin_procedures_path, status: 404 end - def retrieve_procedure_administration + def is_administrateur_through_procedure_administration_as_manager? id = params[:procedure_id] || params[:id] - @procedure_administration = current_administrateur.administrateurs_procedures.find_by(procedure_id: id) - end + current_administrateur.administrateurs_procedures + .exists?(procedure_id: id, manager: true) + end def ensure_not_super_admin! - procedure_administration = retrieve_procedure_administration - if procedure_administration.manager? + if is_administrateur_through_procedure_administration_as_manager? redirect_back fallback_location: root_url, alert: "Interdit aux super admins", status: 403 end end diff --git a/app/controllers/administrateurs/archives_controller.rb b/app/controllers/administrateurs/archives_controller.rb index a7e3db280..e93015927 100644 --- a/app/controllers/administrateurs/archives_controller.rb +++ b/app/controllers/administrateurs/archives_controller.rb @@ -1,7 +1,6 @@ module Administrateurs class ArchivesController < AdministrateurController before_action :retrieve_procedure - before_action :retrieve_procedure_administration before_action :ensure_not_super_admin! helper_method :create_archive_url diff --git a/app/controllers/administrateurs/procedure_administrateurs_controller.rb b/app/controllers/administrateurs/procedure_administrateurs_controller.rb index 3bc56f546..25b244929 100644 --- a/app/controllers/administrateurs/procedure_administrateurs_controller.rb +++ b/app/controllers/administrateurs/procedure_administrateurs_controller.rb @@ -1,8 +1,9 @@ module Administrateurs class ProcedureAdministrateursController < AdministrateurController before_action :retrieve_procedure, except: [:new] - + before_action :ensure_not_super_admin!, only: [:create] def index + @disabled_as_super_admin = is_administrateur_through_procedure_administration_as_manager? end def create @@ -24,6 +25,7 @@ module Administrateurs # Actually add the admin @procedure.administrateurs << administrateur @administrateur = administrateur + @disabled_as_super_admin = is_administrateur_through_procedure_administration_as_manager? flash.notice = "L’administrateur « #{administrateur.email} » a été ajouté à la démarche « #{@procedure.libelle} »." end diff --git a/app/controllers/instructeurs/instructeur_controller.rb b/app/controllers/instructeurs/instructeur_controller.rb index edbb6bfbb..90b74d563 100644 --- a/app/controllers/instructeurs/instructeur_controller.rb +++ b/app/controllers/instructeurs/instructeur_controller.rb @@ -7,16 +7,18 @@ module Instructeurs end def ensure_not_super_admin! - if is_super_admin_through_assign_tos_as_manager? + if is_instructeur_through_assign_tos_as_manager? redirect_back fallback_location: root_url, alert: "Interdit aux super admins", status: 403 end end - def is_super_admin_through_assign_tos_as_manager? + def is_instructeur_through_assign_tos_as_manager? + procedure_id = params[:procedure_id] + current_instructeur.assign_to .where(instructeur: current_instructeur, - groupe_instructeur: current_instructeur.groupe_instructeurs.where(procedure_id: @procedure.id), - manager: true) + groupe_instructeur: current_instructeur.groupe_instructeurs.where(procedure_id: procedure_id), + manager: true) .count .positive? end diff --git a/app/views/administrateurs/procedure_administrateurs/_add_admin_form.html.haml b/app/views/administrateurs/procedure_administrateurs/_add_admin_form.html.haml index 3f744264a..2aa4a8acd 100644 --- a/app/views/administrateurs/procedure_administrateurs/_add_admin_form.html.haml +++ b/app/views/administrateurs/procedure_administrateurs/_add_admin_form.html.haml @@ -1,9 +1,9 @@ = form_for procedure.administrateurs.new(user: User.new), url: { controller: 'procedure_administrateurs' }, - html: { class: 'form', id: "new_administrateur" }, + html: { class: 'form', id: "new_administrateur" }, data: { turbo: true } do |f| = f.label :email do Ajouter un administrateur %p.notice Renseignez l’email d’un administrateur déjà enregistré sur #{APPLICATION_NAME} pour lui permettre de modifier « #{procedure.libelle} ». - = f.email_field :email, placeholder: 'marie.dupont@exemple.fr', required: true - = f.submit 'Ajouter comme administrateur', class: 'button primary send' + = f.email_field :email, placeholder: 'marie.dupont@exemple.fr', required: true, disabled: disabled_as_super_admin + = f.submit 'Ajouter comme administrateur', class: 'button primary send', disabled: disabled_as_super_admin diff --git a/app/views/administrateurs/procedure_administrateurs/create.turbo_stream.haml b/app/views/administrateurs/procedure_administrateurs/create.turbo_stream.haml index b8af77c66..a49c3649e 100644 --- a/app/views/administrateurs/procedure_administrateurs/create.turbo_stream.haml +++ b/app/views/administrateurs/procedure_administrateurs/create.turbo_stream.haml @@ -1,3 +1,3 @@ - if @administrateur.present? = turbo_stream.append "administrateurs", partial: 'administrateur', locals: { procedure: @procedure, administrateur: @administrateur } - = turbo_stream.replace "new_administrateur", partial: 'add_admin_form', locals: { procedure: @procedure } + = turbo_stream.replace "new_administrateur", partial: 'add_admin_form', locals: { procedure: @procedure, disabled_as_super_admin: @disabled_as_super_admin } diff --git a/app/views/administrateurs/procedure_administrateurs/index.html.haml b/app/views/administrateurs/procedure_administrateurs/index.html.haml index 9fbccb6bd..a1ff5499b 100644 --- a/app/views/administrateurs/procedure_administrateurs/index.html.haml +++ b/app/views/administrateurs/procedure_administrateurs/index.html.haml @@ -4,7 +4,8 @@ 'Administrateurs'], preview: false } .container - %h1 Administrateurs de « #{@procedure.libelle} » + %h1 Gérer les administrateurs de « #{@procedure.libelle} » + %table.table %thead %th= 'Adresse email' @@ -15,4 +16,4 @@ %tfoot %tr %th{ colspan: 4 } - = render 'add_admin_form', procedure: @procedure + = render 'add_admin_form', procedure: @procedure, disabled_as_super_admin: @disabled_as_super_admin diff --git a/spec/controllers/administrateurs/procedure_administrateurs_controller_spec.rb b/spec/controllers/administrateurs/procedure_administrateurs_controller_spec.rb index 15122c393..1f277b2fa 100644 --- a/spec/controllers/administrateurs/procedure_administrateurs_controller_spec.rb +++ b/spec/controllers/administrateurs/procedure_administrateurs_controller_spec.rb @@ -1,14 +1,24 @@ describe Administrateurs::ProcedureAdministrateursController, type: :controller do let(:signed_in_admin) { create(:administrateur) } let(:other_admin) { create(:administrateur) } - let(:procedure) { create(:procedure, administrateurs: [signed_in_admin, other_admin]) } + let!(:administrateurs_procedure) { create(:administrateurs_procedure, administrateur: signed_in_admin, procedure: procedure, manager: manager) } + let!(:procedure) { create(:procedure, administrateurs: [other_admin]) } render_views before do sign_in(signed_in_admin.user) end + describe '#create' do + context 'as manager' do + let(:manager) { true } + subject { post :create, params: { procedure_id: procedure.id, administrateur: { email: create(:administrateur).email } }, format: :turbo_stream } + it { is_expected.to have_http_status(:forbidden) } + end + end + describe '#destroy' do + let(:manager) { false } subject do delete :destroy, params: { procedure_id: procedure.id, id: admin_to_remove.id }, format: :turbo_stream end diff --git a/spec/controllers/instructeurs/procedures_controller_spec.rb b/spec/controllers/instructeurs/procedures_controller_spec.rb index ffd65d8e5..f26da91c2 100644 --- a/spec/controllers/instructeurs/procedures_controller_spec.rb +++ b/spec/controllers/instructeurs/procedures_controller_spec.rb @@ -468,9 +468,10 @@ describe Instructeurs::ProceduresController, type: :controller do describe '#download_export' do let(:instructeur) { create(:instructeur) } let!(:procedure) { create(:procedure) } - let!(:gi_0) { procedure.defaut_groupe_instructeur } + let!(:assign_to) { create(:assign_to, instructeur: instructeur, groupe_instructeur: build(:groupe_instructeur, procedure: procedure), manager: manager) } + let!(:gi_0) { assign_to.groupe_instructeur } let!(:gi_1) { create(:groupe_instructeur, label: 'gi_1', procedure: procedure, instructeurs: [instructeur]) } - + let(:manager) { false } before { sign_in(instructeur.user) } subject do @@ -535,6 +536,11 @@ describe Instructeurs::ProceduresController, type: :controller do expect(response).to have_http_status(:ok) end end + + context 'when logged in through super admin' do + let(:manager) { true } + it { is_expected.to have_http_status(:forbidden) } + end end describe '#create_multiple_commentaire' do diff --git a/spec/factories/administrateurs_procedure.rb b/spec/factories/administrateurs_procedure.rb new file mode 100644 index 000000000..5a12b3d42 --- /dev/null +++ b/spec/factories/administrateurs_procedure.rb @@ -0,0 +1,6 @@ +FactoryBot.define do + factory :administrateurs_procedure do + association :administrateur + association :procedure + end +end diff --git a/spec/system/administrateurs/procedure_administrateurs_spec.rb b/spec/system/administrateurs/procedure_administrateurs_spec.rb new file mode 100644 index 000000000..83b886a62 --- /dev/null +++ b/spec/system/administrateurs/procedure_administrateurs_spec.rb @@ -0,0 +1,49 @@ +require 'system/administrateurs/procedure_spec_helper' + +describe 'Administrateurs can manage administrateurs', js: true do + include ProcedureSpecHelper + + let(:administrateur) { create(:administrateur) } + let!(:procedure) { create(:procedure) } + let!(:administrateurs_procedure) { create(:administrateurs_procedure, administrateur: administrateur, procedure: procedure, manager: manager) } + let(:manager) { false } + before do + login_as administrateur.user, scope: :user + end + + scenario 'card is clickable' do + visit admin_procedure_path(procedure) + find('#administrateurs').click + expect(page).to have_css(:h1, text: "Administrateurs de « #{procedure.libelle} »") + end + + context 'as admin not flagged from manager' do + let(:manager) { false } + + scenario 'the administrator can add another administrator' do + another_administrateur = create(:administrateur) + visit admin_procedure_administrateurs_path(procedure) + find('#administrateurs').click + + fill_in('administrateur_email', with: another_administrateur.email) + + click_on 'Ajouter comme administrateur' + + within('.alert-success') do + expect(page).to have_content(another_administrateur.email) + end + end + end + + context 'as admin flagged from manager' do + let(:manager) { true } + scenario 'the administrator from manager can not add another administrator' do + administrateur.administrateurs_procedures.update_all(manager: true) + visit admin_procedure_administrateurs_path(procedure) + + find('#administrateurs').click + + expect(page).to have_css("#administrateur_email[disabled=\"disabled\"]") + end + end +end diff --git a/spec/system/administrateurs/procedure_update_spec.rb b/spec/system/administrateurs/procedure_update_spec.rb index b33599eda..ffb1c159d 100644 --- a/spec/system/administrateurs/procedure_update_spec.rb +++ b/spec/system/administrateurs/procedure_update_spec.rb @@ -58,18 +58,4 @@ describe 'Administrateurs can edit procedures', js: true do expect(page).to have_selector('.breadcrumbs li', text: 'Ma petite démarche') end end - - scenario 'the administrator can add another administrator' do - another_administrateur = create(:administrateur) - visit admin_procedure_path(procedure) - find('#administrateurs').click - - fill_in('administrateur_email', with: another_administrateur.email) - - click_on 'Ajouter comme administrateur' - - within('.alert-success') do - expect(page).to have_content(another_administrateur.email) - end - end end