[B] Correct authorizer discrepancies

* Remove errant `destroy` verb from authority ability mapping
* Ensure that project creation is explicitly checked and authorized
* Validate creating/updating journal issues for journal editors with
  an existing project
* Correct typo in permission authorizer spec to actually check
  a permission record

Author: scryptmouse

api/app/authorizers/permission_authorizer.rb

@@ -1,13 +1,6 @@
-class PermissionAuthorizer < ApplicationAuthorizer
-
-  def self.default(_able, user, options = {})
-    return true if editor_permissions?(user)
-    return false unless options[:for]
-    return false unless options[:for].respond_to? :permissions_manageable_by?
-
-    options[:for].permissions_manageable_by? user
-  end
+# frozen_string_literal: true
 
+class PermissionAuthorizer < ApplicationAuthorizer
   def default(_able, user, _options = {})
     return true if creator_or_has_editor_permissions?(user, resource)
     return false unless resource.resource
@@ -16,4 +9,13 @@ def default(_able, user, _options = {})
     resource.resource.permissions_manageable_by? user
   end
 
+  class << self
+    def default(_able, user, options = {})
+      return true if editor_permissions?(user)
+      return false unless options[:for]
+      return false unless options[:for].respond_to? :permissions_manageable_by?
+
+      options[:for].permissions_manageable_by? user
+    end
+  end
 end

api/app/authorizers/project_authorizer.rb

@@ -25,6 +25,23 @@ class ProjectAuthorizer < ApplicationAuthorizer
     ]
   end.freeze
 
+  # @note This should not actually be caught by any verbs for this model,
+  #   but we add it here to prevent the authorizer from short-circuiting
+  #   to the necessarily-permissive class-level {ProjectAuthorizer.default}.
+  def default(_adjective, user, _options = {})
+    # :nocov:
+    has_any_role?(user, :admin)
+    # :nocov:
+  end
+
+  # Admins, editors, marketeers, and project creators can create projects.
+  #
+  # @param [User] user
+  # @param [Hash] _options
+  def creatable_by?(user, _options = {})
+    has_any_role?(user, :admin, :editor, :marketeer, :project_creator)
+  end
+
   # First, we check to see if the project is a draft. If so, {#drafts_readable_by? it must be readable}.
   # Otherwise, we allow a project to be read.
   #
@@ -43,7 +60,7 @@ def readable_by?(user, options = {})
   # @param [User] user
   # @param [Hash] _options
   def updatable_by?(user, options = {})
-    has_any_role?(user, :admin, :editor, :marketeer, :project_editor) || with_journal { |j| j.updatable_by? user, options}
+    has_any_role?(user, :admin, :editor, :marketeer, :project_editor) || with_journal { |j| j.updatable_by? user, options }
   end
 
   # @!group Project Property Access Control
@@ -173,7 +190,7 @@ def default(_adjective, _user, _options = {})
     #
     # @param [User] user
     # @param [Hash] _options
-    def creatable_by?(user, options = {})
+    def creatable_by?(user, _options = {})
       project_creator_permissions?(user) || user.journal_editor?
     end
 
@@ -183,7 +200,7 @@ def creatable_by?(user, options = {})
     #
     # @param [User] user
     # @param [Hash] _options
-    def deletable_by?(user, options = {})
+    def deletable_by?(user, _options = {})
       has_any_role?(user, :admin, :editor, :project_editor)
     end
 

api/app/controllers/api/v1/journal_issues_controller.rb

@@ -1,7 +1,10 @@
+# frozen_string_literal: true
+
 module API
   module V1
     # Journal Issues controller
     class JournalIssuesController < ApplicationController
+      include AuthorizesJSONAPIRelationships
 
       resourceful! JournalIssue, authorize_options: { except: [:index, :show] } do
         JournalIssue.filtered(
@@ -11,6 +14,8 @@ class JournalIssuesController < ApplicationController
         )
       end
 
+      before_action :maybe_enforce_project_access!, only: :update
+
       def index
         @journal_issues = load_journal_issues
         render_multiple_resources @journal_issues, include: ["journal"]
@@ -34,7 +39,14 @@ def destroy
         @journal_issue.destroy
       end
 
-      protected
+      private
+
+      # Check to make sure that the user is authorized to update the project, if present.
+      #
+      # @return [void]
+      def maybe_enforce_project_access!
+        authorize_jsonapi_relationship_in! journal_issue_params, Project
+      end
 
       def includes
         [:journal, :journal_volume, :project, "project.creators", "project.contributors",
@@ -51,7 +63,6 @@ def scope_visibility
       def scope_for_journal_issues
         JournalIssue.all
       end
-
     end
   end
 end

api/app/controllers/api/v1/journals/relationships/journal_issues_controller.rb

@@ -1,15 +1,20 @@
+# frozen_string_literal: true
+
 module API
   module V1
     module Journals
       module Relationships
         class JournalIssuesController < AbstractJournalChildController
+          include AuthorizesJSONAPIRelationships
 
           resourceful! JournalIssue, authorize_options: { except: [:index] } do
             JournalIssue.filtered(
               with_pagination!(journal_issue_filter_params), scope: @journal.journal_issues.in_reverse_order, user: current_user
             )
           end
 
+          before_action :maybe_enforce_project_access!, only: :create
+
           def index
             @journal_issues = load_journal_issues
             location = api_v1_journal_relationships_journal_issues_url(@journal.id)
@@ -19,24 +24,31 @@ def index
 
           def create
             inputs = {}
+
             inputs[:params] = journal_issue_params.to_h
             inputs[:creator] = current_user
             inputs[:journal] = @journal
 
             authorize_action_for JournalIssue.new(journal: @journal)
 
             outcome = JournalIssues::Create.run inputs
+
             render_single_resource outcome.result
           end
 
           private
 
+          # Check to make sure that the user is authorized to update the project, if present.
+          # @return [void]
+          def maybe_enforce_project_access!
+            authorize_jsonapi_relationship_in! journal_issue_params, Project
+          end
+
           def location
             return "" unless @journal_issue.persisted?
 
             api_v1_journal_issue_url(@journal_issue)
           end
-
         end
       end
     end

api/app/controllers/concerns/authorizes_json_api_relationships.rb

@@ -0,0 +1,39 @@
+# frozen_string_literal: true
+
+module AuthorizesJSONAPIRelationships
+  extend ActiveSupport::Concern
+
+  # Occasionally we need to authorize based on attributes in a relationship.
+  #
+  # This provides a standardized way to do so based on params constructed in {Validation}.
+  #
+  # @param [ActionController::Parameters, #dig] params
+  # @param [Symbol] relationship_key
+  # @param [Class<ApplicationRecord>] model_klass
+  # @param [Symbol] action
+  # @return [void]
+  def authorize_jsonapi_relationship_in!(params, model_klass, action: :update, relationship_key: model_klass.model_name.i18n_key)
+    id = params.dig(:data, :relationships, relationship_key, :data, :id)
+
+    record = load_jsonapi_relationship_for model_klass, id
+
+    # :nocov:
+    return if record.blank?
+    # :nocov:
+
+    Authority.enforce action, record, authority_user
+  end
+
+  # @param [Class<ApplicationRecord>] model_klass
+  # @param [String] id
+  # @return [ApplicationRecord, nil]
+  def load_jsonapi_relationship_for(model_klass, id)
+    case id
+    when model_klass then id
+    when String then model_klass.find id
+    end
+  rescue ActiveRecord::RecordNotFound
+    # intentionally left blank, we don't try to authorize non-existing records
+    # and the failure should be caught elsewhere in the validation stack.
+  end
+end

api/app/services/journal_issues/create.rb

@@ -1,25 +1,28 @@
+# frozen_string_literal: true
+
 module JournalIssues
   class Create < ActiveInteraction::Base
-
     isolatable!
 
     transactional!
 
     record :creator, class: "User"
     record :journal
+
     # Params are filtered in strong parameters with journal_issue_params
     hash :params, strip: false
 
     attr_reader :journal_issue
 
     # @return [JournalIssue]
     def execute
-      @journal_issue =
-        ::Updaters::Default.new(params).update(journal.journal_issues.new(creator: creator))
+      @journal_issue = journal.journal_issues.new(creator: creator)
+
+      ::Updaters::Default.new(params).update(@journal_issue)
 
       create_and_assign_project unless @journal_issue.project.present?
 
-      journal_issue.save if journal_issue.valid?
+      journal_issue.save
 
       return journal_issue
     end
@@ -32,12 +35,12 @@ def create_and_assign_project
       title_parts.push "no. #{journal_issue.number}"
 
       project = Project.create(title: title_parts.join(", "), creator: creator)
-      Content::ScaffoldProjectContent.run project: project,
-                                          configuration: {
-                                            multiple_texts: true
-                                          }
+
+      inputs = { project: project, configuration: { multiple_texts: true } }
+
+      compose Content::ScaffoldProjectContent, inputs
+
       @journal_issue.project = project
     end
-
   end
 end

api/config/initializers/authority.rb

@@ -46,7 +46,6 @@
     read: "readable",
     update: "updatable",
     delete: "deletable",
-    destroy: "deleteable",
     read_deleted: "deleted_readable",
     read_drafts: "drafts_readable",
     fully_read: "fully_readable",

api/db/structure.sql

@@ -7373,3 +7373,5 @@ INSERT INTO "schema_migrations" (version) VALUES
 ('20250129200019'),
 ('20250210192150'),
 ('20250210230256');
+
+

api/spec/authorizers/comment_authorizer_spec.rb

@@ -9,7 +9,7 @@
 
     the_subject_behaves_like "instance abilities", Comment, create: false, read: true, update: false, delete: false
 
-    the_subject_behaves_like "class abilities", Comment, create: false, read: true, update: false, destroy: false
+    the_subject_behaves_like "class abilities", Comment, create: false, read: true, update: false, delete: false
   end
 
   context "when the subject is an admin" do
@@ -34,7 +34,7 @@
 
       the_subject_behaves_like "instance abilities", Comment, create: true, read: true, update: false, delete: false
 
-      the_subject_behaves_like "class abilities", Comment, create: true, read: true, update: true, destroy: true
+      the_subject_behaves_like "class abilities", Comment, create: true, read: true, update: true, delete: true
 
     end
 
@@ -45,14 +45,14 @@
 
       the_subject_behaves_like "instance abilities", Comment, create: false, read: true, update: false, delete: false
 
-      the_subject_behaves_like "class abilities", Comment, create: false, read: true, update: true, destroy: true
+      the_subject_behaves_like "class abilities", Comment, create: false, read: true, update: true, delete: true
     end
   end
 
   context "when the subject is the resource creator" do
     let_it_be(:subject, refind: true) { creator }
 
-    the_subject_behaves_like "instance abilities", Comment, read: true, update: true, destroy: true
+    the_subject_behaves_like "instance abilities", Comment, read: true, update: true, delete: true
   end
 
   context "when the comment is on an annotation in a closed project with disabled engagement" do
@@ -79,7 +79,7 @@
         subject.clear_email_confirmation!
       end
 
-      the_subject_behaves_like "instance abilities", Comment, create: false, read: true, update: true, destroy: true
+      the_subject_behaves_like "instance abilities", Comment, create: false, read: true, update: true, delete: true
     end
   end
 

api/spec/authorizers/export_target_authorizer_spec.rb

@@ -21,6 +21,6 @@
     let(:user) { FactoryBot.create :user, :editor }
     let!(:export_target) { FactoryBot.create :export_target }
 
-    it { is_expected.to be_able_to(:read).on(export_target).and be_unable_to(:create, :update, :destroy).on(export_target) }
+    it { is_expected.to be_able_to(:read).on(export_target).and be_unable_to(:create, :update, :delete).on(export_target) }
   end
 end