From 299bc253a439a4eb49f5a406cbd94a298d33b248 Mon Sep 17 00:00:00 2001
From: Maikel Linke <maikel@email.org.au>
Date: Fri, 21 Apr 2023 15:53:12 +1000
Subject: [PATCH] Expire report download links in a month, not 5mins

---
 app/controllers/admin/reports_controller.rb   |  2 +-
 app/models/report_blob.rb                     |  4 +++
 .../report_mailer/report_ready.html.haml      |  2 +-
 spec/system/admin/reports_spec.rb             | 33 ++++++++++++++++++-
 4 files changed, 38 insertions(+), 3 deletions(-)

diff --git a/app/controllers/admin/reports_controller.rb b/app/controllers/admin/reports_controller.rb
index 57bf771db5..eb24378f4b 100644
--- a/app/controllers/admin/reports_controller.rb
+++ b/app/controllers/admin/reports_controller.rb
@@ -77,7 +77,7 @@ module Admin
       assign_view_data
       if @blob
         @error = ".report_taking_longer_html"
-        @error_url = @blob.url
+        @error_url = @blob.expiring_service_url
       else
         @error = ".report_taking_longer"
         @error_url = ""
diff --git a/app/models/report_blob.rb b/app/models/report_blob.rb
index 08cac98bf4..c1813b1e54 100644
--- a/app/models/report_blob.rb
+++ b/app/models/report_blob.rb
@@ -33,4 +33,8 @@ class ReportBlob < ActiveStorage::Blob
   def result
     @result ||= download.force_encoding(Encoding::UTF_8)
   end
+
+  def expiring_service_url
+    url(expires_in: 1.month)
+  end
 end
diff --git a/app/views/report_mailer/report_ready.html.haml b/app/views/report_mailer/report_ready.html.haml
index 604e265150..1dc2d1e8b3 100644
--- a/app/views/report_mailer/report_ready.html.haml
+++ b/app/views/report_mailer/report_ready.html.haml
@@ -2,4 +2,4 @@
 %p
   = t(".intro")
 %ul
-  %li= link_to(t(".link_label", name: @blob.filename), @blob.url)
+  %li= link_to(t(".link_label", name: @blob.filename), @blob.expiring_service_url)
diff --git a/spec/system/admin/reports_spec.rb b/spec/system/admin/reports_spec.rb
index 3c1f604342..b3ff6a8dfc 100644
--- a/spec/system/admin/reports_spec.rb
+++ b/spec/system/admin/reports_spec.rb
@@ -95,11 +95,42 @@ describe '
       content = File.read(downloaded_filename)
       expect(content).to match "<th>\nFirst Name\n</th>"
 
+      # ActiveStorage links usually expire after 5 minutes.
+      # We need a longer expiry for a better user experience.
+      # Let's test if the link still works after a few hours.
+      Timecop.travel(3.hours.from_now) do
+        expect do
+          File.delete(downloaded_filename)
+          click_link "Download report"
+        end.to_not change { downloaded_filename }
+      end
+
       # We also get an email.
       perform_enqueued_jobs(only: ActionMailer::MailDeliveryJob)
       email = ActionMailer::Base.deliveries.last
-      expect(email.body).to have_link "customers",
+      expect(email.body).to have_link(
+        "customers",
         href: %r"^http://test\.host/rails/active_storage/disk/.*/customers_[0-9]+\.html$"
+      )
+
+      # We want to check that the emailed link works as well:
+      parsed_email = Capybara::Node::Simple.new(email.body.to_s)
+      email_link_href = parsed_email.find(:link, "customers")[:href]
+      report_link = email_link_href.sub("test.host", Rails.application.default_url_options[:host])
+      content = URI.parse(report_link).read
+      expect(content).to match "<th>\nFirst Name\n</th>"
+
+      # Let's also check the expiry of the emailed link:
+      Timecop.travel(3.days.from_now) do
+        content = URI.parse(report_link).read
+        expect(content).to match "<th>\nFirst Name\n</th>"
+      end
+
+      # The link should still expire though:
+      Timecop.travel(3.months.from_now) do
+        expect { URI.parse(report_link).read }
+          .to raise_error OpenURI::HTTPError, "404 Not Found"
+      end
     end
   end