[Repoze-dev] security changes...
Hi all, The current "security policy" abstraction in BFG is a bit brittle because it conflates authentication and authorization APIs. Thus we get things like "RepozeWhoIdentityInheritingACLSecurityPolicy" and "RemoteUserInheritingACLSecurityPolicy". I'd like to tease apart the bits of the current security policy abstraction related to authentication from bits related to authorization, so they can be overridden separately by a BFG application without needing to create a separate security policy each time either needs to change. This is an explicit reversion on my thinking of it a year ago (I didn't think there needed to be any separation) but as I add more security policies, a natural break is becoming clearer. The authorization stuff doesn't really need to change much; one policy would suffice for 90% of applications (the "inheriting ACL" policy). But the APIs related to authentication require constant overriding. 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? side note to Tres: Could you see what you think about the "IAuthenticationPolicy" implementation named "RepozeWhoAuthenticationPolicy" which presumes that the "api factory" is passed down in the environ, also science fiction at this point. # Tweak BFG slightly to allow for separate authentication and # authorization policies, so we don't have security policies named # e.g. "RepozeWhoInheritingACLSecurityPolicy". We'll add # IAuthenticationPolicy objects and IAuthorizationPolicy objects; # these will be adapters. We'll also change ISecurityPolicy to be an # adapter rather than a utility. We'll tweak the function API to use # these adapters. # b/w incompats: the "authorization" policy needs access to the # context, which the APIs it maps to in a current BFG security policy # don't have; code which depends on these APIs will need to change. from zope.interface import implements from zope.interface import Interface class IAuthenticationPolicy(Interface): """ A multi-adapter on context and request """ def authenticated_userid(): """ Return the authenticated userid or None if no authenticated userid can be found """ def effective_principals(): """ Return a sequence representing the effective principals (including the userid and any groups belonged to by the current user, including 'system' groups such as Everyone and Authenticated""" def challenge(): """ Return an IResponse object representing a challenge, such as a login form or a basic auth dialog """ 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""" 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 """ class ISecurityPolicy(Interface): """ A multi-adapter on context and request """ def permits(permission): """ Returns True if the combination of the authorization information in the context and the authentication data in the request allow the action implied by the permission """ def authenticated_userid(): """ Return the userid of the currently authenticated user or None if there is no currently authenticated user """ def effective_principals(): """ Return the list of 'effective' principals for the request. This must include the userid of the currently authenticated user if a user is currently authenticated.""" def principals_allowed_by_permission(permission): """ Return a sequence of principal identifiers allowed by the ``permission`` in the model implied by ``context``. This method may not be supported by a given security policy implementation, in which case, it should raise a ``NotImplementedError`` exception.""" def forbidden(): """ This method should return an IResponse object (an object with the attributes ``status``, ``headerlist``, and ``app_iter``) as a result of a view invocation denial. The ``forbidden`` method of a security policy will be called by ``repoze.bfg`` when view invocation is denied (usually as a result of the ``permit`` method of the same security policy returning False to the Router). The ``forbidden`` method of a security will not be called w
Re: [Repoze-dev] security changes...
2009/5/25 Chris McDonough : > 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
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 It 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 McDonough: >> 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: 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...
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. In an ACL-based system (such as the default ACL policy in BFG), this means there's some ACE stored on a node in the path that allow the principal that permission, and and no nodes in the path that have an ACE which explictly denies the principal that permission. "Principals" in this sense are just strings in ACLs, usually. We never actually talk to the "authentication source" during this process; all the info is stored in the model graph. > The same holds for the version in ISecurityPolicy. ISecurityPolicy is now dead; its functionality is covered by function APIs such as "has_permission" which coordinate asking the authenticator and authorizer for the right info. > 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? - 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 It 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 It 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 Design"http://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