Merge branch 'fix-api-permission-checks' into 'develop-1.102'

Draft: Fix API permissions checks

See merge request tildes/tildes!170

tildes/openapi_beta.yaml

@@ -274,7 +274,7 @@ components:
         - ignored
         - official
         - tags
-        - last_visit_time
+        - last_visited_at
       properties:
         id:
           type: string
@@ -326,7 +326,7 @@ components:
           type: array
           items:
             type: string
-        last_visit_time:
+        last_visited_at:
           type: string
           nullable: true
 
@@ -339,12 +339,19 @@ components:
         - rendered_html
         - created_at
         - edited_at
-        - votes
+        - vote_count
         - removed
         - deleted
         - exemplary
+        - collapsed
+        - collapsed_individual
+        - by_op
+        - by_me
+        - new_comment
         - voted
         - bookmarked
+        - depth
+        - children
       properties:
         id:
           type: string
@@ -358,10 +365,11 @@ components:
           nullable: true
         created_at:
           type: string
+          nullable: true
         edited_at:
           type: string
           nullable: true
-        votes:
+        vote_count:
           type: integer
         removed:
           type: boolean
@@ -372,10 +380,8 @@ components:
           nullable: true
         collapsed:
           type: boolean
-          nullable: true
         collapsed_individual:
           type: boolean
-          nullable: true
         by_op:
           type: boolean
           nullable: true
@@ -403,15 +409,21 @@ components:
       properties:
         username:
           type: string
+        joined_at:
+          type: string
+          nullable: true
+        bio_rendered_html:
+          type: string
+          nullable: true
 
     Pagination:
       type: object
       required:
-        - num_items
+        - item_count
         - next_link
         - prev_link
       properties:
-        num_items:
+        item_count:
           type: integer
           description: The number of items returned in this response.
         next_link:
@@ -426,9 +438,11 @@ components:
       required:
         - message
       properties:
-        field:
-          type: string
         message:
           type: string
+        field:
+          type: string
+          nullable: true
         exception:
           type: string
+          nullable: true

tildes/tildes/templates/user.jinja2

@@ -171,8 +171,8 @@
 {% endif %}
 
 {% if request.has_permission("view_info", user) %}
-<h2>User info</h2>
-<dl>
+  <h2>User info</h2>
+  <dl>
     <dt>Registered</dt>
     <dd>{{ user.created_time.strftime('%B %-d, %Y') }}</dd>
 
@@ -182,7 +182,7 @@
         <dd>{{ user.bio_rendered_html|safe }}</dd>
       </div>
     {% endif %}
-</dl>
+  </dl>
 {% endif %}
 
 {% if request.has_permission('message', user) %}

tildes/tildes/views/api/beta/comment.py

@@ -8,23 +8,51 @@ from tildes.models.comment import Comment
 from tildes.models.comment.comment_tree import CommentInTree
 
 
-def comment_to_dict(request: Request, comment: Comment) -> dict:
-    """Convert a Comment object to a dictionary for JSON serialization."""
+def comment_to_api_dict(request: Request, comment: Comment) -> dict:
+    """Convert a Comment object to a dictionary for JSON serialization.
+
+    The schema is defined in our OpenAPI YAML file.
+    """
+
+    # Some fields do not require permissions
+    comment_id = comment.comment_id36
+    topic_id = comment.topic.topic_id36
+    is_removed = comment.is_removed
+    is_deleted = comment.is_deleted
+    collapsed = (
+        hasattr(comment, "collapsed_state") and comment.collapsed_state == "full"
+    )
+    collapsed_individual = (
+        hasattr(comment, "collapsed_state") and comment.collapsed_state == "individual"
+    )
 
     # Check permissions for viewing comment details (and set safe defaults)
     author = None
+    created_time = None
+    edited_time = None
     rendered_html = None
+    vote_count = 0
     exemplary = None
     by_op = None
     by_me = None
     is_new_comment = None
+    voted = False
+    bookmarked = False
+
     if request.has_permission("view", comment):
         author = comment.user.username
+        created_time = comment.created_time.isoformat()
+        edited_time = (
+            comment.last_edited_time.isoformat() if comment.last_edited_time else None
+        )
         rendered_html = comment.rendered_html
+        vote_count = comment.num_votes
         exemplary = comment.is_label_active("exemplary")
         by_me = request.user == comment.user if request.user else False
+
         if request.has_permission("view_author", comment.topic):
             by_op = comment.user == comment.topic.user
+
         is_new_comment = (
             (comment.created_time > comment.topic.last_visit_time)
             if (
@@ -35,45 +63,53 @@ def comment_to_dict(request: Request, comment: Comment) -> dict:
             else False
         )
 
+        if request.has_permission("vote", comment):
+            voted = comment.user_voted
+        if request.has_permission("bookmark", comment):
+            bookmarked = comment.user_bookmarked
+
     return {
-        "id": comment.comment_id36,
-        "topic_id": comment.topic.topic_id36,
+        "id": comment_id,
+        "topic_id": topic_id,
         "author": author,
         "rendered_html": rendered_html,
-        "created_at": comment.created_time.isoformat(),
-        "edited_at": (
-            comment.last_edited_time.isoformat() if comment.last_edited_time else None
-        ),
-        "votes": comment.num_votes,
-        "removed": comment.is_removed,
-        "deleted": comment.is_deleted,
+        "created_at": created_time,
+        "edited_at": edited_time,
+        "vote_count": vote_count,
+        "removed": is_removed,
+        "deleted": is_deleted,
         "exemplary": exemplary,
-        "collapsed": (
-            (comment.collapsed_state == "full")
-            if hasattr(comment, "collapsed_state")
-            else None
-        ),
-        "collapsed_individual": (
-            (comment.collapsed_state == "individual")
-            if hasattr(comment, "collapsed_state")
-            else None
-        ),
+        "collapsed": collapsed,
+        "collapsed_individual": collapsed_individual,
         "by_op": by_op,
         "by_me": by_me,
         "new_comment": is_new_comment,
-        "voted": comment.user_voted,
-        "bookmarked": comment.user_bookmarked,
+        "voted": voted,
+        "bookmarked": bookmarked,
     }
 
 
-def comment_subtree_to_dict(request: Request, comments: list[CommentInTree]) -> list:
-    """Convert a comment subtree to a list of dictionaries for JSON serialization."""
+def comment_subtree_to_api_dict(
+    request: Request, comments: list[CommentInTree]
+) -> list:
+    """Convert a comment subtree to a list of dictionaries for JSON serialization.
+
+    The schema is defined in our OpenAPI YAML file.
+    """
     comments_list = []
     for comment in comments:
-        comment_dict = comment_to_dict(request, comment)
+        comment_dict = comment_to_api_dict(request, comment)
         comment_dict["depth"] = comment.depth
+
+        if request.has_permission("view", comment) or not comment.removed_marker:
+            # Recursively display reply comments, unless we hit a "removed marker"
             comment_dict["children"] = (
-            comment_subtree_to_dict(request, comment.replies) if comment.replies else []
+                comment_subtree_to_api_dict(request, comment.replies)
+                if comment.replies
+                else []
             )
+        else:
+            comment_dict["children"] = []
+
         comments_list.append(comment_dict)
     return comments_list

tildes/tildes/views/api/beta/topic.py

@@ -3,53 +3,95 @@
 
 """JSON API endpoints related to topics."""
 
-from marshmallow.exceptions import ValidationError
 from pyramid.request import Request
+from pyramid.response import Response
 from pyramid.view import view_config
 from tildes.enums import CommentTreeSortOption, TopicSortOption
 from tildes.models.comment import CommentTree, Comment
 from tildes.models.topic import Topic
-from tildes.schemas.fields import ShortTimePeriod
+from tildes.lib.datetime import SimpleHoursPeriod
 from tildes.lib.id import id36_to_id
 from tildes.views.api.beta.api_utils import (
     build_error_response,
     get_next_and_prev_link,
     query_apply_pagination,
 )
-from tildes.views.api.beta.comment import comment_subtree_to_dict
+from tildes.views.api.beta.comment import comment_subtree_to_api_dict
+
+
+def topic_to_api_dict(request: Request, topic: Topic) -> dict:
+    """Convert a Topic object to a dictionary for JSON serialization.
+
+    The schema is defined in our OpenAPI YAML file.
+    """
+
+    # Some fields do not require permissions
+    topic_id = topic.topic_id36
+    title = topic.title
+    comments_url = topic.permalink
+    group = str(topic.group.path)
+    created_at = topic.created_time.isoformat()
+    edited_at = topic.last_edited_time.isoformat() if topic.last_edited_time else None
+    vote_count = topic.num_votes
+    comment_count = topic.num_comments
+    new_comment_count = topic.comments_since_last_visit
+    official = topic.is_official
+    tags = topic.tags
+    last_visited_at = (
+        topic.last_visit_time.isoformat() if topic.last_visit_time else None
+    )
+
+    # Check permissions for viewing topic details (and set safe defaults)
+    text_html = None
+    url = None
+    content_metadata = None
+    posted_by_user = "unknown user"
+    voted = False
+    bookmarked = False
+    ignored = False
+
+    if request.has_permission("view_content", topic):
+        text_html = topic.rendered_html
+        url = topic.link
+        content_metadata = topic.content_metadata_for_display
+
+    if request.has_permission("view_author", topic):
+        posted_by_user = topic.user.username
 
+    if request.has_permission("vote", topic):
+        voted = topic.user_voted
+
+    if request.has_permission("bookmark", topic):
+        bookmarked = topic.user_bookmarked
+
+    if request.has_permission("ignore", topic):
+        ignored = topic.user_ignored
 
-def topic_to_dict(topic: Topic) -> dict:
-    """Convert a Topic object to a dictionary for JSON serialization."""
     return {
-        "id": topic.topic_id36,
-        "title": topic.title,
-        "text_html": topic.rendered_html,
-        "url": topic.link,
-        "comments_url": topic.permalink,
-        "group": str(topic.group.path),
-        "content_metadata": topic.content_metadata_for_display,
-        "created_at": topic.created_time.isoformat(),
-        "edited_at": (
-            topic.last_edited_time.isoformat() if topic.last_edited_time else None
-        ),
-        "posted_by_user": topic.user.username,
-        "vote_count": topic.num_votes,
-        "comment_count": topic.num_comments,
-        "new_comment_count": topic.comments_since_last_visit,
-        "voted": topic.user_voted,
-        "bookmarked": topic.user_bookmarked,
-        "ignored": topic.user_ignored,
-        "official": topic.is_official,
-        "tags": topic.tags,
-        "last_visit_time": (
-            topic.last_visit_time.isoformat() if topic.last_visit_time else None
-        ),
+        "id": topic_id,
+        "title": title,
+        "text_html": text_html,
+        "url": url,
+        "comments_url": comments_url,
+        "group": group,
+        "content_metadata": content_metadata,
+        "created_at": created_at,
+        "edited_at": edited_at,
+        "posted_by_user": posted_by_user,
+        "vote_count": vote_count,
+        "comment_count": comment_count,
+        "new_comment_count": new_comment_count,
+        "voted": voted,
+        "bookmarked": bookmarked,
+        "ignored": ignored,
+        "official": official,
+        "tags": tags,
+        "last_visited_at": last_visited_at,
     }
 
 
 @view_config(route_name="apibeta.topics", openapi=True, renderer="json")
-def get_topics(request: Request) -> dict:  # noqa
+def get_topics(request: Request) -> dict | Response:  # noqa: MC0001
     """Get a list of topics (without comments)."""
     limit = request.openapi_validated.parameters.query.get("limit", 50)
     period_raw = request.openapi_validated.parameters.query.get("period")
@@ -59,9 +101,12 @@ def get_topics(request: Request) -> dict:  # noqa
     after = request.openapi_validated.parameters.query.get("after", None)
 
     # Parse parameters where necessary
+    if not period_raw or period_raw == "all":
+        period = None
+    else:
         try:
-        period = ShortTimePeriod(allow_none=True).deserialize(period_raw)
-    except ValidationError as exc:
+            period = SimpleHoursPeriod.from_short_form(period_raw)
+        except ValueError as exc:
             return build_error_response(str(exc), field="period")
 
     try:
@@ -94,7 +139,7 @@ def get_topics(request: Request) -> dict:  # noqa
 
     # Build the JSON topic data
     for topic in topics:
-        processed_topic = topic_to_dict(topic)
+        processed_topic = topic_to_api_dict(request, topic)
         processed_topics.append(processed_topic)
 
     # Construct the paging next and previous link if there are more topics
@@ -104,7 +149,7 @@ def get_topics(request: Request) -> dict:  # noqa
     response = {
         "topics": processed_topics,
         "pagination": {
-            "num_items": len(processed_topics),
+            "item_count": len(processed_topics),
             "next_link": next_link,
             "prev_link": prev_link,
         },
@@ -113,7 +158,7 @@ def get_topics(request: Request) -> dict:  # noqa
 
 
 @view_config(route_name="apibeta.topic", openapi=True, renderer="json")
-def get_topic(request: Request) -> dict:
+def get_topic(request: Request) -> dict | Response:
     """Get a single topic (with comments)."""
     topic_id36 = request.openapi_validated.parameters.path.get("topic_id36")
     comment_order_raw = request.openapi_validated.parameters.query.get("order")
@@ -161,11 +206,11 @@ def get_topic(request: Request) -> dict:
         tree.uncollapse_new_comments(topic.last_visit_time)
         tree.finalize_collapsing_maximized()
 
-    commentsjson = comment_subtree_to_dict(request, tree.tree)
+    commentsjson = comment_subtree_to_api_dict(request, tree.tree)
 
     # Construct the final response JSON object
     response = {
-        "topic": topic_to_dict(topic),
+        "topic": topic_to_api_dict(request, topic),
         "comments": commentsjson,
     }
     return response

tildes/tildes/views/api/beta/user.py

@@ -5,6 +5,7 @@
 
 from typing import Union
 from pyramid.request import Request
+from pyramid.response import Response
 from pyramid.view import view_config
 from tildes.models.user.user import User
 from tildes.models.comment import Comment
@@ -15,21 +16,36 @@ from tildes.views.api.beta.api_utils import (
     get_next_and_prev_link,
     query_apply_pagination,
 )
-from tildes.views.api.beta.comment import comment_to_dict
-from tildes.views.api.beta.topic import topic_to_dict
+from tildes.views.api.beta.comment import comment_to_api_dict
+from tildes.views.api.beta.topic import topic_to_api_dict
 
 
-def _user_to_dict(user: User) -> dict:
-    """Convert a User object to a dictionary for JSON serialization."""
+def _user_to_api_dict(request: Request, user: User) -> dict:
+    """Convert a User object to a dictionary for JSON serialization.
+
+    The schema is defined in our OpenAPI YAML file.
+    """
+
+    # Some fields do not require permissions
+    username = user.username
+
+    # Check permissions for viewing user details (and set safe defaults)
+    joined_at = None
+    bio_rendered_html = None
+
+    if request.has_permission("view_info", user):
+        joined_at = user.created_time.isoformat()
+        bio_rendered_html = user.bio_rendered_html
+
     return {
-        "username": user.username,
-        "joined_at": user.created_time.isoformat(),
-        "bio_rendered_html": user.bio_rendered_html,
+        "username": username,
+        "joined_at": joined_at,
+        "bio_rendered_html": bio_rendered_html,
     }
 
 
 @view_config(route_name="apibeta.user", openapi=True, renderer="json")
-def get_user(request: Request) -> dict:  # noqa
+def get_user(request: Request) -> dict | Response:  # noqa: MC0001
     """Get a single user with their comment and post history."""
     username = request.openapi_validated.parameters.path.get("username")
     limit = request.openapi_validated.parameters.query.get("limit", 20)
@@ -85,19 +101,19 @@ def get_user(request: Request) -> dict:  # noqa
     processed_results = []
     for item in combined_results.results:
         if isinstance(item, Topic):
-            processed_results.append(topic_to_dict(item))
+            processed_results.append(topic_to_api_dict(request, item))
         elif isinstance(item, Comment):
-            processed_results.append(comment_to_dict(request, item))
+            processed_results.append(comment_to_api_dict(request, item))
 
     # Construct the paging next and previous link if there are more topics
     (next_link, prev_link) = get_next_and_prev_link(request, combined_results)
 
     # Construct the final response JSON object
     response = {
-        "user": _user_to_dict(user),
+        "user": _user_to_api_dict(request, user),
         "history": processed_results,
         "pagination": {
-            "num_items": len(processed_results),
+            "item_count": len(processed_results),
             "next_link": next_link,
             "prev_link": prev_link,
         },
@@ -106,7 +122,7 @@ def get_user(request: Request) -> dict:  # noqa
 
 
 @view_config(route_name="apibeta.user_comments", openapi=True, renderer="json")
-def get_user_comments(request: Request) -> dict:
+def get_user_comments(request: Request) -> dict | Response:
     """Get comments made by a user."""
     username = request.openapi_validated.parameters.path.get("username")
     limit = request.openapi_validated.parameters.query.get("limit", 50)
@@ -144,7 +160,7 @@ def get_user_comments(request: Request) -> dict:
     # Build the JSON history data
     processed_comments = []
     for comment in query_result.results:
-        processed_comments.append(comment_to_dict(request, comment))
+        processed_comments.append(comment_to_api_dict(request, comment))
 
     # Construct the paging next and previous link if there are more comments
     (next_link, prev_link) = get_next_and_prev_link(request, query_result)
@@ -153,7 +169,7 @@ def get_user_comments(request: Request) -> dict:
     response = {
         "comments": processed_comments,
         "pagination": {
-            "num_items": len(processed_comments),
+            "item_count": len(processed_comments),
             "next_link": next_link,
             "prev_link": prev_link,
         },
@@ -162,7 +178,7 @@ def get_user_comments(request: Request) -> dict:
 
 
 @view_config(route_name="apibeta.user_topics", openapi=True, renderer="json")
-def get_user_topics(request: Request) -> dict:
+def get_user_topics(request: Request) -> dict | Response:
     """Get topics made by a user."""
     username = request.openapi_validated.parameters.path.get("username")
     limit = request.openapi_validated.parameters.query.get("limit", 50)
@@ -200,7 +216,7 @@ def get_user_topics(request: Request) -> dict:
     # Build the JSON history data
     processed_topics = []
     for topic in query_result.results:
-        processed_topics.append(topic_to_dict(topic))
+        processed_topics.append(topic_to_api_dict(request, topic))
 
     # Construct the paging next and previous link if there are more topics
     (next_link, prev_link) = get_next_and_prev_link(request, query_result)
@@ -209,7 +225,7 @@ def get_user_topics(request: Request) -> dict:
     response = {
         "topics": processed_topics,
         "pagination": {
-            "num_items": len(processed_topics),
+            "item_count": len(processed_topics),
             "next_link": next_link,
             "prev_link": prev_link,
         },