Skip to content

Commit

Permalink
Merge pull request #3205 from alphagov/improve-delegatable-language
Browse files Browse the repository at this point in the history
Rename 'delegatable' permissions to 'delegated'
  • Loading branch information
yndajas authored Oct 3, 2024
2 parents 3c45c7c + 8dcc438 commit 7cd74f2
Show file tree
Hide file tree
Showing 46 changed files with 384 additions and 353 deletions.
2 changes: 1 addition & 1 deletion app/controllers/account/permissions_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ def set_permissions
if current_user.govuk_admin?
@permissions = @application.sorted_supported_permissions_grantable_from_ui(include_signin: false)
elsif current_user.publishing_manager?
@permissions = @application.sorted_supported_permissions_grantable_from_ui(include_signin: false, only_delegatable: true)
@permissions = @application.sorted_supported_permissions_grantable_from_ui(include_signin: false, only_delegated: true)
end
end
end
2 changes: 1 addition & 1 deletion app/controllers/supported_permissions_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ def load_and_authorize_application
end

def supported_permission_parameters
params.require(:supported_permission).permit(:name, :delegatable, :default)
params.require(:supported_permission).permit(:name, :delegated, :default)
end

def supported_permission
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/users/permissions_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ def set_permissions
if current_user.govuk_admin?
@permissions = @application.sorted_supported_permissions_grantable_from_ui(include_signin: false)
elsif current_user.publishing_manager?
@permissions = @application.sorted_supported_permissions_grantable_from_ui(include_signin: false, only_delegatable: true)
@permissions = @application.sorted_supported_permissions_grantable_from_ui(include_signin: false, only_delegated: true)
end
end
end
4 changes: 2 additions & 2 deletions app/helpers/application_permissions_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,9 @@ def permissions_updated_description(application_id, user = current_user)
end
end

def notice_about_non_delegatable_permissions(current_user, application, other_grantee = nil)
def notice_about_non_delegated_permissions(current_user, application, other_grantee = nil)
return nil if current_user.govuk_admin?
return nil unless application.has_non_delegatable_non_signin_permissions_grantable_from_ui?
return nil unless application.has_non_delegated_non_signin_permissions_grantable_from_ui?

link = if other_grantee
link_to(
Expand Down
2 changes: 1 addition & 1 deletion app/helpers/application_table_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ def update_permissions_link(application, user = nil)
if current_user.govuk_admin?
return "" unless application.has_non_signin_permissions_grantable_from_ui?
elsif current_user.publishing_manager?
return "" unless application.has_delegatable_non_signin_permissions_grantable_from_ui?
return "" unless application.has_delegated_non_signin_permissions_grantable_from_ui?
end

path = if user.nil?
Expand Down
18 changes: 9 additions & 9 deletions app/models/doorkeeper/application.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,11 @@ class Doorkeeper::Application < ActiveRecord::Base # rubocop:disable Rails/Appli
scope :not_api_only, -> { where(api_only: false) }
scope :with_home_uri, -> { where.not(home_uri: nil).where.not(home_uri: "") }
scope :can_signin, ->(user) { with_signin_permission_for(user) }
scope :with_signin_delegatable,
scope :with_signin_delegated,
lambda {
joins(:supported_permissions)
.merge(SupportedPermission.signin)
.merge(SupportedPermission.delegatable)
.merge(SupportedPermission.delegated)
}
scope :with_signin_permission_for,
lambda { |user|
Expand All @@ -43,10 +43,10 @@ def signin_permission
supported_permissions.signin.first
end

def sorted_supported_permissions_grantable_from_ui(include_signin: true, only_delegatable: false)
def sorted_supported_permissions_grantable_from_ui(include_signin: true, only_delegated: false)
permissions = supported_permissions.grantable_from_ui
permissions = permissions.excluding_signin unless include_signin
permissions = permissions.delegatable if only_delegatable
permissions = permissions.delegated if only_delegated

SupportedPermission.sort_with_signin_first(permissions)
end
Expand All @@ -55,12 +55,12 @@ def has_non_signin_permissions_grantable_from_ui?
(supported_permissions.grantable_from_ui - [signin_permission]).any?
end

def has_delegatable_non_signin_permissions_grantable_from_ui?
(supported_permissions.delegatable.grantable_from_ui - [signin_permission]).any?
def has_delegated_non_signin_permissions_grantable_from_ui?
(supported_permissions.delegated.grantable_from_ui - [signin_permission]).any?
end

def has_non_delegatable_non_signin_permissions_grantable_from_ui?
(supported_permissions.grantable_from_ui.where(delegatable: false) - [signin_permission]).any?
def has_non_delegated_non_signin_permissions_grantable_from_ui?
(supported_permissions.grantable_from_ui.where(delegated: false) - [signin_permission]).any?
end

def url_without_path
Expand Down Expand Up @@ -100,7 +100,7 @@ def substituted_uri(uri)
end

def create_signin_supported_permission
supported_permissions.delegatable.signin.create!
supported_permissions.delegated.signin.create!
end

def create_user_update_supported_permission
Expand Down
2 changes: 1 addition & 1 deletion app/models/supported_permission.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ class SupportedPermission < ApplicationRecord

default_scope { order(:name) }

scope :delegatable, -> { where(delegatable: true) }
scope :delegated, -> { where(delegated: true) }
scope :grantable_from_ui, -> { where(grantable_from_ui: true) }
scope :default, -> { where(default: true) }
scope :signin, -> { where(name: SIGNIN_NAME) }
Expand Down
2 changes: 1 addition & 1 deletion app/policies/account/application_policy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ def remove_signin_permission?
current_user.has_access_to?(record) &&
(
current_user.govuk_admin? ||
current_user.publishing_manager? && record.signin_permission.delegatable?
current_user.publishing_manager? && record.signin_permission.delegated?
)
end

Expand Down
2 changes: 1 addition & 1 deletion app/policies/supported_permission_policy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ def resolve
scope.all
elsif current_user.publishing_manager?
scope
.delegatable
.delegated
.joins(:application)
.where(oauth_applications: { id: publishing_manager_manageable_application_ids })
else
Expand Down
2 changes: 1 addition & 1 deletion app/policies/users/application_policy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ def grant_signin_permission?
return false unless Pundit.policy(current_user, user).edit?
return true if current_user.govuk_admin?

current_user.publishing_manager? && current_user.has_access_to?(application) && application.signin_permission.delegatable?
current_user.publishing_manager? && current_user.has_access_to?(application) && application.signin_permission.delegated?
end

alias_method :remove_signin_permission?, :grant_signin_permission?
Expand Down
2 changes: 1 addition & 1 deletion app/views/account/permissions/edit.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
<% end %>
<% end %>

<% if notice = notice_about_non_delegatable_permissions(current_user, @application, @user) %>
<% if notice = notice_about_non_delegated_permissions(current_user, @application, @user) %>
<div class="govuk-grid-row">
<div class="govuk-grid-column-two-thirds">
<%= render "govuk_publishing_components/components/inset_text", { text: notice } %>
Expand Down
10 changes: 5 additions & 5 deletions app/views/supported_permissions/_form.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -27,15 +27,15 @@
error_items: f.object.errors.full_messages_for(:name).map { |message| { text: message } }
} %>

<%= f.hidden_field "delegatable", value: 0 %>
<%= f.hidden_field "delegated", value: 0 %>
<%= render "govuk_publishing_components/components/checkboxes", {
id: "supported_permission_delegatable",
name: "supported_permission[delegatable]",
id: "supported_permission_delegated",
name: "supported_permission[delegated]",
items: [
{
label: t('supported_permissions.form.delegatable'),
label: t('supported_permissions.form.delegated'),
value: 1,
checked: f.object.delegatable?,
checked: f.object.delegated?,
}
]
} %>
Expand Down
24 changes: 22 additions & 2 deletions app/views/supported_permissions/index.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,26 @@
} %>
</div>

<div class="govuk-grid-row">
<div class="govuk-grid-column-two-thirds">
<%= render "govuk_publishing_components/components/details", {
title: "What is a 'delegated' permission?"
} do %>
<p class="govuk-body">
'Delegating' and 'granting' permissions are separate but related
concepts. Permissions are 'granted' to individual users, while they are
'delegated' as grantable by publishing managers.
</p>
<p class="govuk-body">
When a permission is delegated, then, publishing managers have the
authority to grant the permission to users they manage. When the
'signin' permission is delegated, publishing managers can grant or
remove access to the given application.
</p>
<% end %>
</div>
</div>

<%= render "components/table", {
caption: "Permissions for #{@application.name}",
caption_classes: "govuk-visually-hidden",
Expand All @@ -37,7 +57,7 @@
text: "Created",
},
{
text: "Can be delegated?",
text: "Delegated",
},
{
text: "Is a default permission given to all new users?",
Expand All @@ -55,7 +75,7 @@
text: supported_permission.created_at.to_date.to_fs(:govuk_date),
},
{
text: supported_permission.delegatable? ? 'Yes' : 'No',
text: supported_permission.delegated? ? 'Yes' : 'No',
},
{
text: supported_permission.default? ? 'Yes' : 'No',
Expand Down
2 changes: 1 addition & 1 deletion app/views/users/permissions/edit.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
<% end %>
<% end %>

<% if notice = notice_about_non_delegatable_permissions(current_user, @application, @user) %>
<% if notice = notice_about_non_delegated_permissions(current_user, @application, @user) %>
<div class="govuk-grid-row">
<div class="govuk-grid-column-two-thirds">
<%= render "govuk_publishing_components/components/inset_text", { text: notice } %>
Expand Down
2 changes: 1 addition & 1 deletion config/locales/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ en:

supported_permissions:
form:
delegatable: 'Can be delegated'
delegated: 'Delegated'
default: 'Default permission added to all new users'
placeholder:
name: 'Name of application permission'
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
class RenameSupportedPermissionDelegatableColumn < ActiveRecord::Migration[7.2]
def change
rename_column :supported_permissions, :delegated, :delegated
end
end
4 changes: 2 additions & 2 deletions 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_08_09_112119) do
ActiveRecord::Schema[7.2].define(version: 2024_10_01_140623) do
create_table "batch_invitation_application_permissions", id: :integer, charset: "utf8mb3", force: :cascade do |t|
t.integer "batch_invitation_id", null: false
t.integer "supported_permission_id", null: false
Expand Down Expand Up @@ -133,7 +133,7 @@
t.string "name"
t.datetime "created_at", precision: nil
t.datetime "updated_at", precision: nil
t.boolean "delegatable", default: false
t.boolean "delegated", default: false
t.boolean "grantable_from_ui", default: true, null: false
t.boolean "default", default: false, null: false
t.index ["application_id", "name"], name: "index_supported_permissions_on_application_id_and_name", unique: true
Expand Down
28 changes: 17 additions & 11 deletions docs/access_and_permissions.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@

In addition to providing a single sign on solution for the GOV.UK publishing app suite, Signon can be used to manage users' access to apps and the permissions they have for each app. The permissions can then be used within specific non-Signon apps to dictate behaviour. This document provides a primer on key parts of how the management of access and permissions is implemented.

## Section notes
## Notes

### What can you do?
### "What can you do?" sections

Each "What can you do?" section provides a breakdown of the actions a given user (a 'granter') can take to manage access to and permissions for an app for a given user (a 'grantee').

Expand All @@ -13,12 +13,18 @@ The granter is either:
- a GOV.UK admin, meaning a user with the role "Superadmin" or "Admin"; or
- a publishing manager, meaning a user with the role "Super organisation admin" or "Organisation admin"

Each row represents an app with certain permissions set as delegatable, as indicated by the "Delegatable permissions" column. The "Grant access" column indicates whether a granter can give a grantee the `signin` permission, and the "Revoke access" column relates to removing the `signin` permission.
Each row represents an app with certain permissions set as delegated, as indicated by the "Delegated permissions" column. The "Grant access" column indicates whether a granter can give a grantee the `signin` permission, and the "Revoke access" column relates to removing the `signin` permission.

### Dependencies by route
### "Dependencies by route" sections

Each "Dependencies by route" section provides a breakdown of which actions are determined by dependencies and a tree or trees of relevant dependencies. The aim of this section isn't to go all the way down every node of each dependency tree, but rather to provide enough context on what determines what you can do, and to make it easier to identify shared dependencies. There's a particular focus on the different policies that are hit by each route, since the policies are both fundamental to how everything works and the source of a lot of complexity.

### Delegating vs granting permissions

'Delegating' and 'granting' permissions are separate but related concepts. Permissions are 'granted' to individual users, while they are 'delegated' as grantable by publishing managers.

When a permission is delegated, then, publishing managers have the authority to grant the permission to users they manage. When the 'signin' permission is delegated, publishing managers can grant or remove access to the given application.

## For the current user (self)

In this section, the granter and grantee are the same user: this is about managing your own access and permissions.
Expand All @@ -27,21 +33,21 @@ In this section, the granter and grantee are the same user: this is about managi

#### As a GOV.UK admin

| Delegatable permissions | Grant access | Revoke access | Edit permissions | View permissions |
| Delegated permissions | Grant access | Revoke access | Edit permissions | View permissions |
| ----------------------- | ------------ | ------------- | ---------------- | ---------------- |
| None |||||
| `signin` |||||
| Another permission |||||

#### As a publishing manager

| Delegatable permissions | Grant access | Revoke access | Edit permissions | View permissions |
| Delegated permissions | Grant access | Revoke access | Edit permissions | View permissions |
| ----------------------- | ------------ | ------------- | ---------------- | ---------------- |
| None |||||
| `signin` |||||
| Another permission |||\* ||

\* only delegatable non-signin permissions
\* only delegated non-signin permissions

### Dependencies by route

Expand Down Expand Up @@ -185,7 +191,7 @@ In this section, the granter and grantee are different users: this is about mana

##### With or without access to the app

| Delegatable permissions | Grant access | Revoke access | Edit permissions | View permissions |
| Delegated permissions | Grant access | Revoke access | Edit permissions | View permissions |
| ----------------------- | ------------ | ------------- | ---------------- | ---------------- |
| None |||||
| `signin` |||||
Expand All @@ -195,17 +201,17 @@ In this section, the granter and grantee are different users: this is about mana

##### With access to the app

| Delegatable permissions | Grant access | Revoke access | Edit permissions | View permissions |
| Delegated permissions | Grant access | Revoke access | Edit permissions | View permissions |
| ----------------------- | ------------ | ------------- | ---------------- | ---------------- |
| None |||||
| `signin` |||||
| Another permission |||\* ||

\* only delegatable non-signin permissions
\* only delegated non-signin permissions

##### Without access to the app

| Delegatable permissions | Grant access | Revoke access | Edit permissions | View permissions |
| Delegated permissions | Grant access | Revoke access | Edit permissions | View permissions |
| ----------------------- | ------------ | ------------- | ---------------- | ---------------- |
| None |||||
| `signin` |||||
Expand Down
Loading

0 comments on commit 7cd74f2

Please sign in to comment.