From 4978fdcb7a2b13ad29b0928dcd1e81833fc760a8 Mon Sep 17 00:00:00 2001 From: Braden MacDonald Date: Wed, 6 May 2015 11:16:36 -0700 Subject: [PATCH 1/9] Changes for compatibility with latest python-social-auth (0.2.7) --- common/djangoapps/oauth_exchange/forms.py | 5 +- .../oauth_exchange/tests/test_forms.py | 4 +- common/djangoapps/student/views.py | 11 ++- .../djangoapps/third_party_auth/pipeline.py | 9 ++- .../third_party_auth/tests/specs/base.py | 76 ++++++++++--------- .../tests/test_change_enrollment.py | 6 +- .../third_party_auth/tests/utils.py | 4 +- lms/envs/test.py | 4 +- requirements/edx/base.txt | 2 +- requirements/edx/github.txt | 1 + 10 files changed, 67 insertions(+), 55 deletions(-) diff --git a/common/djangoapps/oauth_exchange/forms.py b/common/djangoapps/oauth_exchange/forms.py index 772de6d156c8..74c48c875bf4 100644 --- a/common/djangoapps/oauth_exchange/forms.py +++ b/common/djangoapps/oauth_exchange/forms.py @@ -10,6 +10,7 @@ from provider.oauth2.models import Client from requests import HTTPError from social.backends import oauth as social_oauth +from social.exceptions import AuthException from third_party_auth import pipeline @@ -48,7 +49,7 @@ def clean(self): if self._errors: return {} - backend = self.request.social_strategy.backend + backend = self.request.backend if not isinstance(backend, social_oauth.BaseOAuth2): raise OAuthValidationError( { @@ -83,7 +84,7 @@ def clean(self): user = None try: user = backend.do_auth(self.cleaned_data.get("access_token")) - except HTTPError: + except (HTTPError, AuthException): pass if user and isinstance(user, User): self.cleaned_data["user"] = user diff --git a/common/djangoapps/oauth_exchange/tests/test_forms.py b/common/djangoapps/oauth_exchange/tests/test_forms.py index 392935c6ab21..1f8c236cfbdc 100644 --- a/common/djangoapps/oauth_exchange/tests/test_forms.py +++ b/common/djangoapps/oauth_exchange/tests/test_forms.py @@ -23,8 +23,10 @@ class AccessTokenExchangeFormTest(AccessTokenExchangeTestMixin): def setUp(self): super(AccessTokenExchangeFormTest, self).setUp() self.request = RequestFactory().post("dummy_url") + redirect_uri = 'dummy_redirect_url' SessionMiddleware().process_request(self.request) - self.request.social_strategy = social_utils.load_strategy(self.request, self.BACKEND) + self.request.social_strategy = social_utils.load_strategy(self.request) + self.request.backend = social_utils.load_backend(self.request.social_strategy, self.BACKEND, redirect_uri) def _assert_error(self, data, expected_error, expected_error_description): form = AccessTokenExchangeForm(request=self.request, data=data) diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index 278df30d887b..167e834cd5f7 100644 --- a/common/djangoapps/student/views.py +++ b/common/djangoapps/student/views.py @@ -1152,7 +1152,7 @@ def login_oauth_token(request, backend): """ warnings.warn("Please use AccessTokenExchangeView instead.", DeprecationWarning) - backend = request.social_strategy.backend + backend = request.backend if isinstance(backend, social_oauth.BaseOAuth1) or isinstance(backend, social_oauth.BaseOAuth2): if "access_token" in request.POST: # Tell third party auth pipeline that this is an API call @@ -1160,7 +1160,7 @@ def login_oauth_token(request, backend): user = None try: user = backend.do_auth(request.POST["access_token"]) - except HTTPError: + except (HTTPError, AuthException): pass # do_auth can return a non-User object if it fails if user and isinstance(user, User): @@ -1470,7 +1470,10 @@ def create_account_with_params(request, params): # next, link the account with social auth, if provided if should_link_with_social_auth: - request.social_strategy = social_utils.load_strategy(backend=params['provider'], request=request) + backend_name = params['provider'] + request.social_strategy = social_utils.load_strategy(request) + redirect_uri = reverse('social:complete', args=(backend_name, )) + request.backend = social_utils.load_backend(request.social_strategy, backend_name, redirect_uri) social_access_token = params.get('access_token') if not social_access_token: raise ValidationError({ @@ -1484,7 +1487,7 @@ def create_account_with_params(request, params): pipeline_user = None error_message = "" try: - pipeline_user = request.social_strategy.backend.do_auth(social_access_token, user=user) + pipeline_user = request.backend.do_auth(social_access_token, user=user) except AuthAlreadyAssociated: error_message = _("The provided access_token is already associated with another user.") except (HTTPError, AuthException): diff --git a/common/djangoapps/third_party_auth/pipeline.py b/common/djangoapps/third_party_auth/pipeline.py index 2ff7ea67b05d..d13d52ac20c6 100644 --- a/common/djangoapps/third_party_auth/pipeline.py +++ b/common/djangoapps/third_party_auth/pipeline.py @@ -445,7 +445,7 @@ def parse_query_params(strategy, response, *args, **kwargs): """Reads whitelisted query params, transforms them into pipeline args.""" auth_entry = strategy.session.get(AUTH_ENTRY_KEY) if not (auth_entry and auth_entry in _AUTH_ENTRY_CHOICES): - raise AuthEntryError(strategy.backend, 'auth_entry missing or invalid') + raise AuthEntryError(strategy.request.backend, 'auth_entry missing or invalid') return {'auth_entry': auth_entry} @@ -526,7 +526,7 @@ def _create_redirect_url(url, strategy): @partial.partial -def set_logged_in_cookie(backend=None, user=None, request=None, auth_entry=None, *args, **kwargs): +def set_logged_in_cookie(backend=None, user=None, strategy=None, auth_entry=None, *args, **kwargs): """This pipeline step sets the "logged in" cookie for authenticated users. Some installations have a marketing site front-end separate from @@ -552,6 +552,7 @@ def set_logged_in_cookie(backend=None, user=None, request=None, auth_entry=None, """ if not is_api(auth_entry) and user is not None and user.is_authenticated(): + request = strategy.request if strategy else None if request is not None: # Check that the cookie isn't already set. # This ensures that we allow the user to continue to the next @@ -692,7 +693,7 @@ def change_enrollment(strategy, auth_entry=None, user=None, *args, **kwargs): @partial.partial -def associate_by_email_if_login_api(auth_entry, strategy, details, user, *args, **kwargs): +def associate_by_email_if_login_api(auth_entry, backend, details, user, *args, **kwargs): """ This pipeline step associates the current social auth with the user with the same email address in the database. It defers to the social library's associate_by_email @@ -701,7 +702,7 @@ def associate_by_email_if_login_api(auth_entry, strategy, details, user, *args, This association is done ONLY if the user entered the pipeline through a LOGIN API. """ if auth_entry == AUTH_ENTRY_LOGIN_API: - association_response = associate_by_email(strategy, details, user, *args, **kwargs) + association_response = associate_by_email(backend, details, user, *args, **kwargs) if ( association_response and association_response.get('user') and diff --git a/common/djangoapps/third_party_auth/tests/specs/base.py b/common/djangoapps/third_party_auth/tests/specs/base.py index 902a1beb1d12..ce37ea67d806 100644 --- a/common/djangoapps/third_party_auth/tests/specs/base.py +++ b/common/djangoapps/third_party_auth/tests/specs/base.py @@ -140,7 +140,7 @@ def assert_exception_redirect_looks_correct(self, expected_uri, auth_entry=None) exception_middleware = middleware.ExceptionMiddleware() request, _ = self.get_request_and_strategy(auth_entry=auth_entry) response = exception_middleware.process_exception( - request, exceptions.AuthCanceled(request.social_strategy.backend)) + request, exceptions.AuthCanceled(request.backend)) location = response.get('Location') self.assertEqual(302, response.status_code) @@ -161,7 +161,7 @@ def assert_first_party_auth_trumps_third_party_auth(self, email=None, password=N """ _, strategy = self.get_request_and_strategy( auth_entry=pipeline.AUTH_ENTRY_LOGIN, redirect_uri='social:complete') - strategy.backend.auth_complete = mock.MagicMock(return_value=self.fake_auth_complete(strategy)) + strategy.request.backend.auth_complete = mock.MagicMock(return_value=self.fake_auth_complete(strategy)) self.create_user_models_for_existing_account( strategy, email, password, self.get_username(), skip_social_auth=True) @@ -287,7 +287,7 @@ def create_user_models_for_existing_account(self, strategy, email, password, use See student.views.register and student.views._do_create_account. """ response_data = self.get_response_data() - uid = strategy.backend.get_user_id(response_data, response_data) + uid = strategy.request.backend.get_user_id(response_data, response_data) user = social_utils.Storage.user.create_user(email=email, password=password, username=username) profile = student_models.UserProfile(user=user) profile.save() @@ -310,7 +310,7 @@ def fake_auth_complete(self, strategy): args = () kwargs = { 'request': strategy.request, - 'backend': strategy.backend, + 'backend': strategy.request.backend, 'user': None, 'response': self.get_response_data(), } @@ -355,8 +355,9 @@ def get_request_and_strategy(self, auth_entry=None, redirect_uri=None): if auth_entry: request.session[pipeline.AUTH_ENTRY_KEY] = auth_entry - strategy = social_utils.load_strategy(backend=self.backend_name, redirect_uri=redirect_uri, request=request) + strategy = social_utils.load_strategy(request=request) request.social_strategy = strategy + request.backend = social_utils.load_backend(strategy, self.backend_name, redirect_uri) return request, strategy @@ -404,7 +405,7 @@ def test_full_pipeline_succeeds_for_linking_account(self): # configure the backend, and mock out wire traffic. request, strategy = self.get_request_and_strategy( auth_entry=pipeline.AUTH_ENTRY_LOGIN, redirect_uri='social:complete') - strategy.backend.auth_complete = mock.MagicMock(return_value=self.fake_auth_complete(strategy)) + request.backend.auth_complete = mock.MagicMock(return_value=self.fake_auth_complete(strategy)) pipeline.analytics.track = mock.MagicMock() request.user = self.create_user_models_for_existing_account( strategy, 'user@example.com', 'password', self.get_username(), skip_social_auth=True) @@ -413,12 +414,12 @@ def test_full_pipeline_succeeds_for_linking_account(self): # expected state. self.client.get( pipeline.get_login_url(self.PROVIDER_CLASS.NAME, pipeline.AUTH_ENTRY_LOGIN)) - actions.do_complete(strategy, social_views._do_login) # pylint: disable-msg=protected-access + actions.do_complete(request.backend, social_views._do_login) # pylint: disable-msg=protected-access mako_middleware_process_request(strategy.request) student_views.signin_user(strategy.request) student_views.login_user(strategy.request) - actions.do_complete(strategy, social_views._do_login) # pylint: disable-msg=protected-access + actions.do_complete(request.backend, social_views._do_login) # pylint: disable-msg=protected-access # First we expect that we're in the unlinked state, and that there # really is no association in the backend. @@ -428,7 +429,7 @@ def test_full_pipeline_succeeds_for_linking_account(self): # We should be redirected back to the complete page, setting # the "logged in" cookie for the marketing site. self.assert_logged_in_cookie_redirect(actions.do_complete( - request.social_strategy, social_views._do_login, request.user, None, # pylint: disable-msg=protected-access + request.backend, social_views._do_login, request.user, None, # pylint: disable-msg=protected-access redirect_field_name=auth.REDIRECT_FIELD_NAME )) @@ -437,7 +438,7 @@ def test_full_pipeline_succeeds_for_linking_account(self): # Fire off the auth pipeline to link. self.assert_redirect_to_dashboard_looks_correct(actions.do_complete( - request.social_strategy, social_views._do_login, request.user, None, # pylint: disable-msg=protected-access + request.backend, social_views._do_login, request.user, None, # pylint: disable-msg=protected-access redirect_field_name=auth.REDIRECT_FIELD_NAME)) # Now we expect to be in the linked state, with a backend entry. @@ -449,7 +450,7 @@ def test_full_pipeline_succeeds_for_unlinking_account(self): # configure the backend, and mock out wire traffic. request, strategy = self.get_request_and_strategy( auth_entry=pipeline.AUTH_ENTRY_LOGIN, redirect_uri='social:complete') - strategy.backend.auth_complete = mock.MagicMock(return_value=self.fake_auth_complete(strategy)) + request.backend.auth_complete = mock.MagicMock(return_value=self.fake_auth_complete(strategy)) user = self.create_user_models_for_existing_account( strategy, 'user@example.com', 'password', self.get_username()) self.assert_social_auth_exists_for_user(user, strategy) @@ -461,12 +462,12 @@ def test_full_pipeline_succeeds_for_unlinking_account(self): # expected state. self.client.get( pipeline.get_login_url(self.PROVIDER_CLASS.NAME, pipeline.AUTH_ENTRY_LOGIN)) - actions.do_complete(strategy, social_views._do_login) # pylint: disable-msg=protected-access + actions.do_complete(request.backend, social_views._do_login) # pylint: disable-msg=protected-access mako_middleware_process_request(strategy.request) student_views.signin_user(strategy.request) student_views.login_user(strategy.request) - actions.do_complete(strategy, social_views._do_login, user=user) # pylint: disable-msg=protected-access + actions.do_complete(request.backend, social_views._do_login, user=user) # pylint: disable-msg=protected-access # First we expect that we're in the linked state, with a backend entry. self.assert_account_settings_context_looks_correct(account_settings_context(request), user, linked=True) @@ -474,7 +475,7 @@ def test_full_pipeline_succeeds_for_unlinking_account(self): # Fire off the disconnect pipeline to unlink. self.assert_redirect_to_dashboard_looks_correct(actions.do_disconnect( - request.social_strategy, request.user, None, redirect_field_name=auth.REDIRECT_FIELD_NAME)) + request.backend, request.user, None, redirect_field_name=auth.REDIRECT_FIELD_NAME)) # Now we expect to be in the unlinked state, with no backend entry. self.assert_account_settings_context_looks_correct(account_settings_context(request), user, linked=False) @@ -490,7 +491,8 @@ def test_linking_already_associated_account_raises_auth_already_associated(self) username = self.get_username() _, strategy = self.get_request_and_strategy( auth_entry=pipeline.AUTH_ENTRY_LOGIN, redirect_uri='social:complete') - strategy.backend.auth_complete = mock.MagicMock(return_value=self.fake_auth_complete(strategy)) + backend = strategy.request.backend + backend.auth_complete = mock.MagicMock(return_value=self.fake_auth_complete(strategy)) linked_user = self.create_user_models_for_existing_account(strategy, email, password, username) unlinked_user = social_utils.Storage.user.create_user( email='other_' + email, password=password, username='other_' + username) @@ -499,7 +501,7 @@ def test_linking_already_associated_account_raises_auth_already_associated(self) self.assert_social_auth_does_not_exist_for_user(unlinked_user, strategy) with self.assertRaises(exceptions.AuthAlreadyAssociated): - actions.do_complete(strategy, social_views._do_login, user=unlinked_user) # pylint: disable-msg=protected-access + actions.do_complete(backend, social_views._do_login, user=unlinked_user) # pylint: disable-msg=protected-access def test_already_associated_exception_populates_dashboard_with_error(self): # Instrument the pipeline with an exception. We test that the @@ -511,19 +513,19 @@ def test_already_associated_exception_populates_dashboard_with_error(self): # that the duplicate error has no effect on the state of the controls. request, strategy = self.get_request_and_strategy( auth_entry=pipeline.AUTH_ENTRY_LOGIN, redirect_uri='social:complete') - strategy.backend.auth_complete = mock.MagicMock(return_value=self.fake_auth_complete(strategy)) + strategy.request.backend.auth_complete = mock.MagicMock(return_value=self.fake_auth_complete(strategy)) user = self.create_user_models_for_existing_account( strategy, 'user@example.com', 'password', self.get_username()) self.assert_social_auth_exists_for_user(user, strategy) self.client.get('/login') self.client.get(pipeline.get_login_url(self.PROVIDER_CLASS.NAME, pipeline.AUTH_ENTRY_LOGIN)) - actions.do_complete(strategy, social_views._do_login) # pylint: disable-msg=protected-access + actions.do_complete(request.backend, social_views._do_login) # pylint: disable-msg=protected-access mako_middleware_process_request(strategy.request) student_views.signin_user(strategy.request) student_views.login_user(strategy.request) - actions.do_complete(strategy, social_views._do_login, user=user) # pylint: disable-msg=protected-access + actions.do_complete(request.backend, social_views._do_login, user=user) # pylint: disable-msg=protected-access # Monkey-patch storage for messaging; pylint: disable-msg=protected-access request._messages = fallback.FallbackStorage(request) @@ -539,7 +541,7 @@ def test_full_pipeline_succeeds_for_signing_in_to_existing_active_account(self): # configure the backend, and mock out wire traffic. request, strategy = self.get_request_and_strategy( auth_entry=pipeline.AUTH_ENTRY_LOGIN, redirect_uri='social:complete') - strategy.backend.auth_complete = mock.MagicMock(return_value=self.fake_auth_complete(strategy)) + strategy.request.backend.auth_complete = mock.MagicMock(return_value=self.fake_auth_complete(strategy)) pipeline.analytics.track = mock.MagicMock() user = self.create_user_models_for_existing_account( strategy, 'user@example.com', 'password', self.get_username()) @@ -559,7 +561,7 @@ def test_full_pipeline_succeeds_for_signing_in_to_existing_active_account(self): # Next, the provider makes a request against /auth/complete/ # to resume the pipeline. # pylint: disable-msg=protected-access - self.assert_redirect_to_login_looks_correct(actions.do_complete(strategy, social_views._do_login)) + self.assert_redirect_to_login_looks_correct(actions.do_complete(request.backend, social_views._do_login)) mako_middleware_process_request(strategy.request) # At this point we know the pipeline has resumed correctly. Next we @@ -574,7 +576,7 @@ def test_full_pipeline_succeeds_for_signing_in_to_existing_active_account(self): # We should be redirected back to the complete page, setting # the "logged in" cookie for the marketing site. self.assert_logged_in_cookie_redirect(actions.do_complete( - request.social_strategy, social_views._do_login, request.user, None, # pylint: disable-msg=protected-access + request.backend, social_views._do_login, request.user, None, # pylint: disable-msg=protected-access redirect_field_name=auth.REDIRECT_FIELD_NAME )) @@ -582,13 +584,13 @@ def test_full_pipeline_succeeds_for_signing_in_to_existing_active_account(self): self.set_logged_in_cookie(request) self.assert_redirect_to_dashboard_looks_correct( - actions.do_complete(strategy, social_views._do_login, user=user)) + actions.do_complete(request.backend, social_views._do_login, user=user)) self.assert_account_settings_context_looks_correct(account_settings_context(request), user) def test_signin_fails_if_account_not_active(self): _, strategy = self.get_request_and_strategy( auth_entry=pipeline.AUTH_ENTRY_LOGIN, redirect_uri='social:complete') - strategy.backend.auth_complete = mock.MagicMock(return_value=self.fake_auth_complete(strategy)) + strategy.request.backend.auth_complete = mock.MagicMock(return_value=self.fake_auth_complete(strategy)) user = self.create_user_models_for_existing_account(strategy, 'user@example.com', 'password', self.get_username()) user.is_active = False @@ -600,7 +602,7 @@ def test_signin_fails_if_account_not_active(self): def test_signin_fails_if_no_account_associated(self): _, strategy = self.get_request_and_strategy( auth_entry=pipeline.AUTH_ENTRY_LOGIN, redirect_uri='social:complete') - strategy.backend.auth_complete = mock.MagicMock(return_value=self.fake_auth_complete(strategy)) + strategy.request.backend.auth_complete = mock.MagicMock(return_value=self.fake_auth_complete(strategy)) self.create_user_models_for_existing_account( strategy, 'user@example.com', 'password', self.get_username(), skip_social_auth=True) @@ -625,7 +627,7 @@ def test_full_pipeline_succeeds_registering_new_account(self): # Mock out wire traffic. request, strategy = self.get_request_and_strategy( auth_entry=pipeline.AUTH_ENTRY_REGISTER, redirect_uri='social:complete') - strategy.backend.auth_complete = mock.MagicMock(return_value=self.fake_auth_complete(strategy)) + strategy.request.backend.auth_complete = mock.MagicMock(return_value=self.fake_auth_complete(strategy)) # Begin! Grab the registration page and check the login control on it. self.assert_register_response_before_pipeline_looks_correct(self.client.get('/register')) @@ -638,7 +640,7 @@ def test_full_pipeline_succeeds_registering_new_account(self): # Next, the provider makes a request against /auth/complete/. # pylint: disable-msg=protected-access - self.assert_redirect_to_register_looks_correct(actions.do_complete(strategy, social_views._do_login)) + self.assert_redirect_to_register_looks_correct(actions.do_complete(request.backend, social_views._do_login)) mako_middleware_process_request(strategy.request) # At this point we know the pipeline has resumed correctly. Next we @@ -675,7 +677,7 @@ def test_full_pipeline_succeeds_registering_new_account(self): # Since the user's account is not yet active, we should be redirected to /login self.assert_redirect_to_login_looks_correct( actions.do_complete( - request.social_strategy, social_views._do_login, request.user, None, # pylint: disable-msg=protected-access + request.backend, social_views._do_login, request.user, None, # pylint: disable-msg=protected-access redirect_field_name=auth.REDIRECT_FIELD_NAME ) ) @@ -687,7 +689,7 @@ def test_full_pipeline_succeeds_registering_new_account(self): # Try again. This time, we should be redirected back to the complete page, setting # the "logged in" cookie for the marketing site. self.assert_logged_in_cookie_redirect(actions.do_complete( - request.social_strategy, social_views._do_login, request.user, None, # pylint: disable-msg=protected-access + request.backend, social_views._do_login, request.user, None, # pylint: disable-msg=protected-access redirect_field_name=auth.REDIRECT_FIELD_NAME )) @@ -698,7 +700,7 @@ def test_full_pipeline_succeeds_registering_new_account(self): # and send the user to the dashboard, where the association will be # displayed. self.assert_redirect_to_dashboard_looks_correct( - actions.do_complete(strategy, social_views._do_login, user=created_user)) + actions.do_complete(strategy.request.backend, social_views._do_login, user=created_user)) self.assert_social_auth_exists_for_user(created_user, strategy) self.assert_account_settings_context_looks_correct(account_settings_context(request), created_user, linked=True) @@ -710,18 +712,20 @@ def test_new_account_registration_assigns_distinct_username_on_collision(self): # Create a colliding username in the backend, then proceed with # assignment via pipeline to make sure a distinct username is created. strategy.storage.user.create_user(username=self.get_username(), email='user@email.com', password='password') - strategy.backend.auth_complete = mock.MagicMock(return_value=self.fake_auth_complete(strategy)) + backend = strategy.request.backend + backend.auth_complete = mock.MagicMock(return_value=self.fake_auth_complete(strategy)) # pylint: disable-msg=protected-access - self.assert_redirect_to_register_looks_correct(actions.do_complete(strategy, social_views._do_login)) + self.assert_redirect_to_register_looks_correct(actions.do_complete(backend, social_views._do_login)) distinct_username = pipeline.get(request)['kwargs']['username'] self.assertNotEqual(original_username, distinct_username) def test_new_account_registration_fails_if_email_exists(self): request, strategy = self.get_request_and_strategy( auth_entry=pipeline.AUTH_ENTRY_REGISTER, redirect_uri='social:complete') - strategy.backend.auth_complete = mock.MagicMock(return_value=self.fake_auth_complete(strategy)) + backend = strategy.request.backend + backend.auth_complete = mock.MagicMock(return_value=self.fake_auth_complete(strategy)) # pylint: disable-msg=protected-access - self.assert_redirect_to_register_looks_correct(actions.do_complete(strategy, social_views._do_login)) + self.assert_redirect_to_register_looks_correct(actions.do_complete(backend, social_views._do_login)) mako_middleware_process_request(strategy.request) self.assert_register_response_in_pipeline_looks_correct( @@ -738,13 +742,13 @@ def test_pipeline_raises_auth_entry_error_if_auth_entry_invalid(self): _, strategy = self.get_request_and_strategy(auth_entry=auth_entry, redirect_uri='social:complete') with self.assertRaises(pipeline.AuthEntryError): - strategy.backend.auth_complete = mock.MagicMock(return_value=self.fake_auth_complete(strategy)) + strategy.request.backend.auth_complete = mock.MagicMock(return_value=self.fake_auth_complete(strategy)) def test_pipeline_raises_auth_entry_error_if_auth_entry_missing(self): _, strategy = self.get_request_and_strategy(auth_entry=None, redirect_uri='social:complete') with self.assertRaises(pipeline.AuthEntryError): - strategy.backend.auth_complete = mock.MagicMock(return_value=self.fake_auth_complete(strategy)) + strategy.request.backend.auth_complete = mock.MagicMock(return_value=self.fake_auth_complete(strategy)) class Oauth2IntegrationTest(IntegrationTest): # pylint: disable-msg=abstract-method diff --git a/common/djangoapps/third_party_auth/tests/test_change_enrollment.py b/common/djangoapps/third_party_auth/tests/test_change_enrollment.py index 3fa46556738f..b79328664a0c 100644 --- a/common/djangoapps/third_party_auth/tests/test_change_enrollment.py +++ b/common/djangoapps/third_party_auth/tests/test_change_enrollment.py @@ -182,6 +182,6 @@ def _fake_strategy(self): request.user = self.user request.session = cache.SessionStore() - return social_utils.load_strategy( - backend=self.BACKEND_NAME, request=request - ) + request.social_strategy = social_utils.load_strategy(request) + request.backend = social_utils.load_backend(request.social_strategy, self.BACKEND_NAME, redirect_uri='') + return request.social_strategy diff --git a/common/djangoapps/third_party_auth/tests/utils.py b/common/djangoapps/third_party_auth/tests/utils.py index 6dcc2662cd3e..208930cdf4eb 100644 --- a/common/djangoapps/third_party_auth/tests/utils.py +++ b/common/djangoapps/third_party_auth/tests/utils.py @@ -66,7 +66,7 @@ def _setup_provider_response_with_body(self, status, body): class ThirdPartyOAuthTestMixinFacebook(object): """Tests oauth with the Facebook backend""" BACKEND = "facebook" - USER_URL = "https://graph.facebook.com/me" + USER_URL = "https://graph.facebook.com/v2.3/me" # In facebook responses, the "id" field is used as the user's identifier UID_FIELD = "id" @@ -74,6 +74,6 @@ class ThirdPartyOAuthTestMixinFacebook(object): class ThirdPartyOAuthTestMixinGoogle(object): """Tests oauth with the Google backend""" BACKEND = "google-oauth2" - USER_URL = "https://www.googleapis.com/oauth2/v1/userinfo" + USER_URL = "https://www.googleapis.com/plus/v1/people/me" # In google-oauth2 responses, the "email" field is used as the user's identifier UID_FIELD = "email" diff --git a/lms/envs/test.py b/lms/envs/test.py index ff0946b28806..0508bcccf751 100644 --- a/lms/envs/test.py +++ b/lms/envs/test.py @@ -242,8 +242,8 @@ "SOCIAL_AUTH_GOOGLE_OAUTH2_SECRET": "test", }, "Facebook": { - "SOCIAL_AUTH_GOOGLE_OAUTH2_KEY": "test", - "SOCIAL_AUTH_GOOGLE_OAUTH2_SECRET": "test", + "SOCIAL_AUTH_FACEBOOK_KEY": "test", + "SOCIAL_AUTH_FACEBOOK_SECRET": "test", }, } diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index fb7d6ba80c2e..06d64c97eba4 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -68,7 +68,7 @@ pyparsing==2.0.1 python-memcached==1.48 python-openid==2.2.5 python-dateutil==2.1 -python-social-auth==0.1.23 +#python-social-auth==0.1.23 pytz==2015.2 pysrt==0.4.7 PyYAML==3.10 diff --git a/requirements/edx/github.txt b/requirements/edx/github.txt index 7ff63f03f089..344d9c72291d 100644 --- a/requirements/edx/github.txt +++ b/requirements/edx/github.txt @@ -25,6 +25,7 @@ git+https://github.com/mfogel/django-settings-context-processor.git@b758c3930862 # back to master when and if this fix is merged back. # fs==0.4.0 git+https://github.com/pmitros/pyfs.git@96e1922348bfe6d99201b9512a9ed946c87b7e0b +-e git+https://github.com/open-craft/python-social-auth.git@789e0dc93017f6956f95c2f07260136e5d20102b#egg=python-social-auth # Our libraries: -e git+https://github.com/edx/XBlock.git@aed464a0e2f7478e93157150ac04133a745f5f46#egg=XBlock From 52673f2ece1939a32bb388a7c94dce64b8c86ef9 Mon Sep 17 00:00:00 2001 From: Braden MacDonald Date: Sun, 3 May 2015 19:13:44 -0700 Subject: [PATCH 2/9] Remove some redundant code --- .../djangoapps/third_party_auth/provider.py | 36 ++++--------------- 1 file changed, 6 insertions(+), 30 deletions(-) diff --git a/common/djangoapps/third_party_auth/provider.py b/common/djangoapps/third_party_auth/provider.py index 4fc1de6a8d23..9f0809d42a48 100644 --- a/common/djangoapps/third_party_auth/provider.py +++ b/common/djangoapps/third_party_auth/provider.py @@ -36,7 +36,7 @@ def get_authentication_backend(cls): return '%s.%s' % (cls.BACKEND_CLASS.__module__, cls.BACKEND_CLASS.__name__) @classmethod - def get_email(cls, unused_provider_details): + def get_email(cls, provider_details): """Gets user's email address. Provider responses can contain arbitrary data. This method can be @@ -44,16 +44,16 @@ def get_email(cls, unused_provider_details): extracted by the social_details pipeline step. Args: - unused_provider_details: dict of string -> string. Data about the + provider_details: dict of string -> string. Data about the user passed back by the provider. Returns: String or None. The user's email address, if any. """ - return None + return provider_details.get('email') @classmethod - def get_name(cls, unused_provider_details): + def get_name(cls, provider_details): """Gets user's name. Provider responses can contain arbitrary data. This method can be @@ -61,13 +61,13 @@ def get_name(cls, unused_provider_details): extracted by the social_details pipeline step. Args: - unused_provider_details: dict of string -> string. Data about the + provider_details: dict of string -> string. Data about the user passed back by the provider. Returns: String or None. The user's full name, if any. """ - return None + return provider_details.get('fullname') @classmethod def get_register_form_data(cls, pipeline_kwargs): @@ -121,14 +121,6 @@ class GoogleOauth2(BaseProvider): 'SOCIAL_AUTH_GOOGLE_OAUTH2_SECRET': None, } - @classmethod - def get_email(cls, provider_details): - return provider_details.get('email') - - @classmethod - def get_name(cls, provider_details): - return provider_details.get('fullname') - class LinkedInOauth2(BaseProvider): """Provider for LinkedIn's Oauth2 auth system.""" @@ -141,14 +133,6 @@ class LinkedInOauth2(BaseProvider): 'SOCIAL_AUTH_LINKEDIN_OAUTH2_SECRET': None, } - @classmethod - def get_email(cls, provider_details): - return provider_details.get('email') - - @classmethod - def get_name(cls, provider_details): - return provider_details.get('fullname') - class FacebookOauth2(BaseProvider): """Provider for LinkedIn's Oauth2 auth system.""" @@ -161,14 +145,6 @@ class FacebookOauth2(BaseProvider): 'SOCIAL_AUTH_FACEBOOK_SECRET': None, } - @classmethod - def get_email(cls, provider_details): - return provider_details.get('email') - - @classmethod - def get_name(cls, provider_details): - return provider_details.get('fullname') - class Registry(object): """Singleton registry of third-party auth providers. From 457358784fc298ec7946493f3ce352ede2a3daf3 Mon Sep 17 00:00:00 2001 From: Braden MacDonald Date: Sun, 3 May 2015 22:49:10 -0700 Subject: [PATCH 3/9] Framework toward implementation of SAML2 TPA Provider --- .../djangoapps/third_party_auth/pipeline.py | 3 +- .../djangoapps/third_party_auth/provider.py | 49 +++++++++++++++++++ common/djangoapps/third_party_auth/saml.py | 21 ++++++++ common/djangoapps/third_party_auth/urls.py | 2 + common/djangoapps/third_party_auth/views.py | 22 +++++++++ common/lib/safe_lxml/safe_lxml/etree.py | 2 +- common/lib/xmodule/xmodule/x_module.py | 1 + lms/envs/aws.py | 17 +++++++ requirements/edx/github.txt | 4 +- 9 files changed, 118 insertions(+), 3 deletions(-) create mode 100644 common/djangoapps/third_party_auth/saml.py create mode 100644 common/djangoapps/third_party_auth/views.py diff --git a/common/djangoapps/third_party_auth/pipeline.py b/common/djangoapps/third_party_auth/pipeline.py index d13d52ac20c6..b70c51406144 100644 --- a/common/djangoapps/third_party_auth/pipeline.py +++ b/common/djangoapps/third_party_auth/pipeline.py @@ -359,7 +359,7 @@ def get_login_url(provider_name, auth_entry, redirect_url=None, enroll_course_id redirect_url=redirect_url, enroll_course_id=enroll_course_id, email_opt_in=email_opt_in - ) + ) + enabled_provider.get_url_query() # Some optional extra data that will show up as FIELDS_STORED_IN_SESSION def get_duplicate_provider(messages): @@ -396,6 +396,7 @@ def get_provider_user_states(user): List of ProviderUserState. The list of states of a user's account with each enabled provider. """ + # TODO: Fix this method to search by provider name, not backend name states = [] found_user_backends = [ social_auth.provider for social_auth in models.DjangoStorage.user.get_social_auth_for_user(user) diff --git a/common/djangoapps/third_party_auth/provider.py b/common/djangoapps/third_party_auth/provider.py index 9f0809d42a48..4a752a3cd9b8 100644 --- a/common/djangoapps/third_party_auth/provider.py +++ b/common/djangoapps/third_party_auth/provider.py @@ -5,6 +5,7 @@ """ from social.backends import google, linkedin, facebook +from .saml import SAMLAuthBackend _DEFAULT_ICON_CLASS = 'fa-signin' @@ -109,6 +110,10 @@ def merge_onto(cls, settings): for key, value in cls.SETTINGS.iteritems(): setattr(settings, key, value) + @classmethod + def get_url_query(self): + return '' + class GoogleOauth2(BaseProvider): """Provider for Google's Oauth2 auth system.""" @@ -146,6 +151,50 @@ class FacebookOauth2(BaseProvider): } +class SAMLProviderMixin(object): + """ Base class for SAML/Shibboleth providers """ + BACKEND_CLASS = SAMLAuthBackend + ICON_CLASS = 'fa-university' + + @classmethod + def get_url_query(cls): + return '&idp={}'.format(cls.IDP["id"]) + + +class TestShibAProvider(SAMLProviderMixin, BaseProvider): + """ Provider for testshib.org public Shibboleth test server. """ + NAME = 'TestShib A' + IDP = { + "id": "testshiba", # Required slug + "entity_id": "https://idp.testshib.org/idp/shibboleth", + "url": "https://idp.testshib.org/idp/profile/SAML2/Redirect/SSO", + "x509cert": """ + MIIEDjCCAvagAwIBAgIBADANBgkqhkiG9w0BAQUFADBnMQswCQYDVQQGEwJVUzEV + MBMGA1UECBMMUGVubnN5bHZhbmlhMRMwEQYDVQQHEwpQaXR0c2J1cmdoMREwDwYD + VQQKEwhUZXN0U2hpYjEZMBcGA1UEAxMQaWRwLnRlc3RzaGliLm9yZzAeFw0wNjA4 + MzAyMTEyMjVaFw0xNjA4MjcyMTEyMjVaMGcxCzAJBgNVBAYTAlVTMRUwEwYDVQQI + EwxQZW5uc3lsdmFuaWExEzARBgNVBAcTClBpdHRzYnVyZ2gxETAPBgNVBAoTCFRl + c3RTaGliMRkwFwYDVQQDExBpZHAudGVzdHNoaWIub3JnMIIBIjANBgkqhkiG9w0B + AQEFAAOCAQ8AMIIBCgKCAQEArYkCGuTmJp9eAOSGHwRJo1SNatB5ZOKqDM9ysg7C + yVTDClcpu93gSP10nH4gkCZOlnESNgttg0r+MqL8tfJC6ybddEFB3YBo8PZajKSe + 3OQ01Ow3yT4I+Wdg1tsTpSge9gEz7SrC07EkYmHuPtd71CHiUaCWDv+xVfUQX0aT + NPFmDixzUjoYzbGDrtAyCqA8f9CN2txIfJnpHE6q6CmKcoLADS4UrNPlhHSzd614 + kR/JYiks0K4kbRqCQF0Dv0P5Di+rEfefC6glV8ysC8dB5/9nb0yh/ojRuJGmgMWH + gWk6h0ihjihqiu4jACovUZ7vVOCgSE5Ipn7OIwqd93zp2wIDAQABo4HEMIHBMB0G + A1UdDgQWBBSsBQ869nh83KqZr5jArr4/7b+QazCBkQYDVR0jBIGJMIGGgBSsBQ86 + 9nh83KqZr5jArr4/7b+Qa6FrpGkwZzELMAkGA1UEBhMCVVMxFTATBgNVBAgTDFBl + bm5zeWx2YW5pYTETMBEGA1UEBxMKUGl0dHNidXJnaDERMA8GA1UEChMIVGVzdFNo + aWIxGTAXBgNVBAMTEGlkcC50ZXN0c2hpYi5vcmeCAQAwDAYDVR0TBAUwAwEB/zAN + BgkqhkiG9w0BAQUFAAOCAQEAjR29PhrCbk8qLN5MFfSVk98t3CT9jHZoYxd8QMRL + I4j7iYQxXiGJTT1FXs1nd4Rha9un+LqTfeMMYqISdDDI6tv8iNpkOAvZZUosVkUo + 93pv1T0RPz35hcHHYq2yee59HJOco2bFlcsH8JBXRSRrJ3Q7Eut+z9uo80JdGNJ4 + /SJy5UorZ8KazGj16lfJhOBXldgrhppQBb0Nq6HKHguqmwRfJ+WkxemZXzhediAj + Geka8nz8JjwxpUjAiSWYKLtJhGEaTqCYxCCX2Dw+dOTqUzHOZ7WKv4JXPK5G/Uhr + 8K/qhmFT2nIQi538n6rVYLeWj8Bbnl+ev0peYzxFyF5sQA== + """ + } + + class Registry(object): """Singleton registry of third-party auth providers. diff --git a/common/djangoapps/third_party_auth/saml.py b/common/djangoapps/third_party_auth/saml.py new file mode 100644 index 000000000000..0346e1946e15 --- /dev/null +++ b/common/djangoapps/third_party_auth/saml.py @@ -0,0 +1,21 @@ +""" +Slightly customized python-social-auth backend for SAML 2.0 support +""" + +from social.backends.saml import SAMLIdentityProvider, SAMLAuth + + +class SAMLAuthBackend(SAMLAuth): + """ + Customized version of SAMLAuth that gets the list of IdPs from third_party_auth's list of + enabled providers. + """ + name = "tpa-saml" + + def get_idp(self, idp_name): + """ Given the name of an IdP, get a SAMLIdentityProvider instance """ + from .provider import Registry # Import here to avoid circular import + for provider in Registry.enabled(): + if issubclass(provider.BACKEND_CLASS, SAMLAuth) and provider.IDP["id"] == idp_name: + return SAMLIdentityProvider(idp_name, **provider.IDP) + raise KeyError("SAML IdP {} not found.".format(idp_name)) diff --git a/common/djangoapps/third_party_auth/urls.py b/common/djangoapps/third_party_auth/urls.py index dc02425ef393..9aba64e88886 100644 --- a/common/djangoapps/third_party_auth/urls.py +++ b/common/djangoapps/third_party_auth/urls.py @@ -2,8 +2,10 @@ from django.conf.urls import include, patterns, url +from .views import saml_metadata_view urlpatterns = patterns( '', + url(r'^auth/saml/metadata.xml', saml_metadata_view), url(r'^auth/', include('social.apps.django_app.urls', namespace='social')), ) diff --git a/common/djangoapps/third_party_auth/views.py b/common/djangoapps/third_party_auth/views.py new file mode 100644 index 000000000000..d0948d9b82ac --- /dev/null +++ b/common/djangoapps/third_party_auth/views.py @@ -0,0 +1,22 @@ +from django.conf import settings +from django.core.urlresolvers import reverse +from django.http import HttpResponse, HttpResponseBadRequest, HttpResponseServerError +from django.views.decorators.csrf import csrf_exempt +from django.views.decorators.cache import never_cache +from social.apps.django_app.utils import load_strategy, load_backend + + +def saml_metadata_view(request): + """ + Get the Service Provider metadata for this edx-platform instance. + You must send this XML to any Shibboleth Identity Provider that you wish to use. + """ + complete_url = reverse('social:complete', args=("tpa-saml", )) + if settings.APPEND_SLASH and not complete_url.endswith('/'): + complete_url = complete_url + '/' # Required for consistency + saml_backend = load_backend(load_strategy(request), "tpa-saml", redirect_uri=complete_url) + metadata, errors = saml_backend.generate_metadata_xml() + + if not errors: + return HttpResponse(content=metadata, content_type='text/xml') + return HttpResponseServerError(content=', '.join(errors)) diff --git a/common/lib/safe_lxml/safe_lxml/etree.py b/common/lib/safe_lxml/safe_lxml/etree.py index 83052b22b639..1b5b9fad1aa5 100644 --- a/common/lib/safe_lxml/safe_lxml/etree.py +++ b/common/lib/safe_lxml/safe_lxml/etree.py @@ -9,7 +9,7 @@ from lxml.etree import * # pylint: disable=wildcard-import, unused-wildcard-import from lxml.etree import XMLParser as _XMLParser -from lxml.etree import _ElementTree # pylint: disable=unused-import +from lxml.etree import _Element, _ElementTree # pylint: disable=unused-import # This should be imported after lxml.etree so that it overrides the following attributes. from defusedxml.lxml import parse, fromstring, XML diff --git a/common/lib/xmodule/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py index 437000d8ad1e..659f5aef68d3 100644 --- a/common/lib/xmodule/xmodule/x_module.py +++ b/common/lib/xmodule/xmodule/x_module.py @@ -1697,6 +1697,7 @@ def render(self, block, view_name, context=None): integrate it into a larger whole. """ + context = context or {} if view_name in PREVIEW_VIEWS: block = self._get_student_block(block) diff --git a/lms/envs/aws.py b/lms/envs/aws.py index 12346c173efc..fe001c429ae4 100644 --- a/lms/envs/aws.py +++ b/lms/envs/aws.py @@ -510,6 +510,23 @@ ##### Third-party auth options ################################################ THIRD_PARTY_AUTH = AUTH_TOKENS.get('THIRD_PARTY_AUTH', THIRD_PARTY_AUTH) +##### SAML configuration for third_party_auth ##### + +if 'SOCIAL_AUTH_TPA_SAML_SP_ENTITY_ID' in ENV_TOKENS: + SOCIAL_AUTH_TPA_SAML_SP_ENTITY_ID = ENV_TOKENS.get('SOCIAL_AUTH_TPA_SAML_SP_ENTITY_ID') + SOCIAL_AUTH_TPA_SAML_SP_NAMEID_FORMAT = ENV_TOKENS.get('SOCIAL_AUTH_TPA_SAML_SP_NAMEID_FORMAT', 'unspecified') + SOCIAL_AUTH_TPA_SAML_ORG_INFO = ENV_TOKENS.get('SOCIAL_AUTH_TPA_SAML_ORG_INFO') + SOCIAL_AUTH_TPA_SAML_TECHNICAL_CONTACT = ENV_TOKENS.get( + 'SOCIAL_AUTH_TPA_SAML_TECHNICAL_CONTACT', + {"givenName": "Technical Support", "emailAddress": TECH_SUPPORT_EMAIL} + ) + SOCIAL_AUTH_TPA_SAML_SUPPORT_CONTACT = ENV_TOKENS.get( + 'SOCIAL_AUTH_TPA_SAML_SUPPORT_CONTACT', + {"givenName": "Support", "emailAddress": TECH_SUPPORT_EMAIL} + ) + SOCIAL_AUTH_TPA_SAML_SP_PUBLIC_CERT = AUTH_TOKENS.get('SOCIAL_AUTH_TPA_SAML_SP_PUBLIC_CERT') + SOCIAL_AUTH_TPA_SAML_SP_PRIVATE_KEY = AUTH_TOKENS.get('SOCIAL_AUTH_TPA_SAML_SP_PRIVATE_KEY') + ##### OAUTH2 Provider ############## if FEATURES.get('ENABLE_OAUTH2_PROVIDER'): OAUTH_OIDC_ISSUER = ENV_TOKENS['OAUTH_OIDC_ISSUER'] diff --git a/requirements/edx/github.txt b/requirements/edx/github.txt index 344d9c72291d..ac037001c68d 100644 --- a/requirements/edx/github.txt +++ b/requirements/edx/github.txt @@ -25,7 +25,9 @@ git+https://github.com/mfogel/django-settings-context-processor.git@b758c3930862 # back to master when and if this fix is merged back. # fs==0.4.0 git+https://github.com/pmitros/pyfs.git@96e1922348bfe6d99201b9512a9ed946c87b7e0b --e git+https://github.com/open-craft/python-social-auth.git@789e0dc93017f6956f95c2f07260136e5d20102b#egg=python-social-auth +# For SAML Support (To be moved to PyPi installation in base.txt once our changes are merged): +-e git+https://github.com/open-craft/python-saml.git@902b4ca21f71b7ba0467817962ed810e19a2927f#egg=python-saml +-e git+https://github.com/open-craft/python-social-auth.git@b4e741d97ecfd51fae9c8a8c1143db7a63dc6e59#egg=python-social-auth # Our libraries: -e git+https://github.com/edx/XBlock.git@aed464a0e2f7478e93157150ac04133a745f5f46#egg=XBlock From 5d15e9252b55096ac4eac255a6c2aea163262159 Mon Sep 17 00:00:00 2001 From: Braden MacDonald Date: Tue, 5 May 2015 23:47:32 -0700 Subject: [PATCH 4/9] Clean up url parameter code --- common/djangoapps/third_party_auth/pipeline.py | 11 ++++++++--- common/djangoapps/third_party_auth/provider.py | 10 ++++++---- 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/common/djangoapps/third_party_auth/pipeline.py b/common/djangoapps/third_party_auth/pipeline.py index b70c51406144..5d139e4fafba 100644 --- a/common/djangoapps/third_party_auth/pipeline.py +++ b/common/djangoapps/third_party_auth/pipeline.py @@ -260,7 +260,8 @@ def _get_enabled_provider_by_name(provider_name): return enabled_provider -def _get_url(view_name, backend_name, auth_entry=None, redirect_url=None, enroll_course_id=None, email_opt_in=None): +def _get_url(view_name, backend_name, auth_entry=None, redirect_url=None, + enroll_course_id=None, email_opt_in=None, extra_params=None): """Creates a URL to hook into social auth endpoints.""" kwargs = {'backend': backend_name} url = reverse(view_name, kwargs=kwargs) @@ -278,6 +279,9 @@ def _get_url(view_name, backend_name, auth_entry=None, redirect_url=None, enroll if email_opt_in: query_params[AUTH_EMAIL_OPT_IN_KEY] = email_opt_in + if extra_params: + query_params.update(extra_params) + return u"{url}?{params}".format( url=url, params=urllib.urlencode(query_params) @@ -358,8 +362,9 @@ def get_login_url(provider_name, auth_entry, redirect_url=None, enroll_course_id auth_entry=auth_entry, redirect_url=redirect_url, enroll_course_id=enroll_course_id, - email_opt_in=email_opt_in - ) + enabled_provider.get_url_query() # Some optional extra data that will show up as FIELDS_STORED_IN_SESSION + email_opt_in=email_opt_in, + extra_params=enabled_provider.get_url_params(), + ) def get_duplicate_provider(messages): diff --git a/common/djangoapps/third_party_auth/provider.py b/common/djangoapps/third_party_auth/provider.py index 4a752a3cd9b8..00b5fc56a41b 100644 --- a/common/djangoapps/third_party_auth/provider.py +++ b/common/djangoapps/third_party_auth/provider.py @@ -111,8 +111,9 @@ def merge_onto(cls, settings): setattr(settings, key, value) @classmethod - def get_url_query(self): - return '' + def get_url_params(cls): + """ Get a dict of GET parameters to append to login links for this provider """ + return {} class GoogleOauth2(BaseProvider): @@ -157,8 +158,9 @@ class SAMLProviderMixin(object): ICON_CLASS = 'fa-university' @classmethod - def get_url_query(cls): - return '&idp={}'.format(cls.IDP["id"]) + def get_url_params(cls): + """ Get a dict of GET parameters to append to login links for this provider """ + return {'idp': cls.IDP["id"]} class TestShibAProvider(SAMLProviderMixin, BaseProvider): From 9f0c7a9bac93f86fff436a81a6d5ad8d5dd7a6c2 Mon Sep 17 00:00:00 2001 From: Braden MacDonald Date: Tue, 5 May 2015 14:21:36 -0700 Subject: [PATCH 5/9] Incorporate fixes to metadata generation --- common/djangoapps/third_party_auth/provider.py | 2 ++ lms/envs/aws.py | 2 ++ requirements/edx/github.txt | 4 ++-- 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/common/djangoapps/third_party_auth/provider.py b/common/djangoapps/third_party_auth/provider.py index 00b5fc56a41b..278914d7fdb2 100644 --- a/common/djangoapps/third_party_auth/provider.py +++ b/common/djangoapps/third_party_auth/provider.py @@ -5,6 +5,7 @@ """ from social.backends import google, linkedin, facebook +from social.backends.saml import OID_EDU_PERSON_PRINCIPAL_NAME from .saml import SAMLAuthBackend _DEFAULT_ICON_CLASS = 'fa-signin' @@ -170,6 +171,7 @@ class TestShibAProvider(SAMLProviderMixin, BaseProvider): "id": "testshiba", # Required slug "entity_id": "https://idp.testshib.org/idp/shibboleth", "url": "https://idp.testshib.org/idp/profile/SAML2/Redirect/SSO", + "attr_email": OID_EDU_PERSON_PRINCIPAL_NAME, "x509cert": """ MIIEDjCCAvagAwIBAgIBADANBgkqhkiG9w0BAQUFADBnMQswCQYDVQQGEwJVUzEV MBMGA1UECBMMUGVubnN5bHZhbmlhMRMwEQYDVQQHEwpQaXR0c2J1cmdoMREwDwYD diff --git a/lms/envs/aws.py b/lms/envs/aws.py index fe001c429ae4..ceb89ad13bdd 100644 --- a/lms/envs/aws.py +++ b/lms/envs/aws.py @@ -515,6 +515,7 @@ if 'SOCIAL_AUTH_TPA_SAML_SP_ENTITY_ID' in ENV_TOKENS: SOCIAL_AUTH_TPA_SAML_SP_ENTITY_ID = ENV_TOKENS.get('SOCIAL_AUTH_TPA_SAML_SP_ENTITY_ID') SOCIAL_AUTH_TPA_SAML_SP_NAMEID_FORMAT = ENV_TOKENS.get('SOCIAL_AUTH_TPA_SAML_SP_NAMEID_FORMAT', 'unspecified') + SOCIAL_AUTH_TPA_SAML_SP_EXTRA = ENV_TOKENS.get('SOCIAL_AUTH_TPA_SAML_SP_EXTRA', {}) SOCIAL_AUTH_TPA_SAML_ORG_INFO = ENV_TOKENS.get('SOCIAL_AUTH_TPA_SAML_ORG_INFO') SOCIAL_AUTH_TPA_SAML_TECHNICAL_CONTACT = ENV_TOKENS.get( 'SOCIAL_AUTH_TPA_SAML_TECHNICAL_CONTACT', @@ -524,6 +525,7 @@ 'SOCIAL_AUTH_TPA_SAML_SUPPORT_CONTACT', {"givenName": "Support", "emailAddress": TECH_SUPPORT_EMAIL} ) + SOCIAL_AUTH_TPA_SAML_SECURITY_CONFIG = ENV_TOKENS.get('SOCIAL_AUTH_TPA_SAML_SECURITY_CONFIG', {}) SOCIAL_AUTH_TPA_SAML_SP_PUBLIC_CERT = AUTH_TOKENS.get('SOCIAL_AUTH_TPA_SAML_SP_PUBLIC_CERT') SOCIAL_AUTH_TPA_SAML_SP_PRIVATE_KEY = AUTH_TOKENS.get('SOCIAL_AUTH_TPA_SAML_SP_PRIVATE_KEY') diff --git a/requirements/edx/github.txt b/requirements/edx/github.txt index ac037001c68d..73f9a08bd473 100644 --- a/requirements/edx/github.txt +++ b/requirements/edx/github.txt @@ -26,8 +26,8 @@ git+https://github.com/mfogel/django-settings-context-processor.git@b758c3930862 # fs==0.4.0 git+https://github.com/pmitros/pyfs.git@96e1922348bfe6d99201b9512a9ed946c87b7e0b # For SAML Support (To be moved to PyPi installation in base.txt once our changes are merged): --e git+https://github.com/open-craft/python-saml.git@902b4ca21f71b7ba0467817962ed810e19a2927f#egg=python-saml --e git+https://github.com/open-craft/python-social-auth.git@b4e741d97ecfd51fae9c8a8c1143db7a63dc6e59#egg=python-social-auth +-e git+https://github.com/open-craft/python-saml.git@9602b8133056d8c3caa7c3038761147df3d4b257#egg=python-saml +-e git+https://github.com/open-craft/python-social-auth.git@11d1cd31aefd9f920eefeb2d7f472f3e0fb45038#egg=python-social-auth # Our libraries: -e git+https://github.com/edx/XBlock.git@aed464a0e2f7478e93157150ac04133a745f5f46#egg=XBlock From dc5eb3697a2c3f3e8dc090b22e7088be5fccfa40 Mon Sep 17 00:00:00 2001 From: Braden MacDonald Date: Tue, 5 May 2015 22:38:12 -0700 Subject: [PATCH 6/9] Changes required for multiple TPA providers to share one PSA backend --- common/djangoapps/student/views.py | 9 +-- .../djangoapps/third_party_auth/pipeline.py | 33 +++++----- .../djangoapps/third_party_auth/provider.py | 65 +++++++++++++++++-- .../third_party_auth/tests/specs/base.py | 4 +- .../tests/test_pipeline_integration.py | 10 +-- .../third_party_auth/tests/test_provider.py | 23 ++++--- .../student_account/test/test_views.py | 2 +- lms/djangoapps/student_account/views.py | 5 +- .../_dashboard_third_party_error.html | 2 +- openedx/core/djangoapps/user_api/views.py | 2 +- 10 files changed, 106 insertions(+), 49 deletions(-) diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index 167e834cd5f7..132e36cf666e 100644 --- a/common/djangoapps/student/views.py +++ b/common/djangoapps/student/views.py @@ -437,7 +437,7 @@ def register_user(request, extra_context=None): # selected provider. if third_party_auth.is_enabled() and pipeline.running(request): running_pipeline = pipeline.get(request) - current_provider = provider.Registry.get_by_backend_name(running_pipeline.get('backend')) + current_provider = provider.Registry.get_from_pipeline(running_pipeline) overrides = current_provider.get_register_form_data(running_pipeline.get('kwargs')) overrides['running_pipeline'] = running_pipeline overrides['selected_provider'] = current_provider.NAME @@ -961,10 +961,11 @@ def login_user(request, error=""): # pylint: disable-msg=too-many-statements,un running_pipeline = pipeline.get(request) username = running_pipeline['kwargs'].get('username') backend_name = running_pipeline['backend'] - requested_provider = provider.Registry.get_by_backend_name(backend_name) + third_party_uid = running_pipeline['kwargs']['uid'] + requested_provider = provider.Registry.get_from_pipeline(running_pipeline) try: - user = pipeline.get_authenticated_user(username, backend_name) + user = pipeline.get_authenticated_user(requested_provider, username, third_party_uid) third_party_auth_successful = True except User.DoesNotExist: AUDIT_LOG.warning( @@ -1520,7 +1521,7 @@ def create_account_with_params(request, params): provider_name = None if third_party_auth.is_enabled() and pipeline.running(request): running_pipeline = pipeline.get(request) - current_provider = provider.Registry.get_by_backend_name(running_pipeline.get('backend')) + current_provider = provider.Registry.get_from_pipeline(running_pipeline) provider_name = current_provider.NAME analytics.track( diff --git a/common/djangoapps/third_party_auth/pipeline.py b/common/djangoapps/third_party_auth/pipeline.py index 5d139e4fafba..ddd5cafdfd8e 100644 --- a/common/djangoapps/third_party_auth/pipeline.py +++ b/common/djangoapps/third_party_auth/pipeline.py @@ -218,7 +218,7 @@ def get(request): return request.session.get('partial_pipeline') -def get_authenticated_user(username, backend_name): +def get_authenticated_user(provider, username, uid): """Gets a saved user authenticated by a particular backend. Between pipeline steps User objects are not saved. We need to reconstitute @@ -227,26 +227,26 @@ def get_authenticated_user(username, backend_name): authenticate(). Args: + provider: the third_party_auth provider in use for the current pipeline. username: string. Username of user to get. - backend_name: string. The name of the third-party auth backend from - the running pipeline. + uid: string. The user ID according to the third party. Returns: User if user is found and has a social auth from the passed - backend_name. + provider. Raises: User.DoesNotExist: if no user matching user is found, or the matching user has no social auth associated with the given backend. AssertionError: if the user is not authenticated. """ - user = models.DjangoStorage.user.user_model().objects.get(username=username) - match = models.DjangoStorage.user.get_social_auth_for_user(user, provider=backend_name) + match = models.DjangoStorage.user.get_social_auth(provider=provider.BACKEND_CLASS.name, uid=uid) - if not match: + if not match or match.user.username != username: raise User.DoesNotExist - user.backend = provider.Registry.get_by_backend_name(backend_name).get_authentication_backend() + user = match.user + user.backend = provider.get_authentication_backend() return user @@ -301,9 +301,7 @@ def get_complete_url(backend_name): Raises: ValueError: if no provider is enabled with the given backend_name. """ - enabled_provider = provider.Registry.get_by_backend_name(backend_name) - - if not enabled_provider: + if not any(provider.Registry.get_enabled_by_backend_name(backend_name)): raise ValueError('Provider with backend %s not enabled' % backend_name) return _get_url('social:complete', backend_name) @@ -379,7 +377,7 @@ def get_duplicate_provider(messages): unfortunately not in a reusable constant. Returns: - provider.BaseProvider child instance. The provider of the duplicate + string name of the python-social-auth backend that has the duplicate account, or None if there is no duplicate (and hence no error). """ social_auth_messages = [m for m in messages if m.message.endswith('is already in use.')] @@ -388,7 +386,8 @@ def get_duplicate_provider(messages): return assert len(social_auth_messages) == 1 - return provider.Registry.get_by_backend_name(social_auth_messages[0].extra_tags.split()[1]) + backend_name = social_auth_messages[0].extra_tags.split()[1] + return backend_name def get_provider_user_states(user): @@ -401,15 +400,13 @@ def get_provider_user_states(user): List of ProviderUserState. The list of states of a user's account with each enabled provider. """ - # TODO: Fix this method to search by provider name, not backend name states = [] - found_user_backends = [ - social_auth.provider for social_auth in models.DjangoStorage.user.get_social_auth_for_user(user) - ] + found_user_auths = list(models.DjangoStorage.user.get_social_auth_for_user(user)) for enabled_provider in provider.Registry.enabled(): + is_connected = any(enabled_provider.match_social_auth(auth) for auth in found_user_auths) states.append( - ProviderUserState(enabled_provider, user, enabled_provider.BACKEND_CLASS.name in found_user_backends) + ProviderUserState(enabled_provider, user, is_connected) ) return states diff --git a/common/djangoapps/third_party_auth/provider.py b/common/djangoapps/third_party_auth/provider.py index 278914d7fdb2..48da6203cc91 100644 --- a/common/djangoapps/third_party_auth/provider.py +++ b/common/djangoapps/third_party_auth/provider.py @@ -116,6 +116,16 @@ def get_url_params(cls): """ Get a dict of GET parameters to append to login links for this provider """ return {} + @classmethod + def is_active_for_pipeline(cls, pipeline): + """ Is this provider being used for the specified pipeline? """ + return cls.BACKEND_CLASS.name == pipeline['backend'] + + @classmethod + def match_social_auth(cls, social_auth): + """ Is this provider being used for this UserSocialAuth entry? """ + return cls.BACKEND_CLASS.name == social_auth.provider + class GoogleOauth2(BaseProvider): """Provider for Google's Oauth2 auth system.""" @@ -163,6 +173,19 @@ def get_url_params(cls): """ Get a dict of GET parameters to append to login links for this provider """ return {'idp': cls.IDP["id"]} + @classmethod + def is_active_for_pipeline(cls, pipeline): + """ Is this provider being used for the specified pipeline? """ + if cls.BACKEND_CLASS.name == pipeline['backend']: + idp_name = pipeline['kwargs']['response']['idp_name'] + return cls.IDP["id"] == idp_name + + @classmethod + def match_social_auth(cls, social_auth): + """ Is this provider being used for this UserSocialAuth entry? """ + prefix = cls.IDP["id"] + ":" + return cls.BACKEND_CLASS.name == social_auth.provider and social_auth.uid.startswith(prefix) + class TestShibAProvider(SAMLProviderMixin, BaseProvider): """ Provider for testshib.org public Shibboleth test server. """ @@ -199,6 +222,18 @@ class TestShibAProvider(SAMLProviderMixin, BaseProvider): } +class TestShibBProvider(SAMLProviderMixin, BaseProvider): + """ Provider for testshib.org public Shibboleth test server. """ + NAME = 'TestShib B' + IDP = { + "id": "testshibB", # Required slug + "entity_id": "https://idp.testshib.org/idp/shibboleth", + "url": "https://IDP.TESTSHIB.ORG/idp/profile/SAML2/Redirect/SSO", + "attr_email": OID_EDU_PERSON_PRINCIPAL_NAME, + "x509cert": TestShibAProvider.IDP["x509cert"], + } + + class Registry(object): """Singleton registry of third-party auth providers. @@ -264,22 +299,40 @@ def get(cls, provider_name): return cls._ENABLED.get(provider_name) @classmethod - def get_by_backend_name(cls, backend_name): - """Gets provider (or None) by backend name. + def get_from_pipeline(cls, running_pipeline): + """Gets the provider that is being used for the specified pipeline (or None). Args: - backend_name: string. The python-social-auth - backends.base.BaseAuth.name (for example, 'google-oauth2') to - try and get a provider for. + running_pipeline: The python-social-auth pipeline being used to + authenticate a user. Raises: RuntimeError: if the registry has not been configured. """ cls._check_configured() for enabled in cls._ENABLED.values(): - if enabled.BACKEND_CLASS.name == backend_name: + if enabled.is_active_for_pipeline(running_pipeline): return enabled + + @classmethod + def get_enabled_by_backend_name(cls, backend_name): + """Generator returning all enabled providers that use the specified + backend. + + Args: + backend_name: The name of a python-social-auth backend used by + one or more providers. + + Raises: + RuntimeError: if the registry has not been configured. + """ + cls._check_configured() + for enabled in cls._ENABLED.values(): + if enabled.BACKEND_CLASS.name == backend_name: + yield enabled + + @classmethod def _reset(cls): """Returns the registry to an unconfigured state; for tests only.""" diff --git a/common/djangoapps/third_party_auth/tests/specs/base.py b/common/djangoapps/third_party_auth/tests/specs/base.py index ce37ea67d806..e9aba5df41d9 100644 --- a/common/djangoapps/third_party_auth/tests/specs/base.py +++ b/common/djangoapps/third_party_auth/tests/specs/base.py @@ -115,12 +115,12 @@ def assert_account_settings_context_looks_correct(self, context, user, duplicate """Asserts the user's account settings page context is in the expected state. If duplicate is True, we expect context['duplicate_provider'] to contain - the duplicate provider object. If linked is passed, we conditionally + the duplicate provider backend name. If linked is passed, we conditionally check that the provider is included in context['auth']['providers'] and its connected state is correct. """ if duplicate: - self.assertEqual(context['duplicate_provider'].NAME, self.PROVIDER_CLASS.NAME) + self.assertEqual(context['duplicate_provider'], self.PROVIDER_CLASS.BACKEND_CLASS.name) else: self.assertIsNone(context['duplicate_provider']) diff --git a/common/djangoapps/third_party_auth/tests/test_pipeline_integration.py b/common/djangoapps/third_party_auth/tests/test_pipeline_integration.py index 8d1f3b7019ae..4812a4748907 100644 --- a/common/djangoapps/third_party_auth/tests/test_pipeline_integration.py +++ b/common/djangoapps/third_party_auth/tests/test_pipeline_integration.py @@ -41,16 +41,16 @@ def get_by_username(self, username): def test_raises_does_not_exist_if_user_missing(self): with self.assertRaises(models.User.DoesNotExist): - pipeline.get_authenticated_user('new_' + self.user.username, 'backend') + pipeline.get_authenticated_user(self.enabled_provider, 'new_' + self.user.username, 'user@example.com') def test_raises_does_not_exist_if_user_found_but_no_association(self): backend_name = 'backend' self.assertIsNotNone(self.get_by_username(self.user.username)) - self.assertIsNone(provider.Registry.get_by_backend_name(backend_name)) + self.assertFalse(any(provider.Registry.get_enabled_by_backend_name(backend_name))) with self.assertRaises(models.User.DoesNotExist): - pipeline.get_authenticated_user(self.user.username, 'backend') + pipeline.get_authenticated_user(self.enabled_provider, self.user.username, 'user@example.com') def test_raises_does_not_exist_if_user_and_association_found_but_no_match(self): self.assertIsNotNone(self.get_by_username(self.user.username)) @@ -58,11 +58,11 @@ def test_raises_does_not_exist_if_user_and_association_found_but_no_match(self): self.user, 'uid', 'other_' + self.enabled_provider.BACKEND_CLASS.name) with self.assertRaises(models.User.DoesNotExist): - pipeline.get_authenticated_user(self.user.username, self.enabled_provider.BACKEND_CLASS.name) + pipeline.get_authenticated_user(self.enabled_provider, self.user.username, 'uid') def test_returns_user_with_is_authenticated_and_backend_set_if_match(self): social_models.DjangoStorage.user.create_social_auth(self.user, 'uid', self.enabled_provider.BACKEND_CLASS.name) - user = pipeline.get_authenticated_user(self.user.username, self.enabled_provider.BACKEND_CLASS.name) + user = pipeline.get_authenticated_user(self.enabled_provider, self.user.username, 'uid') self.assertEqual(self.user, user) self.assertEqual(self.enabled_provider.get_authentication_backend(), user.backend) diff --git a/common/djangoapps/third_party_auth/tests/test_provider.py b/common/djangoapps/third_party_auth/tests/test_provider.py index 20120d73290c..a1de2943bdd3 100644 --- a/common/djangoapps/third_party_auth/tests/test_provider.py +++ b/common/djangoapps/third_party_auth/tests/test_provider.py @@ -1,5 +1,6 @@ """Unit tests for provider.py.""" +from mock import Mock from third_party_auth import provider from third_party_auth.tests import testutil @@ -67,16 +68,22 @@ def test_get_returns_none_if_provider_not_enabled(self): provider.Registry.configure_once([]) self.assertIsNone(provider.Registry.get(provider.LinkedInOauth2.NAME)) - def test_get_by_backend_name_raises_runtime_error_if_not_configured(self): + def test_get_from_pipeline_returns_none_if_provider_not_enabled(self): + provider.Registry.configure_once([]) + self.assertIsNone(provider.Registry.get_from_pipeline(Mock())) + + def test_get_enabled_by_backend_name_raises_runtime_error_if_not_configured(self): with self.assertRaisesRegexp(RuntimeError, '^.*not configured$'): - provider.Registry.get_by_backend_name('') + provider.Registry.get_enabled_by_backend_name('').next() - def test_get_by_backend_name_returns_enabled_provider(self): + def test_get_enabled_by_backend_name_returns_enabled_provider(self): provider.Registry.configure_once([provider.GoogleOauth2.NAME]) - self.assertIs( - provider.GoogleOauth2, - provider.Registry.get_by_backend_name(provider.GoogleOauth2.BACKEND_CLASS.name)) + found = list(provider.Registry.get_enabled_by_backend_name(provider.GoogleOauth2.BACKEND_CLASS.name)) + self.assertEqual(found, [provider.GoogleOauth2]) - def test_get_by_backend_name_returns_none_if_provider_not_enabled(self): + def test_get_enabled_by_backend_name_returns_none_if_provider_not_enabled(self): provider.Registry.configure_once([]) - self.assertIsNone(provider.Registry.get_by_backend_name(provider.GoogleOauth2.BACKEND_CLASS.name)) + self.assertEqual( + [], + list(provider.Registry.get_enabled_by_backend_name(provider.GoogleOauth2.BACKEND_CLASS.name)) + ) diff --git a/lms/djangoapps/student_account/test/test_views.py b/lms/djangoapps/student_account/test/test_views.py index 880a622b7466..50674a0f986f 100644 --- a/lms/djangoapps/student_account/test/test_views.py +++ b/lms/djangoapps/student_account/test/test_views.py @@ -555,7 +555,7 @@ def test_context(self): context['user_preferences_api_url'], reverse('preferences_api', kwargs={'username': self.user.username}) ) - self.assertEqual(context['duplicate_provider'].BACKEND_CLASS.name, 'facebook') + self.assertEqual(context['duplicate_provider'], 'facebook') self.assertEqual(context['auth']['providers'][0]['name'], 'Facebook') self.assertEqual(context['auth']['providers'][1]['name'], 'Google') diff --git a/lms/djangoapps/student_account/views.py b/lms/djangoapps/student_account/views.py index 1838ff2e88c8..458616c74183 100644 --- a/lms/djangoapps/student_account/views.py +++ b/lms/djangoapps/student_account/views.py @@ -232,9 +232,7 @@ def _third_party_auth_context(request): running_pipeline = third_party_auth.pipeline.get(request) if running_pipeline is not None: - current_provider = third_party_auth.provider.Registry.get_by_backend_name( - running_pipeline.get('backend') - ) + current_provider = third_party_auth.provider.Registry.get_from_pipeline(running_pipeline) context["currentProvider"] = current_provider.NAME return context @@ -393,6 +391,7 @@ def account_settings_context(request): # If the user is connected, sending a POST request to this url removes the connection # information for this provider from their edX account. 'disconnect_url': pipeline.get_disconnect_url(state.provider.NAME), + # TODO: Fix python-social-auth disconnect pipeline to allow deleting by UserSocialAuth ID or by uid prefix } for state in auth_states] return context diff --git a/lms/templates/dashboard/_dashboard_third_party_error.html b/lms/templates/dashboard/_dashboard_third_party_error.html index 99ba0ae4fbb7..a7958b94812c 100644 --- a/lms/templates/dashboard/_dashboard_third_party_error.html +++ b/lms/templates/dashboard/_dashboard_third_party_error.html @@ -5,7 +5,7 @@

${_("Could Not Link Accounts")}

## Translators: this message is displayed when a user tries to link their account with a third-party authentication provider (for example, Google or LinkedIn) with a given edX account, but their third-party account is already associated with another edX account. provider_name is the name of the third-party authentication provider, and platform_name is the name of the edX deployment. -

${_("The {provider_name} account you selected is already linked to another {platform_name} account.").format(provider_name='{duplicate_provider}'.format(duplicate_provider=duplicate_provider.NAME), platform_name=platform_name)}

+

${_("The {provider_name} account you selected is already linked to another {platform_name} account.").format(provider_name=duplicate_provider, platform_name=platform_name)}

diff --git a/openedx/core/djangoapps/user_api/views.py b/openedx/core/djangoapps/user_api/views.py index dc9ad7c5c151..0f34a2866441 100644 --- a/openedx/core/djangoapps/user_api/views.py +++ b/openedx/core/djangoapps/user_api/views.py @@ -720,7 +720,7 @@ def _apply_third_party_auth_overrides(self, request, form_desc): if third_party_auth.is_enabled(): running_pipeline = third_party_auth.pipeline.get(request) if running_pipeline: - current_provider = third_party_auth.provider.Registry.get_by_backend_name(running_pipeline.get('backend')) + current_provider = third_party_auth.provider.Registry.get_from_pipeline(running_pipeline) # Override username / email / full name field_overrides = current_provider.get_register_form_data( From 4eae84976138fd03adfc54d24aa6c29abcda89fd Mon Sep 17 00:00:00 2001 From: Braden MacDonald Date: Thu, 7 May 2015 21:53:43 -0700 Subject: [PATCH 7/9] Improve disconnection so multiple SAML providers can be unlinked independently --- .../djangoapps/third_party_auth/pipeline.py | 32 +++++++++++++------ .../third_party_auth/tests/test_pipeline.py | 2 +- .../tests/test_pipeline_integration.py | 20 ++++++++---- lms/djangoapps/student_account/views.py | 3 +- .../student_profile/third_party_auth.html | 2 +- 5 files changed, 38 insertions(+), 21 deletions(-) diff --git a/common/djangoapps/third_party_auth/pipeline.py b/common/djangoapps/third_party_auth/pipeline.py index ddd5cafdfd8e..5d4deaf4cfd0 100644 --- a/common/djangoapps/third_party_auth/pipeline.py +++ b/common/djangoapps/third_party_auth/pipeline.py @@ -199,9 +199,11 @@ class ProviderUserState(object): lms/templates/dashboard.html. """ - def __init__(self, enabled_provider, user, state): + def __init__(self, enabled_provider, user, association_id=None): + # UserSocialAuth row ID + self.association_id = association_id # Boolean. Whether the user has an account associated with the provider - self.has_account = state + self.has_account = association_id is not None # provider.BaseProvider child. Callers must verify that the provider is # enabled. self.provider = enabled_provider @@ -261,9 +263,10 @@ def _get_enabled_provider_by_name(provider_name): def _get_url(view_name, backend_name, auth_entry=None, redirect_url=None, - enroll_course_id=None, email_opt_in=None, extra_params=None): + enroll_course_id=None, email_opt_in=None, extra_params=None, + **kwargs): """Creates a URL to hook into social auth endpoints.""" - kwargs = {'backend': backend_name} + kwargs['backend'] = backend_name url = reverse(view_name, kwargs=kwargs) query_params = OrderedDict() @@ -307,21 +310,26 @@ def get_complete_url(backend_name): return _get_url('social:complete', backend_name) -def get_disconnect_url(provider_name): +def get_disconnect_url(provider_name, association_id): """Gets URL for the endpoint that starts the disconnect pipeline. Args: provider_name: string. Name of the provider.BaseProvider child you want to disconnect from. + association_id: int. Optional ID of a specific row in the UserSocialAuth + table to disconnect (useful if multiple providers use a common backend) Returns: String. URL that starts the disconnection pipeline. Raises: - ValueError: if no provider is enabled with the given backend_name. + ValueError: if no provider is enabled with the given name. """ - enabled_provider = _get_enabled_provider_by_name(provider_name) - return _get_url('social:disconnect', enabled_provider.BACKEND_CLASS.name) + backend_name = _get_enabled_provider_by_name(provider_name).BACKEND_CLASS.name + if association_id: + return _get_url('social:disconnect_individual', backend_name, association_id=association_id) + else: + return _get_url('social:disconnect', backend_name) def get_login_url(provider_name, auth_entry, redirect_url=None, enroll_course_id=None, email_opt_in=None): @@ -404,9 +412,13 @@ def get_provider_user_states(user): found_user_auths = list(models.DjangoStorage.user.get_social_auth_for_user(user)) for enabled_provider in provider.Registry.enabled(): - is_connected = any(enabled_provider.match_social_auth(auth) for auth in found_user_auths) + association_id = None + for auth in found_user_auths: + if enabled_provider.match_social_auth(auth): + association_id = auth.id + break states.append( - ProviderUserState(enabled_provider, user, is_connected) + ProviderUserState(enabled_provider, user, association_id) ) return states diff --git a/common/djangoapps/third_party_auth/tests/test_pipeline.py b/common/djangoapps/third_party_auth/tests/test_pipeline.py index 66c11d904375..462f24e4b2d2 100644 --- a/common/djangoapps/third_party_auth/tests/test_pipeline.py +++ b/common/djangoapps/third_party_auth/tests/test_pipeline.py @@ -38,5 +38,5 @@ class ProviderUserStateTestCase(testutil.TestCase): """Tests ProviderUserState behavior.""" def test_get_unlink_form_name(self): - state = pipeline.ProviderUserState(provider.GoogleOauth2, object(), False) + state = pipeline.ProviderUserState(provider.GoogleOauth2, object(), 1000) self.assertEqual(provider.GoogleOauth2.NAME + '_unlink_form', state.get_unlink_form_name()) diff --git a/common/djangoapps/third_party_auth/tests/test_pipeline_integration.py b/common/djangoapps/third_party_auth/tests/test_pipeline_integration.py index 4812a4748907..e04737cdb09a 100644 --- a/common/djangoapps/third_party_auth/tests/test_pipeline_integration.py +++ b/common/djangoapps/third_party_auth/tests/test_pipeline_integration.py @@ -93,8 +93,9 @@ def test_state_not_returned_for_disabled_provider(self): def test_states_for_enabled_providers_user_has_accounts_associated_with(self): provider.Registry.configure_once([provider.GoogleOauth2.NAME, provider.LinkedInOauth2.NAME]) - social_models.DjangoStorage.user.create_social_auth(self.user, 'uid', provider.GoogleOauth2.BACKEND_CLASS.name) - social_models.DjangoStorage.user.create_social_auth( + user_social_auth_google = social_models.DjangoStorage.user.create_social_auth( + self.user, 'uid', provider.GoogleOauth2.BACKEND_CLASS.name) + user_social_auth_linkedin = social_models.DjangoStorage.user.create_social_auth( self.user, 'uid', provider.LinkedInOauth2.BACKEND_CLASS.name) states = pipeline.get_provider_user_states(self.user) @@ -106,10 +107,12 @@ def test_states_for_enabled_providers_user_has_accounts_associated_with(self): self.assertTrue(google_state.has_account) self.assertEqual(provider.GoogleOauth2, google_state.provider) self.assertEqual(self.user, google_state.user) + self.assertEqual(user_social_auth_google.id, google_state.association_id) self.assertTrue(linkedin_state.has_account) self.assertEqual(provider.LinkedInOauth2, linkedin_state.provider) self.assertEqual(self.user, linkedin_state.user) + self.assertEqual(user_social_auth_linkedin.id, linkedin_state.association_id) def test_states_for_enabled_providers_user_has_no_account_associated_with(self): provider.Registry.configure_once([provider.GoogleOauth2.NAME, provider.LinkedInOauth2.NAME]) @@ -155,13 +158,16 @@ def test_disconnect_url_raises_value_error_if_provider_not_enabled(self): self.assertIsNone(provider.Registry.get(provider_name)) with self.assertRaises(ValueError): - pipeline.get_disconnect_url(provider_name) + pipeline.get_disconnect_url(provider_name, 1000) def test_disconnect_url_returns_expected_format(self): - disconnect_url = pipeline.get_disconnect_url(self.enabled_provider.NAME) - - self.assertTrue(disconnect_url.startswith('/auth/disconnect')) - self.assertIn(self.enabled_provider.BACKEND_CLASS.name, disconnect_url) + disconnect_url = pipeline.get_disconnect_url(self.enabled_provider.NAME, 1000) + disconnect_url = disconnect_url.rstrip('?') + self.assertEqual( + disconnect_url, + '/auth/disconnect/{backend}/{association_id}'.format( + backend=self.enabled_provider.BACKEND_CLASS.name, association_id=1000) + ) def test_login_url_raises_value_error_if_provider_not_enabled(self): provider_name = 'not_enabled' diff --git a/lms/djangoapps/student_account/views.py b/lms/djangoapps/student_account/views.py index 458616c74183..bec0b1bbc782 100644 --- a/lms/djangoapps/student_account/views.py +++ b/lms/djangoapps/student_account/views.py @@ -390,8 +390,7 @@ def account_settings_context(request): ), # If the user is connected, sending a POST request to this url removes the connection # information for this provider from their edX account. - 'disconnect_url': pipeline.get_disconnect_url(state.provider.NAME), - # TODO: Fix python-social-auth disconnect pipeline to allow deleting by UserSocialAuth ID or by uid prefix + 'disconnect_url': pipeline.get_disconnect_url(state.provider.NAME, state.association_id), } for state in auth_states] return context diff --git a/lms/templates/student_profile/third_party_auth.html b/lms/templates/student_profile/third_party_auth.html index 33c948d3fdac..d379854a0904 100644 --- a/lms/templates/student_profile/third_party_auth.html +++ b/lms/templates/student_profile/third_party_auth.html @@ -20,7 +20,7 @@ ${state.provider.NAME}
% if state.has_account: From 00dab675045505f6b01f8521e038a1e5cdf00b8b Mon Sep 17 00:00:00 2001 From: Braden MacDonald Date: Mon, 11 May 2015 22:22:06 -0700 Subject: [PATCH 8/9] Bump python-social-auth version - includes tests --- requirements/edx/base.txt | 1 - requirements/edx/github.txt | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index 06d64c97eba4..5e492d427150 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -68,7 +68,6 @@ pyparsing==2.0.1 python-memcached==1.48 python-openid==2.2.5 python-dateutil==2.1 -#python-social-auth==0.1.23 pytz==2015.2 pysrt==0.4.7 PyYAML==3.10 diff --git a/requirements/edx/github.txt b/requirements/edx/github.txt index 73f9a08bd473..4aba3df9a4c0 100644 --- a/requirements/edx/github.txt +++ b/requirements/edx/github.txt @@ -27,7 +27,7 @@ git+https://github.com/mfogel/django-settings-context-processor.git@b758c3930862 git+https://github.com/pmitros/pyfs.git@96e1922348bfe6d99201b9512a9ed946c87b7e0b # For SAML Support (To be moved to PyPi installation in base.txt once our changes are merged): -e git+https://github.com/open-craft/python-saml.git@9602b8133056d8c3caa7c3038761147df3d4b257#egg=python-saml --e git+https://github.com/open-craft/python-social-auth.git@11d1cd31aefd9f920eefeb2d7f472f3e0fb45038#egg=python-social-auth +-e git+https://github.com/open-craft/python-social-auth.git@17def186d4bb7165f9c37037936997ef39ae2f29#egg=python-social-auth # Our libraries: -e git+https://github.com/edx/XBlock.git@aed464a0e2f7478e93157150ac04133a745f5f46#egg=XBlock From 836253995745c54f4f5a489bd6f213bc5b52e107 Mon Sep 17 00:00:00 2001 From: Braden MacDonald Date: Tue, 12 May 2015 17:53:00 -0700 Subject: [PATCH 9/9] fixup! Improve disconnection so multiple SAML providers can be unlinked independently --- common/djangoapps/third_party_auth/pipeline.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/common/djangoapps/third_party_auth/pipeline.py b/common/djangoapps/third_party_auth/pipeline.py index 5d4deaf4cfd0..0bab461647b0 100644 --- a/common/djangoapps/third_party_auth/pipeline.py +++ b/common/djangoapps/third_party_auth/pipeline.py @@ -264,10 +264,11 @@ def _get_enabled_provider_by_name(provider_name): def _get_url(view_name, backend_name, auth_entry=None, redirect_url=None, enroll_course_id=None, email_opt_in=None, extra_params=None, - **kwargs): + url_params=None): """Creates a URL to hook into social auth endpoints.""" - kwargs['backend'] = backend_name - url = reverse(view_name, kwargs=kwargs) + url_params = url_params or {} + url_params['backend'] = backend_name + url = reverse(view_name, kwargs=url_params) query_params = OrderedDict() if auth_entry: @@ -327,7 +328,7 @@ def get_disconnect_url(provider_name, association_id): """ backend_name = _get_enabled_provider_by_name(provider_name).BACKEND_CLASS.name if association_id: - return _get_url('social:disconnect_individual', backend_name, association_id=association_id) + return _get_url('social:disconnect_individual', backend_name, url_params={'association_id': association_id}) else: return _get_url('social:disconnect', backend_name)