Merge pull request #1642 from fledge-iot/FOGL-9494

FOGL-9494 Additional exception handling related to passwords has been incorporated into the authentication user model API, accompanied by unit tests

Author: ashish-jabble

python/fledge/services/core/api/auth.py

@@ -110,7 +110,6 @@ async def login(request):
     """
     auth_method = request.auth_method if 'auth_method' in dir(request) else "any"
     data = await request.text()
-
     try:
         # Check ott inside request payload.
         _data = json.loads(data)
@@ -175,14 +174,22 @@ async def login(request):
             host, port = peername
         try:
             uid, token, is_admin = await User.Objects.login(username, password, host)
-        except (User.DoesNotExist, User.PasswordDoesNotMatch, ValueError) as ex:
-            raise web.HTTPNotFound(reason=str(ex))
+        except User.PasswordNotSetError as err:
+            msg = str(err)
+            raise web.HTTPBadRequest(reason=msg, body=json.dumps({"message": msg}))
+        except (User.DoesNotExist, User.PasswordDoesNotMatch, ValueError) as err:
+            msg = str(err)
+            raise web.HTTPNotFound(reason=msg, body=json.dumps({"message": msg}))
         except User.PasswordExpired as ex:
             # delete all user token for this user
             await User.Objects.delete_user_tokens(str(ex))
             msg = 'Your password has been expired. Please set your password again.'
             _logger.warning(msg)
             raise web.HTTPUnauthorized(reason=msg)
+        except Exception as exc:
+            msg = str(exc)
+            _logger.error(exc, "Failed to login.")
+            raise web.HTTPInternalServerError(reason=msg, body=json.dumps({"message": msg}))
 
     _logger.info("User with username:<{}> logged in successfully.".format(username))
     return web.json_response({"message": "Logged in successfully.", "uid": uid, "token": token, "admin": is_admin})

python/fledge/services/core/user_model.py

@@ -59,6 +59,9 @@ class DoesNotExist(Exception):
     class UserAlreadyExists(Exception):
         pass
 
+    class PasswordNotSetError(Exception):
+        pass
+
     class PasswordDoesNotMatch(Exception):
         pass
 
@@ -382,7 +385,8 @@ async def login(cls, username, password, host):
                 raise User.DoesNotExist('User does not exist')
 
             found_user = result['rows'][0]
-
+            if not found_user.get('pwd'):
+                raise User.PasswordNotSetError("Password is not set for this user.")
             # check age of password
             t1 = datetime.now()
             t2 = datetime.strptime(found_user['pwd_last_changed'], "%Y-%m-%d %H:%M:%S.%f")
@@ -475,7 +479,6 @@ async def login(cls, username, password, host):
             # Clear failed_attempts on successful login
             if int(found_user['failed_attempts']) > 0:
                 await cls.update(found_user['id'],{'failed_attempts': 0})
-
             uid, jwt_token, is_admin = await cls._get_new_token(storage_client, found_user, host)
             return uid, jwt_token, is_admin
 

tests/unit/python/fledge/services/core/api/test_auth_mandatory.py

@@ -1213,6 +1213,60 @@ async def test_reset_role_and_password(self, client, mocker):
         patch_validate_token.assert_called_once_with(ADMIN_USER_HEADER['Authorization'])
         patch_logger_debug.assert_called_once_with('Received %s request for %s', 'PUT', '/fledge/admin/2/reset')
 
+    @pytest.mark.parametrize("request_data, ret_val", [
+        ({"username": "admin", "password": "fledge"}, (1, "token1", True)),
+        ({"username": "user", "password": "fledge"}, (2, "token2", False))
+    ])
+    async def test_login_auth_password(self, client, request_data, ret_val):
+        async def async_mock():
+            return ret_val
+
+        # Changed in version 3.8: patch() now returns an AsyncMock if the target is an async function.
+        if sys.version_info.major == 3 and sys.version_info.minor >= 8:
+            _rv = await async_mock()
+        else:
+            _rv = asyncio.ensure_future(async_mock())
+
+        with patch.object(middleware._logger, 'debug') as patch_logger:
+            with patch.object(User.Objects, 'login', return_value=_rv) as patch_user_login:
+                with patch.object(auth._logger, 'info') as patch_auth_logger:
+                    resp = await client.post('/fledge/login', data=json.dumps(request_data))
+                    assert 200 == resp.status
+                    r = await resp.text()
+                    actual = json.loads(r)
+                    assert ret_val[0] == actual['uid']
+                    assert ret_val[1] == actual['token']
+                    assert ret_val[2] == actual['admin']
+                patch_auth_logger.assert_called_once_with('User with username:<{}> logged in successfully.'.format(
+                    request_data['username']))
+            # TODO: host arg patch transport.request.extra_info
+            args, kwargs = patch_user_login.call_args
+            assert request_data['username'] == args[0]
+            assert request_data['password'] == args[1]
+            # patch_user_login.assert_called_once_with()
+        patch_logger.assert_called_once_with('Received %s request for %s', 'POST', '/fledge/login')
+
+    @pytest.mark.parametrize("exception_name, status_code, msg", [
+        (User.PasswordNotSetError, 400, 'Password is not set for this user.'),
+        (User.DoesNotExist, 404, 'User does not exist'),
+        (User.PasswordDoesNotMatch, 404, 'Username or Password do not match'),
+        (Exception, 500, 'Internal Server Error')
+    ])
+    async def test_login_fails_when_password_auth_used_but_password_not_set(self, client, exception_name,
+                                                                            status_code, msg):
+        request_data_payload = {"username": "ranveer", "password": "Singh@123"}
+        with patch.object(middleware._logger, 'debug') as patch_logger:
+            with patch.object(User.Objects, 'login', side_effect=exception_name(msg)):
+                with patch.object(auth._logger, 'error') as patch_auth_logger:
+                    resp = await client.post('/fledge/login', data=json.dumps(request_data_payload))
+                    assert status_code == resp.status
+                    assert msg == resp.reason
+                    r = await resp.text()
+                    actual = json.loads(r)
+                    assert {'message': msg} == actual
+                patch_auth_logger.assert_not_called() if status_code != 500 else patch_auth_logger.assert_called()
+        patch_logger.assert_called_once_with('Received %s request for %s', 'POST', '/fledge/login')
+
     @pytest.mark.parametrize("auth_method, request_data, ret_val", [
         ("certificate", "-----BEGIN CERTIFICATE----- Test -----END CERTIFICATE-----", (2, "token2", False))
     ])

tests/unit/python/fledge/services/core/api/test_auth_optional.py

@@ -164,7 +164,10 @@ async def test_bad_login(self, client, request_data):
         ({"username": "admin", "password": 123}, 404, User.PasswordDoesNotMatch, 'Username or Password do not match'),
         ({"username": 1, "password": 1}, 404, ValueError, 'Username should be a valid string'),
         ({"username": "user", "password": "fledge"}, 401, User.PasswordExpired,
-         'Your password has been expired. Please set your password again.')
+         'Your password has been expired. Please set your password again.'),
+        ({"username": "user1", "password": "blah"}, 400, User.PasswordNotSetError,
+         'Password is not set for this user.')
+
     ])
     async def test_login_exception(self, client, request_data, status_code, exception_name, msg):
 

tests/unit/python/fledge/services/core/test_user_model.py

@@ -604,6 +604,45 @@ async def mock_get_category_item():
                 assert payload == p
             mock_get_cat_patch.assert_called_once_with('password', 'expiration')
 
+    async def test_login_with_empty_password(self):
+        async def mock_get_category_item():
+            return {"value": "0"}
+
+        pwd_result = {'count': 1, 'rows': [{'pwd': '', 'id': 3, 'role_id': 2, 'access_method': 'cert',
+                                            'pwd_last_changed': '', 'real_name': 'AJ', 'description': '',
+                                            'hash_algorithm': 'SHA512', 'block_until': '', 'failed_attempts': 0}]}
+        payload = {"return": ["pwd", "id", "role_id", "access_method",
+                              {"column": "pwd_last_changed", "format": "YYYY-MM-DD HH24:MI:SS.MS", "alias":
+                                  "pwd_last_changed"}, "real_name", "description", "hash_algorithm", "block_until",
+                              "failed_attempts"],
+                   "where": {"column": "uname", "condition": "=", "value": "user",
+                             "and": {"column": "enabled", "condition": "=", "value": "t"}}}
+        storage_client_mock = MagicMock(StorageClientAsync)
+
+        # Changed in version 3.8: patch() now returns an AsyncMock if the target is an async function.
+        if sys.version_info.major == 3 and sys.version_info.minor >= 8:
+            _rv1 = await mock_get_category_item()
+            _rv2 = await mock_coro(pwd_result)
+        else:
+            _rv1 = asyncio.ensure_future(mock_get_category_item())
+            _rv2 = asyncio.ensure_future(mock_coro(pwd_result))
+
+        with patch.object(connect, 'get_storage_async', return_value=storage_client_mock):
+            with patch.object(ConfigurationManager, "get_category_item",
+                              return_value=_rv1) as mock_get_cat_patch:
+                with patch.object(storage_client_mock, 'query_tbl_with_payload',
+                                  return_value=_rv2) as query_tbl_patch:
+                    with pytest.raises(Exception) as excinfo:
+                        await User.Objects.login('user', 'blah', '0.0.0.0')
+                    assert str(excinfo.value) == 'Password is not set for this user.'
+                    assert excinfo.type is User.PasswordNotSetError
+                    assert issubclass(excinfo.type, Exception)
+                args, kwargs = query_tbl_patch.call_args
+                assert 'users' == args[0]
+                p = json.loads(args[1])
+                assert payload == p
+            mock_get_cat_patch.assert_called_once_with('password', 'expiration')
+
     async def test_login_age_pwd_expiration(self):
         async def mock_get_category_item():
             return {"value": "30"}