launchpad-dev team mailing list archive

[Henning Eggers <henning.eggers@xxxxxxxxxxxxx>]

Am 05.02.2010 15:04, Guilherme Salgado schrieb:
> On Thu, 2010-02-04 at 18:37 +0100, Henning Eggers wrote:
>>  * Split permission checking up into chunks for each Launchpad
>>    application. At the same time, general policies need to be available
>>    to all parts.
> 
> If we split that file, I'd rather see the code for permission checking
> as close as possible to the pertinent model class instead of having
> per-app security.py modules, as you seem to suggest above.

I am all for that. Splitting up on app level is only the first step to
reduce this massive blob.

> 
>>  * Have a canonical way to use the same checks in model code as in view
>>    code.
> 
> I guess this is because the existing api (check_permission()) doesn't
> take a user as argument?

Right, it assumes the user of the current query. Adding that parameter
would indeed change things.

> 
>> = Possible solutions =
>> I don't have all the answers but these are some of my ideas.
>>
>>  * Make model objects be their own authorization adapter. Why should
>>    it be too much to ask of an object, if a user may edit or admin it?
>>    The implementation can be in the class itself but may just as well
>>    be provided by a mix-in.
> 
> I don't think this is the responsibility of model classes, but it might
> be a good idea to have a single adapter (for every model class) that
> knows about all different permissions on that class.

Yes, good one, too. We have two reasons why model classes have to be
concerned with security. Maybe if we fix those, we can keep the model
classes free of security code.

1. The LP API exposes model classes directly to the web, leaving only
   the Zope security declaration in ZCML as protection (no view).
2. But the Zope security declarations only restrict attribute access.
   Restricting parameter values (like who can set which status) needs
   to be implemented in the model class.

Also, I have noticed that with this we are mixing authorization and
process constraints. E.g. TranslationImportQueueEntry.canSetStatus
allows admins to set the "Approved" status *but* only if an import
target has been set. That second condition is not really an
authorization question and should probably be enforced at a different level.

> 
>>  * Provide helpful system-wide mix-ins that implement common security
>>    policies, e.g. for owners, admins, etc.
>>
>>  * Design these mix-ins in a layered way, so that permissions can simply
>>    be added up and automatically include permissions from more
>>    restrictive policies.
>>
>>  * Provide a gradual transition path. If an object does not provide its
>>    own authorization information, look in the old security.py for it.
> 
> These can be done with the single-adapter model as well.

Certainly. ;-)

Thank you for your input.

Henning