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.

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'"
+    )

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):

tildes/tests/test_topic_permissions.py

@@ -1,7 +1,20 @@
 # Copyright (c) 2018 Tildes contributors <code@tildes.net>
 # 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

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"}

tildes/tildes/lib/auth.py

@@ -0,0 +1,38 @@
+# Copyright (c) 2020 Tildes contributors <code@tildes.net>
+# 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

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 pyramid.security import Allow, Authenticated, Deny, DENY_ALL, Everyone
+from typing import Any, Optional, Sequence, TYPE_CHECKING, Union
+
+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

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"))

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)
 

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)
 

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)
 

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)
 

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:

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 = "*:"
+
+        if self.permission_type == UserPermissionType.DENY:
+            principal += "!"
+
+        principal += self.permission
 
-        return self.permission
+        return principal

tildes/tildes/typing.py

@@ -0,0 +1,10 @@
+# Copyright (c) 2018 Tildes contributors <code@tildes.net>
+# 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]

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:

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))