Merge pull request #673 from rdmorganiser/fix_shibboleth

Fix new Shibboleth setup
rdmorganiser · Aug 18, 2023 · 71c6ad7 · 71c6ad7
2 parents 777e4e1 + 4deabf9
commit 71c6ad7
Show file tree

Hide file tree

Showing 5 changed files with 21 additions and 35 deletions.
diff --git a/rdmo/accounts/templates/account/login_form.html b/rdmo/accounts/templates/account/login_form.html
@@ -1,21 +1,11 @@
 {% load i18n %}
 
 {% if settings.SHIBBOLETH %}
-
-{% if settings.SHIBBOLETH_LOGIN_URL %}
-<p>
+<p style="margin-bottom: 20px;">
     <a class="btn btn-success extend" href="{% url 'shibboleth_login' %}">
         {% trans 'Login with Shibboleth' %}
     </a>
 </p>
-{% else %}
-<p>
-    <a class="btn btn-success extend" href="{% url 'account_login' %}">
-        {% trans 'Login with Shibboleth' %}
-    </a>
-</p>
-{% endif %}
-
 {% endif %}
 
 {% if settings.LOGIN_FORM %}

diff --git a/rdmo/accounts/templates/account/logout.html b/rdmo/accounts/templates/account/logout.html
@@ -9,7 +9,7 @@ <h1>{% trans "Logout" %}</h1>
         {% trans 'Are you sure you want to sign out?' %}
     </p>
 
-    <form method="post" action="{% url 'account_logout' %}">
+    <form method="post" action="{{ settings.LOGOUT_URL }}">
         {% csrf_token %}
 
         {% if redirect_field_value %}

diff --git a/rdmo/accounts/templates/account/logout_form.html b/rdmo/accounts/templates/account/logout_form.html
@@ -1,6 +1,6 @@
 {% load i18n %}
 
-    <form class="logout-form" action="{% url 'account_logout' %}" method="POST">
+    <form class="logout-form" action="{{ settings.LOGOUT_URL }}" method="POST">
         {% csrf_token %}
         <button type="submit" class="btn btn-link">
             {% trans 'Logout' %}

diff --git a/rdmo/accounts/tests/test_views.py b/rdmo/accounts/tests/test_views.py
@@ -95,6 +95,7 @@ def test_post_profile_update(db, client, settings, test_setting):
     else:
         assert response.status_code == 200
 
+
 @pytest.mark.parametrize('test_setting', boolean_toggle)
 def test_post_profile_update_cancel(db, client, settings, test_setting):
     """
@@ -119,6 +120,7 @@ def test_post_profile_update_cancel(db, client, settings, test_setting):
     else:
         assert response.status_code == 200
 
+
 @pytest.mark.parametrize('test_setting', boolean_toggle)
 def test_post_profile_update_cancel2(db, client, settings, test_setting):
     """
@@ -144,6 +146,7 @@ def test_post_profile_update_cancel2(db, client, settings, test_setting):
     else:
         assert response.status_code == 200
 
+
 @pytest.mark.parametrize('test_setting', boolean_toggle)
 def test_post_profile_update_next(db, client, settings, test_setting):
     """
@@ -170,6 +173,7 @@ def test_post_profile_update_next(db, client, settings, test_setting):
     else:
         assert response.status_code == 200
 
+
 @pytest.mark.parametrize('test_setting', boolean_toggle)
 def test_post_profile_update_next2(db, client, settings, test_setting):
     """
@@ -196,6 +200,7 @@ def test_post_profile_update_next2(db, client, settings, test_setting):
     else:
         assert response.status_code == 200
 
+
 @pytest.mark.parametrize('test_setting', boolean_toggle)
 def test_password_change_get(db, client, settings, test_setting):
     """
@@ -630,11 +635,9 @@ def test_home_login_form(db, client, settings, LOGIN_FORM, username, password):
 
 
 @pytest.mark.parametrize('SHIBBOLETH', boolean_toggle)
-@pytest.mark.parametrize('SHIBBOLETH_LOGIN_URL', (None, '/shibboleth/login'))
 @pytest.mark.parametrize('username,password', users)
-def test_shibboleth_for_home_url(db, client, settings, SHIBBOLETH, SHIBBOLETH_LOGIN_URL, username, password):
+def test_shibboleth_for_home_url(db, client, settings, SHIBBOLETH, username, password):
     settings.SHIBBOLETH = SHIBBOLETH
-    settings.SHIBBOLETH_LOGIN_URL  = SHIBBOLETH_LOGIN_URL
     settings.ACCOUNT = False
     reload_app_urlconf_in_testcase('accounts')
     # Anonymous user lands on home
@@ -645,17 +648,13 @@ def test_shibboleth_for_home_url(db, client, settings, SHIBBOLETH, SHIBBOLETH_LO
     if SHIBBOLETH:
         # Anyonymous user is redirected to login
         response.status_code == 200
-
-        if SHIBBOLETH_LOGIN_URL:
-            assertContains(response, 'href="' + reverse('shibboleth_login'))
-        else:
-            assertContains(response, 'href="' + reverse('account_login'))
+        assertContains(response, 'href="' + reverse('shibboleth_login'))
 
 
 @pytest.mark.parametrize('username,password', users)
 def test_shibboleth_login_view(db, client, settings, username, password):
     settings.SHIBBOLETH = True
-    settings.SHIBBOLETH_LOGIN_URL  = '/shibboleth/login'
+    settings.SHIBBOLETH_LOGIN_URL = '/shibboleth/login'
     reload_app_urlconf_in_testcase('accounts')
     # Anonymous user lands on home
     client.login(username='anonymous', password=None)
@@ -681,7 +680,7 @@ def test_shibboleth_login_view(db, client, settings, username, password):
 @pytest.mark.parametrize('username,password', users)
 def test_shibboleth_for_projects_url(db, client, settings, SHIBBOLETH, SHIBBOLETH_LOGIN_URL, username, password):
     settings.SHIBBOLETH = SHIBBOLETH
-    settings.SHIBBOLETH_LOGIN_URL  = SHIBBOLETH_LOGIN_URL
+    settings.SHIBBOLETH_LOGIN_URL = SHIBBOLETH_LOGIN_URL
     settings.ACCOUNT = False
     reload_app_urlconf_in_testcase('accounts')
     client.login(username='anonymous', password=None)
@@ -700,7 +699,7 @@ def test_shibboleth_for_projects_url(db, client, settings, SHIBBOLETH, SHIBBOLET
         response.status_code == 200
         assertContains(response, 'href="/account/shibboleth/login/">')
 
-        # Redirected user logs in 
+        # Redirected user logs in
         client.login(username=username, password=password)
         response = client.get(response)
 
@@ -716,7 +715,7 @@ def test_shibboleth_for_projects_url(db, client, settings, SHIBBOLETH, SHIBBOLET
 @pytest.mark.parametrize('username,password', users)
 def test_shibboleth_logout_username_pattern(db, client, settings, SHIBBOLETH, username, password):
     settings.SHIBBOLETH = SHIBBOLETH
-    settings.SHIBBOLETH_USERNAME_PATTERN  = username
+    settings.SHIBBOLETH_USERNAME_PATTERN = username
     reload_app_urlconf_in_testcase('accounts')
 
     client.login(username=username, password=password)

diff --git a/rdmo/accounts/urls/__init__.py b/rdmo/accounts/urls/__init__.py
@@ -16,6 +16,13 @@
         re_path('^terms-of-use/', terms_of_use, name='terms_of_use')
     ]
 
+if settings.SHIBBOLETH:
+    urlpatterns += [
+        re_path('^shibboleth/login/', shibboleth_login, name='shibboleth_login'),
+        re_path('^shibboleth/logout/', shibboleth_logout, name='shibboleth_logout'),
+        re_path('^logout/', auth_views.LogoutView.as_view(next_page=settings.SHIBBOLETH_LOGOUT_URL), name='account_logout'),
+    ]
+
 if settings.ACCOUNT or settings.SOCIALACCOUNT:
     # include django-allauth urls
     urlpatterns += [
@@ -27,16 +34,6 @@
         re_path('^logout/', auth_views.LogoutView.as_view(next_page=settings.LOGIN_REDIRECT_URL), name='account_logout'),
     ]
 
-if settings.SHIBBOLETH:
-    if settings.SHIBBOLETH_LOGIN_URL:
-        urlpatterns += [
-            re_path('^shibboleth/login/', shibboleth_login, name='shibboleth_login'),
-            re_path('^shibboleth/logout/', shibboleth_logout, name='shibboleth_logout'),
-        ]
-    else:
-        urlpatterns += [
-            re_path('^logout/', auth_views.LogoutView.as_view(next_page=settings.SHIBBOLETH_LOGOUT_URL), name='account_logout'),
-        ]
 
 if settings.ACCOUNT_ALLOW_USER_TOKEN:
     urlpatterns += [