diff --git a/spirit/admin/views.py b/spirit/admin/views.py index 31e35a9d5..00e134c5e 100644 --- a/spirit/admin/views.py +++ b/spirit/admin/views.py @@ -1,12 +1,13 @@ # -*- coding: utf-8 -*- -from django.shortcuts import render, redirect +from django.shortcuts import render from django.contrib import messages from django.utils.translation import gettext as _ from django.contrib.auth import get_user_model import spirit import django +from spirit.core.utils.http import safe_redirect from spirit.category.models import Category from spirit.comment.flag.models import CommentFlag from spirit.comment.like.models import CommentLike @@ -25,7 +26,7 @@ def config_basic(request): if is_post(request) and form.is_valid(): form.save() messages.info(request, _("Settings updated!")) - return redirect(request.GET.get("next", request.get_full_path())) + return safe_redirect(request, "next", request.get_full_path()) return render( request=request, template_name='spirit/admin/config_basic.html', diff --git a/spirit/comment/flag/views.py b/spirit/comment/flag/views.py index a2f295205..9fa4cf163 100644 --- a/spirit/comment/flag/views.py +++ b/spirit/comment/flag/views.py @@ -1,9 +1,10 @@ # -*- coding: utf-8 -*- from django.contrib.auth.decorators import login_required -from django.shortcuts import render, redirect, get_object_or_404 +from django.shortcuts import render, get_object_or_404 -from ...core.utils.views import is_post, post_data +from spirit.core.utils.http import safe_redirect +from spirit.core.utils.views import is_post, post_data from ..models import Comment from .forms import FlagForm @@ -18,7 +19,7 @@ def create(request, comment_id): if is_post(request) and form.is_valid(): form.save() - return redirect(request.POST.get('next', comment.get_absolute_url())) + return safe_redirect(request, 'next', comment.get_absolute_url(), method='POST') return render( request=request, diff --git a/spirit/comment/like/views.py b/spirit/comment/like/views.py index 6b046b996..541d0e57e 100644 --- a/spirit/comment/like/views.py +++ b/spirit/comment/like/views.py @@ -1,9 +1,10 @@ # -*- coding: utf-8 -*- from django.contrib.auth.decorators import login_required -from django.shortcuts import render, redirect, get_object_or_404 +from django.shortcuts import render, get_object_or_404 from django.urls import reverse +from spirit.core.utils.http import safe_redirect from spirit.core.utils.views import is_post, post_data, is_ajax from spirit.core.utils import json_response from spirit.comment.models import Comment @@ -28,7 +29,7 @@ def create(request, comment_id): if is_ajax(request): return json_response({'url_delete': like.get_delete_url()}) - return redirect(request.POST.get('next', comment.get_absolute_url())) + return safe_redirect(request, 'next', comment.get_absolute_url(), method='POST') return render( request=request, @@ -52,7 +53,8 @@ def delete(request, pk): kwargs={'comment_id': like.comment.pk}) return json_response({'url_create': url, }) - return redirect(request.POST.get('next', like.comment.get_absolute_url())) + return safe_redirect( + request, 'next', like.comment.get_absolute_url(), method='POST') return render( request=request, diff --git a/spirit/comment/poll/views.py b/spirit/comment/poll/views.py index 3a904df3f..18838844a 100644 --- a/spirit/comment/poll/views.py +++ b/spirit/comment/poll/views.py @@ -1,7 +1,7 @@ # -*- coding: utf-8 -*- from django.contrib.auth.decorators import login_required -from django.shortcuts import render, redirect, get_object_or_404 +from django.shortcuts import render, get_object_or_404 from django.views.decorators.http import require_POST from django.contrib import messages from django.contrib.auth.views import redirect_to_login @@ -10,8 +10,9 @@ from djconfig import config -from ...core import utils -from ...core.utils.paginator import yt_paginate +from spirit.core.utils.http import safe_redirect +from spirit.core import utils +from spirit.core.utils.paginator import yt_paginate from .models import CommentPoll, CommentPollChoice, CommentPollVote from .forms import PollVoteManyForm @@ -35,7 +36,7 @@ def close_or_open(request, pk, close=True): .filter(pk=poll.pk) .update(close_at=close_at)) - return redirect(request.GET.get('next', poll.get_absolute_url())) + return safe_redirect(request, 'next', poll.get_absolute_url()) @require_POST @@ -55,10 +56,10 @@ def vote(request, pk): CommentPollChoice.decrease_vote_count(poll=poll, voter=request.user) form.save_m2m() CommentPollChoice.increase_vote_count(poll=poll, voter=request.user) - return redirect(request.POST.get('next', poll.get_absolute_url())) + return safe_redirect(request, 'next', poll.get_absolute_url(), method='POST') messages.error(request, utils.render_form_errors(form)) - return redirect(request.POST.get('next', poll.get_absolute_url())) + return safe_redirect(request, 'next', poll.get_absolute_url(), method='POST') @login_required diff --git a/spirit/comment/views.py b/spirit/comment/views.py index e7dff724e..232b0e8c5 100644 --- a/spirit/comment/views.py +++ b/spirit/comment/views.py @@ -8,6 +8,7 @@ from djconfig import config +from spirit.core.utils.http import safe_redirect from spirit.core.utils.views import is_post, post_data, is_ajax from spirit.core.utils.ratelimit.decorators import ratelimit from spirit.core.utils.decorators import moderator_required @@ -41,15 +42,14 @@ def publish(request, topic_id, pk=None): if is_post(request) and not request.is_limited() and form.is_valid(): if not user.st.update_post_hash(form.get_comment_hash()): # Hashed comment may have not been saved yet - return redirect( - request.POST.get('next', None) or - Comment + default_url = lambda: (Comment .get_last_for_topic(topic_id) .get_absolute_url()) + return safe_redirect(request, 'next', default_url, method='POST') comment = form.save() comment_posted(comment=comment, mentions=form.mentions) - return redirect(request.POST.get('next', comment.get_absolute_url())) + return safe_redirect(request, 'next', comment.get_absolute_url(), method='POST') return render( request=request, @@ -67,7 +67,7 @@ def update(request, pk): pre_comment_update(comment=Comment.objects.get(pk=comment.pk)) comment = form.save() post_comment_update(comment=comment) - return redirect(request.POST.get('next', comment.get_absolute_url())) + return safe_redirect(request, 'next', comment.get_absolute_url(), method='POST') return render( request=request, template_name='spirit/comment/update.html', @@ -81,7 +81,7 @@ def delete(request, pk, remove=True): (Comment.objects .filter(pk=pk) .update(is_removed=remove)) - return redirect(request.GET.get('next', comment.get_absolute_url())) + return safe_redirect(request, 'next', comment.get_absolute_url()) return render( request=request, template_name='spirit/comment/moderate.html', @@ -104,7 +104,7 @@ def move(request, topic_id): else: messages.error(request, render_form_errors(form)) - return redirect(request.POST.get('next', topic.get_absolute_url())) + return safe_redirect(request, 'next', topic.get_absolute_url(), method='POST') def find(request, pk): diff --git a/spirit/core/utils/decorators.py b/spirit/core/utils/decorators.py index 5c68224e3..425bc9d06 100644 --- a/spirit/core/utils/decorators.py +++ b/spirit/core/utils/decorators.py @@ -4,9 +4,9 @@ from django.core.exceptions import PermissionDenied from django.contrib.auth.views import redirect_to_login -from django.shortcuts import redirect from spirit.core.conf import settings +from spirit.core.utils.http import safe_redirect def moderator_required(view_func): @@ -48,7 +48,7 @@ def guest_only(view_func): @wraps(view_func) def wrapper(request, *args, **kwargs): if request.user.is_authenticated: - return redirect(request.GET.get('next', request.user.st.get_absolute_url())) + return safe_redirect(request, 'next', request.user.st.get_absolute_url()) return view_func(request, *args, **kwargs) diff --git a/spirit/core/utils/http.py b/spirit/core/utils/http.py new file mode 100644 index 000000000..fb3626fa0 --- /dev/null +++ b/spirit/core/utils/http.py @@ -0,0 +1,29 @@ +# -*- coding: utf-8 -*- + +from django.shortcuts import redirect +from django.utils.encoding import iri_to_uri + +try: + from django.utils.http import url_has_allowed_host_and_scheme +except ImportError: + from django.utils.http import is_safe_url as url_has_allowed_host_and_scheme + + +def _resolve_lazy_url(url): + if callable(url): + return url() + return url + + +def safe_redirect(request, key, default_url='', method='GET'): + next = ( + getattr(request, method).get(key, None) or + _resolve_lazy_url(default_url) + ) + url_is_safe = url_has_allowed_host_and_scheme( + url=next, allowed_hosts=None) + #allowed_hosts=settings.ALLOWED_HOSTS, + #require_https=request.is_secure()) + if url_is_safe: + return redirect(iri_to_uri(next)) + return redirect('/') diff --git a/spirit/topic/favorite/views.py b/spirit/topic/favorite/views.py index f95e7ecaa..04a6241b9 100644 --- a/spirit/topic/favorite/views.py +++ b/spirit/topic/favorite/views.py @@ -2,14 +2,14 @@ from django.contrib.auth.decorators import login_required from django.shortcuts import get_object_or_404 -from django.shortcuts import redirect from django.views.decorators.http import require_POST from django.contrib import messages from .models import TopicFavorite from .forms import FavoriteForm from ..models import Topic -from ...core import utils +from spirit.core import utils +from spirit.core.utils.http import safe_redirect @require_POST @@ -23,7 +23,7 @@ def create(request, topic_id): else: messages.error(request, utils.render_form_errors(form)) - return redirect(request.POST.get('next', topic.get_absolute_url())) + return safe_redirect(request, 'next', topic.get_absolute_url(), method='POST') @require_POST @@ -31,4 +31,4 @@ def create(request, topic_id): def delete(request, pk): favorite = get_object_or_404(TopicFavorite, pk=pk, user=request.user) favorite.delete() - return redirect(request.POST.get('next', favorite.topic.get_absolute_url())) + return safe_redirect(request, 'next', favorite.topic.get_absolute_url(), method='POST') diff --git a/spirit/topic/moderate/views.py b/spirit/topic/moderate/views.py index 1502f2d31..752ec99f6 100644 --- a/spirit/topic/moderate/views.py +++ b/spirit/topic/moderate/views.py @@ -1,10 +1,11 @@ # -*- coding: utf-8 -*- from django.utils import timezone -from django.shortcuts import render, redirect, get_object_or_404 +from django.shortcuts import render, get_object_or_404 from django.contrib import messages from django.utils.translation import gettext as _ +from spirit.core.utils.http import safe_redirect from spirit.core.utils.views import is_post from spirit.core.utils.decorators import moderator_required from spirit.comment.models import Comment @@ -33,8 +34,7 @@ def _moderate(request, pk, field_name, to_value, action=None, message=None): if message is not None: messages.info(request, message) - return redirect(request.POST.get( - 'next', topic.get_absolute_url())) + return safe_redirect(request, 'next', topic.get_absolute_url(), method='POST') return render( request=request, diff --git a/spirit/topic/notification/views.py b/spirit/topic/notification/views.py index 7ed189d6b..da8673ea0 100644 --- a/spirit/topic/notification/views.py +++ b/spirit/topic/notification/views.py @@ -3,7 +3,7 @@ import json from django.contrib.auth.decorators import login_required -from django.shortcuts import render, redirect, get_object_or_404 +from django.shortcuts import render, get_object_or_404 from django.views.decorators.http import require_POST from django.http import Http404, HttpResponse from django.contrib import messages @@ -18,6 +18,7 @@ from spirit.core.utils.paginator import yt_paginate from spirit.core.utils.paginator.infinite_paginator import paginate from spirit.core.utils.views import is_ajax +from spirit.core.utils.http import safe_redirect from spirit.topic.models import Topic from .models import TopicNotification from .forms import NotificationForm, NotificationCreationForm @@ -39,7 +40,7 @@ def create(request, topic_id): else: messages.error(request, utils.render_form_errors(form)) - return redirect(request.POST.get('next', topic.get_absolute_url())) + return safe_redirect(request, 'next', topic.get_absolute_url(), method='POST') @require_POST @@ -53,8 +54,8 @@ def update(request, pk): else: messages.error(request, utils.render_form_errors(form)) - return redirect(request.POST.get( - 'next', notification.topic.get_absolute_url())) + return safe_redirect( + request, 'next', notification.topic.get_absolute_url(), method='POST') @login_required @@ -124,5 +125,5 @@ def mark_all_as_read(request): .for_access(request.user) .filter(is_read=False) .update(is_read=True)) - return redirect(request.POST.get( - 'next', reverse('spirit:topic:notification:index'))) + return safe_redirect( + request, 'next', reverse('spirit:topic:notification:index'), method='POST') diff --git a/spirit/topic/private/views.py b/spirit/topic/private/views.py index bd0f2a155..4ddd48522 100644 --- a/spirit/topic/private/views.py +++ b/spirit/topic/private/views.py @@ -10,14 +10,15 @@ from djconfig import config -from ...core.conf import settings -from ...core import utils -from ...core.utils.views import is_post, post_data -from ...core.utils.paginator import paginate, yt_paginate -from ...core.utils.ratelimit.decorators import ratelimit -from ...comment.forms import CommentForm -from ...comment.utils import comment_posted -from ...comment.models import Comment +from spirit.core.conf import settings +from spirit.core import utils +from spirit.core.utils.http import safe_redirect +from spirit.core.utils.views import is_post, post_data +from spirit.core.utils.paginator import paginate, yt_paginate +from spirit.core.utils.ratelimit.decorators import ratelimit +from spirit.comment.forms import CommentForm +from spirit.comment.utils import comment_posted +from spirit.comment.models import Comment from ..models import Topic from ..utils import topic_viewed from .utils import notify_access @@ -50,9 +51,8 @@ def publish(request, user_id=None): all([tform.is_valid(), cform.is_valid(), tpform.is_valid()]) and not request.is_limited()): if not user.st.update_post_hash(tform.get_topic_hash()): - return redirect( - request.POST.get('next', None) or - tform.category.get_absolute_url()) + return safe_redirect( + request, 'next', lambda: tform.category.get_absolute_url(), method='POST') # wrap in transaction.atomic? topic = tform.save() @@ -123,7 +123,8 @@ def create_access(request, topic_id): else: messages.error(request, utils.render_form_errors(form)) - return redirect(request.POST.get('next', topic_private.get_absolute_url())) + return safe_redirect( + request, 'next', topic_private.get_absolute_url(), method='POST') @login_required @@ -136,7 +137,8 @@ def delete_access(request, pk): if request.user.pk == topic_private.user_id: return redirect(reverse("spirit:topic:private:index")) - return redirect(request.POST.get('next', topic_private.get_absolute_url())) + return safe_redirect( + request, 'next', topic_private.get_absolute_url(), method='POST') return render( request=request, @@ -160,7 +162,8 @@ def join_in(request, topic_id): if is_post(request) and form.is_valid(): topic_private = form.save() notify_access(user=form.get_user(), topic_private=topic_private) - return redirect(request.POST.get('next', topic.get_absolute_url())) + return safe_redirect( + request, 'next', topic.get_absolute_url(), method='POST') return render( request=request, template_name='spirit/topic/private/join.html', diff --git a/spirit/topic/views.py b/spirit/topic/views.py index cfba1d83e..a2a66fccd 100644 --- a/spirit/topic/views.py +++ b/spirit/topic/views.py @@ -6,6 +6,7 @@ from djconfig import config +from spirit.core.utils.http import safe_redirect from spirit.core.utils.views import is_post, post_data from spirit.core.utils.paginator import paginate, yt_paginate from spirit.core.utils.ratelimit.decorators import ratelimit @@ -38,9 +39,9 @@ def publish(request, category_id=None): all([form.is_valid(), cform.is_valid()]) and not request.is_limited()): if not user.st.update_post_hash(form.get_topic_hash()): - return redirect( - request.POST.get('next', None) or - form.get_category().get_absolute_url()) + default_url = lambda: form.get_category().get_absolute_url() + return safe_redirect( + request, 'next', default_url, method='POST') # wrap in transaction.atomic? topic = form.save() cform.topic = topic @@ -66,7 +67,7 @@ def update(request, pk): if topic.category_id != category_id: Comment.create_moderation_action( user=request.user, topic=topic, action=Comment.MOVED) - return redirect(request.POST.get('next', topic.get_absolute_url())) + return safe_redirect(request,'next', topic.get_absolute_url(), method='POST') return render( request=request, template_name='spirit/topic/update.html', diff --git a/spirit/user/admin/views.py b/spirit/user/admin/views.py index 9e5e4230d..a6cdb0c24 100644 --- a/spirit/user/admin/views.py +++ b/spirit/user/admin/views.py @@ -1,15 +1,16 @@ # -*- coding: utf-8 -*- -from django.shortcuts import render, redirect, get_object_or_404 +from django.shortcuts import render, get_object_or_404 from django.contrib.auth import get_user_model from django.contrib import messages from django.utils.translation import gettext as _ from djconfig import config -from ...core.utils.views import is_post, post_data -from ...core.utils.paginator import yt_paginate -from ...core.utils.decorators import administrator_required +from spirit.core.utils.http import safe_redirect +from spirit.core.utils.views import is_post, post_data +from spirit.core.utils.paginator import yt_paginate +from spirit.core.utils.decorators import administrator_required from .forms import UserForm, UserProfileForm User = get_user_model() @@ -24,7 +25,7 @@ def edit(request, user_id): uform.save() form.save() messages.info(request, _("This profile has been updated!")) - return redirect(request.GET.get("next", request.get_full_path())) + return safe_redirect(request, "next", request.get_full_path()) return render( request=request, template_name='spirit/user/admin/edit.html', diff --git a/spirit/user/auth/tests/tests.py b/spirit/user/auth/tests/tests.py index ba092b469..2c1103ad0 100644 --- a/spirit/user/auth/tests/tests.py +++ b/spirit/user/auth/tests/tests.py @@ -55,6 +55,11 @@ def test_login_redirect(self): response = self.client.get(reverse('spirit:user:auth:login') + '?next=/fakepath/') self.assertRedirects(response, '/fakepath/', status_code=302, target_status_code=404) + def test_login_open_redirect(self): + utils.login(self) + response = self.client.get(reverse('spirit:user:auth:login') + '?next=https%3A%2F%2Fevil.com') + self.assertRedirects(response, '/', status_code=302) + @override_settings(ST_CASE_INSENSITIVE_EMAILS=True) def test_login_email_case_insensitive(self): """ diff --git a/spirit/user/auth/views.py b/spirit/user/auth/views.py index 8a90554f8..514f219ac 100644 --- a/spirit/user/auth/views.py +++ b/spirit/user/auth/views.py @@ -9,6 +9,7 @@ from django.urls import reverse_lazy from spirit.core.conf import settings +from spirit.core.utils.http import safe_redirect from spirit.core.utils.views import is_post, post_data from spirit.core.utils.ratelimit.decorators import ratelimit from spirit.user.utils.email import send_activation_email @@ -62,7 +63,8 @@ class _CustomLoginView(django_views.LoginView): def custom_login(request, **kwargs): # Currently, Django 1.5 login view does not redirect somewhere if the user is logged in if request.user.is_authenticated: - return redirect(request.GET.get('next', request.user.st.get_absolute_url())) + return safe_redirect( + request, 'next', request.user.st.get_absolute_url()) if request.method == "POST" and request.is_limited(): return redirect(request.get_full_path()) @@ -73,7 +75,7 @@ def custom_login(request, **kwargs): # TODO: @login_required ? def custom_logout(request, **kwargs): if not request.user.is_authenticated: - return redirect(request.GET.get('next', reverse(settings.LOGIN_URL))) + return safe_redirect(request, 'next', reverse(settings.LOGIN_URL)) if request.method == 'POST': return _logout_view(request, **kwargs) @@ -93,7 +95,7 @@ def custom_password_reset(request, **kwargs): # TODO: @guest_only def register(request, registration_form=RegistrationForm): if request.user.is_authenticated: - return redirect(request.GET.get('next', reverse('spirit:user:update'))) + return safe_redirect(request, 'next', reverse('spirit:user:update')) form = registration_form(data=post_data(request)) if (is_post(request) and @@ -109,7 +111,7 @@ def register(request, registration_form=RegistrationForm): # TODO: email-less activation # if not settings.REGISTER_EMAIL_ACTIVATION_REQUIRED: # login(request, user) - # return redirect(request.GET.get('next', reverse('spirit:user:update'))) + # return safe_redirect(request, 'next', reverse('spirit:user:update')) return redirect(reverse(settings.LOGIN_URL)) return render( @@ -135,7 +137,7 @@ def registration_activation(request, pk, token): # TODO: @guest_only def resend_activation_email(request): if request.user.is_authenticated: - return redirect(request.GET.get('next', reverse('spirit:user:update'))) + return safe_redirect(request, 'next', reverse('spirit:user:update')) form = ResendActivationForm(data=post_data(request)) if is_post(request):