Merge pull request #968 from alphagov/new-api-key-format

Present API key as a combination of secret and service ID

app/main/views/api_keys.py

@@ -3,7 +3,7 @@ from flask_login import login_required
 from app.main import main
 from app.main.forms import CreateKeyForm, Whitelist
 from app import api_key_api_client, service_api_client, notification_api_client, current_service
-from app.utils import user_has_permissions
+from app.utils import user_has_permissions, email_safe
 from app.notify_client.api_key_api_client import KEY_TYPE_NORMAL, KEY_TYPE_TEST, KEY_TYPE_TEAM
 
 
@@ -80,8 +80,12 @@ def create_api_key(service_id):
             key_name=form.key_name.data,
             key_type=form.key_type.data
         )
-        return render_template('views/api/keys/show.html', secret=secret,
-                               key_name=form.key_name.data)
+        return render_template(
+            'views/api/keys/show.html',
+            secret=secret,
+            service_id=service_id,
+            key_name=email_safe(form.key_name.data, whitespace='_')
+        )
     return render_template(
         'views/api/keys/create.html',
         form=form

app/templates/views/api/keys.html

@@ -19,7 +19,6 @@
       <a href="{{ url_for('.create_api_key', service_id=current_service.id) }}" class="button align-with-heading">Create an API key</a>
     </div>
   </div>
-
   {% call(item, row_number) list_table(
     keys,
     empty_message="You haven’t created any API keys yet",
@@ -47,7 +46,7 @@
     {% endcall %}
     {% if item.expiry_date %}
       {% call field(align='right') %}
-        Revoked {{ item.expiry_date|format_datetime_short }}
+        <span class='hint'>Revoked {{ item.expiry_date|format_datetime_short }}</span>
       {% endcall %}
     {% else %}
       {% call field(align='right', status='error') %}
@@ -56,10 +55,6 @@
     {% endif %}
   {% endcall %}
 
-  <div class="bottom-gutter-2">
-    {{ api_key(current_service.id, "Service ID", thing="service ID") }}
-  </div>
-
   {{ page_footer(
     secondary_link=url_for('.api_integration', service_id=current_service.id),
     secondary_link_text='Back to API integration'

app/templates/views/api/keys/show.html

@@ -18,11 +18,31 @@
       once you leave this page.
     </p>
 
-    {{ api_key(secret, key_name) }}
+    <div class="bottom-gutter-2">
 
-    {{ page_footer(
-      secondary_link=url_for('.api_keys', service_id=current_service.id),
-      secondary_link_text='Back to API keys'
-    ) }}
+      {{ api_key(
+        '{}-{}-{}'.format(key_name, service_id, secret),
+        'API key'
+      ) }}
+
+      {{ page_footer(
+        secondary_link=url_for('.api_keys', service_id=current_service.id),
+        secondary_link_text='Back to API keys'
+      ) }}
+
+    </div>
+
+    <h2 class='heading-medium'>For older API clients</h2>
+
+    <p>
+      If the client you’re using needs a service ID and an API key,
+      use these values:
+    </p>
+
+    <div class="bottom-gutter">
+      {{ api_key(service_id, 'Service ID', thing='service ID') }}
+    </div>
+
+    {{ api_key(secret, 'API key') }}
 
 {% endblock %}

app/utils.py

@@ -134,10 +134,10 @@ def generate_previous_next_dict(view, service_id, page, title, url_args):
     }
 
 
-def email_safe(string):
+def email_safe(string, whitespace='.'):
     return "".join([
-        character.lower() if character.isalnum() or character == "." else ""
-        for character in re.sub(r"\s+", ".", string.strip())
+        character.lower() if character.isalnum() or character == whitespace else ""
+        for character in re.sub(r"\s+", whitespace, string.strip())
     ])
 
 

tests/app/main/views/test_api_keys.py

@@ -159,15 +159,23 @@ def test_should_create_api_key_with_type_normal(app_,
         response = client.post(
             url_for('main.create_api_key', service_id=service_id),
             data={
-                'key_name': 'some default key name',
+                'key_name': 'Some default key name 1/2',
                 'key_type': 'normal'
             }
         )
 
     assert response.status_code == 200
-    assert 'some default key name' in response.get_data(as_text=True)
+    page = BeautifulSoup(response.data.decode('utf-8'), 'html.parser')
+    keys = page.find_all('span', {'class': 'api-key-key'})
+    for index, key in enumerate([
+        'some_default_key_name_12-{}-{}'.format(service_id, fake_uuid),
+        service_id,
+        fake_uuid
+    ]):
+        assert keys[index].text.strip() == key
+
     post.assert_called_once_with(url='/service/{}/api-key'.format(service_id), data={
-        'name': 'some default key name',
+        'name': 'Some default key name 1/2',
         'key_type': 'normal',
         'created_by': api_user_active.id
     })