[Framework-Team] Re: PLIP 48 review notes

2006-09-15 Thread Simon Eisenmann
Hi Folks,

thanks for reviewing the bundle. I have a couple of remarks.

SessionCrumbler was not written for this. Instead its a well maintained
product which is in use some hundred production systems worldwide. Are
you sure you want a seperate (fork) of the SessionCrumbler codebase just
for layout reasons?

There is a thing called sessionlazycheck inside the SessionCrumbler
product. This is a tiny python module which allows code to check if
there is a session without actually creating one. This needs to be used 
anywhere inside the system where it might be required to get session
content (i have no idea if there are such places atm, but in the past
there have been a lot).

Really the two issues are non issues cause they are just documentation
issues. But i aggree that this change to plone needs to be known to the
CacheFu Devs.

Some other remark onto sessions. Imho we have to make sure that no
session is created for plone anoymous access. Regarding this would mean
only request without session are cacheable. As soon as a request has a
session it might be personalized and is not cacheable without further
consideration (at least this should be the default for html pages,
images (most important skin stuff) should be cached anytime). 

Best regards,
Simon


Am Dienstag, den 12.09.2006, 22:16 +0200 schrieb Wichert Akkerman:
> Changes from stock Plone
> 
> This bundle features two modifications from standard Plone:
> 
>  * adds the new SessionCrumbler product from collective
>  * Uses a modified PlonePAS
> 
> 
> PlonePAS modifications
> --
> 
> The PlonePAS modifications are restricted to deactivating the
> standard cookie authentication handler and installing the new
> credentials_session_auth plugin.
> 
> SessionCrumbler product
> ---
> 
> One obvious remark is that this product does not comply with the current
> preferred layout: no seperate module for interfaces, and no plugins
> subdirectory - instead everything is put in a the top directory.
> 
> There is a lot of code to support non-PAS sites. Creating a new branch
> to remove all the legacy code, cleaning up the product structure and
> making a PAS-only release 
> 
> The PAS code itself is not complex and implements all require PAS interfaces
> correctly.
> 
> 
> Issues
> ==
> 
> The session cookie is stored in the _ZopeId cookie. This is not compatible
> with CacheFu, which checks only check for __ac cookies and the Authorization
> header. Since sessions are always a per-user thing this should be fixed in
> CacheFu (specifically its squid configuration).
> 
> Using sessions means that ZEO clusters will not work out of the box: the
> session storage is not shared between ZEO clients. This needs to be explicitly
> documented in release notes.
> 
> 


___
Framework-Team mailing list
Framework-Team@lists.plone.org
http://lists.plone.org/mailman/listinfo/framework-team


Re: [Framework-Team] Re: PLIP 48 review notes

2006-09-12 Thread Alec Mitchell

On 9/12/06, Martin Aspeli <[EMAIL PROTECTED]> wrote:

Wichert Akkerman wrote:

> I don't see any objections to merging it. The security benefits
> certainly make it a good idea.

+1 from me as well - I've needed this in the past, and I don't think
people quite understand the implications of SessionCrumbler, which is of
course the default now.

We need to get a good story together on the ZEO clustered session
storage though.


We need to make sure that it can be easily disabled in case the
performance overhead of using Sessions is too much for some
applications.

Alec

___
Framework-Team mailing list
Framework-Team@lists.plone.org
http://lists.plone.org/mailman/listinfo/framework-team


[Framework-Team] Re: PLIP 48 review notes

2006-09-12 Thread Martin Aspeli

Wichert Akkerman wrote:


I don't see any objections to merging it. The security benefits
certainly make it a good idea.


+1 from me as well - I've needed this in the past, and I don't think 
people quite understand the implications of SessionCrumbler, which is of 
course the default now.


We need to get a good story together on the ZEO clustered session 
storage though.


Martin


___
Framework-Team mailing list
Framework-Team@lists.plone.org
http://lists.plone.org/mailman/listinfo/framework-team


Re: [Framework-Team] Re: PLIP 48 review notes

2006-09-12 Thread Wichert Akkerman
Previously Martin Aspeli wrote:
> Wichert Akkerman wrote:
> >Changes from stock Plone
> >
> >This bundle features two modifications from standard Plone:
> >
> > * adds the new SessionCrumbler product from collective
> > * Uses a modified PlonePAS
> 
> Who's responsible for this bundle? Are they willing to do some of the 
> work you've mentioned below?

Simon is. I've cc'ed him on that mail, so hopefully he'll comment on it.

> >PlonePAS modifications
> >--
> >
> >The PlonePAS modifications are restricted to deactivating the
> >standard cookie authentication handler and installing the new
> >credentials_session_auth plugin.
> >
> >SessionCrumbler product
> >---
> >
> >One obvious remark is that this product does not comply with the current
> >preferred layout: no seperate module for interfaces, and no plugins
> >subdirectory - instead everything is put in a the top directory.
> >
> >There is a lot of code to support non-PAS sites. Creating a new branch
> >to remove all the legacy code, cleaning up the product structure and
> >making a PAS-only release  
> 
> ... ?
> 
> I assume you suggest we should do this?

It's not needed, but I would certainly prefer it.

> >The PAS code itself is not complex and implements all require PAS 
> >interfaces
> >correctly.
> >
> >
> >Issues
> >==
> >
> >The session cookie is stored in the _ZopeId cookie. This is not compatible
> >with CacheFu, which checks only check for __ac cookies and the 
> >Authorization
> >header. Since sessions are always a per-user thing this should be fixed in
> >CacheFu (specifically its squid configuration).
> 
> Any idea how hard that'll be? The CacheFu situation is a bit perilous atm.

It's quite trivial luckily; shouldn't be more than 5 minutes of work.

> >Using sessions means that ZEO clusters will not work out of the box: the
> >session storage is not shared between ZEO clients. This needs to be 
> >explicitly
> >documented in release notes.
> 
> Moreover, it needs to be documented how this can be *fixed* (i.e. how to 
> correctly configure ZEO) and/or how to re-enable cookie crumbler.

Indeed.

> What's your overall recommendation?

I don't see any objections to merging it. The security benefits
certainly make it a good idea.

Wichert.

-- 
Wichert Akkerman <[EMAIL PROTECTED]>It is simple to make things.
http://www.wiggy.net/   It is hard to make things simple.

___
Framework-Team mailing list
Framework-Team@lists.plone.org
http://lists.plone.org/mailman/listinfo/framework-team


[Framework-Team] Re: PLIP 48 review notes

2006-09-12 Thread Martin Aspeli

Wichert Akkerman wrote:

Changes from stock Plone

This bundle features two modifications from standard Plone:

 * adds the new SessionCrumbler product from collective
 * Uses a modified PlonePAS


Who's responsible for this bundle? Are they willing to do some of the 
work you've mentioned below?



PlonePAS modifications
--

The PlonePAS modifications are restricted to deactivating the
standard cookie authentication handler and installing the new
credentials_session_auth plugin.

SessionCrumbler product
---

One obvious remark is that this product does not comply with the current
preferred layout: no seperate module for interfaces, and no plugins
subdirectory - instead everything is put in a the top directory.

There is a lot of code to support non-PAS sites. Creating a new branch
to remove all the legacy code, cleaning up the product structure and
making a PAS-only release  


... ?

I assume you suggest we should do this?


The PAS code itself is not complex and implements all require PAS interfaces
correctly.


Issues
==

The session cookie is stored in the _ZopeId cookie. This is not compatible
with CacheFu, which checks only check for __ac cookies and the Authorization
header. Since sessions are always a per-user thing this should be fixed in
CacheFu (specifically its squid configuration).


Any idea how hard that'll be? The CacheFu situation is a bit perilous atm.


Using sessions means that ZEO clusters will not work out of the box: the
session storage is not shared between ZEO clients. This needs to be explicitly
documented in release notes.


Moreover, it needs to be documented how this can be *fixed* (i.e. how to 
correctly configure ZEO) and/or how to re-enable cookie crumbler.


What's your overall recommendation?

Thanks!

Martin


___
Framework-Team mailing list
Framework-Team@lists.plone.org
http://lists.plone.org/mailman/listinfo/framework-team