From 3df82003df04cae46f13e0c476183d132527f44b Mon Sep 17 00:00:00 2001 From: Longze Chen Date: Mon, 30 Sep 2024 09:57:10 -0400 Subject: [PATCH 1/2] Fix incorrectly generated CAS login URL --- framework/auth/cas.py | 5 ++--- tests/test_views.py | 4 ++-- website/project/views/contributor.py | 4 +--- 3 files changed, 5 insertions(+), 8 deletions(-) diff --git a/framework/auth/cas.py b/framework/auth/cas.py index be194e8093f..1084739fdc3 100644 --- a/framework/auth/cas.py +++ b/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) diff --git a/tests/test_views.py b/tests/test_views.py index 3d9de04fc36..f1dbaa3285d 100644 --- a/tests/test_views.py +++ b/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'] diff --git a/website/project/views/contributor.py b/website/project/views/contributor.py index bfe964b2e1e..56b6802d2a6 100644 --- a/website/project/views/contributor.py +++ b/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) From 414a3a756ef2316da8f7cdb890826b9a9a27708d Mon Sep 17 00:00:00 2001 From: Longze Chen Date: Mon, 30 Sep 2024 15:21:23 -0400 Subject: [PATCH 2/2] Fix incorrect tests --- tests/test_auth.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/tests/test_auth.py b/tests/test_auth.py index f59f1498a93..b59c1c065ab 100644 --- a/tests/test_auth.py +++ b/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')