# HG changeset patch # User Brian Neal # Date 1309649745 0 # Node ID 9d3bd73040507b81a25e37b76d0fe8d2ff1c88a5 # Parent 9a4bffdf37c355aebc45e43599728ae6f5764a2c Fixing #221. Also combined all permissions checks into a new module, permissions.py. This allows us to cache user, category, and forum groups information since it rarely changes. diff -r 9a4bffdf37c3 -r 9d3bd7304050 gpp/forums/models.py --- a/gpp/forums/models.py Sat Jul 02 03:52:43 2011 +0000 +++ b/gpp/forums/models.py Sat Jul 02 23:35:45 2011 +0000 @@ -30,24 +30,6 @@ def __unicode__(self): return self.name - def can_access(self, user): - """ - Checks to see if the given user has permission to access - this category. - If this category has no groups assigned to it, return true. - Else, return true if the user belongs to a group that has been - assigned to this category, and false otherwise. - """ - if self.groups.count() == 0: - return True - if user.is_authenticated(): - return self.groups.filter(user__pk=user.id).count() > 0 - return False - - def is_public(self): - """Returns true if this is a public category, viewable by all.""" - return self.groups.count() == 0 - class ForumManager(models.Manager): """ diff -r 9a4bffdf37c3 -r 9d3bd7304050 gpp/forums/permissions.py --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/gpp/forums/permissions.py Sat Jul 02 23:35:45 2011 +0000 @@ -0,0 +1,114 @@ +""" +This module does permissions checking for the forums application. + +""" +from django.core.cache import cache + +# How long (in secs) to cache group information for various entities: +CATEGORY_TIMEOUT = 4 * 60 * 60 +FORUM_TIMEOUT = 4 * 60 * 60 +USER_TIMEOUT = 15 * 60 + + +def can_access(category, user): + """ + This function returns True if the given user can access the forum category + and False otherwise. + + """ + if user.is_superuser: + return True + + # If this category has no groups assigned to it, return True. Else, return + # True if the user belongs to a group that has been assigned to this + # category, and False otherwise. + + # Get the groups assigned to this category. + cat_groups = get_category_groups(category) + + if len(cat_groups) == 0: + return True # No groups => public category + + user_groups = get_user_groups(user) + return bool(user_groups & cat_groups) + + +def can_moderate(forum, user): + """ + Returns True if the user can moderate the forum. + + """ + # Get the simple cases out of the way first: + if not user.is_authenticated(): + return False + elif user.is_superuser: + return True + + # If we get here, we have to see if there is an intersection between the + # user's groups and the forum's moderator groups. + + forum_groups = get_forum_groups(forum) + user_groups = get_user_groups(user) + + return bool(user_groups & forum_groups) + + +def can_post(topic, user): + """ + Returns True if the user can post in the topic and False otherwise. + + """ + if not user.is_authenticated(): + return False + if user.is_superuser or can_moderate(topic.forum, user): + return True + + return not topic.locked and can_access(topic.forum.category, user) + + +def get_user_groups(user): + """ + Returns a set of group ID's that the user belongs to. + + """ + user_groups_key = '%s_groups' % user.username + return _get_groups(user_groups_key, user.groups.all(), USER_TIMEOUT) + + +def get_forum_groups(forum): + """ + Returns a set of group ID's of the forum's moderator groups. + + """ + forum_groups_key = 'forum_%d_mods' % forum.id + return _get_groups(forum_groups_key, forum.moderators.all(), FORUM_TIMEOUT) + + +def get_category_groups(category): + """ + Returns a set of group ID's of the groups that can access this forum + category. + + """ + cat_groups_key = 'cat_%d_groups' % category.id + return _get_groups(cat_groups_key, category.groups.all(), CATEGORY_TIMEOUT) + + +def _get_groups(key, qs, timeout): + """ + This internal function contains the code common to the get_xxx_groups() + functions. Returns a set of group ID's from the cache. If the set is not + found in the cache, the set is generated from the queryset qs and cached + with the given timeout. + + key - the cache key for the set of group ID's + qs - the query set of groups to query if the set is not in the cache + timeout - the cache timeout to use + + """ + groups = cache.get(key) + if groups is None: + groups = set([g.id for g in qs]) + cache.set(key, groups, timeout) + + return groups diff -r 9a4bffdf37c3 -r 9d3bd7304050 gpp/forums/views/favorites.py --- a/gpp/forums/views/favorites.py Sat Jul 02 03:52:43 2011 +0000 +++ b/gpp/forums/views/favorites.py Sat Jul 02 23:35:45 2011 +0000 @@ -13,6 +13,7 @@ from core.paginator import DiggPaginator from forums.models import Topic +import forums.permissions as perms @login_required @@ -23,11 +24,11 @@ user. """ topic = get_object_or_404(Topic.objects.select_related(), id=topic_id) - if topic.forum.category.can_access(request.user): + if perms.can_access(topic.forum.category, request.user): topic.bookmarkers.add(request.user) return HttpResponseRedirect( reverse("forums-favorites_status", args=[topic.id])) - raise Http404 # TODO return HttpResponseForbidden instead + return HttpResponseForbidden() @login_required diff -r 9a4bffdf37c3 -r 9d3bd7304050 gpp/forums/views/main.py --- a/gpp/forums/views/main.py Sat Jul 02 03:52:43 2011 +0000 +++ b/gpp/forums/views/main.py Sat Jul 02 23:35:45 2011 +0000 @@ -21,19 +21,21 @@ from django.utils.text import wrap from django.db.models import F +import antispam +import antispam.utils +from bio.models import UserProfile, BadgeOwnership from core.paginator import DiggPaginator from core.functions import email_admins -from forums.models import Forum, Topic, Post, FlaggedPost, TopicLastVisit, \ - ForumLastVisit, Attachment -from forums.forms import NewTopicForm, NewPostForm, PostForm, MoveTopicForm, \ - SplitTopicForm -from forums.unread import get_forum_unread_status, get_topic_unread_status, \ - get_post_unread_status, get_unread_topics -from bio.models import UserProfile, BadgeOwnership -import antispam -import antispam.utils +from forums.models import (Forum, Topic, Post, FlaggedPost, TopicLastVisit, + ForumLastVisit, Attachment) +from forums.forms import (NewTopicForm, NewPostForm, PostForm, MoveTopicForm, + SplitTopicForm) +from forums.unread import (get_forum_unread_status, get_topic_unread_status, + get_post_unread_status, get_unread_topics) + from forums.attachments import AttachmentProcessor +import forums.permissions as perms ####################################################################### @@ -117,7 +119,7 @@ """ forum = get_object_or_404(Forum.objects.select_related(), slug=slug) - if not forum.category.can_access(request.user): + if not perms.can_access(forum.category, request.user): return HttpResponseForbidden() feed = None @@ -141,7 +143,7 @@ # we do this for the template since it is rendered twice page_nav = render_to_string('forums/pagination.html', {'page': page}) - can_moderate = _can_moderate(forum, request.user) + can_moderate = perms.can_moderate(forum, request.user) return render_to_response('forums/forum_index.html', { 'forum': forum, @@ -160,7 +162,7 @@ topic = get_object_or_404(Topic.objects.select_related( 'forum', 'forum__category', 'last_post'), pk=id) - if not topic.forum.category.can_access(request.user): + if not perms.can_access(topic.forum.category, request.user): return HttpResponseForbidden() topic.view_count = F('view_count') + 1 @@ -222,7 +224,7 @@ # we do this for the template since it is rendered twice page_nav = render_to_string('forums/pagination.html', {'page': page}) - can_moderate = _can_moderate(topic.forum, request.user) + can_moderate = perms.can_moderate(topic.forum, request.user) can_reply = request.user.is_authenticated() and ( not topic.locked or can_moderate) @@ -289,7 +291,7 @@ """ forum = get_object_or_404(Forum.objects.select_related(), slug=slug) - if not forum.category.can_access(request.user): + if not perms.can_access(forum.category, request.user): return HttpResponseForbidden() if request.method == 'POST': @@ -338,7 +340,7 @@ form = NewPostForm(request.POST) if form.is_valid(): - if not _can_post_in_topic(form.topic, request.user): + if not perms.can_post(form.topic, request.user): return HttpResponseForbidden("You don't have permission to post in this topic.") if antispam.utils.spam_check(request, form.cleaned_data['body']): return HttpResponseForbidden(antispam.BUSTED_MESSAGE) @@ -352,7 +354,7 @@ return render_to_response('forums/display_post.html', { 'post': post, - 'can_moderate': _can_moderate(form.topic.forum, request.user), + 'can_moderate': perms.can_moderate(form.topic.forum, request.user), 'can_reply': True, }, context_instance=RequestContext(request)) @@ -419,7 +421,7 @@ """ post = get_object_or_404(Post.objects.select_related(), pk=id) - can_moderate = _can_moderate(post.topic.forum, request.user) + can_moderate = perms.can_moderate(post.topic.forum, request.user) can_edit = can_moderate or request.user == post.user if not can_edit: @@ -578,7 +580,7 @@ to a topic. """ topic = get_object_or_404(Topic.objects.select_related(), pk=topic_id) - can_post = _can_post_in_topic(topic, request.user) + can_post = perms.can_post(topic, request.user) if can_post: if request.method == 'POST': @@ -625,7 +627,7 @@ This view function is for moderators to toggle the sticky status of a topic. """ topic = get_object_or_404(Topic.objects.select_related(), pk=id) - if _can_moderate(topic.forum, request.user): + if perms.can_moderate(topic.forum, request.user): topic.sticky = not topic.sticky topic.save() return HttpResponseRedirect(topic.get_absolute_url()) @@ -639,7 +641,7 @@ This view function is for moderators to toggle the locked status of a topic. """ topic = get_object_or_404(Topic.objects.select_related(), pk=id) - if _can_moderate(topic.forum, request.user): + if perms.can_moderate(topic.forum, request.user): topic.locked = not topic.locked topic.save() return HttpResponseRedirect(topic.get_absolute_url()) @@ -653,7 +655,7 @@ This view function is for moderators to delete an entire topic. """ topic = get_object_or_404(Topic.objects.select_related(), pk=id) - if _can_moderate(topic.forum, request.user): + if perms.can_moderate(topic.forum, request.user): forum_url = topic.forum.get_absolute_url() _delete_topic(topic) return HttpResponseRedirect(forum_url) @@ -667,7 +669,7 @@ This view function is for moderators to move a topic to a different forum. """ topic = get_object_or_404(Topic.objects.select_related(), pk=id) - if not _can_moderate(topic.forum, request.user): + if not perms.can_moderate(topic.forum, request.user): return HttpResponseForbidden() if request.method == 'POST': @@ -696,7 +698,7 @@ stickying and unstickying, moving, and deleting topics. """ forum = get_object_or_404(Forum.objects.select_related(), slug=slug) - if not _can_moderate(forum, request.user): + if not perms.can_moderate(forum, request.user): return HttpResponseForbidden() topics = forum.topics.select_related('user', 'last_post', 'last_post__user') @@ -770,7 +772,7 @@ """ forum = get_object_or_404(Forum.objects.select_related(), slug=slug) - if not forum.category.can_access(request.user): + if not perms.can_access(forum.category, request.user): return HttpResponseForbidden() forum.catchup(request.user) @@ -783,7 +785,7 @@ This view function allows moderators to split posts off to a new topic. """ topic = get_object_or_404(Topic.objects.select_related(), pk=id) - if not _can_moderate(topic.forum, request.user): + if not perms.can_moderate(topic.forum, request.user): return HttpResponseRedirect(topic.get_absolute_url()) if request.method == "POST": @@ -930,7 +932,7 @@ """Displays information about the IP address the post was made from.""" post = get_object_or_404(Post.objects.select_related(), pk=post_id) - if not _can_moderate(post.topic.forum, request.user): + if not perms.can_moderate(post.topic.forum, request.user): return HttpResponseForbidden("You don't have permission for this post.") ip_users = sorted(set(Post.objects.filter( @@ -971,23 +973,6 @@ context_instance=RequestContext(request)) -def _can_moderate(forum, user): - """ - Determines if a user has permission to moderate a given forum. - """ - return user.is_authenticated() and ( - user.is_superuser or user in forum.moderators.all()) - - -def _can_post_in_topic(topic, user): - """ - This function returns true if the given user can post in the given topic - and false otherwise. - """ - return (not topic.locked and topic.forum.category.can_access(user)) or \ - (user.is_superuser or user in topic.forum.moderators.all()) - - def _bump_post_count(user): """ Increments the forum_post_count for the given user. diff -r 9a4bffdf37c3 -r 9d3bd7304050 gpp/forums/views/subscriptions.py --- a/gpp/forums/views/subscriptions.py Sat Jul 02 03:52:43 2011 +0000 +++ b/gpp/forums/views/subscriptions.py Sat Jul 02 23:35:45 2011 +0000 @@ -13,6 +13,7 @@ from django.views.decorators.http import require_POST from forums.models import Topic +import forums.permissions as perms from core.functions import send_mail from core.paginator import DiggPaginator @@ -50,7 +51,7 @@ def subscribe_topic(request, topic_id): """Subscribe the user to the requested topic.""" topic = get_object_or_404(Topic.objects.select_related(), id=topic_id) - if topic.forum.category.can_access(request.user): + if perms.can_access(topic.forum.category, request.user): topic.subscribers.add(request.user) return HttpResponseRedirect( reverse("forums-subscription_status", args=[topic.id]))