From 3da04a3640f55f72b4294559706abdb9d4c6582d Mon Sep 17 00:00:00 2001 From: Wyatt Kirby Date: Fri, 29 May 2026 14:20:07 -0400 Subject: [PATCH 01/12] chore: bump Ruby to >= 3.3, update CI matrix and actions --- .github/workflows/test.yml | 8 ++++---- papers_please.gemspec | 2 ++ 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 9b4c486..ddeb968 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -2,7 +2,7 @@ name: Test & Lint on: push: - branches: [main] + branches: [master] pull_request: jobs: @@ -12,12 +12,12 @@ jobs: strategy: matrix: - ruby-version: ['3.1', '3.0', '2.7'] + ruby-version: ['4.0', '3.4', '3.3'] steps: - - uses: actions/checkout@v3 + - uses: actions/checkout@v6 - name: Set up Ruby - uses: ruby/setup-ruby@359bebbc29cbe6c87da6bc9ea3bc930432750108 + uses: ruby/setup-ruby@v1 with: ruby-version: ${{ matrix.ruby-version }} - name: Install dependencies diff --git a/papers_please.gemspec b/papers_please.gemspec index 8657664..715b09e 100644 --- a/papers_please.gemspec +++ b/papers_please.gemspec @@ -22,6 +22,8 @@ Gem::Specification.new do |spec| spec.executables = spec.files.grep(%r{^exe/}) { |f| File.basename(f) } spec.require_paths = ['lib'] + spec.required_ruby_version = '>= 3.3.0' + spec.add_dependency 'terminal-table' spec.add_development_dependency 'bundler', '~> 2.0' From 27d6b19a1eb23ac9a79eb8b8cf5a5782a38660ae Mon Sep 17 00:00:00 2001 From: Wyatt Kirby Date: Fri, 29 May 2026 14:21:10 -0400 Subject: [PATCH 02/12] chore: replace rubocop with standardrb --- .rubocop.yml | 137 --------------------- .standard.yml | 9 ++ Gemfile | 2 +- Gemfile.lock | 55 ++++++--- Rakefile | 4 +- lib/papers_please.rb | 36 +++--- lib/papers_please/policy.rb | 20 +-- lib/papers_please/railtie.rb | 4 +- lib/papers_please/role.rb | 8 +- lib/papers_please/tasks/papers_please.rake | 2 +- lib/papers_please/version.rb | 2 +- papers_please.gemspec | 43 ++++--- spec/fixtures/access_policy.rb | 14 +-- spec/fixtures/user.rb | 6 +- spec/papers_please_spec.rb | 58 ++++----- spec/permission_spec.rb | 44 +++---- spec/policy_spec.rb | 72 +++++------ spec/role_spec.rb | 78 ++++++------ spec/spec_helper.rb | 14 +-- 19 files changed, 248 insertions(+), 360 deletions(-) delete mode 100644 .rubocop.yml create mode 100644 .standard.yml diff --git a/.rubocop.yml b/.rubocop.yml deleted file mode 100644 index a9e250e..0000000 --- a/.rubocop.yml +++ /dev/null @@ -1,137 +0,0 @@ -require: rubocop-rspec - -AllCops: - NewCops: enable - SuggestExtensions: false - TargetRubyVersion: 3.1 - Include: - - 'lib/**/*.rb' - - 'spec/**/*.rb' - - '**/Gemfile' - - '**/Rakefile' - Exclude: - - 'bin/**/*' - - 'spec/fixtures/**/*.rb' - -Style/HashSyntax: - EnforcedShorthandSyntax: never - -Style/Documentation: - Enabled: false - -Naming/BlockForwarding: - Enabled: false - -Style/RedundantSelf: - Enabled: false - -Style/RedundantReturn: - Enabled: false - -Style/GuardClause: - Enabled: false - -Style/ClassAndModuleChildren: - Enabled: false - -Layout/EmptyLinesAroundClassBody: - Enabled: false - -Style/FrozenStringLiteralComment: - Enabled: false - -Layout/CommentIndentation: - Enabled: false - -Layout/LineLength: - Max: 120 - -Metrics/ClassLength: - Max: 120 - -Metrics/CyclomaticComplexity: - Max: 10 - -Metrics/MethodLength: - Max: 15 - -Metrics/AbcSize: - Max: 25 - -Metrics/ParameterLists: - Max: 8 - -Layout/EmptyLineBetweenDefs: - AllowAdjacentOneLineDefs: true - -Naming/MethodParameterName: - AllowedNames: - - _ - -RSpec/ExampleLength: - Enabled: false - -RSpec/MultipleExpectations: - Enabled: false - -RSpec/MultipleMemoizedHelpers: - Enabled: false - -RSpec/NestedGroups: - Enabled: false - -RSpec/MessageSpies: - Enabled: false - -RSpec/InstanceVariable: - Enabled: false - -RSpec/BeforeAfterAll: - Enabled: false - -RSpec/AnyInstance: - Enabled: false - -RSpec/ContextWording: - Enabled: false - -RSpec/FilePath: - Enabled: false - -RSpec/NamedSubject: - Enabled: false - -RSpec/StubbedMock: - Enabled: false - -RSpec/LetSetup: - Enabled: false - -RSpec/MessageChain: - Enabled: false - -RSpec/RepeatedDescription: - Enabled: false - -RSpec/RepeatedExample: - Enabled: false - -RSpec/ScatteredSetup: - Enabled: false - -RSpec/UnspecifiedException: - Enabled: false - -RSpec/VerifiedDoubles: - Enabled: false - -RSpec/ExpectInHook: - Enabled: false - -Style/ClassVars: - Exclude: - - 'lib/slayer/service.rb' - -Style/MutableConstant: - Exclude: - - 'lib/slayer/version.rb' diff --git a/.standard.yml b/.standard.yml new file mode 100644 index 0000000..f37f0c9 --- /dev/null +++ b/.standard.yml @@ -0,0 +1,9 @@ +ruby_version: 3.3 + +ignore: + - "bin/**/*" + - "vendor/**/*" + - "tmp/**/*" + - "pkg/**/*" + +fix: true diff --git a/Gemfile b/Gemfile index 97cbf52..1cf6315 100644 --- a/Gemfile +++ b/Gemfile @@ -1,6 +1,6 @@ # frozen_string_literal: true -source 'https://rubygems.org' +source "https://rubygems.org" git_source(:github) { |repo_name| "https://github.com/#{repo_name}" } diff --git a/Gemfile.lock b/Gemfile.lock index 3b0f593..83a6f03 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -7,20 +7,22 @@ PATH GEM remote: https://rubygems.org/ specs: - ast (2.4.2) + ast (2.4.3) byebug (11.1.3) diff-lcs (1.5.0) docile (1.4.0) - json (2.6.3) - parallel (1.23.0) - parser (3.2.2.3) + json (2.19.7) + language_server-protocol (3.17.0.5) + lint_roller (1.1.0) + parallel (1.28.0) + parser (3.3.11.1) ast (~> 2.4.1) racc - racc (1.7.1) + prism (1.9.0) + racc (1.8.1) rainbow (3.1.1) rake (13.0.6) - regexp_parser (2.8.1) - rexml (3.2.5) + regexp_parser (2.12.0) rspec (3.12.0) rspec-core (~> 3.12.0) rspec-expectations (~> 3.12.0) @@ -34,20 +36,24 @@ GEM diff-lcs (>= 1.2.0, < 2.0) rspec-support (~> 3.12.0) rspec-support (3.12.1) - rubocop (1.38.0) + rubocop (1.84.2) json (~> 2.3) + language_server-protocol (~> 3.17.0.2) + lint_roller (~> 1.1.0) parallel (~> 1.10) - parser (>= 3.1.2.1) + parser (>= 3.3.0.2) rainbow (>= 2.2.2, < 4.0) - regexp_parser (>= 1.8, < 3.0) - rexml (>= 3.2.5, < 4.0) - rubocop-ast (>= 1.23.0, < 2.0) + regexp_parser (>= 2.9.3, < 3.0) + rubocop-ast (>= 1.49.0, < 2.0) ruby-progressbar (~> 1.7) - unicode-display_width (>= 1.4.0, < 3.0) - rubocop-ast (1.29.0) - parser (>= 3.2.1.0) - rubocop-rspec (2.17.1) - rubocop (~> 1.33) + unicode-display_width (>= 2.4.0, < 4.0) + rubocop-ast (1.49.1) + parser (>= 3.3.7.2) + prism (~> 1.7) + rubocop-performance (1.26.1) + lint_roller (~> 1.1) + rubocop (>= 1.75.0, < 2.0) + rubocop-ast (>= 1.47.1, < 2.0) ruby-progressbar (1.13.0) simplecov (0.22.0) docile (~> 1.1) @@ -55,6 +61,18 @@ GEM simplecov_json_formatter (~> 0.1) simplecov-html (0.12.3) simplecov_json_formatter (0.1.4) + standard (1.54.0) + language_server-protocol (~> 3.17.0.2) + lint_roller (~> 1.0) + rubocop (~> 1.84.0) + standard-custom (~> 1.0.0) + standard-performance (~> 1.8) + standard-custom (1.0.2) + lint_roller (~> 1.0) + rubocop (~> 1.50) + standard-performance (1.9.0) + lint_roller (~> 1.1) + rubocop-performance (~> 1.26.0) terminal-table (3.0.2) unicode-display_width (>= 1.1.1, < 3) unicode-display_width (2.4.2) @@ -68,9 +86,8 @@ DEPENDENCIES papers_please! rake (~> 13.0) rspec (~> 3.12) - rubocop (= 1.38.0) - rubocop-rspec simplecov + standard (~> 1.39) BUNDLED WITH 2.3.9 diff --git a/Rakefile b/Rakefile index 82bb534..b6ae734 100644 --- a/Rakefile +++ b/Rakefile @@ -1,7 +1,7 @@ # frozen_string_literal: true -require 'bundler/gem_tasks' -require 'rspec/core/rake_task' +require "bundler/gem_tasks" +require "rspec/core/rake_task" RSpec::Core::RakeTask.new(:spec) diff --git a/lib/papers_please.rb b/lib/papers_please.rb index 8908e04..c0d6ee3 100644 --- a/lib/papers_please.rb +++ b/lib/papers_please.rb @@ -1,28 +1,28 @@ # frozen_string_literal: true -require 'papers_please/version' -require 'papers_please/errors' -require 'papers_please/policy' -require 'papers_please/role' -require 'papers_please/permission' -require 'papers_please/rails/controller_methods' -require 'papers_please/railtie' if defined? Rails +require "papers_please/version" +require "papers_please/errors" +require "papers_please/policy" +require "papers_please/role" +require "papers_please/permission" +require "papers_please/rails/controller_methods" +require "papers_please/railtie" if defined? Rails module PapersPlease # rubocop:disable Metrics/PerceivedComplexity, Metrics/MethodLength def self.permissions_table(policy_klass) - require 'terminal-table' + require "terminal-table" policy = policy_klass.new(:system) table = ::Terminal::Table.new do |t| t.headings = [ - 'role', - 'subject', - 'permission', - 'has query?', - 'has predicate?', - 'granted by other?' + "role", + "subject", + "permission", + "has query?", + "has predicate?", + "granted by other?" ] policy.roles.each_with_index do |(name, role), index| @@ -35,9 +35,9 @@ def self.permissions_table(policy_klass) first_line_of_role ? name : nil, subject, permission.key, - permission.query ? 'yes' : 'no', - permission.predicate ? 'yes' : 'no', - permission.granted_by_other? ? 'yes' : 'no' + permission.query ? "yes" : "no", + permission.predicate ? "yes" : "no", + permission.granted_by_other? ? "yes" : "no" ] first_line_of_role = false @@ -46,7 +46,7 @@ def self.permissions_table(policy_klass) end end - puts table.to_s + puts table table.to_s end diff --git a/lib/papers_please/policy.rb b/lib/papers_please/policy.rb index 54d01eb..0bef69a 100644 --- a/lib/papers_please/policy.rb +++ b/lib/papers_please/policy.rb @@ -7,10 +7,10 @@ class Policy attr_reader :fallthrough, :user def initialize(user) - @user = user + @user = user @default_scope = nil - @roles = {} - @cache = {} + @roles = {} + @cache = {} configure end @@ -19,12 +19,12 @@ def allow_fallthrough @fallthrough = true end - def default_scope(scope) + def default_scope(scope) # standard:disable Style/TrivialAccessors @default_scope = scope end def configure - raise NotImplementedError, 'The #configure method of the access policy was not implemented' + raise NotImplementedError, "The #configure method of the access policy was not implemented" end # Add a role to the Policy @@ -37,7 +37,7 @@ def add_role(name, predicate = nil) role end - alias role add_role + alias_method :role, :add_role # Add permissions to the Role def add_permissions(keys) @@ -49,7 +49,7 @@ def add_permissions(keys) yield roles[key] end end - alias permit add_permissions + alias_method :permit, :add_permissions # Look up a stored permission block and call with # the current user and subject @@ -101,14 +101,14 @@ def scope_for(action, klass, roles: nil) permission = role.find_permission(action, klass) scope = permission&.fetch(user, klass, action) - next if permission.nil? || (scope.nil? && fallthrough) + next if permission.nil? || (scope.nil? && fallthrough) # standard:disable Style/SafeNavigation return scope end @default_scope || nil end - alias query scope_for + alias_method :query, :scope_for # Fetch roles that apply to the current user def applicable_roles @@ -127,7 +127,7 @@ def permission_granted?(permission, action, subject) if fallthrough permission.nil? ? false : permission.granted?(user, subject, action) else - permission.nil? ? nil : permission.granted?(user, subject, action) + permission&.granted?(user, subject, action) end end diff --git a/lib/papers_please/railtie.rb b/lib/papers_please/railtie.rb index c77f46d..d6e6dad 100644 --- a/lib/papers_please/railtie.rb +++ b/lib/papers_please/railtie.rb @@ -1,11 +1,11 @@ # frozen_string_literal: true -require 'rails/railtie' +require "rails/railtie" module PapersPlease class Railtie < ::Rails::Railtie rake_tasks do - Dir[File.join(File.dirname(__FILE__), 'tasks', '*.rake')].each { |f| load f } + Dir[File.join(File.dirname(__FILE__), "tasks", "*.rake")].each { |f| load f } end initializer :papers_plesae do diff --git a/lib/papers_please/role.rb b/lib/papers_please/role.rb index 657be04..7f88b81 100644 --- a/lib/papers_please/role.rb +++ b/lib/papers_please/role.rb @@ -20,7 +20,7 @@ def add_permission(actions, klass, query: nil, predicate: nil, granted_by: nil) if !granted_by.nil? && !valid_grant?(granted_by) raise InvalidGrant, - 'granted_by must be an array of [Class, Proc]' + "granted_by must be an array of [Class, Proc]" end permissions << make_permission( @@ -33,7 +33,7 @@ def add_permission(actions, klass, query: nil, predicate: nil, granted_by: nil) ) end end - alias grant add_permission + alias_method :grant, :add_permission def find_permission(action, subject) permissions.detect do |permission| @@ -59,7 +59,7 @@ def valid_grant?(tuple) # Wrap actions, translating :manage into :crud def prepare_actions(action) Array(action).flat_map do |a| - a == :manage ? %i[create read update destroy] : [a] + (a == :manage) ? %i[create read update destroy] : [a] end end @@ -105,7 +105,7 @@ def make_base_permission(action, klass, granted_by: nil) def build_predicate_from_query(action, actions, klass, query) # If the action is :create, expanded from :manage # then we set the default all predicate - return (proc { true }) if action == :create && actions == :manage + return proc { true } if action == :create && actions == :manage # Otherwise the default predicate is to check # for inclusion in the returned relationship diff --git a/lib/papers_please/tasks/papers_please.rake b/lib/papers_please/tasks/papers_please.rake index a425e28..cdb6895 100644 --- a/lib/papers_please/tasks/papers_please.rake +++ b/lib/papers_please/tasks/papers_please.rake @@ -1,7 +1,7 @@ # frozen_string_literal: true namespace :papers_please do - desc 'Print out all defined roles and permissions in match order' + desc "Print out all defined roles and permissions in match order" task :roles, [:klass] => :environment do |_, _args| klass = klass ? Object.const_get(klass) : AccessPolicy diff --git a/lib/papers_please/version.rb b/lib/papers_please/version.rb index 6aaa2f8..b694e97 100644 --- a/lib/papers_please/version.rb +++ b/lib/papers_please/version.rb @@ -1,5 +1,5 @@ # frozen_string_literal: true module PapersPlease - VERSION = '0.1.7' + VERSION = "0.1.7" end diff --git a/papers_please.gemspec b/papers_please.gemspec index 715b09e..9631425 100644 --- a/papers_please.gemspec +++ b/papers_please.gemspec @@ -1,36 +1,35 @@ # frozen_string_literal: true -lib = File.expand_path('lib', __dir__) +lib = File.expand_path("lib", __dir__) $LOAD_PATH.unshift(lib) unless $LOAD_PATH.include?(lib) -require 'papers_please/version' +require "papers_please/version" Gem::Specification.new do |spec| - spec.name = 'papers_please' - spec.version = PapersPlease::VERSION - spec.authors = ['Apsis Labs'] - spec.email = ['wyatt@apsis.io'] + spec.name = "papers_please" + spec.version = PapersPlease::VERSION + spec.authors = ["Apsis Labs"] + spec.email = ["wyatt@apsis.io"] - spec.summary = 'A roles & permissions gem for ruby applications.' - spec.homepage = 'http://apsis.io' - spec.license = 'MIT' + spec.summary = "A roles & permissions gem for ruby applications." + spec.homepage = "http://apsis.io" + spec.license = "MIT" - spec.files = `git ls-files -z`.split("\x0").reject do |f| + spec.files = `git ls-files -z`.split("\x0").reject do |f| f.match(%r{^(test|spec|features)/}) end - spec.bindir = 'exe' - spec.executables = spec.files.grep(%r{^exe/}) { |f| File.basename(f) } - spec.require_paths = ['lib'] + spec.bindir = "exe" + spec.executables = spec.files.grep(%r{^exe/}) { |f| File.basename(f) } + spec.require_paths = ["lib"] - spec.required_ruby_version = '>= 3.3.0' + spec.required_ruby_version = ">= 3.3.0" - spec.add_dependency 'terminal-table' + spec.add_dependency "terminal-table" - spec.add_development_dependency 'bundler', '~> 2.0' - spec.add_development_dependency 'byebug' - spec.add_development_dependency 'rake', '~> 13.0' - spec.add_development_dependency 'rspec', '~> 3.12' - spec.add_development_dependency 'rubocop', '= 1.38.0' - spec.add_development_dependency 'rubocop-rspec' - spec.add_development_dependency 'simplecov' + spec.add_development_dependency "bundler", "~> 2.0" + spec.add_development_dependency "byebug" + spec.add_development_dependency "rake", "~> 13.0" + spec.add_development_dependency "rspec", "~> 3.12" + spec.add_development_dependency "standard", "~> 1.39" + spec.add_development_dependency "simplecov" end diff --git a/spec/fixtures/access_policy.rb b/spec/fixtures/access_policy.rb index 3d03d67..034e358 100644 --- a/spec/fixtures/access_policy.rb +++ b/spec/fixtures/access_policy.rb @@ -2,9 +2,9 @@ class AccessPolicy < PapersPlease::Policy def configure - role :admin, (proc { |user| user.admin? }) - role :manager, (proc { |user| user.manager? }) - role :member, (proc { |user| user.member? }) + role :admin, proc { |user| user.admin? } + role :manager, proc { |user| user.manager? } + role :member, proc { |user| user.member? } permit :admin do |role| role.grant :manage, Post @@ -12,14 +12,14 @@ def configure end permit :manager do |role| - role.grant :manage, Post, query: (proc { |u| u.posts }) - role.grant :manage, Attachment, granted_by: [Post, (proc { |_u, a| a.post })] + role.grant :manage, Post, query: proc { |u| u.posts } + role.grant :manage, Attachment, granted_by: [Post, proc { |_u, a| a.post }] end permit :member do |role| role.grant :create, Post - role.grant %i[read update], Post, query: (proc { |u| u.posts }) - role.grant %i[create read update], Attachment, granted_by: [Post, (proc { |_u, a| a.post })] + role.grant %i[read update], Post, query: proc { |u| u.posts } + role.grant %i[create read update], Attachment, granted_by: [Post, proc { |_u, a| a.post }] end end end diff --git a/spec/fixtures/user.rb b/spec/fixtures/user.rb index 1b2dae9..4a9ab26 100644 --- a/spec/fixtures/user.rb +++ b/spec/fixtures/user.rb @@ -10,7 +10,7 @@ def initialize(posts:, admin: false, manager: false, member: false) @member = member end - alias admin? admin - alias manager? manager - alias member? member + alias_method :admin?, :admin + alias_method :manager?, :manager + alias_method :member?, :member end diff --git a/spec/papers_please_spec.rb b/spec/papers_please_spec.rb index fc3d5f6..d37f489 100644 --- a/spec/papers_please_spec.rb +++ b/spec/papers_please_spec.rb @@ -1,18 +1,18 @@ # frozen_string_literal: true RSpec.describe PapersPlease do - it 'has a version number' do + it "has a version number" do expect(PapersPlease::VERSION).not_to be_nil end - context 'permissions table' do - it 'returns a string and does not error' do + context "permissions table" do + it "returns a string and does not error" do res = described_class.permissions_table(AccessPolicy) expect(res).to be_a String end end - context 'access policy' do + context "access policy" do let(:posts) { Array.new(5) { Post.new } } let(:post) { posts.first } let(:attachment) { Attachment.new(post: post) } @@ -24,11 +24,11 @@ let(:manager) { User.new(posts: posts.slice(0, 3), manager: true) } let(:admin) { User.new(posts: posts, admin: true) } - context 'admin' do + context "admin" do before { @policy = AccessPolicy.new(admin) } - describe '#can?' do - it 'grants permissions' do + describe "#can?" do + it "grants permissions" do # Posts expect(@policy.can?(:create, Post)).to be true expect(@policy.can?(:read, post)).to be true @@ -41,31 +41,31 @@ end end - describe '#scope_for' do - it 'creates scope correctly' do + describe "#scope_for" do + it "creates scope correctly" do expect(@policy.scope_for(:read, Post)).to contain_exactly(*Post.all) end end - describe '#authorize!' do - it 'raises exception if not allowed' do + describe "#authorize!" do + it "raises exception if not allowed" do expect { @policy.authorize! :not_real, Post }.to(raise_exception { PapersPlease::AccessDenied }) expect { @policy.authorize! :not_real, post }.to(raise_exception { PapersPlease::AccessDenied }) end - it 'does nothing if allowed' do + it "does nothing if allowed" do expect { @policy.authorize! :create, Post }.not_to raise_exception expect { @policy.authorize! :read, post }.not_to raise_exception end end end - context 'manager' do + context "manager" do before { @policy = AccessPolicy.new(manager) } - describe '#can?' do - describe '#can?' do - it 'grants permissions' do + describe "#can?" do + describe "#can?" do + it "grants permissions" do # Post expect(@policy.can?(:create, Post)).to be true expect(@policy.can?(:read, post)).to be true @@ -94,32 +94,32 @@ end end - describe '#scope_for' do - it 'creates scope correctly' do + describe "#scope_for" do + it "creates scope correctly" do expect(@policy.scope_for(:read, Post)).to contain_exactly(*manager.posts) expect(@policy.scope_for(:update, Post)).to contain_exactly(*manager.posts) expect(@policy.scope_for(:destroy, Post)).to contain_exactly(*manager.posts) end end - describe '#authorize!' do - it 'raises exception if not allowed' do + describe "#authorize!" do + it "raises exception if not allowed" do expect { @policy.authorize! :read, restricted_post }.to(raise_exception { PapersPlease::AccessDenied }) expect { @policy.authorize! :destroy, restricted_post }.to(raise_exception { PapersPlease::AccessDenied }) end - it 'does nothing if allowed' do + it "does nothing if allowed" do expect { @policy.authorize! :create, Post }.not_to raise_exception expect { @policy.authorize! :read, post }.not_to raise_exception end end end - context 'member' do + context "member" do before { @policy = AccessPolicy.new(member) } - describe '#can?' do - it 'grants permissions correctly' do + describe "#can?" do + it "grants permissions correctly" do # Posts expect(@policy.can?(:create, Post)).to be true expect(@policy.can?(:read, post)).to be true @@ -145,19 +145,19 @@ end end - describe '#scope_for' do - it 'creates scope correctly' do + describe "#scope_for" do + it "creates scope correctly" do expect(@policy.scope_for(:read, Post)).to contain_exactly(*member.posts) end end - describe '#authorize!' do - it 'raises exception if not allowed' do + describe "#authorize!" do + it "raises exception if not allowed" do expect { @policy.authorize! :destroy, Post }.to(raise_exception { PapersPlease::AccessDenied }) expect { @policy.authorize! :destroy, post }.to(raise_exception { PapersPlease::AccessDenied }) end - it 'does nothing if allowed' do + it "does nothing if allowed" do expect { @policy.authorize! :create, Post }.not_to raise_exception expect { @policy.authorize! :read, post }.not_to raise_exception end diff --git a/spec/permission_spec.rb b/spec/permission_spec.rb index 2b66155..8078c47 100644 --- a/spec/permission_spec.rb +++ b/spec/permission_spec.rb @@ -1,26 +1,26 @@ # frozen_string_literal: true -require 'spec_helper' +require "spec_helper" RSpec.describe PapersPlease::Permission do subject { described_class } - let(:book) { double('Book') } + let(:book) { double("Book") } - it 'requires a key and subject' do + it "requires a key and subject" do expect { subject.new }.to raise_error(ArgumentError) expect { subject.new(:key) }.to raise_error(ArgumentError) expect { subject.new(:key, book) }.not_to raise_error end - it 'validates arguments' do - expect { subject.new(:key, book, query: 'invalid') }.to raise_error(ArgumentError) - expect { subject.new(:key, book, predicate: 'invalid') }.to raise_error(ArgumentError) - expect { subject.new(:key, book, granted_by: 'invalid') }.to raise_error(ArgumentError) - expect { subject.new(:key, book, granting_class: 'invalid') }.to raise_error(ArgumentError) + it "validates arguments" do + expect { subject.new(:key, book, query: "invalid") }.to raise_error(ArgumentError) + expect { subject.new(:key, book, predicate: "invalid") }.to raise_error(ArgumentError) + expect { subject.new(:key, book, granted_by: "invalid") }.to raise_error(ArgumentError) + expect { subject.new(:key, book, granting_class: "invalid") }.to raise_error(ArgumentError) end - it 'allows query and predicate' do + it "allows query and predicate" do stub_proc = proc { true } perm = subject.new(:key, book, predicate: stub_proc, query: stub_proc) @@ -28,63 +28,63 @@ expect(perm.query).to be stub_proc end - describe '#matches?' do + describe "#matches?" do let(:permission) { subject.new(:read, book) } - it 'matches' do + it "matches" do expect(permission.matches?(:read, book)).to be true end - it 'does not match' do + it "does not match" do expect(permission.matches?(:write, book)).to be false end end - describe '#granted?' do + describe "#granted?" do let(:true_permission) { subject.new(:read, book, predicate: proc { true }) } let(:false_permission) { subject.new(:read, book, predicate: proc { false }) } let(:stub_proc) { proc { true } } let(:stub_perm) { subject.new(:read, book, predicate: stub_proc) } - it 'is true for true permission' do + it "is true for true permission" do expect(true_permission.granted?).to be true end - it 'is false for false permission' do + it "is false for false permission" do expect(false_permission.granted?).to be false end - it 'calls the predicate proc' do + it "calls the predicate proc" do expect(stub_proc).to receive(:call).and_return(true) expect(stub_perm.granted?).to be true end - it 'calls the predicate proc' do + it "calls the predicate proc" do expect(stub_proc).to receive(:call).with(:arg).and_return(true) expect(stub_perm.granted?(:arg)).to be true end end - describe 'fetch' do + describe "fetch" do let(:array_permission) { subject.new(:read, book, query: proc { [] }) } let(:empty_permission) { subject.new(:read, book, query: proc {}) } let(:stub_proc) { proc { [] } } let(:stub_perm) { subject.new(:read, book, query: stub_proc) } - it 'is array for array permission' do + it "is array for array permission" do expect(array_permission.fetch).to eq [] end - it 'is nil for nil permission' do + it "is nil for nil permission" do expect(empty_permission.fetch).to be_nil end - it 'calls the predicate proc' do + it "calls the predicate proc" do expect(stub_proc).to receive(:call).and_return([]) expect(stub_perm.fetch).to eq [] end - it 'calls the predicate proc' do + it "calls the predicate proc" do expect(stub_proc).to receive(:call).with(:arg).and_return(nil) expect(stub_perm.fetch(:arg)).to be_nil end diff --git a/spec/policy_spec.rb b/spec/policy_spec.rb index 4ac94fa..38abce8 100644 --- a/spec/policy_spec.rb +++ b/spec/policy_spec.rb @@ -8,32 +8,32 @@ let(:member) { User.new(posts: posts.slice(0, 3), admin: false, member: true) } let(:admin) { User.new(posts: posts, admin: true, member: false) } - describe '#configure' do + describe "#configure" do let(:post) { Post.new } let(:owner_member) { User.new(posts: [post], member: true) } let(:other_member) { User.new(posts: [], member: true) } - it 'raises not implemented if not overriden' do + it "raises not implemented if not overriden" do klass = Class.new(described_class) expect { klass.new(member) }.to raise_error(NotImplementedError) end - context 'predicate check' do + context "predicate check" do before do allow(owner_member).to receive(:spy_method) allow(post).to receive(:spy_method) end - it 'passes user and object to permission check' do + it "passes user and object to permission check" do klass = Class.new(PapersPlease::Policy) do def configure role :member permit :member do |role| - role.grant :read, Post, predicate: (proc { |u, p| - u.spy_method - p.spy_method - }) + role.grant :read, Post, predicate: proc { |u, p| + u.spy_method + p.spy_method + } end end end @@ -44,22 +44,22 @@ def configure end end - context 'query' do + context "query" do before do allow(owner_member).to receive(:spy_method) allow(Post).to receive(:spy_method) end - it 'passes user and class to scope' do + it "passes user and class to scope" do klass = Class.new(PapersPlease::Policy) do def configure role :member permit :member do |role| - role.grant :read, Post, query: (proc { |u, p| - u.spy_method - p.spy_method - }) + role.grant :read, Post, query: proc { |u, p| + u.spy_method + p.spy_method + } end end end @@ -69,14 +69,14 @@ def configure expect(Post).to have_received(:spy_method) end - it 'allows you to define a default scope when no role matches the query' do + it "allows you to define a default scope when no role matches the query" do klass = Class.new(PapersPlease::Policy) do def configure - role :admin, (proc { |u| u.admin }) + role :admin, proc { |u| u.admin } role :member permit :admin do |role| - role.grant :read, Post, query: (proc { Post.all }) + role.grant :read, Post, query: proc { Post.all } end default_scope :default_scope @@ -88,19 +88,19 @@ def configure end end - describe '#can' do - it 'returns true for passing predicate' do + describe "#can" do + it "returns true for passing predicate" do policy = sample_policy(member) expect(policy.can?(:read, post)).to be true end - it 'return false for failing predicate' do + it "return false for failing predicate" do policy = sample_policy(member) expect(policy.can?(:read, posts.last)).to be false end - context 'with fallthrough' do - it 'returns true when any applicable role has a passing predicate' do + context "with fallthrough" do + it "returns true when any applicable role has a passing predicate" do # Only members can read this new post because it is published, but # does not belong to the admin new_post = Post.new @@ -118,7 +118,7 @@ def configure expect(member_policy.can?(:read, posts.last)).to be false end - it 'returns false if no roles have a passing predicate' do + it "returns false if no roles have a passing predicate" do # No one can read this post because it does not belong to the admin # and it is not published new_post = Post.new @@ -132,8 +132,8 @@ def configure end end - context 'with specific role check' do - it 'allows you to specify a role to check against' do + context "with specific role check" do + it "allows you to specify a role to check against" do # Only members can read this new post because it is published, but # does not belong to the admin new_post = Post.new @@ -149,8 +149,8 @@ def configure end end - describe '#role_that_can' do - it 'returns the symbolic name of the roles that can perform an action' do + describe "#role_that_can" do + it "returns the symbolic name of the roles that can perform an action" do policy_klass = Class.new(PapersPlease::Policy) do def configure role :member @@ -175,8 +175,8 @@ def configure end end - describe '#authorize!' do - it 'raises an AccessDenied error for failing predicate' do + describe "#authorize!" do + it "raises an AccessDenied error for failing predicate" do policy = sample_policy(member) expect { policy.authorize! :read, posts.last }.to raise_error(PapersPlease::AccessDenied) end @@ -187,15 +187,15 @@ def configure def sample_policy_klass Class.new(PapersPlease::Policy) do def configure - role :admin, (proc { |u| u.admin }) - role :member, (proc { |u| u.member }) + role :admin, proc { |u| u.admin } + role :member, proc { |u| u.member } permit :admin do |role| role.grant :read, Post end permit :member do |role| - role.grant :read, Post, predicate: (proc { |u, p| u.posts.include? p }) + role.grant :read, Post, predicate: proc { |u, p| u.posts.include? p } end end end @@ -206,15 +206,15 @@ def sample_fallthrough_policy_klass def configure allow_fallthrough - role :member, (proc { |u| u.admin || u.member }) - role :admin, (proc { |u| u.admin }) + role :member, proc { |u| u.admin || u.member } + role :admin, proc { |u| u.admin } permit :admin do |role| - role.grant :read, Post, predicate: (proc { |u, p| u.posts.include?(p) }) + role.grant :read, Post, predicate: proc { |u, p| u.posts.include?(p) } end permit :member do |role| - role.grant :read, Post, predicate: (proc { |_u, p| p.published }) + role.grant :read, Post, predicate: proc { |_u, p| p.published } end end end diff --git a/spec/role_spec.rb b/spec/role_spec.rb index 5c46a45..7e53849 100644 --- a/spec/role_spec.rb +++ b/spec/role_spec.rb @@ -1,39 +1,39 @@ # frozen_string_literal: true -require 'spec_helper' +require "spec_helper" RSpec.describe PapersPlease::Role do let(:admin_user) { User.new(posts: [], admin: true) } let(:manager_user) { User.new(posts: [], manager: true) } let(:member_user) { User.new(posts: [], member: true) } - describe '#applies_to?' do - context 'with no predicate' do + describe "#applies_to?" do + context "with no predicate" do let(:no_predicate_role) { described_class.new(:admin) } - it 'applies' do + it "applies" do expect(no_predicate_role.applies_to?(member_user)).to be true end end - context 'with predicate' do - let(:admin_role) { described_class.new(:admin, predicate: (proc { |user| user.admin? })) } + context "with predicate" do + let(:admin_role) { described_class.new(:admin, predicate: proc { |user| user.admin? }) } - it 'applies to admin' do + it "applies to admin" do expect(admin_role.applies_to?(admin_user)).to be true end - it 'does not apply to member' do + it "does not apply to member" do expect(admin_role.applies_to?(member_user)).to be false end end end - describe '#add_permission' do - context 'manage expansion' do + describe "#add_permission" do + context "manage expansion" do let(:role) { described_class.new(:admin) } - it 'adds correct permissions' do + it "adds correct permissions" do role.add_permission(:manage, Post) expect(role.permissions.size).to eq 4 @@ -42,28 +42,28 @@ end end - context 'duplicate permissions' do + context "duplicate permissions" do let(:role) { described_class.new(:admin) } - it 'raises error for duplicates' do + it "raises error for duplicates" do role.add_permission(:read, Post) expect { role.add_permission(:read, Post) }.to raise_error(PapersPlease::DuplicatePermission) end - it 'raises error for duplicates in expansions' do + it "raises error for duplicates in expansions" do role.add_permission(:read, Post) expect { role.add_permission(:manage, Post) }.to raise_error(PapersPlease::DuplicatePermission) end end - context 'granted_by' do - let(:granted_by) { (proc { |_u, a| a.post }) } + context "granted_by" do + let(:granted_by) { proc { |_u, a| a.post } } let(:granting_class) { Post } let(:role) { described_class.new(:admin) } - it 'raises exceptions for invalid grants' do + it "raises exceptions for invalid grants" do expect do - role.add_permission(:read, Attachment, granted_by: 'invalid') + role.add_permission(:read, Attachment, granted_by: "invalid") end.to raise_error PapersPlease::InvalidGrant expect do role.add_permission(:read, Attachment, granted_by: granted_by) @@ -74,13 +74,13 @@ expect { role.add_permission(:read, Attachment, granted_by: []) }.to raise_error PapersPlease::InvalidGrant expect do role.add_permission(:read, Attachment, - granted_by: [granted_by, granting_class]) + granted_by: [granted_by, granting_class]) end.to raise_error PapersPlease::InvalidGrant expect { role.add_permission(:read, Attachment, granted_by: [granting_class, granted_by]) }.not_to raise_error end - it 'adds a granted permission' do + it "adds a granted permission" do role.add_permission(:read, Attachment, granted_by: [granting_class, granted_by]) expect(role.permissions.size).to eq 1 permission = role.permissions.first @@ -90,51 +90,51 @@ end end - context 'query and predicate' do - let(:predicate) { (proc { |user| user.admin? }) } - let(:query) { (proc { [] }) } + context "query and predicate" do + let(:predicate) { proc { |user| user.admin? } } + let(:query) { proc { [] } } let(:role) { described_class.new(:admin) } - it 'has correct query' do + it "has correct query" do role.add_permission(:read, Post, query: query, predicate: predicate) expect(role.permissions.size).to eq 1 expect(role.permissions.first.query).to be query end - it 'has correct predicate' do + it "has correct predicate" do role.add_permission(:read, Post, query: query, predicate: predicate) expect(role.permissions.size).to eq 1 expect(role.permissions.first.predicate).to be predicate end end - context 'only query' do + context "only query" do let(:included_post) { Post.new } let(:excluded_post) { Post.new } - let(:query) { (proc { [included_post] }) } + let(:query) { proc { [included_post] } } let(:admin_role) { described_class.new(:admin) } let(:manager_role) { described_class.new(:manager) } - it 'has correct query' do + it "has correct query" do admin_role.add_permission(:read, Post, query: query) expect(admin_role.permissions.size).to eq 1 expect(admin_role.permissions.first.query).to be query expect(admin_role.permissions.first.query.call).to contain_exactly(included_post) end - it 'generates predicate that allows posts in scope' do + it "generates predicate that allows posts in scope" do admin_role.add_permission(:read, Post, query: query) expect(admin_role.permissions.size).to eq 1 expect(admin_role.permissions.first.predicate.call(admin_user, included_post)).to be true end - it 'generates predicate that disallows posts not in scope' do + it "generates predicate that disallows posts not in scope" do admin_role.add_permission(:read, Post, query: query) expect(admin_role.permissions.size).to eq 1 expect(admin_role.permissions.first.predicate.call(admin_user, excluded_post)).to be false end - it 'generates predicate that disallows global access' do + it "generates predicate that disallows global access" do admin_role.add_permission(:read, Post, query: query) expect(admin_role.permissions.size).to eq 1 expect(admin_role.permissions.first.predicate.call(admin_user, Post)).to be false @@ -147,28 +147,28 @@ # end end - context 'only predicate' do - let(:predicate) { (proc { |user| user.admin? }) } + context "only predicate" do + let(:predicate) { proc { |user| user.admin? } } let(:predicate_role) { described_class.new(:admin) } - it 'has nil query' do + it "has nil query" do predicate_role.add_permission(:read, Post, predicate: predicate) expect(predicate_role.permissions.size).to eq 1 expect(predicate_role.permissions.first.query).to be_nil end - it 'has correct predicate' do + it "has correct predicate" do predicate_role.add_permission(:read, Post, predicate: predicate) expect(predicate_role.permissions.size).to eq 1 expect(predicate_role.permissions.first.predicate).to be predicate end end - context 'neither query nor predicate' do + context "neither query nor predicate" do let(:empty_role) { described_class.new(:admin) } - let(:klass) { spy('klass') } + let(:klass) { spy("klass") } - it 'creates an all query' do + it "creates an all query" do empty_role.add_permission(:read, klass) expect(empty_role.permissions.size).to eq 1 @@ -176,7 +176,7 @@ expect(klass).to have_received(:all) end - it 'creates a true predicate' do + it "creates a true predicate" do empty_role.add_permission(:read, klass) expect(empty_role.permissions.first.predicate.call).to be true end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 1d4b97e..e7cc968 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -1,21 +1,21 @@ # frozen_string_literal: true -require 'bundler/setup' -require 'byebug' -require 'simplecov' +require "bundler/setup" +require "byebug" +require "simplecov" SimpleCov.start do - add_filter '/spec' + add_filter "/spec" minimum_coverage(95) end -require 'papers_please' +require "papers_please" -Dir['spec/fixtures/**/*.rb'].each { |f| require File.expand_path(f) } +Dir["spec/fixtures/**/*.rb"].each { |f| require File.expand_path(f) } RSpec.configure do |config| # Enable flags like --only-failures and --next-failure - config.example_status_persistence_file_path = '.rspec_status' + config.example_status_persistence_file_path = ".rspec_status" # Disable RSpec exposing methods globally on `Module` and `main` config.disable_monkey_patching! From ed0164af4e8f61913298cf174a4a240b93342972 Mon Sep 17 00:00:00 2001 From: Wyatt Kirby Date: Fri, 29 May 2026 14:21:37 -0400 Subject: [PATCH 03/12] chore: update gemspec deps, require MFA for releases --- Gemfile.lock | 28 ++++++++++++++-------------- papers_please.gemspec | 6 ++++-- 2 files changed, 18 insertions(+), 16 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index 83a6f03..e81aaa6 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -23,19 +23,19 @@ GEM rainbow (3.1.1) rake (13.0.6) regexp_parser (2.12.0) - rspec (3.12.0) - rspec-core (~> 3.12.0) - rspec-expectations (~> 3.12.0) - rspec-mocks (~> 3.12.0) - rspec-core (3.12.2) - rspec-support (~> 3.12.0) - rspec-expectations (3.12.3) + rspec (3.13.2) + rspec-core (~> 3.13.0) + rspec-expectations (~> 3.13.0) + rspec-mocks (~> 3.13.0) + rspec-core (3.13.6) + rspec-support (~> 3.13.0) + rspec-expectations (3.13.5) diff-lcs (>= 1.2.0, < 2.0) - rspec-support (~> 3.12.0) - rspec-mocks (3.12.5) + rspec-support (~> 3.13.0) + rspec-mocks (3.13.8) diff-lcs (>= 1.2.0, < 2.0) - rspec-support (~> 3.12.0) - rspec-support (3.12.1) + rspec-support (~> 3.13.0) + rspec-support (3.13.7) rubocop (1.84.2) json (~> 2.3) language_server-protocol (~> 3.17.0.2) @@ -81,13 +81,13 @@ PLATFORMS ruby DEPENDENCIES - bundler (~> 2.0) + bundler (>= 2.4.0) byebug papers_please! rake (~> 13.0) - rspec (~> 3.12) + rspec (~> 3.13) simplecov standard (~> 1.39) BUNDLED WITH - 2.3.9 + 2.7.2 diff --git a/papers_please.gemspec b/papers_please.gemspec index 9631425..96a422d 100644 --- a/papers_please.gemspec +++ b/papers_please.gemspec @@ -26,10 +26,12 @@ Gem::Specification.new do |spec| spec.add_dependency "terminal-table" - spec.add_development_dependency "bundler", "~> 2.0" + spec.add_development_dependency "bundler", ">= 2.4.0" spec.add_development_dependency "byebug" spec.add_development_dependency "rake", "~> 13.0" - spec.add_development_dependency "rspec", "~> 3.12" + spec.add_development_dependency "rspec", "~> 3.13" spec.add_development_dependency "standard", "~> 1.39" spec.add_development_dependency "simplecov" + + spec.metadata["rubygems_mfa_required"] = "true" end From 4de814653d218b232cac06a6529d76505754a8d4 Mon Sep 17 00:00:00 2001 From: Wyatt Kirby Date: Fri, 29 May 2026 14:21:51 -0400 Subject: [PATCH 04/12] chore: restructure Rakefile, run standard + specs as default --- .github/workflows/test.yml | 4 +--- Rakefile | 6 +++++- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index ddeb968..eeb21bb 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -22,7 +22,5 @@ jobs: ruby-version: ${{ matrix.ruby-version }} - name: Install dependencies run: bundle install - - name: Rubocop - run: rubocop - - name: Run tests + - name: Lint and test run: bundle exec rake diff --git a/Rakefile b/Rakefile index b6ae734..3e9bc8f 100644 --- a/Rakefile +++ b/Rakefile @@ -5,4 +5,8 @@ require "rspec/core/rake_task" RSpec::Core::RakeTask.new(:spec) -task default: :spec +task :standard do + sh "bundle exec standardrb" +end + +task default: [:standard, :spec] From fae9a378f3aee86ad92024f8dd872e72bc4cc23e Mon Sep 17 00:00:00 2001 From: Wyatt Kirby Date: Fri, 29 May 2026 14:43:44 -0400 Subject: [PATCH 05/12] feat!: add scope:/if:/through: keyword aliases, collection flag, drop :manage expansion BREAKING CHANGE: :manage no longer expands to [:create, :read, :update, :destroy]. Use explicit action lists instead. - Add scope: (aliases query:), if: (aliases predicate:), through: (aliases granted_by:) - Raise ArgumentError when both old and new keyword names are passed - Add collection: flag to grant; when true, auto-predicate is proc { true } - Remove :manage -> CRUD expansion from Role#add_permission - Rewrite README with collection/instance docs, fallthrough docs, upgrade guide - Add specs for keyword aliases and collection permissions --- README.md | 206 +++++++++++++++++++++----------- lib/papers_please/permission.rb | 28 +++-- lib/papers_please/role.rb | 86 ++++++------- spec/collection_spec.rb | 68 +++++++++++ spec/fixtures/access_policy.rb | 11 +- spec/keyword_aliases_spec.rb | 90 ++++++++++++++ spec/role_spec.rb | 21 +--- 7 files changed, 365 insertions(+), 145 deletions(-) create mode 100644 spec/collection_spec.rb create mode 100644 spec/keyword_aliases_spec.rb diff --git a/README.md b/README.md index 0386338..55f63d6 100644 --- a/README.md +++ b/README.md @@ -2,8 +2,14 @@ A roles and permissions gem from Apsis Labs. -**NOTE**: Still under heavy development, definitely not suitable for anything remotely resembling production usage. Very unlikely to even work. +## How It Works +Authorization boils down to a relationship between three things: a **user**, an **action**, and a **target**. `papers_please` lets you define these relationships in a single policy file using two primitives: + +- **`scope:`** — Given a user and an action, which records can they act on? Returns a collection (typically an ActiveRecord relation). +- **`if:`** — Given a user and a specific record, is this action allowed? Returns a boolean. + +When you provide a `scope:` without an `if:`, `papers_please` auto-generates the predicate for you. For **instance-level** permissions (e.g. `:show`, `:edit`), it checks whether the record is included in the scope. For **collection-level** permissions (e.g. `:index`, `:create`), you pass `collection: true` and the predicate becomes unconditionally `true` — the user can access the collection, and the scope controls what they see. ## Example @@ -11,49 +17,63 @@ A roles and permissions gem from Apsis Labs. # app/policies/access_policy.rb class AccessPolicy < PapersPlease::Policy def configure - # Define your roles - role :super, (proc { |u| u.super? }) - role :admin, (proc { |u| u.admin? }) - role :member, (proc { |u| u.member? }) + role :admin, proc { |u| u.admin? } + role :member, proc { |u| u.member? } role :guest - permit :super do |role| - role.grant [:manage], User - end - - permit :admin, :super do |role| - role.grant [:manage, :archive], Post + permit :admin do |role| + role.grant %i[index new create], Post, collection: true + role.grant %i[show edit update destroy], Post end permit :member do |role| - role.grant [:create], Post - role.grant [:update, :read], Post, query: (proc { |u| u.posts }) - role.grant [:archive], Post, query: (proc { |u| u.posts }), predicate: (proc { |u, post| !post.archived? }) + # Collection actions: member can see the index and create posts + role.grant :index, Post, collection: true, scope: proc { |u| u.posts } + role.grant :create, Post, collection: true + + # Instance actions: member can view/edit their own posts + role.grant %i[show edit update], Post, scope: proc { |u| u.posts } + + # Scope + condition: can archive own posts, but only if not already archived + role.grant :archive, Post, + scope: proc { |u| u.posts }, + if: proc { |u, post| !post.archived? } end permit :guest do |role| - role.grant [:read], Post, predicate: (proc { |u, post| !post.archived? }) + role.grant :index, Post, collection: true + role.grant :show, Post, if: proc { |_u, post| !post.archived? } end + # Delegated permissions: attachment access is determined by its post permit :member, :guest do |role| - role.grant [:read], Attachment, granted_by: [Post, (proc { |u, attachment| attachment.post })] + role.grant :show, Attachment, through: [Post, proc { |_u, attachment| attachment.post }] end end end +``` -# app/controllers/posts_controller.rb +### Controller Usage + +```ruby class PostsController < ApplicationController # GET /posts def index - @posts = policy.query(:read, Post) + @posts = policy.scope_for(:index, Post) render json: @posts end # GET /posts/:id def show @post = Post.find(params[:id]) - policy.authorize! :read, @post + policy.authorize! :show, @post + render json: @post + end + # POST /posts + def create + policy.authorize! :create, Post + @post = Post.create!(post_params) render json: @post end @@ -61,59 +81,100 @@ class PostsController < ApplicationController def archive @post = Post.find(params[:id]) policy.authorize! :archive, @post - @post.update!(archived: true) render json: @post end end +``` -class AttachmentsController < ApplicationController - # GET /attachments/:id - def show - @attachment = Attachment.find([:id]) - policy.authorize! :read, @attachment # => proxied to Post permission check +### Checking Permissions + +```ruby +policy = AccessPolicy.new(current_user) + +# Can this user perform this action on this record? +policy.can?(:edit, @post) # => true/false +policy.cannot?(:destroy, @post) # => true/false + +# Raise if not allowed +policy.authorize!(:edit, @post) # => raises PapersPlease::AccessDenied + +# What records can this user access for this action? +policy.scope_for(:index, Post) # => ActiveRecord::Relation +``` + +## Collection vs Instance Permissions + +This is the most important concept to understand. Actions fall into two categories: + +**Collection-level** actions operate on the resource as a whole, not a specific record. Think `:index`, `:new`, `:create`. The question is "can this user access this resource?" not "can this user access this specific record?" + +**Instance-level** actions operate on a specific record. Think `:show`, `:edit`, `:update`, `:destroy`. The question is "can this user act on this particular record?" + +When you provide a `scope:` and let `papers_please` auto-generate the `if:` predicate: + +```ruby +# Instance: auto-predicate checks record ∈ scope +role.grant :show, Post, scope: proc { |u| u.posts } +# policy.can?(:show, some_post) => u.posts.include?(some_post) + +# Collection: auto-predicate is always true +role.grant :index, Post, collection: true, scope: proc { |u| u.posts } +# policy.can?(:index, Post) => true +# policy.scope_for(:index, Post) => u.posts +``` - send_data @attachment.data, type: @attachment.content_type +Without `collection: true`, checking `can?(:index, Post)` would try to check whether the `Post` *class* is included in the scoped collection — which is never what you want. + +## Role Fallthrough + +By default, `papers_please` checks roles in order and returns the result from the **first role that has a matching permission** — even if that result is `false`. This means a denial in an earlier role prevents later roles from granting access. + +Calling `allow_fallthrough` in your policy changes this: when a role denies a permission, the check continues to the next applicable role. Access is granted if **any** role allows it, and denied only if **no** role does. + +```ruby +class AccessPolicy < PapersPlease::Policy + def configure + allow_fallthrough + + role :admin, proc { |u| u.admin? } + role :member, proc { |u| u.member? } + + # An admin who is also a member will match both roles. + # Without fallthrough, if :admin denies :read, the check stops. + # With fallthrough, :member still gets a chance to grant it. + + permit :admin do |role| + role.grant :read, Post, scope: proc { |u| u.posts } + end + + permit :member do |role| + role.grant :read, Post, if: proc { |_u, post| post.published? } + end end end ``` -## A helpful CLI +Most real-world policies should use `allow_fallthrough`. Without it, role ordering becomes load-bearing and easy to get wrong. + +## Delegated Permissions with `through:` + +When a resource's access is determined by a parent, use `through:`: + +```ruby +role.grant :show, Attachment, through: [Post, proc { |_u, attachment| attachment.post }] +``` + +This means: "to check if a user can `:show` an `Attachment`, find its `Post` and check whether the user can `:show` that `Post` instead." + +## A Helpful CLI ```bash $ rails papers_please:roles - -# => -# +---------+------------+------------+------------+----------------+-------------------+ -# | role | subject | permission | has query? | has predicate? | granted by other? | -# +---------+------------+------------+------------+----------------+-------------------+ -# | admin | Post | create | yes | yes | no | -# | | Post | read | yes | yes | no | -# | | Post | update | yes | yes | no | -# | | Post | destroy | yes | yes | no | -# | | Attachment | create | yes | yes | no | -# | | Attachment | read | yes | yes | no | -# | | Attachment | update | yes | yes | no | -# | | Attachment | destroy | yes | yes | no | -# +---------+------------+------------+------------+----------------+-------------------+ -# | manager | Post | create | yes | yes | no | -# | | Post | read | yes | yes | no | -# | | Post | update | yes | yes | no | -# | | Post | destroy | yes | yes | no | -# | | Attachment | create | yes | yes | yes | -# | | Attachment | read | yes | yes | yes | -# | | Attachment | update | yes | yes | yes | -# | | Attachment | destroy | yes | yes | yes | -# +---------+------------+------------+------------+----------------+-------------------+ -# | member | Post | create | yes | yes | no | -# | | Post | read | yes | yes | no | -# | | Post | update | yes | yes | no | -# | | Attachment | create | yes | yes | yes | -# | | Attachment | read | yes | yes | yes | -# | | Attachment | update | yes | yes | yes | -# +---------+------------+------------+------------+----------------+-------------------+ ``` +Prints a table of all roles, subjects, and permissions in your policy. + ## Installation Add this line to your application's Gemfile: @@ -126,29 +187,36 @@ And then execute: $ bundle -Or install it yourself as: +## Upgrading + +### Keyword Renames - $ gem install papers_please +The following keyword arguments have been renamed. The old names still work but may be deprecated in a future release: -## Usage +| Old | New | Meaning | +|---|---|---| +| `query:` | `scope:` | Which records can this user access? | +| `predicate:` | `if:` | Is this specific action allowed? | +| `granted_by:` | `through:` | Delegate permission check to a parent | -TODO: Write usage instructions here +You cannot pass both the old and new name for the same option. -## Theory +### Removed: `:manage` expansion -The structure of `papers_please` is very simple. At its core, it is a mechanism for storing and retrieving `Procs`. In an authorization context, these `Procs` answer two questions: +`:manage` no longer expands to `[:create, :read, :update, :destroy]`. Define your action lists explicitly: -1. Given a specific user and a specific permission, which objects am I allowed to operate on? -2. Given a specific user and a specific object, do I have a specific permission? +```ruby +# Before +role.grant :manage, Post -The machinery of `papers_please` tries to simplify the organization and subsequent access to these questions as much as possible. +# After +role.grant %i[create read update destroy], Post +``` ## Development After checking out the repo, run `bin/setup` to install dependencies. Then, run `rake spec` to run the tests. You can also run `bin/console` for an interactive prompt that will allow you to experiment. -To install this gem onto your local machine, run `bundle exec rake install`. To release a new version, update the version number in `version.rb`, and then run `bundle exec rake release`, which will create a git tag for the version, push git commits and tags, and push the `.gem` file to [rubygems.org](https://rubygems.org). - ### Releasing Create a release from `main`: @@ -164,7 +232,7 @@ Publishing to RubyGems and creating a GitHub Release are handled automatically b ## Contributing -Bug reports and pull requests are welcome on GitHub at https://github.com/wkirby/papers_please. +Bug reports and pull requests are welcome on GitHub at https://github.com/apsislabs/papers_please. ## Special Thanks diff --git a/lib/papers_please/permission.rb b/lib/papers_please/permission.rb index ba0413d..61f0d3c 100644 --- a/lib/papers_please/permission.rb +++ b/lib/papers_please/permission.rb @@ -2,16 +2,22 @@ module PapersPlease class Permission - attr_accessor :key, :subject - attr_reader :query, :predicate, :granted_by, :granting_class + attr_accessor :key, :subject, :collection + attr_reader :scope, :predicate, :granted_by, :granting_class + + alias_method :query, :scope + alias_method :collection?, :collection + + def initialize(key, subject, scope: nil, query: nil, predicate: nil, granted_by: nil, granting_class: nil, collection: false) + raise ArgumentError, "cannot pass both :scope and :query" if scope && query - def initialize(key, subject, query: nil, predicate: nil, granted_by: nil, granting_class: nil) self.key = key self.subject = subject - self.query = query + self.scope = scope || query self.predicate = predicate self.granted_by = granted_by self.granting_class = granting_class + self.collection = collection end def granted_by_other? @@ -23,27 +29,27 @@ def matches?(key, subject) end def granted?(*args) - return predicate.call(*args) if predicate.is_a? Proc + return predicate.call(*args) if predicate.is_a?(Proc) # :nocov: - # as far as we can tell this line is unreachable, but just in case... false # :nocov: end def fetch(*args) - return query.call(*args) if query.is_a? Proc + return scope.call(*args) if scope.is_a?(Proc) nil end - # Setters - def query=(val) - raise ArgumentError, "query must be a Proc, #{val.class} given" if val && !val.is_a?(Proc) + def scope=(val) + raise ArgumentError, "scope must be a Proc, #{val.class} given" if val && !val.is_a?(Proc) - @query = val + @scope = val end + alias_method :query=, :scope= + def predicate=(val) raise ArgumentError, "predicate must be a Proc, #{val.class} given" if val && !val.is_a?(Proc) diff --git a/lib/papers_please/role.rb b/lib/papers_please/role.rb index 7f88b81..66fbbb6 100644 --- a/lib/papers_please/role.rb +++ b/lib/papers_please/role.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + module PapersPlease class Role attr_reader :name, :predicate, :permissions @@ -9,13 +11,15 @@ def initialize(name, predicate: nil) end def applies_to?(user) - return @predicate.call(user) if @predicate.is_a? Proc + return @predicate.call(user) if @predicate.is_a?(Proc) true end - def add_permission(actions, klass, query: nil, predicate: nil, granted_by: nil) - prepare_actions(actions).each do |action| + def add_permission(actions, klass, **opts) + scope, predicate, granted_by, collection = resolve_opts(opts) + + Array(actions).each do |action| raise DuplicatePermission if permission_exists?(action, klass) if !granted_by.nil? && !valid_grant?(granted_by) @@ -24,12 +28,8 @@ def add_permission(actions, klass, query: nil, predicate: nil, granted_by: nil) end permissions << make_permission( - action, - actions, - klass, - query: query, - predicate: predicate, - granted_by: granted_by + action, klass, + scope: scope, predicate: predicate, granted_by: granted_by, collection: collection ) end end @@ -47,6 +47,26 @@ def permission_exists?(action, subject) private + def resolve_opts(opts) + scope = resolve_alias(opts, :scope, :query) + predicate = resolve_alias(opts, :if, :predicate) + granted_by = resolve_alias(opts, :through, :granted_by) + collection = opts.fetch(:collection, false) + + [scope, predicate, granted_by, collection] + end + + def resolve_alias(opts, new_name, old_name) + has_new = opts.key?(new_name) + has_old = opts.key?(old_name) + + if has_new && has_old + raise ArgumentError, "cannot pass both :#{new_name} and :#{old_name}" + end + + has_new ? opts[new_name] : opts[old_name] + end + def valid_grant?(tuple) return false unless tuple.is_a? Array return false unless tuple.length == 2 @@ -56,43 +76,29 @@ def valid_grant?(tuple) true end - # Wrap actions, translating :manage into :crud - def prepare_actions(action) - Array(action).flat_map do |a| - (a == :manage) ? %i[create read update destroy] : [a] - end - end - - # rubocop:disable Metrics/MethodLength - def make_permission(action, actions, klass, query: nil, predicate: nil, granted_by: nil) - has_query = query.is_a?(Proc) + def make_permission(action, klass, scope: nil, predicate: nil, granted_by: nil, collection: false) + has_scope = scope.is_a?(Proc) has_predicate = predicate.is_a?(Proc) - permission = make_base_permission(action, klass, granted_by: granted_by) + permission = make_base_permission(action, klass, granted_by: granted_by, collection: collection) - if has_query && has_predicate - # Both query & predicate provided - permission.query = query + if has_scope && has_predicate + permission.scope = scope permission.predicate = predicate - elsif has_query && !has_predicate - # Only query provided - permission.query = query - permission.predicate = build_predicate_from_query(action, actions, klass, query) - elsif !has_query && has_predicate - # Only predicate provided + elsif has_scope + permission.scope = scope + permission.predicate = collection ? (proc { true }) : build_predicate_from_scope(klass, scope) + elsif has_predicate permission.predicate = predicate else - # Neither provided - permission.query = (proc { klass.all }) + permission.scope = (proc { klass.all }) permission.predicate = (proc { true }) end permission end - # rubocop:enable Metrics/MethodLength - - def make_base_permission(action, klass, granted_by: nil) - permission = Permission.new(action, klass) + def make_base_permission(action, klass, granted_by: nil, collection: false) + permission = Permission.new(action, klass, collection: collection) if granted_by permission.granting_class = granted_by[0] @@ -102,15 +108,9 @@ def make_base_permission(action, klass, granted_by: nil) permission end - def build_predicate_from_query(action, actions, klass, query) - # If the action is :create, expanded from :manage - # then we set the default all predicate - return proc { true } if action == :create && actions == :manage - - # Otherwise the default predicate is to check - # for inclusion in the returned relationship + def build_predicate_from_scope(klass, scope) proc do |user, obj| - res = query.call(user, klass, action) + res = scope.call(user, klass) res.respond_to?(:include?) && res.include?(obj) end end diff --git a/spec/collection_spec.rb b/spec/collection_spec.rb new file mode 100644 index 0000000..4cb3f46 --- /dev/null +++ b/spec/collection_spec.rb @@ -0,0 +1,68 @@ +# frozen_string_literal: true + +require "spec_helper" + +RSpec.describe "Collection permissions" do + let(:user) { User.new(posts: [post], admin: true) } + let(:post) { Post.new } + let(:scope_proc) { proc { |u| u.posts } } + + describe "collection: true" do + let(:role) { PapersPlease::Role.new(:test) } + + it "auto-predicate allows the class" do + role.grant :index, Post, collection: true, scope: scope_proc + permission = role.find_permission(:index, Post) + + expect(permission.collection?).to be true + expect(permission.granted?(user, Post)).to be true + end + + it "scope still works for scoping" do + role.grant :index, Post, collection: true, scope: scope_proc + permission = role.find_permission(:index, Post) + + expect(permission.fetch(user, Post)).to contain_exactly(post) + end + + it "explicit if: overrides collection auto-predicate" do + role.grant :index, Post, collection: true, scope: scope_proc, if: proc { false } + permission = role.find_permission(:index, Post) + + expect(permission.granted?(user, Post)).to be false + end + + it "works without a scope" do + role.grant :create, Post, collection: true + permission = role.find_permission(:create, Post) + + expect(permission.collection?).to be true + expect(permission.granted?(user, Post)).to be true + end + end + + describe "collection: false (default)" do + let(:role) { PapersPlease::Role.new(:test) } + + it "auto-predicate checks inclusion" do + role.grant :show, Post, scope: scope_proc + permission = role.find_permission(:show, post) + + expect(permission.collection?).to be false + expect(permission.granted?(user, post)).to be true + end + + it "auto-predicate rejects non-included instances" do + role.grant :show, Post, scope: scope_proc + + other = Post.new + expect(role.find_permission(:show, other).granted?(user, other)).to be false + end + + it "auto-predicate rejects the class itself" do + role.grant :show, Post, scope: scope_proc + + expect(role.find_permission(:show, Post).granted?(user, Post)).to be false + end + end +end diff --git a/spec/fixtures/access_policy.rb b/spec/fixtures/access_policy.rb index 034e358..61a1fd6 100644 --- a/spec/fixtures/access_policy.rb +++ b/spec/fixtures/access_policy.rb @@ -7,17 +7,18 @@ def configure role :member, proc { |user| user.member? } permit :admin do |role| - role.grant :manage, Post - role.grant :manage, Attachment + role.grant %i[create read update destroy], Post + role.grant %i[create read update destroy], Attachment end permit :manager do |role| - role.grant :manage, Post, query: proc { |u| u.posts } - role.grant :manage, Attachment, granted_by: [Post, proc { |_u, a| a.post }] + role.grant :create, Post, collection: true + role.grant %i[read update destroy], Post, query: proc { |u| u.posts } + role.grant %i[create read update destroy], Attachment, granted_by: [Post, proc { |_u, a| a.post }] end permit :member do |role| - role.grant :create, Post + role.grant :create, Post, collection: true role.grant %i[read update], Post, query: proc { |u| u.posts } role.grant %i[create read update], Attachment, granted_by: [Post, proc { |_u, a| a.post }] end diff --git a/spec/keyword_aliases_spec.rb b/spec/keyword_aliases_spec.rb new file mode 100644 index 0000000..8599b5f --- /dev/null +++ b/spec/keyword_aliases_spec.rb @@ -0,0 +1,90 @@ +# frozen_string_literal: true + +require "spec_helper" + +RSpec.describe "Keyword aliases" do + let(:user) { User.new(posts: [post], admin: true) } + let(:post) { Post.new } + let(:other_post) { Post.new } + let(:scope_proc) { proc { |u| u.posts } } + let(:if_proc) { proc { |_u, p| p == post } } + + describe "scope: aliases query:" do + let(:role) { PapersPlease::Role.new(:test) } + + it "works with scope:" do + role.grant :read, Post, scope: scope_proc + permission = role.find_permission(:read, post) + + expect(permission.scope).to eq scope_proc + expect(permission.query).to eq scope_proc + expect(permission.granted?(user, post)).to be true + expect(permission.granted?(user, other_post)).to be false + end + + it "works with query:" do + role.grant :read, Post, query: scope_proc + permission = role.find_permission(:read, post) + + expect(permission.scope).to eq scope_proc + expect(permission.query).to eq scope_proc + end + + it "raises when both are passed" do + expect { + role.grant :read, Post, scope: scope_proc, query: scope_proc + }.to raise_error(ArgumentError, /cannot pass both/) + end + end + + describe "if: aliases predicate:" do + let(:role) { PapersPlease::Role.new(:test) } + + it "works with if:" do + role.grant :read, Post, if: if_proc + permission = role.find_permission(:read, post) + + expect(permission.predicate).to eq if_proc + expect(permission.granted?(user, post)).to be true + expect(permission.granted?(user, other_post)).to be false + end + + it "works with predicate:" do + role.grant :read, Post, predicate: if_proc + permission = role.find_permission(:read, post) + + expect(permission.predicate).to eq if_proc + end + + it "raises when both are passed" do + expect { + role.grant :read, Post, if: if_proc, predicate: if_proc + }.to raise_error(ArgumentError, /cannot pass both/) + end + end + + describe "through: aliases granted_by:" do + let(:role) { PapersPlease::Role.new(:test) } + let(:grant_tuple) { [Post, proc { |_u, a| a.post }] } + + it "works with through:" do + role.grant :read, Attachment, through: grant_tuple + permission = role.find_permission(:read, Attachment.new(post: post)) + + expect(permission.granted_by_other?).to be true + end + + it "works with granted_by:" do + role.grant :read, Attachment, granted_by: grant_tuple + permission = role.find_permission(:read, Attachment.new(post: post)) + + expect(permission.granted_by_other?).to be true + end + + it "raises when both are passed" do + expect { + role.grant :read, Attachment, through: grant_tuple, granted_by: grant_tuple + }.to raise_error(ArgumentError, /cannot pass both/) + end + end +end diff --git a/spec/role_spec.rb b/spec/role_spec.rb index 7e53849..cfe405d 100644 --- a/spec/role_spec.rb +++ b/spec/role_spec.rb @@ -30,15 +30,13 @@ end describe "#add_permission" do - context "manage expansion" do + context "manage is treated as a literal action" do let(:role) { described_class.new(:admin) } - it "adds correct permissions" do + it "does not expand :manage" do role.add_permission(:manage, Post) - expect(role.permissions.size).to eq 4 - - permission_actions = role.permissions.map(&:key) - expect(permission_actions).to contain_exactly(:create, :read, :update, :destroy) + expect(role.permissions.size).to eq 1 + expect(role.permissions.first.key).to eq(:manage) end end @@ -49,11 +47,6 @@ role.add_permission(:read, Post) expect { role.add_permission(:read, Post) }.to raise_error(PapersPlease::DuplicatePermission) end - - it "raises error for duplicates in expansions" do - role.add_permission(:read, Post) - expect { role.add_permission(:manage, Post) }.to raise_error(PapersPlease::DuplicatePermission) - end end context "granted_by" do @@ -139,12 +132,6 @@ expect(admin_role.permissions.size).to eq 1 expect(admin_role.permissions.first.predicate.call(admin_user, Post)).to be false end - - # it 'generates a default true for :manage' do - # manager_role.add_permission(:manage, Post, query: query) - # expect(manager_role.permissions.size).to eq 4 - # expect(manager_role.permissions.first.predicate.call(manager_user, Post)).to be true - # end end context "only predicate" do From cc4e331cc15212fc7a21341ad41248956552a970 Mon Sep 17 00:00:00 2001 From: Wyatt Kirby Date: Fri, 29 May 2026 14:44:57 -0400 Subject: [PATCH 06/12] feat: add permitted_actions to query which actions a user can perform on a subject --- README.md | 3 +++ lib/papers_please/policy.rb | 12 ++++++++++++ spec/policy_spec.rb | 34 ++++++++++++++++++++++++++++++++++ 3 files changed, 49 insertions(+) diff --git a/README.md b/README.md index 55f63d6..81411a0 100644 --- a/README.md +++ b/README.md @@ -101,6 +101,9 @@ policy.authorize!(:edit, @post) # => raises PapersPlease::AccessDenied # What records can this user access for this action? policy.scope_for(:index, Post) # => ActiveRecord::Relation + +# What actions can this user perform on this record? +policy.permitted_actions(@post) # => [:show, :edit, :update] ``` ## Collection vs Instance Permissions diff --git a/lib/papers_please/policy.rb b/lib/papers_please/policy.rb index 0bef69a..00fa7c8 100644 --- a/lib/papers_please/policy.rb +++ b/lib/papers_please/policy.rb @@ -85,6 +85,18 @@ def get_applicable_roles_by_keys(keys) applicable_roles.slice(*Array(keys)) end + def permitted_actions(subject) + applicable_roles.each_with_object(Set.new) do |(_, role), actions| + role.permissions.each do |permission| + next unless permission.matches?(permission.key, subject) + next if actions.include?(permission.key) + next unless permission.granted?(user, subject, permission.key) + + actions << permission.key + end + end.to_a + end + def roles_that_can(action, subject) applicable_roles.filter do |_, role| permission = role.find_permission(action, subject) diff --git a/spec/policy_spec.rb b/spec/policy_spec.rb index 38abce8..55c5b40 100644 --- a/spec/policy_spec.rb +++ b/spec/policy_spec.rb @@ -175,6 +175,40 @@ def configure end end + describe "#permitted_actions" do + it "returns actions the user can perform on an instance" do + policy_klass = Class.new(PapersPlease::Policy) do + def configure + role :member + + permit :member do |role| + role.grant :read, Post, if: proc { true } + role.grant :update, Post, if: proc { true } + role.grant :destroy, Post, if: proc { false } + end + end + end + + policy = policy_klass.new(member) + expect(policy.permitted_actions(post)).to contain_exactly(:read, :update) + end + + it "returns actions across multiple roles with fallthrough" do + policy = sample_fallthrough_policy(admin) + published_post = Post.new + published_post.published = true + admin.posts << published_post + + actions = policy.permitted_actions(published_post) + expect(actions).to contain_exactly(:read) + end + + it "returns empty array when no actions are permitted" do + policy = sample_policy(member) + expect(policy.permitted_actions(other_post)).to be_empty + end + end + describe "#authorize!" do it "raises an AccessDenied error for failing predicate" do policy = sample_policy(member) From cc38c648e91cb9b3b6e3a26897cbfad61d5f1b7a Mon Sep 17 00:00:00 2001 From: Wyatt Kirby Date: Fri, 29 May 2026 14:48:37 -0400 Subject: [PATCH 07/12] refactor: replace granted_by tuple with Delegation struct, add lambda support via Callable - Add PapersPlease::Delegation struct to replace raw [Class, Proc] tuples - Add PapersPlease::Callable module for arity-safe proc/lambda invocation - Permission now stores a Delegation instead of separate granted_by/granting_class - All proc call sites go through Callable.call for lambda compat - Lambdas with fewer params than the call site provides are handled gracefully --- lib/papers_please.rb | 2 ++ lib/papers_please/callable.rb | 14 ++++++++++ lib/papers_please/delegation.rb | 9 +++++++ lib/papers_please/permission.rb | 43 ++++++++++++++---------------- lib/papers_please/policy.rb | 6 ++--- lib/papers_please/role.rb | 47 +++++++++++++-------------------- spec/keyword_aliases_spec.rb | 33 +++++++++++++++++++++++ spec/permission_spec.rb | 25 ++++++++++++++---- spec/role_spec.rb | 16 +++++------ 9 files changed, 126 insertions(+), 69 deletions(-) create mode 100644 lib/papers_please/callable.rb create mode 100644 lib/papers_please/delegation.rb diff --git a/lib/papers_please.rb b/lib/papers_please.rb index c0d6ee3..a0a9842 100644 --- a/lib/papers_please.rb +++ b/lib/papers_please.rb @@ -2,6 +2,8 @@ require "papers_please/version" require "papers_please/errors" +require "papers_please/callable" +require "papers_please/delegation" require "papers_please/policy" require "papers_please/role" require "papers_please/permission" diff --git a/lib/papers_please/callable.rb b/lib/papers_please/callable.rb new file mode 100644 index 0000000..eeebef7 --- /dev/null +++ b/lib/papers_please/callable.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +module PapersPlease + module Callable + def self.call(callable, *args) + if callable.lambda? + arity = callable.arity + callable.call(*((arity >= 0) ? args.take(arity) : args)) + else + callable.call(*args) + end + end + end +end diff --git a/lib/papers_please/delegation.rb b/lib/papers_please/delegation.rb new file mode 100644 index 0000000..2307b7c --- /dev/null +++ b/lib/papers_please/delegation.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +module PapersPlease + Delegation = Struct.new(:klass, :resolver) do + def resolve(user, subject) + resolver.call(user, subject) + end + end +end diff --git a/lib/papers_please/permission.rb b/lib/papers_please/permission.rb index 61f0d3c..ee25e9e 100644 --- a/lib/papers_please/permission.rb +++ b/lib/papers_please/permission.rb @@ -2,26 +2,33 @@ module PapersPlease class Permission - attr_accessor :key, :subject, :collection - attr_reader :scope, :predicate, :granted_by, :granting_class + attr_accessor :key, :subject, :collection, :delegation + attr_reader :scope, :predicate alias_method :query, :scope alias_method :collection?, :collection - def initialize(key, subject, scope: nil, query: nil, predicate: nil, granted_by: nil, granting_class: nil, collection: false) + def initialize(key, subject, scope: nil, query: nil, predicate: nil, delegation: nil, collection: false) raise ArgumentError, "cannot pass both :scope and :query" if scope && query self.key = key self.subject = subject self.scope = scope || query self.predicate = predicate - self.granted_by = granted_by - self.granting_class = granting_class + self.delegation = delegation self.collection = collection end + def delegated? + delegation.is_a?(Delegation) + end + + def granting_class + delegation&.klass + end + def granted_by_other? - @granting_class.is_a?(Class) && @granted_by.is_a?(Proc) + delegated? end def matches?(key, subject) @@ -29,7 +36,7 @@ def matches?(key, subject) end def granted?(*args) - return predicate.call(*args) if predicate.is_a?(Proc) + return Callable.call(predicate, *args) if predicate # :nocov: false @@ -37,39 +44,29 @@ def granted?(*args) end def fetch(*args) - return scope.call(*args) if scope.is_a?(Proc) + return Callable.call(scope, *args) if scope nil end def scope=(val) - raise ArgumentError, "scope must be a Proc, #{val.class} given" if val && !val.is_a?(Proc) - + validate_callable!(:scope, val) @scope = val end alias_method :query=, :scope= def predicate=(val) - raise ArgumentError, "predicate must be a Proc, #{val.class} given" if val && !val.is_a?(Proc) - + validate_callable!(:predicate, val) @predicate = val end - def granted_by=(val) - raise ArgumentError, "granted_by must be a Proc, #{val.class} given" if val && !val.is_a?(Proc) - - @granted_by = val - end - - def granting_class=(val) - raise ArgumentError, "granting_class must be a Class, #{val.class} given" if val && !val.is_a?(Class) + private - @granting_class = val + def validate_callable!(name, val) + raise ArgumentError, "#{name} must be a Proc, #{val.class} given" if val && !val.is_a?(Proc) end - private - def key_matches?(key) key == @key end diff --git a/lib/papers_please/policy.rb b/lib/papers_please/policy.rb index 00fa7c8..a58898d 100644 --- a/lib/papers_please/policy.rb +++ b/lib/papers_please/policy.rb @@ -144,10 +144,8 @@ def permission_granted?(permission, action, subject) end def get_proxied_permission(permission, action, subject, role) - # Get proxied subject - subject = subject.is_a?(Class) ? permission.granting_class : permission.granted_by.call(user, subject) - - # Get proxied permission + delegation = permission.delegation + subject = subject.is_a?(Class) ? delegation.klass : delegation.resolve(user, subject) permission = role.find_permission(action, subject) [subject, permission] diff --git a/lib/papers_please/role.rb b/lib/papers_please/role.rb index 66fbbb6..cbea546 100644 --- a/lib/papers_please/role.rb +++ b/lib/papers_please/role.rb @@ -11,25 +11,22 @@ def initialize(name, predicate: nil) end def applies_to?(user) - return @predicate.call(user) if @predicate.is_a?(Proc) + return Callable.call(@predicate, user) if @predicate.is_a?(Proc) true end def add_permission(actions, klass, **opts) - scope, predicate, granted_by, collection = resolve_opts(opts) + scope, predicate, through, collection = resolve_opts(opts) Array(actions).each do |action| raise DuplicatePermission if permission_exists?(action, klass) - if !granted_by.nil? && !valid_grant?(granted_by) - raise InvalidGrant, - "granted_by must be an array of [Class, Proc]" - end + delegation = build_delegation(through) permissions << make_permission( action, klass, - scope: scope, predicate: predicate, granted_by: granted_by, collection: collection + scope: scope, predicate: predicate, delegation: delegation, collection: collection ) end end @@ -50,10 +47,10 @@ def permission_exists?(action, subject) def resolve_opts(opts) scope = resolve_alias(opts, :scope, :query) predicate = resolve_alias(opts, :if, :predicate) - granted_by = resolve_alias(opts, :through, :granted_by) + through = resolve_alias(opts, :through, :granted_by) collection = opts.fetch(:collection, false) - [scope, predicate, granted_by, collection] + [scope, predicate, through, collection] end def resolve_alias(opts, new_name, old_name) @@ -67,19 +64,22 @@ def resolve_alias(opts, new_name, old_name) has_new ? opts[new_name] : opts[old_name] end - def valid_grant?(tuple) - return false unless tuple.is_a? Array - return false unless tuple.length == 2 - return false unless tuple[0].is_a? Class - return false unless tuple[1].is_a? Proc + def build_delegation(through) + return nil if through.nil? - true + raise InvalidGrant, "through must be a [Class, Proc], got #{through.inspect}" unless valid_delegation?(through) + + Delegation.new(through[0], through[1]) end - def make_permission(action, klass, scope: nil, predicate: nil, granted_by: nil, collection: false) + def valid_delegation?(tuple) + tuple.is_a?(Array) && tuple.length == 2 && tuple[0].is_a?(Class) && tuple[1].is_a?(Proc) + end + + def make_permission(action, klass, scope: nil, predicate: nil, delegation: nil, collection: false) has_scope = scope.is_a?(Proc) has_predicate = predicate.is_a?(Proc) - permission = make_base_permission(action, klass, granted_by: granted_by, collection: collection) + permission = Permission.new(action, klass, delegation: delegation, collection: collection) if has_scope && has_predicate permission.scope = scope @@ -97,20 +97,9 @@ def make_permission(action, klass, scope: nil, predicate: nil, granted_by: nil, permission end - def make_base_permission(action, klass, granted_by: nil, collection: false) - permission = Permission.new(action, klass, collection: collection) - - if granted_by - permission.granting_class = granted_by[0] - permission.granted_by = granted_by[1] - end - - permission - end - def build_predicate_from_scope(klass, scope) proc do |user, obj| - res = scope.call(user, klass) + res = Callable.call(scope, user, klass) res.respond_to?(:include?) && res.include?(obj) end end diff --git a/spec/keyword_aliases_spec.rb b/spec/keyword_aliases_spec.rb index 8599b5f..ed3afa5 100644 --- a/spec/keyword_aliases_spec.rb +++ b/spec/keyword_aliases_spec.rb @@ -63,6 +63,39 @@ end end + describe "lambda support" do + let(:role) { PapersPlease::Role.new(:test) } + + it "works with single-arg lambda as scope" do + role.grant :read, Post, scope: ->(u) { u.posts } + permission = role.find_permission(:read, post) + + expect(permission.granted?(user, post)).to be true + expect(permission.granted?(user, other_post)).to be false + end + + it "works with two-arg lambda as scope" do + role.grant :read, Post, scope: ->(u, _klass) { u.posts } + permission = role.find_permission(:read, post) + + expect(permission.granted?(user, post)).to be true + end + + it "works with lambda as if" do + role.grant :read, Post, if: ->(u, p) { u.posts.include?(p) } + permission = role.find_permission(:read, post) + + expect(permission.granted?(user, post)).to be true + expect(permission.granted?(user, other_post)).to be false + end + + it "works with lambda as role predicate" do + lambda_role = PapersPlease::Role.new(:test, predicate: ->(u) { u.admin? }) + + expect(lambda_role.applies_to?(user)).to be true + end + end + describe "through: aliases granted_by:" do let(:role) { PapersPlease::Role.new(:test) } let(:grant_tuple) { [Post, proc { |_u, a| a.post }] } diff --git a/spec/permission_spec.rb b/spec/permission_spec.rb index 8078c47..b28873a 100644 --- a/spec/permission_spec.rb +++ b/spec/permission_spec.rb @@ -16,8 +16,6 @@ it "validates arguments" do expect { subject.new(:key, book, query: "invalid") }.to raise_error(ArgumentError) expect { subject.new(:key, book, predicate: "invalid") }.to raise_error(ArgumentError) - expect { subject.new(:key, book, granted_by: "invalid") }.to raise_error(ArgumentError) - expect { subject.new(:key, book, granting_class: "invalid") }.to raise_error(ArgumentError) end it "allows query and predicate" do @@ -65,7 +63,7 @@ end end - describe "fetch" do + describe "#fetch" do let(:array_permission) { subject.new(:read, book, query: proc { [] }) } let(:empty_permission) { subject.new(:read, book, query: proc {}) } let(:stub_proc) { proc { [] } } @@ -79,14 +77,31 @@ expect(empty_permission.fetch).to be_nil end - it "calls the predicate proc" do + it "calls the scope proc" do expect(stub_proc).to receive(:call).and_return([]) expect(stub_perm.fetch).to eq [] end - it "calls the predicate proc" do + it "calls the scope proc with args" do expect(stub_proc).to receive(:call).with(:arg).and_return(nil) expect(stub_perm.fetch(:arg)).to be_nil end end + + describe "#delegated?" do + it "is true with a delegation" do + delegation = PapersPlease::Delegation.new(Post, proc { |_u, a| a }) + perm = subject.new(:read, book, delegation: delegation) + + expect(perm.delegated?).to be true + expect(perm.granting_class).to eq Post + end + + it "is false without a delegation" do + perm = subject.new(:read, book) + + expect(perm.delegated?).to be false + expect(perm.granting_class).to be_nil + end + end end diff --git a/spec/role_spec.rb b/spec/role_spec.rb index cfe405d..909852b 100644 --- a/spec/role_spec.rb +++ b/spec/role_spec.rb @@ -50,7 +50,7 @@ end context "granted_by" do - let(:granted_by) { proc { |_u, a| a.post } } + let(:resolver) { proc { |_u, a| a.post } } let(:granting_class) { Post } let(:role) { described_class.new(:admin) } @@ -59,7 +59,7 @@ role.add_permission(:read, Attachment, granted_by: "invalid") end.to raise_error PapersPlease::InvalidGrant expect do - role.add_permission(:read, Attachment, granted_by: granted_by) + role.add_permission(:read, Attachment, granted_by: resolver) end.to raise_error PapersPlease::InvalidGrant expect do role.add_permission(:read, Attachment, granted_by: granting_class) @@ -67,18 +67,18 @@ expect { role.add_permission(:read, Attachment, granted_by: []) }.to raise_error PapersPlease::InvalidGrant expect do role.add_permission(:read, Attachment, - granted_by: [granted_by, granting_class]) + granted_by: [resolver, granting_class]) end.to raise_error PapersPlease::InvalidGrant - expect { role.add_permission(:read, Attachment, granted_by: [granting_class, granted_by]) }.not_to raise_error + expect { role.add_permission(:read, Attachment, granted_by: [granting_class, resolver]) }.not_to raise_error end - it "adds a granted permission" do - role.add_permission(:read, Attachment, granted_by: [granting_class, granted_by]) + it "adds a delegated permission" do + role.add_permission(:read, Attachment, granted_by: [granting_class, resolver]) expect(role.permissions.size).to eq 1 permission = role.permissions.first - expect(permission.granted_by_other?).to be true - expect(permission.granted_by).to eq granted_by + expect(permission.delegated?).to be true + expect(permission.delegation.resolver).to eq resolver expect(permission.granting_class).to eq granting_class end end From d49c5f93193b9f0f83a37e6aac9352655d9da423 Mon Sep 17 00:00:00 2001 From: Wyatt Kirby Date: Fri, 29 May 2026 14:53:00 -0400 Subject: [PATCH 08/12] fix: remove Callable module, fix subject mutation in can?, normalize call signatures MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Remove PapersPlease::Callable; lambdas work naturally with correct arity - Delegation uses Data.define instead of Struct (immutable) - Permission: attrs are readers not accessors, add matches_subject? - Permission#granted?(user, subject) and #fetch(user, klass) have fixed signatures instead of variadic *args — no more silent action arg that nobody consumed - Policy: fix can? mutating subject across role iterations via reassignment - Policy: fix nil predicate return being treated as truthy with fallthrough - Policy: remove unused @cache, make roles attr_reader - Policy: resolve_delegation returns early on nil permission (delegated permission to nonexistent parent no longer falls through incorrectly) --- lib/papers_please.rb | 1 - lib/papers_please/callable.rb | 14 ------- lib/papers_please/delegation.rb | 2 +- lib/papers_please/permission.rb | 39 +++++++++--------- lib/papers_please/policy.rb | 52 +++++++++--------------- lib/papers_please/role.rb | 12 +++--- spec/keyword_aliases_spec.rb | 17 +++----- spec/permission_spec.rb | 70 ++++++++++++++++++++------------- 8 files changed, 95 insertions(+), 112 deletions(-) delete mode 100644 lib/papers_please/callable.rb diff --git a/lib/papers_please.rb b/lib/papers_please.rb index a0a9842..86eafa7 100644 --- a/lib/papers_please.rb +++ b/lib/papers_please.rb @@ -2,7 +2,6 @@ require "papers_please/version" require "papers_please/errors" -require "papers_please/callable" require "papers_please/delegation" require "papers_please/policy" require "papers_please/role" diff --git a/lib/papers_please/callable.rb b/lib/papers_please/callable.rb deleted file mode 100644 index eeebef7..0000000 --- a/lib/papers_please/callable.rb +++ /dev/null @@ -1,14 +0,0 @@ -# frozen_string_literal: true - -module PapersPlease - module Callable - def self.call(callable, *args) - if callable.lambda? - arity = callable.arity - callable.call(*((arity >= 0) ? args.take(arity) : args)) - else - callable.call(*args) - end - end - end -end diff --git a/lib/papers_please/delegation.rb b/lib/papers_please/delegation.rb index 2307b7c..3864568 100644 --- a/lib/papers_please/delegation.rb +++ b/lib/papers_please/delegation.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module PapersPlease - Delegation = Struct.new(:klass, :resolver) do + Delegation = Data.define(:klass, :resolver) do def resolve(user, subject) resolver.call(user, subject) end diff --git a/lib/papers_please/permission.rb b/lib/papers_please/permission.rb index ee25e9e..6967c9f 100644 --- a/lib/papers_please/permission.rb +++ b/lib/papers_please/permission.rb @@ -2,8 +2,7 @@ module PapersPlease class Permission - attr_accessor :key, :subject, :collection, :delegation - attr_reader :scope, :predicate + attr_reader :key, :subject, :collection, :delegation, :scope, :predicate alias_method :query, :scope alias_method :collection?, :collection @@ -11,12 +10,12 @@ class Permission def initialize(key, subject, scope: nil, query: nil, predicate: nil, delegation: nil, collection: false) raise ArgumentError, "cannot pass both :scope and :query" if scope && query - self.key = key - self.subject = subject - self.scope = scope || query - self.predicate = predicate - self.delegation = delegation - self.collection = collection + @key = key + @subject = subject + @scope = validate_callable!(scope || query) + @predicate = validate_callable!(predicate) + @delegation = delegation + @collection = collection end def delegated? @@ -35,36 +34,40 @@ def matches?(key, subject) key_matches?(key) && subject_matches?(subject) end - def granted?(*args) - return Callable.call(predicate, *args) if predicate + def matches_subject?(subject) + subject_matches?(subject) + end + + def granted?(user, subject) + return predicate.call(user, subject) if predicate # :nocov: false # :nocov: end - def fetch(*args) - return Callable.call(scope, *args) if scope + def fetch(user, klass) + return scope.call(user, klass) if scope nil end def scope=(val) - validate_callable!(:scope, val) - @scope = val + @scope = validate_callable!(val) end alias_method :query=, :scope= def predicate=(val) - validate_callable!(:predicate, val) - @predicate = val + @predicate = validate_callable!(val) end private - def validate_callable!(name, val) - raise ArgumentError, "#{name} must be a Proc, #{val.class} given" if val && !val.is_a?(Proc) + def validate_callable!(val) + raise ArgumentError, "must be a Proc, #{val.class} given" if val && !val.is_a?(Proc) + + val end def key_matches?(key) diff --git a/lib/papers_please/policy.rb b/lib/papers_please/policy.rb index a58898d..4624680 100644 --- a/lib/papers_please/policy.rb +++ b/lib/papers_please/policy.rb @@ -2,15 +2,12 @@ module PapersPlease class Policy - attr_accessor :roles - - attr_reader :fallthrough, :user + attr_reader :roles, :fallthrough, :user def initialize(user) @user = user @default_scope = nil @roles = {} - @cache = {} configure end @@ -27,7 +24,6 @@ def configure raise NotImplementedError, "The #configure method of the access policy was not implemented" end - # Add a role to the Policy def add_role(name, predicate = nil) name = name.to_sym raise DuplicateRole if roles.key?(name) @@ -39,7 +35,6 @@ def add_role(name, predicate = nil) end alias_method :role, :add_role - # Add permissions to the Role def add_permissions(keys) return unless block_given? @@ -51,19 +46,21 @@ def add_permissions(keys) end alias_method :permit, :add_permissions - # Look up a stored permission block and call with - # the current user and subject def can?(action, subject = nil, roles: nil) + check_subject = subject + roles_to_check(roles: roles).each do |_, role| - permission = role&.find_permission(action, subject) + permission = role&.find_permission(action, check_subject) next if permission.nil? - # Proxy permission check if granted by other - subject, permission = get_proxied_permission(permission, action, subject, role) if permission.granted_by_other? + resolved_subject = check_subject + if permission.delegated? + resolved_subject, permission = resolve_delegation(permission, check_subject, role) + next if permission.nil? + end - # Check permission - granted = permission_granted?(permission, action, subject) - next if granted.nil? || (granted == false && fallthrough) + granted = !!permission.granted?(user, resolved_subject) + next if !granted && fallthrough return granted end @@ -88,9 +85,9 @@ def get_applicable_roles_by_keys(keys) def permitted_actions(subject) applicable_roles.each_with_object(Set.new) do |(_, role), actions| role.permissions.each do |permission| - next unless permission.matches?(permission.key, subject) + next unless permission.matches_subject?(subject) next if actions.include?(permission.key) - next unless permission.granted?(user, subject, permission.key) + next unless permission.granted?(user, subject) actions << permission.key end @@ -100,18 +97,16 @@ def permitted_actions(subject) def roles_that_can(action, subject) applicable_roles.filter do |_, role| permission = role.find_permission(action, subject) - permission&.granted?(user, subject, action) || false + permission&.granted?(user, subject) || false end.keys end - # Look up a stored scope block and call with the - # current user and class def scope_for(action, klass, roles: nil) roles_to_check(roles: roles).each do |_, role| next if role.nil? permission = role.find_permission(action, klass) - scope = permission&.fetch(user, klass, action) + scope = permission&.fetch(user, klass) next if permission.nil? || (scope.nil? && fallthrough) # standard:disable Style/SafeNavigation @@ -122,7 +117,6 @@ def scope_for(action, klass, roles: nil) end alias_method :query, :scope_for - # Fetch roles that apply to the current user def applicable_roles @applicable_roles ||= roles.select do |_, role| role.applies_to?(user) @@ -135,20 +129,12 @@ def roles_to_check(roles: nil) roles.nil? ? applicable_roles : get_applicable_roles_by_keys(roles) end - def permission_granted?(permission, action, subject) - if fallthrough - permission.nil? ? false : permission.granted?(user, subject, action) - else - permission&.granted?(user, subject, action) - end - end - - def get_proxied_permission(permission, action, subject, role) + def resolve_delegation(permission, subject, role) delegation = permission.delegation - subject = subject.is_a?(Class) ? delegation.klass : delegation.resolve(user, subject) - permission = role.find_permission(action, subject) + resolved = subject.is_a?(Class) ? delegation.klass : delegation.resolve(user, subject) + permission = role.find_permission(permission.key, resolved) - [subject, permission] + [resolved, permission] end end end diff --git a/lib/papers_please/role.rb b/lib/papers_please/role.rb index cbea546..945b9c1 100644 --- a/lib/papers_please/role.rb +++ b/lib/papers_please/role.rb @@ -11,7 +11,7 @@ def initialize(name, predicate: nil) end def applies_to?(user) - return Callable.call(@predicate, user) if @predicate.is_a?(Proc) + return @predicate.call(user) if @predicate.is_a?(Proc) true end @@ -69,7 +69,7 @@ def build_delegation(through) raise InvalidGrant, "through must be a [Class, Proc], got #{through.inspect}" unless valid_delegation?(through) - Delegation.new(through[0], through[1]) + Delegation.new(klass: through[0], resolver: through[1]) end def valid_delegation?(tuple) @@ -86,12 +86,12 @@ def make_permission(action, klass, scope: nil, predicate: nil, delegation: nil, permission.predicate = predicate elsif has_scope permission.scope = scope - permission.predicate = collection ? (proc { true }) : build_predicate_from_scope(klass, scope) + permission.predicate = collection ? (proc { |_u, _s| true }) : build_predicate_from_scope(klass, scope) elsif has_predicate permission.predicate = predicate else - permission.scope = (proc { klass.all }) - permission.predicate = (proc { true }) + permission.scope = (proc { |_u, _k| klass.all }) + permission.predicate = (proc { |_u, _s| true }) end permission @@ -99,7 +99,7 @@ def make_permission(action, klass, scope: nil, predicate: nil, delegation: nil, def build_predicate_from_scope(klass, scope) proc do |user, obj| - res = Callable.call(scope, user, klass) + res = scope.call(user, klass) res.respond_to?(:include?) && res.include?(obj) end end diff --git a/spec/keyword_aliases_spec.rb b/spec/keyword_aliases_spec.rb index ed3afa5..f793539 100644 --- a/spec/keyword_aliases_spec.rb +++ b/spec/keyword_aliases_spec.rb @@ -63,25 +63,18 @@ end end - describe "lambda support" do + describe "lambdas work as callables" do let(:role) { PapersPlease::Role.new(:test) } - it "works with single-arg lambda as scope" do - role.grant :read, Post, scope: ->(u) { u.posts } - permission = role.find_permission(:read, post) - - expect(permission.granted?(user, post)).to be true - expect(permission.granted?(user, other_post)).to be false - end - - it "works with two-arg lambda as scope" do + it "works as scope" do role.grant :read, Post, scope: ->(u, _klass) { u.posts } permission = role.find_permission(:read, post) expect(permission.granted?(user, post)).to be true + expect(permission.granted?(user, other_post)).to be false end - it "works with lambda as if" do + it "works as if" do role.grant :read, Post, if: ->(u, p) { u.posts.include?(p) } permission = role.find_permission(:read, post) @@ -89,7 +82,7 @@ expect(permission.granted?(user, other_post)).to be false end - it "works with lambda as role predicate" do + it "works as role predicate" do lambda_role = PapersPlease::Role.new(:test, predicate: ->(u) { u.admin? }) expect(lambda_role.applies_to?(user)).to be true diff --git a/spec/permission_spec.rb b/spec/permission_spec.rb index b28873a..6e77db0 100644 --- a/spec/permission_spec.rb +++ b/spec/permission_spec.rb @@ -6,6 +6,7 @@ subject { described_class } let(:book) { double("Book") } + let(:user) { double("User") } it "requires a key and subject" do expect { subject.new }.to raise_error(ArgumentError) @@ -26,6 +27,15 @@ expect(perm.query).to be stub_proc end + it "has immutable key, subject, collection, and delegation" do + perm = subject.new(:key, book) + + expect(perm).not_to respond_to(:key=) + expect(perm).not_to respond_to(:subject=) + expect(perm).not_to respond_to(:collection=) + expect(perm).not_to respond_to(:delegation=) + end + describe "#matches?" do let(:permission) { subject.new(:read, book) } @@ -38,59 +48,65 @@ end end + describe "#matches_subject?" do + let(:permission) { subject.new(:read, book) } + + it "matches the subject" do + expect(permission.matches_subject?(book)).to be true + end + end + describe "#granted?" do - let(:true_permission) { subject.new(:read, book, predicate: proc { true }) } - let(:false_permission) { subject.new(:read, book, predicate: proc { false }) } - let(:stub_proc) { proc { true } } - let(:stub_perm) { subject.new(:read, book, predicate: stub_proc) } + let(:true_permission) { subject.new(:read, book, predicate: proc { |_u, _s| true }) } + let(:false_permission) { subject.new(:read, book, predicate: proc { |_u, _s| false }) } it "is true for true permission" do - expect(true_permission.granted?).to be true + expect(true_permission.granted?(user, book)).to be true end it "is false for false permission" do - expect(false_permission.granted?).to be false + expect(false_permission.granted?(user, book)).to be false end - it "calls the predicate proc" do - expect(stub_proc).to receive(:call).and_return(true) - expect(stub_perm.granted?).to be true - end + it "passes user and subject to predicate" do + called_with = nil + perm = subject.new(:read, book, predicate: proc { |u, s| + called_with = [u, s] + true + }) + perm.granted?(user, book) - it "calls the predicate proc" do - expect(stub_proc).to receive(:call).with(:arg).and_return(true) - expect(stub_perm.granted?(:arg)).to be true + expect(called_with).to eq [user, book] end end describe "#fetch" do - let(:array_permission) { subject.new(:read, book, query: proc { [] }) } - let(:empty_permission) { subject.new(:read, book, query: proc {}) } - let(:stub_proc) { proc { [] } } - let(:stub_perm) { subject.new(:read, book, query: stub_proc) } + let(:array_permission) { subject.new(:read, book, query: proc { |_u, _k| [] }) } + let(:empty_permission) { subject.new(:read, book, query: proc { |_u, _k| }) } it "is array for array permission" do - expect(array_permission.fetch).to eq [] + expect(array_permission.fetch(user, book)).to eq [] end it "is nil for nil permission" do - expect(empty_permission.fetch).to be_nil + expect(empty_permission.fetch(user, book)).to be_nil end - it "calls the scope proc" do - expect(stub_proc).to receive(:call).and_return([]) - expect(stub_perm.fetch).to eq [] - end + it "passes user and klass to scope" do + called_with = nil + perm = subject.new(:read, book, query: proc { |u, k| + called_with = [u, k] + [] + }) + perm.fetch(user, book) - it "calls the scope proc with args" do - expect(stub_proc).to receive(:call).with(:arg).and_return(nil) - expect(stub_perm.fetch(:arg)).to be_nil + expect(called_with).to eq [user, book] end end describe "#delegated?" do it "is true with a delegation" do - delegation = PapersPlease::Delegation.new(Post, proc { |_u, a| a }) + delegation = PapersPlease::Delegation.new(klass: Post, resolver: proc { |_u, a| a }) perm = subject.new(:read, book, delegation: delegation) expect(perm.delegated?).to be true From bf3e67b743c909b7757553cdc1b88faff9235810 Mon Sep 17 00:00:00 2001 From: Wyatt Kirby Date: Fri, 29 May 2026 15:55:50 -0400 Subject: [PATCH 09/12] fix: harden authorization checks --- lib/papers_please/permission.rb | 10 ++++++- lib/papers_please/policy.rb | 14 ++++++++-- lib/papers_please/role.rb | 31 +++++++++++++++++----- spec/collection_spec.rb | 2 +- spec/fixtures/access_policy.rb | 8 +++--- spec/permission_spec.rb | 27 +++++++++++++++++++ spec/policy_spec.rb | 27 +++++++++++++++---- spec/role_spec.rb | 47 ++++++++++++++++++++++++++++----- 8 files changed, 140 insertions(+), 26 deletions(-) diff --git a/lib/papers_please/permission.rb b/lib/papers_please/permission.rb index 6967c9f..71b5ac2 100644 --- a/lib/papers_please/permission.rb +++ b/lib/papers_please/permission.rb @@ -75,7 +75,15 @@ def key_matches?(key) end def subject_matches?(subject) - subject == @subject || subject.class <= @subject + if subject.is_a?(Class) + @subject.is_a?(Class) && !!(subject <= @subject) + else + # Keep literal subject matching, but put the trusted policy subject on + # the left side so an arbitrary runtime object cannot spoof a match by + # overriding #==. Class-backed permissions still match instances by + # checking the instance's class against the configured subject class. + @subject == subject || (@subject.is_a?(Class) && !!(subject.class <= @subject)) + end end end end diff --git a/lib/papers_please/policy.rb b/lib/papers_please/policy.rb index 4624680..e28b261 100644 --- a/lib/papers_please/policy.rb +++ b/lib/papers_please/policy.rb @@ -73,9 +73,9 @@ def cannot?(*args) end def authorize!(action, subject) - raise AccessDenied, "Access denied for #{action} on #{subject}" if cannot?(action, subject) + return subject if can?(action, subject) - subject + raise AccessDenied, "Access denied for #{action.inspect} on #{subject_label(subject)}" end def get_applicable_roles_by_keys(keys) @@ -129,6 +129,16 @@ def roles_to_check(roles: nil) roles.nil? ? applicable_roles : get_applicable_roles_by_keys(roles) end + def subject_label(subject) + klass = Object.instance_method(:class).bind_call(subject) + + if klass == Class + subject.name || "anonymous class" + else + klass.name || "anonymous #{klass}" + end + end + def resolve_delegation(permission, subject, role) delegation = permission.delegation resolved = subject.is_a?(Class) ? delegation.klass : delegation.resolve(user, subject) diff --git a/lib/papers_please/role.rb b/lib/papers_please/role.rb index 945b9c1..d6b5136 100644 --- a/lib/papers_please/role.rb +++ b/lib/papers_please/role.rb @@ -17,11 +17,13 @@ def applies_to?(user) end def add_permission(actions, klass, **opts) - scope, predicate, through, collection = resolve_opts(opts) + scope, predicate, through, collection, allow_all = resolve_opts(opts) Array(actions).each do |action| raise DuplicatePermission if permission_exists?(action, klass) + warn_unconditional_grant(action, klass) if scope.nil? && predicate.nil? && through.nil? && !allow_all + delegation = build_delegation(through) permissions << make_permission( @@ -49,8 +51,9 @@ def resolve_opts(opts) predicate = resolve_alias(opts, :if, :predicate) through = resolve_alias(opts, :through, :granted_by) collection = opts.fetch(:collection, false) + allow_all = opts.fetch(:allow_all, false) - [scope, predicate, through, collection] + [scope, predicate, through, collection, allow_all] end def resolve_alias(opts, new_name, old_name) @@ -76,9 +79,14 @@ def valid_delegation?(tuple) tuple.is_a?(Array) && tuple.length == 2 && tuple[0].is_a?(Class) && tuple[1].is_a?(Proc) end + def warn_unconditional_grant(action, klass) + warn "[papers_please] role.grant #{action.inspect}, #{klass.inspect} grants unconditional access. " \ + "Pass if:/scope: or allow_all: true to make this explicit." + end + def make_permission(action, klass, scope: nil, predicate: nil, delegation: nil, collection: false) - has_scope = scope.is_a?(Proc) - has_predicate = predicate.is_a?(Proc) + has_scope = !scope.nil? + has_predicate = !predicate.nil? permission = Permission.new(action, klass, delegation: delegation, collection: collection) if has_scope && has_predicate @@ -99,8 +107,19 @@ def make_permission(action, klass, scope: nil, predicate: nil, delegation: nil, def build_predicate_from_scope(klass, scope) proc do |user, obj| - res = scope.call(user, klass) - res.respond_to?(:include?) && res.include?(obj) + if klass.is_a?(Class) && obj.is_a?(klass) + res = scope.call(user, klass) + + if res.respond_to?(:exists?) && obj.respond_to?(:id) + res.exists?(obj.id) + elsif res.is_a?(Array) + res.include?(obj) + else + false + end + else + false + end end end end diff --git a/spec/collection_spec.rb b/spec/collection_spec.rb index 4cb3f46..00084d1 100644 --- a/spec/collection_spec.rb +++ b/spec/collection_spec.rb @@ -33,7 +33,7 @@ end it "works without a scope" do - role.grant :create, Post, collection: true + role.grant :create, Post, collection: true, allow_all: true permission = role.find_permission(:create, Post) expect(permission.collection?).to be true diff --git a/spec/fixtures/access_policy.rb b/spec/fixtures/access_policy.rb index 61a1fd6..fd241c6 100644 --- a/spec/fixtures/access_policy.rb +++ b/spec/fixtures/access_policy.rb @@ -7,18 +7,18 @@ def configure role :member, proc { |user| user.member? } permit :admin do |role| - role.grant %i[create read update destroy], Post - role.grant %i[create read update destroy], Attachment + role.grant %i[create read update destroy], Post, allow_all: true + role.grant %i[create read update destroy], Attachment, allow_all: true end permit :manager do |role| - role.grant :create, Post, collection: true + role.grant :create, Post, collection: true, allow_all: true role.grant %i[read update destroy], Post, query: proc { |u| u.posts } role.grant %i[create read update destroy], Attachment, granted_by: [Post, proc { |_u, a| a.post }] end permit :member do |role| - role.grant :create, Post, collection: true + role.grant :create, Post, collection: true, allow_all: true role.grant %i[read update], Post, query: proc { |u| u.posts } role.grant %i[create read update], Attachment, granted_by: [Post, proc { |_u, a| a.post }] end diff --git a/spec/permission_spec.rb b/spec/permission_spec.rb index 6e77db0..dbcf559 100644 --- a/spec/permission_spec.rb +++ b/spec/permission_spec.rb @@ -54,6 +54,33 @@ it "matches the subject" do expect(permission.matches_subject?(book)).to be true end + + it "does not let runtime subjects spoof equality" do + post_permission = subject.new(:read, Post) + evil = Class.new do + def ==(_other) + true + end + end.new + + expect(post_permission.matches_subject?(evil)).to be false + end + + it "matches class subjects and subclasses" do + subclass = Class.new(Post) + post_permission = subject.new(:read, Post) + + expect(post_permission.matches_subject?(Post)).to be true + expect(post_permission.matches_subject?(subclass)).to be true + expect(post_permission.matches_subject?(subclass.new)).to be true + end + + it "matches literal subjects" do + literal_permission = subject.new(:read, :dashboard) + + expect(literal_permission.matches_subject?(:dashboard)).to be true + expect(literal_permission.matches_subject?(:settings)).to be false + end end describe "#granted?" do diff --git a/spec/policy_spec.rb b/spec/policy_spec.rb index 55c5b40..48ca17d 100644 --- a/spec/policy_spec.rb +++ b/spec/policy_spec.rb @@ -157,13 +157,13 @@ def configure role :admin permit :admin do |role| - role.grant :admin, Post - role.grant :foo, Post + role.grant :admin, Post, allow_all: true + role.grant :foo, Post, allow_all: true end permit :member do |role| - role.grant :read, Post - role.grant :foo, Post + role.grant :read, Post, allow_all: true + role.grant :foo, Post, allow_all: true end end end @@ -214,6 +214,23 @@ def configure policy = sample_policy(member) expect { policy.authorize! :read, posts.last }.to raise_error(PapersPlease::AccessDenied) end + + it "does not call subject.to_s when building denial messages" do + policy_klass = Class.new(PapersPlease::Policy) do + def configure + role :member + end + end + evil = Class.new do + def to_s + raise "to_s should not be called" + end + end.new + + expect do + policy_klass.new(member).authorize!(:read, evil) + end.to raise_error(PapersPlease::AccessDenied, /Access denied for :read on/) + end end private @@ -225,7 +242,7 @@ def configure role :member, proc { |u| u.member } permit :admin do |role| - role.grant :read, Post + role.grant :read, Post, allow_all: true end permit :member do |role| diff --git a/spec/role_spec.rb b/spec/role_spec.rb index 909852b..f77de58 100644 --- a/spec/role_spec.rb +++ b/spec/role_spec.rb @@ -34,7 +34,7 @@ let(:role) { described_class.new(:admin) } it "does not expand :manage" do - role.add_permission(:manage, Post) + role.add_permission(:manage, Post, allow_all: true) expect(role.permissions.size).to eq 1 expect(role.permissions.first.key).to eq(:manage) end @@ -44,8 +44,8 @@ let(:role) { described_class.new(:admin) } it "raises error for duplicates" do - role.add_permission(:read, Post) - expect { role.add_permission(:read, Post) }.to raise_error(PapersPlease::DuplicatePermission) + role.add_permission(:read, Post, allow_all: true) + expect { role.add_permission(:read, Post, allow_all: true) }.to raise_error(PapersPlease::DuplicatePermission) end end @@ -88,6 +88,11 @@ let(:query) { proc { [] } } let(:role) { described_class.new(:admin) } + it "raises for non-callable query or predicate values" do + expect { role.add_permission(:read, Post, query: "invalid") }.to raise_error(ArgumentError) + expect { role.add_permission(:read, Post, predicate: "invalid") }.to raise_error(ArgumentError) + end + it "has correct query" do role.add_permission(:read, Post, query: query, predicate: predicate) expect(role.permissions.size).to eq 1 @@ -132,6 +137,28 @@ expect(admin_role.permissions.size).to eq 1 expect(admin_role.permissions.first.predicate.call(admin_user, Post)).to be false end + + it "does not trust arbitrary scope objects with include?" do + evil_scope = Class.new do + def include?(_obj) + true + end + end.new + + admin_role.add_permission(:read, Post, query: proc { evil_scope }) + + expect(admin_role.permissions.first.predicate.call(admin_user, included_post)).to be false + end + + it "uses exists? for ActiveRecord-like scopes" do + relation = double("Relation") + allow(relation).to receive(:exists?).with(123).and_return(true) + allow(included_post).to receive(:id).and_return(123) + + admin_role.add_permission(:read, Post, query: proc { relation }) + + expect(admin_role.permissions.first.predicate.call(admin_user, included_post)).to be true + end end context "only predicate" do @@ -155,16 +182,22 @@ let(:empty_role) { described_class.new(:admin) } let(:klass) { spy("klass") } - it "creates an all query" do - empty_role.add_permission(:read, klass) + it "warns about implicit unconditional grants" do + expect do + empty_role.add_permission(:read, klass) + end.to output(/grants unconditional access/).to_stderr + end + + it "creates an all query when explicitly allowed" do + empty_role.add_permission(:read, klass, allow_all: true) expect(empty_role.permissions.size).to eq 1 empty_role.permissions.first.query.call expect(klass).to have_received(:all) end - it "creates a true predicate" do - empty_role.add_permission(:read, klass) + it "creates a true predicate when explicitly allowed" do + empty_role.add_permission(:read, klass, allow_all: true) expect(empty_role.permissions.first.predicate.call).to be true end end From 593eebe3ec7002a88586d2610ba45a7b53bae1be Mon Sep 17 00:00:00 2001 From: Wyatt Kirby Date: Fri, 29 May 2026 16:16:30 -0400 Subject: [PATCH 10/12] fix: harden policy and rails integration --- README.md | 33 ++++++++ lib/papers_please.rb | 19 +++++ lib/papers_please/policy.rb | 26 ++++-- lib/papers_please/rails/controller_methods.rb | 16 +++- lib/papers_please/railtie.rb | 15 +--- lib/papers_please/role.rb | 49 +++++++---- spec/policy_spec.rb | 47 +++++++++++ spec/rails_controller_methods_spec.rb | 82 +++++++++++++++++++ spec/role_spec.rb | 11 ++- 9 files changed, 256 insertions(+), 42 deletions(-) create mode 100644 spec/rails_controller_methods_spec.rb diff --git a/README.md b/README.md index 81411a0..3ad7ed1 100644 --- a/README.md +++ b/README.md @@ -170,6 +170,39 @@ role.grant :show, Attachment, through: [Post, proc { |_u, attachment| attachment This means: "to check if a user can `:show` an `Attachment`, find its `Post` and check whether the user can `:show` that `Post` instead." +Delegation resolves within the role that declared it. If `:member` delegates `Attachment` access through `Post`, the `:member` role must also define the matching `Post` permission; `papers_please` will not search other applicable roles for that parent permission. + +## Explicit Unconditional Grants + +A grant with no `scope:`, `if:`, or `through:` is an unconditional allow. Make that intent explicit with `allow_all: true`: + +```ruby +role.grant :create, Post, collection: true, allow_all: true +role.grant %i[index show], Post, allow_all: true +``` + +Omitting `allow_all: true` still works for compatibility, but emits a warning. + +## Scope Predicate Behavior + +When an instance-level grant has a `scope:` but no `if:`, `papers_please` generates a predicate from the scope. The generated predicate supports `Array`, `Set`, and ActiveRecord-like objects that respond to `exists?`. It intentionally does not trust arbitrary objects that merely define `include?`. + +For hot paths or complex data sources, prefer an explicit `if:` predicate. Policy instances are intended to be created per user/request; do not reuse one policy object across users or after mutating a user's roles. + +## Rails Integration + +Rails controllers get `can?`, `cannot?`, and `authorize!` methods. Views get `can?` and `cannot?` helpers. + +By default, Rails integration looks for `AccessPolicy`. You can configure a different policy class: + +```ruby +PapersPlease.policy_class = MyPolicy +# or +PapersPlease.policy_class_name = "MyPolicy" +``` + +Do not pass request parameters to the advanced `roles:` option of `can?`; it is intended for trusted internal checks only. + ## A Helpful CLI ```bash diff --git a/lib/papers_please.rb b/lib/papers_please.rb index 86eafa7..8bdee50 100644 --- a/lib/papers_please.rb +++ b/lib/papers_please.rb @@ -10,6 +10,25 @@ require "papers_please/railtie" if defined? Rails module PapersPlease + class << self + attr_writer :policy_class + + def policy_class + @policy_class ||= Object.const_get(policy_class_name) + rescue NameError + raise Error, "PapersPlease.policy_class is not configured and #{policy_class_name} is not defined" + end + + def policy_class_name + @policy_class_name ||= "AccessPolicy" + end + + def policy_class_name=(value) + @policy_class = nil + @policy_class_name = value + end + end + # rubocop:disable Metrics/PerceivedComplexity, Metrics/MethodLength def self.permissions_table(policy_klass) require "terminal-table" diff --git a/lib/papers_please/policy.rb b/lib/papers_please/policy.rb index e28b261..a9fdf31 100644 --- a/lib/papers_please/policy.rb +++ b/lib/papers_please/policy.rb @@ -53,13 +53,7 @@ def can?(action, subject = nil, roles: nil) permission = role&.find_permission(action, check_subject) next if permission.nil? - resolved_subject = check_subject - if permission.delegated? - resolved_subject, permission = resolve_delegation(permission, check_subject, role) - next if permission.nil? - end - - granted = !!permission.granted?(user, resolved_subject) + granted = permission_granted_for_role?(role, permission, check_subject) next if !granted && fallthrough return granted @@ -87,7 +81,7 @@ def permitted_actions(subject) role.permissions.each do |permission| next unless permission.matches_subject?(subject) next if actions.include?(permission.key) - next unless permission.granted?(user, subject) + next unless permission_granted_for_role?(role, permission, subject) actions << permission.key end @@ -97,7 +91,7 @@ def permitted_actions(subject) def roles_that_can(action, subject) applicable_roles.filter do |_, role| permission = role.find_permission(action, subject) - permission&.granted?(user, subject) || false + permission && permission_granted_for_role?(role, permission, subject) end.keys end @@ -139,9 +133,23 @@ def subject_label(subject) end end + def permission_granted_for_role?(role, permission, subject) + resolved_subject = subject + + if permission.delegated? + resolved_subject, permission = resolve_delegation(permission, subject, role) + return false if permission.nil? + end + + !!permission.granted?(user, resolved_subject) + end + def resolve_delegation(permission, subject, role) delegation = permission.delegation resolved = subject.is_a?(Class) ? delegation.klass : delegation.resolve(user, subject) + # Delegation is intentionally same-role only: a delegated grant is an + # indirection to another permission on the role that declared it, not a + # search across every applicable role. permission = role.find_permission(permission.key, resolved) [resolved, permission] diff --git a/lib/papers_please/rails/controller_methods.rb b/lib/papers_please/rails/controller_methods.rb index b58eda2..e272617 100644 --- a/lib/papers_please/rails/controller_methods.rb +++ b/lib/papers_please/rails/controller_methods.rb @@ -2,13 +2,25 @@ module PapersPlease module Rails + def self.install_controller_methods(controller) + collisions = ControllerMethods.instance_methods(false) & controller.instance_methods + warn_method_collisions(controller, collisions) unless collisions.empty? + + controller.include ControllerMethods unless controller < ControllerMethods + end + + def self.warn_method_collisions(controller, collisions) + warn "[papers_please] #{controller} already defines #{collisions.map(&:inspect).join(", ")}; " \ + "including controller helpers may change method lookup." + end + module ControllerMethods def self.included(base) - base.helper_method :can?, :cannot?, :policy if base.respond_to? :helper_method + base.helper_method :can?, :cannot? if base.respond_to? :helper_method end def policy - @policy ||= ::PapersPlease::Policy.new(current_user) + @policy ||= ::PapersPlease.policy_class.new(current_user) end def can?(*args) diff --git a/lib/papers_please/railtie.rb b/lib/papers_please/railtie.rb index d6e6dad..6c67921 100644 --- a/lib/papers_please/railtie.rb +++ b/lib/papers_please/railtie.rb @@ -8,18 +8,9 @@ class Railtie < ::Rails::Railtie Dir[File.join(File.dirname(__FILE__), "tasks", "*.rake")].each { |f| load f } end - initializer :papers_plesae do - if defined? ActionController::Base - ActionController::Base.class_eval do - include PapersPlease::Rails::ControllerMethods - end - end - - if defined? ActionController::API - ActionController::API.class_eval do - include PapersPlease::Rails::ControllerMethods - end - end + initializer :papers_please do + PapersPlease::Rails.install_controller_methods(ActionController::Base) if defined? ActionController::Base + PapersPlease::Rails.install_controller_methods(ActionController::API) if defined? ActionController::API end end end diff --git a/lib/papers_please/role.rb b/lib/papers_please/role.rb index d6b5136..3938fc5 100644 --- a/lib/papers_please/role.rb +++ b/lib/papers_please/role.rb @@ -22,13 +22,18 @@ def add_permission(actions, klass, **opts) Array(actions).each do |action| raise DuplicatePermission if permission_exists?(action, klass) - warn_unconditional_grant(action, klass) if scope.nil? && predicate.nil? && through.nil? && !allow_all + implicit_allow_all = scope.nil? && predicate.nil? && through.nil? + warn_unconditional_grant(action, klass) if implicit_allow_all && !allow_all delegation = build_delegation(through) permissions << make_permission( action, klass, - scope: scope, predicate: predicate, delegation: delegation, collection: collection + scope: scope, + predicate: predicate, + delegation: delegation, + collection: collection, + allow_all: allow_all || implicit_allow_all ) end end @@ -84,7 +89,7 @@ def warn_unconditional_grant(action, klass) "Pass if:/scope: or allow_all: true to make this explicit." end - def make_permission(action, klass, scope: nil, predicate: nil, delegation: nil, collection: false) + def make_permission(action, klass, scope: nil, predicate: nil, delegation: nil, collection: false, allow_all: false) has_scope = !scope.nil? has_predicate = !predicate.nil? permission = Permission.new(action, klass, delegation: delegation, collection: collection) @@ -97,29 +102,37 @@ def make_permission(action, klass, scope: nil, predicate: nil, delegation: nil, permission.predicate = collection ? (proc { |_u, _s| true }) : build_predicate_from_scope(klass, scope) elsif has_predicate permission.predicate = predicate - else - permission.scope = (proc { |_u, _k| klass.all }) + elsif allow_all + permission.scope = build_unconditional_scope(klass) permission.predicate = (proc { |_u, _s| true }) end permission end + def build_unconditional_scope(klass) + return proc { |_u, _k| klass.all } if klass.respond_to?(:all) + + nil + end + def build_predicate_from_scope(klass, scope) proc do |user, obj| - if klass.is_a?(Class) && obj.is_a?(klass) - res = scope.call(user, klass) - - if res.respond_to?(:exists?) && obj.respond_to?(:id) - res.exists?(obj.id) - elsif res.is_a?(Array) - res.include?(obj) - else - false - end - else - false - end + class_backed_instance?(klass, obj) && included_in_scope?(scope.call(user, klass), obj) + end + end + + def class_backed_instance?(klass, obj) + klass.is_a?(Class) && !!(Object.instance_method(:class).bind_call(obj) <= klass) + end + + def included_in_scope?(scope_result, obj) + if scope_result.respond_to?(:exists?) && obj.respond_to?(:id) + scope_result.exists?(obj.id) + elsif scope_result.is_a?(Array) || scope_result.is_a?(Set) + scope_result.include?(obj) + else + false end end end diff --git a/spec/policy_spec.rb b/spec/policy_spec.rb index 48ca17d..24aa5cb 100644 --- a/spec/policy_spec.rb +++ b/spec/policy_spec.rb @@ -149,6 +149,53 @@ def configure end end + describe "delegated permissions" do + let(:attachment) { Attachment.new(post: post) } + + it "resolves delegated permissions through parent permissions on the same role" do + policy_klass = Class.new(PapersPlease::Policy) do + def configure + role :member + + permit :member do |role| + role.grant :read, Post, if: proc { true } + role.grant :read, Attachment, through: [Post, proc { |_u, attachment| attachment.post }] + end + end + end + + policy = policy_klass.new(member) + + expect(policy.can?(:read, attachment)).to be true + expect(policy.permitted_actions(attachment)).to contain_exactly(:read) + expect(policy.roles_that_can(:read, attachment)).to contain_exactly(:member) + end + + it "does not resolve delegated permissions through a different applicable role" do + policy_klass = Class.new(PapersPlease::Policy) do + def configure + allow_fallthrough + role :attachment_reader + role :post_reader + + permit :attachment_reader do |role| + role.grant :read, Attachment, through: [Post, proc { |_u, attachment| attachment.post }] + end + + permit :post_reader do |role| + role.grant :read, Post, if: proc { true } + end + end + end + + policy = policy_klass.new(member) + + expect(policy.can?(:read, attachment)).to be false + expect(policy.permitted_actions(attachment)).to be_empty + expect(policy.roles_that_can(:read, attachment)).to be_empty + end + end + describe "#role_that_can" do it "returns the symbolic name of the roles that can perform an action" do policy_klass = Class.new(PapersPlease::Policy) do diff --git a/spec/rails_controller_methods_spec.rb b/spec/rails_controller_methods_spec.rb new file mode 100644 index 0000000..fd8aee4 --- /dev/null +++ b/spec/rails_controller_methods_spec.rb @@ -0,0 +1,82 @@ +# frozen_string_literal: true + +require "spec_helper" + +RSpec.describe PapersPlease::Rails::ControllerMethods do + around do |example| + original_policy_class = PapersPlease.instance_variable_get(:@policy_class) + original_policy_class_name = PapersPlease.instance_variable_get(:@policy_class_name) + + example.run + ensure + PapersPlease.instance_variable_set(:@policy_class, original_policy_class) + PapersPlease.instance_variable_set(:@policy_class_name, original_policy_class_name) + end + + it "exposes can? and cannot? as helper methods without exposing policy" do + controller_class = Class.new do + class << self + attr_reader :helpers + + def helper_method(*names) + @helpers = names + end + end + + include PapersPlease::Rails::ControllerMethods + end + + expect(controller_class.helpers).to contain_exactly(:can?, :cannot?) + end + + it "warns when installing into a controller with method collisions" do + controller_class = Class.new do + def can?(*) + end + end + + expect do + PapersPlease::Rails.install_controller_methods(controller_class) + end.to output(/already defines :can\?/).to_stderr + expect(controller_class < PapersPlease::Rails::ControllerMethods).to be true + end + + it "uses the configured policy class" do + policy_class = Class.new(PapersPlease::Policy) do + def configure + role :member + + permit :member do |role| + role.grant :read, Post, if: proc { true } + end + end + end + PapersPlease.policy_class = policy_class + + controller = Class.new do + include PapersPlease::Rails::ControllerMethods + + def current_user + :user + end + end.new + + expect(controller.policy).to be_a(policy_class) + expect(controller.can?(:read, Post.new)).to be true + expect(controller.cannot?(:read, :dashboard)).to be true + end + + it "raises a clear error when no policy class is configured" do + PapersPlease.policy_class_name = "MissingAccessPolicy" + + controller = Class.new do + include PapersPlease::Rails::ControllerMethods + + def current_user + :user + end + end.new + + expect { controller.policy }.to raise_error(PapersPlease::Error, /policy_class is not configured/) + end +end diff --git a/spec/role_spec.rb b/spec/role_spec.rb index f77de58..78febc1 100644 --- a/spec/role_spec.rb +++ b/spec/role_spec.rb @@ -73,13 +73,15 @@ expect { role.add_permission(:read, Attachment, granted_by: [granting_class, resolver]) }.not_to raise_error end - it "adds a delegated permission" do + it "adds a delegated permission without synthesizing standalone allow-all checks" do role.add_permission(:read, Attachment, granted_by: [granting_class, resolver]) expect(role.permissions.size).to eq 1 permission = role.permissions.first expect(permission.delegated?).to be true expect(permission.delegation.resolver).to eq resolver expect(permission.granting_class).to eq granting_class + expect(permission.query).to be_nil + expect(permission.predicate).to be_nil end end @@ -159,6 +161,13 @@ def include?(_obj) expect(admin_role.permissions.first.predicate.call(admin_user, included_post)).to be true end + + it "supports Set scopes" do + admin_role.add_permission(:read, Post, query: proc { Set[included_post] }) + + expect(admin_role.permissions.first.predicate.call(admin_user, included_post)).to be true + expect(admin_role.permissions.first.predicate.call(admin_user, excluded_post)).to be false + end end context "only predicate" do From f645654a752af267e89a9d02df17a2d5f8eb85cd Mon Sep 17 00:00:00 2001 From: Wyatt Kirby Date: Fri, 29 May 2026 16:45:47 -0400 Subject: [PATCH 11/12] docs: document papers_please roles task --- README.md | 25 ++++++++++++++++++++-- lib/papers_please/tasks/papers_please.rake | 8 +++---- 2 files changed, 27 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 3ad7ed1..8d6d399 100644 --- a/README.md +++ b/README.md @@ -203,13 +203,34 @@ PapersPlease.policy_class_name = "MyPolicy" Do not pass request parameters to the advanced `roles:` option of `can?`; it is intended for trusted internal checks only. -## A Helpful CLI +## Rake Tasks + +`papers_please` ships with a Rails rake task for inspecting policy configuration: ```bash $ rails papers_please:roles ``` -Prints a table of all roles, subjects, and permissions in your policy. +The task loads the Rails environment, instantiates the configured policy class, and prints a `terminal-table` report of every role and permission in match order. By default it uses `PapersPlease.policy_class`, which resolves to `AccessPolicy` unless you configure a different policy class. + +To inspect a specific policy class, pass its constant name as the task argument: + +```bash +$ rails 'papers_please:roles[AdminPolicy]' +``` + +The table includes: + +| Column | Meaning | +|---|---| +| `role` | Role name, shown once per grouped role section | +| `subject` | Resource class or subject the permission applies to | +| `permission` | Action key granted by the role | +| `has query?` | Whether the grant has a `scope:`/legacy `query:` | +| `has predicate?` | Whether the grant has an `if:`/legacy `predicate:` | +| `granted by other?` | Whether the grant delegates through another subject via `through:`/legacy `granted_by:` | + +This is useful when reviewing role order, finding unconditional grants, or confirming that delegated and scoped permissions were registered as expected. ## Installation diff --git a/lib/papers_please/tasks/papers_please.rake b/lib/papers_please/tasks/papers_please.rake index cdb6895..fb79019 100644 --- a/lib/papers_please/tasks/papers_please.rake +++ b/lib/papers_please/tasks/papers_please.rake @@ -2,10 +2,10 @@ namespace :papers_please do desc "Print out all defined roles and permissions in match order" - task :roles, [:klass] => :environment do |_, _args| - klass = klass ? Object.const_get(klass) : AccessPolicy + task :roles, [:klass] => :environment do |_, args| + policy_klass = args[:klass] ? Object.const_get(args[:klass]) : PapersPlease.policy_class - puts "Generating Role/Permission Table for #{klass}...\n\n" - PapersPlease.permissions_table(klass) + puts "Generating Role/Permission Table for #{policy_klass}...\n\n" + PapersPlease.permissions_table(policy_klass) end end From 58f00c37d1ead48bd0298e2e621013ac28572020 Mon Sep 17 00:00:00 2001 From: Wyatt Kirby Date: Fri, 29 May 2026 17:06:53 -0400 Subject: [PATCH 12/12] feat: add first-class role support --- README.md | 42 ++++++++++++++++++++++ lib/papers_please/policy.rb | 17 +++++++-- lib/papers_please/role.rb | 11 ++++-- spec/policy_spec.rb | 72 +++++++++++++++++++++++++++++++++++++ 4 files changed, 138 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 8d6d399..af03666 100644 --- a/README.md +++ b/README.md @@ -53,6 +53,48 @@ class AccessPolicy < PapersPlease::Policy end ``` +### Role Classes + +For larger policies, extract a role into a class. Register it explicitly with its policy name and, optionally, its applicability predicate: + +```ruby +class AdminRole < PapersPlease::Role + def configure + grant %i[index new create], Post, collection: true, allow_all: true + grant %i[show edit update destroy], Post, allow_all: true + end +end + +class AccessPolicy < PapersPlease::Policy + def configure + role :admin, AdminRole, proc { |u| u.admin? } + role :member + + permit :member do |role| + role.grant :index, Post, collection: true, scope: proc { |u| u.posts } + role.grant %i[show edit update], Post, scope: proc { |u| u.posts } + end + end +end +``` + +Role classes are instantiated for each policy instance; they are not singletons. A role class can use `user` and `policy` readers if configuration needs request-local context, though most policies should prefer predicates and scopes that receive the user at authorization time. + +```ruby +class MemberRole < PapersPlease::Role + def configure + grant :index, Post, collection: true, scope: proc { |u| u.posts } + grant %i[show edit update], Post, scope: proc { |u| u.posts } + end +end + +class AccessPolicy < PapersPlease::Policy + def configure + role :member, MemberRole, proc { |u| u.member? } + end +end +``` + ### Controller Usage ```ruby diff --git a/lib/papers_please/policy.rb b/lib/papers_please/policy.rb index a9fdf31..b829efe 100644 --- a/lib/papers_please/policy.rb +++ b/lib/papers_please/policy.rb @@ -24,11 +24,12 @@ def configure raise NotImplementedError, "The #configure method of the access policy was not implemented" end - def add_role(name, predicate = nil) + def add_role(name, klass_or_predicate = nil, predicate = nil) + role_class, resolved_predicate = resolve_role_class_and_predicate(klass_or_predicate, predicate) name = name.to_sym raise DuplicateRole if roles.key?(name) - role = Role.new(name, predicate: predicate) + role = role_class.new(name, predicate: resolved_predicate, user: user, policy: self) roles[name] = role role @@ -123,6 +124,18 @@ def roles_to_check(roles: nil) roles.nil? ? applicable_roles : get_applicable_roles_by_keys(roles) end + def resolve_role_class_and_predicate(klass_or_predicate, predicate) + if klass_or_predicate.is_a?(Class) + unless klass_or_predicate <= Role + raise ArgumentError, "role class must inherit from PapersPlease::Role" + end + + [klass_or_predicate, predicate] + else + [Role, klass_or_predicate] + end + end + def subject_label(subject) klass = Object.instance_method(:class).bind_call(subject) diff --git a/lib/papers_please/role.rb b/lib/papers_please/role.rb index 3938fc5..5faa97b 100644 --- a/lib/papers_please/role.rb +++ b/lib/papers_please/role.rb @@ -2,12 +2,19 @@ module PapersPlease class Role - attr_reader :name, :predicate, :permissions + attr_reader :name, :predicate, :permissions, :user, :policy - def initialize(name, predicate: nil) + def initialize(name, predicate: nil, user: nil, policy: nil) @name = name @predicate = predicate + @user = user + @policy = policy @permissions = [] + + configure + end + + def configure end def applies_to?(user) diff --git a/spec/policy_spec.rb b/spec/policy_spec.rb index 24aa5cb..16c8994 100644 --- a/spec/policy_spec.rb +++ b/spec/policy_spec.rb @@ -88,6 +88,78 @@ def configure end end + describe "#role" do + it "accepts an explicit role class and configures it" do + admin_role_class = Class.new(PapersPlease::Role) do + def configure + grant :read, Post, allow_all: true + end + end + + policy_klass = Class.new(PapersPlease::Policy) do + define_method(:configure) do + role :admin, admin_role_class, proc { |u| u.admin } + end + end + + expect(policy_klass.new(admin).can?(:read, Post)).to be true + expect(policy_klass.new(member).can?(:read, Post)).to be false + end + + it "instantiates a fresh role object for each policy instance" do + configured_roles = [] + member_role_class = Class.new(PapersPlease::Role) do + define_method(:configure) do + configured_roles << self + grant :read, Post, allow_all: true + end + end + + policy_klass = Class.new(PapersPlease::Policy) do + define_method(:configure) do + role :member, member_role_class + end + end + + first_policy = policy_klass.new(member) + second_policy = policy_klass.new(member) + + expect(first_policy.roles[:member]).not_to equal(second_policy.roles[:member]) + expect(configured_roles).to contain_exactly(first_policy.roles[:member], second_policy.roles[:member]) + end + + it "passes the user and policy to role instances" do + seen = {} + member_role_class = Class.new(PapersPlease::Role) do + define_method(:configure) do + seen[:user] = user + seen[:policy] = policy + end + end + + policy_klass = Class.new(PapersPlease::Policy) do + define_method(:configure) do + role :member, member_role_class + end + end + + policy = policy_klass.new(member) + + expect(seen[:user]).to equal(member) + expect(seen[:policy]).to equal(policy) + end + + it "requires explicit role classes to inherit from PapersPlease::Role" do + policy_klass = Class.new(PapersPlease::Policy) do + def configure + role :member, String + end + end + + expect { policy_klass.new(member) }.to raise_error(ArgumentError, /inherit from PapersPlease::Role/) + end + end + describe "#can" do it "returns true for passing predicate" do policy = sample_policy(member)