diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml new file mode 100644 index 000000000..a0d20db60 --- /dev/null +++ b/.github/workflows/test.yml @@ -0,0 +1,82 @@ +name: Test & lint +on: [push, pull_request] + +env: + RAILS_ENV: test + PGHOST: localhost + PGUSER: postgres + +jobs: + tests: + name: Test & lint + runs-on: ubuntu-latest + + strategy: + fail-fast: false + matrix: + ruby: ['2.4', '2.5', '2.6', '2.7', 'jruby', 'truffleruby'] + gemfile: ['gemfiles/activerecord_4.2.0.gemfile', 'gemfiles/activerecord_5.0.2.gemfile', 'gemfiles/activerecord_5.1.0.gemfile', 'gemfiles/activerecord_5.2.2.gemfile', 'gemfiles/activerecord_6.0.0.gemfile', 'gemfiles/activerecord_6.1.0.gemfile', 'gemfiles/activerecord_master.gemfile'] + exclude: + - gemfile: 'gemfiles/activerecord_4.2.0.gemfile' + ruby: '2.7' # rails 4.2 can't run on ruby 2.7 due to BigDecimal API change + - gemfile: 'gemfiles/activerecord_4.2.0.gemfile' + ruby: 'truffleruby' # TruffleRuby 21.0 targets Ruby 2.7, same as above + - gemfile: 'gemfiles/activerecord_master.gemfile' + ruby: '2.6' # rails 7+ requires ruby 3.0+ + - gemfile: 'gemfiles/activerecord_master.gemfile' + ruby: '2.5' # rails 7+ requires ruby 3.0+ + - gemfile: 'gemfiles/activerecord_6.0.0.gemfile' + ruby: '2.4' # rails 6+ requires ruby 2.5+ + - gemfile: 'gemfiles/activerecord_6.1.0.gemfile' + ruby: '2.4' # rails 6+ requires ruby 2.5+ + - gemfile: 'gemfiles/activerecord_master.gemfile' + ruby: '2.4' # rails 6+ requires ruby 2.5+ + - gemfile: 'gemfiles/activerecord_5.0.2.gemfile' + ruby: 'jruby' # this *should* work - there's a test failure; it's not incompatible like the other excludes. could be an issue in Rails 5.0.2? + - gemfile: 'gemfiles/activerecord_6.1.0.gemfile' + ruby: 'jruby' # this *should* work. it seems like there's an issue with rails 6 on jruby. + - gemfile: 'gemfiles/activerecord_master.gemfile' + ruby: 'jruby' # this *should* work. it seems like there's an issue with rails 6 on jruby. + + env: + BUNDLE_GEMFILE: ${{ matrix.gemfile }} + + services: + postgres: + image: postgres + env: + POSTGRES_USER: postgres + POSTGRES_DB: cancan_postgresql_spec + POSTGRES_PASSWORD: postgres + options: >- + --health-cmd pg_isready + --health-interval 10s + --health-timeout 5s + --health-retries 5 + ports: ["5432:5432"] + + steps: + - name: Checkout code + uses: actions/checkout@v2 + with: + fetch-depth: '20' + + # https://github.com/CanCanCommunity/cancancan/pull/669#issuecomment-748019539 + - name: Nokogiri support for Truffleruby + run: sudo apt-get -yqq install libxml2-dev libxslt-dev + if: ${{ matrix.ruby == 'truffleruby' }} + + - name: Setup Ruby + uses: ruby/setup-ruby@v1 + with: + ruby-version: ${{ matrix.ruby }} + bundler-cache: true + + - name: Run linter + run: bundle exec rubocop + + - name: Run tests on sqlite + run: DB=sqlite bundle exec rake + + - name: Run tests on postgres + run: DB=postgres bundle exec rake diff --git a/.rubocop.yml b/.rubocop.yml index 214673a53..8f5637e7a 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -37,4 +37,5 @@ AllCops: TargetRubyVersion: 2.2.0 Exclude: - 'gemfiles/**/*' + - 'vendor/**/*' - 'Appraisals' diff --git a/.travis.yml b/.travis.yml deleted file mode 100644 index bffcb1a99..000000000 --- a/.travis.yml +++ /dev/null @@ -1,82 +0,0 @@ -language: ruby -cache: bundler -addons: - postgresql: "9.6" -rvm: - - 2.4.2 - - 2.5.1 - - 2.6.3 - - 2.7.0 - - ruby-head - - jruby-9.1.17.0 - - jruby-9.2.11.1 - - jruby-head - - truffleruby-head - -gemfile: - - gemfiles/activerecord_4.2.0.gemfile - - gemfiles/activerecord_5.0.2.gemfile - - gemfiles/activerecord_5.1.0.gemfile - - gemfiles/activerecord_5.2.2.gemfile - - gemfiles/activerecord_6.0.0.gemfile - - gemfiles/activerecord_6.1.0.gemfile - - gemfiles/activerecord_master.gemfile -env: - - DB=sqlite - - DB=postgres - -matrix: - fast_finish: true - exclude: - - rvm: 2.4.2 - gemfile: gemfiles/activerecord_6.0.0.gemfile - - rvm: 2.2.6 - gemfile: gemfiles/activerecord_6.1.0.gemfile - - rvm: 2.3.5 - gemfile: gemfiles/activerecord_6.1.0.gemfile - - rvm: 2.4.2 - gemfile: gemfiles/activerecord_6.1.0.gemfile - - rvm: 2.2.6 - gemfile: gemfiles/activerecord_master.gemfile - - rvm: 2.3.5 - gemfile: gemfiles/activerecord_master.gemfile - - rvm: 2.4.2 - gemfile: gemfiles/activerecord_master.gemfile - - rvm: 2.7.0 - gemfile: gemfiles/activerecord_4.2.0.gemfile - - rvm: ruby-head - gemfile: gemfiles/activerecord_4.2.0.gemfile - - rvm: truffleruby-head - gemfile: gemfiles/activerecord_4.2.0.gemfile - - rvm: jruby-9.1.17.0 - gemfile: gemfiles/activerecord_5.0.2.gemfile - - rvm: jruby-9.1.17.0 - gemfile: gemfiles/activerecord_6.0.0.gemfile - - rvm: jruby-9.1.17.0 - gemfile: gemfiles/activerecord_6.1.0.gemfile - - rvm: jruby-9.1.17.0 - gemfile: gemfiles/activerecord_master.gemfile - - rvm: jruby-9.2.11.1 - gemfile: gemfiles/activerecord_5.0.2.gemfile - - rvm: jruby-9.2.11.1 - gemfile: gemfiles/activerecord_6.0.0.gemfile - - rvm: jruby-9.2.11.1 - gemfile: gemfiles/activerecord_6.1.0.gemfile - - rvm: jruby-9.2.11.1 - gemfile: gemfiles/activerecord_master.gemfile - allow_failures: - - rvm: ruby-head - - rvm: jruby-head - -notifications: - email: - recipients: - - alessandro.rodi@renuo.ch - on_success: change - on_failure: change -before_install: - - rvm get stable - - gem update --system - - gem install bundler -script: - - bundle exec rubocop && bundle exec rake diff --git a/Appraisals b/Appraisals index 8d178d3d0..f7b89691a 100644 --- a/Appraisals +++ b/Appraisals @@ -96,8 +96,8 @@ appraise 'activerecord_6.1.0' do end platforms :ruby, :mswin, :mingw do - gem 'pg', '~> 1.1.4' - gem 'sqlite3', '~> 1.4.0' + gem 'pg', '~> 1.2.3' + gem 'sqlite3', '~> 1.4.2' end end @@ -113,7 +113,7 @@ appraise 'activerecord_master' do end platforms :ruby, :mswin, :mingw do - gem 'pg', '~> 1.1.4' - gem 'sqlite3', '~> 1.4.0' + gem 'pg', '~> 1.2.3' + gem 'sqlite3', '~> 1.4.2' end end diff --git a/CHANGELOG.md b/CHANGELOG.md index d9a426283..7d4b7312d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,12 @@ +## 3.3.0 + +* [#675](https://github.com/CanCanCommunity/cancancan/pull/675): Support modifying the `accessible_by` querying strategy on a per-query basis. ([@ghiculescu][]) +* [#714](https://github.com/CanCanCommunity/cancancan/pull/714): Don't hold unnecessary references to subjects in @rules_index. ([@mtoneil][]) + +## 3.2.2 + +* Added funding metadata to Gemspec. ([@coorasse][]) + ## 3.2.1 * [#674](https://github.com/CanCanCommunity/cancancan/pull/674): Fix accidental dependency on ActiveRecord in 3.2.0. ([@ghiculescu][]) @@ -679,3 +688,4 @@ Please read the [guide on migrating from CanCanCan 2.x to 3.0](https://github.co [@ayumu838]: https://github.com/ayumu838 [@Liberatys]: https://github.com/Liberatys [@ghiculescu]: https://github.com/ghiculescu +[@mtoneil]: https://github.com/mtoneil diff --git a/README.md b/README.md index 1758bae2d..bd5bf3ffb 100644 --- a/README.md +++ b/README.md @@ -3,7 +3,7 @@ [![Gem Version](https://badge.fury.io/rb/cancancan.svg)](http://badge.fury.io/rb/cancancan) -[![Travis badge](https://travis-ci.org/CanCanCommunity/cancancan.svg?branch=develop)](https://travis-ci.org/CanCanCommunity/cancancan) +[![Github Actions badge](https://github.com/CanCanCommunity/cancancan/actions/workflows/test.yml/badge.svg)](https://github.com/CanCanCommunity/cancancan/actions/workflows/test.yml/badge.svg) [![Code Climate Badge](https://codeclimate.com/github/CanCanCommunity/cancancan.svg)](https://codeclimate.com/github/CanCanCommunity/cancancan) [Wiki](./docs) | @@ -36,6 +36,16 @@ of models automatically and reduce duplicated code.

+ + Bullet Train + +
+
+ + Goboony + +
+
Do you want to sponsor CanCanCan and show your logo here? Check our [Sponsors Page](https://github.com/sponsors/coorasse). @@ -275,7 +285,7 @@ If you find a bug please add an [issue on GitHub](https://github.com/CanCanCommu CanCanCan uses [appraisals](https://github.com/thoughtbot/appraisal) to test the code base against multiple versions of Rails, as well as the different model adapters. -When first developing, you need to run `bundle install` and then `appraisal install`, to install the different sets. +When first developing, you need to run `bundle install` and then `bundle exec appraisal install`, to install the different sets. You can then run all appraisal files (like CI does), with `appraisal rake` or just run a specific set `DB='sqlite' bundle exec appraisal activerecord_5.2.2 rake`. diff --git a/docs/Defining-Abilities-with-Blocks.md b/docs/Defining-Abilities-with-Blocks.md index 14f8506b5..1c16c114b 100644 --- a/docs/Defining-Abilities-with-Blocks.md +++ b/docs/Defining-Abilities-with-Blocks.md @@ -94,8 +94,8 @@ Here the block will be triggered for every `can?` check, even when only a class ## Additional Docs -* [[Defining Abilities]] -* [[Checking Abilities]] -* [[Testing Abilities]] -* [[Debugging Abilities]] -* [[Ability Precedence]] +* [Defining Abilities](./Defining-Abilities.md) +* [Checking Abilities](./Checking-Abilities.md) +* [Testing Abilities](./Testing-Abilities.md) +* [Debugging Abilities](./Debugging-Abilities.md) +* [Ability Precedence](./Ability-Precedence.md) diff --git a/docs/Defining-Abilities.md b/docs/Defining-Abilities.md index 5e2fd564b..a5144e820 100644 --- a/docs/Defining-Abilities.md +++ b/docs/Defining-Abilities.md @@ -18,7 +18,7 @@ class Ability end ``` -The `current_user` model is passed into the initialize method, so the permissions can be modified based on any user attributes. CanCanCan makes no assumption about how roles are handled in your application. See [[Role Based Authorization]] for an example. +The `current_user` model is passed into the initialize method, so the permissions can be modified based on any user attributes. CanCanCan makes no assumption about how roles are handled in your application. See [Role Based Authorization](./Role-Based-Authorization.md) for an example. ## The `can` Method @@ -36,7 +36,7 @@ can :read, :all # user can read any object can :manage, :all # user can perform any action on any object ``` -Common actions are `:read`, `:create`, `:update` and `:destroy` but it can be anything. See [[Action Aliases]] and [[Custom Actions]] for more information on actions. +Common actions are `:read`, `:create`, `:update` and `:destroy` but it can be anything. See [Action Aliases](./Action-Aliases.md) and [Custom Actions](./Custom-Actions.md) for more information on actions. You can pass an array for either of these parameters to match any one. For example, here the user will have the ability to update or destroy both articles and comments. @@ -74,7 +74,7 @@ A hash of conditions can be passed to further restrict which records this permis can :read, Project, active: true, user_id: user.id ``` -It is important to only use database columns for these conditions so it can be reused for [[Fetching Records]]. +It is important to only use database columns for these conditions so it can be reused for [Fetching Records](./Fetching-Records.md). You can use nested hashes to define conditions on associations. Here the project can only be read if the category it belongs to is visible. diff --git a/docs/Ensure-Authorization.md b/docs/Ensure-Authorization.md index 0e55cd35a..6722b3c54 100644 --- a/docs/Ensure-Authorization.md +++ b/docs/Ensure-Authorization.md @@ -6,7 +6,7 @@ class ApplicationController < ActionController::Base end ``` -This will add an `after_filter` to ensure authorization takes place in every inherited controller action. If no authorization happens it will raise a `CanCan::AuthorizationNotPerformed` exception. You can skip this check by adding `skip_authorization_check` to that controller. Both of these methods take the same arguments as `before_filter` so you can exclude certain actions with `:only` and `:except`. +This will add an `after_action` to ensure authorization takes place in every inherited controller action. If no authorization happens it will raise a `CanCan::AuthorizationNotPerformed` exception. You can skip this check by adding `skip_authorization_check` to that controller. Both of these methods take the same arguments as `before_action` so you can exclude certain actions with `:only` and `:except`. ```ruby class UsersController < ApplicationController diff --git a/docs/Nested-Resources.md b/docs/Nested-Resources.md index 3fd597d6a..18d37d1ce 100644 --- a/docs/Nested-Resources.md +++ b/docs/Nested-Resources.md @@ -28,6 +28,46 @@ If the name of the association doesn't match the resource name, for instance `ha If the resource name (`:project` in this case) does not match the controller then it will be considered a parent resource. You can manually specify parent/child resources using the `parent: false` option. +## Securing `through` changes + +If you are using `through` you need to be wary of potential changes to the parent model. For example, consider this controller: + +```ruby +class TasksController < ApplicationController + load_and_authorize_resource :project + load_and_authorize_resource :task, through: :project + + def update + @task.update(task_params) + end + + private + + def task_params + params.require(:task).permit(:project_id) + end +end +``` + +Now consider a request to `/projects/1/tasks/42` with params `{ task: { project_id: 2 } }`. + +- `load_and_authorize_resource :project` will load project 1 and authorize it. +- `load_and_authorize_resource :task, through: :project` will load task 42 from project 1, and authorize it. +- `@task.update(task_params)` will change the task's project ID from 1, to 2. +- Project 2 is never authorized! An attacker could inject a project belonging to another customer here. + +How you handle this depends on your intended behavior. + +- If you don't want a task's project ID to ever change, don't permit it as a param. +- If you allow tasks to be moved between projects, manually verify the ID change and avoid mass assigning it. + +```ruby + def update + @task.project = Project.find(task_params[:project_id]) + authorize!(@task) + @task.assign(task_params.except(:project_id)) + end +``` ## Nested through method @@ -153,7 +193,7 @@ in ability.rb can :create, User, groups_users: {group: {CONDITION_ON_GROUP} } ``` -Don't forget the **inverse_of** option, is the trick to make it works correctly. +Don't forget the **inverse_of** option, it is the trick to make it work correctly. Remember to define the ability through the **groups_users** model (i.e. don't write `can :create, User, groups: {CONDITION_ON_GROUP}`) diff --git a/gemfiles/activerecord_6.1.0.gemfile b/gemfiles/activerecord_6.1.0.gemfile index 6c556b841..857cfa4e2 100644 --- a/gemfiles/activerecord_6.1.0.gemfile +++ b/gemfiles/activerecord_6.1.0.gemfile @@ -13,8 +13,8 @@ platforms :jruby do end platforms :ruby, :mswin, :mingw do - gem "pg", "~> 1.1.4" - gem "sqlite3", "~> 1.4.0" + gem "pg", "~> 1.2.3" + gem "sqlite3", "~> 1.4.2" end gemspec path: "../" diff --git a/gemfiles/activerecord_master.gemfile b/gemfiles/activerecord_master.gemfile index 739e08baf..8328cadc1 100644 --- a/gemfiles/activerecord_master.gemfile +++ b/gemfiles/activerecord_master.gemfile @@ -13,8 +13,8 @@ platforms :jruby do end platforms :ruby, :mswin, :mingw do - gem "pg", "~> 1.1.4" - gem "sqlite3", "~> 1.4.0" + gem "pg", "~> 1.2.3" + gem "sqlite3", "~> 1.4.2" end gemspec path: "../" diff --git a/lib/cancan/ability/rules.rb b/lib/cancan/ability/rules.rb index 0d1297be7..c7491a6ec 100644 --- a/lib/cancan/ability/rules.rb +++ b/lib/cancan/ability/rules.rb @@ -19,12 +19,13 @@ def add_rule(rule) end def add_rule_to_index(rule, position) - @rules_index ||= Hash.new { |h, k| h[k] = [] } + @rules_index ||= {} subjects = rule.subjects.compact subjects << :all if subjects.empty? subjects.each do |subject| + @rules_index[subject] ||= [] @rules_index[subject] << position end end @@ -48,7 +49,9 @@ def possible_relevant_rules(subject) rules else positions = @rules_index.values_at(subject, *alternative_subjects(subject)) - positions.flatten!.sort! + positions.compact! + positions.flatten! + positions.sort! positions.map { |i| @rules[i] } end end diff --git a/lib/cancan/config.rb b/lib/cancan/config.rb index 19243395e..a9106526b 100644 --- a/lib/cancan/config.rb +++ b/lib/cancan/config.rb @@ -21,7 +21,9 @@ def self.valid_accessible_by_strategies # `distinct` is not reliable in some cases. See # https://github.com/CanCanCommunity/cancancan/pull/605 def self.accessible_by_strategy - @accessible_by_strategy || default_accessible_by_strategy + return @accessible_by_strategy if @accessible_by_strategy + + @accessible_by_strategy = default_accessible_by_strategy end def self.default_accessible_by_strategy @@ -36,9 +38,7 @@ def self.default_accessible_by_strategy end def self.accessible_by_strategy=(value) - unless valid_accessible_by_strategies.include?(value) - raise ArgumentError, "accessible_by_strategy must be one of #{valid_accessible_by_strategies.join(', ')}" - end + validate_accessible_by_strategy!(value) if value == :subquery && does_not_support_subquery_strategy? raise ArgumentError, 'accessible_by_strategy = :subquery requires ActiveRecord 5 or newer' @@ -47,6 +47,26 @@ def self.accessible_by_strategy=(value) @accessible_by_strategy = value end + def self.with_accessible_by_strategy(value) + return yield if value == accessible_by_strategy + + validate_accessible_by_strategy!(value) + + begin + strategy_was = accessible_by_strategy + @accessible_by_strategy = value + yield + ensure + @accessible_by_strategy = strategy_was + end + end + + def self.validate_accessible_by_strategy!(value) + return if valid_accessible_by_strategies.include?(value) + + raise ArgumentError, "accessible_by_strategy must be one of #{valid_accessible_by_strategies.join(', ')}" + end + def self.does_not_support_subquery_strategy? !defined?(CanCan::ModelAdapters::ActiveRecordAdapter) || CanCan::ModelAdapters::ActiveRecordAdapter.version_lower?('5.0.0') diff --git a/lib/cancan/model_additions.rb b/lib/cancan/model_additions.rb index 96732133b..ef2d26eb3 100644 --- a/lib/cancan/model_additions.rb +++ b/lib/cancan/model_additions.rb @@ -20,8 +20,10 @@ module ClassMethods # @articles = Article.accessible_by(current_ability, :update) # # Here only the articles which the user can update are returned. - def accessible_by(ability, action = :index) - ability.model_adapter(self, action).database_records + def accessible_by(ability, action = :index, strategy: CanCan.accessible_by_strategy) + CanCan.with_accessible_by_strategy(strategy) do + ability.model_adapter(self, action).database_records + end end end diff --git a/lib/cancan/version.rb b/lib/cancan/version.rb index b94820c8a..d436325d1 100644 --- a/lib/cancan/version.rb +++ b/lib/cancan/version.rb @@ -1,5 +1,5 @@ # frozen_string_literal: true module CanCan - VERSION = '3.2.2'.freeze + VERSION = '3.3.0'.freeze end diff --git a/logo/bullet_train.png b/logo/bullet_train.png new file mode 100644 index 000000000..0e40a801d Binary files /dev/null and b/logo/bullet_train.png differ diff --git a/logo/goboony.png b/logo/goboony.png new file mode 100644 index 000000000..7d77a4782 Binary files /dev/null and b/logo/goboony.png differ diff --git a/spec/cancan/ability_spec.rb b/spec/cancan/ability_spec.rb index 7a6899db7..7c998fdb5 100644 --- a/spec/cancan/ability_spec.rb +++ b/spec/cancan/ability_spec.rb @@ -674,6 +674,14 @@ def active? expect(@ability.permitted_attributes(:read, Integer)).to eq([:to_s]) end + it 'does not retain references to subjects that do not have direct rules' do + @ability.can :read, String + + @ability.can?(:read, 'foo') + + expect(@ability.instance_variable_get(:@rules_index)).not_to have_key('foo') + end + describe 'unauthorized message' do after(:each) do I18n.backend = nil diff --git a/spec/cancan/model_adapters/active_record_adapter_spec.rb b/spec/cancan/model_adapters/active_record_adapter_spec.rb index df2a019e2..2ac33756b 100644 --- a/spec/cancan/model_adapters/active_record_adapter_spec.rb +++ b/spec/cancan/model_adapters/active_record_adapter_spec.rb @@ -817,6 +817,101 @@ class Transaction < ActiveRecord::Base end end + if CanCan::ModelAdapters::ActiveRecordAdapter.version_greater_or_equal?('5.0.0') + context 'switching strategies' do + before do + CanCan.accessible_by_strategy = :left_join # default - should be ignored in these tests + end + + it 'allows you to switch strategies with a keyword argument' do + u = User.create!(name: 'pippo') + Article.create!(mentioned_users: [u]) + + ability = Ability.new(u) + ability.can :read, Article, mentions: { user: { name: u.name } } + + subquery_sql = Article.accessible_by(ability, strategy: :subquery).to_sql + left_join_sql = Article.accessible_by(ability, strategy: :left_join).to_sql + + expect(subquery_sql.strip.squeeze(' ')).to eq(%( + SELECT "articles".* + FROM "articles" + WHERE "articles"."id" IN + (SELECT "articles"."id" + FROM "articles" + LEFT OUTER JOIN "legacy_mentions" ON "legacy_mentions"."article_id" = "articles"."id" + LEFT OUTER JOIN "users" ON "users"."id" = "legacy_mentions"."user_id" + WHERE "users"."name" = 'pippo') + ).gsub(/\s+/, ' ').strip) + + expect(left_join_sql.strip.squeeze(' ')).to eq(%( + SELECT DISTINCT "articles".* + FROM "articles" + LEFT OUTER JOIN "legacy_mentions" ON "legacy_mentions"."article_id" = "articles"."id" + LEFT OUTER JOIN "users" ON "users"."id" = "legacy_mentions"."user_id" + WHERE "users"."name" = 'pippo').gsub(/\s+/, ' ').strip) + end + + it 'allows you to switch strategies with a block' do + u = User.create!(name: 'pippo') + Article.create!(mentioned_users: [u]) + + ability = Ability.new(u) + ability.can :read, Article, mentions: { user: { name: u.name } } + + subquery_sql = CanCan.with_accessible_by_strategy(:subquery) { Article.accessible_by(ability).to_sql } + left_join_sql = CanCan.with_accessible_by_strategy(:left_join) { Article.accessible_by(ability).to_sql } + + expect(subquery_sql.strip.squeeze(' ')).to eq(%( + SELECT "articles".* + FROM "articles" + WHERE "articles"."id" IN + (SELECT "articles"."id" + FROM "articles" + LEFT OUTER JOIN "legacy_mentions" ON "legacy_mentions"."article_id" = "articles"."id" + LEFT OUTER JOIN "users" ON "users"."id" = "legacy_mentions"."user_id" + WHERE "users"."name" = 'pippo') + ).gsub(/\s+/, ' ').strip) + + expect(left_join_sql.strip.squeeze(' ')).to eq(%( + SELECT DISTINCT "articles".* + FROM "articles" + LEFT OUTER JOIN "legacy_mentions" ON "legacy_mentions"."article_id" = "articles"."id" + LEFT OUTER JOIN "users" ON "users"."id" = "legacy_mentions"."user_id" + WHERE "users"."name" = 'pippo').gsub(/\s+/, ' ').strip) + end + + it 'allows you to switch strategies with a block, and to_sql called outside the block' do + u = User.create!(name: 'pippo') + Article.create!(mentioned_users: [u]) + + ability = Ability.new(u) + ability.can :read, Article, mentions: { user: { name: u.name } } + + subquery_sql = CanCan.with_accessible_by_strategy(:subquery) { Article.accessible_by(ability) }.to_sql + left_join_sql = CanCan.with_accessible_by_strategy(:left_join) { Article.accessible_by(ability) }.to_sql + + expect(subquery_sql.strip.squeeze(' ')).to eq(%( + SELECT "articles".* + FROM "articles" + WHERE "articles"."id" IN + (SELECT "articles"."id" + FROM "articles" + LEFT OUTER JOIN "legacy_mentions" ON "legacy_mentions"."article_id" = "articles"."id" + LEFT OUTER JOIN "users" ON "users"."id" = "legacy_mentions"."user_id" + WHERE "users"."name" = 'pippo') + ).gsub(/\s+/, ' ').strip) + + expect(left_join_sql.strip.squeeze(' ')).to eq(%( + SELECT DISTINCT "articles".* + FROM "articles" + LEFT OUTER JOIN "legacy_mentions" ON "legacy_mentions"."article_id" = "articles"."id" + LEFT OUTER JOIN "users" ON "users"."id" = "legacy_mentions"."user_id" + WHERE "users"."name" = 'pippo').gsub(/\s+/, ' ').strip) + end + end + end + CanCan.valid_accessible_by_strategies.each do |strategy| context "when a model has renamed primary_key with #{strategy} strategy" do before :each do diff --git a/spec/support/sql_helpers.rb b/spec/support/sql_helpers.rb index 199a34188..20b783c14 100644 --- a/spec/support/sql_helpers.rb +++ b/spec/support/sql_helpers.rb @@ -31,11 +31,13 @@ def connect_db def connect_postgres ActiveRecord::Base.establish_connection(adapter: 'postgresql', host: 'localhost', - database: 'postgres', schema_search_path: 'public') + database: 'postgres', schema_search_path: 'public', + user: 'postgres', password: 'postgres') ActiveRecord::Base.connection.drop_database('cancan_postgresql_spec') ActiveRecord::Base.connection.create_database('cancan_postgresql_spec', 'encoding' => 'utf-8', 'adapter' => 'postgresql') ActiveRecord::Base.establish_connection(adapter: 'postgresql', host: 'localhost', - database: 'cancan_postgresql_spec') + database: 'cancan_postgresql_spec', + user: 'postgres', password: 'postgres') end end