From 20827b12d841a1afb6317992f74d56a4adfd4658 Mon Sep 17 00:00:00 2001 From: c-simpson Date: Mon, 10 May 2021 17:14:58 -0400 Subject: [PATCH 01/11] Updated user_create() for JSON input Also updated jwt_op for requesting user. --- src/server/api/user_api.py | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/src/server/api/user_api.py b/src/server/api/user_api.py index 7910693e..bc34d271 100644 --- a/src/server/api/user_api.py +++ b/src/server/api/user_api.py @@ -208,7 +208,7 @@ def user_create(): Requires admin role - Form POST Parameters + Form POST JSON Parameters ---------- username : str full_name : str @@ -222,12 +222,18 @@ def user_create(): Duplicate user: 409 + DB error """ - new_user = request.form["username"] - fullname = request.form["full_name"] - userpw = request.form["password"] - user_role = request.form["role"] - requesting_user = jwt_ops.get_jwt_user() + try: + post_dict = json.loads(request.data) + new_user = post_dict["username"] + fullname = post_dict["full_name"] + userpw = post_dict["password"] + user_role = post_dict["role"] + except: + return jsonify("Missing one or more parameters"), 400 + + + requesting_user = jwt_ops.validate_decode_jwt()['sub'] pw_hash = hash_password(userpw) From ba4f13ad51d6de1db940a558cf72c13b7a324e79 Mon Sep 17 00:00:00 2001 From: c-simpson Date: Tue, 11 May 2021 15:59:21 -0400 Subject: [PATCH 02/11] Added check_username() --- src/server/api/user_api.py | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/src/server/api/user_api.py b/src/server/api/user_api.py index bc34d271..5c87dbcd 100644 --- a/src/server/api/user_api.py +++ b/src/server/api/user_api.py @@ -293,6 +293,34 @@ def get_user_count(): return jsonify(user_count[0]) +@user_api.route("/api/admin/user/check_name", methods=["POST"]) +@jwt_ops.admin_required +def check_username(): + """Return 1 if username exists already, else 0.""" + + try: + post_dict = json.loads(request.data) + test_username = post_dict["username"] + except: + return jsonify("Missing username"), 400 + + with engine.connect() as connection: + + s = text( """select count(username) from pdp_users where username=:u """ ) + s = s.bindparams(u=test_username) + result = connection.execute(s) + + if result.rowcount: # As we're doing a count() we *should* get a result + user_exists = result.fetchone()[0] + else: + log_user_action(test_username, "Failure", "Error when checking username") + return jsonify("Error checking username"), 500 + + return jsonify(user_exists) + + + + # TODO: A single do-all update_user() @user_api.route("/api/admin/user/deactivate", methods=["POST"]) @jwt_ops.admin_required From b1e4087bb8f81fe494ad64bf958e1b2fdd7c747e Mon Sep 17 00:00:00 2001 From: c-simpson Date: Tue, 11 May 2021 16:21:08 -0400 Subject: [PATCH 03/11] Created tests for check_username Also added admin header to currentFiles and statistics. moved in file --- src/server/test_api.py | 63 ++++++++++++++++++++++++++++++++++-------- 1 file changed, 52 insertions(+), 11 deletions(-) diff --git a/src/server/test_api.py b/src/server/test_api.py index 9320f996..1de33117 100644 --- a/src/server/test_api.py +++ b/src/server/test_api.py @@ -70,17 +70,6 @@ def test_client_dns(): # Simple API tests ################################################ -def test_currentFiles(): - """360 view Current Files list""" - response = requests.get(SERVER_URL + "/api/listCurrentFiles") - assert response.status_code == 200 - - -def test_statistics(): - """360 view Statistics""" - response = requests.get(SERVER_URL + "/api/statistics") - assert response.status_code == 200 - def test_usertest(): """Verify liveness test works""" @@ -181,6 +170,36 @@ def test_admingetusers(state: State): userlist = response.json() assert len(userlist) > 1 +def test_check_usernames(state: State): + """Verify logged-in base_admin can test usernames, gets correct result - existing user """ + # Build auth string value including token from state + b_string = 'Bearer ' + state.state['base_admin'] + + assert len(b_string) > 24 + + auth_hdr = {'Authorization' : b_string} + + data = {"username":"base_admin"} + response = requests.post(SERVER_URL + "/api/admin/user/check_name", headers=auth_hdr, json=data) + assert response.status_code == 200 + + is_user = response.json() + assert is_user == 1 + +def test_check_badusernames(state: State): + """Verify logged-in base_admin can test usernames, gets correct result - nonexistant user """ + # Build auth string value including token from state + b_string = 'Bearer ' + state.state['base_admin'] + assert len(b_string) > 24 + auth_hdr = {'Authorization' : b_string} + + data = {"username":"got_no_username_like_this"} + response = requests.post(SERVER_URL + "/api/admin/user/check_name", headers=auth_hdr, json=data) + assert response.status_code == 200 + + is_user = response.json() + assert is_user == 0 + def test_usergetusers(state: State): """Verify logged-in base_user *cannot* use JWT to get user list """ @@ -192,3 +211,25 @@ def test_usergetusers(state: State): auth_hdr = {'Authorization' : b_string} response = requests.get(SERVER_URL + "/api/admin/user/get_users", headers=auth_hdr) assert response.status_code == 403 + + +def test_currentFiles(state: State): + """360 view Current Files list""" + + b_string = 'Bearer ' + state.state['base_admin'] + assert len(b_string) > 24 + auth_hdr = {'Authorization' : b_string} + + response = requests.get(SERVER_URL + "/api/listCurrentFiles", headers=auth_hdr) + assert response.status_code == 200 + + +def test_statistics(state: State): + """360 view Statistics""" + + b_string = 'Bearer ' + state.state['base_admin'] + assert len(b_string) > 24 + auth_hdr = {'Authorization' : b_string} + + response = requests.get(SERVER_URL + "/api/statistics", headers=auth_hdr) + assert response.status_code == 200 \ No newline at end of file From 621ad10ddafcb090a23de4dc9563303a6ac9031c Mon Sep 17 00:00:00 2001 From: c-simpson Date: Tue, 11 May 2021 17:44:22 -0400 Subject: [PATCH 04/11] user_update() WIP --- src/server/api/user_api.py | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/src/server/api/user_api.py b/src/server/api/user_api.py index 5c87dbcd..5baad13b 100644 --- a/src/server/api/user_api.py +++ b/src/server/api/user_api.py @@ -318,8 +318,35 @@ def check_username(): return jsonify(user_exists) +@user_api.route("/api/admin/user/update", methods=["POST"]) +# @jwt_ops.admin_required +def user_update(): + """Update existing user record + """ + + post_dict = json.loads(request.data) + + try: + username = post_dict["username"] + except: + return jsonify("Must specify username"), 400 + + # We should get 1+ values to update + + update_dict = {"username": username} + + # Need to be a bit defensive here & select what we want instead of taking what we're given + for key in ["full_name", "password", "role"]: + try: + val = post_dict[key] + update_dict[key] = val + except: + pass + #TODO: WIP + print(update_dict) + return jsonify("foo") # TODO: A single do-all update_user() @user_api.route("/api/admin/user/deactivate", methods=["POST"]) From c1bdb2c60d3006ff7f16c1708170bc4f98823773 Mon Sep 17 00:00:00 2001 From: c-simpson Date: Tue, 11 May 2021 20:58:33 -0400 Subject: [PATCH 05/11] Added user_get_info() --- src/server/api/user_api.py | 31 ++++++++++++++++++++++++++----- 1 file changed, 26 insertions(+), 5 deletions(-) diff --git a/src/server/api/user_api.py b/src/server/api/user_api.py index 5baad13b..348bdaf0 100644 --- a/src/server/api/user_api.py +++ b/src/server/api/user_api.py @@ -319,7 +319,7 @@ def check_username(): return jsonify(user_exists) @user_api.route("/api/admin/user/update", methods=["POST"]) -# @jwt_ops.admin_required +@jwt_ops.admin_required def user_update(): """Update existing user record """ @@ -346,7 +346,7 @@ def user_update(): #TODO: WIP print(update_dict) - return jsonify("foo") + return jsonify("Work in process") # TODO: A single do-all update_user() @user_api.route("/api/admin/user/deactivate", methods=["POST"]) @@ -370,9 +370,6 @@ def user_activate(): def user_get_list(): """Return list of users""" - # pu = Table("pdp_users", metadata, autoload=True, autoload_with=engine) - # pr = Table("pdp_user_roles", metadata, autoload=True, autoload_with=engine) - with engine.connect() as connection: s = text( @@ -392,3 +389,27 @@ def user_get_list(): return jsonify(ul), 200 +@user_api.route("/api/admin/user/get_info/", methods=["GET"]) +@jwt_ops.admin_required +def user_get_info(username): + """Return info on a specified user""" + + with engine.connect() as connection: + + s = text( + """ select username, full_name, active, pr.role + from pdp_users as pu + left join pdp_user_roles as pr on pu.role = pr._id + where username=:u + """ + ) + s = s.bindparams(u=username) + result = connection.execute(s) + + if result.rowcount: + user_row = result.fetchone() + else: + log_user_action(username, "Failure", "Error when getting user info") + return jsonify("Username not found"), 400 + + return jsonify( dict(zip(result.keys(), user_row)) ), 200 \ No newline at end of file From 6f076b10f48d0e60709d65fa9b130cc65f850bec Mon Sep 17 00:00:00 2001 From: c-simpson Date: Tue, 11 May 2021 21:29:07 -0400 Subject: [PATCH 06/11] Added full_names to created base users --- src/server/user_mgmt/base_users.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/server/user_mgmt/base_users.py b/src/server/user_mgmt/base_users.py index a35bd4b3..29629d51 100644 --- a/src/server/user_mgmt/base_users.py +++ b/src/server/user_mgmt/base_users.py @@ -58,28 +58,28 @@ def create_base_users(): # TODO: Just call create_user for each # user pw_hash = user_api.hash_password(BASEUSER_PW) ins_stmt = pu.insert().values( - username="base_user", password=pw_hash, active="Y", role=1, + username="base_user", full_name="Base User", password=pw_hash, active="Y", role=1, ) connection.execute(ins_stmt) # INactive user # Reuse pw hash ins_stmt = pu.insert().values( - username="base_user_inact", password=pw_hash, active="N", role=1, + username="base_user_inact", full_name="Inactive User", password=pw_hash, active="N", role=1, ) connection.execute(ins_stmt) # editor pw_hash = user_api.hash_password(BASEEDITOR_PW) ins_stmt = pu.insert().values( - username="base_editor", password=pw_hash, active="Y", role=2, + username="base_editor", full_name="Base Editor", password=pw_hash, active="Y", role=2, ) connection.execute(ins_stmt) # admin pw_hash = user_api.hash_password(BASEADMIN_PW) ins_stmt = pu.insert().values( - username="base_admin", password=pw_hash, active="Y", role=9, + username="base_admin", full_name="Base Admin", password=pw_hash, active="Y", role=9, ) connection.execute(ins_stmt) From 5c180470e9551cde308d47a345f4400d1417aa5e Mon Sep 17 00:00:00 2001 From: c-simpson Date: Wed, 12 May 2021 16:06:49 -0400 Subject: [PATCH 07/11] user/update WIP --- src/server/api/user_api.py | 37 +++++++++++++++++++++++++++++++++---- 1 file changed, 33 insertions(+), 4 deletions(-) diff --git a/src/server/api/user_api.py b/src/server/api/user_api.py index 348bdaf0..394ab439 100644 --- a/src/server/api/user_api.py +++ b/src/server/api/user_api.py @@ -319,7 +319,7 @@ def check_username(): return jsonify(user_exists) @user_api.route("/api/admin/user/update", methods=["POST"]) -@jwt_ops.admin_required +#@jwt_ops.admin_required #TODO: remove comment def user_update(): """Update existing user record """ @@ -333,18 +333,47 @@ def user_update(): # We should get 1+ values to update - update_dict = {"username": username} + update_dict = {} # Need to be a bit defensive here & select what we want instead of taking what we're given - for key in ["full_name", "password", "role"]: + for key in ["full_name", "active", "role", "password"]: try: val = post_dict[key] update_dict[key] = val except: pass - #TODO: WIP print(update_dict) + + # If updating password, need to hash first + + + # We have a variable number of columns to update. + # We could generate a text query on the fly, but this seems the perfect place to use the ORM + # and let it handle the update for us. + + + # with engine.connect() as connection: + + # for k,v in update_dict.items(): #TODO: Make less awful + # s = text( """UPDATE pdp_users SET :key=:val where username=:u """ ) + # s = s.bindparams(u=username, key=k, val=v) + # result = connection.execute(s) + + from sqlalchemy import update + from sqlalchemy.orm import Session + + with Session(engine) as session: + + PU = Table("pdp_users", metadata, autoload=True, autoload_with=engine) + # pr = Table("pdp_user_roles", metadata, autoload=True, autoload_with=engine) + + + stmt = update(PU).where(PU.username == username).values(update_dict).\ + execution_options(synchronize_session="fetch") + + result = session.execute(stmt) + return jsonify("Work in process") From 4cca9e7b069cec46970f4097f83c576c7fc21e06 Mon Sep 17 00:00:00 2001 From: c-simpson Date: Thu, 13 May 2021 22:45:30 -0400 Subject: [PATCH 08/11] Functional but incomplete user/update Removed stubs for activate. deactivate --- src/server/api/user_api.py | 48 ++++++++++++-------------------------- 1 file changed, 15 insertions(+), 33 deletions(-) diff --git a/src/server/api/user_api.py b/src/server/api/user_api.py index 394ab439..ec83944f 100644 --- a/src/server/api/user_api.py +++ b/src/server/api/user_api.py @@ -343,55 +343,37 @@ def user_update(): except: pass - print(update_dict) + if not update_dict: + return jsonify("No changed items specified") - # If updating password, need to hash first + # TODO: If updating password, need to hash first # We have a variable number of columns to update. # We could generate a text query on the fly, but this seems the perfect place to use the ORM # and let it handle the update for us. - - # with engine.connect() as connection: - - # for k,v in update_dict.items(): #TODO: Make less awful - # s = text( """UPDATE pdp_users SET :key=:val where username=:u """ ) - # s = s.bindparams(u=username, key=k, val=v) - # result = connection.execute(s) - from sqlalchemy import update - from sqlalchemy.orm import Session + from sqlalchemy.orm import Session, sessionmaker - with Session(engine) as session: + Session = sessionmaker(engine) - PU = Table("pdp_users", metadata, autoload=True, autoload_with=engine) - # pr = Table("pdp_user_roles", metadata, autoload=True, autoload_with=engine) + session = Session() + # #TODO: Figure out context manager doesn't work or do try/finally + PU = Table("pdp_users", metadata, autoload=True, autoload_with=engine) + # pr = Table("pdp_user_roles", metadata, autoload=True, autoload_with=engine) - stmt = update(PU).where(PU.username == username).values(update_dict).\ - execution_options(synchronize_session="fetch") - result = session.execute(stmt) + stmt = update(PU).where(PU.columns.username == username).values(update_dict).\ + execution_options(synchronize_session="fetch") + result = session.execute(stmt) - return jsonify("Work in process") + session.commit() + session.close() -# TODO: A single do-all update_user() -@user_api.route("/api/admin/user/deactivate", methods=["POST"]) -@jwt_ops.admin_required -def user_deactivate(): - """Mark user as inactive in DB""" - # TODO - return "", 200 - - -@user_api.route("/api/admin/user/activate", methods=["POST"]) -@jwt_ops.admin_required -def user_activate(): - """Mark user as active in DB""" - # TODO - return "", 200 + return jsonify("Updated") @user_api.route("/api/admin/user/get_users", methods=["GET"]) From e20f5dd7d28047cb23fda8aef303ebcddbbfa04c Mon Sep 17 00:00:00 2001 From: c-simpson Date: Fri, 14 May 2021 09:57:12 -0400 Subject: [PATCH 09/11] Added password hashing, strength check --- src/server/api/user_api.py | 41 +++++++++++++++++++++++++++++++++----- 1 file changed, 36 insertions(+), 5 deletions(-) diff --git a/src/server/api/user_api.py b/src/server/api/user_api.py index ec83944f..d5cea534 100644 --- a/src/server/api/user_api.py +++ b/src/server/api/user_api.py @@ -33,6 +33,31 @@ def log_user_action(user, event_class, detail): except Exception as e: print(e) +def password_is_strong(password): + """ Check plain-text password against strength rules.""" + + def has_digit(test_string): + """Test if any character is a digit.""" + for c in test_string: + if c.isdigit(): + return True + return False + + def has_alpha(test_string): + """Test if any character is alphabetic.""" + for c in test_string: + if c.isalpha(): + return True + return False + + if (len(password) > 11 + and has_alpha(password) + and has_digit(password) ): + return True + + else: + return False + def hash_password(password): """ Generate salt+hash for storing in db""" @@ -331,8 +356,6 @@ def user_update(): except: return jsonify("Must specify username"), 400 - # We should get 1+ values to update - update_dict = {} # Need to be a bit defensive here & select what we want instead of taking what we're given @@ -343,10 +366,17 @@ def user_update(): except: pass - if not update_dict: - return jsonify("No changed items specified") - # TODO: If updating password, need to hash first + if not update_dict: + return jsonify("No changed items specified") # If nothing to do, declare victory + + if "password" in update_dict.keys(): + + if password_is_strong(update_dict['password']): + update_dict['password'] = hash_password(update_dict['password']) + else: + return jsonify("Password too weak") + # We have a variable number of columns to update. @@ -364,6 +394,7 @@ def user_update(): PU = Table("pdp_users", metadata, autoload=True, autoload_with=engine) # pr = Table("pdp_user_roles", metadata, autoload=True, autoload_with=engine) + #TODO: Check tendered role or join roles table for update stmt = update(PU).where(PU.columns.username == username).values(update_dict).\ execution_options(synchronize_session="fetch") From 9937bf598ce5a59a838f62474a122db50d190177 Mon Sep 17 00:00:00 2001 From: c-simpson Date: Tue, 25 May 2021 16:50:52 -0400 Subject: [PATCH 10/11] Reduced pw strength test to just 12+ chars --- src/server/api/user_api.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/server/api/user_api.py b/src/server/api/user_api.py index d5cea534..cd7db013 100644 --- a/src/server/api/user_api.py +++ b/src/server/api/user_api.py @@ -51,8 +51,9 @@ def has_alpha(test_string): return False if (len(password) > 11 - and has_alpha(password) - and has_digit(password) ): + # and has_alpha(password) + # and has_digit(password) + ): return True else: @@ -344,7 +345,7 @@ def check_username(): return jsonify(user_exists) @user_api.route("/api/admin/user/update", methods=["POST"]) -#@jwt_ops.admin_required #TODO: remove comment +@jwt_ops.admin_required def user_update(): """Update existing user record """ @@ -389,7 +390,7 @@ def user_update(): Session = sessionmaker(engine) session = Session() - # #TODO: Figure out context manager doesn't work or do try/finally + # #TODO: Figure out why context manager doesn't work or do try/finally PU = Table("pdp_users", metadata, autoload=True, autoload_with=engine) # pr = Table("pdp_user_roles", metadata, autoload=True, autoload_with=engine) From 48235103b0ffabf9066cb56dcca248063c469403 Mon Sep 17 00:00:00 2001 From: c-simpson Date: Tue, 25 May 2021 16:58:05 -0400 Subject: [PATCH 11/11] 295: Added logging for refresh --- src/server/api/user_api.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/server/api/user_api.py b/src/server/api/user_api.py index cd7db013..c6212cc7 100644 --- a/src/server/api/user_api.py +++ b/src/server/api/user_api.py @@ -217,6 +217,7 @@ def user_refresh(): if is_active[0].lower() == 'y': # In the user DB and still Active? token = jwt_ops.create_token(user_name,old_jwt['role']) + log_user_action(user_name, "Success", "Refreshed token") return token else: