Use resourceful routes for granting/revoking user roles

This commit is contained in:
Anton Khorev 2024-10-28 03:44:10 +03:00
parent e15a92a302
commit b8247478f4
8 changed files with 59 additions and 58 deletions

View file

@ -68,7 +68,7 @@ class Ability
can [:index, :show, :resolve, :ignore, :reopen], Issue
can :create, IssueComment
can [:set_status, :destroy, :index], User
can [:grant, :revoke], UserRole
can [:create, :destroy], UserRole
end
end
end

View file

@ -6,9 +6,10 @@ module UserMethods
##
# ensure that there is a "user" instance variable
def lookup_user
@user = User.active.find_by!(:display_name => params[:display_name])
display_name = params[:display_name] || params[:user_display_name]
@user = User.active.find_by!(:display_name => display_name)
rescue ActiveRecord::RecordNotFound
render_unknown_user params[:display_name]
render_unknown_user display_name
end
##

View file

@ -9,15 +9,15 @@ class UserRolesController < ApplicationController
before_action :lookup_user
before_action :require_valid_role
before_action :not_in_role, :only => [:grant]
before_action :in_role, :only => [:revoke]
before_action :not_in_role, :only => :create
before_action :in_role, :only => :destroy
def grant
def create
@user.roles.create(:role => @role, :granter => current_user)
redirect_to user_path(@user)
end
def revoke
def destroy
# checks that administrator role is not revoked from current user
if current_user == @user && @role == "administrator"
flash[:error] = t("user_role.filter.not_revoke_admin_current_user")

View file

@ -7,12 +7,12 @@ module UserRolesHelper
if current_user&.administrator?
if user.role?(role)
link_to role_icon_svg_tag(role, false, t("users.show.role.revoke.#{role}")),
revoke_role_path(user, role),
:method => :post,
user_role_path(user, role),
:method => :delete,
:data => { :confirm => t("user_role.revoke.are_you_sure", :name => user.display_name, :role => role) }
else
link_to role_icon_svg_tag(role, true, t("users.show.role.grant.#{role}")),
grant_role_path(user, role),
user_role_path(user, role),
:method => :post,
:data => { :confirm => t("user_role.grant.are_you_sure", :name => user.display_name, :role => role) }
end

View file

@ -266,7 +266,9 @@ OpenStreetMap::Application.routes.draw do
post "/diary_comments/:comment/unhide" => "diary_comments#unhide", :comment => /\d+/, :as => :unhide_diary_comment
# user pages
resources :users, :path => "user", :param => :display_name, :only => [:show, :destroy]
resources :users, :path => "user", :param => :display_name, :only => [:show, :destroy] do
resource :role, :controller => "user_roles", :path => "roles/:role", :only => [:create, :destroy]
end
get "/user/:display_name/account", :to => redirect(:path => "/account/edit")
post "/user/:display_name/set_status" => "users#set_status", :as => :set_status_user
@ -323,9 +325,7 @@ OpenStreetMap::Application.routes.draw do
end
resources :user_mutes, :only => [:index]
# roles and banning pages
post "/user/:display_name/role/:role/grant" => "user_roles#grant", :as => "grant_role"
post "/user/:display_name/role/:role/revoke" => "user_roles#revoke", :as => "revoke_role"
# banning pages
get "/user/:display_name/blocks" => "user_blocks#blocks_on", :as => "user_blocks_on"
get "/user/:display_name/blocks_by" => "user_blocks#blocks_by", :as => "user_blocks_by"
get "/blocks/new/:display_name" => "user_blocks#new", :as => "new_user_block"

View file

@ -45,7 +45,7 @@ class GuestAbilityTest < AbilityTest
test "user roles permissions for a guest" do
ability = Ability.new nil
[:grant, :revoke].each do |action|
[:create, :destroy].each do |action|
assert ability.cannot?(action, UserRole), "should not be able to #{action} UserRoles"
end
end
@ -86,7 +86,7 @@ class ModeratorAbilityTest < AbilityTest
test "User Roles permissions" do
ability = Ability.new create(:moderator_user)
[:grant, :revoke].each do |action|
[:create, :destroy].each do |action|
assert ability.cannot?(action, UserRole), "should not be able to #{action} UserRoles"
end
@ -159,7 +159,7 @@ class AdministratorAbilityTest < AbilityTest
test "User Roles permissions for an administrator" do
ability = Ability.new create(:administrator_user)
[:grant, :revoke].each do |action|
[:create, :destroy].each do |action|
assert ability.can?(action, UserRole), "should be able to #{action} UserRoles"
end
end

View file

@ -5,32 +5,32 @@ class UserRolesControllerTest < ActionDispatch::IntegrationTest
# test all routes which lead to this controller
def test_routes
assert_routing(
{ :path => "/user/username/role/rolename/grant", :method => :post },
{ :controller => "user_roles", :action => "grant", :display_name => "username", :role => "rolename" }
{ :path => "/user/username/roles/rolename", :method => :post },
{ :controller => "user_roles", :action => "create", :user_display_name => "username", :role => "rolename" }
)
assert_routing(
{ :path => "/user/username/role/rolename/revoke", :method => :post },
{ :controller => "user_roles", :action => "revoke", :display_name => "username", :role => "rolename" }
{ :path => "/user/username/roles/rolename", :method => :delete },
{ :controller => "user_roles", :action => "destroy", :user_display_name => "username", :role => "rolename" }
)
end
##
# test the grant action
def test_grant
# test the grant role action
def test_update
target_user = create(:user)
normal_user = create(:user)
administrator_user = create(:administrator_user)
super_user = create(:super_user)
# Granting should fail when not logged in
post grant_role_path(target_user, "moderator")
post user_role_path(target_user, "moderator")
assert_response :forbidden
# Login as an unprivileged user
session_for(normal_user)
# Granting should still fail
post grant_role_path(target_user, "moderator")
post user_role_path(target_user, "moderator")
assert_redirected_to :controller => :errors, :action => :forbidden
# Login as an administrator
@ -39,7 +39,7 @@ class UserRolesControllerTest < ActionDispatch::IntegrationTest
UserRole::ALL_ROLES.each do |role|
# Granting a role to a non-existent user should fail
assert_difference "UserRole.count", 0 do
post grant_role_path("non_existent_user", role)
post user_role_path("non_existent_user", role)
end
assert_response :not_found
assert_template "users/no_such_user"
@ -47,20 +47,20 @@ class UserRolesControllerTest < ActionDispatch::IntegrationTest
# Granting a role to a user that already has it should fail
assert_no_difference "UserRole.count" do
post grant_role_path(super_user, role)
post user_role_path(super_user, role)
end
assert_redirected_to user_path(super_user)
assert_equal "The user already has role #{role}.", flash[:error]
# Granting a role to a user that doesn't have it should work...
assert_difference "UserRole.count", 1 do
post grant_role_path(target_user, role)
post user_role_path(target_user, role)
end
assert_redirected_to user_path(target_user)
# ...but trying a second time should fail
assert_no_difference "UserRole.count" do
post grant_role_path(target_user, role)
post user_role_path(target_user, role)
end
assert_redirected_to user_path(target_user)
assert_equal "The user already has role #{role}.", flash[:error]
@ -68,29 +68,29 @@ class UserRolesControllerTest < ActionDispatch::IntegrationTest
# Granting a non-existent role should fail
assert_difference "UserRole.count", 0 do
post grant_role_path(target_user, "no_such_role")
post user_role_path(target_user, "no_such_role")
end
assert_redirected_to user_path(target_user)
assert_equal "The string 'no_such_role' is not a valid role.", flash[:error]
end
##
# test the revoke action
def test_revoke
# test the revoke role action
def test_destroy
target_user = create(:user)
normal_user = create(:user)
administrator_user = create(:administrator_user)
super_user = create(:super_user)
# Revoking should fail when not logged in
post revoke_role_path(target_user, "moderator")
delete user_role_path(target_user, "moderator")
assert_response :forbidden
# Login as an unprivileged user
session_for(normal_user)
# Revoking should still fail
post revoke_role_path(target_user, "moderator")
delete user_role_path(target_user, "moderator")
assert_redirected_to :controller => :errors, :action => :forbidden
# Login as an administrator
@ -99,7 +99,7 @@ class UserRolesControllerTest < ActionDispatch::IntegrationTest
UserRole::ALL_ROLES.each do |role|
# Removing a role from a non-existent user should fail
assert_difference "UserRole.count", 0 do
post revoke_role_path("non_existent_user", role)
delete user_role_path("non_existent_user", role)
end
assert_response :not_found
assert_template "users/no_such_user"
@ -107,20 +107,20 @@ class UserRolesControllerTest < ActionDispatch::IntegrationTest
# Removing a role from a user that doesn't have it should fail
assert_no_difference "UserRole.count" do
post revoke_role_path(target_user, role)
delete user_role_path(target_user, role)
end
assert_redirected_to user_path(target_user)
assert_equal "The user does not have role #{role}.", flash[:error]
# Removing a role from a user that has it should work...
assert_difference "UserRole.count", -1 do
post revoke_role_path(super_user, role)
delete user_role_path(super_user, role)
end
assert_redirected_to user_path(super_user)
# ...but trying a second time should fail
assert_no_difference "UserRole.count" do
post revoke_role_path(super_user, role)
delete user_role_path(super_user, role)
end
assert_redirected_to user_path(super_user)
assert_equal "The user does not have role #{role}.", flash[:error]
@ -128,13 +128,13 @@ class UserRolesControllerTest < ActionDispatch::IntegrationTest
# Revoking a non-existent role should fail
assert_difference "UserRole.count", 0 do
post revoke_role_path(target_user, "no_such_role")
delete user_role_path(target_user, "no_such_role")
end
assert_redirected_to user_path(target_user)
assert_equal "The string 'no_such_role' is not a valid role.", flash[:error]
# Revoking administrator role from current user should fail
post revoke_role_path(administrator_user, "administrator")
delete user_role_path(administrator_user, "administrator")
assert_redirected_to user_path(administrator_user)
assert_equal "Cannot revoke administrator role from current user.", flash[:error]
end

View file

@ -31,7 +31,7 @@ class UserRolesHelperTest < ActionView::TestCase
create(:user) do |user|
icon = role_icon(user, "moderator")
icon_dom = Rails::Dom::Testing.html_document_fragment.parse(icon)
assert_dom icon_dom, "a:root[href='#{grant_role_path(user, 'moderator')}']", :count => 1 do
assert_dom icon_dom, "a:root[href='#{user_role_path(user, 'moderator')}'][data-method='post']", :count => 1 do
assert_dom "> svg", :count => 1 do
assert_dom "> title", :text => "Grant moderator access"
end
@ -39,7 +39,7 @@ class UserRolesHelperTest < ActionView::TestCase
icon = role_icon(user, "importer")
icon_dom = Rails::Dom::Testing.html_document_fragment.parse(icon)
assert_dom icon_dom, "a:root[href='#{grant_role_path(user, 'importer')}']", :count => 1 do
assert_dom icon_dom, "a:root[href='#{user_role_path(user, 'importer')}'][data-method='post']", :count => 1 do
assert_dom "> svg", :count => 1 do
assert_dom "> title", :text => "Grant importer access"
end
@ -49,7 +49,7 @@ class UserRolesHelperTest < ActionView::TestCase
create(:moderator_user) do |user|
icon = role_icon(user, "moderator")
icon_dom = Rails::Dom::Testing.html_document_fragment.parse(icon)
assert_dom icon_dom, "a:root[href='#{revoke_role_path(user, 'moderator')}']", :count => 1 do
assert_dom icon_dom, "a:root[href='#{user_role_path(user, 'moderator')}'][data-method='delete']", :count => 1 do
assert_dom "> svg", :count => 1 do
assert_dom "> title", :text => "Revoke moderator access"
end
@ -57,7 +57,7 @@ class UserRolesHelperTest < ActionView::TestCase
icon = role_icon(user, "importer")
icon_dom = Rails::Dom::Testing.html_document_fragment.parse(icon)
assert_dom icon_dom, "a:root[href='#{grant_role_path(user, 'importer')}']", :count => 1 do
assert_dom icon_dom, "a:root[href='#{user_role_path(user, 'importer')}'][data-method='post']", :count => 1 do
assert_dom "> svg", :count => 1 do
assert_dom "> title", :text => "Grant importer access"
end
@ -67,7 +67,7 @@ class UserRolesHelperTest < ActionView::TestCase
create(:importer_user) do |user|
icon = role_icon(user, "moderator")
icon_dom = Rails::Dom::Testing.html_document_fragment.parse(icon)
assert_dom icon_dom, "a:root[href='#{grant_role_path(user, 'moderator')}']", :count => 1 do
assert_dom icon_dom, "a:root[href='#{user_role_path(user, 'moderator')}'][data-method='post']", :count => 1 do
assert_dom "> svg", :count => 1 do
assert_dom "> title", :text => "Grant moderator access"
end
@ -75,7 +75,7 @@ class UserRolesHelperTest < ActionView::TestCase
icon = role_icon(user, "importer")
icon_dom = Rails::Dom::Testing.html_document_fragment.parse(icon)
assert_dom icon_dom, "a:root[href='#{revoke_role_path(user, 'importer')}']", :count => 1 do
assert_dom icon_dom, "a:root[href='#{user_role_path(user, 'importer')}'][data-method='delete']", :count => 1 do
assert_dom "> svg", :count => 1 do
assert_dom "> title", :text => "Revoke importer access"
end
@ -113,13 +113,13 @@ class UserRolesHelperTest < ActionView::TestCase
icons = role_icons(user)
icons_dom = Rails::Dom::Testing.html_document_fragment.parse(icons)
assert_dom icons_dom, "a:root", :count => 3
assert_dom icons_dom, "a:root[href='#{grant_role_path(user, 'administrator')}']" do
assert_dom icons_dom, "a:root[href='#{user_role_path(user, 'administrator')}'][data-method='post']" do
assert_dom "> svg > title", :text => "Grant administrator access"
end
assert_dom icons_dom, "a:root[href='#{grant_role_path(user, 'moderator')}']" do
assert_dom icons_dom, "a:root[href='#{user_role_path(user, 'moderator')}'][data-method='post']" do
assert_dom "> svg > title", :text => "Grant moderator access"
end
assert_dom icons_dom, "a:root[href='#{grant_role_path(user, 'importer')}']" do
assert_dom icons_dom, "a:root[href='#{user_role_path(user, 'importer')}'][data-method='post']" do
assert_dom "> svg > title", :text => "Grant importer access"
end
end
@ -128,13 +128,13 @@ class UserRolesHelperTest < ActionView::TestCase
icons = role_icons(user)
icons_dom = Rails::Dom::Testing.html_document_fragment.parse(icons)
assert_dom icons_dom, "a:root", :count => 3
assert_dom icons_dom, "a:root[href='#{grant_role_path(user, 'administrator')}']" do
assert_dom icons_dom, "a:root[href='#{user_role_path(user, 'administrator')}'][data-method='post']" do
assert_dom "> svg > title", :text => "Grant administrator access"
end
assert_dom icons_dom, "a:root[href='#{revoke_role_path(user, 'moderator')}']" do
assert_dom icons_dom, "a:root[href='#{user_role_path(user, 'moderator')}'][data-method='delete']" do
assert_dom "> svg > title", :text => "Revoke moderator access"
end
assert_dom icons_dom, "a:root[href='#{grant_role_path(user, 'importer')}']" do
assert_dom icons_dom, "a:root[href='#{user_role_path(user, 'importer')}'][data-method='post']" do
assert_dom "> svg > title", :text => "Grant importer access"
end
end
@ -143,13 +143,13 @@ class UserRolesHelperTest < ActionView::TestCase
icons = role_icons(user)
icons_dom = Rails::Dom::Testing.html_document_fragment.parse(icons)
assert_dom icons_dom, "a:root", :count => 3
assert_dom icons_dom, "a:root[href='#{grant_role_path(user, 'administrator')}']" do
assert_dom icons_dom, "a:root[href='#{user_role_path(user, 'administrator')}'][data-method='post']" do
assert_dom "> svg > title", :text => "Grant administrator access"
end
assert_dom icons_dom, "a:root[href='#{grant_role_path(user, 'moderator')}']" do
assert_dom icons_dom, "a:root[href='#{user_role_path(user, 'moderator')}'][data-method='post']" do
assert_dom "> svg > title", :text => "Grant moderator access"
end
assert_dom icons_dom, "a:root[href='#{revoke_role_path(user, 'importer')}']" do
assert_dom icons_dom, "a:root[href='#{user_role_path(user, 'importer')}'][data-method='delete']" do
assert_dom "> svg > title", :text => "Revoke importer access"
end
end
@ -158,13 +158,13 @@ class UserRolesHelperTest < ActionView::TestCase
icons = role_icons(user)
icons_dom = Rails::Dom::Testing.html_document_fragment.parse(icons)
assert_dom icons_dom, "a:root", :count => 3
assert_dom icons_dom, "a:root[href='#{revoke_role_path(user, 'administrator')}']" do
assert_dom icons_dom, "a:root[href='#{user_role_path(user, 'administrator')}'][data-method='delete']" do
assert_dom "> svg > title", :text => "Revoke administrator access"
end
assert_dom icons_dom, "a:root[href='#{revoke_role_path(user, 'moderator')}']" do
assert_dom icons_dom, "a:root[href='#{user_role_path(user, 'moderator')}'][data-method='delete']" do
assert_dom "> svg > title", :text => "Revoke moderator access"
end
assert_dom icons_dom, "a:root[href='#{revoke_role_path(user, 'importer')}']" do
assert_dom icons_dom, "a:root[href='#{user_role_path(user, 'importer')}'][data-method='delete']" do
assert_dom "> svg > title", :text => "Revoke importer access"
end
end