Skip to content

Fix minor bug in the login workflow #2144

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
portante merged 1 commit into from
Mar 27, 2021
Merged

Fix minor bug in the login workflow #2144

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
117 changes: 48 additions & 69 deletions lib/pbench/server/api/resources/users_api.py
View file
Open in desktop
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
Open in desktop
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