From e8c3a5cddef6b9887b5aa771dbb121a6a1a0c3de Mon Sep 17 00:00:00 2001 From: Leo Hemsted Date: Thu, 10 Nov 2016 11:07:12 +0000 Subject: [PATCH] add check for inactive services to auth handler cleaned up some auth code to marginally improve efficiency of error checking and hopefully make it easier to read fixed some incorrect auth headers in the deactivate tests --- app/authentication/auth.py | 22 ++++++++++--------- app/models.py | 2 +- .../app/authentication/test_authentication.py | 11 ++++++++++ tests/app/service/test_deactivate.py | 10 ++++----- 4 files changed, 29 insertions(+), 16 deletions(-) diff --git a/app/authentication/auth.py b/app/authentication/auth.py index 841222279..68e7acf8b 100644 --- a/app/authentication/auth.py +++ b/app/authentication/auth.py @@ -46,10 +46,19 @@ def requires_auth(): return handle_admin_key(auth_token, current_app.config.get('ADMIN_CLIENT_SECRET')) try: - api_keys = get_model_api_keys(client) + service = dao_fetch_service_by_id(client) except DataError: raise AuthError("Invalid token: service id is not the right data type", 403) - for api_key in api_keys: + except NoResultFound: + raise AuthError("Invalid token: service not found", 403) + + if not service.api_keys: + raise AuthError("Invalid token: service has no API keys", 403) + + if not service.active: + raise AuthError("Invalid token: service is archived", 403) + + for api_key in service.api_keys: try: get_decode_errors(auth_token, api_key.unsigned_secret) except TokenDecodeError: @@ -60,15 +69,8 @@ def requires_auth(): _request_ctx_stack.top.api_user = api_key return - - try: - dao_fetch_service_by_id(client) - except NoResultFound: - raise AuthError("Invalid token: service not found", 403) - - if not api_keys: - raise AuthError("Invalid token: service has no API keys", 403) else: + # service has API keys, but none matching the one the user provided raise AuthError("Invalid token: signature, api token is not valid", 403) diff --git a/app/models.py b/app/models.py index cf46530ca..75b9615fc 100644 --- a/app/models.py +++ b/app/models.py @@ -188,7 +188,7 @@ class ApiKey(db.Model, Versioned): name = db.Column(db.String(255), nullable=False) secret = db.Column(db.String(255), unique=True, nullable=False) service_id = db.Column(UUID(as_uuid=True), db.ForeignKey('services.id'), index=True, nullable=False) - service = db.relationship('Service', backref=db.backref('api_keys', lazy='dynamic')) + service = db.relationship('Service', backref='api_keys') key_type = db.Column(db.String(255), db.ForeignKey('key_types.name'), index=True, nullable=False) expiry_date = db.Column(db.DateTime) created_at = db.Column( diff --git a/tests/app/authentication/test_authentication.py b/tests/app/authentication/test_authentication.py index 3a6d5b4d2..7c445687a 100644 --- a/tests/app/authentication/test_authentication.py +++ b/tests/app/authentication/test_authentication.py @@ -223,6 +223,17 @@ def test_authentication_returns_error_when_service_doesnt_exit( assert error_message['message'] == {'token': ['Invalid token: service not found']} +def test_authentication_returns_error_when_service_inactive(client, sample_api_key): + sample_api_key.service.active = False + token = create_jwt_token(secret=str(sample_api_key.id), client_id=str(sample_api_key.service_id)) + + response = client.get('/service', headers={'Authorization': 'Bearer {}'.format(token)}) + + assert response.status_code == 403 + error_message = json.loads(response.get_data()) + assert error_message['message'] == {'token': ['Invalid token: service is archived']} + + def test_authentication_returns_error_when_service_has_no_secrets(notify_api, sample_service, fake_uuid): diff --git a/tests/app/service/test_deactivate.py b/tests/app/service/test_deactivate.py index 50e552816..d81de0e10 100644 --- a/tests/app/service/test_deactivate.py +++ b/tests/app/service/test_deactivate.py @@ -11,19 +11,19 @@ from tests.app.conftest import ( def test_deactivate_only_allows_post(client, sample_service): - auth_header = create_authorization_header(service_id=str(sample_service.id)) + auth_header = create_authorization_header() response = client.get('/service/{}/deactivate'.format(uuid.uuid4()), headers=[auth_header]) assert response.status_code == 405 def test_deactivate_service_errors_with_bad_service_id(client, sample_service): - auth_header = create_authorization_header(service_id=str(sample_service.id)) + auth_header = create_authorization_header() response = client.post('/service/{}/deactivate'.format(uuid.uuid4()), headers=[auth_header]) assert response.status_code == 404 def test_deactivating_inactive_service_does_nothing(client, sample_service): - auth_header = create_authorization_header(service_id=str(sample_service.id)) + auth_header = create_authorization_header() sample_service.active = False response = client.post('/service/{}/deactivate'.format(sample_service.id), headers=[auth_header]) assert response.status_code == 204 @@ -37,7 +37,7 @@ def deactivated_service(client, notify_db, notify_db_session, sample_service): create_api_key(notify_db, notify_db_session) create_api_key(notify_db, notify_db_session) - auth_header = create_authorization_header(service_id=str(sample_service.id)) + auth_header = create_authorization_header() response = client.post('/service/{}/deactivate'.format(sample_service.id), headers=[auth_header]) assert response.status_code == 204 assert response.data == b'' @@ -50,7 +50,7 @@ def test_deactivating_service_changes_name_and_email(deactivated_service): def test_deactivating_service_revokes_api_keys(deactivated_service): - assert deactivated_service.api_keys.count() == 2 + assert len(deactivated_service.api_keys) == 2 for key in deactivated_service.api_keys: assert key.expiry_date is not None assert key.version == 2