[z3-five] Security and Five

[Martijn Faassen]

Hey,

Thanks for all the work on Five; I'm really happy to see all this work 
on the security system. Looks like you've been making a lot of progress!

Some procedural matters first:

When I saw the first checkin, I was initially a somewhat shocked to see 
the 'content' directive used for protecting views; I see in recent 
checkins that has been amended; good, as it doesn't make much sense. :) 
I am also likely going to get rid of the new subpackages you added just 
to handle the content directive and merge them into the main modules.
Next time we introducing changes that introduce new packages like that, 
especially if there is already a different way, we should have some 
discussion on the list first, or alternatively check in as a branch and 
then bring it up (that's also good if tests are still lacking).

Anyway, I'll try to do a more comprehensive review of it all today and 
refactor some of the code where I think it's necessary. I'll also try to 
devise some more tests. :)

Sidnei da Silva wrote:
> I've checked in the work i've been doing on security decls in
> zcml. However, I've tried to use the five:content directive to protect
> views,

Which in my opinion is a definite misapplication of the five:content 
directive anyway; it should be applied to content. That said, it's nice 
to be able to do five:content on actual content classes now -- I hadn't 
imagined we'd be at that stage already.

> and found out that it doesn't quite work because when the page
> directive is used, a metaclass is created, and the security decls
> didnt took effect yet because they are processed later.
> 
> It seems to me that the right way to handle this is to have a
> <require> and <allow> subdirectives to the <five:page> directive
> instead. I don't remember from the top of my head how security in
> views is handled.

Whenever in doubt, study Zope 3. :)

the code is hiding out in zope.app.publisher.browser, much of it in 
viewcode.py. This time though it bounces us back to zope.app.component, 
as IBasicViewInformation is defined there, which is used by IPage.

What it defines that is relevant is the following:

     permission = Permission(
         title=u"Permission",
         description=u"The permission needed to use the view.",
         required=False
         )

Permission is in my mind the most important one; the actual view is the 
only thing on the view class that by default should be exposed to the 
outside world. Everything else should be private. Since in Zope 2 we 
don't have Zope 3's security model, attribute and template views run in 
trusted mode. It is very important to protect them right on the outside, 
but protecting anything else that doesn't get called by the web doesn't 
have any effect anyway. In which case, why bother? :)

Following that line of argument, the following two also defined on 
IBasicViewInformation aren't very useful and we should ignore them for now:

     allowed_interface = Tokens(
         title=u"Interface that is also allowed if user has permission.",
         description=u"""
         By default, 'permission' only applies to viewing the view and
         any possible sub views. By specifying this attribute, you can
         make the permission also apply to everything described in the
         supplied interface.

         Multiple interfaces can be provided, separated by
         whitespace.""",
         required=False,
         value_type=GlobalObject()
         )

     allowed_attributes = Tokens(
         title=u"View attributes that are also allowed if user has 
permission.",
         description=u"""
         By default, 'permission' only applies to viewing the view and
         any possible sub views. By specifying 'allowed_attributes',
         you can make the permission also apply to the extra attributes
         on the view object.""",
         required=False,
         value_type=PythonIdentifier()
         )

These only make sense for things that are indirectly called by either 
attribute or template-based page. This isn't going to happen anyway. 
This also explains why doing permission definition using five:content on 
content classes is of only limited utility, as views on that content, 
being trusted code, will break right through that anwyay.

So, the right way to go ahead is simple: just add the permission 
attribute back to IPage and make it work.

> If I am correct, doing security decls in python as it was done in
> FiveViewsDemo before my changes didnt work either. Martijn, can u confirm?

I haven't tested this, but I'll try that out. We really need unit tests 
for this. I don't really know how Zope 2 behaves with security and 
subclasses...

It may be easiest to the permission attribute work by doing an explicit 
check in Viewable.__bobo_traverse__. This would allow us to store the 
permission on the view in a different way than Zope 2's system mandates, 
and we could forgo messing around with classes.

> For content, the security decls in zcml should be working just fine as
> there's no metaclass involved.

Yeah, for content it looks nice; we're following the Zope 3 model of 
ZCML for content there, right?

The five: ZCML namespace shouldn't diverge from Zope 3's. Sometimes we 
may feel we want to create a new directive altogether, but all 
directives derived from Zope 3 should follow Zope 3 strictly. The only 
thing we can do to alter the schema is to do a subset instead, dropping 
some fields we cannot support.

Regards,

Martijn