Andal.LND/edx-platform
4
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Migrate to latest, split python-social-auth.

Browse Source 
PSA was monolothic, now split, with new features, like
a DB-backed partial pipeline. FB OAuth2 version also upped.

Partial pipelines don't get cleared except when necessary.
They persist for special cases like change of browser while
still mid-pipeline (i.e. email validation step).

Refactor, cleanup, and update of a lot of small things as well.

PLEASE NOTE the new `social_auth_partial` table.
This commit is contained in:
Uman Shahzad
2017-05-19 20:46:01 -04:00
parent 4dd4f97900
commit 8b65ca17c5
48 changed files with 270 additions and 234 deletions
Download Patch File Download Diff File Expand all files Collapse all files

9
openedx/core/djangoapps/auth_exchange/forms.py
View File

@@ -10,8 +10,8 @@ from provider.forms import OAuthForm, OAuthValidationError
from provider.oauth2.forms import ScopeChoiceField, ScopeMixin
from provider.oauth2.models import Client
from requests import HTTPError
from social.backends import oauth as social_oauth
from social.exceptions import AuthException
from social_core.backends import oauth as social_oauth
from social_core.exceptions import AuthException
from third_party_auth import pipeline
@@ -90,15 +90,16 @@ class AccessTokenExchangeForm(ScopeMixin, OAuthForm):
self.cleaned_data["client"] = client
user = None
access_token = self.cleaned_data.get("access_token")
try:
user = backend.do_auth(self.cleaned_data.get("access_token"), allow_inactive_user=True)
user = backend.do_auth(access_token, allow_inactive_user=True)
except (HTTPError, AuthException):
pass
if user and isinstance(user, User):
self.cleaned_data["user"] = user
else:
# Ensure user does not re-enter the pipeline
self.request.social_strategy.clean_partial_pipeline()
self.request.social_strategy.clean_partial_pipeline(access_token)
raise OAuthValidationError(
{
"error": "invalid_grant",

18
openedx/core/djangoapps/auth_exchange/tests/test_forms.py
View File

@@ -10,12 +10,13 @@ from django.test import TestCase
from django.test.client import RequestFactory
import httpretty
from provider import scope
import social.apps.django_app.utils as social_utils
from social_django.models import Partial
import social_django.utils as social_utils
from third_party_auth.tests.utils import ThirdPartyOAuthTestMixinFacebook, ThirdPartyOAuthTestMixinGoogle
from ..forms import AccessTokenExchangeForm
from .utils import AccessTokenExchangeTestMixin
from .utils import AccessTokenExchangeTestMixin, TPA_FEATURE_ENABLED, TPA_FEATURES_KEY
from .mixins import DOPAdapterMixin, DOTAdapterMixin
@@ -32,13 +33,16 @@ class AccessTokenExchangeFormTest(AccessTokenExchangeTestMixin):
# pylint: disable=no-member
self.request.backend = social_utils.load_backend(self.request.social_strategy, self.BACKEND, redirect_uri)
def tearDown(self):
super(AccessTokenExchangeFormTest, self).tearDown()
Partial.objects.all().delete()
def _assert_error(self, data, expected_error, expected_error_description):
form = AccessTokenExchangeForm(request=self.request, oauth2_adapter=self.oauth2_adapter, data=data)
self.assertEqual(
form.errors,
{"error": expected_error, "error_description": expected_error_description}
)
self.assertNotIn("partial_pipeline", self.request.session)
def _assert_success(self, data, expected_scopes):
form = AccessTokenExchangeForm(request=self.request, oauth2_adapter=self.oauth2_adapter, data=data)
@@ -49,7 +53,7 @@ class AccessTokenExchangeFormTest(AccessTokenExchangeTestMixin):
# This is necessary because cms does not implement third party auth
@unittest.skipUnless(settings.FEATURES.get("ENABLE_THIRD_PARTY_AUTH"), "third party auth not enabled")
@unittest.skipUnless(TPA_FEATURE_ENABLED, TPA_FEATURES_KEY + " not enabled")
@httpretty.activate
class DOPAccessTokenExchangeFormTestFacebook(
DOPAdapterMixin,
@@ -65,7 +69,7 @@ class DOPAccessTokenExchangeFormTestFacebook(
# This is necessary because cms does not implement third party auth
@unittest.skipUnless(settings.FEATURES.get("ENABLE_THIRD_PARTY_AUTH"), "third party auth not enabled")
@unittest.skipUnless(TPA_FEATURE_ENABLED, TPA_FEATURES_KEY + " not enabled")
@httpretty.activate
class DOTAccessTokenExchangeFormTestFacebook(
DOTAdapterMixin,
@@ -81,7 +85,7 @@ class DOTAccessTokenExchangeFormTestFacebook(
# This is necessary because cms does not implement third party auth
@unittest.skipUnless(settings.FEATURES.get("ENABLE_THIRD_PARTY_AUTH"), "third party auth not enabled")
@unittest.skipUnless(TPA_FEATURE_ENABLED, TPA_FEATURES_KEY + " not enabled")
@httpretty.activate
class DOPAccessTokenExchangeFormTestGoogle(
DOPAdapterMixin,
@@ -97,7 +101,7 @@ class DOPAccessTokenExchangeFormTestGoogle(
# This is necessary because cms does not implement third party auth
@unittest.skipUnless(settings.FEATURES.get("ENABLE_THIRD_PARTY_AUTH"), "third party auth not enabled")
@unittest.skipUnless(TPA_FEATURE_ENABLED, TPA_FEATURES_KEY + " not enabled")
@httpretty.activate
class DOTAccessTokenExchangeFormTestGoogle(
DOTAdapterMixin,

16
openedx/core/djangoapps/auth_exchange/tests/test_views.py
View File

@@ -17,11 +17,12 @@ import httpretty
import provider.constants
from provider.oauth2.models import AccessToken, Client
from rest_framework.test import APIClient
from social_django.models import Partial
from student.tests.factories import UserFactory
from third_party_auth.tests.utils import ThirdPartyOAuthTestMixinFacebook, ThirdPartyOAuthTestMixinGoogle
from .mixins import DOPAdapterMixin, DOTAdapterMixin
from .utils import AccessTokenExchangeTestMixin
from .utils import AccessTokenExchangeTestMixin, TPA_FEATURE_ENABLED, TPA_FEATURES_KEY
@ddt.ddt
@@ -34,6 +35,10 @@ class AccessTokenExchangeViewTest(AccessTokenExchangeTestMixin):
self.url = reverse("exchange_access_token", kwargs={"backend": self.BACKEND})
self.csrf_client = APIClient(enforce_csrf_checks=True)
def tearDown(self):
super(AccessTokenExchangeViewTest, self).tearDown()
Partial.objects.all().delete()
def _assert_error(self, data, expected_error, expected_error_description):
response = self.csrf_client.post(self.url, data)
self.assertEqual(response.status_code, 400)
@@ -42,7 +47,6 @@ class AccessTokenExchangeViewTest(AccessTokenExchangeTestMixin):
json.loads(response.content),
{u"error": expected_error, u"error_description": expected_error_description}
)
self.assertNotIn("partial_pipeline", self.client.session)
def _assert_success(self, data, expected_scopes):
response = self.csrf_client.post(self.url, data)
@@ -101,7 +105,7 @@ class AccessTokenExchangeViewTest(AccessTokenExchangeTestMixin):
# This is necessary because cms does not implement third party auth
@unittest.skipUnless(settings.FEATURES.get("ENABLE_THIRD_PARTY_AUTH"), "third party auth not enabled")
@unittest.skipUnless(TPA_FEATURE_ENABLED, TPA_FEATURES_KEY + " not enabled")
@httpretty.activate
class DOPAccessTokenExchangeViewTestFacebook(
DOPAdapterMixin,
@@ -115,7 +119,7 @@ class DOPAccessTokenExchangeViewTestFacebook(
pass
@unittest.skipUnless(settings.FEATURES.get("ENABLE_THIRD_PARTY_AUTH"), "third party auth not enabled")
@unittest.skipUnless(TPA_FEATURE_ENABLED, TPA_FEATURES_KEY + " not enabled")
@httpretty.activate
class DOTAccessTokenExchangeViewTestFacebook(
DOTAdapterMixin,
@@ -130,7 +134,7 @@ class DOTAccessTokenExchangeViewTestFacebook(
# This is necessary because cms does not implement third party auth
@unittest.skipUnless(settings.FEATURES.get("ENABLE_THIRD_PARTY_AUTH"), "third party auth not enabled")
@unittest.skipUnless(TPA_FEATURE_ENABLED, TPA_FEATURES_KEY + " not enabled")
@httpretty.activate
class DOPAccessTokenExchangeViewTestGoogle(
DOPAdapterMixin,
@@ -146,7 +150,7 @@ class DOPAccessTokenExchangeViewTestGoogle(
# This is necessary because cms does not implement third party auth
@unittest.skipUnless(settings.FEATURES.get("ENABLE_THIRD_PARTY_AUTH"), "third party auth not enabled")
@unittest.skipUnless(TPA_FEATURE_ENABLED, TPA_FEATURES_KEY + " not enabled")
@httpretty.activate
class DOTAccessTokenExchangeViewTestGoogle(
DOTAdapterMixin,

11
openedx/core/djangoapps/auth_exchange/tests/utils.py
View File

@@ -2,10 +2,16 @@
Test utilities for OAuth access token exchange
"""
from social.apps.django_app.default.models import UserSocialAuth
from django.conf import settings
from social_django.models import UserSocialAuth, Partial
from third_party_auth.tests.utils import ThirdPartyOAuthTestMixin
TPA_FEATURES_KEY = 'ENABLE_THIRD_PARTY_AUTH'
TPA_FEATURE_ENABLED = TPA_FEATURES_KEY in settings.FEATURES
class AccessTokenExchangeTestMixin(ThirdPartyOAuthTestMixin):
"""
A mixin to define test cases for access token exchange. The following
@@ -86,16 +92,19 @@ class AccessTokenExchangeTestMixin(ThirdPartyOAuthTestMixin):
def test_no_linked_user(self):
UserSocialAuth.objects.all().delete()
Partial.objects.all().delete()
self._setup_provider_response(success=True)
self._assert_error(self.data, "invalid_grant", "access_token is not valid")
def test_user_automatically_linked_by_email(self):
UserSocialAuth.objects.all().delete()
Partial.objects.all().delete()
self._setup_provider_response(success=True, email=self.user.email)
self._assert_success(self.data, expected_scopes=[])
def test_inactive_user_not_automatically_linked(self):
UserSocialAuth.objects.all().delete()
Partial.objects.all().delete()
self._setup_provider_response(success=True, email=self.user.email)
self.user.is_active = False
self.user.save() # pylint: disable=no-member

8
openedx/core/djangoapps/auth_exchange/views.py
View File

@@ -10,7 +10,7 @@ The following are currently implemented:
# pylint: disable=abstract-method
import django.contrib.auth as auth
import social.apps.django_app.utils as social_utils
import social_django.utils as social_utils
from django.conf import settings
from django.contrib.auth import login
from django.http import HttpResponse
@@ -37,7 +37,7 @@ class AccessTokenExchangeBase(APIView):
OAuth access token.
"""
@method_decorator(csrf_exempt)
@method_decorator(social_utils.strategy("social:complete"))
@method_decorator(social_utils.psa("social:complete"))
def dispatch(self, *args, **kwargs):
return super(AccessTokenExchangeBase, self).dispatch(*args, **kwargs)
@@ -137,11 +137,11 @@ class DOTAccessTokenExchangeView(AccessTokenExchangeBase, DOTAccessTokenView):
request.extra_credentials = None
request.grant_type = client.authorization_grant_type
def error_response(self, form_errors):
def error_response(self, form_errors, **kwargs):
"""
Return an error response consisting of the errors in the form
"""
return Response(status=400, data=form_errors)
return Response(status=400, data=form_errors, **kwargs)
class LoginWithAccessTokenView(APIView):

2
openedx/core/djangoapps/bookmarks/tests/test_views.py
View File

@@ -268,7 +268,7 @@ class BookmarksListViewTests(BookmarksViewsTestsBase):
self.assertEqual(response.data['developer_message'], u'Parameter usage_id not provided.')
# Send empty data dictionary.
with self.assertNumQueries(7): # No queries for bookmark table.
with self.assertNumQueries(8): # No queries for bookmark table.
response = self.send_post(
client=self.client,
url=reverse('bookmarks'),

16
openedx/core/djangoapps/user_api/accounts/tests/test_views.py
View File

@@ -174,7 +174,7 @@ class TestOwnUsernameAPI(CacheIsolationTestCase, UserAPITestCase):
Test that a client (logged in) can get her own username.
"""
self.client.login(username=self.user.username, password=TEST_PASSWORD)
self._verify_get_own_username(14)
self._verify_get_own_username(15)
def test_get_username_inactive(self):
"""
@@ -184,7 +184,7 @@ class TestOwnUsernameAPI(CacheIsolationTestCase, UserAPITestCase):
self.client.login(username=self.user.username, password=TEST_PASSWORD)
self.user.is_active = False
self.user.save()
self._verify_get_own_username(14)
self._verify_get_own_username(15)
def test_get_username_not_logged_in(self):
"""
@@ -193,7 +193,7 @@ class TestOwnUsernameAPI(CacheIsolationTestCase, UserAPITestCase):
"""
# verify that the endpoint is inaccessible when not logged in
self._verify_get_own_username(12, expected_status=401)
self._verify_get_own_username(13, expected_status=401)
@ddt.ddt
@@ -305,7 +305,7 @@ class TestAccountsAPI(CacheIsolationTestCase, UserAPITestCase):
"""
self.different_client.login(username=self.different_user.username, password=TEST_PASSWORD)
self.create_mock_profile(self.user)
with self.assertNumQueries(18):
with self.assertNumQueries(19):
response = self.send_get(self.different_client)
self._verify_full_shareable_account_response(response, account_privacy=ALL_USERS_VISIBILITY)
@@ -320,7 +320,7 @@ class TestAccountsAPI(CacheIsolationTestCase, UserAPITestCase):
"""
self.different_client.login(username=self.different_user.username, password=TEST_PASSWORD)
self.create_mock_profile(self.user)
with self.assertNumQueries(18):
with self.assertNumQueries(19):
response = self.send_get(self.different_client)
self._verify_private_account_response(response, account_privacy=PRIVATE_VISIBILITY)
@@ -395,12 +395,12 @@ class TestAccountsAPI(CacheIsolationTestCase, UserAPITestCase):
self.assertEqual(False, data["accomplishments_shared"])
self.client.login(username=self.user.username, password=TEST_PASSWORD)
verify_get_own_information(16)
verify_get_own_information(17)
# Now make sure that the user can get the same information, even if not active
self.user.is_active = False
self.user.save()
verify_get_own_information(10)
verify_get_own_information(11)
def test_get_account_empty_string(self):
"""
@@ -414,7 +414,7 @@ class TestAccountsAPI(CacheIsolationTestCase, UserAPITestCase):
legacy_profile.save()
self.client.login(username=self.user.username, password=TEST_PASSWORD)
with self.assertNumQueries(16):
with self.assertNumQueries(17):
response = self.send_get(self.client)
for empty_field in ("level_of_education", "gender", "country", "bio"):
self.assertIsNone(response.data[empty_field])

12
openedx/core/djangoapps/user_api/tests/test_views.py
View File

@@ -16,7 +16,7 @@ from django.test.testcases import TransactionTestCase
from django.test.utils import override_settings
from opaque_keys.edx.locations import SlashSeparatedCourseKey
from pytz import common_timezones_set, UTC
from social.apps.django_app.default.models import UserSocialAuth
from social_django.models import UserSocialAuth, Partial
from django_comment_common import models
from openedx.core.djangoapps.site_configuration.helpers import get_value
@@ -1976,6 +1976,10 @@ class ThirdPartyRegistrationTestMixin(ThirdPartyOAuthTestMixin, CacheIsolationTe
super(ThirdPartyRegistrationTestMixin, self).setUp()
self.url = reverse('user_api_registration')
def tearDown(self):
super(ThirdPartyRegistrationTestMixin, self).tearDown()
Partial.objects.all().delete()
def data(self, user=None):
"""Returns the request data for the endpoint."""
return {
@@ -1996,7 +2000,6 @@ class ThirdPartyRegistrationTestMixin(ThirdPartyOAuthTestMixin, CacheIsolationTe
for conflict_attribute in ["username", "email"]:
self.assertIn(conflict_attribute, errors)
self.assertIn("belongs to an existing account", errors[conflict_attribute][0]["user_message"])
self.assertNotIn("partial_pipeline", self.client.session)
def _assert_access_token_error(self, response, expected_error_message):
"""Assert that the given response was an error for the access_token field with the given error message."""
@@ -2006,7 +2009,6 @@ class ThirdPartyRegistrationTestMixin(ThirdPartyOAuthTestMixin, CacheIsolationTe
response_json,
{"access_token": [{"user_message": expected_error_message}]}
)
self.assertNotIn("partial_pipeline", self.client.session)
def _assert_third_party_session_expired_error(self, response, expected_error_message):
"""Assert that given response is an error due to third party session expiry"""
@@ -2109,8 +2111,6 @@ class ThirdPartyRegistrationTestMixin(ThirdPartyOAuthTestMixin, CacheIsolationTe
# to identify that request is made using browser
data.update({"social_auth_provider": "Google"})
response = self.client.post(self.url, data)
# NO partial_pipeline in session means pipeline is expired
self.assertNotIn("partial_pipeline", self.client.session)
self._assert_third_party_session_expired_error(
response,
u"Registration using {provider} has timed out.".format(provider="Google")
@@ -2127,7 +2127,7 @@ class TestFacebookRegistrationView(
def test_social_auth_exception(self):
"""
According to the do_auth method in social.backends.facebook.py,
According to the do_auth method in social_core.backends.facebook.py,
the Facebook API sometimes responds back a JSON with just False as value.
"""
self._setup_provider_response_with_body(200, json.dumps("false"))

2
openedx/features/course_experience/tests/views/test_course_home.py
View File

@@ -89,7 +89,7 @@ class TestCourseHomePage(SharedModuleStoreTestCase):
course_home_url(self.course)
# Fetch the view and verify the query counts
with self.assertNumQueries(47):
with self.assertNumQueries(48):
with check_mongo_calls(5):
url = course_home_url(self.course)
self.client.get(url)

2
openedx/features/course_experience/tests/views/test_course_updates.py
View File

@@ -124,7 +124,7 @@ class TestCourseUpdatesPage(SharedModuleStoreTestCase):
course_updates_url(self.course)
# Fetch the view and verify that the query counts haven't changed
with self.assertNumQueries(35):
with self.assertNumQueries(36):
with check_mongo_calls(4):
url = course_updates_url(self.course)
self.client.get(url)

2
openedx/features/enterprise_support/api.py
View File

@@ -320,7 +320,7 @@ def insert_enterprise_pipeline_elements(pipeline):
'enterprise.tpa_pipeline.handle_enterprise_logistration',
)
# Find the item we need to insert the data sharing consent elements before
insert_point = pipeline.index('social.pipeline.social_auth.load_extra_data')
insert_point = pipeline.index('social_core.pipeline.social_auth.load_extra_data')
for index, element in enumerate(additional_elements):
pipeline.insert(insert_point + index, element)

4
openedx/features/enterprise_support/tests/test_api.py
View File

@@ -40,11 +40,11 @@ class TestEnterpriseApi(unittest.TestCase):
the utilities to return the expected values.
"""
self.assertTrue(enterprise_enabled())
pipeline = ['abc', 'social.pipeline.social_auth.load_extra_data', 'def']
pipeline = ['abc', 'social_core.pipeline.social_auth.load_extra_data', 'def']
insert_enterprise_pipeline_elements(pipeline)
self.assertEqual(pipeline, ['abc',
'enterprise.tpa_pipeline.handle_enterprise_logistration',
'social.pipeline.social_auth.load_extra_data',
'social_core.pipeline.social_auth.load_extra_data',
'def'])
@override_settings(ENABLE_ENTERPRISE_INTEGRATION=True)