diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 9b4c486..eeb21bb 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,17 +12,15 @@ 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 run: bundle install - - name: Rubocop - run: rubocop - - name: Run tests + - name: Lint and test run: bundle exec rake 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..e81aaa6 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -7,47 +7,53 @@ 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) - 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) + regexp_parser (2.12.0) + 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) - rubocop (1.38.0) + rspec-support (~> 3.13.0) + rspec-support (3.13.7) + 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) @@ -63,14 +81,13 @@ PLATFORMS ruby DEPENDENCIES - bundler (~> 2.0) + bundler (>= 2.4.0) byebug papers_please! rake (~> 13.0) - rspec (~> 3.12) - rubocop (= 1.38.0) - rubocop-rspec + rspec (~> 3.13) simplecov + standard (~> 1.39) BUNDLED WITH - 2.3.9 + 2.7.2 diff --git a/README.md b/README.md index 0386338..af03666 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,105 @@ 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 +``` + +### 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 -# app/controllers/posts_controller.rb +```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 +123,157 @@ 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 + +# What actions can this user perform on this record? +policy.permitted_actions(@post) # => [:show, :edit, :update] +``` + +## 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: - send_data @attachment.data, type: @attachment.content_type +```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 +``` + +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." + +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. + +## Rake Tasks + +`papers_please` ships with a Rails rake task for inspecting policy configuration: ```bash $ rails papers_please:roles +``` + +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. -# => -# +---------+------------+------------+------------+----------------+-------------------+ -# | 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 | -# +---------+------------+------------+------------+----------------+-------------------+ +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 Add this line to your application's Gemfile: @@ -126,29 +286,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 +331,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/Rakefile b/Rakefile index 82bb534..3e9bc8f 100644 --- a/Rakefile +++ b/Rakefile @@ -1,8 +1,12 @@ # 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) -task default: :spec +task :standard do + sh "bundle exec standardrb" +end + +task default: [:standard, :spec] diff --git a/lib/papers_please.rb b/lib/papers_please.rb index 8908e04..8bdee50 100644 --- a/lib/papers_please.rb +++ b/lib/papers_please.rb @@ -1,28 +1,48 @@ # 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/delegation" +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 + 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' + 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 +55,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 +66,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/delegation.rb b/lib/papers_please/delegation.rb new file mode 100644 index 0000000..3864568 --- /dev/null +++ b/lib/papers_please/delegation.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +module PapersPlease + Delegation = Data.define(: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 ba0413d..71b5ac2 100644 --- a/lib/papers_please/permission.rb +++ b/lib/papers_please/permission.rb @@ -2,74 +2,88 @@ module PapersPlease class Permission - attr_accessor :key, :subject - attr_reader :query, :predicate, :granted_by, :granting_class - - def initialize(key, subject, query: nil, predicate: nil, granted_by: nil, granting_class: nil) - self.key = key - self.subject = subject - self.query = query - self.predicate = predicate - self.granted_by = granted_by - self.granting_class = granting_class + attr_reader :key, :subject, :collection, :delegation, :scope, :predicate + + alias_method :query, :scope + alias_method :collection?, :collection + + 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 + + @key = key + @subject = subject + @scope = validate_callable!(scope || query) + @predicate = validate_callable!(predicate) + @delegation = delegation + @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) key_matches?(key) && subject_matches?(subject) end - def granted?(*args) - return predicate.call(*args) if predicate.is_a? Proc + def matches_subject?(subject) + subject_matches?(subject) + end + + def granted?(user, subject) + return predicate.call(user, subject) if predicate # :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 + def fetch(user, klass) + return scope.call(user, klass) if scope nil end - # Setters - def query=(val) - raise ArgumentError, "query must be a Proc, #{val.class} given" if val && !val.is_a?(Proc) - - @query = val + def scope=(val) + @scope = validate_callable!(val) end - def predicate=(val) - raise ArgumentError, "predicate must be a Proc, #{val.class} given" if val && !val.is_a?(Proc) + alias_method :query=, :scope= - @predicate = val + def predicate=(val) + @predicate = validate_callable!(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 + private - def granting_class=(val) - raise ArgumentError, "granting_class must be a Class, #{val.class} given" if val && !val.is_a?(Class) + def validate_callable!(val) + raise ArgumentError, "must be a Proc, #{val.class} given" if val && !val.is_a?(Proc) - @granting_class = val + val end - private - def key_matches?(key) key == @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 54d01eb..b829efe 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 + @user = user @default_scope = nil - @roles = {} - @cache = {} + @roles = {} configure end @@ -19,27 +16,26 @@ 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 - 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 end - alias role add_role + alias_method :role, :add_role - # Add permissions to the Role def add_permissions(keys) return unless block_given? @@ -49,21 +45,17 @@ 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 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? - - # Check permission - granted = permission_granted?(permission, action, subject) - next if granted.nil? || (granted == false && fallthrough) + granted = permission_granted_for_role?(role, permission, check_subject) + next if !granted && fallthrough return granted end @@ -76,41 +68,50 @@ 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) 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_subject?(subject) + next if actions.include?(permission.key) + next unless permission_granted_for_role?(role, permission, subject) + + 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) - permission&.granted?(user, subject, action) || false + permission && permission_granted_for_role?(role, permission, subject) 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) + 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 @applicable_roles ||= roles.select do |_, role| role.applies_to?(user) @@ -123,22 +124,48 @@ 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) + 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) + + if klass == Class + subject.name || "anonymous class" else - permission.nil? ? nil : permission.granted?(user, subject, action) + klass.name || "anonymous #{klass}" end 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) + 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 - # Get proxied permission - permission = role.find_permission(action, subject) + 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) - [subject, permission] + [resolved, permission] end end end 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 c77f46d..6c67921 100644 --- a/lib/papers_please/railtie.rb +++ b/lib/papers_please/railtie.rb @@ -1,25 +1,16 @@ # 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 - 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 657be04..5faa97b 100644 --- a/lib/papers_please/role.rb +++ b/lib/papers_please/role.rb @@ -1,39 +1,50 @@ +# frozen_string_literal: true + 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) - 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, through, collection, allow_all = 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 + 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, - actions, - klass, - query: query, + action, klass, + scope: scope, predicate: predicate, - granted_by: granted_by + delegation: delegation, + collection: collection, + allow_all: allow_all || implicit_allow_all ) end end - alias grant add_permission + alias_method :grant, :add_permission def find_permission(action, subject) permissions.detect do |permission| @@ -47,71 +58,88 @@ def permission_exists?(action, subject) private - 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 resolve_opts(opts) + scope = resolve_alias(opts, :scope, :query) + 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) - true + [scope, predicate, through, collection, allow_all] 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] + 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 - # rubocop:disable Metrics/MethodLength - def make_permission(action, actions, klass, query: nil, predicate: nil, granted_by: nil) - has_query = query.is_a?(Proc) - has_predicate = predicate.is_a?(Proc) - permission = make_base_permission(action, klass, granted_by: granted_by) + def build_delegation(through) + return nil if through.nil? + + raise InvalidGrant, "through must be a [Class, Proc], got #{through.inspect}" unless valid_delegation?(through) - if has_query && has_predicate - # Both query & predicate provided - permission.query = query + Delegation.new(klass: through[0], resolver: through[1]) + end + + 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, allow_all: false) + has_scope = !scope.nil? + has_predicate = !predicate.nil? + permission = Permission.new(action, klass, delegation: delegation, collection: collection) + + 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 { |_u, _s| true }) : build_predicate_from_scope(klass, scope) + elsif has_predicate permission.predicate = predicate - else - # Neither provided - permission.query = (proc { klass.all }) - permission.predicate = (proc { true }) + elsif allow_all + permission.scope = build_unconditional_scope(klass) + permission.predicate = (proc { |_u, _s| true }) end permission end - # rubocop:enable Metrics/MethodLength + def build_unconditional_scope(klass) + return proc { |_u, _k| klass.all } if klass.respond_to?(:all) - def make_base_permission(action, klass, granted_by: nil) - permission = Permission.new(action, klass) + nil + end - if granted_by - permission.granting_class = granted_by[0] - permission.granted_by = granted_by[1] + def build_predicate_from_scope(klass, scope) + proc do |user, obj| + class_backed_instance?(klass, obj) && included_in_scope?(scope.call(user, klass), obj) end - - 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 + def class_backed_instance?(klass, obj) + klass.is_a?(Class) && !!(Object.instance_method(:class).bind_call(obj) <= klass) + end - # Otherwise the default predicate is to check - # for inclusion in the returned relationship - proc do |user, obj| - res = query.call(user, klass, action) - res.respond_to?(:include?) && res.include?(obj) + 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/lib/papers_please/tasks/papers_please.rake b/lib/papers_please/tasks/papers_please.rake index a425e28..fb79019 100644 --- a/lib/papers_please/tasks/papers_please.rake +++ b/lib/papers_please/tasks/papers_please.rake @@ -1,11 +1,11 @@ # frozen_string_literal: true 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 + desc "Print out all defined roles and permissions in match order" + 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 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 8657664..96a422d 100644 --- a/papers_please.gemspec +++ b/papers_please.gemspec @@ -1,34 +1,37 @@ # 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.add_dependency 'terminal-table' + spec.required_ruby_version = ">= 3.3.0" - 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_dependency "terminal-table" + + 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.13" + spec.add_development_dependency "standard", "~> 1.39" + spec.add_development_dependency "simplecov" + + spec.metadata["rubygems_mfa_required"] = "true" end diff --git a/spec/collection_spec.rb b/spec/collection_spec.rb new file mode 100644 index 0000000..00084d1 --- /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, allow_all: 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 3d03d67..fd241c6 100644 --- a/spec/fixtures/access_policy.rb +++ b/spec/fixtures/access_policy.rb @@ -2,24 +2,25 @@ 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 - role.grant :manage, 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 :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, 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 - 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 :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 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/keyword_aliases_spec.rb b/spec/keyword_aliases_spec.rb new file mode 100644 index 0000000..f793539 --- /dev/null +++ b/spec/keyword_aliases_spec.rb @@ -0,0 +1,116 @@ +# 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 "lambdas work as callables" do + let(:role) { PapersPlease::Role.new(:test) } + + 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 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 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 }] } + + 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/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..dbcf559 100644 --- a/spec/permission_spec.rb +++ b/spec/permission_spec.rb @@ -1,26 +1,25 @@ # 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") } + let(:user) { double("User") } - 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) 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,65 +27,124 @@ expect(perm.query).to be stub_proc end - describe '#matches?' do + 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) } - 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 - 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) } + describe "#matches_subject?" do + let(:permission) { subject.new(:read, book) } + + 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) - it 'is true for true permission' do - expect(true_permission.granted?).to be true + expect(literal_permission.matches_subject?(:dashboard)).to be true + expect(literal_permission.matches_subject?(:settings)).to be false end + end - it 'is false for false permission' do - expect(false_permission.granted?).to be false + describe "#granted?" do + 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?(user, book)).to be true end - it 'calls the predicate proc' do - expect(stub_proc).to receive(:call).and_return(true) - expect(stub_perm.granted?).to be true + it "is false for false permission" do + expect(false_permission.granted?(user, book)).to be false end - 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 + 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) + + 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) } + describe "#fetch" do + 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 [] + it "is array for array permission" do + expect(array_permission.fetch(user, book)).to eq [] end - it 'is nil for nil permission' do - expect(empty_permission.fetch).to be_nil + it "is nil for nil permission" do + expect(empty_permission.fetch(user, book)).to be_nil end - it 'calls the predicate proc' do - expect(stub_proc).to receive(:call).and_return([]) - expect(stub_perm.fetch).to eq [] + 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) + + expect(called_with).to eq [user, book] end + end + + describe "#delegated?" do + it "is true with a delegation" do + delegation = PapersPlease::Delegation.new(klass: Post, resolver: 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) - 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 + expect(perm.delegated?).to be false + expect(perm.granting_class).to be_nil end end end diff --git a/spec/policy_spec.rb b/spec/policy_spec.rb index 4ac94fa..16c8994 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,91 @@ def configure end end - describe '#can' do - it 'returns true for passing predicate' do + 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) 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 +190,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 +204,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,21 +221,68 @@ def configure end end - describe '#role_that_can' do - it 'returns the symbolic name of the roles that can perform an action' do + 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 def configure role :member 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 @@ -175,11 +294,62 @@ def configure end end - describe '#authorize!' do - it 'raises an AccessDenied error for failing predicate' do + 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) 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 @@ -187,15 +357,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 + role.grant :read, Post, allow_all: true 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 +376,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/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 5c46a45..78febc1 100644 --- a/spec/role_spec.rb +++ b/spec/role_spec.rb @@ -1,72 +1,65 @@ # 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 is treated as a literal action" do let(:role) { described_class.new(:admin) } - it 'adds correct permissions' 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) + it "does not expand :manage" do + role.add_permission(:manage, Post, allow_all: true) + expect(role.permissions.size).to eq 1 + expect(role.permissions.first.key).to eq(:manage) end end - context 'duplicate permissions' do + context "duplicate permissions" do 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) - 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) + it "raises error for duplicates" do + role.add_permission(:read, Post, allow_all: true) + expect { role.add_permission(:read, Post, allow_all: true) }.to raise_error(PapersPlease::DuplicatePermission) end end - context 'granted_by' do - let(:granted_by) { (proc { |_u, a| a.post }) } + context "granted_by" do + let(:resolver) { 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) + 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) @@ -74,110 +67,146 @@ 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 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.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 + expect(permission.query).to be_nil + expect(permission.predicate).to be_nil 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 "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 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 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 + 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 + + 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 - 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 "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' do - empty_role.add_permission(:read, klass) + 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 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!