From 2486c17dc9d48266e834fda36590560a2a0d28bd Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Thu, 21 Jan 2016 16:53:53 +0000 Subject: [PATCH 1/4] Add unique constraint for api_key on service_id and name --- app/models.py | 8 ++++-- .../0008_unique_service_to_key_name.py | 26 +++++++++++++++++++ tests/app/dao/test_api_key_dao.py | 13 ++++++++++ 3 files changed, 45 insertions(+), 2 deletions(-) create mode 100644 migrations/versions/0008_unique_service_to_key_name.py diff --git a/app/models.py b/app/models.py index 4721cdb62..c42e972c8 100644 --- a/app/models.py +++ b/app/models.py @@ -1,6 +1,7 @@ +from sqlalchemy import UniqueConstraint + from . import db import datetime - from sqlalchemy.dialects.postgresql import UUID from app.encryption import ( hashpw, @@ -95,6 +96,10 @@ class ApiKey(db.Model): service = db.relationship('Service', backref=db.backref('api_keys', lazy='dynamic')) expiry_date = db.Column(db.DateTime) + __table_args__ = ( + UniqueConstraint('service_id', 'name', name='uix_service_to_key_name'), + ) + TEMPLATE_TYPES = ['sms', 'email', 'letter'] @@ -123,7 +128,6 @@ class Template(db.Model): class Job(db.Model): - __tablename__ = 'jobs' id = db.Column(UUID(as_uuid=True), primary_key=True) diff --git a/migrations/versions/0008_unique_service_to_key_name.py b/migrations/versions/0008_unique_service_to_key_name.py new file mode 100644 index 000000000..6226b23c0 --- /dev/null +++ b/migrations/versions/0008_unique_service_to_key_name.py @@ -0,0 +1,26 @@ +"""empty message + +Revision ID: 0008_unique_service_to_key_name +Revises: 0007_change_to_api_keys +Create Date: 2016-01-21 16:14:51.773001 + +""" + +# revision identifiers, used by Alembic. +revision = '0008_unique_service_to_key_name' +down_revision = '0007_change_to_api_keys' + +from alembic import op +import sqlalchemy as sa + + +def upgrade(): + ### commands auto generated by Alembic - please adjust! ### + op.create_unique_constraint('uix_service_to_key_name', 'api_key', ['service_id', 'name']) + ### end Alembic commands ### + + +def downgrade(): + ### commands auto generated by Alembic - please adjust! ### + op.drop_constraint('uix_service_to_key_name', 'api_key', type_='unique') + ### end Alembic commands ### diff --git a/tests/app/dao/test_api_key_dao.py b/tests/app/dao/test_api_key_dao.py index 994b863e7..fbb33e517 100644 --- a/tests/app/dao/test_api_key_dao.py +++ b/tests/app/dao/test_api_key_dao.py @@ -73,3 +73,16 @@ def test_get_unsigned_secret_returns_key(notify_api, unsigned_api_key = get_unsigned_secret(sample_api_key.id) assert sample_api_key.secret != unsigned_api_key assert unsigned_api_key == _get_secret(sample_api_key.secret) + + +def test_should_not_allow_duplicate_key_names_per_service(notify_api, + notify_db, + notify_db_session, + sample_api_key): + api_key = ApiKey( + **{'id': sample_api_key.id + 1, 'service_id': sample_api_key.service_id, 'name': sample_api_key.name}) + try: + save_model_api_key(api_key) + fail("should throw IntegrityError") + except: + pass From 8fa1cac1c64cc3d92dfbe6708d1e2b478ca86a7c Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Fri, 22 Jan 2016 09:59:02 +0000 Subject: [PATCH 2/4] Fix the user url. Add test for authentication to test paths with path params --- app/user/rest.py | 2 +- .../app/authentication/test_authentication.py | 28 +++++++++++++++---- 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/app/user/rest.py b/app/user/rest.py index 71f3327b7..1bd7e8b4e 100644 --- a/app/user/rest.py +++ b/app/user/rest.py @@ -71,7 +71,7 @@ def verify_user_password(user_id): @user.route('/', methods=['GET']) -@user.route('/', methods=['GET']) +@user.route('', methods=['GET']) def get_user(user_id=None): try: users = get_model_users(user_id=user_id) diff --git a/tests/app/authentication/test_authentication.py b/tests/app/authentication/test_authentication.py index 578dfc169..537dd750a 100644 --- a/tests/app/authentication/test_authentication.py +++ b/tests/app/authentication/test_authentication.py @@ -76,7 +76,17 @@ def test_should_allow_valid_token(notify_api, notify_db, notify_db_session, samp 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'), + response = client.get(url_for('service.get_service', service_id=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): + 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=sample_api_key.service_id), headers={'Authorization': 'Bearer {}'.format(token)}) assert response.status_code == 200 @@ -89,7 +99,7 @@ def test_should_allow_valid_token_when_service_has_multiple_keys(notify_api, not 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'), + response = client.get(url_for('service.get_service', service_id=sample_api_key.service_id), headers={'Authorization': 'Bearer {}'.format(token)}) assert response.status_code == 200 @@ -205,10 +215,16 @@ def test_authentication_returns_token_expired_when_service_uses_expired_key_and_ def __create_get_token(service_id): - return create_jwt_token(request_method="GET", - request_path=url_for('service.get_service'), - secret=get_unsigned_secrets(service_id)[0], - client_id=service_id) + if service_id: + return create_jwt_token(request_method="GET", + request_path=url_for('service.get_service', service_id=service_id), + secret=get_unsigned_secrets(service_id)[0], + client_id=service_id) + else: + return create_jwt_token(request_method="GET", + request_path=url_for('service.get_service'), + secret=get_unsigned_secrets(service_id)[0], + client_id=service_id) def __create_post_token(service_id, request_body): From e657958af492ac32117ba7b51d6cc2a5446fd3a7 Mon Sep 17 00:00:00 2001 From: Nicholas Staples Date: Fri, 22 Jan 2016 10:44:34 +0000 Subject: [PATCH 3/4] Templates fix with tests working. --- app/dao/templates_dao.py | 2 +- app/service/rest.py | 20 +++++++++++++- tests/app/service/test_rest.py | 48 ++++++++++++++++++++++++++++++++++ 3 files changed, 68 insertions(+), 2 deletions(-) diff --git a/app/dao/templates_dao.py b/app/dao/templates_dao.py index fa5ce2270..b6b7f6a8d 100644 --- a/app/dao/templates_dao.py +++ b/app/dao/templates_dao.py @@ -30,5 +30,5 @@ def get_model_templates(template_id=None, service_id=None): elif template_id: return Template.query.filter_by(id=template_id).one() elif service_id: - return Template.query.filter_by(service=Service.get(service_id)).one() + return Template.query.filter_by(service=Service.query.get(service_id)).all() return Template.query.all() diff --git a/app/service/rest.py b/app/service/rest.py index eddab42bb..775410797 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -13,7 +13,7 @@ from app.dao.templates_dao import ( from app.dao.api_key_dao import (save_model_api_key, get_model_api_keys, get_unsigned_secret) from app.models import ApiKey from app.schemas import ( - services_schema, service_schema, template_schema, api_keys_schema) + services_schema, service_schema, template_schema, templates_schema, api_keys_schema) from flask import Blueprint service = Blueprint('service', __name__) @@ -183,3 +183,21 @@ def update_template(service_id, template_id): except DAOException as e: return jsonify(result="error", message=str(e)), 400 return jsonify(data=template_schema.dump(template).data), status_code + + +@service.route('//template/', methods=['GET']) +@service.route('//template', methods=['GET']) +def get_service_template(service_id, template_id=None): + try: + templates = get_model_templates(service_id=service_id, template_id=template_id) + except DataError: + return jsonify(result="error", message="Invalid template id"), 400 + except NoResultFound: + return jsonify(result="error", message="Template not found"), 404 + if isinstance(templates, list): + data, errors = templates_schema.dump(templates) + else: + data, errors = template_schema.dump(templates) + if errors: + return jsonify(result="error", message=str(errors)) + return jsonify(data=data) diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index f5d00a7e5..e12853a14 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -547,3 +547,51 @@ def test_create_template_unicode_content(notify_api, notify_db, notify_db_sessio assert json_resp['data']['name'] == template_name assert json_resp['data']['template_type'] == template_type assert json_resp['data']['content'] == template_content + + +def test_get_template_list(notify_api, notify_db, notify_db_session, sample_template): + """ + Tests GET endpoint '/' to retrieve entire template list. + """ + with notify_api.test_request_context(): + with notify_api.test_client() as client: + auth_header = create_authorization_header( + service_id=sample_template.service_id, + path=url_for( + 'service.get_service_template', + service_id=sample_template.service_id), + method='GET') + response = client.get( + url_for( + 'service.get_service_template', + service_id=sample_template.service_id), + headers=[auth_header]) + assert response.status_code == 200 + json_resp = json.loads(response.get_data(as_text=True)) + assert len(json_resp['data']) == 1 + assert json_resp['data'][0]['name'] == sample_template.name + assert json_resp['data'][0]['id'] == sample_template.id + + +def test_get_template(notify_api, notify_db, notify_db_session, sample_template): + """ + Tests GET endpoint '/' to retrieve a single template. + """ + with notify_api.test_request_context(): + with notify_api.test_client() as client: + auth_header = create_authorization_header( + service_id=sample_template.service_id, + path=url_for( + 'service.get_service_template', + template_id=sample_template.id, + service_id=sample_template.service_id), + method='GET') + resp = client.get(url_for( + 'service.get_service_template', + template_id=sample_template.id, + service_id=sample_template.service_id), + headers=[auth_header]) + assert resp.status_code == 200 + json_resp = json.loads(resp.get_data(as_text=True)) + assert json_resp['data']['name'] == sample_template.name + assert json_resp['data']['id'] == sample_template.id From 5a937d6e712b1039630aebfaf757dac3fca205b6 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Fri, 22 Jan 2016 12:47:59 +0000 Subject: [PATCH 4/4] Added user_id as a query param for get_services. Need to add this query param for the services page on the admin app. Do not add the query param to path in the token. --- app/service/rest.py | 3 ++- tests/app/dao/test_services_dao.py | 18 ++++++++++++++++++ tests/app/service/test_rest.py | 21 +++++++++++++++++++++ 3 files changed, 41 insertions(+), 1 deletion(-) diff --git a/app/service/rest.py b/app/service/rest.py index 775410797..c114b5555 100644 --- a/app/service/rest.py +++ b/app/service/rest.py @@ -65,8 +65,9 @@ def update_service(service_id): @service.route('/', methods=['GET']) @service.route('', methods=['GET']) def get_service(service_id=None): + user_id = request.args.get('user_id', None) try: - services = get_model_services(service_id=service_id) + services = get_model_services(service_id=service_id, user_id=user_id) except DataError: return jsonify(result="error", message="Invalid service id"), 400 except NoResultFound: diff --git a/tests/app/dao/test_services_dao.py b/tests/app/dao/test_services_dao.py index 66c377e4d..ab5655025 100644 --- a/tests/app/dao/test_services_dao.py +++ b/tests/app/dao/test_services_dao.py @@ -2,6 +2,7 @@ import pytest from app.dao.services_dao import ( save_model_service, get_model_services, DAOException, delete_model_service) from tests.app.conftest import sample_service as create_sample_service +from tests.app.conftest import sample_user as create_sample_user from app.models import Service @@ -47,6 +48,23 @@ def test_get_user_service(notify_api, notify_db, notify_db_session, sample_user) assert Service.query.count() == 1 +def test_get_services_for_user(notify_api, notify_db, notify_db_session, sample_service): + assert Service.query.count() == 1 + service_name = "Random service" + second_user = create_sample_user(notify_db, notify_db_session, 'an@other.gov.uk') + create_sample_service(notify_db, notify_db_session, service_name='another service', user=second_user) + + sample_service = create_sample_service(notify_db, + notify_db_session, + service_name=service_name, + user=sample_service.users[0]) + assert Service.query.count() == 3 + services = get_model_services(user_id=sample_service.users[0].id) + assert len(services) == 2 + assert service_name in [x.name for x in services] + assert 'Sample service' in [x.name for x in services] + + def test_missing_user_attribute(notify_api, notify_db, notify_db_session): assert Service.query.count() == 0 try: diff --git a/tests/app/service/test_rest.py b/tests/app/service/test_rest.py index e12853a14..6c2b2843e 100644 --- a/tests/app/service/test_rest.py +++ b/tests/app/service/test_rest.py @@ -6,6 +6,7 @@ from app.dao.services_dao import save_model_service from app.models import (Service, ApiKey, Template) from tests import create_authorization_header from tests.app.conftest import sample_user as create_sample_user +from tests.app.conftest import sample_service as create_sample_service def test_get_service_list(notify_api, notify_db, notify_db_session, sample_service, sample_admin_service_id): @@ -45,6 +46,26 @@ def test_get_service(notify_api, notify_db, notify_db_session, sample_service, s assert json_resp['data']['id'] == sample_service.id +def test_get_service_for_user(notify_api, notify_db, notify_db_session, sample_service): + second_user = create_sample_user(notify_db, notify_db_session, 'an@other.gov.uk') + create_sample_service(notify_db, notify_db_session, service_name='Second Service', user=second_user) + create_sample_service(notify_db, notify_db_session, service_name='Another Service', user=sample_service.users[0]) + with notify_api.test_request_context(): + with notify_api.test_client() as client: + auth_header = create_authorization_header( + path='/service', + method='GET') + resp = client.get('/service?user_id={}'.format(sample_service.users[0].id), + headers=[auth_header]) + assert resp.status_code == 200 + json_resp = json.loads(resp.get_data(as_text=True)) + assert len(json_resp['data']) == 2 + print(x for x in json_resp['data']) + assert 'Another Service' in [x.get('name') for x in json_resp['data']] + assert 'Sample service' in [x.get('name') for x in json_resp['data']] + assert 'Second Service' not in [x.get('name') for x in json_resp['data']] + + def test_post_service(notify_api, notify_db, notify_db_session, sample_user, sample_admin_service_id): """ Tests POST endpoint '/' to create a service.