Stop auto-generating auth bypass tokens on edition creation

Remove the before_create callback that assigned every edition an
auth_bypass_id, making preview token generation opt-in: a draft has no
token until a publisher generates one.

Editions now have no token by default: the factory carries a
:with_auth_bypass_id trait for the cases that still need one, and the
tests assert the empty-token default (presenter payloads serialise an
empty auth_bypass_ids array).

Author: TonyGDS

app/models/edition.rb

@@ -77,7 +77,6 @@ class Edition < ApplicationRecord
   POST_PUBLICATION_STATES = %w[published superseded withdrawn unpublished].freeze
   PUBLICLY_VISIBLE_STATES = %w[published withdrawn].freeze
 
-  before_create :set_auth_bypass_id
   before_save :set_public_timestamp
   after_validation :update_revalidated_at, if: -> { validation_context == :publish }
   after_create :update_document_edition_references

test/components/admin/editions/show/preview_component_test.rb

@@ -62,7 +62,7 @@ class Admin::Editions::Show::PreviewComponentTest < ViewComponent::TestCase
   end
 
   test "renders the copy link, regenerate and delete controls when the edition has a preview token" do
-    edition = build_stubbed(:publication, document: @document)
+    edition = build_stubbed(:publication, :with_auth_bypass_id, document: @document)
 
     render_inline(Admin::Editions::Show::PreviewComponent.new(edition:))
 

test/factories/editions.rb

@@ -8,7 +8,10 @@
     change_note { "change-note" }
     summary { "edition-summary" }
     previously_published { false }
-    auth_bypass_id { SecureRandom.uuid }
+
+    trait(:with_auth_bypass_id) do
+      auth_bypass_id { SecureRandom.uuid }
+    end
 
     trait(:with_organisations) do
       transient do

test/functional/admin/editions_controller_test.rb

@@ -369,20 +369,18 @@ class Admin::EditionsControllerTest < ActionController::TestCase
     assert_not flash["html_safe"]
   end
 
-  test "update_bypass_id generates a new preview token and redirects with a notice" do
+  test "update_bypass_id generates a preview token and redirects with a notice" do
     edition = create(:draft_publication)
-    previous_auth_bypass_id = edition.auth_bypass_id
 
     patch :update_bypass_id, params: { id: edition }
 
     assert_not_nil edition.reload.auth_bypass_id
-    assert_not_equal previous_auth_bypass_id, edition.auth_bypass_id
     assert_redirected_to admin_publication_path(edition)
     assert_equal "New document preview link generated", flash[:notice]
   end
 
   test "destroy_bypass_id deletes the preview token and redirects with a notice" do
-    edition = create(:draft_publication)
+    edition = create(:draft_publication, :with_auth_bypass_id)
     assert_not_nil edition.auth_bypass_id
 
     delete :destroy_bypass_id, params: { id: edition }

test/functional/admin/file_attachments_controller_test.rb

@@ -84,7 +84,13 @@ def valid_file_attachment_params
     attachment = create(:file_attachment, attachable: @edition)
     model_type = attachment.attachment_data.class.to_s
 
-    AssetManagerCreateAssetJob.expects(:perform_async).with(anything, has_entries("assetable_id" => kind_of(Integer), "asset_variant" => Asset.variants[:original], "assetable_type" => model_type), anything, @edition.class.to_s, @edition.id, [@edition.auth_bypass_id])
+    AssetManagerCreateAssetJob.expects(:perform_async).with do |_path, asset_params, _draft, attachable_class, attachable_id, _auth_bypass_ids|
+      asset_params["assetable_id"].is_a?(Integer) &&
+        asset_params["asset_variant"] == Asset.variants[:original] &&
+        asset_params["assetable_type"] == model_type &&
+        attachable_class == @edition.class.to_s &&
+        attachable_id == @edition.id
+    end
 
     put :update,
         params: {

test/functional/admin/uploads_controller_test.rb

@@ -120,7 +120,13 @@ def post_to_upload_files(*files)
   test "POST :create triggers a job to be queued to store the attachments in Asset Manager" do
     model_type = AttachmentData.to_s
 
-    AssetManagerCreateAssetJob.expects(:perform_async).with(anything, has_entries("assetable_id" => kind_of(Integer), "asset_variant" => Asset.variants[:original], "assetable_type" => model_type), anything, @edition.class.to_s, @edition.id, [@edition.auth_bypass_id]).twice
+    AssetManagerCreateAssetJob.expects(:perform_async).with { |_path, asset_params, _draft, attachable_class, attachable_id, _auth_bypass_ids|
+      asset_params["assetable_id"].is_a?(Integer) &&
+        asset_params["asset_variant"] == Asset.variants[:original] &&
+        asset_params["assetable_type"] == model_type &&
+        attachable_class == @edition.class.to_s &&
+        attachable_id == @edition.id
+    }.twice
 
     post :create, params: { edition_id: @edition, upload: valid_create_params }
   end

test/integration/asset_access_options_integration_test.rb

@@ -23,57 +23,83 @@ class AssetAccessOptionsIntegrationTest < ActionDispatch::IntegrationTest
       stub_asset(asset_manager_id, draft: true)
     end
 
-    context "given a draft document with file attachment" do
-      let(:edition) { create(:detailed_guide, organisations: [organisation]) }
-
+    context "forwarding the preview token to Asset Manager" do
       before do
-        add_file_attachment_with_asset("sample.docx", to: edition)
+        add_file_attachment_with_asset("logo.png", to: edition)
         edition.save!
       end
 
-      context "when document is marked as access limited in Whitehall" do
+      context "when the draft has a preview token" do
+        let(:edition) { create(:detailed_guide, :with_auth_bypass_id) }
+
+        it "sends the attachment with the document's auth_bypass_id" do
+          Services.asset_manager.expects(:create_asset).at_least_once.with(
+            has_entries(auth_bypass_ids: [edition.auth_bypass_id]),
+          ).returns(asset_manager_response)
+
+          AssetManagerCreateAssetJob.drain
+        end
+      end
+
+      context "when the draft has no preview token" do
+        let(:edition) { create(:detailed_guide) }
+
+        it "sends the attachment with an empty auth_bypass_ids" do
+          Services.asset_manager.expects(:create_asset).at_least_once.with(
+            has_entries(auth_bypass_ids: []),
+          ).returns(asset_manager_response)
+
+          AssetManagerCreateAssetJob.drain
+        end
+      end
+    end
+
+    context "applying access limiting to a draft's attachments" do
+      context "when a draft is marked as access limited" do
+        let(:edition) { create(:detailed_guide, organisations: [organisation]) }
+
         before do
+          add_file_attachment_with_asset("sample.docx", to: edition)
+          edition.save!
           visit edit_admin_edition_path(edition)
           check "Limit access"
           click_button "Save"
           assert_text "Your document has been saved"
         end
 
-        it "marks attachment as access limited in Asset Manager" do
+        it "marks the attachment as access limited in Asset Manager" do
           Services.asset_manager
                   .expects(:update_asset)
                   .at_least_once.with(asset_manager_id, has_entry("access_limited_organisation_ids", [organisation.content_id]))
 
           AssetManagerAttachmentMetadataJob.drain
         end
       end
-    end
 
-    context "given a draft document with an image attachment" do
-      let(:edition) { create(:draft_detailed_guide) }
+      context "when a draft is unmarked as access limited" do
+        let(:edition) { create(:detailed_guide, organisations: [organisation], access_limited: true) }
 
-      before do
-        visit admin_edition_path(edition)
-        click_link "Edit draft"
-        click_link "Images"
-        attach_file "images[][image_data_attributes][file]", path_to_attachment("minister-of-funk.960x640.jpg")
-        click_button "Upload"
-      end
+        before do
+          add_file_attachment_with_asset("sample.docx", to: edition)
+          edition.save!
+          visit edit_admin_edition_path(edition)
+          uncheck "Limit access"
+          click_button "Save"
+          assert_text "Your document has been saved"
+        end
 
-      # Note that there is no access limiting applied to non attachments. This is existing behaviour that probably needs changing.
-      it "sends an image to asset manager with the document's auth_bypass_id" do
-        Services.asset_manager.expects(:create_asset).at_least_once.with(
-          has_entry(auth_bypass_ids: [edition.auth_bypass_id]),
-        ).returns(asset_manager_response)
+        it "unmarks the attachment as access limited in Asset Manager" do
+          Services.asset_manager
+                  .expects(:update_asset)
+                  .at_least_once.with(asset_manager_id, has_entry("access_limited_organisation_ids", []))
 
-        AssetManagerCreateAssetJob.drain
+          AssetManagerAttachmentMetadataJob.drain
+        end
       end
-    end
 
-    context "given an access-limited draft document" do
-      let(:edition) { create(:detailed_guide, organisations: [organisation], access_limited: true) }
+      context "when an attachment is added to an access-limited draft" do
+        let(:edition) { create(:detailed_guide, organisations: [organisation], access_limited: true) }
 
-      context "when an attachment is added to the draft document" do
         before do
           visit admin_edition_path(edition)
           click_link "Add attachments"
@@ -84,46 +110,18 @@ class AssetAccessOptionsIntegrationTest < ActionDispatch::IntegrationTest
           assert_text "Attachment 'logo.png' uploaded"
         end
 
-        it "marks attachment as access limited and sends it with an auth_bypass_id in Asset Manager" do
+        it "marks the attachment as access limited in Asset Manager" do
           Services.asset_manager.expects(:create_asset).with(
-            has_entries(
-              access_limited_organisation_ids: [organisation.content_id],
-              auth_bypass_ids: [edition.auth_bypass_id],
-            ),
+            has_entries(access_limited_organisation_ids: [organisation.content_id]),
           ).returns(asset_manager_response)
 
           AssetManagerCreateAssetJob.drain
         end
       end
 
-      context "when an html attachment is added to the draft document" do
-        let(:edition) { create(:publication, :policy_paper) }
-
-        before do
-          visit admin_edition_path(edition)
-          click_link "Edit attachments"
-          click_link "Add new HTML attachment"
-          fill_in "Title", with: "html-attachment"
-          fill_in "Body", with: "some html content"
-        end
-
-        it "sends an html attachment to publishing api with its edition's auth_bypass_id" do
-          Services.publishing_api.expects(:put_content)
-                  .with(anything, has_entries(title: edition.title))
-          Services.publishing_api.expects(:put_content)
-                  .with(anything, has_entries(title: edition.attachments.first.title))
+      context "when multiple files are uploaded to an access-limited draft" do
+        let(:edition) { create(:detailed_guide, organisations: [organisation], access_limited: true) }
 
-          Services.publishing_api.expects(:put_content).at_least_once
-                  .with(anything, has_entries(
-                                    title: "html-attachment",
-                                    auth_bypass_ids: [edition.auth_bypass_id],
-                                  ))
-
-          click_button "Save"
-        end
-      end
-
-      context "when uploaded to draft document" do
         before do
           visit admin_edition_path(edition)
           click_link "Add attachments"
@@ -136,49 +134,24 @@ class AssetAccessOptionsIntegrationTest < ActionDispatch::IntegrationTest
           assert find("li", text: "logo.png")
         end
 
-        it "marks attachment as access limited in Asset Manager" do
+        it "marks each attachment as access limited in Asset Manager" do
           Services.asset_manager
                   .expects(:create_asset)
                   .at_least(2)
                   .with(
-                    has_entries(
-                      access_limited_organisation_ids: [organisation.content_id],
-                      auth_bypass_ids: [edition.auth_bypass_id],
-                    ),
+                    has_entries(access_limited_organisation_ids: [organisation.content_id]),
                   ).returns(asset_manager_response)
 
           AssetManagerCreateAssetJob.drain
         end
       end
-    end
 
-    context "given an access-limited draft document and a file attachment" do
-      let(:edition) { create(:detailed_guide, organisations: [organisation], access_limited: true) }
-
-      before do
-        add_file_attachment_with_asset("sample.docx", to: edition)
-        edition.save!
-      end
+      context "when an attachment is replaced on an access-limited draft" do
+        let(:edition) { create(:detailed_guide, organisations: [organisation], access_limited: true) }
 
-      context "when document is unmarked as access limited in Whitehall" do
-        before do
-          visit edit_admin_edition_path(edition)
-          uncheck "Limit access"
-          click_button "Save"
-          assert_text "Your document has been saved"
-        end
-
-        it "unmarks attachment as access limited in Asset Manager" do
-          Services.asset_manager
-                  .expects(:update_asset)
-                  .at_least_once.with(asset_manager_id, has_entry("access_limited_organisation_ids", []))
-
-          AssetManagerAttachmentMetadataJob.drain
-        end
-      end
-
-      context "when attachment is replaced" do
         before do
+          add_file_attachment_with_asset("sample.docx", to: edition)
+          edition.save!
           visit admin_edition_path(edition)
           click_link "Edit attachments"
           click_link "Edit"
@@ -187,25 +160,20 @@ class AssetAccessOptionsIntegrationTest < ActionDispatch::IntegrationTest
           assert_text "Attachment 'sample.docx' updated"
         end
 
-        it "marks replacement attachment as access limited in Asset Manager" do
+        it "marks the replacement attachment as access limited in Asset Manager" do
           Services.asset_manager.stubs(:create_asset).returns(asset_manager_response)
-          Services.asset_manager.expects(:create_asset).with { |params|
-            params[:access_limited_organisation_ids] == [organisation.content_id] &&
-              params[:auth_bypass_ids] == [edition.auth_bypass_id]
-          }.returns(asset_manager_response)
+          Services.asset_manager.expects(:create_asset).with { |params| params[:access_limited_organisation_ids] == [organisation.content_id] }.returns(asset_manager_response)
 
           AssetManagerCreateAssetJob.drain
         end
       end
-    end
 
-    context "given a draft access-limited consultation" do
-      # the edition has to have same organisation as logged in user, otherwise it's not visible when access_limited = true
-      let(:edition) { create(:consultation, organisations: [organisation], access_limited: true) }
-      let(:outcome_attributes) { FactoryBot.attributes_for(:consultation_outcome) }
-      let!(:outcome) { edition.create_outcome!(outcome_attributes) }
+      context "when an attachment is added to an access-limited consultation outcome" do
+        # the edition has to have same organisation as logged in user, otherwise it's not visible when access_limited = true
+        let(:edition) { create(:consultation, organisations: [organisation], access_limited: true) }
+        let(:outcome_attributes) { FactoryBot.attributes_for(:consultation_outcome) }
+        let!(:outcome) { edition.create_outcome!(outcome_attributes) }
 
-      context "when an attachment is added to the consultation's outcome" do
         before do
           visit admin_consultation_path(edition)
           click_link "Edit draft"
@@ -217,33 +185,13 @@ class AssetAccessOptionsIntegrationTest < ActionDispatch::IntegrationTest
           assert_text "Attachment 'asset-title' uploaded"
         end
 
-        it "marks attachment as access limited in Asset Manager and sends with the consultation's auth_bypass_id" do
+        it "marks the attachment as access limited in Asset Manager" do
           Services.asset_manager.expects(:create_asset).with(
-            has_entries(
-              access_limited_organisation_ids: [organisation.content_id],
-              auth_bypass_ids: [edition.auth_bypass_id],
-            ),
+            has_entries(access_limited_organisation_ids: [organisation.content_id]),
           ).returns(asset_manager_response)
           AssetManagerCreateAssetJob.drain
         end
       end
-
-      it "sends a consultation form to asset manager with the consultation's auth_bypass_id" do
-        visit admin_consultation_path(edition)
-        click_link "Edit draft"
-        name_of_form_uploader = "edition[consultation_participation_attributes][consultation_response_form_attributes][consultation_response_form_data_attributes][file]"
-        fill_in "edition[consultation_participation_attributes][consultation_response_form_attributes][title]", with: "Consultation response form"
-        attach_file name_of_form_uploader, path_to_attachment("simple.pdf")
-        click_button "Save"
-
-        # Note that there is no access limiting applied to non attachments. This is existing behaviour that probably needs changing.
-        Services.asset_manager.expects(:create_asset).with { |args|
-          args[:file].path =~ /simple\.pdf/
-          args[:auth_bypass_ids] == [edition.auth_bypass_id]
-        }.returns(asset_manager_response)
-
-        AssetManagerCreateAssetJob.drain
-      end
     end
 
   private