Skip to content

Commit

Permalink
Begin work on read-only admin permissions.
Browse files Browse the repository at this point in the history
Still needs UI work, but the start of a permission to allow admins to
view other admins, without giving them add/edit privileges.
  • Loading branch information
GUI committed Jan 23, 2024
1 parent de21cf8 commit bdf7188
Show file tree
Hide file tree
Showing 18 changed files with 149 additions and 43 deletions.
4 changes: 2 additions & 2 deletions src/api-umbrella/admin-ui/app/templates/application.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,14 @@
</div>
</li>
{{/if}}
{{#if (or this.currentAdmin.permissions.user_view this.currentAdmin.permissions.admin_manage)}}
{{#if (or this.currentAdmin.permissions.user_view this.currentAdmin.permissions.admin_view)}}
<li class="nav-item dropdown nav-users">
<a href="#" class="nav-link dropdown-toggle" role="button" data-toggle="dropdown" aria-haspopup="true" aria-expanded="false">{{t "Users"}}</a>
<div class="dropdown-menu">
{{#if this.currentAdmin.permissions.user_view}}
<LinkTo @route="api_users" class="dropdown-item"><FaIcon @icon="user" @fixedWidth={{true}} /> {{t "API Users"}}</LinkTo>
{{/if}}
{{#if this.currentAdmin.permissions.admin_manage}}
{{#if this.currentAdmin.permissions.admin_view}}
<LinkTo @route="admins" class="dropdown-item"><FaIcon @icon="user" @fixedWidth={{true}} /> {{t "Admin Accounts"}}</LinkTo>
<div class="dropdown-divider"></div>
<div role="presentation" class="dropdown-header">{{t "Permissions Management"}}</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,6 @@
{{else}}
{{f.checkbox-field "sendInviteEmail" label="Send invite email"}}
{{/if}}
{{else}}
{{f.static-field "notes" label=(t "Notes")}}
{{/if}}
</fieldset>

Expand Down Expand Up @@ -91,4 +89,4 @@
{{/if}}
</FieldsFor>
</form>
</div>
</div>
13 changes: 9 additions & 4 deletions src/api-umbrella/proxy/startup/seed_database.lua
Original file line number Diff line number Diff line change
Expand Up @@ -298,19 +298,24 @@ local function seed_admin_permissions()
display_order = 3,
},
{
id = "admin_manage",
name = "Admin Accounts - View & Manage",
id = "admin_view",
name = "Admin Accounts - View",
display_order = 4,
},
{
id = "admin_manage",
name = "Admin Accounts - Manage",
display_order = 5,
},
{
id = "backend_manage",
name = "API Backend Configuration - View & Manage",
display_order = 5,
display_order = 6,
},
{
id = "backend_publish",
name = "API Backend Configuration - Publish",
display_order = 6,
display_order = 7,
},
}

Expand Down
1 change: 1 addition & 0 deletions src/api-umbrella/web-app/actions/admin/sessions.lua
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,7 @@ function _M.auth(self)
response["admin"]["permissions"]["analytics"] = json_null_default(current_admin:allows_permission("analytics"))
response["admin"]["permissions"]["user_view"] = json_null_default(current_admin:allows_permission("user_view"))
response["admin"]["permissions"]["user_manage"] = json_null_default(current_admin:allows_permission("user_manage"))
response["admin"]["permissions"]["admin_view"] = json_null_default(current_admin:allows_permission("admin_view"))
response["admin"]["permissions"]["admin_manage"] = json_null_default(current_admin:allows_permission("admin_manage"))
response["admin"]["permissions"]["backend_manage"] = json_null_default(current_admin:allows_permission("backend_manage"))
response["admin"]["permissions"]["backend_publish"] = json_null_default(current_admin:allows_permission("backend_publish"))
Expand Down
12 changes: 9 additions & 3 deletions src/api-umbrella/web-app/policies/admin_group_policy.lua
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ function _M.authorized_query_scope(current_admin, permission_id)
end

if not permission_id then
permission_id = "admin_manage"
permission_id = "admin_view"
end

local api_scope_ids = current_admin:nested_api_scope_ids_with_permission(permission_id)
Expand Down Expand Up @@ -52,7 +52,7 @@ function _M.authorize_show(current_admin, data, permission_id)
end

if not permission_id then
permission_id = "admin_manage"
permission_id = "admin_view"
end

local all_scopes_allowed = false
Expand All @@ -75,6 +75,12 @@ function _M.authorize_show(current_admin, data, permission_id)
end
end

_M.authorize_modify = _M.authorize_show
function _M.authorize_modify(current_admin, data, permission_id)
if not permission_id then
permission_id = "admin_manage"
end

_M.authorize_show(current_admin, data, permission_id)
end

return _M
16 changes: 10 additions & 6 deletions src/api-umbrella/web-app/policies/admin_policy.lua
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ function _M.authorized_query_scope(current_admin, permission_id)
end

if not permission_id then
permission_id = "admin_manage"
permission_id = "admin_view"
end

local api_scope_ids = current_admin:nested_api_scope_ids_with_permission(permission_id)
Expand Down Expand Up @@ -46,7 +46,7 @@ function _M.authorized_query_scope(current_admin, permission_id)
end
end

function _M.is_authorized_modify(current_admin, data, permission_id)
function _M.is_authorized_show(current_admin, data, permission_id)
assert(current_admin)
assert(data)

Expand All @@ -59,7 +59,7 @@ function _M.is_authorized_modify(current_admin, data, permission_id)
end

if not permission_id then
permission_id = "admin_manage"
permission_id = "admin_view"
end

local all_groups_allowed = false
Expand All @@ -84,7 +84,11 @@ function _M.is_authorized_modify(current_admin, data, permission_id)
end

function _M.authorize_modify(current_admin, data, permission_id)
local allowed = _M.is_authorized_modify(current_admin, data, permission_id)
if not permission_id then
permission_id = "admin_manage"
end

local allowed = _M.is_authorized_show(current_admin, data, permission_id)
if allowed then
return true
else
Expand All @@ -94,11 +98,11 @@ end

function _M.authorize_show(current_admin, data, permission_id)
-- Allow admins to always view their own record, even if they don't have the
-- admin_manage privilege (so they can view their admin token).
-- admin_view privilege (so they can view their admin token).
--
-- TODO: An admin should also be able to update their own password if using
-- local password authentication, but that is not yet implemented.
local allowed = _M.is_authorized_modify(current_admin, data, permission_id) or (current_admin.id and data["id"] and current_admin.id == data["id"])
local allowed = _M.is_authorized_show(current_admin, data, permission_id) or (current_admin.id and data["id"] and current_admin.id == data["id"])
if allowed then
return true
else
Expand Down
12 changes: 9 additions & 3 deletions src/api-umbrella/web-app/policies/api_scope_policy.lua
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ function _M.authorized_query_scope(current_admin, permission_id)
end

if not permission_id then
permission_id = "admin_manage"
permission_id = "admin_view"
end

local api_scope_ids = current_admin:nested_api_scope_ids_with_permission(permission_id)
Expand All @@ -40,7 +40,7 @@ function _M.authorize_show(current_admin, data, permission_id)
end

if not permission_id then
permission_id = "admin_manage"
permission_id = "admin_view"
end

local any_scopes_allowed = false
Expand All @@ -59,6 +59,12 @@ function _M.authorize_show(current_admin, data, permission_id)
end
end

_M.authorize_modify = _M.authorize_show
function _M.authorize_modify(current_admin, data, permission_id)
if not permission_id then
permission_id = "admin_manage"
end

_M.authorize_show(current_admin, data, permission_id)
end

return _M
4 changes: 3 additions & 1 deletion test/admin_ui/test_admin_groups.rb
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,8 @@ def test_update
assert_checked_field("Analytics", :visible => :all)
assert_checked_field("API Users - View", :visible => :all)
assert_checked_field("API Users - Manage", :visible => :all)
assert_checked_field("Admin Accounts - View & Manage", :visible => :all)
assert_checked_field("Admin Accounts - View", :visible => :all)
assert_checked_field("Admin Accounts - Manage", :visible => :all)
assert_checked_field("API Backend Configuration - View & Manage", :visible => :all)
assert_checked_field("API Backend Configuration - Publish", :visible => :all)

Expand All @@ -65,6 +66,7 @@ def test_update
"analytics",
"user_view",
"user_manage",
"admin_view",
"admin_manage",
"backend_manage",
].sort, admin_group.permission_ids.sort)
Expand Down
2 changes: 1 addition & 1 deletion test/admin_ui/test_admins.rb
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ def test_non_admin_manager_views_own_profile
assert_text("Email")
assert_text(admin.username)
refute_field("Notes")
assert_text("Notes")
refute_text("Notes")
# TODO: Admins should be able to set their own password, even if they lack
# the admin_manage role.
refute_text("Change Your Password")
Expand Down
26 changes: 24 additions & 2 deletions test/admin_ui/test_permissions_navigation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,9 @@ def test_superuser
])
end

def test_admin_manage_permitted
def test_admin_view_permitted
admin = FactoryBot.create(:limited_admin, :groups => [
FactoryBot.create(:google_admin_group, :admin_manage_permission),
FactoryBot.create(:google_admin_group, :admin_view_permission),
])
admin_login(admin)

Expand All @@ -61,6 +61,28 @@ def test_admin_manage_permitted
])
end

def test_admin_view_forbidden
admin = FactoryBot.create(:limited_admin, :groups => [
FactoryBot.create(:google_admin_group, :permission_ids => []),
])
admin_login(admin)

assert_nav([
"API Umbrella",
])
end

def test_admin_manage_permitted
admin = FactoryBot.create(:limited_admin, :groups => [
FactoryBot.create(:google_admin_group, :admin_manage_permission),
])
admin_login(admin)

assert_nav([
"API Umbrella",
])
end

def test_admin_manage_forbidden
admin = FactoryBot.create(:limited_admin, :groups => [
FactoryBot.create(:google_admin_group, :permission_ids => []),
Expand Down
2 changes: 2 additions & 0 deletions test/apis/admin/test_auth.rb
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ def test_authenticated

assert_equal([
"admin_manage",
"admin_view",
"analytics",
"backend_manage",
"backend_publish",
Expand All @@ -83,6 +84,7 @@ def test_authenticated_no_cross_site_access
def test_limited_admin_permissions
permissions = [
"admin_manage",
"admin_view",
"analytics",
"backend_manage",
"backend_publish",
Expand Down
22 changes: 17 additions & 5 deletions test/apis/v1/admin_groups/test_admin_permissions.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,14 @@ def setup
setup_server
end

def test_default_permissions_single_scope
def test_default_permissions_admin_view_single_scope
factory = :google_admin_group
assert_default_admin_permissions(factory, :required_permissions => ["admin_manage"])
assert_default_admin_permissions(factory, :required_permissions => ["admin_view"])
end

def test_default_permissions_admin_manage_single_scope
factory = :google_admin_group
assert_default_admin_permissions(factory, :required_permissions => ["admin_view", "admin_manage"])
end

def test_multi_scope_permitted_as_superuser
Expand Down Expand Up @@ -106,9 +111,16 @@ def test_forbids_updating_unpermitted_groups_with_permitted_values
def assert_admin_permitted(factory, admin)
assert_admin_permitted_index(factory, admin)
assert_admin_permitted_show(factory, admin)
assert_admin_permitted_create(factory, admin)
assert_admin_permitted_update(factory, admin)
assert_admin_permitted_destroy(factory, admin)
permission_ids = admin.groups.map { |group| group.permission_ids }.flatten.uniq
if permission_ids.include?("admin_view") && !permission_ids.include?("admin_manage")
assert_admin_forbidden_create(factory, admin)
assert_admin_forbidden_update(factory, admin)
assert_admin_forbidden_destroy(factory, admin)
else
assert_admin_permitted_create(factory, admin)
assert_admin_permitted_update(factory, admin)
assert_admin_permitted_destroy(factory, admin)
end
end

def assert_admin_forbidden(factory, admin)
Expand Down
3 changes: 2 additions & 1 deletion test/apis/v1/admin_permissions/test_index.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@ def test_all_permissions_in_display_order
"Analytics",
"API Users - View",
"API Users - Manage",
"Admin Accounts - View & Manage",
"Admin Accounts - View",
"Admin Accounts - Manage",
"API Backend Configuration - View & Manage",
"API Backend Configuration - Publish",
], permission_names)
Expand Down
22 changes: 17 additions & 5 deletions test/apis/v1/admins/test_admin_permissions.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,14 @@ def setup
setup_server
end

def test_default_permissions_single_scope
def test_default_permissions_admin_view_single_scope
factory = :google_admin
assert_default_admin_permissions(factory, :required_permissions => ["admin_manage"])
assert_default_admin_permissions(factory, :required_permissions => ["admin_view"])
end

def test_default_permissions_admin_manage_single_scope
factory = :google_admin
assert_default_admin_permissions(factory, :required_permissions => ["admin_view", "admin_manage"])
end

def test_multi_group_multi_scope_permitted_as_superuser
Expand Down Expand Up @@ -270,9 +275,16 @@ def test_permits_any_admin_to_view_but_not_edit_own_record
def assert_admin_permitted(factory, admin)
assert_admin_permitted_index(factory, admin)
assert_admin_permitted_show(factory, admin)
assert_admin_permitted_create(factory, admin)
assert_admin_permitted_update(factory, admin)
assert_admin_permitted_destroy(factory, admin)
permission_ids = admin.groups.map { |group| group.permission_ids }.flatten.uniq
if permission_ids.include?("admin_view") && !permission_ids.include?("admin_manage")
assert_admin_forbidden_create(factory, admin)
assert_admin_forbidden_update(factory, admin)
assert_admin_forbidden_destroy(factory, admin)
else
assert_admin_permitted_create(factory, admin)
assert_admin_permitted_update(factory, admin)
assert_admin_permitted_destroy(factory, admin)
end
end

def assert_admin_forbidden(factory, admin)
Expand Down
22 changes: 17 additions & 5 deletions test/apis/v1/api_scopes/test_admin_permissions.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,14 @@ def setup
setup_server
end

def test_default_permissions
def test_default_admin_view_permissions
factory = :google_api_scope
assert_default_admin_permissions(factory, :required_permissions => ["admin_manage"])
assert_default_admin_permissions(factory, :required_permissions => ["admin_view"])
end

def test_default_admin_manage_permissions
factory = :google_api_scope
assert_default_admin_permissions(factory, :required_permissions => ["admin_view", "admin_manage"])
end

def test_forbids_updating_permitted_scopes_with_unpermitted_values
Expand Down Expand Up @@ -68,9 +73,16 @@ def test_forbids_updating_unpermitted_scopes_with_permitted_values
def assert_admin_permitted(factory, admin)
assert_admin_permitted_index(factory, admin)
assert_admin_permitted_show(factory, admin)
assert_admin_permitted_create(factory, admin)
assert_admin_permitted_update(factory, admin)
assert_admin_permitted_destroy(factory, admin)
permission_ids = admin.groups.map { |group| group.permission_ids }.flatten.uniq
if permission_ids.include?("admin_view") && !permission_ids.include?("admin_manage")
assert_admin_forbidden_create(factory, admin)
assert_admin_forbidden_update(factory, admin)
assert_admin_forbidden_destroy(factory, admin)
else
assert_admin_permitted_create(factory, admin)
assert_admin_permitted_update(factory, admin)
assert_admin_permitted_destroy(factory, admin)
end
end

def assert_admin_forbidden(factory, admin)
Expand Down
Loading

0 comments on commit bdf7188

Please sign in to comment.