[z3-five] Security and Five

[Sidnei da Silva]

On Mon, Jun 21, 2004 at 10:51:25AM +0200, Martijn Faassen wrote:
| 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!

Thank you! I appreciate that you appreciate it ;)

| 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).

Thats fine. I didn't knew of any policy about it so I've fallback to
the 'collective' policy -- get things done first, worry about beauty
later :)

| 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. :)

Feel free to do so!



| 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.

Yeah, I was surprised too when I've seen it working for the first time.

| >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. :)

That's what I did. At first I thought your views were different, but
then I realized they were pretty much the same as z3 and used the same
approach.

| 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? :)

That's right.

| 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.

Yeah, thats true. For views it doesn't matter much (currently), but
for direct access to the object it does matter. Remember if a method
has a docstring, it can be called, and I think that by default the
permission is public!

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

Thats sort of what I did.

| >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...

I've tested it myself, and the current way is working almost
perfectly. Content methods are being protected the way its expected,
and view *methods* too. views using *templates* are not being
protected the right way for some reason. If I try to check the
security for the 'index' attribute and its a ViewPageTemplateFile, it
raises Unauthorized.

| 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.

I've checked the security in BrowserView, and it works fine now.

| >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?

Yes. I've added the <deny> for declarePrivate, but I hope thats
fine. I was thinking of adding a couple others like setDefaultAccess()
and declareObjectProtected(permission). How do you feel about it?

| 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.

And adding fields is fine? :)

-- 
Sidnei da Silva <sidnei at awkly.org>
http://awkly.org - dreamcatching :: making your dreams come true
http://www.enfoldsystems.com
http://plone.org/about/team#dreamcatcher

Old mail has arrived.