launchpad-dev team mailing list archive

[Celso Providelo <celso.providelo@xxxxxxxxxxxxx>]

On Thu, Jul 23, 2009 at 9:00 AM, Julian
Edwards<julian.edwards@xxxxxxxxxxxxx> wrote:
> On Thursday 23 July 2009 06:29:44 Bjorn Tillenius wrote:
>> Not that this can_upload() function (at least the last time I looked)
>> doesn't really give you the answer you're looking for. The above checks
>> whether the person has been granted upload priviliges to the source
>> package directly, and not indirectly through a component or a
>> packageset. Which means that you have to call this function quite a few
>> times with different arguments to get your answer. This was at least
>> true for component permissions, but I didn't confirm how it handled
>> packagesets.
>
> Actually you only need to call it twice, once for the package and once for the
> component.  Packageset checks are done in the former.

Right, in the same way we implicitly check packagesets when querying
permission for a given package name, we could also check against the
source *current* component if we knew the context distroseries, no
need to callsites to call the canUpload() again.

Based on the discussion we had on #lp-dev yesterday we need to
consolidate all this logic in a single point, something like:

can_upload(user, archives, source_name, distroseries, pocket) -> Yes or None

  * check upload right to the pocket (archive, distroseries, pocket)
  * check upload rights on the source_name/packageset (archive, source_name)
  * figure out the component where the source is currently published
(archive, distroseries)
  * check upload rights on the component (archive, component)

This function can immediately replace NascentUpload.verify_acl() and
UploadPolicy.checkUpload() in archiveuploader/ if it respects some
specialties (checks for users with no permissions at all ate treated
differently).

I think refactoring the existing code in archiveuploader/ domain is
the easier and path to get what we need for package-branches without
introducing any, possibly unneeded, extra abstraction to the system.

We can think about benefiting of the check in the zope permission domain later.

-- 
Celso Providelo <celso.providelo@xxxxxxxxxxxxx>
IRC: cprov,  Jabber: cprov@xxxxxxxxxx, Skype: cprovidelo
1024D/681B6469 C858 2652 1A6E F6A6 037B  B3F7 9FF2 583E 681B 6469