Feedback changes

common/djangoapps/third_party_auth/pipeline.py

@@ -84,6 +84,7 @@ import student
 from edxmako.shortcuts import render_to_string
 from eventtracking import tracker
 from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers
+from third_party_auth.utils import user_exists
 
 from . import provider
 
@@ -501,7 +502,7 @@ def set_pipeline_timeout(strategy, user, *args, **kwargs):
         # choice of the user.
 
 
-def redirect_to_custom_form(request, auth_entry, kwargs):
+def redirect_to_custom_form(request, auth_entry, details, kwargs):
     """
     If auth_entry is found in AUTH_ENTRY_CUSTOM, this is used to send provider
     data to an external server's registration/login page.
@@ -520,7 +521,7 @@ def redirect_to_custom_form(request, auth_entry, kwargs):
         "auth_entry": auth_entry,
         "backend_name": backend_name,
         "provider_id": provider_id,
-        "user_details": kwargs['details'],
+        "user_details": details,
     })
     digest = hmac.new(secret_key, msg=data_str, digestmod=hashlib.sha256).digest()
     # Store the data in the session temporarily, then redirect to a page that will POST it to
@@ -535,7 +536,7 @@ def redirect_to_custom_form(request, auth_entry, kwargs):
 
 @partial.partial
 def ensure_user_information(strategy, auth_entry, backend=None, user=None, social=None, current_partial=None,
-                            allow_inactive_user=False, *args, **kwargs):
+                            allow_inactive_user=False, details=None, *args, **kwargs):
     """
     Ensure that we have the necessary information about a user (either an
     existing account or registration data) to proceed with the pipeline.
@@ -567,13 +568,10 @@ def ensure_user_information(strategy, auth_entry, backend=None, user=None, socia
                 (current_provider.skip_email_verification or current_provider.send_to_registration_first))
 
     if not user:
-        details = kwargs.get('details', {})
-        if 'email' in details or 'username' in details:
-            # pylint: disable=invalid-name
-            qs = {'email': details['email']} if details.get('email') else \
-                {'username': details.get('username')}
-            if User.objects.filter(**qs).exists():
-                return dispatch_to_login()
+        if user_exists(details or {}):
+            # User has not already authenticated and the details sent over from
+            # identity provider belong to an existing user.
+            return dispatch_to_login()
 
         if is_api(auth_entry):
             return HttpResponseBadRequest()
@@ -591,7 +589,7 @@ def ensure_user_information(strategy, auth_entry, backend=None, user=None, socia
             raise AuthEntryError(backend, 'auth_entry is wrong. Settings requires a user.')
         elif auth_entry in AUTH_ENTRY_CUSTOM:
             # Pass the username, email, etc. via query params to the custom entry page:
-            return redirect_to_custom_form(strategy.request, auth_entry, kwargs)
+            return redirect_to_custom_form(strategy.request, auth_entry, details or {}, kwargs)
         else:
             raise AuthEntryError(backend, 'auth_entry invalid')
 

common/djangoapps/third_party_auth/utils.py

+"""
+Utility functions for third_party_auth
+"""
+
+from django.contrib.auth.models import User
+
+
+def user_exists(details):
+    """
+    Return True if user with given details exist in the system.
+
+    Arguments:
+        details (dict): dictionary containing user infor like email, username etc.
+
+    Returns:
+        (bool): True if user with given details exists, `False` otherwise.
+    """
+    # Send the user to the login page if we already have an
+    # account that matches the authentication details.
+    user_queryset_filter = {}
+    email = details.get('email')
+    username = details.get('username')
+    if email:
+        user_queryset_filter['email'] = email
+    elif username:
+        user_queryset_filter['username'] = username
+
+    return user_queryset_filter and User.objects.filter(user_queryset_filter).exists()

lms/djangoapps/student_account/test/test_views.py

@@ -641,7 +641,6 @@ class StudentAccountLoginAndRegistrationTest(ThirdPartyAuthTestMixin, UrlResetMi
             "errorMessage": None,
             "registerFormSubmitButtonText": "Create Account",
             "hideSignInLink": False,
-            "syncLearnerProfileData": False,
         }
         if expected_ec is not None:
             # If we set an EnterpriseCustomer, third-party auth providers ought to be hidden.

lms/djangoapps/student_account/views.py

@@ -47,6 +47,7 @@ from student.views import register_user as old_register_view
 from student.views import signin_user as old_login_view
 from third_party_auth import pipeline
 from third_party_auth.decorators import xframe_allow_whitelisted
+from third_party_auth.utils import user_exists
 from util.bad_request_rate_limiter import BadRequestRateLimiter
 from util.date_utils import strftime_localized
 
@@ -133,12 +134,10 @@ def login_and_registration_form(request, initial_mode="login"):
     running_pipeline = pipeline.get(request)
     if running_pipeline:
         pipeline_user_details = running_pipeline['kwargs']['details']
-        if 'email' in pipeline_user_details or 'username' in pipeline_user_details:
-            # pylint: disable=invalid-name
-            qs = {'email': pipeline_user_details['email']} if pipeline_user_details.get('email') else \
-                {'username': pipeline_user_details.get('username')}
-            if User.objects.filter(**qs).exists():
-                account_creation_allowed = False
+        if user_exists(pipeline_user_details):
+            # Details sent over from identity provider belong to an existing user.
+            # So, force user to login and do not allow account creation
+            account_creation_allowed = False
 
     # Otherwise, render the combined login/registration page
     context = {
@@ -252,7 +251,6 @@ def update_context_for_enterprise(request, context):
     context = context.copy()
 
     sidebar_context = enterprise_sidebar_context(request)
-    sync_learner_profile_data = context['data']['third_party_auth']['syncLearnerProfileData']
 
     if sidebar_context:
         context['data']['registration_form_desc']['fields'] = enterprise_fields_only(
@@ -264,11 +262,6 @@ def update_context_for_enterprise(request, context):
     else:
         context['enable_enterprise_sidebar'] = False
 
-    if sync_learner_profile_data:
-        context['data']['hide_auth_warnings'] = True
-        enterprise_customer = enterprise_customer_for_request(request)
-        context['data']['enterprise_name'] = enterprise_customer and enterprise_customer['name']
-
     return context
 
 
@@ -349,7 +342,6 @@ def _third_party_auth_context(request, redirect_to, tpa_hint=None):
         "finishAuthUrl": None,
         "errorMessage": None,
         "registerFormSubmitButtonText": _("Create Account"),
-        "syncLearnerProfileData": False,
         "hideSignInLink": False,
     }
 
@@ -383,7 +375,6 @@ def _third_party_auth_context(request, redirect_to, tpa_hint=None):
                 context["currentProvider"] = current_provider.name
                 context["finishAuthUrl"] = pipeline.get_complete_url(current_provider.backend_name)
                 context["hideSignInLink"] = bool(current_provider.sync_learner_profile_data)
-                context["syncLearnerProfileData"] = current_provider.sync_learner_profile_data
 
                 if current_provider.skip_registration_form:
                     # For enterprise (and later for everyone), we need to get explicit consent to the

lms/static/sass/views/_login-register.scss

@@ -259,20 +259,6 @@
       display: none;
     }
 
-    input[readonly] {
-      font-family: OpenSans-Regular;
-      background-color: #e6e4e4;
-      color: #292b2c;
-      font-size: 22px;
-      border: none;
-      margin: 5px 0;
-
-      &:focus {
-        outline: none;
-      }
-    }
-
-
     .auto-register-message {
       font-size: 1.1em;
       line-height: 1.3em;
@@ -460,8 +446,7 @@
       outline: solid 1px $gray-l3;
       cursor: pointer;
 
-      &:active,
-      &:focus {
+      &:active, &:focus {
         outline: auto;
       }
     }
@@ -477,8 +462,7 @@
       }
     }
 
-    .tip,
-    .label-optional {
+    .tip, .label-optional {
       @extend %t-copy-sub1;
 
       color: $uxpl-gray-base;

lms/templates/student_account/form_field.underscore

@@ -91,8 +91,7 @@
             <% } %>
             <% if ( restrictions.min_length ) { %> minlength="<%- restrictions.min_length %>"<% } %>
             <% if ( restrictions.max_length ) { %> maxlength="<%- restrictions.max_length %>"<% } %>
-            <% if ( restrictions.readonly )   { %> readonly <% } %>
-            <% if ( required ) { %> required <% } %>
+            <% if ( required ) { %> required<% } %>
             <% if ( typeof errorMessages !== 'undefined' ) {
                 _.each(errorMessages, function( msg, type ) {%>
                     data-errormsg-<%- type %>="<%- msg %>"

openedx/core/djangoapps/user_api/api.py

@@ -835,10 +835,38 @@ class RegistrationFormFactory(object):
                 current_provider = third_party_auth.provider.Registry.get_from_pipeline(running_pipeline)
 
                 if current_provider:
-                    self._override_registration_fields_using_identity_provider_configuration(
-                        request, current_provider, running_pipeline, form_desc
+                    # Override username / email / full name
+                    field_overrides = current_provider.get_register_form_data(
+                        running_pipeline.get('kwargs')
                     )
 
+                    # When the TPA Provider is configured to skip the registration form and we are in an
+                    # enterprise context, we need to hide all fields except for terms of service and
+                    # ensure that the user explicitly checks that field.
+                    hide_registration_fields_except_tos = (
+                        (
+                            current_provider.skip_registration_form and enterprise_customer_for_request(request)
+                        ) or current_provider.sync_learner_profile_data
+                    )
+
+                    for field_name in self.DEFAULT_FIELDS + self.EXTRA_FIELDS:
+                        if field_name in field_overrides:
+                            form_desc.override_field_properties(
+                                field_name, default=field_overrides[field_name]
+                            )
+
+                            if (field_name not in ['terms_of_service', 'honor_code']
+                                    and field_overrides[field_name]
+                                    and hide_registration_fields_except_tos):
+
+                                form_desc.override_field_properties(
+                                    field_name,
+                                    field_type="hidden",
+                                    label="",
+                                    instructions="",
+                                )
+
+                    # Hide the password field
                     form_desc.override_field_properties(
                         "password",
                         default="",
@@ -856,37 +884,3 @@ class RegistrationFormFactory(object):
                         default=current_provider.name if current_provider.name else "Third Party",
                         required=False,
                     )
-
-    # pylint: disable=invalid-name
-    def _override_registration_fields_using_identity_provider_configuration(
-            self, request, current_provider, running_pipeline, form_desc
-    ):
-        """
-        Apply form field overrides based on configuration of third party auth provider.
-        """
-        field_overrides = current_provider.get_register_form_data(
-            running_pipeline.get('kwargs')
-        )
-        # When the TPA Provider is configured to skip the registration form and we are in an
-        # enterprise context, we need to hide all fields except for terms of service and
-        # ensure that the user explicitly checks that field.
-        hide_registration_fields_except_tos = (
-            (
-                current_provider.skip_registration_form and enterprise_customer_for_request(request)
-            ) or current_provider.sync_learner_profile_data
-        )
-
-        for field_name in self.DEFAULT_FIELDS + self.EXTRA_FIELDS:
-            if field_name in field_overrides:
-                form_desc.override_field_properties(
-                    field_name, default=field_overrides[field_name]
-                )
-
-                if field_name not in ['terms_of_service', 'honor_code'] and field_overrides[field_name] and \
-                        hide_registration_fields_except_tos:
-                    form_desc.override_field_properties(
-                        field_name,
-                        field_type="hidden",
-                        label="",
-                        instructions="",
-                    )

openedx/core/djangoapps/user_api/helpers.py

@@ -124,9 +124,9 @@ class FormDescription(object):
     ALLOWED_TYPES = ["text", "email", "select", "textarea", "checkbox", "password", "hidden"]
 
     ALLOWED_RESTRICTIONS = {
-        "text": ["min_length", "max_length", "readonly"],
-        "password": ["min_length", "max_length", "readonly"],
-        "email": ["min_length", "max_length", "readonly"],
+        "text": ["min_length", "max_length"],
+        "password": ["min_length", "max_length"],
+        "email": ["min_length", "max_length"],
     }
 
     FIELD_TYPE_MAP = {