Here's a postmortem of poor API design in the Pyramid web framework. Unfortunately, when designing a framework, you get exactly one shot at creating "the right" API. In this case, we didn't. This design mistake isn't nearly fatal, it's just annoying. We won't be changing any Pyramid APIs, even though the design choices we discuss below are indeed poor, because the APIs involved are already in fairly wide use. But it's useful to record why these decisions were wrong for future reference and for the benefit of other framework developers.
Pyramid has the concept of a pluggable "authentication policy", which interrogates the request, checking and validating credentials. It has an API that looks like this:
class IAuthenticationPolicy(Interface):
""" An object representing a Pyramid authentication policy. """
def authenticated_userid(request):
""" Return the authenticated userid or ``None`` if no authenticated
userid can be found. This method of the policy should ensure that a
record exists in whatever persistent store is used related to the
user (the user should not have been deleted); if a record associated
with the current id does not exist in a persistent store, it should
return ``None``."""
def unauthenticated_userid(request):
""" Return the *unauthenticated* userid. This method performs the
same duty as ``authenticated_userid`` but is permitted to return the
userid based only on data present in the request; it neednt (and
shouldn't) check any persistent store to ensure that the user record
related to the request userid exists."""
def effective_principals(request):
""" 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 remember(request, principal, **kw):
""" Return a set of headers suitable for 'remembering' the
principal named ``principal`` when set in a response. An
individual authentication policy and its consumers can decide
on the composition and meaning of **kw. """
def forget(request):
""" Return a set of headers suitable for 'forgetting' the
current user on subsequent requests. """
Pyramid also has the concept of a pluggable authorization policy. The
responsibility of an authorization policy is to determine whether a user is
permitted to perform a particular action. It depends on information
retrieved from the authentication policy (namely, the "principals" associated
with a user, as usually retrieved from effective_principals). Here's the
API of an authorization policy:
class IAuthorizationPolicy(Interface):
""" An object representing a Pyramid authorization policy. """
def permits(context, 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(context, permission):
""" Return a set of principal identifiers allowed by the permission """
Here are the mistakes in these APIs:
authenticated_userid API is too complex: in
particular, the method is required to return None if the user does not
exist in persistent storage. This means that the authentication policy is
responsible not only for interrogating the request for credentials but
also requires cooperation from code that inspects the user's database.
This has the unfortunate real-world effect that using a "stock"
authentication policy shipped with Pyramid requires that a "knob" function
called a "groupfinder" be passed to the authentication policy at its
construction time. A "groupfinder" knows about the user's particular user
store and either returns a list of groups or the None value if the user
doesn't exist in the database.effective_principals API is too complex:
particular, the method is required to return group information. Like,
authenticated_userid, this also means that the authentication policy is
responsible both for interrogating the request for credentials but also
requires cooperation from code that inspects the user's database. If a
"stock" authentication policy is used, the "groupfinder" detailed in the
above bullet point is used for this purpose too.Why, exactly, are these choices wrong?
How would we fix it if we had it to do all over again?
We'd ditch the idea of an "authentication policy", and instead just have an "identity policy". The responsibilities of the identity policy would be to return the identity, remember the identity, and forget the identity. It would make no judgment about whether the userid related to the identity "exists" in the application, it just tells you who the request claims the user to be:
class IIdentityPolicy(Interface):
""" An object representing a Pyramid identity policy. """
def identify(request):
""" Return the claimed identity of the user associated request or
``None`` if no identity can be found associated with the request."""
def remember(request, identity, **kw):
""" Return a set of headers which can be used to remember
``identity`` for a subsequent request. An individual identity
policy and its consumers can decide on the composition and meaning of
**kw."""
def forget(request):
""" Return a set of headers which can be used to 'forget' the current
identity on subsequent requests."""
Users would rarely need to create a custom identity policy. The stock implementations would be used almost exclusively.
We'd simplify the interface of an "authorization policy". Make it just an instance that implements two methods: "authorized_userid" and "permits". "authorized_userid" would be passed the identity from the identity policy and will return a userid. "permits" would be passed the context, the identity, and a permission. The responsibility for determining if the identity "exists" is moved to the authorization policy, as well as determining group memberships and so forth:
class IAuthorizationPolicy(Interface):
""" An object representing a Pyramid authorization policy. """
def permits(context, identity, permission):
""" Return True if the userid is allowed the permission in the
current context, else return False"""
def authorized_userid(self, identity):
""" Return the userid of the user identified by the identity
or 'None' if no user exists related to the identity """
The framework would no longer deal in "principals" at all. Users would influence authorization by supplying a custom authorization policy that might deal internally in "principals", but needn't do that if it's unnecessary (if there are no group memberships). We would move the ACL checking code to a library of some kind that would be used by authentication policy implementers as necessary, and we'd supply a default authorization policy that just didn't consider group memberships at all.
Here's a scifi example of how such a system might work:
from zope.interface import Interface
from zope.interface import implements
from pyramid.location import lineage
from pyramid import security
from pyramid.decorator import reify
from pyramid.interfaces import IIdentityPolicy
from pyramid.interfaces import IAuthorizationPolicy
class CookieIdentityPolicy(object):
implements(IIdentityPolicy)
def identify(self, request):
return request.cookies.get('userid')
def remember(self, request, identity, **kw):
return [('Set-Cookie', 'userid=%s' % identity)]
def forget(request):
return [
('Set-Cookie',
'userid=expired; Expires=Wed, 31-Dec-1971 01:01:01 GMT')
]
class DictionaryAuthorizationPolicy(object):
implements(IAuthorizationPolicy)
def __init__(self, data):
self.data = data
self.acl_helper = ACLHelper()
def permits(self, context, identity, permission):
record = self.data.get(identity)
if record is not None:
principals = record.get('principals', [])
return self.acl_helper.permits(context, principals, permission)
return False
def authorized_userid(self, identity):
if identity in self.data:
return identity
class ACLHelper(object):
def permits(self, context, principals, permission):
""" Return an instance of
:class:`pyramid.security.ACLAllowed` instance if the policy
permits access, return an instance of
:class:`pyramid.security.ACLDenied` if not."""
acl = '<No ACL found on any object in resource lineage>'
for location in lineage(context):
try:
acl = location.__acl__
except AttributeError:
continue
for ace in acl:
ace_action, ace_principal, ace_permissions = ace
if ace_principal in principals:
if not hasattr(ace_permissions, '__iter__'):
ace_permissions = [ace_permissions]
if permission in ace_permissions:
if ace_action == security.Allow:
return security.ACLAllowed(ace, acl, permission,
principals, location)
else:
return security.ACLDenied(ace, acl, permission,
principals, location)
# default deny (if no ACL in lineage at all, or if none of the
# principals were mentioned in any ACE we found)
return security.ACLDenied(
'<default deny>',
acl,
permission,
principals,
context)
def emulate_router():
from pyramid import testing
ipol = CookieIdentityPolicy()
apol = DictionaryAuthorizationPolicy(
{'chris':{'principals':['chris', 'group:foo']}})
request = testing.DummyRequest()
context1 = testing.DummyModel()
context1.__acl__ = [(security.Allow, 'group:foo', 'view')]
context2 = testing.DummyModel()
context2.__acl__ = [(security.Allow, 'others', 'edit')]
request.cookies['userid'] = 'chris'
identity = ipol.identify(request)
permitted = apol.permits(context1, identity, 'view')
assert(bool(permitted))
permitted = apol.permits(context2, identity, 'edit')
assert(not bool(permitted))
def emulate_user():
class User(object):
pass
class DummyDBConnection(object):
def query(self, q):
return User()
from pyramid.registry import Registry
ipol = CookieIdentityPolicy()
apol = DictionaryAuthorizationPolicy(
{'chris':{'principals':['chris', 'group:foo']}})
# this would go in pyramid.security
def authorized_userid(request):
reg = request.registry
ipol = reg.getUtility(IIdentityPolicy)
apol = reg.getUtility(IAuthorizationPolicy)
identity = ipol.identify(request)
if identity is not None:
userid = apol.authorized_userid(identity)
if userid is not None:
return userid
class RequestWithUser(object):
registry = Registry()
registry.settings = {}
registry.settings['dbconn'] = DummyDBConnection()
registry.registerUtility(ipol, IIdentityPolicy)
registry.registerUtility(apol, IAuthorizationPolicy)
cookies = {'userid':'chris'}
@reify
def user(self):
userid = authorized_userid(self)
if userid is not None:
return self.registry.settings['dbconn'].query({'user':userid})
request = RequestWithUser()
assert(request.user.__class__ is User)
if __name__ == '__main__':
emulate_router()
emulate_user()
But, c'est la vie, that won't happen at this point. Sorry about the bad design.
When using a signed cookie system such as mod_auth_tkt/pubcookie/etc. it is possible for an application to return the userid, and even some group tokens and user data, without reference to a persistent store - only the shared login server needs that.
I would have thought that the persistent storage was only an implementation detail, though clearly not all implementations would allow modification of user data or decorate a user object with extra attributes.
If one needs to differentiate between distributes (openid) userids and local userids, surely that would be better done by assigning local users a member role?
Laurence