Skip to content
This repository has been archived by the owner on Mar 28, 2019. It is now read-only.

Commit

Permalink
Major refactor of usage of prefixed_userid.
Browse files Browse the repository at this point in the history
Now relies on Pyramid multiauth last version that triggers
a policy selected event when effective_principals() is called
and provides a user_id attribute on triggered event.
  • Loading branch information
leplatrem committed Feb 10, 2016
1 parent a5d5754 commit ef8fec6
Show file tree
Hide file tree
Showing 9 changed files with 49 additions and 41 deletions.
4 changes: 2 additions & 2 deletions cliquet/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -155,9 +155,9 @@ def add_api_capability(config, identifier, description="", url="", **kw):
step_func(config)

# Custom helpers.
config.add_request_method(authentication.prefixed_userid, reify=True)
config.add_request_method(follow_subrequest)
config.add_request_method(lambda request: {'id': request.prefixed_userid},
config.add_request_method(authentication.prefixed_userid, property=True)
config.add_request_method(lambda r: {'id': r.prefixed_userid},
name='get_user_info')
config.add_request_method(current_resource_name, reify=True)
config.commit()
Expand Down
22 changes: 11 additions & 11 deletions cliquet/authentication.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,19 +9,12 @@ def prefixed_userid(request):
If a custom authn policy is used, without authn_type, this method returns
the user id without prefix.
"""
user_id = request.authenticated_userid
# If pyramid_multiauth is used, the previous line will haved triggered
# a MultiAuthPolicySelected event, and will set a ``authn_type``
# attribute on request.
# If pyramid_multiauth is used, a ``authn_type`` is set on request
# when a policy succesfully authenticates a user.
# (see :func:`cliquet.initializatio.setup_authentication`)

# Prefix the user id with the authn policy type name.
authn_type = getattr(request, 'authn_type', None)
if authn_type is None:
return user_id

if user_id:
return '%s:%s' % (authn_type.lower(), user_id)
if authn_type is not None:
return authn_type + ':' + request.selected_userid


class BasicAuthAuthenticationPolicy(base_auth.BasicAuthAuthenticationPolicy):
Expand All @@ -38,6 +31,13 @@ def noop_check(*a):
*args,
**kwargs)

def effective_principals(self, request):
"""Bypass default Pyramid construction of principals because
Pyramid multiauth already adds userid, Authenticated and Everyone
principals.
"""
return []

def unauthenticated_userid(self, request):
settings = request.registry.settings

Expand Down
28 changes: 16 additions & 12 deletions cliquet/authorization.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
import functools

from pyramid.settings import aslist
from pyramid.security import IAuthorizationPolicy, Authenticated
from zope.interface import implementer

from cliquet import utils
from cliquet.storage import exceptions as storage_exceptions
from cliquet.authentication import prefixed_userid

# A permission is called "dynamic" when it's computed at request time.
DYNAMIC = 'dynamic'
Expand All @@ -23,16 +25,17 @@ def groupfinder(userid, request):
if not backend:
return []

# Query the permission backend only once per request (e.g. batch).
# XXX
# return backend.user_principals(request.unauthenticated_userid)
if request.prefixed_userid:
userid = request.prefixed_userid

# reify_key = request.prefixed_userid + '_principals'
# if reify_key not in request.bound_data:
# principals = backend.user_principals(request.prefixed_userid)
# request.bound_data[reify_key] = principals
# Query the permission backend only once per request (e.g. batch).
reify_key = userid + '_principals'
if reify_key not in request.bound_data:
principals = backend.user_principals(userid)
request.bound_data[reify_key] = principals

# return request.bound_data[reify_key]
principals = request.bound_data[reify_key]
return principals


@implementer(IAuthorizationPolicy)
Expand All @@ -46,9 +49,10 @@ def permits(self, context, principals, permission):
return Authenticated in principals

# Add prefixed user id to principals.
if context.prefixed_userid:
principals = principals + [context.prefixed_userid]
prefix, user_id = context.prefixed_userid.split(':', 1)
prefixed_userid = context.get_prefixed_userid()
if prefixed_userid and ':' in prefixed_userid:
principals = principals + [prefixed_userid]
prefix, user_id = prefixed_userid.split(':', 1)
# Remove unprefixed user id to avoid conflicts.
# (it is added via Pyramid Authn policy effective principals)
if user_id in principals:
Expand Down Expand Up @@ -108,7 +112,7 @@ class RouteFactory(object):

def __init__(self, request):
# Make it available for the authorization policy.
self.prefixed_userid = request.prefixed_userid
self.get_prefixed_userid = functools.partial(prefixed_userid, request)

self._check_permission = request.registry.permission.check_permission

Expand Down
10 changes: 7 additions & 3 deletions cliquet/initialization.py
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,11 @@ def setup_authentication(config):

# Track policy used, for prefixing user_id and for logging.
def on_policy_selected(event):
event.request.authn_type = event.policy_name.lower()
authn_type = event.policy_name.lower()
event.request.authn_type = authn_type
event.request.selected_userid = event.userid
# Add authentication info to context.
logger.bind(uid=event.userid, authn_type=authn_type)

config.add_subscriber(on_policy_selected, MultiAuthPolicySelected)

Expand Down Expand Up @@ -346,9 +350,9 @@ def on_new_request(event):
path=event.request.path,
method=request.method,
querystring=dict(request.GET),
uid=request.unauthenticated_userid,
lang=request.headers.get('Accept-Language'),
authn_type=getattr(request, 'authn_type', None),
uid=None,
authn_type=None,
errno=None)

config.add_subscriber(on_new_request, NewRequest)
Expand Down
2 changes: 1 addition & 1 deletion cliquet/resource/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ def get_parent_id(self, request):
:rtype: str
"""
return getattr(request, 'prefixed_userid', None)
return request.prefixed_userid

def _get_known_fields(self):
"""Return all the `field` defined in the ressource mapping."""
Expand Down
10 changes: 5 additions & 5 deletions cliquet/tests/test_authorization.py
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ def setUp(self):
self.authz = AuthorizationPolicy()
self.authz.get_bound_permissions = mock.sentinel.get_bound_perms
self.context = mock.MagicMock()
self.context.prefixed_userid = None
self.context.get_prefixed_userid.return_value = None
self.context.allowed_principals = []
self.context.object_id = mock.sentinel.object_id
self.context.required_permission = 'read'
Expand Down Expand Up @@ -193,15 +193,15 @@ def test_permits_takes_route_factory_allowed_principals_into_account(self):
self.assertTrue(has_permission)

def test_prefixed_userid_is_added_to_principals(self):
self.context.prefixed_userid = 'fxa:userid'
self.context.get_prefixed_userid.return_value = 'fxa:userid'
self.authz.permits(self.context, self.principals, 'foobar')
self.context.check_permission.assert_called_with(
'foobar',
self.principals + ['fxa:userid', 'fxa_userid'],
get_bound_permissions=mock.sentinel.get_bound_perms)

def test_unprefixed_userid_is_removed_from_principals(self):
self.context.prefixed_userid = 'fxa:userid'
self.context.get_prefixed_userid.return_value = 'fxa:userid'
self.authz.permits(self.context, ['userid'], 'foobar')
self.context.check_permission.assert_called_with(
'foobar',
Expand All @@ -224,7 +224,7 @@ def test_permits_returns_true_if_collection_and_shared_records(self):
allowed = self.authz.permits(self.context, ['userid'], 'dynamic')
self.context.fetch_shared_records.assert_called_with(
'read',
['userid', 'basicauth:bob', 'basicauth_bob'],
['userid'],
get_bound_permissions=mock.sentinel.get_bound_perms)
self.assertTrue(allowed)

Expand All @@ -245,6 +245,6 @@ def test_permits_returns_false_if_collection_is_unknown(self):
allowed = self.authz.permits(self.context, ['userid'], 'dynamic')
self.context.fetch_shared_records.assert_called_with(
'read',
['userid', 'basicauth:bob', 'basicauth_bob'],
['userid'],
get_bound_permissions=mock.sentinel.get_bound_perms)
self.assertFalse(allowed)
6 changes: 3 additions & 3 deletions cliquet/tests/test_initialization.py
Original file line number Diff line number Diff line change
Expand Up @@ -338,15 +338,15 @@ def test_statsd_counts_unique_users(self, mocked, digest_mocked):
cliquet.initialize(self.config, '0.0.1', 'project_name')
app = webtest.TestApp(self.config.make_wsgi_app())
headers = {'Authorization': 'Basic bWF0Og=='}
app.get('/v0/__heartbeat__', headers=headers)
mocked().count.assert_any_call('users', unique='mat')
app.get('/v0/', headers=headers)
mocked().count.assert_any_call('users', unique='basicauth:mat')

@mock.patch('cliquet.statsd.Client')
def test_statsd_counts_authentication_types(self, mocked):
cliquet.initialize(self.config, '0.0.1', 'project_name')
app = webtest.TestApp(self.config.make_wsgi_app())
headers = {'Authorization': 'Basic bWF0Og=='}
app.get('/v0/__heartbeat__', headers=headers)
app.get('/v0/', headers=headers)
mocked().count.assert_any_call('authn_type.basicauth')


Expand Down
4 changes: 2 additions & 2 deletions cliquet/views/errors.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
from functools import wraps

from pyramid import httpexceptions
from pyramid.security import forget, NO_PERMISSION_REQUIRED
from pyramid.security import forget, NO_PERMISSION_REQUIRED, Authenticated
from pyramid.view import (
forbidden_view_config, notfound_view_config, view_config
)
Expand Down Expand Up @@ -32,7 +32,7 @@ def authorization_required(request):
"""Distinguish authentication required (``401 Unauthorized``) from
not allowed (``403 Forbidden``).
"""
if not request.authenticated_userid:
if Authenticated not in request.effective_principals:
error_msg = "Please authenticate yourself to use this endpoint."
response = http_error(httpexceptions.HTTPUnauthorized(),
errno=ERRORS.MISSING_AUTH_TOKEN,
Expand Down
4 changes: 2 additions & 2 deletions cliquet/views/hello.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,8 @@ def get_hello(request):
value = settings[setting]
data['settings'][setting] = value

prefixed_userid = getattr(request, 'prefixed_userid', None)
if prefixed_userid:
# Try to authenticate the current user:
if request.authenticated_userid:
data['user'] = request.get_user_info()

# Application can register and expose arbitrary capabilities.
Expand Down

0 comments on commit ef8fec6

Please sign in to comment.