Skip to content

Commit

Permalink
#367 made user authorization case-insensitive
Browse files Browse the repository at this point in the history
  • Loading branch information
bugy committed Nov 18, 2020
1 parent 6379e61 commit 85f923c
Show file tree
Hide file tree
Showing 2 changed files with 53 additions and 12 deletions.
45 changes: 33 additions & 12 deletions src/auth/authorization.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,39 +5,60 @@
GROUP_PREFIX = '@'


def _normalize_user(user):
if user:
return user.lower()
return user


def _normalize_users(allowed_users):
if isinstance(allowed_users, list):
if ANY_USER in allowed_users:
return ANY_USER

return [_normalize_user(user) for user in allowed_users]

return allowed_users


class Authorizer:
def __init__(self, app_allowed_users, admin_users, full_history_users, groups_provider):
self._app_allowed_users = app_allowed_users
self._admin_users = admin_users
self._full_history_users = full_history_users
self._app_allowed_users = _normalize_users(app_allowed_users)
self._admin_users = _normalize_users(admin_users)
self._full_history_users = _normalize_users(full_history_users)

self._groups_provider = groups_provider

def is_allowed_in_app(self, user_id):
return self.is_allowed(user_id, self._app_allowed_users)
return self._is_allowed_internal(user_id, self._app_allowed_users)

def is_admin(self, user_id):
return self.is_allowed(user_id, self._admin_users)
return self._is_allowed_internal(user_id, self._admin_users)

def has_full_history_access(self, user_id):
return self.is_admin(user_id) or self.is_allowed(user_id, self._full_history_users)
return self.is_admin(user_id) or self._is_allowed_internal(user_id, self._full_history_users)

def is_allowed(self, user_id, allowed_users):
if not allowed_users:
normalized_users = _normalize_users(allowed_users)

return self._is_allowed_internal(user_id, normalized_users)

def _is_allowed_internal(self, user_id, normalized_allowed_users):
if not normalized_allowed_users:
return False

if (allowed_users == ANY_USER) or (ANY_USER in allowed_users):
if normalized_allowed_users == ANY_USER:
return True

if user_id in allowed_users:
if _normalize_user(user_id) in normalized_allowed_users:
return True

user_groups = self._groups_provider.get_groups(user_id)
if not user_groups:
return False

for group in user_groups:
if (GROUP_PREFIX + group) in allowed_users:
if _normalize_user(GROUP_PREFIX + group) in normalized_allowed_users:
return True

return False
Expand Down Expand Up @@ -92,10 +113,10 @@ def __init__(self, groups) -> None:
if member.startswith(GROUP_PREFIX):
self._lazy_group_parents[member[1:]].append(group)
else:
self._user_groups[member].append(group)
self._user_groups[_normalize_user(member)].append(group)

def get_groups(self, user, known_groups=None):
user_groups = set(self._user_groups[user])
user_groups = set(self._user_groups[_normalize_user(user)])

if known_groups:
for known_group in known_groups:
Expand Down
20 changes: 20 additions & 0 deletions src/tests/authorization_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@ class TestIsAllowed(unittest.TestCase):
def test_allowed_from_single_user(self):
self.assertTrue(self.authorizer.is_allowed('user1', ['user1']))

def test_allowed_from_single_user_ignore_case(self):
self.assertTrue(self.authorizer.is_allowed('USer1', ['usER1']))

def test_not_allowed_from_single_user(self):
self.assertFalse(self.authorizer.is_allowed('user1', ['user2']))

Expand All @@ -22,6 +25,10 @@ def test_allowed_from_single_group(self):
self.user_groups['user1'] = ['group1']
self.assertTrue(self.authorizer.is_allowed('user1', ['@group1']))

def test_allowed_from_single_group_ignore_case(self):
self.user_groups['user1'] = ['Group1']
self.assertTrue(self.authorizer.is_allowed('user1', ['@groUP1']))

def test_not_allowed_from_single_group_invalid_name_in_provider(self):
self.user_groups['user1'] = ['@group1']
self.assertFalse(self.authorizer.is_allowed('user1', ['@group1']))
Expand Down Expand Up @@ -66,6 +73,9 @@ class TestIsAllowedInApp(unittest.TestCase):
def test_single_user_allowed(self):
self.assertAllowed('user1', ['user1'], True)

def test_single_user_allowed_ignore_case(self):
self.assertAllowed('User1', ['uSEr1'], True)

def test_multiple_users_allowed(self):
self.assertAllowed('user2', ['user1', 'user2', 'user3'], True)

Expand Down Expand Up @@ -98,6 +108,9 @@ class TestIsAdmin(unittest.TestCase):
def test_single_admin_allowed(self):
self.assertAdmin('admin1', ['admin1'], True)

def test_single_admin_allowed_ignore_case(self):
self.assertAdmin('adMin1', ['AdmiN1'], True)

def test_multiple_admins_allowed(self):
self.assertAdmin('admin2', ['admin1', 'admin2', 'admin3'], True)

Expand Down Expand Up @@ -130,6 +143,9 @@ class TestHistoryAccess(unittest.TestCase):
def test_user_in_the_list(self):
self.assert_has_access('user1', [], ['user1'], True)

def test_user_in_the_list_ignore_case(self):
self.assert_has_access('useR1', [], ['UsEr1'], True)

def test_any_user_allowed(self):
self.assert_has_access('user2', [], [ANY_USER], True)

Expand Down Expand Up @@ -163,6 +179,10 @@ def test_single_user_in_single_group(self):
provider = PreconfiguredGroupProvider({'group1': ['user1']})
self.assertCountEqual(provider.get_groups('user1'), ['group1'])

def test_single_user_in_single_group_ignore_case(self):
provider = PreconfiguredGroupProvider({'group1': ['USER1']})
self.assertCountEqual(provider.get_groups('User1'), ['group1'])

def test_two_users_in_different_groups(self):
provider = PreconfiguredGroupProvider(
{'group1': ['user1'],
Expand Down

0 comments on commit 85f923c

Please sign in to comment.