Start to use the media_id in "admin" URLs.
We have a bunch of URLs that are more for internal use. At least they're definitely not intended to be posted somewhere for long term useage. When those things affect a media, it's much better to reference the media by its id. This can't change, ever. This is better for races. Like someone posting a comment while the owner corrects a typo in the slug.
This commit is contained in:
parent
8b271c28bd
commit
461dd9717c
@ -69,7 +69,7 @@ def user_may_delete_media(controller):
|
|||||||
"""
|
"""
|
||||||
@wraps(controller)
|
@wraps(controller)
|
||||||
def wrapper(request, *args, **kwargs):
|
def wrapper(request, *args, **kwargs):
|
||||||
uploader_id = MediaEntry.query.get(request.matchdict['media']).uploader
|
uploader_id = kwargs['media'].uploader
|
||||||
if not (request.user.is_admin or
|
if not (request.user.is_admin or
|
||||||
request.user.id == uploader_id):
|
request.user.id == uploader_id):
|
||||||
raise Forbidden()
|
raise Forbidden()
|
||||||
@ -209,12 +209,16 @@ def get_media_entry_by_id(controller):
|
|||||||
@wraps(controller)
|
@wraps(controller)
|
||||||
def wrapper(request, *args, **kwargs):
|
def wrapper(request, *args, **kwargs):
|
||||||
media = MediaEntry.query.filter_by(
|
media = MediaEntry.query.filter_by(
|
||||||
id=request.matchdict['media'],
|
id=request.matchdict['media_id'],
|
||||||
state=u'processed').first()
|
state=u'processed').first()
|
||||||
# Still no media? Okay, 404.
|
# Still no media? Okay, 404.
|
||||||
if not media:
|
if not media:
|
||||||
return render_404(request)
|
return render_404(request)
|
||||||
|
|
||||||
|
given_username = request.matchdict.get('user')
|
||||||
|
if given_username and (given_username != media.get_uploader.username):
|
||||||
|
return render_404(request)
|
||||||
|
|
||||||
return controller(request, media=media, *args, **kwargs)
|
return controller(request, media=media, *args, **kwargs)
|
||||||
|
|
||||||
return wrapper
|
return wrapper
|
||||||
|
@ -27,6 +27,7 @@ from mediagoblin.auth import lib as auth_lib
|
|||||||
from mediagoblin.edit import forms
|
from mediagoblin.edit import forms
|
||||||
from mediagoblin.edit.lib import may_edit_media
|
from mediagoblin.edit.lib import may_edit_media
|
||||||
from mediagoblin.decorators import (require_active_login, active_user_from_url,
|
from mediagoblin.decorators import (require_active_login, active_user_from_url,
|
||||||
|
get_media_entry_by_id,
|
||||||
get_user_media_entry, user_may_alter_collection, get_user_collection)
|
get_user_media_entry, user_may_alter_collection, get_user_collection)
|
||||||
from mediagoblin.tools.response import render_to_response, redirect
|
from mediagoblin.tools.response import render_to_response, redirect
|
||||||
from mediagoblin.tools.translate import pass_to_ugettext as _
|
from mediagoblin.tools.translate import pass_to_ugettext as _
|
||||||
@ -37,7 +38,7 @@ from mediagoblin.db.util import check_media_slug_used, check_collection_slug_use
|
|||||||
import mimetypes
|
import mimetypes
|
||||||
|
|
||||||
|
|
||||||
@get_user_media_entry
|
@get_media_entry_by_id
|
||||||
@require_active_login
|
@require_active_login
|
||||||
def edit_media(request, media):
|
def edit_media(request, media):
|
||||||
if not may_edit_media(request, media):
|
if not may_edit_media(request, media):
|
||||||
|
@ -29,7 +29,7 @@
|
|||||||
|
|
||||||
<form action="{{ request.urlgen('mediagoblin.edit.edit_media',
|
<form action="{{ request.urlgen('mediagoblin.edit.edit_media',
|
||||||
user= media.get_uploader.username,
|
user= media.get_uploader.username,
|
||||||
media= media.id) }}"
|
media_id=media.id) }}"
|
||||||
method="POST" enctype="multipart/form-data">
|
method="POST" enctype="multipart/form-data">
|
||||||
<div class="form_box_xl edit_box">
|
<div class="form_box_xl edit_box">
|
||||||
<h1>{% trans media_title=media.title %}Editing {{ media_title }}{% endtrans %}</h1>
|
<h1>{% trans media_title=media.title %}Editing {{ media_title }}{% endtrans %}</h1>
|
||||||
|
@ -83,11 +83,11 @@
|
|||||||
request.user.is_admin) %}
|
request.user.is_admin) %}
|
||||||
{% set edit_url = request.urlgen('mediagoblin.edit.edit_media',
|
{% set edit_url = request.urlgen('mediagoblin.edit.edit_media',
|
||||||
user= media.get_uploader.username,
|
user= media.get_uploader.username,
|
||||||
media= media.id) %}
|
media_id=media.id) %}
|
||||||
<a class="button_action" href="{{ edit_url }}">{% trans %}Edit{% endtrans %}</a>
|
<a class="button_action" href="{{ edit_url }}">{% trans %}Edit{% endtrans %}</a>
|
||||||
{% set delete_url = request.urlgen('mediagoblin.user_pages.media_confirm_delete',
|
{% set delete_url = request.urlgen('mediagoblin.user_pages.media_confirm_delete',
|
||||||
user= media.get_uploader.username,
|
user= media.get_uploader.username,
|
||||||
media= media.id) %}
|
media_id=media.id) %}
|
||||||
<a class="button_action" href="{{ delete_url }}">{% trans %}Delete{% endtrans %}</a>
|
<a class="button_action" href="{{ delete_url }}">{% trans %}Delete{% endtrans %}</a>
|
||||||
{% endif %}
|
{% endif %}
|
||||||
{% autoescape False %}
|
{% autoescape False %}
|
||||||
@ -104,7 +104,7 @@
|
|||||||
{% if request.user %}
|
{% if request.user %}
|
||||||
<form action="{{ request.urlgen('mediagoblin.user_pages.media_post_comment',
|
<form action="{{ request.urlgen('mediagoblin.user_pages.media_post_comment',
|
||||||
user= media.get_uploader.username,
|
user= media.get_uploader.username,
|
||||||
media=media.id) }}" method="POST" id="form_comment">
|
media_id=media.id) }}" method="POST" id="form_comment">
|
||||||
<p>
|
<p>
|
||||||
{% trans %}You can use <a href="http://daringfireball.net/projects/markdown/basics">Markdown</a> for formatting.{% endtrans %}
|
{% trans %}You can use <a href="http://daringfireball.net/projects/markdown/basics">Markdown</a> for formatting.{% endtrans %}
|
||||||
</p>
|
</p>
|
||||||
|
@ -23,7 +23,7 @@
|
|||||||
|
|
||||||
<form action="{{ request.urlgen('mediagoblin.user_pages.media_confirm_delete',
|
<form action="{{ request.urlgen('mediagoblin.user_pages.media_confirm_delete',
|
||||||
user=media.get_uploader.username,
|
user=media.get_uploader.username,
|
||||||
media=media.id) }}"
|
media_id=media.id) }}"
|
||||||
method="POST" enctype="multipart/form-data">
|
method="POST" enctype="multipart/form-data">
|
||||||
<div class="form_box">
|
<div class="form_box">
|
||||||
<h1>
|
<h1>
|
||||||
|
@ -161,11 +161,17 @@ class TestSubmission:
|
|||||||
media = self.check_media(request, {'title': u'Balanced Goblin'}, 1)
|
media = self.check_media(request, {'title': u'Balanced Goblin'}, 1)
|
||||||
media_id = media.id
|
media_id = media.id
|
||||||
|
|
||||||
|
# At least render the edit page
|
||||||
|
edit_url = request.urlgen(
|
||||||
|
'mediagoblin.edit.edit_media',
|
||||||
|
user=self.test_user.username, media_id=media_id)
|
||||||
|
self.test_app.get(edit_url)
|
||||||
|
|
||||||
# Add a comment, so we can test for its deletion later.
|
# Add a comment, so we can test for its deletion later.
|
||||||
self.check_comments(request, media_id, 0)
|
self.check_comments(request, media_id, 0)
|
||||||
comment_url = request.urlgen(
|
comment_url = request.urlgen(
|
||||||
'mediagoblin.user_pages.media_post_comment',
|
'mediagoblin.user_pages.media_post_comment',
|
||||||
user=self.test_user.username, media=media_id)
|
user=self.test_user.username, media_id=media_id)
|
||||||
response = self.do_post({'comment_content': 'i love this test'},
|
response = self.do_post({'comment_content': 'i love this test'},
|
||||||
url=comment_url, do_follow=True)[0]
|
url=comment_url, do_follow=True)[0]
|
||||||
self.check_comments(request, media_id, 1)
|
self.check_comments(request, media_id, 1)
|
||||||
@ -174,7 +180,7 @@ class TestSubmission:
|
|||||||
# ---------------------------------------------------
|
# ---------------------------------------------------
|
||||||
delete_url = request.urlgen(
|
delete_url = request.urlgen(
|
||||||
'mediagoblin.user_pages.media_confirm_delete',
|
'mediagoblin.user_pages.media_confirm_delete',
|
||||||
user=self.test_user.username, media=media_id)
|
user=self.test_user.username, media_id=media_id)
|
||||||
# Empty data means don't confirm
|
# Empty data means don't confirm
|
||||||
response = self.do_post({}, do_follow=True, url=delete_url)[0]
|
response = self.do_post({}, do_follow=True, url=delete_url)[0]
|
||||||
media = self.check_media(request, {'title': u'Balanced Goblin'}, 1)
|
media = self.check_media(request, {'title': u'Balanced Goblin'}, 1)
|
||||||
|
@ -24,12 +24,12 @@ add_route('mediagoblin.user_pages.media_home',
|
|||||||
'mediagoblin.user_pages.views:media_home')
|
'mediagoblin.user_pages.views:media_home')
|
||||||
|
|
||||||
add_route('mediagoblin.user_pages.media_confirm_delete',
|
add_route('mediagoblin.user_pages.media_confirm_delete',
|
||||||
'/u/<string:user>/m/<string:media>/confirm-delete/',
|
'/u/<string:user>/m/<int:media_id>/confirm-delete/',
|
||||||
'mediagoblin.user_pages.views:media_confirm_delete')
|
'mediagoblin.user_pages.views:media_confirm_delete')
|
||||||
|
|
||||||
# Submission handling of new comments. TODO: only allow for POST methods
|
# Submission handling of new comments. TODO: only allow for POST methods
|
||||||
add_route('mediagoblin.user_pages.media_post_comment',
|
add_route('mediagoblin.user_pages.media_post_comment',
|
||||||
'/u/<string:user>/m/<string:media>/comment/add/',
|
'/u/<string:user>/m/<int:media_id>/comment/add/',
|
||||||
'mediagoblin.user_pages.views:media_post_comment')
|
'mediagoblin.user_pages.views:media_post_comment')
|
||||||
|
|
||||||
add_route('mediagoblin.user_pages.user_gallery',
|
add_route('mediagoblin.user_pages.user_gallery',
|
||||||
@ -74,7 +74,7 @@ add_route('mediagoblin.user_pages.processing_panel',
|
|||||||
|
|
||||||
# Stray edit routes
|
# Stray edit routes
|
||||||
add_route('mediagoblin.edit.edit_media',
|
add_route('mediagoblin.edit.edit_media',
|
||||||
'/u/<string:user>/m/<string:media>/edit/',
|
'/u/<string:user>/m/<int:media_id>/edit/',
|
||||||
'mediagoblin.edit.views:edit_media')
|
'mediagoblin.edit.views:edit_media')
|
||||||
|
|
||||||
add_route('mediagoblin.edit.attachments',
|
add_route('mediagoblin.edit.attachments',
|
||||||
|
@ -28,6 +28,7 @@ from mediagoblin.user_pages import forms as user_forms
|
|||||||
from mediagoblin.user_pages.lib import send_comment_email
|
from mediagoblin.user_pages.lib import send_comment_email
|
||||||
|
|
||||||
from mediagoblin.decorators import (uses_pagination, get_user_media_entry,
|
from mediagoblin.decorators import (uses_pagination, get_user_media_entry,
|
||||||
|
get_media_entry_by_id,
|
||||||
require_active_login, user_may_delete_media, user_may_alter_collection,
|
require_active_login, user_may_delete_media, user_may_alter_collection,
|
||||||
get_user_collection, get_user_collection_item, active_user_from_url)
|
get_user_collection, get_user_collection_item, active_user_from_url)
|
||||||
|
|
||||||
@ -138,7 +139,7 @@ def media_home(request, media, page, **kwargs):
|
|||||||
'app_config': mg_globals.app_config})
|
'app_config': mg_globals.app_config})
|
||||||
|
|
||||||
|
|
||||||
@get_user_media_entry
|
@get_media_entry_by_id
|
||||||
@require_active_login
|
@require_active_login
|
||||||
def media_post_comment(request, media):
|
def media_post_comment(request, media):
|
||||||
"""
|
"""
|
||||||
@ -258,7 +259,7 @@ def media_collect(request, media):
|
|||||||
|
|
||||||
|
|
||||||
#TODO: Why does @user_may_delete_media not implicate @require_active_login?
|
#TODO: Why does @user_may_delete_media not implicate @require_active_login?
|
||||||
@get_user_media_entry
|
@get_media_entry_by_id
|
||||||
@require_active_login
|
@require_active_login
|
||||||
@user_may_delete_media
|
@user_may_delete_media
|
||||||
def media_confirm_delete(request, media):
|
def media_confirm_delete(request, media):
|
||||||
|
Loading…
x
Reference in New Issue
Block a user