diff --git a/core/urls.py b/core/urls.py index 2c3a66ff..96bdd7ad 100644 --- a/core/urls.py +++ b/core/urls.py @@ -1,6 +1,5 @@ -from django.conf import settings from django.contrib.auth.views import ( - LogoutView, PasswordResetCompleteView, PasswordResetDoneView, + PasswordResetCompleteView, PasswordResetDoneView, ) from django.urls import include, path, re_path from django.utils.translation import pgettext_lazy @@ -9,7 +8,8 @@ from .forms import SystemPasswordResetForm, SystemPasswordResetRequestForm from .views import ( # isort:skip - HomeView, RegisterView, LoginView, AccountRestoreRequestView, + HomeView, + RegisterView, LoginView, AccountRestoreRequestView, LogoutView, PasswordResetView, PasswordResetConfirmView, UsernameRemindView, UsernameRemindDoneView, AgreementView, AgreementRejectView, @@ -36,10 +36,7 @@ AccountRestoreRequestView.as_view(), name='login_restore'), path( pgettext_lazy("URL", 'logout/'), - LogoutView.as_view( - redirect_field_name=settings.REDIRECT_FIELD_NAME, - ), - name='logout'), + LogoutView.as_view(), name='logout'), path( pgettext_lazy("URL", 'agreement/'), include([ diff --git a/core/views.py b/core/views.py index 8144e15e..c6e6f5f8 100644 --- a/core/views.py +++ b/core/views.py @@ -9,7 +9,7 @@ from django.contrib import messages from django.contrib.auth import authenticate, get_user_model, login, logout from django.contrib.auth.views import ( - LoginView as LoginBuiltinView, + LoginView as LoginBuiltinView, LogoutView as LogoutBuiltinView, PasswordChangeDoneView as PasswordChangeDoneBuiltinView, PasswordChangeView as PasswordChangeBuiltinView, PasswordResetConfirmView as PasswordResetConfirmBuiltinView, @@ -128,6 +128,42 @@ def get_redirect_url(self): else: return redirect_to + def render_to_response(self, context, **response_kwargs) -> HttpResponse: + # The view is rendered, which means that the user is not logged in. + response = super().render_to_response(context, **response_kwargs) + response.delete_cookie( + 'seen_at', + domain=settings.SESSION_COOKIE_DOMAIN, path=settings.SESSION_COOKIE_PATH, + ) + return response + + def form_valid(self, form) -> HttpResponse: + # User successfully logged in. + response = super().form_valid(form) + response.set_cookie( + 'seen_at', str(int(datetime.now().timestamp())), + max_age=( + settings.SESSION_COOKIE_AGE + if not settings.SESSION_EXPIRE_AT_BROWSER_CLOSE + else None + ), + domain=settings.SESSION_COOKIE_DOMAIN, path=settings.SESSION_COOKIE_PATH, + secure=settings.SESSION_COOKIE_SECURE, httponly=True, samesite='Lax', + ) + return response + + +class LogoutView(LogoutBuiltinView): + redirect_field_name = settings.REDIRECT_FIELD_NAME + + def dispatch(self, request, *args, **kwargs): + response = super().dispatch(request, *args, **kwargs) + response.delete_cookie( + 'seen_at', + domain=settings.SESSION_COOKIE_DOMAIN, path=settings.SESSION_COOKIE_PATH, + ) + return response + class RegisterView(generic.CreateView): model = User diff --git a/pasportaservo/settings/sentry.py b/pasportaservo/settings/sentry.py index fd35a7a3..a0c6d8c6 100644 --- a/pasportaservo/settings/sentry.py +++ b/pasportaservo/settings/sentry.py @@ -38,12 +38,13 @@ class TracesSampler: + r'|facebookexternalhit|feedfetcher|feedburner', re.IGNORECASE ) + authenticated_user_cookie_re = re.compile(r'\bseen_at=[A-Za-z0-9]') def __call__(self, sampling_context: SamplingContext) -> float: if sampling_context['parent_sampled'] is not None: return sampling_context['parent_sampled'] - request_info = ( + request_info: tuple[str, str] = ( sampling_context['wsgi_environ'].get('PATH_INFO', ''), sampling_context['wsgi_environ'].get('REQUEST_METHOD'), ) @@ -51,12 +52,17 @@ def __call__(self, sampling_context: SamplingContext) -> float: bot_match = re.search( self.robot_crawler_re, sampling_context['wsgi_environ'].get('HTTP_USER_AGENT', '')) - if bot_match or request_info[0].startswith((STATIC_URL, MEDIA_URL)): + if ( + bot_match + or request_info[0].startswith((STATIC_URL, MEDIA_URL)) + or not request_info[0].endswith('/') + ): return 0 if not hasattr(self, 'paths'): self.configure_paths() sampling_rate = 0 + reduced_sampling_if_anonymous = False match request_info: case (path, method) \ @@ -65,13 +71,22 @@ def __call__(self, sampling_context: SamplingContext) -> float: case (path, _) \ if path.startswith(self.paths['exploration']): sampling_rate = 0.75 + reduced_sampling_if_anonymous = True case (path, method) \ if path.startswith(self.paths['interesting']): sampling_rate = 0.50 if method == 'POST' else 0.40 + reduced_sampling_if_anonymous = True case (path, _) \ if path.startswith(self.paths['action_link']): sampling_rate = 1.00 + if reduced_sampling_if_anonymous and sampling_rate > 0: + user_match = re.search( + self.authenticated_user_cookie_re, + sampling_context['wsgi_environ'].get('HTTP_COOKIE', '')) + if not user_match: + sampling_rate *= 0.25 + return sampling_rate def trim_path(self, path: str) -> str: diff --git a/tests/views/mixins.py b/tests/views/mixins.py index fd64edbc..519df614 100644 --- a/tests/views/mixins.py +++ b/tests/views/mixins.py @@ -18,6 +18,7 @@ def test_view_form(self): self, user=self.user if self.view_page.redirects_unauthenticated else None, reuse_for_lang=lang) + assert 'object' in page.form # Verify that the expected form class is in use on the view. self.assertIsNotNone(page.form['object']) self.assertIsInstance(page.form['object'], page.form_class) diff --git a/tests/views/pages/__init__.py b/tests/views/pages/__init__.py index ae0e369c..a31484d8 100644 --- a/tests/views/pages/__init__.py +++ b/tests/views/pages/__init__.py @@ -1,4 +1,4 @@ -from .account import AccountSettingsPage, RegisterPage +from .account import AccountSettingsPage, LoginPage, RegisterPage from .admin import MassMailPage, MassMailResultPage from .general import ( AboutPage, FAQPage, HomePage, OkayPage, PrivacyPage, TCPage, diff --git a/tests/views/pages/account.py b/tests/views/pages/account.py index 66018e3a..58d39f08 100644 --- a/tests/views/pages/account.py +++ b/tests/views/pages/account.py @@ -5,8 +5,8 @@ from lxml.html import HtmlElement from pyquery import PyQuery -from core.forms import UserRegistrationForm -from core.views import AccountSettingsView, RegisterView +from core.forms import UserAuthenticationForm, UserRegistrationForm +from core.views import AccountSettingsView, LoginView, RegisterView from .base import PageTemplate, PageWithFormTemplate @@ -35,6 +35,30 @@ class RegisterPage(PageWithFormTemplate): } +class LoginPage(PageWithFormTemplate): + view_class = LoginView + form_class = UserAuthenticationForm + url = reverse_lazy('login') + explicit_url = { + 'en': '/login/', + 'eo': '/ensaluti/', + } + template = 'registration/login.html' + title = { + 'en': "Log in & Find accommodation | Pasporta Servo", + 'eo': "Ensalutu & Trovu loĝejon | Pasporta Servo", + } + redirects_unauthenticated = False + redirects_logged_in = True + form = { + 'selector': ".login", + 'title': { + 'en': "Log In", + 'eo': "Ensaluto", + }, + } + + class AccountSettingsPage(PageTemplate): view_class = AccountSettingsView url = reverse_lazy('account_settings') diff --git a/tests/views/pages/base.py b/tests/views/pages/base.py index 173f22ec..09ea195b 100644 --- a/tests/views/pages/base.py +++ b/tests/views/pages/base.py @@ -222,7 +222,7 @@ class _RenderedFormDefinitionBase(TypedDict): title: dict[str, str] class RenderedFormDefinition(_RenderedFormDefinitionBase, total=False): - object: type[Form] | type[ModelForm] | None + object: Form | ModelForm | None form_class: type[Form] | type[ModelForm] form: RenderedFormDefinition = { diff --git a/tests/views/test_session_views.py b/tests/views/test_session_views.py index d78a7f74..ae94da33 100644 --- a/tests/views/test_session_views.py +++ b/tests/views/test_session_views.py @@ -7,90 +7,15 @@ from django_webtest import WebTest -from core.forms import UserAuthenticationForm - -from ..assertions import AdditionalAsserts from ..factories import UserFactory +from .mixins import FormViewTestsMixin +from .pages import LoginPage +from .testcasebase import BasicViewTests @tag('views', 'views-session', 'views-login') -class LoginViewTests(AdditionalAsserts, WebTest): - @classmethod - def setUpTestData(cls): - cls.url = reverse_lazy('login') - cls.user = UserFactory(profile=None) - - def test_view_url(self): - # Verify that the view can be found at the expected URL. - response = self.app.get(self.url, status='*') - self.assertEqual(response.status_code, 200) - - test_data = { - 'en': '/login/', - 'eo': '/ensaluti/', - } - for lang, expected_url in test_data.items(): - with ( - override_settings(LANGUAGE_CODE=lang), - self.subTest(lang=lang, attempted=expected_url) - ): - response = self.app.get(expected_url, status='*') - self.assertEqual(response.status_code, 200) - - def test_view_title(self): - # Verify that the view has the expected element. - test_data = { - 'en': "Log in & Find accommodation | Pasporta Servo", - 'eo': "Ensalutu & Trovu loĝejon | Pasporta Servo", - } - for lang in test_data: - with ( - override_settings(LANGUAGE_CODE=lang), - self.subTest(lang=lang) - ): - page = self.app.get(self.url, status=200) - self.assertHTMLEqual(page.pyquery("title").html(), test_data[lang]) - - def test_view_header(self): - # The view's header is expected to have "login" and "register" links, - # no username or link to profile, and no use notice. - test_data = { - 'en': {'session': "log in", 'profile': "register"}, - 'eo': {'session': "ensaluti", 'profile': "registriĝi"}, - } - for lang in test_data: - with ( - override_settings(LANGUAGE_CODE=lang), - self.subTest(lang=lang) - ): - page = self.app.get(self.url, status=200) - self.assertEqual( - page.pyquery("header .nav-session").text(), - test_data[lang]['session'] - ) - self.assertEqual( - page.pyquery("header .nav-profile").text(), - test_data[lang]['profile'] - ) - self.assertEqual(page.pyquery("header .use-notice").text(), "") - - def test_view_template(self): - page = self.app.get(self.url, status=200) - self.assertTemplateUsed(page, 'registration/login.html') - - def test_login_form(self): - # Verify that the expected form class is in use on the view. - with override_settings(LANGUAGE_CODE='en'): - page = self.app.get(self.url, status=200) - self.assertTrue('form' in page.context) - self.assertIsInstance(page.context['form'], UserAuthenticationForm) - - # The form is expected to be titled "Log In". - form_title_selector = ".base-form.login > h4" - self.assertEqual(page.pyquery(form_title_selector).text(), "Log In") - with override_settings(LANGUAGE_CODE='eo'): - page = self.app.get(self.url, status=200) - self.assertEqual(page.pyquery(form_title_selector).text(), "Ensaluto") +class LoginViewTests(FormViewTestsMixin, BasicViewTests): + view_page = LoginPage def test_recovery_options(self): # The view is expected to provide recovery / reminder options to the @@ -105,8 +30,8 @@ def test_recovery_options(self): override_settings(LANGUAGE_CODE=lang), self.subTest(lang=lang) ): - page = self.app.get(self.url, status=200) - recovery_elem = page.pyquery(".base-form.login form .recovery") + page = self.view_page.open(self, reuse_for_lang=lang) + recovery_elem = page.get_form_element(".recovery") self.assertEqual(recovery_elem.text(), test_data[lang]) recovery_option_elems = recovery_elem.children("a") for i, expected_recovery_option_url in enumerate(expected_urls): @@ -122,17 +47,21 @@ def incorrect_credentials_tests(self, expected_error): # are supplied. # Django mitigates timing-related user enumeration attacks – therefore # we don't need to test for that. - page = self.app.get(self.url, status=200) + page = self.view_page.open(self) for username in (uuid4(), self.user.username): with self.subTest(username=username): - page.form['username'] = username - page.form['password'] = "wrong-password" - page = page.form.submit() - self.assertEqual(page.status_code, 200) - self.assertTrue('form' in page.context) - self.assertGreaterEqual(len(page.context['form'].non_field_errors()), 1) + page.submit({ + 'username': username, + 'password': "wrong-password", + }) + self.assertEqual(page.response.status_code, 200) + self.assertTrue('object' in page.form) + assert 'object' in page.form + self.assertIsNotNone(page.form['object']) + assert page.form['object'] is not None + self.assertGreaterEqual(len(page.form['object'].non_field_errors()), 1) self.assertStartsWith( - page.context['form'].non_field_errors()[0], + page.form['object'].non_field_errors()[0], expected_error ) @@ -150,96 +79,73 @@ def test_incorrect_credentials(self): "Bonvole enigu ĝustajn salutnomon kaj pasvorton.") def test_correct_credentials(self): - # The view is expected to redirect to the home page when the correct - # credentials are supplied. - page = self.app.get(self.url, status=200) test_data = { 'username': self.user.username, 'password': "adm1n", } - page = self.app.post( - self.url, - { - **test_data, - 'csrfmiddlewaretoken': page.context['csrf_token'], - }, - status='*') - self.assertEqual(page.status_code, 302) - self.assertEqual(page.location, '/') - page = page.follow() + + # The view is expected to redirect to the home page when the correct + # credentials are supplied. + page = self.view_page.open(self) + page.submit(test_data) + self.assertEqual(page.response.status_code, 302) + self.assertEqual(page.response.location, '/') + page.follow() # Verify that the user is now authenticated. - self.assertTrue('user' in page.context) - self.assertEqual(page.context['user'], self.user) + self.assertEqual(page.get_user(), self.user) # The view is expected to redirect to the provided destination when # the correct credentials are supplied and the next page parameter # is included. - self.app.reset() - page = self.app.get(self.url, status=200) - page = self.app.post( - self.url, - { - **test_data, - 'csrfmiddlewaretoken': page.context['csrf_token'], - settings.REDIRECT_FIELD_NAME: '/some/where/else/', - }, - status='*') - self.assertEqual(page.status_code, 302) - self.assertEqual(page.location, '/some/where/else/') + page = self.view_page.open(self) + page.submit(test_data, redirect_to='/some/where/else/') + self.assertEqual(page.response.status_code, 302) + self.assertEqual(page.response.location, '/some/where/else/') # Verify fallback support for third-party libraries that do not use # the customized next page parameter's name. The view is expected to # redirect to the destination provided in the 'next' parameter. - self.app.reset() - page = self.app.get(self.url, status=200) - page = self.app.post( - self.url, - { - **test_data, - 'csrfmiddlewaretoken': page.context['csrf_token'], - 'next': '/there-and-beyond', - }, - status='*') - self.assertEqual(page.status_code, 302) - self.assertEqual(page.location, '/there-and-beyond') + page = self.view_page.open(self) + page.submit(test_data | {'next': '/there-and-beyond'}) + self.assertEqual(page.response.status_code, 302) + self.assertEqual(page.response.location, '/there-and-beyond') def test_redirect_if_logged_in(self): # The view is expected to redirect to the home page when the user is # already authenticated. - page = self.app.get(self.url, user=self.user) - self.assertEqual(page.status_code, 302) - self.assertEqual(page.location, '/') + page = self.view_page.open(self, status='*', user=self.user) + self.assertEqual(page.response.status_code, 302) + self.assertEqual(page.response.location, '/') # The view is expected to redirect to the provided destination when # the user is already authenticated and the next page parameter is # included. - page = self.app.get( - f'{self.url}?{settings.REDIRECT_FIELD_NAME}=/some/where/else/', - user=self.user) - self.assertEqual(page.status_code, 302) - self.assertEqual(page.location, '/some/where/else/') + page = self.view_page.open( + self, status='*', user=self.user, redirect_to='/some/where/else/') + self.assertEqual(page.response.status_code, 302) + self.assertEqual(page.response.location, '/some/where/else/') # Verify fallback support for third-party libraries that do not use # the customized next page parameter's name. The view is expected to # redirect to the destination provided in the 'next' parameter. - page = self.app.get( - f'{self.url}?next=/there-and-beyond', - user=self.user) - self.assertEqual(page.status_code, 302) - self.assertEqual(page.location, '/there-and-beyond') + page = self.view_page.open( + self, status='*', user=self.user, + extra_params={'next': '/there-and-beyond'}) + self.assertEqual(page.response.status_code, 302) + self.assertEqual(page.response.location, '/there-and-beyond') # The provided destination is expected to be discarded when it is # not within the website. - page = self.app.get( - f'{self.url}?{settings.REDIRECT_FIELD_NAME}=https://far.away/', - user=self.user) - self.assertEqual(page.status_code, 302) - self.assertEqual(page.location, '/') + page = self.view_page.open( + self, status='*', user=self.user, redirect_to='https://far.away/') + self.assertEqual(page.response.status_code, 302) + self.assertEqual(page.response.location, '/') def test_redirect_loop(self): + base_login_page_url = str(self.url) login_page_urls = [ - self.url.rstrip('/'), - self.url + ('/' if self.url[-1] != '/' else ''), + base_login_page_url.rstrip('/'), + base_login_page_url + ('/' if base_login_page_url[-1] != '/' else ''), ] # The view is expected to redirect to the default destination (the @@ -249,12 +155,11 @@ def test_redirect_loop(self): with self.subTest(redirect_url=redirect_to, user="authenticated"): with self.assertNotRaises(Exception): self.app.reset() - page = self.app.get( - f'{self.url}?{settings.REDIRECT_FIELD_NAME}={redirect_to}', - user=self.user, - auto_follow=True) - self.assertEqual(page.status_code, 200) - self.assertEqual(page.request.path, '/') + page = self.view_page.open( + self, status='*', user=self.user, redirect_to=redirect_to) + page.follow() + self.assertEqual(page.response.status_code, 200) + self.assertEqual(page.response.request.path, '/') # The view is expected to redirect to the default destination (the # home page) when an anonymous user successfully authenticates and @@ -262,20 +167,17 @@ def test_redirect_loop(self): for redirect_to in login_page_urls: with self.subTest(redirect_url=redirect_to, user="anonymous"): self.app.reset() - page = self.app.get( - f'{self.url}?{settings.REDIRECT_FIELD_NAME}={redirect_to}') - self.assertEqual(page.status_code, 200) + page = self.view_page.open( + self, status='*', redirect_to=redirect_to) + self.assertEqual(page.response.status_code, 200) with self.assertNotRaises(Exception): - page = self.app.post( - page.request.url, - { - 'username': self.user.username, - 'password': "adm1n", - 'csrfmiddlewaretoken': page.context['csrf_token'], - }) - page = page.maybe_follow() - self.assertEqual(page.status_code, 200) - self.assertEqual(page.request.path, '/') + page.submit({ + 'username': self.user.username, + 'password': "adm1n", + }) + page.follow() + self.assertEqual(page.response.status_code, 200) + self.assertEqual(page.response.request.path, '/') @override_settings(LANGUAGE_CODE='en') def test_user_with_deprecated_hash(self): @@ -286,15 +188,19 @@ def test_user_with_deprecated_hash(self): 'prepend': 'django.contrib.auth.hashers.MD5PasswordHasher', }): user = UserFactory(profile=None, password="madeIn=2009") - page = self.app.get(self.url, status=200) - page.form['username'] = user.username - page.form['password'] = "madeIn=2009" - page = page.form.submit() - self.assertEqual(page.status_code, 200) - self.assertTrue('form' in page.context) - self.assertGreaterEqual(len(page.context['form'].non_field_errors()), 1) + page = self.view_page.open(self) + page.submit({ + 'username': user.username, + 'password': "madeIn=2009", + }) + self.assertEqual(page.response.status_code, 200) + self.assertTrue('object' in page.form) + assert 'object' in page.form + self.assertIsNotNone(page.form['object']) + assert page.form['object'] is not None + self.assertGreaterEqual(len(page.form['object'].non_field_errors()), 1) self.assertStartsWith( - page.context['form'].non_field_errors()[0], + page.form['object'].non_field_errors()[0], "Please enter the correct username and password." ) @@ -302,28 +208,32 @@ def inactive_user_tests(self, inactive_user, expected_errors): # A user who supplied the correct credentials but whose account was # deactivated, is expected to see the appropriate notification and # to be denied login. - page = self.app.get(self.url, status=200) - page.form['username'] = inactive_user.username - page.form['password'] = "adm1n" - page = page.form.submit() - self.assertEqual(page.status_code, 200) - self.assertTrue('form' in page.context) - self.assertLength(page.context['form'].non_field_errors(), 2) + page = self.view_page.open(self) + page.submit({ + 'username': inactive_user.username, + 'password': "adm1n", + }) + self.assertEqual(page.response.status_code, 200) + self.assertTrue('object' in page.form) + assert 'object' in page.form + self.assertIsNotNone(page.form['object']) + assert page.form['object'] is not None + self.assertLength(page.form['object'].non_field_errors(), 2) self.assertEqual( - page.context['form'].non_field_errors()[0], + page.form['object'].non_field_errors()[0], expected_errors[0] ) self.assertStartsWith( - page.context['form'].non_field_errors()[1], + page.form['object'].non_field_errors()[1], expected_errors[1] ) self.assertIn( expected_errors[2], - page.context['form'].non_field_errors()[1] + page.form['object'].non_field_errors()[1] ) notification_link_target = [ elem.attr("href") - for elem in page.pyquery(".base-form.login form .alert > a").items() + for elem in page.get_form_element(".alert > a").items() if elem.text() == expected_errors[2] ] self.assertLength(notification_link_target, 1) @@ -356,6 +266,24 @@ def test_inactive_user(self): ) ) + def test_custom_cookies(self): + # When a non-authenticated user accesses the login view, + # they are expected to have no `seen_at` cookie. + page = self.view_page.open(self) + self.assertNotIn('seen_at', self.app.cookies) + + # After successful authentication, the user is expected + # to be issued with a valid `seen_at` cookie. + page.submit({ + 'username': self.user.username, + 'password': "adm1n", + }) + cookies = self.app.cookies + self.assertIn('seen_at', cookies) + self.assertIsNotNone(cookies['seen_at']) + assert cookies['seen_at'] is not None + self.assertTrue(cookies['seen_at'].isdigit()) + @tag('views', 'views-session', 'views-logout') class LogoutViewTests(WebTest): @@ -439,9 +367,10 @@ def test_redirect_logged_in_user(self): self.redirect_tests(user=self.user) def test_redirect_loop(self): + base_logout_page_url = str(self.url) logout_page_urls = [ - self.url.rstrip('/'), - self.url + ('/' if self.url[-1] != '/' else ''), + base_logout_page_url.rstrip('/'), + base_logout_page_url + ('/' if base_logout_page_url[-1] != '/' else ''), ] # The view is expected to redirect to the default destination (the @@ -461,3 +390,18 @@ def test_redirect_loop(self): auto_follow=True) self.assertEqual(page.status_code, 200) self.assertEqual(page.request.path, '/') + + def test_custom_cookies(self): + page = self.app.get('/', user=self.user) + self.assertNotIn('seen_at', self.app.cookies) + self.app.set_cookie('seen_at', "DUMMY_VALUE") + + # After successful logout, the `seen_at` cookie of the + # previously authenticated user is expected to be removed. + page = self.app.post( + self.url, + { + 'csrfmiddlewaretoken': page.context['csrf_token'], + }, + user=self.user) + self.assertNotIn('seen_at', self.app.cookies)