-
Notifications
You must be signed in to change notification settings - Fork 4.3k
Add create account functionality to common/djangoapps/third_party_auth #2994
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Closed
johncox-google
wants to merge
14
commits into
openedx:master
from
johncox-google:johncox/feature/auth
Closed
Changes from all commits
Commits
Show all changes
14 commits
Select commit
Hold shift + click to select a range
3b400f0
Remove Mozilla Persona provider now that Mozilla stopped Persona deve…
johncox-google b4b3d9f
Add new user creation feature
johncox-google c937d83
Update AUTHORS and use .format in template
johncox-google 972ff0c
Address review comments
johncox-google 4688a85
Remove comments per review; whitespace fix
johncox-google efc089d
Hide password field in pipeline; add translator comments
johncox-google ea70486
Add sign in to existing account support
johncox-google 9f239b5
Address review comments
johncox-google fb6fec5
Add tests.
johncox-google 322cac6
Bump python-social-auth version for linkedin oauth2 changes
johncox-google d7f086d
Correctly sends users through the entire pipeline during login
johncox-google 0341e5a
Address review comments
johncox-google 9b4b228
Fix translators commenting.
johncox-google b59c9b1
Add comments for sentence fragment
johncox-google File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -85,6 +85,8 @@ | |
| validate_password_dictionary | ||
| ) | ||
|
|
||
| from third_party_auth import pipeline, provider | ||
|
|
||
| log = logging.getLogger("edx.student") | ||
| AUDIT_LOG = logging.getLogger("audit") | ||
|
|
||
|
|
@@ -363,11 +365,16 @@ def signin_user(request): | |
| context = { | ||
| 'course_id': request.GET.get('course_id'), | ||
| 'enrollment_action': request.GET.get('enrollment_action'), | ||
| # Bool injected into JS to submit form if we're inside a running third- | ||
| # party auth pipeline; distinct from the actual instance of the running | ||
| # pipeline, if any. | ||
| 'pipeline_running': 'true' if pipeline.running(request) else 'false', | ||
| 'platform_name': microsite.get_value( | ||
| 'platform_name', | ||
| settings.PLATFORM_NAME | ||
| ), | ||
| } | ||
|
|
||
| return render_to_response('login.html', context) | ||
|
|
||
|
|
||
|
|
@@ -385,17 +392,34 @@ def register_user(request, extra_context=None): | |
|
|
||
| context = { | ||
| 'course_id': request.GET.get('course_id'), | ||
| 'email': '', | ||
| 'enrollment_action': request.GET.get('enrollment_action'), | ||
| 'name': '', | ||
| 'running_pipeline': None, | ||
| 'platform_name': microsite.get_value( | ||
| 'platform_name', | ||
| settings.PLATFORM_NAME | ||
| ), | ||
| 'selected_provider': '', | ||
| 'username': '', | ||
| } | ||
|
|
||
| if extra_context is not None: | ||
| context.update(extra_context) | ||
|
|
||
| if context.get("extauth_domain", '').startswith(external_auth.views.SHIBBOLETH_DOMAIN_PREFIX): | ||
| return render_to_response('register-shib.html', context) | ||
|
|
||
| # If third-party auth is enabled, prepopulate the form with data from the | ||
| # selected provider. | ||
| if settings.FEATURES.get('ENABLE_THIRD_PARTY_AUTH') and pipeline.running(request): | ||
| running_pipeline = pipeline.get(request) | ||
| current_provider = provider.Registry.get_by_backend_name(running_pipeline.get('backend')) | ||
| overrides = current_provider.get_register_form_data(running_pipeline.get('kwargs')) | ||
| overrides['running_pipeline'] = running_pipeline | ||
| overrides['selected_provider'] = current_provider.NAME | ||
| context.update(overrides) | ||
|
|
||
| return render_to_response('register.html', context) | ||
|
|
||
|
|
||
|
|
@@ -532,6 +556,7 @@ def dashboard(request): | |
| 'language_options': language_options, | ||
| 'current_language': current_language, | ||
| 'current_language_code': cur_lang_code, | ||
| 'user': user, | ||
| } | ||
|
|
||
| return render_to_response('dashboard.html', context) | ||
|
|
@@ -690,31 +715,70 @@ def accounts_login(request): | |
| return external_auth.views.course_specific_login(request, course_id) | ||
|
|
||
| context = { | ||
| 'pipeline_running': 'false', | ||
| 'platform_name': settings.PLATFORM_NAME, | ||
| } | ||
| return render_to_response('login.html', context) | ||
|
|
||
|
|
||
| # Need different levels of logging | ||
| @ensure_csrf_cookie | ||
| def login_user(request, error=""): | ||
| def login_user(request, error=""): # pylint: disable-msg=too-many-statements,unused-argument | ||
| """AJAX request to log in the user.""" | ||
| if 'email' not in request.POST or 'password' not in request.POST: | ||
| return JsonResponse({ | ||
| "success": False, | ||
| "value": _('There was an error receiving your login information. Please email us.'), # TODO: User error message | ||
| }) # TODO: this should be status code 400 # pylint: disable=fixme | ||
|
|
||
| email = request.POST['email'] | ||
| password = request.POST['password'] | ||
| try: | ||
| user = User.objects.get(email=email) | ||
| except User.DoesNotExist: | ||
| if settings.FEATURES['SQUELCH_PII_IN_LOGS']: | ||
| AUDIT_LOG.warning(u"Login failed - Unknown user email") | ||
| else: | ||
| AUDIT_LOG.warning(u"Login failed - Unknown user email: {0}".format(email)) | ||
| user = None | ||
| backend_name = None | ||
| email = None | ||
| password = None | ||
| redirect_url = None | ||
| response = None | ||
| running_pipeline = None | ||
| third_party_auth_requested = settings.FEATURES.get('ENABLE_THIRD_PARTY_AUTH') and pipeline.running(request) | ||
| third_party_auth_successful = False | ||
| trumped_by_first_party_auth = bool(request.POST.get('email')) or bool(request.POST.get('password')) | ||
| user = None | ||
|
|
||
| if third_party_auth_requested and not trumped_by_first_party_auth: | ||
| # The user has already authenticated via third-party auth and has not | ||
| # asked to do first party auth by supplying a username or password. We | ||
| # now want to put them through the same logging and cookie calculation | ||
| # logic as with first-party auth. | ||
| 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) | ||
|
|
||
| try: | ||
| user = pipeline.get_authenticated_user(username, backend_name) | ||
| third_party_auth_successful = True | ||
| except User.DoesNotExist: | ||
| AUDIT_LOG.warning( | ||
| u'Login failed - user with username {username} has no social auth with backend_name {backend_name}'.format( | ||
| username=username, backend_name=backend_name)) | ||
| return JsonResponse({ | ||
| "success": False, | ||
| # Translators: provider_name is the name of an external, third-party user authentication service (like | ||
| # Google or LinkedIn). | ||
| "value": _('There is no {platform_name} account associated with your {provider_name} account. Please use your {platform_name} credentials or pick another provider.').format( | ||
| platform_name=settings.PLATFORM_NAME, provider_name=requested_provider.NAME) | ||
| }) # TODO: this should be a status code 401 # pylint: disable=fixme | ||
|
|
||
| else: | ||
|
|
||
| if 'email' not in request.POST or 'password' not in request.POST: | ||
| return JsonResponse({ | ||
| "success": False, | ||
| "value": _('There was an error receiving your login information. Please email us.'), # TODO: User error message | ||
| }) # TODO: this should be status code 400 # pylint: disable=fixme | ||
|
|
||
| email = request.POST['email'] | ||
| password = request.POST['password'] | ||
| try: | ||
| user = User.objects.get(email=email) | ||
| except User.DoesNotExist: | ||
| if settings.FEATURES['SQUELCH_PII_IN_LOGS']: | ||
| AUDIT_LOG.warning(u"Login failed - Unknown user email") | ||
| else: | ||
| AUDIT_LOG.warning(u"Login failed - Unknown user email: {0}".format(email)) | ||
|
|
||
| # check if the user has a linked shibboleth account, if so, redirect the user to shib-login | ||
| # This behavior is pretty much like what gmail does for shibboleth. Try entering some @stanford.edu | ||
|
|
@@ -753,14 +817,17 @@ def login_user(request, error=""): | |
| # username so that authentication is guaranteed to fail and we can take | ||
| # advantage of the ratelimited backend | ||
| username = user.username if user else "" | ||
| try: | ||
| user = authenticate(username=username, password=password, request=request) | ||
| # this occurs when there are too many attempts from the same IP address | ||
| except RateLimitException: | ||
| return JsonResponse({ | ||
| "success": False, | ||
| "value": _('Too many failed login attempts. Try again later.'), | ||
| }) # TODO: this should be status code 429 # pylint: disable=fixme | ||
|
|
||
| if not third_party_auth_successful: | ||
| try: | ||
| user = authenticate(username=username, password=password, request=request) | ||
| # this occurs when there are too many attempts from the same IP address | ||
| except RateLimitException: | ||
| return JsonResponse({ | ||
| "success": False, | ||
| "value": _('Too many failed login attempts. Try again later.'), | ||
| }) # TODO: this should be status code 429 # pylint: disable=fixme | ||
|
|
||
| if user is None: | ||
| # tick the failed login counters if the user exists in the database | ||
| if user_found_by_email_lookup and LoginFailures.is_feature_enabled(): | ||
|
|
@@ -801,7 +868,9 @@ def login_user(request, error=""): | |
|
|
||
| redirect_url = try_change_enrollment(request) | ||
|
|
||
| dog_stats_api.increment("common.student.successful_login") | ||
| if third_party_auth_successful: | ||
| redirect_url = pipeline.get_complete_url(backend_name) | ||
|
|
||
| response = JsonResponse({ | ||
| "success": True, | ||
| "redirect_url": redirect_url, | ||
|
|
@@ -1032,11 +1101,15 @@ def create_account(request, post_override=None): | |
| JSON call to create new edX account. | ||
| Used by form in signup_modal.html, which is included into navigation.html | ||
| """ | ||
| js = {'success': False} | ||
| js = {'success': False} # pylint: disable-msg=invalid-name | ||
|
|
||
| post_vars = post_override if post_override else request.POST | ||
| extra_fields = getattr(settings, 'REGISTRATION_EXTRA_FIELDS', {}) | ||
|
|
||
| if settings.FEATURES.get('ENABLE_THIRD_PARTY_AUTH') and pipeline.running(request): | ||
| post_vars = dict(post_vars.items()) | ||
| post_vars.update({'password': pipeline.make_random_password()}) | ||
|
|
||
| # if doing signup for an external authorization, then get email, password, name from the eamap | ||
| # don't use the ones from the form, since the user could have hacked those | ||
| # unless originally we didn't get a valid email or name from the external auth | ||
|
|
@@ -1234,9 +1307,17 @@ def create_account(request, post_override=None): | |
| login_user.save() | ||
| AUDIT_LOG.info(u"Login activated on extauth account - {0} ({1})".format(login_user.username, login_user.email)) | ||
|
|
||
| dog_stats_api.increment("common.student.account_created") | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why count another account creation?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't know where this came from -- probably a bad merge. Removed. |
||
| redirect_url = try_change_enrollment(request) | ||
|
|
||
| # Resume the third-party-auth pipeline if necessary. | ||
| if settings.FEATURES.get('ENABLE_THIRD_PARTY_AUTH') and pipeline.running(request): | ||
| running_pipeline = pipeline.get(request) | ||
| redirect_url = pipeline.get_complete_url(running_pipeline['backend']) | ||
|
|
||
| response = JsonResponse({ | ||
| 'success': True, | ||
| 'redirect_url': try_change_enrollment(request), | ||
| 'redirect_url': redirect_url, | ||
| }) | ||
|
|
||
| # set the login cookie for the edx marketing site | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| """Middleware classes for third_party_auth.""" | ||
|
|
||
| from social.apps.django_app.middleware import SocialAuthExceptionMiddleware | ||
|
|
||
| from . import pipeline | ||
|
|
||
|
|
||
| class ExceptionMiddleware(SocialAuthExceptionMiddleware): | ||
| """Custom middleware that handles conditional redirection.""" | ||
|
|
||
| def get_redirect_uri(self, request, exception): | ||
| # Safe because it's already been validated by | ||
| # pipeline.parse_query_params. If that pipeline step ever moves later | ||
| # in the pipeline stack, we'd need to validate this value because it | ||
| # would be an injection point for attacker data. | ||
| auth_entry = request.session.get(pipeline.AUTH_ENTRY_KEY) | ||
| # Fall back to django settings's SOCIAL_AUTH_LOGIN_ERROR_URL. | ||
| return '/' + auth_entry if auth_entry else super(ExceptionMiddleware, self).get_redirect_uri(request, exception) |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see that you have just moved this code block, but since you're here anyway - this condition should be entered into the audit log, because it means that something wrong happened.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm going to push back on refactoring existing code as part of the auth PRs, even when I agree with a suggested change in isolation. The auth feature is big and high risk, so I think there's a lot of value in scoping the PRs strictly to what's necessary for auth.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, I'll put in a PR after this gets merged in then.