Implementation of api key pages.

Revoke page will show the correct key name
Show api keys shows a well formatted expiry date
Fix tests for api key endpoints.

app/__init__.py

@@ -1,6 +1,7 @@
 import os
 import re
 
+import dateutil
 from flask import Flask, session, Markup, escape, render_template
 from flask._compat import string_types
 from flask.ext.sqlalchemy import SQLAlchemy
@@ -54,6 +55,7 @@ def create_app(config_name, config_overrides=None):
     application.add_template_filter(placeholders)
     application.add_template_filter(replace_placeholders)
     application.add_template_filter(nl2br)
+    application.add_template_filter(format_datetime)
 
     application.after_request(useful_headers_after_request)
     register_errorhandlers(application)
@@ -134,6 +136,12 @@ def replace_placeholders(template, values):
     ))
 
 
+def format_datetime(date):
+    date = dateutil.parser.parse(date)
+    native = date.replace(tzinfo=None)
+    return native.strftime('%A %d %B %Y at %H:%M')
+
+
 # https://www.owasp.org/index.php/List_of_useful_HTTP_headers
 def useful_headers_after_request(response):
     response.headers.add('X-Frame-Options', 'deny')

app/main/views/api_keys.py

@@ -36,18 +36,17 @@ def create_api_key(service_id):
     )
 
 
-@main.route("/services/<int:service_id>/api-keys/show")
-@login_required
-def show_api_key(service_id):
-    return render_template('views/api-keys/show.html', service_id=service_id)
-
-
 @main.route("/services/<int:service_id>/api-keys/revoke/<int:key_id>", methods=['GET', 'POST'])
 @login_required
 def revoke_api_key(service_id, key_id):
+    key_name = api_key_api_client.get_api_keys(service_id=service_id, key_id=key_id)['apiKeys'][0]['name']
     if request.method == 'GET':
-        return render_template('views/api-keys/revoke.html', service_id=service_id)
+        return render_template(
+            'views/api-keys/revoke.html',
+            service_id=service_id,
+            key_name=key_name
+        )
     elif request.method == 'POST':
         api_key_api_client.revoke_api_key(service_id=service_id, key_id=key_id)
-        flash('‘Test key 1’ was revoked')
+        flash('‘{}’ was revoked'.format(key_name))
         return redirect(url_for('.api_keys', service_id=service_id))

app/notify_client/api_key_api_client.py

@@ -12,8 +12,11 @@ class ApiKeyApiClient(BaseAPIClient):
         self.client_id = app.config['ADMIN_CLIENT_USER_NAME']
         self.secret = app.config['ADMIN_CLIENT_SECRET']
 
-    def get_api_keys(self, service_id, *params):
+    def get_api_keys(self, service_id, key_id=None, *params):
-        return self.get(url='/service/{}/api-keys'.format(service_id))
+        if key_id:
+            return self.get(url='/service/{}/api-keys/{}'.format(service_id, key_id))
+        else:
+            return self.get(url='/service/{}/api-keys'.format(service_id))
 
     def create_api_key(self, service_id, key_name, *params):
         data = {"name": key_name}

app/templates/views/api-keys.html

@@ -42,7 +42,7 @@
     {% endcall %}
     {% if item.expiry_date %}
       {% call field(align='right', status='default') %}
-        Revoked {{ item.expiry_date }}
+        Revoked {{ item.expiry_date|format_datetime }}
       {% endcall %}
     {% else %}
       {% call field(align='right', status='error') %}

app/templates/views/api-keys/revoke.html

@@ -16,7 +16,7 @@
         </h1>
 
         <p>
-          ‘Test key 1’ will no longer let you connect to GOV.UK Notify.
+          ‘{{ key_name }}’ will no longer let you connect to GOV.UK Notify.
         </p>
         <p>
           You can’t undo this.

tests/__init__.py

@@ -39,6 +39,12 @@ def template_json(id_, name, type_, content, service_id):
     }
 
 
+def api_key_json(id_, name, expiry_date=None):
+    return {'id': id_,
+            'name': name,
+            'expiry_date': expiry_date
+            }
+
 TEST_USER_EMAIL = 'test@user.gov.uk'
 
 

tests/app/main/views/test_api_keys.py

@@ -1,3 +1,4 @@
+from datetime import date
 from flask import url_for
 
 
@@ -13,16 +14,37 @@ def test_should_show_documentation_page(app_,
         assert response.status_code == 200
 
 
-def test_should_show_api_keys_page(app_,
+def test_should_show_empty_api_keys_page(app_,
-                                   db_,
+                                         db_,
-                                   db_session,
+                                         db_session,
-                                   active_user):
+                                         active_user,
+                                         mock_get_no_api_keys):
     with app_.test_request_context():
         with app_.test_client() as client:
             client.login(active_user)
             response = client.get(url_for('main.api_keys', service_id=123))
 
         assert response.status_code == 200
+        assert 'You haven’t created any API keys yet' in response.get_data(as_text=True)
+        assert 'Create a new API key' in response.get_data(as_text=True)
+        mock_get_no_api_keys.assert_called_once_with(service_id=123)
+
+
+def test_should_show_api_keys_page(app_,
+                                   db_,
+                                   db_session,
+                                   active_user,
+                                   mock_get_api_keys):
+    with app_.test_request_context():
+        with app_.test_client() as client:
+            client.login(active_user)
+            response = client.get(url_for('main.api_keys', service_id=123))
+
+        assert response.status_code == 200
+        assert 'some key name' in response.get_data(as_text=True)
+        assert 'another key name' in response.get_data(as_text=True)
+        assert 'Revoked Thursday 01 January 1970 at 00:00' in response.get_data(as_text=True)
+        mock_get_api_keys.assert_called_once_with(service_id=123)
 
 
 def test_should_show_name_api_key_page(app_,
@@ -37,47 +59,43 @@ def test_should_show_name_api_key_page(app_,
         assert response.status_code == 200
 
 
-def test_should_redirect_to_new_api_key(app_,
+def test_should_render_show_api_key(app_,
-                                        db_,
+                                    db_,
-                                        db_session,
+                                    db_session,
-                                        active_user):
+                                    active_user,
+                                    mock_create_api_key):
     with app_.test_request_context():
         with app_.test_client() as client:
             client.login(active_user)
-            response = client.post(url_for('main.create_api_key', service_id=123))
+            response = client.post(url_for('main.create_api_key', service_id=123),
-
+                                   data={'key_name': 'some default key name'})
-        assert response.status_code == 302
-        assert response.location == url_for('main.show_api_key', service_id=123, _external=True)
-
-
-def test_should_show_new_api_key(app_,
-                                 db_,
-                                 db_session,
-                                 active_user):
-    with app_.test_request_context():
-        with app_.test_client() as client:
-            client.login(active_user)
-            response = client.get(url_for('main.show_api_key', service_id=123))
 
         assert response.status_code == 200
+        assert 'some default key name' in response.get_data(as_text=True)
+        mock_create_api_key.assert_called_once_with(service_id=123, key_name='some default key name')
 
 
 def test_should_show_confirm_revoke_api_key(app_,
                                             db_,
                                             db_session,
-                                            active_user):
+                                            active_user,
+                                            mock_get_api_keys):
     with app_.test_request_context():
         with app_.test_client() as client:
             client.login(active_user)
             response = client.get(url_for('main.revoke_api_key', service_id=123, key_id=321))
 
         assert response.status_code == 200
+        assert 'some key name' in response.get_data(as_text=True)
+        mock_get_api_keys.assert_called_once_with(service_id=123, key_id=321)
 
 
 def test_should_redirect_after_revoking_api_key(app_,
                                                 db_,
                                                 db_session,
-                                                active_user):
+                                                active_user,
+                                                mock_revoke_api_key,
+                                                mock_get_api_keys):
     with app_.test_request_context():
         with app_.test_client() as client:
             client.login(active_user)
@@ -85,3 +103,5 @@ def test_should_redirect_after_revoking_api_key(app_,
 
         assert response.status_code == 302
         assert response.location == url_for('.api_keys', service_id=123, _external=True)
+        mock_revoke_api_key.assert_called_once_with(service_id=123, key_id=321)
+        mock_get_api_keys.assert_called_once_with(service_id=123, key_id=321)

tests/conftest.py

@@ -1,14 +1,17 @@
 import os
+from datetime import date
+
 import pytest
 from alembic.command import upgrade
 from alembic.config import Config
 from flask.ext.migrate import Migrate, MigrateCommand
 from flask.ext.script import Manager
 from sqlalchemy.schema import MetaData
-from . import (
+
-    create_test_user, create_another_test_user, service_json, TestClient,
-    get_test_user, template_json)
 from app import create_app, db
+from . import (
+    create_test_user, service_json, TestClient,
+    get_test_user, template_json, api_key_json)
 
 
 @pytest.fixture(scope='session')
@@ -89,6 +92,7 @@ def mock_get_service(mocker, active_user):
             service_id, "Test Service", [active_user.id], limit=1000,
             active=False, restricted=True)
         return {'data': service, 'token': 1}
+
     return mocker.patch('app.notifications_api_client.get_service', side_effect=_create)
 
 
@@ -99,6 +103,7 @@ def mock_create_service(mocker):
             101, service_name, [user_id], limit=limit,
             active=active, restricted=restricted)
         return {'data': service}
+
     mock_class = mocker.patch(
         'app.notifications_api_client.create_service', side_effect=_create)
     return mock_class
@@ -116,6 +121,7 @@ def mock_update_service(mocker):
             service_id, service_name, users, limit=limit,
             active=active, restricted=restricted)
         return {'data': service}
+
     mock_class = mocker.patch(
         'app.notifications_api_client.update_service', side_effect=_update)
     return mock_class
@@ -129,6 +135,7 @@ def mock_get_services(mocker, active_user):
         service_two = service_json(
             2, "service_two", [active_user.id], 1000, True, False)
         return {'data': [service_one, service_two]}
+
     mock_class = mocker.patch(
         'app.notifications_api_client.get_services', side_effect=_create)
     return mock_class
@@ -138,6 +145,7 @@ def mock_get_services(mocker, active_user):
 def mock_delete_service(mocker, mock_get_service):
     def _delete(service_id):
         return mock_get_service.side_effect(service_id)
+
     mock_class = mocker.patch(
         'app.notifications_api_client.delete_service', side_effect=_delete)
     return mock_class
@@ -149,6 +157,7 @@ def mock_get_service_template(mocker):
         template = template_json(
             template_id, "Template Name", "sms", "template content", service_id)
         return {'data': template}
+
     return mocker.patch(
         'app.notifications_api_client.get_service_template',
         side_effect=_create)
@@ -160,6 +169,7 @@ def mock_create_service_template(mocker):
         template = template_json(
             101, name, type_, content, service)
         return {'data': template}
+
     mock_class = mocker.patch(
         'app.notifications_api_client.create_service_template',
         side_effect=_create)
@@ -172,6 +182,7 @@ def mock_update_service_template(mocker):
         template = template_json(
             id_, name, type_, content, service)
         return {'data': template}
+
     mock_class = mocker.patch(
         'app.notifications_api_client.update_service_template',
         side_effect=_update)
@@ -186,6 +197,7 @@ def mock_get_service_templates(mocker):
         template_two = template_json(
             2, "template_two", "sms", "template two content", service_id)
         return {'data': [template_one, template_two]}
+
     mock_class = mocker.patch(
         'app.notifications_api_client.get_service_templates',
         side_effect=_create)
@@ -199,6 +211,7 @@ def mock_delete_service_template(mocker):
             template_id, "Template to delete",
             "sms", "content to be deleted", service_id)
         return {'data': template}
+
     return mocker.patch(
         'app.notifications_api_client.delete_service_template', side_effect=_delete)
 
@@ -212,5 +225,41 @@ def mock_register_user(mocker, user_data):
     return mock_class
 
 
-def mock_create_api_key(mocker, key_name):
+@pytest.fixture(scope='function')
-    mock_class = mocker.patch('app.api')
+def mock_create_api_key(mocker):
+    def _create(service_id, key_name):
+        import uuid
+        return {'data': str(uuid.uuid4())}
+
+    mock_class = mocker.patch('app.api_key_api_client.create_api_key', side_effect=_create)
+    return mock_class
+
+
+@pytest.fixture(scope='function')
+def mock_revoke_api_key(mocker):
+    def _revoke(service_id, key_id):
+        return {}
+
+    mock_class = mocker.patch('app.api_key_api_client.revoke_api_key', side_effect=_revoke)
+    return mock_class
+
+
+@pytest.fixture(scope='function')
+def mock_get_api_keys(mocker):
+    def _get_keys(service_id, key_id=None):
+        keys = {'apiKeys': [api_key_json(1, 'some key name'),
+                            api_key_json(2, 'another key name', expiry_date=str(date.fromtimestamp(0)))]}
+        return keys
+
+    mock_class = mocker.patch('app.api_key_api_client.get_api_keys', side_effect=_get_keys)
+    return mock_class
+
+
+@pytest.fixture(scope='function')
+def mock_get_no_api_keys(mocker):
+    def _get_keys(service_id):
+        keys = {'apiKeys': []}
+        return keys
+
+    mock_class = mocker.patch('app.api_key_api_client.get_api_keys', side_effect=_get_keys)
+    return mock_class