launchpad-reviewers team mailing list archive

["j.c.sackett" <jcsackett@xxxxxxxxxxxxx>]

j.c.sackett has proposed merging lp:~jcsackett/launchpad/404-not-403-for-private-blueprints into lp:launchpad.

Commit message:
Checks permissions on specs before returning them in traversal so private specs can 404 instead of 403.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1076477 in Launchpad itself: "Unauthorized access to a private/proprietary specification does not 404"
  https://bugs.launchpad.net/launchpad/+bug/1076477

For more details, see:
https://code.launchpad.net/~jcsackett/launchpad/404-not-403-for-private-blueprints/+merge/136442

Summary
=======
Private specifications right now return 403 when someone without permissions
tries to access them. Per our usual rules, they should 404 to not leak the
fact of their existence.

Bugs, branches, and other private artifacts cause a 404 by checking for
permissions on the view context before returning it and returning None to
traversal if the user doesn't have the required permissions. Specs should use
the same mechanism.

Preimp
======
Spoke with Rick Harding about earlier decisions around necessary permissions
on blueprints.

Implementation
==============
In the stepthrough traversal for +spec on products, check for LimitedView on
the spec. If the permission is not available, return None.

Tests
=====
bin/test -vvct test_private_specification_without_authorization

QA
==
Attempt, without the permissions to do so, to view a proprietary blueprint on
a public product that allows proprietary blueprints. It should 404.

LoC
===
Part of private projects.

Lint
====

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/registry/browser/product.py
  lib/lp/blueprints/browser/tests/test_specification.py
-- 
https://code.launchpad.net/~jcsackett/launchpad/404-not-403-for-private-blueprints/+merge/136442
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jcsackett/launchpad/404-not-403-for-private-blueprints into lp:launchpad.

=== modified file 'lib/lp/blueprints/browser/tests/test_specification.py'
--- lib/lp/blueprints/browser/tests/test_specification.py	2012-11-26 13:48:45 +0000
+++ lib/lp/blueprints/browser/tests/test_specification.py	2012-11-27 15:16:27 +0000
@@ -184,6 +184,20 @@
             extract_text(html), DocTestMatches(
                 "... Registered by Some Person ... ago ..."))
 
+    def test_private_specification_without_authorization(self):
+        # Users without access get a 404 when trying to view private
+        # specifications.
+        owner = self.factory.makePerson()
+        policy = SpecificationSharingPolicy.PROPRIETARY
+        product = self.factory.makeProduct(owner=owner,
+            specification_sharing_policy=policy)
+        with person_logged_in(owner):
+            spec = self.factory.makeSpecification(
+                product=product, owner=owner,
+                information_type=InformationType.PROPRIETARY)
+            url = canonical_url(spec)
+        self.assertRaises(NotFound, self.getUserBrowser, url=url, user=None)
+
     def test_view_for_subscriber_with_artifact_grant(self):
         # Users with an artifact grant for a specification related to a
         # private  product can view the specification page.

=== modified file 'lib/lp/registry/browser/product.py'
--- lib/lp/registry/browser/product.py	2012-11-23 21:06:56 +0000
+++ lib/lp/registry/browser/product.py	2012-11-27 15:16:27 +0000
@@ -229,7 +229,10 @@
 
     @stepthrough('+spec')
     def traverse_spec(self, name):
-        return self.context.getSpecification(name)
+        spec = self.context.getSpecification(name)
+        if not check_permission('launchpad.LimitedView', spec):
+            return None
+        return spec
 
     @stepthrough('+milestone')
     def traverse_milestone(self, name):