Merge pull request #11476 from alphagov/fix-access-limiting-bug

Raise validation error if badly access-limiting draft on-create [WHIT-3458]

Author: ChrisBAshton

app/controllers/admin/editions_controller.rb

@@ -32,7 +32,7 @@ def enforce_permissions!
     when "choose_type", "change_type", "change_type_preview", "apply_change_type"
       enforce_permission!(:create, edition_class || Edition)
     when "create"
-      enforce_permission!(:create, @edition)
+      enforce_permission!(:create, @edition) if @edition.persisted?
     when "edit", "update", "revise", "diff", "update_bypass_id", "features"
       enforce_permission!(:update, @edition)
     when "destroy", "confirm_destroy"
@@ -95,6 +95,7 @@ def create
       updater.perform!
       redirect_to show_or_edit_path, saved_confirmation_notice
     else
+      flash.now[:alert] = updater.failure_reason
       build_edition_dependencies
       render :new
     end

app/services/draft_edition_updater.rb

@@ -28,6 +28,9 @@ def should_check_current_user_will_retain_access?
   end
 
   def access_limit_excludes_current_user?
+    # Some users may not necessarily belong to an organisation. We should fail-closed for them.
+    return true unless @options[:current_user].organisation
+
     edition.organisation_association_enabled? && edition.edition_organisations.map(&:organisation_id).exclude?(@options[:current_user].organisation.id)
   end
 end

app/views/admin/errors/forbidden.html.erb

@@ -5,7 +5,7 @@
 <div class="govuk-grid-row govuk-!-margin-bottom-0">
   <div class="govuk-grid-column-two-thirds">
     <p class="govuk-body">You do not have permission to access this page.</p>
-    <% if @edition && can?(:perform_administrative_tasks, Edition) %>
+    <% if @edition&.persisted? && can?(:perform_administrative_tasks, Edition) %>
       <p class="govuk-body">
         <a href="<%= edit_access_limited_admin_edition_path(@edition) %>" class="govuk-link">
           As an administrator, you can update the access permissions of the page.

features/admin-limit-access-editions.feature

@@ -15,3 +15,11 @@ Feature: Viewing most recent editions in admin
     And I view the current edition of document "Road accidents"
     Then I am told I do not have permissions to access this page
     And I should see a link to edit the access
+
+  Scenario: Attempting to access limit a new (unpersisted) document against a different org
+    Given I am an editor
+    When I begin drafting a new document
+    And I set the Lead organisation to an org I am not in
+    And I check the "Limit access to publishers from organisations associated with this document before you publish" box
+    When I click "Save"
+    Then I should see the validation error "Access can only be limited by users belonging to an organisation tagged to the document"

features/step_definitions/admin_limit_access_steps.rb

@@ -14,3 +14,30 @@
 Then(/^I should see a link to edit the access/) do
   expect(page).to have_link("As an administrator, you can update the access permissions of the page.", href: edit_access_limited_admin_edition_path(@edition))
 end
+
+When(/^I begin drafting a new document$/) do
+  @org = create(:organisation, name: "Some made up org")
+
+  visit "/government/admin/publications/new"
+  select "Policy paper", from: "edition_publication_type_id"
+  fill_in "edition[title]", with: "Test publication"
+  fill_in "edition[summary]", with: "Test summary"
+  fill_in "edition[body]", with: "Test body"
+  check "Applies to all UK nations"
+end
+
+When(/^I set the Lead organisation to an org I am not in$/) do
+  select @org.name, from: "edition_lead_organisation_ids_1"
+end
+
+When(/^I check the "(.+)" box$/) do |box_name|
+  check box_name
+end
+
+When(/^I click "(.+)"$/) do |button_name|
+  click_on button_name
+end
+
+Then(/^I should see the validation error "(.+)"$/) do |string|
+  expect(page).to have_content string
+end