From be77f323d3e76ee3d8b61b2525f55b945437789b Mon Sep 17 00:00:00 2001 From: Deimos Date: Thu, 9 Aug 2018 15:27:58 -0600 Subject: [PATCH] Refactor Topic ACL, update tests --- tildes/tests/test_topic.py | 66 ------------------- tildes/tests/test_topic_permissions.py | 85 ++++++++++++++++++++++++ tildes/tildes/models/topic/topic.py | 91 ++++++++++++++++++-------- 3 files changed, 150 insertions(+), 92 deletions(-) create mode 100644 tildes/tests/test_topic_permissions.py diff --git a/tildes/tests/test_topic.py b/tildes/tests/test_topic.py index 2427624..3f5369c 100644 --- a/tildes/tests/test_topic.py +++ b/tildes/tests/test_topic.py @@ -2,11 +2,6 @@ from datetime import timedelta from freezegun import freeze_time from marshmallow.fields import URL -from pyramid.security import ( - Authenticated, - Everyone, - principals_allowed_by_permission, -) from pytest import raises from tildes.lib.datetime import utc_now @@ -103,67 +98,6 @@ def test_edit_markdown_on_text_topic(text_topic): assert text_topic.rendered_html != original_html -def test_topic_viewing_permission(text_topic): - """Ensure that anyone can view a topic by default.""" - principals = principals_allowed_by_permission(text_topic, 'view') - assert Everyone in principals - - -def test_text_topic_editing_permission(text_topic): - """Ensure a text topic's owner (and nobody else) is able to edit it.""" - principals = principals_allowed_by_permission(text_topic, 'edit') - assert principals == {text_topic.user.user_id} - - -def test_deleted_topic_editing_permission(text_topic): - """Ensure a deleted topic can't be edited.""" - text_topic.is_deleted = True - principals = principals_allowed_by_permission(text_topic, 'edit') - assert not principals - - -def test_link_topic_editing_permission(link_topic): - """Ensure that nobody has edit permission on a link topic.""" - principals = principals_allowed_by_permission(link_topic, 'edit') - assert not principals - - -def test_topic_deleting_permission(text_topic): - """Ensure that the topic's owner (and nobody else) is able to delete it.""" - principals = principals_allowed_by_permission(text_topic, 'delete') - assert principals == {text_topic.user.user_id} - - -def test_deleted_topic_deleting_permission(text_topic): - """Ensure a deleted topic can't be deleted (again).""" - text_topic.is_deleted = True - assert not principals_allowed_by_permission(text_topic, 'delete') - - -def test_topic_view_author_permission(text_topic): - """Ensure anyone can view a topic's author normally.""" - principals = principals_allowed_by_permission(text_topic, 'view_author') - assert Everyone in principals - - -def test_deleted_topic_view_author_forbidden(text_topic): - """Ensure nobody can view the author of a deleted topic.""" - text_topic.is_deleted = True - principals = principals_allowed_by_permission(text_topic, 'view_author') - assert not principals - - -def test_topic_comment_permission(text_topic): - """Ensure authed users have comment perms before deletion, not after.""" - principals = principals_allowed_by_permission(text_topic, 'comment') - assert Authenticated in principals - - text_topic.is_deleted = True - - principals = principals_allowed_by_permission(text_topic, 'comment') - assert Authenticated not in principals - - def test_edit_grace_period(text_topic): """Ensure last_edited_time isn't set if the edit is inside grace period.""" one_sec = timedelta(seconds=1) diff --git a/tildes/tests/test_topic_permissions.py b/tildes/tests/test_topic_permissions.py new file mode 100644 index 0000000..3a72262 --- /dev/null +++ b/tildes/tests/test_topic_permissions.py @@ -0,0 +1,85 @@ +from pyramid.security import ( + Authenticated, + Everyone, + principals_allowed_by_permission, +) + + +def test_topic_viewing_permission(text_topic): + """Ensure that anyone can view a topic by default.""" + principals = principals_allowed_by_permission(text_topic, 'view') + assert Everyone in principals + + +def test_deleted_topic_permissions_removed(topic): + """Ensure that deleted topics lose all permissions except "view".""" + topic.is_deleted = True + + assert principals_allowed_by_permission(topic, 'view') == {Everyone} + + all_permissions = [ + perm for (_, _, perm) in topic.__acl__() if perm != 'view'] + for permission in all_permissions: + assert not principals_allowed_by_permission(topic, permission) + + +def test_text_topic_editing_permission(text_topic): + """Ensure a text topic's owner (and nobody else) is able to edit it.""" + principals = principals_allowed_by_permission(text_topic, 'edit') + assert principals == {text_topic.user.user_id} + + +def test_link_topic_editing_permission(link_topic): + """Ensure that nobody has edit permission on a link topic.""" + principals = principals_allowed_by_permission(link_topic, 'edit') + assert not principals + + +def test_topic_deleting_permission(text_topic): + """Ensure that the topic's owner (and nobody else) is able to delete it.""" + principals = principals_allowed_by_permission(text_topic, 'delete') + assert principals == {text_topic.user.user_id} + + +def test_topic_view_author_permission(text_topic): + """Ensure anyone can view a topic's author normally.""" + principals = principals_allowed_by_permission(text_topic, 'view_author') + assert Everyone in principals + + +def test_removed_topic_view_author_permission(topic): + """Ensure only admins and the author can view a removed topic's author.""" + topic.is_removed = True + principals = principals_allowed_by_permission(topic, 'view_author') + assert principals == {'admin', topic.user_id} + + +def test_topic_view_content_permission(text_topic): + """Ensure anyone can view a topic's content normally.""" + principals = principals_allowed_by_permission(text_topic, 'view_content') + assert Everyone in principals + + +def test_removed_topic_view_content_permission(topic): + """Ensure only admins and the author can view a removed topic's content.""" + topic.is_removed = True + principals = principals_allowed_by_permission(topic, 'view_content') + assert principals == {'admin', topic.user_id} + + +def test_topic_comment_permission(text_topic): + """Ensure authed users have comment perms on a topic by default.""" + principals = principals_allowed_by_permission(text_topic, 'comment') + assert Authenticated in principals + + +def test_locked_topic_comment_permission(topic): + """Ensure only admins can post (top-level) comments on locked topics.""" + topic.is_locked = True + assert principals_allowed_by_permission(topic, 'comment') == {'admin'} + + +def test_removed_topic_comment_permission(topic): + """Ensure only admins can post (top-level) comments on removed topics.""" + topic.is_removed = True + assert principals_allowed_by_permission(topic, 'comment') == {'admin'} diff --git a/tildes/tildes/models/topic/topic.py b/tildes/tildes/models/topic/topic.py index ea2e11e..1491d67 100644 --- a/tildes/tildes/models/topic/topic.py +++ b/tildes/tildes/models/topic/topic.py @@ -226,39 +226,78 @@ class Topic(DatabaseModel): def __acl__(self) -> Sequence[Tuple[str, Any, str]]: """Pyramid security ACL.""" - acl = [ - (Allow, Everyone, 'view'), - (Allow, 'admin', 'lock'), - (Allow, 'admin', 'move'), - (Allow, 'admin', 'edit_title'), - (Allow, 'admin', 'tag'), - ] - - if not (self.is_locked or self.is_deleted or self.is_removed): - acl.append((Allow, Authenticated, 'comment')) - else: + acl = [] + + # deleted topics allow "general" viewing, but nothing else + if self.is_deleted: + acl.append((Allow, Everyone, 'view')) + acl.append(DENY_ALL) + + # view: + # - everyone gets "general" viewing permission for all topics + acl.append((Allow, Everyone, 'view')) + + # view_author: + # - removed topics' author is only visible to the author and admins + # - otherwise, everyone can view the author + if self.is_removed: + acl.append((Allow, 'admin', 'view_author')) + acl.append((Allow, self.user_id, 'view_author')) + acl.append((Deny, Everyone, 'view_author')) + + acl.append((Allow, Everyone, 'view_author')) + + # view_content: + # - removed topics' content is only visible to the author and admins + # - otherwise, everyone can view the content + if self.is_removed: + acl.append((Allow, 'admin', 'view_content')) + acl.append((Allow, self.user_id, 'view_content')) + acl.append((Deny, Everyone, 'view_content')) + + acl.append((Allow, Everyone, 'view_content')) + + # vote: + # - removed topics can't be voted on by anyone + # - otherwise, logged-in users except the author can vote + if self.is_removed: + acl.append((Deny, Everyone, 'vote')) + + acl.append((Deny, self.user_id, 'vote')) + acl.append((Allow, Authenticated, 'vote')) + + # comment: + # - removed topics can only be commented on by admins + # - locked topics can only be commented on by admins + # - otherwise, logged-in users can comment + if self.is_removed: acl.append((Allow, 'admin', 'comment')) + acl.append((Deny, Everyone, 'comment')) - if not (self.is_deleted or self.is_removed): - acl.append((Allow, Everyone, 'view_content')) - acl.append((Allow, Everyone, 'view_author')) + if self.is_locked: + acl.append((Allow, 'admin', 'comment')) + acl.append((Deny, Everyone, 'comment')) - # everyone except the topic's author can vote on it - acl.append((Deny, self.user_id, 'vote')) - acl.append((Allow, Authenticated, 'vote')) + acl.append((Allow, Authenticated, 'comment')) - acl.append((Allow, self.user_id, 'tag')) + # edit: + # - only text topics can be edited, only by the author + if self.is_text_type: + acl.append((Allow, self.user_id, 'edit')) - if not self.is_deleted: - acl.append((Allow, 'admin', 'view')) + # delete: + # - only the author can delete + acl.append((Allow, self.user_id, 'delete')) - acl.append((Allow, self.user_id, 'view')) - acl.append((Allow, self.user_id, 'view_author')) - acl.append((Allow, self.user_id, 'view_content')) + # tag: + # - only the author and admins can tag topics + acl.append((Allow, self.user_id, 'tag')) + acl.append((Allow, 'admin', 'tag')) - if self.is_text_type: - acl.append((Allow, self.user_id, 'edit')) - acl.append((Allow, self.user_id, 'delete')) + # admin tools + acl.append((Allow, 'admin', 'lock')) + acl.append((Allow, 'admin', 'move')) + acl.append((Allow, 'admin', 'edit_title')) acl.append(DENY_ALL)