Add ability to (manually) delete user accounts

Until now, if people want their account deleted, I've just been banning
it. This will let me do it properly, but some additional cleanup should
be added in the future once I think through what's safe to get rid of
from deleted accounts.

tildes/alembic/versions/a0e0b6206146_users_add_is_deleted_deleted_time.py

@@ -0,0 +1,54 @@
+"""Users: Add is_deleted/deleted_time
+
+Revision ID: a0e0b6206146
+Revises: 53567981cdf4
+Create Date: 2018-11-13 23:49:20.764289
+
+"""
+from alembic import op
+import sqlalchemy as sa
+
+
+# revision identifiers, used by Alembic.
+revision = "a0e0b6206146"
+down_revision = "53567981cdf4"
+branch_labels = None
+depends_on = None
+
+
+def upgrade():
+    op.add_column(
+        "users", sa.Column("deleted_time", sa.TIMESTAMP(timezone=True), nullable=True)
+    )
+    op.add_column(
+        "users",
+        sa.Column("is_deleted", sa.Boolean(), server_default="false", nullable=False),
+    )
+    op.create_index(op.f("ix_users_is_deleted"), "users", ["is_deleted"], unique=False)
+
+    op.execute(
+        """
+        CREATE OR REPLACE FUNCTION set_user_deleted_time() RETURNS TRIGGER AS $$
+        BEGIN
+            NEW.deleted_time := current_timestamp;
+
+            RETURN NEW;
+        END;
+        $$ LANGUAGE plpgsql;
+
+        CREATE TRIGGER delete_user_set_deleted_time_update
+            BEFORE UPDATE ON users
+            FOR EACH ROW
+            WHEN (OLD.is_deleted = false AND NEW.is_deleted = true)
+            EXECUTE PROCEDURE set_user_deleted_time();
+    """
+    )
+
+
+def downgrade():
+    op.execute("DROP TRIGGER delete_user_set_deleted_time_update ON users")
+    op.execute("DROP FUNCTION set_user_deleted_time")
+
+    op.drop_index(op.f("ix_users_is_deleted"), table_name="users")
+    op.drop_column("users", "is_deleted")
+    op.drop_column("users", "deleted_time")

tildes/sql/init/triggers/users/users.sql

@@ -0,0 +1,17 @@
+-- Copyright (c) 2018 Tildes contributors <code@tildes.net>
+-- SPDX-License-Identifier: AGPL-3.0-or-later
+
+-- set user.deleted_time when it's deleted
+CREATE OR REPLACE FUNCTION set_user_deleted_time() RETURNS TRIGGER AS $$
+BEGIN
+    NEW.deleted_time := current_timestamp;
+
+    RETURN NEW;
+END;
+$$ LANGUAGE plpgsql;
+
+CREATE TRIGGER delete_user_set_deleted_time_update
+    BEFORE UPDATE ON users
+    FOR EACH ROW
+    WHEN (OLD.is_deleted = false AND NEW.is_deleted = true)
+    EXECUTE PROCEDURE set_user_deleted_time();

tildes/tests/test_user.py

@@ -2,6 +2,7 @@
 # SPDX-License-Identifier: AGPL-3.0-or-later
 
 from marshmallow.exceptions import ValidationError
+from pyramid.security import principals_allowed_by_permission
 from pytest import raises
 from sqlalchemy.exc import IntegrityError
 
@@ -115,3 +116,21 @@ def test_change_password_to_username(session_user):
     """Ensure users can't change password to the same as their username."""
     with raises(ValidationError):
         session_user.change_password("session user password", session_user.username)
+
+
+def test_deleted_user_no_message_permission():
+    """Ensure nobody can message a deleted user."""
+    deleted_user = User("Deleted_User", "password")
+    deleted_user.is_deleted = True
+
+    principals = principals_allowed_by_permission(deleted_user, "message")
+    assert not principals
+
+
+def test_banned_user_no_message_permission():
+    """Ensure nobody can message a banned user."""
+    banned_user = User("Banned_User", "password")
+    banned_user.is_banned = True
+
+    principals = principals_allowed_by_permission(banned_user, "message")
+    assert not principals

tildes/tildes/auth.py

@@ -51,8 +51,9 @@ def auth_callback(user_id: int, request: Request) -> Optional[Sequence[str]]:
     if not request.user:
         return None
 
-    # if the user is banned, log them out - is there a better place to do this?
-    if request.user.is_banned:
+    # if the user is deleted or banned, log them out
+    # (is there a better place to do this?)
+    if request.user.is_banned or request.user.is_deleted:
         request.session.invalidate()
         raise HTTPFound("/")
 

tildes/tildes/models/user/user.py

@@ -51,6 +51,8 @@ class User(DatabaseModel):
           deletions, and updates to unread_user_ids in message_conversations.
         - last_exemplary_label_time will be set when a row for an exemplary label is
           inserted into comment_labels.
+      Internal:
+        - deleted_time will be set when is_deleted is set to true
     """
 
     schema_class = UserSchema
@@ -96,6 +98,10 @@ class User(DatabaseModel):
     )
     open_new_tab_text: bool = Column(Boolean, nullable=False, server_default="false")
     theme_default: str = Column(Text)
+    is_deleted: bool = Column(
+        Boolean, nullable=False, server_default="false", index=True
+    )
+    deleted_time: Optional[datetime] = Column(TIMESTAMP(timezone=True))
     is_banned: bool = Column(Boolean, nullable=False, server_default="false")
     permissions: Any = Column(JSONB(none_as_null=True))
     home_default_order: Optional[TopicSortOption] = Column(ENUM(TopicSortOption))
@@ -137,11 +143,13 @@ class User(DatabaseModel):
         acl.append((Allow, Everyone, "view"))
 
         # message:
-        #  - banned users can't be messaged
-        #  - otherwise, anyone can message a user except themself
-        if not self.is_banned:
-            acl.append((Deny, self.user_id, "message"))
-            acl.append((Allow, Authenticated, "message"))
+        #  - deleted and banned users can't be messaged
+        #  - otherwise, logged-in users can message anyone except themselves
+        if self.is_banned or self.is_deleted:
+            acl.append((Deny, Everyone, "message"))
+
+        acl.append((Deny, self.user_id, "message"))
+        acl.append((Allow, Authenticated, "message"))
 
         # grant the user all other permissions on themself
         acl.append((Allow, self.user_id, ALL_PERMISSIONS))

tildes/tildes/resources/user.py

@@ -14,6 +14,6 @@ from tildes.schemas.user import UserSchema
 @use_kwargs(UserSchema(only=("username",)), locations=("matchdict",))
 def user_by_username(request: Request, username: str) -> User:
     """Get a user specified by {username} in the route or 404 if not found."""
-    query = request.query(User).filter(User.username == username)
+    query = request.query(User).include_deleted().filter(User.username == username)
 
     return get_resource(request, query)

tildes/tildes/templates/user.jinja2

@@ -13,63 +13,70 @@
 <a class="site-header-context" href="/user/{{ user.username }}">{{ user.username }}</a>
 {% endblock %}
 
-{# Only show the heading if they can't see the type tabs #}
+{# Only show the heading if they can't see the type tabs and the user isn't deleted #}
 {% block main_heading %}
-  {% if not request.has_permission('view_types', user) %}
+  {% if not user.is_deleted and not request.has_permission('view_types', user) %}
     {{ user.username }}'s recent activity
   {% endif %}
 {% endblock %}
 
 {% block content %}
-{% if request.has_permission('view_types', user) %}
-  <div class="listing-options">
-    <menu class="tab tab-listing-order">
-      <li class="tab-item{{' active' if not post_type else ''}}">
-        <a href="{{ request.current_listing_normal_url() }}">Recent activity</a>
-      </li>
-      <li class="tab-item{{' active' if post_type == 'topic' else ''}}">
-        <a href="{{ request.current_listing_normal_url({'type': 'topic'}) }}">Topics</a>
-      </li>
-      <li class="tab-item{{ ' active' if post_type == 'comment' else ''}}">
-        <a href="{{ request.current_listing_normal_url({'type': 'comment'}) }}">Comments</a>
-      </li>
-    </menu>
+{% if user.is_deleted %}
+  <div class="empty">
+    <h2 class="empty-title">This user has deleted their account</h2>
+    <div class="empty-action"><a href="/" class="btn btn-primary">Back to the home page</a></div>
   </div>
-{% endif %}
-
-{% if posts %}
-  <ol class="post-listing">
-  {% for post in posts if request.has_permission('view', post) %}
-    <li>
-    {% if post is topic %}
-      {{ render_topic_for_listing(post, show_group=True) }}
-    {% elif post is comment %}
-      <h2>Comment on <a href="{{ post.topic.permalink }}">{{ post.topic.title }}</a> in {{ group_linked(post.topic.group.path) }}</h2>
-      {{ render_single_comment(post) }}
-    {% endif %}
-    </li>
-  {% endfor %}
-  </ol>
+{% else %}
+  {% if request.has_permission('view_types', user) %}
+    <div class="listing-options">
+      <menu class="tab tab-listing-order">
+        <li class="tab-item{{' active' if not post_type else ''}}">
+          <a href="{{ request.current_listing_normal_url() }}">Recent activity</a>
+        </li>
+        <li class="tab-item{{' active' if post_type == 'topic' else ''}}">
+          <a href="{{ request.current_listing_normal_url({'type': 'topic'}) }}">Topics</a>
+        </li>
+        <li class="tab-item{{ ' active' if post_type == 'comment' else ''}}">
+          <a href="{{ request.current_listing_normal_url({'type': 'comment'}) }}">Comments</a>
+        </li>
+      </menu>
+    </div>
+  {% endif %}
 
-  {% if post_type and (posts.has_prev_page or posts.has_next_page) %}
-    <div class="pagination">
-      {% if posts.has_prev_page %}
-        <a class="page-item btn" id="prev-page"
-          href="{{ request.current_listing_base_url({'before': posts.prev_page_before_id36}) }}"
-        >Prev</a>
+  {% if posts %}
+    <ol class="post-listing">
+    {% for post in posts if request.has_permission('view', post) %}
+      <li>
+      {% if post is topic %}
+        {{ render_topic_for_listing(post, show_group=True) }}
+      {% elif post is comment %}
+        <h2>Comment on <a href="{{ post.topic.permalink }}">{{ post.topic.title }}</a> in {{ group_linked(post.topic.group.path) }}</h2>
+        {{ render_single_comment(post) }}
       {% endif %}
+      </li>
+    {% endfor %}
+    </ol>
 
-      {% if posts.has_next_page %}
-        <a class="page-item btn" id="next-page"
-          href="{{ request.current_listing_base_url({'after': posts.next_page_after_id36}) }}"
-        >Next</a>
-      {% endif %}
+    {% if post_type and (posts.has_prev_page or posts.has_next_page) %}
+      <div class="pagination">
+        {% if posts.has_prev_page %}
+          <a class="page-item btn" id="prev-page"
+            href="{{ request.current_listing_base_url({'before': posts.prev_page_before_id36}) }}"
+          >Prev</a>
+        {% endif %}
+
+        {% if posts.has_next_page %}
+          <a class="page-item btn" id="next-page"
+            href="{{ request.current_listing_base_url({'after': posts.next_page_after_id36}) }}"
+          >Next</a>
+        {% endif %}
+      </div>
+    {% endif %}
+  {% else %}
+    <div class="empty">
+      <h2 class="empty-title">This user hasn't made any posts</h2>
     </div>
   {% endif %}
-{% else %}
-  <div class="empty">
-    <h2 class="empty-title">This user hasn't made any posts</h2>
-  </div>
 {% endif %}
 
 {{ comment_label_options_template(comment_label_options) }}