Colin Watson has proposed merging ~cjwatson/launchpad:tighten-bug-permissions into launchpad:master.
Commit message: Tighten bug permissions in view of locking Requested reviews: Launchpad code reviewers (launchpad-reviewers) For more details, see: https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/414911 Bug permissions have historically been quite lax, conflating the ability to view a bug with the ability to make various changes to it. Now that we support locking metadata changes to bugs, this has become a problem. This commit tightens up the permissions structure: we should now (mostly) consistently require `launchpad.Append` on bugs or bug tasks to add comments or attachments, and `launchpad.Edit` to make other metadata changes. Similarly, hide editing UI on bug pages if it can't be used, rather than showing unusable edit buttons on locked bugs. -- Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:tighten-bug-permissions into launchpad:master.
diff --git a/lib/lp/bugs/browser/bug.py b/lib/lp/bugs/browser/bug.py index f495e7b..b67c65f 100644 --- a/lib/lp/bugs/browser/bug.py +++ b/lib/lp/bugs/browser/bug.py @@ -115,6 +115,7 @@ from lp.services.searchbuilder import ( from lp.services.webapp import ( canonical_url, ContextMenu, + enabled_with_permission, LaunchpadView, Link, Navigation, @@ -225,25 +226,30 @@ class BugContextMenu(ContextMenu): ContextMenu.__init__(self, getUtility(ILaunchBag).bugtask) @cachedproperty + @enabled_with_permission('launchpad.Edit') def editdescription(self): """Return the 'Edit description/tags' Link.""" text = 'Update description / tags' return Link('+edit', text, icon='edit') + @enabled_with_permission('launchpad.Edit') def visibility(self): """Return the 'Set privacy/security' Link.""" text = 'Change privacy/security' return Link('+secrecy', text) + @enabled_with_permission('launchpad.Edit') def markduplicate(self): """Return the 'Mark as duplicate' Link.""" return Link('+duplicate', 'Mark as duplicate') + @enabled_with_permission('launchpad.Edit') def addupstream(self): - """Return the 'lso affects project' Link.""" + """Return the 'Also affects project' Link.""" text = 'Also affects project' return Link('+choose-affected-product', text, icon='add') + @enabled_with_permission('launchpad.Edit') def adddistro(self): """Return the 'Also affects distribution/package' Link.""" text = 'Also affects distribution/package' @@ -318,16 +324,19 @@ class BugContextMenu(ContextMenu): else: return Link('+nominate', '', enabled=False, icon='milestone') + @enabled_with_permission('launchpad.Append') def addcomment(self): """Return the 'Comment or attach file' Link.""" text = 'Add attachment or patch' return Link('+addcomment', text, icon='add') + @enabled_with_permission('launchpad.Edit') def addbranch(self): """Return the 'Add branch' Link.""" text = 'Link a related branch' return Link('+addbranch', text, icon='add') + @enabled_with_permission('launchpad.Edit') def linktocve(self): """Return the 'Link to CVE' Link.""" text = structured( @@ -337,6 +346,7 @@ class BugContextMenu(ContextMenu): '</abbr>') return Link('+linkcve', text, icon='add') + @enabled_with_permission('launchpad.Edit') def unlinkcve(self): """Return 'Remove CVE link' Link.""" enabled = self.context.bug.has_cves @@ -347,12 +357,14 @@ class BugContextMenu(ContextMenu): def _bug_question(self): return self.context.bug.getQuestionCreatedFromBug() + @enabled_with_permission('launchpad.Edit') def createquestion(self): """Create a question from this bug.""" text = 'Convert to a question' enabled = self._bug_question is None return Link('+create-question', text, enabled=enabled, icon='add') + @enabled_with_permission('launchpad.Edit') def removequestion(self): """Remove the created question from this bug.""" text = 'Convert back to a bug' diff --git a/lib/lp/bugs/browser/bugtask.py b/lib/lp/bugs/browser/bugtask.py index 76e1bb8..ff7d305 100644 --- a/lib/lp/bugs/browser/bugtask.py +++ b/lib/lp/bugs/browser/bugtask.py @@ -1705,10 +1705,17 @@ class BugTasksNominationsView(LaunchpadView): """Ensure we always have a bug context.""" LaunchpadView.__init__(self, IBug(context), request) + def displayAffectedUsers(self): + """Return True if UI related to affected users should be displayed.""" + # Hide this UI when the bug is viewed in a CVE context. + return self.request.getNearest(ICveSet) == (None, None) + def displayAlsoAffectsLinks(self): """Return True if the Also Affects links should be displayed.""" - # Hide the links when the bug is viewed in a CVE context - return self.request.getNearest(ICveSet) == (None, None) + return ( + check_permission('launchpad.Edit', self.context) and + # Hide the links when the bug is viewed in a CVE context. + self.request.getNearest(ICveSet) == (None, None)) @cachedproperty def current_user_affected_status(self): @@ -2065,7 +2072,6 @@ class BugTaskTableRowView(LaunchpadView, BugTaskBugWatchMixin, task_link = edit_link = canonical_url( self.context, view_name='+editstatus') delete_link = canonical_url(self.context, view_name='+delete') - can_edit = check_permission('launchpad.Edit', self.context) bugtask_id = self.context.id launchbag = getUtility(ILaunchBag) is_primary = self.context.id == launchbag.bugtask.id @@ -2073,12 +2079,15 @@ class BugTaskTableRowView(LaunchpadView, BugTaskBugWatchMixin, # Looking at many_bugtasks is an important optimization. With # 150+ bugtasks, it can save three or four seconds of rendering # time. - expandable=(not self.many_bugtasks and self.canSeeTaskDetails()), + expandable=( + not self.many_bugtasks and + self.user_can_edit and + self.canSeeTaskDetails()), indent_task=ISeriesBugTarget.providedBy(self.context.target), is_conjoined_replica=self.is_conjoined_replica, task_link=task_link, edit_link=edit_link, - can_edit=can_edit, + can_edit=self.user_can_edit, link=link, id=bugtask_id, row_id='tasksummary%d' % bugtask_id, @@ -2145,8 +2154,10 @@ class BugTaskTableRowView(LaunchpadView, BugTaskBugWatchMixin, def displayEditForm(self): """Return true if the BugTask edit form should be shown.""" - # Hide the edit form when the bug is viewed in a CVE context - return self.request.getNearest(ICveSet) == (None, None) + return ( + check_permission('launchpad.Edit', self.context) and + # Hide the edit form when the bug is viewed in a CVE context. + self.request.getNearest(ICveSet) == (None, None)) @property def status_widget_items(self): @@ -2224,12 +2235,20 @@ class BugTaskTableRowView(LaunchpadView, BugTaskBugWatchMixin, return canonical_url(self.context) @cachedproperty + def user_can_edit(self): + """Can the user edit the task at all?""" + return check_permission('launchpad.Edit', self.context) + + @cachedproperty def user_can_edit_importance(self): """Can the user edit the Importance field? If yes, return True, otherwise return False. """ - return self.user_can_edit_status and self.user_has_privileges + return ( + self.user_can_edit and + self.user_can_edit_status and + self.user_has_privileges) @cachedproperty def user_can_edit_status(self): @@ -2237,6 +2256,8 @@ class BugTaskTableRowView(LaunchpadView, BugTaskBugWatchMixin, If yes, return True, otherwise return False. """ + if not self.user_can_edit: + return False bugtask = self.context edit_allowed = bugtask.pillar.official_malone or bugtask.bugwatch if bugtask.bugwatch: @@ -2251,7 +2272,7 @@ class BugTaskTableRowView(LaunchpadView, BugTaskBugWatchMixin, If yes, return True, otherwise return False. """ - return self.user is not None + return self.user is not None and self.user_can_edit @cachedproperty def user_can_delete_bugtask(self): diff --git a/lib/lp/bugs/browser/configure.zcml b/lib/lp/bugs/browser/configure.zcml index d9c36a8..85e1340 100644 --- a/lib/lp/bugs/browser/configure.zcml +++ b/lib/lp/bugs/browser/configure.zcml @@ -522,7 +522,7 @@ name="+duplicate" for="lp.bugs.interfaces.bugtask.IBugTask" class="lp.bugs.browser.bug.BugMarkAsDuplicateView" - permission="launchpad.AnyPerson" + permission="launchpad.Edit" template="../templates/bug-mark-as-duplicate.pt"/> <browser:page for="lp.bugs.interfaces.bugtask.IBugTask" @@ -540,13 +540,13 @@ name="+addcomment" for="lp.bugs.interfaces.bugtask.IBugTask" class="lp.bugs.browser.bugmessage.BugMessageAddFormView" - permission="launchpad.Edit" + permission="launchpad.Append" template="../templates/bug-comment-add.pt"/> <browser:page name="+addcomment-form" for="lp.bugs.interfaces.bugtask.IBugTask" class="lp.bugs.browser.bugmessage.BugMessageAddFormView" - permission="launchpad.Edit" + permission="launchpad.Append" template="../templates/bug-comment-add-form.pt"/> <browser:page name="+nominate" @@ -558,7 +558,7 @@ name="+addbranch" for="lp.bugs.interfaces.bugtask.IBugTask" class="lp.bugs.browser.bugbranch.BugBranchAddView" - permission="launchpad.AnyPerson" + permission="launchpad.Edit" template="../templates/bug-branch-add.pt"/> <browser:page name="+index" @@ -576,18 +576,18 @@ name="+choose-affected-product" for="lp.bugs.interfaces.bugtask.IBugTask" class="lp.bugs.browser.bugalsoaffects.BugAlsoAffectsProductMetaView" - permission="launchpad.AnyPerson"/> + permission="launchpad.Edit"/> <browser:page name="+affects-new-product" for="lp.bugs.interfaces.bugtask.IBugTask" class="lp.bugs.browser.bugalsoaffects.BugAlsoAffectsProductWithProductCreationView" - permission="launchpad.AnyPerson" + permission="launchpad.Edit" template="../templates/bugtask-affects-new-product.pt"/> <browser:page name="+distrotask" for="lp.bugs.interfaces.bugtask.IBugTask" class="lp.bugs.browser.bugalsoaffects.BugAlsoAffectsDistroMetaView" - permission="launchpad.AnyPerson"/> + permission="launchpad.Edit"/> <browser:page name="+editstatus" for="lp.bugs.interfaces.bugtask.IBugTask" @@ -640,14 +640,14 @@ name="+linkcve" for="lp.bugs.interfaces.bugtask.IBugTask" class="lp.bugs.browser.cve.CveLinkView" - permission="launchpad.AnyPerson" + permission="launchpad.Edit" template="../templates/bug-cve.pt"> </browser:page> <browser:page name="+unlinkcve" for="lp.bugs.interfaces.bugtask.IBugTask" class="lp.bugs.browser.cve.CveUnlinkView" - permission="launchpad.AnyPerson" + permission="launchpad.Edit" template="../templates/bug-removecve.pt"> </browser:page> <browser:page diff --git a/lib/lp/bugs/browser/tests/test_bug_views.py b/lib/lp/bugs/browser/tests/test_bug_views.py index 9188dfd..b82ae92 100644 --- a/lib/lp/bugs/browser/tests/test_bug_views.py +++ b/lib/lp/bugs/browser/tests/test_bug_views.py @@ -24,11 +24,13 @@ from testtools.matchers import ( Not, ) from zope.component import getUtility +from zope.security.interfaces import Unauthorized from zope.security.proxy import removeSecurityProxy from lp.app.enums import InformationType from lp.app.interfaces.services import IService from lp.bugs.adapters.bugchange import BugAttachmentChange +from lp.bugs.enums import BugLockStatus from lp.registry.enums import BugSharingPolicy from lp.registry.interfaces.accesspolicy import ( IAccessPolicyGrantSource, @@ -560,6 +562,24 @@ class TestBugSecrecyViews(TestCaseWithFactory): contents = view.render() self.assertTrue(bug.title in contents) + def test_locked_target_owner(self): + # The target owner can change privacy of locked bugs. + bug = self.factory.makeBug() + target_owner = bug.default_bugtask.target.owner + with person_logged_in(target_owner): + bug.lock(who=target_owner, status=BugLockStatus.COMMENT_ONLY) + self.createInitializedSecrecyView(person=target_owner, bug=bug) + self.assertEqual(InformationType.USERDATA, bug.information_type) + + def test_locked_unprivileged(self): + # Unprivileged users cannot change privacy of locked bugs. + bug = self.factory.makeBug() + target_owner = bug.default_bugtask.target.owner + with person_logged_in(target_owner): + bug.lock(who=target_owner, status=BugLockStatus.COMMENT_ONLY) + self.assertRaises( + Unauthorized, self.createInitializedSecrecyView, bug=bug) + class TestBugTextViewPrivateTeams(TestCaseWithFactory): """ Test for rendering BugTextView with private team artifacts. @@ -787,6 +807,36 @@ class TestBugMarkAsDuplicateView(TestCaseWithFactory): {'id': 'affected-software', 'class': 'listing'}) self.assertIsNotNone(table) + def test_locked_target_owner(self): + # The target owner can set a locked bug as a duplicate. + target_owner = self.bug.default_bugtask.target.owner + with person_logged_in(target_owner): + self.bug.lock(who=target_owner, status=BugLockStatus.COMMENT_ONLY) + form = { + "field.actions.change": "Set Duplicate", + "field.duplicateof": str(self.duplicate_bug.id), + } + create_initialized_view( + self.bug.default_bugtask, name="+duplicate", + principal=target_owner, form=form) + self.assertEqual(self.duplicate_bug, self.bug.duplicateof) + + def test_locked_unprivileged(self): + # Unprivileged users (even the bug reporter) cannot set a locked bug + # as a duplicate. + target_owner = self.bug.default_bugtask.target.owner + with person_logged_in(target_owner): + self.bug.lock(who=target_owner, status=BugLockStatus.COMMENT_ONLY) + with person_logged_in(self.bug_owner): + form = { + "field.actions.change": "Set Duplicate", + "field.duplicateof": str(self.duplicate_bug.id), + } + self.assertRaises( + Unauthorized, create_initialized_view, + self.bug.default_bugtask, name="+duplicate", + principal=self.bug_owner, form=form) + class TestBugActivityView(TestCaseWithFactory): diff --git a/lib/lp/bugs/configure.zcml b/lib/lp/bugs/configure.zcml index 33f6685..3064699 100644 --- a/lib/lp/bugs/configure.zcml +++ b/lib/lp/bugs/configure.zcml @@ -735,6 +735,7 @@ unlinkBranch"/> <require permission="launchpad.Edit" + interface="lp.bugs.interfaces.bug.IBugEdit" set_attributes=" name tags diff --git a/lib/lp/bugs/doc/bug.txt b/lib/lp/bugs/doc/bug.txt index 34e8de7..028e325 100644 --- a/lib/lp/bugs/doc/bug.txt +++ b/lib/lp/bugs/doc/bug.txt @@ -183,7 +183,7 @@ private. A bug cannot be made private by an anonymous user. Traceback (most recent call last): ... zope.security.interfaces.Unauthorized: - (..., 'setPrivate', 'launchpad.Append') + (..., 'setPrivate', 'launchpad.Edit') We have to be logged in, so let's do that: diff --git a/lib/lp/bugs/interfaces/bug.py b/lib/lp/bugs/interfaces/bug.py index 7ffc619..7459bd5 100644 --- a/lib/lp/bugs/interfaces/bug.py +++ b/lib/lp/bugs/interfaces/bug.py @@ -11,6 +11,7 @@ __all__ = [ 'IBugAppend', 'IBugBecameQuestionEvent', 'IBugDelta', + 'IBugEdit', 'IBugMute', 'IBugPublic', 'IBugSet', @@ -726,20 +727,6 @@ class IBugAppend(Interface): :param update_heat: Whether to update the bug heat. """ - @operation_parameters( - target=Reference(schema=Interface, title=_('Target'))) - @call_with(owner=REQUEST_USER) - @export_factory_operation(Interface, []) - def addNomination(owner, target): - """Nominate a bug for an IDistroSeries or IProductSeries. - - :owner: An IPerson. - :target: An IDistroSeries or IProductSeries. - - This method creates and returns a BugNomination. (See - lp.bugs.model.bugnomination.BugNomination.) - """ - @call_with(owner=REQUEST_USER) @rename_parameters_as( bugtracker='bug_tracker', remotebug='remote_bug') @@ -753,33 +740,6 @@ class IBugAppend(Interface): def removeWatch(bug_watch, owner): """Remove a bug watch from the bug.""" - @call_with(owner=REQUEST_USER) - @operation_parameters(target=copy_field(IBugTask['target'])) - @export_factory_operation(IBugTask, []) - def addTask(owner, target): - """Create a new bug task on this bug. - - :raises IllegalTarget: if the bug task cannot be added to the bug. - """ - - def convertToQuestion(person, comment=None): - """Create and return a Question from this Bug. - - Bugs that are also in external bug trackers cannot be converted - to questions. This is also true for bugs that are being developed. - - The `IQuestionTarget` is provided by the `IBugTask` that is not - Invalid and is not a conjoined replica. Only one question can be - made from a bug. - - An AssertionError is raised if the bug has zero or many BugTasks - that can provide a QuestionTarget. It will also be raised if a - question was previously created from the bug. - - :person: The `IPerson` creating a question from this bug - :comment: A string. An explanation of why the bug is a question. - """ - def expireNotifications(): """Expire any pending notifications that have not been emailed. @@ -811,81 +771,6 @@ class IBugAppend(Interface): file_alias.restricted. """ - @call_with(user=REQUEST_USER) - @operation_parameters( - # Really IBranchMergeProposal, patched in _schema_circular_imports.py. - merge_proposal=Reference( - Interface, title=_('Merge proposal'), required=True)) - @export_write_operation() - @operation_for_version('devel') - def linkMergeProposal(merge_proposal, user, check_permissions=True): - """Ensure that this MP is linked to this bug.""" - - @call_with(user=REQUEST_USER) - @operation_parameters( - # Really IBranchMergeProposal, patched in _schema_circular_imports.py. - merge_proposal=Reference( - Interface, title=_('Merge proposal'), required=True)) - @export_write_operation() - @operation_for_version('devel') - def unlinkMergeProposal(merge_proposal, user, check_permissions=True): - """Ensure that any links between this bug and the given MP are removed. - """ - - @call_with(user=REQUEST_USER) - @operation_parameters(cve=Reference(ICve, title=_('CVE'), required=True)) - @export_write_operation() - def linkCVE(cve, user, check_permissions=True): - """Ensure that this CVE is linked to this bug.""" - - @call_with(user=REQUEST_USER) - @operation_parameters(cve=Reference(ICve, title=_('CVE'), required=True)) - @export_write_operation() - def unlinkCVE(cve, user, check_permissions=True): - """Ensure that any links between this bug and the given CVE are - removed. - """ - - @mutator_for(IBugPublic['private']) - @operation_parameters(private=copy_field(IBugPublic['private'])) - @call_with(who=REQUEST_USER) - @export_write_operation() - def setPrivate(private, who): - """Set bug privacy. - - :private: True/False. - :who: The IPerson who is making the change. - - Return True if a change is made, False otherwise. - """ - - @mutator_for(IBugView['security_related']) - @operation_parameters( - security_related=copy_field(IBugView['security_related'])) - @call_with(who=REQUEST_USER) - @export_write_operation() - def setSecurityRelated(security_related, who): - """Set bug security. - - :security_related: True/False. - :who: The IPerson who is making the change. - - Return True if a change is made, False otherwise. - """ - - @operation_parameters( - information_type=copy_field(IBugPublic['information_type']), - ) - @call_with(who=REQUEST_USER) - @export_write_operation() - @operation_for_version("devel") - def transitionToInformationType(information_type, who): - """Set the information type for this bug. - - :information_type: The `InformationType` to transition to. - :who: The `IPerson` who is making the change. - """ - def linkMessage(message, bugwatch=None, user=None, remote_comment_id=None): """Add a comment to this bug. @@ -907,12 +792,6 @@ class IBugAppend(Interface): def markUserAffected(user, affected=True): """Mark :user: as affected by this bug.""" - @mutator_for(IBugView['duplicateof']) - @operation_parameters(duplicate_of=copy_field(IBugView['duplicateof'])) - @export_write_operation() - def markAsDuplicate(duplicate_of): - """Mark this bug as a duplicate of another.""" - @operation_parameters( comment_number=Int( title=_('The number of the comment in the list of messages.'), @@ -983,6 +862,132 @@ class IBugAppend(Interface): def unsubscribeFromDupes(person, unsubscribed_by): """Remove this person's subscription from all dupes of this bug.""" + +class IBugEdit(Interface): + """IBug attributes that require launchpad.Edit permission.""" + + @operation_parameters( + target=Reference(schema=Interface, title=_('Target'))) + @call_with(owner=REQUEST_USER) + @export_factory_operation(Interface, []) + def addNomination(owner, target): + """Nominate a bug for an IDistroSeries or IProductSeries. + + :owner: An IPerson. + :target: An IDistroSeries or IProductSeries. + + This method creates and returns a BugNomination. (See + lp.bugs.model.bugnomination.BugNomination.) + """ + + @call_with(owner=REQUEST_USER) + @operation_parameters(target=copy_field(IBugTask['target'])) + @export_factory_operation(IBugTask, []) + def addTask(owner, target): + """Create a new bug task on this bug. + + :raises IllegalTarget: if the bug task cannot be added to the bug. + """ + + def convertToQuestion(person, comment=None): + """Create and return a Question from this Bug. + + Bugs that are also in external bug trackers cannot be converted + to questions. This is also true for bugs that are being developed. + + The `IQuestionTarget` is provided by the `IBugTask` that is not + Invalid and is not a conjoined replica. Only one question can be + made from a bug. + + An AssertionError is raised if the bug has zero or many BugTasks + that can provide a QuestionTarget. It will also be raised if a + question was previously created from the bug. + + :person: The `IPerson` creating a question from this bug + :comment: A string. An explanation of why the bug is a question. + """ + + @call_with(user=REQUEST_USER) + @operation_parameters( + # Really IBranchMergeProposal, patched in _schema_circular_imports.py. + merge_proposal=Reference( + Interface, title=_('Merge proposal'), required=True)) + @export_write_operation() + @operation_for_version('devel') + def linkMergeProposal(merge_proposal, user, check_permissions=True): + """Ensure that this MP is linked to this bug.""" + + @call_with(user=REQUEST_USER) + @operation_parameters( + # Really IBranchMergeProposal, patched in _schema_circular_imports.py. + merge_proposal=Reference( + Interface, title=_('Merge proposal'), required=True)) + @export_write_operation() + @operation_for_version('devel') + def unlinkMergeProposal(merge_proposal, user, check_permissions=True): + """Ensure that any links between this bug and the given MP are removed. + """ + + @call_with(user=REQUEST_USER) + @operation_parameters(cve=Reference(ICve, title=_('CVE'), required=True)) + @export_write_operation() + def linkCVE(cve, user, check_permissions=True): + """Ensure that this CVE is linked to this bug.""" + + @call_with(user=REQUEST_USER) + @operation_parameters(cve=Reference(ICve, title=_('CVE'), required=True)) + @export_write_operation() + def unlinkCVE(cve, user, check_permissions=True): + """Ensure that any links between this bug and the given CVE are + removed. + """ + + @mutator_for(IBugPublic['private']) + @operation_parameters(private=copy_field(IBugPublic['private'])) + @call_with(who=REQUEST_USER) + @export_write_operation() + def setPrivate(private, who): + """Set bug privacy. + + :private: True/False. + :who: The IPerson who is making the change. + + Return True if a change is made, False otherwise. + """ + + @mutator_for(IBugView['security_related']) + @operation_parameters( + security_related=copy_field(IBugView['security_related'])) + @call_with(who=REQUEST_USER) + @export_write_operation() + def setSecurityRelated(security_related, who): + """Set bug security. + + :security_related: True/False. + :who: The IPerson who is making the change. + + Return True if a change is made, False otherwise. + """ + + @operation_parameters( + information_type=copy_field(IBugPublic['information_type']), + ) + @call_with(who=REQUEST_USER) + @export_write_operation() + @operation_for_version("devel") + def transitionToInformationType(information_type, who): + """Set the information type for this bug. + + :information_type: The `InformationType` to transition to. + :who: The `IPerson` who is making the change. + """ + + @mutator_for(IBugView['duplicateof']) + @operation_parameters(duplicate_of=copy_field(IBugView['duplicateof'])) + @export_write_operation() + def markAsDuplicate(duplicate_of): + """Mark this bug as a duplicate of another.""" + def setStatus(target, status, user): """Set the status of the bugtask related to the specified target. @@ -997,6 +1002,7 @@ class IBugAppend(Interface): Return None if no bugtask was edited. """ + class IBugModerate(Interface): """IBug attributes that require the launchpad.Moderate permission.""" @@ -1044,7 +1050,8 @@ class IBugModerate(Interface): @exported_as_webservice_entry() -class IBug(IBugPublic, IBugView, IBugAppend, IBugModerate, IHasLinkedBranches): +class IBug(IBugPublic, IBugView, IBugAppend, IBugEdit, IBugModerate, + IHasLinkedBranches): """The core bug entry.""" linked_bugbranches = exported( diff --git a/lib/lp/bugs/security.py b/lib/lp/bugs/security.py index 9ec2b05..57ad9fa 100644 --- a/lib/lp/bugs/security.py +++ b/lib/lp/bugs/security.py @@ -38,6 +38,19 @@ class EditBugNominationStatus(AuthorizationBase): return self.obj.canApprove(user.person) +class AppendBugTask(DelegatedAuthorization): + """Security adapter for appending to bug tasks. + + This has the same semantics as `AppendBug`, but can be used where the + context is a bug task rather than a bug. + """ + permission = 'launchpad.Append' + usedfor = IHasBug + + def __init__(self, obj): + super().__init__(obj, obj.bug) + + class EditBugTask(DelegatedAuthorization): """Permission checker for editing objects linked to a bug. diff --git a/lib/lp/bugs/stories/bug-also-affects/xx-bug-also-affects.txt b/lib/lp/bugs/stories/bug-also-affects/xx-bug-also-affects.txt index 67bf508..d58193d 100644 --- a/lib/lp/bugs/stories/bug-also-affects/xx-bug-also-affects.txt +++ b/lib/lp/bugs/stories/bug-also-affects/xx-bug-also-affects.txt @@ -716,7 +716,7 @@ logged in. --> None >>> print_assigned_bugtasks(anon_browser) - GNOME Terminal ... --> + GNOME Terminal --> auto-dark-master-o-bugs >>> anon_browser.contents.count( diff --git a/lib/lp/bugs/stories/bug-release-management/xx-anonymous-bug-nomination.txt b/lib/lp/bugs/stories/bug-release-management/xx-anonymous-bug-nomination.txt index 52c7c53..ffbf5d9 100644 --- a/lib/lp/bugs/stories/bug-release-management/xx-anonymous-bug-nomination.txt +++ b/lib/lp/bugs/stories/bug-release-management/xx-anonymous-bug-nomination.txt @@ -5,8 +5,13 @@ Anonymous users should not be able to nominate bugs for release because launchpad.Edit permission is required to do so.: >>> anon_browser.open('http://bugs.launchpad.test/jokosher/+bug/12') + >>> anon_browser.getLink('Nominate for series') + Traceback (most recent call last): + ... + zope.testbrowser.browser.LinkNotFoundError - >>> anon_browser.getLink('Nominate for series').click() + >>> anon_browser.open( + ... 'http://bugs.launchpad.test/jokosher/+bug/12/+nominate') Traceback (most recent call last): ... zope.security.interfaces.Unauthorized: ...'launchpad.Edit'... diff --git a/lib/lp/bugs/stories/bugs/xx-bug-comment-attach-file.txt b/lib/lp/bugs/stories/bugs/xx-bug-comment-attach-file.txt index a870168..3494cc4 100644 --- a/lib/lp/bugs/stories/bugs/xx-bug-comment-attach-file.txt +++ b/lib/lp/bugs/stories/bugs/xx-bug-comment-attach-file.txt @@ -1,14 +1,14 @@ Bug commenting and attaching files is done on the same form. When a comment is submitted, the attachment is optional. Accessing the -comment form requires launchpad.Edit permission on the bugtask. In this +comment form requires launchpad.Append permission on the bugtask. In this case, it means being logged in. >>> anon_browser.open( ... "http://bugs.launchpad.test/firefox/+bug/1/+addcomment-form") Traceback (most recent call last): .. - zope.security.interfaces.Unauthorized: ...launchpad.Edit... + zope.security.interfaces.Unauthorized: ...launchpad.Append... So let's login and add a comment. diff --git a/lib/lp/bugs/stories/bugs/xx-bug-create-question.txt b/lib/lp/bugs/stories/bugs/xx-bug-create-question.txt index b2d2ee3..2b65420 100644 --- a/lib/lp/bugs/stories/bugs/xx-bug-create-question.txt +++ b/lib/lp/bugs/stories/bugs/xx-bug-create-question.txt @@ -16,7 +16,14 @@ An anonymous user cannot create a question. >>> anon_browser.title 'Bug #10 ... : Bugs : linux-source-2.6.15 package : Ubuntu' - >>> anon_browser.getLink('Convert to a question').click() + >>> anon_browser.getLink('Convert to a question') + Traceback (most recent call last): + ... + zope.testbrowser.browser.LinkNotFoundError + + >>> anon_browser.open( + ... 'http://bugs.launchpad.test' + ... '/ubuntu/+source/linux-source-2.6.15/+bug/10/+create-question') Traceback (most recent call last): ... zope.security.interfaces.Unauthorized: ... diff --git a/lib/lp/bugs/stories/bugs/xx-bug-edit.txt b/lib/lp/bugs/stories/bugs/xx-bug-edit.txt index 02195c1..7700e4a 100644 --- a/lib/lp/bugs/stories/bugs/xx-bug-edit.txt +++ b/lib/lp/bugs/stories/bugs/xx-bug-edit.txt @@ -94,3 +94,37 @@ Now, let's add 'new-tag' again, and confirm it this time. False >>> 'new-tag' in tags_div True + + +Locked bugs +----------- + +The metadata of a locked bug can only be edited by privileged users. + + >>> from zope.component import getUtility + >>> from lp.bugs.enums import BugLockStatus + >>> from lp.bugs.interfaces.bug import IBugSet + >>> from lp.testing.pages import setupBrowserForUser + + >>> login(ANONYMOUS) + >>> bug_1 = getUtility(IBugSet).get(1) + >>> target_owner = bug_1.default_bugtask.target.owner + >>> _ = login_person(target_owner) + >>> bug_1.lock(who=target_owner, status=BugLockStatus.COMMENT_ONLY) + >>> logout() + + >>> target_owner_browser = setupBrowserForUser(target_owner) + >>> target_owner_browser.open('http://bugs.launchpad.test/firefox/+bug/1') + >>> target_owner_browser.getLink(url='+edit').click() + >>> target_owner_browser.getControl('Description').value = 'Now locked.' + >>> target_owner_browser.getControl('Change').click() + + >>> user_browser.open('http://bugs.launchpad.test/firefox/+bug/1') + >>> user_browser.getLink(url='+edit') + Traceback (most recent call last): + ... + zope.testbrowser.browser.LinkNotFoundError + >>> user_browser.open('http://bugs.launchpad.test/firefox/+bug/1/+edit') + Traceback (most recent call last): + ... + zope.security.interfaces.Unauthorized: ... diff --git a/lib/lp/bugs/stories/bugs/xx-bug-index.txt b/lib/lp/bugs/stories/bugs/xx-bug-index.txt index 3af7922..590df67 100644 --- a/lib/lp/bugs/stories/bugs/xx-bug-index.txt +++ b/lib/lp/bugs/stories/bugs/xx-bug-index.txt @@ -154,9 +154,9 @@ initialization code if there's more than 10 bug tasks. On the bug page, for every bug task there's one expander. - >>> browser.open(bug_url) + >>> user_browser.open(bug_url) >>> print(len(find_tags_by_class( - ... browser.contents, 'bugtask-expander'))) + ... user_browser.contents, 'bugtask-expander'))) 1 We add four more tasks. @@ -168,9 +168,9 @@ We add four more tasks. And the expander appears five times. - >>> browser.open(bug_url) + >>> user_browser.open(bug_url) >>> print(len(find_tags_by_class( - ... browser.contents, 'bugtask-expander'))) + ... user_browser.contents, 'bugtask-expander'))) 5 But on pages with more than ten bug tasks, we don't include the expander @@ -181,8 +181,49 @@ at all. ... _ = bug.addTask(bug.owner, factory.makeProduct()) >>> logout() - >>> browser.open(bug_url) + >>> user_browser.open(bug_url) >>> print(len(find_tags_by_class( - ... browser.contents, 'bugtask-expander'))) + ... user_browser.contents, 'bugtask-expander'))) 0 +We also don't include the expander for anonymous requests. + + >>> anon_browser.open(bug_url) + >>> print(len(find_tags_by_class( + ... anon_browser.contents, 'bugtask-expander'))) + 0 + + +Locked bugs +----------- + +Unprivileged users viewing locked bugs don't see the bugtask expander, nor +do they see any edit links. + + >>> import re + >>> from lp.bugs.enums import BugLockStatus + >>> from lp.testing.pages import setupBrowserForUser + + >>> login('foo....@canonical.com') + >>> locked_bug = factory.makeBug() + >>> target_owner = locked_bug.default_bugtask.target.owner + >>> locked_bug.lock(who=target_owner, status=BugLockStatus.COMMENT_ONLY) + >>> locked_bug_url = canonical_url(locked_bug) + >>> logout() + + >>> target_owner_browser = setupBrowserForUser(target_owner) + >>> target_owner_browser.open(locked_bug_url) + >>> print(len(find_tags_by_class( + ... target_owner_browser.contents, 'bugtask-expander'))) + 1 + >>> len(find_main_content(target_owner_browser.contents).find_all( + ... 'a', href=re.compile(r'.*/\+edit.*'))) + 5 + + >>> user_browser.open(locked_bug_url) + >>> print(len(find_tags_by_class( + ... user_browser.contents, 'bugtask-expander'))) + 0 + >>> len(find_main_content(user_browser.contents).find_all( + ... 'a', href=re.compile(r'.*/\+edit.*'))) + 0 diff --git a/lib/lp/bugs/stories/bugtask-management/xx-view-editable-bug-task.txt b/lib/lp/bugs/stories/bugtask-management/xx-view-editable-bug-task.txt index c906387..10c906c 100644 --- a/lib/lp/bugs/stories/bugtask-management/xx-view-editable-bug-task.txt +++ b/lib/lp/bugs/stories/bugtask-management/xx-view-editable-bug-task.txt @@ -32,3 +32,24 @@ Any logged-in user can edit a bugtask. >>> browser.open("http://launchpad.test/firefox/+bug/6/+editstatus") >>> print(browser.title) Edit status ... + +But only privileged users can edit bugtasks on locked bugs. + + >>> from zope.component import getUtility + >>> from lp.bugs.enums import BugLockStatus + >>> from lp.bugs.interfaces.bug import IBugSet + + >>> login("t...@canonical.com") + >>> getUtility(IBugSet).get(6).lock( + ... who=sample_person, status=BugLockStatus.COMMENT_ONLY) + >>> logout() + + >>> browser = setupBrowser(auth="Basic t...@canonical.com:test") + >>> browser.open("http://launchpad.test/firefox/+bug/6/+editstatus") + >>> print(browser.title) + Edit status ... + + >>> user_browser.open("http://launchpad.test/firefox/+bug/6/+editstatus") + Traceback (most recent call last): + ... + zope.security.interfaces.Unauthorized: ... diff --git a/lib/lp/bugs/templates/bugtask-index.pt b/lib/lp/bugs/templates/bugtask-index.pt index 911161b..287bb85 100644 --- a/lib/lp/bugs/templates/bugtask-index.pt +++ b/lib/lp/bugs/templates/bugtask-index.pt @@ -179,14 +179,16 @@ class="unofficial-tag" tal:attributes="href python: tag[1]">tag</a> </span> - <a href="+edit" title="Add tags" id="tags-trigger" - class="sprite add" - tal:condition="not: context/bug/tags">Add tags</a> - <a href="+edit" title="Edit tags" id="tags-trigger" - class="sprite edit action-icon" - tal:condition="context/bug/tags">Edit</a> - <a href="/+help-bugs/tag-help.html" target="help" - class="sprite maybe action-icon">Tag help</a> + <tal:can-edit condition="context/required:launchpad.Edit"> + <a href="+edit" title="Add tags" id="tags-trigger" + class="sprite add" + tal:condition="not: context/bug/tags">Add tags</a> + <a href="+edit" title="Edit tags" id="tags-trigger" + class="sprite edit action-icon" + tal:condition="context/bug/tags">Edit</a> + <a href="/+help-bugs/tag-help.html" target="help" + class="sprite maybe action-icon">Tag help</a> + </tal:can-edit> </div> <script type="text/javascript"> diff --git a/lib/lp/bugs/templates/bugtask-tasks-and-nominations-table-row.pt b/lib/lp/bugs/templates/bugtask-tasks-and-nominations-table-row.pt index 4992385..8702dd7 100644 --- a/lib/lp/bugs/templates/bugtask-tasks-and-nominations-table-row.pt +++ b/lib/lp/bugs/templates/bugtask-tasks-and-nominations-table-row.pt @@ -33,7 +33,8 @@ class context/target/image:sprite_css" tal:content="context/bugtargetdisplayname" /> </span> - <button class="lazr-btn yui3-activator-act yui3-activator-hidden"> + <button class="lazr-btn yui3-activator-act yui3-activator-hidden" + tal:condition="data/can_edit"> Edit </button> <div class="yui3-activator-message-box yui3-activator-hidden"></div> @@ -152,7 +153,8 @@ tal:attributes="href data/edit_link"> <img class="editicon" src="/@@/edit" /> </a> - <button class="lazr-btn yui3-activator-act yui3-activator-hidden"> + <button class="lazr-btn yui3-activator-act yui3-activator-hidden" + tal:condition="view/user_can_edit_assignee"> Edit </button> <div class="yui3-activator-message-box yui3-activator-hidden"></div> diff --git a/lib/lp/bugs/templates/bugtasks-and-nominations-portal.pt b/lib/lp/bugs/templates/bugtasks-and-nominations-portal.pt index 3f53c63..25d02d9 100644 --- a/lib/lp/bugs/templates/bugtasks-and-nominations-portal.pt +++ b/lib/lp/bugs/templates/bugtasks-and-nominations-portal.pt @@ -4,7 +4,7 @@ omit-tag=""> <tal:affects-me-too - tal:condition="view/displayAlsoAffectsLinks"> + tal:condition="view/displayAffectedUsers"> <tal:editable condition="context/menu:context/affectsmetoo/enabled"> <div class="actions">
_______________________________________________ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp