diff --git a/app/main/views/letters.py b/app/main/views/letters.py index 0458493ea..4c8e1bd12 100644 --- a/app/main/views/letters.py +++ b/app/main/views/letters.py @@ -8,7 +8,7 @@ from app.utils import user_has_permissions @main.route("/services//letters") @login_required -@user_has_permissions('manage_templates', admin_override=True) +@user_has_permissions('manage_templates', 'send_letters', admin_override=True, any_=True) def letters(service_id): if not current_service['can_send_letters']: abort(403) diff --git a/tests/app/main/views/test_letters.py b/tests/app/main/views/test_letters.py index c2c6451a6..8eef4599a 100644 --- a/tests/app/main/views/test_letters.py +++ b/tests/app/main/views/test_letters.py @@ -15,3 +15,42 @@ def test_letters_access_restricted(logged_in_client, mocker, can_send_letters, r response = logged_in_client.get(url_for('main.letters', service_id=service['id'])) assert response.status_code == response_code + + +@pytest.mark.parametrize('permission', [ + 'send_letters', + 'manage_templates' +]) +def test_letters_lets_in_with_permissions( + client, + mocker, + mock_login, + mock_has_permissions, + api_user_active, + permission, +): + service = service_json(can_send_letters=True) + mocker.patch('app.service_api_client.get_service', return_value={"data": service}) + + api_user_active._permissions[str(service['id'])] = [permission] + + client.login(api_user_active) + response = client.get(url_for('main.letters', service_id=service['id'])) + + assert response.status_code == 200 + + +def test_letters_rejects_without_permissions( + client, + mocker, + mock_login, + mock_has_permissions, + api_user_active, +): + service = service_json(can_send_letters=True) + mocker.patch('app.service_api_client.get_service', return_value={"data": service}) + + client.login(api_user_active) + response = client.get(url_for('main.letters', service_id=service['id'])) + + assert response.status_code == 200 diff --git a/tests/app/main/views/test_service_settings.py b/tests/app/main/views/test_service_settings.py index 924cce7a2..60ff51134 100644 --- a/tests/app/main/views/test_service_settings.py +++ b/tests/app/main/views/test_service_settings.py @@ -475,75 +475,76 @@ def test_should_redirect_delete_confirmation(app_, assert mock_delete_service.called -def test_route_permissions(mocker, app_, api_user_active, service_one, mock_get_organisation): - routes = [ - 'main.service_settings', - 'main.service_name_change', - 'main.service_name_change_confirm', - 'main.service_request_to_go_live', - 'main.service_delete', - 'main.service_delete_confirm'] +@pytest.mark.parametrize('route', [ + 'main.service_settings', + 'main.service_name_change', + 'main.service_name_change_confirm', + 'main.service_request_to_go_live', + 'main.service_delete', + 'main.service_delete_confirm' +]) +def test_route_permissions(mocker, app_, api_user_active, service_one, route): with app_.test_request_context(): - for route in routes: - validate_route_permission( - mocker, - app_, - "GET", - 200, - url_for(route, service_id=service_one['id']), - ['manage_settings'], - api_user_active, - service_one) + validate_route_permission( + mocker, + app_, + "GET", + 200, + url_for(route, service_id=service_one['id']), + ['manage_settings'], + api_user_active, + service_one) -def test_route_invalid_permissions(mocker, app_, api_user_active, service_one, mock_get_organisation): - routes = [ - 'main.service_settings', - 'main.service_name_change', - 'main.service_name_change_confirm', - 'main.service_request_to_go_live', - 'main.service_switch_live', - 'main.service_switch_research_mode', - 'main.service_delete', - 'main.service_delete_confirm'] +@pytest.mark.parametrize('route', [ + 'main.service_settings', + 'main.service_name_change', + 'main.service_name_change_confirm', + 'main.service_request_to_go_live', + 'main.service_switch_live', + 'main.service_switch_research_mode', + 'main.service_switch_can_send_letters', + 'main.service_delete', + 'main.service_delete_confirm' +]) +def test_route_invalid_permissions(mocker, app_, api_user_active, service_one, route): with app_.test_request_context(): - for route in routes: - validate_route_permission( - mocker, - app_, - "GET", - 403, - url_for(route, service_id=service_one['id']), - ['blah'], - api_user_active, - service_one) + validate_route_permission( + mocker, + app_, + "GET", + 403, + url_for(route, service_id=service_one['id']), + ['blah'], + api_user_active, + service_one) -def test_route_for_platform_admin(mocker, app_, platform_admin_user, service_one, mock_get_organisation): - routes = [ - 'main.service_settings', - 'main.service_name_change', - 'main.service_name_change_confirm', - 'main.service_request_to_go_live', - 'main.service_delete', - 'main.service_delete_confirm' - ] +@pytest.mark.parametrize('route', [ + 'main.service_settings', + 'main.service_name_change', + 'main.service_name_change_confirm', + 'main.service_request_to_go_live', + 'main.service_delete', + 'main.service_delete_confirm' +]) +def test_route_for_platform_admin(mocker, app_, platform_admin_user, service_one, route): with app_.test_request_context(): - for route in routes: - validate_route_permission(mocker, - app_, - "GET", - 200, - url_for(route, service_id=service_one['id']), - [], - platform_admin_user, - service_one) + validate_route_permission(mocker, + app_, + "GET", + 200, + url_for(route, service_id=service_one['id']), + [], + platform_admin_user, + service_one) def test_route_for_platform_admin_update_service(mocker, app_, platform_admin_user, service_one): routes = [ 'main.service_switch_live', - 'main.service_switch_research_mode' + 'main.service_switch_research_mode', + 'main.service_switch_can_send_letters' ] with app_.test_request_context(): for route in routes: @@ -817,22 +818,24 @@ def test_should_set_branding_and_organisations( ) -def test_switch_service_enable_letters(logged_in_client, service_one, mocker): +def test_switch_service_enable_letters(client, platform_admin_user, service_one, mocker): mocked_fn = mocker.patch('app.service_api_client.update_service_with_properties', return_value=service_one) - response = logged_in_client.get(url_for('main.service_switch_can_send_letters', service_id=service_one['id'])) + client.login(platform_admin_user, mocker, service_one) + response = client.get(url_for('main.service_switch_can_send_letters', service_id=service_one['id'])) assert response.status_code == 302 assert response.location == url_for('main.service_settings', service_id=service_one['id'], _external=True) assert mocked_fn.call_args == call(service_one['id'], {'can_send_letters': True}) -def test_switch_service_disable_letters(logged_in_client, mocker): +def test_switch_service_disable_letters(client, platform_admin_user, mocker): service = service_json("1234", "Test Service", [], can_send_letters=True) mocker.patch('app.service_api_client.get_service', return_value={"data": service}) mocked_fn = mocker.patch('app.service_api_client.update_service_with_properties', return_value=service) - response = logged_in_client.get(url_for('main.service_switch_can_send_letters', service_id=service['id'])) + client.login(platform_admin_user, mocker, service) + response = client.get(url_for('main.service_switch_can_send_letters', service_id=service['id'])) assert response.status_code == 302 assert response.location == url_for('main.service_settings', service_id=service['id'], _external=True)