Bugfix: Error if logout attempted while not logged in (#3405)

* fix error if logout attempted while not logged in

* add test for anon logout

backend/djangooidc/tests/test_views.py

@@ -1,11 +1,16 @@
 from unittest.mock import patch
 
+from django.contrib.auth import get_user_model
 from django.http import HttpResponse
 from django.test import Client, TestCase
 from django.urls import reverse
 
 from .common import less_console_noise
 
+from model_bakery import baker
+
+User = get_user_model()
+
 
 @patch("djangooidc.views.CLIENT", autospec=True)
 class ViewsTest(TestCase):
@@ -78,8 +83,18 @@ def test_login_callback_raises(self, mock_auth, mock_client):
         self.assertTemplateUsed(response, "401.html")
         self.assertIn("Unauthorized", response.content.decode("utf-8"))
 
+    def test_logout_anonymous_redirect_url(self, mock_client):
+        # attempting to logout as an anonymous user should redirect to homepage
+        response = self.client.get(reverse("logout"))
+
+        self.assertEqual(response.status_code, 302)
+        self.assertEqual(response.url, "/")
+
     def test_logout_redirect_url(self, mock_client):
         # setup
+        user = baker.make(User)
+        self.client.force_login(user)
+
         session = self.client.session
         session["state"] = "TEST"  # nosec B105
         session.save()

backend/djangooidc/views.py

@@ -5,7 +5,7 @@
 from django.conf import settings
 from django.contrib.auth import logout as auth_logout
 from django.contrib.auth import authenticate, login
-from django.http import HttpResponseRedirect
+from django.http import HttpRequest, HttpResponseRedirect
 from django.shortcuts import redirect, render
 from urllib.parse import parse_qs, urlencode
 
@@ -71,18 +71,22 @@ def login_callback(request):
         user = authenticate(request=request, **userinfo)
         if user:
             login(request, user)
-            logger.info("Successfully logged in user %s" % user)
+            logger.info(f"user {user} logged in")
             return redirect(request.session.get("next", "/"))
         else:
             raise o_e.BannedUser()
     except Exception as err:
         return error_page(request, err)
 
 
-def logout(request, next_page=None):
+def logout(request: HttpRequest, next_page=None):
     """Redirect the user to the authentication provider (OP) logout page."""
     try:
-        username = request.user.username
+        if not request.user.is_authenticated:
+            return HttpResponseRedirect("/")
+
+        logger.info(f"user {request.user.username} logging out")
+
         request_args = {
             "client_id": CLIENT.client_id,
             "state": request.session["state"],
@@ -108,7 +112,6 @@ def logout(request, next_page=None):
         # Always remove Django session stuff - even if not logged out from OP.
         # Don't wait for the callback as it may never come.
         auth_logout(request)
-        logger.info("Successfully logged out user %s" % username)
         next_page = getattr(settings, "LOGOUT_REDIRECT_URL", None)
         if next_page:
             request.session["next"] = next_page

backend/users/auth.py

@@ -21,7 +21,7 @@ def claim_audit_access(user, all_emails):
         access_invites = Access.objects.filter(email__iexact=email).update(
             user_id=user.id, email=email
         )
-        logger.debug(f"{user.email} granted access to {access_invites} new audits")
+        logger.debug(f"{user.email} granted access to {access_invites} audits")
 
 
 def claim_permissions(user, all_emails):