Skip to content

E
edx-platform
Commit 82668c34 by Matt Drayer Committed by Jonathan Piacenti
Options

mattdrayer/api-userdetailpostfix: Addressed several issues related to updating… 

mattdrayer/api-userdetailpostfix: Addressed several issues related to updating user/profile information

FInal fix
parent c907daa4
Showing with 178 additions and 111 deletions
lms/djangoapps/api_manager/models.py
...@@ -122,4 +122,7 @@ class CourseContentGroupRelationship(TimeStampedModel): ...@@ -122,4 +122,7 @@ class CourseContentGroupRelationship(TimeStampedModel):
record_active = models.BooleanField(default=True) record_active = models.BooleanField(default=True)
class Meta: class Meta:
"""
Mapping model to enable grouping of course content such as chapters
"""
unique_together = ("course_id", "content_id", "group") unique_together = ("course_id", "content_id", "group")
lms/djangoapps/api_manager/users/test_user_password_reset.py
...@@ -123,6 +123,37 @@ class UserPasswordResetTest(TestCase): ...@@ -123,6 +123,37 @@ class UserPasswordResetTest(TestCase):
) )
self._assert_response(response, status=200) self._assert_response(response, status=200)
@override_settings(ADVANCED_SECURITY_CONFIG={'MIN_DIFFERENT_STAFF_PASSWORDS_BEFORE_REUSE': 20,
'MIN_TIME_IN_DAYS_BETWEEN_ALLOWED_RESETS': 0})
def test_password_reset_not_allowable_reuse_staff_user(self):
"""
Try resetting staff user password with an already-used password
Hits a very specific LOC in the view code
"""
response = self._do_post_request(
self.user_url, 'test2', 'Test.Me64!', email='test@edx.org',
first_name='John', last_name='Doe', secure=True, is_staff=True
)
self._assert_response(response, status=201)
user_id = response.data['id']
pass_reset_url = "%s/%s" % (self.user_url, str(user_id))
response = self._do_post_pass_reset_request(
pass_reset_url, password='Test.Me64#', secure=True
)
self._assert_response(response, status=200)
response = self._do_post_pass_reset_request(
pass_reset_url, password='Test.Me64#', secure=True
)
message = _(
"You are re-using a password that you have used recently. You must "
"have 20 distinct password(s) before reusing a previous password."
)
self._assert_response(response, status=403, message=message)
@override_settings(ADVANCED_SECURITY_CONFIG={'MIN_TIME_IN_DAYS_BETWEEN_ALLOWED_RESETS': 1}) @override_settings(ADVANCED_SECURITY_CONFIG={'MIN_TIME_IN_DAYS_BETWEEN_ALLOWED_RESETS': 1})
def test_is_password_reset_too_frequent(self): def test_is_password_reset_too_frequent(self):
""" """
...@@ -202,6 +233,8 @@ class UserPasswordResetTest(TestCase): ...@@ -202,6 +233,8 @@ class UserPasswordResetTest(TestCase):
post_params['first_name'] = kwargs.get('first_name') post_params['first_name'] = kwargs.get('first_name')
if kwargs.get('last_name'): if kwargs.get('last_name'):
post_params['last_name'] = kwargs.get('last_name') post_params['last_name'] = kwargs.get('last_name')
if kwargs.get('is_staff'):
post_params['is_staff'] = kwargs.get('is_staff')
headers = {'X-Edx-Api-Key': TEST_API_KEY, 'Content-Type': 'application/json'} headers = {'X-Edx-Api-Key': TEST_API_KEY, 'Content-Type': 'application/json'}
if kwargs.get('secure', False): if kwargs.get('secure', False):
......
lms/djangoapps/api_manager/users/tests.py
...@@ -34,7 +34,8 @@ class SecureClient(Client): ...@@ -34,7 +34,8 @@ class SecureClient(Client):
@override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE) @override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE)
@override_settings(EDX_API_KEY=TEST_API_KEY) @override_settings(EDX_API_KEY=TEST_API_KEY)
@patch.dict("django.conf.settings.FEATURES", {'ENFORCE_PASSWORD_POLICY': False}) @override_settings(PASSWORD_MIN_LENGTH=4)
@patch.dict("django.conf.settings.FEATURES", {'ENFORCE_PASSWORD_POLICY': True})
class UsersApiTests(TestCase): class UsersApiTests(TestCase):
""" Test suite for Users API views """ """ Test suite for Users API views """
...@@ -144,19 +145,30 @@ class UsersApiTests(TestCase): ...@@ -144,19 +145,30 @@ class UsersApiTests(TestCase):
def test_user_detail_post(self): def test_user_detail_post(self):
test_uri = '/api/users' test_uri = '/api/users'
local_username = self.test_username + str(randint(11, 99)) local_username = self.test_username + str(randint(11, 99))
data = {'email': self.test_email, 'username': local_username, 'password': data = {'email': self.test_email,
self.test_password, 'first_name': self.test_first_name, 'last_name': self.test_last_name} 'username': local_username, 'password':self.test_password,
'first_name': self.test_first_name, 'last_name': self.test_last_name}
response = self.do_post(test_uri, data) response = self.do_post(test_uri, data)
self.assertEqual(response.status_code, 201)
test_uri = test_uri + '/' + str(response.data['id']) test_uri = test_uri + '/' + str(response.data['id'])
data = {'is_active': False} auth_data = {'username': local_username, 'password': self.test_password}
self.do_post('/api/sessions', auth_data)
self.assertEqual(response.status_code, 201)
data = {'is_active': False, 'is_staff': True}
response = self.do_post(test_uri, data) response = self.do_post(test_uri, data)
self.assertEqual(response.status_code, 200) self.assertEqual(response.status_code, 200)
self.assertEqual(response.data['is_active'], False) self.assertEqual(response.data['is_active'], False)
self.assertEqual(response.data['is_staff'], True)
response = self.do_get(test_uri) response = self.do_get(test_uri)
self.assertEqual(response.status_code, 200) self.assertEqual(response.status_code, 200)
self.assertEqual(response.data['email'], self.test_email)
self.assertEqual(response.data['username'], local_username)
self.assertEqual(response.data['first_name'], self.test_first_name)
self.assertEqual(response.data['last_name'], self.test_last_name)
self.assertEqual(response.data['full_name'], '{} {}'.format(self.test_first_name, self.test_last_name))
self.assertEqual(response.data['is_active'], False) self.assertEqual(response.data['is_active'], False)
def test_user_detail_post_username(self): def test_user_detail_post_duplicate_username(self):
""" """
Create two users, then pass the same first username in request in order to update username of second user. Create two users, then pass the same first username in request in order to update username of second user.
Must return bad request against username, Already exist! Must return bad request against username, Already exist!
...@@ -188,6 +200,19 @@ class UsersApiTests(TestCase): ...@@ -188,6 +200,19 @@ class UsersApiTests(TestCase):
self.assertEqual(response.status_code, 400) self.assertEqual(response.status_code, 400)
self.assertEqual(response.data['message'], message) self.assertEqual(response.data['message'], message)
def test_user_detail_post_invalid_password(self):
test_uri = '/api/users'
local_username = self.test_username + str(randint(11, 99))
data = {'email': self.test_email,
'username': local_username, 'password': self.test_password,
'first_name': self.test_first_name, 'last_name': self.test_last_name}
response = self.do_post(test_uri, data)
self.assertEqual(response.status_code, 201)
test_uri = test_uri + '/' + str(response.data['id'])
data = {'password': 'x'}
response = self.do_post(test_uri, data)
self.assertEqual(response.status_code, 400)
def test_user_detail_post_user_profile_added_updated(self): def test_user_detail_post_user_profile_added_updated(self):
""" """
Create a user, then add the user profile Create a user, then add the user profile
......
lms/djangoapps/api_manager/users/views.py
...@@ -122,12 +122,12 @@ class UsersList(APIView): ...@@ -122,12 +122,12 @@ class UsersList(APIView):
first_name = request.DATA.get('first_name', '') first_name = request.DATA.get('first_name', '')
last_name = request.DATA.get('last_name', '') last_name = request.DATA.get('last_name', '')
is_active = request.DATA.get('is_active', None) is_active = request.DATA.get('is_active', None)
is_staff = request.DATA.get('is_staff', False)
city = request.DATA.get('city', '') city = request.DATA.get('city', '')
country = request.DATA.get('country', '') country = request.DATA.get('country', '')
level_of_education = request.DATA.get('level_of_education', '') level_of_education = request.DATA.get('level_of_education', '')
year_of_birth = request.DATA.get('year_of_birth', '') year_of_birth = request.DATA.get('year_of_birth', '')
gender = request.DATA.get('gender', '') gender = request.DATA.get('gender', '')
# enforce password complexity as an optional feature # enforce password complexity as an optional feature
if settings.FEATURES.get('ENFORCE_PASSWORD_POLICY', False): if settings.FEATURES.get('ENFORCE_PASSWORD_POLICY', False):
try: try:
...@@ -154,7 +154,7 @@ class UsersList(APIView): ...@@ -154,7 +154,7 @@ class UsersList(APIView):
# Create the User, UserProfile, and UserPreference records # Create the User, UserProfile, and UserPreference records
try: try:
user = User.objects.create(email=email, username=username) user = User.objects.create(email=email, username=username, is_staff=is_staff)
except IntegrityError: except IntegrityError:
user = None user = None
else: else:
...@@ -163,6 +163,8 @@ class UsersList(APIView): ...@@ -163,6 +163,8 @@ class UsersList(APIView):
user.last_name = last_name user.last_name = last_name
if is_active is not None: if is_active is not None:
user.is_active = is_active user.is_active = is_active
if is_staff is not None:
user.is_staff = is_staff
user.save() user.save()
profile = UserProfile(user=user) profile = UserProfile(user=user)
...@@ -238,17 +240,10 @@ class UsersDetail(APIView): ...@@ -238,17 +240,10 @@ class UsersDetail(APIView):
POST provides the ability to update information about an existing user POST provides the ability to update information about an existing user
""" """
response_data = {} response_data = {}
first_name = request.DATA.get('first_name', '')
last_name = request.DATA.get('last_name', '')
city = request.DATA.get('city', '')
country = request.DATA.get('country', '')
level_of_education = request.DATA.get('level_of_education', '')
year_of_birth = request.DATA.get('year_of_birth', '')
gender = request.DATA.get('gender', '')
base_uri = _generate_base_uri(request) base_uri = _generate_base_uri(request)
response_data['uri'] = _generate_base_uri(request) response_data['uri'] = _generate_base_uri(request)
first_name = request.DATA.get('first_name') # Used in multiple spots below
last_name = request.DATA.get('last_name') # Used in multiple spots below
# Add some rate limiting here by re-using the RateLimitMixin as a helper class # Add some rate limiting here by re-using the RateLimitMixin as a helper class
limiter = BadRequestRateLimiter() limiter = BadRequestRateLimiter()
if limiter.is_rate_limit_exceeded(request): if limiter.is_rate_limit_exceeded(request):
...@@ -260,112 +255,123 @@ class UsersDetail(APIView): ...@@ -260,112 +255,123 @@ class UsersDetail(APIView):
except ObjectDoesNotExist: except ObjectDoesNotExist:
limiter.tick_bad_request_counter(request) limiter.tick_bad_request_counter(request)
existing_user = None existing_user = None
if existing_user: if existing_user is None:
username = request.DATA.get('username', None) return Response({}, status=status.HTTP_404_NOT_FOUND)
if username:
try:
validate_slug(username)
except ValidationError:
status_code = status.HTTP_400_BAD_REQUEST
response_data['message'] = _('Username should only consist of A-Z and 0-9, with no spaces.')
return Response(response_data, status=status_code)
existing_username = User.objects.filter(username=username).filter(~Q(id=user_id))
if existing_username:
status_code = status.HTTP_409_CONFLICT
response_data['message'] = "User '%s' already exists" % (username)
response_data['field_conflict'] = "username"
return Response(response_data, status=status_code)
existing_user.username = username # Ok, valid User, now update the provided fields
response_data['username'] = existing_user.username if first_name:
existing_user.save() existing_user.first_name = first_name
if last_name:
existing_user.last_name = last_name
is_active = request.DATA.get('is_active')
if is_active is not None:
existing_user.is_active = is_active
response_data['is_active'] = existing_user.is_active
is_staff = request.DATA.get('is_staff')
if is_staff is not None:
existing_user.is_staff = is_staff
response_data['is_staff'] = existing_user.is_staff
existing_user.save()
username = request.DATA.get('username', None)
if username:
try:
validate_slug(username)
except ValidationError:
status_code = status.HTTP_400_BAD_REQUEST
response_data['message'] = _('Username should only consist of A-Z and 0-9, with no spaces.')
return Response(response_data, status=status_code)
is_active = request.DATA.get('is_active', None) existing_username = User.objects.filter(username=username).filter(~Q(id=user_id))
if is_active is not None: if existing_username:
existing_user.is_active = is_active status_code = status.HTTP_409_CONFLICT
response_data['is_active'] = existing_user.is_active response_data['message'] = "User '%s' already exists" % (username)
existing_user.save() response_data['field_conflict'] = "username"
return Response(response_data, status=status_code)
password = request.DATA.get('password')
if password:
old_password_hash = existing_user.password
_serialize_user(response_data, existing_user)
password = request.DATA['password']
if settings.FEATURES.get('ENFORCE_PASSWORD_POLICY', False):
try:
validate_password_length(password)
validate_password_complexity(password)
validate_password_dictionary(password)
except ValidationError, err:
# bad user? tick the rate limiter counter
AUDIT_LOG.warning("API::Bad password in password_reset.")
status_code = status.HTTP_400_BAD_REQUEST
response_data['message'] = _('Password: ') + '; '.join(err.messages)
return Response(response_data, status=status_code)
# also, check the password reuse policy
err_msg = None
if not PasswordHistory.is_allowable_password_reuse(existing_user, password):
if existing_user.is_staff:
num_distinct = settings.ADVANCED_SECURITY_CONFIG['MIN_DIFFERENT_STAFF_PASSWORDS_BEFORE_REUSE']
else:
num_distinct = settings.ADVANCED_SECURITY_CONFIG['MIN_DIFFERENT_STUDENT_PASSWORDS_BEFORE_REUSE']
err_msg = _(
"You are re-using a password that you have used recently. You must "
"have {0} distinct password(s) before reusing a previous password."
).format(num_distinct)
# also, check to see if passwords are getting reset too frequent
if PasswordHistory.is_password_reset_too_soon(existing_user):
num_days = settings.ADVANCED_SECURITY_CONFIG['MIN_TIME_IN_DAYS_BETWEEN_ALLOWED_RESETS']
err_msg = _(
"You are resetting passwords too frequently. Due to security policies, "
"{0} day(s) must elapse between password resets"
).format(num_days)
if err_msg:
# We have an password reset attempt which violates some security policy,
status_code = status.HTTP_403_FORBIDDEN
response_data['message'] = err_msg
return Response(response_data, status=status_code)
existing_user.is_active = True existing_user.username = username
existing_user.set_password(password) response_data['username'] = existing_user.username
existing_user.save() existing_user.save()
update_user_password_hash = existing_user.password
if update_user_password_hash != old_password_hash: password = request.DATA.get('password')
# add this account creation to password history if password:
# NOTE, this will be a NOP unless the feature has been turned on in configuration old_password_hash = existing_user.password
password_history_entry = PasswordHistory() _serialize_user(response_data, existing_user)
password_history_entry.create(existing_user) if settings.FEATURES.get('ENFORCE_PASSWORD_POLICY', False):
try:
validate_password_length(password)
validate_password_complexity(password)
validate_password_dictionary(password)
except ValidationError, err:
# bad user? tick the rate limiter counter
AUDIT_LOG.warning("API::Bad password in password_reset.")
status_code = status.HTTP_400_BAD_REQUEST
response_data['message'] = _('Password: ') + '; '.join(err.messages)
return Response(response_data, status=status_code)
# also, check the password reuse policy
err_msg = None
if not PasswordHistory.is_allowable_password_reuse(existing_user, password):
if existing_user.is_staff:
num_distinct = settings.ADVANCED_SECURITY_CONFIG['MIN_DIFFERENT_STAFF_PASSWORDS_BEFORE_REUSE']
else:
num_distinct = settings.ADVANCED_SECURITY_CONFIG['MIN_DIFFERENT_STUDENT_PASSWORDS_BEFORE_REUSE']
err_msg = _(
"You are re-using a password that you have used recently. You must "
"have {0} distinct password(s) before reusing a previous password."
).format(num_distinct)
# also, check to see if passwords are getting reset too frequent
if PasswordHistory.is_password_reset_too_soon(existing_user):
num_days = settings.ADVANCED_SECURITY_CONFIG['MIN_TIME_IN_DAYS_BETWEEN_ALLOWED_RESETS']
err_msg = _(
"You are resetting passwords too frequently. Due to security policies, "
"{0} day(s) must elapse between password resets"
).format(num_days)
if err_msg:
# We have an password reset attempt which violates some security policy,
status_code = status.HTTP_403_FORBIDDEN
response_data['message'] = err_msg
return Response(response_data, status=status_code)
existing_user_profile = UserProfile.objects.get(user_id=user_id) existing_user.is_active = True
if existing_user_profile: existing_user.set_password(password)
existing_user.save()
update_user_password_hash = existing_user.password
if update_user_password_hash != old_password_hash:
# add this account creation to password history
# NOTE, this will be a NOP unless the feature has been turned on in configuration
password_history_entry = PasswordHistory()
password_history_entry.create(existing_user)
# Also update the UserProfile record for this User
existing_user_profile = UserProfile.objects.get(user_id=user_id)
if existing_user_profile:
if first_name and last_name:
existing_user_profile.name = '{} {}'.format(first_name, last_name) existing_user_profile.name = '{} {}'.format(first_name, last_name)
city = request.DATA.get('city')
if city:
existing_user_profile.city = city existing_user_profile.city = city
country = request.DATA.get('country')
if country:
existing_user_profile.country = country existing_user_profile.country = country
level_of_education = request.DATA.get('level_of_education')
if level_of_education:
existing_user_profile.level_of_education = level_of_education existing_user_profile.level_of_education = level_of_education
year_of_birth = request.DATA.get('year_of_birth')
try:
year_of_birth = int(year_of_birth)
except (ValueError, TypeError):
# If they give us garbage, just ignore it instead
# of asking them to put an integer.
year_of_birth = None
if year_of_birth:
existing_user_profile.year_of_birth = year_of_birth
gender = request.DATA.get('gender')
if gender:
existing_user_profile.gender = gender existing_user_profile.gender = gender
existing_user_profile.save()
try: return Response(response_data, status=status.HTTP_200_OK)
existing_user_profile.year_of_birth = int(year_of_birth)
except ValueError:
# If they give us garbage, just ignore it instead
# of asking them to put an integer.
existing_user_profile.year_of_birth = None
existing_user_profile.save()
status_code = status.HTTP_200_OK
else:
status_code = status.HTTP_404_NOT_FOUND
response_data['message'] = 'User not exist'
return Response(response_data, status=status_code)
class UsersGroupsList(APIView): class UsersGroupsList(APIView):
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment