Address comments

Author: npalaska

lib/pbench/server/api/auth.py

@@ -89,7 +89,7 @@ def verify_auth(auth_token):
         """
         Validates the auth token
         :param auth_token:
-        :return: integer|string
+        :return: User object/None
         """
         try:
             payload = jwt.decode(
@@ -99,7 +99,6 @@ def verify_auth(auth_token):
             if ActiveTokens.valid(auth_token):
                 user = User.query(id=user_id)
                 return user
-            return None
         except jwt.ExpiredSignatureError:
             try:
                 ActiveTokens.delete(auth_token)
@@ -113,12 +112,10 @@ def verify_auth(auth_token):
                 "User attempted Pbench expired token '{}', Token deleted from the database and no longer tracked",
                 auth_token,
             )
-            return None
         except jwt.InvalidTokenError:
             Auth.logger.warning("User attempted invalid Pbench token '{}'", auth_token)
-            return None
         except Exception:
             Auth.logger.exception(
                 "Exception occurred while verifying the auth token '{}'", auth_token
             )
-            return None
+        return None

lib/pbench/server/api/resources/users_api.py

@@ -4,7 +4,6 @@
 from flask_bcrypt import check_password_hash
 from email_validator import EmailNotValidError
 from sqlalchemy.exc import SQLAlchemyError, IntegrityError
-from configparser import NoOptionError
 from pbench.server.database.models.users import User
 from pbench.server.database.models.active_tokens import ActiveTokens
 from pbench.server.api.auth import Auth
@@ -139,13 +138,9 @@ def __init__(self, config, logger, auth):
         self.server_config = config
         self.logger = logger
         self.auth = auth
-        try:
-            self.token_expire_duration = self.server_config.get(
-                "pbench-server", "token_expiration_duration"
-            )
-        except NoOptionError:
-            # Defaults to 60 minutes
-            self.token_expire_duration = 60
+        self.token_expire_duration = self.server_config.get(
+            "pbench-server", "token_expiration_duration"
+        )
 
     @Auth.token_auth.login_required(optional=True)
     def post(self):
@@ -198,7 +193,7 @@ def post(self):
             self.logger.warning(
                 "No user found in the db for Username: {} while login", username
             )
-            abort(403, message="No such user, please register first")
+            abort(403, message="Bad login")
 
         # Validate the password
         if not check_password_hash(user.password, password):
@@ -443,7 +438,7 @@ def put(self, username):
                     field,
                     post_data[field],
                 )
-                abort(400, message="Invalid data in update request payload")
+                abort(403, message="Invalid update request payload")
         try:
             user.update(**post_data)
         except Exception:
@@ -500,7 +495,7 @@ def delete(self, username):
         else:
             if user.is_admin():
                 self.logger.warning("Admin attempted to delete admin user")
-                abort(405, message="Admin user can not be deleted")
+                abort(403, message="Admin user can not be deleted")
             self.logger.info("User entry deleted for user with username {}", username)
 
         response_object = {

lib/pbench/server/database/alembic/env.py

@@ -29,8 +29,6 @@
 log.addHandler(handler)
 
 # add your model's MetaData object here for 'autogenerate' support:
-# from myapp import mymodel
-# target_metadata = mymodel.Base.metadata
 
 target_metadata = Database.Base.metadata
 

lib/pbench/server/database/models/users.py

@@ -121,8 +121,6 @@ def is_admin(self):
     def is_admin_username(username):
         # TODO: Need to add an interface to fetch admins list instead of hard-coding the names, preferably via sql query
         admins = ["admin"]
-        if username in admins:
-            return True
-        return False
+        return username in admins
 
     # TODO: Add password recovery mechanism

lib/pbench/test/unit/server/test_user_auth.py

@@ -219,7 +219,7 @@ def test_non_registered_user_login(client, server_config):
         with client:
             response = login_user(client, server_config, "username", "12345")
             data = response.json
-            assert data["message"] == "No such user, please register first"
+            assert data["message"] == "Bad login"
             assert response.status_code == 403
 
     @staticmethod
@@ -279,8 +279,8 @@ def test_update_user(client, server_config):
                 headers=dict(Authorization="Bearer " + data_login["auth_token"]),
             )
             data = response.json
-            assert response.status_code == 400
-            assert data["message"] == "Invalid data in update request payload"
+            assert response.status_code == 403
+            assert data["message"] == "Invalid update request payload"
 
             # Test password update
             response = client.put(