diff --git a/lib/pbench/server/api/resources/users_api.py b/lib/pbench/server/api/resources/users_api.py index cc056bfe7d..2eefbdcf3c 100644 --- a/lib/pbench/server/api/resources/users_api.py +++ b/lib/pbench/server/api/resources/users_api.py @@ -35,11 +35,12 @@ def post(self): Content-Type: application/json Accept: application/json - :return: JSON Payload - if we succeed to add a user entry in the database, the returned response_object will look like the following: - response_object = { - "message": "Successfully registered."/"failure message", - } + :return: + Success: 201 with empty payload + Failure: , + response_object = { + "message": "failure message" + } To get the auth token user has to perform the login action """ # get the post data @@ -114,13 +115,7 @@ def post(self): self.logger.info( "New user registered, username: {}, email: {}", username, email_id ) - - response_object = { - "message": "Successfully registered.", - } - response = jsonify(response_object) - response.status_code = 201 - return make_response(response, 201) + return "", 201 except EmailNotValidError: self.logger.warning("Invalid email {}", email_id) abort(400, message=f"Invalid email: {email_id}") @@ -142,7 +137,6 @@ def __init__(self, config, logger, auth): "pbench-server", "token_expiration_duration" ) - @Auth.token_auth.login_required(optional=True) def post(self): """ Post request for logging in user. @@ -160,11 +154,15 @@ def post(self): Accept: application/json :return: JSON Payload - if we succeed to decrypt the password hash, the returned response_object will include the auth_token - response_object = { - "message": "Successfully logged in."/"failure message", - "auth_token": "", # Will not present if failed - } + Success: 200, + response_object = { + "auth_token": "" + "username": + } + Failure: , + response_object = { + "message": "failure message" + } """ # get the post data post_data = request.get_json() @@ -217,17 +215,6 @@ def post(self): 500, message="INTERNAL ERROR", ) - # check if the user is already logged in, in case the request has a an authorization header included - # We would still proceed to login and return a new auth token to the user - if user == self.auth.token_auth.current_user(): - self.logger.warning("User already logged in and trying to re-login") - if self.auth.token_auth.get_auth()["token"] == auth_token: - # If a user attempts to re-login immediately within a millisecond, - # the new auth token will be same as the one present in the header - # Therefore we will not fulfill the re-login request - self.logger.info("Attempted immediate re-login") - abort(403, message="Retry login after some time") - # Add the new auth token to the database for later access try: token = ActiveTokens(token=auth_token) @@ -250,7 +237,6 @@ def post(self): abort(500, message="INTERNAL ERROR") response_object = { - "message": "Successfully logged in.", "auth_token": auth_token, "username": username, } @@ -276,10 +262,12 @@ def post(self): Required headers include Authorization: Bearer - :return: JSON Payload - response_object = { - "message": "Successfully logged out."/"failure message", - } + :return: + Success: 200 with empty payload + Failure: , + response_object = { + "message": "failure message" + } """ auth_token = self.auth.token_auth.get_auth()["token"] user = self.auth.token_auth.current_user() @@ -293,10 +281,7 @@ def post(self): self.logger.exception("Exception occurred during deleting the user entry") abort(500, message="INTERNAL ERROR") - response_object = { - "message": "Successfully logged out.", - } - return make_response(jsonify(response_object), 200) + return "", 200 class UserAPI(Resource): @@ -321,15 +306,17 @@ def get(self, username): Authorization: Bearer Pbench_auth_token (user received upon login) :return: JSON Payload - response_object = { - "message": "Success"/"failure message", - "data": { + Success: 200, + response_object = { "username": , "first_name": , "last_name": , - "registered_on": registered_on, + "registered_on": , + } + Failure: , + response_object = { + "message": "failure message" } - } """ try: user, verified = self.auth.verify_user(username) @@ -339,18 +326,12 @@ def get(self, username): # TODO: Check if the user has the right privileges if verified: - response_object = { - "status": "success", - "data": user.get_json(), - } + response_object = user.get_json() return make_response(jsonify(response_object), 200) elif user.is_admin(): try: target_user = User.query(username=username) - response_object = { - "message": "success", - "data": target_user.get_json(), - } + response_object = target_user.get_json() return make_response(jsonify(response_object), 200) except Exception: self.logger.exception( @@ -359,7 +340,7 @@ def get(self, username): abort(500, message="INTERNAL ERROR") else: self.logger.warning( - "User {} is not authorized to delete user {}.", user.username, username, + "User {} is not authorized to get user {}.", user.username, username, ) abort( 403, @@ -387,15 +368,17 @@ def put(self, username): Authorization: Bearer Pbench_auth_token (user received upon login) :return: JSON Payload - response_object = { - "message": "Success"/"failure message", - "data": { + Success: 200, + response_object = { "username": , "first_name": , "last_name": , - "registered_on": registered_on, + "registered_on": , + } + Failure: , + response_object = { + "message": "failure message" } - } """ post_data = request.get_json() if not post_data: @@ -445,10 +428,7 @@ def put(self, username): self.logger.exception("Exception occurred during updating user object") abort(500, message="INTERNAL ERROR") - response_object = { - "message": "success", - "data": user.get_json(), - } + response_object = user.get_json() return make_response(jsonify(response_object), 200) @Auth.token_auth.login_required() @@ -463,10 +443,12 @@ def delete(self, username): Accept: application/json Authorization: Bearer Pbench_auth_token (user received upon login) - :return: JSON Payload - response_object = { - "message": "Successfully deleted."/"failure message", - } + :return: + Success: 200 with empty payload + Failure: , + response_object = { + "message": "failure message" + } """ try: user, verified = self.auth.verify_user(username) @@ -498,7 +480,4 @@ def delete(self, username): abort(403, message="Admin user can not be deleted") self.logger.info("User entry deleted for user with username {}", username) - response_object = { - "message": "Successfully deleted.", - } - return make_response(jsonify(response_object), 200) + return "", 200 diff --git a/lib/pbench/server/database/models/users.py b/lib/pbench/server/database/models/users.py index 94e9c10b12..cb9c368be4 100644 --- a/lib/pbench/server/database/models/users.py +++ b/lib/pbench/server/database/models/users.py @@ -34,6 +34,11 @@ def get_json(self): @staticmethod def get_protected(): + """ + Return protected column names that are not allowed for external updates. + auth_tokens is already protected from external updates via PUT api since + it is of type sqlalchemy relationship ORM package and not a sqlalchemy column. + """ return ["registered_on", "id"] @staticmethod diff --git a/lib/pbench/test/unit/server/test_user_auth.py b/lib/pbench/test/unit/server/test_user_auth.py index 7f6923c0b0..c967622218 100644 --- a/lib/pbench/test/unit/server/test_user_auth.py +++ b/lib/pbench/test/unit/server/test_user_auth.py @@ -42,8 +42,6 @@ def test_registration(client, server_config, pytestconfig): email="user@domain.com", password="12345", ) - data = response.json - assert data["message"] == "Successfully registered." assert response.content_type == "application/json" assert response.status_code, 201 @@ -123,12 +121,10 @@ def test_user_login(client, server_config): email="user@domain.com", password="12345", ) - data_register = resp_register.json - assert data_register["message"] == "Successfully registered." + assert resp_register.status_code == 201 # registered user login response = login_user(client, server_config, "user", "12345") data = response.json - assert data["message"] == "Successfully logged in." assert data["auth_token"] assert data["username"] == "user" assert response.content_type == "application/json" @@ -148,12 +144,11 @@ def test_user_relogin(client, server_config): email="user@domain.com", password="12345", ) - data_register = resp_register.json - assert data_register["message"] == "Successfully registered." + assert resp_register.status_code == 201 + # registered user login response = login_user(client, server_config, "user", "12345") data = response.json - assert data["message"] == "Successfully logged in." assert data["auth_token"] assert response.content_type == "application/json" assert response.status_code == 200 @@ -165,8 +160,6 @@ def test_user_relogin(client, server_config): headers=dict(Authorization="Bearer " + data["auth_token"]), json={"username": "user", "password": "12345"}, ) - data = response.json - assert data["message"] == "Successfully logged in." assert response.status_code == 200 # Re-login without auth header @@ -175,8 +168,6 @@ def test_user_relogin(client, server_config): f"{server_config.rest_uri}/login", json={"username": "user", "password": "12345"}, ) - data = response.json - assert data["message"] == "Successfully logged in." assert response.status_code == 200 @staticmethod @@ -193,8 +184,8 @@ def test_user_login_with_wrong_password(client, server_config): email="user@domain.com", password="12345", ) - data_register = resp_register.json - assert data_register["message"] == "Successfully registered." + assert resp_register.status_code == 201 + # registered user login response = login_user(client, server_config, "user", "123456") data = response.json @@ -235,22 +226,21 @@ def test_get_user(client, server_config): email="user@domain.com", password="12345", ) - data_register = resp_register.json - assert data_register["message"] == "Successfully registered." + assert resp_register.status_code == 201 response = login_user(client, server_config, "username", "12345") + assert response.status_code == 200 data_login = response.json - assert data_login["message"] == "Successfully logged in." response = client.get( f"{server_config.rest_uri}/user/username", headers=dict(Authorization="Bearer " + data_login["auth_token"]), ) data = response.json - assert data["data"] is not None - assert data["data"]["email"] == "user@domain.com" - assert data["data"]["username"] == "username" - assert data["data"]["first_name"] == "firstname" assert response.status_code == 200 + assert data is not None + assert data["email"] == "user@domain.com" + assert data["username"] == "username" + assert data["first_name"] == "firstname" @staticmethod def test_update_user(client, server_config): @@ -265,12 +255,11 @@ def test_update_user(client, server_config): email="user@domain.com", password="12345", ) - data_register = resp_register.json - assert data_register["message"] == "Successfully registered." + assert resp_register.status_code == 201 response = login_user(client, server_config, "username", "12345") + assert response.status_code == 200 data_login = response.json - assert data_login["message"] == "Successfully logged in." new_registration_time = datetime.datetime.now() response = client.put( @@ -278,8 +267,8 @@ def test_update_user(client, server_config): json={"registered_on": new_registration_time, "first_name": "newname"}, headers=dict(Authorization="Bearer " + data_login["auth_token"]), ) - data = response.json assert response.status_code == 403 + data = response.json assert data["message"] == "Invalid update request payload" # Test password update @@ -290,13 +279,39 @@ def test_update_user(client, server_config): ) data = response.json assert response.status_code == 200 - assert data["data"]["first_name"] == "firstname" - assert data["data"]["email"] == "user@domain.com" + assert data["first_name"] == "firstname" + assert data["email"] == "user@domain.com" time.sleep(1) response = login_user(client, server_config, "username", "newpass") - data_login = response.json assert response.status_code == 200 - assert data_login["message"] == "Successfully logged in." + + @staticmethod + def test_external_token_update(client, server_config): + """ Test for external attempt at updating auth token""" + with client: + resp_register = register_user( + client, + server_config, + username="username", + firstname="firstname", + lastname="lastName", + email="user@domain.com", + password="12345", + ) + assert resp_register.status_code == 201 + + response = login_user(client, server_config, "username", "12345") + assert response.status_code == 200 + data_login = response.json + + response = client.put( + f"{server_config.rest_uri}/user/username", + json={"auth_tokens": "external_auth_token"}, + headers=dict(Authorization="Bearer " + data_login["auth_token"]), + ) + assert response.status_code == 400 + data = response.json + assert data["message"] == "Invalid fields in update request payload" @staticmethod def test_malformed_auth_token(client, server_config): @@ -311,7 +326,8 @@ def test_malformed_auth_token(client, server_config): email="user@domain.com", password="12345", ) - assert resp_register.json["message"] == "Successfully registered." + assert resp_register.status_code == 201 + response = client.get( f"{server_config.rest_uri}/user/username", headers=dict(Authorization="Bearer" + "malformed"), @@ -333,20 +349,18 @@ def test_valid_logout(client, server_config): email="user@domain.com", password="12345", ) - data_register = resp_register.json - assert data_register["message"] == "Successfully registered." + assert resp_register.status_code == 201 + # user login resp_login = login_user(client, server_config, "user", "12345") data_login = resp_login.json - assert data_login["message"] == "Successfully logged in." assert data_login["auth_token"] # valid token logout response = client.post( f"{server_config.rest_uri}/logout", headers=dict(Authorization="Bearer " + data_login["auth_token"]), ) - data = response.json - assert data["message"] == "Successfully logged out." + assert response.status_code == 200 # Check if the token has been successfully removed from the database assert ( not Database.db_session.query(ActiveTokens) @@ -369,22 +383,20 @@ def test_invalid_logout(client, server_config): email="user@domain.com", password="12345", ) - data_register = resp_register.json - assert data_register["message"] == "Successfully registered." assert resp_register.status_code == 201 + # user login resp_login = login_user(client, server_config, "username", "12345") data_login = resp_login.json - assert data_login["message"] == "Successfully logged in." - assert data_login["auth_token"] assert resp_login.status_code == 200 + assert data_login["auth_token"] # log out with the current token logout_response = client.post( f"{server_config.rest_uri}/logout", headers=dict(Authorization="Bearer " + data_login["auth_token"]), ) - assert logout_response.json["message"] == "Successfully logged out." + assert logout_response.status_code == 200 # invalid token logout response = client.post( @@ -407,19 +419,15 @@ def test_delete_user(client, server_config): email="user@domain.com", password="12345", ) - data_register = resp_register.json - assert data_register["message"] == "Successfully registered." + assert resp_register.status_code == 201 # user login resp_login = login_user(client, server_config, "username", "12345") data_login = resp_login.json - assert data_login["message"] == "Successfully logged in." assert data_login["auth_token"] response = client.delete( f"{server_config.rest_uri}/user/username", headers=dict(Authorization="Bearer " + data_login["auth_token"]), ) - data = response.json - assert data["message"] == "Successfully deleted." assert response.status_code == 200