[Zope-CMF] [dev] Unauthorized handling - a proposal
Hi! Current situation: - By default Zope publishes Unauthorized exceptions as HTTP/1.1 401 Unauthorized responses including a basic authentication challenge. - If the user is not logged in, CMF converts Unauthorized exceptions into redirects. The redirect sends them to the login form and has a came_from= in the query string. - If the user is already logged in, the default Zope behavior is used. Or the request is redirected to the unauth_page if specified. I don't know if anybody is using the unauth_page feature. I think a good default behavior would be to return HTTP/1.1 403 Forbidden responses for authenticated users without enough privileges. - The Unauthorized handling is currently done by the CookieCrumbler. It hooks into the error handling process by overriding some methods of the HTTPResponse objects. Internal Zope changes did partially break this in Zope 2.12.5, there is no guarantee the hooks will work in future. Proposal: Meanwhile a much better hook exists for exception handling: Exception views. I propose to move most of the Unauthorized handling to a new exception view in the ICMFDefaultSkin layer. All Unauthorized exceptions inside a CMF site are converted by the view. Into a Redirect exception for anonymous users and into a Forbidden exception for authenticated users. The redirect target is looked up in the 'user/login' Action, making CookieCrumbler's auto_login_page setting obsolete. The unauth_page setting will no longer be supported. CookieCrumbler and therefore CMFCore will loose the redirect feature. If there are no objections, I'll check in that change on CMF trunk. Cheers, Yuppie ___ Zope-CMF maillist - Zope-CMF@zope.org https://mail.zope.org/mailman/listinfo/zope-cmf See https://bugs.launchpad.net/zope-cmf/ for bug reports and feature requests
Re: [Zope-CMF] [dev] Unauthorized handling - a proposal
Am 20.04.2010, 12:53 Uhr, schrieb yuppie y.2...@wcm-solutions.de: Proposal: Meanwhile a much better hook exists for exception handling: Exception views. I propose to move most of the Unauthorized handling to a new exception view in the ICMFDefaultSkin layer. Please add a separate folder for these views. We need to separate them from the content ones. BTW. (For pedagogic reason I'm not too keen on EditForms being used when objects aren't being edited.) All Unauthorized exceptions inside a CMF site are converted by the view. Into a Redirect exception for anonymous users and into a Forbidden exception for authenticated users. The redirect target is looked up in the 'user/login' Action, making CookieCrumbler's auto_login_page setting obsolete. The unauth_page setting will no longer be supported. CookieCrumbler and therefore CMFCore will loose the redirect feature. If there are no objections, I'll check in that change on CMF trunk. This is great. Having looked at the CookieCrumbler code, and its change log, it's that it hasn't seen much love in the last five years, while Zope and the rest of the CMF have come along in leaps and bounds. So +1 from me but as per my other e-mails - what will the CookieCrumbler do afterwards? Using a view means that there is a hardcoded relation between the login form and the login cookies. This is the current interface for ICookieCrumbler with proposed deprecation decorators class ICookieCrumbler(Interface): Reads cookies during traversal and simulates the HTTP auth headers. @deprecate auth_cookie = Attribute(The key of the authorisation cookie) @deprecate name_cookie = Attribute(They key of the authorised user cookie) @deprecate pw_cookie = Attribute(The key of the password cookie) persist_cookie = Attribute(The key of the persistent cookie) local_cookie_path = Attribute(If True, the cookie tied to the local path?) cache_header_value = Attribute(If present, the login page will not be cached) log_username = Attribute(If True, the username will in appear in Zope's log) def delRequestVar(req, name): No errors of any sort may propagate, and we don't care *what* they are, even to log them. def getCookiePath(): Get the path for the cookie the parent URL if local_cookie_path is True otherwise / return path @deprecate def getCookieMethod(name, default=None): Allow overridable cookie set/expiration methods. return getattr(name, default) def defaultSetAuthCookie(resp, cookie_name, cookie_value): Set the authorisation cookie def defaultExpireAuthCookie(resp, cookie_name): Expire the cookie def _setAuthHeader(ac, request, response): Set the auth headers for both the Zope and Medusa http request objects. @deprecate def modifyRequest(req, resp): Copies cookie-supplied credentials to the basic auth fields. Returns a flag indicating what the user is trying to do with cookies: ATTEMPT_NONE, ATTEMPT_LOGIN, or ATTEMPT_RESUME. If cookie login is disabled for this request, raises CookieCrumblerDisabled. def __call__(container, req): The __before_publishing_traverse__ hook. @deprecate def credentialsChanged(user, name, pw): # XXX: this method violates the rules for tools/utilities: # it depends on self.REQUEST @deprecate def _cleanupResponse(): # XXX: this method violates the rules for tools/utilities: # it depends on self.REQUEST @deprecate def unauthorized(): Remove authentication cookies and redirect to standard unauthorized @deprecate def _unauthorized(): Remove authentication cookies and redirect to standard _unauthorized @deprecate def getUnauthorizedURL(): Redirects to the login page. @deprecate def logout(): Logs out the user and redirects to the logout page. def propertyLabel(id): Return a label for the given property id I'm more than happy to help with this if there is anything I can do that doesn't mean you spend more time answering my questions than it would take to do the work! :-) Charlie -- Charlie Clark Managing Director Clark Consulting Research German Office Helmholtzstr. 20 Düsseldorf D- 40215 Tel: +49-211-600-3657 Mobile: +49-178-782-6226 ___ Zope-CMF maillist - Zope-CMF@zope.org https://mail.zope.org/mailman/listinfo/zope-cmf See https://bugs.launchpad.net/zope-cmf/ for bug reports and feature requests
Re: [Zope-CMF] [dev] Unauthorized handling - a proposal
Hi! Charlie Clark wrote: Please add a separate folder for these views. We need to separate them from the content ones. Why? 'browser' is a generic name, that folder is usually used for *all* browser views. Which folder names would you propose? BTW. (For pedagogic reason I'm not too keen on EditForms being used when objects aren't being edited.) Why? Is this about names or implementation? EditFormBase is for all kinds of editable forms, ContentEditFormBase for content objects. So +1 from me but as per my other e-mails - what will the CookieCrumbler do afterwards? Using a view means that there is a hardcoded relation between the login form and the login cookies. That doesn't have to be hardcoded. We could look up the cookie settings in the CookieCrumbler. This is the current interface for ICookieCrumbler with proposed deprecation decorators class ICookieCrumbler(Interface): Reads cookies during traversal and simulates the HTTP auth headers. @deprecate auth_cookie = Attribute(The key of the authorisation cookie) @deprecate name_cookie = Attribute(They key of the authorised user cookie) @deprecate pw_cookie = Attribute(The key of the password cookie) Why deprecated? We can keep these 3 configurable and look them up. persist_cookie = Attribute(The key of the persistent cookie) local_cookie_path = Attribute(If True, the cookie tied to the local path?) cache_header_value = Attribute(If present, the login page will not be cached) log_username = Attribute(If True, the username will in appear in Zope's log) def delRequestVar(req, name): No errors of any sort may propagate, and we don't care *what* they are, even to log them. def getCookiePath(): Get the path for the cookie the parent URL if local_cookie_path is True otherwise / return path @deprecate def getCookieMethod(name, default=None): Allow overridable cookie set/expiration methods. return getattr(name, default) Why deprecated? def defaultSetAuthCookie(resp, cookie_name, cookie_value): Set the authorisation cookie def defaultExpireAuthCookie(resp, cookie_name): Expire the cookie def _setAuthHeader(ac, request, response): Set the auth headers for both the Zope and Medusa http request objects. @deprecate def modifyRequest(req, resp): Copies cookie-supplied credentials to the basic auth fields. Returns a flag indicating what the user is trying to do with cookies: ATTEMPT_NONE, ATTEMPT_LOGIN, or ATTEMPT_RESUME. If cookie login is disabled for this request, raises CookieCrumblerDisabled. Why deprecated? def __call__(container, req): The __before_publishing_traverse__ hook. @deprecate def credentialsChanged(user, name, pw): # XXX: this method violates the rules for tools/utilities: # it depends on self.REQUEST Why deprecated? @deprecate def _cleanupResponse(): # XXX: this method violates the rules for tools/utilities: # it depends on self.REQUEST +1 for deprecating (or maybe just removing) it @deprecate def unauthorized(): Remove authentication cookies and redirect to standard unauthorized +1 for deprecating (or maybe just removing) it @deprecate def _unauthorized(): Remove authentication cookies and redirect to standard _unauthorized +1 for deprecating (or maybe just removing) it @deprecate def getUnauthorizedURL(): Redirects to the login page. +1 for deprecating (or maybe just removing) it @deprecate def logout(): Logs out the user and redirects to the logout page. Why deprecated? def propertyLabel(id): Return a label for the given property id I'm more than happy to help with this if there is anything I can do that doesn't mean you spend more time answering my questions than it would take to do the work! :-) I'm only working on the login process. Could you perhaps start working on the logout process before we bring everything together? Cheers, Yuppie ___ Zope-CMF maillist - Zope-CMF@zope.org https://mail.zope.org/mailman/listinfo/zope-cmf See https://bugs.launchpad.net/zope-cmf/ for bug reports and feature requests
Re: [Zope-CMF] [dev] Unauthorized handling - a proposal
Am 20.04.2010, 14:19 Uhr, schrieb yuppie y.2...@wcm-solutions.de: Hi! Charlie Clark wrote: Please add a separate folder for these views. We need to separate them from the content ones. Why? 'browser' is a generic name, that folder is usually used for *all* browser views. Which folder names would you propose? It makes it more manageable. Anyone looking at this for the first time is likely to get a shock when trying to work out which views are where. I'd like to have separate folders for content, login or authentication, workflow, etc. BTW. (For pedagogic reason I'm not too keen on EditForms being used when objects aren't being edited.) Why? Is this about names or implementation? EditFormBase is for all kinds of editable forms, ContentEditFormBase for content objects. It's just about names. Providing consistency with the formlib heritage - granted PageForm is not going to win any prizes for intuitive but I think it's better than EditFormBase vs. ContentEditFormBase. I've always associated ContentEditFormBase with IContentish So +1 from me but as per my other e-mails - what will the CookieCrumbler do afterwards? Using a view means that there is a hardcoded relation between the login form and the login cookies. That doesn't have to be hardcoded. We could look up the cookie settings in the CookieCrumbler. Would there be any advantage in this? CookieCrumbler doesn't seem to need them itself if the authentication is handled in a view. This is the current interface for ICookieCrumbler with proposed deprecation decorators class ICookieCrumbler(Interface): Reads cookies during traversal and simulates the HTTP auth headers. @deprecate auth_cookie = Attribute(The key of the authorisation cookie) @deprecate name_cookie = Attribute(They key of the authorised user cookie) @deprecate pw_cookie = Attribute(The key of the password cookie) Why deprecated? We can keep these 3 configurable and look them up. Sure, keep them if there is a real benefit in the context of the CMF for this. persist_cookie = Attribute(The key of the persistent cookie) local_cookie_path = Attribute(If True, the cookie tied to the local path?) cache_header_value = Attribute(If present, the login page will not be cached) log_username = Attribute(If True, the username will in appear in Zope's log) def delRequestVar(req, name): No errors of any sort may propagate, and we don't care *what* they are, even to log them. def getCookiePath(): Get the path for the cookie the parent URL if local_cookie_path is True otherwise / return path @deprecate def getCookieMethod(name, default=None): Allow overridable cookie set/expiration methods. return getattr(name, default) Why deprecated? I'm assuming that this related to Methods that could be stored within the CookieCrumbler when it was still a folder. def defaultSetAuthCookie(resp, cookie_name, cookie_value): Set the authorisation cookie def defaultExpireAuthCookie(resp, cookie_name): Expire the cookie def _setAuthHeader(ac, request, response): Set the auth headers for both the Zope and Medusa http request objects. @deprecate def modifyRequest(req, resp): Copies cookie-supplied credentials to the basic auth fields. Returns a flag indicating what the user is trying to do with cookies: ATTEMPT_NONE, ATTEMPT_LOGIN, or ATTEMPT_RESUME. If cookie login is disabled for this request, raises CookieCrumblerDisabled. Why deprecated? I'm assuming that the login happens outside of CookieCrumbler. Please correct me if I'm wrong on this. def __call__(container, req): The __before_publishing_traverse__ hook. @deprecate def credentialsChanged(user, name, pw): # XXX: this method violates the rules for tools/utilities: # it depends on self.REQUEST Why deprecated? Look at the docstring. @deprecate def _cleanupResponse(): # XXX: this method violates the rules for tools/utilities: # it depends on self.REQUEST +1 for deprecating (or maybe just removing) it @deprecate def unauthorized(): Remove authentication cookies and redirect to standard unauthorized +1 for deprecating (or maybe just removing) it Let's bin the methods you are replacing. @deprecate def _unauthorized(): Remove authentication cookies and redirect to standard _unauthorized +1 for deprecating (or maybe just removing) it @deprecate def getUnauthorizedURL(): Redirects to the login page. +1 for deprecating (or maybe just removing) it
Re: [Zope-CMF] [dev] Unauthorized handling - a proposal
Am 20.04.2010, 16:21 Uhr, schrieb yuppie y.2...@wcm-solutions.de:
You mean subfolders of browser? At the top level I would expect more
than just the browser views inside folders with these names.
Well, yes. Best practice would be document/browser, etc. but I think it's
pragmatic to do something similar with the browser views that we already
have with default skin: zpt-content, zpt-generic. I assume any suggestion
is likely to be greeted by a collective shrug from everyone else,
For now I'll keep it where it is. If we agree on a new structure, things
can be easily moved around.
Indeed.
Using a view means that there is a hardcoded relation between the
login
form and the login cookies.
That doesn't have to be hardcoded. We could look up the cookie settings
in the CookieCrumbler.
Would there be any advantage in this? CookieCrumbler doesn't seem to
need
them itself if the authentication is handled in a view.
For now I plan to move less out of CookieCrumbler than you seem to
expect.
I'm guilty of quite a few assumptions which is why I created the interface
and dropped in here for discussion.
@deprecate
def getCookieMethod(name, default=None):
Allow overridable cookie set/expiration methods.
return getattr(name, default)
Why deprecated?
I'm assuming that this related to Methods that could be stored within
the
CookieCrumbler when it was still a folder.
Could be. +1 if you are sure it is useless.
I'm not sure on any of this. CookieCrumbler isn't clear on many things.
@deprecate
def modifyRequest(req, resp):
Copies cookie-supplied credentials to the basic auth
fields.
Returns a flag indicating what the user is trying to do
with
cookies: ATTEMPT_NONE, ATTEMPT_LOGIN, or ATTEMPT_RESUME.
If
cookie login is disabled for this request, raises
CookieCrumblerDisabled.
Why deprecated?
I'm assuming that the login happens outside of CookieCrumbler. Please
correct me if I'm wrong on this.
For now I just want to remove these lines from __call__:
@@ -261,12 +260,6 @@
if req.get('disable_cookie_login__', 0):
return
-if (self.unauth_page or
-attempt == ATTEMPT_LOGIN or attempt == ATTEMPT_NONE):
-# Modify the unauthorized response.
-req._hold(ResponseCleanup(resp))
-resp.unauthorized = self.unauthorized
-resp._unauthorized = self._unauthorized
if attempt != ATTEMPT_NONE:
# Trying to log in or resume a session
if self.cache_header_value:
Thanks for the clarification.
@deprecate
def credentialsChanged(user, name, pw):
# XXX: this method violates the rules for
tools/utilities:
# it depends on self.REQUEST
Why deprecated?
Look at the docstring.
But is it obsolete?
I don't know - it only seems to have relevance in the final statement of
__call__
@deprecate
def logout():
Logs out the user and redirects to the logout page.
Why deprecated?
Surely this should be handled directly by the logout form or view? If it
is kept to do the logging out, then the signature can be changed to
require the request to be passed in. Redirection should be handled by
the
logout page.
Right. If you write a view for that, the method might become useless.
But that itself would imply the logout view becoming the default action.
We can keep it around for access from untrusted code.
I'll update the interface to reflect this discussion.
Charlie
--
Charlie Clark
Managing Director
Clark Consulting Research
German Office
Helmholtzstr. 20
Düsseldorf
D- 40215
Tel: +49-211-600-3657
Mobile: +49-178-782-6226
___
Zope-CMF maillist - Zope-CMF@zope.org
https://mail.zope.org/mailman/listinfo/zope-cmf
See https://bugs.launchpad.net/zope-cmf/ for bug reports and feature requests
