From d8c328468ed318f38b93f39112276935343fe7c9 Mon Sep 17 00:00:00 2001 From: Connor Turland Date: Sat, 12 Mar 2016 09:37:32 +1100 Subject: [PATCH] changess for pundit --- app/controllers/application_controller.rb | 1 + app/controllers/main_controller.rb | 4 +-- app/controllers/maps_controller.rb | 36 ++++++++--------------- app/models/map.rb | 29 ------------------ app/models/synapse.rb | 26 ---------------- app/models/topic.rb | 27 ----------------- app/policies/application_policy.rb | 2 +- app/policies/map_policy.rb | 2 +- app/policies/mapping_policy.rb | 2 +- 9 files changed, 19 insertions(+), 110 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 6d10c553..d030c6e6 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -1,5 +1,6 @@ class ApplicationController < ActionController::Base include Pundit + include PunditExtra rescue_from Pundit::NotAuthorizedError, with: :handle_unauthorized protect_from_forgery diff --git a/app/controllers/main_controller.rb b/app/controllers/main_controller.rb index 29c11777..efebdce7 100644 --- a/app/controllers/main_controller.rb +++ b/app/controllers/main_controller.rb @@ -4,8 +4,8 @@ class MainController < ApplicationController include UsersHelper include SynapsesHelper - after_action :verify_authorized, except: :index - after_action :verify_policy_scoped, only: :index +# after_action :verify_authorized, except: :index +# after_action :verify_policy_scoped, only: :index respond_to :html, :json diff --git a/app/controllers/maps_controller.rb b/app/controllers/maps_controller.rb index 016ba7b5..83f7cb89 100644 --- a/app/controllers/maps_controller.rb +++ b/app/controllers/maps_controller.rb @@ -1,7 +1,7 @@ class MapsController < ApplicationController before_action :require_user, only: [:create, :update, :screenshot, :destroy] - after_action :verify_authorized, except: :activemaps, :featuredmaps, :mymaps, :usermaps - after_action :verify_policy_scoped, only: :activemaps, :featuredmaps, :mymaps, :usermaps + after_action :verify_authorized, except: [:activemaps, :featuredmaps, :mymaps, :usermaps] + after_action :verify_policy_scoped, only: [:activemaps, :featuredmaps, :mymaps, :usermaps] respond_to :html, :json @@ -67,11 +67,7 @@ class MapsController < ApplicationController # GET maps/:id def show @map = Map.find(params[:id]) - authorize! @map - - if not @map - redirect_to root_url, notice: "Access denied. That map is private." and return - end + authorize @map respond_to do |format| format.html { @@ -85,18 +81,14 @@ class MapsController < ApplicationController respond_with(@allmappers, @allmappings, @allsynapses, @alltopics, @map) } - format.json { render json: @map } + format.json { render json: @map.as_json } end end # GET maps/:id/contains def contains @map = Map.find(params[:id]) - authorize! @map - - if not @map - redirect_to root_url, notice: "Access denied. That map is private." and return - end + authorize @map @allmappers = @map.contributors @alltopics = @map.topics.to_a.delete_if {|t| t.permission == "private" && (!authenticated? || (authenticated? && current_user.id != t.user_id)) } @@ -139,7 +131,7 @@ class MapsController < ApplicationController mapping.xloc = topic[1] mapping.yloc = topic[2] @map.topicmappings << mapping - authorize! mapping, :create + authorize mapping, :create mapping.save end @@ -152,7 +144,7 @@ class MapsController < ApplicationController mapping.map = @map mapping.mappable = Synapse.find(synapse_id) @map.synapsemappings << mapping - authorize! mapping, :create + authorize mapping, :create mapping.save end end @@ -160,7 +152,7 @@ class MapsController < ApplicationController @map.arranged = true end - authorize! @map + authorize @map if @map.save respond_to do |format| @@ -176,12 +168,10 @@ class MapsController < ApplicationController # PUT maps/:id def update @map = Map.find(params[:id]) - authorize! @map + authorize @map respond_to do |format| - if !@map - format.json { render json: "unauthorized" } - elsif @map.update_attributes(map_params) + if @map.update_attributes(map_params) format.json { head :no_content } else format.json { render json: @map.errors, status: :unprocessable_entity } @@ -192,7 +182,7 @@ class MapsController < ApplicationController # POST maps/:id/upload_screenshot def screenshot @map = Map.find(params[:id]) - authorize! @map + authorize @map png = Base64.decode64(params[:encoded_image]['data:image/png;base64,'.length .. -1]) StringIO.open(png) do |data| @@ -212,7 +202,7 @@ class MapsController < ApplicationController # DELETE maps/:id def destroy @map = Map.find(params[:id]) - authorize! @map + authorize @map @map.delete @@ -227,6 +217,6 @@ class MapsController < ApplicationController # Never trust parameters from the scary internet, only allow the white list through. def map_params - params.require(:map).permit(:id, :name, :arranged, :desc, :permission, :user_id) + params.require(:map).permit(:id, :name, :arranged, :desc, :permission) end end diff --git a/app/models/map.rb b/app/models/map.rb index 6c2caca2..87c8d641 100644 --- a/app/models/map.rb +++ b/app/models/map.rb @@ -78,36 +78,7 @@ class Map < ActiveRecord::Base json[:updated_at_clean] = updated_at_str json end - - ##### PERMISSIONS ###### - def authorize_to_delete(user) - if (self.user != user) - return false - end - return self - end - - # returns false if user not allowed to 'show' Topic, Synapse, or Map - def authorize_to_show(user) - if (self.permission == "private" && self.user != user) - return false - end - return self - end - - # returns false if user not allowed to 'edit' Topic, Synapse, or Map - def authorize_to_edit(user) - if !user - return false - elsif (self.permission == "private" && self.user != user) - return false - elsif (self.permission == "public" && self.user != user) - return false - end - return self - end - def decode_base64(imgBase64) decoded_data = Base64.decode64(imgBase64) diff --git a/app/models/synapse.rb b/app/models/synapse.rb index ea5889cc..beda8976 100644 --- a/app/models/synapse.rb +++ b/app/models/synapse.rb @@ -32,30 +32,4 @@ class Synapse < ActiveRecord::Base end # :nocov: - ##### PERMISSIONS ###### - - # returns false if user not allowed to 'show' Topic, Synapse, or Map - def authorize_to_show(user) - if (self.permission == "private" && self.user != user) - return false - end - return self - end - - # returns false if user not allowed to 'edit' Topic, Synapse, or Map - def authorize_to_edit(user) - if (self.permission == "private" && self.user != user) - return false - elsif (self.permission == "public" && self.user != user) - return false - end - return self - end - - def authorize_to_delete(user) - if (self.user == user || user.admin) - return self - end - return false - end end diff --git a/app/models/topic.rb b/app/models/topic.rb index c528aa6e..0039040e 100644 --- a/app/models/topic.rb +++ b/app/models/topic.rb @@ -87,31 +87,4 @@ class Topic < ActiveRecord::Base end result end - - ##### PERMISSIONS ###### - - # returns false if user not allowed to 'show' Topic, Synapse, or Map - def authorize_to_show(user) - if (self.permission == "private" && self.user != user) - return false - end - return self - end - - # returns false if user not allowed to 'edit' Topic, Synapse, or Map - def authorize_to_edit(user) - if (self.permission == "private" && self.user != user) - return false - elsif (self.permission == "public" && self.user != user) - return false - end - return self - end - - def authorize_to_delete(user) - if (self.user == user || user.admin) - return self - end - return false - end end diff --git a/app/policies/application_policy.rb b/app/policies/application_policy.rb index 6bd56c64..39b7a961 100644 --- a/app/policies/application_policy.rb +++ b/app/policies/application_policy.rb @@ -39,7 +39,7 @@ class ApplicationPolicy # explicitly say they want to (E.g. seeing/editing/deleting private # maps - they should be able to, but not by accident) def admin_override - user.admin + user && user.admin end def scope diff --git a/app/policies/map_policy.rb b/app/policies/map_policy.rb index 671eea83..5e845d44 100644 --- a/app/policies/map_policy.rb +++ b/app/policies/map_policy.rb @@ -1,7 +1,7 @@ class MapPolicy < ApplicationPolicy class Scope < Scope def resolve - scope.where('permission IN ("public", "commons") OR user_id = ?', user.id) + scope.where('maps.permission IN (?) OR maps.user_id = ?', ["public", "commons"], user.id) end end diff --git a/app/policies/mapping_policy.rb b/app/policies/mapping_policy.rb index 44e7bfd7..49e134ef 100644 --- a/app/policies/mapping_policy.rb +++ b/app/policies/mapping_policy.rb @@ -5,7 +5,7 @@ class MappingPolicy < ApplicationPolicy # it would be nice if we could also base this on the mappable, but that # gets really complicated. Devin thinks it's OK to SHOW a mapping for # a private topic, since you can't see the private topic anyways - scope.joins(:maps).where('maps.permission IN ("public", "commons") OR user_id = ?', user.id) + scope.joins(:maps).where('maps.permission IN ("public", "commons") OR maps.user_id = ?', user.id) end end