Re: [Repoze-dev] security changes...
Previously Chris McDonough wrote: def remember(self, principal, token): Return a set of headers suitable for 'remembering' the principal on subsequent requests def forget(): Return a set of headers suitable for 'forgetting' the current user on subsequent requests It would be nice if there was also a handy utility function to apply those headers to a response. I suspect that code is often repeated now. class IAuthorizationPolicy(Interface): An adapter on context def permits(self, principals, permission): Return True if any of the principals is allowed the permission in the current context, else return False def principals_allowed_by_permission(self, permission): Return a set of principal identifiers allowed by the permission There are situations where principals_allowed_by_permission may not be possible, or at least very expensive. In LDAP/AD environments for example getting a list of all principles is often not doable. This should be reflected in the API somehow. Perhaps allow for principals_allowed_by_permission to return a value indicating it is not willing to support this? The same holds for the version in ISecurityPolicy. I wonder if we should also set the exceptions that can be thrown by those routines. Depending on the implementation that can be DB-API exceptions, LDAP exceptions, or anything else which would mean that people using this API will end up have to use a bare try/except. Wichert. -- Wichert Akkerman wich...@wiggy.netIt is simple to make things. http://www.wiggy.net/ It is hard to make things simple. ___ Repoze-dev mailing list Repoze-dev@lists.repoze.org http://lists.repoze.org/listinfo/repoze-dev
Re: [Repoze-dev] security changes...
On 5/26/09 1:50 AM, Malthe Borch wrote: 2009/5/25 Chris McDonoughchr...@plope.com: So to the end of breaking them apart, I just wrote up a bit of science fiction in code form. Could you take a look at the below and let me know what you think? It's very clear. Cool. I've implemented most of it on the authchanges branch (with slight modifications). Some more tests need writing, but it's about done. I'm not sure if this will land in 0.8.2 or if I'll start a new 0.9 series or what. I actually bagged the ISecurityPolicy adapter I described in the first email entirely, and instead you register an effective security policy using a ZCML directive like so: securitypolicy authentication=repoze.who.authentication.RemoteUserAuthenticationPolicy authorization=repoze.who.authorization.ACLAuthorizationPolicy / Both authentication and authorization above are adapters (the authentication one adapts on both context and request, while the authorization one just adapts on context). As a result, mortal users will not be expected to need to use any ZCA stuff to work with the authentication or authorization APIs; instead they use stuff like authenticated_userid, has_permission, etc (which under the hood consult both effective authentication and authorization adapters as necessary). While doing testing, they can use the repoze.bfg.testing API to prevent needing to register adapters against the authorization and authentication interfaces directly. In terms of style, for interfaces that represent would-be adapters, how do you feel about something like: def __init__(context): Adapter. ``context`` is ... Perhaps less ZCA-savvy users will find this style more informative in terms of what's really going on. If I haven't hidden the presence of interfaces (and other ZCA abstractions) deeply enough that no one will ever need to know they're there, I've kinda failed, because they really are an implementation detail. I don't use them for docs in repoze.bfg, so folks who don't know ZCA won't be reading them much anyway. Thanks! - C ___ Repoze-dev mailing list Repoze-dev@lists.repoze.org http://lists.repoze.org/listinfo/repoze-dev
Re: [Repoze-dev] security changes...
Previously Chris McDonough wrote: On 5/26/09 3:52 AM, Wichert Akkerman wrote: Previously Chris McDonough wrote: def remember(self, principal, token): Return a set of headers suitable for 'remembering' the principal on subsequent requests def forget(): Return a set of headers suitable for 'forgetting' the current user on subsequent requests It would be nice if there was also a handy utility function to apply those headers to a response. I suspect that code is often repeated now. class IAuthorizationPolicy(Interface): An adapter on context def permits(self, principals, permission): Return True if any of the principals is allowed the permission in the current context, else return False def principals_allowed_by_permission(self, permission): Return a set of principal identifiers allowed by the permission There are situations where principals_allowed_by_permission may not be possible, or at least very expensive. In LDAP/AD environments for example getting a list of all principles is often not doable. This should be reflected in the API somehow. Perhaps allow for principals_allowed_by_permission to return a value indicating it is not willing to support this? I don't think principals_allowed_by_permission does what you think it does. Or at least what I meant it to do. In a traversal-based app, it's essentially a walk of the object (model) graph from the root along some path to a particular context collecting identifiers for users who have that permission. Ok. I see the difference here: you use the proper definition of principals, why I mentally had expandede groups to turn it into a list of users. That requires access to some storage, while just generating a list of principals does not. I agree that using proper principals here makes sense, and makes my point obsolete. I wonder if we should also set the exceptions that can be thrown by those routines. Depending on the implementation that can be DB-API exceptions, LDAP exceptions, or anything else which would mean that people using this API will end up have to use a bare try/except. Can you provide an example situation where some system wasn't up to this task? A common one are transient errors, such as a connection failure during a database restart. LDAP can be difficult since it will raise an exception if a search would return more than a fixed limit of results (often set to 500). AD environments, especially with forests, may not allow you to query the list of principles at all (which makes sense, there can be many tens of thousands of principles). Wichert. -- Wichert Akkerman wich...@wiggy.netIt is simple to make things. http://www.wiggy.net/ It is hard to make things simple. ___ Repoze-dev mailing list Repoze-dev@lists.repoze.org http://lists.repoze.org/listinfo/repoze-dev
Re: [Repoze-dev] security changes...
On 5/26/09 4:07 AM, Chris McDonough wrote: On 5/26/09 3:52 AM, Wichert Akkerman wrote: Previously Chris McDonough wrote: def remember(self, principal, token): Return a set of headers suitable for 'remembering' the principal on subsequent requests def forget(): Return a set of headers suitable for 'forgetting' the current user on subsequent requests It would be nice if there was also a handy utility function to apply those headers to a response. I suspect that code is often repeated now. Oops, missed this one... This might mean the difference between: from repoze.bfg.security import logout def someview(context, request): response = render_template_to_response('some.pt') logout(response) return response vs. from repoze.bfg.security import forget def someview(context, request): response = render_template_to_response('some.pt') headers = forget() response.headerlist.extend(headers) return response and... from repoze.bfg.security import login def someview(context, request): response = render_template_to_response('some.pt') login(response, 'fred') return response vs. from repoze.bfg.security import remember def someview(context, request): response = render_template_to_response('some.pt') headers = remember('fred') response.headerlist.extend(headers) return response But since BFG doesn't have a global response object like Pylons does, it's not possible to do: def someview(context, request): logout() return render_template_to_response('some.pt') .. if that's what you mean. - C ___ Repoze-dev mailing list Repoze-dev@lists.repoze.org http://lists.repoze.org/listinfo/repoze-dev
Re: [Repoze-dev] security changes...
Previously Chris McDonough wrote: On 5/26/09 4:07 AM, Chris McDonough wrote: On 5/26/09 3:52 AM, Wichert Akkerman wrote: Previously Chris McDonough wrote: def remember(self, principal, token): Return a set of headers suitable for 'remembering' the principal on subsequent requests def forget(): Return a set of headers suitable for 'forgetting' the current user on subsequent requests It would be nice if there was also a handy utility function to apply those headers to a response. I suspect that code is often repeated now. Oops, missed this one... This might mean the difference between: from repoze.bfg.security import logout def someview(context, request): response = render_template_to_response('some.pt') logout(response) return response vs. from repoze.bfg.security import forget def someview(context, request): response = render_template_to_response('some.pt') headers = forget() response.headerlist.extend(headers) return response The first variant feels better. I can not see a use case where you would ever want to manipulate the headers returned by logout()/remember() before adding them to the response, so folding the response header updating into the function itself makes sense. Saves one often duplicated line of code. But since BFG doesn't have a global response object like Pylons does, it's not possible to do: def someview(context, request): logout() return render_template_to_response('some.pt') .. if that's what you mean. Luckily that is not what I meant :) Wichert. -- Wichert Akkerman wich...@wiggy.netIt is simple to make things. http://www.wiggy.net/ It is hard to make things simple. ___ Repoze-dev mailing list Repoze-dev@lists.repoze.org http://lists.repoze.org/listinfo/repoze-dev
Re: [Repoze-dev] security changes...
On 5/26/09 4:16 AM, Wichert Akkerman wrote: Previously Chris McDonough wrote: On 5/26/09 3:52 AM, Wichert Akkerman wrote: Previously Chris McDonough wrote: def remember(self, principal, token): Return a set of headers suitable for 'remembering' the principal on subsequent requests def forget(): Return a set of headers suitable for 'forgetting' the current user on subsequent requests It would be nice if there was also a handy utility function to apply those headers to a response. I suspect that code is often repeated now. class IAuthorizationPolicy(Interface): An adapter on context def permits(self, principals, permission): Return True if any of the principals is allowed the permission in the current context, else return False def principals_allowed_by_permission(self, permission): Return a set of principal identifiers allowed by the permission There are situations where principals_allowed_by_permission may not be possible, or at least very expensive. In LDAP/AD environments for example getting a list of all principles is often not doable. This should be reflected in the API somehow. Perhaps allow for principals_allowed_by_permission to return a value indicating it is not willing to support this? I don't think principals_allowed_by_permission does what you think it does. Or at least what I meant it to do. In a traversal-based app, it's essentially a walk of the object (model) graph from the root along some path to a particular context collecting identifiers for users who have that permission. Ok. I see the difference here: you use the proper definition of principals, why I mentally had expandede groups to turn it into a list of users. That requires access to some storage, while just generating a list of principals does not. I agree that using proper principals here makes sense, and makes my point obsolete. Ah, right. No. The ACLs contain all necessary principal ids; no authorization policy's principals_allowed_by_permission function should ever need to go to the authentication source. FTR, the primary function of principals_allowed_by_permission is to be able to compute a set of principals that have some permission on an object for storage in a security index (e.g. CMF's AllowedRolesAndUsers index). The IAuthenticationPolicy thingy is an adapter, so it uses its context as the current object for that purpose. That said, once some application gets a list of principals from principals_allowed_by_permission, that application might need to expand one of them into a different set (e.g. expand group:foo to its constituents), but that's outside the scope of this thing, thankfully. I wonder if we should also set the exceptions that can be thrown by those routines. Depending on the implementation that can be DB-API exceptions, LDAP exceptions, or anything else which would mean that people using this API will end up have to use a bare try/except. Can you provide an example situation where some system wasn't up to this task? A common one are transient errors, such as a connection failure during a database restart. LDAP can be difficult since it will raise an exception if a search would return more than a fixed limit of results (often set to 500). AD environments, especially with forests, may not allow you to query the list of principles at all (which makes sense, there can be many tens of thousands of principles). Yup; ostensibly we won't need to worry about that here. Thanks! - C ___ Repoze-dev mailing list Repoze-dev@lists.repoze.org http://lists.repoze.org/listinfo/repoze-dev
Re: [Repoze-dev] security changes...
On 5/26/09 4:20 AM, Wichert Akkerman wrote: from repoze.bfg.security import logout def someview(context, request): response = render_template_to_response('some.pt') logout(response) return response vs. from repoze.bfg.security import forget def someview(context, request): response = render_template_to_response('some.pt') headers = forget() response.headerlist.extend(headers) return response The first variant feels better. I can not see a use case where you would ever want to manipulate the headers returned by logout()/remember() before adding them to the response, so folding the response header updating into the function itself makes sense. Saves one often duplicated line of code. OK. If nothing earth-shattering pops up, I'll add login and logout convenience functions. - C ___ Repoze-dev mailing list Repoze-dev@lists.repoze.org http://lists.repoze.org/listinfo/repoze-dev
Re: [Repoze-dev] security changes...
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Wichert Akkerman wrote: Previously Chris McDonough wrote: def remember(self, principal, token): Return a set of headers suitable for 'remembering' the principal on subsequent requests def forget(): Return a set of headers suitable for 'forgetting' the current user on subsequent requests It would be nice if there was also a handy utility function to apply those headers to a response. I suspect that code is often repeated now. - -1: Unlike Zope2, there *is* no pre-existing, must-use-it-or-else response object lying around. The application's whole job in life is to fabricate the response: muddying that up with convenience helpers is just obscuring that. Tres. - -- === Tres Seaver +1 540-429-0999 tsea...@palladion.com Palladion Software Excellence by Designhttp://palladion.com -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.6 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org iD8DBQFKHBPc+gerLs4ltQ4RAhjcAJ46/rdID9RqA5YparT8Eed0IA2rdgCfen92 1lJJYOoiY6G3OdMWp/pFNks= =+++Q -END PGP SIGNATURE- ___ Repoze-dev mailing list Repoze-dev@lists.repoze.org http://lists.repoze.org/listinfo/repoze-dev
Re: [Repoze-dev] security changes...
2009/5/25 Chris McDonough chr...@plope.com: So to the end of breaking them apart, I just wrote up a bit of science fiction in code form. Could you take a look at the below and let me know what you think? It's very clear. In terms of style, for interfaces that represent would-be adapters, how do you feel about something like: def __init__(context): Adapter. ``context`` is ... Perhaps less ZCA-savvy users will find this style more informative in terms of what's really going on. \malthe ___ Repoze-dev mailing list Repoze-dev@lists.repoze.org http://lists.repoze.org/listinfo/repoze-dev