feat: add "active" field to output and align on field name for username

- update tests to use username
- add some new checks on user.active

fence/blueprints/admin.py

@@ -78,7 +78,7 @@ def create_user():
 
     Returns a json object
     """
-    username = request.get_json().get("name", None)
+    username = request.get_json().get("username", None)
     role = request.get_json().get("role", None)
     email = request.get_json().get("email", None)
     display_name = request.get_json().get("display_name", None)
@@ -110,11 +110,13 @@ def update_user(username):
 
     Returns a json object
     """
-    name = request.get_json().get("name", None)
+    new_username = request.get_json().get("username", None)
     role = request.get_json().get("role", None)
     email = request.get_json().get("email", None)
     return jsonify(
-        admin.update_user(current_app.scoped_session(), username, role, email, name)
+        admin.update_user(
+            current_app.scoped_session(), username, role, email, new_username
+        )
     )
 
 

fence/resources/admin/admin_users.py

@@ -78,7 +78,7 @@ def get_all_users(current_session):
     users_names = []
     for user in users:
         new_user = {}
-        new_user["name"] = user.username
+        new_user["username"] = user.username
         if user.is_admin:
             new_user["role"] = "admin"
         else:
@@ -124,7 +124,7 @@ def create_user(
     except NotFound:
         logger.debug(f"User not found for: {username}. Checking again ignoring case...")
         user_list = [
-            user["name"].upper() for user in get_all_users(current_session)["users"]
+            user["username"].upper() for user in get_all_users(current_session)["users"]
         ]
         if username.upper() in user_list:
             logger.debug(f"User already exists for: {username}")
@@ -168,7 +168,7 @@ def create_user(
 def update_user(current_session, username, role, email, new_name):
     usr = us.get_user(current_session, username)
     user_list = [
-        user["name"].upper() for user in get_all_users(current_session)["users"]
+        user["username"].upper() for user in get_all_users(current_session)["users"]
     ]
     if (
         new_name

fence/resources/user/__init__.py

@@ -91,6 +91,7 @@ def get_user_info(current_session, username):
         "phone_number": user.phone_number,
         "email": user.email,
         "is_admin": user.is_admin,
+        "active": user.active,
         "role": role,
         "project_access": dict(user.project_access),
         "certificates_uploaded": [],

tests/admin/test_admin_users.py

@@ -17,7 +17,7 @@ def test_get_user(db_session, awg_users):
     assert "test_group_1" in info["groups"]
     assert "test_group_2" in info["groups"]
     assert info["message"] == ""
-    assert info["email"] == None
+    assert info["email"] is None
     assert info["certificates_uploaded"] == []
     assert info["resources_granted"] == []
     assert info["project_access"]["phs_project_1"] == ["read"]
@@ -34,6 +34,7 @@ def test_create_user(db_session, oauth_client):
     assert user.phone_number is None
     assert user.identity_provider is None
     assert len(user.tags) == 0
+    assert user.active == True
 
 
 def test_create_user_with_all_fields_set(db_session, oauth_client):
@@ -133,7 +134,7 @@ def test_create_already_existing_user(db_session, awg_users):
 
 def test_get_all_users(db_session, awg_users):
     user_list = adm.get_all_users(db_session)
-    user_name_list = [item["name"] for item in user_list["users"]]
+    user_name_list = [item["username"] for item in user_list["users"]]
     assert "awg_user" in user_name_list
     assert "awg_user_2" in user_name_list
 

tests/admin/test_admin_users_endpoints.py

@@ -249,7 +249,7 @@ def test_get_user(
     assert r.status_code == 200
     # should at least have the users added from above (may have more from other tests)
     assert len(r.json["users"]) >= 4
-    usernames = [user["name"] for user in r.json["users"]]
+    usernames = [user["username"] for user in r.json["users"]]
     assert "test_a" in usernames
     assert "test_b" in usernames
     assert "test_amazing_user_with_an_fancy_but_extremely_long_name" in usernames
@@ -274,7 +274,11 @@ def test_post_user(client, admin_user, encoded_admin_jwt, db_session):
             "Content-Type": "application/json",
         },
         data=json.dumps(
-            {"name": "new_test_user", "role": "user", "email": "new_test_user@fake.com"}
+            {
+                "username": "new_test_user",
+                "role": "user",
+                "email": "new_test_user@fake.com",
+            }
         ),
     )
     assert r.status_code == 200
@@ -284,10 +288,12 @@ def test_post_user(client, admin_user, encoded_admin_jwt, db_session):
     assert r.json["email"] == "new_test_user@fake.com"
     assert r.json["project_access"] == {}
     assert r.json["groups"] == []
+    assert r.json["active"] == True
     new_test_user = db_session.query(User).filter_by(username="new_test_user").one()
     assert new_test_user.username == "new_test_user"
     assert new_test_user.is_admin == False
     assert new_test_user.email == "new_test_user@fake.com"
+    assert new_test_user.active == True
 
 
 def test_post_user_no_fields_defined(client, admin_user, encoded_admin_jwt, db_session):
@@ -341,7 +347,7 @@ def test_post_user_only_username_defined(
             "Authorization": "Bearer " + encoded_admin_jwt,
             "Content-Type": "application/json",
         },
-        data=json.dumps({"name": "new_test_user"}),
+        data=json.dumps({"username": "new_test_user"}),
     )
     assert r.status_code == 200
     assert r.json["username"] == "new_test_user"
@@ -366,7 +372,7 @@ def test_post_user_already_exists(
             "Authorization": "Bearer " + encoded_admin_jwt,
             "Content-Type": "application/json",
         },
-        data=json.dumps({"name": "test_a"}),
+        data=json.dumps({"username": "test_a"}),
     )
     assert r.status_code == 400
 
@@ -392,7 +398,7 @@ def test_put_user_username(
         },
         data=json.dumps(
             {
-                "name": "test_a_updated",
+                "username": "test_a_updated",
                 "role": "admin",
                 "email": "test_a_updated@fake.com",
             }
@@ -422,7 +428,7 @@ def test_put_user_username_nonexistent(
             "Authorization": "Bearer " + encoded_admin_jwt,
             "Content-Type": "application/json",
         },
-        data=json.dumps({"name": "test_nonexistent_updated"}),
+        data=json.dumps({"username": "test_nonexistent_updated"}),
     )
     assert r.status_code == 404
     assert (
@@ -442,7 +448,7 @@ def test_put_user_username_already_exists(
             "Authorization": "Bearer " + encoded_admin_jwt,
             "Content-Type": "application/json",
         },
-        data=json.dumps({"name": "test_b"}),
+        data=json.dumps({"username": "test_b"}),
     )
     assert r.status_code == 400
     assert db_session.query(User).filter_by(username="test_a").one()
@@ -455,7 +461,7 @@ def test_put_user_username_try_delete_username(
     """PUT /users/<username>: [update_user] try to delete username"""
     """
     This probably shouldn't be allowed. Conveniently, the code flow ends up
-    the same as though the user had not tried to update 'name' at all,
+    the same as though the user had not tried to update 'username' at all,
     since they pass in None. Right now, this just returns a 200 without
     updating anything or sending any message to the user. So the test has
     been written to ensure this behavior, but maybe it should be noted that
@@ -467,7 +473,7 @@ def test_put_user_username_try_delete_username(
             "Authorization": "Bearer " + encoded_admin_jwt,
             "Content-Type": "application/json",
         },
-        data=json.dumps({"name": None}),
+        data=json.dumps({"username": None}),
     )
     assert r.status_code == 200
     user = db_session.query(User).filter_by(username="test_a").one()