From 37aeeea2394c9660f7a1803196d25010ee8d3546 Mon Sep 17 00:00:00 2001 From: Jens Langhammer Date: Sun, 10 Mar 2019 02:08:09 +0100 Subject: [PATCH] slightly refactor Factor View, add more unittests --- passbook/core/auth/view.py | 51 +++++----- passbook/core/tests/test_auth_view.py | 130 ++++++++++++++++++++++++++ 2 files changed, 158 insertions(+), 23 deletions(-) create mode 100644 passbook/core/tests/test_auth_view.py diff --git a/passbook/core/auth/view.py b/passbook/core/auth/view.py index 81e6b4816..710967b14 100644 --- a/passbook/core/auth/view.py +++ b/passbook/core/auth/view.py @@ -39,35 +39,41 @@ class AuthenticationView(UserPassesTestMixin, View): # Allow only not authenticated users to login def test_func(self): - return self.request.user.is_authenticated is False + return AuthenticationView.SESSION_PENDING_USER in self.request.session def handle_no_permission(self): # Function from UserPassesTestMixin if 'next' in self.request.GET: return redirect(self.request.GET.get('next')) - return _redirect_with_qs('passbook_core:overview', self.request.GET) + if self.request.user.is_authenticated: + return _redirect_with_qs('passbook_core:overview', self.request.GET) + return _redirect_with_qs('passbook_core:auth-login', self.request.GET) + + def get_pending_factors(self): + """Loading pending factors from Database or load from session variable""" + # Write pending factors to session + if AuthenticationView.SESSION_PENDING_FACTORS in self.request.session: + return self.request.session[AuthenticationView.SESSION_PENDING_FACTORS] + # Get an initial list of factors which are currently enabled + # and apply to the current user. We check policies here and block the request + _all_factors = Factor.objects.filter(enabled=True).order_by('order').select_subclasses() + pending_factors = [] + for factor in _all_factors: + policy_engine = PolicyEngine(factor.policies.all()) + policy_engine.for_user(self.pending_user).with_request(self.request).build() + if policy_engine.passing: + pending_factors.append((factor.uuid.hex, factor.type)) + return pending_factors def dispatch(self, request, *args, **kwargs): + # Check if user passes test (i.e. SESSION_PENDING_USER is set) + user_test_result = self.get_test_func()() + if not user_test_result: + return self.handle_no_permission() # Extract pending user from session (only remember uid) - if AuthenticationView.SESSION_PENDING_USER in request.session: - self.pending_user = get_object_or_404( - User, id=self.request.session[AuthenticationView.SESSION_PENDING_USER]) - else: - # No Pending user, redirect to login screen - return _redirect_with_qs('passbook_core:auth-login', request.GET) - # Write pending factors to session - if AuthenticationView.SESSION_PENDING_FACTORS in request.session: - self.pending_factors = request.session[AuthenticationView.SESSION_PENDING_FACTORS] - else: - # Get an initial list of factors which are currently enabled - # and apply to the current user. We check policies here and block the request - _all_factors = Factor.objects.filter(enabled=True).order_by('order').select_subclasses() - self.pending_factors = [] - for factor in _all_factors: - policy_engine = PolicyEngine(factor.policies.all()) - policy_engine.for_user(self.pending_user).with_request(request).build() - if policy_engine.passing: - self.pending_factors.append((factor.uuid.hex, factor.type)) + self.pending_user = get_object_or_404( + User, id=self.request.session[AuthenticationView.SESSION_PENDING_USER]) + self.pending_factors = self.get_pending_factors() # Read and instantiate factor from session factor_uuid, factor_class = None, None if AuthenticationView.SESSION_FACTOR not in request.session: @@ -107,11 +113,11 @@ class AuthenticationView(UserPassesTestMixin, View): next_factor = None if self.pending_factors: next_factor = self.pending_factors.pop() + # Save updated pening_factor list to session self.request.session[AuthenticationView.SESSION_PENDING_FACTORS] = \ self.pending_factors self.request.session[AuthenticationView.SESSION_FACTOR] = next_factor LOGGER.debug("Rendering Factor is %s", next_factor) - # return _redirect_with_qs('passbook_core:auth-process', kwargs={'factor': next_factor}) return _redirect_with_qs('passbook_core:auth-process', self.request.GET) # User passed all factors LOGGER.debug("User passed all factors, logging in") @@ -126,7 +132,6 @@ class AuthenticationView(UserPassesTestMixin, View): def _user_passed(self): """User Successfully passed all factors""" - # user = authenticate(request=self.request, ) backend = self.request.session[AuthenticationView.SESSION_USER_BACKEND] login(self.request, self.pending_user, backend=backend) LOGGER.debug("Logged in user %s", self.pending_user) diff --git a/passbook/core/tests/test_auth_view.py b/passbook/core/tests/test_auth_view.py new file mode 100644 index 000000000..573c214c8 --- /dev/null +++ b/passbook/core/tests/test_auth_view.py @@ -0,0 +1,130 @@ +"""passbook Core Authentication Test""" +import string +from random import SystemRandom + +from django.contrib.auth.models import AnonymousUser +from django.contrib.sessions.middleware import SessionMiddleware +from django.test import RequestFactory, TestCase +from django.urls import reverse + +from passbook.core.auth.view import AuthenticationView +from passbook.core.models import DummyFactor, PasswordFactor, User + + +class TestFactorAuthentication(TestCase): + """passbook Core Authentication Test""" + + def setUp(self): + super().setUp() + self.password = ''.join(SystemRandom().choice( + string.ascii_uppercase + string.digits) for _ in range(8)) + self.factor, _ = PasswordFactor.objects.get_or_create(name='password', + slug='password', + backends=[]) + self.user = User.objects.create_user(username='test', + email='test@test.test', + password=self.password) + + def test_unauthenticated_raw(self): + """test direct call to AuthenticationView""" + response = self.client.get(reverse('passbook_core:auth-process')) + # Response should be 302 since no pending user is set + self.assertEqual(response.status_code, 302) + self.assertEqual(response.url, reverse('passbook_core:auth-login')) + + def test_unauthenticated_prepared(self): + """test direct call but with pending_uesr in session""" + request = RequestFactory().get(reverse('passbook_core:auth-process')) + request.user = AnonymousUser() + request.session = {} + request.session[AuthenticationView.SESSION_PENDING_USER] = self.user.pk + + response = AuthenticationView.as_view()(request) + self.assertEqual(response.status_code, 200) + + def test_no_factors(self): + """Test with all factors disabled""" + self.factor.enabled = False + self.factor.save() + request = RequestFactory().get(reverse('passbook_core:auth-process')) + request.user = AnonymousUser() + request.session = {} + request.session[AuthenticationView.SESSION_PENDING_USER] = self.user.pk + + response = AuthenticationView.as_view()(request) + self.assertEqual(response.status_code, 302) + self.assertEqual(response.url, reverse('passbook_core:auth-denied')) + self.factor.enabled = True + self.factor.save() + + def test_authenticated(self): + """Test with already logged in user""" + self.client.force_login(self.user) + response = self.client.get(reverse('passbook_core:auth-process')) + # Response should be 302 since no pending user is set + self.assertEqual(response.status_code, 302) + self.assertEqual(response.url, reverse('passbook_core:overview')) + self.client.logout() + + def test_unauthenticated_post(self): + """Test post request as unauthenticated user""" + request = RequestFactory().post(reverse('passbook_core:auth-process'), data={ + 'password': self.password + }) + request.user = AnonymousUser() + middleware = SessionMiddleware() + middleware.process_request(request) + request.session.save() + request.session[AuthenticationView.SESSION_PENDING_USER] = self.user.pk + + response = AuthenticationView.as_view()(request) + self.assertEqual(response.status_code, 302) + self.assertEqual(response.url, reverse('passbook_core:overview')) + self.client.logout() + + def test_unauthenticated_post_invalid(self): + """Test post request as unauthenticated user""" + request = RequestFactory().post(reverse('passbook_core:auth-process'), data={ + 'password': self.password + 'a' + }) + request.user = AnonymousUser() + middleware = SessionMiddleware() + middleware.process_request(request) + request.session.save() + request.session[AuthenticationView.SESSION_PENDING_USER] = self.user.pk + + response = AuthenticationView.as_view()(request) + self.assertEqual(response.status_code, 200) + self.client.logout() + + def test_multifactor(self): + """Test view with multiple active factors""" + DummyFactor.objects.get_or_create(name='dummy', + slug='dummy', + order=1) + request = RequestFactory().post(reverse('passbook_core:auth-process'), data={ + 'password': self.password + }) + request.user = AnonymousUser() + middleware = SessionMiddleware() + middleware.process_request(request) + request.session.save() + request.session[AuthenticationView.SESSION_PENDING_USER] = self.user.pk + + response = AuthenticationView.as_view()(request) + session_copy = request.session.items() + self.assertEqual(response.status_code, 302) + # Verify view redirects to itself after auth + self.assertEqual(response.url, reverse('passbook_core:auth-process')) + + # Run another request with same session which should result in a logged in user + request = RequestFactory().post(reverse('passbook_core:auth-process')) + request.user = AnonymousUser() + middleware = SessionMiddleware() + middleware.process_request(request) + for key, value in session_copy: + request.session[key] = value + request.session.save() + response = AuthenticationView.as_view()(request) + self.assertEqual(response.status_code, 302) + self.assertEqual(response.url, reverse('passbook_core:overview'))