fix first party download from preview

app/controllers/submissions_download_controller.rb

@@ -5,30 +5,20 @@ class SubmissionsDownloadController < ApplicationController
   skip_authorization_check
 
   TTL = 40.minutes
-  FILES_TTL = 5.minutes
 
   def index
-    @submitter = Submitter.find_signed(params[:sig], purpose: :download_completed) if params[:sig].present?
+    @submission = Submission.find_by!(slug: params[:submission_slug] || params[:submissions_preview_slug])
 
-    signature_valid =
+    last_submitter = @submission.submitters.where.not(completed_at: nil).order(:completed_at).last
-      if @submitter&.slug == params[:submitter_slug]
-        true
-      else
-        @submitter = nil
-      end
-
-    @submitter ||= Submitter.find_by!(slug: params[:submitter_slug])
-
-    Submissions::EnsureResultGenerated.call(@submitter)
-
-    last_submitter = @submitter.submission.submitters.where.not(completed_at: nil).order(:completed_at).last
-
-    return head :not_found unless last_submitter
 
     Submissions::EnsureResultGenerated.call(last_submitter)
 
-    if !signature_valid && !current_user_submitter?(last_submitter)
+    unless current_user_submitter?(last_submitter)
-      return head :not_found unless Submitters::AuthorizedForForm.call(@submitter, current_user, request)
+      unless Submitters::AuthorizedForForm.call(last_submitter, current_user, request)
+        Rollbar.info("2FA download error: #{last_submitter.id}") if defined?(Rollbar)
+
+        return head :not_found
+      end
 
       if last_submitter.completed_at < TTL.ago
         Rollbar.info("TTL: #{last_submitter.id}") if defined?(Rollbar)
@@ -40,14 +30,14 @@ class SubmissionsDownloadController < ApplicationController
     if params[:combined] == 'true'
       respond_with_combined(last_submitter)
     else
-      render json: build_urls(last_submitter)
+      render json: Submitters.build_document_urls(last_submitter)
     end
   end
 
   private
 
   def respond_with_combined(submitter)
-    url = build_combined_url(submitter)
+    url = Submitters.build_combined_url(submitter)
 
     if url
       render json: [url]
@@ -59,34 +49,4 @@ class SubmissionsDownloadController < ApplicationController
   def current_user_submitter?(submitter)
     current_user && current_ability.can?(:read, submitter)
   end
-
-  def build_urls(submitter)
-    filename_format = AccountConfig.find_or_initialize_by(account_id: submitter.account_id,
-                                                          key: AccountConfig::DOCUMENT_FILENAME_FORMAT_KEY)&.value
-
-    Submitters.select_attachments_for_download(submitter).map do |attachment|
-      ActiveStorage::Blob.proxy_path(
-        attachment.blob,
-        expires_at: FILES_TTL.from_now.to_i,
-        filename: Submitters.build_document_filename(submitter, attachment.blob, filename_format)
-      )
-    end
-  end
-
-  def build_combined_url(submitter)
-    return if submitter.submission.submitters.exists?(completed_at: nil)
-    return if submitter.submission.submitters.order(:completed_at).last != submitter
-
-    attachment = submitter.submission.combined_document_attachment
-    attachment ||= Submissions::EnsureCombinedGenerated.call(submitter)
-
-    filename_format = AccountConfig.find_or_initialize_by(account_id: submitter.account_id,
-                                                          key: AccountConfig::DOCUMENT_FILENAME_FORMAT_KEY)&.value
-
-    ActiveStorage::Blob.proxy_path(
-      attachment.blob,
-      expires_at: FILES_TTL.from_now.to_i,
-      filename: Submitters.build_document_filename(submitter, attachment.blob, filename_format)
-    )
-  end
 end

app/controllers/submissions_preview_controller.rb

@@ -10,13 +10,15 @@ class SubmissionsPreviewController < ApplicationController
   TTL = 40.minutes
 
   def show
-    submitter = Submitter.find_signed(params[:sig], purpose: :download_completed) if params[:sig].present?
+    @sig_submitter = Submitter.find_signed(params[:sig], purpose: :download_completed) if params[:sig].present?
 
     signature_valid =
-      if submitter && submitter.submission.slug == params[:slug]
+      if @sig_submitter && @sig_submitter.submission.slug == params[:slug]
-        @submission = submitter.submission
+        @submission = @sig_submitter.submission
 
         true
+      else
+        @sig_submitter = nil
       end
 
     @submission ||= Submission.find_by!(slug: params[:slug])

app/controllers/submitters_download_controller.rb

@@ -0,0 +1,66 @@
+# frozen_string_literal: true
+
+class SubmittersDownloadController < ApplicationController
+  skip_before_action :authenticate_user!
+  skip_authorization_check
+
+  TTL = 40.minutes
+  FILES_TTL = 5.minutes
+
+  def index
+    @submitter = Submitter.find_signed(params[:sig], purpose: :download_completed) if params[:sig].present?
+
+    signature_valid =
+      if @submitter&.slug == params[:submitter_slug]
+        true
+      else
+        @submitter = nil
+      end
+
+    @submitter ||= Submitter.find_by!(slug: params[:submitter_slug])
+
+    Submissions::EnsureResultGenerated.call(@submitter)
+
+    last_submitter = @submitter.submission.submitters.where.not(completed_at: nil).order(:completed_at).last
+
+    return head :not_found unless last_submitter
+
+    Submissions::EnsureResultGenerated.call(last_submitter)
+
+    if !signature_valid && !current_user_submitter?(last_submitter)
+      unless Submitters::AuthorizedForForm.call(@submitter, current_user, request)
+        Rollbar.info("2FA download error: #{last_submitter.id}") if defined?(Rollbar)
+
+        return head :not_found
+      end
+
+      if last_submitter.completed_at < TTL.ago
+        Rollbar.info("TTL: #{last_submitter.id}") if defined?(Rollbar)
+
+        return head :not_found
+      end
+    end
+
+    if params[:combined] == 'true'
+      respond_with_combined(last_submitter)
+    else
+      render json: Submitters.build_document_urls(last_submitter)
+    end
+  end
+
+  private
+
+  def respond_with_combined(submitter)
+    url = Submitters.build_combined_url(submitter)
+
+    if url
+      render json: [url]
+    else
+      head :not_found
+    end
+  end
+
+  def current_user_submitter?(submitter)
+    current_user && current_ability.can?(:read, submitter)
+  end
+end

app/views/submissions/show.html.erb

@@ -15,7 +15,6 @@
       <span class="text-xl md:text-3xl font-semibold focus:text-clip" style="overflow: hidden; display: -webkit-box; -webkit-box-orient: vertical; -webkit-line-clamp: 2;"><% (@submission.name || @submission.template.name).split(/(_)/).each do |item| %><%= item %><wbr><% end %></span>
     </a>
     <div class="space-x-3 flex items-center">
-      <% last_submitter = @submission.submitters.to_a.select(&:completed_at?).max_by(&:completed_at) %>
       <% is_all_completed = @submission.submitters.to_a.all?(&:completed_at?) %>
       <% if signed_in? && can?(:create, @submission) && @submission.archived_at? && !is_all_completed %>
         <%= button_to button_title(title: t('unarchive'), disabled_with: t('unarchive')[0..-2], icon: svg_icon('rotate', class: 'w-6 h-6')), submission_unarchive_index_path(@submission), class: 'btn btn-primary btn-ghost text-base hidden md:flex' %>
@@ -31,10 +30,10 @@
           <span class="hidden md:block"><%= t('event_log') %></span>
         <% end %>
       <% end %>
-      <% if last_submitter %>
+      <% if is_all_completed || @submission.submitters.to_a.any?(&:completed_at?) %>
         <% if is_all_completed || !is_combined_enabled %>
           <div class="join relative">
-            <download-button data-src="<%= submitter_download_index_path(last_submitter.slug, { sig: params[:sig], combined: is_combined_enabled }.compact) %>" class="base-button <%= '!rounded-r-none !pr-2' if is_all_completed && !is_combined_enabled %>">
+            <download-button data-src="<%= @sig_submitter ? submitter_download_index_path(@sig_submitter.slug, { sig: params[:sig], combined: is_combined_enabled }.compact) : submissions_preview_download_index_path(@submission.slug) %>" class="base-button <%= '!rounded-r-none !pr-2' if is_all_completed && !is_combined_enabled %>">
               <span class="flex items-center justify-center space-x-2" data-target="download-button.defaultButton">
                 <%= svg_icon('download', class: 'w-6 h-6') %>
                 <span class="hidden md:inline"><%= t('download') %></span>
@@ -53,7 +52,7 @@
                 </label>
                 <ul tabindex="0" class="z-10 dropdown-content p-2 mt-2 shadow menu text-base bg-base-100 rounded-box text-right">
                   <li>
-                    <download-button data-src="<%= submitter_download_index_path(last_submitter.slug, { sig: params[:sig], combined: true }.compact) %>" class="flex items-center">
+                    <download-button data-src="<%= @sig_submitter ? submitter_download_index_path(@sig_submitter.slug, { sig: params[:sig], combined: true }.compact) : submissions_preview_download_index_path(@submission.slug, combined: true) %>" class="flex items-center">
                       <span class="flex items-center justify-center space-x-2" data-target="download-button.defaultButton">
                         <%= svg_icon('download', class: 'w-6 h-6 flex-shrink-0') %>
                         <span class="whitespace-nowrap"><%= t('download_combined_pdf') %></span>

config/routes.rb

@@ -155,12 +155,13 @@ Rails.application.routes.draw do
 
   resources :submissions_preview, only: %i[show], path: 'e', param: 'slug' do
     get :completed
+    resources :download, only: %i[index], controller: 'submissions_download'
   end
 
   resources :send_submission_email, only: %i[create]
 
   resources :submitters, only: %i[], param: 'slug' do
-    resources :download, only: %i[index], controller: 'submissions_download'
+    resources :download, only: %i[index], controller: 'submitters_download'
     resources :send_email, only: %i[create], controller: 'submitters_send_email'
     resources :debug, only: %i[index], controller: 'submissions_debug' if Rails.env.development?
   end

lib/submitters.rb

@@ -25,6 +25,8 @@ module Submitters
     drv scr ins isp mst paf prf shb shs slk ws wsc inf1 inf2
   ].freeze)
 
+  FILES_TTL = 5.minutes
+
   module_function
 
   def search(current_user, submitters, keyword)
@@ -254,6 +256,36 @@ module Submitters
     true
   end
 
+  def build_document_urls(submitter, ttl: FILES_TTL)
+    filename_format = AccountConfig.find_or_initialize_by(account_id: submitter.account_id,
+                                                          key: AccountConfig::DOCUMENT_FILENAME_FORMAT_KEY)&.value
+
+    Submitters.select_attachments_for_download(submitter).map do |attachment|
+      ActiveStorage::Blob.proxy_path(
+        attachment.blob,
+        expires_at: ttl.from_now.to_i,
+        filename: Submitters.build_document_filename(submitter, attachment.blob, filename_format)
+      )
+    end
+  end
+
+  def build_combined_url(submitter, ttl: FILES_TTL)
+    return if submitter.submission.submitters.exists?(completed_at: nil)
+    return if submitter.submission.submitters.order(:completed_at).last != submitter
+
+    attachment = submitter.submission.combined_document_attachment
+    attachment ||= Submissions::EnsureCombinedGenerated.call(submitter)
+
+    filename_format = AccountConfig.find_or_initialize_by(account_id: submitter.account_id,
+                                                          key: AccountConfig::DOCUMENT_FILENAME_FORMAT_KEY)&.value
+
+    ActiveStorage::Blob.proxy_path(
+      attachment.blob,
+      expires_at: ttl.from_now.to_i,
+      filename: Submitters.build_document_filename(submitter, attachment.blob, filename_format)
+    )
+  end
+
   def populate_completed_is_first
     Account.find_each do |account|
       submissions_index = {}