From eb07b5be0ce7c51938ed9b00bae04ebe9a75110c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Fleschenberg?= Date: Thu, 27 Feb 2020 17:55:29 +0100 Subject: [PATCH] Fixed #15619 -- Deprecated log out via GET requests. Thanks Florian Apolloner for the implementation idea. Co-Authored-By: Mariusz Felisiak --- django/contrib/auth/views.py | 25 +++++++++-- docs/internals/deprecation.txt | 4 ++ docs/releases/4.1.txt | 30 +++++++++++++ docs/topics/auth/default.txt | 14 +++++- tests/auth_tests/test_signals.py | 4 +- tests/auth_tests/test_views.py | 75 ++++++++++++++++++++++---------- 6 files changed, 122 insertions(+), 30 deletions(-) diff --git a/django/contrib/auth/views.py b/django/contrib/auth/views.py index 6de053f492..f86debde00 100644 --- a/django/contrib/auth/views.py +++ b/django/contrib/auth/views.py @@ -1,3 +1,4 @@ +import warnings from urllib.parse import urlparse, urlunparse from django.conf import settings @@ -21,6 +22,7 @@ from django.http import HttpResponseRedirect, QueryDict from django.shortcuts import resolve_url from django.urls import reverse_lazy from django.utils.decorators import method_decorator +from django.utils.deprecation import RemovedInDjango50Warning from django.utils.http import url_has_allowed_host_and_scheme, urlsafe_base64_decode from django.utils.translation import gettext_lazy as _ from django.views.decorators.cache import never_cache @@ -117,23 +119,38 @@ class LogoutView(SuccessURLAllowedHostsMixin, TemplateView): Log out the user and display the 'You are logged out' message. """ + # RemovedInDjango50Warning: when the deprecation ends, remove "get" and + # "head" from http_method_names. + http_method_names = ["get", "head", "post", "options"] next_page = None redirect_field_name = REDIRECT_FIELD_NAME template_name = "registration/logged_out.html" extra_context = None + # RemovedInDjango50Warning: when the deprecation ends, move + # @method_decorator(csrf_protect) from post() to dispatch(). @method_decorator(never_cache) def dispatch(self, request, *args, **kwargs): + if request.method.lower() == "get": + warnings.warn( + "Log out via GET requests is deprecated and will be removed in Django " + "5.0. Use POST requests for logging out.", + RemovedInDjango50Warning, + ) + return super().dispatch(request, *args, **kwargs) + + @method_decorator(csrf_protect) + def post(self, request, *args, **kwargs): + """Logout may be done via POST.""" auth_logout(request) next_page = self.get_next_page() if next_page: # Redirect to this page until the session has been cleared. return HttpResponseRedirect(next_page) - return super().dispatch(request, *args, **kwargs) + return super().get(request, *args, **kwargs) - def post(self, request, *args, **kwargs): - """Logout may be done via POST.""" - return self.get(request, *args, **kwargs) + # RemovedInDjango50Warning. + get = post def get_next_page(self): if self.next_page is not None: diff --git a/docs/internals/deprecation.txt b/docs/internals/deprecation.txt index 2c85eafc85..31eb732297 100644 --- a/docs/internals/deprecation.txt +++ b/docs/internals/deprecation.txt @@ -90,6 +90,10 @@ details on these changes. * ``created=True`` will be required in the signature of ``RemoteUserBackend.configure_user()`` subclasses. +* Support for logging out via ``GET`` requests in the + ``django.contrib.auth.views.LogoutView`` and + ``django.contrib.auth.views.logout_then_login()`` will be removed. + .. _deprecation-removed-in-4.1: 4.1 diff --git a/docs/releases/4.1.txt b/docs/releases/4.1.txt index 39dd2faba9..c7159bd4f2 100644 --- a/docs/releases/4.1.txt +++ b/docs/releases/4.1.txt @@ -446,6 +446,36 @@ Miscellaneous Features deprecated in 4.1 ========================== +Log out via GET +--------------- + +Logging out via ``GET`` requests to the :py:class:`built-in logout view +` is deprecated. Use ``POST`` requests +instead. + +If you want to retain the user experience of an HTML link, you can use a form +that is styled to appear as a link: + +.. code-block:: html + +
+ {% csrf_token %} + +
+ +.. code-block:: css + + #logout-form { + display: inline; + } + #logout-form button { + background: none; + border: none; + cursor: pointer; + padding: 0; + text-decoration: underline; + } + Miscellaneous ------------- diff --git a/docs/topics/auth/default.txt b/docs/topics/auth/default.txt index bef1486cdd..3e319560fb 100644 --- a/docs/topics/auth/default.txt +++ b/docs/topics/auth/default.txt @@ -1160,7 +1160,12 @@ implementation details see :ref:`using-the-views`. .. class:: LogoutView - Logs a user out. + Logs a user out on ``POST`` requests. + + .. deprecated:: 4.1 + + Support for logging out on ``GET`` requests is deprecated and will be + removed in Django 5.0. **URL name:** ``logout`` @@ -1212,7 +1217,7 @@ implementation details see :ref:`using-the-views`. .. function:: logout_then_login(request, login_url=None) - Logs a user out, then redirects to the login page. + Logs a user out on ``POST`` requests, then redirects to the login page. **URL name:** No default URL provided @@ -1221,6 +1226,11 @@ implementation details see :ref:`using-the-views`. * ``login_url``: The URL of the login page to redirect to. Defaults to :setting:`settings.LOGIN_URL ` if not supplied. + .. deprecated:: 4.1 + + Support for logging out on ``GET`` requests is deprecated and will be + removed in Django 5.0. + .. class:: PasswordChangeView **URL name:** ``password_change`` diff --git a/tests/auth_tests/test_signals.py b/tests/auth_tests/test_signals.py index a573fcb9dc..b97377e2c9 100644 --- a/tests/auth_tests/test_signals.py +++ b/tests/auth_tests/test_signals.py @@ -60,13 +60,13 @@ class SignalTestCase(TestCase): def test_logout_anonymous(self): # The log_out function will still trigger the signal for anonymous # users. - self.client.get("/logout/next_page/") + self.client.post("/logout/next_page/") self.assertEqual(len(self.logged_out), 1) self.assertIsNone(self.logged_out[0]) def test_logout(self): self.client.login(username="testclient", password="password") - self.client.get("/logout/next_page/") + self.client.post("/logout/next_page/") self.assertEqual(len(self.logged_out), 1) self.assertEqual(self.logged_out[0].username, "testclient") diff --git a/tests/auth_tests/test_views.py b/tests/auth_tests/test_views.py index e056d0f002..622a40de22 100644 --- a/tests/auth_tests/test_views.py +++ b/tests/auth_tests/test_views.py @@ -29,9 +29,10 @@ from django.core.exceptions import ImproperlyConfigured from django.db import connection from django.http import HttpRequest, HttpResponse from django.middleware.csrf import CsrfViewMiddleware, get_token -from django.test import Client, TestCase, override_settings +from django.test import Client, TestCase, ignore_warnings, override_settings from django.test.client import RedirectCycleError from django.urls import NoReverseMatch, reverse, reverse_lazy +from django.utils.deprecation import RemovedInDjango50Warning from django.utils.http import urlsafe_base64_encode from .client import PasswordResetConfirmClient @@ -538,7 +539,7 @@ class ChangePasswordTest(AuthViewsTestCase): ) def logout(self): - self.client.get("/logout/") + self.client.post("/logout/") def test_password_change_fails_with_invalid_old_password(self): self.login() @@ -979,7 +980,10 @@ class LogoutThenLoginTests(AuthViewsTestCase): def test_default_logout_then_login(self): self.login() req = HttpRequest() - req.method = "GET" + req.method = "POST" + csrf_token = get_token(req) + req.COOKIES[settings.CSRF_COOKIE_NAME] = csrf_token + req.POST = {"csrfmiddlewaretoken": csrf_token} req.session = self.client.session response = logout_then_login(req) self.confirm_logged_out() @@ -988,12 +992,28 @@ class LogoutThenLoginTests(AuthViewsTestCase): def test_logout_then_login_with_custom_login(self): self.login() req = HttpRequest() - req.method = "GET" + req.method = "POST" + csrf_token = get_token(req) + req.COOKIES[settings.CSRF_COOKIE_NAME] = csrf_token + req.POST = {"csrfmiddlewaretoken": csrf_token} req.session = self.client.session response = logout_then_login(req, login_url="/custom/") self.confirm_logged_out() self.assertRedirects(response, "/custom/", fetch_redirect_response=False) + @ignore_warnings(category=RemovedInDjango50Warning) + @override_settings(LOGIN_URL="/login/") + def test_default_logout_then_login_get(self): + self.login() + req = HttpRequest() + req.method = "GET" + req.session = self.client.session + response = logout_then_login(req) + # RemovedInDjango50Warning: When the deprecation ends, replace with + # self.assertEqual(response.status_code, 405) + self.confirm_logged_out() + self.assertRedirects(response, "/login/", fetch_redirect_response=False) + class LoginRedirectAuthenticatedUser(AuthViewsTestCase): dont_redirect_url = "/login/redirect_authenticated_user_default/" @@ -1136,7 +1156,7 @@ class LogoutTest(AuthViewsTestCase): def test_logout_default(self): "Logout without next_page option renders the default template" self.login() - response = self.client.get("/logout/") + response = self.client.post("/logout/") self.assertContains(response, "Logged out") self.confirm_logged_out() @@ -1146,10 +1166,21 @@ class LogoutTest(AuthViewsTestCase): self.assertContains(response, "Logged out") self.confirm_logged_out() + def test_logout_with_get_raises_deprecation_warning(self): + self.login() + msg = ( + "Log out via GET requests is deprecated and will be removed in Django 5.0. " + "Use POST requests for logging out." + ) + with self.assertWarnsMessage(RemovedInDjango50Warning, msg): + response = self.client.get("/logout/") + self.assertContains(response, "Logged out") + self.confirm_logged_out() + def test_14377(self): # Bug 14377 self.login() - response = self.client.get("/logout/") + response = self.client.post("/logout/") self.assertIn("site", response.context) def test_logout_doesnt_cache(self): @@ -1157,16 +1188,16 @@ class LogoutTest(AuthViewsTestCase): The logout() view should send "no-cache" headers for reasons described in #25490. """ - response = self.client.get("/logout/") + response = self.client.post("/logout/") self.assertIn("no-store", response.headers["Cache-Control"]) def test_logout_with_overridden_redirect_url(self): # Bug 11223 self.login() - response = self.client.get("/logout/next_page/") + response = self.client.post("/logout/next_page/") self.assertRedirects(response, "/somewhere/", fetch_redirect_response=False) - response = self.client.get("/logout/next_page/?next=/login/") + response = self.client.post("/logout/next_page/?next=/login/") self.assertRedirects(response, "/login/", fetch_redirect_response=False) self.confirm_logged_out() @@ -1174,28 +1205,28 @@ class LogoutTest(AuthViewsTestCase): def test_logout_with_next_page_specified(self): "Logout with next_page option given redirects to specified resource" self.login() - response = self.client.get("/logout/next_page/") + response = self.client.post("/logout/next_page/") self.assertRedirects(response, "/somewhere/", fetch_redirect_response=False) self.confirm_logged_out() def test_logout_with_redirect_argument(self): "Logout with query string redirects to specified resource" self.login() - response = self.client.get("/logout/?next=/login/") + response = self.client.post("/logout/?next=/login/") self.assertRedirects(response, "/login/", fetch_redirect_response=False) self.confirm_logged_out() def test_logout_with_custom_redirect_argument(self): "Logout with custom query string redirects to specified resource" self.login() - response = self.client.get("/logout/custom_query/?follow=/somewhere/") + response = self.client.post("/logout/custom_query/?follow=/somewhere/") self.assertRedirects(response, "/somewhere/", fetch_redirect_response=False) self.confirm_logged_out() def test_logout_with_named_redirect(self): "Logout resolves names or URLs passed as next_page." self.login() - response = self.client.get("/logout/next_page/named/") + response = self.client.post("/logout/next_page/named/") self.assertRedirects( response, "/password_reset/", fetch_redirect_response=False ) @@ -1203,7 +1234,7 @@ class LogoutTest(AuthViewsTestCase): def test_success_url_allowed_hosts_same_host(self): self.login() - response = self.client.get("/logout/allowed_hosts/?next=https://testserver/") + response = self.client.post("/logout/allowed_hosts/?next=https://testserver/") self.assertRedirects( response, "https://testserver/", fetch_redirect_response=False ) @@ -1211,7 +1242,7 @@ class LogoutTest(AuthViewsTestCase): def test_success_url_allowed_hosts_safe_host(self): self.login() - response = self.client.get("/logout/allowed_hosts/?next=https://otherserver/") + response = self.client.post("/logout/allowed_hosts/?next=https://otherserver/") self.assertRedirects( response, "https://otherserver/", fetch_redirect_response=False ) @@ -1219,7 +1250,7 @@ class LogoutTest(AuthViewsTestCase): def test_success_url_allowed_hosts_unsafe_host(self): self.login() - response = self.client.get("/logout/allowed_hosts/?next=https://evil/") + response = self.client.post("/logout/allowed_hosts/?next=https://evil/") self.assertRedirects( response, "/logout/allowed_hosts/", fetch_redirect_response=False ) @@ -1246,7 +1277,7 @@ class LogoutTest(AuthViewsTestCase): "bad_url": quote(bad_url), } self.login() - response = self.client.get(nasty_url) + response = self.client.post(nasty_url) self.assertEqual(response.status_code, 302) self.assertNotIn( bad_url, response.url, "%s should be blocked" % bad_url @@ -1272,7 +1303,7 @@ class LogoutTest(AuthViewsTestCase): "good_url": quote(good_url), } self.login() - response = self.client.get(safe_url) + response = self.client.post(safe_url) self.assertEqual(response.status_code, 302) self.assertIn(good_url, response.url, "%s should be allowed" % good_url) self.confirm_logged_out() @@ -1286,7 +1317,7 @@ class LogoutTest(AuthViewsTestCase): "next_url": quote(non_https_next_url), } self.login() - response = self.client.get(url, secure=True) + response = self.client.post(url, secure=True) self.assertRedirects(response, logout_url, fetch_redirect_response=False) self.confirm_logged_out() @@ -1295,19 +1326,19 @@ class LogoutTest(AuthViewsTestCase): self.login() self.client.post("/setlang/", {"language": "pl"}) self.assertEqual(self.client.cookies[settings.LANGUAGE_COOKIE_NAME].value, "pl") - self.client.get("/logout/") + self.client.post("/logout/") self.assertEqual(self.client.cookies[settings.LANGUAGE_COOKIE_NAME].value, "pl") @override_settings(LOGOUT_REDIRECT_URL="/custom/") def test_logout_redirect_url_setting(self): self.login() - response = self.client.get("/logout/") + response = self.client.post("/logout/") self.assertRedirects(response, "/custom/", fetch_redirect_response=False) @override_settings(LOGOUT_REDIRECT_URL="logout") def test_logout_redirect_url_named_setting(self): self.login() - response = self.client.get("/logout/") + response = self.client.post("/logout/") self.assertRedirects(response, "/logout/", fetch_redirect_response=False)