← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] lp:~wallyworld/launchpad/bug-porlet-subscribers-errors-884222 into lp:launchpad

 

Ian Booth has proposed merging lp:~wallyworld/launchpad/bug-porlet-subscribers-errors-884222 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #884222 in Launchpad itself: "Fix BugTask:+bug-portlet-subscribers-details Unauthorized errors"
  https://bugs.launchpad.net/launchpad/+bug/884222

For more details, see:
https://code.launchpad.net/~wallyworld/launchpad/bug-porlet-subscribers-errors-884222/+merge/81101

Bug assignees and subscribers which are private teams are not currently rendered unless the logged in user is a team member ie has launchpad.View permission for the team. We want private teams who are assignees or subscribers to be rendered if the logged in user can see the bug.

== Implementation ==

Introduce a new permission - launchpad.See
The permission allows a user to see that a private team exists (ie see it's name, url etc). 
A new interface IPersonSee declares the attributes controlled by this new permission.

The idea is that views which need to allow private teams to be rendered will cache the new permission for the logged in user. This causes checkPermission() to return True without invoking a security adaptor. For other times, a security adaptor is written which delegates to the launchpad.View permission check.

The TeamFormatterAPI class has been altered to check launchpad.See permission instead of the launchpad.View permission. Unless the launchpad.See permission has been cached, this will just default back to launchpad.View as previously.

The BugPortletSubscribersWithDetails and BugTasksAndNominationsView views have been updated to cache the launchpad.See permissions on the assignee and subscribers objects.

NB The permission caching implementation is done using the logged in user, so it is not feasible in this branch to allow a non-logged in user to see private teams.

== Tests ==

Add new test class: TestTalesFormatterAPI
(with associated tests)

Add new test class: TestBugTextViewPrivateTeams
(with associated tests)

Add new test to TestBugTaskView:
- test_bugtask_listing_for_private_assignees 

Add new tests to BugPortletSubscribersWithDetailsTests:
- test_data_private_team_subscription_authenticated_user
- test_data_private_team_subscription_no_user

== Lint ==

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/canonical/launchpad/permissions.zcml
  lib/canonical/launchpad/security.py
  lib/lp/app/browser/tales.py
  lib/lp/app/tests/test_tales.py
  lib/lp/bugs/browser/bug.py
  lib/lp/bugs/browser/bugsubscription.py
  lib/lp/bugs/browser/bugtask.py
  lib/lp/bugs/browser/tests/test_bug_views.py
  lib/lp/bugs/browser/tests/test_bugsubscription_views.py
  lib/lp/bugs/browser/tests/test_bugtask.py
  lib/lp/registry/configure.zcml
  lib/lp/registry/doc/private-team-visibility.txt
  lib/lp/registry/interfaces/person.py
-- 
https://code.launchpad.net/~wallyworld/launchpad/bug-porlet-subscribers-errors-884222/+merge/81101
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wallyworld/launchpad/bug-porlet-subscribers-errors-884222 into lp:launchpad.
=== modified file 'lib/canonical/launchpad/permissions.zcml'
--- lib/canonical/launchpad/permissions.zcml	2011-10-17 10:38:53 +0000
+++ lib/canonical/launchpad/permissions.zcml	2011-11-15 13:33:24 +0000
@@ -8,6 +8,12 @@
     id="zope.Public" title="Public stuff" access_level="read" />
   <permission
     id="launchpad.View" title="Viewing something" access_level="read" />
+  <!-- This permission allows users to know that an entity exists.
+  ie they can traverse to its URL, and they can see basic information like
+  name, displayname
+  -->
+  <permission
+    id="launchpad.See" title="See that something exists" access_level="read" />
 
   <permission
     id="launchpad.AnyPerson" title="Any Authenticated Person"

=== modified file 'lib/canonical/launchpad/security.py'
--- lib/canonical/launchpad/security.py	2011-09-07 21:53:15 +0000
+++ lib/canonical/launchpad/security.py	2011-11-15 13:33:24 +0000
@@ -123,6 +123,7 @@
 from lp.registry.interfaces.packaging import IPackaging
 from lp.registry.interfaces.person import (
     IPerson,
+    IPersonSee,
     IPersonSet,
     ITeam,
     PersonVisibility,
@@ -815,6 +816,32 @@
         return False
 
 
+class PublicOrPrivateTeamsExistence(AuthorizationBase):
+    """Restrict knowing about private teams' existence.
+
+    Knowing the existence of a private team allow traversing to its URL and
+    displaying basic information like name, displayname.
+    """
+    permission = 'launchpad.See'
+    usedfor = IPersonSee
+
+    def checkUnauthenticated(self):
+        """Unauthenticated users can only view public teams."""
+        if self.obj.visibility == PersonVisibility.PUBLIC:
+            return True
+        return False
+
+    def checkAuthenticated(self, user):
+        """By default, we simply perform a View permission check.
+
+        The context in which the permission is required is
+        responsible for pre-caching the launchpad.See permission on each
+        team which requires it.
+        """
+        return self.forwardCheckAuthenticated(
+            user, self.obj, 'launchpad.View')
+
+
 class EditPollByTeamOwnerOrTeamAdminsOrAdmins(
         EditTeamMembershipByTeamOwnerOrTeamAdminsOrAdmins):
     permission = 'launchpad.Edit'

=== modified file 'lib/lp/app/browser/tales.py'
--- lib/lp/app/browser/tales.py	2011-11-04 20:56:50 +0000
+++ lib/lp/app/browser/tales.py	2011-11-15 13:33:24 +0000
@@ -1265,7 +1265,7 @@
         The default URL for a team is to the mainsite. None is returned
         when the user does not have permission to review the team.
         """
-        if not check_permission('launchpad.View', self._context):
+        if not check_permission('launchpad.See', self._context):
             # This person has no permission to view the team details.
             self._report_visibility_leak()
             return None
@@ -1273,7 +1273,7 @@
 
     def api_url(self, context):
         """See `ObjectFormatterAPI`."""
-        if not check_permission('launchpad.View', self._context):
+        if not check_permission('launchpad.See', self._context):
             # This person has no permission to view the team details.
             self._report_visibility_leak()
             return None
@@ -1286,7 +1286,7 @@
         when the user does not have permission to review the team.
         """
         person = self._context
-        if not check_permission('launchpad.View', person):
+        if not check_permission('launchpad.See', person):
             # This person has no permission to view the team details.
             self._report_visibility_leak()
             return '<span class="sprite team">%s</span>' % cgi.escape(
@@ -1296,7 +1296,7 @@
     def displayname(self, view_name, rootsite=None):
         """See `PersonFormatterAPI`."""
         person = self._context
-        if not check_permission('launchpad.View', person):
+        if not check_permission('launchpad.See', person):
             # This person has no permission to view the team details.
             self._report_visibility_leak()
             return self.hidden
@@ -1305,7 +1305,7 @@
     def unique_displayname(self, view_name):
         """See `PersonFormatterAPI`."""
         person = self._context
-        if not check_permission('launchpad.View', person):
+        if not check_permission('launchpad.See', person):
             # This person has no permission to view the team details.
             self._report_visibility_leak()
             return self.hidden

=== modified file 'lib/lp/app/tests/test_tales.py'
--- lib/lp/app/tests/test_tales.py	2011-11-04 18:22:02 +0000
+++ lib/lp/app/tests/test_tales.py	2011-11-15 13:33:24 +0000
@@ -15,6 +15,11 @@
     IPathAdapter,
     TraversalError,
     )
+from canonical.launchpad.webapp.authorization import (
+    clear_cache,
+    precache_permission_for_objects,
+    )
+from canonical.launchpad.webapp.servers import LaunchpadTestRequest
 
 from canonical.testing.layers import (
     DatabaseFunctionalLayer,
@@ -28,7 +33,9 @@
     PersonFormatterAPI,
     )
 from lp.registry.interfaces.irc import IIrcIDSet
+from lp.registry.interfaces.person import PersonVisibility
 from lp.testing import (
+    login_person,
     test_tales,
     TestCase,
     TestCaseWithFactory,
@@ -142,6 +149,86 @@
         self.assertEqual(expected, result)
 
 
+class TestTalesFormatterAPI(TestCaseWithFactory):
+    """ Test permissions required to access TalesFormatterAPI methods.
+
+    A user must have launchpad.See permission to use TestTalesFormatterAPI
+    with private teams.
+    """
+    layer = DatabaseFunctionalLayer
+
+    def setUp(self):
+        super(TestTalesFormatterAPI, self).setUp()
+        self.team = self.factory.makeTeam(
+            name='team', displayname='a team',
+            visibility=PersonVisibility.PRIVATE)
+
+    def _make_formatter(self, cache_permission=False):
+        # Helper to create the formatter and optionally cache the permission.
+        formatter = getAdapter(self.team, IPathAdapter, 'fmt')
+        clear_cache()
+        request = LaunchpadTestRequest()
+        any_person = self.factory.makePerson()
+        if cache_permission:
+            login_person(any_person, request)
+            precache_permission_for_objects(
+                request, 'launchpad.See', [self.team])
+        return formatter, request, any_person
+
+    def _tales_value(self, attr, request):
+        # Evaluate the given formatted attribute value on team.
+        return test_tales(
+            "team/fmt:%s" % attr, team=self.team, request=request)
+
+    def _test_can_view_attribute_no_login(self, attr, hidden=None):
+        # Test attribute access with no login.
+        formatter, request, ignore = self._make_formatter()
+        value = self._tales_value(attr, request)
+        if value is not None:
+            if hidden is None:
+                hidden = formatter.hidden
+            self.assertEqual(hidden, value)
+
+    def _test_can_view_attribute_no_permission(self, attr, hidden=None):
+        # Test attribute access when user has no permission.
+        formatter, request, any_person = self._make_formatter()
+        login_person(any_person, request)
+        value = self._tales_value(attr, request)
+        if value is not None:
+            if hidden is None:
+                hidden = formatter.hidden
+            self.assertEqual(hidden, value)
+
+    def _test_can_view_attribute_with_permission(self, attr):
+        # Test attribute access when user has launchpad.See permission.
+        formatter, request, any_person = self._make_formatter(
+            cache_permission=True)
+        self.assertNotEqual(
+            formatter.hidden, self._tales_value(attr, request))
+
+    def _test_can_view_attribute(self, attr, hidden=None):
+        # Test the visibility of the given attribute
+        self._test_can_view_attribute_no_login(attr, hidden)
+        self._test_can_view_attribute_no_permission(attr, hidden)
+        self._test_can_view_attribute_with_permission(attr)
+
+    def test_can_view_displayname(self):
+        self._test_can_view_attribute('displayname')
+
+    def test_can_view_unique_displayname(self):
+        self._test_can_view_attribute('unique_displayname')
+
+    def test_can_view_link(self):
+        self._test_can_view_attribute(
+            'link', u'<span class="sprite team">&lt;hidden&gt;</span>')
+
+    def test_can_view_api_url(self):
+        self._test_can_view_attribute('api_url')
+
+    def test_can_view_url(self):
+        self._test_can_view_attribute('url')
+
+
 class TestObjectFormatterAPI(TestCaseWithFactory):
     """Tests for ObjectFormatterAPI"""
 

=== modified file 'lib/lp/bugs/browser/bug.py'
--- lib/lp/bugs/browser/bug.py	2011-10-21 04:37:49 +0000
+++ lib/lp/bugs/browser/bug.py	2011-11-15 13:33:24 +0000
@@ -75,7 +75,10 @@
     stepthrough,
     structured,
     )
-from canonical.launchpad.webapp.authorization import check_permission
+from canonical.launchpad.webapp.authorization import (
+    check_permission,
+    precache_permission_for_objects,
+    )
 from canonical.launchpad.webapp.interfaces import (
     ICanonicalUrlData,
     ILaunchBag,
@@ -941,11 +944,28 @@
 class BugTextView(LaunchpadView):
     """View for simple text page displaying information for a bug."""
 
+    def initialize(self):
+        # If we have made it to here then the logged in user can see the
+        # bug, hence they can see any subscribers.
+        authorised_people = []
+        for task in self.bugtasks:
+            if task.assignee is not None:
+                authorised_people.append(task.assignee)
+        authorised_people.extend(self.subscribers)
+        precache_permission_for_objects(
+            self.request, 'launchpad.See', authorised_people)
+
     @cachedproperty
     def bugtasks(self):
         """Cache bugtasks and avoid hitting the DB twice."""
         return list(self.context.bugtasks)
 
+    @cachedproperty
+    def subscribers(self):
+        """Cache subscribers and avoid hitting the DB twice."""
+        return [sub.person for sub in self.context.subscriptions
+                if self.user or not sub.person.private]
+
     def bug_text(self):
 
         """Return the bug information for text display."""
@@ -994,8 +1014,8 @@
         text.append('tags: %s' % ' '.join(bug.tags))
 
         text.append('subscribers: ')
-        for subscription in bug.subscriptions:
-            text.append(' %s' % subscription.person.unique_displayname)
+        for subscriber in self.subscribers:
+            text.append(' %s' % subscriber.unique_displayname)
 
         return ''.join(line + '\n' for line in text)
 
@@ -1026,7 +1046,8 @@
         if component:
             text.append('component: %s' % component.name)
 
-        if task.assignee:
+        if (task.assignee
+            and check_permission('launchpad.See', task.assignee)):
             text.append('assignee: %s' % task.assignee.unique_displayname)
         else:
             text.append('assignee: ')

=== modified file 'lib/lp/bugs/browser/bugsubscription.py'
--- lib/lp/bugs/browser/bugsubscription.py	2011-11-14 08:30:52 +0000
+++ lib/lp/bugs/browser/bugsubscription.py	2011-11-15 13:33:24 +0000
@@ -36,7 +36,10 @@
     canonical_url,
     LaunchpadView,
     )
-from canonical.launchpad.webapp.authorization import check_permission
+from canonical.launchpad.webapp.authorization import (
+    check_permission,
+    precache_permission_for_objects,
+    )
 from canonical.launchpad.webapp.launchpadform import ReturnToReferrerMixin
 from canonical.launchpad.webapp.menu import structured
 from lp.app.browser.launchpadform import (
@@ -543,11 +546,17 @@
         details = list(bug.getDirectSubscribersWithDetails())
         for person, subscribed_by, subscription in details:
             can_edit = subscription.canBeUnsubscribedByUser(self.user)
-            if person == self.user or (person.private and not can_edit):
-                # Skip the current user viewing the page,
-                # and private teams user is not a member of.
+            if person == self.user:
+                # Skip the current user viewing the page.
+                continue
+            if self.user is None and person.private:
+                # Do not include private teams if there's no logged in user.
                 continue
 
+            # If we have made it to here then the logged in user can see the
+            # bug, hence they can see any subscribers.
+#            precache_permission_for_objects(
+#                        self.request, 'launchpad.See', [person])
             subscriber = {
                 'name': person.name,
                 'display_name': person.displayname,
@@ -572,9 +581,18 @@
         data = self.direct_subscriber_data(bug)
 
         others = list(bug.getIndirectSubscribers())
+        # If we have made it to here then the logged in user can see the
+        # bug, hence they can see any subscribers.
+        include_private = self.user is not None
+        if include_private:
+            precache_permission_for_objects(
+                self.request, 'launchpad.See', others)
         for person in others:
             if person == self.user:
-                # Skip the current user viewing the page.
+                # Skip the current user viewing the page,
+                continue
+            if not include_private and person.private:
+                # Do not include private teams if there's no logged in user.
                 continue
             subscriber = {
                 'name': person.name,

=== modified file 'lib/lp/bugs/browser/bugtask.py'
--- lib/lp/bugs/browser/bugtask.py	2011-11-14 17:57:46 +0000
+++ lib/lp/bugs/browser/bugtask.py	2011-11-15 13:33:24 +0000
@@ -150,7 +150,10 @@
     redirection,
     stepthrough,
     )
-from canonical.launchpad.webapp.authorization import check_permission
+from canonical.launchpad.webapp.authorization import (
+    check_permission,
+    precache_permission_for_objects,
+    )
 from canonical.launchpad.webapp.batching import TableBatchNavigator
 from canonical.launchpad.webapp.breadcrumb import Breadcrumb
 from canonical.launchpad.webapp.interfaces import ILaunchBag
@@ -3434,6 +3437,13 @@
         self.cached_milestone_source = CachedMilestoneSourceFactory()
         self.user_is_subscribed = self.context.isSubscribed(self.user)
 
+        # If we have made it to here then the logged in user can see the
+        # bug, hence they can see any assignees.
+        authorised_people = [task.assignee for task in self.bugtasks
+                             if task.assignee is not None]
+        precache_permission_for_objects(
+            self.request, 'launchpad.See', authorised_people)
+
         # Pull all of the related milestones, if any, into the storm cache,
         # since they'll be needed for the vocabulary used in this view.
         if self.bugtasks:
@@ -3957,7 +3967,7 @@
 
     @property
     def user_can_edit_assignee(self):
-        """Can the user edit the Milestone field?
+        """Can the user edit the Assignee field?
 
         If yes, return True, otherwise return False.
         """

=== modified file 'lib/lp/bugs/browser/tests/test_bug_views.py'
--- lib/lp/bugs/browser/tests/test_bug_views.py	2011-11-04 14:01:34 +0000
+++ lib/lp/bugs/browser/tests/test_bug_views.py	2011-11-15 13:33:24 +0000
@@ -17,13 +17,18 @@
 from canonical.launchpad.testing.pages import find_tag_by_id
 from canonical.testing.layers import DatabaseFunctionalLayer
 
+from lp.registry.interfaces.person import PersonVisibility
 from lp.services.features.testing import FeatureFixture
 from lp.testing import (
     BrowserTestCase,
+    login_person,
     person_logged_in,
     TestCaseWithFactory,
     )
-from lp.testing.views import create_initialized_view
+from lp.testing.views import (
+    create_view,
+    create_initialized_view,
+    )
 
 
 class TestPrivateBugLinks(BrowserTestCase):
@@ -365,3 +370,56 @@
         self.createInitializedSecrecyView(bug=bug, security_related=True)
         with person_logged_in(owner):
             self.assertTrue(bug.security_related)
+
+
+class TestBugTextViewPrivateTeams(TestCaseWithFactory):
+    """ Test for rendering BugTextView with private team artifacts.
+
+    If an authenticated user can see the bug, they can see a the name of
+    private teams which are assignees or subscribers.
+    """
+    layer = DatabaseFunctionalLayer
+
+    def _makeBug(self):
+        owner = self.factory.makePerson()
+        private_assignee = self.factory.makeTeam(
+            name='bugassignee',
+            visibility=PersonVisibility.PRIVATE)
+        private_subscriber = self.factory.makeTeam(
+            name='bugsubscriber',
+            visibility=PersonVisibility.PRIVATE)
+
+        bug = self.factory.makeBug(owner=owner)
+        with person_logged_in(owner):
+            bug.default_bugtask.transitionToAssignee(private_assignee)
+            bug.subscribe(private_subscriber, owner)
+        return bug, private_assignee, private_subscriber
+
+    def test_unauthenticated_view(self):
+        # Unauthenticated users cannot see private assignees or subscribers.
+        bug, assignee, subscriber = self._makeBug()
+        bug_view = create_initialized_view(bug, name='+text')
+        view_text = bug_view.render()
+        # We don't see the assignee.
+        self.assertIn(
+            "assignee: \n", view_text)
+        # Nor do we see the subscriber.
+        self.assertNotIn(
+            removeSecurityProxy(subscriber).unique_displayname, view_text)
+
+    def test_authenticated_view(self):
+        # Authenticated users can see private assignees or subscribers.
+        bug, assignee, subscriber = self._makeBug()
+        request = LaunchpadTestRequest()
+        bug_view = create_view(bug, name='+text', request=request)
+        any_person = self.factory.makePerson()
+        login_person(any_person, request)
+        bug_view.initialize()
+        view_text = bug_view.render()
+        naked_subscriber = removeSecurityProxy(subscriber)
+        self.assertIn(
+            "assignee: %s" % assignee.unique_displayname, view_text)
+        self.assertTextMatchesExpressionIgnoreWhitespace(
+            "subscribers:\n.*%s \(%s\)"
+            % (naked_subscriber.displayname, naked_subscriber.name),
+            view_text)

=== modified file 'lib/lp/bugs/browser/tests/test_bugsubscription_views.py'
--- lib/lp/bugs/browser/tests/test_bugsubscription_views.py	2011-11-02 21:52:06 +0000
+++ lib/lp/bugs/browser/tests/test_bugsubscription_views.py	2011-11-15 13:33:24 +0000
@@ -14,6 +14,7 @@
 
 from canonical.launchpad.ftests import LaunchpadFormHarness
 from canonical.launchpad.webapp import canonical_url
+from canonical.launchpad.webapp.servers import LaunchpadTestRequest
 from canonical.testing.layers import LaunchpadFunctionalLayer
 from lazr.restful.interfaces import IWebServiceClientRequest
 from lp.bugs.browser.bugsubscription import (
@@ -25,9 +26,11 @@
 from lp.bugs.enum import BugNotificationLevel
 from lp.registry.interfaces.person import (
     IPersonSet,
+    PersonVisibility,
     TeamSubscriptionPolicy,
     )
 from lp.testing import (
+    login_person,
     person_logged_in,
     StormStatementRecorder,
     TestCaseWithFactory,
@@ -615,6 +618,80 @@
         self.assertEqual(
             dumps([expected_result]), harness.view.subscriber_data_js)
 
+    def _test_data_private_team_subscription(self, authenticated_user):
+        # For a private team subscription, the team name and url are rendered
+        # for authenticated users.
+        bug = self._makeBugWithNoSubscribers()
+
+        # Set up a private direct subscriber.
+        teamowner = self.factory.makePerson(
+            name="team-owner", displayname="Team Owner")
+        direct_subscriber = self.factory.makeTeam(
+            name='team', displayname='Team Name', owner=teamowner,
+            visibility=PersonVisibility.PRIVATE)
+        with person_logged_in(direct_subscriber.teamowner):
+            bug.subscribe(direct_subscriber, direct_subscriber.teamowner,
+                          level=BugNotificationLevel.LIFECYCLE)
+
+        # Set up a private indirect subscriber.
+        indirect_teamowner = self.factory.makePerson(
+            name="indirect-team-owner", displayname="Indirect Team Owner")
+        indirect_subscriber = self.factory.makeTeam(
+            name='indirect-team', displayname='Indirect Team Name',
+            owner=indirect_teamowner, visibility=PersonVisibility.PRIVATE)
+        with person_logged_in(indirect_teamowner):
+            bug.default_bugtask.target.addSubscription(
+                indirect_subscriber, indirect_teamowner)
+
+        request = LaunchpadTestRequest()
+        expected_result = []
+        view = create_initialized_view(
+            bug, '+bug-portlet-subscribers-details', request=request)
+        if authenticated_user:
+            any_person = self.factory.makePerson()
+            login_person(any_person, request)
+            api_request = IWebServiceClientRequest(request)
+            naked_subscriber = removeSecurityProxy(direct_subscriber)
+            naked_indirect_subscriber = removeSecurityProxy(
+                indirect_subscriber)
+            expected_result = [{
+                'subscriber': {
+                    'name': 'team',
+                    'display_name': 'Team Name',
+                    'is_team': True,
+                    'can_edit': False,
+                    'web_link': canonical_url(naked_subscriber,
+                        rootsite='mainsite'),
+                    'self_link': absoluteURL(naked_subscriber, api_request),
+                    'display_subscribed_by': \
+                        'Subscribed by Team Owner (team-owner)',
+                    },
+                'subscription_level': "Lifecycle",
+                },
+                {
+                'subscriber': {
+                    'name': 'indirect-team',
+                    'display_name': 'Indirect Team Name',
+                    'is_team': True,
+                    'can_edit': False,
+                    'web_link': canonical_url(naked_indirect_subscriber,
+                        rootsite='mainsite'),
+                    'self_link': absoluteURL(
+                        naked_indirect_subscriber, api_request),
+                    },
+                'subscription_level': "Maybe",
+                }]
+        self.assertEqual(
+            dumps(expected_result), view.subscriber_data_js)
+
+    def test_data_private_team_subscription_authenticated_user(self):
+        # For a logged in user, private team subscriptions are rendered.
+        self._test_data_private_team_subscription(authenticated_user=True)
+
+    def test_data_private_team_subscription_no_user(self):
+        # For a user with no login, private team subscriptions are hidden.
+        self._test_data_private_team_subscription(authenticated_user=False)
+
     def test_data_team_subscription_owner_looks(self):
         # For a team subscription, subscriber_data_js has can_edit
         # set to true for team owner.

=== modified file 'lib/lp/bugs/browser/tests/test_bugtask.py'
--- lib/lp/bugs/browser/tests/test_bugtask.py	2011-11-14 15:59:34 +0000
+++ lib/lp/bugs/browser/tests/test_bugtask.py	2011-11-15 13:33:24 +0000
@@ -1,5 +1,7 @@
 # Copyright 2009-2011 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
+from BeautifulSoup import BeautifulSoup
+from lp.registry.interfaces.person import PersonVisibility
 
 __metaclass__ = type
 
@@ -731,6 +733,36 @@
             content)
         self.assertIn(series.product.displayname, content)
 
+    def test_bugtask_listing_for_private_assignees(self):
+        # Private assignees are rendered in the bug portal view.
+
+        # Create a bugtask with a private assignee.
+        product_foo = self.factory.makeProduct(name="foo")
+        foo_bug = self.factory.makeBug(product=product_foo)
+        assignee = self.factory.makeTeam(
+            name="assignee",
+            visibility=PersonVisibility.PRIVATE)
+        foo_bug.default_bugtask.transitionToAssignee(assignee)
+
+        # Render the view.
+        request = LaunchpadTestRequest()
+        any_person = self.factory.makePerson()
+        login_person(any_person, request)
+        foo_bugtasks_and_nominations_view = getMultiAdapter(
+            (foo_bug, request), name="+bugtasks-and-nominations-portal")
+        foo_bugtasks_and_nominations_view.initialize()
+        task_and_nomination_views = (
+            foo_bugtasks_and_nominations_view.getBugTaskAndNominationViews())
+        getUtility(ILaunchBag).add(foo_bug.default_bugtask)
+        self.assertEqual(1, len(task_and_nomination_views))
+        content = task_and_nomination_views[0]()
+
+        # Check the result.
+        soup = BeautifulSoup(content)
+        tag = soup.find('label', attrs={'for': "foo.assignee.assigned_to"})
+        tag_text = tag.renderContents().strip()
+        self.assertEqual(assignee.unique_displayname, tag_text)
+
 
 class TestBugTaskDeleteLinks(TestCaseWithFactory):
     """ Test that the delete icons/links are correctly rendered.

=== modified file 'lib/lp/registry/configure.zcml'
--- lib/lp/registry/configure.zcml	2011-10-12 10:38:10 +0000
+++ lib/lp/registry/configure.zcml	2011-11-15 13:33:24 +0000
@@ -931,6 +931,9 @@
             <allow
                 interface="lp.registry.interfaces.person.IPersonCommAdminWriteRestricted"/>
             <require
+                permission="launchpad.See"
+                interface="lp.registry.interfaces.person.IPersonSee"/>
+            <require
                 permission="launchpad.View"
                 interface="lp.registry.interfaces.person.IPersonViewRestricted"/>
             <require

=== modified file 'lib/lp/registry/doc/private-team-visibility.txt'
--- lib/lp/registry/doc/private-team-visibility.txt	2010-08-20 01:29:08 +0000
+++ lib/lp/registry/doc/private-team-visibility.txt	2011-11-15 13:33:24 +0000
@@ -58,7 +58,7 @@
     Traceback (most recent call last):
     ...
     Unauthorized: (<Person at ... priv-team (Priv Team)>,
-        'name', 'launchpad.View')
+        'name', 'launchpad.See')
 
 Public teams can join private teams.  When adding one team to another
 the team is invited to join and that invitation must be accepted by
@@ -79,7 +79,7 @@
     Traceback (most recent call last):
     ...
     Unauthorized: (<Person at ... priv-team (Priv Team)>,
-        'name', 'launchpad.View')
+        'name', 'launchpad.See')
 
     >>> login_person(priv_owner)
     >>> ignored = priv_team.addMember(pub_team, reviewer=priv_owner)
@@ -106,4 +106,4 @@
     Traceback (most recent call last):
     ...
     Unauthorized: (<Person at ... priv-team (Priv Team)>,
-        'name', 'launchpad.View')
+        'name', 'launchpad.See')

=== modified file 'lib/lp/registry/interfaces/person.py'
--- lib/lp/registry/interfaces/person.py	2011-11-14 12:23:59 +0000
+++ lib/lp/registry/interfaces/person.py	2011-11-15 13:33:24 +0000
@@ -20,6 +20,7 @@
     'IPersonSettings',
     'ISoftwareCenterAgentAPI',
     'ISoftwareCenterAgentApplication',
+    'IPersonSee',
     'IPersonViewRestricted',
     'IRequestPeopleMerge',
     'ITeam',
@@ -1469,8 +1470,8 @@
         """
 
 
-class IPersonViewRestricted(Interface):
-    """IPerson attributes that require launchpad.View permission."""
+class IPersonSee(Interface):
+    """IPerson attributes that require launchpad.See permission."""
 
     name = exported(
         PersonNameField(
@@ -1489,6 +1490,11 @@
         exported_as='display_name')
     unique_displayname = TextLine(
         title=_('Return a string of the form $displayname ($name).'))
+
+
+class IPersonViewRestricted(Interface):
+    """IPerson attributes that require launchpad.View permission."""
+
     active_member_count = Attribute(
         "The number of real people who are members of this team.")
     # activemembers.value_type.schema will be set to IPerson once
@@ -1874,9 +1880,10 @@
         """
 
 
-class IPerson(IPersonPublic, IPersonViewRestricted, IPersonEditRestricted,
-              IPersonCommAdminWriteRestricted, IPersonSpecialRestricted,
-              IHasStanding, ISetLocation, IRootContext):
+class IPerson(IPersonPublic, IPersonSee, IPersonViewRestricted,
+              IPersonEditRestricted, IPersonCommAdminWriteRestricted,
+              IPersonSpecialRestricted, IHasStanding, ISetLocation,
+              IRootContext):
     """A Person."""
     export_as_webservice_entry(plural_name='people')
 


Follow ups