launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #05106
[Merge] lp:~wallyworld/launchpad/bug-subscription-behaviour-860376 into lp:launchpad
Ian Booth has proposed merging lp:~wallyworld/launchpad/bug-subscription-behaviour-860376 into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~wallyworld/launchpad/bug-subscription-behaviour-860376/+merge/77160
Bug 854405 and Bug 672596 provided for people to be subscribed/unsubscribed from a bug when it's private and/or security_related attributes changed. There is an aspect to what was delivered which needs to be reverted since it can allow sensitive information to be viewed by authorised people. This is the case for the Ubuntu folks and they requested that the functionality be reverted.
== Implementation ==
I've put the new functionality behind a feature flag until we agree on exactly how it should be implemented.
disclosure.enhanced_private_bug_subscriptions.enabled
== Tests ==
I turned on the feature flags in some parts of some tests. In the security-teams.txt test, I tested the important bit - the subscribing of the security contact - with and without the feature flag. In the bugnotification-sending.txt test, I reverted it to how it was before the feature flag. The following tests were re-run:
lib/lp/bugs/doc/bug-heat.txt
lib/lp/bugs/doc/bug.txt
lib/lp/bugs/doc/bugnotification-email.txt
lib/lp/bugs/doc/bugnotification-sending.txt
lib/lp/bugs/doc/bugsubscription.txt
lib/lp/bugs/doc/initial-bug-contacts.txt
lib/lp/bugs/doc/security-teams.txt
lib/lp/bugs/model/tests/test_bug.py
lib/lp/bugs/model/tests/test_bugsummary.py
lib/lp/bugs/stories/bug-privacy/xx-bug-privacy.txt
lib/lp/bugs/tests/test_bugchanges.py
== Lint ==
Some drive by cleanup was done.
Linting changed files:
lib/lp/bugs/doc/bug-heat.txt
lib/lp/bugs/doc/bugnotification-sending.txt
lib/lp/bugs/doc/bugsubscription.txt
lib/lp/bugs/doc/initial-bug-contacts.txt
lib/lp/bugs/doc/security-teams.txt
lib/lp/bugs/model/bug.py
lib/lp/bugs/model/tests/test_bug.py
lib/lp/bugs/stories/bug-privacy/xx-bug-privacy.txt
lib/lp/services/features/flags.py
./lib/lp/bugs/doc/bug-heat.txt
71: source exceeds 78 characters.
./lib/lp/bugs/doc/bugnotification-sending.txt
286: want exceeds 78 characters.
./lib/lp/bugs/doc/bugsubscription.txt
390: source exceeds 78 characters.
./lib/lp/bugs/doc/initial-bug-contacts.txt
296: source exceeds 78 characters.
./lib/lp/bugs/doc/security-teams.txt
290: source exceeds 78 characters.
./lib/lp/bugs/model/bug.py
1663: Line exceeds 78 characters.
1663: E501 line too long (81 characters)
./lib/lp/bugs/stories/bug-privacy/xx-bug-privacy.txt
28: source exceeds 78 characters.
38: source exceeds 78 characters.
78: source exceeds 78 characters.
134: source exceeds 78 characters.
--
https://code.launchpad.net/~wallyworld/launchpad/bug-subscription-behaviour-860376/+merge/77160
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wallyworld/launchpad/bug-subscription-behaviour-860376 into lp:launchpad.
=== modified file 'lib/lp/bugs/doc/bug-heat.txt'
--- lib/lp/bugs/doc/bug-heat.txt 2011-09-21 13:27:17 +0000
+++ lib/lp/bugs/doc/bug-heat.txt 2011-09-27 12:33:26 +0000
@@ -65,11 +65,18 @@
Events which trigger bug heat updates
-------------------------------------
+We current use a feature flag for some privacy related subscription stuff.
+
+ >>> from lp.services.features.testing import FeatureFixture
+ >>> feature_flag = {'disclosure.enhanced_private_bug_subscriptions.enabled': 'on'}
+ >>> flags = FeatureFixture(feature_flag)
+ >>> flags.setUp()
+
There are several events which will cause a bug's heat to be updated.
First, as stated above, heat will be calculated when the bug is created.
>>> bug = factory.makeBug(owner=bug_owner)
- >>> bug.heat
+ >>> bug.heatb
6
When the bug is marked private it gains a subscriber - the owner of the
@@ -187,6 +194,10 @@
>>> bug.heat
8
+Clean up the feature flag.
+
+ >>> flags.cleanUp()
+
Getting bugs whose heat is outdated
-----------------------------------
=== modified file 'lib/lp/bugs/doc/bugnotification-sending.txt'
--- lib/lp/bugs/doc/bugnotification-sending.txt 2011-09-21 13:27:17 +0000
+++ lib/lp/bugs/doc/bugnotification-sending.txt 2011-09-27 12:33:26 +0000
@@ -768,13 +768,8 @@
>>> bug_three.private
True
-When the bug is made private, the bugtask pillar owners and bug supervisors
-are subscribed in addition any other direct subscribers which already exist.
-
>>> for message in trigger_and_get_email_messages(bug_three):
... print message['To'], message.get_all('X-Launchpad-Bug-Private')
- foo.bar@xxxxxxxxxxxxx ['yes']
- mark@xxxxxxxxxxx ['yes']
test@xxxxxxxxxxxxx ['yes']
@@ -791,8 +786,6 @@
>>> for message in trigger_and_get_email_messages(bug_three):
... print message['To'], (
... message.get_all('X-Launchpad-Bug-Security-Vulnerability'))
- foo.bar@xxxxxxxxxxxxx ['no']
- mark@xxxxxxxxxxx ['no']
test@xxxxxxxxxxxxx ['no']
The presence of the security flag on a bug is, surprise, denoted by a
@@ -807,8 +800,6 @@
>>> for message in trigger_and_get_email_messages(bug_three):
... print message['To'], (
... message.get_all('X-Launchpad-Bug-Security-Vulnerability'))
- foo.bar@xxxxxxxxxxxxx ['yes']
- mark@xxxxxxxxxxx ['yes']
test@xxxxxxxxxxxxx ['yes']
=== modified file 'lib/lp/bugs/doc/bugsubscription.txt'
--- lib/lp/bugs/doc/bugsubscription.txt 2011-09-22 10:32:41 +0000
+++ lib/lp/bugs/doc/bugsubscription.txt 2011-09-27 12:33:26 +0000
@@ -383,6 +383,14 @@
will be automatically subscribed, and only specifically allowed existing
direct subscribers (eg bugtask pillar maintainers) will remain subscribed.
+We current use a feature flag to control who is subscribed when a bug is made
+private.
+
+ >>> from lp.services.features.testing import FeatureFixture
+ >>> feature_flag = {'disclosure.enhanced_private_bug_subscriptions.enabled': 'on'}
+ >>> flags = FeatureFixture(feature_flag)
+ >>> flags.setUp()
+
>>> from zope.event import notify
>>> from lazr.lifecycle.event import ObjectModifiedEvent
@@ -447,6 +455,10 @@
Scott James Remnant
Ubuntu Team
+Clean up the feature flag.
+
+ >>> flags.cleanUp()
+
To find out which email addresses should receive a notification email on
a bug, and why, IBug.getBugNotificationRecipients() assembles an
INotificationRecipientSet instance for us:
=== modified file 'lib/lp/bugs/doc/initial-bug-contacts.txt'
--- lib/lp/bugs/doc/initial-bug-contacts.txt 2011-09-22 01:45:12 +0000
+++ lib/lp/bugs/doc/initial-bug-contacts.txt 2011-09-27 12:33:26 +0000
@@ -288,6 +288,15 @@
u'Sample Person', u'Steve Alexander', u'Ubuntu Gnome Team',
u'Ubuntu Team']
+
+We current use a feature flag to control who is subscribed when a bug is made
+private.
+
+ >>> from lp.services.features.testing import FeatureFixture
+ >>> feature_flag = {'disclosure.enhanced_private_bug_subscriptions.enabled': 'on'}
+ >>> flags = FeatureFixture(feature_flag)
+ >>> flags.setUp()
+
If a bug is private, no changes are made to the subscriber list when a
bug is reassigned to a different package.
@@ -322,6 +331,10 @@
[u'Foo Bar', u'Mark Shuttleworth', u'Sample Person',
u'Steve Alexander', u'Ubuntu Team']
+Clean up the feature flag.
+
+ >>> flags.cleanUp()
+
Product Bug Supervisors and Bug Tasks
-------------------------------------
=== modified file 'lib/lp/bugs/doc/security-teams.txt'
--- lib/lp/bugs/doc/security-teams.txt 2011-09-21 13:27:17 +0000
+++ lib/lp/bugs/doc/security-teams.txt 2011-09-27 12:33:26 +0000
@@ -232,28 +232,30 @@
>>> bug_product_changed = ObjectModifiedEvent(
... bug_in_evolution, old_state, ["product"])
-First, let's publish the change event with the bug still marked private,
-and notice that the subscription list doesn't change:
+First, let's set the bug to non security related with the bug still marked,
+private and notice that the subscription list doesn't change:
>>> bug.private
True
- >>> notify(bug_product_changed)
+ >>> bug.setSecurityRelated(False, getUtility(ILaunchBag).user)
+ True
>>> subscriber_names(bug)
[u'name12', u'name16']
-Also verify the same event again, when marked public does cause
+Now the bug is marked as security related, when also marked public does cause
stub to get subscribed:
>>> bug.setPrivate(False, getUtility(ILaunchBag).user)
True
+ >>> bug.setSecurityRelated(True, getUtility(ILaunchBag).user)
+ True
+
>>> bug.security_related
True
- >>> notify(bug_product_changed)
-
>>> subscriber_names(bug)
[u'name12', u'name16', u'stub']
@@ -278,7 +280,16 @@
When a bug becomes security-related, the security contacts for the pillars it
-affects are subscribed to it.
+affects are subscribed to it. This happens regardless of whether the feature
+flag is set.
+
+We current use a feature flag to control who is subscribed when a bug is made
+security related.
+
+ >>> from lp.services.features.testing import FeatureFixture
+ >>> feature_flag = {'disclosure.enhanced_private_bug_subscriptions.enabled': 'on'}
+ >>> flags = FeatureFixture(feature_flag)
+ >>> flags.setUp()
>>> from zope.event import notify
>>> from lazr.lifecycle.event import ObjectModifiedEvent
@@ -305,3 +316,30 @@
Bug Reporter
Distribution Security Contact
Product Security Contact
+
+Clean up the feature flag.
+
+ >>> flags.cleanUp()
+
+And once more without the feature flag.
+
+ >>> product = factory.makeProduct()
+ >>> product.security_contact = factory.makePerson(
+ ... displayname='Product Security Contact')
+ >>> distribution = factory.makeDistribution()
+ >>> distribution.security_contact = factory.makePerson(
+ ... displayname='Distribution Security Contact')
+ >>> reporter = factory.makePerson(displayname=u'Bug Reporter')
+ >>> bug = factory.makeBug(product=product, owner=reporter)
+ >>> bug.addTask(owner=reporter, target=distribution)
+ <BugTask ...>
+ >>> old_state = Snapshot(bug, providing=IBug)
+ >>> bug.setSecurityRelated(True, getUtility(ILaunchBag).user)
+ True
+ >>> notify(ObjectModifiedEvent(bug, old_state, ['security_related']))
+ >>> for subscriber_name in sorted(
+ ... s.displayname for s in bug.getDirectSubscribers()):
+ ... print subscriber_name
+ Bug Reporter
+ Distribution Security Contact
+ Product Security Contact
=== modified file 'lib/lp/bugs/model/bug.py'
--- lib/lp/bugs/model/bug.py 2011-09-26 18:50:32 +0000
+++ lib/lp/bugs/model/bug.py 2011-09-27 12:33:26 +0000
@@ -199,6 +199,7 @@
from lp.registry.model.pillar import pillar_sort_key
from lp.registry.model.teammembership import TeamParticipation
from lp.services.database.stormbase import StormBase
+from lp.services.features import getFeatureFlag
from lp.services.fields import DuplicateBug
from lp.services.messages.interfaces.message import (
IMessage,
@@ -1656,11 +1657,14 @@
security_related_changed = False
bug_before_modification = Snapshot(self, providing=providedBy(self))
- # Before we update the privacy or security_related status, we need to
- # reconcile the subscribers to avoid leaking private information.
- if (self.private != private
- or self.security_related != security_related):
- self.reconcileSubscribers(private, security_related, who)
+ f_flag_str = 'disclosure.enhanced_private_bug_subscriptions.enabled'
+ f_flag = bool(getFeatureFlag(f_flag_str))
+ if f_flag:
+ # Before we update the privacy or security_related status, we need to
+ # reconcile the subscribers to avoid leaking private information.
+ if (self.private != private
+ or self.security_related != security_related):
+ self.reconcileSubscribers(private, security_related, who)
if self.private != private:
private_changed = True
@@ -1693,6 +1697,14 @@
changed_fields.append('private')
if security_related_changed:
changed_fields.append('security_related')
+ if not f_flag and security_related:
+ # The bug turned out to be security-related, subscribe the
+ # security contact. We do it here only if the feature flag
+ # is not set, otherwise it's done in
+ # reconcileSubscribers().
+ for pillar in self.affected_pillars:
+ if pillar.security_contact is not None:
+ self.subscribe(pillar.security_contact, who)
notify(ObjectModifiedEvent(
self, bug_before_modification, changed_fields, user=who))
@@ -2240,6 +2252,7 @@
BugActivity.datechanged <= end_date)
return activity_in_range
+
@ProxyFactory
def get_also_notified_subscribers(
bug_or_bugtask, recipients=None, level=None):
=== modified file 'lib/lp/bugs/model/tests/test_bug.py'
--- lib/lp/bugs/model/tests/test_bug.py 2011-09-23 22:35:35 +0000
+++ lib/lp/bugs/model/tests/test_bug.py 2011-09-27 12:33:26 +0000
@@ -30,6 +30,7 @@
BugSubscriptionInfo,
)
from lp.registry.interfaces.person import PersonVisibility
+from lp.services.features.testing import FeatureFixture
from lp.testing import (
feature_flags,
login_person,
@@ -479,6 +480,14 @@
layer = DatabaseFunctionalLayer
+ def setUp(self):
+ super(TestBugPrivateAndSecurityRelatedUpdatesMixin, self).setUp()
+ f_flag_str = 'disclosure.enhanced_private_bug_subscriptions.enabled'
+ feature_flag = {f_flag_str: 'on'}
+ flags = FeatureFixture(feature_flag)
+ flags.setUp()
+ self.addCleanup(flags.cleanUp)
+
def test_setPrivate_subscribes_person_who_makes_bug_private(self):
# When setPrivate(True) is called on a bug, the person who is
# marking the bug private is subscribed to the bug.
@@ -845,7 +854,7 @@
# that bug that falls within a given date range.
bug = self.factory.makeBug(
date_created=self.now - timedelta(days=365))
- self._makeActivityForBug(bug, activity_ages=[200,100])
+ self._makeActivityForBug(bug, activity_ages=[200, 100])
start_date = self.now - timedelta(days=250)
end_date = self.now - timedelta(days=150)
activity = bug.getActivityForDateRange(
@@ -858,7 +867,7 @@
# falls on the start_ and end_ dates.
bug = self.factory.makeBug(
date_created=self.now - timedelta(days=365))
- self._makeActivityForBug(bug, activity_ages=[300,200,100])
+ self._makeActivityForBug(bug, activity_ages=[300, 200, 100])
start_date = self.now - timedelta(days=300)
end_date = self.now - timedelta(days=100)
activity = bug.getActivityForDateRange(
=== modified file 'lib/lp/bugs/stories/bug-privacy/xx-bug-privacy.txt'
--- lib/lp/bugs/stories/bug-privacy/xx-bug-privacy.txt 2011-09-22 01:45:12 +0000
+++ lib/lp/bugs/stories/bug-privacy/xx-bug-privacy.txt 2011-09-27 12:33:26 +0000
@@ -21,38 +21,50 @@
Sample Person
Ubuntu Team
+We current use a feature flag to control who is subscribed when a bug is made
+private.
+
+ >>> from lp.services.features.testing import FeatureFixture
+ >>> feature_flag = {'disclosure.enhanced_private_bug_subscriptions.enabled': 'on'}
+ >>> flags = FeatureFixture(feature_flag)
+ >>> flags.setUp()
+
Foo Bar is not Cc'd on this bug, but is able to set the bug private
anyway, because he is an admin.
- >>> browser.open(
- ... "http://bugs.launchpad.dev/debian/+source/mozilla-firefox/";
- ... "+bug/2/+secrecy")
- >>> browser.getControl("This bug report should be private").selected = True
- >>> browser.getControl("Change").click()
- >>> print browser.url
- http://bugs.launchpad.dev/debian/+source/mozilla-firefox/+bug/2
+ >>> browser.open(
+ ... "http://bugs.launchpad.dev/debian/+source/mozilla-firefox/";
+ ... "+bug/2/+secrecy")
+ >>> browser.getControl("This bug report should be private").selected = True
+ >>> browser.getControl("Change").click()
+ >>> print browser.url
+ http://bugs.launchpad.dev/debian/+source/mozilla-firefox/+bug/2
Subscribers have been updated according to the privacy rules.
- >>> browser.open(
- ... "http://launchpad.dev/bugs/2/+bug-portlet-subscribers-details";)
- >>> print_direct_subscribers(browser.contents)
- Mark Shuttleworth (Unsubscribe)
- Sample Person (Unsubscribe)
- Steve Alexander (Unsubscribe)
- Ubuntu Team (Unsubscribe)
-
- >>> print_also_notified(browser.contents)
- Also notified:
+ >>> browser.open(
+ ... "http://launchpad.dev/bugs/2/+bug-portlet-subscribers-details";)
+ >>> print_direct_subscribers(browser.contents)
+ Mark Shuttleworth (Unsubscribe)
+ Sample Person (Unsubscribe)
+ Steve Alexander (Unsubscribe)
+ Ubuntu Team (Unsubscribe)
+
+ >>> print_also_notified(browser.contents)
+ Also notified:
+
+Clean up the feature flag.
+
+ >>> flags.cleanUp()
When we go back to the secrecy form, the previously set value is pre-selected.
- >>> browser.open(
- ... "http://bugs.launchpad.dev/debian/+source/mozilla-firefox/";
- ... "+bug/2/+secrecy")
- >>> browser.getControl("This bug report should be private").selected
- True
+ >>> browser.open(
+ ... "http://bugs.launchpad.dev/debian/+source/mozilla-firefox/";
+ ... "+bug/2/+secrecy")
+ >>> browser.getControl("This bug report should be private").selected
+ True
Foo Bar files a security (private) bug on Ubuntu Linux. He gets
redirected to the bug page.
=== modified file 'lib/lp/services/features/flags.py'
--- lib/lp/services/features/flags.py 2011-09-20 03:27:52 +0000
+++ lib/lp/services/features/flags.py 2011-09-27 12:33:26 +0000
@@ -138,6 +138,11 @@
('Enables the application of additional privacy filter terms in bug '
'queries to allow defined project roles to see private bugs.'),
''),
+ ('disclosure.enhanced_private_bug_subscriptions.enabled',
+ 'boolean',
+ ('Enables the auto subscribing and unsubscribing of users as a bug '
+ 'transitions between public, private and security related states.'),
+ ''),
('bugs.autoconfirm.enabled_distribution_names',
'space delimited',
('Enables auto-confirming bugtasks for distributions (and their '
