diff --git a/src/open_inwoner/accounts/backends.py b/src/open_inwoner/accounts/backends.py index 5890faf6ab..bf55a60c82 100644 --- a/src/open_inwoner/accounts/backends.py +++ b/src/open_inwoner/accounts/backends.py @@ -4,7 +4,7 @@ from django.contrib.auth import get_user_model from django.contrib.auth.backends import ModelBackend from django.contrib.auth.hashers import check_password -from django.urls import reverse_lazy +from django.urls import reverse, reverse_lazy from axes.backends import AxesBackend from mozilla_django_oidc_db.backends import OIDCAuthenticationBackend @@ -79,6 +79,10 @@ def authenticate(self, request, *args, **kwargs): if request and request.path != self.callback_path: return + config = SiteConfiguration.get_solo() + if request and config.openid_enabled_for_admin: + request.session["oidc_login_next"] = reverse("admin:index") + return super().authenticate(request, *args, **kwargs) def create_user(self, claims): @@ -106,6 +110,8 @@ def create_user(self, claims): # TODO verify we want unusable_password existing_user.set_unusable_password() existing_user.save() + # Ensure `make_user_staff` is used + self.update_user(existing_user, claims) return existing_user else: logger.debug("Creating OIDC user: %s", unique_id) @@ -116,9 +122,13 @@ def create_user(self, claims): "login_type": LoginTypeChoices.oidc, } user = self.UserModel.objects.create_user(**kwargs) + # Ensure `make_user_staff` is used self.update_user(user, claims) # TODO verify we want unusable_password user.set_unusable_password() + # Saving after using `set_unusable_password` is required, otherwise the user + # will not actually be authenticated, see: https://taiga.maykinmedia.nl/project/open-inwoner/issue/2101 + user.save() return user diff --git a/src/open_inwoner/accounts/tests/test_oidc_views.py b/src/open_inwoner/accounts/tests/test_oidc_views.py index fb5346087d..b6a6eeb6a8 100644 --- a/src/open_inwoner/accounts/tests/test_oidc_views.py +++ b/src/open_inwoner/accounts/tests/test_oidc_views.py @@ -19,6 +19,8 @@ GENERIC_DIGID_ERROR_MSG, GENERIC_EHERKENNING_ERROR_MSG, ) +from open_inwoner.configurations.choices import OpenIDDisplayChoices +from open_inwoner.configurations.models import SiteConfiguration from open_inwoner.kvk.branches import KVK_BRANCH_SESSION_VARIABLE from ..choices import LoginTypeChoices @@ -34,10 +36,15 @@ class OIDCFlowTests(TestCase): @patch("mozilla_django_oidc_db.backends.OIDCAuthenticationBackend.get_token") @patch( "mozilla_django_oidc_db.mixins.OpenIDConnectConfig.get_solo", - return_value=OpenIDConnectConfig(id=1, enabled=True), + return_value=OpenIDConnectConfig(id=1, enabled=True, make_users_staff=True), + ) + @patch( + "open_inwoner.configurations.models.SiteConfiguration.get_solo", + return_value=SiteConfiguration(openid_display=OpenIDDisplayChoices.admin), ) - def test_existing_email_updates_user( + def test_existing_email_updates_admin_user( self, + mock_config_get_solo, mock_get_solo, mock_get_token, mock_verify_token, @@ -62,18 +69,126 @@ def test_existing_email_updates_user( callback_url, {"code": "mock", "state": "mock"} ) + self.assertRedirects( + callback_response, reverse("admin:index"), fetch_redirect_response=True + ) + + self.assertEqual(User.objects.count(), 1) + user.refresh_from_db() + self.assertEqual(user.oidc_id, "some_username") + self.assertEqual(user.login_type, LoginTypeChoices.oidc) + self.assertEqual(user.is_staff, True) + + @patch("mozilla_django_oidc_db.backends.OIDCAuthenticationBackend.get_userinfo") + @patch("mozilla_django_oidc_db.backends.OIDCAuthenticationBackend.store_tokens") + @patch("mozilla_django_oidc_db.backends.OIDCAuthenticationBackend.verify_token") + @patch("mozilla_django_oidc_db.backends.OIDCAuthenticationBackend.get_token") + @patch( + "mozilla_django_oidc_db.mixins.OpenIDConnectConfig.get_solo", + return_value=OpenIDConnectConfig(id=1, enabled=True, make_users_staff=False), + ) + @patch( + "open_inwoner.configurations.models.SiteConfiguration.get_solo", + return_value=SiteConfiguration(openid_display=OpenIDDisplayChoices.regular), + ) + def test_existing_email_updates_regular_user( + self, + mock_config_get_solo, + mock_get_solo, + mock_get_token, + mock_verify_token, + mock_store_tokens, + mock_get_userinfo, + ): + # set up a user with a colliding email address + # sub is the oidc_id field in our db + mock_get_userinfo.return_value = { + "email": "existing_user@example.com", + "sub": "some_username", + } + user = UserFactory.create(email="existing_user@example.com") + self.assertEqual(user.oidc_id, "") + session = self.client.session + session["oidc_states"] = {"mock": {"nonce": "nonce"}} + session.save() + callback_url = reverse("oidc_authentication_callback") + + # enter the login flow + callback_response = self.client.get( + callback_url, {"code": "mock", "state": "mock"} + ) + self.assertRedirects( - callback_response, reverse("pages-root"), fetch_redirect_response=False + callback_response, reverse("pages-root"), fetch_redirect_response=True ) - self.assertTrue(User.objects.filter(oidc_id="some_username").exists()) + + self.assertEqual(User.objects.count(), 1) + + user.refresh_from_db() + self.assertEqual(user.oidc_id, "some_username") + self.assertEqual(user.login_type, LoginTypeChoices.oidc) + self.assertEqual(user.is_staff, False) - db_user = User.objects.filter(oidc_id="some_username").first() + @patch("mozilla_django_oidc_db.backends.OIDCAuthenticationBackend.get_userinfo") + @patch("mozilla_django_oidc_db.backends.OIDCAuthenticationBackend.store_tokens") + @patch("mozilla_django_oidc_db.backends.OIDCAuthenticationBackend.verify_token") + @patch("mozilla_django_oidc_db.backends.OIDCAuthenticationBackend.get_token") + @patch( + "mozilla_django_oidc_db.mixins.OpenIDConnectConfig.get_solo", + return_value=OpenIDConnectConfig( + id=1, + enabled=True, + make_users_staff=False, + claim_mapping={"first_name": "first_name"}, + ), + ) + @patch( + "open_inwoner.configurations.models.SiteConfiguration.get_solo", + return_value=SiteConfiguration(openid_display=OpenIDDisplayChoices.regular), + ) + def test_existing_oidc_id_updates_regular_user( + self, + mock_config_get_solo, + mock_get_solo, + mock_get_token, + mock_verify_token, + mock_store_tokens, + mock_get_userinfo, + ): + # set up a user with a colliding email address + # sub is the oidc_id field in our db + mock_get_userinfo.return_value = { + "email": "existing_user@example.com", + "sub": "some_username", + "first_name": "bar", + } + user = UserFactory.create( + oidc_id="some_username", first_name="Foo", login_type=LoginTypeChoices.oidc + ) + session = self.client.session + session["oidc_states"] = {"mock": {"nonce": "nonce"}} + session.save() + callback_url = reverse("oidc_authentication_callback") - self.assertEqual(db_user.id, user.id) - self.assertEqual(db_user.login_type, LoginTypeChoices.oidc) + # enter the login flow + callback_response = self.client.get( + callback_url, {"code": "mock", "state": "mock"} + ) + + self.assertRedirects( + callback_response, reverse("pages-root"), fetch_redirect_response=True + ) + + self.assertEqual(User.objects.count(), 1) + + user.refresh_from_db() + + self.assertEqual(user.oidc_id, "some_username") + self.assertEqual(user.first_name, "bar") + self.assertEqual(user.is_staff, False) @patch("mozilla_django_oidc_db.backends.OIDCAuthenticationBackend.get_userinfo") @patch("mozilla_django_oidc_db.backends.OIDCAuthenticationBackend.store_tokens") @@ -83,8 +198,13 @@ def test_existing_email_updates_user( "mozilla_django_oidc_db.mixins.OpenIDConnectConfig.get_solo", return_value=OpenIDConnectConfig(id=1, enabled=True), ) + @patch( + "open_inwoner.configurations.models.SiteConfiguration.get_solo", + return_value=SiteConfiguration(openid_display=OpenIDDisplayChoices.regular), + ) def test_existing_case_sensitive_email_updates_user( self, + mock_config_get_solo, mock_get_solo, mock_get_token, mock_verify_token, @@ -123,6 +243,7 @@ def test_existing_case_sensitive_email_updates_user( self.assertEqual(db_user.id, user.id) self.assertEqual(db_user.login_type, LoginTypeChoices.oidc) + self.assertEqual(db_user.is_staff, False) @patch("mozilla_django_oidc_db.backends.OIDCAuthenticationBackend.get_userinfo") @patch("mozilla_django_oidc_db.backends.OIDCAuthenticationBackend.store_tokens") @@ -130,10 +251,15 @@ def test_existing_case_sensitive_email_updates_user( @patch("mozilla_django_oidc_db.backends.OIDCAuthenticationBackend.get_token") @patch( "mozilla_django_oidc_db.mixins.OpenIDConnectConfig.get_solo", - return_value=OpenIDConnectConfig(id=1, enabled=True), + return_value=OpenIDConnectConfig(id=1, enabled=True, make_users_staff=True), ) - def test_new_user_is_created_when_new_email( + @patch( + "open_inwoner.configurations.models.SiteConfiguration.get_solo", + return_value=SiteConfiguration(openid_display=OpenIDDisplayChoices.admin), + ) + def test_new_admin_user_is_created_when_new_email( self, + mock_config_get_solo, mock_get_solo, mock_get_token, mock_verify_token, @@ -159,13 +285,61 @@ def test_new_user_is_created_when_new_email( ) self.assertRedirects( - callback_response, reverse("pages-root"), fetch_redirect_response=False + callback_response, reverse("admin:index"), fetch_redirect_response=True + ) + + new_user = User.objects.get(email="new_user@example.com") + + self.assertEqual(new_user.oidc_id, "some_username") + self.assertEqual(new_user.login_type, LoginTypeChoices.oidc) + self.assertEqual(new_user.is_staff, True) + + @patch("mozilla_django_oidc_db.backends.OIDCAuthenticationBackend.get_userinfo") + @patch("mozilla_django_oidc_db.backends.OIDCAuthenticationBackend.store_tokens") + @patch("mozilla_django_oidc_db.backends.OIDCAuthenticationBackend.verify_token") + @patch("mozilla_django_oidc_db.backends.OIDCAuthenticationBackend.get_token") + @patch( + "mozilla_django_oidc_db.mixins.OpenIDConnectConfig.get_solo", + return_value=OpenIDConnectConfig(id=1, enabled=True, make_users_staff=False), + ) + @patch( + "open_inwoner.configurations.models.SiteConfiguration.get_solo", + return_value=SiteConfiguration(openid_display=OpenIDDisplayChoices.regular), + ) + def test_new_regular_user_is_created_when_new_email( + self, + mock_config_get_solo, + mock_get_solo, + mock_get_token, + mock_verify_token, + mock_store_tokens, + mock_get_userinfo, + ): + # set up a user with a non existing email address + mock_get_userinfo.return_value = { + "email": "new_user@example.com", + "sub": "some_username", + } + UserFactory.create(email="existing_user@example.com") + session = self.client.session + session["oidc_states"] = {"mock": {"nonce": "nonce"}} + session.save() + callback_url = reverse("oidc_authentication_callback") + + # enter the login flow + callback_response = self.client.get( + callback_url, {"code": "mock", "state": "mock"} ) - new_user = User.objects.filter(email="new_user@example.com") - self.assertTrue(new_user.exists()) - self.assertEqual(new_user.get().oidc_id, "some_username") - self.assertEqual(new_user.get().login_type, LoginTypeChoices.oidc) + self.assertRedirects( + callback_response, reverse("pages-root"), fetch_redirect_response=True + ) + + new_user = User.objects.get(email="new_user@example.com") + + self.assertEqual(new_user.oidc_id, "some_username") + self.assertEqual(new_user.login_type, LoginTypeChoices.oidc) + self.assertEqual(new_user.is_staff, False) def test_error_page_direct_access_forbidden(self): error_url = reverse("admin-oidc-error") @@ -182,8 +356,13 @@ def test_error_page_direct_access_forbidden(self): "mozilla_django_oidc_db.mixins.OpenIDConnectConfig.get_solo", return_value=OpenIDConnectConfig(id=1, enabled=True), ) + @patch( + "open_inwoner.configurations.models.SiteConfiguration.get_solo", + return_value=SiteConfiguration(openid_display=OpenIDDisplayChoices.regular), + ) def test_error_first_cleared_after_succesful_login( self, + mock_config_get_solo, mock_get_solo, mock_get_token, mock_verify_token, diff --git a/src/open_inwoner/conf/fixtures/django-admin-index.json b/src/open_inwoner/conf/fixtures/django-admin-index.json index dfad09f9da..193bc56189 100644 --- a/src/open_inwoner/conf/fixtures/django-admin-index.json +++ b/src/open_inwoner/conf/fixtures/django-admin-index.json @@ -26,6 +26,10 @@ "accounts", "user" ], + [ + "mozilla_django_oidc_db", + "openidconnectconfig" + ], [ "userfeed", "feeditemdata" diff --git a/src/open_inwoner/configurations/tests/test_oidc.py b/src/open_inwoner/configurations/tests/test_oidc.py index 7b75528334..4ee990c058 100644 --- a/src/open_inwoner/configurations/tests/test_oidc.py +++ b/src/open_inwoner/configurations/tests/test_oidc.py @@ -41,7 +41,10 @@ def test_admin_only_enabled(self): oidc_login_option = response.pyquery.find(".admin-login-option") - self.assertEqual(oidc_login_option.text(), _("Login with organization account")) + self.assertEqual( + oidc_login_option.text(), + "{} {}".format(_("or"), _("Login with organization account")), + ) def test_admin_only_disabled(self): """Assert that the OIDC login option is only displayed for regular users""" diff --git a/src/open_inwoner/scss/admin/_app_overrides.scss b/src/open_inwoner/scss/admin/_app_overrides.scss index 4ec4753af8..cac2e9c8e6 100644 --- a/src/open_inwoner/scss/admin/_app_overrides.scss +++ b/src/open_inwoner/scss/admin/_app_overrides.scss @@ -104,3 +104,10 @@ $djai-border-width: 8px; } } } + +/* Extra login links in admin login screen */ +.admin-login-option { + text-align: center; + clear: both; + padding-top: 1em; +} diff --git a/src/open_inwoner/templates/admin/login.html b/src/open_inwoner/templates/admin/login.html new file mode 100644 index 0000000000..b5a0131649 --- /dev/null +++ b/src/open_inwoner/templates/admin/login.html @@ -0,0 +1,17 @@ +{% extends "admin/login.html" %} +{% load solo_tags i18n %} + + +{% block content %} +{{ block.super }} + +{% get_solo 'mozilla_django_oidc_db.OpenIDConnectConfig' as oidc_config %} +{% get_solo 'configurations.SiteConfiguration' as site_config %} +{% if oidc_config.enabled and site_config.openid_enabled_for_admin %} +