Merge lp:~sinzui/launchpad/rollback-private-traversal into lp:launchpad

[Curtis Hovey]

[r=sinzui] Rollback revisions 14489, 14490, and 14493 because they block p3a subscribers.

Rollback revisions 14489, 14490, and 14493 because they block p3a subscribers.

[Curtis Hovey (sinzui)]

Self-approving to unblock the next release.

review: Approve (code)

=== modified file 'lib/canonical/launchpad/interfaces/_schema_circular_imports.py'
--- lib/canonical/launchpad/interfaces/_schema_circular_imports.py	2011-12-12 04:47:21 +0000
+++ lib/canonical/launchpad/interfaces/_schema_circular_imports.py	2011-12-13 14:07:37 +0000
@@ -135,7 +135,7 @@
     )
 from lp.registry.interfaces.person import (
     IPerson,
-    IPersonViewRestricted,
+    IPersonPublic,
     ITeam,
     )
 from lp.registry.interfaces.pillar import (
@@ -310,10 +310,10 @@
 
 IPreviewDiff['branch_merge_proposal'].schema = IBranchMergeProposal
 
-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)
+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)
 
 IHasBuildRecords['getBuildRecords'].queryTaggedValue(
     LAZR_WEBSERVICE_EXPORTED)[

=== modified file 'lib/canonical/launchpad/security.py'
--- lib/canonical/launchpad/security.py	2011-12-12 04:47:21 +0000
+++ lib/canonical/launchpad/security.py	2011-12-13 14:07:37 +0000
@@ -63,7 +63,6 @@
     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
@@ -801,35 +800,6 @@
             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):
@@ -843,14 +813,33 @@
                 ppa.id for ppa in self.obj.ppas if ppa.private)
             if len(subscriber_archive_ids.intersection(team_ppa_ids)) > 0:
                 return True
-
-            # 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
+        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')
 
 
 class EditPollByTeamOwnerOrTeamAdminsOrAdmins(

=== modified file 'lib/canonical/launchpad/webapp/authorization.py'
--- lib/canonical/launchpad/webapp/authorization.py	2011-12-12 04:47:21 +0000
+++ lib/canonical/launchpad/webapp/authorization.py	2011-12-13 14:07:37 +0000
@@ -192,7 +192,7 @@
                     objecttoauthorize, {})
                 if permission in object_cache:
                     return object_cache[permission]
-            principal = removeAllProxies(participation.principal)
+            principal = 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 04:47:21 +0000
+++ lib/canonical/launchpad/webapp/interaction.py	2011-12-13 14:07:37 +0000
@@ -163,8 +163,7 @@
         return setupInteraction(ANONYMOUS, participation)
     else:
         # Bypass zope's security because IEmailAddress.email is not public.
-        naked_person = removeSecurityProxy(person)
-        naked_email = removeSecurityProxy(naked_person.preferredemail)
+        naked_email = removeSecurityProxy(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 18:46:47 +0000
+++ lib/lp/app/browser/launchpad.py	2011-12-13 14:07:37 +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.LimitedView', person)):
+                if (person.is_team
+                    and not check_permission('launchpad.View', 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 04:47:21 +0000
+++ lib/lp/app/browser/tales.py	2011-12-13 14:07:37 +0000
@@ -1266,14 +1266,6 @@
                 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 04:47:21 +0000
+++ lib/lp/app/tests/test_tales.py	2011-12-13 14:07:37 +0000
@@ -149,20 +149,18 @@
         self.assertEqual(expected, result)
 
 
-class TestTeamFormatterAPI(TestCaseWithFactory):
-    """ Test permissions required to access TeamFormatterAPI methods.
+class TestTalesFormatterAPI(TestCaseWithFactory):
+    """ Test permissions required to access TalesFormatterAPI methods.
 
     A user must have launchpad.LimitedView permission to use
-    TeamFormatterAPI with private teams.
+    TestTalesFormatterAPI with private teams.
     """
-    layer = LaunchpadFunctionalLayer
+    layer = DatabaseFunctionalLayer
 
     def setUp(self):
-        super(TestTeamFormatterAPI, self).setUp()
-        icon = self.factory.makeLibraryFileAlias(
-            filename='smurf.png', content_type='image/png')
+        super(TestTalesFormatterAPI, self).setUp()
         self.team = self.factory.makeTeam(
-            name='team', displayname='a team', icon=icon,
+            name='team', displayname='a team',
             visibility=PersonVisibility.PRIVATE)
 
     def _make_formatter(self, cache_permission=False):
@@ -177,11 +175,10 @@
                 request, 'launchpad.LimitedView', [self.team])
         return formatter, request, any_person
 
-    def _tales_value(self, attr, request, path='fmt'):
+    def _tales_value(self, attr, request):
         # Evaluate the given formatted attribute value on team.
-        result = test_tales(
-            "team/%s:%s" % (path, attr), team=self.team, request=request)
-        return result
+        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.
@@ -231,10 +228,6 @@
     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 04:47:21 +0000
+++ lib/lp/bugs/browser/tests/test_bugsubscription_views.py	2011-12-13 14:07:37 +0000
@@ -629,7 +629,7 @@
         direct_subscriber = self.factory.makeTeam(
             name='team', displayname='Team Name', owner=teamowner,
             visibility=PersonVisibility.PRIVATE)
-        with person_logged_in(teamowner):
+        with person_logged_in(direct_subscriber.teamowner):
             bug.subscribe(direct_subscriber, direct_subscriber.teamowner,
                           level=BugNotificationLevel.LIFECYCLE)
 

=== modified file 'lib/lp/code/browser/branch.py'
--- lib/lp/code/browser/branch.py	2011-12-01 11:39:30 +0000
+++ lib/lp/code/browser/branch.py	2011-12-13 14:07:37 +0000
@@ -96,10 +96,7 @@
     stepthrough,
     stepto,
     )
-from canonical.launchpad.webapp.authorization import (
-    check_permission,
-    precache_permission_for_objects,
-    )
+from canonical.launchpad.webapp.authorization import check_permission
 from canonical.launchpad.webapp.interfaces import ICanonicalUrlData
 from canonical.launchpad.webapp.menu import structured
 from lp.app.browser.launchpad import Hierarchy
@@ -445,11 +442,6 @@
     def initialize(self):
         self.branch = self.context
         self.notices = []
-        # Cache permission so private team owner can be rendered.
-        authorised_people = [self.branch.owner]
-        if self.user is not None:
-            precache_permission_for_objects(
-                self.request, "launchpad.LimitedView", authorised_people)
         # Replace our context with a decorated branch, if it is not already
         # decorated.
         if not isinstance(self.context, DecoratedBranch):

=== modified file 'lib/lp/code/browser/branchsubscription.py'
--- lib/lp/code/browser/branchsubscription.py	2011-12-01 11:27:29 +0000
+++ lib/lp/code/browser/branchsubscription.py	2011-12-13 14:07:37 +0000
@@ -19,10 +19,7 @@
     canonical_url,
     LaunchpadView,
     )
-from canonical.launchpad.webapp.authorization import (
-    check_permission,
-    precache_permission_for_objects,
-    )
+from canonical.launchpad.webapp.authorization import check_permission
 from canonical.launchpad.webapp.interfaces import IPrimaryContext
 from canonical.launchpad.webapp.menu import structured
 from lp.app.browser.launchpadform import (
@@ -51,18 +48,9 @@
 
     def subscriptions(self):
         """Return a decorated list of branch subscriptions."""
-
-        # Cache permissions so private subscribers can be rendered.
-        if self.user is not None:
-            subscribers = [
-                subscription.person
-                for subscription in self.context.subscriptions]
-            precache_permission_for_objects(
-                self.request, "launchpad.LimitedView", subscribers)
-
         visible_subscriptions = [
             subscription for subscription in self.context.subscriptions
-            if check_permission('launchpad.LimitedView', subscription.person)]
+            if check_permission('launchpad.View', subscription.person)]
         return sorted(
             visible_subscriptions,
             key=lambda subscription: subscription.person.displayname)

=== modified file 'lib/lp/code/browser/tests/test_branch.py'
--- lib/lp/code/browser/tests/test_branch.py	2011-12-12 18:22:50 +0000
+++ lib/lp/code/browser/tests/test_branch.py	2011-12-13 14:07:37 +0000
@@ -5,14 +5,12 @@
 
 __metaclass__ = type
 
-from BeautifulSoup import BeautifulSoup
 from datetime import (
     datetime,
     )
 from textwrap import dedent
 
 import pytz
-from zope.publisher.interfaces import NotFound
 from zope.security.proxy import removeSecurityProxy
 
 from canonical.config import config
@@ -28,7 +26,6 @@
     extract_text,
     find_tag_by_id,
     setupBrowser,
-    setupBrowserForUser
     )
 from lp.app.interfaces.headings import IRootContext
 from lp.bugs.interfaces.bugtask import (
@@ -53,7 +50,6 @@
     BranchVisibilityRule,
     )
 from lp.code.interfaces.branchtarget import IBranchTarget
-from lp.registry.interfaces.person import PersonVisibility
 from lp.testing import (
     BrowserTestCase,
     login,
@@ -545,102 +541,6 @@
         self.assertEqual(linked_bug_urls[1], links[4]['href'])
 
 
-class TestBranchViewPrivateArtifacts(BrowserTestCase):
-    """ Tests that branches with private team artifacts can be viewed.
-
-    A Branch may be associated with a private team as follows:
-    - the owner is a private team
-    - a subscriber is a private team
-
-    A logged in user who is not authorised to see the private team(s) still
-    needs to be able to view the branch. The private team will be rendered in
-    the normal way, displaying the team name and Launchpad URL.
-    """
-
-    layer = DatabaseFunctionalLayer
-
-    def _getBrowser(self, user=None):
-        if user is None:
-            browser = setupBrowser()
-            logout()
-            return browser
-        else:
-            login_person(user)
-            return setupBrowserForUser(user=user)
-
-    def test_view_branch_with_private_owner(self):
-        # A branch with a private owner is rendered.
-        private_owner = self.factory.makeTeam(
-            displayname="PrivateTeam", visibility=PersonVisibility.PRIVATE)
-        branch = self.factory.makeAnyBranch(owner=private_owner)
-        # Ensure the branch owner is rendered.
-        url = canonical_url(branch, rootsite='code')
-        user = self.factory.makePerson()
-        browser = self._getBrowser(user)
-        browser.open(url)
-        soup = BeautifulSoup(browser.contents)
-        self.assertIsNotNone(soup.find('a', text="PrivateTeam"))
-
-    def test_view_private_branch_with_private_owner(self):
-        # A private branch with a private owner is rendered.
-        private_owner = self.factory.makeTeam(
-            displayname="PrivateTeam", visibility=PersonVisibility.PRIVATE)
-        branch = self.factory.makeAnyBranch(owner=private_owner)
-        # Ensure the branch owner is rendered.
-        url = canonical_url(branch, rootsite='code')
-        user = self.factory.makePerson()
-        # Subscribe the user so they can see the branch.
-        with person_logged_in(private_owner):
-            self.factory.makeBranchSubscription(branch, user, private_owner)
-        browser = self._getBrowser(user)
-        browser.open(url)
-        soup = BeautifulSoup(browser.contents)
-        self.assertIsNotNone(soup.find('a', text="PrivateTeam"))
-
-    def test_anonymous_view_branch_with_private_owner(self):
-        # A branch with a private owner is not rendered for anon users.
-        private_owner = self.factory.makeTeam(
-            visibility=PersonVisibility.PRIVATE)
-        branch = self.factory.makeAnyBranch(owner=private_owner)
-        # Viewing the branch results in an error.
-        url = canonical_url(branch, rootsite='code')
-        browser = self._getBrowser()
-        self.assertRaises(NotFound, browser.open, url)
-
-    def test_view_branch_with_private_subscriber(self):
-        # A branch with a private subscriber is rendered.
-        private_subscriber = self.factory.makeTeam(
-            name="privateteam", visibility=PersonVisibility.PRIVATE)
-        branch = self.factory.makeAnyBranch()
-        with person_logged_in(branch.owner):
-            self.factory.makeBranchSubscription(
-                branch, private_subscriber, branch.owner)
-        # Ensure the branch subscriber is rendered.
-        url = canonical_url(branch, rootsite='code')
-        user = self.factory.makePerson()
-        browser = self._getBrowser(user)
-        browser.open(url)
-        soup = BeautifulSoup(browser.contents)
-        self.assertIsNotNone(
-            soup.find('div', attrs={'id': 'subscriber-privateteam'}))
-
-    def test_anonymous_view_branch_with_private_subscriber(self):
-        # A branch with a private subscriber is not rendered for anon users.
-        private_subscriber = self.factory.makeTeam(
-            name="privateteam", visibility=PersonVisibility.PRIVATE)
-        branch = self.factory.makeAnyBranch()
-        with person_logged_in(branch.owner):
-            self.factory.makeBranchSubscription(
-                branch, private_subscriber, branch.owner)
-        # Viewing the branch results in an error.
-        url = canonical_url(branch, rootsite='code')
-        browser = self._getBrowser()
-        browser.open(url)
-        soup = BeautifulSoup(browser.contents)
-        self.assertIsNone(
-            soup.find('div', attrs={'id': 'subscriber-privateteam'}))
-
-
 class TestBranchAddView(TestCaseWithFactory):
     """Test the BranchAddView view."""
 

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

=== modified file 'lib/lp/code/stories/branches/xx-subscribing-branches.txt'
--- lib/lp/code/stories/branches/xx-subscribing-branches.txt	2011-12-12 18:29:00 +0000
+++ lib/lp/code/stories/branches/xx-subscribing-branches.txt	2011-12-13 14:07:37 +0000
@@ -254,19 +254,15 @@
     Mark Shuttleworth
 
 
-Private team's in public subscriptions
-======================================
-
-If a private team is subscribed to a publicthe branch, it is visible
-to everyone.
-
-    >>> from lp.testing import login, logout
+Private team exclusion
+======================
+
+If a private team is subscribed to the branch, it is excluded from the list of
+subscribers if the user is not an admin or a member of the team.
+
+    >>> from lp.testing import ANONYMOUS, login, logout
+    >>> login('[e-mail]')
     >>> from lp.registry.interfaces.person import PersonVisibility
-    >>> from lp.code.enums import (
-    ...     BranchSubscriptionNotificationLevel,
-    ...     CodeReviewNotificationLevel)
-
-    >>> login('[e-mail]')
     >>> private_team = factory.makeTeam(
     ...     name='shh', displayname='Shh',
     ...     visibility=PersonVisibility.PRIVATE)
@@ -275,16 +271,26 @@
     >>> ignored = private_team.addMember(member, private_team.teamowner)
     >>> owner = factory.makePerson(name='branch-owner')
     >>> branch = factory.makeAnyBranch(owner=owner)
+    >>> from lp.code.enums import (
+    ...     BranchSubscriptionNotificationLevel,
+    ...     CodeReviewNotificationLevel)
     >>> ignored = branch.subscribe(
     ...     private_team, BranchSubscriptionNotificationLevel.NOEMAIL, None,
     ...     CodeReviewNotificationLevel.NOEMAIL, private_team.teamowner)
     >>> url = canonical_url(branch)
     >>> logout()
 
-No-priv is not a member of the private team, but he can see the team's
-display name in the subscriber list.
+No-priv is not a member of the private team, so the private team is not shown
+as a subscriber.
 
     >>> browser.open(url)
     >>> print_subscribers(browser.contents)
     Branch-owner
+
+A team member looking at the branch will see the private team subscribed.
+
+    >>> private_browser = setupBrowser(auth='Basic [e-mail]:test')
+    >>> private_browser.open(url)
+    >>> print_subscribers(private_browser.contents)
+    Branch-owner
     Shh

=== modified file 'lib/lp/registry/browser/tests/test_mailinglists.py'
--- lib/lp/registry/browser/tests/test_mailinglists.py	2011-12-12 04:47:21 +0000
+++ lib/lp/registry/browser/tests/test_mailinglists.py	2011-12-13 14:07:37 +0000
@@ -28,12 +28,11 @@
     """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(owner)
-        self.factory.makeMailingList(team=team, owner=owner)
+        login_person(team.teamowner)
+        team_list = self.factory.makeMailingList(
+            team=team, owner=team.teamowner)
         return team
 
 
@@ -130,11 +129,10 @@
 
     def test_private_message_message_without_list(self):
         # Private teams have private archives.
-        owner = self.factory.makePerson()
         team = self.factory.makeTeam(
-            owner=owner, visibility=PersonVisibility.PRIVATE)
-        login_person(owner)
+            visibility=PersonVisibility.PRIVATE)
+        login_person(team.teamowner)
         view = create_initialized_view(
-            team, name='+mailinglist', principal=owner)
+            team, name='+mailinglist', principal=team.teamowner)
         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-13 10:35:57 +0000
+++ lib/lp/registry/browser/tests/test_person_view.py	2011-12-13 14:07:37 +0000
@@ -534,11 +534,9 @@
 
     def test_active_participations_with_direct_private_team(self):
         # Users cannot see private teams that they are not members of.
-        owner = self.factory.makePerson()
-        team = self.factory.makeTeam(
-            owner=owner, visibility=PersonVisibility.PRIVATE)
-        login_person(owner)
-        team.addMember(self.user, owner)
+        team = self.factory.makeTeam(visibility=PersonVisibility.PRIVATE)
+        login_person(team.teamowner)
+        team.addMember(self.user, team.teamowner)
         # The team is included in active_participations.
         login_person(self.user)
         view = create_view(
@@ -553,13 +551,11 @@
 
     def test_active_participations_with_indirect_private_team(self):
         # Users cannot see private teams that they are not members of.
-        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)
+        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)
         # 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 04:47:21 +0000
+++ lib/lp/registry/configure.zcml	2011-12-13 14:07:37 +0000
@@ -912,6 +912,8 @@
             <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"/>
@@ -922,9 +924,6 @@
                 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
@@ -932,10 +931,9 @@
                 interface="lp.registry.interfaces.person.IPersonEditRestricted"/>
             <require
                 permission="launchpad.Edit"
-                set_schema="lp.registry.interfaces.person.IPersonViewRestricted
-                            lp.registry.interfaces.person.IPersonPublic
+                set_schema="lp.registry.interfaces.person.IPersonPublic
                             lp.registry.interfaces.person.ITeamPublic"
-                set_attributes="displayname icon logo"/>
+                set_attributes="displayname"/>
             <require
                 permission="launchpad.Moderate"
                 set_attributes="name"/>

=== modified file 'lib/lp/registry/doc/person.txt'
--- lib/lp/registry/doc/person.txt	2011-12-12 04:47:21 +0000
+++ lib/lp/registry/doc/person.txt	2011-12-13 14:07:37 +0000
@@ -696,7 +696,6 @@
     >>> 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)
 
@@ -807,7 +806,7 @@
 
 But Owner, a member of that team, will see it in the results.
 
-    >>> login_person(private_team_owner)
+    >>> login_person(private_team.teamowner)
     >>> 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 04:47:21 +0000
+++ lib/lp/registry/doc/private-team-visibility.txt	2011-12-13 14:07:37 +0000
@@ -107,64 +107,3 @@
     ...
     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-13 00:32:04 +0000
+++ lib/lp/registry/interfaces/person.py	2011-12-13 14:07:37 +0000
@@ -15,7 +15,7 @@
     'IObjectReassignment',
     'IPerson',
     'IPersonClaim',
-    'IPersonPublic',
+    'IPersonPublic',  # Required for a monkey patch in interfaces/archive.py
     'IPersonSet',
     'IPersonSettings',
     'ISoftwareCenterAgentAPI',
@@ -657,61 +657,26 @@
         required=False, default=False)
 
 
-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.
-    """
+class IPersonPublic(IHasBranches, IHasSpecifications,
+                    IHasMergeProposals, IHasLogo, IHasMugshot, IHasIcon,
+                    IHasLocation, IHasRequestedReviews, IObjectWithLocation,
+                    IPrivacy, IHasBugs, IHasRecipes, IHasTranslationImports,
+                    IPersonSettings, IQuestionsPerson):
+    """Public attributes for a Person."""
 
     id = Int(title=_('ID'), required=True, readonly=True)
-    # 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))
-    account_status = Choice(
-        title=_("The status of this person's account"), required=False,
-        readonly=True, vocabulary=AccountStatus)
-    account_status_comment = Text(
-        title=_("Why are you deactivating your account?"), required=False,
-        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).'))
+    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.")))
     # 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
@@ -725,6 +690,7 @@
             "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,
@@ -735,30 +701,6 @@
                 "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,
@@ -813,9 +755,26 @@
                 readonly=False, required=False,
                 value_type=Reference(schema=ISSHKey)))
 
+    account_status = Choice(
+        title=_("The status of this person's account"), required=False,
+        readonly=True, vocabulary=AccountStatus)
+
+    account_status_comment = Text(
+        title=_("Why are you deactivating your account?"), required=False,
+        readonly=True)
+
     # 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))
@@ -920,6 +879,7 @@
         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. "
@@ -956,6 +916,9 @@
             "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"),
@@ -1024,6 +987,18 @@
             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.")
 
@@ -1488,6 +1463,32 @@
         :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
@@ -2564,19 +2565,18 @@
     ]:
     IPersonViewRestricted[name].value_type.schema = IPerson
 
-IPersonViewRestricted['sub_teams'].value_type.schema = ITeam
-IPersonViewRestricted['super_teams'].value_type.schema = ITeam
+IPersonPublic['sub_teams'].value_type.schema = ITeam
+IPersonPublic['super_teams'].value_type.schema = ITeam
 # XXX: salgado, 2008-08-01: Uncomment these when teams_*participated_in are
 # exported again.
-# IPersonViewRestricted['teams_participated_in'].value_type.schema = ITeam
-# IPersonViewRestricted[
-#   'teams_indirectly_participated_in'].value_type.schema = ITeam
+# IPersonPublic['teams_participated_in'].value_type.schema = ITeam
+# IPersonPublic['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.
-    # (IPersonViewRestricted['findPathToTeam'], 'team'),
-    # (IPersonViewRestricted['inTeam'], 'team'),
+    # (IPersonPublic['findPathToTeam'], 'team'),
+    # (IPersonPublic['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 04:47:21 +0000
+++ lib/lp/registry/interfaces/role.py	2011-12-13 14:07:37 +0000
@@ -119,8 +119,7 @@
     def inTeam(team):
         """Is this person a member or the owner of `team`?
 
-        Passed through to the *unproxied* same method in
-        `IPersonViewRestricted`.
+        Passed through to the same method in 'IPersonPublic'.
         """
 
     def isOwner(obj):

=== modified file 'lib/lp/registry/model/personroles.py'
--- lib/lp/registry/model/personroles.py	2011-12-12 04:47:21 +0000
+++ lib/lp/registry/model/personroles.py	2011-12-13 14:07:37 +0000
@@ -11,7 +11,6 @@
     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
@@ -30,8 +29,7 @@
     def __init__(self, person):
         self.person = person
         self._celebrities = getUtility(ILaunchpadCelebrities)
-        # Use an unproxied inTeam() method for security checks.
-        self.inTeam = removeSecurityProxy(self.person).inTeam
+        self.inTeam = self.person.inTeam
 
     def __getattr__(self, name):
         """Handle all in_* attributes."""
@@ -41,7 +39,7 @@
             raise AttributeError(errortext)
         attribute = name[len(prefix):]
         try:
-            return self.inTeam(getattr(self._celebrities, attribute))
+            return self.person.inTeam(getattr(self._celebrities, attribute))
         except AttributeError:
             raise AttributeError(errortext)
 
@@ -51,28 +49,28 @@
 
     def isOwner(self, obj):
         """See IPersonRoles."""
-        return self.inTeam(obj.owner)
+        return self.person.inTeam(obj.owner)
 
     def isBugSupervisor(self, obj):
         """See IPersonRoles."""
         return (IHasBugSupervisor.providedBy(obj)
-                and self.inTeam(obj.bug_supervisor))
+                and self.person.inTeam(obj.bug_supervisor))
 
     def isSecurityContact(self, obj):
         """See IPersonRoles."""
         return (IHasSecurityContact.providedBy(obj)
-                and self.inTeam(obj.security_contact))
+                and self.person.inTeam(obj.security_contact))
 
     def isDriver(self, obj):
         """See IPersonRoles."""
-        return self.inTeam(obj.driver)
+        return self.person.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.inTeam(driver):
+            if self.person.inTeam(driver):
                 return True
         return False
 
@@ -80,6 +78,6 @@
         """See IPersonRoles."""
         for attr in attributes:
             role = getattr(obj, attr)
-            if self.inTeam(role):
+            if self.person.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 04:47:21 +0000
+++ lib/lp/registry/stories/webservice/xx-derivedistroseries.txt	2011-12-13 14:07:37 +0000
@@ -19,7 +19,6 @@
     >>> 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
@@ -42,7 +41,7 @@
     >>> logout()
 
     >>> soyuz_team_webservice = webservice_for_person(
-    ...     soyuz_team_owner, permission=OAuthPermission.WRITE_PUBLIC)
+    ...     soyuz_team.teamowner, 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 04:47:21 +0000
+++ lib/lp/registry/tests/test_person_vocabularies.py	2011-12-13 14:07:37 +0000
@@ -311,10 +311,9 @@
     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(
-            owner=owner, visibility=PersonVisibility.PRIVATE)
-        login_person(owner)
+            visibility=PersonVisibility.PRIVATE)
+        login_person(team.teamowner)
         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 04:47:21 +0000
+++ lib/lp/registry/tests/test_team_webservice.py	2011-12-13 14:07:37 +0000
@@ -5,6 +5,8 @@
 
 import httplib
 
+from zope.security.proxy import removeSecurityProxy
+
 from lazr.restfulclient.errors import HTTPError
 
 from canonical.testing.layers import DatabaseFunctionalLayer
@@ -77,9 +79,8 @@
         # 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')
-        owner = self.factory.makePerson()
-        self.team = self.factory.makeTeam(name='test-team', owner=owner)
-        login_person(owner)
+        self.team = self.factory.makeTeam(name='test-team')
+        login_person(self.team.teamowner)
         self.team.subscriptionpolicy = TeamSubscriptionPolicy.OPEN
         logout()
 
@@ -87,7 +88,7 @@
         test_person = launchpad.people['test-person']
         test_team = launchpad.people['test-team']
         test_person.join(team=test_team.self_link)
-        login_person(owner)
+        login_person(self.team.teamowner)
         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 05:21:01 +0000
+++ lib/lp/soyuz/tests/test_archive_subscriptions.py	2011-12-13 14:07:37 +0000
@@ -13,7 +13,6 @@
     TestCaseWithFactory,
     )
 from lp.testing.mail_helpers import pop_notifications
-from lp.testing.views import create_initialized_view
 
 
 class TestArchiveSubscriptions(TestCaseWithFactory):
@@ -27,11 +26,9 @@
     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", owner=self.owner)
-        login_person(self.owner)
+            visibility=PersonVisibility.PRIVATE, name="subscribertest")
+        login_person(self.private_team.teamowner)
         self.archive = self.factory.makeArchive(
             private=True, owner=self.private_team)
         self.subscriber = self.factory.makePerson()
@@ -48,7 +45,7 @@
         login_person(self.subscriber)
         self.assertRaises(Unauthorized, get_name)
 
-        login_person(self.owner)
+        login_person(self.private_team.teamowner)
         self.archive.newSubscription(
             self.subscriber, registrant=self.archive.owner)
 
@@ -56,33 +53,6 @@
         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 04:47:21 +0000
+++ lib/lp/testing/factory.py	2011-12-13 14:07:37 +0000
@@ -772,7 +772,7 @@
             address, person, email_status, account)
 
     def makeTeam(self, owner=None, displayname=None, email=None, name=None,
-                 description=None, icon=None, logo=None,
+                 description=None,
                  subscription_policy=TeamSubscriptionPolicy.OPEN,
                  visibility=None, members=None):
         """Create and return a new, arbitrary Team.
@@ -783,11 +783,9 @@
         :param displayname: The team's display name.  If not given we'll use
             the auto-generated name.
         :param description: Team team's description.
-        :type description string:
+        :type 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
@@ -812,19 +810,15 @@
         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.
-            naked_team.visibility = visibility
+            removeSecurityProxy(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
@@ -1130,7 +1124,7 @@
 
         if registrant is None:
             if owner.is_team:
-                registrant = removeSecurityProxy(owner).teamowner
+                registrant = owner.teamowner
             else:
                 registrant = owner