From 0d629f4bcb4e930da7ae8b7c8c7065e8910e9f7c Mon Sep 17 00:00:00 2001 From: Rob Percival Date: Mon, 6 Nov 2023 12:05:13 +0000 Subject: [PATCH] Pass `redirect_name` from `do_complete` action to Backend Almost all other arguments to `do_complete` were being passed to the backend, except for this. Without it, the backend cannot reliably access the redirect URL in the session state (which is useful if the backend wants to set the redirect URL, e.g. based on SAML RelayState). --- social_core/actions.py | 2 +- social_core/tests/actions/actions.py | 5 +++- social_core/tests/actions/test_login.py | 33 ++++++++++++++++++++++++- 3 files changed, 37 insertions(+), 3 deletions(-) diff --git a/social_core/actions.py b/social_core/actions.py index 2d74149a7..4ac2b43ab 100644 --- a/social_core/actions.py +++ b/social_core/actions.py @@ -46,7 +46,7 @@ def do_complete(backend, login, user=None, redirect_name="next", *args, **kwargs # clean partial data after usage backend.strategy.clean_partial_pipeline(partial.token) else: - user = backend.complete(user=user, *args, **kwargs) + user = backend.complete(user=user, redirect_name=redirect_name, *args, **kwargs) # pop redirect value before the session is trashed on login(), but after # the pipeline so that the pipeline can change the redirect if needed diff --git a/social_core/tests/actions/actions.py b/social_core/tests/actions/actions.py index 017b4adc3..9db7a2fff 100644 --- a/social_core/tests/actions/actions.py +++ b/social_core/tests/actions/actions.py @@ -53,6 +53,7 @@ class BaseActionTest(unittest.TestCase): def __init__(self, *args, **kwargs): self.strategy = None + self.backend = None super().__init__(*args, **kwargs) def setUp(self): @@ -63,7 +64,9 @@ def setUp(self): TestAssociation.reset_cache() Backend = module_member("social_core.backends.github.GithubOAuth2") self.strategy = self.strategy or TestStrategy(TestStorage) - self.backend = Backend(self.strategy, redirect_uri="/complete/github") + self.backend = self.backend or Backend( + self.strategy, redirect_uri="/complete/github" + ) self.user = None def tearDown(self): diff --git a/social_core/tests/actions/test_login.py b/social_core/tests/actions/test_login.py index 7a5654907..09819fdfe 100644 --- a/social_core/tests/actions/test_login.py +++ b/social_core/tests/actions/test_login.py @@ -1,8 +1,33 @@ +from ...backends.base import BaseAuth from ...utils import PARTIAL_TOKEN_SESSION_NAME -from ..models import User +from ..models import TestUserSocialAuth, User from .actions import BaseActionTest +class BackendThatControlsRedirect(BaseAuth): + """ + A fake backend that sets the URL to redirect to after login. + + It is not always possible to set the redirect URL in the session state prior to auth and then retrieve it when + auth is complete, because the session cookie might not be available post-auth. For example, for SAML, a POST request + redirects the user from the IdP (Identity Provider) back to the SP (Service Provider) to complete the auth process, + but the session cookie will not be present if the session cookie's `SameSite` attribute is not set to "None". + To mitigate this, SAML provides a `RelayState` parameter to pass data like a redirect URL from the SP to the IdP + and back again. In that case, the redirect URL is only known in `auth_complete`, and must be communicated back to + the `do_complete` action via session state so that it can issue the intended redirect. + """ + + ACCESS_TOKEN_URL = "https://example.com/oauth/access_token" + + def auth_url(self): + return "https://example.com/oauth/auth?state=foo" + + def auth_complete(self, *args, **kwargs): + # Put the redirect URL in the session state, as this is where the `do_complete` action looks for it. + self.strategy.session_set(kwargs["redirect_name"], "/after-login") + return kwargs["user"] + + class LoginActionTest(BaseActionTest): def test_login(self): self.do_login() @@ -37,6 +62,12 @@ def test_redirect_value(self): redirect = self.do_login(after_complete_checks=False) self.assertEqual(redirect.url, "/after-login") + def test_redirect_value_set_by_backend(self): + self.backend = BackendThatControlsRedirect(self.strategy) + self.user = TestUserSocialAuth.create_user("test-user") + redirect = self.do_login(after_complete_checks=False) + self.assertEqual(redirect.url, "/after-login") + def test_login_with_invalid_partial_pipeline(self): def before_complete(): partial_token = self.strategy.session_get(PARTIAL_TOKEN_SESSION_NAME)