From 94b9e1bf10b678a3f443c4bd9925e949e13e5abe Mon Sep 17 00:00:00 2001 From: Deimos Date: Sat, 29 Feb 2020 14:40:54 -0700 Subject: [PATCH] Rework permissions/ACL system This is a major rework of the permissions system to enable various new capabilities and clean up some of the oddities that were there. Highlights: - The concept of "admin" permission is removed. All permissions must be granted individually. - Permissions can now be granted on a group-specific level, such as giving a user the ability to tag topics only in a specific group. - Permissions can also be denied for a specific group (or all groups), enabling uses like "tag topics in all groups except ~music". - Removed the two cases where "all permissions" were granted: users on themselves and the sender and recipient on messages. This was dangerous, we should always grant permissions explicitly. - Eliminated all the granular permissions for changing a user's settings (which were all granted implicitly), and replaced with an overall "change_settings" permission. --- ...6_rename_column_for_restricted_posting_.py | 40 ++++++++ tildes/tests/test_comment.py | 26 ++++- tildes/tests/test_topic_permissions.py | 39 ++++++-- tildes/tests/test_user.py | 6 +- tildes/tildes/lib/auth.py | 38 +++++++ tildes/tildes/models/comment/comment.py | 60 +++++++---- .../models/comment/comment_notification.py | 5 +- tildes/tildes/models/group/group.py | 28 ++++-- tildes/tildes/models/group/group_wiki_page.py | 11 +-- tildes/tildes/models/message/message.py | 17 ++-- tildes/tildes/models/topic/topic.py | 99 +++++++++++-------- tildes/tildes/models/user/user.py | 32 +++--- tildes/tildes/models/user/user_permissions.py | 23 +++-- tildes/tildes/typing.py | 10 ++ tildes/tildes/views/api/web/user.py | 32 +++--- tildes/tildes/views/user.py | 4 +- 16 files changed, 328 insertions(+), 142 deletions(-) create mode 100644 tildes/alembic/versions/84dc19f6e876_rename_column_for_restricted_posting_.py create mode 100644 tildes/tildes/lib/auth.py create mode 100644 tildes/tildes/typing.py diff --git a/tildes/alembic/versions/84dc19f6e876_rename_column_for_restricted_posting_.py b/tildes/alembic/versions/84dc19f6e876_rename_column_for_restricted_posting_.py new file mode 100644 index 0000000..2b32bd0 --- /dev/null +++ b/tildes/alembic/versions/84dc19f6e876_rename_column_for_restricted_posting_.py @@ -0,0 +1,40 @@ +"""Rename column for restricted-posting groups + +Revision ID: 84dc19f6e876 +Revises: 054aaef690cd +Create Date: 2020-02-29 03:03:31.968814 + +""" +from alembic import op +import sqlalchemy as sa + + +# revision identifiers, used by Alembic. +revision = "84dc19f6e876" +down_revision = "054aaef690cd" +branch_labels = None +depends_on = None + + +def upgrade(): + op.alter_column( + "groups", + "is_admin_posting_only", + new_column_name="requires_permission_to_post_topics", + ) + + op.execute( + "update user_permissions set permission = 'wiki.edit' where permission = 'wiki'" + ) + + +def downgrade(): + op.alter_column( + "groups", + "requires_permission_to_post_topics", + new_column_name="is_admin_posting_only", + ) + + op.execute( + "update user_permissions set permission = 'wiki' where permission = 'wiki.edit'" + ) diff --git a/tildes/tests/test_comment.py b/tildes/tests/test_comment.py index 92a56f2..7114500 100644 --- a/tildes/tests/test_comment.py +++ b/tildes/tests/test_comment.py @@ -4,15 +4,27 @@ from datetime import timedelta from freezegun import freeze_time -from pyramid.security import Authenticated, Everyone, principals_allowed_by_permission +from pyramid.security import ( + Allow, + Authenticated, + Everyone, + principals_allowed_by_permission, +) from tildes.enums import CommentTreeSortOption +from tildes.lib.auth import aces_for_permission from tildes.lib.datetime import utc_now from tildes.models.comment import Comment, CommentTree, EDIT_GRACE_PERIOD from tildes.schemas.comment import CommentSchema from tildes.schemas.fields import Markdown +def _principals_granted_permission(permission, group_id): + aces = aces_for_permission(permission, group_id) + + return set([ace[1] for ace in aces if ace[0] == Allow]) + + def test_comment_creation_validates_schema(mocker, session_user, topic): """Ensure that comment creation goes through schema validation.""" mocker.spy(CommentSchema, "load") @@ -68,9 +80,12 @@ def test_comment_replying_permission(comment): def test_comment_reply_locked_thread_permission(comment): - """Ensure that only admins can reply in locked threads.""" + """Ensure that only users with lock permission can reply in locked threads.""" comment.topic.is_locked = True - assert principals_allowed_by_permission(comment, "reply") == {"admin"} + + allowed = principals_allowed_by_permission(comment, "reply") + granted = _principals_granted_permission("topic.lock", comment.topic.group_id) + assert allowed == granted def test_deleted_comment_permissions_removed(comment): @@ -85,8 +100,9 @@ def test_deleted_comment_permissions_removed(comment): def test_removed_comment_view_permission(comment): """Ensure a removed comment can only be viewed by certain users.""" comment.is_removed = True - principals = principals_allowed_by_permission(comment, "view") - assert principals == {"admin", comment.user_id, "comment.remove"} + allowed = principals_allowed_by_permission(comment, "view") + granted = _principals_granted_permission("comment.remove", comment.topic.group_id) + assert allowed == granted | {comment.user_id} def test_edit_grace_period(comment): diff --git a/tildes/tests/test_topic_permissions.py b/tildes/tests/test_topic_permissions.py index 24411ea..9040245 100644 --- a/tildes/tests/test_topic_permissions.py +++ b/tildes/tests/test_topic_permissions.py @@ -1,7 +1,20 @@ # Copyright (c) 2018 Tildes contributors # SPDX-License-Identifier: AGPL-3.0-or-later -from pyramid.security import Authenticated, Everyone, principals_allowed_by_permission +from pyramid.security import ( + Allow, + Authenticated, + Everyone, + principals_allowed_by_permission, +) + +from tildes.lib.auth import aces_for_permission + + +def _principals_granted_permission(permission, group_id): + aces = aces_for_permission(permission, group_id) + + return set([ace[1] for ace in aces if ace[0] == Allow]) def test_topic_viewing_permission(text_topic): @@ -46,10 +59,11 @@ def test_topic_view_author_permission(text_topic): def test_removed_topic_view_author_permission(topic): - """Ensure only a removed topic's author can only be viewed by certain users.""" + """Ensure removed topic's author can only be viewed by certain users.""" topic.is_removed = True - principals = principals_allowed_by_permission(topic, "view_author") - assert principals == {"admin", topic.user_id, "topic.remove"} + allowed = principals_allowed_by_permission(topic, "view_author") + granted = _principals_granted_permission("topic.remove", topic.group_id) + assert allowed == granted | {topic.user_id} def test_topic_view_content_permission(text_topic): @@ -61,8 +75,9 @@ def test_topic_view_content_permission(text_topic): def test_removed_topic_view_content_permission(topic): """Ensure a removed topic's content can only be viewed by certain users.""" topic.is_removed = True - principals = principals_allowed_by_permission(topic, "view_content") - assert principals == {"admin", topic.user_id, "topic.remove"} + allowed = principals_allowed_by_permission(topic, "view_content") + granted = _principals_granted_permission("topic.remove", topic.group_id) + assert allowed == granted | {topic.user_id} def test_topic_comment_permission(text_topic): @@ -72,12 +87,16 @@ def test_topic_comment_permission(text_topic): def test_locked_topic_comment_permission(topic): - """Ensure only admins can post (top-level) comments on locked topics.""" + """Ensure only users with lock permission can post comments on locked topics.""" topic.is_locked = True - assert principals_allowed_by_permission(topic, "comment") == {"admin"} + allowed = principals_allowed_by_permission(topic, "comment") + granted = _principals_granted_permission("topic.lock", topic.group_id) + assert allowed == granted def test_removed_topic_comment_permission(topic): - """Ensure only admins can post (top-level) comments on removed topics.""" + """Ensure only users with remove permission can post comments on removed topics.""" topic.is_removed = True - assert principals_allowed_by_permission(topic, "comment") == {"admin"} + allowed = principals_allowed_by_permission(topic, "comment") + granted = _principals_granted_permission("topic.remove", topic.group_id) + assert allowed == granted diff --git a/tildes/tests/test_user.py b/tildes/tests/test_user.py index 639b425..5680f8e 100644 --- a/tildes/tests/test_user.py +++ b/tildes/tests/test_user.py @@ -154,9 +154,9 @@ def test_banned_user_no_message_permission(): assert not principals -def test_only_admin_has_ban_permission(): - """Ensure only admins have ban permissions.""" +def test_ban_permission_manually_granted(): + """Ensure it requires manually granting ban permissions.""" user = User("Test_User", "password") principals = principals_allowed_by_permission(user, "ban") - assert principals == {"admin"} + assert principals == {"*:user.ban"} diff --git a/tildes/tildes/lib/auth.py b/tildes/tildes/lib/auth.py new file mode 100644 index 0000000..c59e431 --- /dev/null +++ b/tildes/tildes/lib/auth.py @@ -0,0 +1,38 @@ +# Copyright (c) 2020 Tildes contributors +# SPDX-License-Identifier: AGPL-3.0-or-later + +"""Functions to help with authorization, such as generating ACLs.""" + +from typing import List, Optional + +from pyramid.security import Allow, Deny + +from tildes.typing import AceType + + +def aces_for_permission( + required_permission: str, + group_id: Optional[int] = None, + granted_permission: Optional[str] = None, +) -> List[AceType]: + """Return the ACEs for manually-granted (or denied) entries in UserPermissions.""" + aces = [] + + # If the granted permission wasn't specified, use the required one without the type. + # So if required is "topic.lock", the granted permission defaults to "lock". + if granted_permission is None: + granted_permission = required_permission.split(".", maxsplit=1)[1] + + contexts = ["*"] + if group_id is not None: + contexts.append(str(group_id)) + + # add Deny entries first + for context in contexts: + aces.append((Deny, f"{context}:!{required_permission}", granted_permission)) + + # then Allow entries + for context in contexts: + aces.append((Allow, f"{context}:{required_permission}", granted_permission)) + + return aces diff --git a/tildes/tildes/models/comment/comment.py b/tildes/tildes/models/comment/comment.py index 664a404..6ae24b9 100644 --- a/tildes/tildes/models/comment/comment.py +++ b/tildes/tildes/models/comment/comment.py @@ -5,14 +5,22 @@ from collections import Counter from datetime import datetime, timedelta -from typing import Any, Optional, Sequence, Tuple, TYPE_CHECKING, Union +from typing import Any, Optional, Sequence, TYPE_CHECKING, Union -from pyramid.security import Allow, Authenticated, Deny, DENY_ALL, Everyone +from pyramid.security import ( + Allow, + Authenticated, + Deny, + DENY_ALL, + Everyone, + principals_allowed_by_permission, +) from sqlalchemy import Boolean, Column, ForeignKey, Index, Integer, Text, TIMESTAMP from sqlalchemy.dialects.postgresql import TSVECTOR from sqlalchemy.orm import deferred, relationship from sqlalchemy.sql.expression import text +from tildes.lib.auth import aces_for_permission from tildes.lib.datetime import utc_now from tildes.lib.id import id_to_id36 from tildes.lib.markdown import convert_markdown_to_safe_html @@ -22,6 +30,7 @@ from tildes.models import DatabaseModel from tildes.models.topic import Topic from tildes.models.user import User from tildes.schemas.comment import CommentSchema +from tildes.typing import AclType if TYPE_CHECKING: # workaround for mypy issues with @hybrid_property from builtins import property as hybrid_property @@ -148,28 +157,36 @@ class Comment(DatabaseModel): def _update_creation_metric(self) -> None: incr_counter("comments") - def __acl__(self) -> Sequence[Tuple[str, Any, str]]: + def __acl__(self) -> AclType: """Pyramid security ACL.""" - acl = [] - # nobody has any permissions on deleted comments if self.is_deleted: - acl.append(DENY_ALL) + return [DENY_ALL] + + acl = [] + + acl.extend(aces_for_permission("comment.view_labels", self.topic.group_id)) + acl.extend(aces_for_permission("comment.remove", self.topic.group_id)) # view: - # - removed comments can only be viewed by admins, the author, and users with - # remove permission + # - removed comments can only be viewed by the author, and users with remove + # permission # - otherwise, everyone can view if self.is_removed: - acl.append((Allow, "admin", "view")) acl.append((Allow, self.user_id, "view")) - acl.append((Allow, "comment.remove", "view")) + acl.extend( + aces_for_permission( + required_permission="comment.remove", + granted_permission="view", + group_id=self.topic.group_id, + ) + ) acl.append((Deny, Everyone, "view")) acl.append((Allow, Everyone, "view")) # view exemplary reasons: - # - only author gets shown the reasons (admins can see as well with all labels) + # - only author gets shown the reasons ("view_labels" does this too) acl.append((Allow, self.user_id, "view_exemplary_reasons")) # vote: @@ -195,15 +212,22 @@ class Comment(DatabaseModel): acl.append((Allow, "comment.label", "label")) # reply: - # - removed comments can only be replied to by admins - # - if the topic is locked, only admins can reply + # - removed comments can only be replied to by users who can remove + # - if the topic is locked, only users that can lock the topic can reply # - otherwise, logged-in users can reply if self.is_removed: - acl.append((Allow, "admin", "reply")) + acl.extend( + aces_for_permission( + required_permission="comment.remove", + granted_permission="reply", + group_id=self.topic.group_id, + ) + ) acl.append((Deny, Everyone, "reply")) if self.topic.is_locked: - acl.append((Allow, "admin", "reply")) + lock_principals = principals_allowed_by_permission(self.topic, "lock") + acl.extend([(Allow, principal, "reply") for principal in lock_principals]) acl.append((Deny, Everyone, "reply")) acl.append((Allow, Authenticated, "reply")) @@ -224,12 +248,6 @@ class Comment(DatabaseModel): # - logged-in users can bookmark comments acl.append((Allow, Authenticated, "bookmark")) - # tools that require specifically granted permissions - acl.append((Allow, "admin", "remove")) - acl.append((Allow, "comment.remove", "remove")) - - acl.append((Allow, "admin", "view_labels")) - acl.append(DENY_ALL) return acl diff --git a/tildes/tildes/models/comment/comment_notification.py b/tildes/tildes/models/comment/comment_notification.py index 4c14797..4b83026 100644 --- a/tildes/tildes/models/comment/comment_notification.py +++ b/tildes/tildes/models/comment/comment_notification.py @@ -5,7 +5,7 @@ import re from datetime import datetime -from typing import Any, List, Sequence, Tuple +from typing import List, Tuple from pyramid.security import Allow, DENY_ALL from sqlalchemy import Boolean, Column, ForeignKey, Integer, TIMESTAMP @@ -18,6 +18,7 @@ from tildes.lib.markdown import LinkifyFilter from tildes.models import DatabaseModel from tildes.models.topic import TopicIgnore from tildes.models.user import User +from tildes.typing import AclType from .comment import Comment @@ -66,7 +67,7 @@ class CommentNotification(DatabaseModel): self.comment = comment self.notification_type = notification_type - def __acl__(self) -> Sequence[Tuple[str, Any, str]]: + def __acl__(self) -> AclType: """Pyramid security ACL.""" acl = [] acl.append((Allow, self.user_id, "mark_read")) diff --git a/tildes/tildes/models/group/group.py b/tildes/tildes/models/group/group.py index 6d13e73..7a3ba04 100644 --- a/tildes/tildes/models/group/group.py +++ b/tildes/tildes/models/group/group.py @@ -4,7 +4,7 @@ """Contains the Group class.""" from datetime import datetime -from typing import Any, List, Optional, Sequence, Tuple +from typing import List, Optional from pyramid.security import Allow, Authenticated, Deny, DENY_ALL, Everyone from sqlalchemy import Boolean, CheckConstraint, Column, Index, Integer, Text, TIMESTAMP @@ -13,10 +13,12 @@ from sqlalchemy.orm import deferred from sqlalchemy.sql.expression import text from sqlalchemy_utils import Ltree, LtreeType +from tildes.lib.auth import aces_for_permission from tildes.lib.database import TagList from tildes.lib.markdown import convert_markdown_to_safe_html from tildes.models import DatabaseModel from tildes.schemas.group import GroupSchema, SHORT_DESCRIPTION_MAX_LENGTH +from tildes.typing import AclType class Group(DatabaseModel): @@ -50,7 +52,7 @@ class Group(DatabaseModel): _sidebar_markdown: str = deferred(Column("sidebar_markdown", Text)) sidebar_rendered_html: str = deferred(Column(Text)) num_subscriptions: int = Column(Integer, nullable=False, server_default="0") - is_admin_posting_only: bool = Column( + requires_permission_to_post_topics: bool = Column( Boolean, nullable=False, server_default="false" ) is_user_treated_as_topic_source: bool = Column( @@ -101,7 +103,7 @@ class Group(DatabaseModel): self.path = path self.short_description = short_desc - def __acl__(self) -> Sequence[Tuple[str, Any, str]]: + def __acl__(self) -> AclType: """Pyramid security ACL.""" acl = [] @@ -114,18 +116,24 @@ class Group(DatabaseModel): acl.append((Allow, Authenticated, "subscribe")) # post_topic: - # - only admins can post in admin-posting-only groups + # - only users with specifically-granted permission can post topics in groups + # that require permission to post # - otherwise, all logged-in users can post - if self.is_admin_posting_only: - acl.append((Allow, "admin", "post_topic")) + if self.requires_permission_to_post_topics: + acl.append((Allow, f"{self.group_id}:post_topic", "post_topic")) acl.append((Deny, Everyone, "post_topic")) acl.append((Allow, Authenticated, "post_topic")) - # wiki_page_create - # - permission must be granted specifically - acl.append((Allow, "admin", "wiki_page_create")) - acl.append((Allow, "wiki", "wiki_page_create")) + # wiki_page_create: + # - requires being granted the "wiki.edit" permission + acl.extend( + aces_for_permission( + required_permission="wiki.edit", + granted_permission="wiki_page_create", + group_id=self.group_id, + ) + ) acl.append(DENY_ALL) diff --git a/tildes/tildes/models/group/group_wiki_page.py b/tildes/tildes/models/group/group_wiki_page.py index c251337..2b45dca 100644 --- a/tildes/tildes/models/group/group_wiki_page.py +++ b/tildes/tildes/models/group/group_wiki_page.py @@ -5,7 +5,7 @@ from datetime import datetime from pathlib import Path, PurePath -from typing import Any, List, Optional, Sequence, Tuple +from typing import List, Optional from pygit2 import Repository, Signature from pyramid.security import Allow, DENY_ALL, Everyone @@ -13,6 +13,7 @@ from sqlalchemy import CheckConstraint, Column, ForeignKey, Integer, Text, TIMES from sqlalchemy.orm import relationship from sqlalchemy.sql.expression import text +from tildes.lib.auth import aces_for_permission from tildes.lib.database import CIText from tildes.lib.datetime import utc_now from tildes.lib.html import add_anchors_to_headings @@ -21,6 +22,7 @@ from tildes.lib.string import convert_to_url_slug from tildes.models import DatabaseModel from tildes.models.user import User from tildes.schemas.group_wiki_page import GroupWikiPageSchema, PAGE_NAME_MAX_LENGTH +from tildes.typing import AclType from .group import Group @@ -72,7 +74,7 @@ class GroupWikiPage(DatabaseModel): self.edit(markdown, user, "Create page") - def __acl__(self) -> Sequence[Tuple[str, Any, str]]: + def __acl__(self) -> AclType: """Pyramid security ACL.""" acl = [] @@ -80,10 +82,7 @@ class GroupWikiPage(DatabaseModel): # - all wiki pages can be viewed by everyone acl.append((Allow, Everyone, "view")) - # edit: - # - permission must be granted specifically - acl.append((Allow, "admin", "edit")) - acl.append((Allow, "wiki", "edit")) + acl.extend(aces_for_permission("wiki.edit", self.group_id)) acl.append(DENY_ALL) diff --git a/tildes/tildes/models/message/message.py b/tildes/tildes/models/message/message.py index 6a48ff5..7984795 100644 --- a/tildes/tildes/models/message/message.py +++ b/tildes/tildes/models/message/message.py @@ -13,9 +13,9 @@ but it simplifies a lot of things when organizing them into threads. """ from datetime import datetime -from typing import Any, List, Optional, Sequence, Tuple +from typing import List, Optional, Sequence -from pyramid.security import ALL_PERMISSIONS, Allow, DENY_ALL +from pyramid.security import Allow, DENY_ALL from sqlalchemy import ( CheckConstraint, Column, @@ -39,6 +39,7 @@ from tildes.schemas.message import ( MessageReplySchema, SUBJECT_MAX_LENGTH, ) +from tildes.typing import AclType class MessageConversation(DatabaseModel): @@ -121,12 +122,14 @@ class MessageConversation(DatabaseModel): def _update_creation_metric(self) -> None: incr_counter("messages", type="conversation") - def __acl__(self) -> Sequence[Tuple[str, Any, str]]: + def __acl__(self) -> AclType: """Pyramid security ACL.""" - acl = [ - (Allow, self.sender_id, ALL_PERMISSIONS), - (Allow, self.recipient_id, ALL_PERMISSIONS), - ] + acl = [] + + # grant permissions to both sender and receiver + for principal in (self.sender_id, self.recipient_id): + acl.append((Allow, principal, "view")) + acl.append((Allow, principal, "reply")) acl.append(DENY_ALL) diff --git a/tildes/tildes/models/topic/topic.py b/tildes/tildes/models/topic/topic.py index 91c41e6..90f37b2 100644 --- a/tildes/tildes/models/topic/topic.py +++ b/tildes/tildes/models/topic/topic.py @@ -6,7 +6,7 @@ from datetime import datetime, timedelta from itertools import chain from pathlib import PurePosixPath -from typing import Any, Dict, Iterable, List, Optional, Sequence, Tuple, TYPE_CHECKING +from typing import Any, Dict, Iterable, List, Optional, TYPE_CHECKING from urllib.parse import urlparse from pyramid.security import Allow, Authenticated, Deny, DENY_ALL, Everyone @@ -27,6 +27,7 @@ from sqlalchemy.sql.expression import desc, text from titlecase import titlecase from tildes.enums import ContentMetadataFields, TopicContentType, TopicType +from tildes.lib.auth import aces_for_permission from tildes.lib.database import TagList from tildes.lib.datetime import utc_from_timestamp, utc_now from tildes.lib.id import id_to_id36 @@ -39,6 +40,7 @@ from tildes.models import DatabaseModel from tildes.models.group import Group from tildes.models.user import User from tildes.schemas.topic import TITLE_MAX_LENGTH, TopicSchema +from tildes.typing import AclType if TYPE_CHECKING: # workaround for mypy issues with @hybrid_property from builtins import property as hybrid_property @@ -243,39 +245,51 @@ class Topic(DatabaseModel): def _update_creation_metric(self) -> None: incr_counter("topics", type=self.topic_type.name.lower()) - def __acl__(self) -> Sequence[Tuple[str, Any, str]]: # noqa + def __acl__(self) -> AclType: # noqa """Pyramid security ACL.""" - acl = [] - # deleted topics allow "general" viewing, but nothing else if self.is_deleted: - acl.append((Allow, Everyone, "view")) - acl.append(DENY_ALL) + return [(Allow, Everyone, "view"), DENY_ALL] + + acl = [] + + # permissions that need to be granted specifically + acl.extend(aces_for_permission("topic.move", self.group_id)) + acl.extend(aces_for_permission("topic.remove", self.group_id)) + acl.extend(aces_for_permission("topic.lock", self.group_id)) # 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, admins, and users - # with remove permission + # - removed topics' author is only visible to author and users who can remove # - 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((Allow, "topic.remove", "view_author")) + acl.extend( + aces_for_permission( + required_permission="topic.remove", + granted_permission="view_author", + group_id=self.group_id, + ) + ) acl.append((Deny, Everyone, "view_author")) acl.append((Allow, Everyone, "view_author")) # view_content: - # - removed topics' content is only visible to the author, admins and users - # with remove permissions + # - removed topics' content is only visible to author and users who can remove # - 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((Allow, "topic.remove", "view_content")) + acl.extend( + aces_for_permission( + required_permission="topic.remove", + granted_permission="view_content", + group_id=self.group_id, + ) + ) acl.append((Deny, Everyone, "view_content")) acl.append((Allow, Everyone, "view_content")) @@ -294,15 +308,27 @@ class Topic(DatabaseModel): acl.append((Allow, Authenticated, "vote")) # comment: - # - removed topics can only be commented on by admins - # - locked topics can only be commented on by admins + # - removed topics can only be commented on by users who can remove + # - locked topics can only be commented on by users who can lock # - otherwise, logged-in users can comment if self.is_removed: - acl.append((Allow, "admin", "comment")) + acl.extend( + aces_for_permission( + required_permission="topic.remove", + granted_permission="comment", + group_id=self.group_id, + ) + ) acl.append((Deny, Everyone, "comment")) if self.is_locked: - acl.append((Allow, "admin", "comment")) + acl.extend( + aces_for_permission( + required_permission="topic.lock", + granted_permission="comment", + group_id=self.group_id, + ) + ) acl.append((Deny, Everyone, "comment")) acl.append((Allow, Authenticated, "comment")) @@ -310,22 +336,27 @@ class Topic(DatabaseModel): # edit: # - only text topics can be edited # - authors can edit their own topics - # - admins can edit topics belonging to the generic/automatic user + # - topics by the generic/automatic user can be edited with permission if self.is_text_type: acl.append((Allow, self.user_id, "edit")) if self.user_id == -1: - acl.append((Allow, "admin", "edit")) + acl.extend( + aces_for_permission( + required_permission="topic.edit_by_generic_user", + granted_permission="edit", + group_id=self.group_id, + ) + ) # delete: # - only the author can delete acl.append((Allow, self.user_id, "delete")) # tag: - # - allow tagging by the author, admins, and people with "topic.tag" principal + # - allow tagging by the author, and users specifically granted permission acl.append((Allow, self.user_id, "tag")) - acl.append((Allow, "admin", "tag")) - acl.append((Allow, "topic.tag", "tag")) + acl.extend(aces_for_permission("topic.tag", self.group_id)) # bookmark: # - logged-in users can bookmark topics @@ -336,28 +367,16 @@ class Topic(DatabaseModel): acl.append((Allow, Authenticated, "ignore")) # edit_title: - # - allow admins or people with the "topic.edit_title" permission to always - # edit titles # - allow users to edit their own topic's title for the first 5 minutes - acl.append((Allow, "admin", "edit_title")) - acl.append((Allow, "topic.edit_title", "edit_title")) - + # - otherwise, only if granted permission specifically if self.age < timedelta(minutes=5): acl.append((Allow, self.user_id, "edit_title")) + acl.extend(aces_for_permission("topic.edit_title", self.group_id)) - # tools that require specifically granted permissions - acl.append((Allow, "admin", "lock")) - acl.append((Allow, "topic.lock", "lock")) - - acl.append((Allow, "admin", "remove")) - acl.append((Allow, "topic.remove", "remove")) - - acl.append((Allow, "admin", "move")) - acl.append((Allow, "topic.move", "move")) - + # edit_link: + # - only if granted specifically, only on link topics if self.is_link_type: - acl.append((Allow, "admin", "edit_link")) - acl.append((Allow, "topic.edit_link", "edit_link")) + acl.extend(aces_for_permission("topic.edit_link", self.group_id)) acl.append(DENY_ALL) diff --git a/tildes/tildes/models/user/user.py b/tildes/tildes/models/user/user.py index ea3a2d5..a305a5e 100644 --- a/tildes/tildes/models/user/user.py +++ b/tildes/tildes/models/user/user.py @@ -4,11 +4,10 @@ """Contains the User class.""" from datetime import datetime, timedelta -from typing import Any, List, NoReturn, Optional, Sequence, Tuple +from typing import List, NoReturn, Optional from pyotp import TOTP from pyramid.security import ( - ALL_PERMISSIONS, Allow, Authenticated, Deny, @@ -46,6 +45,7 @@ from tildes.schemas.user import ( EMAIL_ADDRESS_NOTE_MAX_LENGTH, UserSchema, ) +from tildes.typing import AclType class User(DatabaseModel): @@ -177,7 +177,7 @@ class User(DatabaseModel): self.username = username self.password = password # type: ignore - def __acl__(self) -> Sequence[Tuple[str, Any, str]]: + def __acl__(self) -> AclType: """Pyramid security ACL.""" acl = [] @@ -207,15 +207,24 @@ class User(DatabaseModel): acl.append((Allow, Authenticated, "message")) # ban: - # - admins can ban non-deleted users except themselves + # - deleted users can't be banned, otherwise only when permission granted if self.is_deleted: acl.append((Deny, Everyone, "ban")) - acl.append((Deny, self.user_id, "ban")) # required so users can't self-ban - acl.append((Allow, "admin", "ban")) + acl.append((Allow, "*:user.ban", "ban")) - # grant the user all other permissions on themself - acl.append((Allow, self.user_id, ALL_PERMISSIONS)) + # view_removed_posts: + # - must be granted specifically + acl.append((Allow, "*:user.view_removed_posts", "view_removed_posts")) + + # grant the user the various permissions they need on themself + for permission in ( + "change_settings", + "generate_invite", + "search_posts", + "view_removed_posts", + ): + acl.append((Allow, self.user_id, permission)) acl.append(DENY_ALL) @@ -310,7 +319,7 @@ class User(DatabaseModel): # give the user the "comment.label" permission if they're over a week old if self.age > timedelta(days=7): - principals.append("comment.label") + principals.append("*:comment.label") return principals @@ -319,11 +328,6 @@ class User(DatabaseModel): """Return whether this is a "real" user (not a special-purpose internal one).""" return self.user_id > 0 - @property - def is_admin(self) -> bool: - """Return whether the user has admin permissions.""" - return "admin" in self.auth_principals - def is_label_available(self, label: CommentLabelOption) -> bool: """Return whether the user has a particular label available.""" if label == CommentLabelOption.EXEMPLARY: diff --git a/tildes/tildes/models/user/user_permissions.py b/tildes/tildes/models/user/user_permissions.py index 2733b03..76665b4 100644 --- a/tildes/tildes/models/user/user_permissions.py +++ b/tildes/tildes/models/user/user_permissions.py @@ -33,12 +33,23 @@ class UserPermissions(DatabaseModel): def auth_principal(self) -> str: """Return the permission as a string usable as an auth principal. - WARNING: This isn't currently complete, and only handles ALLOW for all groups. + The principal is made up of two parts, separated by a colon. The first part is + the group_id the permission applies to, or a * for all groups. The second part + is the permission name, prefixed by a ! if the permission is being denied + instead of allowed: + + - "5:topic.tag" for allowing the topic.tag permission in group id 5 + - "*:topic.tag" for allowing the topic.tag permission in all groups + - "3:!topic.tag" for denying the topic.tag permission in group id 3 """ - if self.permission_type != UserPermissionType.ALLOW: - raise ValueError("Not an ALLOW permission.") - if self.group_id: - raise ValueError("Not an all-groups permission.") + principal = f"{self.group_id}:" + else: + principal = "*:" - return self.permission + if self.permission_type == UserPermissionType.DENY: + principal += "!" + + principal += self.permission + + return principal diff --git a/tildes/tildes/typing.py b/tildes/tildes/typing.py new file mode 100644 index 0000000..73ee6f6 --- /dev/null +++ b/tildes/tildes/typing.py @@ -0,0 +1,10 @@ +# Copyright (c) 2018 Tildes contributors +# SPDX-License-Identifier: AGPL-3.0-or-later + +"""Custom type aliases to use in type annotations.""" + +from typing import Any, List, Tuple + +# types for an ACE (Access Control Entry), and the ACL (Access Control List) of them +AceType = Tuple[str, Any, str] +AclType = List[AceType] diff --git a/tildes/tildes/views/api/web/user.py b/tildes/tildes/views/api/web/user.py index 5bf5693..536127c 100644 --- a/tildes/tildes/views/api/web/user.py +++ b/tildes/tildes/views/api/web/user.py @@ -38,7 +38,7 @@ PASSWORD_FIELD = UserSchema(only=("password",)).fields["password"] route_name="user", request_method="PATCH", request_param="ic-trigger-name=password-change", - permission="change_password", + permission="change_settings", ) @use_kwargs( { @@ -68,7 +68,7 @@ def patch_change_password( route_name="user", request_method="PATCH", request_param="ic-trigger-name=account-recovery-email", - permission="change_email_address", + permission="change_settings", ) @use_kwargs(UserSchema(only=("email_address", "email_address_note"))) def patch_change_email_address( @@ -100,7 +100,7 @@ def patch_change_email_address( request_method="POST", request_param="ic-trigger-name=enable-two-factor", renderer="two_factor_enabled.jinja2", - permission="change_two_factor", + permission="change_settings", ) @use_kwargs({"code": String()}) def post_enable_two_factor(request: Request, code: str) -> dict: @@ -130,7 +130,7 @@ def post_enable_two_factor(request: Request, code: str) -> dict: request_method="POST", request_param="ic-trigger-name=disable-two-factor", renderer="two_factor_disabled.jinja2", - permission="change_two_factor", + permission="change_settings", ) @use_kwargs({"code": String()}) def post_disable_two_factor(request: Request, code: str) -> Response: @@ -150,7 +150,7 @@ def post_disable_two_factor(request: Request, code: str) -> Response: request_method="POST", request_param="ic-trigger-name=view-two-factor-backup-codes", renderer="two_factor_backup_codes.jinja2", - permission="change_two_factor", + permission="change_settings", ) @use_kwargs({"code": String()}) def post_view_two_factor_backup_codes(request: Request, code: str) -> Response: @@ -172,7 +172,7 @@ def post_view_two_factor_backup_codes(request: Request, code: str) -> Response: route_name="user", request_method="PATCH", request_param="ic-trigger-name=show-tags-in-listings", - permission="change_show_tags_in_listings_setting", + permission="change_settings", ) def patch_change_show_tags_in_listings(request: Request) -> Response: """Change the user's "show tags in listings" setting.""" @@ -188,7 +188,7 @@ def patch_change_show_tags_in_listings(request: Request) -> Response: route_name="user", request_method="PATCH", request_param="ic-trigger-name=account-default-comment-sort-order", - permission="change_comment_sort_order_setting", + permission="change_settings", ) def patch_change_comment_sort_order(request: Request) -> Response: """Change the user's default comment sort order setting.""" @@ -204,7 +204,7 @@ def patch_change_comment_sort_order(request: Request) -> Response: route_name="user", request_method="PATCH", request_param="ic-trigger-name=auto-mark-notifications-read", - permission="change_auto_mark_notifications_read_setting", + permission="change_settings", ) def patch_change_auto_mark_notifications(request: Request) -> Response: """Change the user's "automatically mark notifications read" setting.""" @@ -220,7 +220,7 @@ def patch_change_auto_mark_notifications(request: Request) -> Response: route_name="user", request_method="PATCH", request_param="ic-trigger-name=interact-mark-notifications-read", - permission="change_interact_mark_notifications_read_setting", + permission="change_settings", ) def patch_change_interact_mark_notifications(request: Request) -> Response: """Change the user's "automatically mark notifications read on interact" setting.""" @@ -236,7 +236,7 @@ def patch_change_interact_mark_notifications(request: Request) -> Response: route_name="user", request_method="PATCH", request_param="ic-trigger-name=open-links-new-tab", - permission="change_open_links_new_tab_setting", + permission="change_settings", ) def patch_change_open_links_new_tab(request: Request) -> Response: """Change the user's "open links in new tabs" setting.""" @@ -260,7 +260,7 @@ def patch_change_open_links_new_tab(request: Request) -> Response: route_name="user", request_method="PATCH", request_param="ic-trigger-name=collapse-old-comments", - permission="change_collapse_old_comments_setting", + permission="change_settings", ) def patch_change_collapse_old_comments(request: Request) -> Response: """Change the user's "collapse old comments" setting.""" @@ -276,7 +276,7 @@ def patch_change_collapse_old_comments(request: Request) -> Response: route_name="user", request_method="PATCH", request_param="ic-trigger-name=account-default-theme", - permission="change_account_default_theme_setting", + permission="change_settings", ) def patch_change_account_default_theme(request: Request) -> Response: """Change the user's "theme account default" setting.""" @@ -292,7 +292,7 @@ def patch_change_account_default_theme(request: Request) -> Response: route_name="user", request_method="PATCH", request_param="ic-trigger-name=user-bio", - permission="edit_bio", + permission="change_settings", ) @use_kwargs({"markdown": String()}) def patch_change_user_bio(request: Request, markdown: str) -> dict: @@ -307,7 +307,7 @@ def patch_change_user_bio(request: Request, markdown: str) -> dict: @ic_view_config( route_name="user_invite_code", request_method="GET", - permission="view_invite_code", + permission="generate_invite", renderer="invite_code.jinja2", ) def get_invite_code(request: Request) -> dict: @@ -346,7 +346,7 @@ def get_invite_code(request: Request) -> dict: @ic_view_config( route_name="user_default_listing_options", request_method="PUT", - permission="edit_default_listing_options", + permission="change_settings", ) @use_kwargs( { @@ -373,7 +373,7 @@ def put_default_listing_options( @ic_view_config( route_name="user_filtered_topic_tags", request_method="PUT", - permission="edit_filtered_topic_tags", + permission="change_settings", ) @use_kwargs({"tags": String()}) def put_filtered_topic_tags(request: Request, tags: str) -> dict: diff --git a/tildes/tildes/views/user.py b/tildes/tildes/views/user.py index e08e285..a5eb16f 100644 --- a/tildes/tildes/views/user.py +++ b/tildes/tildes/views/user.py @@ -200,8 +200,8 @@ def _get_user_posts( query = query.join_all_relationships() - # include removed posts if the user's looking at their own page or is an admin - if request.user and (user == request.user or request.user.is_admin): + # include removed posts if the viewer has permission + if request.has_permission("view_removed_posts", user): query = query.include_removed() result_sets.append(query.get_page(per_page))