Merge lp:~cjwatson/launchpad/builders-visibility into lp:launchpad

Proposed by Colin Watson
Status: Merged
Approved by: William Grant
Approved revision: no longer in the source branch.
Merged at revision: 16679
Proposed branch: lp:~cjwatson/launchpad/builders-visibility
Merge into: lp:launchpad
Diff against target: 67 lines (+36/-2)
2 files modified
lib/lp/app/browser/tales.py (+2/-1)
lib/lp/app/tests/test_tales.py (+34/-1)
To merge this branch: bzr merge lp:~cjwatson/launchpad/builders-visibility
Reviewer Review Type Date Requested Status
William Grant code Approve
Review via email: mp+170818@code.launchpad.net

Commit message

Render links to public builds in private archives owned by private teams as "private job".

Description of the change

== Summary ==

There are a couple of cases where builds can be otherwise public (copies of sources already published in public archives; recipe builds from public branches) but be for archives owned by private teams. This causes PackageBuildFormatterAPI to fail to fetch the archive owner name, which in turn makes /builders return 403.

This has been a long-standing problem, and at least two frequent causes are known, but as far as I can tell there is just one underlying cause. Note that the original traceback in bug 760303 was addressed by the fix for bug 728673, but the case of recipe builds from public branches in archives owned by private teams still remained.

== Proposed fix ==

Explicitly check visibility of the archive, and if that fails render the link to the build as "private job". It's close enough and much safer.

== LOC Rationale ==

+34

I think this is tolerable to address a frequent operational issue which causes a good deal of complaint on LP-related channels.

== Tests ==

bin/test -vvct lp.app.tests.test_tales.TestPackageBuildFormatterAPI

== Demo and Q/A ==

Copy a public SPPH to a private archive owned by a private team on DF and set it building; check that /builders is still visible when logged out.

To post a comment you must log in.
Revision history for this message
William Grant (wgrant) :
review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/app/browser/tales.py'
2--- lib/lp/app/browser/tales.py 2013-05-02 00:40:14 +0000
3+++ lib/lp/app/browser/tales.py 2013-06-21 13:54:27 +0000
4@@ -1749,7 +1749,8 @@
5
6 def link(self, view_name, rootsite=None):
7 build = self._context
8- if not check_permission('launchpad.View', build):
9+ if (not check_permission('launchpad.View', build) or
10+ not check_permission('launchpad.View', build.archive)):
11 return 'private job'
12
13 url = self.url(view_name=view_name, rootsite=rootsite)
14
15=== modified file 'lib/lp/app/tests/test_tales.py'
16--- lib/lp/app/tests/test_tales.py 2012-11-14 18:02:13 +0000
17+++ lib/lp/app/tests/test_tales.py 2013-06-21 13:54:27 +0000
18@@ -1,4 +1,4 @@
19-# Copyright 2009-2010 Canonical Ltd. This software is licensed under the
20+# Copyright 2009-2013 Canonical Ltd. This software is licensed under the
21 # GNU Affero General Public License version 3 (see the file LICENSE).
22
23 """tales.py doctests."""
24@@ -28,6 +28,7 @@
25 from lp.registry.interfaces.irc import IIrcIDSet
26 from lp.registry.interfaces.person import PersonVisibility
27 from lp.services.webapp.authorization import (
28+ check_permission,
29 clear_cache,
30 precache_permission_for_objects,
31 )
32@@ -474,3 +475,35 @@
33 """Values in seconds are reported as "less than a minute."""
34 self.assertEqual('less than a minute',
35 self.getDurationsince(timedelta(0, 59)))
36+
37+
38+class TestPackageBuildFormatterAPI(TestCaseWithFactory):
39+ """Tests for PackageBuildFormatterAPI."""
40+
41+ layer = LaunchpadFunctionalLayer
42+
43+ def _make_public_build_for_private_team(self):
44+ spph = self.factory.makeSourcePackagePublishingHistory()
45+ team_owner = self.factory.makePerson()
46+ private_team = self.factory.makeTeam(
47+ owner=team_owner, visibility=PersonVisibility.PRIVATE)
48+ p3a = self.factory.makeArchive(owner=private_team, private=True)
49+ build = self.factory.makeBinaryPackageBuild(
50+ source_package_release=spph.sourcepackagerelease, archive=p3a)
51+ return build, p3a, team_owner
52+
53+ def test_public_build_private_team_no_permission(self):
54+ # A `PackageBuild` for a public `SourcePackageRelease` in an archive
55+ # for a private team is rendered gracefully when the user has no
56+ # permission.
57+ build, _, _ = self._make_public_build_for_private_team()
58+ # Make sure this is a valid test; the build itself must be public.
59+ self.assertTrue(check_permission('launchpad.View', build))
60+ self.assertEqual('private job', format_link(build))
61+
62+ def test_public_build_private_team_with_permission(self):
63+ # Members of a private team can see their builds.
64+ build, p3a, team_owner = self._make_public_build_for_private_team()
65+ login_person(team_owner)
66+ self.assertIn(
67+ "[%s/%s]" % (p3a.owner.name, p3a.name), format_link(build))