remove kwargs and add sqlalchemy validators

Author: npalaska

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

@@ -1,4 +1,5 @@
 import jwt
+import datetime
 from flask import request, jsonify, make_response
 from flask_restful import Resource, abort
 from flask_bcrypt import check_password_hash
@@ -126,6 +127,7 @@ def post(self):
                 first_name=first_name,
                 last_name=last_name,
                 email=email_id,
+                registered_on=datetime.datetime.now(),
             )
 
             # insert the user
@@ -439,33 +441,7 @@ def put(self, username):
             abort(500, message="INTERNAL ERROR")
 
         # TODO: Check if the user has the right privileges
-        if verified:
-            # Check if the user payload contain fields that are either non-updatabale or
-            # are not present in the user db. If any key in the payload does not match
-            # with the column name we will abort the update request.
-            non_updatable = list(
-                set(post_data.keys()) - set(User.__table__.columns.keys())
-            )
-            if non_updatable:
-                self.logger.warning(
-                    "User trying to update fields that are not present in the user database. Fields: {}",
-                    non_updatable,
-                )
-                abort(400, message="Invalid fields in update request payload")
-            try:
-                # 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.
-                user.update(**post_data)
-            except ValueError:
-                self.logger.warning(
-                    "Either provided values to update the user does not adhere to the user model "
-                    "datatype or user attempted to update the protected properties."
-                )
-                abort(400, message="Invalid fields in update request payload")
-            except Exception:
-                self.logger.exception("Exception occurred during updating user object")
-                abort(500, message="INTERNAL ERROR")
-        else:
+        if not verified:
             self.logger.warning(
                 "Username retrieved from the auth token {} is different from the username provided",
                 self.auth.token_auth.current_user().id,
@@ -474,6 +450,41 @@ def put(self, username):
                 403, message="Authentication token does not belong to the current user"
             )
 
+        # Check if the user payload contain fields that are either non-updatabale or
+        # are not present in the user db. If any key in the payload does not match
+        # with the column name we will abort the update request.
+        non_existent = list(set(post_data.keys()) - set(User.__table__.columns.keys()))
+        non_updatable = list(
+            set(post_data.keys()).intersection(set(User.get_protected()))
+        )
+        if non_updatable:
+            for field in non_updatable:
+                if getattr(user, field) != post_data[f"{field}"]:
+                    self.logger.warning(
+                        "Either provided values to update the user does not adhere to the user model "
+                        "datatype or user attempted to update the protected properties."
+                    )
+                    abort(400, message="Invalid data in update request payload")
+        if non_existent:
+            self.logger.warning(
+                "User trying to update fields that are not present in the user database. Fields: {}",
+                non_existent,
+            )
+            abort(400, message="Invalid fields in update request payload")
+        try:
+            # 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.
+            user.update(**post_data)
+        except ValueError:
+            self.logger.warning(
+                "Either provided values to update the user does not adhere to the user model "
+                "datatype or user attempted to update the protected properties."
+            )
+            abort(400, message="Invalid fields in update request payload")
+        except Exception:
+            self.logger.exception("Exception occurred during updating user object")
+            abort(500, message="INTERNAL ERROR")
+
         return_data = user.get_json()
         response_object = {
             "status": "success",

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

@@ -1,4 +1,3 @@
-import datetime
 from flask_bcrypt import generate_password_hash
 from email_validator import validate_email
 from sqlalchemy import Column, Integer, String, DateTime, LargeBinary
@@ -21,15 +20,6 @@ class User(Database.Base):
     email = Column(String(255), unique=True, nullable=False)
     auth_tokens = relationship("ActiveTokens", backref="users")
 
-    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.password = generate_password_hash(kwargs.get("password"))
-        self.email = kwargs.get("email")
-        self.registered_on = datetime.datetime.now()
-
     def __str__(self):
         return f"User, id: {self.id}, username: {self.username}"
 
@@ -42,6 +32,10 @@ def get_json(self):
             "registered_on": self.registered_on,
         }
 
+    @staticmethod
+    def get_protected():
+        return ["registered_on", "id"]
+
     @staticmethod
     def query(id=None, username=None, email=None):
         # Currently we would only query with single argument. Argument need to be either username/id/email
@@ -72,13 +66,9 @@ def add(self):
             Database.db_session.rollback()
             raise e
 
-    # Prevent update on "registered_on" field
-    @validates("registered_on")
-    def evaluate_registered_on(self, key, value):
-        if self.registered_on:  # Field already exists
-            if self.registered_on.strftime("%Y-%m-%d %H:%M:%S.%f") != value:
-                raise ValueError("registered_on cannot be modified.")
-        return value
+    @validates("password")
+    def evaluate_password(self, key, value):
+        return generate_password_hash(value)
 
     # validate the email field
     @validates("email")
@@ -101,8 +91,6 @@ def update(self, **kwargs):
                     # Insert the auth token
                     self.auth_tokens.append(value)
                     Database.db_session.add(value)
-                elif key == "password":
-                    setattr(self, key, generate_password_hash(value))
                 else:
                     setattr(self, key, value)
             Database.db_session.commit()

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

@@ -92,6 +92,7 @@ def test_registration_with_registered_user(client, server_config):
             username="user",
             first_name="firstname",
             last_name="lastname",
+            registered_on=datetime.datetime.now(),
         )
         Database.db_session.add(user)
         Database.db_session.commit()
@@ -286,7 +287,7 @@ def test_update_user(client, server_config):
             )
             data = response.json
             assert response.status_code == 400
-            assert data["message"] == "Invalid fields in update request payload"
+            assert data["message"] == "Invalid data in update request payload"
 
             # Test password update
             response = client.put(
@@ -302,6 +303,7 @@ def test_update_user(client, server_config):
             time.sleep(1)
             response = login_user(client, server_config, "username", "newpass")
             data_login = response.json
+            assert response.status_code == 200
             assert data_login["status"] == "success"
             assert data_login["message"] == "Successfully logged in."