Launchpad itself

Merge lp:~bac/launchpad/bug-498179 into lp:launchpad

  1. bug-498179
  2. Merge into devel
Proposed by Brad Crittenden
Status: Merged
Approved by: Edwin Grubbs
Approved revision: not available
Merged at revision: not available
Proposed branch: lp:~bac/launchpad/bug-498179
Merge into: lp:launchpad
Diff against target: 125 lines (+99/-0)
3 files modified
lib/canonical/launchpad/security.py (+5/-0)
lib/lp/registry/doc/private-team-roles.txt (+1/-0)
lib/lp/registry/doc/private-team-visibility.txt (+93/-0)
To merge this branch: bzr merge lp:~bac/launchpad/bug-498179
Reviewer Review Type Date Requested Status
Edwin Grubbs (community) code Approve
Review via email: mp+16806@code.launchpad.net

Commit message

Allow admins of a public team invited to join a private team to see the information for the private team.

To post a comment you must log in.
Revision history for this message
Brad Crittenden (bac) wrote :

= Summary =

Private and Private Membership teams can invite public teams to become members. When
that happens an admin of the public team must go to a page to accept the membership.
 Since the admin is likely not a member of the private team she is barred from
viewing anything related to the private team, resulting in an OOPS as reported in bug
498179.

== Proposed fix ==

Change the security rules to allow the admins of invited teams to see the private parts.

== Pre-implementation notes ==

None.

== Implementation details ==

As above.

== Tests ==

bin/test -vvt private-team-visibility.txt

== Demo and Q/A ==

Create a private team.
Create a public team.
As an admin on the private team invite the private team to be a member.
As an admin of the public team attempt to accept the invitation.

= Launchpad lint =

Checking for conflicts. and issues in doctests and templates.
Running jslint, xmllint, pyflakes, and pylint.
Using normal rules.

Linting changed files:
  lib/canonical/launchpad/security.py
  lib/lp/registry/doc/private-team-roles.txt
  lib/lp/registry/doc/private-team-visibility.txt

Revision history for this message
Edwin Grubbs (edwin-grubbs) wrote :
Download full text (3.4 KiB)

Hi Brad,

This is a nice improvement. Besides what we discussed on IRC, I have a couple of minor comments below.

merge-conditional

-Edwin

IRC Log
-------

[3:00 PM] <EdwinGrubbs> bac: What is the purpose of adding the pub_member to the pub_team? It appears to have no effect on the test.
[3:00 PM] <bac> EdwinGrubbs: likely a mistake
[3:01 PM] <bac> EdwinGrubbs: oh, i was going to show that even though pub_owner could see private parts the mere pub_member could not
[3:01 PM] <bac> EdwinGrubbs: i think that's still a good thing to do, i just forgot to do it
[3:05 PM] <bac> EdwinGrubbs: this is what i intended: http://pastebin.ubuntu.com/351429/
{{{
=== modified file 'lib/lp/registry/doc/private-team-visibility.txt'
--- lib/lp/registry/doc/private-team-visibility.txt 2010-01-04 19:33:33 +0000
+++ lib/lp/registry/doc/private-team-visibility.txt 2010-01-04 21:04:34 +0000
@@ -75,9 +75,18 @@
     >>> pub_owner in priv_team.activemembers
     False

-But the public team's owner can see the priv-team's bits since his team
+The public team's owner can now see the priv-team's bits since his team
 has been invited to join.

     >>> login_person(pub_owner)
     >>> print priv_team.name
     priv-team
+
+But a non-admin member of the public team still cannot see anything
+about the team.
+
+ >>> login_person(pub_member)
+ >>> print priv_team.name
+ Traceback (most recent call last):
+ ...
+ Unauthorized: (<Person at ... priv-team (Priv Team)>, 'name', 'launchpad.View')
}}}

[3:06 PM] <EdwinGrubbs> that looks good
[3:30 PM] <EdwinGrubbs> bac: I was thinking about the test to see if the user is in getDirectAdministrators(), and it seems like it would cause a problem for teams that are owned by another team. Therefore, you should probably use can_edit_team(invited_team, user)
[3:32 PM] <bac> EdwinGrubbs: good idea, i think

Comments on Diff
----------------

>=== modified file 'lib/lp/registry/doc/private-team-roles.txt'
>--- lib/lp/registry/doc/private-team-roles.txt 2009-10-26 20:08:10 +0000
>+++ lib/lp/registry/doc/private-team-roles.txt 2010-01-04 20:51:08 +0000
>@@ -243,6 +243,7 @@
> >>> import transaction
> >>> transaction.abort()
>
>+
> Structural Subscriptions
> ========================
>
>
>=== added file 'lib/lp/registry/doc/private-team-visibility.txt'
>--- lib/lp/registry/doc/private-team-visibility.txt 1970-01-01 00:00:00 +0000
>+++ lib/lp/registry/doc/private-team-visibility.txt 2010-01-04 20:51:08 +0000
>@@ -0,0 +1,83 @@
>+=========================
>+ Private team visibility
>+=========================
>+
>+Private and private membership team restrict the visibility of their

s/team/teams/

>+attributes to select sets of users in order to prevent leaking
>+confidential data.
>+
>+Private teams restrict the viewing of the membership list
>+to administrators and other members of the team.

You might want to say "Launchpad administrators", since
the team admins are also members, so it can sound redundant even
though it isn't.

>+ >>> from lp.registry.interfaces.person import PersonVisibility
>+ >>> priv_owner = factory.makePerson(name="priv-owner")
>+ >>> priv_member = factory.makePerson(name="...

Read more...

review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/canonical/launchpad/security.py'
2--- lib/canonical/launchpad/security.py 2010-01-04 22:50:37 +0000
3+++ lib/canonical/launchpad/security.py 2010-01-04 23:19:15 +0000
4@@ -698,6 +698,11 @@
5 admins = getUtility(ILaunchpadCelebrities).admin
6 if user.inTeam(admins) or user.inTeam(self.obj):
7 return True
8+ # We also grant visibility of the private team to administrators of
9+ # other teams that have been invited to join the private team.
10+ for invitee in self.obj.invited_members:
11+ if invitee.is_team and invitee in user.getAdministratedTeams():
12+ return True
13 return False
14
15
16
17=== modified file 'lib/lp/registry/doc/private-team-roles.txt'
18--- lib/lp/registry/doc/private-team-roles.txt 2009-10-26 20:08:10 +0000
19+++ lib/lp/registry/doc/private-team-roles.txt 2010-01-04 23:19:15 +0000
20@@ -243,6 +243,7 @@
21 >>> import transaction
22 >>> transaction.abort()
23
24+
25 Structural Subscriptions
26 ========================
27
28
29=== added file 'lib/lp/registry/doc/private-team-visibility.txt'
30--- lib/lp/registry/doc/private-team-visibility.txt 1970-01-01 00:00:00 +0000
31+++ lib/lp/registry/doc/private-team-visibility.txt 2010-01-04 23:19:15 +0000
32@@ -0,0 +1,93 @@
33+=========================
34+ Private team visibility
35+=========================
36+
37+Private and private membership teams restrict the visibility of their
38+attributes to select sets of users in order to prevent leaking
39+confidential data.
40+
41+Private teams restrict the viewing of the membership list
42+to team administrators, other members of the team, and Launchpad
43+administrators .
44+
45+ >>> from lp.registry.interfaces.person import PersonVisibility
46+ >>> priv_owner = factory.makePerson(name="priv-owner")
47+ >>> priv_member = factory.makePerson(name="priv-member")
48+ >>> login('commercial-member@canonical.com')
49+ >>> priv_team = factory.makeTeam(owner=priv_owner, name="priv-team",
50+ ... visibility=PersonVisibility.PRIVATE)
51+ >>> login_person(priv_owner)
52+ >>> priv_team.addMember(priv_member, reviewer=priv_owner)
53+
54+The team owner can see the membership.
55+
56+ >>> members = priv_team.activemembers
57+ >>> for member in members:
58+ ... print member.name
59+ priv-member
60+ priv-owner
61+
62+A team member can access the membership.
63+
64+ >>> login_person(priv_member)
65+ >>> members = priv_team.activemembers
66+
67+A person who is not in the team cannot see the membership and cannot
68+see other details of the team, such as the name.
69+
70+ >>> login('no-priv@canonical.com')
71+ >>> members = priv_team.activemembers
72+ Traceback (most recent call last):
73+ ...
74+ Unauthorized: (<Person at ... priv-team (Priv Team)>, 'activemembers', 'launchpad.View')
75+
76+ >>> print priv_team.name
77+ Traceback (most recent call last):
78+ ...
79+ Unauthorized: (<Person at ... priv-team (Priv Team)>, 'name', 'launchpad.View')
80+
81+Public teams can join private teams. When adding one team to another
82+the team is invited to join and that invitation must be accepted by
83+one of the invited team's admins. Normally the admin of the invited
84+team is not a member of the private team and therefore cannot even see
85+the page to accept the invitation! To resolve that situation the
86+rules for viewing a private team include admins of invited teams.
87+
88+ >>> pub_owner = factory.makePerson(name="pub-owner")
89+ >>> pub_member = factory.makePerson(name="pub-member")
90+ >>> pub_team = factory.makeTeam(owner=pub_owner, name="pubteam")
91+ >>> login_person(pub_owner)
92+ >>> pub_team.addMember(pub_member, reviewer=pub_owner)
93+
94+At this point the public team owner cannot see the priv-team's bits.
95+
96+ >>> print priv_team.name
97+ Traceback (most recent call last):
98+ ...
99+ Unauthorized: (<Person at ... priv-team (Priv Team)>, 'name', 'launchpad.View')
100+
101+ >>> login_person(priv_owner)
102+ >>> priv_team.addMember(pub_team, reviewer=priv_owner)
103+
104+The public team is not yet a member of the priv-team.
105+
106+ >>> pub_team in priv_team.activemembers
107+ False
108+ >>> pub_owner in priv_team.activemembers
109+ False
110+
111+The public team's owner can now see the priv-team's bits since his team
112+has been invited to join.
113+
114+ >>> login_person(pub_owner)
115+ >>> print priv_team.name
116+ priv-team
117+
118+But a non-admin member of the public team still cannot see anything
119+about the team.
120+
121+ >>> login_person(pub_member)
122+ >>> print priv_team.name
123+ Traceback (most recent call last):
124+ ...
125+ Unauthorized: (<Person at ... priv-team (Priv Team)>, 'name', 'launchpad.View')