Skip to content

Commit

Permalink
Merge pull request #1362 from alphagov/per-department-permissions
Browse files Browse the repository at this point in the history
Allow Per-department permissions
  • Loading branch information
KludgeKML authored Aug 20, 2024
2 parents a1d5b2d + 55f32b0 commit cfd4f66
Show file tree
Hide file tree
Showing 42 changed files with 855 additions and 74 deletions.
4 changes: 4 additions & 0 deletions app/controllers/application_controller.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
class ApplicationController < ActionController::Base
include GDS::SSO::ControllerMethods
include ServicePermissions

helper_method :gds_editor?

before_action :authenticate_user!
# Prevent CSRF attacks by raising an exception.
# For APIs, you may want to use :null_session instead.
Expand Down
31 changes: 31 additions & 0 deletions app/controllers/concerns/service_permissions.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
module ServicePermissions
def gds_editor?
current_user.permissions.include?("GDS Editor")
end

def service_owner?(service)
service.organisation_slugs.include?(current_user.organisation_slug)
end

def permission_for_service?(service)
gds_editor? || service_owner?(service)
end

def org_name_for_current_user
GdsApi.organisations.organisation(current_user.organisation_slug).to_hash["title"]
rescue GdsApi::HTTPUnavailable
current_user.organisation_slug
end

def redirect_unless_gds_editor
redirect_to services_path unless gds_editor?
end

def forbid_unless_permission
raise GDS::SSO::PermissionDeniedError, "You do not have permission to view this page" unless permission_for_service?(@service)
end

def forbid_unless_gds_editor
raise GDS::SSO::PermissionDeniedError, "You do not have permission to view this page" unless gds_editor?
end
end
5 changes: 5 additions & 0 deletions app/controllers/links_controller.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
class LinksController < ApplicationController
before_action :load_dependencies, only: %i[edit update destroy]
before_action :set_back_url_before_post_request, only: %i[edit update destroy]

before_action :forbid_unless_permission, only: %i[edit update]
before_action :forbid_unless_gds_editor, only: %i[destroy homepage_links_status_csv links_status_csv bad_links_url_and_status_csv]
before_action :redirect_unless_gds_editor, only: %i[index]

helper_method :back_url

def index
Expand Down
3 changes: 3 additions & 0 deletions app/controllers/local_authorities_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@ class LocalAuthoritiesController < ApplicationController

before_action :set_authority, except: %i[index bad_homepage_url_and_status_csv]

before_action :forbid_unless_gds_editor, only: %i[update download_links_csv upload_links_csv bad_homepage_url_and_status_csv]
before_action :redirect_unless_gds_editor, only: %i[index show edit_url download_links_form upload_links_form]

def index
Rails.logger.info(params)

Expand Down
29 changes: 25 additions & 4 deletions app/controllers/services_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,13 @@ class ServicesController < ApplicationController

before_action :set_service, except: :index

before_action :forbid_unless_permission, only: %i[show download_links_form download_links_csv upload_links_form upload_links_csv]
before_action :forbid_unless_gds_editor, only: %i[update_owner_form update_owner]

helper_method :org_name_for_current_user

def index
@services = Service.enabled.order(broken_link_count: :desc)
raise "Missing Data" if @services.empty?
@services = services_for_user(current_user).enabled.order(broken_link_count: :desc)

@breadcrumbs = index_breadcrumbs
end
Expand All @@ -18,6 +22,16 @@ def show
@breadcrumbs = service_breadcrumbs(@service)
end

def update_owner_form
@breadcrumbs = service_breadcrumbs(@service) + [{ title: "Update Owner", url: update_owner_form_service_path(@service) }]
end

def update_owner
@service.update!(organisation_slugs: params["service"]["organisation_slugs"].split(" "))

redirect_to service_path(@service, filter: "broken_links")
end

def download_links_form
@breadcrumbs = service_breadcrumbs(@service) + [{ title: "Download Links", url: download_links_form_service_path(@service) }]
end
Expand All @@ -34,12 +48,19 @@ def upload_links_form
end

def upload_links_csv
attempt_import(:service, @service)
redirect_to service_path(@service)
return redirect_to service_path(@service) if attempt_import(:service, @service)

redirect_to(upload_links_form_service_path(@service))
end

private

def services_for_user(user)
return Service.all if gds_editor?

Service.where(":organisation_slugs = ANY(organisation_slugs)", organisation_slugs: user.organisation_slug)
end

def set_service
@service = Service.find_by!(slug: params[:service_slug])
end
Expand Down
2 changes: 1 addition & 1 deletion app/presenters/links_table_presenter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ def rows
{ text: link.analytics.to_i, format: "numeric" },
{ text: "<span class=\"app-service-label\">#{title}</span>".html_safe },
{ text: "<span class=\"app-interaction-label\">#{link.interaction.label}</span>".html_safe },
{ text: @view_context.link_to(link.local_authority.name, @view_context.local_authority_path(link.local_authority), class: "govuk-link") },
{ text: link.local_authority.name },
{ text: "<span class=\"govuk-tag govuk-tag--#{pres.status_tag_colour}\">#{pres.status_description}</span>".html_safe },
{ text: @view_context.link_to("Edit <span class=\"govuk-visually-hidden\">#{link.local_authority.name} - #{si.service.label} - #{si.interaction.label}</span>".html_safe, @view_context.edit_link_path(link.local_authority, link.service, link.interaction), class: "govuk-link") },
]
Expand Down
1 change: 1 addition & 0 deletions app/presenters/service_presenter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ def summary_list(view_context)

summary_items = [
{ field: "Local Government Service List (LGSL) Code", value: lgsl_code },
{ field: "Owning Departments", value: organisation_slugs.join(", ") },
{ field: "Page title(s) on GOV.UK", value: govuk_links.compact.any? ? govuk_links.compact.join("</br>").html_safe : "Not used on GOV.UK" },
]

Expand Down
36 changes: 24 additions & 12 deletions app/views/layouts/application.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,11 @@
browser_title: (yield :page_title),
} do %>
<%= render "govuk_publishing_components/components/layout_header", {
product_name: "Local Links Manager",
environment:,
navigation_items: [
{
<%
menu_option = []

if gds_editor?
menu_option += [{
text: "Broken Links",
href: root_path,
active: current_page?(root_path),
Expand All @@ -29,13 +29,25 @@
text: "Councils",
href: local_authorities_path(filter: %w[only_active]),
active: current_page?(local_authorities_path),
},
{
text: "Services",
href: services_path,
active: current_page?(services_path),
},
],
}]
end

menu_option << {
text: "Services",
href: services_path,
active: current_page?(services_path),
}

menu_option << {
text: "Switch app",
href: Plek.external_url_for("signon"),
}
%>
<%= render "govuk_publishing_components/components/layout_header", {
product_name: "Local Links Manager",
environment:,
navigation_items: menu_option,
} %>

<div class="govuk-width-container">
Expand Down
2 changes: 1 addition & 1 deletion app/views/links/edit.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
<%= render "govuk_publishing_components/components/button", { text: "Update" } %>
<% end %>
<% if @link.url %>
<% if @link.url && gds_editor? %>
<div class="app-danger-block">
<%= form_for @link, url: link_path(@local_authority.slug, @service.slug, @interaction.slug), method: "delete", data: { confirm: "Are you sure you wish to delete this link?" } do |f| %>
<p class="govuk-body">You can also delete this link if you are certain it is no longer necessary (NB: this cannot be undone)</p>
Expand Down
5 changes: 3 additions & 2 deletions app/views/services/index.html.erb
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
<% content_for :page_title, "Services" %>
<% title = gds_editor? ? "Services" : "Services for #{org_name_for_current_user}" %>
<% content_for :page_title, title %>
<% breadcrumb :services %>
<%= render "govuk_publishing_components/components/heading", {
text: "Services (#{@services.count})",
text: "#{title} (#{@services.count})",
heading_level: 1,
font_size: "l",
margin_bottom: 5,
Expand Down
13 changes: 8 additions & 5 deletions app/views/services/show.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,15 @@
<aside class="app-side__wrapper">
<div class="app-side">
<div class="app-side__actions">
<%
items = []
items << link_to("Update Owner", update_owner_form_service_path(@service), class: "govuk-link") if gds_editor?
items << link_to("Download Links", download_links_form_service_path(@service), class: "govuk-link")
items << link_to("Upload Links", upload_links_form_service_path(@service), class: "govuk-link")
items << link_to(@filter_var == "broken_links" ? "Show only broken links" : "Show all links", service_path(@service.slug, filter: @filter_var), class: "govuk-link")
%>
<%= render "govuk_publishing_components/components/list", {
items: [
link_to("Download Links", download_links_form_service_path(@service), class: "govuk-link"),
link_to("Upload Links", upload_links_form_service_path(@service), class: "govuk-link"),
link_to(@filter_var == "broken_links" ? "Show only broken links" : "Show all links", service_path(@service.slug, filter: @filter_var), class: "govuk-link"),
],
items:,
margin_bottom: 0,
} %>
</div>
Expand Down
23 changes: 23 additions & 0 deletions app/views/services/update_owner_form.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<% content_for :page_title, "Update Owner" %>
<%= render "govuk_publishing_components/components/heading", {
text: "Update Owner",
heading_level: 1,
font_size: "l",
margin_bottom: 5,
} %>
<%= form_for(@service, url: update_owner_service_path(@service)) do %>
<%= render "govuk_publishing_components/components/input", {
label: { text: "Organisation Slugs" },
hint: "For multiple owning organisations, list slugs separated by spaces",
name: "service[organisation_slugs]",
value: @service&.organisation_slugs,
} %>
<%= render "govuk_publishing_components/components/button", { text: "Submit" } %>
<%= render "govuk_publishing_components/components/button", {
text: "Cancel",
secondary_quiet: true,
href: service_path(@service, filter: "broken_links"),
} %>
<% end %>
2 changes: 2 additions & 0 deletions config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@
post "download_links_csv"
get "upload_links_form"
post "upload_links_csv"
get "update-owner-form"
patch "update-owner"
end
end

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
class ModifyServicesAddOrganisationSlug < ActiveRecord::Migration[7.1]
def change
add_column :services, :organisation_slugs, :string, array: true, default: []
end
end
3 changes: 2 additions & 1 deletion db/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
#
# It's strongly recommended that you check this file into your version control system.

ActiveRecord::Schema[7.1].define(version: 2024_03_18_105241) do
ActiveRecord::Schema[7.1].define(version: 2024_07_30_142812) do
# These are extensions that must be enabled in order to support this database
enable_extension "pgcrypto"
enable_extension "plpgsql"
Expand Down Expand Up @@ -106,6 +106,7 @@
t.string "slug", null: false
t.boolean "enabled", default: false, null: false
t.integer "broken_link_count", default: 0
t.string "organisation_slugs", default: [], array: true
t.index ["label"], name: "index_services_on_label", unique: true
t.index ["lgsl_code"], name: "index_services_on_lgsl_code", unique: true
end
Expand Down
41 changes: 41 additions & 0 deletions docs/adr/adr-001-department-permissions.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
# Decision Record: Department Permissions

## Introduction

In July 2024 Places Manager was opened up to departments to allow them to
directly control their own datasets. It had long been suggested that the
same openness should be a feature of Local Links Manager, since departments
often have more information about particular services than GDS. This would form
part of a future possible three-way access system in which GDS Editors could
edit any link, departments could edit links in particular services, and local
authorities could edit links in their authority. This ADR moves towards this
by opening up the second of these three ways.

## Requirements

Each service would need to be owned by zero, one, or more than one
departments. Only editors with Local Links Managers access permission in
Signon should be allowed to view and edit those services, with GDS editors
allowed to access all services for troubleshooting and incident response.

We followed the pattern from Places Manager, which was itself based on the
style of permission in Whitehall and other publishing apps, where Signon
provides the current user's organisational slug and access can be limited
based on that. A "GDS Editor" special permission is also typical of these
apps.

## Resulting changes

- Add an `organisational_slugs` field to each service, to be filled in by
us for existing services before departments are given access.
- Add a `GDS Editor` permission. Anyone with this permission can see and
edit links for all services. Anyone without this permission can
only edit links in services whose organisational_slugs field contains
the same slug as reported for them by Signon.
- Add a UI to edit the organisational slugs. Like Places Manager, this
will be a simple string field, with space separation for multiple owners,
editable only by someone with the `GDS Editor` permission. We will not at
the moment add in an organisational drop-down, so any GDS Editor
making changes to the organisation slug will need to know the correct one,
but it is assumed that people with this permission will know how to find
that out.
17 changes: 17 additions & 0 deletions docs/permissions.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
# Permissions

## Named Permissions

- `GDS Editor`: gives the user permission to do all actions in the app.

## Department Permissions

Other permissions are based on the organisation_slug of the current user. If a user does not have the `GDS Editor` permission they will be able to:

- visit the Services page `/services`, which will only show services where the current user's organisation slug is contained in the service's organisation_slugs array.
- visit the specific service pages of those services
- download a csv of links for those services
- download a csv of new links for those services
- edit links in those services.

The organisation slugs array is editable only by people with the `GDS Editor` permission.
5 changes: 5 additions & 0 deletions docs/service-owners.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
# Service Owners

Most services currently do not have an owner, but a service can be assigned one or more owning organisations by a user with the `GDS Editor` [permission](/docs/permissions.md). Visit the service, and select the "Update Owner" action from the sidebar. You can then enter one or more organisations by their organisation slug (the final part of their organisation URL on gov.uk, eg: 'government-digital-service' from https://www.gov.uk/government/organisations/government-digital-service), separating organisations with a space if there are more than one.

This allows users from that department with access to Local Links Manager to edit the service as detailed in the [Permissions page](/docs/permissions.md)
2 changes: 1 addition & 1 deletion spec/controllers/links_controller_spec.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
RSpec.describe LinksController, type: :controller do
before do
login_as_stub_user
login_as_gds_editor
@local_authority = create(:local_authority)
@service = create(:service)
@interaction = create(:interaction)
Expand Down
Loading

0 comments on commit cfd4f66

Please sign in to comment.