Use default bcrypt log rounds, check json payload before updating user fields

Author: npalaska

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

@@ -19,9 +19,6 @@ class RegisterUser(Resource):
     def __init__(self, config, logger):
         self.server_config = config
         self.logger = logger
-        self.bcrypt_log_rounds = int(
-            self.server_config.get("pbench-server", "bycrypt_log_rounds")
-        )
         try:
             self.token_expire_duration = self.server_config.get(
                 "pbench-server", "token_expiration_duration"
@@ -145,7 +142,6 @@ def post(self):
 
         try:
             user = User(
-                bcrypt_log_rounds=self.bcrypt_log_rounds,
                 username=username,
                 password=password,
                 first_name=first_name,
@@ -440,15 +436,31 @@ def put(self, username):
                 }
         """
         post_data = request.get_json()
+        if not post_data:
+            self.logger.warning("Invalid json object: {}", request.url)
+            abort(400, message="Invalid json object in request")
+
         try:
             user, verified = self.auth.verify_user(username)
         except Exception:
             self.logger.exception("Exception occurred while verifying the user")
             abort(500, message="INTERNAL ERROR")
 
         # TODO: Check if the user has the right privileges
-        if verified or user.is_admin():
+        if verified:
             try:
+                # Log if the user payload contain fields that are either non-updatabale or
+                # are not present in the user db.
+                non_updatable = list(
+                    set(post_data.keys()) - set(User.__table__.columns.keys())
+                )
+                if "registered_on" in post_data.keys():
+                    non_updatable.append("registered_on")
+                if non_updatable:
+                    self.logger.warning(
+                        "User trying to update fields that are either non-updatable or does not present in the user database. Fields: {}",
+                        non_updatable,
+                    )
                 # We will update the user object with the keys and values provided in the request payload.
                 # THe keys need to match with the column names in the user model. However, if any key in
                 # the payload does not match with the column name we just skip that field.

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

@@ -14,21 +14,17 @@ class User(Database.Base):
     username = Column(String(255), unique=True, nullable=False)
     first_name = Column(String(255), unique=False, nullable=False)
     last_name = Column(String(255), unique=False, nullable=False)
-    password = Column(LargeBinary(500), nullable=False)
+    password = Column(LargeBinary(128), nullable=False)
     registered_on = Column(DateTime, nullable=False)
-    bcrypt_log_rounds = Column(Integer, nullable=False)
     email = Column(String(255), unique=True, nullable=False)
     auth_tokens = relationship("ActiveTokens", backref="users")
 
-    def __init__(self, bcrypt_log_rounds, **kwargs):
+    def __init__(self, **kwargs):
         super().__init__(**kwargs)
         self.username = kwargs.get("username")
         self.first_name = kwargs.get("first_name")
         self.last_name = kwargs.get("last_name")
-        self.bcrypt_log_rounds = bcrypt_log_rounds
-        self.password = generate_password_hash(
-            kwargs.get("password"), bcrypt_log_rounds
-        )
+        self.password = generate_password_hash(kwargs.get("password"))
         self.email = kwargs.get("email")
         self.registered_on = datetime.datetime.now()
 
@@ -75,11 +71,9 @@ def update(self, **kwargs):
                     self.auth_tokens.append(value)
                     Database.db_session.add(value)
                 elif key == "password":
-                    setattr(
-                        self, key, generate_password_hash(value, self.bcrypt_log_rounds)
-                    )
-                # Prevent update on "registered_on" and "bcrypt_log_rounds" fields
-                elif key in ["registered_on", "bcrypt_log_rounds"]:
+                    setattr(self, key, generate_password_hash(value))
+                # Prevent update on "registered_on" field
+                elif key == "registered_on":
                     continue
                 else:
                     setattr(self, key, value)

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

@@ -87,7 +87,6 @@ def test_registration_email_validity(client, server_config):
     def test_registration_with_registered_user(client, server_config):
         """ Test registration with already registered email"""
         user = User(
-            int(server_config.get("pbench-server", "bycrypt_log_rounds")),
             email="[email protected]",
             password="12345",
             username="user",

server/lib/config/pbench-server-default.cfg

@@ -29,8 +29,6 @@ mailto=%(admin-email)s
 mailfrom=%(user)s@%(host)s
 commit_id=unknown
 
-# bycrypt log round number for hashing passwords
-bycrypt_log_rounds = 5
 # Token expiration duration in minutes, can be overridden in the main config file, defaults to 10 mins
 token_expiration_duration = 10