Skip to content

Commit

Permalink
Straightforward API inclusion
Browse files Browse the repository at this point in the history
Clean up the boundaries between h.api and the rest of the application
so that it can be included in a straightforward manner.

The h.api module becomes more or less oblivious to authentication and
authorization, except that it still defines ACL properties for its
resources and it defines the token route.

The token route can be ripped out next and placed in another module,
since the h.api module needn't even know that OAuth tokens are in use
and in this way it can cease to rely on `request.create_token_response`.

This commit reintroduces pyramid_multiauth as a straightforward way to
support SessionAuthenticationPolicy and RemoteAuthenticationPolicy in
the same application.

The case of the default client in h.auth and the session view callable
are now oblivious to type of authentication policy in use.
  • Loading branch information
tilgovi committed Jul 20, 2015
1 parent 14ae4ad commit d1101db
Show file tree
Hide file tree
Showing 16 changed files with 97 additions and 107 deletions.
4 changes: 3 additions & 1 deletion .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@ python:
install:
- gem install compass
- pip install -U pip wheel
- pip install coveralls pyramid_redis_sessions
- pip install coveralls
- pip install pyramid_multiauth
- pip install pyramid_redis_sessions
- npm install
- make
script:
Expand Down
17 changes: 6 additions & 11 deletions conf/development.ini
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,6 @@
pipeline: h


[app:api]
use: egg:h#api

h.db.should_create_all: True

# Set a default persistent secret for development. DO NOT copy this into a
# production settings file.
h.client_id: nosuchid
h.client_secret: nosuchsecret


[app:h]
use: egg:h

Expand All @@ -24,12 +13,18 @@ h.autologin: True

mail.default_sender: "Annotation Daemon" <no-reply@localhost>

multiauth.groupfinder: h.auth.effective_principals
multiauth.policies: remote session
multiauth.policy.remote.use: pyramid.authentication.RemoteUserAuthenticationPolicy
multiauth.policy.session.use: pyramid.authentication.SessionAuthenticationPolicy

pyramid.debug_all: True
pyramid.reload_templates: True
pyramid.includes:
h.session
pyramid_debugtoolbar
pyramid_mailer
pyramid_multiauth
pyramid_tm
h.testing

Expand Down
6 changes: 6 additions & 0 deletions conf/production.ini
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,15 @@ mail.default_sender: "Annotation Daemon" <no-reply@localhost>
#mail.host: localhost
#mail.port: 25

# Authentication configuration -- see the pyramid_multiauth documentation
multiauth.policies: remote session
multiauth.policy.remote.use: pyramid.authentication.RemoteUserAuthenticationPolicy
multiauth.policy.session.use: pyramid.authentication.SessionAuthenticationPolicy

# Include any deployment-specific pyramid add-ons here
pyramid.includes:
pyramid_mailer
pyramid_multiauth
pyramid_redis_sessions
pyramid_tm

Expand Down
11 changes: 11 additions & 0 deletions h/api/__init__.py
Original file line number Diff line number Diff line change
@@ -1 +1,12 @@
# -*- coding: utf-8 -*-
__all__ = ('index', 'create_root', 'includeme')
from h.api.resources import create_root
from h.api.views import index


def includeme(config):
config.add_route('api', '/', factory=create_root)
config.include('h.features')
config.include('h.api.db')
config.include('h.api.queue')
config.include('h.api.views')
2 changes: 1 addition & 1 deletion h/api/resources.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ class Root(Resource):
]


def create_root(_):
def create_root(request):
"""
Returns a new traversal tree root.
"""
Expand Down
9 changes: 0 additions & 9 deletions h/api/subscribers.py

This file was deleted.

16 changes: 0 additions & 16 deletions h/api/tweens.py

This file was deleted.

1 change: 1 addition & 0 deletions h/api/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ def api_config(**kwargs):


@api_config(context=Root)
@api_config(route_name='api')
def index(context, request):
"""Return the API descriptor document.
Expand Down
69 changes: 18 additions & 51 deletions h/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,33 +5,14 @@
import os

from pyramid.config import Configurator
from pyramid.wsgi import wsgiapp2

from h.api.middleware import permit_cors
from h.auth import acl_authz, remote_authn, session_authn
from h.config import settings_from_environment
from h.security import derive_key

log = logging.getLogger(__name__)


def strip_vhm(view):
"""A view decorator that strips the X-Vhm-Root header from the request.
The X-Vhm-Root header is used by Pyramid for virtual hosting. The router
treats the value of this header as a traversal prefix. When a view
callable itself is an embedded Pyramid application, the inner application
should not process this header again. In this situation, this decorator
makes the virtual root work as intended.
"""
@functools.wraps(view)
def wrapped(context, request):
request.headers.pop('X-Vhm-Root', None)
return view(context, request)

return wrapped


def create_app(global_config, **settings):
"""Configure and add static routes and views. Return the WSGI app."""
settings = get_settings(global_config, **settings)
Expand All @@ -57,50 +38,36 @@ def create_app(global_config, **settings):
config.add_jinja2_renderer('.xml')

config.add_tween('h.tweens.csrf_tween_factory')
config.add_tween('h.tweens.auth_token')
config.add_subscriber('h.subscribers.set_user_from_oauth',
'pyramid.events.NewRequest')

config.set_authentication_policy(session_authn)
config.set_authorization_policy(acl_authz)
config.include('h.accounts')
config.include('h.auth')
config.include('h.claim')
config.include('h.notification')
config.include('h.queue')
config.include('h.streamer')

api_app = create_api(settings)
api_view = wsgiapp2(api_app)
config.add_view(api_view, name='api', decorator=strip_vhm)
# Add the view again with the 'index' route name, otherwise it will
# not take precedence over the index when a virtual root is in use.
config.add_view(api_view, name='api', decorator=strip_vhm,
route_name='index')

return config.make_wsgi_app()

config.include('h.api', route_prefix='/api')

def create_api(global_config, **settings):
settings = get_settings(global_config, **settings)

config = Configurator(settings=settings)

config.set_authentication_policy(remote_authn)
config.set_authorization_policy(acl_authz)
config.set_root_factory('h.api.resources.create_root')

config.add_subscriber('h.api.subscribers.set_user_from_oauth',
'pyramid.events.ContextFound')
config.add_tween('h.api.tweens.auth_token')
# Set the traversal path for the api index route.
config.add_route('api', '/api/', traverse='/api/')

config.include('h.features')

config.include('h.auth')
config.include('h.api.db')
config.include('h.api.queue')
config.include('h.api.views')
# Support virtual hosting the API at the root with X-Vhm-Root.
config.add_view('h.api.views.index',
context='h.api.resources.Root',
renderer='json',
route_name='index')

app = config.make_wsgi_app()
app = permit_cors(app,
allow_headers=('Authorization',
'X-Annotator-Auth-Token'),
allow_headers=(
'Authorization',
'Content-Type',
'X-Annotator-Auth-Token',
'X-Client-Id',
),
allow_methods=('HEAD', 'GET', 'POST', 'PUT', 'DELETE'))

return app
Expand Down
14 changes: 5 additions & 9 deletions h/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,6 @@
from pyramid.exceptions import BadCSRFToken
from pyramid import session

from pyramid.authentication import RemoteUserAuthenticationPolicy
from pyramid.authentication import SessionAuthenticationPolicy
from pyramid.authorization import ACLAuthorizationPolicy
from pyramid.util import action_method

from .interfaces import IClientFactory
Expand Down Expand Up @@ -70,7 +67,7 @@ def authenticate_client(self, request):
return False
client_id = request.registry.settings['h.client_id']
client = get_client(request, client_id)
user = request.session.get('userid')
user = request.authenticated_userid
elif request.client_secret is not None:
client_id = request.client_id
client_secret = request.client_secret
Expand Down Expand Up @@ -142,8 +139,10 @@ def effective_principals(userid, request):
consumer_group = 'consumer:{}'.format(request.client.client_id)
additional_principals.append(consumer_group)

if models.User.get_by_id(request, userid).admin:
additional_principals.append('group:admin')
primary_user = models.User.get_by_id(request, userid)
if primary_user is not None:
if primary_user.admin:
additional_principals.append('group:admin')

return additional_principals

Expand Down Expand Up @@ -226,9 +225,6 @@ def register():
config.action(IClientFactory, register, introspectables=(intr,))


remote_authn = RemoteUserAuthenticationPolicy(callback=effective_principals)
session_authn = SessionAuthenticationPolicy('', callback=effective_principals)
acl_authz = ACLAuthorizationPolicy()
validator = RequestValidator()


Expand Down
4 changes: 3 additions & 1 deletion h/resources.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# -*- coding: utf-8 -*-
from pyramid import security

from .api import create_root as create_api
from .models import Annotation


Expand Down Expand Up @@ -56,11 +57,12 @@ class Root(Resource):
]


def create_root(_):
def create_root(request):
"""
Returns a new traversal tree root.
"""
r = Root()
r.add('api', create_api(request))
r.add('app', Application())
r.add('stream', Stream())
r.add('a', AnnotationFactory())
Expand Down
3 changes: 2 additions & 1 deletion h/session.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,9 @@


def model(request):
session = {k: v for k, v in request.session.items() if k[0] != '_'}
session = {}
session['csrf'] = request.session.get_csrf_token()
session['userid'] = request.authenticated_userid
return session


Expand Down
15 changes: 10 additions & 5 deletions h/subscribers.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,8 @@

def add_renderer_globals(event):
request = event['request']
# Set the base url to use in the <base> tag
event['base_url'] = request.resource_url(request.root, '')
# Set the service url to use for API discovery
event['service_url'] = request.resource_url(request.root, 'api', '')
# Allow templates to check for feature flags
event['base_url'] = request.route_url('index')
event['service_url'] = request.route_url('api')
event['feature'] = request.feature

# Add Google Analytics
Expand All @@ -18,3 +15,11 @@ def add_renderer_globals(event):
event['ga_create_options'] = "'none'"
else:
event['ga_create_options'] = "'auto'"


def set_user_from_oauth(event):
"""A subscriber that checks requests for OAuth credentials and sets the
'REMOTE_USER' environment key to the authorized user (or ``None``)."""
request = event.request
request.verify_request()
request.environ['REMOTE_USER'] = getattr(request, 'user', None)
9 changes: 7 additions & 2 deletions h/test/auth_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ def __init__(self, request, client_id):
class TestRequestValidator(unittest.TestCase):

def setUp(self):
self.config = testing.setUp()
self.security = testing.DummySecurityPolicy()
self.config.set_authorization_policy(self.security)
self.config.set_authentication_policy(self.security)
self.client_patcher = patch('h.auth.get_client')
self.client = self.client_patcher.start()
self.decode_patcher = patch('jwt.decode')
Expand All @@ -30,6 +34,7 @@ def setUp(self):
self.request.registry.settings['h.client_secret'] = SECRET

def tearDown(self):
testing.tearDown()
self.client_patcher.stop()
self.decode_patcher.stop()

Expand All @@ -55,16 +60,16 @@ def test_authenticate_client_not_ok(self):

def test_authenticate_client_csrf_ok(self):
client = MockClient(self.request, KEY)
self.security.userid = 'hooper'
self.request.client_id = None
self.request.client_secret = None
self.client.return_value = client
with patch('pyramid.session.check_csrf_token') as csrf:
csrf.return_value = True
self.request.session = {'userid': 'hopper'}
res = self.validator.authenticate_client(self.request)
assert res is True
assert self.request.client is client
assert self.request.user == 'hopper'
assert self.request.user == 'hooper'

def test_authenticate_client_csrf_not_ok(self):
self.request.client_id = None
Expand Down

0 comments on commit d1101db

Please sign in to comment.