Skip to content

Commit

Permalink
Add a check for departmental_editor? permission for user
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
baisa committed Mar 6, 2025
1 parent 374de00 commit 7e581f9
Show file tree
Hide file tree
Showing 8 changed files with 69 additions and 27 deletions.
2 changes: 1 addition & 1 deletion app/controllers/application_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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 <a href='https://www.gov.uk/guidance/contact-the-government-digital-service/request-a-thing#change-govuk-content'>raise a content request</a> with GDS to get it updated.".html_safe, status: :not_found unless edition.is_accessible_to?(current_user)
end
end
2 changes: 2 additions & 0 deletions app/models/edition.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
26 changes: 21 additions & 5 deletions test/functional/editions_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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 }

Expand All @@ -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
Expand Down
24 changes: 20 additions & 4 deletions test/functional/legacy_editions_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand All @@ -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
Expand Down
32 changes: 16 additions & 16 deletions test/models/edition_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions test/support/factories.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion test/unit/presenters/filtered_editions_presenter_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 7e581f9

Please sign in to comment.