[Zope-dev] Proposal: refactoring of zope.app.security

[Martijn Faassen]

Dan Korostelev wrote:
> 2009/3/11 Martijn Faassen <faassen at startifact.com>:
[snip]
>>> - Move global principal registry, its IPrincipal/IGroup
>>> implementations and its directives into new zope.principalregistry
>>> package.
>> Why not name it zope.principal?
> 
> It sounds confusing to me. Also, the global principal registry is just
> an (one of) implementation of IAuthentication, while zope.principal
> sounds like something from the "core" of principals.

Ah, I understand now that IPrincipal itself would be hanging out in 
another package?

You should write up a short description of what zope.principalregistry 
does. In fact whenever we propose a new package we should describe its 
"mission statement" in just a couple of lines. That'll help us think 
about it better.

>>> - Move LocalPermission into new zope.localpermission package. I
>>> personally didn't ever need local permissions.
>> You're talking about locally defined permissions, correct, not about
>> giving someone a permission locally?
> 
> I'm talking about local persistent permissions that can be added to
> ZODB and registered per site. It's zope.securitypolicy that gives
> local permissions, so, LocalPermission has nothing to do with local
> grants.

Right, that's what I thought. Agreed they should go off into a package 
on its own, probably to eventually die. :) (I have bad memories of 
struggling with this stuff in a sprint in 2003).

>>> - Rewrite PermissionsVocabulary and PermissionIdsVocabulary not to
>>> depend on zope.app.component and move them into zope.security. It's
>>> generally useful there and won't introduce any new dependencies.
>>>
>>> - Move zcml definition of zope.Public permission. Maybe move security
>>> declaration for the `zope.security.permission.Permission` class as
>>> well.
>> Move them where?
> 
> Oh, that's a typo. Move them to zope.security.

+1

The security declaration I assume can move without introducing 
dependencies on ZCML directives not in zope.security? I think we 
actually moved them in there during the sprint already, if I'm thinking 
about the right directives.

>>> Not sure about:
>>>
>>> - ILoginPassword and its basic implementations. The interface should
>>> probably go into zope.authentication while implementations - into
>>> zope.publisher. It will add a dependency on zope.authentication to
>>> zope.publisher, but the zope.authentication are expected to be really
>>> tiny and already installed for most applications, so I believe that
>>> it's okay.
>> Why do you think the implementations of ILoginPassword should go into
>> zope.publisher? Why not simply into zope.authentication?
> 
> Because that implementations are zope.publisher specific, while
> zope.authentication as I see it could be used with any request objects
> and that way it could not depend on zope.publisher.

It really seems odd we're suddenly introducing a new dependency for 
zope.publisher that wasn't there before. You'd think if that dependency 
isn't necessary now it shouldn't be necessary in the futur either. Could 
you delay this refactoring to later in the process at least and bring it 
up again separately when you get to it?

Anyway, how theoretical is it that zope.authentication could be used 
with "any request objects"? Does zope.authentication touch the request 
API in your mind at all? Refer to IBrowserRequest?

A short description of what "zope.authentication" is for would again be 
good, I think.

>>> - PrincipalLogging - the adapter from
>>> zope.security.interfaces.IPrincipal to
>>> zope.publisher.interfaces.ILoggingInfo. I'd just move it into
>>> zope.publisher, because it's already tied to zope.security.
>> Wouldn't zope.publisher then need to depend on zope.principalregistry
>> for the IPrincipal interface?
> 
> No. IPrincipal is defined in zope.security.

+1 to this move then.

>> In general, I'd be careful to execute each of these as a separate step,
>> and not try to do them all at once. It's quite possible that while
>> you're moving stuff around (including tests) that you'll suddenly think
>> of a better place, so better treat this on a case by case basis when you
>> actually start.
> 
> Yeah, I was already planning to that step by step.

To avoid overwhelming people with a huge amount of information in one 
mail, I suggest you post to the list for each step. I think this 
overview posting is good, and we should keep it up, but it's also hard 
to judge things individually and a lot to digest, so we should also 
discuss the steps (either before you do it if there are still 
uncertainties, or after you do it).

I also would like you to write a couple of sentences describing 
zope.authentication, zope.principalregistry and any other new package 
you are thinking of creating. More or less something that could go into 
a README.txt in the introduction, or that could go into the description 
field in setup.py. Something sort so people know what these packages are 
for and can start reasoning about them.

Regards,

Martijn