Merge lp:~jcsackett/launchpad/private-bugs-without-supervision into lp:launchpad

Proposed by j.c.sackett on 2012-04-11
Status: Superseded
Proposed branch: lp:~jcsackett/launchpad/private-bugs-without-supervision
Merge into: lp:launchpad
Diff against target: 725 lines (+103/-296) 9 files modified
To merge this branch: bzr merge lp:~jcsackett/launchpad/private-bugs-without-supervision
Reviewer Review Type Date Requested Status
Curtis Hovey code 2012-04-11 Approve on 2012-04-11
Review via email: mp+101462@code.launchpad.net

This proposal has been superseded by a proposal from 2012-04-17.

Commit Message

Fixes a subscription error on setPrivate

Description of the Change

Summary
=======
The constantly changing setup of subscription and privacy rules led to a
regression, wherein a person marking a bug private would lose access to the
bug, preventing privacy from happening. Because we put the subscriber
reconcilation behind a feature flag and didn't activate it, the absence of a
bug supervisor prevents marking bugs private.

This branch ensures that:
* The bug reporter stays attached
* The bug assignee stays attached
* The bug supervisor is attached if it exists
* The pillar maintainer is attached if the supervisor doesn't exist

Preimp
======
Spoke with Curtis Hovey

Implementation
==============
The reconcileSubscribers, method, which is put behind a feature flag, is
removed as the implementation is now out of date and is unlikely to ever be
enabled. The flag and flag related checks are also removed.

The no f_flag fallback has been cleaned up; it now ensures the bug
supervisor/maintainer fallback works, and ensure the other expected
subscriptions.

Tests
=====
bin/test -vvct test_bugvisibility

QA
==
Look at the setup in the bug; that should work.

Lint
====

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/bugs/tests/test_bugvisibility.py
  lib/lp/bugs/model/bug.py
  lib/lp/services/features/flags.py

To post a comment you must log in.
Curtis Hovey (sinzui) wrote :

This branch reproduces the situation that drove William to put it behind the feature flag to kill it. Let's not repeat this mistake again.

These rules cannot apply to Ubuntu. it's bug supervisor does not get automatic access to private bugs, this is why we spent an extra three months to create 3 sharing policies instead of one, Ubuntu is not like any other project in Lp. Ubuntu's maintainer does not get access to private or security bugs. recall the success criteria for sharing is that that maintainer (or us) can setup Ubuntu so that ubuntu-security is the only party that can see embargoed security bugs, and apport is the only party that can see user data bugs.

There is, or was, code that checked that the project has .private_bugs or .hasCurrentCommercialSubscription() which we know will be working with private bugs. I am not certain we want check these attributes. Maybe we should just check if the pillar is not Ubuntu -- I do not think any other project took advantage of Lp defects to create policies different from what is documented.

review: Needs Fixing (code)
j.c.sackett (jcsackett) wrote :

> This branch reproduces the situation that drove William to put it behind the
> feature flag to kill it. Let's not repeat this mistake again.
>
> These rules cannot apply to Ubuntu. it's bug supervisor does not get automatic
> access to private bugs, this is why we spent an extra three months to create 3
> sharing policies instead of one, Ubuntu is not like any other project in Lp.
> Ubuntu's maintainer does not get access to private or security bugs. recall
> the success criteria for sharing is that that maintainer (or us) can setup
> Ubuntu so that ubuntu-security is the only party that can see embargoed
> security bugs, and apport is the only party that can see user data bugs.

That's a little perplexing; if look at the deleted clause starting on line 19, you can see that the supervisor is already being given access to USERDATA bugs. So if that was the mistake, it was not resolved by the feature flag. I can certainly make the check for Ubuntu, but I want to be clear that it's an additional change from the current behavior (not that there's anything wrong with that).

> There is, or was, code that checked that the project has .private_bugs or
> .hasCurrentCommercialSubscription() which we know will be working with private
> bugs. I am not certain we want check these attributes. Maybe we should just
> check if the pillar is not Ubuntu -- I do not think any other project took
> advantage of Lp defects to create policies different from what is documented.

I'm going to vote for "was". Or, possibly, that check is only being done on bug creation. I think just adding a check for the pillar being Ubuntu is sufficient--we already special case that, so it doesn't feel too wrong to make that check.

I will update the code so that if the pillar is Ubuntu, the bug supervisor is not given access to USERDATA bugs.

Curtis Hovey (sinzui) wrote :

We discusses this on IRC. The issue is that while we think the rules for Ubuntu are wrong at this moment, Ubuntu has not reported any issue with changing bugs to private, only projects have this issue. Maybe this is not an issue because apport does not using this path through the code. We will talk with the purple squad to resolve this.

review: Needs Information (code)
15077. By j.c.sackett on 2012-04-11

Merged in devel.

Curtis Hovey (sinzui) wrote :

We discussed this and believe your change is correct. This is good to land.

review: Approve (code)
15078. By j.c.sackett on 2012-04-12

Merged in devel.

15079. By j.c.sackett on 2012-04-13

Merged in devel.

15080. By j.c.sackett on 2012-04-13

Removed setup requiring removed feature flag.

15081. By j.c.sackett on 2012-04-16

Merged in devel.

15082. By j.c.sackett on 2012-04-16

One test fix.

15083. By j.c.sackett on 2012-04-16

Test fix.

15084. By j.c.sackett on 2012-04-16

More test fixes.

15085. By j.c.sackett on 2012-04-17

Merged in devel.

15086. By j.c.sackett on 2012-04-17

Another test fix.

15087. By j.c.sackett on 2012-04-17

More testfix.

15088. By j.c.sackett on 2012-04-17

Updated another test.

15089. By j.c.sackett on 2012-04-17

test_bug passing.

15090. By j.c.sackett on 2012-04-17

Doctests are a pain to update.

15091. By j.c.sackett on 2012-04-17

Updated security-teams.txt

15092. By j.c.sackett on 2012-04-17

Last doctest update.

15093. By j.c.sackett on 2012-04-17

Last test fix.

Unmerged revisions

Preview Diff

1=== modified file 'lib/lp/bugs/doc/bugnotification-sending.txt'
2--- lib/lp/bugs/doc/bugnotification-sending.txt 2012-03-23 06:10:28 +0000
3+++ lib/lp/bugs/doc/bugnotification-sending.txt 2012-04-17 17:52:46 +0000
4@@ -756,9 +756,10 @@
5
6 >>> for message in trigger_and_get_email_messages(bug_three):
7 ... print message['To'], message.get_all('X-Launchpad-Bug-Private')
8+ foo.bar@canonical.com ['yes']
9+ mark@example.com ['yes']
10 test@canonical.com ['yes']
11
12-
13 The X-Launchpad-Bug-Security-Vulnerability header
14 -------------------------------------------------
15
16@@ -772,6 +773,8 @@
17 >>> for message in trigger_and_get_email_messages(bug_three):
18 ... print message['To'], (
19 ... message.get_all('X-Launchpad-Bug-Security-Vulnerability'))
20+ foo.bar@canonical.com ['no']
21+ mark@example.com ['no']
22 test@canonical.com ['no']
23
24 The presence of the security flag on a bug is, surprise, denoted by a
25@@ -786,6 +789,8 @@
26 >>> for message in trigger_and_get_email_messages(bug_three):
27 ... print message['To'], (
28 ... message.get_all('X-Launchpad-Bug-Security-Vulnerability'))
29+ foo.bar@canonical.com ['yes']
30+ mark@example.com ['yes']
31 test@canonical.com ['yes']
32
33
34
35=== modified file 'lib/lp/bugs/doc/bugsubscription.txt'
36--- lib/lp/bugs/doc/bugsubscription.txt 2012-04-03 06:14:09 +0000
37+++ lib/lp/bugs/doc/bugsubscription.txt 2012-04-17 17:52:46 +0000
38@@ -380,17 +380,7 @@
39 ()
40
41 When a bug is marked private, specific people like the bugtask bug supervisors
42-will be automatically subscribed, and only specifically allowed existing
43-direct subscribers (eg bugtask pillar maintainers) will remain subscribed.
44-
45-We currently use a feature flag to control who is subscribed when a bug is
46-made private.
47-
48- >>> from lp.services.features.testing import FeatureFixture
49- >>> feature_flag = {
50- ... 'disclosure.enhanced_private_bug_subscriptions.enabled': 'on'}
51- >>> flags = FeatureFixture(feature_flag)
52- >>> flags.setUp()
53+will be automatically subscribed.
54
55 >>> from zope.event import notify
56
57@@ -438,26 +428,20 @@
58 >>> print_displayname(linux_source_bug.getDirectSubscribers())
59 Foo Bar
60 Mark Shuttleworth
61+ Robert Collins
62+ Ubuntu Team
63
64 >>> print_displayname(linux_source_bug.getIndirectSubscribers())
65 Martin Pitt
66 No Privileges Person
67- Robert Collins
68 Sample Person
69 Scott James Remnant
70- Ubuntu Team
71
72 >>> print_displayname(linux_source_bug.getAlsoNotifiedSubscribers())
73 Martin Pitt
74 No Privileges Person
75- Robert Collins
76 Sample Person
77 Scott James Remnant
78- Ubuntu Team
79-
80-Clean up the feature flag.
81-
82- >>> flags.cleanUp()
83
84 To find out which email addresses should receive a notification email on
85 a bug, and why, IBug.getBugNotificationRecipients() assembles an
86@@ -472,8 +456,8 @@
87 [('foo.bar@canonical.com', 'Subscriber'),
88 ('mark@example.com', 'Subscriber'),
89 ('no-priv@canonical.com', u'Subscriber (linux-source-2.6.15 in Ubuntu)'),
90- ('robertc@robertcollins.net', u'Subscriber (Mozilla Firefox)'),
91- ('support@ubuntu.com', u'Subscriber (Ubuntu) @ubuntu-team'),
92+ ('robertc@robertcollins.net', 'Subscriber'),
93+ ('support@ubuntu.com', u'Subscriber @ubuntu-team'),
94 ('test@canonical.com', 'Assignee')]
95
96 If IBug.getBugNotificationRecipients() is passed a BugNotificationLevel
97@@ -486,8 +470,8 @@
98 >>> [(address, recipients.getReason(address)[1]) for address in addresses]
99 [('foo.bar@canonical.com', 'Subscriber'),
100 ('mark@example.com', 'Subscriber'),
101- ('robertc@robertcollins.net', u'Subscriber (Mozilla Firefox)'),
102- ('support@ubuntu.com', u'Subscriber (Ubuntu) @ubuntu-team'),
103+ ('robertc@robertcollins.net', 'Subscriber'),
104+ ('support@ubuntu.com', u'Subscriber @ubuntu-team'),
105 ('test@canonical.com', 'Assignee')]
106
107 When Sample Person is unsubscribed from linux_source_bug, he is no
108@@ -501,8 +485,8 @@
109 >>> [(address, recipients.getReason(address)[1]) for address in addresses]
110 [('foo.bar@canonical.com', 'Subscriber'),
111 ('mark@example.com', 'Subscriber'),
112- ('robertc@robertcollins.net', u'Subscriber (Mozilla Firefox)'),
113- ('support@ubuntu.com', u'Subscriber (Ubuntu) @ubuntu-team'),
114+ ('robertc@robertcollins.net', 'Subscriber'),
115+ ('support@ubuntu.com', u'Subscriber @ubuntu-team'),
116 ('test@canonical.com', 'Assignee')]
117
118 ...but remains included for the level LIFECYCLE.
119@@ -515,8 +499,8 @@
120 [('foo.bar@canonical.com', 'Subscriber'),
121 ('mark@example.com', 'Subscriber'),
122 ('no-priv@canonical.com', u'Subscriber (linux-source-2.6.15 in Ubuntu)'),
123- ('robertc@robertcollins.net', u'Subscriber (Mozilla Firefox)'),
124- ('support@ubuntu.com', u'Subscriber (Ubuntu) @ubuntu-team'),
125+ ('robertc@robertcollins.net', 'Subscriber'),
126+ ('support@ubuntu.com', u'Subscriber @ubuntu-team'),
127 ('test@canonical.com', 'Assignee')]
128
129 To find out if someone is already directly subscribed to a bug, call
130
131=== modified file 'lib/lp/bugs/doc/initial-bug-contacts.txt'
132--- lib/lp/bugs/doc/initial-bug-contacts.txt 2012-04-03 06:14:09 +0000
133+++ lib/lp/bugs/doc/initial-bug-contacts.txt 2012-04-17 17:52:46 +0000
134@@ -288,16 +288,6 @@
135 u'Sample Person', u'Steve Alexander', u'Ubuntu Gnome Team',
136 u'Ubuntu Team']
137
138-
139-We currently use a feature flag to control who is subscribed when a bug is
140-made private.
141-
142- >>> from lp.services.features.testing import FeatureFixture
143- >>> feature_flag = {
144- ... 'disclosure.enhanced_private_bug_subscriptions.enabled': 'on'}
145- >>> flags = FeatureFixture(feature_flag)
146- >>> flags.setUp()
147-
148 If a bug is private, no changes are made to the subscriber list when a
149 bug is reassigned to a different package.
150
151@@ -329,13 +319,9 @@
152 the unallowed subscribers are removed:
153
154 >>> subscriber_names(bug_one_in_ubuntu_firefox.bug)
155- [u'Foo Bar', u'Mark Shuttleworth', u'Sample Person',
156+ [u'Foo Bar', u'Sample Person',
157 u'Steve Alexander', u'Ubuntu Team']
158
159-Clean up the feature flag.
160-
161- >>> flags.cleanUp()
162-
163
164 Product Bug Supervisors and Bug Tasks
165 -------------------------------------
166
167=== modified file 'lib/lp/bugs/doc/security-teams.txt'
168--- lib/lp/bugs/doc/security-teams.txt 2012-04-04 05:46:26 +0000
169+++ lib/lp/bugs/doc/security-teams.txt 2012-04-17 17:52:46 +0000
170@@ -280,17 +280,7 @@
171
172
173 When a bug becomes security-related, the security contacts for the pillars it
174-affects are subscribed to it. This happens regardless of whether the feature
175-flag is set.
176-
177-We currently use a feature flag to control who is subscribed when a bug is
178-made security related.
179-
180- >>> from lp.services.features.testing import FeatureFixture
181- >>> feature_flag = {
182- ... 'disclosure.enhanced_private_bug_subscriptions.enabled': 'on'}
183- >>> security_flags = FeatureFixture(feature_flag)
184- >>> security_flags.setUp()
185+affects are subscribed to it.
186
187 >>> from zope.event import notify
188 >>> from lazr.lifecycle.event import ObjectModifiedEvent
189@@ -317,30 +307,3 @@
190 Bug Reporter
191 Distribution Security Contact
192 Product Security Contact
193-
194-Clean up the feature flag.
195-
196- >>> security_flags.cleanUp()
197-
198-And once more without the feature flag.
199-
200- >>> product = factory.makeProduct()
201- >>> product.security_contact = factory.makePerson(
202- ... displayname='Product Security Contact')
203- >>> distribution = factory.makeDistribution()
204- >>> distribution.security_contact = factory.makePerson(
205- ... displayname='Distribution Security Contact')
206- >>> reporter = factory.makePerson(displayname=u'Bug Reporter')
207- >>> bug = factory.makeBug(product=product, owner=reporter)
208- >>> bug.addTask(owner=reporter, target=distribution)
209- <BugTask ...>
210- >>> old_state = Snapshot(bug, providing=IBug)
211- >>> bug.setSecurityRelated(True, getUtility(ILaunchBag).user)
212- True
213- >>> notify(ObjectModifiedEvent(bug, old_state, ['security_related']))
214- >>> for subscriber_name in sorted(
215- ... s.displayname for s in bug.getDirectSubscribers()):
216- ... print subscriber_name
217- Bug Reporter
218- Distribution Security Contact
219- Product Security Contact
220
221=== modified file 'lib/lp/bugs/model/bug.py'
222--- lib/lp/bugs/model/bug.py 2012-04-17 01:11:47 +0000
223+++ lib/lp/bugs/model/bug.py 2012-04-17 17:52:46 +0000
224@@ -1733,10 +1733,6 @@
225 "Cannot transition the information type to proprietary.")
226 if self.information_type == information_type:
227 return False
228- f_flag_str = 'disclosure.enhanced_private_bug_subscriptions.enabled'
229- f_flag = bool(getFeatureFlag(f_flag_str))
230- if f_flag:
231- self.reconcileSubscribers(information_type, who)
232 if (information_type == InformationType.PROPRIETARY and
233 len(self.affected_pillars) > 1):
234 raise BugCannotBePrivate(
235@@ -1744,34 +1740,47 @@
236 if information_type in PRIVATE_INFORMATION_TYPES:
237 self.who_made_private = who
238 self.date_made_private = UTC_NOW
239+ missing_subscribers = set([who, self.owner])
240 else:
241 self.who_made_private = None
242 self.date_made_private = None
243+ missing_subscribers = set()
244 # XXX: This should be a bulk update. RBC 20100827
245 # bug=https://bugs.launchpad.net/storm/+bug/625071
246 for attachment in self.attachments_unpopulated:
247 attachment.libraryfile.restricted = (
248 information_type in PRIVATE_INFORMATION_TYPES)
249 self.updateHeat()
250- if not f_flag and information_type == InformationType.USERDATA:
251- # If we didn't call reconcileSubscribers(), we may have
252- # bug supervisors who should be on this bug, but aren't.
253- supervisors = set()
254- for bugtask in self.bugtasks:
255- supervisors.add(bugtask.pillar.bug_supervisor)
256- if None in supervisors:
257- supervisors.remove(None)
258- for s in supervisors:
259- if not get_structural_subscriptions_for_bug(self, s):
260- self.subscribe(s, who)
261- if not f_flag and information_type in SECURITY_INFORMATION_TYPES:
262- # The bug turned out to be security-related, subscribe the
263- # security contact. We do it here only if the feature flag
264- # is not set, otherwise it's done in
265- # reconcileSubscribers().
266- for pillar in self.affected_pillars:
267+
268+ # There are several people we need to ensure are subscribed.
269+ # If the information type is userdata, we need to check for bug
270+ # supervisors who aren't subscribed and should be. If there is no
271+ # bug supervisor, we need to subscribe the maintainer.
272+ pillars = self.affected_pillars
273+ if information_type == InformationType.USERDATA:
274+ for pillar in pillars:
275+ if pillar.bug_supervisor is not None:
276+ missing_subscribers.add(pillar.bug_supervisor)
277+ else:
278+ missing_subscribers.add(pillar.owner)
279+
280+ # If the information type is security related, we need to ensure
281+ # the security contacts are subscribed. If there is no security
282+ # contact, we need to subscribe the maintainer.
283+ if information_type in SECURITY_INFORMATION_TYPES:
284+ for pillar in pillars:
285 if pillar.security_contact is not None:
286- self.subscribe(pillar.security_contact, who)
287+ missing_subscribers.add(pillar.security_contact)
288+ else:
289+ missing_subscribers.add(pillar.owner)
290+
291+ for s in missing_subscribers:
292+ # Don't subscribe someone if they're already subscribed via a
293+ # team.
294+ already_subscribed_teams = self.getSubscribersForPerson(s)
295+ if already_subscribed_teams.is_empty():
296+ self.subscribe(s, who)
297+
298 self.information_type = information_type
299 # Set the legacy attributes for now.
300 self._private = information_type in PRIVATE_INFORMATION_TYPES
301@@ -1838,81 +1847,6 @@
302 bug_supervisors.append(pillar.bug_supervisor)
303 return bug_supervisors, security_contacts
304
305- def reconcileSubscribers(self, information_type, who):
306- """ Ensure only appropriate people are subscribed to private bugs.
307-
308- When a bug is marked as either private = True or security_related =
309- True, we need to ensure that only people who are authorised to know
310- about the privileged contents of the bug remain directly subscribed
311- to it. So we:
312- 1. Get the required subscribers depending on the bug status.
313- 2. Get the auto removed subscribers depending on the bug status.
314- eg security contacts when a bug is updated to security related =
315- false.
316- 3. Get the allowed subscribers = required subscribers
317- + bugtask owners
318- 4. Remove any current direct subscribers who are not allowed or are
319- to be auto removed.
320- 5. Add any subscribers who are required.
321- """
322- current_direct_subscribers = (
323- self.getSubscriptionInfo().direct_subscribers)
324- required_subscribers = self.getRequiredSubscribers(
325- information_type, who)
326- removed_bug_supervisors, removed_security_contacts = (
327- self.getAutoRemovedSubscribers(information_type))
328- for subscriber in removed_bug_supervisors:
329- recipients = BugNotificationRecipients()
330- recipients.addBugSupervisor(subscriber)
331- notification_text = ("This bug is no longer private so the bug "
332- "supervisor was unsubscribed. They will no longer be "
333- "notified of changes to this bug for privacy related "
334- "reasons, but may receive notifications about this bug from "
335- "other subscriptions.")
336- self.unsubscribe(
337- subscriber, who, ignore_permissions=True,
338- send_notification=True,
339- notification_text=notification_text,
340- recipients=recipients)
341- for subscriber in removed_security_contacts:
342- recipients = BugNotificationRecipients()
343- recipients.addSecurityContact(subscriber)
344- notification_text = ("This bug is no longer security related so "
345- "the security contact was unsubscribed. They will no longer "
346- "be notified of changes to this bug for security related "
347- "reasons, but may receive notifications about this bug "
348- "from other subscriptions.")
349- self.unsubscribe(
350- subscriber, who, ignore_permissions=True,
351- send_notification=True,
352- notification_text=notification_text,
353- recipients=recipients)
354-
355- # If this bug is for a project that is marked as having private bugs
356- # by default, and the bug is private or security related, we will
357- # unsubscribe any unauthorised direct subscribers.
358- pillar = self.default_bugtask.pillar
359- private_project = IProduct.providedBy(pillar) and pillar.private_bugs
360- privileged_info = information_type != InformationType.PUBLIC
361- if private_project and privileged_info:
362- allowed_subscribers = set()
363- allowed_subscribers.add(self.owner)
364- for bugtask in self.bugtasks:
365- allowed_subscribers.add(bugtask.owner)
366- allowed_subscribers.add(bugtask.pillar.owner)
367- allowed_subscribers.update(set(bugtask.pillar.drivers))
368- allowed_subscribers = required_subscribers.union(
369- allowed_subscribers)
370- subscribers_to_remove = (
371- current_direct_subscribers.difference(allowed_subscribers))
372- for subscriber in subscribers_to_remove:
373- self.unsubscribe(subscriber, who, ignore_permissions=True)
374-
375- subscribers_to_add = (
376- required_subscribers.difference(current_direct_subscribers))
377- for subscriber in subscribers_to_add:
378- self.subscribe(subscriber, who)
379-
380 def getBugTask(self, target):
381 """See `IBug`."""
382 for bugtask in self.bugtasks:
383
384=== modified file 'lib/lp/bugs/model/tests/test_bug.py'
385--- lib/lp/bugs/model/tests/test_bug.py 2012-04-17 01:11:47 +0000
386+++ lib/lp/bugs/model/tests/test_bug.py 2012-04-17 17:52:46 +0000
387@@ -567,14 +567,6 @@
388
389 layer = DatabaseFunctionalLayer
390
391- def setUp(self):
392- super(TestBugPrivateAndSecurityRelatedUpdatesMixin, self).setUp()
393- f_flag_str = 'disclosure.enhanced_private_bug_subscriptions.enabled'
394- feature_flag = {f_flag_str: 'on'}
395- flags = FeatureFixture(feature_flag)
396- flags.setUp()
397- self.addCleanup(flags.cleanUp)
398-
399 def test_setPrivate_subscribes_person_who_makes_bug_private(self):
400 # When setPrivate(True) is called on a bug, the person who is
401 # marking the bug private is subscribed to the bug.
402@@ -589,8 +581,8 @@
403 # marking the bug private will not be subscribed if they're
404 # already a member of a team which is a direct subscriber.
405 bug = self.factory.makeBug()
406- team = self.factory.makeTeam()
407- person = team.teamowner
408+ person = self.factory.makePerson(name='teamowner')
409+ team = self.factory.makeTeam(owner=person, name='team')
410 with person_logged_in(person):
411 bug.subscribe(team, person)
412 bug.setPrivate(True, person)
413@@ -632,78 +624,71 @@
414 return (bug, bug_owner, naked_bugtask_a, naked_bugtask_b,
415 naked_default_bugtask)
416
417- def test_setPrivateTrueAndSecurityRelatedTrue(self):
418- # When a bug is marked as private=true and security_related=true, the
419- # direct subscribers should include:
420+ def test_transition_to_EMBARGOEDSECURITY_information_type(self):
421+ # When a bug is marked as EMBARGOEDSECURITY, the direct subscribers
422+ # should include:
423 # - the bug reporter
424 # - the bugtask pillar security contacts (if set)
425 # - the person changing the state
426 # - and bug/pillar owners, drivers if they are already subscribed
427- # If the bug is for a private project, then other direct subscribers
428- # should be unsubscribed.
429
430 (bug, bug_owner, bugtask_a, bugtask_b, default_bugtask) = (
431 self.createBugTasksAndSubscribers())
432 initial_subscribers = set((
433- self.factory.makePerson(), bugtask_a.owner, bug_owner,
434- bugtask_a.pillar.security_contact, bugtask_a.pillar.driver))
435+ self.factory.makePerson(name='subscriber'), bugtask_a.owner,
436+ bug_owner, bugtask_a.pillar.security_contact,
437+ bugtask_a.pillar.driver))
438+ initial_subscribers.update(bug.getDirectSubscribers())
439
440 with person_logged_in(bug_owner):
441 for subscriber in initial_subscribers:
442 bug.subscribe(subscriber, bug_owner)
443- who = self.factory.makePerson()
444+ who = self.factory.makePerson(name='who')
445 bug.transitionToInformationType(
446 InformationType.EMBARGOEDSECURITY, who=who)
447 subscribers = bug.getDirectSubscribers()
448+ initial_subscribers.update(bug.getDirectSubscribers())
449 expected_subscribers = set((
450 bugtask_a.owner,
451- default_bugtask.pillar.bug_supervisor,
452 default_bugtask.pillar.driver,
453 default_bugtask.pillar.security_contact,
454 bug_owner, who))
455- if not self.private_project:
456- expected_subscribers.update(initial_subscribers)
457+ expected_subscribers.update(initial_subscribers)
458 self.assertContentEqual(expected_subscribers, subscribers)
459
460- def test_setPrivateTrueAndSecurityRelatedFalse(self):
461- # When a bug is marked as private=true and security_related=false, the
462- # direct subscribers should include:
463+ def test_transition_to_USERDATA_information_type(self):
464+ # When a bug is marked as USERDATA, the direct subscribers should
465+ # include:
466 # - the bug reporter
467 # - the bugtask pillar bug supervisors (if set)
468 # - the person changing the state
469 # - and bug/pillar owners, drivers if they are already subscribed
470- # If the bug is for a private project, then other direct subscribers
471- # should be unsubscribed.
472
473 (bug, bug_owner, bugtask_a, bugtask_b, default_bugtask) = (
474 self.createBugTasksAndSubscribers(private_security_related=True))
475 initial_subscribers = set((
476- self.factory.makePerson(), bug_owner,
477+ self.factory.makePerson(name='subscriber'), bug_owner,
478 bugtask_a.pillar.security_contact, bugtask_a.pillar.driver))
479
480 with person_logged_in(bug_owner):
481 for subscriber in initial_subscribers:
482 bug.subscribe(subscriber, bug_owner)
483- who = self.factory.makePerson()
484+ who = self.factory.makePerson(name='who')
485 bug.transitionToInformationType(InformationType.USERDATA, who)
486 subscribers = bug.getDirectSubscribers()
487 expected_subscribers = set((
488 default_bugtask.pillar.bug_supervisor,
489 default_bugtask.pillar.driver,
490 bug_owner, who))
491- if not self.private_project:
492- expected_subscribers.update(initial_subscribers)
493- expected_subscribers.remove(bugtask_a.pillar.security_contact)
494+ expected_subscribers.update(initial_subscribers)
495 self.assertContentEqual(expected_subscribers, subscribers)
496
497- def test_setPrivateFalseAndSecurityRelatedTrue(self):
498- # When a bug is marked as private=false and security_related=true, the
499- # direct subscribers should include:
500+ def test_transition_to_UNEMBARGOEDSECURITY_information_type(self):
501+ # When a security bug is unembargoed, direct subscribers should
502+ # include:
503 # - the bug reporter
504 # - the bugtask pillar security contacts (if set)
505 # - and bug/pillar owners, drivers if they are already subscribed
506- # If the bug is for a private project, then other direct subscribers
507- # should be unsubscribed.
508
509 (bug, bug_owner, bugtask_a, bugtask_b, default_bugtask) = (
510 self.createBugTasksAndSubscribers(private_security_related=True))
511@@ -715,7 +700,7 @@
512 with person_logged_in(bug_owner):
513 for subscriber in initial_subscribers:
514 bug.subscribe(subscriber, bug_owner)
515- who = self.factory.makePerson()
516+ who = self.factory.makePerson(name='who')
517 bug.transitionToInformationType(
518 InformationType.UNEMBARGOEDSECURITY, who)
519 subscribers = bug.getDirectSubscribers()
520@@ -723,36 +708,33 @@
521 default_bugtask.pillar.driver,
522 default_bugtask.pillar.security_contact,
523 bug_owner))
524- if not self.private_project:
525- expected_subscribers.update(initial_subscribers)
526- expected_subscribers.remove(default_bugtask.pillar.bug_supervisor)
527+ expected_subscribers.update(initial_subscribers)
528 self.assertContentEqual(expected_subscribers, subscribers)
529
530- def test_setPrivateFalseAndSecurityRelatedFalse(self):
531- # When a bug is marked as private=false and security_related=false,
532- # any existing subscriptions are left alone.
533+ def test_transition_to_PUBLIC_information_type(self):
534+ # Subscriptions aren't altered when a bug is transitioned to the
535+ # PUBLIC information type.
536
537 (bug, bug_owner, bugtask_a, bugtask_b, default_bugtask) = (
538 self.createBugTasksAndSubscribers(private_security_related=True))
539 initial_subscribers = set((
540- self.factory.makePerson(), bug_owner,
541+ self.factory.makePerson(name='subscriber'), bug_owner,
542 bugtask_a.pillar.security_contact, bugtask_a.pillar.driver))
543
544 with person_logged_in(bug_owner):
545 for subscriber in initial_subscribers:
546 bug.subscribe(subscriber, bug_owner)
547- who = self.factory.makePerson()
548- expected_direct_subscribers = set(bug.getDirectSubscribers())
549+ who = self.factory.makePerson(name='who')
550+ subscribers_before_public = set(bug.getDirectSubscribers())
551 bug.transitionToInformationType(InformationType.PUBLIC, who)
552- subscribers = set(bug.getDirectSubscribers())
553- expected_direct_subscribers.difference_update(
554- (default_bugtask.pillar.security_contact,
555- default_bugtask.pillar.bug_supervisor,
556- bugtask_a.pillar.security_contact))
557- self.assertContentEqual(expected_direct_subscribers, subscribers)
558+ subscribers_after_public = set(bug.getDirectSubscribers())
559+ self.assertContentEqual(
560+ subscribers_before_public,
561+ subscribers_after_public)
562
563 def test_setPillarOwnerSubscribedIfNoBugSupervisor(self):
564- # The pillar owner is subscribed if the bug supervisor is not set.
565+ # The pillar owner is subscribed if the bug supervisor is not set and
566+ # the bug is marked as USERDATA.
567
568 bug_owner = self.factory.makePerson(name='bugowner')
569 bug = self.factory.makeBug(owner=bug_owner)
570@@ -766,18 +748,19 @@
571 subscribers)
572
573 def test_setPillarOwnerSubscribedIfNoSecurityContact(self):
574- # The pillar owner is subscribed if the security contact is not set.
575+ # The pillar owner is subscribed if the security contact is not set
576+ # and the bug is marked as EMBARGOEDSECURITY.
577
578 bug_owner = self.factory.makePerson(name='bugowner')
579 bug = self.factory.makeBug(owner=bug_owner)
580 with person_logged_in(bug_owner):
581- who = self.factory.makePerson()
582+ who = self.factory.makePerson(name='who')
583 bug.transitionToInformationType(
584- InformationType.UNEMBARGOEDSECURITY, who)
585+ InformationType.EMBARGOEDSECURITY, who)
586 subscribers = bug.getDirectSubscribers()
587 naked_bugtask = removeSecurityProxy(bug).default_bugtask
588 self.assertContentEqual(
589- set((naked_bugtask.pillar.owner, bug_owner)),
590+ set((naked_bugtask.pillar.owner, bug_owner, who)),
591 subscribers)
592
593 def _fetch_notifications(self, bug, reason_header):
594@@ -833,32 +816,6 @@
595 actual_recipients.append(recipient.person)
596 self.assertContentEqual(expected_recipients, actual_recipients)
597
598- def test_bugSupervisorUnsubscribedIfBugMadePublic(self):
599- # The bug supervisors are unsubscribed if a bug is made public and an
600- # email is sent telling them they have been unsubscribed.
601-
602- (bug, bug_owner, bugtask_a, bugtask_b, default_bugtask) = (
603- self.createBugTasksAndSubscribers(private_security_related=True))
604-
605- with person_logged_in(bug_owner):
606- bug.subscribe(default_bugtask.pillar.bug_supervisor, bug_owner)
607- who = self.factory.makePerson(name="who")
608- bug.transitionToInformationType(
609- InformationType.UNEMBARGOEDSECURITY, who)
610- subscribers = bug.getDirectSubscribers()
611- self.assertNotIn(
612- default_bugtask.pillar.bug_supervisor, subscribers)
613-
614- expected_recipients = [
615- default_bugtask.pillar.bug_supervisor,
616- ]
617- expected_body_text = '** This bug is no longer private'
618- expected_reason_body = ('You received this bug notification '
619- 'because you are a bug supervisor.')
620- self._check_notifications(
621- bug, expected_recipients, expected_body_text,
622- expected_reason_body, False, True, 'Bug Supervisor')
623-
624 def test_structural_bug_supervisor_becomes_direct_on_private(self):
625 # If a bug supervisor has a structural subscription to the bug, and
626 # the bug is marked as private, the supervisor should get a direct
627@@ -876,31 +833,6 @@
628 bug.transitionToInformationType(InformationType.USERDATA, who)
629 self.assertTrue(bug_supervisor in bug.getDirectSubscribers())
630
631- def test_securityContactUnsubscribedIfBugNotSecurityRelated(self):
632- # The security contacts are unsubscribed if a bug has security_related
633- # set to false and an email is sent telling them they have been
634- # unsubscribed.
635-
636- (bug, bug_owner, bugtask_a, bugtask_b, default_bugtask) = (
637- self.createBugTasksAndSubscribers(private_security_related=True))
638-
639- with person_logged_in(bug_owner):
640- bug.subscribe(bugtask_a.pillar.security_contact, bug_owner)
641- who = self.factory.makePerson(name="who")
642- bug.transitionToInformationType(InformationType.USERDATA, who)
643- subscribers = bug.getDirectSubscribers()
644- self.assertFalse(bugtask_a.pillar.security_contact in subscribers)
645-
646- expected_recipients = [
647- bugtask_a.pillar.security_contact,
648- ]
649- expected_body_text = '** This bug is no longer security related'
650- expected_reason_body = ('You received this bug notification '
651- 'because you are a security contact.')
652- self._check_notifications(
653- bug, expected_recipients, expected_body_text,
654- expected_reason_body, True, False, 'Security Contact')
655-
656
657 class TestBugPrivacy(TestCaseWithFactory):
658
659
660=== modified file 'lib/lp/bugs/tests/test_bug_mirror_access_triggers.py'
661--- lib/lp/bugs/tests/test_bug_mirror_access_triggers.py 2012-04-03 06:14:09 +0000
662+++ lib/lp/bugs/tests/test_bug_mirror_access_triggers.py 2012-04-17 17:52:46 +0000
663@@ -141,7 +141,9 @@
664 bug.setPrivate(True, bug.owner)
665 self.assertIsNot(
666 None, getUtility(IAccessArtifactSource).find([bug]).one())
667- self.assertEqual((1, 1), self.assertMirrored(bug))
668+ # There are two grants--one for the reporter, one for the product
669+ # owner or supervisor (if set). There is only one policy, USERDATA.
670+ self.assertEqual((2, 1), self.assertMirrored(bug))
671
672 def test_security_related(self):
673 # Setting the security_related flag uses EMBARGOEDSECURITY
674@@ -153,7 +155,9 @@
675 [InformationType.USERDATA],
676 self.getPolicyTypesForArtifact(artifact))
677 bug.setSecurityRelated(True, bug.owner)
678- self.assertEqual((1, 1), self.assertMirrored(bug))
679+ # Both the reporter and either the product owner or the product's
680+ # security contact have grants.
681+ self.assertEqual((2, 1), self.assertMirrored(bug))
682 self.assertContentEqual(
683 [InformationType.EMBARGOEDSECURITY],
684 self.getPolicyTypesForArtifact(artifact))
685
686=== modified file 'lib/lp/bugs/tests/test_bugvisibility.py'
687--- lib/lp/bugs/tests/test_bugvisibility.py 2012-02-15 02:01:14 +0000
688+++ lib/lp/bugs/tests/test_bugvisibility.py 2012-04-17 17:52:46 +0000
689@@ -3,11 +3,17 @@
690
691 """Tests for visibility of a bug."""
692
693+from contextlib import contextmanager
694+
695+from lp.registry.interfaces.person import TeamSubscriptionPolicy
696 from lp.testing import (
697 celebrity_logged_in,
698 TestCaseWithFactory,
699 )
700-from lp.testing.layers import LaunchpadFunctionalLayer
701+from lp.testing.layers import (
702+ DatabaseFunctionalLayer,
703+ LaunchpadFunctionalLayer,
704+ )
705
706
707 class TestPublicBugVisibility(TestCaseWithFactory):
708
709=== modified file 'lib/lp/services/features/flags.py'
710--- lib/lp/services/features/flags.py 2012-04-11 05:21:33 +0000
711+++ lib/lp/services/features/flags.py 2012-04-17 17:52:46 +0000
712@@ -216,13 +216,6 @@
713 '',
714 '',
715 ''),
716- ('disclosure.enhanced_private_bug_subscriptions.enabled',
717- 'boolean',
718- ('Enables the auto subscribing and unsubscribing of users as a bug '
719- 'transitions between public, private and security related states.'),
720- '',
721- '',
722- ''),
723 ('disclosure.users_hide_own_bug_comments.enabled',
724 'boolean',
725 'Allows users in project roles and comment owners to hide bug comments.',