mirror of
				https://github.com/varvet/pundit.git
				synced 2022-11-09 12:30:11 -05:00 
			
		
		
		
	Require users to explicitly define Scope#resolve
Closes https://github.com/varvet/pundit/pull/711 (original issue and pull request) > A01:2021-Broken Access Control is the category with the most serious web application security risk. > > Using scope.all in templates violates the principle of least privilege or deny by default, where access should only be granted for particular capabilities, roles, or users. > > This change improves the security of default templates > > Ref: https://owasp.org/Top10/A01_2021-Broken_Access_Control/ — by @tagliala (github.com/tagliala) Co-authored-by: Duncan Stuart <dgmstuart@gmail.com>
This commit is contained in:
		
							parent
							
								
									54675c6f9b
								
							
						
					
					
						commit
						44cfa73d72
					
				
					 5 changed files with 53 additions and 4 deletions
				
			
		| 
						 | 
				
			
			@ -6,6 +6,10 @@
 | 
			
		|||
 | 
			
		||||
- Using `policy_class` and a namespaced record now passes only the record when instantiating the policy. (#697, #689, #694, #666)
 | 
			
		||||
 | 
			
		||||
### Changed
 | 
			
		||||
 | 
			
		||||
- Require users to explicitly define Scope#resolve in generated policies (#711, #722)
 | 
			
		||||
 | 
			
		||||
### Deprecated
 | 
			
		||||
 | 
			
		||||
- Deprecate `include Pundit` in favor of `include Pundit::Authorization` (#621)
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
| 
						 | 
				
			
			@ -43,7 +43,7 @@ class ApplicationPolicy
 | 
			
		|||
    end
 | 
			
		||||
 | 
			
		||||
    def resolve
 | 
			
		||||
      scope.all
 | 
			
		||||
      raise NotImplementedError, "You must define #resolve in #{self.class}"
 | 
			
		||||
    end
 | 
			
		||||
 | 
			
		||||
    private
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
| 
						 | 
				
			
			@ -1,9 +1,10 @@
 | 
			
		|||
<% module_namespacing  do -%>
 | 
			
		||||
class <%= class_name %>Policy < ApplicationPolicy
 | 
			
		||||
  class Scope < Scope
 | 
			
		||||
    def resolve
 | 
			
		||||
      scope.all
 | 
			
		||||
    end
 | 
			
		||||
    # NOTE: Be explicit about which records you allow access to!
 | 
			
		||||
    # def resolve
 | 
			
		||||
    #   scope.all
 | 
			
		||||
    # end
 | 
			
		||||
  end
 | 
			
		||||
end
 | 
			
		||||
<% end  -%>
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
| 
						 | 
				
			
			@ -24,6 +24,7 @@ Gem::Specification.new do |gem|
 | 
			
		|||
  gem.add_development_dependency "activemodel", ">= 3.0.0"
 | 
			
		||||
  gem.add_development_dependency "bundler"
 | 
			
		||||
  gem.add_development_dependency "pry"
 | 
			
		||||
  gem.add_development_dependency "railties", ">= 3.0.0"
 | 
			
		||||
  gem.add_development_dependency "rake"
 | 
			
		||||
  gem.add_development_dependency "rspec", ">= 3.0.0"
 | 
			
		||||
  gem.add_development_dependency "rubocop", "1.24.0"
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
							
								
								
									
										43
									
								
								spec/generators_spec.rb
									
										
									
									
									
										Normal file
									
								
							
							
						
						
									
										43
									
								
								spec/generators_spec.rb
									
										
									
									
									
										Normal file
									
								
							| 
						 | 
				
			
			@ -0,0 +1,43 @@
 | 
			
		|||
# frozen_string_literal: true
 | 
			
		||||
 | 
			
		||||
require "spec_helper"
 | 
			
		||||
require "tmpdir"
 | 
			
		||||
 | 
			
		||||
require "rails/generators"
 | 
			
		||||
require "generators/pundit/install/install_generator"
 | 
			
		||||
require "generators/pundit/policy/policy_generator"
 | 
			
		||||
 | 
			
		||||
RSpec.describe "generators" do
 | 
			
		||||
  before(:all) do
 | 
			
		||||
    @tmpdir = Dir.mktmpdir
 | 
			
		||||
 | 
			
		||||
    Dir.chdir(@tmpdir) do
 | 
			
		||||
      Pundit::Generators::InstallGenerator.new([], { quiet: true }).invoke_all
 | 
			
		||||
      Pundit::Generators::PolicyGenerator.new(%w[Widget], { quiet: true }).invoke_all
 | 
			
		||||
 | 
			
		||||
      require "./app/policies/application_policy"
 | 
			
		||||
      require "./app/policies/widget_policy"
 | 
			
		||||
    end
 | 
			
		||||
  end
 | 
			
		||||
 | 
			
		||||
  after(:all) do
 | 
			
		||||
    FileUtils.remove_entry(@tmpdir)
 | 
			
		||||
  end
 | 
			
		||||
 | 
			
		||||
  describe "WidgetPolicy", type: :policy do
 | 
			
		||||
    permissions :index?, :show?, :create?, :new?, :update?, :edit?, :destroy? do
 | 
			
		||||
      it "has safe defaults" do
 | 
			
		||||
        expect(WidgetPolicy).not_to permit(double("User"), double("Widget"))
 | 
			
		||||
      end
 | 
			
		||||
    end
 | 
			
		||||
 | 
			
		||||
    describe "WidgetPolicy::Scope" do
 | 
			
		||||
      describe "#resolve" do
 | 
			
		||||
        it "raises a descriptive error" do
 | 
			
		||||
          scope = WidgetPolicy::Scope.new(double("User"), double("User.all"))
 | 
			
		||||
          expect { scope.resolve }.to raise_error(NotImplementedError, /WidgetPolicy::Scope/)
 | 
			
		||||
        end
 | 
			
		||||
      end
 | 
			
		||||
    end
 | 
			
		||||
  end
 | 
			
		||||
end
 | 
			
		||||
		Loading…
	
	Add table
		Add a link
		
	
		Reference in a new issue