diff --git a/app/controllers/admin/editions_controller.rb b/app/controllers/admin/editions_controller.rb index 0eab4e8a34b..ecd16de83b8 100644 --- a/app/controllers/admin/editions_controller.rb +++ b/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 diff --git a/app/services/draft_edition_updater.rb b/app/services/draft_edition_updater.rb index 04011ecceb6..210d1567cd7 100644 --- a/app/services/draft_edition_updater.rb +++ b/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 diff --git a/app/views/admin/errors/forbidden.html.erb b/app/views/admin/errors/forbidden.html.erb index bdf47a30494..742899b2f63 100644 --- a/app/views/admin/errors/forbidden.html.erb +++ b/app/views/admin/errors/forbidden.html.erb @@ -5,7 +5,7 @@

You do not have permission to access this page.

- <% if @edition && can?(:perform_administrative_tasks, Edition) %> + <% if @edition&.persisted? && can?(:perform_administrative_tasks, Edition) %>

As an administrator, you can update the access permissions of the page. diff --git a/features/admin-limit-access-editions.feature b/features/admin-limit-access-editions.feature index 3eb474be4b1..594243629fb 100644 --- a/features/admin-limit-access-editions.feature +++ b/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" diff --git a/features/step_definitions/admin_limit_access_steps.rb b/features/step_definitions/admin_limit_access_steps.rb index 22c624e9b80..2ead9af659e 100644 --- a/features/step_definitions/admin_limit_access_steps.rb +++ b/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