Skip to content
Permalink
Browse files

Merge pull request #551 from fedora-infra/feature/refactor-acls

Use validate_acls to produce the can_edit value.
  • Loading branch information...
lmacken committed Sep 10, 2015
2 parents 1231fa9 + 3592136 commit b5b1eb6824accbf2bb275f4f29509445b121a250
Showing with 49 additions and 29 deletions.
  1. +24 −16 bodhi/security.py
  2. +6 −10 bodhi/services/updates.py
  3. +17 −0 bodhi/tests/functional/test_updates.py
  4. +2 −3 bodhi/validators.py
@@ -12,14 +12,15 @@
# along with this program; if not, write to the Free Software
# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.

from pyramid.security import (Allow, Deny, Everyone, Authenticated,
ALL_PERMISSIONS, DENY_ALL)
from cornice.errors import Errors

from pyramid.security import (Allow, ALL_PERMISSIONS, DENY_ALL)
from pyramid.security import remember, forget
from pyramid.httpexceptions import HTTPFound
from pyramid.threadlocal import get_current_registry

from . import log
from .models import User, Group, Update
from .models import User, Group


#
@@ -35,19 +36,10 @@ def admin_only_acl(request):

def packagers_allowed_acl(request):
"""Generate an ACL for update submission"""
return [(Allow, 'group:' + group, ALL_PERMISSIONS) for group in
request.registry.settings['mandatory_packager_groups'].split()] + \
[DENY_ALL]


def package_maintainers_only_acl(request):
"""An ACL that only allows package maintainers for a given package"""
acl = admin_only_acl(request)
update = Update.get(request.matchdict['id'], request.db)
if update:
for committer in update.get_maintainers():
acl.insert(0, (Allow, committer.name, ALL_PERMISSIONS))
return acl
groups = request.registry.settings['mandatory_packager_groups'].split()
return [
(Allow, 'group:' + group, ALL_PERMISSIONS) for group in groups
] + [DENY_ALL]


#
@@ -189,3 +181,19 @@ def __contains__(self, item):

cors_origins_ro = CorsOrigins('cors_origins_ro')
cors_origins_rw = CorsOrigins('cors_origins_rw')


class ProtectedRequest(object):
""" A proxy to the request object.
The point here is that you can set 'errors' on this request, but they
will be sent to /dev/null and hidden from cornice. Otherwise, this
object behaves just like a normal request object.
"""
def __init__(self, real_request):
# Hide errors added to this from the real request
self.errors = Errors()
# But proxy other attributes to the real request
self.real_request = real_request
for attr in ['db', 'registry', 'validated', 'buildinfo', 'user']:
setattr(self, attr, getattr(self.real_request, attr))
@@ -16,7 +16,6 @@
import math

from cornice import Service
from pyramid.security import has_permission
from sqlalchemy import func, distinct
from sqlalchemy.sql import or_

@@ -47,18 +46,12 @@
update = Service(name='update', path='/updates/{id}',
validators=(validate_update_id,),
description='Update submission service',
# This acl only checks if the user is an admin or a commiters to the packages,
# where as the validate_acls method which is attached to the @post on this
# services does this as well as checking against the groups. So, this acl
# should be unnecessary at the moment.
#acl=bodhi.security.package_maintainers_only_acl,
acl=bodhi.security.packagers_allowed_acl,
cors_origins=bodhi.security.cors_origins_ro)

update_edit = Service(name='update_edit', path='/updates/{id}/edit',
validators=(validate_update_id,),
description='Update submission service',
#acl=bodhi.security.package_maintainers_only_acl,
acl=bodhi.security.packagers_allowed_acl,
cors_origins=bodhi.security.cors_origins_rw)

@@ -74,11 +67,9 @@

update_request = Service(name='update_request', path='/updates/{id}/request',
description='Update request service',
#acl=bodhi.security.package_maintainers_only_acl,
acl=bodhi.security.packagers_allowed_acl,
cors_origins=bodhi.security.cors_origins_rw)


@update.get(accept=('application/json', 'text/json'), renderer='json',
error_handler=bodhi.services.errors.json_handler)
@update.get(accept=('application/javascript'), renderer='jsonp',
@@ -87,7 +78,12 @@
error_handler=bodhi.services.errors.html_handler)
def get_update(request):
"""Return a single update from an id, title, or alias"""
can_edit = bool(has_permission('edit', request.context, request))

proxy_request = bodhi.security.ProtectedRequest(request)
validate_acls(proxy_request)
# If validate_acls produced 0 errors, then we can edit this update.
can_edit = len(proxy_request.errors) == 0

return dict(update=request.validated['update'], can_edit=can_edit)


@@ -248,6 +248,7 @@ def test_provenpackager_request_privs(self, publish, *args):
user = User(name=u'bob')
session.add(user)
session.add(User(name=u'ralph')) # Add a non proventester
session.add(User(name=u'someuser')) # An unrelated user with no privs
session.flush()
group = session.query(Group).filter_by(name=u'provenpackager').one()
user.groups.append(group)
@@ -320,6 +321,22 @@ def test_provenpackager_request_privs(self, publish, *args):
eq_(update.request, None)
eq_(update.status, UpdateStatus.obsolete)

# Test that bob has can_edit True, provenpackager
app = TestApp(main({}, testing=u'bob', **self.app_settings))
res = app.get('/updates/%s' % nvr, status=200)
eq_(res.json_body['can_edit'], True)

# Test that ralph has can_edit True, they submitted it.
app = TestApp(main({}, testing=u'ralph', **self.app_settings))
res = app.get('/updates/%s' % nvr, status=200)
eq_(res.json_body['can_edit'], True)

# Test that someuser has can_edit False, they are unrelated
# This check *failed* with the old acls code.
app = TestApp(main({}, testing=u'someuser', **self.app_settings))
res = app.get('/updates/%s' % nvr, status=200)
eq_(res.json_body['can_edit'], False)

@mock.patch(**mock_valid_requirements)
def test_pkgdb_outage(self, *args):
"Test the case where our call to the pkgdb throws an exception"
@@ -285,10 +285,10 @@ def validate_acls(request):
admin_groups = settings['admin_packager_groups'].split()
for group in admin_groups:
if group in user_groups:
log.debug(
'{} is in {} admin group'.format(user.name, group))
log.debug('{} is in {} admin group'.format(user.name, group))
has_access = True
break

if has_access:
continue

@@ -721,7 +721,6 @@ def validate_override_builds(request):
def _validate_override_build(request, nvr, db):
""" Workhorse function for validate_override_builds """
build = Build.get(nvr, db)
print "in _validate with build", build
if build is not None:
if not build.release:
# Oddly, the build has no associated release. Let's try to figure

0 comments on commit b5b1eb6

Please sign in to comment.
You can’t perform that action at this time.