Merge tag '24.07.3' into develop

[ENG-6096] Fix incorrectly generated CAS login URL #10763

Author: cslzchen

framework/auth/cas.py

@@ -1,5 +1,4 @@
 from furl import furl
-from urllib.parse import unquote_plus
 
 from django.utils import timezone
 from rest_framework import status as http_status
@@ -304,11 +303,11 @@ def make_response_from_ticket(ticket, service_url):
                     f'CAS response - redirect existing external IdP login to verification key login: user=[{user._id}]',
                     LogLevel.INFO
                 )
-                return redirect(get_logout_url(unquote_plus(get_login_url(
+                return redirect(get_logout_url(get_login_url(
                     service_url,
                     username=user.username,
                     verification_key=user.verification_key
-                ))))
+                )))
 
             # if user is authenticated by CAS
             print_cas_log(f'CAS response - finalizing authentication: user=[{user._id}]', LogLevel.INFO)

tests/test_auth.py

@@ -135,10 +135,9 @@ def test_successful_external_login_cas_redirect(self, mock_service_validate, moc
         resp = cas.make_response_from_ticket(ticket, service_url)
         assert resp.status_code == 302, 'redirect to CAS login'
         assert quote_plus('/login?service=') in resp.location
-
-        # the valid username will be double quoted as it is furl quoted in both get_login_url and get_logout_url in order
-        assert quote_plus(f'username={user.username}') in resp.location
-        assert quote_plus(f'verification_key={user.verification_key}') in resp.location
+        # the valid username and verification key should be double-quoted
+        assert quote_plus(f'username={quote_plus(user.username)}') in resp.location
+        assert quote_plus(f'verification_key={quote_plus(user.verification_key)}') in resp.location
 
     @mock.patch('framework.auth.cas.get_user_from_cas_resp')
     @mock.patch('framework.auth.cas.CasClient.service_validate')

tests/test_views.py

@@ -2611,7 +2611,7 @@ def test_claim_user_when_user_is_registered_with_orcid(self, mock_response_from_
         assert res1.status_code == 302
         res = self.app.resolve_redirect(self.app.get(url))
         service_url = f'http://localhost{url}'
-        expected = cas.get_logout_url(service_url=unquote_plus(cas.get_login_url(service_url=service_url)))
+        expected = cas.get_logout_url(service_url=cas.get_login_url(service_url=service_url))
         assert res1.location == expected
 
         # user logged in with orcid automatically becomes a contributor
@@ -2631,7 +2631,7 @@ def test_claim_user_when_user_is_registered_with_orcid(self, mock_response_from_
         # And the redirect URL must equal to the originial service URL
         assert res.status_code == 302
         redirect_url = res.headers['Location']
-        assert unquote_plus(redirect_url) == url
+        assert redirect_url == url
         # The response of this request is expected have the `Set-Cookie` header with OSF cookie.
         # And the cookie must belong to the ORCiD user.
         raw_set_cookie = res.headers['Set-Cookie']

website/project/views/contributor.py

@@ -1,5 +1,3 @@
-from urllib.parse import unquote_plus
-
 from rest_framework import status as http_status
 
 from flask import request
@@ -669,7 +667,7 @@ def claim_user_registered(auth, node, **kwargs):
     current_user = auth.user
     current_session = get_session()
 
-    sign_out_url = cas.get_logout_url(service_url=unquote_plus(cas.get_login_url(service_url=request.url)))
+    sign_out_url = cas.get_logout_url(service_url=cas.get_login_url(service_url=request.url))
     if not current_user:
         return redirect(sign_out_url)