Skip to content

Commit

Permalink
fix login workflow, unit test for extrenal auth_token update
Browse files Browse the repository at this point in the history
  • Loading branch information
@npalaska
npalaska committed Mar 26, 2021
1 parent 3912bb7 commit 29861a2
Show file tree
Hide file tree
Showing 3 changed files with 106 additions and 114 deletions.
117 changes: 48 additions & 69 deletions lib/pbench/server/api/resources/users_api.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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: <status_Code>,
response_object = {
"message": "failure message"
}
To get the auth token user has to perform the login action
"""
# get the post data
Expand Down Expand Up @@ -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}")
Expand All @@ -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.
Expand All @@ -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": "<authorization_token>", # Will not present if failed
}
Success: 200,
response_object = {
"auth_token": "<authorization_token>"
"username": <username>
}
Failure: <status_Code>,
response_object = {
"message": "failure message"
}
"""
# get the post data
post_data = request.get_json()
Expand Down Expand Up @@ -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)
Expand All @@ -250,7 +237,6 @@ def post(self):
abort(500, message="INTERNAL ERROR")

response_object = {
"message": "Successfully logged in.",
"auth_token": auth_token,
"username": username,
}
Expand All @@ -276,10 +262,12 @@ def post(self):
Required headers include
Authorization: Bearer <Pbench authentication token (user received upon login)>
:return: JSON Payload
response_object = {
"message": "Successfully logged out."/"failure message",
}
:return:
Success: 200 with empty payload
Failure: <status_Code>,
response_object = {
"message": "failure message"
}
"""
auth_token = self.auth.token_auth.get_auth()["token"]
user = self.auth.token_auth.current_user()
Expand All @@ -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):
Expand All @@ -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": <username>,
"first_name": <firstName>,
"last_name": <lastName>,
"registered_on": registered_on,
"registered_on": <registered_on>,
}
Failure: <status_Code>,
response_object = {
"message": "failure message"
}
}
"""
try:
user, verified = self.auth.verify_user(username)
Expand All @@ -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(
Expand All @@ -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,
Expand Down Expand Up @@ -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": <username>,
"first_name": <firstName>,
"last_name": <lastName>,
"registered_on": registered_on,
"registered_on": <registered_on>,
}
Failure: <status_Code>,
response_object = {
"message": "failure message"
}
}
"""
post_data = request.get_json()
if not post_data:
Expand Down Expand Up @@ -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()
Expand All @@ -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: <status_Code>,
response_object = {
"message": "failure message"
}
"""
try:
user, verified = self.auth.verify_user(username)
Expand Down Expand Up @@ -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
5 changes: 5 additions & 0 deletions lib/pbench/server/database/models/users.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading

0 comments on commit 29861a2

Please sign in to comment.