Merge pull request #678 from alphagov/better-auth-error-message

Improve error messages if you make an API call with a bad token

app/authentication/auth.py

@@ -1,8 +1,11 @@
 from flask import request, jsonify, _request_ctx_stack, current_app
+from sqlalchemy.orm.exc import NoResultFound
+
 from notifications_python_client.authentication import decode_jwt_token, get_token_issuer
 from notifications_python_client.errors import TokenDecodeError, TokenExpiredError
 
 from app.dao.api_key_dao import get_model_api_keys
+from app.dao.services_dao import dao_fetch_service_by_id
 
 
 class AuthError(Exception):
@@ -43,13 +46,18 @@ def requires_auth():
             continue
 
         if api_key.expiry_date:
-            raise AuthError("Invalid token: revoked", 403)
+            raise AuthError("Invalid token: API key revoked", 403)
 
         _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: no api keys for service", 403)
+        raise AuthError("Invalid token: service has no API keys", 403)
     else:
         raise AuthError("Invalid token: signature", 403)
 

tests/app/authentication/test_authentication.py

@@ -169,7 +169,7 @@ def test_authentication_returns_token_expired_when_service_uses_expired_key_and_
                 headers={'Authorization': 'Bearer {}'.format(token)})
             assert response.status_code == 403
             data = json.loads(response.get_data())
-            assert data['message'] == {"token": ['Invalid token: revoked']}
+            assert data['message'] == {"token": ['Invalid token: API key revoked']}
 
 
 def test_authentication_returns_error_when_admin_client_has_no_secrets(notify_api,
@@ -192,6 +192,28 @@ def test_authentication_returns_error_when_admin_client_has_no_secrets(notify_ap
             notify_api.config['ADMIN_CLIENT_SECRET'] = api_secret
 
 
+def test_authentication_returns_error_when_service_doesnt_exit(
+    notify_api,
+    notify_db,
+    notify_db_session,
+    sample_service,
+    fake_uuid
+):
+    with notify_api.test_request_context(), notify_api.test_client() as client:
+        # get service ID and secret the wrong way around
+        token = create_jwt_token(
+            secret=str(sample_service.id),
+            client_id=fake_uuid
+        )
+        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 not found']}
+
+
 def test_authentication_returns_error_when_service_has_no_secrets(notify_api,
                                                                   notify_db,
                                                                   notify_db_session,
@@ -208,7 +230,7 @@ def test_authentication_returns_error_when_service_has_no_secrets(notify_api,
                 headers={'Authorization': 'Bearer {}'.format(token)})
             assert response.status_code == 403
             error_message = json.loads(response.get_data())
-            assert error_message['message'] == {'token': ['Invalid token: no api keys for service']}
+            assert error_message['message'] == {'token': ['Invalid token: service has no API keys']}
 
 
 def test_should_attach_the_current_api_key_to_current_app(notify_api, sample_service, sample_api_key):