diff --git a/app/__init__.py b/app/__init__.py index 1914a4adb..80da769a3 100644 --- a/app/__init__.py +++ b/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') diff --git a/app/main/views/api_keys.py b/app/main/views/api_keys.py index d32332a2b..f3790d174 100644 --- a/app/main/views/api_keys.py +++ b/app/main/views/api_keys.py @@ -36,18 +36,17 @@ def create_api_key(service_id): ) -@main.route("/services//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//api-keys/revoke/", 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)) diff --git a/app/notify_client/api_key_api_client.py b/app/notify_client/api_key_api_client.py index d538cb816..660f839c8 100644 --- a/app/notify_client/api_key_api_client.py +++ b/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): - return self.get(url='/service/{}/api-keys'.format(service_id)) + def get_api_keys(self, service_id, key_id=None, *params): + 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} @@ -21,4 +24,4 @@ class ApiKeyApiClient(BaseAPIClient): return key['data'] def revoke_api_key(self, service_id, key_id, *params): - return self.post(url='/service/{0}/api-key/revoke/{1}'.format(service_id, key_id), data=None) \ No newline at end of file + return self.post(url='/service/{0}/api-key/revoke/{1}'.format(service_id, key_id), data=None) diff --git a/app/templates/views/api-keys.html b/app/templates/views/api-keys.html index dbd48cab0..1886cd8ac 100644 --- a/app/templates/views/api-keys.html +++ b/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') %} diff --git a/app/templates/views/api-keys/revoke.html b/app/templates/views/api-keys/revoke.html index 3597a6270..abf066c65 100644 --- a/app/templates/views/api-keys/revoke.html +++ b/app/templates/views/api-keys/revoke.html @@ -16,7 +16,7 @@

- ‘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.

You can’t undo this. diff --git a/tests/__init__.py b/tests/__init__.py index fc5f15fb3..c0806dcbb 100644 --- a/tests/__init__.py +++ b/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' diff --git a/tests/app/main/views/test_api_keys.py b/tests/app/main/views/test_api_keys.py index 1e274486d..94ef8fe41 100644 --- a/tests/app/main/views/test_api_keys.py +++ b/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_, - db_, - db_session, - active_user): +def test_should_show_empty_api_keys_page(app_, + db_, + db_session, + 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_, - db_, - db_session, - active_user): +def test_should_render_show_api_key(app_, + db_, + db_session, + 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)) - - 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)) + response = client.post(url_for('main.create_api_key', service_id=123), + data={'key_name': 'some default key name'}) 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) diff --git a/tests/conftest.py b/tests/conftest.py index 7bbf805d8..ab89761f1 100644 --- a/tests/conftest.py +++ b/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): - mock_class = mocker.patch('app.api') +@pytest.fixture(scope='function') +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