Merge lp:~maxiberta/canonical-identity-provider/smarter-forget-password-link into lp:canonical-identity-provider

Proposed by Maximiliano Bertacchini on 2019-01-09
Status: Merged
Approved by: Maximiliano Bertacchini on 2019-01-15
Approved revision: 1676
Merge reported by: Otto Co-Pilot
Merged at revision: not available
Proposed branch: lp:~maxiberta/canonical-identity-provider/smarter-forget-password-link
Merge into: lp:canonical-identity-provider
Diff against target: 143 lines (+34/-22)
7 files modified
src/identityprovider/tests/openid_server/per_version/test_sso_workflow_reset_password.py (+6/-9)
src/identityprovider/tests/sso_server/test_prefilled_email.py (+5/-4)
src/webui/templates/common/forgot_password_link.html (+0/-5)
src/webui/templates/registration/_login_form.html (+5/-1)
src/webui/templates/registration/login.html (+1/-1)
src/webui/tests/test_views_registration.py (+14/-0)
src/webui/views/registration.py (+3/-2)
To merge this branch: bzr merge lp:~maxiberta/canonical-identity-provider/smarter-forget-password-link
Reviewer Review Type Date Requested Status
Tom Wardill 2019-01-09 Approve on 2019-01-14
Review via email: mp+361579@code.launchpad.net

Commit message

Pre-fill forgot password form with email value entered in the login form.

To post a comment you must log in.
Maximiliano Bertacchini (maxiberta) wrote :

Some notes:
- Turned login's "Forgot your password?" into <input> so that it can POST the current value of the email field.
- The new <input> link has the "formnovalidate" attribute set to skip client-side form validations, e.g. non-empty password. Thus whatever value is currently set in the email field is POSTed to and validated by the forgot-password view.
- The new <input> link has the "formaction" attribute defined to make its destination URL more explicit and easier to test.
- The new <input> link has some css magic to make it look like an <a>.

Tom Wardill (twom) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/identityprovider/tests/openid_server/per_version/test_sso_workflow_reset_password.py'
2--- src/identityprovider/tests/openid_server/per_version/test_sso_workflow_reset_password.py 2018-02-08 15:12:52 +0000
3+++ src/identityprovider/tests/openid_server/per_version/test_sso_workflow_reset_password.py 2019-01-09 18:49:43 +0000
4@@ -21,9 +21,8 @@
5 # At this point, we are at the login page. Lets try to recover a
6 # password for an unregistered email.
7 link = self.get_attribute_from_response(
8- response,
9- 'a[data-qa-id="forgot_password_link"]',
10- 'href')
11+ response, 'input[name="forgot_password"]', 'formaction')
12+
13 data = dict(email='no-account@example.com')
14 response = self.client.post(link, data=data, follow=True)
15
16@@ -36,9 +35,8 @@
17 response = self.do_openid_dance()
18
19 link = self.get_attribute_from_response(
20- response,
21- 'a[data-qa-id="forgot_password_link"]',
22- 'href')
23+ response, 'input[name="forgot_password"]', 'formaction')
24+
25 data = dict(email='support@ubuntu.com')
26 response = self.client.post(link, data=data, follow=True)
27
28@@ -51,9 +49,8 @@
29
30 response = self.do_openid_dance()
31 link = self.get_attribute_from_response(
32- response,
33- 'a[data-qa-id="forgot_password_link"]',
34- 'href')
35+ response, 'input[name="forgot_password"]', 'formaction')
36+
37 data = dict(email=self.default_email)
38 response = self.client.post(link, data=data, follow=True)
39
40
41=== modified file 'src/identityprovider/tests/sso_server/test_prefilled_email.py'
42--- src/identityprovider/tests/sso_server/test_prefilled_email.py 2013-11-12 13:44:06 +0000
43+++ src/identityprovider/tests/sso_server/test_prefilled_email.py 2019-01-09 18:49:43 +0000
44@@ -13,11 +13,12 @@
45 """Forgot Password form."""
46 response = self.login(email=self.new_email)
47
48+ email = self.get_attribute_from_response(
49+ response, 'input[name="email"]', 'value')
50 link = self.get_attribute_from_response(
51- response,
52- 'a[data-qa-id="forgot_password_link"]',
53- 'href')
54+ response, 'input[name="forgot_password"]', 'formaction')
55
56- response = self.client.get(link)
57+ response = self.client.post(
58+ link, data={'email': email, 'forgot_password': ''})
59 email_field = self.get_from_response(response, 'input[name="email"]')
60 self.assertEqual(email_field[0].get('value'), self.new_email)
61
62=== removed file 'src/webui/templates/common/forgot_password_link.html'
63--- src/webui/templates/common/forgot_password_link.html 2017-02-26 01:29:52 +0000
64+++ src/webui/templates/common/forgot_password_link.html 1970-01-01 00:00:00 +0000
65@@ -1,5 +0,0 @@
66-{% load i18n url_with_token %}
67-{% if not readonly %}
68- <a href="{% url_with_token 'forgot_password' %}{% if form.email.data %}?email={{ form.email.data|urlencode }}{% endif %}"
69- id="forgotpw-link" data-qa-id="forgot_password_link">{% trans "Forgot your password?" %}</a>
70-{% endif %}
71
72=== modified file 'src/webui/templates/registration/_login_form.html'
73--- src/webui/templates/registration/_login_form.html 2017-02-26 01:29:52 +0000
74+++ src/webui/templates/registration/_login_form.html 2019-01-09 18:49:43 +0000
75@@ -43,7 +43,11 @@
76 <button type="submit" class="btn cta" name="continue" data-qa-id="login_button">
77 <span>{% trans "Log in" %}</span>
78 </button>
79- <p class="forgot-password">{% include "common/forgot_password_link.html" %}</p>
80+ {% if not readonly %}
81+ <input type="submit" class="forgot-password" name="forgot_password" formnovalidate formaction="{% url_with_token 'forgot_password' %}"
82+ style="border:none; outline:none; background:none; cursor:pointer; color:#dd4814;"
83+ value="{% trans 'Forgot your password?' %}">
84+ {% endif %}
85
86 {% comment %}
87 {% if token %}{% trans "or" %}
88
89=== modified file 'src/webui/templates/registration/login.html'
90--- src/webui/templates/registration/login.html 2014-12-10 15:30:54 +0000
91+++ src/webui/templates/registration/login.html 2019-01-09 18:49:43 +0000
92@@ -26,7 +26,7 @@
93 <h1 class="u1-h-main">{% trans "One account to log in to everything on Ubuntu" %}</h1>
94 {% endblock %}
95
96-/div>
97+</div>
98
99 {% block extra_css %}
100 {% if rpconfig and rpconfig.logo_url %}
101
102=== modified file 'src/webui/tests/test_views_registration.py'
103--- src/webui/tests/test_views_registration.py 2018-12-20 18:23:07 +0000
104+++ src/webui/tests/test_views_registration.py 2019-01-09 18:49:43 +0000
105@@ -496,6 +496,20 @@
106 ctx = response.context_data
107 self.assertEqual(ctx['form']['email'].value(), 'test@test.com')
108
109+ def test_post_with_initial_data(self):
110+ data = dict(email='test@test.com', forgot_password='')
111+ response = self.post(data=data)
112+ self.assert_form_displayed(response)
113+ ctx = response.context_data
114+ self.assertEqual(ctx['form']['email'].value(), 'test@test.com')
115+
116+ def test_post_with_initial_data_no_email(self):
117+ data = dict(forgot_password='')
118+ response = self.post(data=data)
119+ self.assert_form_displayed(response)
120+ ctx = response.context_data
121+ self.assertIsNone(ctx['form']['email'].value())
122+
123 def test_post_required_fields(self):
124 response = self.post()
125 self.assert_form_displayed(response, email=FIELD_REQUIRED)
126
127=== modified file 'src/webui/views/registration.py'
128--- src/webui/views/registration.py 2018-12-20 18:23:07 +0000
129+++ src/webui/views/registration.py 2019-01-09 18:49:43 +0000
130@@ -186,10 +186,11 @@
131 def collect_stats(key):
132 stats.increment('flows.forgot_password', key=key, rpconfig=rpconfig)
133
134- if request.method == 'GET':
135+ if request.method == 'GET' or 'forgot_password' in request.POST:
136 # track forgot password requests
137 collect_stats('requested')
138- form = GenericEmailForm(initial={'email': request.GET.get('email')})
139+ email = request.GET.get('email', request.POST.get('email'))
140+ form = GenericEmailForm(initial={'email': email})
141
142 elif request.method == 'POST':
143 form = GenericEmailForm(request.POST)