diff --git a/app/__init__.py b/app/__init__.py index 7395965b8..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 @@ -8,6 +9,7 @@ from flask_login import LoginManager from flask_wtf import CsrfProtect from werkzeug.exceptions import abort from app.notify_client.api_client import NotificationsAdminAPIClient +from app.notify_client.api_key_api_client import ApiKeyApiClient from app.notify_client.user_api_client import UserApiClient from app.its_dangerous_session import ItsdangerousSessionInterface import app.proxy_fix @@ -20,6 +22,7 @@ csrf = CsrfProtect() notifications_api_client = NotificationsAdminAPIClient() user_api_client = UserApiClient() +api_key_api_client = ApiKeyApiClient() def create_app(config_name, config_overrides=None): @@ -34,6 +37,7 @@ def create_app(config_name, config_overrides=None): notifications_api_client.init_app(application) user_api_client.init_app(application) + api_key_api_client.init_app(application) login_manager.init_app(application) login_manager.login_view = 'main.sign_in' @@ -51,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) @@ -131,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/assets/javascripts/apiKey.js b/app/assets/javascripts/apiKey.js index 30d87278d..215be5672 100644 --- a/app/assets/javascripts/apiKey.js +++ b/app/assets/javascripts/apiKey.js @@ -1,15 +1,11 @@ (function(Modules) { "use strict"; + if (!document.queryCommandSupported('copy')) return; + Modules.ApiKey = function() { const states = { - 'initial': ` - - `, - 'keyVisibleBasic': key => ` - ${key} - `, 'keyVisible': key => ` ${key} @@ -33,23 +29,22 @@ this.start = function(component) { - const $component = $(component).html(states.initial).attr('aria-live', 'polite'), + const $component = $(component), key = $component.data('key'); $component - .on( - 'click', '.api-key-button-show', () => - $component.html( - document.queryCommandSupported('copy') ? - states.keyVisible(key) : states.keyVisibleBasic(key) - ) - ) + .html(states.keyVisible(key)) + .attr('aria-live', 'polite') .on( 'click', '.api-key-button-copy', () => this.copyKey( $('.api-key-key', component)[0], () => $component.html(states.keyCopied) ) + ) + .on( + 'click', '.api-key-button-show', () => + $component.html(states.keyVisible(key)) ); }; diff --git a/app/assets/stylesheets/components/api-key.scss b/app/assets/stylesheets/components/api-key.scss index 22c787355..e17b45ef4 100644 --- a/app/assets/stylesheets/components/api-key.scss +++ b/app/assets/stylesheets/components/api-key.scss @@ -1,5 +1,10 @@ .api-key { + &-name { + @include bold-19; + margin-bottom: 5px; + } + &-key { font-family: monospace; display: block; diff --git a/app/assets/stylesheets/components/table.scss b/app/assets/stylesheets/components/table.scss index ed749b6e3..ce6930370 100644 --- a/app/assets/stylesheets/components/table.scss +++ b/app/assets/stylesheets/components/table.scss @@ -1,3 +1,7 @@ +.table { + margin-bottom: $gutter; +} + .table-heading { text-align: left; } @@ -16,8 +20,15 @@ } &-error { + color: $error-colour; font-weight: bold; + + a:link, + a:visited { + color: $error-colour; + } + } } diff --git a/app/main/forms.py b/app/main/forms.py index 70af82666..fab516607 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -235,3 +235,17 @@ class ChangeMobileNumberForm(Form): class ConfirmMobileNumberForm(Form): sms_code = sms_code() + + +class CreateKeyForm(Form): + def __init__(self, existing_key_names=[], *args, **kwargs): + self.existing_key_names = [x.lower() for x in existing_key_names] + super(CreateKeyForm, self).__init__(*args, **kwargs) + + key_name = StringField(u'Description of key', validators=[ + DataRequired(message='You need to give the key a name') + ]) + + def validate_key_name(self, key_name): + if key_name.data.lower() in self.existing_key_names: + raise ValidationError('A key with this name already exists') diff --git a/app/main/views/api_keys.py b/app/main/views/api_keys.py index 0241346a8..e2d77bf31 100644 --- a/app/main/views/api_keys.py +++ b/app/main/views/api_keys.py @@ -1,9 +1,55 @@ -from flask import render_template +from flask import request, render_template, redirect, url_for, flash from flask_login import login_required from app.main import main +from app.main.forms import CreateKeyForm +from app import api_key_api_client + + +@main.route("/services//documentation") +@login_required +def documentation(service_id): + return render_template('views/documentation.html', service_id=service_id) @main.route("/services//api-keys") @login_required def api_keys(service_id): - return render_template('views/api-keys.html', service_id=service_id) + return render_template( + 'views/api-keys.html', + service_id=service_id, + keys=api_key_api_client.get_api_keys(service_id=service_id)['apiKeys'] + ) + + +@main.route("/services//api-keys/create", methods=['GET', 'POST']) +@login_required +def create_api_key(service_id): + key_names = [ + key['name'] for key in api_key_api_client.get_api_keys(service_id=service_id)['apiKeys'] + ] + form = CreateKeyForm(key_names) + if form.validate_on_submit(): + secret = api_key_api_client.create_api_key(service_id=service_id, key_name=form.key_name.data) + return render_template('views/api-keys/show.html', service_id=service_id, secret=secret, + key_name=form.key_name.data) + return render_template( + 'views/api-keys/create.html', + service_id=service_id, + key_name=form.key_name + ) + + +@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, + key_name=key_name + ) + elif request.method == 'POST': + api_key_api_client.revoke_api_key(service_id=service_id, key_id=key_id) + 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 new file mode 100644 index 000000000..660f839c8 --- /dev/null +++ b/app/notify_client/api_key_api_client.py @@ -0,0 +1,27 @@ +from client.base import BaseAPIClient + + +class ApiKeyApiClient(BaseAPIClient): + def __init__(self, base_url=None, client_id=None, secret=None): + super(self.__class__, self).__init__(base_url=base_url or 'base_url', + client_id=client_id or 'client_id', + secret=secret or 'secret') + + def init_app(self, app): + self.base_url = app.config['API_HOST_NAME'] + self.client_id = app.config['ADMIN_CLIENT_USER_NAME'] + self.secret = app.config['ADMIN_CLIENT_SECRET'] + + 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} + key = self.post(url='/service/{}/api-key'.format(service_id), data=data) + 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) diff --git a/app/templates/components/api-key.html b/app/templates/components/api-key.html index 014136bc4..2a2eb2525 100644 --- a/app/templates/components/api-key.html +++ b/app/templates/components/api-key.html @@ -1,4 +1,7 @@ -{% macro api_key(key) %} +{% macro api_key(key, name) %} +

+ {{ name }} +

{{ key }}
diff --git a/app/templates/components/table.html b/app/templates/components/table.html index fb41a6aa7..b5fe9e9d2 100644 --- a/app/templates/components/table.html +++ b/app/templates/components/table.html @@ -55,3 +55,7 @@ {% macro right_aligned_field_heading(text) %} {{ text }} {%- endmacro %} + +{% macro hidden_field_heading(text) %} + {{ text }} +{%- endmacro %} diff --git a/app/templates/main_nav.html b/app/templates/main_nav.html index 65cc28c57..d78cdd3ca 100644 --- a/app/templates/main_nav.html +++ b/app/templates/main_nav.html @@ -9,7 +9,8 @@
  • Templates
    • Manage users
      • diff --git a/app/templates/views/api-keys.html b/app/templates/views/api-keys.html index 7cc837971..6a65eae71 100644 --- a/app/templates/views/api-keys.html +++ b/app/templates/views/api-keys.html @@ -1,6 +1,5 @@ {% extends "withnav_template.html" %} -{% from "components/page-footer.html" import page_footer %} -{% from "components/api-key.html" import api_key %} +{% from "components/table.html" import list_table, field, hidden_field_heading %} {% block page_title %} GOV.UK Notify | API keys and documentation @@ -8,49 +7,51 @@ {% block maincolumn_content %} -
      -
      +

      + API keys +

      -

      - API keys and documentation -

      +

      + To connect to the API you will need to send your service ID, encrypted with + an API key. The API key stays secret. +

      -

      - How to integrate GOV.UK Notify into your service -

      +

      + There are client libraries available which can do this for you. See + the + developer documentation for more information. +

      -

      - blah blah blah this is where we tell you how the API works -

      +

      + Service ID +

      +

      + {{ service_id }} +

      -

      Repositories

      + {% call(item) list_table( + keys, + empty_message="You haven’t created any API keys yet", + caption="API keys", + caption_visible=False, + field_headings=['Key name', hidden_field_heading('Action')] + ) %} + {% call field() %} + {{ item.name }} + {% endcall %} + {% if item.expiry_date %} + {% call field(align='right', status='default') %} + Revoked {{ item.expiry_date|format_datetime }} + {% endcall %} + {% else %} + {% call field(align='right', status='error') %} + Revoke + {% endcall %} + {% endif %} + {% endcall %} - - -

      - GOV.UK Notify API -

      - -

      - GOV.UK Notify Python client -

      - -

      API key for [service name]

      - - {{ api_key('d30512af92e1386d63b90e5973b49a10') }} - -

      API endpoint

      - -

      - https://www.notify.works/api/endpoint -

      - -
      -
      - - {{ page_footer( - back_link=url_for('.service_dashboard', service_id=service_id), - back_link_text='Back to dashboard' - ) }} +

      + Create a new API key +

      {% endblock %} diff --git a/app/templates/views/api-keys/create.html b/app/templates/views/api-keys/create.html new file mode 100644 index 000000000..570a54194 --- /dev/null +++ b/app/templates/views/api-keys/create.html @@ -0,0 +1,24 @@ +{% extends "withnav_template.html" %} +{% from "components/page-footer.html" import page_footer %} +{% from "components/textbox.html" import textbox %} + +{% block page_title %} + GOV.UK Notify | API keys and documentation +{% endblock %} + +{% block maincolumn_content %} + +

      + Add a new API key +

      + +
      + {{ textbox(key_name, hint='eg CRM application') }} + {{ page_footer( + 'Continue', + back_link=url_for('.api_keys', service_id=service_id), + back_link_text='Back to API keys' + ) }} +
      + +{% endblock %} diff --git a/app/templates/views/api-keys/revoke.html b/app/templates/views/api-keys/revoke.html new file mode 100644 index 000000000..abf066c65 --- /dev/null +++ b/app/templates/views/api-keys/revoke.html @@ -0,0 +1,37 @@ +{% extends "withnav_template.html" %} +{% from "components/page-footer.html" import page_footer %} +{% from "components/api-key.html" import api_key %} + +{% block page_title %} + GOV.UK Notify | API keys and documentation +{% endblock %} + +{% block maincolumn_content %} + +
      +
      + +

      + Revoke API key +

      + +

      + ‘{{ key_name }}’ will no longer let you connect to GOV.UK Notify. +

      +

      + You can’t undo this. +

      + +
      +
      + +
      + {{ page_footer( + 'Revoke this API key', + back_link=url_for('.api_keys', service_id=service_id), + back_link_text='Back to API keys', + destructive=True + ) }} +
      + +{% endblock %} diff --git a/app/templates/views/api-keys/show.html b/app/templates/views/api-keys/show.html new file mode 100644 index 000000000..a19a038b2 --- /dev/null +++ b/app/templates/views/api-keys/show.html @@ -0,0 +1,33 @@ +{% extends "withnav_template.html" %} +{% from "components/page-footer.html" import page_footer %} +{% from "components/api-key.html" import api_key %} + +{% block page_title %} + GOV.UK Notify | API keys and documentation +{% endblock %} + +{% block maincolumn_content %} + +
      +
      + +

      + New API key +

      + +

      + Copy your key to somewhere safe. You won’t be able to see it again + once you leave this page. +

      + + {{ api_key(secret, key_name) }} + +
      +
      + + {{ page_footer( + back_link=url_for('.api_keys', service_id=service_id), + back_link_text='Back to API keys' + ) }} + +{% endblock %} diff --git a/app/templates/views/documentation.html b/app/templates/views/documentation.html new file mode 100644 index 000000000..944c84b5c --- /dev/null +++ b/app/templates/views/documentation.html @@ -0,0 +1,49 @@ +{% extends "withnav_template.html" %} +{% from "components/page-footer.html" import page_footer %} +{% from "components/api-key.html" import api_key %} + +{% block page_title %} + GOV.UK Notify | API keys and documentation +{% endblock %} + +{% block maincolumn_content %} + +
      +
      + +

      + Developer documentation +

      + +

      + How to integrate GOV.UK Notify into your service +

      + +

      + blah blah blah this is where we tell you how the API works +

      + +

      Repositories

      + +

      + GOV.UK Notify API +

      + +

      + GOV.UK Notify Python client +

      + +

      API endpoint

      + +

      + https://www.notify.works/api/endpoint +

      + +

      + API keys for your service +

      + +
      +
      + +{% endblock %} 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/test_create_api_key_form.py b/tests/app/main/test_create_api_key_form.py new file mode 100644 index 000000000..c19114b58 --- /dev/null +++ b/tests/app/main/test_create_api_key_form.py @@ -0,0 +1,16 @@ +from werkzeug.datastructures import MultiDict + +from app.main.forms import CreateKeyForm + + +def test_return_validation_error_when_key_name_exists(app_, + db_, + db_session): + def _get_names(): + return ['some key', 'another key'] + + with app_.test_request_context(): + form = CreateKeyForm(_get_names(), + formdata=MultiDict([('key_name', 'Some key')])) + form.validate() + assert {'key_name': ['A key with this name already exists']} == form.errors diff --git a/tests/app/main/views/test_api_keys.py b/tests/app/main/views/test_api_keys.py index 226c06a9a..71eed1353 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 @@ -7,9 +8,116 @@ def test_should_show_api_keys_and_documentation_page(app_, mock_api_user, mock_user_loader, mock_user_dao_get_by_email): + with app_.test_request_context(): + with app_.test_client() as client: + client.login(mock_api_user) + response = client.get(url_for('main.documentation', service_id=123)) + + assert response.status_code == 200 + + +def test_should_show_empty_api_keys_page(app_, + db_, + db_session, + mock_api_user, + mock_user_loader, + mock_user_dao_get_by_email, + mock_get_no_api_keys): with app_.test_request_context(): with app_.test_client() as client: client.login(mock_api_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, + mock_api_user, + mock_user_loader, + mock_user_dao_get_by_email, + mock_get_api_keys): + with app_.test_request_context(): + with app_.test_client() as client: + client.login(mock_api_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_, + db_, + db_session, + mock_api_user, + mock_user_loader, + mock_user_dao_get_by_email, + mock_get_api_keys): + with app_.test_request_context(): + with app_.test_client() as client: + client.login(mock_api_user) + response = client.get(url_for('main.create_api_key', service_id=123)) + + assert response.status_code == 200 + + +def test_should_render_show_api_key(app_, + db_, + db_session, + mock_api_user, + mock_user_loader, + mock_user_dao_get_by_email, + mock_create_api_key, + mock_get_api_keys): + with app_.test_request_context(): + with app_.test_client() as client: + client.login(mock_api_user) + 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, + mock_api_user, + mock_user_loader, + mock_user_dao_get_by_email, + mock_get_api_keys): + with app_.test_request_context(): + with app_.test_client() as client: + client.login(mock_api_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, + mock_api_user, + mock_user_loader, + mock_user_dao_get_by_email, + mock_revoke_api_key, + mock_get_api_keys): + with app_.test_request_context(): + with app_.test_client() as client: + client.login(mock_api_user) + response = client.post(url_for('main.revoke_api_key', service_id=123, key_id=321)) + + 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 30c96e081..b713cf664 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, mock_api_user): service_id, "Test Service", [mock_api_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, mock_api_user): service_two = service_json( 2, "service_two", [mock_api_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, mock_api_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) @@ -314,3 +327,43 @@ def mock_user_dao_get_new_password(mocker, mock_api_user): mock_class = mocker.patch('app.main.dao.users_dao.get_user_by_email') mock_class.return_value = mock_api_user return mock_class + + +@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