Hi people! One of most large packages that really wants to be refactored but still wasn't touched is zope.app.security. 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). - Move global principal registry, its IPrincipal/IGroup implementations and its directives into new zope.principalregistry package. - Move LocalPermission into new zope.localpermission package. I personally didn't ever need local permissions. - 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. - 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. - 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. - 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. -- WBR, Dan Korostelev
Hi Dan
Betreff: [Zope-dev] Proposal: refactoring of zope.app.security
Hi people!
One of most large packages that really wants to be refactored but still wasn't touched is zope.app.security. 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).
Should we move the password "managers" registry and vocabulary to zope.authentication too?
- Move global principal registry, its IPrincipal/IGroup implementations and its directives into new zope.principalregistry package.
- Move LocalPermission into new zope.localpermission package. I personally didn't ever need local permissions.
- 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.
- 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.
- 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.
- 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.
That's an important feature. It could be really hard to nearly impossible to find out if an authentication provides logout or not without the ILogoutSupport adapter or a similar concept. We should think about how to replace that pattern if we skip the existing one. Regards Roger Ineichen
2009/3/11 Roger Ineichen <dev@projekt01.ch>:
Betreff: [Zope-dev] Proposal: refactoring of zope.app.security
- 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).
Should we move the password "managers" registry and vocabulary to zope.authentication too?
No, I think they need to be just moved back into zope.password. The zope.authentication is expected to be tiny package that contains only interface definitions and PrincipalSource.
- 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.
That's an important feature. It could be really hard to nearly impossible to find out if an authentication provides logout or not without the ILogoutSupport adapter or a similar concept.
We should think about how to replace that pattern if we skip the existing one.
Okay then. The interface will be moved into zope.authentication as well. :-) Thanks for comment on that. -- WBR, Dan Korostelev
Dan Korostelev wrote:
2009/3/11 Roger Ineichen <dev@projekt01.ch>:
Betreff: [Zope-dev] Proposal: refactoring of zope.app.security
- 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). Should we move the password "managers" registry and vocabulary to zope.authentication too?
No, I think they need to be just moved back into zope.password. The zope.authentication is expected to be tiny package that contains only interface definitions and PrincipalSource.
You would expect from the naming that bits of zope.app.authentication would end up in zope.authentication as well. I think that at least some of the bits in zope.app.authentication could be factored into something like zope.pluggableauth though. Regards, Martijn
2009/3/11 Martijn Faassen <faassen@startifact.com>:
Dan Korostelev wrote:
2009/3/11 Roger Ineichen <dev@projekt01.ch>:
Betreff: [Zope-dev] Proposal: refactoring of zope.app.security
- 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). Should we move the password "managers" registry and vocabulary to zope.authentication too?
No, I think they need to be just moved back into zope.password. The zope.authentication is expected to be tiny package that contains only interface definitions and PrincipalSource.
You would expect from the naming that bits of zope.app.authentication would end up in zope.authentication as well.
Yep, that can be expected from the naming, but really, zope.app.authentication is just one of implementations, like z3c.authenticator and we need a package that contains IAuthentication interface & co. that are currently contained in zope.app.security. I think that there's no better name for that package than zope.authentication.
I think that at least some of the bits in zope.app.authentication could be factored into something like zope.pluggableauth though.
I agree. However, may be the better name is zope.pluggableauthentication? Anyway, that can be done later. There's no need to touch zope.app.authentication to do zope.app.security refactorings, when zope.password is now extracted -- WBR, Dan Korostelev
Dan Korostelev wrote:
2009/3/11 Martijn Faassen <faassen@startifact.com>: [snip]
You would expect from the naming that bits of zope.app.authentication would end up in zope.authentication as well.
Yep, that can be expected from the naming, but really, zope.app.authentication is just one of implementations, like z3c.authenticator and we need a package that contains IAuthentication interface & co. that are currently contained in zope.app.security. I think that there's no better name for that package than zope.authentication.
Yes, I imagine it'll be more likely in some pluggableauthentication package in the end (without the ZMI bits).
I agree. However, may be the better name is zope.pluggableauthentication? Anyway, that can be done later. There's no need to touch zope.app.authentication to do zope.app.security refactorings, when zope.password is now extracted
Agreed that we can do this later. Let's do this in steps as much as we can anyway! Your proposed name is a bit long and therefore unwieldy, and pluggableauth is comprehensible enough, but I really don't care that much about it. Anyway, later. :) Regards, Martijn
2009/3/11 Dan Korostelev <nadako@gmail.com>:
Hi people!
One of most large packages that really wants to be refactored but still wasn't touched is zope.app.security. 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).
- Move global principal registry, its IPrincipal/IGroup implementations and its directives into new zope.principalregistry package.
- Move LocalPermission into new zope.localpermission package. I personally didn't ever need local permissions.
- 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.
- 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.
- 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.
- 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.
Anyone? If there's no more opinions/objections/suggestions, I'd start refactoring. -- WBR, Dan Korostelev
Hey, Dan Korostelev wrote:
If there's no more opinions/objections/suggestions, I'd start refactoring.
See my replies just now. I'm in favor of starting the refactoring work, as long as you do it step by step. Regards, Martijn
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
2009/3/11 Martijn Faassen <faassen@startifact.com>:
- 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?
No, I don't think that anything from zope.app.authentication should go to zope.authentication. It probably should go to something like zope.pluggableauthentication. I expect zope.authentication to be tiny package that only define interfaces and very basic utilities.
- 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.
- 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.
- 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.
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.
- 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.
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. -- WBR, Dan Korostelev
Dan Korostelev wrote:
2009/3/11 Martijn Faassen <faassen@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
2009/3/11 Martijn Faassen <faassen@startifact.com>:
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.
I'll add a short README.txt to each package. The zope.principalregistry is an non-persistent implementation of IAuthentication that allows us to define principals via zcml (the "principal" directive and company) or with simple python calls. It's also quite useful for tests.
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).
Yeah, that's a bit from "TTW development" that is not used much anymore, I want believe. ;-)
- 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.
Yeah, it's the "class" directive that was moved to zope.security on that sprint.
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?
Yep, I think that this refactoring will one of the last, so let's wait with that indeed.
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?
No. The IAuthentication is meant to work with any request object, even not zope.publisher's one, like WebOb, or something. The only bits in current zope.app.security that is tied to zope.publisher is the implementations if ILoginPassword for zope.publisher's HTTP and FTP requests. BTW, the only thing that uses ILoginPassword is the "global principal registry", but the interface is very useful in general.
A short description of what "zope.authentication" is for would again be good, I think.
It's basically the interface-only package that defines IAuthentication, PrincipalLookupError, IUnauthenticatedPrincipal, and so on, as well as little helper things, like the "checkPrincipal" function and the PrincipalSource/PrincipalTerms. It's intented to be used by any package that doesn't depend on particular authentication implementation, like zope.securitypolicy and others.
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).
Okay, I'll mail with a little description of work done for each step.
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.
I described them above. But I'll remind about them in any mail that I'll post about this refactoring process. Also, when I'll be done, I'll write an general overview of new and changed packages (and after that we could also add it to 34to35.rst). -- WBR, Dan Korostelev
On Wed, Mar 11, 2009 at 1:52 PM, Dan Korostelev <nadako@gmail.com> wrote:
2009/3/11 Martijn Faassen <faassen@startifact.com>:
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.
I'll add a short README.txt to each package. The zope.principalregistry is an non-persistent implementation of IAuthentication that allows us to define principals via zcml (the "principal" directive and company) or with simple python calls. It's also quite useful for tests.
<bikeshed> Maybe we can use the "description" or "long_description" field in the setup.py for the "package mission statement" </bikeshed> -- Benji York Senior Software Engineer Zope Corporation
2009/3/12 Benji York <benji@zope.com>:
On Wed, Mar 11, 2009 at 1:52 PM, Dan Korostelev <nadako@gmail.com> wrote:
2009/3/11 Martijn Faassen <faassen@startifact.com>:
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.
I'll add a short README.txt to each package. The zope.principalregistry is an non-persistent implementation of IAuthentication that allows us to define principals via zcml (the "principal" directive and company) or with simple python calls. It's also quite useful for tests.
<bikeshed> Maybe we can use the "description" or "long_description" field in the setup.py for the "package mission statement" </bikeshed>
Of course there will be description and long_description. Moreover, the long_description in most cases will start with contents of the README.txt, like it's done in most of packages already. :) -- WBR, Dan Korostelev
Benji York wrote:
On Wed, Mar 11, 2009 at 1:52 PM, Dan Korostelev <nadako@gmail.com> wrote:
2009/3/11 Martijn Faassen <faassen@startifact.com>:
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. I'll add a short README.txt to each package. The zope.principalregistry is an non-persistent implementation of IAuthentication that allows us to define principals via zcml (the "principal" directive and company) or with simple python calls. It's also quite useful for tests.
<bikeshed> Maybe we can use the "description" or "long_description" field in the setup.py for the "package mission statement" </bikeshed>
Well, eventually this will presumably be part of the README.txt which typically ends up in the long_description. But if no such README.txt is there, sure. Regards, Martijn
Hey, Dan Korostelev wrote:
The zope.principalregistry is an non-persistent implementation of IAuthentication that allows us to define principals via zcml (the "principal" directive and company) or with simple python calls. It's also quite useful for tests.
It defines the registry and the ZCML directives to fill the registry both, right? I mean, we could think about calling it zope.principalzcml but if the registry is there I imagine it's indeed better to make it zope.principalregistry. Regards, Martijn
2009/3/12 Martijn Faassen <faassen@startifact.com>:
Hey,
Dan Korostelev wrote:
The zope.principalregistry is an non-persistent implementation of IAuthentication that allows us to define principals via zcml (the "principal" directive and company) or with simple python calls. It's also quite useful for tests.
It defines the registry and the ZCML directives to fill the registry both, right? I mean, we could think about calling it zope.principalzcml but if the registry is there I imagine it's indeed better to make it zope.principalregistry.
Registry is there as well, so it's a fully-functional non-persistent implementation of IAuthentication utility. -- WBR, Dan Korostelev
On Wed, 2009-03-11 at 15:42 +0100, Martijn Faassen wrote:
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?
Sorry, I've stared at the issue for a while but can't remember that I had (something like) that during the sprint. -- Christian Theune · ct@gocept.com gocept gmbh & co. kg · forsterstraße 29 · 06112 halle (saale) · germany http://gocept.com · tel +49 345 1229889 7 · fax +49 345 1229889 1 Zope and Plone consulting and development
2009/3/20 Christian Theune <ct@gocept.com>:
- _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?
Sorry, I've stared at the issue for a while but can't remember that I had (something like) that during the sprint.
It's nevermind now :) I just added those protections to zope.security itself. -- WBR, Dan Korostelev
participants (5)
-
Benji York -
Christian Theune -
Dan Korostelev -
Martijn Faassen -
Roger Ineichen