From 88e2cc93dfbdef74af501c1d0874ff15270f674b Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Thu, 27 May 2021 17:29:28 +0100 Subject: [PATCH 1/7] Add image of security key MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When referring to something that’s not part of the Notify system, like a spreadsheet or a paper letter or a security key we’ve found it’s helpful to give people a visual representation of it. This commit does the same for security keys. --- app/assets/images/security-key.svg | 73 +++++++++++++++++++ .../views/user-profile/security-keys.html | 62 +++++++++++----- 2 files changed, 117 insertions(+), 18 deletions(-) create mode 100644 app/assets/images/security-key.svg diff --git a/app/assets/images/security-key.svg b/app/assets/images/security-key.svg new file mode 100644 index 000000000..f848b7a89 --- /dev/null +++ b/app/assets/images/security-key.svg @@ -0,0 +1,73 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/app/templates/views/user-profile/security-keys.html b/app/templates/views/user-profile/security-keys.html index c1df142a8..d9822f041 100644 --- a/app/templates/views/user-profile/security-keys.html +++ b/app/templates/views/user-profile/security-keys.html @@ -18,14 +18,16 @@ {% endblock %} {% block maincolumn_content %} - {{ page_header( - page_title, - back_link=url_for('.user_profile') - ) }} -
-
- {% if credentials %} + + {{ govukBackLink({ "href": url_for('.user_profile') }) }} +
+ {% if credentials %} +
+ {{ page_header( + page_title, + back_link=url_for('.user_profile') + ) }}
{% call mapping_table( caption=page_title, @@ -45,11 +47,44 @@ {% endcall %}
- {% else %} + {{ govukButton({ + "element": "button", + "text": "Register a key", + "classes": "govuk-button--secondary webauthn__api-required", + "attributes": { + "data-module": "register-security-key", + "data-csrf-token": csrf_token(), + } + }) }} +
+ {% else %} +
+ {{ page_header(page_title) }}

- Security keys are an alternative way of signing in to Notify. + Security keys are an alternative way of signing in to Notify, + instead of getting a code in a text message

+

+ You can buy any key that’s compatible with the WebAuthn + standard. +

+ {{ govukButton({ + "element": "button", + "text": "Register a key", + "classes": "govuk-button--secondary webauthn__api-required", + "attributes": { + "data-module": "register-security-key", + "data-csrf-token": csrf_token(), + } + }) }} +
+
+ +
+
+
+
{% endif %} @@ -63,15 +98,6 @@ "text": "JavaScript is not available for this page. Security keys need JavaScript to work." }) }} - {{ govukButton({ - "element": "button", - "text": "Register a key", - "classes": "govuk-button--secondary webauthn__api-required", - "attributes": { - "data-module": "register-security-key", - "data-csrf-token": csrf_token(), - } - }) }}
{% endblock %} From 7f88aa67590d613ee3d26e5d64352515d8482b9a Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Thu, 27 May 2021 17:38:24 +0100 Subject: [PATCH 2/7] Make listing of keys on user profile a bit nicer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We can use the `optional_text_field` macro to grey out the text when nothing is set up. And adding ‘registered’ makes the language consistent through to the next page. --- app/templates/views/user-profile.html | 5 ++++- tests/app/main/views/test_user_profile.py | 13 +++++++++---- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/app/templates/views/user-profile.html b/app/templates/views/user-profile.html index 25f60cf2f..4044a707c 100644 --- a/app/templates/views/user-profile.html +++ b/app/templates/views/user-profile.html @@ -48,7 +48,10 @@ {% if current_user.platform_admin %} {% call row(id='security-keys') %} {{ text_field('Security keys') }} - {{ text_field(current_user.webauthn_credentials|length) }} + {{ optional_text_field( + ('{} registered'.format(current_user.webauthn_credentials|length)) if current_user.webauthn_credentials else None, + default='None registered' + ) }} {{ edit_field('Change', url_for('.user_profile_security_keys')) }} {% endcall %} {% endif %} diff --git a/tests/app/main/views/test_user_profile.py b/tests/app/main/views/test_user_profile.py index 277a70ef9..e2cac7438 100644 --- a/tests/app/main/views/test_user_profile.py +++ b/tests/app/main/views/test_user_profile.py @@ -37,20 +37,25 @@ def test_overview_page_shows_disable_for_platform_admin( assert ' '.join(disable_platform_admin_row.text.split()) == 'Use platform admin view Yes Change' -@pytest.mark.parametrize('has_keys', [False, True]) +@pytest.mark.parametrize('key_count, expected_row_text', [ + (0, 'Security keys None registered Change'), + (1, 'Security keys 1 registered Change'), + (2, 'Security keys 2 registered Change'), +]) def test_overview_page_shows_security_keys_for_platform_admin( mocker, client_request, platform_admin_user, - has_keys, webauthn_credential, + key_count, + expected_row_text, ): client_request.login(platform_admin_user) - credentials = [webauthn_credential] if has_keys else [] + credentials = [webauthn_credential for _ in range(key_count)] mocker.patch('app.user_api_client.get_webauthn_credentials_for_user', return_value=credentials) page = client_request.get('main.user_profile') security_keys_row = page.select_one('#security-keys') - assert ' '.join(security_keys_row.text.split()) == f'Security keys {len(credentials)} Change' + assert ' '.join(security_keys_row.text.split()) == expected_row_text def test_should_show_name_page( From 68e7d2916e5c7c47d58f4d39e14bee581dc9d675 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Fri, 28 May 2021 13:52:05 +0100 Subject: [PATCH 3/7] Fix nesting of if statement --- app/templates/views/user-profile/security-keys.html | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/app/templates/views/user-profile/security-keys.html b/app/templates/views/user-profile/security-keys.html index d9822f041..59b30ce0a 100644 --- a/app/templates/views/user-profile/security-keys.html +++ b/app/templates/views/user-profile/security-keys.html @@ -82,12 +82,11 @@
+ {% endif %}
- {% endif %} - {{ govukErrorMessage({ "classes": "webauthn__api-missing", "text": "Your browser does not support security keys. Try signing in to Notify using a different browser." From 268a7d1881715db395407427c5ff23ccf008beb8 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Fri, 28 May 2021 13:52:20 +0100 Subject: [PATCH 4/7] Make image display smaller on mobile --- app/assets/stylesheets/views/webauthn.scss | 10 ++++++++++ app/templates/views/user-profile/security-keys.html | 2 +- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/app/assets/stylesheets/views/webauthn.scss b/app/assets/stylesheets/views/webauthn.scss index 3e0de527e..7f47049b0 100644 --- a/app/assets/stylesheets/views/webauthn.scss +++ b/app/assets/stylesheets/views/webauthn.scss @@ -23,3 +23,13 @@ display: block; } } + +.webauthn-illustration { + + @include media-down(mobile) { + margin: govuk-spacing(6) govuk-spacing(9) 0 govuk-spacing(9); + } + + margin: govuk-spacing(9) auto 0 auto; + +} diff --git a/app/templates/views/user-profile/security-keys.html b/app/templates/views/user-profile/security-keys.html index 59b30ce0a..fc249b8f1 100644 --- a/app/templates/views/user-profile/security-keys.html +++ b/app/templates/views/user-profile/security-keys.html @@ -80,7 +80,7 @@ }) }}
- +
{% endif %}
From 597846f657b5f8ab6b19be4b9a72ba3ddc6c8f0e Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Fri, 28 May 2021 13:55:31 +0100 Subject: [PATCH 5/7] Refactor error button and error messages into variable --- .../views/user-profile/security-keys.html | 58 ++++++++----------- 1 file changed, 24 insertions(+), 34 deletions(-) diff --git a/app/templates/views/user-profile/security-keys.html b/app/templates/views/user-profile/security-keys.html index fc249b8f1..2025754d8 100644 --- a/app/templates/views/user-profile/security-keys.html +++ b/app/templates/views/user-profile/security-keys.html @@ -19,6 +19,28 @@ {% block maincolumn_content %} +{% set webauthn_button %} + {{ govukButton({ + "element": "button", + "text": "Register a key", + "classes": "govuk-button--secondary webauthn__api-required", + "attributes": { + "data-module": "register-security-key", + "data-csrf-token": csrf_token(), + } + }) }} + + {{ govukErrorMessage({ + "classes": "webauthn__api-missing", + "text": "Your browser does not support security keys. Try signing in to Notify using a different browser." + }) }} + + {{ govukErrorMessage({ + "classes": "webauthn__no-js", + "text": "JavaScript is not available for this page. Security keys need JavaScript to work." + }) }} +{% endset %} + {{ govukBackLink({ "href": url_for('.user_profile') }) }}
@@ -46,17 +68,7 @@ {% endfor %} {% endcall %}
- - {{ govukButton({ - "element": "button", - "text": "Register a key", - "classes": "govuk-button--secondary webauthn__api-required", - "attributes": { - "data-module": "register-security-key", - "data-csrf-token": csrf_token(), - } - }) }} - + {{ webauthn_button }}
{% else %}
@@ -69,34 +81,12 @@ You can buy any key that’s compatible with the WebAuthn standard.

- {{ govukButton({ - "element": "button", - "text": "Register a key", - "classes": "govuk-button--secondary webauthn__api-required", - "attributes": { - "data-module": "register-security-key", - "data-csrf-token": csrf_token(), - } - }) }} + {{ webauthn_button }}
{% endif %} -
-
- - {{ govukErrorMessage({ - "classes": "webauthn__api-missing", - "text": "Your browser does not support security keys. Try signing in to Notify using a different browser." - }) }} - - {{ govukErrorMessage({ - "classes": "webauthn__no-js", - "text": "JavaScript is not available for this page. Security keys need JavaScript to work." - }) }} - -
{% endblock %} From 3f7124b04efe1d114acb27820f6cd0961efd19a9 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Fri, 28 May 2021 13:57:31 +0100 Subject: [PATCH 6/7] Remove uneeded clearfix --- app/templates/views/user-profile/security-keys.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/templates/views/user-profile/security-keys.html b/app/templates/views/user-profile/security-keys.html index 2025754d8..f603efc08 100644 --- a/app/templates/views/user-profile/security-keys.html +++ b/app/templates/views/user-profile/security-keys.html @@ -43,7 +43,7 @@ {{ govukBackLink({ "href": url_for('.user_profile') }) }} -
+
{% if credentials %}
{{ page_header( From 77ea7af90905549fb2c7c674e8c5dd1dcb24950c Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Fri, 28 May 2021 16:19:17 +0100 Subject: [PATCH 7/7] Fix double back link --- app/templates/views/user-profile/security-keys.html | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/app/templates/views/user-profile/security-keys.html b/app/templates/views/user-profile/security-keys.html index f603efc08..95799c710 100644 --- a/app/templates/views/user-profile/security-keys.html +++ b/app/templates/views/user-profile/security-keys.html @@ -46,10 +46,7 @@
{% if credentials %}
- {{ page_header( - page_title, - back_link=url_for('.user_profile') - ) }} + {{ page_header(page_title) }}
{% call mapping_table( caption=page_title,