Skip to content

Commit

Permalink
WIP: Allow GOV.UK Admins to remove their access to apps
Browse files Browse the repository at this point in the history
  • Loading branch information
chrisroos committed Sep 19, 2023
1 parent 29eabf9 commit c8ccbbc
Show file tree
Hide file tree
Showing 8 changed files with 74 additions and 2 deletions.
16 changes: 16 additions & 0 deletions app/controllers/account/signin_permissions_controller.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
class Account::SigninPermissionsController < ApplicationController
layout "admin_layout"

before_action :authenticate_user!

def create
Expand All @@ -8,4 +10,18 @@ def create
current_user.grant_application_signin_permission(application)
redirect_to account_applications_path
end

def delete
authorize :account_applications, :remove_signin_permission?

@application = Doorkeeper::Application.find(params[:application_id])
end

def destroy
authorize :account_applications, :remove_signin_permission?

application = Doorkeeper::Application.find(params[:application_id])
current_user.remove_application_signin_permission(application)
redirect_to account_applications_path
end
end
7 changes: 7 additions & 0 deletions app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,13 @@ def grant_application_permissions(application, supported_permission_names)
end
end

def remove_application_signin_permission(application)
# TODO: Ensure this is idempotent
# TODO: Handle case where application.signin_permission is nil
# TODO: Can there be more than one of these application_permissions?
application_permissions.where(supported_permission: application.signin_permission).first.destroy!
end

def grant_permission(supported_permission)
application_permissions.where(supported_permission_id: supported_permission.id).first_or_create!
end
Expand Down
1 change: 1 addition & 0 deletions app/policies/account_applications_policy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,5 @@ def index?
end

alias_method :grant_signin_permission?, :index?
alias_method :remove_signin_permission?, :index? # TODO: Test this policy
end
10 changes: 9 additions & 1 deletion app/views/account/applications/index.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,22 @@
<thead class="govuk-table__head">
<tr class="govuk-table__row">
<th scope="col" class="govuk-table__header govuk-!-width-one-quarter">Name</th>
<th scope="col" class="govuk-table__header govuk-!-width-three-quarters">Description</th>
<th scope="col" class="govuk-table__header govuk-!-width-two-quarters">Description</th>
<th scope="col" class="govuk-table__header govuk-!-width-one-quarter"></th>
</tr>
</thead>
<tbody class="govuk-table__body">
<% @applications_with_signin.each do |application| %>
<tr class="govuk-table__row">
<td class="govuk-table__cell"><%= application.name %></td>
<td class="govuk-table__cell"><%= application.description %></td>
<td class="govuk-table__cell">
<%= link_to delete_account_application_signin_permission_path(application),
class: "govuk-button govuk-!-margin-0",
data: { module: "govuk-button" } do %>
Remove access<span class="govuk-visually-hidden"> to <%= application.name %></span>
<% end %>
</td>
</tr>
<% end %>
</tbody>
Expand Down
3 changes: 3 additions & 0 deletions app/views/account/signin_permissions/delete.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<%# TODO: Implement Calum's designs %>

<%= button_to "Confirm", account_application_signin_permission_path(@application), method: :delete %>
4 changes: 3 additions & 1 deletion config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,9 @@
resource :account, only: [:show]
namespace :account do
resources :applications, only: [:index] do
resource :signin_permission, only: [:create]
resource :signin_permission, only: [:create, :destroy] do

Check failure on line 55 in config/routes.rb

View workflow job for this annotation

GitHub Actions / Lint Ruby / Run RuboCop

Style/SymbolArray: Use `%i` or `%I` for an array of symbols. (https://rubystyle.guide#percent-i)
get :delete
end
end
end

Expand Down
22 changes: 22 additions & 0 deletions test/integration/account_applications_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -79,4 +79,26 @@ class AccountApplicationsTest < ActionDispatch::IntegrationTest
assert table.has_content?("app-name")
end
end

context "removing access to apps" do
setup do
application = create(:application, name: "app-name", description: "app-description")
@user = FactoryBot.create(:admin_user)
@user.grant_application_signin_permission(application)
end

should "allow admins to remove their access to apps" do
visit new_user_session_path
signin_with @user

visit account_applications_path

click_on "Remove access to app-name"
click_on "Confirm"

heading = find("h2", text: "Apps you don't have access to")
table = find("table[aria-labelledby='#{heading['id']}']")
assert table.has_content?("app-name")
end
end
end
13 changes: 13 additions & 0 deletions test/models/user_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -510,6 +510,19 @@ def setup
assert user.has_access_to?(app)
end

test "removes signin permission" do
app = create(:application)
user = create(:user, with_signin_permissions_for: [app])

assert user.has_access_to?(app)

user.remove_application_signin_permission(app)
# TODO: How can I avoid this reload?
user.reload

assert_not user.has_access_to?(app)
end

test "can grant permissions to users and return the created permission" do
app = create(:application, name: "my_app", with_supported_permissions: ["Create publications", "Delete publications"])
user = create(:user)
Expand Down

0 comments on commit c8ccbbc

Please sign in to comment.