launchpad-reviewers team mailing list archive

[Stuart Bishop <stuart.bishop@xxxxxxxxxxxxx>]

Stuart Bishop has proposed merging lp:~stub/launchpad/session into lp:launchpad with lp:~stub/launchpad/trivial as a prerequisite.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~stub/launchpad/session/+merge/72397

= Summary =

Deal with Bug #828638

== Proposed fix ==

Force the session cookie to be reset on login.

== Pre-implementation notes ==

== Implementation details ==

== Tests ==

Can't work out how to test this automatically, and only partially locally.

== Demo and Q/A ==


= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/canonical/launchpad/webapp/login.py
-- 
https://code.launchpad.net/~stub/launchpad/session/+merge/72397
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~stub/launchpad/session into lp:launchpad.

=== modified file 'lib/canonical/launchpad/webapp/login.py'
--- lib/canonical/launchpad/webapp/login.py	2011-07-15 00:10:51 +0000
+++ lib/canonical/launchpad/webapp/login.py	2011-08-22 10:15:28 +0000
@@ -86,7 +86,7 @@
             # a tuple containing (object, attribute_requested, permission).
             lp_permission = getUtility(ILaunchpadPermission, self.context[2])
             if lp_permission.access_level != "read":
-                self.request.response.setStatus(503) # Service Unavailable
+                self.request.response.setStatus(503)  # Service Unavailable
                 return self.read_only_page()
 
         if IUnauthenticatedPrincipal.providedBy(self.request.principal):
@@ -102,7 +102,7 @@
                 # must ensure that form pages require the same rights
                 # as the pages that process those forms.  So, we should never
                 # need to newly authenticate on a POST.
-                self.request.response.setStatus(500) # Internal Server Error
+                self.request.response.setStatus(500)  # Internal Server Error
                 self.request.response.setHeader('Content-type', 'text/plain')
                 return ('Application error.  Unauthenticated user POSTing to '
                         'page that requires authentication.')
@@ -126,7 +126,7 @@
             # Maybe render page with a link to the redirection?
             return ''
         else:
-            self.request.response.setStatus(403) # Forbidden
+            self.request.response.setStatus(403)  # Forbidden
             return self.forbidden_page()
 
     def getRedirectURL(self, current_url, query_string):
@@ -436,6 +436,13 @@
 
 def logInPrincipal(request, principal, email):
     """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
+    # not be a problem as authentication must be done before
+    # authorization and authorization before we do any actual work.
+    client_id_manager = getUtility(IClientIdManager)
+    new_client_id = client_id_manager.generateUniqueId()
+    client_id_manager.setRequestId(request, new_client_id)
     session = ISession(request)
     authdata = session['launchpad.authenticateduser']
     assert principal.id is not None, 'principal.id is None!'
@@ -531,5 +538,5 @@
     def __call__(self):
         assert IUnauthenticatedPrincipal.providedBy(self.request.principal), (
             "Feeds user should always be anonymous.")
-        self.request.response.setStatus(403) # Forbidden
+        self.request.response.setStatus(403)  # Forbidden
         return self.forbidden_page()