Upgrade python and django (#4019)

.github/workflows/ci.yml

@@ -51,7 +51,7 @@ jobs:
 
       - uses: actions/setup-python@v4
         with:
-          python-version: '3.9'
+          python-version: '3.11.4'
           cache: 'pip'
           cache-dependency-path: |
             **/requirements.txt

Dockerfile

@@ -1,4 +1,4 @@
-FROM python:3.9.13
+FROM python:3.11.4
 LABEL maintainer "ODL DevOps <[email protected]>"
 
 # Add package files, install updated node and pip

app.json

@@ -533,6 +533,10 @@
       "description": "Google Recaptcha secret key",
       "required": false
     },
+    "REQUESTS_TIMEOUT": {
+      "description": "Default timeout for requests",
+      "required": false
+    },
     "RSS_FEED_EPISODE_LIMIT":{
       "description": "Number of episodes included in aggregated rss feed",
       "required": false

authentication/backends/base_jwt_test.py

@@ -45,10 +45,16 @@ def test_auth_complete(user, auth, mock_strategy):
     jwt_payload_handler = api_settings.JWT_PAYLOAD_HANDLER
     jwt_encode_handler = api_settings.JWT_ENCODE_HANDLER
     jwt_payload = jwt_payload_handler(user)
+    jwt_payload_resp = {
+        **jwt_payload,
+        "exp": int(jwt_payload["exp"].timestamp()),
+        "user_profile_id": list(jwt_payload["user_profile_id"]),
+        "jti": str(jwt_payload["jti"]),
+    }
     jwt_value = jwt_encode_handler(jwt_payload)
     mock_strategy.request.COOKIES[api_settings.JWT_AUTH_COOKIE] = jwt_value
 
     assert auth.auth_complete(1, arg2=2) == mock_strategy.authenticate.return_value
     mock_strategy.authenticate.assert_called_once_with(
-        1, arg2=2, response=jwt_payload, backend=auth
+        1, arg2=2, response=jwt_payload_resp, backend=auth
     )

authentication/middleware.py

@@ -1,9 +1,10 @@
 """Authentication middleware"""
+from urllib.parse import quote
+
 from django.db.models import Q
 from django.http import HttpResponseForbidden
 from django.shortcuts import redirect
 from django.utils.deprecation import MiddlewareMixin
-from django.utils.http import urlquote
 from ipware import get_client_ip
 from rest_framework.permissions import SAFE_METHODS
 
@@ -37,7 +38,7 @@ def process_exception(self, request, exception):
 
             if url:
                 url += ("?" in url and "&" or "?") + "message={0}&backend={1}".format(
-                    urlquote(message), backend_name
+                    quote(message), backend_name
                 )
                 return redirect(url)
 

authentication/middleware_test.py

@@ -1,9 +1,10 @@
 """Tests for auth middleware"""
+from urllib.parse import quote
+
 import pytest
 
 from django.contrib.sessions.middleware import SessionMiddleware
 from django.shortcuts import reverse
-from django.utils.http import urlquote
 from rest_framework import status
 from social_core.exceptions import AuthAlreadyAssociated
 from social_django.utils import load_backend, load_strategy
@@ -16,46 +17,47 @@
 from open_discussions.factories import UserFactory
 
 
-def test_process_exception_no_strategy(rf, settings):
+def test_process_exception_no_strategy(mocker, rf, settings):
     """Tests that if the request has no strategy it does nothing"""
     settings.DEBUG = False
     request = rf.get(reverse("social:complete", args=("email",)))
-    middleware = SocialAuthExceptionRedirectMiddleware()
+    middleware = SocialAuthExceptionRedirectMiddleware(mocker.Mock())
     assert middleware.process_exception(request, None) is None
 
 
-def test_process_exception(rf, settings):
+def test_process_exception(mocker, rf, settings):
     """Tests that a process_exception handles auth exceptions correctly"""
     settings.DEBUG = False
     msg = "error message"
     request = rf.get(reverse("social:complete", args=("email",)))
     # social_django depends on request.sesssion, so use the middleware to set that
-    SessionMiddleware().process_request(request)
+    SessionMiddleware(mocker.Mock()).process_request(request)
     strategy = load_strategy(request)
     backend = load_backend(strategy, "email", None)
+    exc = AuthAlreadyAssociated(backend, msg)
     request.social_strategy = strategy
     request.backend = backend
 
-    middleware = SocialAuthExceptionRedirectMiddleware()
-    result = middleware.process_exception(request, AuthAlreadyAssociated(backend, msg))
+    middleware = SocialAuthExceptionRedirectMiddleware(mocker.Mock())
+    result = middleware.process_exception(request, exc)
     assert result.status_code == status.HTTP_302_FOUND
     assert result.url == "{}?message={}&backend={}".format(
-        reverse("login"), urlquote(msg), backend.name
+        reverse("login"), quote(str(exc)), backend.name
     )
 
 
-def test_process_exception_non_auth_error(rf, settings):
+def test_process_exception_non_auth_error(mocker, rf, settings):
     """Tests that a process_exception handles non-auth exceptions correctly"""
     settings.DEBUG = False
     request = rf.get(reverse("social:complete", args=("email",)))
     # social_django depends on request.sesssion, so use the middleware to set that
-    SessionMiddleware().process_request(request)
+    SessionMiddleware(mocker.Mock()).process_request(request)
     strategy = load_strategy(request)
     backend = load_backend(strategy, "email", None)
     request.social_strategy = strategy
     request.backend = backend
 
-    middleware = SocialAuthExceptionRedirectMiddleware()
+    middleware = SocialAuthExceptionRedirectMiddleware(mocker.Mock())
     assert (
         middleware.process_exception(request, Exception("something bad happened"))
         is None
@@ -83,7 +85,7 @@ def test_process_view_blocked_ip_middleware(  # pylint:disable=too-many-argument
         "authentication.middleware.get_client_ip", return_value=(user_ip, is_routable)
     )
 
-    middleware = BlockedIPMiddleware()
+    middleware = BlockedIPMiddleware(mocker.Mock())
     if is_blocked and is_routable and not exempt_view and not is_super:
         assert middleware.process_view(request, callback, None, {}).status_code == 403
     else:

authentication/pipeline/user_test.py

@@ -29,10 +29,10 @@ def validate_email_auth_request_not_email_backend(mocker):
     "has_user,expected", [(True, {"flow": SocialAuthState.FLOW_LOGIN}), (False, {})]
 )
 @pytest.mark.django_db
-def test_validate_email_auth_request(rf, has_user, expected):
+def test_validate_email_auth_request(mocker, rf, has_user, expected):
     """Test that validate_email_auth_request returns correctly given the input"""
     request = rf.post("/complete/email")
-    middleware = SessionMiddleware()
+    middleware = SessionMiddleware(mocker.Mock())
     middleware.process_request(request)
     request.session.save()
     strategy = load_strategy(request)
@@ -88,15 +88,15 @@ def test_user_password_not_email_backend(mocker):
 
 
 @pytest.mark.parametrize("user_password", ["abc123", "def456"])
-def test_user_password_login(rf, user, user_password):
+def test_user_password_login(mocker, rf, user, user_password):
     """Tests that user_password works for login case"""
     request_password = "abc123"
     user.set_password(user_password)
     user.save()
     request = rf.post(
         "/complete/email", {"password": request_password, "email": user.email}
     )
-    middleware = SessionMiddleware()
+    middleware = SessionMiddleware(mocker.Mock())
     middleware.process_request(request)
     request.session.save()
     strategy = load_strategy(request)
@@ -124,15 +124,15 @@ def test_user_password_login(rf, user, user_password):
             )
 
 
-def test_user_password_not_login(rf, user):
+def test_user_password_not_login(mocker, rf, user):
     """
     Tests that user_password performs denies authentication
     for an existing user if password not provided regardless of auth_type
     """
     user.set_password("abc123")
     user.save()
     request = rf.post("/complete/email", {"email": user.email})
-    middleware = SessionMiddleware()
+    middleware = SessionMiddleware(mocker.Mock())
     middleware.process_request(request)
     request.session.save()
     strategy = load_strategy(request)
@@ -148,12 +148,12 @@ def test_user_password_not_login(rf, user):
         )
 
 
-def test_user_password_not_exists(rf):
+def test_user_password_not_exists(mocker, rf):
     """Tests that user_password raises auth error for nonexistent user"""
     request = rf.post(
         "/complete/email", {"password": "abc123", "email": "doesntexist@localhost"}
     )
-    middleware = SessionMiddleware()
+    middleware = SessionMiddleware(mocker.Mock())
     middleware.process_request(request)
     request.session.save()
     strategy = load_strategy(request)

authentication/strategy.py

@@ -1,5 +1,4 @@
 """Custom strategy"""
-from rest_framework.request import Request
 from social_django.strategy import DjangoStrategy
 
 from authentication import api as auth_api
@@ -27,14 +26,6 @@ def __init__(self, storage, drf_request=None, tpl=None):
         request = drf_request._request  # pylint: disable=protected-access
         super().__init__(storage, request=request, tpl=tpl)
 
-    def clean_authenticate_args(self, *args, **kwargs):
-        """Cleanup request argument if present, which is passed to authenticate as for Django 1.11"""
-        # this is similar to what DjangoStrategy does, but is specific to DRF's Request type
-        if len(args) > 0 and isinstance(args[0], Request):
-            kwargs["request"], args = args[0], args[1:]
-
-        return super().clean_authenticate_args(*args, **kwargs)
-
     def request_data(self, merge=True):
         """Returns the request data"""
         if not self.drf_request:

authentication/urls.py

@@ -1,5 +1,5 @@
 """URL configurations for authentication"""
-from django.conf.urls import url
+from django.urls import re_path
 from django.contrib.auth import views as auth_views
 
 from authentication.views import (
@@ -15,43 +15,43 @@
 
 
 urlpatterns = [
-    url(r"^api/v0/login/email/$", LoginEmailView.as_view(), name="psa-login-email"),
-    url(
+    re_path(r"^api/v0/login/email/$", LoginEmailView.as_view(), name="psa-login-email"),
+    re_path(
         r"^api/v0/login/password/$",
         LoginPasswordView.as_view(),
         name="psa-login-password",
     ),
-    url(
+    re_path(
         r"^api/v0/register/email/$",
         RegisterEmailView.as_view(),
         name="psa-register-email",
     ),
-    url(
+    re_path(
         r"^api/v0/register/confirm/$",
         RegisterConfirmView.as_view(),
         name="psa-register-confirm",
     ),
-    url(
+    re_path(
         r"^api/v0/register/details/$",
         RegisterDetailsView.as_view(),
         name="psa-register-details",
     ),
-    url(
+    re_path(
         r"^api/v0/password_reset/$",
         CustomDjoserAPIView.as_view({"post": "reset_password"}),
         name="password-reset-api",
     ),
-    url(
+    re_path(
         r"^api/v0/password_reset/confirm/$",
         CustomDjoserAPIView.as_view({"post": "reset_password_confirm"}),
         name="password-reset-confirm-api",
     ),
-    url(
+    re_path(
         r"^api/v0/set_password/$",
         CustomDjoserAPIView.as_view({"post": "set_password"}),
         name="set-password-api",
     ),
-    url(r"^api/v0/auths/$", get_social_auth_types, name="get-auth-types-api"),
-    url(r"^login/complete$", login_complete, name="login-complete"),
-    url(r"^logout/$", auth_views.LogoutView.as_view(), name="logout"),
+    re_path(r"^api/v0/auths/$", get_social_auth_types, name="get-auth-types-api"),
+    re_path(r"^login/complete$", login_complete, name="login-complete"),
+    re_path(r"^logout/$", auth_views.LogoutView.as_view(), name="logout"),
 ]

authentication/views.py

@@ -45,7 +45,7 @@ def get_serializer_cls(self):  # pragma: no cover
 
     def post(self, request):
         """Processes a request"""
-        if request.session.get("is_hijacked_user", False):
+        if request._cached_user.is_hijacked:
             return Response(status=status.HTTP_403_FORBIDDEN)
 
         serializer_cls = self.get_serializer_cls()
@@ -87,14 +87,15 @@ def get_serializer_cls(self):
 
     def post(self, request):
         """Verify recaptcha response before proceeding"""
-        if request.session.get("is_hijacked_user", False):
+        if request._cached_user.is_hijacked:
             return Response(status=status.HTTP_403_FORBIDDEN)
         if settings.RECAPTCHA_SITE_KEY:
             r = requests.post(
                 "https://www.google.com/recaptcha/api/siteverify?secret={key}&response={captcha}".format(
                     key=quote(settings.RECAPTCHA_SECRET_KEY),
                     captcha=quote(request.data["recaptcha"]),
-                )
+                ),
+                timeout=settings.REQUESTS_TIMEOUT,
             )
             response = r.json()
             if not response["success"]:
@@ -134,7 +135,7 @@ def login_complete(request, **kwargs):  # pylint: disable=unused-argument
     """View that completes the login"""
     # redirect to home
     response = redirect("/")
-    if request.session.get("is_hijacked_user", False):
+    if request._cached_user.is_hijacked:
         return response
     if api_settings.JWT_AUTH_COOKIE in request.COOKIES:
         # to clear a cookie, it's most reliable to set it to expire immediately

authentication/views_test.py

@@ -2,6 +2,7 @@
 # pylint: disable=redefined-outer-name
 
 from django.contrib.auth import get_user, get_user_model
+from django.test import Client
 from django.urls import reverse
 import factory
 import pytest
@@ -13,8 +14,9 @@
 from authentication.serializers import PARTIAL_PIPELINE_TOKEN_KEY
 from authentication.utils import SocialAuthState
 from open_discussions import features
-from open_discussions.factories import UserSocialAuthFactory
+from open_discussions.factories import UserSocialAuthFactory, UserFactory
 from open_discussions.test_utils import any_instance_of, MockResponse
+from profiles.models import Profile
 
 pytestmark = [pytest.mark.django_db, pytest.mark.usefixtures("indexing_user")]
 lazy = pytest.lazy_fixture
@@ -41,6 +43,12 @@ def mm_user(user):
     return user
 
 
+@pytest.fixture(autouse=True)
+def admin_profile(admin_user):
+    """Create a profile for the admin user"""
+    Profile.objects.get_or_create(user=admin_user)
+
+
 @pytest.fixture
 def enrollment_job_mock(mocker):
     """Fixture for user enrollment celery task"""
@@ -649,8 +657,12 @@ def test_login_email_error(client, mocker):
 
 def test_login_email_hijacked(client, user, admin_user):
     """Test that a 403 response is returned for email login view if user is hijacked"""
+    admin_user = UserFactory.create(is_superuser=True)
+    user = UserFactory.create()
     client.force_login(admin_user)
-    client.post("/hijack/{}/".format(user.id))
+    client.post(
+        reverse("hijack:acquire"), data={"user_pk": user.pk}, format="multipart"
+    )
     response = client.post(
         reverse("psa-login-email"),
         {"flow": SocialAuthState.FLOW_LOGIN, "email": "[email protected]"},
@@ -661,7 +673,9 @@ def test_login_email_hijacked(client, user, admin_user):
 def test_register_email_hijacked(client, user, admin_user):
     """Test that a 403 response is returned for email register view if user is hijacked"""
     client.force_login(admin_user)
-    client.post("/hijack/{}/".format(user.id))
+    client.post(
+        reverse("hijack:acquire"), data={"user_pk": user.pk}, format="multipart"
+    )
     response = client.post(
         reverse("psa-register-email"),
         {"flow": SocialAuthState.FLOW_LOGIN, "email": "[email protected]"},
@@ -675,9 +689,12 @@ def test_login_complete(
     settings, client, logged_in_user, admin_user, test_jwt_token, hijacked
 ):  # pylint: disable=unused-argument
     """Verify that the jwt-complete view invalidates the JWT auth cookie"""
+    client = Client()
+    if test_jwt_token:
+        client.cookies[settings.OPEN_DISCUSSIONS_COOKIE_NAME] = test_jwt_token
     if hijacked:
         client.force_login(admin_user)
-        client.post("/hijack/{}/".format(logged_in_user.id))
+        client.post(reverse("hijack:acquire"), {"user_pk": logged_in_user.pk})
 
     response = client.get(reverse("login-complete"))