Abel Deuring has proposed merging lp:~adeuring/launchpad/product-lp-limitedview into lp:launchpad.
Requested reviews: Launchpad code reviewers (launchpad-reviewers) For more details, see: https://code.launchpad.net/~adeuring/launchpad/product-lp-limitedview/+merge/133046 This branch splits the existing interface IProductLimitedView into IProductView and IProductLimitedView. IProductView now requires the permission lauchpad.View, and IProductLimitedView requires the permission launchpad.LimitedView. We want to make it possible to use artifact grants on bugs/branches/ blueprints for prioprietary/embargoed products, so that grantees can see the artifacts but not all details about the product. Even a call of canonical_url(artifact) requires access to artifact.target.name, so people with artifact grants must be able to see target.name, as well as several other attributes. I don't know yet which attributes or even how many attributes need to be moved from IProductView to IProductLimitedView, but I suspect that it will be quite a few. Doing this all in one branch has the risk of a "diff length explosion", so I started with just one attribute. The new security adapter LimitedViewProduct allows access not only for people with grants on the product itself, but it also checks if the current user has any artifact grants related to the product. The new check is implementd in SharingService.userHasGrantsOnPillar(), which is just a modified variant of the existing method getSharedArtifacts(): While the latter method returns all artifacts a user may access, the former method just checks, if the user has any artifact grants. The common SQL query is generated in the new method getArtifactGrantsForPersonOnPillar(). test: bin/test -vvt lp.registry.tests.test_product.TestProduct.test_access_LimitedView bin/test -vvt lp.registry.tests.test_product.TestProduct.test_get_permissions no lint -- https://code.launchpad.net/~adeuring/launchpad/product-lp-limitedview/+merge/133046 Your team Launchpad code reviewers is requested to review the proposed merge of lp:~adeuring/launchpad/product-lp-limitedview into lp:launchpad.
=== modified file 'lib/lp/registry/configure.zcml' --- lib/lp/registry/configure.zcml 2012-10-25 14:36:30 +0000 +++ lib/lp/registry/configure.zcml 2012-11-06 11:54:23 +0000 @@ -1248,10 +1248,13 @@ <allow interface="lp.registry.interfaces.pillar.IPillar"/> <require - permission="launchpad.View" + permission="launchpad.LimitedView" interface="lp.registry.interfaces.product.IProductLimitedView"/> <require permission="launchpad.View" + interface="lp.registry.interfaces.product.IProductView"/> + <require + permission="launchpad.View" interface="lp.bugs.interfaces.bugsummary.IBugSummaryDimension"/> <require permission="launchpad.View" === modified file 'lib/lp/registry/interfaces/product.py' --- lib/lp/registry/interfaces/product.py 2012-10-24 14:21:58 +0000 +++ lib/lp/registry/interfaces/product.py 2012-11-06 11:54:23 +0000 @@ -428,7 +428,22 @@ """True if the given user has access to this product.""" -class IProductLimitedView( +class IProductLimitedView(Interface): + """Attributes that must be visible for person with artifact grants + on bugs, branches or specifications for the product, + """ + + name = exported( + ProductNameField( + title=_('Name'), + constraint=name_validator, + description=_( + "At least one lowercase letter or number, followed by " + "letters, numbers, dots, hyphens or pluses. " + "Keep this name short; it is used in URLs as shown above."))) + + +class IProductView( IBugTarget, ICanGetMilestonesDirectly, IHasAppointedDriver, IHasBranches, IHasDrivers, IHasExternalBugTracker, IHasIcon, IHasLogo, IHasMergeProposals, IHasMilestones, IHasExpirableBugs, @@ -488,15 +503,6 @@ "required because there might be a project driver and also a " "driver appointed in the overarching project group.") - name = exported( - ProductNameField( - title=_('Name'), - constraint=name_validator, - description=_( - "At least one lowercase letter or number, followed by " - "letters, numbers, dots, hyphens or pluses. " - "Keep this name short; it is used in URLs as shown above."))) - displayname = exported( TextLine( title=_('Display Name'), @@ -874,9 +880,9 @@ class IProductEditRestricted(IOfficialBugTagTargetRestricted): """`IProduct` properties which require launchpad.Edit permission.""" - @mutator_for(IProductLimitedView['bug_sharing_policy']) + @mutator_for(IProductView['bug_sharing_policy']) @operation_parameters(bug_sharing_policy=copy_field( - IProductLimitedView['bug_sharing_policy'])) + IProductView['bug_sharing_policy'])) @export_write_operation() @operation_for_version("devel") def setBugSharingPolicy(bug_sharing_policy): @@ -885,10 +891,10 @@ Checks authorization and entitlement. """ - @mutator_for(IProductLimitedView['branch_sharing_policy']) + @mutator_for(IProductView['branch_sharing_policy']) @operation_parameters( branch_sharing_policy=copy_field( - IProductLimitedView['branch_sharing_policy'])) + IProductView['branch_sharing_policy'])) @export_write_operation() @operation_for_version("devel") def setBranchSharingPolicy(branch_sharing_policy): @@ -897,10 +903,10 @@ Checks authorization and entitlement. """ - @mutator_for(IProductLimitedView['specification_sharing_policy']) + @mutator_for(IProductView['specification_sharing_policy']) @operation_parameters( specification_sharing_policy=copy_field( - IProductLimitedView['specification_sharing_policy'])) + IProductView['specification_sharing_policy'])) @export_write_operation() @operation_for_version("devel") def setSpecificationSharingPolicy(specification_sharing_policy): @@ -912,7 +918,7 @@ class IProduct( IHasBugSupervisor, IProductEditRestricted, - IProductModerateRestricted, IProductDriverRestricted, + IProductModerateRestricted, IProductDriverRestricted, IProductView, IProductLimitedView, IProductPublic, IQuestionTarget, IRootContext, IStructuralSubscriptionTarget, IInformationType, IPillar): """A Product. === modified file 'lib/lp/registry/interfaces/sharingservice.py' --- lib/lp/registry/interfaces/sharingservice.py 2012-10-10 03:20:17 +0000 +++ lib/lp/registry/interfaces/sharingservice.py 2012-11-06 11:54:23 +0000 @@ -117,6 +117,9 @@ :return: a (bugtasks, branches, specifications) tuple """ + def userHasGrantsOnPillar(pillar, user): + """Return True if user has any grants on pillar else return False.""" + @export_read_operation() @call_with(user=REQUEST_USER) @operation_parameters( === modified file 'lib/lp/registry/services/sharingservice.py' --- lib/lp/registry/services/sharingservice.py 2012-10-10 02:45:36 +0000 +++ lib/lp/registry/services/sharingservice.py 2012-11-06 11:54:23 +0000 @@ -186,16 +186,21 @@ """See `ISharingService`.""" return self._getSharedPillars(person, user, Distribution) + def getArtifactGrantsForPersonOnPillar(self, pillar, person): + """Return the artifact grants for the given person and pillar.""" + policies = getUtility(IAccessPolicySource).findByPillar([pillar]) + flat_source = getUtility(IAccessPolicyGrantFlatSource) + return flat_source.findArtifactsByGrantee(person, policies) + @available_with_permission('launchpad.Driver', 'pillar') def getSharedArtifacts(self, pillar, person, user, include_bugs=True, include_branches=True, include_specifications=True): """See `ISharingService`.""" - policies = getUtility(IAccessPolicySource).findByPillar([pillar]) - flat_source = getUtility(IAccessPolicyGrantFlatSource) bug_ids = set() branch_ids = set() specification_ids = set() - for artifact in flat_source.findArtifactsByGrantee(person, policies): + for artifact in self.getArtifactGrantsForPersonOnPillar( + pillar, person): if artifact.bug_id and include_bugs: bug_ids.add(artifact.bug_id) elif artifact.branch_id and include_branches: @@ -222,6 +227,11 @@ return bugtasks, branches, specifications + def userHasGrantsOnPillar(self, pillar, user): + """See `ISharingService`.""" + return not self.getArtifactGrantsForPersonOnPillar( + pillar, user).is_empty() + @available_with_permission('launchpad.Driver', 'pillar') def getSharedBugs(self, pillar, person, user): """See `ISharingService`.""" === modified file 'lib/lp/registry/tests/test_product.py' --- lib/lp/registry/tests/test_product.py 2012-11-02 20:08:55 +0000 +++ lib/lp/registry/tests/test_product.py 2012-11-06 11:54:23 +0000 @@ -551,6 +551,7 @@ CheckerPublic: set(( 'active', 'id', 'information_type', 'pillar_category', 'private', 'userCanView',)), + 'launchpad.LimitedView': set(('name', )), 'launchpad.View': set(( '_getOfficialTagClause', '_all_specifications', '_valid_specifications', 'active_or_packaged_series', @@ -594,7 +595,7 @@ 'homepageurl', 'icon', 'invitesTranslationEdits', 'invitesTranslationSuggestions', 'license_info', 'license_status', 'licenses', 'logo', 'milestones', - 'mugshot', 'name', 'name_with_project', 'newCodeImport', + 'mugshot', 'name_with_project', 'newCodeImport', 'obsolete_translatable_series', 'official_answers', 'official_anything', 'official_blueprints', 'official_bug_tags', 'official_codehosting', 'official_malone', 'owner', @@ -766,6 +767,57 @@ for attribute_name in names: getattr(product, attribute_name) + def test_access_LimitedView_public_product(self): + # Everybody can access attributes of public products that + # require the permission launchpad.LimitedView. + product = self.factory.makeProduct() + names = self.expected_get_permissions['launchpad.LimitedView'] + with person_logged_in(None): + for attribute_name in names: + getattr(product, attribute_name) + ordinary_user = self.factory.makePerson() + with person_logged_in(ordinary_user): + for attribute_name in names: + getattr(product, attribute_name) + + def test_access_LimitedView_proprietary_product(self): + # Anonymous users and ordinary logged in users cannot access + # attributes of private products that require the permission + # launchpad.LimitedView. + owner = self.factory.makePerson() + product = self.factory.makeProduct( + owner=owner, + information_type=InformationType.PROPRIETARY) + names = self.expected_get_permissions['launchpad.LimitedView'] + with person_logged_in(None): + for attribute_name in names: + self.assertRaises( + Unauthorized, getattr, product, attribute_name) + user = self.factory.makePerson() + with person_logged_in(user): + for attribute_name in names: + self.assertRaises( + Unauthorized, getattr, product, attribute_name) + # Users with a grant on an artifact related to the product + # can access the attributes. + with person_logged_in(owner): + bug = self.factory.makeBug( + target=product, information_type=InformationType.PROPRIETARY) + getUtility(IService, 'sharing').ensureAccessGrants( + [user], owner, bugs=[bug]) + with person_logged_in(user): + for attribute_name in names: + getattr(product, attribute_name) + # Users with a policy grant for the product also have access. + user2 = self.factory.makePerson() + with person_logged_in(owner): + getUtility(IService, 'sharing').sharePillarInformation( + product, user2, owner, + {InformationType.PROPRIETARY: SharingPermission.ALL}) + with person_logged_in(user2): + for attribute_name in names: + getattr(product, attribute_name) + def test_access_launchpad_AnyAllowedPerson_public_product(self): # Only logged in persons have access to properties of public products # that require the permission launchpad.AnyAllowedPerson. === modified file 'lib/lp/security.py' --- lib/lp/security.py 2012-11-03 13:49:04 +0000 +++ lib/lp/security.py 2012-11-06 11:54:23 +0000 @@ -37,6 +37,7 @@ AuthorizationBase, DelegatedAuthorization, ) +from lp.app.interfaces.services import IService from lp.archivepublisher.interfaces.publisherconfig import IPublisherConfig from lp.blueprints.interfaces.specification import ( ISpecification, @@ -441,6 +442,17 @@ return self.obj.userCanView(None) +class LimitedViewProduct(ViewProduct): + permission = 'launchpad.LimitedView' + usedfor = IProduct + + def checkAuthenticated(self, user): + return ( + super(LimitedViewProduct, self).checkAuthenticated(user) or + getUtility(IService, 'sharing').userHasGrantsOnPillar( + self.obj, user)) + + class ChangeProduct(ViewProduct): """Used for attributes of IProduct that are accessible to any logged in user for public product but only to persons with access grants
_______________________________________________ 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