[ckan-dev] proposed authz changes

[Friedrich Lindenberg]

Hi David,

On Thu, Apr 14, 2011 at 7:03 PM, David Read <david.read at okfn.org> wrote:
> 1. The first thing the is_authorized method is a hook to a plugin
> which can override. This can only ALLOW an action or FALLTHROUGH (rely
> on the authz table). Extend this to allow DENY, so the plugin can stop
> a user being authorized for someone.

I've thought about plugins in authz before but I hesitated: my feeling
is that authorization is one place where its really important to
enumerate the rules fairly naively. If you need to delegate
authorization, I'd much rather think about abstracting the queries
against the PackageRole table to run against something else.

> 2. is_authorized method currently takes (username, action, object)
> but for action=create_package, the object supplied is System, and for
> action=edit the object supplied is the package. Instead we should pass
> the proposed new object / change.
>
> Reason: an extension can then stop you creating a package with
> particular values inside it, e.g. an allowable department for this
> user

This can complement but not replace the existing mechanism. We will
still need a mechanism to check whether a user can see the creation
masks (and any links to it) at all. Also, with your use case, I'm
uncertain of whether it is actually worse to make the validation
system aware of users or to make the authz system aware of contents.
Since so far we've used the validation system to do all the horrible
stuff (tm) and it hasn't bitten us, I propose we stick with that. I
also think that the error you want to emit is a validation error and
not a 403 page.

> 3. Actions are so close to the actions in the logic layer, we should
> just merge them. 'package_create' is the same. 'update' becomes
> 'package_update', 'group_update' or 'package_relationship_update'
> depending on the object type.

Nice, but veeeery granular?

> 4. Rename these two classes to better reflect what they are
>  * AuthorizationGroup -> UserGroup
>  * Group -> PackageGroup

"Collection" would be my vote, its meaningful and I think camel-cased
names cannot go into the UI and we should match model and
presentation.

- Friedrich