Re: [Zope-CMF] [dev] Unauthorized handling - a proposal
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 yuppie wrote: > 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. +1. Tres. - -- === Tres Seaver +1 540-429-0999 tsea...@palladion.com Palladion Software "Excellence by Design"http://palladion.com -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.9 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org iEYEARECAAYFAkvN2RQACgkQ+gerLs4ltQ58FwCdHh/mOORuBz8pvTmGr2cJtHba NM4AoLeUafYGzUko6uM2uRhqQ0SzY5P4 =DHRR -END PGP SIGNATURE- ___ 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, 16:21 Uhr, schrieb yuppie : > 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
Re: [Zope-CMF] [dev] Unauthorized handling - a proposal
Hi! Charlie Clark wrote: > Am 20.04.2010, 14:19 Uhr, schrieb yuppie: >> 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. You mean subfolders of browser? At the top level I would expect more than just the browser views inside folders with these names. For now I'll keep it where it is. If we agree on a new structure, things can be easily moved around. >>> 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. >>>@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. >>>@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: >>>@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? >>>@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. 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 : > 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(): >>
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
[Zope-CMF] CMF Tests: 5 OK
Summary of messages to the cmf-tests list. Period Mon Apr 19 12:00:00 2010 UTC to Tue Apr 20 12:00:00 2010 UTC. There were 5 messages: 5 from CMF Tests. Tests passed OK --- Subject: OK : CMF-2.1 Zope-2.10 Python-2.4.6 : Linux From: CMF Tests Date: Mon Apr 19 21:46:29 EDT 2010 URL: http://mail.zope.org/pipermail/cmf-tests/2010-April/012880.html Subject: OK : CMF-2.1 Zope-2.11 Python-2.4.6 : Linux From: CMF Tests Date: Mon Apr 19 21:48:29 EDT 2010 URL: http://mail.zope.org/pipermail/cmf-tests/2010-April/012881.html Subject: OK : CMF-2.2 Zope-2.12 Python-2.6.4 : Linux From: CMF Tests Date: Mon Apr 19 21:50:29 EDT 2010 URL: http://mail.zope.org/pipermail/cmf-tests/2010-April/012882.html Subject: OK : CMF-trunk Zope-2.12 Python-2.6.4 : Linux From: CMF Tests Date: Mon Apr 19 21:52:29 EDT 2010 URL: http://mail.zope.org/pipermail/cmf-tests/2010-April/012883.html Subject: OK : CMF-trunk Zope-trunk Python-2.6.4 : Linux From: CMF Tests Date: Mon Apr 19 21:54:29 EDT 2010 URL: http://mail.zope.org/pipermail/cmf-tests/2010-April/012884.html ___ 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 : > 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
[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