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

use LoginSessionView.post for logistration

Browse Source 
- retires toggle ENABLE_LOGIN_POST_WITHOUT_SHIM
- permanently points to LoginSessionView.post which no longer has shim

This is Part 2 of clean-up, and should be done once the toggle
is no longer required and the shim is no longer required.

ARCH-1253
This commit is contained in:
Robert Raposa
2019-12-10 19:32:55 -05:00
parent 9f72c69e8a
commit e19c4eee8a
3 changed files with 39 additions and 257 deletions
Download Patch File Download Diff File Expand all files Collapse all files

178
openedx/core/djangoapps/user_authn/views/login.py
View File

@@ -336,6 +336,33 @@ def finish_auth(request): # pylint: disable=unused-argument
def login_user(request):
"""
AJAX request to log in the user.
Arguments:
request (HttpRequest)
Required params:
email, password
Optional params:
analytics: a JSON-encoded object with additional info to include in the login analytics event. The only
supported field is "enroll_course_id" to indicate that the user logged in while enrolling in a particular
course.
Returns:
HttpResponse: 200 if successful.
Ex. {'success': true}
HttpResponse: 400 if the request failed.
Ex. {'success': false, 'value': '{'success': false, 'value: 'Email or password is incorrect.'}
HttpResponse: 403 if successful authentication with a third party provider but does not have a linked account.
Ex. {'success': false, 'error_code': 'third-party-auth-with-no-linked-account'}
Example Usage:
POST /login_ajax
with POST params `email`, `password`
200 {'success': true}
"""
_parse_analytics_param_for_course_id(request)
@@ -444,34 +471,17 @@ class LoginSessionView(APIView):
def post(self, request):
"""Log in a user.
You must send all required form fields with the request.
You can optionally send an `analytics` param with a JSON-encoded
object with additional info to include in the login analytics event.
Currently, the only supported field is "enroll_course_id" to indicate
that the user logged in while enrolling in a particular course.
Arguments:
request (HttpRequest)
Returns:
HttpResponse: 200 on success
HttpResponse: 400 if the request is not valid.
HttpResponse: 403 if authentication failed.
403 with content "third-party-auth" if the user
has successfully authenticated with a third party provider
but does not have a linked account.
HttpResponse: 302 if redirecting to another page.
See `login_user` for details.
Example Usage:
POST /user_api/v1/login_session
with POST params `email`, `password`, and `remember`.
with POST params `email`, `password`.
200 OK
200 {'success': true}
"""
return shim_student_view(login_user, check_logged_in=True)(request)
return login_user(request)
@method_decorator(sensitive_post_parameters("password"))
def dispatch(self, request, *args, **kwargs):
@@ -507,129 +517,3 @@ def _parse_analytics_param_for_course_id(request):
analytics=analytics
)
)
def shim_student_view(view_func, check_logged_in=False):
"""Create a "shim" view for a view function from the student Django app.
UPDATE: This shim is only used to wrap `login_user`, which now lives in
the user_authn Django app (not the student app).
Specifically, we need to:
* Strip out enrollment params, since the client for the new registration/login
page will communicate with the enrollment API to update enrollments.
* Return responses with HTTP status codes indicating success/failure
(instead of always using status 200, but setting "success" to False in
the JSON-serialized content of the response)
* Use status code 403 to indicate a login failure.
The shim will preserve any cookies set by the view.
Arguments:
view_func (function): The view function from the student Django app.
Keyword Args:
check_logged_in (boolean): If true, check whether the user successfully
authenticated and if not set the status to 403. This argument is
used to test the shim by skipping this check.
Returns:
function
"""
@wraps(view_func)
def _inner(request): # pylint: disable=missing-docstring
# Call the original view to generate a response.
# We can safely modify the status code or content
# of the response, but to be safe we won't mess
# with the headers.
response = view_func(request)
# Most responses from this view are JSON-encoded
# dictionaries with keys "success", "value", and
# (sometimes) "redirect_url".
#
# We want to communicate some of this information
# using HTTP status codes instead.
#
# We ignore the "redirect_url" parameter, because we don't need it:
# 1) It's used to redirect on change enrollment, which
# our client will handle directly
# (that's why we strip out the enrollment params from the request)
# 2) It's used by third party auth when a user has already successfully
# authenticated and we're not sending login credentials. However,
# this case is never encountered in practice: on the old login page,
# the login form would be submitted directly, so third party auth
# would always be "trumped" by first party auth. If a user has
# successfully authenticated with us, we redirect them to the dashboard
# regardless of how they authenticated; and if a user is completing
# the third party auth pipeline, we redirect them from the pipeline
# completion end-point directly.
try:
response_dict = json.loads(response.content.decode('utf-8'))
msg = response_dict.get("value", u"")
success = response_dict.get("success")
set_custom_metric('shim_original_response_is_json', True)
set_custom_metric('shim_original_redirect_url', response_dict.get("redirect_url"))
set_custom_metric('shim_original_redirect', response_dict.get("redirect"))
except (ValueError, TypeError):
msg = response.content
success = True
set_custom_metric('shim_original_response_is_json', False)
set_custom_metric('shim_original_response_msg', msg)
set_custom_metric('shim_original_response_success', success)
set_custom_metric('shim_original_response_status', response.status_code)
# If the user is not authenticated when we expect them to be
# send the appropriate status code.
# We check whether the user attribute is set to make
# it easier to test this without necessarily running
# the request through authentication middleware.
is_authenticated = (
getattr(request, 'user', None) is not None
and request.user.is_authenticated
)
if check_logged_in and not is_authenticated:
# If we get a 403 status code from the student view
# this means we've successfully authenticated with a
# third party provider, but we don't have a linked
# EdX account. Send a helpful error code so the client
# knows this occurred.
if response.status_code == 403:
response.content = "third-party-auth"
# Otherwise, it's a general authentication failure.
# Ensure that the status code is a 403 and pass
# along the message from the view.
else:
response.status_code = 403
response.content = msg
# If an error condition occurs, send a status 400
elif response.status_code != 200 or not success:
# login_user sends status 200 even when an error occurs
# If the JSON-serialized content has a value "success" set to False,
# then we know an error occurred.
# NOTE: temporary metric added so we can remove this code once the
# original response is 400 instead of 200.
set_custom_metric('shim_adjusted_status_code', bool(response.status_code == 200))
if response.status_code == 200:
response.status_code = 400
response.content = msg
# If the response is successful, then return the content
# of the response directly rather than including it
# in a JSON-serialized dictionary.
else:
response.content = msg
set_custom_metric('shim_final_response_msg', response.content)
set_custom_metric('shim_final_response_status', response.status_code)
# Return the response, preserving the original headers.
# This is really important, since the student views set cookies
# that are used elsewhere in the system (such as the marketing site).
return response
return _inner

21
openedx/core/djangoapps/user_authn/views/login_form.py
View File

@@ -77,20 +77,6 @@ def _apply_third_party_auth_overrides(request, form_desc):
)
# .. toggle_name: FEATURES[ENABLE_LOGIN_POST_WITHOUT_SHIM]
# .. toggle_implementation: DjangoSetting
# .. toggle_default: False
# .. toggle_description: Toggle for enabling login post without shim_student_view (using `login_api`).
# .. toggle_category: n/a
# .. toggle_use_cases: incremental_release
# .. toggle_creation_date: 2019-12-10
# .. toggle_expiration_date: 2020-06-01
# .. toggle_warnings: n/a
# .. toggle_tickets: ARCH-1253
# .. toggle_status: supported
ENABLE_LOGIN_POST_WITHOUT_SHIM = 'ENABLE_LOGIN_POST_WITHOUT_SHIM'
def get_login_session_form(request):
"""Return a description of the login form.
@@ -105,12 +91,7 @@ def get_login_session_form(request):
HttpResponse
"""
if settings.FEATURES.get(ENABLE_LOGIN_POST_WITHOUT_SHIM):
submit_url = reverse("login_api")
else:
submit_url = reverse("user_api_login_session")
form_desc = FormDescription("post", submit_url)
form_desc = FormDescription("post", reverse("user_api_login_session"))
_apply_third_party_auth_overrides(request, form_desc)
# Translators: This label appears above a field on the login form

97
openedx/core/djangoapps/user_authn/views/tests/test_login.py
View File

@@ -30,17 +30,14 @@ from openedx.core.djangoapps.user_api.config.waffle import PREVENT_AUTH_USER_WRI
from openedx.core.djangoapps.user_api.accounts import EMAIL_MIN_LENGTH, EMAIL_MAX_LENGTH
from openedx.core.djangoapps.user_authn.cookies import jwt_cookies
from openedx.core.djangoapps.user_authn.views.login import (
shim_student_view,
AllowedAuthUser,
ENABLE_LOGIN_USING_THIRDPARTY_AUTH_ONLY
)
from openedx.core.djangoapps.user_authn.views.login_form import ENABLE_LOGIN_POST_WITHOUT_SHIM
from openedx.core.djangoapps.user_authn.tests.utils import setup_login_oauth_client
from openedx.core.djangolib.testing.utils import CacheIsolationTestCase, skip_unless_lms
from openedx.core.djangoapps.site_configuration.tests.mixins import SiteMixin
from openedx.core.lib.api.test_utils import ApiTestCase
from student.tests.factories import RegistrationFactory, UserFactory, UserProfileFactory
from util.json_request import JsonResponse
from util.password_policy_validators import DEFAULT_MAX_PASSWORD_LENGTH
@@ -662,26 +659,15 @@ class LoginSessionViewTest(ApiTestCase):
response = self.client.patch(self.url)
self.assertHttpMethodNotAllowed(response)
@ddt.data(
{ENABLE_LOGIN_POST_WITHOUT_SHIM: True},
{ENABLE_LOGIN_POST_WITHOUT_SHIM: False},
{},
)
def test_login_form(self, features_setting):
with patch.dict("django.conf.settings.FEATURES", features_setting):
# Retrieve the login form
response = self.client.get(self.url, content_type="application/json")
self.assertHttpOK(response)
if ENABLE_LOGIN_POST_WITHOUT_SHIM in features_setting and features_setting[ENABLE_LOGIN_POST_WITHOUT_SHIM]:
submit_url = reverse("login_api")
else:
submit_url = reverse("user_api_login_session")
def test_login_form(self):
# Retrieve the login form
response = self.client.get(self.url, content_type="application/json")
self.assertHttpOK(response)
# Verify that the form description matches what we expect
form_desc = json.loads(response.content.decode('utf-8'))
self.assertEqual(form_desc["method"], "post")
self.assertEqual(form_desc["submit_url"], submit_url)
self.assertEqual(form_desc["submit_url"], reverse("user_api_login_session"))
self.assertEqual(form_desc["fields"], [
{
"name": "email",
@@ -785,14 +771,14 @@ class LoginSessionViewTest(ApiTestCase):
"email": self.EMAIL,
"password": "invalid"
})
self.assertHttpForbidden(response)
self.assertHttpBadRequest(response)
# Invalid email address
response = self.client.post(self.url, {
"email": "invalid@example.com",
"password": self.PASSWORD,
})
self.assertHttpForbidden(response)
self.assertHttpBadRequest(response)
def test_missing_login_params(self):
# Create a test user
@@ -812,72 +798,3 @@ class LoginSessionViewTest(ApiTestCase):
# Missing both email and password
response = self.client.post(self.url, {})
@ddt.ddt
class StudentViewShimTest(TestCase):
"Tests of the student view shim."
def setUp(self):
super(StudentViewShimTest, self).setUp()
self.captured_request = None
def test_third_party_auth_login_failure(self):
mocked_response_content = {
"success": False,
"error_code": "third-party-auth-with-no-linked-account",
"value": "Test message."
}
view = self._shimmed_view(
JsonResponse(mocked_response_content, status=403),
check_logged_in=True
)
response = view(HttpRequest())
self.assertEqual(response.status_code, 403)
self.assertEqual(response.content, b"third-party-auth")
def test_non_json_response(self):
view = self._shimmed_view(HttpResponse(content="Not a JSON dict"))
response = view(HttpRequest())
self.assertEqual(response.status_code, 200)
self.assertEqual(response.content, b"Not a JSON dict")
@ddt.data("redirect", "redirect_url")
def test_ignore_redirect_from_json(self, redirect_key):
view = self._shimmed_view(
HttpResponse(content=json.dumps({
"success": True,
redirect_key: "/redirect"
}))
)
response = view(HttpRequest())
self.assertEqual(response.status_code, 200)
self.assertEqual(response.content.decode('utf-8'), "")
def test_error_from_json(self):
view = self._shimmed_view(
HttpResponse(content=json.dumps({
"success": False,
"value": "Error!"
}))
)
response = view(HttpRequest())
self.assertEqual(response.status_code, 400)
self.assertEqual(response.content, b"Error!")
def test_preserve_headers(self):
view_response = HttpResponse()
view_response["test-header"] = "test"
view = self._shimmed_view(view_response)
response = view(HttpRequest())
self.assertEqual(response["test-header"], "test")
def test_check_logged_in(self):
view = self._shimmed_view(HttpResponse(), check_logged_in=True)
response = view(HttpRequest())
self.assertEqual(response.status_code, 403)
def _shimmed_view(self, response, check_logged_in=False):
def stub_view(request):
self.captured_request = request
return response
return shim_student_view(stub_view, check_logged_in=check_logged_in)