Skip to content

Commit

Permalink
Use role in insensitive case in keystoneauth.
Browse files Browse the repository at this point in the history
Using insensitive case could tolerate human error. For example,
user maybe set like this "operator_roles = Admin, swiftoperator"

- also fix a mistake in test, ['admin'] is correct value for roles, not
  'admin' (it will be looped as ['a', 'd', 'm', 'i', 'n'])
- add test for insensitive cases

Fixes: bug #1013120
Change-Id: I56d71da8bc503e48e92dd743692ba6fc237f029e
(cherry picked from commit 7dd9661)
  • Loading branch information
pyKun authored and notmyname committed Mar 27, 2013
1 parent 9e777bd commit 80ea2d7
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 6 deletions.
11 changes: 6 additions & 5 deletions swift/common/middleware/keystoneauth.py
Original file line number Diff line number Diff line change
Expand Up @@ -81,9 +81,9 @@ def __init__(self, app, conf):
self.logger = swift_utils.get_logger(conf, log_route='keystoneauth')
self.reseller_prefix = conf.get('reseller_prefix', 'AUTH_').strip()
self.operator_roles = conf.get('operator_roles',
'admin, swiftoperator')
'admin, swiftoperator').lower()
self.reseller_admin_role = conf.get('reseller_admin_role',
'ResellerAdmin')
'ResellerAdmin').lower()
config_is_admin = conf.get('is_admin', "false").lower()
self.is_admin = swift_utils.config_true_value(config_is_admin)
config_overrides = conf.get('allow_overrides', 't').lower()
Expand All @@ -106,7 +106,8 @@ def __call__(self, environ, start_response):
environ['keystone.identity'] = identity
environ['REMOTE_USER'] = identity.get('tenant')
environ['swift.authorize'] = self.authorize
if self.reseller_admin_role in identity.get('roles', []):
user_roles = (r.lower() for r in identity.get('roles', []))
if self.reseller_admin_role in user_roles:
environ['reseller_request'] = True
else:
self.logger.debug('Authorizing as anonymous')
Expand Down Expand Up @@ -175,7 +176,7 @@ def authorize(self, req):
except ValueError:
return HTTPNotFound(request=req)

user_roles = env_identity.get('roles', [])
user_roles = [r.lower() for r in env_identity.get('roles', [])]

# Give unconditional access to a user with the reseller_admin
# role.
Expand Down Expand Up @@ -230,7 +231,7 @@ def authorize(self, req):

# Check if we have the role in the userroles and allow it
for user_role in user_roles:
if user_role in roles:
if user_role in (r.lower() for r in roles):
log_msg = 'user %s:%s allowed in ACL: %s authorizing'
self.logger.debug(log_msg % (tenant_name, user, user_role))
return
Expand Down
14 changes: 13 additions & 1 deletion test/unit/common/middleware/test_keystoneauth.py
Original file line number Diff line number Diff line change
Expand Up @@ -206,8 +206,20 @@ def test_authorize_succeeds_for_reseller_admin(self):
req = self._check_authenticate(identity=identity)
self.assertTrue(req.environ.get('swift_owner'))

def test_authorize_succeeds_for_insensitive_reseller_admin(self):
roles = [self.test_auth.reseller_admin_role.upper()]
identity = self._get_identity(roles=roles)
req = self._check_authenticate(identity=identity)
self.assertTrue(req.environ.get('swift_owner'))

def test_authorize_succeeds_as_owner_for_operator_role(self):
roles = self.test_auth.operator_roles.split(',')[0]
roles = self.test_auth.operator_roles.split(',')
identity = self._get_identity(roles=roles)
req = self._check_authenticate(identity=identity)
self.assertTrue(req.environ.get('swift_owner'))

def test_authorize_succeeds_as_owner_for_insensitive_operator_role(self):
roles = [r.upper() for r in self.test_auth.operator_roles.split(',')]
identity = self._get_identity(roles=roles)
req = self._check_authenticate(identity=identity)
self.assertTrue(req.environ.get('swift_owner'))
Expand Down

0 comments on commit 80ea2d7

Please sign in to comment.