Implement PR bug finds

Mostly, fix the busted User/Group get permissions implementation

flaskbb/forum/models.py

@@ -14,7 +14,6 @@ from flask import url_for, abort
 from sqlalchemy.orm import aliased
 
 from flaskbb.extensions import db
-from flaskbb.utils.decorators import can_access_forum, can_access_topic
 from flaskbb.utils.helpers import slugify, get_categories_and_forums, \
     get_forums
 from flaskbb.utils.database import CRUDMixin
@@ -324,7 +323,6 @@ class Topic(db.Model, CRUDMixin):
         return "<{} {}>".format(self.__class__.__name__, self.id)
 
     @classmethod
-    @can_access_topic
     def get_topic(cls, topic_id, user):
         topic = Topic.query.filter_by(id=topic_id).first_or_404()
         return topic
@@ -805,7 +803,6 @@ class Forum(db.Model, CRUDMixin):
 
     # Classmethods
     @classmethod
-    @can_access_forum
     def get_forum(cls, forum_id, user):
         """Returns the forum and forumsread object as a tuple for the user.
 

flaskbb/forum/views.py

@@ -11,26 +11,39 @@
 """
 import datetime
 
-from flask import (Blueprint, redirect, url_for, current_app,
-                   request, flash)
+from flask import Blueprint, redirect, url_for, current_app, request, flash
 from flask_login import login_required, current_user
 from flask_babelex import gettext as _
 from flask_allows import Permission, And
-from flaskbb.extensions import db
+from flaskbb.extensions import db, allows
 from flaskbb.utils.settings import flaskbb_config
-from flaskbb.utils.helpers import (get_online_users, time_diff, format_quote,
-                                   render_template, do_topic_action)
+from flaskbb.utils.helpers import (
+    get_online_users, time_diff, format_quote, render_template, do_topic_action
+)
 
-from flaskbb.utils.requirements import (CanPostReply, CanPostTopic,
-                                        IsAtleastModeratorInForum,
-                                        CanDeleteTopic, CanEditPost,
-                                        CanDeletePost)
+from flaskbb.utils.requirements import (
+    CanAccessForum,
+    CanAccessTopic,
+    CanDeletePost,
+    CanDeleteTopic,
+    CanEditPost,
+    CanPostReply,
+    CanPostTopic,
+    IsAtleastModeratorInForum,
+)
 
 
-from flaskbb.forum.models import (Category, Forum, Topic, Post, ForumsRead,
-                                  TopicsRead)
-from flaskbb.forum.forms import (QuickreplyForm, ReplyForm, NewTopicForm,
-                                 ReportForm, UserSearchForm, SearchPageForm)
+from flaskbb.forum.models import (
+    Category, Forum, Topic, Post, ForumsRead, TopicsRead
+)
+from flaskbb.forum.forms import (
+    NewTopicForm,
+    QuickreplyForm,
+    ReplyForm,
+    ReportForm,
+    SearchPageForm,
+    UserSearchForm,
+)
 from flaskbb.user.models import User
 
 forum = Blueprint("forum", __name__)
@@ -79,11 +92,13 @@ def view_category(category_id, slug=None):
 
 @forum.route("/forum/<int:forum_id>")
 @forum.route("/forum/<int:forum_id>-<slug>")
+@allows.requires(CanAccessForum())
 def view_forum(forum_id, slug=None):
     page = request.args.get('page', 1, type=int)
 
-    forum_instance, forumsread = Forum.get_forum(forum_id=forum_id,
-                                                 user=current_user)
+    forum_instance, forumsread = Forum.get_forum(
+        forum_id=forum_id, user=current_user
+    )
 
     if forum_instance.external:
         return redirect(forum_instance.external)
@@ -101,6 +116,7 @@ def view_forum(forum_id, slug=None):
 
 @forum.route("/topic/<int:topic_id>", methods=["POST", "GET"])
 @forum.route("/topic/<int:topic_id>-<slug>", methods=["POST", "GET"])
+@allows.requires(CanAccessTopic())
 def view_topic(topic_id, slug=None):
     page = request.args.get('page', 1, type=int)
 

flaskbb/management/forms.py

@@ -9,21 +9,27 @@
     :license: BSD, see LICENSE for more details.
 """
 from flask_wtf import Form
-from wtforms import (StringField, TextAreaField, PasswordField, IntegerField,
-                     BooleanField, SelectField, SubmitField,
-		     HiddenField)
-from wtforms.validators import (DataRequired, Optional, Email, regexp, Length,
-                                URL, ValidationError)
-from wtforms.ext.sqlalchemy.fields import (QuerySelectField,
-                                           QuerySelectMultipleField)
+from wtforms import (
+    BooleanField, HiddenField, IntegerField, PasswordField,
+    SelectField, StringField, SubmitField, TextAreaField,
+)
+from wtforms.validators import (
+    DataRequired, Optional, Email, regexp, Length, URL, ValidationError
+)
+from wtforms.ext.sqlalchemy.fields import (
+    QuerySelectField, QuerySelectMultipleField
+)
 from sqlalchemy.orm.session import make_transient, make_transient_to_detached
 from flask_babelex import lazy_gettext as _
 
 from flaskbb.utils.fields import BirthdayField
-from flaskbb.utils.widgets import SelectBirthdayWidget, MultiSelect
+from flaskbb.utils.widgets import SelectBirthdayWidget
 from flaskbb.extensions import db
 from flaskbb.forum.models import Forum, Category
 from flaskbb.user.models import User, Group
+from flaskbb.utils.requirements import IsAtleastModerator
+from flask_allows import Permission
+
 
 USERNAME_RE = r'^[\w.+-]+$'
 is_username = regexp(USERNAME_RE,
@@ -337,35 +343,20 @@ class ForumForm(Form):
                                     "moderators."))
 
     def validate_moderators(self, field):
-        approved_moderators = list()
+        approved_moderators = []
 
         if field.data:
-            # convert the CSV string in a list
-            moderators = field.data.split(",")
-            # remove leading and ending spaces
-            moderators = [mod.strip() for mod in moderators]
-            for moderator in moderators:
-                # Check if the usernames exist
-                user = User.query.filter_by(username=moderator).first()
-
-                # Check if the user has the permissions to moderate a forum
-                if user:
-                    if not (user.get_permissions()["mod"] or
-                            user.get_permissions()["admin"] or
-                            user.get_permissions()["super_mod"]):
-                        raise ValidationError(
-                            _("%(user)s is not in a moderators group.",
-                              user=user.username)
-                        )
-                    else:
-                        approved_moderators.append(user)
+            moderators = [mod.strip() for mod in field.data.split(',')]
+            users = User.query.filter(User.username.in_(moderators))
+            for user in users:
+                if not Permission(IsAtleastModerator, identity=user):
+                    raise ValidationError(
+                        _("%(user)s is not in a moderators group.",
+                            user=user.username)
+                    )
                 else:
-                    raise ValidationError(_("User %(moderator)s not found.",
-                                            moderator=moderator))
-            field.data = approved_moderators
-
-        else:
-            field.data = approved_moderators
+                    approved_moderators.append(user)
+        field.data = approved_moderators
 
     def save(self):
         data = self.data

flaskbb/management/views.py

@@ -17,14 +17,15 @@ from flask import (Blueprint, current_app, request, redirect, url_for, flash,
 from flask_login import current_user
 from flask_plugins import get_all_plugins, get_plugin, get_plugin_from_all
 from flask_babelex import gettext as _
-from flask_allows import Permission
+from flask_allows import Permission, Not
 
 from flaskbb import __version__ as flaskbb_version
 from flaskbb._compat import iteritems
 from flaskbb.forum.forms import UserSearchForm
 from flaskbb.utils.settings import flaskbb_config
-from flaskbb.utils.requirements import (IsAtleastModerator, IsAdmin,
-                                        CanBanUser, CanEditUser)
+from flaskbb.utils.requirements import (
+    IsAtleastModerator, IsAdmin, CanBanUser, CanEditUser, IsAtleastSuperModerator
+)
 from flaskbb.extensions import db, allows
 from flaskbb.utils.helpers import render_template, time_diff, get_online_users
 from flaskbb.user.models import Guest, User, Group
@@ -138,7 +139,7 @@ def users():
 def edit_user(user_id):
     user = User.query.filter_by(id=user_id).first_or_404()
 
-    if not Permission(CanEditUser):
+    if not Permission(CanEditUser, identity=current_user):
         flash(_("You are not allowed to edit this user."), "danger")
         return redirect(url_for("management.users"))
 
@@ -147,12 +148,15 @@ def edit_user(user_id):
 
     filt = db.or_(Group.id.in_(g.id for g in current_user.groups), member_group)
 
-    if any(current_user.permissions[p] for p in ['super_mod', 'admin']):
+    if Permission(IsAtleastSuperModerator, identity=current_user):
         filt = db.or_(filt, Group.mod)
 
-    if current_user.permissions['admin']:
+    if Permission(IsAdmin, identity=current_user):
         filt = db.or_(filt, Group.admin, Group.super_mod)
 
+    if Permission(CanBanUser, identity=current_user):
+        filt = db.or_(filt, Group.banned)
+
     group_query = Group.query.filter(filt)
 
     form = EditUserForm(user)
@@ -254,7 +258,7 @@ def banned_users():
 @management.route("/users/<int:user_id>/ban", methods=["POST"])
 @allows.requires(IsAtleastModerator)
 def ban_user(user_id=None):
-    if not Permission(CanBanUser):
+    if not Permission(CanBanUser, identity=current_user):
         flash(_("You do not have the permissions to ban this user."), "danger")
         return redirect(url_for("management.overview"))
 
@@ -267,10 +271,11 @@ def ban_user(user_id=None):
         for user in users:
             # don't let a user ban himself and do not allow a moderator to ban
             # a admin user
-            if current_user.id == user.id or \
-                    user.get_permissions()['admin'] and \
-                    (current_user.permissions['mod'] or
-                     current_user.permissions['super_mod']):
+            if (
+                current_user.id == user.id or
+                Permission(IsAdmin, identity=user)
+                and Permission(Not(IsAdmin), current_user)
+            ):
                 continue
 
             elif user.ban():
@@ -293,9 +298,8 @@ def ban_user(user_id=None):
     user = User.query.filter_by(id=user_id).first_or_404()
 
     # Do not allow moderators to ban admins
-    if user.get_permissions()['admin'] and \
-            (current_user.permissions['mod'] or
-             current_user.permissions['super_mod']):
+    if Permission(IsAdmin, identity=user) and \
+       Permission(Not(IsAdmin), identity=current_user):
 
         flash(_("A moderator cannot ban an admin user."), "danger")
         return redirect(url_for("management.overview"))
@@ -312,7 +316,7 @@ def ban_user(user_id=None):
 @management.route("/users/<int:user_id>/unban", methods=["POST"])
 @allows.requires(IsAtleastModerator)
 def unban_user(user_id=None):
-    if not Permission(CanBanUser):
+    if not Permission(CanBanUser, identity=current_user):
         flash(_("You do not have the permissions to unban this user."),
               "danger")
         return redirect(url_for("management.overview"))

flaskbb/templates/forum/forum.html

@@ -124,10 +124,12 @@
         </div>
     </div>
 
+    {% if current_user|can_moderate(forum) %}
     <div class="row controls-row">
         <a class="btn btn-default" href="{{ url_for('forum.manage_forum', forum_id=forum.id, slug=forum.slug) }}">
             <span class="fa fa-tasks"></span> {% trans %}Moderation Mode{% endtrans %}
         </a>
     </div>
+    {% endif %}
 </div>
 {% endblock %}

flaskbb/user/models.py

@@ -362,7 +362,7 @@ class User(db.Model, UserMixin, CRUDMixin):
         for group in self.groups:
             columns = set(group.__table__.columns.keys()) - set(exclude)
             for c in columns:
-                perms[c] = getattr(group, c)
+                perms[c] = getattr(group, c) or perms.get(c, False)
         return perms
 
     @cache.memoize(timeout=max_integer)
@@ -499,7 +499,7 @@ class Guest(AnonymousUserMixin):
         for group in self.groups:
             columns = set(group.__table__.columns.keys()) - set(exclude)
             for c in columns:
-                perms[c] = getattr(group, c)
+                perms[c] = getattr(group, c) or perms.get(c, False)
         return perms
 
     @classmethod

flaskbb/utils/decorators.py

@@ -1,88 +0,0 @@
-# -*- coding: utf-8 -*-
-"""
-    flaskbb.utils.decorators
-    ~~~~~~~~~~~~~~~~~~~~~~~~
-
-    A place for our decorators.
-
-    :copyright: (c) 2014 by the FlaskBB Team.
-    :license: BSD, see LICENSE for more details.
-"""
-from functools import wraps
-
-from flask import abort
-from flask_login import current_user
-
-
-def admin_required(f):
-    @wraps(f)
-    def decorated(*args, **kwargs):
-        if current_user.is_anonymous():
-            abort(403)
-        if not current_user.permissions['admin']:
-            abort(403)
-        return f(*args, **kwargs)
-    return decorated
-
-
-def moderator_required(f):
-    @wraps(f)
-    def decorated(*args, **kwargs):
-        if current_user.is_anonymous():
-            abort(403)
-
-        if not any([current_user.permissions['admin'],
-                    current_user.permissions['super_mod'],
-                    current_user.permissions['mod']]):
-            abort(403)
-
-        return f(*args, **kwargs)
-    return decorated
-
-
-def can_access_forum(func):
-    def decorated(*args, **kwargs):
-        forum_id = kwargs['forum_id'] if 'forum_id' in kwargs else args[1]
-        from flaskbb.forum.models import Forum
-        from flaskbb.user.models import Group
-
-        # get list of user group ids
-        if current_user.is_authenticated():
-            user_groups = [gr.id for gr in current_user.groups]
-        else:
-            user_groups = [Group.get_guest_group().id]
-
-        user_forums = Forum.query.filter(
-            Forum.id == forum_id, Forum.groups.any(Group.id.in_(user_groups))
-        ).all()
-
-        if len(user_forums) < 1:
-            abort(403)
-
-        return func(*args, **kwargs)
-    return decorated
-
-
-def can_access_topic(func):
-    def decorated(*args, **kwargs):
-        topic_id = kwargs['topic_id'] if 'topic_id' in kwargs else args[1]
-        from flaskbb.forum.models import Forum, Topic
-        from flaskbb.user.models import Group
-
-        topic = Topic.query.filter_by(id=topic_id).first()
-        # get list of user group ids
-        if current_user.is_authenticated():
-            user_groups = [gr.id for gr in current_user.groups]
-        else:
-            user_groups = [Group.get_guest_group().id]
-
-        user_forums = Forum.query.filter(
-            Forum.id == topic.forum.id,
-            Forum.groups.any(Group.id.in_(user_groups))
-        ).all()
-
-        if len(user_forums) < 1:
-            abort(403)
-
-        return func(*args, **kwargs)
-    return decorated

flaskbb/utils/requirements.py

@@ -11,6 +11,7 @@
 from flask_allows import Requirement, Or, And
 from flaskbb.exceptions import FlaskBBError
 from flaskbb.forum.models import Post, Topic, Forum
+from flaskbb.user.models import Group
 
 
 class Has(Requirement):
@@ -162,6 +163,28 @@ class ForumNotLocked(Requirement):
             return Forum.query.get(view_args['forum_id'])
 
 
+class CanAccessForum(Requirement):
+    def fulfill(self, user, request):
+        forum_id = request.view_args['forum_id']
+        group_ids = [g.id for g in user.groups]
+
+        return Forum.query.filter(
+            Forum.id == forum_id,
+            Forum.groups.any(Group.id.in_(group_ids))
+        ).count()
+
+
+class CanAccessTopic(Requirement):
+    def fulfill(self, user, request):
+        topic_id = request.view_args['topic_id']
+        group_ids = [g.id for g in user.groups]
+
+        return Forum.query.join(Topic, Topic.forum_id == Forum.id).filter(
+            Topic.id == topic_id,
+            Forum.groups.any(Group.id.in_(group_ids))
+        ).count()
+
+
 def IsAtleastModeratorInForum(forum_id=None, forum=None):
     return Or(IsAtleastSuperModerator, IsModeratorInForum(forum_id=forum_id,
                                                           forum=forum))