From ce8360a7333543c40448d1df14b3fc25225c6849 Mon Sep 17 00:00:00 2001 From: catherine meng Date: Tue, 5 Dec 2023 11:13:41 -0800 Subject: [PATCH 1/9] feat(888): add migration script to add user with fam roles to fam app admin table, refs: #888 --- ...grate_fam_role_assignment_to_admin_table.sql | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) create mode 100644 server/flyway/sql/V34__migrate_fam_role_assignment_to_admin_table.sql diff --git a/server/flyway/sql/V34__migrate_fam_role_assignment_to_admin_table.sql b/server/flyway/sql/V34__migrate_fam_role_assignment_to_admin_table.sql new file mode 100644 index 000000000..d3d518889 --- /dev/null +++ b/server/flyway/sql/V34__migrate_fam_role_assignment_to_admin_table.sql @@ -0,0 +1,17 @@ +-- migrate the fam roles to fam admin management table +-- first select all fam roles +-- and then select role_id and application_id this role is admin of, based on if the role name contains the application_name +-- -- for example, role name: FAM_ACCESS_ADMIN contains application name: FAM +-- -- role name: FOM_DEV_ACCESS_ADMIN contains application name: FOM_DEV +-- select the user_id and application_id the user is admin of, insert into fam_application_admin +INSERT INTO fam_application_admin (user_id, application_id, create_user, create_date) +SELECT admins.user_id, admins.application_id, CURRENT_USER, CURRENT_DATE FROM ( + SELECT user_id, application_id FROM fam_user_role_xref c JOIN ( + SELECT role_id, application_id FROM fam_application a JOIN ( + SELECT role_id, role_name FROM fam_role WHERE application_id=1 + ) b ON b.role_name LIKE '%' || a.application_name || '%' + ) d ON c.role_id = d.role_id +) admins; + + + From 626a60bdc403629fa3eeeb3e1b79e5e48d08b9d3 Mon Sep 17 00:00:00 2001 From: catherine meng Date: Tue, 5 Dec 2023 15:51:34 -0800 Subject: [PATCH 2/9] feat(888): - flyway script to migrate fam role to fam application admin table - update auth function to read fam application admin when user login through fam itself - update the auth function test to verify the admin role can be added as expected refs: #888 --- server/auth_function/lambda_function.py | 39 +++++++++++++++++++ server/auth_function/test/conftest.py | 28 +++++++++++++ server/auth_function/test/constant.py | 1 + .../auth_function/test/lamda_function_test.py | 4 +- ...ate_fam_role_assignment_to_admin_table.sql | 18 ++++----- 5 files changed, 79 insertions(+), 11 deletions(-) diff --git a/server/auth_function/lambda_function.py b/server/auth_function/lambda_function.py index 310e3794c..4ffaffd20 100644 --- a/server/auth_function/lambda_function.py +++ b/server/auth_function/lambda_function.py @@ -207,6 +207,45 @@ def handle_event(db_connection, event) -> event_type.Event: for record in cursor: role_list.append(record[0]) + # check if login through FAM + query_application = """ + SELECT application.application_name + FROM app_fam.fam_application application + JOIN app_fam.fam_application_client client ON + application.application_id = client.application_id + WHERE + client.cognito_client_id = {cognito_client_id}; + """ + sql_query_application = sql.SQL(query_application).format( + cognito_client_id=sql.Literal(cognito_client_id), + ) + cursor.execute(sql_query_application) + # if login through FAM, check fam app admin and add to role list + for record in cursor: + if record[0] == "FAM": + query_fam_app_admin = """ + SELECT application.application_name + FROM app_fam.fam_application_admin app_admin + INNER JOIN app_fam.fam_application application ON + app_admin.application_id = application.application_id + JOIN app_fam.fam_application_client client ON + app_admin.application_id = client.application_id + JOIN app_fam.fam_user fam_user ON + app_admin.user_id = fam_user.user_id + WHERE + fam_user.user_guid = {user_guid} + AND fam_user.user_type_code = {user_type_code} + AND client.cognito_client_id = {cognito_client_id}; + """ + sql_query_fam_app_admin = sql.SQL(query_fam_app_admin).format( + user_guid=sql.Literal(user_guid), + user_type_code=sql.Literal(user_type_code), + cognito_client_id=sql.Literal(cognito_client_id), + ) + cursor.execute(sql_query_fam_app_admin) + for record in cursor: + role_list.append(f"{record[0]}_ADMIN") + event["response"]["claimsOverrideDetails"] = { "groupOverrideDetails": { "groupsToOverride": role_list, diff --git a/server/auth_function/test/conftest.py b/server/auth_function/test/conftest.py index 9cc34e618..0fe1ac39b 100644 --- a/server/auth_function/test/conftest.py +++ b/server/auth_function/test/conftest.py @@ -249,6 +249,34 @@ def create_user_role_xref_record(db_pg_transaction, test_user_properties): ), ) +@pytest.fixture(scope="function") +def create_fam_application_admin_record(db_pg_transaction, test_user_properties): + initial_user = test_user_properties + cursor = db_pg_transaction.cursor() + raw_query = """ + insert into app_fam.fam_application_admin + (user_id, + application_id, + create_user, + update_user) + VALUES ( + (select user_id from app_fam.fam_user where + user_name = %s + and user_type_code = %s), + (select application_id from app_fam.fam_application + where application_name = 'FAM'), + CURRENT_USER, + CURRENT_USER + ) + """ + cursor.execute( + raw_query, + ( + initial_user.get("idp_username"), + initial_user.get("idp_type_code") + ), + ) + @pytest.fixture(scope="function") def initial_user_without_guid_or_cognito_id(db_pg_transaction, cognito_event): diff --git a/server/auth_function/test/constant.py b/server/auth_function/test/constant.py index 3f7bcd56d..22883b1ba 100644 --- a/server/auth_function/test/constant.py +++ b/server/auth_function/test/constant.py @@ -1 +1,2 @@ TEST_ROLE_NAME = "EXPECTED" +TEST_ADMIN_ROLE_NAME = "FAM_ADMIN" diff --git a/server/auth_function/test/lamda_function_test.py b/server/auth_function/test/lamda_function_test.py index cdc46f56f..d2723f391 100644 --- a/server/auth_function/test/lamda_function_test.py +++ b/server/auth_function/test/lamda_function_test.py @@ -5,7 +5,7 @@ import pytest from psycopg2 import sql -from constant import TEST_ROLE_NAME +from constant import TEST_ROLE_NAME, TEST_ADMIN_ROLE_NAME modulePath = os.path.join(os.path.dirname(__file__), "..") sys.path.append(modulePath) @@ -143,6 +143,7 @@ def test_direct_role_assignment( create_test_fam_role, create_test_fam_cognito_client, create_user_role_xref_record, + create_fam_application_admin_record ): """role doesn't have childreen (ie no forest client roles associated and the user is getting assigned directly to the role""" @@ -157,6 +158,7 @@ def test_direct_role_assignment( ]["groupsToOverride"] LOGGER.debug(f"override groups: {override_groups}") assert TEST_ROLE_NAME in override_groups + assert TEST_ADMIN_ROLE_NAME in override_groups @pytest.mark.parametrize( diff --git a/server/flyway/sql/V34__migrate_fam_role_assignment_to_admin_table.sql b/server/flyway/sql/V34__migrate_fam_role_assignment_to_admin_table.sql index d3d518889..f1e60f800 100644 --- a/server/flyway/sql/V34__migrate_fam_role_assignment_to_admin_table.sql +++ b/server/flyway/sql/V34__migrate_fam_role_assignment_to_admin_table.sql @@ -4,14 +4,12 @@ -- -- for example, role name: FAM_ACCESS_ADMIN contains application name: FAM -- -- role name: FOM_DEV_ACCESS_ADMIN contains application name: FOM_DEV -- select the user_id and application_id the user is admin of, insert into fam_application_admin -INSERT INTO fam_application_admin (user_id, application_id, create_user, create_date) -SELECT admins.user_id, admins.application_id, CURRENT_USER, CURRENT_DATE FROM ( - SELECT user_id, application_id FROM fam_user_role_xref c JOIN ( - SELECT role_id, application_id FROM fam_application a JOIN ( - SELECT role_id, role_name FROM fam_role WHERE application_id=1 - ) b ON b.role_name LIKE '%' || a.application_name || '%' - ) d ON c.role_id = d.role_id -) admins; - - +INSERT INTO app_fam.fam_application_admin (user_id, application_id, create_user, create_date) +SELECT user_role_xref.user_id, application.application_id, CURRENT_USER, CURRENT_DATE +FROM app_fam.fam_role role +JOIN app_fam.fam_application application + ON role.role_name LIKE '%' || application.application_name || '%' +JOIN app_fam.fam_user_role_xref user_role_xref + ON role.role_id = user_role_xref.role_id +WHERE role.application_id=1; From 8df4d3235c1fcb81741fd55fc996e98d012aeabe Mon Sep 17 00:00:00 2001 From: catherine meng Date: Tue, 5 Dec 2023 16:11:33 -0800 Subject: [PATCH 3/9] fix(888): fix admin management test, refs: #888 --- server/admin_management/tests/constants.py | 2 +- .../test_application_admin_repository.py | 13 ++++++++----- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/server/admin_management/tests/constants.py b/server/admin_management/tests/constants.py index 93b23655a..30b01dea9 100644 --- a/server/admin_management/tests/constants.py +++ b/server/admin_management/tests/constants.py @@ -21,7 +21,7 @@ TEST_APPLICATION_NAME_FAM = "FAM" # -------------------- test application admin data ------------------ # -TEST_NEW_APPLICATION_ADMIN_USER_ID = 1 +TEST_NEW_APPLICATION_ADMIN_USER_ID = 6 # the first 5 users already are fam admin TEST_NEW_APPLICATION_ADMIN = { "user_type_code": famConstants.UserType.BCEID, "user_name": "TEST_USER", diff --git a/server/admin_management/tests/repositories/test_application_admin_repository.py b/server/admin_management/tests/repositories/test_application_admin_repository.py index 068dd0ec9..184390d4e 100644 --- a/server/admin_management/tests/repositories/test_application_admin_repository.py +++ b/server/admin_management/tests/repositories/test_application_admin_repository.py @@ -40,11 +40,12 @@ def test_create_application_admin_and_get( def test_get_application_admin_by_application_id( application_admin_repo: ApplicationAdminRepository, ): - # find application admin, no data initially - application_admin = application_admin_repo.get_application_admin_by_application_id( + # find application admin and get count + application_admins = application_admin_repo.get_application_admin_by_application_id( TEST_APPLICATION_ID_FAM ) - assert len(application_admin) == 0 + assert application_admins is not None + application_admin_count = len(application_admins) # create a new application admin new_application_admin = application_admin_repo.create_application_admin( @@ -54,10 +55,12 @@ def test_get_application_admin_by_application_id( ) assert new_application_admin.application_id == TEST_APPLICATION_ID_FAM # get the new application admin by application id - application_admin = application_admin_repo.get_application_admin_by_application_id( + application_admins = application_admin_repo.get_application_admin_by_application_id( TEST_APPLICATION_ID_FAM ) - assert application_admin is not None + assert application_admins is not None + assert len(application_admins) == application_admin_count + 1 + def test_get_application_admin_by_id( From b73466cd65c8392a352e6da577db9d6d67d5a06a Mon Sep 17 00:00:00 2001 From: catherine meng Date: Tue, 5 Dec 2023 17:04:18 -0800 Subject: [PATCH 4/9] fix(888): fix admin management test, add unique index in fam application admin table, refs: #888 --- server/admin_management/tests/constants.py | 3 ++- .../repositories/test_application_admin_repository.py | 7 ++++--- .../V34__migrate_fam_role_assignment_to_admin_table.sql | 3 +++ 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/server/admin_management/tests/constants.py b/server/admin_management/tests/constants.py index 30b01dea9..1bafedd18 100644 --- a/server/admin_management/tests/constants.py +++ b/server/admin_management/tests/constants.py @@ -21,7 +21,8 @@ TEST_APPLICATION_NAME_FAM = "FAM" # -------------------- test application admin data ------------------ # -TEST_NEW_APPLICATION_ADMIN_USER_ID = 6 # the first 5 users already are fam admin +TEST_APPLICATION_ADMIN_ID = 5 +TEST_NEW_APPLICATION_ADMIN_USER_ID = 1 TEST_NEW_APPLICATION_ADMIN = { "user_type_code": famConstants.UserType.BCEID, "user_name": "TEST_USER", diff --git a/server/admin_management/tests/repositories/test_application_admin_repository.py b/server/admin_management/tests/repositories/test_application_admin_repository.py index 184390d4e..e129a9a02 100644 --- a/server/admin_management/tests/repositories/test_application_admin_repository.py +++ b/server/admin_management/tests/repositories/test_application_admin_repository.py @@ -4,6 +4,7 @@ from tests.constants import ( TEST_APPLICATION_ID_FAM, + TEST_APPLICATION_ADMIN_ID, TEST_NEW_APPLICATION_ADMIN_USER_ID, TEST_CREATOR, ) @@ -17,16 +18,16 @@ def test_create_application_admin_and_get( ): # create a new application admin new_application_admin = application_admin_repo.create_application_admin( - TEST_APPLICATION_ID_FAM, + TEST_APPLICATION_ADMIN_ID, TEST_NEW_APPLICATION_ADMIN_USER_ID, TEST_CREATOR, ) - assert new_application_admin.application_id == TEST_APPLICATION_ID_FAM + assert new_application_admin.application_id == TEST_APPLICATION_ADMIN_ID assert new_application_admin.user_id == TEST_NEW_APPLICATION_ADMIN_USER_ID # get the new created application admin application_admin = application_admin_repo.get_application_admin_by_app_and_user_id( - TEST_APPLICATION_ID_FAM, + TEST_APPLICATION_ADMIN_ID, TEST_NEW_APPLICATION_ADMIN_USER_ID, ) assert new_application_admin.user_id == application_admin.user_id diff --git a/server/flyway/sql/V34__migrate_fam_role_assignment_to_admin_table.sql b/server/flyway/sql/V34__migrate_fam_role_assignment_to_admin_table.sql index f1e60f800..bb6f76865 100644 --- a/server/flyway/sql/V34__migrate_fam_role_assignment_to_admin_table.sql +++ b/server/flyway/sql/V34__migrate_fam_role_assignment_to_admin_table.sql @@ -1,3 +1,5 @@ +CREATE UNIQUE INDEX fam_usr_app_admin_uk ON app_fam.fam_application_admin(user_id, application_id); + -- migrate the fam roles to fam admin management table -- first select all fam roles -- and then select role_id and application_id this role is admin of, based on if the role name contains the application_name @@ -13,3 +15,4 @@ JOIN app_fam.fam_user_role_xref user_role_xref ON role.role_id = user_role_xref.role_id WHERE role.application_id=1; + From 2989413ba70fad9172c9b50b1fd3a6da1c142667 Mon Sep 17 00:00:00 2001 From: catherine meng Date: Tue, 5 Dec 2023 17:23:35 -0800 Subject: [PATCH 5/9] fix(888): fix admin management test constants, refs: #888 --- server/admin_management/tests/constants.py | 4 ++-- .../test_application_admin_repository.py | 14 ++++++-------- .../tests/routers/test_application_admin_router.py | 8 ++++---- 3 files changed, 12 insertions(+), 14 deletions(-) diff --git a/server/admin_management/tests/constants.py b/server/admin_management/tests/constants.py index 1bafedd18..a270ce12d 100644 --- a/server/admin_management/tests/constants.py +++ b/server/admin_management/tests/constants.py @@ -21,10 +21,10 @@ TEST_APPLICATION_NAME_FAM = "FAM" # -------------------- test application admin data ------------------ # -TEST_APPLICATION_ADMIN_ID = 5 +TEST_APPLICATION_ADMIN_ID = 3 TEST_NEW_APPLICATION_ADMIN_USER_ID = 1 TEST_NEW_APPLICATION_ADMIN = { "user_type_code": famConstants.UserType.BCEID, "user_name": "TEST_USER", - "application_id": TEST_APPLICATION_ID_FAM, + "application_id": TEST_APPLICATION_ADMIN_ID, } diff --git a/server/admin_management/tests/repositories/test_application_admin_repository.py b/server/admin_management/tests/repositories/test_application_admin_repository.py index e129a9a02..bac1582dd 100644 --- a/server/admin_management/tests/repositories/test_application_admin_repository.py +++ b/server/admin_management/tests/repositories/test_application_admin_repository.py @@ -3,7 +3,6 @@ from api.app.repositories.application_admin_repository import ApplicationAdminRepository from tests.constants import ( - TEST_APPLICATION_ID_FAM, TEST_APPLICATION_ADMIN_ID, TEST_NEW_APPLICATION_ADMIN_USER_ID, TEST_CREATOR, @@ -43,33 +42,32 @@ def test_get_application_admin_by_application_id( ): # find application admin and get count application_admins = application_admin_repo.get_application_admin_by_application_id( - TEST_APPLICATION_ID_FAM + TEST_APPLICATION_ADMIN_ID ) assert application_admins is not None application_admin_count = len(application_admins) # create a new application admin new_application_admin = application_admin_repo.create_application_admin( - TEST_APPLICATION_ID_FAM, + TEST_APPLICATION_ADMIN_ID, TEST_NEW_APPLICATION_ADMIN_USER_ID, TEST_CREATOR, ) - assert new_application_admin.application_id == TEST_APPLICATION_ID_FAM + assert new_application_admin.application_id == TEST_APPLICATION_ADMIN_ID # get the new application admin by application id application_admins = application_admin_repo.get_application_admin_by_application_id( - TEST_APPLICATION_ID_FAM + TEST_APPLICATION_ADMIN_ID ) assert application_admins is not None assert len(application_admins) == application_admin_count + 1 - def test_get_application_admin_by_id( application_admin_repo: ApplicationAdminRepository, ): # create a new application admin new_application_admin = application_admin_repo.create_application_admin( - TEST_APPLICATION_ID_FAM, + TEST_APPLICATION_ADMIN_ID, TEST_NEW_APPLICATION_ADMIN_USER_ID, TEST_CREATOR, ) @@ -86,7 +84,7 @@ def test_get_application_admin_by_id( def test_delete_application_admin(application_admin_repo: ApplicationAdminRepository): # create a new application admin new_application_admin = application_admin_repo.create_application_admin( - TEST_APPLICATION_ID_FAM, + TEST_APPLICATION_ADMIN_ID, TEST_NEW_APPLICATION_ADMIN_USER_ID, TEST_CREATOR, ) diff --git a/server/admin_management/tests/routers/test_application_admin_router.py b/server/admin_management/tests/routers/test_application_admin_router.py index 0fbc88134..030040931 100644 --- a/server/admin_management/tests/routers/test_application_admin_router.py +++ b/server/admin_management/tests/routers/test_application_admin_router.py @@ -14,7 +14,7 @@ TEST_INVALID_USER_TYPE, TEST_NEW_APPLICATION_ADMIN, TEST_NOT_EXIST_APPLICATION_ID, - TEST_APPLICATION_ID_FAM, + TEST_APPLICATION_ADMIN_ID, TEST_FOM_DEV_ADMIN_ROLE, INVALID_APPLICATION_ID, ) @@ -134,7 +134,7 @@ def test_get_application_admin_by_application_id( # test get with invalid role token = jwt_utils.create_jwt_token(test_rsa_key, [TEST_FOM_DEV_ADMIN_ROLE]) response = test_client_fixture.get( - f"{endPoint}/{TEST_APPLICATION_ID_FAM}/admins", headers=jwt_utils.headers(token) + f"{endPoint}/{TEST_APPLICATION_ADMIN_ID}/admins", headers=jwt_utils.headers(token) ) assert response.status_code == HTTPStatus.FORBIDDEN assert response.json() is not None @@ -143,7 +143,7 @@ def test_get_application_admin_by_application_id( # get application admin by application id, get original length token = jwt_utils.create_jwt_token(test_rsa_key) response = test_client_fixture.get( - f"{endPoint}/{TEST_APPLICATION_ID_FAM}/admins", + f"{endPoint}/{TEST_APPLICATION_ADMIN_ID}/admins", headers=jwt_utils.headers(token), ) assert response.status_code == HTTPStatus.OK @@ -161,7 +161,7 @@ def test_get_application_admin_by_application_id( ) # get the application by application id again, verify length adds one response = test_client_fixture.get( - f"{endPoint}/{TEST_APPLICATION_ID_FAM}/admins", + f"{endPoint}/{TEST_APPLICATION_ADMIN_ID}/admins", headers=jwt_utils.headers(token), ) assert response.status_code == HTTPStatus.OK From 7c17509afe01f53a42ed04ac20c163f602fb355e Mon Sep 17 00:00:00 2001 From: catherine meng Date: Wed, 6 Dec 2023 13:09:56 -0800 Subject: [PATCH 6/9] fix(888): rename admin management test constants, refs: #888 --- server/admin_management/tests/constants.py | 4 ++-- .../test_application_admin_repository.py | 20 +++++++++---------- .../routers/test_application_admin_router.py | 8 ++++---- 3 files changed, 16 insertions(+), 16 deletions(-) diff --git a/server/admin_management/tests/constants.py b/server/admin_management/tests/constants.py index a270ce12d..1ba2e926a 100644 --- a/server/admin_management/tests/constants.py +++ b/server/admin_management/tests/constants.py @@ -21,10 +21,10 @@ TEST_APPLICATION_NAME_FAM = "FAM" # -------------------- test application admin data ------------------ # -TEST_APPLICATION_ADMIN_ID = 3 +TEST_APPLICATION_ADMIN_APPLICATION_ID = 3 TEST_NEW_APPLICATION_ADMIN_USER_ID = 1 TEST_NEW_APPLICATION_ADMIN = { "user_type_code": famConstants.UserType.BCEID, "user_name": "TEST_USER", - "application_id": TEST_APPLICATION_ADMIN_ID, + "application_id": TEST_APPLICATION_ADMIN_APPLICATION_ID, } diff --git a/server/admin_management/tests/repositories/test_application_admin_repository.py b/server/admin_management/tests/repositories/test_application_admin_repository.py index bac1582dd..928d60028 100644 --- a/server/admin_management/tests/repositories/test_application_admin_repository.py +++ b/server/admin_management/tests/repositories/test_application_admin_repository.py @@ -3,7 +3,7 @@ from api.app.repositories.application_admin_repository import ApplicationAdminRepository from tests.constants import ( - TEST_APPLICATION_ADMIN_ID, + TEST_APPLICATION_ADMIN_APPLICATION_ID, TEST_NEW_APPLICATION_ADMIN_USER_ID, TEST_CREATOR, ) @@ -17,16 +17,16 @@ def test_create_application_admin_and_get( ): # create a new application admin new_application_admin = application_admin_repo.create_application_admin( - TEST_APPLICATION_ADMIN_ID, + TEST_APPLICATION_ADMIN_APPLICATION_ID, TEST_NEW_APPLICATION_ADMIN_USER_ID, TEST_CREATOR, ) - assert new_application_admin.application_id == TEST_APPLICATION_ADMIN_ID + assert new_application_admin.application_id == TEST_APPLICATION_ADMIN_APPLICATION_ID assert new_application_admin.user_id == TEST_NEW_APPLICATION_ADMIN_USER_ID # get the new created application admin application_admin = application_admin_repo.get_application_admin_by_app_and_user_id( - TEST_APPLICATION_ADMIN_ID, + TEST_APPLICATION_ADMIN_APPLICATION_ID, TEST_NEW_APPLICATION_ADMIN_USER_ID, ) assert new_application_admin.user_id == application_admin.user_id @@ -42,21 +42,21 @@ def test_get_application_admin_by_application_id( ): # find application admin and get count application_admins = application_admin_repo.get_application_admin_by_application_id( - TEST_APPLICATION_ADMIN_ID + TEST_APPLICATION_ADMIN_APPLICATION_ID ) assert application_admins is not None application_admin_count = len(application_admins) # create a new application admin new_application_admin = application_admin_repo.create_application_admin( - TEST_APPLICATION_ADMIN_ID, + TEST_APPLICATION_ADMIN_APPLICATION_ID, TEST_NEW_APPLICATION_ADMIN_USER_ID, TEST_CREATOR, ) - assert new_application_admin.application_id == TEST_APPLICATION_ADMIN_ID + assert new_application_admin.application_id == TEST_APPLICATION_ADMIN_APPLICATION_ID # get the new application admin by application id application_admins = application_admin_repo.get_application_admin_by_application_id( - TEST_APPLICATION_ADMIN_ID + TEST_APPLICATION_ADMIN_APPLICATION_ID ) assert application_admins is not None assert len(application_admins) == application_admin_count + 1 @@ -67,7 +67,7 @@ def test_get_application_admin_by_id( ): # create a new application admin new_application_admin = application_admin_repo.create_application_admin( - TEST_APPLICATION_ADMIN_ID, + TEST_APPLICATION_ADMIN_APPLICATION_ID, TEST_NEW_APPLICATION_ADMIN_USER_ID, TEST_CREATOR, ) @@ -84,7 +84,7 @@ def test_get_application_admin_by_id( def test_delete_application_admin(application_admin_repo: ApplicationAdminRepository): # create a new application admin new_application_admin = application_admin_repo.create_application_admin( - TEST_APPLICATION_ADMIN_ID, + TEST_APPLICATION_ADMIN_APPLICATION_ID, TEST_NEW_APPLICATION_ADMIN_USER_ID, TEST_CREATOR, ) diff --git a/server/admin_management/tests/routers/test_application_admin_router.py b/server/admin_management/tests/routers/test_application_admin_router.py index 030040931..d11a59b90 100644 --- a/server/admin_management/tests/routers/test_application_admin_router.py +++ b/server/admin_management/tests/routers/test_application_admin_router.py @@ -14,7 +14,7 @@ TEST_INVALID_USER_TYPE, TEST_NEW_APPLICATION_ADMIN, TEST_NOT_EXIST_APPLICATION_ID, - TEST_APPLICATION_ADMIN_ID, + TEST_APPLICATION_ADMIN_APPLICATION_ID, TEST_FOM_DEV_ADMIN_ROLE, INVALID_APPLICATION_ID, ) @@ -134,7 +134,7 @@ def test_get_application_admin_by_application_id( # test get with invalid role token = jwt_utils.create_jwt_token(test_rsa_key, [TEST_FOM_DEV_ADMIN_ROLE]) response = test_client_fixture.get( - f"{endPoint}/{TEST_APPLICATION_ADMIN_ID}/admins", headers=jwt_utils.headers(token) + f"{endPoint}/{TEST_APPLICATION_ADMIN_APPLICATION_ID}/admins", headers=jwt_utils.headers(token) ) assert response.status_code == HTTPStatus.FORBIDDEN assert response.json() is not None @@ -143,7 +143,7 @@ def test_get_application_admin_by_application_id( # get application admin by application id, get original length token = jwt_utils.create_jwt_token(test_rsa_key) response = test_client_fixture.get( - f"{endPoint}/{TEST_APPLICATION_ADMIN_ID}/admins", + f"{endPoint}/{TEST_APPLICATION_ADMIN_APPLICATION_ID}/admins", headers=jwt_utils.headers(token), ) assert response.status_code == HTTPStatus.OK @@ -161,7 +161,7 @@ def test_get_application_admin_by_application_id( ) # get the application by application id again, verify length adds one response = test_client_fixture.get( - f"{endPoint}/{TEST_APPLICATION_ADMIN_ID}/admins", + f"{endPoint}/{TEST_APPLICATION_ADMIN_APPLICATION_ID}/admins", headers=jwt_utils.headers(token), ) assert response.status_code == HTTPStatus.OK From 08172d8b517dd04510123af5937bc39b5a32aedc Mon Sep 17 00:00:00 2001 From: catherine meng Date: Wed, 6 Dec 2023 14:16:32 -0800 Subject: [PATCH 7/9] fix(888): rename fam app admin unique constrain, refs: #888 --- .../tests/routers/test_application_admin_router.py | 1 - .../sql/V34__migrate_fam_role_assignment_to_admin_table.sql | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/server/admin_management/tests/routers/test_application_admin_router.py b/server/admin_management/tests/routers/test_application_admin_router.py index d11a59b90..903218772 100644 --- a/server/admin_management/tests/routers/test_application_admin_router.py +++ b/server/admin_management/tests/routers/test_application_admin_router.py @@ -12,7 +12,6 @@ from tests.constants import ( TEST_NEW_APPLICATION_ADMIN, TEST_INVALID_USER_TYPE, - TEST_NEW_APPLICATION_ADMIN, TEST_NOT_EXIST_APPLICATION_ID, TEST_APPLICATION_ADMIN_APPLICATION_ID, TEST_FOM_DEV_ADMIN_ROLE, diff --git a/server/flyway/sql/V34__migrate_fam_role_assignment_to_admin_table.sql b/server/flyway/sql/V34__migrate_fam_role_assignment_to_admin_table.sql index bb6f76865..5f0373380 100644 --- a/server/flyway/sql/V34__migrate_fam_role_assignment_to_admin_table.sql +++ b/server/flyway/sql/V34__migrate_fam_role_assignment_to_admin_table.sql @@ -1,4 +1,4 @@ -CREATE UNIQUE INDEX fam_usr_app_admin_uk ON app_fam.fam_application_admin(user_id, application_id); +CREATE UNIQUE INDEX fam_app_admin_usr_app_uk ON app_fam.fam_application_admin(user_id, application_id); -- migrate the fam roles to fam admin management table -- first select all fam roles From d28c7dd05e47e8c5cc53b8bcec492674896ef8c1 Mon Sep 17 00:00:00 2001 From: catherine meng Date: Thu, 7 Dec 2023 11:05:19 -0800 Subject: [PATCH 8/9] fix(888): update backend and admin management hardcode access admin role to be admin, refs: #888 --- .../api/app/routers/router_guards.py | 2 +- server/admin_management/tests/constants.py | 2 +- server/admin_management/tests/jwt_utils.py | 4 ++-- .../backend/api/app/crud/crud_application.py | 2 +- .../backend/api/app/routers/router_guards.py | 4 ++-- .../backend/testspg/crud/test_crud_utils.py | 8 +++---- .../testspg/crud/test_idim_proxy_service.py | 2 +- server/backend/testspg/jwt_utils.py | 4 ++-- .../testspg/router/test_router_application.py | 24 +++++++++---------- .../testspg/router/test_router_idim_proxy.py | 2 +- .../test_router_user_role_assignment.py | 6 ++--- 11 files changed, 30 insertions(+), 30 deletions(-) diff --git a/server/admin_management/api/app/routers/router_guards.py b/server/admin_management/api/app/routers/router_guards.py index a2283e03e..caf846182 100644 --- a/server/admin_management/api/app/routers/router_guards.py +++ b/server/admin_management/api/app/routers/router_guards.py @@ -39,7 +39,7 @@ def authorize_by_fam_admin(claims: dict = Depends(validate_token)): - required_role = "FAM_ACCESS_ADMIN" + required_role = "FAM_ADMIN" access_roles = get_access_roles(claims) if required_role not in access_roles: diff --git a/server/admin_management/tests/constants.py b/server/admin_management/tests/constants.py index 1ba2e926a..b55a4364b 100644 --- a/server/admin_management/tests/constants.py +++ b/server/admin_management/tests/constants.py @@ -2,7 +2,7 @@ TEST_CREATOR = "TESTER" -TEST_FOM_DEV_ADMIN_ROLE = "FOM_DEV_ACCESS_ADMIN" +TEST_FOM_DEV_ADMIN_ROLE = "FOM_DEV_ADMIN" INVALID_APPLICATION_ID = "invalid_application_id" # ---------------------- test user data ----------------------------- # diff --git a/server/admin_management/tests/jwt_utils.py b/server/admin_management/tests/jwt_utils.py index b74e4db97..1d4bfc6d0 100644 --- a/server/admin_management/tests/jwt_utils.py +++ b/server/admin_management/tests/jwt_utils.py @@ -16,7 +16,7 @@ def create_jwt_claims(): return { "sub": "51b661cf-4109-4616-b7a5-178daf51fc12", - "cognito:groups": ["FAM_ACCESS_ADMIN"], + "cognito:groups": ["FAM_ADMIN"], "iss": f"https://cognito-idp.{COGNITO_REGION}.amazonaws.com/{COGNITO_USER_POOL_ID}", "version": 2, "client_id": COGNITO_CLIENT_ID, @@ -33,7 +33,7 @@ def create_jwt_claims(): def create_jwt_token( test_rsa_key, - roles=["FAM_ACCESS_ADMIN"], + roles=["FAM_ADMIN"], claims=create_jwt_claims(), test_algorithm="RS256", test_headers={"kid": "12345"}, diff --git a/server/backend/api/app/crud/crud_application.py b/server/backend/api/app/crud/crud_application.py index 55c434d5e..3d8739eee 100644 --- a/server/backend/api/app/crud/crud_application.py +++ b/server/backend/api/app/crud/crud_application.py @@ -40,7 +40,7 @@ def get_applications_by_granted_apps(db: Session, access_roles: List[str]) -> Li LOGGER.debug(f"Running get_applications_by_granted_app, access_roles: {access_roles}") # Filter out others and only contains Access Admin roles - ACCESS_ADMIN_ROLE_SUFFIX = "_ACCESS_ADMIN" + ACCESS_ADMIN_ROLE_SUFFIX = "_ADMIN" admin_access_roles = filter( lambda x: x.endswith(ACCESS_ADMIN_ROLE_SUFFIX), access_roles ) diff --git a/server/backend/api/app/routers/router_guards.py b/server/backend/api/app/routers/router_guards.py index 12892a0f0..0a0e28525 100644 --- a/server/backend/api/app/routers/router_guards.py +++ b/server/backend/api/app/routers/router_guards.py @@ -61,7 +61,7 @@ def authorize_by_app_id( headers={"WWW-Authenticate": "Bearer"}, ) - required_role = f"{application.application_name.upper()}_ACCESS_ADMIN" + required_role = f"{application.application_name.upper()}_ADMIN" access_roles = get_access_roles(claims) if required_role not in access_roles: @@ -117,7 +117,7 @@ def authorize_by_application_role( ): """ This router validation is currently design to validate logged on "admin" - has authority to perform actions for application with roles in [app]_ACCESS_ADMIN. + has authority to perform actions for application with roles in [app]_ADMIN. This function basically is the same and depends on (authorize_by_app_id()) but for the need that some routers contains target role_id in the request (instead of application_id). """ diff --git a/server/backend/testspg/crud/test_crud_utils.py b/server/backend/testspg/crud/test_crud_utils.py index 5e4c75f54..6e0849d34 100644 --- a/server/backend/testspg/crud/test_crud_utils.py +++ b/server/backend/testspg/crud/test_crud_utils.py @@ -26,14 +26,14 @@ def test_to_upper(str_list_to_test, expcted_str_list): @pytest.mark.parametrize("str_list_to_test, str_to_replace, replace_with, expcted_str_list", [ ( - ['FAM_ACCESS_ADMIN', 'FOM_DEV_ACCESS_ADMIN', 'FOM_TEST_ACCESS_ADMIN'], - "_ACCESS_ADMIN", "", + ['FAM_ADMIN', 'FOM_DEV_ADMIN', 'FOM_TEST_ADMIN'], + "_ADMIN", "", ['FAM', 'FOM_DEV', 'FOM_TEST'] ), ( ['FAM_ACCESS', 'FOM_DEV', 'FOM'], - "_ACCESS", "_ACCESS_ADMIN", - ['FAM_ACCESS_ADMIN', 'FOM_DEV', 'FOM'] + "_ACCESS", "_ADMIN", + ['FAM_ADMIN', 'FOM_DEV', 'FOM'] ), (None, "something", "some_other_thing", None) ]) diff --git a/server/backend/testspg/crud/test_idim_proxy_service.py b/server/backend/testspg/crud/test_idim_proxy_service.py index 4c1258ce2..8ccd5e9ce 100644 --- a/server/backend/testspg/crud/test_idim_proxy_service.py +++ b/server/backend/testspg/crud/test_idim_proxy_service.py @@ -23,7 +23,7 @@ def setup_class(self): "cognito_user_id": "dev-idir_e72a12c916a44f39e5dcdffae7@idir", "user_name": "IANLIU", "user_type": "I", - "access_roles": ["FAM_ACCESS_ADMIN", "FOM_DEV_ACCESS_ADMIN"] + "access_roles": ["FAM_ADMIN", "FOM_DEV_ADMIN"] } ) diff --git a/server/backend/testspg/jwt_utils.py b/server/backend/testspg/jwt_utils.py index 7a2ed51d5..02ef95894 100644 --- a/server/backend/testspg/jwt_utils.py +++ b/server/backend/testspg/jwt_utils.py @@ -17,7 +17,7 @@ def create_jwt_claims(): return { "sub": "51b661cf-4109-4616-b7a5-178daf51fc12", "cognito:groups": [ - "FAM_ACCESS_ADMIN" + "FAM_ADMIN" ], "iss": f"https://cognito-idp.{COGNITO_REGION}.amazonaws.com/{COGNITO_USER_POOL_ID}", "version": 2, @@ -35,7 +35,7 @@ def create_jwt_claims(): def create_jwt_token( test_rsa_key, - roles=["FAM_ACCESS_ADMIN"], + roles=["FAM_ADMIN"], claims=create_jwt_claims(), test_algorithm='RS256', test_headers={"kid": "12345"}, diff --git a/server/backend/testspg/router/test_router_application.py b/server/backend/testspg/router/test_router_application.py index 8a5055fcf..59098d1a5 100644 --- a/server/backend/testspg/router/test_router_application.py +++ b/server/backend/testspg/router/test_router_application.py @@ -20,24 +20,24 @@ def test_get_applications( test_client_fixture: starlette.testclient.TestClient, test_rsa_key ): - # Test Accss Roles: FAM_ACCESS_ADMIN only - access_roles_fam_only = ["FAM_ACCESS_ADMIN"] + # Test Accss Roles: FAM_ADMIN only + access_roles_fam_only = ["FAM_ADMIN"] token = jwt_utils.create_jwt_token(test_rsa_key, access_roles_fam_only) response = test_client_fixture.get(f"{endPoint}", headers=jwt_utils.headers(token)) data = response.json() assert len(data) == 1 assert data[0]["application_name"] == "FAM" - # Test Accss Roles: FOM_DEV_ACCESS_ADMIN only - access_roles_fom_dev_only = ["FOM_DEV_ACCESS_ADMIN"] + # Test Accss Roles: FOM_DEV_ADMIN only + access_roles_fom_dev_only = ["FOM_DEV_ADMIN"] token = jwt_utils.create_jwt_token(test_rsa_key, access_roles_fom_dev_only) response = test_client_fixture.get(f"{endPoint}", headers=jwt_utils.headers(token)) data = response.json() assert len(data) == 1 assert data[0]["application_name"] == "FOM_DEV" - # Test Accss Roles: both FAM_ACCESS_ADMIN and FOM_DEV_ACCESS_ADMIN - access_roles_fam_fom_dev = ["FAM_ACCESS_ADMIN", "FOM_DEV_ACCESS_ADMIN"] + # Test Accss Roles: both FAM_ADMIN and FOM_DEV_ADMIN + access_roles_fam_fom_dev = ["FAM_ADMIN", "FOM_DEV_ADMIN"] token = jwt_utils.create_jwt_token(test_rsa_key, access_roles_fam_fom_dev) response = test_client_fixture.get(f"{endPoint}", headers=jwt_utils.headers(token)) data = response.json() @@ -55,8 +55,8 @@ def test_get_applications( assert "update_date" in app assert "app_environment" in app - # Test Accss Roles: on NO_APP_ACCESS_ADMIN - access_roles_no_app = ["NO_APP_ACCESS_ADMIN"] + # Test Accss Roles: on NO_APP_ADMIN + access_roles_no_app = ["NO_APP_ADMIN"] token = jwt_utils.create_jwt_token(test_rsa_key, access_roles_no_app) response = test_client_fixture.get(f"{endPoint}", headers=jwt_utils.headers(token)) data = response.json() @@ -69,7 +69,7 @@ def test_get_fam_application_roles( ): # create a concrete role with an abstract role as parent # this role won't be returned - access_roles_fom_dev_only = ["FOM_DEV_ACCESS_ADMIN"] + access_roles_fom_dev_only = ["FOM_DEV_ADMIN"] token = jwt_utils.create_jwt_token(test_rsa_key, access_roles_fom_dev_only) response = test_client_fixture.post( @@ -134,7 +134,7 @@ def test_get_fam_application_user_role_assignment_no_role_assignments( test_client_fixture: starlette.testclient.TestClient, test_rsa_key ): - access_roles_fom_dev_only = ["FOM_DEV_ACCESS_ADMIN"] + access_roles_fom_dev_only = ["FOM_DEV_ADMIN"] # test no user role assignment for the application role_assignment_end_point = endPoint + \ @@ -150,7 +150,7 @@ def test_get_fam_application_user_role_assignment_concrete_role( test_client_fixture: starlette.testclient.TestClient, test_rsa_key ): - access_roles_fom_dev_only = ["FOM_DEV_ACCESS_ADMIN"] + access_roles_fom_dev_only = ["FOM_DEV_ADMIN"] role_assignment_end_point = endPoint + \ f"/{TEST_FOM_DEV_APPLICATION_ID}/user_role_assignment" @@ -184,7 +184,7 @@ def test_get_fam_application_user_role_assignment_abstract_role( test_client_fixture: starlette.testclient.TestClient, test_rsa_key ): - access_roles_fom_dev_only = ["FOM_DEV_ACCESS_ADMIN"] + access_roles_fom_dev_only = ["FOM_DEV_ADMIN"] role_assignment_end_point = endPoint + \ f"/{TEST_FOM_DEV_APPLICATION_ID}/user_role_assignment" diff --git a/server/backend/testspg/router/test_router_idim_proxy.py b/server/backend/testspg/router/test_router_idim_proxy.py index ab6d60ab4..65615eb8e 100644 --- a/server/backend/testspg/router/test_router_idim_proxy.py +++ b/server/backend/testspg/router/test_router_idim_proxy.py @@ -21,7 +21,7 @@ "cognito_user_id": "dev-idir_e72a12c916a44f39e5dcdffae7@idir", "user_name": "IANLIU", "user_type_code": UserType.IDIR, - "access_roles": ["FAM_ACCESS_ADMIN", "FOM_DEV_ACCESS_ADMIN"] + "access_roles": ["FAM_ADMIN", "FOM_DEV_ADMIN"] } diff --git a/server/backend/testspg/router/test_router_user_role_assignment.py b/server/backend/testspg/router/test_router_user_role_assignment.py index 8b11427e9..6f96466d2 100644 --- a/server/backend/testspg/router/test_router_user_role_assignment.py +++ b/server/backend/testspg/router/test_router_user_role_assignment.py @@ -28,8 +28,8 @@ LOGGER = logging.getLogger(__name__) endPoint = f"{apiPrefix}/user_role_assignment" -FOM_DEV_ADMIN_ROLE = "FOM_DEV_ACCESS_ADMIN" -FOM_TEST_ADMIN_ROLE = "FOM_TEST_ACCESS_ADMIN" +FOM_DEV_ADMIN_ROLE = "FOM_DEV_ADMIN" +FOM_TEST_ADMIN_ROLE = "FOM_TEST_ADMIN" ERROR_DUPLICATE_USER_ROLE = "Role already assigned to user." @pytest.fixture(scope="function") @@ -68,7 +68,7 @@ def test_create_user_role_assignment_not_authorized( ): """ test user has no authentication to the app - user without FOM_DEV_ACCESS_ADMIN role cannot grant FOM_DEV roles + user without FOM_DEV_ADMIN role cannot grant FOM_DEV roles """ token = jwt_utils.create_jwt_token(test_rsa_key) response = test_client_fixture.post( From 438c59d0f7f9f718c93d1fe9697da08a91e96c8f Mon Sep 17 00:00:00 2001 From: catherine meng Date: Thu, 7 Dec 2023 16:55:04 -0800 Subject: [PATCH 9/9] test(888): deploy to tools space for testing, refs: #888 --- .github/workflows/tools_deployment.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/tools_deployment.yml b/.github/workflows/tools_deployment.yml index 8971728f4..0b2e26a1f 100644 --- a/.github/workflows/tools_deployment.yml +++ b/.github/workflows/tools_deployment.yml @@ -5,7 +5,7 @@ on: push: branches: # Enable a specific branch to temporarily test in the tools environment. - - "feat/1010-admin-api-gateway" + - "feat/888-transfer-to-admin-table" # When use GHA OIDC provider and for action to create the JWT, it is required to have the id-token: write permission # permission can be added at job level or workflow level. Ref: https://github.com/aws-actions/configure-aws-credentials#OIDC