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.

Move the factory's auth_bypass_id into a :with_auth_bypass_id trait so
test editions match production (no token by default), give that trait
only to the tests asserting a token is present, and have the canonical
presenter snapshots assert 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

features/preview-unpublished-editions.feature

@@ -15,8 +15,8 @@ Feature: Previewing unpublished editions
   Scenario: Generating and deleting a shareable document preview link
     Given I am an editor
     And a draft publication "Test publication" exists
-    Then there should be a shareable preview link for the publication "Test publication"
-    When I delete the shareable preview link for the publication "Test publication"
     Then there should not be a shareable preview link for the publication "Test publication"
     When I generate a shareable preview link for the publication "Test publication"
     Then there should be a shareable preview link for the publication "Test publication"
+    When I delete the shareable preview link for the publication "Test publication"
+    Then there should not be a shareable preview link for the publication "Test publication"

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,7 @@ 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(anything, has_entries("assetable_id" => kind_of(Integer), "asset_variant" => Asset.variants[:original], "assetable_type" => model_type), anything, @edition.class.to_s, @edition.id, anything)
 
     put :update,
         params: {

test/functional/admin/uploads_controller_test.rb

@@ -120,7 +120,7 @@ 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(anything, has_entries("assetable_id" => kind_of(Integer), "asset_variant" => Asset.variants[:original], "assetable_type" => model_type), anything, @edition.class.to_s, @edition.id, anything).twice
 
     post :create, params: { edition_id: @edition, upload: valid_create_params }
   end

test/integration/asset_access_options_integration_test.rb

@@ -50,7 +50,7 @@ class AssetAccessOptionsIntegrationTest < ActionDispatch::IntegrationTest
     end
 
     context "given a draft document with an image attachment" do
-      let(:edition) { create(:draft_detailed_guide) }
+      let(:edition) { create(:draft_detailed_guide, :with_auth_bypass_id) }
 
       before do
         visit admin_edition_path(edition)
@@ -71,7 +71,7 @@ class AssetAccessOptionsIntegrationTest < ActionDispatch::IntegrationTest
     end
 
     context "given an access-limited draft document" do
-      let(:edition) { create(:detailed_guide, organisations: [organisation], access_limited: true) }
+      let(:edition) { create(:detailed_guide, :with_auth_bypass_id, organisations: [organisation], access_limited: true) }
 
       context "when an attachment is added to the draft document" do
         before do
@@ -97,7 +97,7 @@ class AssetAccessOptionsIntegrationTest < ActionDispatch::IntegrationTest
       end
 
       context "when an html attachment is added to the draft document" do
-        let(:edition) { create(:publication, :policy_paper) }
+        let(:edition) { create(:publication, :policy_paper, :with_auth_bypass_id) }
 
         before do
           visit admin_edition_path(edition)
@@ -153,7 +153,7 @@ class AssetAccessOptionsIntegrationTest < ActionDispatch::IntegrationTest
     end
 
     context "given an access-limited draft document and a file attachment" do
-      let(:edition) { create(:detailed_guide, organisations: [organisation], access_limited: true) }
+      let(:edition) { create(:detailed_guide, :with_auth_bypass_id, organisations: [organisation], access_limited: true) }
 
       before do
         add_file_attachment_with_asset("sample.docx", to: edition)
@@ -201,7 +201,7 @@ class AssetAccessOptionsIntegrationTest < ActionDispatch::IntegrationTest
 
     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(:edition) { create(:consultation, :with_auth_bypass_id, organisations: [organisation], access_limited: true) }
       let(:outcome_attributes) { FactoryBot.attributes_for(:consultation_outcome) }
       let!(:outcome) { edition.create_outcome!(outcome_attributes) }
 

test/integration/jobs/publishing_api_document_republishing_job_integration_test.rb

@@ -84,27 +84,30 @@ class PublishingApiDocumentRepublishingJobIntegrationTest < ActiveSupport::TestC
       publication_presenter = PublishingApiPresenters.presenter_for(edition, update_type: "republish")
       draft_publication_presenter = PublishingApiPresenters.presenter_for(draft_edition, update_type: "republish")
       html_attachment_presenter = PublishingApiPresenters.presenter_for(edition.attachments.first, update_type: "republish")
-      draft_html_attachment_presenter = PublishingApiPresenters.presenter_for(draft_edition.attachments.first, update_type: "republish")
 
       WebMock.reset!
 
+      # The live edition and its new draft share the HTML attachment's content_id and,
+      # now that neither carries an auth bypass token, present identical content, so the
+      # attachment content is PUT once per edition version.
+      html_attachment_content_request = stub_publishing_api_put_content(html_attachment_presenter.content_id, html_attachment_presenter.content)
+
       requests = [
         stub_publishing_api_patch_links(publication_presenter.content_id, links: publication_presenter.links),
         stub_publishing_api_put_content(publication_presenter.content_id, with_locale(:en) { publication_presenter.content }),
         stub_publishing_api_put_content(publication_presenter.content_id, with_locale(:es) { publication_presenter.content }),
         stub_publishing_api_publish(publication_presenter.content_id, locale: "en", update_type: nil),
         stub_publishing_api_publish(publication_presenter.content_id, locale: "es", update_type: nil),
-        stub_publishing_api_put_content(html_attachment_presenter.content_id, html_attachment_presenter.content),
         stub_publishing_api_patch_links(html_attachment_presenter.content_id, links: html_attachment_presenter.links),
         stub_publishing_api_publish(html_attachment_presenter.content_id, locale: "en", update_type: nil),
         stub_publishing_api_put_content(draft_publication_presenter.content_id, with_locale(:en) { draft_publication_presenter.content }),
         stub_publishing_api_put_content(draft_publication_presenter.content_id, with_locale(:es) { draft_publication_presenter.content }),
-        stub_publishing_api_put_content(draft_html_attachment_presenter.content_id, draft_html_attachment_presenter.content),
       ]
 
       PublishingApiDocumentRepublishingJob.new.perform(edition.document.id, false)
 
       assert_all_requested(requests)
+      assert_requested(html_attachment_content_request, times: 2)
     end
 
     test "Should only publish live edition when document is published with invalid draft" do
@@ -195,10 +198,14 @@ class PublishingApiDocumentRepublishingJobIntegrationTest < ActiveSupport::TestC
       publication_presenter = PublishingApiPresenters.presenter_for(edition, update_type: "republish")
       draft_publication_presenter = PublishingApiPresenters.presenter_for(draft_edition, update_type: "republish")
       html_attachment_presenter = PublishingApiPresenters.presenter_for(edition.attachments.first, update_type: "republish")
-      draft_html_attachment_presenter = PublishingApiPresenters.presenter_for(draft_edition.attachments.first, update_type: "republish")
 
       WebMock.reset!
 
+      # The live edition and its new draft share the HTML attachment's content_id and,
+      # now that neither carries an auth bypass token, present identical content, so the
+      # attachment content is PUT once per edition version.
+      html_attachment_content_request = stub_publishing_api_put_content(html_attachment_presenter.content_id, html_attachment_presenter.content)
+
       requests = [
         stub_publishing_api_put_content(publication_presenter.content_id, publication_presenter.content),
         stub_publishing_api_put_content(publication_presenter.content_id, with_locale(:es) { publication_presenter.content }),
@@ -212,7 +219,6 @@ class PublishingApiDocumentRepublishingJobIntegrationTest < ActiveSupport::TestC
           locale: "es",
           allow_draft: true,
         }),
-        stub_publishing_api_put_content(html_attachment_presenter.content_id, html_attachment_presenter.content),
         stub_publishing_api_patch_links(html_attachment_presenter.content_id, links: html_attachment_presenter.links),
         stub_publishing_api_unpublish(html_attachment_presenter.content_id, body: {
           type: "redirect",
@@ -222,12 +228,12 @@ class PublishingApiDocumentRepublishingJobIntegrationTest < ActiveSupport::TestC
         }),
         stub_publishing_api_put_content(draft_publication_presenter.content_id, with_locale(:en) { draft_publication_presenter.content }),
         stub_publishing_api_put_content(draft_publication_presenter.content_id, with_locale(:es) { draft_publication_presenter.content }),
-        stub_publishing_api_put_content(draft_html_attachment_presenter.content_id, draft_html_attachment_presenter.content),
       ]
 
       PublishingApiDocumentRepublishingJob.new.perform(edition.document.id, false)
 
       assert_all_requested(requests)
+      assert_requested(html_attachment_content_request, times: 2)
     end
 
     test "Should only unpublish live edition when document is unpublished with invalid draft" do

test/integration/shareable_preview_generate_new_link_test.rb

@@ -8,8 +8,8 @@ class ShareablePreviewGenerateNewLinkIntegrationTest < ActionDispatch::Integrati
   include Admin::EditionRoutesHelper
 
   describe "shareable preview generate new link feature" do
-    context "for draft documents" do
-      let(:edition) { create(:draft_fatality_notice) }
+    context "for draft documents with an existing shareable link" do
+      let(:edition) { create(:draft_fatality_notice, :with_auth_bypass_id) }
 
       before do
         create_setup(edition)