Merge pull request #75 from novist/permissions-improvements

Permissions improvements

flaskbb/forum/views.py

@@ -114,15 +114,11 @@ def view_topic(topic_id, slug=None):
 
     form = None
 
-    if not topic.locked \
-        and not topic.forum.locked \
-        and can_post_reply(user=current_user,
-                           forum=topic.forum):
-
-            form = QuickreplyForm()
-            if form.validate_on_submit():
-                post = form.save(current_user, topic)
-                return view_post(post.id)
+    if can_post_reply(user=current_user, topic=topic):
+        form = QuickreplyForm()
+        if form.validate_on_submit():
+            post = form.save(current_user, topic)
+            return view_post(post.id)
 
     return render_template("forum/topic.html", topic=topic, posts=posts,
                            last_seen=time_diff(), form=form)
@@ -148,14 +144,8 @@ def view_post(post_id):
 def new_topic(forum_id, slug=None):
     forum = Forum.query.filter_by(id=forum_id).first_or_404()
 
-    if forum.locked:
-        flash("This forum is locked; you cannot submit new topics or posts.",
-              "danger")
-        return redirect(forum.url)
-
     if not can_post_topic(user=current_user, forum=forum):
-        flash("You do not have the permissions to create a new topic.",
-              "danger")
+        flash("You do not have the permissions to create a new topic.", "danger")
         return redirect(forum.url)
 
     form = NewTopicForm()
@@ -177,8 +167,7 @@ def new_topic(forum_id, slug=None):
 def delete_topic(topic_id, slug=None):
     topic = Topic.query.filter_by(id=topic_id).first_or_404()
 
-    if not can_delete_topic(user=current_user, forum=topic.forum,
-                            post_user_id=topic.first_post.user_id):
+    if not can_delete_topic(user=current_user, topic=topic):
 
         flash("You do not have the permissions to delete the topic", "danger")
         return redirect(topic.forum.url)
@@ -272,17 +261,8 @@ def merge_topic(old_id, new_id, old_slug=None, new_slug=None):
 def new_post(topic_id, slug=None):
     topic = Topic.query.filter_by(id=topic_id).first_or_404()
 
-    if topic.forum.locked:
-        flash("This forum is locked; you cannot submit new topics or posts.",
-              "danger")
-        return redirect(topic.forum.url)
-
-    if topic.locked:
-        flash("The topic is locked.", "danger")
-        return redirect(topic.forum.url)
-
-    if not can_post_reply(user=current_user, forum=topic.forum):
-        flash("You do not have the permissions to delete the topic", "danger")
+    if not can_post_reply(user=current_user, topic=topic):
+        flash("You do not have the permissions to post here", "danger")
         return redirect(topic.forum.url)
 
     form = ReplyForm()
@@ -303,16 +283,7 @@ def reply_post(topic_id, post_id):
     topic = Topic.query.filter_by(id=topic_id).first_or_404()
     post = Post.query.filter_by(id=post_id).first_or_404()
 
-    if post.topic.forum.locked:
-        flash("This forum is locked; you cannot submit new topics or posts.",
-              "danger")
-        return redirect(post.topic.forum.url)
-
-    if post.topic.locked:
-        flash("The topic is locked.", "danger")
-        return redirect(post.topic.forum.url)
-
-    if not can_post_reply(user=current_user, forum=topic.forum):
+    if not can_post_reply(user=current_user, topic=topic):
         flash("You do not have the permissions to post in this topic", "danger")
         return redirect(topic.forum.url)
 
@@ -335,17 +306,7 @@ def reply_post(topic_id, post_id):
 def edit_post(post_id):
     post = Post.query.filter_by(id=post_id).first_or_404()
 
-    if post.topic.forum.locked:
-        flash("This forum is locked; you cannot submit new topics or posts.",
-              "danger")
-        return redirect(post.topic.forum.url)
-
-    if post.topic.locked:
-        flash("The topic is locked.", "danger")
-        return redirect(post.topic.forum.url)
-
-    if not can_edit_post(user=current_user, forum=post.topic.forum,
-                         post_user_id=post.user_id):
+    if not can_edit_post(user=current_user, post=post):
         flash("You do not have the permissions to edit this post", "danger")
         return redirect(post.topic.url)
 
@@ -373,8 +334,7 @@ def delete_post(post_id, slug=None):
 
     # TODO: Bulk delete
 
-    if not can_delete_post(user=current_user, forum=post.topic.forum,
-                           post_user_id=post.user_id):
+    if not can_delete_post(user=current_user, post=post):
         flash("You do not have the permissions to edit this post", "danger")
         return redirect(post.topic.url)
 

flaskbb/templates/forum/topic.html

@@ -116,7 +116,7 @@
                         Report
                     </a> |
                     {% endif %}
-                    {% if current_user|edit_post(post.user_id, topic.forum) %}
+                    {% if current_user|edit_post(post) %}
                     <a href="{{ url_for('forum.edit_post', post_id=post.id) }}">Edit</a> |
                     {% endif %}
                     {% if topic.first_post_id == post.id %}
@@ -128,7 +128,7 @@
                         <a href="{{ url_for('forum.delete_post', post_id=post.id) }}">Delete</a> |
                         {% endif %}
                     {% endif %}
-                    {% if current_user|post_reply(topic.forum) and not (topic.locked or topic.forum.locked) %}
+                    {% if current_user|post_reply(topic) %}
                         <!-- Quick quote -->
                         <a href="#" class="quote_btn" data-post-id="{{ post.id }}">Quote</a> |
                         <!-- Full quote/reply -->

flaskbb/templates/forum/topic_controls.html

@@ -1,4 +1,3 @@
-
 <div class="pull-left" style="padding-bottom: 10px">
     {{ render_pagination(posts, topic.url) }}
 </div> <!-- end span pagination -->
@@ -35,7 +34,7 @@
         </a>
         {% endif %}
 
-        {% if current_user|post_reply(topic.forum) and not (topic.locked or topic.forum.locked) %}
+        {% if current_user|post_reply(topic) %}
         <a href="{{ url_for('forum.new_post', topic_id=topic.id, slug=topic.slug) }}" class="btn btn-primary">
             <span class="fa fa-pencil"></span> Reply
         </a>

flaskbb/utils/permissions.py

@@ -29,7 +29,7 @@ def check_perm(user, perm, forum, post_user_id=None):
         return True
     if post_user_id and user.is_authenticated():
         return user.permissions[perm] and user.id == post_user_id
-    return user.permissions[perm]
+    return not user.permissions['banned'] and user.permissions[perm]
 
 
 def is_moderator(user):
@@ -92,31 +92,38 @@ def can_moderate(user, forum=None, perm=None):
     return user.permissions['super_mod'] or user.permissions['admin']
 
 
-def can_edit_post(user, post_user_id, forum):
+def can_edit_post(user, post):
     """Check if the post can be edited by the user"""
-
-    return check_perm(user=user, perm='editpost', forum=forum,
-                      post_user_id=post_user_id)
+    topic = post.topic
+    if can_moderate(user, topic.forum):
+        return True
+    if topic.locked or topic.forum.locked:
+        return False
+    return check_perm(user=user, perm='editpost', forum=post.topic.forum,
+                      post_user_id=post.user_id)
 
 
-def can_delete_post(user, post_user_id, forum):
+def can_delete_post(user, post):
     """Check if the post can be deleted by the user"""
 
-    return check_perm(user=user, perm='deletepost', forum=forum,
-                      post_user_id=post_user_id)
+    return check_perm(user=user, perm='deletepost', forum=post.topic.forum,
+                      post_user_id=post.user_id)
 
 
-def can_delete_topic(user, post_user_id, forum):
+def can_delete_topic(user, topic):
     """Check if the topic can be deleted by the user"""
 
-    return check_perm(user=user, perm='deletetopic', forum=forum,
-                      post_user_id=post_user_id)
+    return check_perm(user=user, perm='deletetopic', forum=topic.forum,
+                      post_user_id=topic.user_id)
 
 
-def can_post_reply(user, forum):
+def can_post_reply(user, topic):
     """Check if the user is allowed to post in the forum"""
-
-    return check_perm(user=user, perm='postreply', forum=forum)
+    if can_moderate(user, topic.forum):
+        return True
+    if topic.locked or topic.forum.locked:
+        return False
+    return check_perm(user=user, perm='postreply', forum=topic.forum)
 
 
 def can_post_topic(user, forum):

tests/unit/utils/test_permissions.py

@@ -13,13 +13,13 @@ def test_moderator_permissions_in_forum(
 
     assert moderator_user in forum.moderators
 
-    assert can_post_reply(moderator_user, forum)
+    assert can_post_reply(moderator_user, topic)
     assert can_post_topic(moderator_user, forum)
-    assert can_edit_post(moderator_user, topic.user_id, forum)
+    assert can_edit_post(moderator_user, topic.first_post)
 
     assert can_moderate(moderator_user, forum)
-    assert can_delete_post(moderator_user, topic.user_id, forum)
-    assert can_delete_topic(moderator_user, topic.user_id, forum)
+    assert can_delete_post(moderator_user, topic.first_post)
+    assert can_delete_topic(moderator_user, topic)
 
 
 def test_moderator_permissions_without_forum(
@@ -32,17 +32,17 @@ def test_moderator_permissions_without_forum(
     assert not moderator_user in forum.moderators
     assert not can_moderate(moderator_user, forum)
 
-    assert can_post_reply(moderator_user, forum)
+    assert can_post_reply(moderator_user, topic)
     assert can_post_topic(moderator_user, forum)
 
-    assert not can_edit_post(moderator_user, topic.user_id, forum)
-    assert not can_delete_post(moderator_user, topic.user_id, forum)
-    assert not can_delete_topic(moderator_user, topic.user_id, forum)
+    assert not can_edit_post(moderator_user, topic.first_post)
+    assert not can_delete_post(moderator_user, topic.first_post)
+    assert not can_delete_topic(moderator_user, topic)
 
     # Test with own topic
-    assert can_delete_post(moderator_user, topic_moderator.user_id, forum)
-    assert can_delete_topic(moderator_user, topic_moderator.user_id, forum)
-    assert can_edit_post(moderator_user, topic_moderator.user_id, forum)
+    assert can_delete_post(moderator_user, topic_moderator.first_post)
+    assert can_delete_topic(moderator_user, topic_moderator)
+    assert can_edit_post(moderator_user, topic_moderator.first_post)
 
     # Test moderator permissions
     assert can_edit_user(moderator_user)
@@ -53,12 +53,12 @@ def test_normal_permissions(forum, user, topic):
     """Test the permissions for a normal user."""
     assert not can_moderate(user, forum)
 
-    assert can_post_reply(user, forum)
+    assert can_post_reply(user, topic)
     assert can_post_topic(user, forum)
 
-    assert can_edit_post(user, topic.user_id, forum)
-    assert not can_delete_post(user, topic.user_id, forum)
-    assert not can_delete_topic(user, topic.user_id, forum)
+    assert can_edit_post(user, topic.first_post)
+    assert not can_delete_post(user, topic.first_post)
+    assert not can_delete_topic(user, topic)
 
     assert not can_edit_user(user)
     assert not can_ban_user(user)
@@ -68,12 +68,12 @@ def test_admin_permissions(forum, admin_user, topic):
     """Test the permissions for a admin user."""
     assert can_moderate(admin_user, forum)
 
-    assert can_post_reply(admin_user, forum)
+    assert can_post_reply(admin_user, topic)
     assert can_post_topic(admin_user, forum)
 
-    assert can_edit_post(admin_user, topic.user_id, forum)
-    assert can_delete_post(admin_user, topic.user_id, forum)
-    assert can_delete_topic(admin_user, topic.user_id, forum)
+    assert can_edit_post(admin_user, topic.first_post)
+    assert can_delete_post(admin_user, topic.first_post)
+    assert can_delete_topic(admin_user, topic)
 
     assert can_edit_user(admin_user)
     assert can_ban_user(admin_user)
@@ -83,12 +83,12 @@ def test_super_moderator_permissions(forum, super_moderator_user, topic):
     """Test the permissions for a super moderator user."""
     assert can_moderate(super_moderator_user, forum)
 
-    assert can_post_reply(super_moderator_user, forum)
+    assert can_post_reply(super_moderator_user, topic)
     assert can_post_topic(super_moderator_user, forum)
 
-    assert can_edit_post(super_moderator_user, topic.user_id, forum)
-    assert can_delete_post(super_moderator_user, topic.user_id, forum)
-    assert can_delete_topic(super_moderator_user, topic.user_id, forum)
+    assert can_edit_post(super_moderator_user, topic.first_post)
+    assert can_delete_post(super_moderator_user, topic.first_post)
+    assert can_delete_topic(super_moderator_user, topic)
 
     assert can_edit_user(super_moderator_user)
     assert can_ban_user(super_moderator_user)