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

[Martijn Faassen]

Hey,

Dan Korostelev wrote:
> One of most large packages that really wants to be refactored but
> still wasn't touched is zope.app.security. 

In fact we did touch it a bit, moving some bits from it into 
zope.security. And your zope.password work affected its dependencies of 
course. But I know what you mean. :)

> It has much in it and it
> brings many dependencies, including zope.app.form and company. And
> even some zope.* packages, like zope.securitypolicy still depend on
> it. So, let's finally refactor it :)

> Here's a sketch of a refactoring plan I wrote after taking a quick
> look at the current package:
> 
> - Move IAuthentication and other interfaces into new
> zope.authentication package. Also move there PrincipalSource and the
> "checkPrincipal" utility function. Also move there the PrincipalTerms
> class, however that will add dependency on zope.browser (which is
> really really tiny, as you may know).

Do you expect bits of zope.app.authentication could also move to 
zope.authentication or does that look implausible?

> - Move global principal registry, its IPrincipal/IGroup
> implementations and its directives into new zope.principalregistry
> package.

Why not name it zope.principal?

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

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

> - Leave all browser views, globalmodules.zcml, _protections.zcml,
> other zope.* permission definitions in zope.app.security as well as
> backward-compatibility imports.
> 
> - Just to note: the "settings" module was recently moved to
> zope.securitypolicy as there's the right place for it.
> 
> 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?

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

> - ILogoutSupported flag interface/adapter. Looks like it's only ever
> used for enabling/disabling the "logout" button in the UI. I'd
> deprecate it and leave in zope.app.security.
> 
> - _protections.py module. It defines a NoProxy checker for
> zope.i18nmessageid.Message and adds __name__ and __parent__ attributes
> to _available_by_default. This module was executed in
> zope.app.security.__init__ and generally does useful things for most
> of applications. The problem is that neither zope.i18nmessage, nor
> zope.location already depend on zope.security. One solution is to move
> the protections in that packages, placing the code into "try/except
> ImportError" block to avoid hard dependency.

That last solution seems reasonable. I think Christian Theune has had 
some dealings with a strategy like this during our dependency 
refactoring sprint; Christian?

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.

Regards,

Martijn