Merge lp:~deryck/launchpad/reauth-for-email-363916 into lp:launchpad

[Deryck Hodge]

Require users to re-authenticate when changing email address info.

This branch ensures that a user has to re-authenticate when editing email info. This builds on the work I did in the pre-req branch to add support for PAPE extension's max_auth_age option when logging in.

The work here is really to add a check on the freshness of a login. If the login is less than 2 minutes old, we allow the user to edit his or her email settings. If not, we signal for a fresh login. To do this, the lp session is updated to store the logintime, and then a helper method is added to check the logintime in the session. There is a test added for this. The only other new work is to use this isFreshLogin helper in the view and redirect too +login if a fresher login is required. All the rest is updating doctests to ensure we have a fresh login to avoid the isFreshLogin, and there is a new setupBrowser function to help do this in doctests.

In later branches, I'll apply this for ssh and gpg and will factor out the code in lp.registry.browser.person to a function to ensure_fresh_login. But I thought it was nice to get this reviewed as is, to see how it fits together better.

The work does add about 100 lines of code, but I have preceeding branches that refactored tests to prepare for this work. And in total, I'm still about -200 LOC, even after this branch lands.

There is a bit of lint still in the long subscriptions doctest, but none that I added. I didn't, however, fix the existing lint because the diff would have grown significantly for very minor lint changes (mostly too long lines in the documentation parts of the test).

[Richard Harding (rharding)]

Should the setupBrowserFreshLogin provide the ability to setup based on email/etc to skip some test boilerplate? It could use kwargs to allow passing an email or name for the getByXXX

#515
Can we add the else just to jump out the if condition returning a value

#517
Should the timing be a constant in the module for easy finding/change/use?

review: Approve

[Deryck Hodge (deryck)]

Thanks for the review, Rick!

The test helper is following the pattern of setupBrowserForUser, which takes a real user. Not sure I want to get into all that refactoring. By refactoring, I mean that I don't like those kind of 1/2 refactors where there are now 2 ways to do something. Anyway, it's a good suggestion, but I'll wait on doing it for now.

As for line #515, I think that's just a style preference and we don't have a rule about being explicit there, I don't think. I prefer the return by itself, rather than inside an else statement. Less code and all, for the same result.

For the timing question, I wondered even if it should be a config value, but wasn't sure. So I'll keep thinking on that, but roll on forward for now. If I changed it, I'll do it in one of the follow on branches.

Thanks, again, for the review!

=== modified file 'lib/lp/registry/browser/person.py'
--- lib/lp/registry/browser/person.py	2012-08-07 02:31:56 +0000
+++ lib/lp/registry/browser/person.py	2012-08-07 18:27:30 +0000
@@ -257,7 +257,10 @@
     ILaunchBag,
     IOpenLaunchBag,
     )
-from lp.services.webapp.login import logoutPerson
+from lp.services.webapp.login import (
+    isFreshLogin,
+    logoutPerson,
+    )
 from lp.services.webapp.menu import get_current_view
 from lp.services.webapp.publisher import LaunchpadView
 from lp.services.worlddata.interfaces.country import ICountry
@@ -2810,6 +2813,11 @@
     label = 'Change your e-mail settings'
 
     def initialize(self):
+        if not isFreshLogin(self.request):
+            reauth_query = '+login?reauth=1'
+            base_url = canonical_url(self.context, view_name='+editemails')
+            login_url = '%s/%s' % (base_url, reauth_query)
+            self.request.response.redirect(login_url)
         if self.context.is_team:
             # +editemails is not available on teams.
             name = self.request['PATH_INFO'].split('/')[-1]

=== modified file 'lib/lp/registry/stories/gpg-coc/xx-gpg-coc.txt'
--- lib/lp/registry/stories/gpg-coc/xx-gpg-coc.txt	2012-06-29 14:34:11 +0000
+++ lib/lp/registry/stories/gpg-coc/xx-gpg-coc.txt	2012-08-07 18:27:30 +0000
@@ -141,7 +141,13 @@
 added as an email address:
 
     >>> from lp.testing.pages import strip_label
+    >>> from lp.registry.interfaces.person import IPersonSet
+    >>> from lp.testing.pages import setupBrowserFreshLogin
 
+    >>> login(ANONYMOUS)
+    >>> person = getUtility(IPersonSet).getByEmail('[e-mail]')
+    >>> logout()
+    >>> browser = setupBrowserFreshLogin(person)
     >>> browser.open("http://launchpad.dev/~name12/+editemails")
     >>> control = browser.getControl(name='field.UNVALIDATED_SELECTED')
     >>> [strip_label(label) for label in sorted(control.displayOptions)]

=== modified file 'lib/lp/registry/stories/mailinglists/subscriptions.txt'
--- lib/lp/registry/stories/mailinglists/subscriptions.txt	2012-01-15 11:06:57 +0000
+++ lib/lp/registry/stories/mailinglists/subscriptions.txt	2012-08-07 18:27:30 +0000
@@ -60,8 +60,15 @@
     >>> admin_browser.getControl('New member').value = 'no-team-memberships'
     >>> admin_browser.getControl('Add Member').click()
 
-    >>> no_team_browser = setupBrowser(
-    ...     auth="Basic [e-mail]:test")
+    >>> from lp.testing.pages import setupBrowserFreshLogin
+    >>> from zope.component import getUtility
+    >>> from lp.registry.interfaces.person import IPersonSet
+    >>> login(ANONYMOUS)
+    >>> person = getUtility(IPersonSet).getByEmail(
+    ...     '[e-mail]')
+    >>> logout()
+    >>> no_team_browser = setupBrowserFreshLogin(person)
+
     >>> no_team_browser.open('http://launchpad.dev/people/+me/+editemails')
     >>> rosetta_admins = no_team_browser.getControl(
     ...     name='field.subscription.rosetta-admins')
@@ -77,15 +84,19 @@
 he's a member of.  Mailing lists show up in this list regardless of whether
 it's currently the team contact method.
 
-    >>> browser.open('http://launchpad.dev/~carlos')
-    >>> browser.getLink(url="+editemails").click()
+    >>> login(ANONYMOUS)
+    >>> carlos =  getUtility(IPersonSet).getByName('carlos')
+    >>> logout()
+    >>> carlos_browser = setupBrowserFreshLogin(carlos)
+    >>> carlos_browser.open('http://launchpad.dev/~carlos')
+    >>> carlos_browser.getLink(url="+editemails").click()
 
     >>> from lp.services.helpers import backslashreplace
-    >>> print backslashreplace(browser.title)
+    >>> print backslashreplace(carlos_browser.title)
     Change your e-mail settings...
 
-    >>> admins = browser.getControl(name='field.subscription.admins')
-    >>> rosetta_admins = browser.getControl(
+    >>> admins = carlos_browser.getControl(name='field.subscription.admins')
+    >>> rosetta_admins = carlos_browser.getControl(
     ...     name='field.subscription.rosetta-admins')
 
     >>> admins.displayOptions
@@ -100,7 +111,7 @@
 However, testing-spanish-team's list doesn't show up because its creation has
 not been completed (specifically, Mailman hasn't constructed it yet).
 
-    >>> browser.getControl(name='field.subscription.testing-spanish-team')
+    >>> carlos_browser.getControl(name='field.subscription.testing-spanish-team')
     Traceback (most recent call last):
     ...
     LookupError: name 'field.subscription.testing-spanish-team'
@@ -110,16 +121,16 @@
 him to update his subscription.  So this is not the same as subscribing
 explicitly with whatever is his preferred email address.
 
-    >>> admins = browser.getControl(name='field.subscription.admins')
+    >>> admins = carlos_browser.getControl(name='field.subscription.admins')
     >>> admins.value = ['Preferred address']
-    >>> browser.getControl('Update Subscriptions').click()
+    >>> carlos_browser.getControl('Update Subscriptions').click()
 
-    >>> for msg in get_feedback_messages(browser.contents):
+    >>> for msg in get_feedback_messages(carlos_browser.contents):
     ...     print msg
     Subscriptions updated.
 
-    >>> admins = browser.getControl(name='field.subscription.admins')
-    >>> rosetta_admins = browser.getControl(
+    >>> admins = carlos_browser.getControl(name='field.subscription.admins')
+    >>> rosetta_admins = carlos_browser.getControl(
     ...     name='field.subscription.rosetta-admins')
     >>> print admins.value
     ['Preferred address']
@@ -131,10 +142,10 @@
 
     >>> admins.value = ['[e-mail]']
     >>> rosetta_admins.value = ['[e-mail]']
-    >>> browser.getControl('Update Subscriptions').click()
+    >>> carlos_browser.getControl('Update Subscriptions').click()
 
-    >>> admins = browser.getControl(name='field.subscription.admins')
-    >>> rosetta_admins = browser.getControl(
+    >>> admins = carlos_browser.getControl(name='field.subscription.admins')
+    >>> rosetta_admins = carlos_browser.getControl(
     ...     name='field.subscription.rosetta-admins')
     >>> print admins.value
     ['[e-mail]']
@@ -146,10 +157,10 @@
 
     >>> admins.value = ['Preferred address']
     >>> rosetta_admins.value = ['[e-mail]']
-    >>> browser.getControl('Update Subscriptions').click()
+    >>> carlos_browser.getControl('Update Subscriptions').click()
 
-    >>> admins = browser.getControl(name='field.subscription.admins')
-    >>> rosetta_admins = browser.getControl(
+    >>> admins = carlos_browser.getControl(name='field.subscription.admins')
+    >>> rosetta_admins = carlos_browser.getControl(
     ...     name='field.subscription.rosetta-admins')
     >>> print admins.value
     ['Preferred address']
@@ -159,15 +170,15 @@
 Finally, he can unsubscribe from any mailing list by setting the subscription
 menu item to "Don't subscribe".
 
-    >>> admins = browser.getControl(name='field.subscription.admins')
-    >>> rosetta_admins = browser.getControl(
+    >>> admins = carlos_browser.getControl(name='field.subscription.admins')
+    >>> rosetta_admins = carlos_browser.getControl(
     ...     name='field.subscription.rosetta-admins')
     >>> admins.value = ["Don't subscribe"]
     >>> rosetta_admins.value = ["Don't subscribe"]
-    >>> browser.getControl('Update Subscriptions').click()
+    >>> carlos_browser.getControl('Update Subscriptions').click()
 
-    >>> admins = browser.getControl(name='field.subscription.admins')
-    >>> rosetta_admins = browser.getControl(
+    >>> admins = carlos_browser.getControl(name='field.subscription.admins')
+    >>> rosetta_admins = carlos_browser.getControl(
     ...     name='field.subscription.rosetta-admins')
     >>> print admins.value
     ["Don't subscribe"]
@@ -186,12 +197,12 @@
 to.  We will use Carlos, as he is an administrator for the Rosetta
 Admins team, and he should know if the list is available.
 
-    >>> browser.open('http://launchpad.dev/~carlos')
-    >>> browser.getLink(url="+editemails").click()
-    >>> print backslashreplace(browser.title)
+    >>> carlos_browser.open('http://launchpad.dev/~carlos')
+    >>> carlos_browser.getLink(url="+editemails").click()
+    >>> print backslashreplace(carlos_browser.title)
     Change your e-mail settings...
 
-    >>> rosetta_admins = browser.getControl(
+    >>> rosetta_admins = carlos_browser.getControl(
     ...     name='field.subscription.rosetta-admins')
     >>> rosetta_admins.displayOptions
     ['Preferred address', "Don't subscribe",
@@ -220,12 +231,16 @@
 the list.  The list does not show up on his Subscription Management
 screen.
 
-    >>> browser.open('http://launchpad.dev/~jdub')
-    >>> browser.getLink(url="+editemails").click()
-    >>> print browser.title
+    >>> login(ANONYMOUS)
+    >>> jdub = getUtility(IPersonSet).getByName('jdub')
+    >>> logout()
+    >>> jdub_browser = setupBrowserFreshLogin(jdub)
+    >>> jdub_browser.open('http://launchpad.dev/~jdub')
+    >>> jdub_browser.getLink(url="+editemails").click()
+    >>> print jdub_browser.title
     Change your e-mail settings...
 
-    >>> browser.getControl(
+    >>> jdub_browser.getControl(
     ...     name='field.subscription.rosetta-admins')
     Traceback (most recent call last):
     ...
@@ -244,12 +259,12 @@
 
 His mailing list subscription is now available to be managed.
 
-    >>> browser.open('http://launchpad.dev/~jdub')
-    >>> browser.getLink(url="+editemails").click()
-    >>> print browser.title
+    >>> jdub_browser.open('http://launchpad.dev/~jdub')
+    >>> jdub_browser.getLink(url="+editemails").click()
+    >>> print jdub_browser.title
     Change your e-mail settings...
 
-    >>> rosetta_team = browser.getControl(
+    >>> rosetta_team = jdub_browser.getControl(
     ...     name='field.subscription.rosetta-admins')
 
     >>> rosetta_team.displayOptions
@@ -309,60 +324,60 @@
 Carlos can see the subscribe link on the admin team's Overview
 page, because he is not subscribed to the team mailing list.
 
-    >>> browser = setupBrowser(auth='Basic [e-mail]:test')
-    >>> browser.open('http://launchpad.dev/~admins')
-    >>> browser.getLink('Subscribe to mailing list').click()
-    >>> print browser.url
+    >>> carlos_browser.open('http://launchpad.dev/~admins')
+    >>> carlos_browser.getLink('Subscribe to mailing list').click()
+    >>> print carlos_browser.url
     http://launchpad.dev/~carlos/+editemails
 
 The unsubscribe link is visible for the rosetta admins team, which
 has an active mailing list.
 
     # Subscribe to the list using the normal technique.
-    >>> browser.open('http://launchpad.dev/~carlos')
-    >>> browser.getLink(url="+editemails").click()
-    >>> rosetta_admins = browser.getControl(
+    >>> carlos_browser.open('http://launchpad.dev/~carlos')
+    >>> carlos_browser.getLink(url="+editemails").click()
+    >>> rosetta_admins = carlos_browser.getControl(
     ...     name='field.subscription.rosetta-admins')
     >>> rosetta_admins.value = ['Preferred address']
-    >>> browser.getControl('Update Subscriptions').click()
+    >>> carlos_browser.getControl('Update Subscriptions').click()
     >>> print rosetta_admins.value
     ['Preferred address']
-    >>> for tag in find_tags_by_class(browser.contents, 'informational'):
+    >>> for tag in find_tags_by_class(
+    ...     carlos_browser.contents, 'informational'):
     ...     print tag.renderContents()
     Subscriptions updated.
 
-    >>> browser.open('http://launchpad.dev/~rosetta-admins')
-    >>> browser.getControl('Unsubscribe')
+    >>> carlos_browser.open('http://launchpad.dev/~rosetta-admins')
+    >>> carlos_browser.getControl('Unsubscribe')
     <SubmitControl name='unsubscribe' type='submit'>
 
 Clicking the link will unsubscribe you from the list immediately.
 
-    >>> browser.getControl('Unsubscribe').click()
-    >>> print get_feedback_messages(browser.contents)
+    >>> carlos_browser.getControl('Unsubscribe').click()
+    >>> print get_feedback_messages(carlos_browser.contents)
     [u'You have been unsubscribed from the team mailing list.']
 
-    >>> browser.open('http://launchpad.dev/~rosetta-admins')
+    >>> carlos_browser.open('http://launchpad.dev/~rosetta-admins')
     >>> print extract_text(
-    ...     find_tag_by_id(browser.contents, 'mailing-lists'))
+    ...     find_tag_by_id(carlos_browser.contents, 'mailing-lists'))
     Mailing list...
     Subscribe to mailing list...
 
 The Ubuntu translators team, which does not have any lists configured,
 does not show either link.
 
-    >>> browser.open('http://launchpad.dev/~ubuntu-translators')
+    >>> carlos_browser.open('http://launchpad.dev/~ubuntu-translators')
     >>> print extract_text(
-    ...     find_portlet(browser.contents, 'Mailing list'))
+    ...     find_portlet(carlos_browser.contents, 'Mailing list'))
     Mailing list
     This team does not use Launchpad to host a mailing list.
     Create a mailing list
 
-    >>> browser.getLink('Subscribe')
+    >>> carlos_browser.getLink('Subscribe')
     Traceback (most recent call last):
     ...
     LinkNotFoundError
 
-    >>> browser.getLink('Unsubscribe')
+    >>> carlos_browser.getLink('Unsubscribe')
     Traceback (most recent call last):
     ...
     LinkNotFoundError
@@ -375,9 +390,9 @@
 mailing list subscribers, if there is an active mailing list.  The
 rosetta admins team has such a list and carlos is the owner.
 
-    >>> browser.open('http://launchpad.dev/~rosetta-admins')
+    >>> carlos_browser.open('http://launchpad.dev/~rosetta-admins')
     >>> print extract_text(
-    ...     find_portlet(browser.contents, 'Mailing list'))
+    ...     find_portlet(carlos_browser.contents, 'Mailing list'))
     Mailing list
     [e-mail]
     Policy: You must be a team member to subscribe to the team mailing list.
@@ -389,11 +404,11 @@
 (Jeff Waugh has asked to subscribe but he's not considered a subscriber
 because his membership on Rosetta Admins hasn't been approved)
 
-    >>> browser.getLink('View subscribers').click()
-    >>> print browser.title
+    >>> carlos_browser.getLink('View subscribers').click()
+    >>> print carlos_browser.title
     Mailing list subscribers for the Rosetta Administrators team...
 
-    >>> print extract_text(find_tag_by_id(browser.contents, 'subscribers'))
+    >>> print extract_text(find_tag_by_id(carlos_browser.contents, 'subscribers'))
     Nobody has subscribed to this team's mailing list yet.
 
 If it had subscribers, though, they'd be shown on that page, in a batched
@@ -407,8 +422,6 @@
     ... """)
     >>> login('[e-mail]')
 
-    >>> from lp.registry.interfaces.person import IPersonSet
-    >>> from zope.component import getUtility
     >>> person_set = getUtility(IPersonSet)
     >>> jdub = person_set.getByName('jdub')
     >>> mark = person_set.getByName('mark')
@@ -420,14 +433,14 @@
     >>> ignored = rosetta_admins.addMember(jordi, reviewer=mark)
     >>> rosetta_admins.mailing_list.subscribe(jordi)
     >>> logout()
-    >>> browser.reload()
-    >>> print extract_text(find_tag_by_id(browser.contents, 'subscribers'))
+    >>> carlos_browser.reload()
+    >>> print extract_text(find_tag_by_id(carlos_browser.contents, 'subscribers'))
     The following people are subscribed...
     Guilherme Salgado
     1 of 2 results...
 
-    >>> browser.getLink('Next').click()
-    >>> print extract_text(find_tag_by_id(browser.contents, 'subscribers'))
+    >>> carlos_browser.getLink('Next').click()
+    >>> print extract_text(find_tag_by_id(carlos_browser.contents, 'subscribers'))
     The following people are subscribed...
     Jordi Mallach
     2 of 2 results...
@@ -452,15 +465,14 @@
 Launchpad may automatically subscribe a person to a team's mailing
 list based on a setting in the person's Email preferences page.
 
-    >>> browser = setupBrowser(auth='Basic [e-mail]:test')
-    >>> browser.open('http://launchpad.dev/~carlos')
-    >>> browser.getLink(url="+editemails").click()
-    >>> print backslashreplace(browser.title)
+    >>> carlos_browser.open('http://launchpad.dev/~carlos')
+    >>> carlos_browser.getLink(url="+editemails").click()
+    >>> print backslashreplace(carlos_browser.title)
     Change your e-mail settings...
 
 Carlos's default setting, 'Ask me when I join a team', is still in place.
 
-    >>> print_radio_button_field(browser.contents,
+    >>> print_radio_button_field(carlos_browser.contents,
     ...     'mailing_list_auto_subscribe_policy')
     ( ) Never subscribe to mailing lists
     (*) Ask me when I join a team
@@ -471,32 +483,32 @@
 
     # A convenient helper for setting and submitting a new
     # auto-subscribe policy.
-    >>> def set_autosubscribe_policy_and_submit(newvalue):
-    ...     control = browser.getControl(
+    >>> def set_autosubscribe_policy_and_submit(newvalue, current_browser):
+    ...     control = current_browser.getControl(
     ...         name='field.mailing_list_auto_subscribe_policy')
     ...     control.value = [newvalue]
-    ...     browser.getControl('Update Policy').click()
-    ...     print_radio_button_field(browser.contents,
+    ...     current_browser.getControl('Update Policy').click()
+    ...     print_radio_button_field(current_browser.contents,
     ...         'mailing_list_auto_subscribe_policy')
 
-    >>> original_value = browser.getControl(
+    >>> original_value = carlos_browser.getControl(
     ...     name='field.mailing_list_auto_subscribe_policy').value.pop()
 
-    >>> set_autosubscribe_policy_and_submit('ALWAYS')
+    >>> set_autosubscribe_policy_and_submit('ALWAYS', carlos_browser)
     ( ) Never subscribe to mailing lists
     ( ) Ask me when I join a team
     (*) Always subscribe me to mailing lists
 
     # We only need to check this once.
-    >>> get_feedback_messages(browser.contents)
+    >>> get_feedback_messages(carlos_browser.contents)
     [u'Your auto-subscription policy has been updated.']
 
-    >>> set_autosubscribe_policy_and_submit('NEVER')
+    >>> set_autosubscribe_policy_and_submit('NEVER', carlos_browser)
     (*) Never subscribe to mailing lists
     ( ) Ask me when I join a team
     ( ) Always subscribe me to mailing lists
 
-    >>> set_autosubscribe_policy_and_submit('ON_REGISTRATION')
+    >>> set_autosubscribe_policy_and_submit('ON_REGISTRATION', carlos_browser)
     ( ) Never subscribe to mailing lists
     (*) Ask me when I join a team
     ( ) Always subscribe me to mailing lists
@@ -505,7 +517,7 @@
 
     # Restores the original value while performing the test.
     >>> assert original_value == 'ON_REGISTRATION'
-    >>> set_autosubscribe_policy_and_submit(original_value)
+    >>> set_autosubscribe_policy_and_submit(original_value, carlos_browser)
     ( ) Never subscribe to mailing lists
     (*) Ask me when I join a team
     ( ) Always subscribe me to mailing lists
@@ -516,7 +528,7 @@
 behavior.
 
     >>> print extract_text(
-    ...     find_tag_by_id(browser.contents, 'notification-info'))
+    ...     find_tag_by_id(carlos_browser.contents, 'notification-info'))
     When a team you are a member of creates a new mailing list, you will
     receive an email notification offering you the opportunity to join the new
     mailing list. Launchpad can also automatically subscribe you to a team's
@@ -528,9 +540,11 @@
 team. Users who have chosen the 'On registration' or 'Always'
 subscription settings will see the box checked by default.
 
-    >>> browser = setupBrowser(
-    ...     auth='Basic [e-mail]:test')
-
+    >>> login(ANONYMOUS)
+    >>> james = getUtility(IPersonSet).getByEmail(
+    ...     '[e-mail]')
+    >>> logout()
+    >>> browser = setupBrowserFreshLogin(james)
     >>> browser.open('http://launchpad.dev/~jblack')
     >>> browser.getLink(url="+editemails").click()
     >>> print_radio_button_field(browser.contents,
@@ -550,7 +564,7 @@
     # Change James' setting
     >>> browser.open('http://launchpad.dev/~jblack')
     >>> browser.getLink(url="+editemails").click()
-    >>> set_autosubscribe_policy_and_submit('ALWAYS')
+    >>> set_autosubscribe_policy_and_submit('ALWAYS', browser)
     ( ) Never subscribe to mailing lists
     ( ) Ask me when I join a team
     (*) Always subscribe me to mailing lists
@@ -566,7 +580,7 @@
     # Change James' setting
     >>> browser.open('http://launchpad.dev/~jblack')
     >>> browser.getLink(url="+editemails").click()
-    >>> set_autosubscribe_policy_and_submit('NEVER')
+    >>> set_autosubscribe_policy_and_submit('NEVER', browser)
     (*) Never subscribe to mailing lists
     ( ) Ask me when I join a team
     ( ) Always subscribe me to mailing lists
@@ -579,7 +593,7 @@
     # Restore James' setting.
     >>> browser.open('http://launchpad.dev/~jblack')
     >>> browser.getLink(url="+editemails").click()
-    >>> set_autosubscribe_policy_and_submit('ON_REGISTRATION')
+    >>> set_autosubscribe_policy_and_submit('ON_REGISTRATION', browser)
     ( ) Never subscribe to mailing lists
     (*) Ask me when I join a team
     ( ) Always subscribe me to mailing lists

=== modified file 'lib/lp/services/webapp/login.py'
--- lib/lp/services/webapp/login.py	2012-08-07 18:27:27 +0000
+++ lib/lp/services/webapp/login.py	2012-08-07 18:27:30 +0000
@@ -338,13 +338,13 @@
         finally:
             timeline_action.finish()
 
-    def login(self, person):
+    def login(self, person, when=None):
         loginsource = getUtility(IPlacelessLoginSource)
         # We don't have a logged in principal, so we must remove the security
         # proxy of the account's preferred email.
         email = removeSecurityProxy(person.preferredemail).email
         logInPrincipal(
-            self.request, loginsource.getPrincipalByLogin(email), email)
+            self.request, loginsource.getPrincipalByLogin(email), email, when)
 
     @cachedproperty
     def sreg_response(self):
@@ -475,7 +475,18 @@
     template = ViewPageTemplateFile("templates/login-already.pt")
 
 
-def logInPrincipal(request, principal, email):
+def isFreshLogin(request):
+    """Return True if the principal login happened in the last 120 seconds."""
+    session = ISession(request)
+    authdata = session['launchpad.authenticateduser']
+    logintime = authdata.get('logintime', None)
+    if logintime is not None:
+        now = datetime.utcnow()
+        return logintime > now - timedelta(seconds=120)
+    return False
+
+
+def logInPrincipal(request, principal, email, when=None):
     """Log the principal in. Password validation must be done in callsites."""
     # Force a fresh session, per Bug #828638. Any changes to any
     # existing session made this request will be lost, but that should
@@ -488,8 +499,10 @@
     authdata = session['launchpad.authenticateduser']
     assert principal.id is not None, 'principal.id is None!'
     request.setPrincipal(principal)
+    if when is None:
+        when = datetime.utcnow()
     authdata['accountid'] = principal.id
-    authdata['logintime'] = datetime.utcnow()
+    authdata['logintime'] = when
     authdata['login'] = email
     notify(CookieAuthLoggedInEvent(request, email))
 

=== modified file 'lib/lp/services/webapp/tests/test_session.py'
--- lib/lp/services/webapp/tests/test_session.py	2012-03-22 23:21:24 +0000
+++ lib/lp/services/webapp/tests/test_session.py	2012-08-07 18:27:30 +0000
@@ -1,14 +1,25 @@
 # Copyright 2009-2012 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
+import datetime
+
 from testtools import TestCase
 from testtools.matchers import Contains
 
+from lp.services.webapp.login import (
+    isFreshLogin,
+    OpenIDCallbackView,
+    )
 from lp.services.webapp.servers import LaunchpadTestRequest
 from lp.services.webapp.session import (
     get_cookie_domain,
     LaunchpadCookieClientIdManager,
     )
+from lp.testing import (
+    person_logged_in,
+    TestCaseWithFactory,
+    )
+from lp.testing.layers import DatabaseFunctionalLayer
 
 
 class GetCookieDomainTestCase(TestCase):
@@ -55,3 +66,34 @@
         self.assertThat(
             dict(request.response.getHeaders())['Set-Cookie'],
             Contains('; httponly;'))
+
+
+class TestSessionRelatedFunctions(TestCaseWithFactory):
+
+    layer = DatabaseFunctionalLayer
+
+    def setupLoggedInRequest(self, user, request, when=None):
+        """Test helper to login a user for a request."""
+        with person_logged_in(user):
+            view = OpenIDCallbackView(user, request)
+            view.login(user, when)
+
+    def test_isFreshLogin_returns_false_for_anonymous(self):
+        """isFreshLogin should return False for anonymous views."""
+        request = LaunchpadTestRequest()
+        self.assertFalse(isFreshLogin(request))
+
+    def test_isFreshLogin_returns_true(self):
+        """isFreshLogin should return True with a fresh logged in user."""
+        user = self.factory.makePerson()
+        request = LaunchpadTestRequest()
+        self.setupLoggedInRequest(user, request)
+        self.assertTrue(isFreshLogin(request))
+
+    def test_isFreshLogin_returns_false(self):
+        """isFreshLogin should be False for users logged in over 2 minutes."""
+        user = self.factory.makePerson()
+        request = LaunchpadTestRequest()
+        when = datetime.datetime.utcnow() - datetime.timedelta(seconds=180)
+        self.setupLoggedInRequest(user, request, when)
+        self.assertFalse(isFreshLogin(request))

=== modified file 'lib/lp/testing/pages.py'
--- lib/lp/testing/pages.py	2012-06-06 16:04:34 +0000
+++ lib/lp/testing/pages.py	2012-08-07 18:27:30 +0000
@@ -5,6 +5,7 @@
 
 __metaclass__ = type
 
+from datetime import datetime
 import doctest
 import os
 import pdb
@@ -37,18 +38,21 @@
     SimpleCookie,
     )
 from zope.component import getUtility
+from zope.session.interfaces import ISession
 from zope.security.proxy import removeSecurityProxy
 from zope.testbrowser.testing import Browser
 
 from lp.app.interfaces.launchpad import ILaunchpadCelebrities
 from lp.registry.errors import NameAlreadyTaken
 from lp.registry.interfaces.teammembership import TeamMembershipStatus
+from lp.services.config import config
 from lp.services.oauth.interfaces import (
     IOAuthConsumerSet,
     OAUTH_REALM,
     )
 from lp.services.webapp import canonical_url
 from lp.services.webapp.interfaces import OAuthPermission
+from lp.services.webapp.servers import LaunchpadTestRequest
 from lp.services.webapp.url import urlsplit
 from lp.testing import (
     ANONYMOUS,
@@ -684,6 +688,24 @@
     return setupBrowser(auth="Basic %s:test" % str(email))
 
 
+def setupBrowserFreshLogin(user):
+    """Create a test browser with a recently logged in user.
+
+    The request is not shared by the browser, so we create
+    a session of the test request and set a cookie to reference
+    the session in the test browser.
+    """
+    request = LaunchpadTestRequest()
+    session = ISession(request)
+    authdata = session['launchpad.authenticateduser']
+    authdata['logintime'] = datetime.utcnow()
+    namespace = config.launchpad_session.cookie
+    cookie = '%s=%s' % (namespace, session.client_id)
+    browser = setupBrowserForUser(user)
+    browser.addHeader('Cookie', cookie)
+    return browser
+
+
 def safe_canonical_url(*args, **kwargs):
     """Generate a bytestring URL for an object"""
     return str(canonical_url(*args, **kwargs))