Merge lp:~canonical-isd-hackers/canonical-identity-provider/700496-forget-pw-unkown-email into lp:canonical-identity-provider/release

[Simon Davy]

Fix for #700496 - send invitation email when a user does forgot password flow with invalid email

Fix for #700496 - send invitation email when a user does forgot password flow with invalid email

[David Owen (dsowen)]

Impl. looks good. I wonder if it would be better to actually initiate the registration process for the user, and send them a token/link instead of just an invitation?

[David Owen (dsowen)]

Invitation-only sounds good; see comments on bug.

review: Approve

=== modified file 'identityprovider/decorators.py'
--- identityprovider/decorators.py	2011-11-17 22:58:56 +0000
+++ identityprovider/decorators.py	2011-12-15 16:33:40 +0000
@@ -17,8 +17,8 @@
 from identityprovider import auth
 from identityprovider.cookies import set_test_cookie, test_cookie_worked
 
-
 def guest_required(func):
+    @functools.wraps(func)
     def _guest_required_decorator(request, *args, **kwargs):
         if request.user.is_authenticated():
             return HttpResponseRedirect('/')

=== added file 'identityprovider/templates/launchpad/email/invitation.txt'
--- identityprovider/templates/launchpad/email/invitation.txt	1970-01-01 00:00:00 +0000
+++ identityprovider/templates/launchpad/email/invitation.txt	2011-12-15 16:33:40 +0000
@@ -0,0 +1,12 @@
+{% load i18n %}
+{% blocktrans %}Hello
+
+You recently requested a password reset for the email account {{ email }} for
+the Launchpad Login Service
+
+This email is not associated with any current accounts. You are very welcome
+to create a Launchpad account - to do so, please go to {{ signup }}
+
+Thanks
+
+The Launchpad Login Service team{% endblocktrans %}

=== added file 'identityprovider/templates/ubuntu/email/invitation.txt'
--- identityprovider/templates/ubuntu/email/invitation.txt	1970-01-01 00:00:00 +0000
+++ identityprovider/templates/ubuntu/email/invitation.txt	2011-12-15 16:33:40 +0000
@@ -0,0 +1,13 @@
+{% load i18n %}
+{% blocktrans %}Hello
+
+You recently requested a password reset for the email account {{ email }} for
+the Ubuntu Single Sign On service.
+
+This email is not associated with any current accounts. You are very welcome
+to create an Ubuntu SSO account - to do so, please go to {{ signup }}
+
+Thanks
+
+The Ubuntu Single Sign On team
+https://login.ubuntu.com/{% endblocktrans %}

=== modified file 'identityprovider/tests/test_loginservice.py'
--- identityprovider/tests/test_loginservice.py	2011-07-15 11:50:53 +0000
+++ identityprovider/tests/test_loginservice.py	2011-12-15 16:33:40 +0000
@@ -36,7 +36,11 @@
         self.assertEquals(response.status_code, 302)
         self.assertEquals(response['location'],
                           'http://testserver/+email-sent')
-        self.assertEquals(len(mail.outbox), 0)
+        self.assertEquals(len(mail.outbox), 1)
+        self.assertEquals(mail.outbox[0].subject,
+            u"%s: Password reset request" % current_brand.name)
+        self.assertTrue('[e-mail]' in mail.outbox[0].body)
+        self.assertTrue('+new_account' in mail.outbox[0].body)
 
 
 class ForgottenPasswordTest(SSOBaseTestCase):

=== modified file 'identityprovider/urls.py'
--- identityprovider/urls.py	2011-05-11 15:46:48 +0000
+++ identityprovider/urls.py	2011-12-15 16:33:40 +0000
@@ -29,7 +29,10 @@
 urlpatterns += patterns('identityprovider.views.ui',
     (r'^%(optional_token)s\+login$' % repls, 'login'),
     (r'^%(optional_token)s\+logout$' % repls, 'logout'),
-    (r'^%(optional_token)s\+new_account$' % repls, 'new_account'),
+    # expanded new_account urls these as per comment below to allow for 
+    # reverse look up
+    (r'^\+new_account$' % repls, 'new_account'),
+    (r'^%(token)s\+new_account$' % repls, 'new_account'),
     (r'^%(optional_token)s\+forgot_password$' % repls, 'forgot_password'),
     (r'^%(optional_token)s\+enter_token$' % repls, 'enter_token'),
     (r'^token/%(authtoken)s/$' % repls, 'claim_token'),

=== modified file 'identityprovider/views/ui.py'
--- identityprovider/views/ui.py	2011-11-29 15:51:53 +0000
+++ identityprovider/views/ui.py	2011-12-15 16:33:40 +0000
@@ -660,6 +660,60 @@
     return render_to_response('account/suspended.html', context)
 
 
+def _handle_reset_request(request, email, token):
+    # options for actions to be taken
+    NONE, RESET, CREATE = range(3)
+    # assume invalid
+    send_email = NONE
+    # looks like we need this complexity below because it seems the correct
+    # action can by indicated by both exceptions AND flags :/
+    try:
+        person, account = get_person_and_account_by_email(email)
+        if account:
+            if account.can_reset_password:
+                send_email = RESET
+            else:
+                send_email = NONE
+                # log why email was not sent
+                condition = ("account '%s' is not active" %
+                            account.displayname)
+                logger.debug("In view 'forgot_password' email was not "
+                            "sent out because %s" % condition)
+        else:
+            send_email = CREATE
+    except CannotResetPasswordException:
+        send_email = NONE
+    except PersonAndAccountNotFoundException:
+        send_email = CREATE
+
+    # we should know now the action we need to take
+    if send_email == RESET:
+        # send reset email
+        if token is not None:
+            # Redirected from an OpenID request, must get back after
+            # confirmation
+            redirection_url = reverse(
+                'identityprovider.views.server.decide',
+                kwargs={'token': token})
+        else:
+            redirection_url = '/'
+        authtoken = AuthTokenFactory().new(
+            account, email, email, LoginTokenType.PASSWORDRECOVERY,
+            redirection_url=redirection_url)
+        authtoken.sendPasswordResetEmail()
+        # TODO: remove this if pos?
+        set_session_token_info(request.session, authtoken)
+    elif send_email == CREATE:
+        # they've tried to reset with an invalid email, so send them an email
+        # on how to create an account
+        brand = unicode(current_brand.name)
+        subject = u"%s: %s" % (brand, u"Password reset request")
+        context = { 'email': email, 'signup': reverse(new_account) }
+        tmpl = '%s/email/invitation.txt' % current_brand.template_dir
+        body = render_to_string(tmpl, context)
+        send_mail(subject, body, settings.NOREPLY_FROM_ADDRESS, [email])
+
+
 @guest_required
 @check_readonly
 @requires_cookies
@@ -687,35 +741,15 @@
                 stats.increment('flows.forgot_password', key='error.captcha',
                     rpconfig=rpconfig)
                 return response
+
             email = form.cleaned_data['email']
-            send_email = True
-            try:
-                person, account = get_person_and_account_by_email(email)
-                if account is not None and not account.can_reset_password:
-                    send_email = False
-                    # log why email was not sent
-                    condition = ("account '%s' is not active" %
-                                 account.displayname)
-                    logger.debug("In view 'forgot_password' email was not "
-                                 "sent out because %s" % condition)
-            except (CannotResetPasswordException,
-                    PersonAndAccountNotFoundException):
-                send_email = False
-            view, args, kwargs = resolve(request.get_full_path())
-            if send_email:
-                if token is not None:
-                    # Redirected from an OpenID request, must get back after
-                    # confirmation
-                    redirection_url = reverse(
-                        'identityprovider.views.server.decide',
-                        kwargs={'token': token})
-                else:
-                    redirection_url = '/'
-                authtoken = AuthTokenFactory().new(
-                    account, email, email, LoginTokenType.PASSWORDRECOVERY,
-                    redirection_url=redirection_url)
-                authtoken.sendPasswordResetEmail()
-                set_session_token_info(request.session, authtoken)
+
+            # handle the reset request
+            _handle_reset_request(request, email, token)
+
+            #regardless of result, we always display the same information back
+            # to the user
+            # TODO: remove this from session and display via direct render?
             request.session['email_feedback'] = settings.FEEDBACK_TO_ADDRESS
             request.session['email_heading'] = _("Forgotten your password?")
             request.session['email_reason'] = _("We’ve just emailed "

=== modified file 'requirements.txt'
--- requirements.txt	2011-12-02 20:39:13 +0000
+++ requirements.txt	2011-12-15 16:33:40 +0000
@@ -7,7 +7,7 @@
 svn+http://django-saml2-idp.googlecode.com/svn/tags/0.14#egg=saml2idp
 
 # testing dependencies
-BeautifulSoup==3.1.0.1
+BeautifulSoup
 NoseDjango==0.8.1
 nosexcover
 coverage==3.4