diff --git a/app/dao/users_dao.py b/app/dao/users_dao.py index 10d6f301e..c480263b5 100644 --- a/app/dao/users_dao.py +++ b/app/dao/users_dao.py @@ -60,7 +60,7 @@ def delete_model_user(user): def get_model_users(user_id=None): if user_id: - return User.query.filter_by(id=user_id).one() + return User.query.filter_by(id=user_id).first() return User.query.filter_by().all() diff --git a/app/user/rest.py b/app/user/rest.py index 6cf661c64..b1b807f81 100644 --- a/app/user/rest.py +++ b/app/user/rest.py @@ -2,7 +2,7 @@ from datetime import datetime from flask import (jsonify, request, abort, Blueprint, current_app) from sqlalchemy.exc import DataError from sqlalchemy.orm.exc import NoResultFound -from app.dao.services_dao import get_model_services +from app.dao.services_dao import dao_fetch_service_by_id_and_user from app.aws_sqs import add_notification_to_queue from app.dao.users_dao import ( get_model_users, @@ -40,29 +40,22 @@ def create_user(): return jsonify(data=user_schema.dump(user).data), 201 -@user.route('/', methods=['PUT', 'DELETE']) +@user.route('/', methods=['PUT']) def update_user(user_id): - try: - user = get_model_users(user_id=user_id) - except DataError: - return jsonify(result="error", message="Invalid user id"), 400 - except NoResultFound: + user = get_model_users(user_id=user_id) + if not user: return jsonify(result="error", message="User not found"), 404 - if request.method == 'DELETE': - status_code = 202 - delete_model_user(user) - else: - req_json = request.get_json() - update_dct, errors = user_schema_load_json.load(req_json) - pwd = req_json.get('password', None) - # TODO password validation, it is already done on the admin app - # but would be good to have the same validation here. - if pwd is not None and not pwd: - errors.update({'password': ['Invalid data for field']}) - if errors: - return jsonify(result="error", message=errors), 400 - status_code = 200 - save_model_user(user, update_dict=update_dct, pwd=pwd) + req_json = request.get_json() + update_dct, errors = user_schema_load_json.load(req_json) + pwd = req_json.get('password', None) + # TODO password validation, it is already done on the admin app + # but would be good to have the same validation here. + if pwd is not None and not pwd: + errors.update({'password': ['Invalid data for field']}) + if errors: + return jsonify(result="error", message=errors), 400 + status_code = 200 + save_model_user(user, update_dict=update_dct, pwd=pwd) return jsonify(data=user_schema.dump(user).data), status_code @@ -164,23 +157,3 @@ def get_user(user_id=None): return jsonify(result="error", message="User not found"), 404 result = users_schema.dump(users) if isinstance(users, list) else user_schema.dump(users) return jsonify(data=result.data) - - -@user.route('//service', methods=['GET']) -@user.route('//service/', methods=['GET']) -def get_service_by_user_id(user_id, service_id=None): - try: - user = get_model_users(user_id=user_id) - except DataError: - return jsonify(result="error", message="Invalid user id"), 400 - except NoResultFound: - return jsonify(result="error", message="User not found"), 404 - - try: - services = get_model_services(user_id=user.id, service_id=service_id) - except DataError: - return jsonify(result="error", message="Invalid service id"), 400 - except NoResultFound: - return jsonify(result="error", message="Service not found"), 404 - services, errors = services_schema.dump(services) if isinstance(services, list) else service_schema.dump(services) - return jsonify(data=services) diff --git a/tests/app/authentication/test_authentication.py b/tests/app/authentication/test_authentication.py index 18156f9e2..5381bd713 100644 --- a/tests/app/authentication/test_authentication.py +++ b/tests/app/authentication/test_authentication.py @@ -9,7 +9,7 @@ from app.models import ApiKey, Service def test_should_not_allow_request_with_no_token(notify_api): with notify_api.test_request_context(): with notify_api.test_client() as client: - response = client.get(url_for('service.get_service')) + response = client.get('/service') assert response.status_code == 401 data = json.loads(response.get_data()) assert data['error'] == 'Unauthorized, authentication token must be provided' @@ -18,89 +18,103 @@ def test_should_not_allow_request_with_no_token(notify_api): def test_should_not_allow_request_with_incorrect_header(notify_api): with notify_api.test_request_context(): with notify_api.test_client() as client: - response = client.get(url_for('service.get_service'), - headers={'Authorization': 'Basic 1234'}) + response = client.get( + '/service', + headers={'Authorization': 'Basic 1234'}) assert response.status_code == 401 data = json.loads(response.get_data()) assert data['error'] == 'Unauthorized, authentication bearer scheme must be used' -def test_should_not_allow_request_with_incorrect_token(notify_api, notify_db, notify_db_session, sample_api_key): +def test_should_not_allow_request_with_incorrect_token(notify_api, sample_user): with notify_api.test_request_context(): with notify_api.test_client() as client: - response = client.get(url_for('service.get_service'), - headers={'Authorization': 'Bearer 1234'}) + response = client.get( + '/service', + headers={'Authorization': 'Bearer 1234'}) assert response.status_code == 403 data = json.loads(response.get_data()) assert data['error'] == 'Invalid token: signature' -def test_should_not_allow_incorrect_path(notify_api, notify_db, notify_db_session, sample_api_key): +def test_should_not_allow_incorrect_path(notify_api, sample_api_key): with notify_api.test_request_context(): with notify_api.test_client() as client: - token = create_jwt_token(request_method="GET", - request_path="/bad", - secret=get_unsigned_secrets(sample_api_key.service_id)[0], - client_id=str(sample_api_key.service_id)) - response = client.get(url_for('service.get_service'), - headers={'Authorization': "Bearer {}".format(token)}) + token = create_jwt_token( + request_method="GET", + request_path="/bad", + secret=get_unsigned_secrets(sample_api_key.service_id)[0], + client_id=str(sample_api_key.service_id) + ) + response = client.get( + '/service', + headers={'Authorization': "Bearer {}".format(token)}) assert response.status_code == 403 data = json.loads(response.get_data()) assert data['error'] == 'Invalid token: request' -def test_should_not_allow_incorrect_method(notify_api, notify_db, notify_db_session, sample_api_key): +def test_should_not_allow_incorrect_method(notify_api, sample_api_key): with notify_api.test_request_context(): with notify_api.test_client() as client: token = __create_post_token(sample_api_key.service_id, {}) - response = client.get(url_for('service.get_service'), - headers={'Authorization': "Bearer {}".format(token)}) + response = client.get( + '/service', + headers={'Authorization': "Bearer {}".format(token)} + ) assert response.status_code == 403 data = json.loads(response.get_data()) assert data['error'] == 'Invalid token: request' -def test_should_not_allow_invalid_secret(notify_api, notify_db, notify_db_session, sample_api_key): +def test_should_not_allow_invalid_secret(notify_api, sample_api_key): with notify_api.test_request_context(): with notify_api.test_client() as client: - token = create_jwt_token(request_method="POST", request_path="/", secret="not-so-secret", - client_id=str(sample_api_key.service_id)) - response = client.get(url_for('service.get_service'), - headers={'Authorization': "Bearer {}".format(token)}) + token = create_jwt_token( + request_method="POST", + request_path="/service", + secret="not-so-secret", + client_id=str(sample_api_key.service_id)) + response = client.get( + '/service', + headers={'Authorization': "Bearer {}".format(token)} + ) assert response.status_code == 403 data = json.loads(response.get_data()) assert data['error'] == 'Invalid token: signature' -def test_should_allow_valid_token(notify_api, notify_db, notify_db_session, sample_api_key): +def test_should_allow_valid_token(notify_api, sample_api_key): with notify_api.test_request_context(): with notify_api.test_client() as client: token = __create_get_token(sample_api_key.service_id) - response = client.get(url_for('service.get_service', service_id=str(sample_api_key.service_id)), - headers={'Authorization': 'Bearer {}'.format(token)}) + response = client.get( + '/service/{}'.format(str(sample_api_key.service_id)), + headers={'Authorization': 'Bearer {}'.format(token)} + ) assert response.status_code == 200 -def test_should_allow_valid_token_for_request_with_path_params(notify_api, notify_db, notify_db_session, - sample_api_key): +def test_should_allow_valid_token_for_request_with_path_params(notify_api, sample_api_key): with notify_api.test_request_context(): with notify_api.test_client() as client: token = __create_get_token(sample_api_key.service_id) - response = client.get(url_for('service.get_service', service_id=str(sample_api_key.service_id)), - headers={'Authorization': 'Bearer {}'.format(token)}) + response = client.get( + '/service/{}'.format(str(sample_api_key.service_id)), + headers={'Authorization': 'Bearer {}'.format(token)}) assert response.status_code == 200 -def test_should_allow_valid_token_when_service_has_multiple_keys(notify_api, notify_db, notify_db_session, - sample_api_key): +def test_should_allow_valid_token_when_service_has_multiple_keys(notify_api, sample_api_key): with notify_api.test_request_context(): with notify_api.test_client() as client: data = {'service_id': sample_api_key.service_id, 'name': 'some key name'} api_key = ApiKey(**data) save_model_api_key(api_key) token = __create_get_token(sample_api_key.service_id) - response = client.get(url_for('service.get_service', service_id=str(sample_api_key.service_id)), - headers={'Authorization': 'Bearer {}'.format(token)}) + response = client.get( + '/service/{}'.format(str(sample_api_key.service_id)), + headers={'Authorization': 'Bearer {}'.format(token)}) assert response.status_code == 200 @@ -111,26 +125,28 @@ JSON_BODY = json.dumps({ }) -def test_should_allow_valid_token_with_post_body(notify_api, notify_db, notify_db_session, sample_api_key): +def test_should_allow_valid_token_with_post_body(notify_api, sample_api_key): with notify_api.test_request_context(): with notify_api.test_client() as client: service = Service.query.get(sample_api_key.service_id) - data = {'name': 'new name', - 'users': [service.users[0].id], - 'limit': 1000, - 'restricted': False, - 'active': False} + data = { + 'email_from': 'new name', + 'name': 'new name', + 'users': [service.users[0].id], + 'limit': 1000, + 'restricted': False, + 'active': False} token = create_jwt_token( - request_method="PUT", - request_path=url_for('service.update_service', service_id=str(sample_api_key.service_id)), + request_method="POST", + request_path='/service/{}'.format(str(sample_api_key.service_id)), secret=get_unsigned_secret(sample_api_key.id), client_id=str(sample_api_key.service_id), request_body=json.dumps(data) ) headers = [('Content-Type', 'application/json'), ('Authorization', 'Bearer {}'.format(token))] - response = client.put( - url_for('service.update_service', service_id=service.id), + response = client.post( + '/service/{}'.format(service.id), data=json.dumps(data), headers=headers) assert response.status_code == 200 @@ -140,9 +156,10 @@ def test_should_not_allow_valid_token_with_invalid_post_body(notify_api, notify_ with notify_api.test_request_context(): with notify_api.test_client() as client: token = __create_post_token(str(sample_api_key.service_id), JSON_BODY) - response = client.post(url_for('service.create_service'), - data="spurious", - headers={'Authorization': 'Bearer {}'.format(token)}) + response = client.post( + '/service', + data="spurious", + headers={'Authorization': 'Bearer {}'.format(token)}) assert response.status_code == 403 data = json.loads(response.get_data()) assert data['error'] == 'Invalid token: payload' @@ -154,19 +171,22 @@ def test_authentication_passes_admin_client_token(notify_api, sample_api_key): with notify_api.test_request_context(): with notify_api.test_client() as client: - token = create_jwt_token(request_method="GET", - request_path=url_for('service.get_service'), - secret=current_app.config.get('ADMIN_CLIENT_SECRET'), - client_id=current_app.config.get('ADMIN_CLIENT_USER_NAME')) - response = client.get(url_for('service.get_service'), - headers={'Authorization': 'Bearer {}'.format(token)}) + token = create_jwt_token( + request_method="GET", + request_path='/service', + secret=current_app.config.get('ADMIN_CLIENT_SECRET'), + client_id=current_app.config.get('ADMIN_CLIENT_USER_NAME')) + response = client.get( + '/service', + headers={'Authorization': 'Bearer {}'.format(token)}) assert response.status_code == 200 -def test_authentication_passes_when_service_has_multiple_keys_some_expired(notify_api, - notify_db, - notify_db_session, - sample_api_key): +def test_authentication_passes_when_service_has_multiple_keys_some_expired( + notify_api, + notify_db, + notify_db_session, + sample_api_key): with notify_api.test_request_context(): with notify_api.test_client() as client: exprired_key = {'service_id': sample_api_key.service_id, 'name': 'expired_key', @@ -176,12 +196,14 @@ def test_authentication_passes_when_service_has_multiple_keys_some_expired(notif another_key = {'service_id': sample_api_key.service_id, 'name': 'another_key'} api_key = ApiKey(**another_key) save_model_api_key(api_key) - token = create_jwt_token(request_method="GET", - request_path=url_for('service.get_service'), - secret=get_unsigned_secret(api_key.id), - client_id=str(sample_api_key.service_id)) - response = client.get(url_for('service.get_service'), - headers={'Authorization': 'Bearer {}'.format(token)}) + token = create_jwt_token( + request_method="GET", + request_path='/service', + secret=get_unsigned_secret(api_key.id), + client_id=str(sample_api_key.service_id)) + response = client.get( + '/service', + headers={'Authorization': 'Bearer {}'.format(token)}) assert response.status_code == 200 @@ -197,18 +219,20 @@ def test_authentication_returns_token_expired_when_service_uses_expired_key_and_ another_key = {'service_id': sample_api_key.service_id, 'name': 'another_key'} api_key = ApiKey(**another_key) save_model_api_key(api_key) - token = create_jwt_token(request_method="GET", - request_path=url_for('service.get_service'), - secret=get_unsigned_secret(expired_api_key.id), - client_id=str(sample_api_key.service_id)) + token = create_jwt_token( + request_method="GET", + request_path='/service', + secret=get_unsigned_secret(expired_api_key.id), + client_id=str(sample_api_key.service_id)) # expire the key expire_the_key = {'id': expired_api_key.id, 'service_id': str(sample_api_key.service_id), 'name': 'expired_key', 'expiry_date': datetime.now() + timedelta(hours=-2)} save_model_api_key(expired_api_key, expire_the_key) - response = client.get(url_for('service.get_service'), - headers={'Authorization': 'Bearer {}'.format(token)}) + response = client.get( + '/service', + headers={'Authorization': 'Bearer {}'.format(token)}) assert response.status_code == 403 data = json.loads(response.get_data()) assert data['error'] == 'Invalid token: signature' @@ -220,14 +244,16 @@ def test_authentication_returns_error_when_api_client_has_no_secrets(notify_api, with notify_api.test_request_context(): with notify_api.test_client() as client: api_secret = notify_api.config.get('ADMIN_CLIENT_SECRET') - token = create_jwt_token(request_method="GET", - request_path=url_for('service.get_service'), - secret=api_secret, - client_id=notify_api.config.get('ADMIN_CLIENT_USER_NAME') - ) + token = create_jwt_token( + request_method="GET", + request_path='/service', + secret=api_secret, + client_id=notify_api.config.get('ADMIN_CLIENT_USER_NAME') + ) notify_api.config['ADMIN_CLIENT_SECRET'] = '' - response = client.get(url_for('service.get_service'), - headers={'Authorization': 'Bearer {}'.format(token)}) + response = client.get( + '/service', + headers={'Authorization': 'Bearer {}'.format(token)}) assert response.status_code == 403 error_message = json.loads(response.get_data()) assert error_message['error'] == 'Invalid token: signature' @@ -237,12 +263,12 @@ def test_authentication_returns_error_when_api_client_has_no_secrets(notify_api, def __create_get_token(service_id): if service_id: return create_jwt_token(request_method="GET", - request_path=url_for('service.get_service', service_id=service_id), + request_path='/service/{}'.format(service_id), secret=get_unsigned_secrets(service_id)[0], client_id=str(service_id)) else: return create_jwt_token(request_method="GET", - request_path=url_for('service.get_service'), + request_path='/service', secret=get_unsigned_secrets(service_id)[0], client_id=service_id) @@ -250,7 +276,7 @@ def __create_get_token(service_id): def __create_post_token(service_id, request_body): return create_jwt_token( request_method="POST", - request_path=url_for('service.create_service'), + request_path='/service', secret=get_unsigned_secrets(service_id)[0], client_id=str(service_id), request_body=request_body diff --git a/tests/app/conftest.py b/tests/app/conftest.py index c3ba5c07b..8cfb1b8d3 100644 --- a/tests/app/conftest.py +++ b/tests/app/conftest.py @@ -2,7 +2,7 @@ import pytest from app.models import (User, Service, Template, ApiKey, Job, Notification) from app.dao.users_dao import (save_model_user, create_user_code, create_secret_code) -from app.dao.services_dao import save_model_service +from app.dao.services_dao import dao_create_service from app.dao.templates_dao import save_model_template from app.dao.api_key_dao import save_model_api_key from app.dao.jobs_dao import save_job @@ -13,8 +13,9 @@ import uuid @pytest.fixture(scope='function') def service_factory(notify_db, notify_db_session): class ServiceFactory(object): - def get(self, service_name): - user = sample_user(notify_db, notify_db_session) + def get(self, service_name, user=None): + if not user: + user = sample_user(notify_db, notify_db_session) service = sample_service(notify_db, notify_db_session, service_name, user) sample_template(notify_db, notify_db_session, service=service) return service @@ -90,11 +91,13 @@ def sample_service(notify_db, 'users': [user], 'limit': 1000, 'active': False, - 'restricted': False} + 'restricted': False, + 'email_from': service_name + } service = Service.query.filter_by(name=service_name).first() if not service: service = Service(**data) - save_model_service(service) + dao_create_service(service, user) return service @@ -104,6 +107,7 @@ def sample_template(notify_db, template_name="Template Name", template_type="sms", content="This is a template", + subject_line=None, service=None): if service is None: service = sample_service(notify_db, notify_db_session) @@ -114,6 +118,10 @@ def sample_template(notify_db, 'content': content, 'service': service } + if subject_line: + data.update({ + 'subject': subject_line + }) template = Template(**data) save_model_template(template) return template diff --git a/tests/app/user/test_rest.py b/tests/app/user/test_rest.py index 008da0558..e9153d8a3 100644 --- a/tests/app/user/test_rest.py +++ b/tests/app/user/test_rest.py @@ -248,15 +248,18 @@ def test_put_user_not_exists(notify_api, notify_db, notify_db_session, sample_us assert User.query.count() == 2 new_email = 'new@digital.cabinet-office.gov.uk' data = {'email_address': new_email} - auth_header = create_authorization_header(service_id=sample_admin_service_id, - path=url_for('user.update_user', user_id="9999"), - method='PUT', - request_body=json.dumps(data)) + auth_header = create_authorization_header( + service_id=sample_admin_service_id, + path=url_for('user.update_user', user_id="9999"), + method='PUT', + request_body=json.dumps(data)) headers = [('Content-Type', 'application/json'), auth_header] resp = client.put( url_for('user.update_user', user_id="9999"), data=json.dumps(data), headers=headers) + json_resp = json.loads(resp.get_data(as_text=True)) + print(json_resp) assert resp.status_code == 404 assert User.query.count() == 2 user = User.query.filter_by(id=sample_user.id).first() @@ -264,146 +267,3 @@ def test_put_user_not_exists(notify_api, notify_db, notify_db_session, sample_us assert json_resp == {'result': 'error', 'message': 'User not found'} assert user == sample_user assert user.email_address != new_email - - -def test_get_user_services(notify_api, notify_db, notify_db_session, sample_service, sample_admin_service_id): - """ - Tests GET endpoint "//service/" to retrieve services for a user. - """ - with notify_api.test_request_context(): - with notify_api.test_client() as client: - user = User.query.get(sample_service.users[0].id) - another_name = "another name" - create_sample_service( - notify_db, - notify_db_session, - service_name=another_name, - user=user) - assert Service.query.count() == 3 - auth_header = create_authorization_header(service_id=sample_admin_service_id, - path=url_for('user.get_service_by_user_id', user_id=user.id), - method='GET') - resp = client.get( - url_for('user.get_service_by_user_id', user_id=user.id), - headers=[('Content-Type', 'application/json'), auth_header]) - assert resp.status_code == 200 - json_resp = json.loads(resp.get_data(as_text=True)) - assert len(json_resp['data']) == 2 - - -def test_get_user_service(notify_api, notify_db, notify_db_session, sample_service, sample_admin_service_id): - """ - Tests GET endpoint "//service/" to retrieve a service for a user. - """ - with notify_api.test_request_context(): - with notify_api.test_client() as client: - user = User.query.first() - another_name = "another name" - another_service = create_sample_service( - notify_db, - notify_db_session, - service_name=another_name, - user=user) - assert Service.query.count() == 3 - auth_header = create_authorization_header(service_id=sample_admin_service_id, - path=url_for('user.get_service_by_user_id', user_id=user.id, - service_id=another_service.id), - method='GET') - resp = client.get( - url_for('user.get_service_by_user_id', user_id=user.id, service_id=another_service.id), - headers=[('Content-Type', 'application/json'), auth_header]) - assert resp.status_code == 200 - json_resp = json.loads(resp.get_data(as_text=True)) - assert json_resp['data']['name'] == another_name - assert json_resp['data']['id'] == str(another_service.id) - - -def test_get_user_service_user_not_exists(notify_api, notify_db, notify_db_session, sample_service, - sample_admin_service_id): - """ - Tests GET endpoint "//service/" 404 is returned for invalid user. - """ - with notify_api.test_request_context(): - with notify_api.test_client() as client: - assert Service.query.count() == 2 - auth_header = create_authorization_header(service_id=sample_admin_service_id, - path=url_for('user.get_service_by_user_id', user_id="123423", - service_id=sample_service.id), - method='GET') - print('** service users{}'.format(sample_service.users[0].id)) - resp = client.get( - url_for('user.get_service_by_user_id', user_id="123423", service_id=sample_service.id), - headers=[('Content-Type', 'application/json'), auth_header]) - assert resp.status_code == 404 - json_resp = json.loads(resp.get_data(as_text=True)) - assert "User not found" in json_resp['message'] - - -def test_get_user_service_service_not_exists(notify_api, notify_db, notify_db_session, sample_service, - sample_admin_service_id): - """ - Tests GET endpoint "//service/" 404 is returned for invalid service. - """ - with notify_api.test_request_context(): - with notify_api.test_client() as client: - user = User.query.first() - assert Service.query.count() == 2 - import uuid - missing_service_id = uuid.uuid4() - auth_header = create_authorization_header(service_id=sample_admin_service_id, - path=url_for('user.get_service_by_user_id', user_id=user.id, - service_id=missing_service_id), - method='GET') - resp = client.get( - url_for('user.get_service_by_user_id', user_id=user.id, service_id=missing_service_id), - headers=[('Content-Type', 'application/json'), auth_header]) - assert resp.status_code == 404 - json_resp = json.loads(resp.get_data(as_text=True)) - assert "Service not found" in json_resp['message'] - - -def test_delete_user(notify_api, notify_db, notify_db_session, sample_user, sample_admin_service_id): - """ - Tests DELETE endpoint '/' delete user. - """ - with notify_api.test_request_context(): - with notify_api.test_client() as client: - assert User.query.count() == 2 - auth_header = create_authorization_header(service_id=sample_admin_service_id, - path=url_for('user.update_user', user_id=sample_user.id), - method='DELETE') - resp = client.delete( - url_for('user.update_user', user_id=sample_user.id), - headers=[('Content-Type', 'application/json'), auth_header]) - assert resp.status_code == 202 - json_resp = json.loads(resp.get_data(as_text=True)) - assert User.query.count() == 1 - expected = { - "name": "Test User", - "email_address": sample_user.email_address, - "mobile_number": "+447700900986", - "password_changed_at": None, - "id": sample_user.id, - "logged_in_at": None, - "state": "active", - "failed_login_count": 0 - - } - assert json_resp['data'] == expected - - -def test_delete_user_not_exists(notify_api, notify_db, notify_db_session, sample_user, sample_admin_service_id): - """ - Tests DELETE endpoint '/' delete user. - """ - with notify_api.test_request_context(): - with notify_api.test_client() as client: - assert User.query.count() == 2 - auth_header = create_authorization_header(service_id=sample_admin_service_id, - path=url_for('user.update_user', user_id='99999'), - method='DELETE') - resp = client.delete( - url_for('user.update_user', user_id="99999"), - headers=[('Content-Type', 'application/json'), auth_header]) - assert resp.status_code == 404 - assert User.query.count() == 2