From 2aa14bc41ca5897dc3d647622322f40387c05b11 Mon Sep 17 00:00:00 2001 From: Katie Smith Date: Thu, 14 Mar 2019 16:55:48 +0000 Subject: [PATCH] Set folder permissions when adding a user to a service This sets the folder permissions for a user when adding them to a service. If a user is being added to a service after accepting an invite, we need to account for the possibility that the folders we are trying to add them to have been deleted before they accepted the invite. --- app/dao/services_dao.py | 11 ++++++- app/dao/template_folder_dao.py | 4 +++ app/service/rest.py | 3 +- tests/app/dao/test_services_dao.py | 47 ++++++++++++++++++++++++++++ tests/app/service/test_rest.py | 49 ++++++++++++++++++++++++++++-- 5 files changed, 110 insertions(+), 4 deletions(-) diff --git a/app/dao/services_dao.py b/app/dao/services_dao.py index e881bf020..4bef1e99c 100644 --- a/app/dao/services_dao.py +++ b/app/dao/services_dao.py @@ -14,6 +14,7 @@ from app.dao.dao_utils import ( from app.dao.organisation_dao import dao_get_organisation_by_email_address from app.dao.service_sms_sender_dao import insert_service_sms_sender from app.dao.service_user_dao import dao_get_service_user +from app.dao.template_folder_dao import dao_get_valid_template_folders_by_id from app.models import ( AnnualBilling, ApiKey, @@ -208,13 +209,21 @@ def dao_update_service(service): db.session.add(service) -def dao_add_user_to_service(service, user, permissions=None): +def dao_add_user_to_service(service, user, permissions=None, folder_permissions=None): permissions = permissions or [] + folder_permissions = folder_permissions or [] + try: from app.dao.permissions_dao import permission_dao service.users.append(user) permission_dao.set_user_service_permission(user, service, permissions, _commit=False) db.session.add(service) + + service_user = dao_get_service_user(user.id, service.id) + valid_template_folders = dao_get_valid_template_folders_by_id(folder_permissions) + service_user.folders = valid_template_folders + db.session.add(service_user) + except Exception as e: db.session.rollback() raise e diff --git a/app/dao/template_folder_dao.py b/app/dao/template_folder_dao.py index a162f79c0..6264f9bcf 100644 --- a/app/dao/template_folder_dao.py +++ b/app/dao/template_folder_dao.py @@ -10,6 +10,10 @@ def dao_get_template_folder_by_id_and_service_id(template_folder_id, service_id) ).one() +def dao_get_valid_template_folders_by_id(folder_ids): + return TemplateFolder.query.filter(TemplateFolder.id.in_(folder_ids)).all() + + @transactional def dao_create_template_folder(template_folder): db.session.add(template_folder) diff --git a/app/service/rest.py b/app/service/rest.py index 52651e2f8..ecaa3a58b 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -292,8 +292,9 @@ def add_user_to_service(service_id, user_id): Permission(service_id=service_id, user_id=user_id, permission=p['permission']) for p in data['permissions'] ] + folder_permissions = data.get('folder_permissions', []) - dao_add_user_to_service(service, user, permissions) + dao_add_user_to_service(service, user, permissions, folder_permissions) data = service_schema.dump(service).data return jsonify(data=data), 201 diff --git a/tests/app/dao/test_services_dao.py b/tests/app/dao/test_services_dao.py index cbc6cb24c..a8665554f 100644 --- a/tests/app/dao/test_services_dao.py +++ b/tests/app/dao/test_services_dao.py @@ -46,6 +46,7 @@ from app.models import ( InvitedUser, Service, ServicePermission, + ServiceUser, KEY_TYPE_NORMAL, KEY_TYPE_TEAM, KEY_TYPE_TEST, @@ -192,6 +193,52 @@ def test_should_add_user_to_service(notify_db_session): assert new_user in Service.query.first().users +def test_dao_add_user_to_service_sets_folder_permissions(sample_user, sample_service): + folder_1 = create_template_folder(sample_service) + folder_2 = create_template_folder(sample_service) + + assert not folder_1.users + assert not folder_2.users + + folder_permissions = [str(folder_1.id), str(folder_2.id)] + + dao_add_user_to_service(sample_service, sample_user, folder_permissions=folder_permissions) + + service_user = dao_get_service_user(user_id=sample_user.id, service_id=sample_service.id) + assert len(service_user.folders) == 2 + assert folder_1 in service_user.folders + assert folder_2 in service_user.folders + + +def test_dao_add_user_to_service_ignores_folders_which_do_not_exist_when_setting_permissions( + sample_user, + sample_service, + fake_uuid +): + valid_folder = create_template_folder(sample_service) + folder_permissions = [fake_uuid, str(valid_folder.id)] + + dao_add_user_to_service(sample_service, sample_user, folder_permissions=folder_permissions) + + service_user = dao_get_service_user(sample_user.id, sample_service.id) + + assert service_user.folders == [valid_folder] + + +def test_dao_add_user_to_service_raises_error_if_adding_folder_permissions_for_a_different_service( + sample_user, + sample_service, +): + other_service = create_service(service_name='other service') + other_service_folder = create_template_folder(other_service) + folder_permissions = [str(other_service_folder.id)] + + with pytest.raises(IntegrityError) as e: + dao_add_user_to_service(sample_service, sample_user, folder_permissions=folder_permissions) + assert 'insert or update on table "user_folder_permissions" violates foreign key constraint' in str(e.value) + assert ServiceUser.query.count() == 0 + + def test_should_remove_user_from_service(notify_db_session): user = create_user() service = Service(name="service_name", diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index 89b4d40d4..80d4eff42 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -11,6 +11,7 @@ from freezegun import freeze_time from app.dao.organisation_dao import dao_add_service_to_organisation from app.dao.service_sms_sender_dao import dao_get_sms_senders_by_service_id from app.dao.services_dao import dao_remove_user_from_service +from app.dao.service_user_dao import dao_get_service_user from app.dao.templates_dao import dao_redact_template from app.dao.users_dao import save_model_user from app.models import ( @@ -38,6 +39,7 @@ from tests.app.db import ( create_service, create_service_with_inbound_number, create_template, + create_template_folder, create_notification, create_reply_to_email, create_letter_contact, @@ -1123,7 +1125,8 @@ def test_add_existing_user_to_another_service_with_all_permissions( {"permission": "manage_api_keys"}, {"permission": "manage_templates"}, {"permission": "view_activity"}, - ] + ], + "folder_permissions": [] } auth_header = create_authorization_header() @@ -1181,7 +1184,8 @@ def test_add_existing_user_to_another_service_with_send_permissions(notify_api, {"permission": "send_emails"}, {"permission": "send_letters"}, {"permission": "send_texts"}, - ] + ], + "folder_permissions": [] } auth_header = create_authorization_header() @@ -1254,6 +1258,47 @@ def test_add_existing_user_to_another_service_with_manage_permissions(notify_api assert sorted(expected_permissions) == sorted(permissions) +def test_add_existing_user_to_another_service_with_folder_permissions(notify_api, + notify_db, + notify_db_session, + sample_service, + sample_user): + with notify_api.test_request_context(): + with notify_api.test_client() as client: + # they must exist in db first + user_to_add = User( + name='Invited User', + email_address='invited@digital.cabinet-office.gov.uk', + password='password', + mobile_number='+4477123456' + ) + save_model_user(user_to_add) + + folder_1 = create_template_folder(sample_service) + folder_2 = create_template_folder(sample_service) + + data = { + "permissions": [{"permission": "manage_api_keys"}], + "folder_permissions": [str(folder_1.id), str(folder_2.id)] + } + + auth_header = create_authorization_header() + + resp = client.post( + '/service/{}/users/{}'.format(sample_service.id, user_to_add.id), + headers=[('Content-Type', 'application/json'), auth_header], + data=json.dumps(data) + ) + + assert resp.status_code == 201 + + new_user = dao_get_service_user(user_id=user_to_add.id, service_id=sample_service.id) + + assert len(new_user.folders) == 2 + assert folder_1 in new_user.folders + assert folder_2 in new_user.folders + + def test_add_existing_user_to_another_service_with_manage_api_keys(notify_api, notify_db, notify_db_session,