heckyel/mediagoblin
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

I did some more code-keeping in this commit. I added a lot of documentation, so

Browse Source 
that most of my functions do indeed have effective docstrings. I also changed
the decorators so that they imply eachother in a logical way. I also modified
the one decorator get_media_comment_by_id to be more usable with the variable
urls of mediagoblin.user_pages.views:file_a_report. I also noticed a few tests
had broken, so I went through them and fixed them up, finding that mostly there
were problems in my actual writing of the tests. I also did a few other small
tasks such as creating a new User method to check whether or not a User is ban-
-ned.

===============================================================================
    Added in documentation
===============================================================================
--\  mediagoblin/db/models.py
--\  mediagoblin/decorators.py
--\  mediagoblin/moderation/forms.py
--\  mediagoblin/moderation/tools.py
--\  mediagoblin/moderation/views.py
--\  mediagoblin/user_pages/lib.py

===============================================================================
    Rearranged decorators to be more efficient
===============================================================================
--\  mediagoblin/decorators.py
--| Made it so that user_not_banned is encapsulated in require_active_login
--| Made it so that require_active_login is encapsulated in user_has_privilege
--| Changed get_media_comment_by_id into get_optional_media_comment_by_id. It
  | now returns valid code if the MediaComment id is absent. This makes it pos-
  | -sible to use this decorator for the function:
  |         mediagoblin.user_pages.views:file_a_report

--\  mediagoblin/user_pages/views.py
--| Replaced the mediagoblin.user_pages.views:file_a_comment_report with the
  | decorator mentioned above

--\  mediagoblin/user_pages/routing.py

        -----------------------------------------------------------
        |     took out unnecessary @user_not_banned decorators    |
        -----------------------------------------------------------
--\  mediagoblin/submit/views.py
--\  mediagoblin/user_pages/views.py

===============================================================================
    Fixed broken tests
===============================================================================
--\  mediagoblin/tests/test_auth.py
--\  mediagoblin/tests/test_privileges.py
--\  mediagoblin/tests/test_submission.py

===============================================================================
    Fixed broken code
===============================================================================
--\  mediagoblin/tools/response.py

===============================================================================
    Other Tasks
===============================================================================
--\  mediagoblin/db/models.py
--| Added in User.is_banned() method
--\  mediagoblin/decorators.py
--| Utitilized User.is_banned() method in the user_not_banned decorator

--\  mediagoblin/moderation/views.py
--| Made it impossible for an admin to ban themself.
--| Got rid of a vestigial print statement

--\  mediagoblin/templates/mediagoblin/base.html
--| Made it so the top panel does not show up for users that are banned.

--\  mediagoblin/templates/mediagoblin/moderation/user.html
--| Rearranged the javascript slightly

===============================================================================
This commit is contained in:
tilly-Q 2013-09-03 16:19:07 -04:00
parent dc31cd1b65
commit 8e91df8734
15 changed files with 276 additions and 96 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

69
mediagoblin/db/models.py
View File

@ -105,6 +105,17 @@ class User(Base, UserMixin):
_log.info('Deleted user "{0}" account'.format(self.username))
def has_privilege(self,*priv_names):
"""
This method checks to make sure a user has all the correct privileges
to access a piece of content.
:param priv_names A variable number of unicode objects which rep-
-resent the different privileges which may give
the user access to this content. If you pass
multiple arguments, the user will be granted
access if they have ANY of the privileges
passed.
"""
if len(priv_names) == 1:
priv = Privilege.query.filter(
Privilege.privilege_name==priv_names[0]).one()
@ -114,6 +125,16 @@ class User(Base, UserMixin):
self.has_privilege(*priv_names[1:])
return False
def is_banned(self):
"""
Checks if this user is banned.
:returns True if self is banned
:returns False if self is not
"""
return UserBan.query.get(self.id) is not None
class Client(Base):
"""
Model representing a client - Used for API Auth
@ -655,15 +676,21 @@ with_polymorphic(
class ReportBase(Base):
"""
This is the basic report table which the other reports are based off of.
:keyword reporter_id
:keyword report_content
:keyword reported_user_id
:keyword created
:keyword resolved
:keyword result
:keyword discriminator
This is the basic report object which the other reports are based off of.
:keyword reporter_id Holds the id of the user who created
the report, as an Integer column.
:keyword report_content Hold the explanation left by the repor-
-ter to indicate why they filed the
report in the first place, as a
Unicode column.
:keyword reported_user_id Holds the id of the user who created
the content which was reported, as
an Integer column.
:keyword created Holds a datetime column of when the re-
-port was filed.
:keyword discriminator This column distinguishes between the
different types of reports.
"""
__tablename__ = 'core__reports'
id = Column(Integer, primary_key=True)
@ -698,7 +725,9 @@ class ReportBase(Base):
class CommentReport(ReportBase):
"""
A class to keep track of reports that have been filed on comments
Reports that have been filed on comments.
:keyword comment_id Holds the integer value of the reported
comment's ID
"""
__tablename__ = 'core__reports_on_comments'
__mapper_args__ = {'polymorphic_identity': 'comment_report'}
@ -713,7 +742,9 @@ class CommentReport(ReportBase):
class MediaReport(ReportBase):
"""
A class to keep track of reports that have been filed on media entries
Reports that have been filed on media entries
:keyword media_entry_id Holds the integer value of the reported
media entry's ID
"""
__tablename__ = 'core__reports_on_media'
__mapper_args__ = {'polymorphic_identity': 'media_report'}
@ -729,7 +760,23 @@ class MediaReport(ReportBase):
class ArchivedReport(ReportBase):
"""
A table to keep track of reports that have been resolved
Reports that have been resolved. The media_entry and comment columns must
be optional so that if the media entry/comment is deleted, the archive can
still exist.
:keyword comment_id Holds the Integer value of the reported
comment's ID. This column is optio-
-nal.
:keyword media_entry_id Holds the Integer value of the reported
media entry's ID. This column is
optional.
:keyword resolver_id Holds the id of the moderator/admin who
resolved the report.
:keyword resolved Holds the DateTime object which descri-
-bes when this report was resolved
:keyword result Holds the UnicodeText column of the
resolver's reasons for resolving
the report this way. Some of this
is auto-generated
"""
__tablename__ = 'core__reports_archived'
__mapper_args__ = {'polymorphic_identity': 'archived_report'}

111
mediagoblin/decorators.py
View File

@ -31,11 +31,29 @@ from mediagoblin.tools.translate import pass_to_ugettext as _
from mediagoblin.oauth.tools.request import decode_authorization_header
from mediagoblin.oauth.oauth import GMGRequestValidator
def require_active_login(controller):
def user_not_banned(controller):
"""
Require an active login from the user.
Requires that the user has not been banned. Otherwise redirects to the page
explaining why they have been banned
"""
@wraps(controller)
def wrapper(request, *args, **kwargs):
if request.user:
if request.user.is_banned():
return render_user_banned(request)
return controller(request, *args, **kwargs)
return wrapper
def require_active_login(controller):
"""
Require an active login from the user. If the user is banned, redirects to
the "You are Banned" page.
"""
@wraps(controller)
@user_not_banned
def new_controller_func(request, *args, **kwargs):
if request.user and \
not request.user.has_privilege(u'active'):
@ -55,6 +73,34 @@ def require_active_login(controller):
return new_controller_func
def user_has_privilege(privilege_name):
"""
Requires that a user have a particular privilege in order to access a page.
In order to require that a user have multiple privileges, use this
decorator twice on the same view. This decorator also makes sure that the
user is not banned, or else it redirects them to the "You are Banned" page.
:param privilege_name A unicode object that is that represents
the privilege object. This object is
the name of the privilege, as assigned
in the Privilege.privilege_name column
"""
def user_has_privilege_decorator(controller):
@wraps(controller)
@require_active_login
def wrapper(request, *args, **kwargs):
user_id = request.user.id
if not request.user.has_privilege(privilege_name):
raise Forbidden()
return controller(request, *args, **kwargs)
return wrapper
return user_has_privilege_decorator
def active_user_from_url(controller):
"""Retrieve User() from <user> URL pattern and pass in as url_user=...
@ -69,22 +115,6 @@ def active_user_from_url(controller):
return wrapper
def user_has_privilege(privilege_name):
def user_has_privilege_decorator(controller):
@wraps(controller)
def wrapper(request, *args, **kwargs):
user_id = request.user.id
if UserBan.query.filter(UserBan.user_id==user_id).count():
return render_user_banned(request)
elif not request.user.has_privilege(privilege_name):
raise Forbidden()
return controller(request, *args, **kwargs)
return wrapper
return user_has_privilege_decorator
def user_may_delete_media(controller):
"""
@ -274,20 +304,31 @@ def allow_registration(controller):
return wrapper
def get_media_comment_by_id(controller):
def get_optional_media_comment_by_id(controller):
"""
Pass in a MediaComment based off of a url component
Pass in a MediaComment based off of a url component. Because of this decor-
-ator's use in filing Media or Comment Reports, it has two valid outcomes.
:returns The view function being wrapped with kwarg `comment` set to
the MediaComment who's id is in the URL. If there is a
comment id in the URL and if it is valid.
:returns The view function being wrapped with kwarg `comment` set to
None. If there is no comment id in the URL.
:returns A 404 Error page, if there is a comment if in the URL and it
is invalid.
"""
@wraps(controller)
def wrapper(request, *args, **kwargs):
comment = MediaComment.query.filter_by(
id=request.matchdict['comment']).first()
# Still no media? Okay, 404.
if not comment:
return render_404(request)
if 'comment' in request.matchdict:
comment = MediaComment.query.filter_by(
id=request.matchdict['comment']).first()
return controller(request, comment=comment, *args, **kwargs)
if comment is None:
return render_404(request)
return controller(request, comment=comment, *args, **kwargs)
else:
return controller(request, comment=None, *args, **kwargs)
return wrapper
@ -308,7 +349,7 @@ def auth_enabled(controller):
def require_admin_or_moderator_login(controller):
"""
Require an login from an administrator or a moderator.
Require a login from an administrator or a moderator.
"""
@wraps(controller)
def new_controller_func(request, *args, **kwargs):
@ -330,22 +371,6 @@ def require_admin_or_moderator_login(controller):
return new_controller_func
def user_not_banned(controller):
"""
Requires that the user has not been banned. Otherwise redirects to the page
explaining why they have been banned
"""
@wraps(controller)
def wrapper(request, *args, **kwargs):
if request.user:
user_banned = UserBan.query.get(request.user.id)
if user_banned:
return render_user_banned(request)
return controller(request, *args, **kwargs)
return wrapper
def oauth_required(controller):
""" Used to wrap API endpoints where oauth is required """

67
mediagoblin/moderation/forms.py
View File

@ -35,10 +35,19 @@ class MultiCheckboxField(wtforms.SelectMultipleField):
option_widget = wtforms.widgets.CheckboxInput()
# ============ Forms for mediagoblin.moderation.user page ================== #
class PrivilegeAddRemoveForm(wtforms.Form):
"""
This form is used by an admin to give/take away a privilege directly from
their user page.
"""
privilege_name = wtforms.HiddenField('',[wtforms.validators.required()])
class BanForm(wtforms.Form):
"""
This form is used by an admin to ban a user directly from their user page.
"""
user_banned_until = wtforms.DateField(
_(u'User will be banned until:'),
format='%Y-%m-%d',
@ -47,7 +56,54 @@ class BanForm(wtforms.Form):
_(u'Why are you banning this User?'),
validators=[wtforms.validators.optional()])
# =========== Forms for mediagoblin.moderation.report page ================= #
class ReportResolutionForm(wtforms.Form):
"""
This form carries all the information necessary to take punitive actions
against a user who created content that has been reported.
:param action_to_resolve A list of Unicode objects representing
a choice from the ACTION_CHOICES const-
-ant. Every choice passed affects what
punitive actions will be taken against
the user.
:param targeted_user A HiddenField object that holds the id
of the user that was reported.
:param take_away_privileges A list of Unicode objects which repres-
-ent the privileges that are being tak-
-en away. This field is optional and
only relevant if u'takeaway' is in the
`action_to_resolve` list.
:param user_banned_until A DateField object that holds the date
that the user will be unbanned. This
field is optional and only relevant if
u'userban' is in the action_to_resolve
list. If the user is being banned and
this field is blank, the user is banned
indefinitely.
:param why_user_was_banned A TextArea object that holds the
reason that a user was banned, to disp-
-lay to them when they try to log in.
This field is optional and only relevant
if u'userban' is in the
`action_to_resolve` list.
:param message_to_user A TextArea object that holds a message
which will be emailed to the user. This
is only relevant if the u'sendmessage'
option is in the `action_to_resolve`
list.
:param resolution_content A TextArea object that is required for
every report filed. It represents the
reasons that the moderator/admin resol-
-ved the report in such a way.
"""
action_to_resolve = MultiCheckboxField(
_(u'What action will you take to resolve the report?'),
validators=[wtforms.validators.optional()],
@ -67,7 +123,18 @@ class ReportResolutionForm(wtforms.Form):
validators=[wtforms.validators.optional()])
resolution_content = wtforms.TextAreaField()
# ======== Forms for mediagoblin.moderation.report_panel page ============== #
class ReportPanelSortingForm(wtforms.Form):
"""
This form is used for sorting and filtering through different reports in
the mediagoblin.moderation.reports_panel view.
Parameters that start with 'active_' refer to a sort/filter for the active
reports.
Parameters that start with 'closed_' refer to a sort/filter for the closed
reports.
"""
active_p = wtforms.IntegerField(
_(u'Page'),
validators=[wtforms.validators.optional()])

34
mediagoblin/moderation/tools.py
View File

@ -131,6 +131,23 @@ def take_punitive_actions(request, form, report, user):
report_id=report.id)
def take_away_privileges(user,*privileges):
"""
Take away all of the privileges passed as arguments.
:param user A Unicode object representing the target user's
User.username value.
:param privileges A variable number of Unicode objects describing
the privileges being taken away.
:returns True If ALL of the privileges were taken away
successfully.
:returns False If ANY of the privileges were not taken away
successfully. This means the user did not have
(one of) the privilege(s) to begin with.
"""
if len(privileges) == 1:
privilege = Privilege.query.filter(
Privilege.privilege_name==privileges[0]).first()
@ -146,6 +163,23 @@ def take_away_privileges(user,*privileges):
take_away_privileges(user, *privileges[1:]))
def give_privileges(user,*privileges):
"""
Take away all of the privileges passed as arguments.
:param user A Unicode object representing the target user's
User.username value.
:param privileges A variable number of Unicode objects describing
the privileges being granted.
:returns True If ALL of the privileges were granted successf-
-ully.
:returns False If ANY of the privileges were not granted succ-
essfully. This means the user already had (one
of) the privilege(s) to begin with.
"""
if len(privileges) == 1:
privilege = Privilege.query.filter(
Privilege.privilege_name==privileges[0]).first()

7
mediagoblin/moderation/views.py
View File

@ -169,7 +169,8 @@ def moderation_reports_detail(request):
@active_user_from_url
def give_or_take_away_privilege(request, url_user):
'''
A form action to give or take away a particular privilege from a user
A form action to give or take away a particular privilege from a user.
Can only be used by an admin.
'''
form = moderation_forms.PrivilegeAddRemoveForm(request.form)
if request.method == "POST" and form.validate():
@ -193,10 +194,10 @@ def ban_or_unban(request, url_user):
A page to ban or unban a user. Only can be used by an admin.
"""
form = moderation_forms.BanForm(request.form)
print "accessed page"
if request.method == "POST" and form.validate():
already_banned = unban_user(url_user.id)
if not already_banned:
same_as_requesting_user = (request.user.id == url_user.id)
if not already_banned and not same_as_requesting_user:
user_ban = ban_user(url_user.id,
expiration_date = form.user_banned_until.data,
reason = form.why_user_was_banned.data)

1
mediagoblin/submit/views.py
View File

@ -125,7 +125,6 @@ def submit_start(request):
@require_active_login
@user_not_banned
def add_collection(request, media=None):
"""
View to create a new collection

2
mediagoblin/templates/mediagoblin/base.html
View File

@ -59,7 +59,7 @@
{% block mediagoblin_header_title %}{% endblock %}
<div class="header_right">
{%- if request.user %}
{% if request.user and request.user.status == 'active' %}
{% if request.user and request.user.status == 'active' and not request.user.is_banned() %}
{% set notification_count = get_notification_count(request.user.id) %}
{% if notification_count %}

4
mediagoblin/templates/mediagoblin/moderation/user.html
View File

@ -193,7 +193,9 @@ $(document).ready(function(){
$('#hidden_privilege_name').val($(this).attr('id'));
});
init_user_banned_form();
$('.ban_user_submit').click(function(){submit_user_banned_form()});
$('#ban_user_submit').click(function(){
submit_user_banned_form()
});
});
</script>
{% endblock %}

14
mediagoblin/tests/test_auth.py
View File

@ -99,6 +99,12 @@ def test_register_views(test_app):
assert new_user.status == u'needs_email_verification'
assert new_user.email_verified == False
## Make sure that the proper privileges are granted on registration
assert new_user.has_privilege(u'commenter')
assert new_user.has_privilege(u'uploader')
assert new_user.has_privilege(u'reporter')
assert not new_user.has_privilege(u'active')
## Make sure user is logged in
request = template.TEMPLATE_TEST_CONTEXT[
'mediagoblin/user_pages/user.html']['request']
@ -330,14 +336,6 @@ def test_authentication_views(test_app):
'next' : '/u/chris/'})
assert urlparse.urlsplit(response.location)[2] == '/u/chris/'
def test_basic_privileges_granted_on_registration(test_app):
user = User.query.filter(User.username==u'angrygirl').first()
assert User.has_privilege(u'commenter')
assert User.has_privilege(u'uploader')
assert User.has_privilege(u'reporter')
assert not User.has_privilege(u'active')
@pytest.fixture()
def authentication_disabled_app(request):
return get_app(

6
mediagoblin/tests/test_privileges.py
View File

@ -15,7 +15,7 @@
# along with this program. If not, see <http://www.gnu.org/licenses/>.
import pytest
from datetime import datetime, timedelta
from datetime import date, timedelta
from webtest import AppError
from mediagoblin.tests.tools import fixture_add_user, fixture_media_entry
@ -88,7 +88,7 @@ class TestPrivilegeFunctionality:
user_ban.delete()
user_ban = UserBan(user_id=uid,
reason=u'Testing whether user is banned',
expiration_date= datetime.now() + timedelta(days=20))
expiration_date= date.today() + timedelta(days=20))
user_ban.save()
response = self.test_app.get('/')
@ -100,7 +100,7 @@ class TestPrivilegeFunctionality:
#----------------------------------------------------------------------
user_ban = UserBan.query.get(uid)
user_ban.delete()
exp_date = datetime.now() - timedelta(days=20)
exp_date = date.today() - timedelta(days=20)
user_ban = UserBan(user_id=uid,
reason=u'Testing whether user is banned',
expiration_date= exp_date)

2
mediagoblin/tests/test_submission.py
View File

@ -46,7 +46,7 @@ class TestSubmission:
# TODO: Possibly abstract into a decorator like:
# @as_authenticated_user('chris')
fixture_add_user(privileges=[u'active',u'uploader'])
fixture_add_user(privileges=[u'active',u'uploader', u'commenter'])
self.login()

4
mediagoblin/tools/response.py
View File

@ -22,7 +22,7 @@ from mediagoblin.tools.template import render_template
from mediagoblin.tools.translate import (lazy_pass_to_ugettext as _,
pass_to_ugettext)
from mediagoblin.db.models import UserBan, User
from datetime import datetime
from datetime import date
class Response(wz_Response):
"""Set default response mimetype to HTML, otherwise we get text/plain"""
@ -80,7 +80,7 @@ def render_user_banned(request):
"""
user_ban = UserBan.query.get(request.user.id)
if (user_ban.expiration_date is not None and
datetime.now()>user_ban.expiration_date):
date.today()>user_ban.expiration_date):
user_ban.delete()
return redirect(request,

20
mediagoblin/user_pages/lib.py
View File

@ -83,12 +83,22 @@ def build_report_object(report_form, media_entry=None, comment=None):
This function is used to convert a form object (from a User filing a
report) into either a MediaReport or CommentReport object.
:param report_form should be a MediaReportForm or a CommentReportForm
object
:param
:param report_form A MediaReportForm or a CommentReportForm object
with valid information from a POST request.
:param media_entry A MediaEntry object. The MediaEntry being repo-
-rted by a MediaReport. In a CommentReport,
this will be None.
:param comment A MediaComment object. The MediaComment being
reported by a CommentReport. In a MediaReport
this will be None.
:returns either of MediaReport or a CommentReport object that has not been
saved. In case of an improper form_dict, returns None
:returns A MediaReport object if a valid MediaReportForm is
passed as kwarg media_entry. This MediaReport has
not been saved.
:returns A CommentReport object if a valid CommentReportForm
is passed as kwarg comment. This CommentReport
has not been saved.
:returns None if the form_dict is invalid.
"""
if report_form.validate() and comment is not None:

2
mediagoblin/user_pages/routing.py
View File

@ -50,7 +50,7 @@ add_route('mediagoblin.user_pages.media_home.view_comment',
add_route('mediagoblin.user_pages.media_home.report_comment',
'/u/<string:user>/m/<string:media>/c/<int:comment>/report/',
'mediagoblin.user_pages.views:file_a_comment_report')
'mediagoblin.user_pages.views:file_a_report')
# User's tags gallery
add_route('mediagoblin.user_pages.user_tag_gallery',

29
mediagoblin/user_pages/views.py
View File

@ -34,10 +34,10 @@ from mediagoblin.notifications import trigger_notification, \
add_comment_subscription, mark_comment_notification_seen
from mediagoblin.decorators import (uses_pagination, get_user_media_entry,
get_media_entry_by_id, user_has_privilege,
get_media_entry_by_id, user_has_privilege, user_not_banned,
require_active_login, user_may_delete_media, user_may_alter_collection,
get_user_collection, get_user_collection_item, active_user_from_url,
get_media_comment_by_id, user_not_banned)
get_optional_media_comment_by_id)
from werkzeug.contrib.atom import AtomFeed
from werkzeug.exceptions import MethodNotAllowed
@ -161,7 +161,6 @@ def media_home(request, media, page, **kwargs):
@get_media_entry_by_id
@require_active_login
@user_has_privilege(u'commenter')
def media_post_comment(request, media):
"""
@ -291,7 +290,6 @@ def media_collect(request, media):
#TODO: Why does @user_may_delete_media not implicate @require_active_login?
@user_not_banned
@get_media_entry_by_id
@require_active_login
@user_may_delete_media
@ -380,7 +378,6 @@ def collection_list(request, url_user=None):
@get_user_collection_item
@require_active_login
@user_may_alter_collection
@user_not_banned
def collection_item_confirm_remove(request, collection_item):
form = user_forms.ConfirmCollectionItemRemoveForm(request.form)
@ -420,7 +417,7 @@ def collection_item_confirm_remove(request, collection_item):
{'collection_item': collection_item,
'form': form})
@user_not_banned
@get_user_collection
@require_active_login
@user_may_alter_collection
@ -604,7 +601,6 @@ def collection_atom_feed(request):
return feed.get_response()
@user_not_banned
@require_active_login
def processing_panel(request):
"""
@ -649,21 +645,27 @@ def processing_panel(request):
'failed_entries': failed_entries,
'processed_entries': processed_entries})
@require_active_login
@get_user_media_entry
@user_has_privilege(u'reporter')
def file_a_report(request, media, comment=None):
@get_optional_media_comment_by_id
def file_a_report(request, media, comment):
"""
This view handles the filing of a MediaReport or a CommentReport.
"""
if comment is not None:
if not comment.get_media_entry.id == media.id:
return render_404(request)
form = user_forms.CommentReportForm(request.form)
form.reporter_id.data = request.user.id
context = {'media': media,
'comment':comment,
'form':form}
else:
form = user_forms.MediaReportForm(request.form)
form.reporter_id.data = request.user.id
context = {'media': media,
'form':form}
form.reporter_id.data = request.user.id
if request.method == "POST":
report_object = build_report_object(form,
@ -683,8 +685,3 @@ def file_a_report(request, media, comment=None):
'mediagoblin/user_pages/report.html',
context)
@require_active_login
@get_user_media_entry
@get_media_comment_by_id
def file_a_comment_report(request, media, comment):
return file_a_report(request, comment=comment)