Merge pull request #1294 from CDL-Dryad/permissions

Additional Pundit scopes and variable fixes

app/controllers/stash_api/datasets_controller.rb

@@ -272,7 +272,8 @@ def em_reformat_response(metadata:, deposit_request:)
 
     # get /datasets
     def index
-      ds_query = StashEngine::Identifier.user_viewable(user: @user) # this limits to a user's list based on their role/permissions (or public ones)
+      ds_query = StashEngine::IdentifierPolicy::Scope.new(@user, StashEngine::Identifier).resolve
+      # this limits to a user's list based on their role/permissions (or public ones)
       # These filter conditions were things Daisie put in, because of some queries she needed to make.
       # We probably want to think about the query interface before we do full blown filtering and be sure it is thought out
       # and we are ready to support whatever we decide.

app/controllers/stash_api/versions_controller.rb

@@ -39,7 +39,7 @@ def download
 
     def paged_versions_for_dataset
       id = StashEngine::Identifier.find_with_id(params[:dataset_id])
-      limited_resources = id.resources.visible_to_user(user: @user)
+      limited_resources = StashEngine::ResourcePolicy::VersionScope.new(@user, id.resources).resolve
       all_count = limited_resources.count
       results = limited_resources.limit(per_page).offset(per_page * (page - 1))
       results = results.map { |i| Version.new(resource_id: i.id).metadata_with_links }

app/controllers/stash_engine/dashboard_controller.rb

@@ -21,7 +21,7 @@ def user_datasets
       @page_size = params[:page_size] || '10'
       respond_to do |format|
         format.js do
-          @datasets = policy_scope(Identifier).page(@page).per(@page_size)
+          @datasets = policy_scope(Identifier, policy_scope_class: IdentifierPolicy::DashboardScope).page(@page).per(@page_size)
           @display_resources = @datasets.map { |dataset| StashDatacite::ResourcesController::DatasetPresenter.new(dataset&.latest_resource) }
         end
       end

app/models/stash_engine/identifier.rb

@@ -41,21 +41,6 @@ class Identifier < ApplicationRecord
       where(pub_state: %w[published embargoed])
     end
 
-    scope :user_viewable, ->(user: nil) do
-      if user.nil?
-        publicly_viewable
-      elsif user.limited_curator?
-        all
-      else
-        tenant_admin = (user.tenant_id if user.role == 'admin')
-        with_visibility(states: %w[published embargoed],
-                        tenant_id: tenant_admin,
-                        journal_issns: user.journals_as_admin.map(&:single_issn),
-                        funder_ids: user.funders_as_admin.map(&:funder_id),
-                        user_id: user.id)
-      end
-    end
-
     scope :cited_by_pubmed, -> do
       ids = publicly_viewable.map(&:id)
       joins(:internal_data)

app/models/stash_engine/resource.rb

@@ -219,21 +219,6 @@ def ensure_state_and_version
       joins(:last_curation_activity).joins(JOIN_FOR_INTERNAL_DATA).joins(JOIN_FOR_CONTRIBUTORS).distinct.where(str, *arr)
     end
 
-    scope :visible_to_user, ->(user:) do
-      if user.nil?
-        with_visibility(states: %w[published embargoed])
-      elsif user.limited_curator?
-        all
-      else
-        tenant_admin = (user.tenant_id if user.role == 'admin')
-        with_visibility(states: %w[published embargoed],
-                        tenant_id: tenant_admin,
-                        funder_ids: user.funders_as_admin.map(&:funder_id),
-                        journal_issns: user.journals_as_admin.map(&:single_issn),
-                        user_id: user.id)
-      end
-    end
-
     # limits to the latest resource for each dataset if added to resources
     scope :latest_per_dataset, (-> do
       joins('INNER JOIN stash_engine_identifiers ON stash_engine_resources.id = stash_engine_identifiers.latest_resource_id')

app/policies/stash_engine/admin_dataset_funder_policy.rb

@@ -1,7 +1,7 @@
 module StashEngine
   class AdminDatasetFunderPolicy < ApplicationPolicy
     def index?
-      user.admin?
+      @user.admin?
     end
 
     class Scope

app/policies/stash_engine/admin_datasets/curation_table_row_policy.rb

@@ -11,19 +11,19 @@ def initialize(user, dataset)
 
       def curation_activity_change?
         super &&
-        dataset.resource&.permission_to_edit?(user: user) &&
-        dataset.resource_state == 'submitted'
+        @dataset.resource&.permission_to_edit?(user: @user) &&
+        @dataset.resource_state == 'submitted'
       end
 
       def current_editor_change?
         super &&
-        dataset.resource&.permission_to_edit?(user: user) &&
-        dataset.resource&.curatable?
+        @dataset.resource&.permission_to_edit?(user: @user) &&
+        @dataset.resource&.curatable?
       end
 
       def edit?
-        (dataset.resource&.permission_to_edit?(user: user) && dataset.resource_state == 'submitted') ||
-        (dataset.resource_state == 'in_progress' && dataset.resource.current_editor_id == user.id)
+        (@dataset.resource&.permission_to_edit?(user: @user) && @dataset.resource_state == 'submitted') ||
+        (@dataset.resource_state == 'in_progress' && @dataset.resource.current_editor_id == @user.id)
       end
 
     end

app/policies/stash_engine/admin_datasets_policy.rb

@@ -2,7 +2,7 @@ module StashEngine
   class AdminDatasetsPolicy < ApplicationPolicy
 
     def index?
-      user.admin?
+      @user.admin?
     end
 
     def activity_log?
@@ -18,27 +18,27 @@ def data_popup?
     end
 
     def curation_activity_change?
-      user.curator?
+      @user.curator?
     end
 
     def curation_activity_popup?
       curation_activity_change?
     end
 
     def current_editor_change?
-      user.curator?
+      @user.curator?
     end
 
     def current_editor_popup?
       current_editor_change?
     end
 
     def create_salesforce_case?
-      user.limited_curator?
+      @user.limited_curator?
     end
 
     def waiver_add?
-      user.superuser?
+      @user.superuser?
     end
 
     def waiver_popup?

app/policies/stash_engine/gmail_auth_policy.rb

@@ -8,7 +8,7 @@ def initialize(user, _record)
     end
 
     def index?
-      user.superuser?
+      @user.superuser?
     end
   end
 end

app/policies/stash_engine/identifier_policy.rb

@@ -7,6 +7,32 @@ def initialize(user, scope)
         @scope = scope
       end
 
+      def resolve
+        if !@user || @user.nil?
+          @scope.publicly_viewable
+        elsif @user.limited_curator?
+          @scope.all
+        else
+          tenant_admin = (@user.tenant_id if @user.role == 'admin')
+          @scope.with_visibility(states: %w[published embargoed],
+                                 tenant_id: tenant_admin,
+                                 journal_issns: @user.journals_as_admin.map(&:single_issn),
+                                 funder_ids: @user.funders_as_admin.map(&:funder_id),
+                                 user_id: @user.id)
+        end
+      end
+
+      private
+
+      attr_reader :user, :scope
+    end
+
+    class DashboardScope
+      def initialize(user, scope)
+        @user = user
+        @scope = scope
+      end
+
       def resolve
         @scope
           .joins(latest_resource: :last_curation_activity)

app/policies/stash_engine/resource_policy.rb

@@ -23,5 +23,31 @@ def resolve
 
       attr_reader :user, :scope
     end
+
+    class VersionScope
+      def initialize(user, scope)
+        @user = user
+        @scope = scope
+      end
+
+      def resolve
+        if @user.nil?
+          @scope.with_visibility(states: %w[published embargoed])
+        elsif @user.limited_curator?
+          @scope.all
+        else
+          tenant_admin = (@user.tenant_id if @user.role == 'admin')
+          @scope.with_visibility(states: %w[published embargoed],
+                                 tenant_id: tenant_admin,
+                                 funder_ids: @user.funders_as_admin.map(&:funder_id),
+                                 journal_issns: @user.journals_as_admin.map(&:single_issn),
+                                 user_id: @user.id)
+        end
+      end
+
+      private
+
+      attr_reader :user, :scope
+    end
   end
 end

app/policies/stash_engine/tenant_policy.rb

@@ -1,7 +1,7 @@
 module StashEngine
   class TenantPolicy < ApplicationPolicy
     def index?
-      user.present?
+      @user.present?
     end
 
     class Scope

spec/models/stash_engine/identifier_spec.rb

@@ -964,19 +964,19 @@ module StashEngine
       end
 
       it 'user_viewable for a regular user' do
-        identifiers = Identifier.user_viewable(user: @user)
+        identifiers = Pundit.policy_scope!(@user, Identifier)
         expect(identifiers.count).to eq(6) # 5 public plus mine in curation
         expect(identifiers.map(&:id)).to include(@identifiers[1].id) # this is my private one
       end
 
       it 'user_viewable for an admin' do
-        identifiers = Identifier.user_viewable(user: @user2)
+        identifiers = Pundit.policy_scope!(@user2, Identifier)
         expect(identifiers.count).to eq(6)
         expect(identifiers.map(&:id)).to include(@identifiers[1].id) # this is some ucop joe blow private one
       end
 
       it 'user_viewable for a curator, they love it all' do
-        identifiers = Identifier.user_viewable(user: @user3)
+        identifiers = Pundit.policy_scope!(@user3, Identifier)
         expect(identifiers.count).to eq(@identifiers.length)
       end
 

spec/models/stash_engine/resources_spec.rb

@@ -1511,36 +1511,36 @@ module StashEngine
           end
 
           it 'shows all resources for the identifier to the curator' do
-            resources = @identifier.resources.visible_to_user(user: @user3)
+            resources = StashEngine::ResourcePolicy::VersionScope.new(@user3, @identifier.resources).resolve
             expect(resources.count).to eq(3)
           end
 
           it 'shows all resources to the owner' do
-            resources = @identifier.resources.visible_to_user(user: @user3)
+            resources = StashEngine::ResourcePolicy::VersionScope.new(@user3, @identifier.resources).resolve
             expect(resources.count).to eq(3)
           end
 
           it 'only shows curated-visible resources to a non-user' do
-            resources = @identifier.resources.visible_to_user(user: nil)
+            resources = StashEngine::ResourcePolicy::VersionScope.new(nil, @identifier.resources).resolve
             expect(resources.count).to eq(2)
           end
 
           it 'shows all resources to an admin for this tenant (ucop)' do
-            resources = @identifier.resources.visible_to_user(user: @user2)
+            resources = StashEngine::ResourcePolicy::VersionScope.new(@user2, @identifier.resources).resolve
             expect(resources.count).to eq(3)
           end
 
           it 'shows all resources to an admin for this journal' do
             journal = Journal.create(title: 'Test Journal', issn: '1234-4321')
             InternalDatum.create(identifier_id: @identifier.id, data_type: 'publicationISSN', value: journal.single_issn)
             JournalRole.create(journal: journal, user: @user2, role: 'admin')
-            resources = @identifier.resources.visible_to_user(user: @user2)
+            resources = StashEngine::ResourcePolicy::VersionScope.new(@user2, @identifier.resources).resolve
             expect(resources.count).to eq(3)
           end
 
           it 'only shows curated-visible resources to a random user' do
             @user4 = create(:user, first_name: 'Gorgon', last_name: 'Grup', email: '[email protected]', tenant_id: 'ucb', role: 'user')
-            resources = @identifier.resources.visible_to_user(user: @user4)
+            resources = StashEngine::ResourcePolicy::VersionScope.new(@user4, @identifier.resources).resolve
             expect(resources.count).to eq(2)
           end
         end