Re: [Repoze-dev] security changes...

2009-05-26 Thread Wichert Akkerman
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...

2009-05-26 Thread Chris McDonough
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...

2009-05-26 Thread Wichert Akkerman
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...

2009-05-26 Thread Chris McDonough
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...

2009-05-26 Thread Wichert Akkerman
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...

2009-05-26 Thread Chris McDonough
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...

2009-05-26 Thread Chris McDonough
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...

2009-05-26 Thread Tres Seaver
-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


[Repoze-dev] security changes...

2009-05-25 Thread Chris McDonough
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 when
 an ``IForbiddenResponseFactory`` utility is registered;
 instead the 

Re: [Repoze-dev] security changes...

2009-05-25 Thread Malthe Borch
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