Merge pull request #23 from alphagov/allow-multiple-api-keys

Allow multiple api keys

app/authentication/auth.py

@@ -1,8 +1,7 @@
 from flask import request, jsonify, _request_ctx_stack
 from client.authentication import decode_jwt_token, get_token_issuer
 from client.errors import TokenDecodeError, TokenRequestError, TokenExpiredError, TokenPayloadError
-
+from app.dao.api_key_dao import get_unsigned_secrets
-from app.dao.api_key_dao import get_unsigned_secret
 
 
 def authentication_response(message, code):
@@ -21,28 +20,36 @@ def requires_auth():
     if auth_scheme != 'Bearer ':
         return authentication_response('Unauthorized, authentication bearer scheme must be used', 401)
 
-    try:
     auth_token = auth_header[7:]
+    try:
         api_client = fetch_client(get_token_issuer(auth_token))
+    except TokenDecodeError:
+        return authentication_response("Invalid token: signature", 403)
     if api_client is None:
         authentication_response("Invalid credentials", 403)
-
+    # If the api_client does not have any secrets return response saying that
+    errors_resp = authentication_response("Invalid token: api client has no secrets", 403)
+    for secret in api_client['secret']:
+        try:
             decode_jwt_token(
                 auth_token,
-            api_client['secret'],
+                secret,
                 request.method,
                 request.path,
                 request.data.decode() if request.data else None
             )
             _request_ctx_stack.top.api_user = api_client
+            return
         except TokenRequestError:
-        return authentication_response("Invalid token: request", 403)
+            errors_resp = authentication_response("Invalid token: request", 403)
         except TokenExpiredError:
-        return authentication_response("Invalid token: expired", 403)
+            errors_resp = authentication_response("Invalid token: expired", 403)
         except TokenPayloadError:
-        return authentication_response("Invalid token: payload", 403)
+            errors_resp = authentication_response("Invalid token: payload", 403)
         except TokenDecodeError:
-        return authentication_response("Invalid token: signature", 403)
+            errors_resp = authentication_response("Invalid token: signature", 403)
+
+    return errors_resp
 
 
 def fetch_client(client):
@@ -50,10 +57,10 @@ def fetch_client(client):
     if client == current_app.config.get('ADMIN_CLIENT_USER_NAME'):
         return {
             "client": client,
-            "secret": current_app.config.get('ADMIN_CLIENT_SECRET')
+            "secret": [current_app.config.get('ADMIN_CLIENT_SECRET')]
         }
     else:
         return {
             "client": client,
-            "secret": get_unsigned_secret(client)
+            "secret": get_unsigned_secrets(client)
         }

app/dao/api_key_dao.py

@@ -30,12 +30,20 @@ def get_model_api_keys(service_id=None, raise_=True):
     return ApiKey.query.filter_by().all()
 
 
-def get_unsigned_secret(service_id):
+def get_unsigned_secrets(service_id):
     """
-    There should only be one valid api_keys for each service.
     This method can only be exposed to the Authentication of the api calls.
     """
-    api_key = ApiKey.query.filter_by(service_id=service_id, expiry_date=None).one()
+    api_keys = ApiKey.query.filter_by(service_id=service_id, expiry_date=None).all()
+    keys = [_get_secret(x.secret) for x in api_keys]
+    return keys
+
+
+def get_unsigned_secret(key_id):
+    """
+    This method can only be exposed to the Authentication of the api calls.
+    """
+    api_key = ApiKey.query.filter_by(id=key_id, expiry_date=None).one()
     return _get_secret(api_key.secret)
 
 

app/service/rest.py

@@ -75,7 +75,7 @@ def get_service(service_id=None):
     return jsonify(data=data)
 
 
-@service.route('/<int:service_id>/api-key/renew', methods=['POST'])
+@service.route('/<int:service_id>/api-key', methods=['POST'])
 def renew_api_key(service_id=None):
     try:
         service = get_model_services(service_id=service_id)
@@ -85,17 +85,14 @@ def renew_api_key(service_id=None):
         return jsonify(result="error", message="Service not found"), 404
 
     try:
-        service_api_key = get_model_api_keys(service_id=service_id, raise_=False)
-        if service_api_key:
-            # expire existing api_key
-            save_model_api_key(service_api_key, update_dict={'id': service_api_key.id, 'expiry_date': datetime.now()})
         # create a new one
         # TODO: what validation should be done here?
         secret_name = request.get_json()['name']
-        save_model_api_key(ApiKey(service=service, name=secret_name))
+        key = ApiKey(service=service, name=secret_name)
+        save_model_api_key(key)
     except DAOException as e:
         return jsonify(result='error', message=str(e)), 400
-    unsigned_api_key = get_unsigned_secret(service_id)
+    unsigned_api_key = get_unsigned_secret(key.id)
     return jsonify(data=unsigned_api_key), 201
 
 

tests/__init__.py

@@ -1,13 +1,13 @@
 from flask import current_app
 from client.authentication import create_jwt_token
 
-from app.dao.api_key_dao import get_unsigned_secret
+from app.dao.api_key_dao import get_unsigned_secrets
 
 
 def create_authorization_header(path, method, request_body=None, service_id=None):
     if service_id:
         client_id = service_id
-        secret = get_unsigned_secret(service_id)
+        secret = get_unsigned_secrets(service_id)[0]
     else:
         client_id = current_app.config.get('ADMIN_CLIENT_USER_NAME')
         secret = current_app.config.get('ADMIN_CLIENT_SECRET')

tests/app/authentication/test_authentication.py

@@ -1,7 +1,9 @@
-from client.authentication import create_jwt_token
+from datetime import datetime, timedelta
-from flask import json, url_for
 
-from app.dao.api_key_dao import get_unsigned_secret
+from client.authentication import create_jwt_token
+from flask import json, url_for, current_app
+from app.dao.api_key_dao import get_unsigned_secrets, save_model_api_key, get_unsigned_secret
+from app.models import ApiKey, Service
 
 
 def test_should_not_allow_request_with_no_token(notify_api):
@@ -23,7 +25,7 @@ def test_should_not_allow_request_with_incorrect_header(notify_api):
             assert data['error'] == 'Unauthorized, authentication bearer scheme must be used'
 
 
-def test_should_not_allow_request_with_incorrect_token(notify_api):
+def test_should_not_allow_request_with_incorrect_token(notify_api, notify_db, notify_db_session, sample_api_key):
     with notify_api.test_request_context():
         with notify_api.test_client() as client:
             response = client.get(url_for('service.get_service'),
@@ -38,7 +40,7 @@ def test_should_not_allow_incorrect_path(notify_api, notify_db, notify_db_sessio
         with notify_api.test_client() as client:
             token = create_jwt_token(request_method="GET",
                                      request_path="/bad",
-                                     secret=get_unsigned_secret(sample_api_key.service_id),
+                                     secret=get_unsigned_secrets(sample_api_key.service_id)[0],
                                      client_id=sample_api_key.service_id)
             response = client.get(url_for('service.get_service'),
                                   headers={'Authorization': "Bearer {}".format(token)})
@@ -79,28 +81,49 @@ def test_should_allow_valid_token(notify_api, notify_db, notify_db_session, samp
             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):
+    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'),
+                                  headers={'Authorization': 'Bearer {}'.format(token)})
+            assert response.status_code == 200
+
+
 JSON_BODY = json.dumps({
-    'name': 'new name'
+    "key1": "value1",
+    "key2": "value2",
+    "key3": "value3"
 })
 
 
-# def test_should_allow_valid_token_with_post_body(
+def test_should_allow_valid_token_with_post_body(notify_api, notify_db, notify_db_session, sample_api_key):
-#         notify_api, notify_db, notify_db_session, sample_api_key):
+    with notify_api.test_request_context():
-#     with notify_api.test_request_context():
+        with notify_api.test_client() as client:
-#         with notify_api.test_client() as client:
+            service = Service.query.get(sample_api_key.service_id)
-#             token = create_jwt_token(
+            data = {'name': 'new name',
-#                 request_method="PUT",
+                    'users': [service.users[0].id],
-#                 request_path=url_for('service.update_service', service_id=sample_api_key.service_id),
+                    'limit': 1000,
-#                 secret=get_unsigned_secret(sample_api_key.service_id),
+                    'restricted': False,
-#                 client_id=sample_api_key.service_id,
+                    'active': False}
-#                 request_body=JSON_BODY
+
-#             )
+            token = create_jwt_token(
-#             response = client.put(
+                request_method="PUT",
-#                 url_for('service.update_service', service_id=sample_api_key.service_id),
+                request_path=url_for('service.update_service', service_id=sample_api_key.service_id),
-#                 data=JSON_BODY,
+                secret=get_unsigned_secret(sample_api_key.id),
-#                 headers=[('Content-type', 'application-json'), ('Authorization', 'Bearer {}'.format(token))]
+                client_id=sample_api_key.service_id,
-#             )
+                request_body=json.dumps(data)
-#             assert response.status_code == 200
+            )
+            headers = [('Content-Type', 'application/json'), ('Authorization', 'Bearer {}'.format(token))]
+            response = client.put(
+                url_for('service.update_service', service_id=service.id),
+                data=json.dumps(data),
+                headers=headers)
+            assert response.status_code == 200
 
 
 def test_should_not_allow_valid_token_with_invalid_post_body(notify_api, notify_db, notify_db_session, sample_api_key):
@@ -115,10 +138,76 @@ def test_should_not_allow_valid_token_with_invalid_post_body(notify_api, notify_
             assert data['error'] == 'Invalid token: payload'
 
 
+def test_authentication_passes_admin_client_token(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_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)})
+            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):
+    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',
+                            'expiry_date': datetime.now()}
+            expired_api_key = ApiKey(**exprired_key)
+            save_model_api_key(expired_api_key)
+            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=sample_api_key.service_id)
+            response = client.get(url_for('service.get_service'),
+                                  headers={'Authorization': 'Bearer {}'.format(token)})
+            assert response.status_code == 200
+
+
+def test_authentication_returns_token_expired_when_service_uses_expired_key_and_has_multiple_keys(notify_api,
+                                                                                                  notify_db,
+                                                                                                  notify_db_session,
+                                                                                                  sample_api_key):
+    with notify_api.test_request_context():
+        with notify_api.test_client() as client:
+            expired_key = {'service_id': sample_api_key.service_id, 'name': 'expired_key'}
+            expired_api_key = ApiKey(**expired_key)
+            save_model_api_key(expired_api_key)
+            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=sample_api_key.service_id)
+            # expire the key
+            expire_the_key = {'id': expired_api_key.id,
+                              'service_id': 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)})
+            assert response.status_code == 403
+            data = json.loads(response.get_data())
+            assert data['error'] == 'Invalid token: signature'
+
+
 def __create_get_token(service_id):
     return create_jwt_token(request_method="GET",
                             request_path=url_for('service.get_service'),
-                            secret=get_unsigned_secret(service_id),
+                            secret=get_unsigned_secrets(service_id)[0],
                             client_id=service_id)
 
 
@@ -126,7 +215,7 @@ def __create_post_token(service_id, request_body):
     return create_jwt_token(
         request_method="POST",
         request_path=url_for('service.create_service'),
-        secret=get_unsigned_secret(service_id),
+        secret=get_unsigned_secrets(service_id)[0],
         client_id=service_id,
         request_body=request_body
     )

tests/app/conftest.py

@@ -69,7 +69,7 @@ def sample_api_key(notify_db,
                    service=None):
     if service is None:
         service = sample_service(notify_db, notify_db_session)
-    data = {'service_id': service.id, 'name': service.name}
+    data = {'service_id': service.id, 'name': uuid.uuid4()}
     api_key = ApiKey(**data)
     save_model_api_key(api_key)
     return api_key

tests/app/dao/test_api_key_dao.py

@@ -5,6 +5,7 @@ from sqlalchemy.orm.exc import NoResultFound
 
 from app.dao.api_key_dao import (save_model_api_key,
                                  get_model_api_keys,
+                                 get_unsigned_secrets,
                                  get_unsigned_secret,
                                  _generate_secret,
                                  _get_secret)
@@ -60,10 +61,20 @@ def test_should_return_api_key_for_service(notify_api, notify_db, notify_db_sess
     assert api_key == sample_api_key
 
 
-def test_should_return_unsigned_api_key_for_service_id(notify_api,
+def test_should_return_unsigned_api_keys_for_service_id(notify_api,
                                                         notify_db,
                                                         notify_db_session,
                                                         sample_api_key):
-    unsigned_api_key = get_unsigned_secret(sample_api_key.service_id)
+    unsigned_api_key = get_unsigned_secrets(sample_api_key.service_id)
+    assert len(unsigned_api_key) == 1
+    assert sample_api_key.secret != unsigned_api_key[0]
+    assert unsigned_api_key[0] == _get_secret(sample_api_key.secret)
+
+
+def test_get_unsigned_secret_returns_key(notify_api,
+                                         notify_db,
+                                         notify_db_session,
+                                         sample_api_key):
+    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)

tests/app/service/test_api_key_endpoints.py

@@ -0,0 +1,81 @@
+import json
+
+from flask import url_for
+
+from app.models import ApiKey
+from tests import create_authorization_header
+
+
+def test_api_key_should_create_new_api_key_for_service(notify_api, notify_db,
+                                                       notify_db_session,
+                                                       sample_service):
+    with notify_api.test_request_context():
+        with notify_api.test_client() as client:
+            data = {'name': 'some secret name'}
+            auth_header = create_authorization_header(path=url_for('service.renew_api_key',
+                                                                   service_id=sample_service.id),
+                                                      method='POST',
+                                                      request_body=json.dumps(data))
+            response = client.post(url_for('service.renew_api_key', service_id=sample_service.id),
+                                   data=json.dumps(data),
+                                   headers=[('Content-Type', 'application/json'), auth_header])
+            assert response.status_code == 201
+            assert response.get_data is not None
+            saved_api_key = ApiKey.query.filter_by(service_id=sample_service.id).first()
+            assert saved_api_key.service_id == sample_service.id
+            assert saved_api_key.name == 'some secret name'
+
+
+def test_api_key_should_return_error_when_service_does_not_exist(notify_api, notify_db, notify_db_session,
+                                                                 sample_service):
+    with notify_api.test_request_context():
+        with notify_api.test_client() as client:
+            auth_header = create_authorization_header(path=url_for('service.renew_api_key', service_id="123"),
+                                                      method='POST')
+            response = client.post(url_for('service.renew_api_key', service_id=123),
+                                   headers=[('Content-Type', 'application/json'), auth_header])
+            assert response.status_code == 404
+
+
+def test_revoke_should_expire_api_key_for_service(notify_api, notify_db, notify_db_session,
+                                                  sample_api_key):
+    with notify_api.test_request_context():
+        with notify_api.test_client() as client:
+            assert ApiKey.query.count() == 1
+            auth_header = create_authorization_header(path=url_for('service.revoke_api_key',
+                                                                   service_id=sample_api_key.service_id),
+                                                      method='POST')
+            response = client.post(url_for('service.revoke_api_key', service_id=sample_api_key.service_id),
+                                   headers=[auth_header])
+            assert response.status_code == 202
+            api_keys_for_service = ApiKey.query.filter_by(service_id=sample_api_key.service_id).first()
+            assert api_keys_for_service.expiry_date is not None
+
+
+def test_api_key_should_create_multiple_new_api_key_for_service(notify_api, notify_db,
+                                                                notify_db_session,
+                                                                sample_service):
+    with notify_api.test_request_context():
+        with notify_api.test_client() as client:
+            assert ApiKey.query.count() == 0
+            data = {'name': 'some secret name'}
+            auth_header = create_authorization_header(path=url_for('service.renew_api_key',
+                                                                   service_id=sample_service.id),
+                                                      method='POST',
+                                                      request_body=json.dumps(data))
+            response = client.post(url_for('service.renew_api_key', service_id=sample_service.id),
+                                   data=json.dumps(data),
+                                   headers=[('Content-Type', 'application/json'), auth_header])
+            assert response.status_code == 201
+            assert ApiKey.query.count() == 1
+            data = {'name': 'another secret name'}
+            auth_header = create_authorization_header(path=url_for('service.renew_api_key',
+                                                                   service_id=sample_service.id),
+                                                      method='POST',
+                                                      request_body=json.dumps(data))
+            response2 = client.post(url_for('service.renew_api_key', service_id=sample_service.id),
+                                    data=json.dumps(data),
+                                    headers=[('Content-Type', 'application/json'), auth_header])
+            assert response2.status_code == 201
+            assert response2.get_data != response.get_data
+            assert ApiKey.query.count() == 2

tests/app/service/test_rest.py

@@ -310,82 +310,6 @@ def test_delete_service_not_exists(notify_api, notify_db, notify_db_session, sam
             assert Service.query.count() == 2
 
 
-def test_renew_api_key_should_create_new_api_key_for_service(notify_api, notify_db,
-                                                             notify_db_session,
-                                                             sample_service,
-                                                             sample_admin_service_id):
-    with notify_api.test_request_context():
-        with notify_api.test_client() as client:
-            data = {'name': 'some secret name'}
-            auth_header = create_authorization_header(service_id=sample_admin_service_id,
-                                                      path=url_for('service.renew_api_key',
-                                                                   service_id=sample_service.id),
-                                                      method='POST',
-                                                      request_body=json.dumps(data))
-            response = client.post(url_for('service.renew_api_key', service_id=sample_service.id),
-                                   data=json.dumps(data),
-                                   headers=[('Content-Type', 'application/json'), auth_header])
-            assert response.status_code == 201
-            assert response.get_data is not None
-            saved_api_key = ApiKey.query.filter_by(service_id=sample_service.id).first()
-            assert saved_api_key.service_id == sample_service.id
-            assert saved_api_key.name == 'some secret name'
-
-
-def test_renew_api_key_should_expire_the_old_api_key_and_create_a_new_api_key(notify_api, notify_db, notify_db_session,
-                                                                              sample_api_key, sample_admin_service_id):
-    with notify_api.test_request_context():
-        with notify_api.test_client() as client:
-            assert ApiKey.query.count() == 2
-            data = {'name': 'some secret name'}
-            auth_header = create_authorization_header(service_id=sample_admin_service_id,
-                                                      path=url_for('service.renew_api_key',
-                                                                   service_id=sample_api_key.service_id),
-                                                      method='POST',
-                                                      request_body=json.dumps(data))
-
-            response = client.post(url_for('service.renew_api_key', service_id=sample_api_key.service_id),
-                                   data=json.dumps(data),
-                                   headers=[('Content-Type', 'application/json'), auth_header])
-            assert response.status_code == 201
-            assert ApiKey.query.count() == 3
-            all_api_keys = ApiKey.query.filter_by(service_id=sample_api_key.service_id).all()
-            for x in all_api_keys:
-                if x.id == sample_api_key.id:
-                    assert x.expiry_date is not None
-                else:
-                    assert x.expiry_date is None
-                    assert x.secret is not sample_api_key.secret
-
-
-def test_renew_api_key_should_return_error_when_service_does_not_exist(notify_api, notify_db, notify_db_session,
-                                                                       sample_service, sample_admin_service_id):
-    with notify_api.test_request_context():
-        with notify_api.test_client() as client:
-            auth_header = create_authorization_header(service_id=sample_admin_service_id,
-                                                      path=url_for('service.renew_api_key', service_id="123"),
-                                                      method='POST')
-            response = client.post(url_for('service.renew_api_key', service_id=123),
-                                   headers=[('Content-Type', 'application/json'), auth_header])
-            assert response.status_code == 404
-
-
-def test_revoke_api_key_should_expire_api_key_for_service(notify_api, notify_db, notify_db_session,
-                                                          sample_api_key, sample_admin_service_id):
-    with notify_api.test_request_context():
-        with notify_api.test_client() as client:
-            assert ApiKey.query.count() == 2
-            auth_header = create_authorization_header(service_id=sample_admin_service_id,
-                                                      path=url_for('service.revoke_api_key',
-                                                                   service_id=sample_api_key.service_id),
-                                                      method='POST')
-            response = client.post(url_for('service.revoke_api_key', service_id=sample_api_key.service_id),
-                                   headers=[auth_header])
-            assert response.status_code == 202
-            api_keys_for_service = ApiKey.query.filter_by(service_id=sample_api_key.service_id).first()
-            assert api_keys_for_service.expiry_date is not None
-
-
 def test_create_service_should_create_new_service_for_user(notify_api, notify_db, notify_db_session, sample_user,
                                                            sample_admin_service_id):
     with notify_api.test_request_context():