Merge lp:~wallyworld/launchpad/private-owned-entity-traversal-2 into lp:launchpad

[Ian Booth]

[r=sinzui] Users with access to private team's PPAs or branches have limited view permission on the team.

This branch is a re-submission of lp:~wallyworld/launchpad/private-owned-entity-traversal as described in mp
https://code.launchpad.net/~wallyworld/launchpad/private-owned-entity-traversal/+merge/84048

Some tweaks were made to fix a traversal issue found during qa on qastaging.

== Implementation ==

The issue was that rendering the ppa page required access to IPerson properties:
- title
- is_probationary

These properties were defined under IPersonViewRestricted and not IPersonLimitedView.
title simple delegates to displayname
is_probationary returns false for teams anyway

So I simply moved the properties to IPersonLimitedView.

== Tests ==

Add one new test to test the failure as seen on qastaging:
TestArchiveSubscriptions.test_subscriber_can_browse_private_team_ppa()

== Lint ==

Linting changed files:
  lib/canonical/launchpad/security.py
  lib/canonical/launchpad/interfaces/_schema_circular_imports.py
  lib/canonical/launchpad/webapp/authorization.py
  lib/canonical/launchpad/webapp/interaction.py
  lib/lp/app/browser/launchpad.py
  lib/lp/app/browser/tales.py
  lib/lp/app/tests/test_tales.py
  lib/lp/bugs/browser/tests/test_bugsubscription_views.py
  lib/lp/code/browser/tests/test_branchlisting.py
  lib/lp/registry/configure.zcml
  lib/lp/registry/browser/tests/test_mailinglists.py
  lib/lp/registry/browser/tests/test_person_view.py
  lib/lp/registry/doc/person.txt
  lib/lp/registry/doc/private-team-visibility.txt
  lib/lp/registry/interfaces/person.py
  lib/lp/registry/interfaces/role.py
  lib/lp/registry/model/personroles.py
  lib/lp/registry/stories/webservice/xx-derivedistroseries.txt
  lib/lp/registry/tests/test_person_vocabularies.py
  lib/lp/registry/tests/test_team_webservice.py
  lib/lp/soyuz/tests/test_archive_subscriptions.py
  lib/lp/testing/factory.py

[Curtis Hovey (sinzui)]

Thank you for the prompt follow up. I am going to use ec2 land now so that this might be merged and in buildbot when you awake.

review: Approve (code)

=== modified file 'lib/canonical/launchpad/interfaces/_schema_circular_imports.py'
--- lib/canonical/launchpad/interfaces/_schema_circular_imports.py	2011-12-12 02:45:10 +0000
+++ lib/canonical/launchpad/interfaces/_schema_circular_imports.py	2011-12-12 05:28:08 +0000
@@ -135,7 +135,7 @@
     )
 from lp.registry.interfaces.person import (
     IPerson,
-    IPersonPublic,
+    IPersonViewRestricted,
     ITeam,
     )
 from lp.registry.interfaces.pillar import (
@@ -310,10 +310,10 @@
 
 IPreviewDiff['branch_merge_proposal'].schema = IBranchMergeProposal
 
-patch_reference_property(IPersonPublic, 'archive', IArchive)
-patch_collection_property(IPersonPublic, 'ppas', IArchive)
-patch_entry_return_type(IPersonPublic, 'getPPAByName', IArchive)
-patch_entry_return_type(IPersonPublic, 'createPPA', IArchive)
+patch_reference_property(IPersonViewRestricted, 'archive', IArchive)
+patch_collection_property(IPersonViewRestricted, 'ppas', IArchive)
+patch_entry_return_type(IPersonViewRestricted, 'getPPAByName', IArchive)
+patch_entry_return_type(IPersonViewRestricted, 'createPPA', IArchive)
 
 IHasBuildRecords['getBuildRecords'].queryTaggedValue(
     LAZR_WEBSERVICE_EXPORTED)[

=== modified file 'lib/canonical/launchpad/security.py'
--- lib/canonical/launchpad/security.py	2011-12-12 02:45:10 +0000
+++ lib/canonical/launchpad/security.py	2011-12-12 05:28:08 +0000
@@ -63,6 +63,7 @@
     IBranch,
     user_has_special_branch_access,
     )
+from lp.code.interfaces.branchcollection import IBranchCollection
 from lp.code.interfaces.branchmergeproposal import IBranchMergeProposal
 from lp.code.interfaces.branchmergequeue import IBranchMergeQueue
 from lp.code.interfaces.codeimport import ICodeImport
@@ -800,6 +801,35 @@
             if (invitee.is_team and
                 invitee in user.person.getAdministratedTeams()):
                 return True
+        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.LimitedView'
+    usedfor = IPersonLimitedView
+
+    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.
+
+        We also grant limited viewability to users who can see PPAs and
+        branches owned by the team. In other scenarios, the context in which
+        the permission is required is responsible for pre-caching the
+        launchpad.LimitedView permission on each team which requires it.
+        """
+        if self.forwardCheckAuthenticated(
+            user, self.obj, 'launchpad.View'):
+            return True
 
         if (self.obj.is_team
             and self.obj.visibility == PersonVisibility.PRIVATE):
@@ -813,33 +843,14 @@
                 ppa.id for ppa in self.obj.ppas if ppa.private)
             if len(subscriber_archive_ids.intersection(team_ppa_ids)) > 0:
                 return True
-        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.LimitedView'
-    usedfor = IPersonLimitedView
-
-    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.LimitedView permission on
-        each team which requires it.
-        """
-        return self.forwardCheckAuthenticated(
-            user, self.obj, 'launchpad.View')
+
+            # Grant visibility to people with subscriptions to branches owned
+            # by the private team.
+            owned_branches = getUtility(IBranchCollection).ownedBy(self.obj)
+            if owned_branches.visibleByUser(user.person).count() > 0:
+                return True
+
+        return False
 
 
 class EditPollByTeamOwnerOrTeamAdminsOrAdmins(

=== modified file 'lib/canonical/launchpad/webapp/authorization.py'
--- lib/canonical/launchpad/webapp/authorization.py	2011-12-12 02:45:10 +0000
+++ lib/canonical/launchpad/webapp/authorization.py	2011-12-12 05:28:08 +0000
@@ -192,7 +192,7 @@
                     objecttoauthorize, {})
                 if permission in object_cache:
                     return object_cache[permission]
-            principal = participation.principal
+            principal = removeAllProxies(participation.principal)
 
         if (principal is not None and
             not isinstance(principal, UnauthenticatedPrincipal)):

=== modified file 'lib/canonical/launchpad/webapp/interaction.py'
--- lib/canonical/launchpad/webapp/interaction.py	2011-12-12 02:45:10 +0000
+++ lib/canonical/launchpad/webapp/interaction.py	2011-12-12 05:28:08 +0000
@@ -163,7 +163,8 @@
         return setupInteraction(ANONYMOUS, participation)
     else:
         # Bypass zope's security because IEmailAddress.email is not public.
-        naked_email = removeSecurityProxy(person.preferredemail)
+        naked_person = removeSecurityProxy(person)
+        naked_email = removeSecurityProxy(naked_person.preferredemail)
         return setupInteractionByEmail(naked_email.email, participation)
 
 

=== modified file 'lib/lp/app/browser/launchpad.py'
--- lib/lp/app/browser/launchpad.py	2011-12-12 02:45:10 +0000
+++ lib/lp/app/browser/launchpad.py	2011-12-12 05:28:08 +0000
@@ -724,8 +724,8 @@
                 # Check to see if this is a team, and if so, whether the
                 # logged in user is allowed to view the team, by virtue of
                 # team membership or Launchpad administration.
-                if (person.is_team
-                    and not check_permission('launchpad.View', person)):
+                if (person.is_team and
+                    not check_permission('launchpad.LimitedView', person)):
                     raise NotFound(self.context, name)
                 # Only admins are permitted to see suspended users.
                 if person.account_status == AccountStatus.SUSPENDED:

=== modified file 'lib/lp/app/browser/tales.py'
--- lib/lp/app/browser/tales.py	2011-12-12 02:45:10 +0000
+++ lib/lp/app/browser/tales.py	2011-12-12 05:28:08 +0000
@@ -1266,6 +1266,14 @@
                 self.hidden)
         return super(TeamFormatterAPI, self).link(view_name, rootsite)
 
+    def icon(self, view_name):
+        team = self._context
+        if not check_permission('launchpad.LimitedView', team):
+            css_class = ObjectImageDisplayAPI(team).sprite_css()
+            return '<span class="' + css_class + '"></span>'
+        else:
+            return super(TeamFormatterAPI, self).icon(view_name)
+
     def displayname(self, view_name, rootsite=None):
         """See `PersonFormatterAPI`."""
         person = self._context

=== modified file 'lib/lp/app/tests/test_tales.py'
--- lib/lp/app/tests/test_tales.py	2011-12-12 02:45:10 +0000
+++ lib/lp/app/tests/test_tales.py	2011-12-12 05:28:08 +0000
@@ -149,18 +149,20 @@
         self.assertEqual(expected, result)
 
 
-class TestTalesFormatterAPI(TestCaseWithFactory):
-    """ Test permissions required to access TalesFormatterAPI methods.
+class TestTeamFormatterAPI(TestCaseWithFactory):
+    """ Test permissions required to access TeamFormatterAPI methods.
 
     A user must have launchpad.LimitedView permission to use
-    TestTalesFormatterAPI with private teams.
+    TeamFormatterAPI with private teams.
     """
-    layer = DatabaseFunctionalLayer
+    layer = LaunchpadFunctionalLayer
 
     def setUp(self):
-        super(TestTalesFormatterAPI, self).setUp()
+        super(TestTeamFormatterAPI, self).setUp()
+        icon = self.factory.makeLibraryFileAlias(
+            filename='smurf.png', content_type='image/png')
         self.team = self.factory.makeTeam(
-            name='team', displayname='a team',
+            name='team', displayname='a team', icon=icon,
             visibility=PersonVisibility.PRIVATE)
 
     def _make_formatter(self, cache_permission=False):
@@ -175,10 +177,11 @@
                 request, 'launchpad.LimitedView', [self.team])
         return formatter, request, any_person
 
-    def _tales_value(self, attr, request):
+    def _tales_value(self, attr, request, path='fmt'):
         # Evaluate the given formatted attribute value on team.
-        return test_tales(
-            "team/fmt:%s" % attr, team=self.team, request=request)
+        result = test_tales(
+            "team/%s:%s" % (path, attr), team=self.team, request=request)
+        return result
 
     def _test_can_view_attribute_no_login(self, attr, hidden=None):
         # Test attribute access with no login.
@@ -228,6 +231,10 @@
     def test_can_view_url(self):
         self._test_can_view_attribute('url')
 
+    def test_can_view_icon(self):
+        self._test_can_view_attribute(
+            'icon', '<span class="sprite team"></span>')
+
 
 class TestObjectFormatterAPI(TestCaseWithFactory):
     """Tests for ObjectFormatterAPI"""

=== modified file 'lib/lp/bugs/browser/tests/test_bugsubscription_views.py'
--- lib/lp/bugs/browser/tests/test_bugsubscription_views.py	2011-12-12 02:45:10 +0000
+++ lib/lp/bugs/browser/tests/test_bugsubscription_views.py	2011-12-12 05:28:08 +0000
@@ -629,7 +629,7 @@
         direct_subscriber = self.factory.makeTeam(
             name='team', displayname='Team Name', owner=teamowner,
             visibility=PersonVisibility.PRIVATE)
-        with person_logged_in(direct_subscriber.teamowner):
+        with person_logged_in(teamowner):
             bug.subscribe(direct_subscriber, direct_subscriber.teamowner,
                           level=BugNotificationLevel.LIFECYCLE)
 

=== modified file 'lib/lp/code/browser/tests/test_branchlisting.py'
--- lib/lp/code/browser/tests/test_branchlisting.py	2011-12-12 02:45:10 +0000
+++ lib/lp/code/browser/tests/test_branchlisting.py	2011-12-12 05:28:08 +0000
@@ -613,13 +613,14 @@
     layer = DatabaseFunctionalLayer
 
     def _make_branch_for_private_team(self):
+        owner = self.factory.makePerson()
         private_team = self.factory.makeTeam(
-            name='shh', displayname='Shh',
+            name='shh', displayname='Shh', owner=owner,
             visibility=PersonVisibility.PRIVATE)
         member = self.factory.makePerson(
             email='[e-mail]', password='test')
-        with person_logged_in(private_team.teamowner):
-            private_team.addMember(member, private_team.teamowner)
+        with person_logged_in(owner):
+            private_team.addMember(member, owner)
         branch = self.factory.makeProductBranch(owner=private_team)
         return private_team, member, branch
 

=== modified file 'lib/lp/registry/browser/tests/test_mailinglists.py'
--- lib/lp/registry/browser/tests/test_mailinglists.py	2011-12-12 02:45:10 +0000
+++ lib/lp/registry/browser/tests/test_mailinglists.py	2011-12-12 05:28:08 +0000
@@ -28,11 +28,12 @@
     """Verify the content in +mailing-list-portlet."""
 
     def makeTeamWithMailingList(self, name=None, owner=None, visibility=None):
+        if owner is None:
+            owner = self.factory.makePerson()
         team = self.factory.makeTeam(
             name=name, owner=owner, visibility=visibility)
-        login_person(team.teamowner)
-        team_list = self.factory.makeMailingList(
-            team=team, owner=team.teamowner)
+        login_person(owner)
+        self.factory.makeMailingList(team=team, owner=owner)
         return team
 
 
@@ -129,10 +130,11 @@
 
     def test_private_message_message_without_list(self):
         # Private teams have private archives.
+        owner = self.factory.makePerson()
         team = self.factory.makeTeam(
-            visibility=PersonVisibility.PRIVATE)
-        login_person(team.teamowner)
+            owner=owner, visibility=PersonVisibility.PRIVATE)
+        login_person(owner)
         view = create_initialized_view(
-            team, name='+mailinglist', principal=team.teamowner)
+            team, name='+mailinglist', principal=owner)
         element = find_tag_by_id(view(), 'mailing-list-archive')
         self.assertEqual('private', extract_text(element))

=== modified file 'lib/lp/registry/browser/tests/test_person_view.py'
--- lib/lp/registry/browser/tests/test_person_view.py	2011-12-12 02:45:10 +0000
+++ lib/lp/registry/browser/tests/test_person_view.py	2011-12-12 05:28:08 +0000
@@ -475,9 +475,11 @@
 
     def test_active_participations_with_direct_private_team(self):
         # Users cannot see private teams that they are not members of.
-        team = self.factory.makeTeam(visibility=PersonVisibility.PRIVATE)
-        login_person(team.teamowner)
-        team.addMember(self.user, team.teamowner)
+        owner = self.factory.makePerson()
+        team = self.factory.makeTeam(
+            owner=owner, visibility=PersonVisibility.PRIVATE)
+        login_person(owner)
+        team.addMember(self.user, owner)
         # The team is included in active_participations.
         login_person(self.user)
         view = create_view(
@@ -492,11 +494,13 @@
 
     def test_active_participations_with_indirect_private_team(self):
         # Users cannot see private teams that they are not members of.
-        team = self.factory.makeTeam(visibility=PersonVisibility.PRIVATE)
-        direct_team = self.factory.makeTeam(owner=team.teamowner)
-        login_person(team.teamowner)
-        direct_team.addMember(self.user, team.teamowner)
-        team.addMember(direct_team, team.teamowner)
+        owner = self.factory.makePerson()
+        team = self.factory.makeTeam(
+            owner=owner, visibility=PersonVisibility.PRIVATE)
+        direct_team = self.factory.makeTeam(owner=owner)
+        login_person(owner)
+        direct_team.addMember(self.user, owner)
+        team.addMember(direct_team, owner)
         # The team is included in active_participations.
         login_person(self.user)
         view = create_view(

=== modified file 'lib/lp/registry/configure.zcml'
--- lib/lp/registry/configure.zcml	2011-12-12 02:45:10 +0000
+++ lib/lp/registry/configure.zcml	2011-12-12 05:28:08 +0000
@@ -912,8 +912,6 @@
             <allow
                 interface="lp.registry.interfaces.person.IPersonPublic"/>
             <allow
-                interface="lp.registry.interfaces.person.ITeamPublic"/>
-            <allow
                 interface="lp.registry.interfaces.person.IHasStanding"/>
             <allow
                 interface="lp.registry.interfaces.person.IPersonCommAdminWriteRestricted"/>
@@ -924,6 +922,9 @@
                 permission="launchpad.View"
                 interface="lp.registry.interfaces.person.IPersonViewRestricted"/>
             <require
+                permission="launchpad.View"
+                interface="lp.registry.interfaces.person.ITeamPublic"/>
+            <require
                 permission="launchpad.Edit"
                 interface="lp.registry.interfaces.location.ISetLocation"/>
             <require
@@ -931,9 +932,10 @@
                 interface="lp.registry.interfaces.person.IPersonEditRestricted"/>
             <require
                 permission="launchpad.Edit"
-                set_schema="lp.registry.interfaces.person.IPersonPublic
+                set_schema="lp.registry.interfaces.person.IPersonViewRestricted
+                            lp.registry.interfaces.person.IPersonPublic
                             lp.registry.interfaces.person.ITeamPublic"
-                set_attributes="displayname"/>
+                set_attributes="displayname icon logo"/>
             <require
                 permission="launchpad.Moderate"
                 set_attributes="name"/>

=== modified file 'lib/lp/registry/doc/person.txt'
--- lib/lp/registry/doc/person.txt	2011-12-12 02:45:10 +0000
+++ lib/lp/registry/doc/person.txt	2011-12-12 05:28:08 +0000
@@ -696,6 +696,7 @@
     >>> private_team = factory.makeTeam(salgado, name='private-team',
     ...     displayname='Private Team',
     ...     visibility=PersonVisibility.PRIVATE)
+    >>> private_team_owner = private_team.teamowner
     >>> bug_subscription = BugSubscription(bug=bug, person=private_team,
     ...     subscribed_by=guadamen)
 
@@ -806,7 +807,7 @@
 
 But Owner, a member of that team, will see it in the results.
 
-    >>> login_person(private_team.teamowner)
+    >>> login_person(private_team_owner)
     >>> print_people(personset.find('team'))
     Another a new team (new3): []
     Hoary Gnome Team (name21): []

=== modified file 'lib/lp/registry/doc/private-team-visibility.txt'
--- lib/lp/registry/doc/private-team-visibility.txt	2011-12-12 02:45:10 +0000
+++ lib/lp/registry/doc/private-team-visibility.txt	2011-12-12 05:28:08 +0000
@@ -107,3 +107,64 @@
     ...
     Unauthorized: (<Person at ... priv-team (Priv Team)>,
         'name', 'launchpad.LimitedView')
+
+A person with visibility to any of the branches owned by the private team will
+be granted limited view permission on the team.
+
+For private branches, a user needs to be subscribed to the branch for the
+branch (and hence team) to be visible.
+
+    >>> login_person(priv_owner)
+    >>> private_team_branch = factory.makeBranch(
+    ...     owner=priv_team, private=True)
+    >>> login_person(pub_member)
+    >>> check_permission('launchpad.LimitedView', priv_team)
+    False
+
+    >>> login_person(priv_owner)
+    >>> sub = factory.makeBranchSubscription(
+    ...     branch=private_team_branch, person=pub_member,
+    ...     subscribed_by=priv_owner)
+
+    >>> login_person(pub_member)
+    >>> check_permission('launchpad.LimitedView', priv_team)
+    True
+
+Subscribers to the teams private PPA have limited view permission.
+
+    >>> login_person(priv_owner)
+    >>> archive = factory.makeArchive(private=True, owner=priv_team)
+    >>> archive_subscriber = factory.makePerson()
+    >>> sub = archive.newSubscription(
+    ...     archive_subscriber, registrant=archive.owner)
+
+    >>> login_person(archive_subscriber)
+    >>> check_permission('launchpad.LimitedView', priv_team)
+    True
+
+Users with LimitedView can know identifying information like name,
+displayname, and unique_name, but cannot know other information like
+teamowner.
+
+    >>> print priv_team.name
+    priv-team
+
+    >>> print priv_team.displayname
+    Priv Team
+
+    >>> print priv_team.unique_displayname
+    Priv Team (priv-team)
+
+    >>> print priv_team.icon
+    None
+
+    >>> print priv_team.allmembers
+    Traceback (most recent call last):
+    ...
+    Unauthorized: (<Person at ... priv-team (Priv Team)>,
+        'allmembers', 'launchpad.View')
+
+Anonymous users do not have permission.
+    >>> login(ANONYMOUS)
+    >>> check_permission('launchpad.LimitedView', priv_team)
+    False

=== modified file 'lib/lp/registry/interfaces/person.py'
--- lib/lp/registry/interfaces/person.py	2011-12-12 02:45:10 +0000
+++ lib/lp/registry/interfaces/person.py	2011-12-12 05:28:08 +0000
@@ -15,7 +15,7 @@
     'IObjectReassignment',
     'IPerson',
     'IPersonClaim',
-    'IPersonPublic',  # Required for a monkey patch in interfaces/archive.py
+    'IPersonPublic',
     'IPersonSet',
     'IPersonSettings',
     'ISoftwareCenterAgentAPI',
@@ -657,26 +657,55 @@
         required=False, default=False)
 
 
-class IPersonPublic(IHasBranches, IHasSpecifications,
-                    IHasMergeProposals, IHasLogo, IHasMugshot, IHasIcon,
-                    IHasLocation, IHasRequestedReviews, IObjectWithLocation,
-                    IPrivacy, IHasBugs, IHasRecipes, IHasTranslationImports,
-                    IPersonSettings, IQuestionsPerson):
-    """Public attributes for a Person."""
+class IPersonPublic(IPrivacy):
+    """Public attributes for a Person.
+
+    Very few attributes on a person can be public because private teams
+    are also persons. The public attributes are generally information
+    needed by the system to determine if the principal in the current
+    interaction can work with the object.
+    """
 
     id = Int(title=_('ID'), required=True, readonly=True)
-    account = Object(schema=IAccount)
-    accountID = Int(title=_('Account ID'), required=True, readonly=True)
-    password = PasswordField(
-        title=_('Password'), required=True, readonly=False)
-    karma = exported(
-        Int(title=_('Karma'), readonly=True,
-            description=_('The cached total karma for this person.')))
-    homepage_content = exported(
-        Text(title=_("Homepage Content"), required=False,
-            description=_(
-                "The content of your profile page. Use plain text, "
-                "paragraphs are preserved and URLs are linked in pages.")))
+    # This is redefined from IPrivacy.private because the attribute is
+    # read-only. It is a summary of the team's visibility.
+    private = exported(Bool(
+            title=_("This team is private"),
+            readonly=True, required=False,
+            description=_("Private teams are visible only to "
+                          "their members.")))
+    is_valid_person = Bool(
+        title=_("This is an active user and not a team."), readonly=True)
+    is_valid_person_or_team = exported(
+        Bool(title=_("This is an active user or a team."), readonly=True),
+        exported_as='is_valid')
+    is_merge_pending = exported(Bool(
+        title=_("Is this person due to be merged with another?"),
+        required=False, default=False))
+    is_team = exported(
+        Bool(title=_('Is this object a team?'), readonly=True))
+
+
+class IPersonLimitedView(IHasIcon, IHasLogo):
+    """IPerson attributes that require launchpad.LimitedView permission."""
+
+    name = exported(
+        PersonNameField(
+            title=_('Name'), required=True, readonly=False,
+            constraint=name_validator,
+            description=_(
+                "A short unique name, beginning with a lower-case "
+                "letter or number, and containing only letters, "
+                "numbers, dots, hyphens, or plus signs.")))
+    displayname = exported(
+        StrippedTextLine(
+            title=_('Display Name'), required=True, readonly=False,
+            description=_(
+                "Your name as you would like it displayed throughout "
+                "Launchpad. Most people use their full name here.")),
+        exported_as='display_name')
+    unique_displayname = TextLine(
+        title=_('Return a string of the form $displayname ($name).'))
     # NB at this stage we do not allow individual people to have their own
     # icon, only teams get that. People can however have a logo and mugshot
     # The icon is only used for teams; that's why we use /@@/team as the
@@ -690,7 +719,6 @@
             "displayed whenever the team name is listed - for example "
             "in listings of bugs or on a person's membership table."))
     iconID = Int(title=_('Icon ID'), required=True, readonly=True)
-
     logo = exported(
         LogoImageUpload(
             title=_("Logo"), required=False,
@@ -701,6 +729,30 @@
                 "is a logo, a small picture or a personal mascot. It should "
                 "be no bigger than 50kb in size.")))
     logoID = Int(title=_('Logo ID'), required=True, readonly=True)
+    # title is required for the Launchpad Page Layout main template
+    title = Attribute('Person Page Title')
+    is_probationary = exported(
+        Bool(title=_("Is this a probationary user?"), readonly=True))
+
+
+class IPersonViewRestricted(IHasBranches, IHasSpecifications,
+                    IHasMergeProposals, IHasMugshot,
+                    IHasLocation, IHasRequestedReviews, IObjectWithLocation,
+                    IHasBugs, IHasRecipes, IHasTranslationImports,
+                    IPersonSettings, IQuestionsPerson):
+    """IPerson attributes that require launchpad.View permission."""
+    account = Object(schema=IAccount)
+    accountID = Int(title=_('Account ID'), required=True, readonly=True)
+    password = PasswordField(
+        title=_('Password'), required=True, readonly=False)
+    karma = exported(
+        Int(title=_('Karma'), readonly=True,
+            description=_('The cached total karma for this person.')))
+    homepage_content = exported(
+        Text(title=_("Homepage Content"), required=False,
+            description=_(
+                "The content of your profile page. Use plain text, "
+                "paragraphs are preserved and URLs are linked in pages.")))
 
     mugshot = exported(MugshotImageUpload(
         title=_("Mugshot"), required=False,
@@ -766,15 +818,6 @@
     # Properties of the Person object.
     karma_category_caches = Attribute(
         'The caches of karma scores, by karma category.')
-    is_team = exported(
-        Bool(title=_('Is this object a team?'), readonly=True))
-    is_valid_person = Bool(
-        title=_("This is an active user and not a team."), readonly=True)
-    is_valid_person_or_team = exported(
-        Bool(title=_("This is an active user or a team."), readonly=True),
-        exported_as='is_valid')
-    is_probationary = exported(
-        Bool(title=_("Is this a probationary user?"), readonly=True))
     is_ubuntu_coc_signer = exported(
     Bool(title=_("Signed Ubuntu Code of Conduct"),
             readonly=True))
@@ -879,7 +922,6 @@
         exported_as='team_owner')
     teamownerID = Int(title=_("The Team Owner's ID or None"), required=False,
                       readonly=True)
-
     preferredemail = exported(
         Reference(title=_("Preferred email address"),
                description=_("The preferred email address for this person. "
@@ -916,9 +958,6 @@
             "this is set to None, then this Person has not been merged "
             "into another and is still valid"))
 
-    # title is required for the Launchpad Page Layout main template
-    title = Attribute('Person Page Title')
-
     archive = exported(
         Reference(
             title=_("Default PPA"),
@@ -987,18 +1026,6 @@
             readonly=True, required=False,
             value_type=Reference(schema=Interface)))  # HWSubmission
 
-    # This is redefined from IPrivacy.private because the attribute is
-    # read-only. It is a summary of the team's visibility.
-    private = exported(Bool(
-            title=_("This team is private"),
-            readonly=True, required=False,
-            description=_("Private teams are visible only to "
-                          "their members.")))
-
-    is_merge_pending = exported(Bool(
-        title=_("Is this person due to be merged with another?"),
-        required=False, default=False))
-
     administrated_teams = Attribute(
         u"the teams that this person/team is an administrator of.")
 
@@ -1463,32 +1490,6 @@
         :return: a boolean.
         """
 
-
-class IPersonLimitedView(Interface):
-    """IPerson attributes that require launchpad.LimitedView permission."""
-
-    name = exported(
-        PersonNameField(
-            title=_('Name'), required=True, readonly=False,
-            constraint=name_validator,
-            description=_(
-                "A short unique name, beginning with a lower-case "
-                "letter or number, and containing only letters, "
-                "numbers, dots, hyphens, or plus signs.")))
-    displayname = exported(
-        StrippedTextLine(
-            title=_('Display Name'), required=True, readonly=False,
-            description=_(
-                "Your name as you would like it displayed throughout "
-                "Launchpad. Most people use their full name here.")),
-        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
@@ -2565,18 +2566,19 @@
     ]:
     IPersonViewRestricted[name].value_type.schema = IPerson
 
-IPersonPublic['sub_teams'].value_type.schema = ITeam
-IPersonPublic['super_teams'].value_type.schema = ITeam
+IPersonViewRestricted['sub_teams'].value_type.schema = ITeam
+IPersonViewRestricted['super_teams'].value_type.schema = ITeam
 # XXX: salgado, 2008-08-01: Uncomment these when teams_*participated_in are
 # exported again.
-# IPersonPublic['teams_participated_in'].value_type.schema = ITeam
-# IPersonPublic['teams_indirectly_participated_in'].value_type.schema = ITeam
+# IPersonViewRestricted['teams_participated_in'].value_type.schema = ITeam
+# IPersonViewRestricted[
+#   'teams_indirectly_participated_in'].value_type.schema = ITeam
 
 # Fix schema of operation parameters. We need zope.deferredimport!
 params_to_fix = [
     # XXX: salgado, 2008-08-01: Uncomment these when they are exported again.
-    # (IPersonPublic['findPathToTeam'], 'team'),
-    # (IPersonPublic['inTeam'], 'team'),
+    # (IPersonViewRestricted['findPathToTeam'], 'team'),
+    # (IPersonViewRestricted['inTeam'], 'team'),
     (IPersonEditRestricted['join'], 'team'),
     (IPersonEditRestricted['leave'], 'team'),
     (IPersonEditRestricted['addMember'], 'person'),

=== modified file 'lib/lp/registry/interfaces/role.py'
--- lib/lp/registry/interfaces/role.py	2011-12-12 02:45:10 +0000
+++ lib/lp/registry/interfaces/role.py	2011-12-12 05:28:08 +0000
@@ -119,7 +119,8 @@
     def inTeam(team):
         """Is this person a member or the owner of `team`?
 
-        Passed through to the same method in 'IPersonPublic'.
+        Passed through to the *unproxied* same method in
+        `IPersonViewRestricted`.
         """
 
     def isOwner(obj):

=== modified file 'lib/lp/registry/model/personroles.py'
--- lib/lp/registry/model/personroles.py	2011-12-12 02:45:10 +0000
+++ lib/lp/registry/model/personroles.py	2011-12-12 05:28:08 +0000
@@ -11,6 +11,7 @@
     getUtility,
     )
 from zope.interface import implements
+from zope.security.proxy import removeSecurityProxy
 
 from lp.app.interfaces.launchpad import ILaunchpadCelebrities
 from lp.bugs.interfaces.bugsupervisor import IHasBugSupervisor
@@ -29,7 +30,8 @@
     def __init__(self, person):
         self.person = person
         self._celebrities = getUtility(ILaunchpadCelebrities)
-        self.inTeam = self.person.inTeam
+        # Use an unproxied inTeam() method for security checks.
+        self.inTeam = removeSecurityProxy(self.person).inTeam
 
     def __getattr__(self, name):
         """Handle all in_* attributes."""
@@ -39,7 +41,7 @@
             raise AttributeError(errortext)
         attribute = name[len(prefix):]
         try:
-            return self.person.inTeam(getattr(self._celebrities, attribute))
+            return self.inTeam(getattr(self._celebrities, attribute))
         except AttributeError:
             raise AttributeError(errortext)
 
@@ -49,28 +51,28 @@
 
     def isOwner(self, obj):
         """See IPersonRoles."""
-        return self.person.inTeam(obj.owner)
+        return self.inTeam(obj.owner)
 
     def isBugSupervisor(self, obj):
         """See IPersonRoles."""
         return (IHasBugSupervisor.providedBy(obj)
-                and self.person.inTeam(obj.bug_supervisor))
+                and self.inTeam(obj.bug_supervisor))
 
     def isSecurityContact(self, obj):
         """See IPersonRoles."""
         return (IHasSecurityContact.providedBy(obj)
-                and self.person.inTeam(obj.security_contact))
+                and self.inTeam(obj.security_contact))
 
     def isDriver(self, obj):
         """See IPersonRoles."""
-        return self.person.inTeam(obj.driver)
+        return self.inTeam(obj.driver)
 
     def isOneOfDrivers(self, obj):
         """See IPersonRoles."""
         if not IHasDrivers.providedBy(obj):
             return self.isDriver(obj)
         for driver in obj.drivers:
-            if self.person.inTeam(driver):
+            if self.inTeam(driver):
                 return True
         return False
 
@@ -78,6 +80,6 @@
         """See IPersonRoles."""
         for attr in attributes:
             role = getattr(obj, attr)
-            if self.person.inTeam(role):
+            if self.inTeam(role):
                 return True
         return False

=== modified file 'lib/lp/registry/stories/webservice/xx-derivedistroseries.txt'
--- lib/lp/registry/stories/webservice/xx-derivedistroseries.txt	2011-12-12 02:45:10 +0000
+++ lib/lp/registry/stories/webservice/xx-derivedistroseries.txt	2011-12-12 05:28:08 +0000
@@ -19,6 +19,7 @@
     >>> soyuz_team = factory.makeTeam(
     ...     name='soyuz-team',
     ...     subscription_policy=TeamSubscriptionPolicy.RESTRICTED)
+    >>> soyuz_team_owner = soyuz_team.teamowner
     >>> parent_series = factory.makeDistroSeries(name="parentseries")
     >>> arch = factory.makeDistroArchSeries(distroseries=parent_series)
     >>> removeSecurityProxy(parent_series).nominatedarchindep = arch
@@ -41,7 +42,7 @@
     >>> logout()
 
     >>> soyuz_team_webservice = webservice_for_person(
-    ...     soyuz_team.teamowner, permission=OAuthPermission.WRITE_PUBLIC)
+    ...     soyuz_team_owner, permission=OAuthPermission.WRITE_PUBLIC)
 
 
 Calling

=== modified file 'lib/lp/registry/tests/test_person_vocabularies.py'
--- lib/lp/registry/tests/test_person_vocabularies.py	2011-12-12 02:45:10 +0000
+++ lib/lp/registry/tests/test_person_vocabularies.py	2011-12-12 05:28:08 +0000
@@ -311,9 +311,10 @@
     def test_private_team_cannot_be_a_member_of_itself(self):
         # A private team should be filtered by the vocab.extra_clause
         # when provided a search term.
+        owner = self.factory.makePerson()
         team = self.factory.makeTeam(
-            visibility=PersonVisibility.PRIVATE)
-        login_person(team.teamowner)
+            owner=owner, visibility=PersonVisibility.PRIVATE)
+        login_person(owner)
         self.assertNotIn(team, self.searchVocabulary(team, team.name))
 
 

=== modified file 'lib/lp/registry/tests/test_team_webservice.py'
--- lib/lp/registry/tests/test_team_webservice.py	2011-12-12 02:45:10 +0000
+++ lib/lp/registry/tests/test_team_webservice.py	2011-12-12 05:28:08 +0000
@@ -5,8 +5,6 @@
 
 import httplib
 
-from zope.security.proxy import removeSecurityProxy
-
 from lazr.restfulclient.errors import HTTPError
 
 from canonical.testing.layers import DatabaseFunctionalLayer
@@ -79,8 +77,9 @@
         # Calling person.join with a team that has an open membership
         # subscription policy should add that that user to the team.
         self.person = self.factory.makePerson(name='test-person')
-        self.team = self.factory.makeTeam(name='test-team')
-        login_person(self.team.teamowner)
+        owner = self.factory.makePerson()
+        self.team = self.factory.makeTeam(name='test-team', owner=owner)
+        login_person(owner)
         self.team.subscriptionpolicy = TeamSubscriptionPolicy.OPEN
         logout()
 
@@ -88,7 +87,7 @@
         test_person = launchpad.people['test-person']
         test_team = launchpad.people['test-team']
         test_person.join(team=test_team.self_link)
-        login_person(self.team.teamowner)
+        login_person(owner)
         self.assertEqual(
             ['test-team'],
             [membership.team.name

=== modified file 'lib/lp/soyuz/tests/test_archive_subscriptions.py'
--- lib/lp/soyuz/tests/test_archive_subscriptions.py	2011-12-12 02:45:10 +0000
+++ lib/lp/soyuz/tests/test_archive_subscriptions.py	2011-12-12 05:28:08 +0000
@@ -13,6 +13,7 @@
     TestCaseWithFactory,
     )
 from lp.testing.mail_helpers import pop_notifications
+from lp.testing.views import create_initialized_view
 
 
 class TestArchiveSubscriptions(TestCaseWithFactory):
@@ -26,9 +27,11 @@
     def setUp(self):
         """Create a test archive."""
         super(TestArchiveSubscriptions, self).setUp()
+        self.owner = self.factory.makePerson()
         self.private_team = self.factory.makeTeam(
-            visibility=PersonVisibility.PRIVATE, name="subscribertest")
-        login_person(self.private_team.teamowner)
+            visibility=PersonVisibility.PRIVATE,
+            name="subscribertest", owner=self.owner)
+        login_person(self.owner)
         self.archive = self.factory.makeArchive(
             private=True, owner=self.private_team)
         self.subscriber = self.factory.makePerson()
@@ -45,7 +48,7 @@
         login_person(self.subscriber)
         self.assertRaises(Unauthorized, get_name)
 
-        login_person(self.private_team.teamowner)
+        login_person(self.owner)
         self.archive.newSubscription(
             self.subscriber, registrant=self.archive.owner)
 
@@ -53,6 +56,33 @@
         login_person(self.subscriber)
         self.assertEqual(self.archive.owner.name, "subscribertest")
 
+    def test_subscriber_can_browse_private_team_ppa(self):
+        # As per bug 597783, we need to make sure a subscriber can see
+        # a private team's PPA after they have been given a subscription.
+        # This test ensures the subscriber can correctly load the PPA's view,
+        # thus ensuring that all attributes necessary to render the view have
+        # the necessary security permissions.
+
+        # Before a subscription, accessing the view name will raise.
+        login_person(self.subscriber)
+        view = create_initialized_view(
+            self.archive, '+index', principal=self.subscriber)
+        self.assertRaises(Unauthorized, view.render)
+
+        login_person(self.owner)
+        self.archive.newSubscription(
+            self.subscriber, registrant=self.archive.owner)
+
+        # When a subscription exists, it's fine.
+        login_person(self.subscriber)
+        self.assertIn(self.archive.displayname, view.render())
+
+        # Just to double check, by default, the subscriber still can't see the
+        # +packages view which requires extra permissions.
+        self.assertRaises(
+            Unauthorized, create_initialized_view,
+            self.archive, '+packages', principal=self.subscriber)
+
     def test_new_subscription_sends_email(self):
         # Creating a new subscription sends an email to all members
         # of the person or team subscribed.

=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py	2011-12-12 02:45:10 +0000
+++ lib/lp/testing/factory.py	2011-12-12 05:28:08 +0000
@@ -772,7 +772,7 @@
             address, person, email_status, account)
 
     def makeTeam(self, owner=None, displayname=None, email=None, name=None,
-                 description=None,
+                 description=None, icon=None, logo=None,
                  subscription_policy=TeamSubscriptionPolicy.OPEN,
                  visibility=None, members=None):
         """Create and return a new, arbitrary Team.
@@ -783,9 +783,11 @@
         :param displayname: The team's display name.  If not given we'll use
             the auto-generated name.
         :param description: Team team's description.
-        :type string:
+        :type description string:
         :param email: The email address to use as the team's contact address.
         :type email: string
+        :param icon: The team's icon.
+        :param logo: The team's logo.
         :param subscription_policy: The subscription policy of the team.
         :type subscription_policy: `TeamSubscriptionPolicy`
         :param visibility: The team's visibility. If it's None, the default
@@ -810,15 +812,19 @@
         team = getUtility(IPersonSet).newTeam(
             owner, name, displayname, teamdescription=description,
             subscriptionpolicy=subscription_policy)
+        naked_team = removeSecurityProxy(team)
         if visibility is not None:
             # Visibility is normally restricted to launchpad.Commercial, so
             # removing the security proxy as we don't care here.
-            removeSecurityProxy(team).visibility = visibility
+            naked_team.visibility = visibility
         if email is not None:
             team.setContactAddress(
                 getUtility(IEmailAddressSet).new(email, team))
+        if icon is not None:
+            naked_team.icon = icon
+        if logo is not None:
+            naked_team.logo = logo
         if members is not None:
-            naked_team = removeSecurityProxy(team)
             for member in members:
                 naked_team.addMember(member, owner)
         return team
@@ -1124,7 +1130,7 @@
 
         if registrant is None:
             if owner.is_team:
-                registrant = owner.teamowner
+                registrant = removeSecurityProxy(owner).teamowner
             else:
                 registrant = owner