From ba083035702ad7ed0fc0362637d8454bc8113497 Mon Sep 17 00:00:00 2001 From: Spencer Axelrod Date: Mon, 4 Nov 2024 03:20:24 -0600 Subject: [PATCH 01/13] initial unit test for s3 bucket regex --- .secrets.baseline | 6 ++--- tests/data/conftest.py | 21 +++++++++++++++++ tests/data/test_indexed_file.py | 42 +++++++++++++++++++++++++++++++-- 3 files changed, 64 insertions(+), 5 deletions(-) diff --git a/.secrets.baseline b/.secrets.baseline index 82a2c6b2a..5d9b840b5 100644 --- a/.secrets.baseline +++ b/.secrets.baseline @@ -314,14 +314,14 @@ "filename": "tests/data/test_indexed_file.py", "hashed_secret": "a62f2225bf70bfaccbc7f1ef2a397836717377de", "is_verified": false, - "line_number": 411 + "line_number": 449 }, { "type": "Secret Keyword", "filename": "tests/data/test_indexed_file.py", "hashed_secret": "c258a8d1264cc59de81f8b1975ac06732b1cf182", "is_verified": false, - "line_number": 432 + "line_number": 470 } ], "tests/keys/2018-05-01T21:29:02Z/jwt_private_key.pem": [ @@ -422,5 +422,5 @@ } ] }, - "generated_at": "2024-08-22T19:43:39Z" + "generated_at": "2024-11-04T09:20:13Z" } diff --git a/tests/data/conftest.py b/tests/data/conftest.py index a015dbfc7..f3c2edfec 100755 --- a/tests/data/conftest.py +++ b/tests/data/conftest.py @@ -2,6 +2,8 @@ Fixtures to support tests/data """ import pytest +from unittest.mock import MagicMock +from fence.blueprints.data.indexd import S3IndexedFileLocation @pytest.fixture(scope="function", params=("upload", "download")) @@ -21,3 +23,22 @@ def supported_protocol(request): Note that "az" is an internal mapping for a supported protocol """ return request.param + + +@pytest.fixture(params=["invalid_bucket*name", "validbucketname-alreadyvalid"]) +def s3_indexed_file_location(request): + """ + Provides a mock s3 file location instance, parameterized with a valid and invalid bucket_name + """ + mock_url = "only/needed/for/instantiation" + location = S3IndexedFileLocation(url=mock_url) + + # Mock parsed_url attributes, made an always valid value for fallback upon failed regex validation + location.parsed_url = MagicMock() + location.parsed_url.netloc = "validbucketname-netloc" + location.parsed_url.path = "/test/object" + + # Set bucket_name based on the parameter, can be valid or invalid + location.bucket_name = MagicMock(return_value=request.param) + + return location diff --git a/tests/data/test_indexed_file.py b/tests/data/test_indexed_file.py index b310c8204..20937855e 100755 --- a/tests/data/test_indexed_file.py +++ b/tests/data/test_indexed_file.py @@ -3,13 +3,17 @@ """ import json from unittest import mock -from mock import patch +from mock import patch, MagicMock import gen3cirrus import pytest import fence.blueprints.data.indexd as indexd -from fence.blueprints.data.indexd import IndexedFile, GoogleStorageIndexedFileLocation +from fence.blueprints.data.indexd import ( + IndexedFile, + GoogleStorageIndexedFileLocation, + S3IndexedFileLocation, +) from fence.models import ( AssumeRoleCacheGCP, GoogleServiceAccountKey, @@ -359,6 +363,40 @@ def test_internal_get_signed_url( ) +@patch("fence.blueprints.data.indexd.get_value") +def test_get_signed_url_s3_bucket_name(mock_get_value, s3_indexed_file_location, app): + # Mock config with buckets for s3_buckets.get(bucket_name) + mock_get_value.side_effect = lambda config, key, error: { + "AWS_CREDENTIALS": {"aws_access_key_id": "mock_key"}, + "S3_BUCKETS": { + "invalid_bucket*name": {"endpoint_url": "https://custom.endpoint.com"}, + "validbucketname-alreadyvalid": { + "endpoint_url": "https://custom.endpoint2.com" + }, + }, + }.get(key, error) + + with patch("fence.blueprints.data.indexd.flask.current_app", return_value=app): + # patch get_credential_to_access_bucket() ensure get_signed_url can proceed without actually accessing AWS credentials + with patch.object( + S3IndexedFileLocation, "get_credential_to_access_bucket" + ) as mock_get_credential: + mock_get_credential.return_value = { + "aws_access_key_id": "mock_key", + "aws_secret_access_key": "mock_secret", # pragma: allowlist secret + } + + result_url = s3_indexed_file_location.get_signed_url( + "download", expires_in=3600 + ) + + # Check that real_bucket_name fell back to parsed_url.netloc, or otherwise used the already valid bucket + if s3_indexed_file_location.bucket_name() == "invalid_bucket*name": + assert "validbucketname-netloc" in result_url + else: + assert "validbucketname-alreadyvalid" in result_url + + @pytest.mark.parametrize("supported_action", ["download"], indirect=True) def test_internal_get_signed_url_no_location_match( app, supported_action, supported_protocol, indexd_client_accepting_record From 0beab8f5fa3797dc4bf9272b904492f59bca24db Mon Sep 17 00:00:00 2001 From: pieterlukasse Date: Tue, 10 Sep 2024 19:30:59 +0200 Subject: [PATCH 02/13] feat: add soft-delete --- fence/blueprints/admin.py | 14 ++++++++++++++ fence/resources/admin/admin_users.py | 12 ++++++++++++ 2 files changed, 26 insertions(+) diff --git a/fence/blueprints/admin.py b/fence/blueprints/admin.py index ed64a9272..f081d6cb9 100644 --- a/fence/blueprints/admin.py +++ b/fence/blueprints/admin.py @@ -133,6 +133,20 @@ def delete_user(username): return response +@blueprint.route("/users//soft", methods=["DELETE"]) +@blueprint.route("/user//soft", methods=["DELETE"]) +@admin_login_required +@debug_log +def soft_delete_user(username): + """ + Soft-remove the user by marking it as active=False. + + Returns json object + """ + response = jsonify(admin.soft_delete_user(current_app.scoped_session(), username)) + return response + + @blueprint.route("/users//groups", methods=["GET"]) @blueprint.route("/user//groups", methods=["GET"]) @admin_login_required diff --git a/fence/resources/admin/admin_users.py b/fence/resources/admin/admin_users.py index 956da6cd7..e6cd27e34 100644 --- a/fence/resources/admin/admin_users.py +++ b/fence/resources/admin/admin_users.py @@ -28,6 +28,7 @@ "update_user", "add_user_to_projects", "delete_user", + "soft_delete_user", "add_user_to_groups", "connect_user_to_group", "remove_user_from_groups", @@ -359,6 +360,17 @@ def raise_unavailable(gpg_email): logger.info("Done with Google deletions.") +def soft_delete_user(current_session, username): + """ + Soft-remove the user by marking it as active=False. + """ + logger.debug("Soft-delete user.") + usr = us.get_user(current_session, username) + usr.active = False + current_session.commit() + return us.get_user_info(current_session, usr.username) + + def delete_user(current_session, username): """ Remove a user from both the userdatamodel From bfd381f8a1f3c229ffdaf0274241a6f8fc11e15f Mon Sep 17 00:00:00 2001 From: pieterlukasse Date: Tue, 22 Oct 2024 18:28:24 +0200 Subject: [PATCH 03/13] 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 | 8 +++++--- fence/resources/admin/admin_users.py | 6 +++--- fence/resources/user/__init__.py | 1 + tests/admin/test_admin_users.py | 5 +++-- tests/admin/test_admin_users_endpoints.py | 24 ++++++++++++++--------- 5 files changed, 27 insertions(+), 17 deletions(-) diff --git a/fence/blueprints/admin.py b/fence/blueprints/admin.py index f081d6cb9..a34b385ca 100644 --- a/fence/blueprints/admin.py +++ b/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 + ) ) diff --git a/fence/resources/admin/admin_users.py b/fence/resources/admin/admin_users.py index e6cd27e34..eea25998c 100644 --- a/fence/resources/admin/admin_users.py +++ b/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 diff --git a/fence/resources/user/__init__.py b/fence/resources/user/__init__.py index 3543f875f..152e75057 100644 --- a/fence/resources/user/__init__.py +++ b/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": [], diff --git a/tests/admin/test_admin_users.py b/tests/admin/test_admin_users.py index 23543478e..152442f48 100644 --- a/tests/admin/test_admin_users.py +++ b/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 diff --git a/tests/admin/test_admin_users_endpoints.py b/tests/admin/test_admin_users_endpoints.py index 8416fb9d0..f8ee5dedf 100644 --- a/tests/admin/test_admin_users_endpoints.py +++ b/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/: [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() From f030f7f3e91015824a74846cceadd1369b928729 Mon Sep 17 00:00:00 2001 From: pieterlukasse Date: Wed, 6 Nov 2024 22:10:29 +0100 Subject: [PATCH 04/13] feat: add /admin/user soft-delete tests --- tests/admin/test_admin_users.py | 28 ++++++++++++++ tests/admin/test_admin_users_endpoints.py | 46 +++++++++++++++++++++++ 2 files changed, 74 insertions(+) diff --git a/tests/admin/test_admin_users.py b/tests/admin/test_admin_users.py index 152442f48..1f0060eea 100644 --- a/tests/admin/test_admin_users.py +++ b/tests/admin/test_admin_users.py @@ -99,6 +99,34 @@ def test_delete_user(db_session, awg_users, cloud_manager): assert user_groups == [] +def test_soft_delete_user(db_session, awg_users): + """ + Tests adm.soft_delete_user() by querying an existing User, + asserting it is not inactive, and then checking it became inactive + after it was soft-deleted. + """ + username = "awg_user" + user = db_session.query(User).filter(User.username == username).first() + assert user != None + assert user.username == username + assert user.active is None + adm.soft_delete_user(db_session, username) + user = db_session.query(User).filter(User.username == username).first() + assert user != None + assert user.username == username + # soft-deleted user should have "active" explicitly set to False now: + assert user.active == False + + +def test_soft_delete_user_not_found(db_session, awg_users): + """ + Check that adm.soft_delete_user() fails with NotFound + when called for a username that is not found in db. + """ + with pytest.raises(NotFound, match="user non_existing_user not found"): + adm.soft_delete_user(db_session, "non_existing_user") + + def test_update_user_without_conflict(db_session, awg_users, oauth_client): user = db_session.query(User).filter(User.username == "awg_user").first() assert user != None diff --git a/tests/admin/test_admin_users_endpoints.py b/tests/admin/test_admin_users_endpoints.py index f8ee5dedf..b3e601f87 100644 --- a/tests/admin/test_admin_users_endpoints.py +++ b/tests/admin/test_admin_users_endpoints.py @@ -691,6 +691,52 @@ def assert_google_proxy_group_data_deleted(db_session): assert len(gbag) == 1 +def test_soft_delete_user_username( + client, + encoded_admin_jwt, + db_session, + load_non_google_user_data, +): + """ + Test soft-delete user endpoint by checking that the result is an + deactivated user. + """ + username = "test_user_d" + user = db_session.query(User).filter_by(username=username).one() + assert user.username == username + assert user.active is None + # now soft-delete and assert "active" changed to False: + r = client.delete( + f"/admin/users/{username}/soft", + headers={"Authorization": "Bearer " + encoded_admin_jwt}, + ) + assert r.status_code == 200 + assert r.json["username"] == username + assert r.json["active"] == False + user = db_session.query(User).filter_by(username=username).one() + assert user.username == username + assert user.active == False + + +def test_soft_delete_user_user_not_found( + client, + encoded_admin_jwt, + db_session, +): + """ + Test soft-delete user endpoint returns error when user is not found. + """ + username = "non_existing_user" + user = db_session.query(User).filter_by(username=username).first() + assert user is None + # now call soft-delete and assert it fails: + r = client.delete( + f"/admin/users/{username}/soft", + headers={"Authorization": "Bearer " + encoded_admin_jwt}, + ) + assert r.status_code == 404 + + def test_delete_user_username( app, client, From 0ae48f638c2baae23c5b67bcad190430e7b43d81 Mon Sep 17 00:00:00 2001 From: pieterlukasse Date: Thu, 7 Nov 2024 17:18:36 +0100 Subject: [PATCH 05/13] feat: add username to soft-delete log Co-authored-by: Pauline Ribeyre <4224001+paulineribeyre@users.noreply.github.com> --- fence/resources/admin/admin_users.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fence/resources/admin/admin_users.py b/fence/resources/admin/admin_users.py index eea25998c..37b586476 100644 --- a/fence/resources/admin/admin_users.py +++ b/fence/resources/admin/admin_users.py @@ -364,7 +364,7 @@ def soft_delete_user(current_session, username): """ Soft-remove the user by marking it as active=False. """ - logger.debug("Soft-delete user.") + logger.debug(f"Soft-delete user '{username}'") usr = us.get_user(current_session, username) usr.active = False current_session.commit() From 3ecd0f4cdca0c6683b55743c40f35fa2ab0c5026 Mon Sep 17 00:00:00 2001 From: pieterlukasse Date: Fri, 8 Nov 2024 21:06:45 +0100 Subject: [PATCH 06/13] feat: update swagger docs for some of the /admin endpoints --- openapis/swagger.yaml | 117 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 117 insertions(+) diff --git a/openapis/swagger.yaml b/openapis/swagger.yaml index 17ea7eb17..7416bddc6 100644 --- a/openapis/swagger.yaml +++ b/openapis/swagger.yaml @@ -387,6 +387,73 @@ paths: '*/*': schema: $ref: '#/components/schemas/UserInfo' + '/admin/user/{username}': + get: + tags: + - 'admin/user' + summary: Return info about a given user + description: Admin method to retrieve info about any given user + parameters: + - name: username + required: true + in: path + description: Username of user to find + schema: + type: string + security: + - OAuth2: + - user + operationId: getUserInfo + responses: + '200': + description: successful operation + content: + '*/*': + schema: + $ref: '#/components/schemas/UserInfo' + '404': + description: Couldn't find user + '/admin/user/{username}/soft': + delete: + tags: + - 'admin/user' + summary: Soft-delete user + description: Admin method to soft-delete a user, setting the user to inactive + security: + - OAuth2: + - admin + operationId: softDeleteUser + parameters: + - name: username + required: true + in: path + description: Username of user to deactivate + schema: + type: string + responses: + '204': + description: successful deletion + '404': + description: Couldn't find user + /admin/user: + post: + tags: + - admin/user + summary: Add a new user + description: Admin method to add a new user + security: + - OAuth2: + - admin + operationId: createUser + responses: + '200': + description: New user + content: + '*/*': + schema: + $ref: '#/components/schemas/UserInfo' + requestBody: + $ref: '#/components/requestBodies/NewUser' /logout: get: tags: @@ -1683,6 +1750,13 @@ components: $ref: '#/components/schemas/APIKeyScopes' description: API key scopes required: false + NewUser: + content: + application/json: + schema: + $ref: '#/components/schemas/NewUser' + description: New User + required: true securitySchemes: OAuth2: type: oauth2 @@ -1693,6 +1767,46 @@ components: scopes: user: generic user access schemas: + NewUser: + type: object + required: + - username + - role + - email + properties: + username: + type: string + description: 'This value is deprecated in favor of name.' + role: + type: string + description: 'Set to "admin" if the user should be given admin rights. Any other value is not parsed or used, and results in user being a normal/regular user.' + email: + type: string + description: 'The email of the end-user' + display_name: + type: string + description: 'The display name of the end-user.' + phone_number: + type: string + description: 'The phone number of the end-user.' + idp_name: + type: string + description: | + Name of the preferred Identity Provider used to autheticate the user. Given instances of Fence + may or may not have all of these available (the set of IDPs available is a configuration). + * *google* - Google/GSuite + * *ras* - NIH's Researcher Auth Service (RAS) + * *itrust* - NIH Login / iTrust / eRA Commons + * *fence* - Upstream Fence (the idp used depends on the specific configuration, consult the Gen3 Operators) + * *orcid* - ORCHID + * *microsoft* - Microsoft + * *elixir* - Elixir + * *synapse* - Synapse + * *cognito* - AWS Cognito + * More may be added in the future... + tags: + type: object + description: User's tags RequestUploadBlank: type: object required: @@ -2335,6 +2449,9 @@ components: is_admin: type: boolean description: 'Boolean value stating if end-user is an admin or not' + active: + type: boolean + description: 'Boolean value stating if user is active or not' role: type: string description: '' From 27c5d74c8401ebcfa8ce0b3d11b01f8c90377573 Mon Sep 17 00:00:00 2001 From: pieterlukasse Date: Mon, 11 Nov 2024 15:19:48 +0100 Subject: [PATCH 07/13] feat: add migration script to make User.active not-nullable --- fence/auth.py | 5 ++-- ...808_make_user_active_field_non_nullable.py | 26 +++++++++++++++++++ tests/admin/test_admin_users.py | 2 +- tests/admin/test_admin_users_endpoints.py | 2 +- 4 files changed, 30 insertions(+), 5 deletions(-) create mode 100644 migrations/versions/3a5712474808_make_user_active_field_non_nullable.py diff --git a/fence/auth.py b/fence/auth.py index e7c1a1fb8..3861951f8 100644 --- a/fence/auth.py +++ b/fence/auth.py @@ -101,9 +101,8 @@ def set_flask_session_values(user): user = query_for_user(session=current_app.scoped_session(), username=username) if user: - if user.active is False: - # Abort login if user.active is False (user.active is None or True are both - # considered active in this case): + if user.active == False: + # Abort login if user.active == False: raise Unauthorized( "User is known but not authorized/activated in the system" ) diff --git a/migrations/versions/3a5712474808_make_user_active_field_non_nullable.py b/migrations/versions/3a5712474808_make_user_active_field_non_nullable.py new file mode 100644 index 000000000..d781947d1 --- /dev/null +++ b/migrations/versions/3a5712474808_make_user_active_field_non_nullable.py @@ -0,0 +1,26 @@ +"""Make User.active field non nullable + +Revision ID: 3a5712474808 +Revises: 9b3a5a7145d7 +Create Date: 2024-11-08 22:00:41.161934 + +""" +from alembic import op +import sqlalchemy as sa + + +# revision identifiers, used by Alembic. +revision = "3a5712474808" # pragma: allowlist secret +down_revision = "9b3a5a7145d7" # pragma: allowlist secret +branch_labels = None +depends_on = None + + +def upgrade(): + op.execute('UPDATE "User" SET active = True WHERE active IS NULL') + op.alter_column("User", "active", nullable=False, server_default="True") + + +def downgrade(): + op.alter_column("User", "active", nullable=True, server_default=None) + op.execute('UPDATE "User" SET active = NULL WHERE active = True') diff --git a/tests/admin/test_admin_users.py b/tests/admin/test_admin_users.py index 1f0060eea..2bc50d6d6 100644 --- a/tests/admin/test_admin_users.py +++ b/tests/admin/test_admin_users.py @@ -109,7 +109,7 @@ def test_soft_delete_user(db_session, awg_users): user = db_session.query(User).filter(User.username == username).first() assert user != None assert user.username == username - assert user.active is None + assert user.active == True adm.soft_delete_user(db_session, username) user = db_session.query(User).filter(User.username == username).first() assert user != None diff --git a/tests/admin/test_admin_users_endpoints.py b/tests/admin/test_admin_users_endpoints.py index b3e601f87..a598c7b65 100644 --- a/tests/admin/test_admin_users_endpoints.py +++ b/tests/admin/test_admin_users_endpoints.py @@ -704,7 +704,7 @@ def test_soft_delete_user_username( username = "test_user_d" user = db_session.query(User).filter_by(username=username).one() assert user.username == username - assert user.active is None + assert user.active == True # now soft-delete and assert "active" changed to False: r = client.delete( f"/admin/users/{username}/soft", From d76c071b6d78809d246cde4ff3e1e22537184492 Mon Sep 17 00:00:00 2001 From: pieterlukasse Date: Tue, 12 Nov 2024 20:07:25 +0100 Subject: [PATCH 08/13] fix: improve migration logic for .active field based on this dicussion: https://github.com/uc-cdis/fence/pull/1189#discussion_r1838528076 --- ...12474808_make_user_active_field_non_nullable.py | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/migrations/versions/3a5712474808_make_user_active_field_non_nullable.py b/migrations/versions/3a5712474808_make_user_active_field_non_nullable.py index d781947d1..cc9aebb51 100644 --- a/migrations/versions/3a5712474808_make_user_active_field_non_nullable.py +++ b/migrations/versions/3a5712474808_make_user_active_field_non_nullable.py @@ -7,6 +7,8 @@ """ from alembic import op import sqlalchemy as sa +from userdatamodel.models import User +from sqlalchemy.orm import Session # revision identifiers, used by Alembic. @@ -17,10 +19,18 @@ def upgrade(): - op.execute('UPDATE "User" SET active = True WHERE active IS NULL') + conn = op.get_bind() + session = Session(bind=conn) + session.query(User) + active_users_count = session.query(User).filter(User.active.is_(True)).count() + if active_users_count > 0: + # if we have at least one user where "active" is explicitly set to "True", then we'll assume NULL is to mean "False": + op.execute('UPDATE "User" SET active = False WHERE active IS NULL') + else: + # else, we assume NULL means "True" + op.execute('UPDATE "User" SET active = True WHERE active IS NULL') op.alter_column("User", "active", nullable=False, server_default="True") def downgrade(): op.alter_column("User", "active", nullable=True, server_default=None) - op.execute('UPDATE "User" SET active = NULL WHERE active = True') From afe6a482b7e822bff21094e3d956f83c4993d820 Mon Sep 17 00:00:00 2001 From: pieterlukasse Date: Wed, 13 Nov 2024 19:57:38 +0100 Subject: [PATCH 09/13] feat: upgrade userdatamodel to 3.0.0 --- poetry.lock | 15 ++++++++------- pyproject.toml | 2 +- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/poetry.lock b/poetry.lock index f28981dca..38f7a1b35 100644 --- a/poetry.lock +++ b/poetry.lock @@ -2424,17 +2424,18 @@ zstd = ["zstandard (>=0.18.0)"] [[package]] name = "userdatamodel" -version = "2.4.3" -description = "" +version = "3.0.0" +description = "PDC data model for user management" optional = false -python-versions = "*" +python-versions = "<4.0,>=3.9" files = [ - {file = "userdatamodel-2.4.3.tar.gz", hash = "sha256:760e41bc7f29bc16b3a9964956b52a3386db971a64dc3c873d362e135adda5b9"}, + {file = "userdatamodel-3.0.0-py3-none-any.whl", hash = "sha256:9385fcf2b0077a687a836a2006bc6785f8484bee0ec0e87a6866c9f41288a373"}, + {file = "userdatamodel-3.0.0.tar.gz", hash = "sha256:09f80968c7bbef44ec3f9f5525cfa69a9b71a695a493b8c603bc15798e397a0c"}, ] [package.dependencies] -cdislogging = "*" -sqlalchemy = ">=1.3.3" +cdislogging = ">=1,<2" +sqlalchemy = ">=1.3.0" [[package]] name = "werkzeug" @@ -2582,4 +2583,4 @@ type = ["pytest-mypy"] [metadata] lock-version = "2.0" python-versions = ">=3.9,<4.0.0" -content-hash = "655105408255f1d68d57e3517f8d961b68d1389fe93aed82d1c1daed7042c444" +content-hash = "bb16bd1f8272b907b2cc9612f8d3850c57140a1c73a3b43c00f48066f964ec96" diff --git a/pyproject.toml b/pyproject.toml index 38ed3e908..24030aa61 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -47,7 +47,7 @@ pyyaml = "^6.0.1" requests = ">=2.18.0" retry = "^0.9.2" sqlalchemy = "^1.3.3" -userdatamodel = ">=2.4.3" +userdatamodel = ">=3.0.0" werkzeug = ">=3.0.0" cachelib = "^0.2.0" azure-storage-blob = "^12.6.0" From b74484b379fe929005955a65b712724a5a148053 Mon Sep 17 00:00:00 2001 From: pieterlukasse Date: Thu, 14 Nov 2024 14:24:52 +0100 Subject: [PATCH 10/13] feat: update userdatamodel to 3.0.1 --- poetry.lock | 8 ++++---- pyproject.toml | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/poetry.lock b/poetry.lock index 38f7a1b35..37f05f237 100644 --- a/poetry.lock +++ b/poetry.lock @@ -2424,13 +2424,13 @@ zstd = ["zstandard (>=0.18.0)"] [[package]] name = "userdatamodel" -version = "3.0.0" +version = "3.0.1" description = "PDC data model for user management" optional = false python-versions = "<4.0,>=3.9" files = [ - {file = "userdatamodel-3.0.0-py3-none-any.whl", hash = "sha256:9385fcf2b0077a687a836a2006bc6785f8484bee0ec0e87a6866c9f41288a373"}, - {file = "userdatamodel-3.0.0.tar.gz", hash = "sha256:09f80968c7bbef44ec3f9f5525cfa69a9b71a695a493b8c603bc15798e397a0c"}, + {file = "userdatamodel-3.0.1-py3-none-any.whl", hash = "sha256:28e34acf1998e81b32b8e35ac61d950cc48c88ef3545ac45939be0116f20d3d5"}, + {file = "userdatamodel-3.0.1.tar.gz", hash = "sha256:775fa5d11a32c44d11322945a817ae265b7787c90ea05fefcd1742839cca111a"}, ] [package.dependencies] @@ -2583,4 +2583,4 @@ type = ["pytest-mypy"] [metadata] lock-version = "2.0" python-versions = ">=3.9,<4.0.0" -content-hash = "bb16bd1f8272b907b2cc9612f8d3850c57140a1c73a3b43c00f48066f964ec96" +content-hash = "cfbc8d55dad030a712faa8c311773f99bf60401c614cf93f91e7880b6df568ba" diff --git a/pyproject.toml b/pyproject.toml index 24030aa61..bb5e567b0 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -47,7 +47,7 @@ pyyaml = "^6.0.1" requests = ">=2.18.0" retry = "^0.9.2" sqlalchemy = "^1.3.3" -userdatamodel = ">=3.0.0" +userdatamodel = ">=3.0.1" werkzeug = ">=3.0.0" cachelib = "^0.2.0" azure-storage-blob = "^12.6.0" From f685f6523c58d1357c0eb8ad42720d37533eb142 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Fri, 15 Nov 2024 13:44:49 +0000 Subject: [PATCH 11/13] Bump werkzeug from 3.0.4 to 3.0.6 Bumps [werkzeug](https://github.com/pallets/werkzeug) from 3.0.4 to 3.0.6. - [Release notes](https://github.com/pallets/werkzeug/releases) - [Changelog](https://github.com/pallets/werkzeug/blob/main/CHANGES.rst) - [Commits](https://github.com/pallets/werkzeug/compare/3.0.4...3.0.6) --- updated-dependencies: - dependency-name: werkzeug dependency-type: direct:production ... Signed-off-by: dependabot[bot] --- poetry.lock | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/poetry.lock b/poetry.lock index 37f05f237..0fe3aae25 100644 --- a/poetry.lock +++ b/poetry.lock @@ -2439,13 +2439,13 @@ sqlalchemy = ">=1.3.0" [[package]] name = "werkzeug" -version = "3.0.4" +version = "3.0.6" description = "The comprehensive WSGI web application library." optional = false python-versions = ">=3.8" files = [ - {file = "werkzeug-3.0.4-py3-none-any.whl", hash = "sha256:02c9eb92b7d6c06f31a782811505d2157837cea66aaede3e217c7c27c039476c"}, - {file = "werkzeug-3.0.4.tar.gz", hash = "sha256:34f2371506b250df4d4f84bfe7b0921e4762525762bbd936614909fe25cd7306"}, + {file = "werkzeug-3.0.6-py3-none-any.whl", hash = "sha256:1bc0c2310d2fbb07b1dd1105eba2f7af72f322e1e455f2f93c993bee8c8a5f17"}, + {file = "werkzeug-3.0.6.tar.gz", hash = "sha256:a8dd59d4de28ca70471a34cba79bed5f7ef2e036a76b3ab0835474246eb41f8d"}, ] [package.dependencies] From 08fc76a7fd58c85c50506f094bf3e8b4597a085c Mon Sep 17 00:00:00 2001 From: pieterlukasse Date: Mon, 18 Nov 2024 20:16:18 +0100 Subject: [PATCH 12/13] feat: deprecate legacy unused /admin/ endpoints --- fence/blueprints/admin.py | 145 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 145 insertions(+) diff --git a/fence/blueprints/admin.py b/fence/blueprints/admin.py index a34b385ca..41343068e 100644 --- a/fence/blueprints/admin.py +++ b/fence/blueprints/admin.py @@ -61,10 +61,15 @@ def get_user(username): @debug_log def get_all_users(): """ + DEPRECATED: This endpoint is deprecated and will be removed in a future release. + Get the information of all users from our userdatamodel database Returns a json object. """ + logger.warning( + f"Deprecated endpoint accessed: {request.path}. This endpoint is deprecated and will be removed in a future release." + ) return jsonify(admin.get_all_users(current_app.scoped_session())) @@ -106,10 +111,15 @@ def create_user(): @debug_log def update_user(username): """ + DEPRECATED: This endpoint is deprecated and will be removed in a future release. + Create a user on the userdatamodel database Returns a json object """ + logger.warning( + f"Deprecated endpoint accessed: {request.path}. This endpoint is deprecated and will be removed in a future release." + ) new_username = request.get_json().get("username", None) role = request.get_json().get("role", None) email = request.get_json().get("email", None) @@ -126,11 +136,16 @@ def update_user(username): @debug_log def delete_user(username): """ + DEPRECATED: This endpoint is deprecated and will be removed in a future release. + Remove the user from the userdatamodel database and all associated storage solutions. Returns json object """ + logger.warning( + f"Deprecated endpoint accessed: {request.path}. This endpoint is deprecated and will be removed in a future release." + ) response = jsonify(admin.delete_user(current_app.scoped_session(), username)) return response @@ -155,10 +170,15 @@ def soft_delete_user(username): @debug_log def get_user_groups(username): """ + DEPRECATED: This endpoint is deprecated and will be removed in a future release. + Get the information of a user from our userdatamodel database. Returns a json object """ + logger.warning( + f"Deprecated endpoint accessed: {request.path}. This endpoint is deprecated and will be removed in a future release." + ) return jsonify(admin.get_user_groups(current_app.scoped_session(), username)) @@ -168,10 +188,15 @@ def get_user_groups(username): @debug_log def add_user_to_groups(username): """ + DEPRECATED: This endpoint is deprecated and will be removed in a future release. + Create a user to group relationship in the database Returns a json object """ + logger.warning( + f"Deprecated endpoint accessed: {request.path}. This endpoint is deprecated and will be removed in a future release." + ) groups = request.get_json().get("groups", []) return jsonify( admin.add_user_to_groups(current_app.scoped_session(), username, groups=groups) @@ -184,10 +209,15 @@ def add_user_to_groups(username): @debug_log def remove_user_from_groups(username): """ + DEPRECATED: This endpoint is deprecated and will be removed in a future release. + Create a user to group relationship in the database Returns a json object """ + logger.warning( + f"Deprecated endpoint accessed: {request.path}. This endpoint is deprecated and will be removed in a future release." + ) groups = request.get_json().get("groups", []) return jsonify( admin.remove_user_from_groups( @@ -202,10 +232,15 @@ def remove_user_from_groups(username): @debug_log def remove_user_from_projects(username): """ + DEPRECATED: This endpoint is deprecated and will be removed in a future release. + Create a user to group relationship in the database Returns a json object """ + logger.warning( + f"Deprecated endpoint accessed: {request.path}. This endpoint is deprecated and will be removed in a future release." + ) projects = request.get_json().get("projects", []) return jsonify( admin.remove_user_from_projects( @@ -220,11 +255,16 @@ def remove_user_from_projects(username): @debug_log def add_user_to_projects(username): """ + DEPRECATED: This endpoint is deprecated and will be removed in a future release. + Create a user to project relationship on the database and add the access to the the object store associated with it Returns a json object """ + logger.warning( + f"Deprecated endpoint accessed: {request.path}. This endpoint is deprecated and will be removed in a future release." + ) projects = request.get_json().get("projects", []) return jsonify( admin.add_user_to_projects( @@ -241,10 +281,15 @@ def add_user_to_projects(username): @debug_log def get_project(projectname): """ + DEPRECATED: This endpoint is deprecated and will be removed in a future release. + Get the information related to a project from the userdatamodel database Returns a json object """ + logger.warning( + f"Deprecated endpoint accessed: {request.path}. This endpoint is deprecated and will be removed in a future release." + ) return jsonify(admin.get_project_info(current_app.scoped_session(), projectname)) @@ -253,10 +298,15 @@ def get_project(projectname): @debug_log def get_all_projects(): """ + DEPRECATED: This endpoint is deprecated and will be removed in a future release. + Get the information related to a project from the userdatamodel database Returns a json object """ + logger.warning( + f"Deprecated endpoint accessed: {request.path}. This endpoint is deprecated and will be removed in a future release." + ) return jsonify(admin.get_all_projects(current_app.scoped_session())) @@ -265,9 +315,14 @@ def get_all_projects(): @debug_log def create_project(projectname): """ + DEPRECATED: This endpoint is deprecated and will be removed in a future release. + Create a new project on the specified storage Returns a json object """ + logger.warning( + f"Deprecated endpoint accessed: {request.path}. This endpoint is deprecated and will be removed in a future release." + ) auth_id = request.get_json().get("auth_id") storage_accesses = request.get_json().get("storage_accesses", []) response = jsonify( @@ -283,9 +338,14 @@ def create_project(projectname): @debug_log def delete_project(projectname): """ + DEPRECATED: This endpoint is deprecated and will be removed in a future release. + Remove project. No Buckets should be associated with it. Returns a json object. """ + logger.warning( + f"Deprecated endpoint accessed: {request.path}. This endpoint is deprecated and will be removed in a future release." + ) response = jsonify(admin.delete_project(current_app.scoped_session(), projectname)) return response @@ -295,9 +355,14 @@ def delete_project(projectname): @debug_log def remove_projects_from_group(groupname): """ + DEPRECATED: This endpoint is deprecated and will be removed in a future release. + Create a user to group relationship in the database Returns a json object """ + logger.warning( + f"Deprecated endpoint accessed: {request.path}. This endpoint is deprecated and will be removed in a future release." + ) projects = request.get_json().get("projects", []) return jsonify( admin.remove_projects_from_group( @@ -310,9 +375,14 @@ def remove_projects_from_group(groupname): @admin_login_required def add_project_to_groups(projectname): """ + DEPRECATED: This endpoint is deprecated and will be removed in a future release. + Create a user to group relationship in the database Returns a json object """ + logger.warning( + f"Deprecated endpoint accessed: {request.path}. This endpoint is deprecated and will be removed in a future release." + ) groups = request.get_json().get("groups", []) return jsonify( admin.add_project_to_groups( @@ -325,9 +395,14 @@ def add_project_to_groups(projectname): @admin_login_required def create_bucket_in_project(projectname, bucketname): """ + DEPRECATED: This endpoint is deprecated and will be removed in a future release. + Create a bucket in the selected project. Returns a json object. """ + logger.warning( + f"Deprecated endpoint accessed: {request.path}. This endpoint is deprecated and will be removed in a future release." + ) providername = request.get_json().get("provider") response = jsonify( admin.create_bucket_on_project( @@ -341,11 +416,16 @@ def create_bucket_in_project(projectname, bucketname): @admin_login_required def delete_bucket_from_project(projectname, bucketname): """ + DEPRECATED: This endpoint is deprecated and will be removed in a future release. + Delete a bucket from the selected project, both in the userdatamodel database and in the storage client associated with that bucket. Returns a json object. """ + logger.warning( + f"Deprecated endpoint accessed: {request.path}. This endpoint is deprecated and will be removed in a future release." + ) return jsonify( admin.delete_bucket_on_project( current_app.scoped_session(), projectname, bucketname @@ -357,10 +437,15 @@ def delete_bucket_from_project(projectname, bucketname): @admin_login_required def list_buckets_from_project(projectname): """ + DEPRECATED: This endpoint is deprecated and will be removed in a future release. + Retrieve the information regarding the buckets created within a project. Returns a json object. """ + logger.warning( + f"Deprecated endpoint accessed: {request.path}. This endpoint is deprecated and will be removed in a future release." + ) response = jsonify( admin.list_buckets_on_project_by_name(current_app.scoped_session(), projectname) ) @@ -374,10 +459,15 @@ def list_buckets_from_project(projectname): @admin_login_required def get_group_info(groupname): """ + DEPRECATED: This endpoint is deprecated and will be removed in a future release. + Retrieve the information regarding the buckets created within a project. Returns a json object. """ + logger.warning( + f"Deprecated endpoint accessed: {request.path}. This endpoint is deprecated and will be removed in a future release." + ) return jsonify(admin.get_group_info(current_app.scoped_session(), groupname)) @@ -385,10 +475,15 @@ def get_group_info(groupname): @admin_login_required def get_all_groups(): """ + DEPRECATED: This endpoint is deprecated and will be removed in a future release. + Retrieve the information regarding the buckets created within a project. Returns a json object. """ + logger.warning( + f"Deprecated endpoint accessed: {request.path}. This endpoint is deprecated and will be removed in a future release." + ) return jsonify(admin.get_all_groups(current_app.scoped_session())) @@ -396,10 +491,15 @@ def get_all_groups(): @admin_login_required def get_group_users(groupname): """ + DEPRECATED: This endpoint is deprecated and will be removed in a future release. + Retrieve the information regarding the buckets created within a project. Returns a json object. """ + logger.warning( + f"Deprecated endpoint accessed: {request.path}. This endpoint is deprecated and will be removed in a future release." + ) return jsonify(admin.get_group_users(current_app.scoped_session(), groupname)) @@ -407,10 +507,15 @@ def get_group_users(groupname): @admin_login_required def create_group(): """ + DEPRECATED: This endpoint is deprecated and will be removed in a future release. + Retrieve the information regarding the buckets created within a project. Returns a json object. """ + logger.warning( + f"Deprecated endpoint accessed: {request.path}. This endpoint is deprecated and will be removed in a future release." + ) groupname = request.get_json().get("name") description = request.get_json().get("description") grp = admin.create_group(current_app.scoped_session(), groupname, description) @@ -426,10 +531,15 @@ def create_group(): @admin_login_required def update_group(groupname): """ + DEPRECATED: This endpoint is deprecated and will be removed in a future release. + Retrieve the information regarding the buckets created within a project. Returns a json object. """ + logger.warning( + f"Deprecated endpoint accessed: {request.path}. This endpoint is deprecated and will be removed in a future release." + ) name = request.get_json().get("name", None) description = request.get_json().get("description", None) response = jsonify( @@ -442,10 +552,15 @@ def update_group(groupname): @admin_login_required def delete_group(groupname): """ + DEPRECATED: This endpoint is deprecated and will be removed in a future release. + Retrieve the information regarding the buckets created within a project. Returns a json object. """ + logger.warning( + f"Deprecated endpoint accessed: {request.path}. This endpoint is deprecated and will be removed in a future release." + ) response = jsonify(admin.delete_group(current_app.scoped_session(), groupname)) return response @@ -454,9 +569,14 @@ def delete_group(groupname): @admin_login_required def add_projects_to_group(groupname): """ + DEPRECATED: This endpoint is deprecated and will be removed in a future release. + Create a user to group relationship in the database Returns a json object """ + logger.warning( + f"Deprecated endpoint accessed: {request.path}. This endpoint is deprecated and will be removed in a future release." + ) projects = request.get_json().get("projects", []) response = jsonify( admin.add_projects_to_group(current_app.scoped_session(), groupname, projects) @@ -468,9 +588,14 @@ def add_projects_to_group(groupname): @admin_login_required def get_group_projects(groupname): """ + DEPRECATED: This endpoint is deprecated and will be removed in a future release. + Create a user to group relationship in the database Returns a json object """ + logger.warning( + f"Deprecated endpoint accessed: {request.path}. This endpoint is deprecated and will be removed in a future release." + ) values = admin.get_group_projects(current_app.scoped_session(), groupname) return jsonify({"projects": values}) @@ -483,9 +608,14 @@ def get_group_projects(groupname): @admin_login_required def get_cloud_provider(providername): """ + DEPRECATED: This endpoint is deprecated and will be removed in a future release. + Retriev the information related to a cloud provider Returns a json object. """ + logger.warning( + f"Deprecated endpoint accessed: {request.path}. This endpoint is deprecated and will be removed in a future release." + ) return jsonify(admin.get_provider(current_app.scoped_session(), providername)) @@ -494,9 +624,14 @@ def get_cloud_provider(providername): @admin_login_required def create_cloud_provider(providername): """ + DEPRECATED: This endpoint is deprecated and will be removed in a future release. + Create a cloud provider. Returns a json object """ + logger.warning( + f"Deprecated endpoint accessed: {request.path}. This endpoint is deprecated and will be removed in a future release." + ) backend_name = request.get_json().get("backend") service_name = request.get_json().get("service") response = jsonify( @@ -515,11 +650,16 @@ def create_cloud_provider(providername): @admin_login_required def delete_cloud_provider(providername): """ + DEPRECATED: This endpoint is deprecated and will be removed in a future release. + Deletes a cloud provider from the userdatamodel All projects associated with it should be deassociated or removed. Returns a json object. """ + logger.warning( + f"Deprecated endpoint accessed: {request.path}. This endpoint is deprecated and will be removed in a future release." + ) response = jsonify( admin.delete_provider(current_app.scoped_session(), providername) ) @@ -530,10 +670,15 @@ def delete_cloud_provider(providername): @admin_login_required def get_registered_users(): """ + DEPRECATED: This endpoint is deprecated and will be removed in a future release. + - List registration info for every user for which there exists registration info. - Endpoint accessible to admins only. - Response json structure is provisional. """ + logger.warning( + f"Deprecated endpoint accessed: {request.path}. This endpoint is deprecated and will be removed in a future release." + ) registered_users = ( current_app.scoped_session() .query(User) From 819405b8f14107621cd93bd9a4f482779542b92a Mon Sep 17 00:00:00 2001 From: pieterlukasse Date: Tue, 19 Nov 2024 19:38:05 +0100 Subject: [PATCH 13/13] feat: remove role from POST /admin/user endpoint --- fence/blueprints/admin.py | 2 -- fence/resources/admin/admin_users.py | 4 +--- openapis/swagger.yaml | 4 ---- tests/admin/test_admin_users.py | 5 ++--- 4 files changed, 3 insertions(+), 12 deletions(-) diff --git a/fence/blueprints/admin.py b/fence/blueprints/admin.py index 41343068e..b3e898460 100644 --- a/fence/blueprints/admin.py +++ b/fence/blueprints/admin.py @@ -84,7 +84,6 @@ def create_user(): Returns a json object """ 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) phone_number = request.get_json().get("phone_number", None) @@ -95,7 +94,6 @@ def create_user(): admin.create_user( current_app.scoped_session(), username, - role, email, display_name, phone_number, diff --git a/fence/resources/admin/admin_users.py b/fence/resources/admin/admin_users.py index 37b586476..69aae8cb5 100644 --- a/fence/resources/admin/admin_users.py +++ b/fence/resources/admin/admin_users.py @@ -98,7 +98,6 @@ def get_user_groups(current_session, username): def create_user( current_session, username, - role, email, display_name=None, phone_number=None, @@ -136,9 +135,8 @@ def create_user( ) ) logger.debug(f"User does not yet exist for: {username}. Creating a new one...") - is_admin = role == "admin" email_add = email - usr = User(username=username, active=True, is_admin=is_admin, email=email_add) + usr = User(username=username, active=True, email=email_add) usr.display_name = display_name usr.phone_number = phone_number diff --git a/openapis/swagger.yaml b/openapis/swagger.yaml index 7416bddc6..6ab2e33eb 100644 --- a/openapis/swagger.yaml +++ b/openapis/swagger.yaml @@ -1771,15 +1771,11 @@ components: type: object required: - username - - role - email properties: username: type: string description: 'This value is deprecated in favor of name.' - role: - type: string - description: 'Set to "admin" if the user should be given admin rights. Any other value is not parsed or used, and results in user being a normal/regular user.' email: type: string description: 'The email of the end-user' diff --git a/tests/admin/test_admin_users.py b/tests/admin/test_admin_users.py index 2bc50d6d6..159f34294 100644 --- a/tests/admin/test_admin_users.py +++ b/tests/admin/test_admin_users.py @@ -25,10 +25,10 @@ def test_get_user(db_session, awg_users): def test_create_user(db_session, oauth_client): - adm.create_user(db_session, "insert_user", "admin", "insert_user@fake.com") + adm.create_user(db_session, "insert_user", "insert_user@fake.com") user = db_session.query(User).filter(User.username == "insert_user").first() assert user.username == "insert_user" - assert user.is_admin == True + assert user.is_admin == False # DEPRECATED field. assert user.email == "insert_user@fake.com" assert user.display_name is None assert user.phone_number is None @@ -46,7 +46,6 @@ def test_create_user_with_all_fields_set(db_session, oauth_client): adm.create_user( db_session, "insert_user", - None, "insert_user@fake.com", "Dummy Name", "+310000",