Skip to content

Commit

Permalink
moderation: add self-action prevention
Browse files Browse the repository at this point in the history
* The problem is that an admin could block his own
  account. With this change it is possible to prevent
  the admin from doing that.
* Prevent self-action for: block, deactivate, impersonate, restore, activate and approve.
* Update tests for self-action prevention
* closes <inveniosoftware/invenio-administration#203>
  • Loading branch information
Samk13 committed Apr 30, 2024
1 parent 4c31af2 commit b26d3c4
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 7 deletions.
20 changes: 13 additions & 7 deletions invenio_users_resources/services/users/service.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# -*- coding: utf-8 -*-
#
# Copyright (C) 2022 KTH Royal Institute of Technology
# Copyright (C) 2022-2024 KTH Royal Institute of Technology
# Copyright (C) 2022 TU Wien.
# Copyright (C) 2022 European Union.
# Copyright (C) 2022 CERN.
Expand Down Expand Up @@ -132,7 +132,8 @@ def rebuild_index(self, identity, uow=None):
def block(self, identity, id_, uow=None):
"""Blocks a user."""
user = UserAggregate.get_record(id_)
if user is None:
is_self_action = str(identity.id) == id_
if user is None or is_self_action:
# return 403 even on empty resource due to security implications
raise PermissionDeniedError()

Expand All @@ -156,7 +157,8 @@ def block(self, identity, id_, uow=None):
def restore(self, identity, id_, uow=None):
"""Restores a user."""
user = UserAggregate.get_record(id_)
if user is None:
is_self_action = str(identity.id) == id_
if user is None or is_self_action:
# return 403 even on empty resource due to security implications
raise PermissionDeniedError()

Expand All @@ -181,7 +183,8 @@ def restore(self, identity, id_, uow=None):
def approve(self, identity, id_, uow=None):
"""Approves a user."""
user = UserAggregate.get_record(id_)
if user is None:
is_self_action = str(identity.id) == id_
if user is None or is_self_action:
# return 403 even on empty resource due to security implications
raise PermissionDeniedError()

Expand All @@ -205,7 +208,8 @@ def approve(self, identity, id_, uow=None):
def deactivate(self, identity, id_, uow=None):
"""Deactivates a user."""
user = UserAggregate.get_record(id_)
if user is None:
is_self_action = str(identity.id) == id_
if user is None or is_self_action:
# return 403 even on empty resource due to security implications
raise PermissionDeniedError()
self.require_permission(identity, "manage", record=user)
Expand All @@ -221,7 +225,8 @@ def deactivate(self, identity, id_, uow=None):
def activate(self, identity, id_, uow=None):
"""Activate a user."""
user = UserAggregate.get_record(id_)
if user is None:
is_self_action = str(identity.id) == id_
if user is None or is_self_action:
# return 403 even on empty resource due to security implications
raise PermissionDeniedError()
self.require_permission(identity, "manage", record=user)
Expand All @@ -234,7 +239,8 @@ def activate(self, identity, id_, uow=None):
def can_impersonate(self, identity, id_):
"""Check permissions if a user can be impersonated."""
user = UserAggregate.get_record(id_)
if user is None:
is_self_action = str(identity.id) == id_
if user is None or is_self_action:
# return 403 even on empty resource due to security implications
raise PermissionDeniedError()
self.require_permission(identity, "impersonate", record=user)
Expand Down
19 changes: 19 additions & 0 deletions tests/resources/test_resources_users.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
#
# Copyright (C) 2022 European Union.
# Copyright (C) 2022 CERN.
# Copyright (C) 2024 KTH Royal Institute of Technology
#
# Invenio-Users-Resources is free software; you can redistribute it and/or
# modify it under the terms of the MIT License; see LICENSE file for more
Expand Down Expand Up @@ -126,6 +127,10 @@ def test_approve_user(client, headers, user_pub, user_moderator, db):
assert res.status_code == 200
assert res.json["verified_at"] is not None

# Test user tries to approve themselves
res = client.post(f"/users/{user_moderator.id}/approve", headers=headers)
assert res.status_code == 403


def test_block_user(client, headers, user_pub, user_moderator, db):
"""Tests block user endpoint."""
Expand All @@ -137,6 +142,13 @@ def test_block_user(client, headers, user_pub, user_moderator, db):
assert res.status_code == 200
assert res.json["blocked_at"] is not None

# Test user tries to block themselves
res = client.post(f"/users/{user_moderator.id}/block", headers=headers)
assert res.status_code == 403

res = client.get(f"/users/{user_moderator.id}")
assert res.status_code == 200


def test_deactivate_user(client, headers, user_pub, user_moderator, db):
"""Tests deactivate user endpoint."""
Expand All @@ -148,6 +160,13 @@ def test_deactivate_user(client, headers, user_pub, user_moderator, db):
assert res.status_code == 200
assert res.json["active"] == False

# Test user tries to deactivate themselves
res = client.post(f"/users/{user_moderator.id}/deactivate", headers=headers)
assert res.status_code == 403

res = client.get(f"/users/{user_moderator.id}")
assert res.status_code == 200


def test_management_permissions(client, headers, user_pub, db):
"""Test permissions at the resource level."""
Expand Down

0 comments on commit b26d3c4

Please sign in to comment.