From 7e581f902bf80a3c46703404bdcd585332558992 Mon Sep 17 00:00:00 2001 From: Barbara Date: Tue, 28 Jan 2025 12:23:21 +0000 Subject: [PATCH] Add a check for departmental_editor? permission for user This is to enable specifying restriction to access Mainstream content by departments. It is an addition to giving access by matching owning_org_content_ids with users organisation_content_id, to ensure we do not block access for some departments. --- app/controllers/application_controller.rb | 2 +- app/models/edition.rb | 2 ++ app/models/user.rb | 4 +++ test/functional/editions_controller_test.rb | 26 ++++++++++++--- .../legacy_editions_controller_test.rb | 24 +++++++++++--- test/models/edition_test.rb | 32 +++++++++---------- test/support/factories.rb | 4 +++ .../filtered_editions_presenter_test.rb | 2 +- 8 files changed, 69 insertions(+), 27 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 4937abfab..fc0c14cda 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -65,6 +65,6 @@ def require_editor_permissions end def require_user_accessibility_to_edition(edition) - render plain: "404 Not Found", status: :not_found unless edition.is_accessible_to?(current_user) + render html: "You do not have permission to access this page - please raise a content request with GDS to get it updated.".html_safe, status: :not_found unless edition.is_accessible_to?(current_user) end end diff --git a/app/models/edition.rb b/app/models/edition.rb index fe571ad09..de5d7a41a 100644 --- a/app/models/edition.rb +++ b/app/models/edition.rb @@ -45,6 +45,7 @@ class ResurrectionError < RuntimeError lambda { |user| return all unless Flipflop.enabled?(:restrict_access_by_org) return all if user.gds_editor? + return all unless user.departmental_editor? where(owning_org_content_ids: user.organisation_content_id) } @@ -517,6 +518,7 @@ def paths def is_accessible_to?(user) return true unless Flipflop.enabled?(:restrict_access_by_org) return true if user.gds_editor? + return true unless user.departmental_editor? owning_org_content_ids.include?(user.organisation_content_id) end diff --git a/app/models/user.rb b/app/models/user.rb index 81a0666f3..586bd5cd4 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -84,6 +84,10 @@ def welsh_editor? permissions.include?("welsh_editor") end + def departmental_editor? + permissions.include?("departmental_editor") + end + def has_editor_permissions?(resource) govuk_editor? || (welsh_editor? && resource.artefact.welsh?) end diff --git a/test/functional/editions_controller_test.rb b/test/functional/editions_controller_test.rb index 2e81245e5..ac419a736 100644 --- a/test/functional/editions_controller_test.rb +++ b/test/functional/editions_controller_test.rb @@ -122,14 +122,22 @@ class EditionsControllerTest < ActionController::TestCase end context "when 'restrict_access_by_org' feature toggle is disabled" do - %i[show metadata history admin related_external_links unpublish].each do |action| + %i[show metadata history related_external_links].each do |action| context "##{action}" do setup do @edition = FactoryBot.create(:edition, owning_org_content_ids: %w[org-two]) end - should "return a 200 when requesting an edition owned by a different organisation" do - login_as(FactoryBot.create(:user, :govuk_editor, organisation_content_id: "org-one")) + should "return a 200 when requesting the #{action} tab on an edition owned by a different organisation and user has departmental_editor permission" do + login_as(FactoryBot.create(:user, :departmental_editor, organisation_content_id: "org-one")) + + get action, params: { id: @edition.id } + + assert_response :ok + end + + should "return a 200 when requesting the #{action} tab on an edition owned by a different organisation and user does not have departmental_editor permission" do + login_as(FactoryBot.create(:user, organisation_content_id: "org-one")) get action, params: { id: @edition.id } @@ -156,13 +164,21 @@ class EditionsControllerTest < ActionController::TestCase @edition = FactoryBot.create(:edition, owning_org_content_ids: %w[org-two]) end - should "return a 404 when requesting an edition owned by a different organisation" do - login_as(FactoryBot.create(:user, organisation_content_id: "org-one")) + should "return a 404 when requesting the #{action} tab on an edition owned by a different organisation and user has departmental_editor permission" do + login_as(FactoryBot.create(:user, :departmental_editor, organisation_content_id: "org-one")) get action, params: { id: @edition.id } assert_response :not_found end + + should "return a 200 when requesting the #{action} tab on an edition owned by a different organisation and user does not have departmental_editor permission" do + login_as(FactoryBot.create(:user, :govuk_editor, organisation_content_id: "org-one")) + + get action, params: { id: @edition.id } + + assert_response :ok + end end end end diff --git a/test/functional/legacy_editions_controller_test.rb b/test/functional/legacy_editions_controller_test.rb index 4469b12d4..135806aa9 100644 --- a/test/functional/legacy_editions_controller_test.rb +++ b/test/functional/legacy_editions_controller_test.rb @@ -1317,7 +1317,15 @@ class LegacyEditionsControllerTest < ActionController::TestCase @edition = FactoryBot.create(:edition, owning_org_content_ids: %w[org-two]) end - should "return a 200 when requesting an edition owned by a different organisation" do + should "return a 200 when requesting the #{action} tab on an edition owned by a different organisation and user has departmental_editor permission" do + login_as(FactoryBot.create(:user, :departmental_editor, organisation_content_id: "org-one")) + + get action, params: { id: @edition.id } + + assert_response :ok + end + + should "return a 200 when requesting the #{action} tab on an edition owned by a different organisation and user does not have departmental_editor permission" do login_as(FactoryBot.create(:user, organisation_content_id: "org-one")) get action, params: { id: @edition.id } @@ -1339,19 +1347,27 @@ class LegacyEditionsControllerTest < ActionController::TestCase test_strategy.switch!(:restrict_access_by_org, false) end - %i[show metadata history admin unpublish duplicate update linking update_tagging update_related_external_links review destroy progress diff process_unpublish diagram].each do |action| + %i[show metadata history admin unpublish].each do |action| context "##{action}" do setup do @edition = FactoryBot.create(:edition, owning_org_content_ids: %w[org-two]) end - should "return a 404 when requesting an edition owned by a different organisation" do - login_as(FactoryBot.create(:user, :govuk_editor, organisation_content_id: "org-one")) + should "return a 404 when requesting the #{action} tab on an edition owned by a different organisation and user has departmental_editor permission" do + login_as(FactoryBot.create(:user, :departmental_editor, organisation_content_id: "org-one")) get action, params: { id: @edition.id } assert_response :not_found end + + should "return a 200 when requesting the #{action} tab on an edition owned by a different organisation and user does not have departmental_editor permission" do + login_as(FactoryBot.create(:user, :govuk_editor, organisation_content_id: "org-one")) + + get action, params: { id: @edition.id } + + assert_response :ok + end end end end diff --git a/test/models/edition_test.rb b/test/models/edition_test.rb index 31fd3e252..a0920b993 100644 --- a/test/models/edition_test.rb +++ b/test/models/edition_test.rb @@ -1271,36 +1271,36 @@ def draft_second_edition_from(published_edition) end context "accessible_to scope" do - should "omit editions that are owned by an organisation that is different to the user's" do + should "omit editions that are owned by an organisation that is different to the user's when user has departmental_editor permission" do FactoryBot.create(:edition, owning_org_content_ids: %w[one]) - user = FactoryBot.create(:user, organisation_content_id: "two") + user = FactoryBot.create(:user, :departmental_editor, organisation_content_id: "two") query_result = Edition.accessible_to(user) assert_empty query_result end - should "omit editions that are owned by an organisation when the user has no organisation" do + should "omit editions that are owned by an organisation when the user has no organisation and has departmental_editor permission" do FactoryBot.create(:edition, owning_org_content_ids: %w[one]) - user = FactoryBot.create(:user, organisation_content_id: nil) + user = FactoryBot.create(:user, :departmental_editor, organisation_content_id: nil) query_result = Edition.accessible_to(user) assert_empty query_result end - should "omit editions not owned by any organisation" do + should "omit editions not owned by any organisation when user has departmental_editor permission" do FactoryBot.create(:edition, owning_org_content_ids: []) - user = FactoryBot.create(:user, organisation_content_id: "two") + user = FactoryBot.create(:user, :departmental_editor, organisation_content_id: "two") query_result = Edition.accessible_to(user) assert_empty query_result end - should "omit editions not owned by any organisation when the user has no organisation" do + should "omit editions not owned by any organisation when the user has no organisation and has departmental_editor permission" do FactoryBot.create(:edition, owning_org_content_ids: []) - user = FactoryBot.create(:user, organisation_content_id: nil) + user = FactoryBot.create(:user, :departmental_editor, organisation_content_id: nil) query_result = Edition.accessible_to(user) @@ -1336,30 +1336,30 @@ def draft_second_edition_from(published_edition) end context "#is_accessible_to?" do - should "return false for editions that are owned by an organisation that is different to the user's" do + should "return false for editions that are owned by an organisation that is different to the user's and user has departmental_editor permission" do edition = FactoryBot.create(:edition, owning_org_content_ids: %w[one]) - user = FactoryBot.create(:user, organisation_content_id: "two") + user = FactoryBot.create(:user, :departmental_editor, organisation_content_id: "two") assert_not edition.is_accessible_to?(user) end - should "return false for editions that are owned by an organisation when the user has no organisation" do + should "return false for editions that are owned by an organisation when the user has no organisation and has departmental_editor permission" do edition = FactoryBot.create(:edition, owning_org_content_ids: %w[one]) - user = FactoryBot.create(:user, organisation_content_id: nil) + user = FactoryBot.create(:user, :departmental_editor, organisation_content_id: nil) assert_not edition.is_accessible_to?(user) end - should "return false for editions not owned by any organisation" do + should "return false for editions not owned by any organisation and user has departmental_editor permission" do edition = FactoryBot.create(:edition, owning_org_content_ids: []) - user = FactoryBot.create(:user, organisation_content_id: "two") + user = FactoryBot.create(:user, :departmental_editor, organisation_content_id: "two") assert_not edition.is_accessible_to?(user) end - should "return false for editions not owned by any organisation when the user has no organisation" do + should "return false for editions not owned by any organisation when the user has no organisation and has departmental_editor permission" do edition = FactoryBot.create(:edition, owning_org_content_ids: []) - user = FactoryBot.create(:user, organisation_content_id: nil) + user = FactoryBot.create(:user, :departmental_editor, organisation_content_id: nil) assert_not edition.is_accessible_to?(user) end diff --git a/test/support/factories.rb b/test/support/factories.rb index d01d31bc6..dde825ca4 100644 --- a/test/support/factories.rb +++ b/test/support/factories.rb @@ -21,6 +21,10 @@ trait :welsh_editor do permissions { %w[welsh_editor signin] } end + + trait :departmental_editor do + permissions { %w[departmental_editor signin] } + end end trait :homepage_editor do diff --git a/test/unit/presenters/filtered_editions_presenter_test.rb b/test/unit/presenters/filtered_editions_presenter_test.rb index 1f2f4da82..1512737c6 100644 --- a/test/unit/presenters/filtered_editions_presenter_test.rb +++ b/test/unit/presenters/filtered_editions_presenter_test.rb @@ -161,7 +161,7 @@ def a_gds_user end should "filter out editions not accessible to the user" do - user = FactoryBot.create(:user, organisation_content_id: "an-org") + user = FactoryBot.create(:user, :departmental_editor, organisation_content_id: "an-org") FactoryBot.create(:guide_edition, owning_org_content_ids: %w[another-org]) filtered_editions = FilteredEditionsPresenter.new(user).editions