From 585f3dd6c72078b3b54cdec108793beaf4f231e8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juanjo=20Baz=C3=A1n?= Date: Wed, 27 Apr 2016 13:27:20 +0200 Subject: [PATCH 1/4] removes use of current_user in management favors managed_user to avoid conflicts with logged admin users --- app/controllers/management/base_controller.rb | 10 +++++----- app/controllers/management/proposals_controller.rb | 8 +++----- .../management/spending_proposals_controller.rb | 4 ++-- 3 files changed, 10 insertions(+), 12 deletions(-) diff --git a/app/controllers/management/base_controller.rb b/app/controllers/management/base_controller.rb index f01bbebbc..f7a9c2214 100644 --- a/app/controllers/management/base_controller.rb +++ b/app/controllers/management/base_controller.rb @@ -16,16 +16,16 @@ class Management::BaseController < ActionController::Base session[:manager] end - def managed_user - @managed_user ||= Verification::Management::ManagedUser.find(session[:document_type], session[:document_number]) - end - def current_user managed_user end + def managed_user + @managed_user ||= Verification::Management::ManagedUser.find(session[:document_type], session[:document_number]) + end + def check_verified_user(alert_msg) - unless current_user.level_two_or_three_verified? + unless managed_user.level_two_or_three_verified? redirect_to management_document_verifications_path, alert: alert_msg end end diff --git a/app/controllers/management/proposals_controller.rb b/app/controllers/management/proposals_controller.rb index 3f4284a06..a1d0ed439 100644 --- a/app/controllers/management/proposals_controller.rb +++ b/app/controllers/management/proposals_controller.rb @@ -17,7 +17,7 @@ class Management::ProposalsController < Management::BaseController end def vote - @proposal.register_vote(current_user, 'yes') + @proposal.register_vote(managed_user, 'yes') set_proposal_votes(@proposal) end @@ -44,14 +44,12 @@ class Management::ProposalsController < Management::BaseController check_verified_user t("management.proposals.alert.unverified_user") end - ### Duplicated in application_controller. Move to a concern. def set_proposal_votes(proposals) - @proposal_votes = current_user ? current_user.proposal_votes(proposals) : {} + @proposal_votes = managed_user ? managed_user.proposal_votes(proposals) : {} end def set_comment_flags(comments) - @comment_flags = current_user ? current_user.comment_flags(comments) : {} + @comment_flags = managed_user ? managed_user.comment_flags(comments) : {} end - ### end diff --git a/app/controllers/management/spending_proposals_controller.rb b/app/controllers/management/spending_proposals_controller.rb index 49378ca63..b505d0901 100644 --- a/app/controllers/management/spending_proposals_controller.rb +++ b/app/controllers/management/spending_proposals_controller.rb @@ -28,7 +28,7 @@ class Management::SpendingProposalsController < Management::BaseController end def vote - @spending_proposal.register_vote(current_user, 'yes') + @spending_proposal.register_vote(managed_user, 'yes') set_spending_proposal_votes(@spending_proposal) end @@ -54,7 +54,7 @@ class Management::SpendingProposalsController < Management::BaseController # This should not be necessary. Maybe we could create a specific show view for managers. def set_spending_proposal_votes(spending_proposals) - @spending_proposal_votes = current_user ? current_user.spending_proposal_votes(spending_proposals) : {} + @spending_proposal_votes = managed_user ? managed_user.spending_proposal_votes(spending_proposals) : {} end def set_geozone_name From 5ec1964d30655c68ff2db6f507398c78ea7d561c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juanjo=20Baz=C3=A1n?= Date: Wed, 27 Apr 2016 14:21:09 +0200 Subject: [PATCH 2/4] changes specs (flaky tests in travis) --- .../features/admin/spending_proposals_spec.rb | 27 ++++++++++--------- 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/spec/features/admin/spending_proposals_spec.rb b/spec/features/admin/spending_proposals_spec.rb index f8cc12133..fe1b87fc3 100644 --- a/spec/features/admin/spending_proposals_spec.rb +++ b/spec/features/admin/spending_proposals_spec.rb @@ -87,11 +87,11 @@ feature 'Admin spending proposals' do expect(page).to have_link("Realocate visitors") click_link("Realocate visitors") - click_on("Edit classification") + click_link("Edit classification") expect(page).to have_button("Update") - click_on("Back") + click_link("Back") expect(page).to_not have_button("Update") - click_on("Back") + click_link("Back") expect(page).to_not have_link("Destroy the city") expect(page).to have_link("Realocate visitors") @@ -131,11 +131,11 @@ feature 'Admin spending proposals' do expect(page).to have_link("Realocate visitors") click_link("Realocate visitors") - click_on("Edit classification") + click_link("Edit classification") expect(page).to have_button("Update") - click_on("Back") + click_link("Back") expect(page).to_not have_button("Update") - click_on("Back") + click_link("Back") expect(page).to have_content('There is 1 spending proposal') expect(page).to_not have_link("Destroy the city") @@ -178,11 +178,11 @@ feature 'Admin spending proposals' do expect(page).to have_link("Realocate visitors") click_link("Realocate visitors") - click_on("Edit classification") + click_link("Edit classification") expect(page).to have_button("Update") - click_on("Back") + click_link("Back") expect(page).to_not have_button("Update") - click_on("Back") + click_link("Back") expect(page).to have_content('There is 1 spending proposal') expect(page).to_not have_link("Destroy the city") @@ -287,11 +287,11 @@ feature 'Admin spending proposals' do expect(page).to have_content("More schools") click_link("Educate the children") - click_on("Edit classification") + click_link("Edit classification") expect(page).to have_button("Update") - click_on("Back") + click_link("Back") expect(page).to_not have_button("Update") - click_on("Back") + click_link("Back") expect(page).to_not have_content("More hospitals") expect(page).to have_content("Educate the children") @@ -401,6 +401,9 @@ feature 'Admin spending proposals' do click_link 'Edit classification' find('.js-add-tag-link', text: 'Education').click + + fill_in 'spending_proposal_title', with: 'Updated title' + click_button 'Update' expect(page).to have_content 'Investment project updated succesfully.' From 2eabab657a78feb8239c4ed6490ef684c38b0618 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juanjo=20Baz=C3=A1n?= Date: Wed, 27 Apr 2016 17:23:58 +0200 Subject: [PATCH 3/4] allows admin to sign in into management --- .../management/sessions_controller.rb | 19 +++++++++++++++---- .../management/sessions_controller_spec.rb | 15 +++++++++++++-- 2 files changed, 28 insertions(+), 6 deletions(-) diff --git a/app/controllers/management/sessions_controller.rb b/app/controllers/management/sessions_controller.rb index bad38663e..d3a85402b 100644 --- a/app/controllers/management/sessions_controller.rb +++ b/app/controllers/management/sessions_controller.rb @@ -4,11 +4,10 @@ class Management::SessionsController < ActionController::Base def create destroy_session - if manager = ManagerAuthenticator.new(params).auth - session[:manager] = manager + if admin? || manager? redirect_to management_root_path else - raise ActionController::RoutingError.new('Not Found') + raise CanCan::AccessDenied end end @@ -25,4 +24,16 @@ class Management::SessionsController < ActionController::Base session[:document_number] = nil end -end \ No newline at end of file + def admin? + if current_user.try(:administrator?) + session[:manager] = {login: "admin_user_#{current_user.id}"} + end + end + + def manager? + if manager = ManagerAuthenticator.new(params).auth + session[:manager] = manager + end + end + +end diff --git a/spec/controllers/management/sessions_controller_spec.rb b/spec/controllers/management/sessions_controller_spec.rb index af53f4ae0..a07067ed9 100644 --- a/spec/controllers/management/sessions_controller_spec.rb +++ b/spec/controllers/management/sessions_controller_spec.rb @@ -3,9 +3,9 @@ require 'rails_helper' describe Management::SessionsController do describe 'Sign in' do - it "should return 404 if wrong credentials" do + it "should deny access if wrong manager credentials" do allow_any_instance_of(ManagerAuthenticator).to receive(:auth).and_return(false) - expect { get :create, login: "nonexistent" , clave_usuario: "wrong"}.to raise_error "Not Found" + expect { get :create, login: "nonexistent" , clave_usuario: "wrong"}.to raise_error CanCan::AccessDenied end it "should redirect to management root path if right credentials" do @@ -15,6 +15,17 @@ describe Management::SessionsController do get :create, login: "JJB033" , clave_usuario: "31415926", fecha_conexion: "20151031135905" expect(response).to be_redirect end + + it "should redirect to management root path if user is admin" do + sign_in create(:administrator).user + get :create + expect(response).to be_redirect + end + + it "should deny access if user is not admin" do + sign_in create(:user) + expect { get :create}.to raise_error CanCan::AccessDenied + end end describe 'Sign out' do From 325d4e054415eddd17140936b15c2a5f9b0fbe2f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juanjo=20Baz=C3=A1n?= Date: Wed, 27 Apr 2016 17:24:33 +0200 Subject: [PATCH 4/4] adds link to management in admin header --- app/views/shared/_admin_login_items.html.erb | 6 ++++++ config/locales/en.yml | 1 + config/locales/es.yml | 1 + config/routes.rb | 2 +- 4 files changed, 9 insertions(+), 1 deletion(-) diff --git a/app/views/shared/_admin_login_items.html.erb b/app/views/shared/_admin_login_items.html.erb index 390b8c924..3625432a8 100644 --- a/app/views/shared/_admin_login_items.html.erb +++ b/app/views/shared/_admin_login_items.html.erb @@ -16,4 +16,10 @@ <%= link_to t("layouts.header.valuation"), valuation_root_path %> <% end %> + + <% if current_user.administrator? %> +
  • + <%= link_to t("layouts.header.management"), management_sign_in_path %> +
    • + <% end %> <% end %> diff --git a/config/locales/en.yml b/config/locales/en.yml index de37b5975..5cdf877bc 100755 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -191,6 +191,7 @@ en: external_link_transparency_url: https://transparencia.madrid.es locale: 'Language:' logo: Madrid + management: Management moderation: Moderation valuation: Valuation more_information: More information diff --git a/config/locales/es.yml b/config/locales/es.yml index d2d01e407..30668e54f 100755 --- a/config/locales/es.yml +++ b/config/locales/es.yml @@ -191,6 +191,7 @@ es: external_link_transparency_url: https://transparencia.madrid.es locale: 'Idioma:' logo: Madrid + management: Gestión moderation: Moderar valuation: Evaluación more_information: Más información diff --git a/config/routes.rb b/config/routes.rb index 9c125664b..1c7af6800 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -229,7 +229,7 @@ Rails.application.routes.draw do resource :account, controller: "account", only: [:show] - get 'sign_in', to: 'sessions#create' + get 'sign_in', to: 'sessions#create', as: :sign_in resource :session, only: [:create, :destroy] resources :proposals, only: [:index, :new, :create, :show] do