From 97779adb1bcaa18e548892e1e093857d67da7260 Mon Sep 17 00:00:00 2001 From: JC Avena Date: Sat, 2 Jul 2016 18:26:53 -0500 Subject: [PATCH 1/2] Users should only be able to edit their own facilities. Non Admin users do not see a link to edit their own facilities. The /edit path still works, making this security merely cosmetic since any user can access the edit page for any facility. We have a facility_users link table we can use to filter access. Perhaps the table needs to be updated to include a facility role as well. Roles could be 'admin', 'owner', 'staff'. We can use these roles to enable/disable certain behaviors per facilities in the future. Even without the extra role, we can still use the table to keep non facility users out of all but the "show" action. This change implements the access filter for facilities. Access is restricted for anything other than the "show" action for non facility user. If we think this is a reasonable approach, I can keep going down this path and improve this to include the roles as well. --- app/controllers/application_controller.rb | 12 ++++++++++++ app/controllers/facilities_controller.rb | 15 ++++++++++++--- app/models/user.rb | 4 ++++ 3 files changed, 28 insertions(+), 3 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index b084d5f..701c638 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -8,6 +8,18 @@ def after_sign_in_path_for(resource) new_facility_path end + def render_unauthorized(text) + render_status_code(text, 401) + end + + def render_not_found(text) + render_status_code(text, 404) + end + + def render_status_code(text, code) + render(:text => text, :status => code, :layout => 'application') and return + end + protected diff --git a/app/controllers/facilities_controller.rb b/app/controllers/facilities_controller.rb index 34a1a92..dd3ffdb 100644 --- a/app/controllers/facilities_controller.rb +++ b/app/controllers/facilities_controller.rb @@ -1,8 +1,8 @@ class FacilitiesController < ApplicationController + before_action :set_facility_user before_action :set_facility, only: [:show, :edit, :update, :destroy] before_action :authenticate_user!, except: [:show] - before_action :set_facility_user - + # GET /facilities # GET /facilities.json def index @@ -68,7 +68,16 @@ def destroy private # Use callbacks to share common setup or constraints between actions. def set_facility - @facility = Facility.find(params[:id]) + + if current_user.is_site_admin? || action_name == 'show' + @facility = Facility.find(params[:id]) + else #restrict edit/update/destroy to only facilities associated with the user. + begin + @facility = current_user.facilities.find(params[:id]) + rescue + render_not_found('Sorry, that facility could not be found.') + end + end end def set_facility_user diff --git a/app/models/user.rb b/app/models/user.rb index 7c74876..8a94bf5 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -25,5 +25,9 @@ def full_name def display_name full_name || email end + + def is_site_admin? + site_admin? + end end From 5c12c8066b9f009dd6152660743c056a5ca5fc6a Mon Sep 17 00:00:00 2001 From: JC Avena Date: Mon, 4 Jul 2016 16:10:32 -0500 Subject: [PATCH 2/2] Fix some style alerts. --- app/controllers/application_controller.rb | 3 +-- app/controllers/facilities_controller.rb | 7 +++---- app/models/user.rb | 5 ----- 3 files changed, 4 insertions(+), 11 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 701c638..55ca1a2 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -17,10 +17,9 @@ def render_not_found(text) end def render_status_code(text, code) - render(:text => text, :status => code, :layout => 'application') and return + render(text: text, status: code, layout: 'application') && return end - protected def configure_permitted_parameters diff --git a/app/controllers/facilities_controller.rb b/app/controllers/facilities_controller.rb index dd3ffdb..39139c6 100644 --- a/app/controllers/facilities_controller.rb +++ b/app/controllers/facilities_controller.rb @@ -2,7 +2,7 @@ class FacilitiesController < ApplicationController before_action :set_facility_user before_action :set_facility, only: [:show, :edit, :update, :destroy] before_action :authenticate_user!, except: [:show] - + # GET /facilities # GET /facilities.json def index @@ -68,10 +68,9 @@ def destroy private # Use callbacks to share common setup or constraints between actions. def set_facility - - if current_user.is_site_admin? || action_name == 'show' + if current_user.site_admin? || action_name == 'show' @facility = Facility.find(params[:id]) - else #restrict edit/update/destroy to only facilities associated with the user. + else # restrict edit/update/destroy to only facilities associated with the user. begin @facility = current_user.facilities.find(params[:id]) rescue diff --git a/app/models/user.rb b/app/models/user.rb index 8a94bf5..86736ab 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -25,9 +25,4 @@ def full_name def display_name full_name || email end - - def is_site_admin? - site_admin? - end - end