Revision history for this message
| Muharem Hrnjadovic (al-maisan) wrote : | # |
| 1 | === modified file 'lib/canonical/launchpad/security.py' |
| 2 | --- lib/canonical/launchpad/security.py 2009-10-19 10:33:14 +0000 |
| 3 | +++ lib/canonical/launchpad/security.py 2009-10-20 07:51:12 +0000 |
| 4 | @@ -84,7 +84,8 @@ |
| 5 | from lp.registry.interfaces.productseries import IProductSeries |
| 6 | from lp.registry.interfaces.project import IProject, IProjectSet |
| 7 | from lp.code.interfaces.seriessourcepackagebranch import ( |
| 8 | - ISeriesSourcePackageBranch, IMakeOfficialBranchLinks) |
| 9 | + IFindOfficialBranchLinks, IMakeOfficialBranchLinks, |
| 10 | + ISeriesSourcePackageBranch) |
| 11 | from lp.registry.interfaces.sourcepackage import ISourcePackage |
| 12 | from lp.soyuz.interfaces.sourcepackagerelease import ( |
| 13 | ISourcePackageRelease) |
| 14 | @@ -1598,48 +1599,47 @@ |
| 15 | |
| 16 | def can_upload_linked_package(person, branch): |
| 17 | """True if person may upload the package linked to `branch`.""" |
| 18 | - def get_current_release(): |
| 19 | + |
| 20 | + def get_current_release(sspb): |
| 21 | """Get current release for the source package linked to branch.""" |
| 22 | - ds = branch.distroseries |
| 23 | - package = branch.sourcepackage |
| 24 | - releases = ds.getCurrentSourceReleases([package.sourcepackagename]) |
| 25 | + package = sspb.sourcepackage |
| 26 | + releases = sspb.distroseries.getCurrentSourceReleases( |
| 27 | + [package.sourcepackagename]) |
| 28 | return releases.get(package, None) |
| 29 | |
| 30 | - def get_property_value(obj, property_name): |
| 31 | - """Get value of requested property or None.""" |
| 32 | - value = None |
| 33 | - try: |
| 34 | - value = getattr(obj, property_name) |
| 35 | - except AttributeError: |
| 36 | - return None |
| 37 | - return value |
| 38 | - |
| 39 | - # Check whether we're dealing with a source package branch and |
| 40 | - # whether person is authorised to upload the respective source |
| 41 | + # No associated `SeriesSourcePackageBranch` data -> not an official |
| 42 | + # branch. Abort. |
| 43 | + result_set = getUtility(IFindOfficialBranchLinks).findForBranch(branch) |
| 44 | + if result_set.count() < 1: |
| 45 | + return False |
| 46 | + |
| 47 | + # XXX al-maisan, 2009-10-20: a branch may currently be associated with a |
| 48 | + # number of (distroseries, sourcepackagename, pocket) combinations. |
| 49 | + # This does not seem right. But until the database model is fixed we work |
| 50 | + # around this by assuming that things are fine as long as we find at least |
| 51 | + # one combination that allows us to upload the corresponding source |
| 52 | # package. |
| 53 | - if branch.sourcepackage is None: |
| 54 | - # No package .. hmm .. this can't be a source package branch |
| 55 | - # then. Abort. |
| 56 | - return False |
| 57 | - |
| 58 | - if branch.distroseries is None: |
| 59 | - # No distro series? Abort. |
| 60 | - return False |
| 61 | - |
| 62 | - current_release = get_current_release() |
| 63 | - |
| 64 | - # Do we have a pocket and, if yes, can we upload to it? |
| 65 | - pocket = get_property_value(current_release, 'pocket') |
| 66 | - if not (pocket is None or branch.distroseries.canUploadToPocket(pocket)): |
| 67 | - return False |
| 68 | - |
| 69 | - archive = branch.distroseries.distribution.main_archive |
| 70 | - component = get_property_value(current_release, 'component') |
| 71 | + |
| 72 | + # See whether we can upload to any of the distro series/pocket |
| 73 | + # combinations. |
| 74 | + sspb = None |
| 75 | + for sspb in result_set: |
| 76 | + # Can we upload to the respective pocket? |
| 77 | + if sspb.distroseries.canUploadToPocket(sspb.pocket): |
| 78 | + break |
| 79 | + else: |
| 80 | + # Loop terminated normally i.e. we could not upload to any of the |
| 81 | + # (distroseries, pocket) combinations found. |
| 82 | + return False |
| 83 | + |
| 84 | + archive = sspb.distroseries.distribution.main_archive |
| 85 | + # Find the component the linked source package was published in. |
| 86 | + current_release = get_current_release(sspb) |
| 87 | + component = getattr(current_release, 'component', None) |
| 88 | |
| 89 | # Is person authorised to upload the source package this branch |
| 90 | # is targeting? |
| 91 | - result = verify_upload( |
| 92 | - person, branch.sourcepackage.sourcepackagename, archive, component) |
| 93 | + result = verify_upload(person, sspb.sourcepackagename, archive, component) |
| 94 | # verify_upload() indicates that person *is* allowed to upload by |
| 95 | # returning None. |
| 96 | return result is None |
| 97 | |
| 98 | === modified file 'lib/lp/code/tests/test_branch.py' |
| 99 | --- lib/lp/code/tests/test_branch.py 2009-10-19 10:12:44 +0000 |
| 100 | +++ lib/lp/code/tests/test_branch.py 2009-10-20 07:39:30 +0000 |
| 101 | @@ -17,6 +17,8 @@ |
| 102 | BranchSubscriptionDiffSize, BranchSubscriptionNotificationLevel, |
| 103 | CodeReviewNotificationLevel) |
| 104 | from lp.code.interfaces.linkedbranch import ICanHasLinkedBranch |
| 105 | +from lp.registry.interfaces.distroseries import DistroSeriesStatus |
| 106 | +from lp.registry.interfaces.pocket import PackagePublishingPocket |
| 107 | from lp.soyuz.interfaces.archivepermission import IArchivePermissionSet |
| 108 | from lp.testing import run_with_login, TestCaseWithFactory |
| 109 | |
| 110 | @@ -232,7 +234,14 @@ |
| 111 | def makeOfficialPackageBranch(self): |
| 112 | """Make a branch linked to the pocket of a source package.""" |
| 113 | branch = self.factory.makePackageBranch() |
| 114 | - pocket = self.factory.getAnyPocket() |
| 115 | + # Make sure the (distroseries, pocket) combination used allows us to |
| 116 | + # upload to it. |
| 117 | + stable_states = ( |
| 118 | + DistroSeriesStatus.SUPPORTED, DistroSeriesStatus.CURRENT) |
| 119 | + if branch.distroseries.status in stable_states: |
| 120 | + pocket = PackagePublishingPocket.BACKPORTS |
| 121 | + else: |
| 122 | + pocket = PackagePublishingPocket.RELEASE |
| 123 | sourcepackage = branch.sourcepackage |
| 124 | suite_sourcepackage = sourcepackage.getSuiteSourcePackage(pocket) |
| 125 | registrant = self.factory.makePerson() |
Jonathan Lange wrote:
> On Mon, Oct 19, 2009 at 3:15 PM, Muharem Hrnjadovic
> <email address hidden> wrote:
>> Jonathan Lange wrote:
>>> On Mon, Oct 19, 2009 at 12:51 PM, Muharem Hrnjadovic
>>> <email address hidden> wrote:
>>>> Jonathan Lange wrote:
>>>>> Review: Needs Fixing
[..]
>>>> thanks for looking :) What is the recommended way to test for an
>>>> officially linked branch? Test the URL?
>>>> Check for the presence of a SeriesSourcePac
>>>> Use SeriesSourcePac
>>>>
>>> I would use getUtility(
>>> would do this check instead of checking that the branch is a
>>> sourcepackage branch, and I would check for upload rights to any of
>>> the packages that are officially linked.
Right. This is what the revised can_upload_
does.
Please see the enclosed incremental diff for details.
>> Packages? Is this a one-branch-
>> looking at the code it looked like a 1-to-1 relationship to me ..
>>
>
> One branch can belong to at most one package.
>
> However, a branch can be an official package branch for many
> (sourcepackage, pocket) combinations, even if the branch itself does
> not belong to a source package. This is deliberate.
>
>> Anyway, after revising the code as suggested I get security exceptions
>> (please see http://
>> I guess the ISeriesSourcePa
>> a read-only and an edit portion respectively. As it stands all access
>> to it requires the "launchpad.Edit" permissions.
>>
>> <class
>>
>> class="
>> <allow
>> interface=
>> <require
>> permission=
>>
>> set_schema=
>> </class>
>>
>
> Something's wrong here.
>
> ISeriesSourcePa
> package branches (e.g. lp:ubuntu/emacs-snapshot). This works
> anonymously. (bazaar_identity calls
> branch.
> IFindOfficialBr
>
>>From the pastebin,
> ForbiddenAttribute: ('pocket', <storm.
>
> This means that you cannot get pocket from a ResultSet. This makes
> sense, since result sets are storm objects and don't have pockets.
>
> File "/home/
> line 1616, in can_upload_
> if not branch.
> AttributeError: 'NoneType' object has no attribute 'canUploadToPocket'
>
> This error is correct, since the condition is checking the wrong
> thing. The question is not, "can we upload to this pocket in the
> distroseries that the branch belongs to" but rather "can we upload to
> this pocket in the distroseries that the branch is official for".
>
> i.e. sspb.distroseri
I am both relieved (that the error is so easily fixed) and embarrassed
that I made such a blunder :P Thanks for pointing it out!
Best regards
--
Muharem Hrnjadovic <email address hidden>
Public key id : B2BBFCFC
Key fingerprint : A5A3 CC67 2B87 D641 103F 5602 219F 6B60 B2BB FCFC