Skip to content

Commit

Permalink
review fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
drehelis committed Dec 11, 2020
1 parent 6379e61 commit 9f40c57
Show file tree
Hide file tree
Showing 7 changed files with 77 additions and 15 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
1 change: 1 addition & 0 deletions src/model/external_model.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ def parameter_to_external(parameter):
'type': parameter.type,
'min': parameter.min,
'max': parameter.max,
'max_length': parameter.max_length,
'values': parameter.values,
'secure': parameter.secure,
'fileRecursive': parameter.file_recursive,
Expand Down
7 changes: 6 additions & 1 deletion src/model/parameter_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
'type',
'min',
'max',
'max_length',
'constant',
'_values_provider',
'values',
Expand Down Expand Up @@ -71,6 +72,7 @@ def _reload(self):
self.required = read_bool_from_config('required', config, default=False)
self.min = config.get('min')
self.max = config.get('max')
self.max_length = config.get('max_length')
self.secure = read_bool_from_config('secure', config, default=False)
self.separator = config.get('separator', ',')
self.multiple_arguments = read_bool_from_config('multiple_arguments', config, default=False)
Expand Down Expand Up @@ -277,6 +279,9 @@ def validate_value(self, value, *, ignore_required=False):
return None

if self.type == 'text':
if (not is_empty(self.max_length)) and (len(value) > int(self.max_length)):
return 'is longer than allowed char length (' \
+ str(len(value)) + ' > ' + str(self.max_length) + ')'
return None

if self.type == 'file_upload':
Expand All @@ -291,7 +296,7 @@ def validate_value(self, value, *, ignore_required=False):
int_value = int(value)

if (not is_empty(self.max)) and (int_value > int(self.max)):
return 'is greater than allowed value (' \
return 'is longer than allowed value (' \
+ value_string + ' > ' + str(self.max) + ')'

if (not is_empty(self.min)) and (int_value < int(self.min)):
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
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,7 @@
secure: 'secure',
min: 'min',
max: 'max',
max_length: 'max_length',
multipleArguments: 'multiple_arguments',
sameArgParam: 'same_arg_param',
separator: 'separator',
Expand Down Expand Up @@ -169,6 +170,7 @@
description: null,
min: null,
max: null,
max_length: null,
allowedValues: null,
allowedValuesScript: null,
allowedValuesFromScript: null,
Expand All @@ -193,6 +195,7 @@
descriptionField,
minField,
maxField: Object.assign({}, maxField),
maxLengthField,
allowedValuesScriptField,
allowedValuesFromScriptField,
defaultValueField: Object.assign({}, defaultValueField),
Expand Down Expand Up @@ -221,6 +224,7 @@
this.required = get(config, 'required', false);
this.min = config['min'];
this.max = config['max'];
this.max_length = config['max_length'];
this.constant = !!get(config, 'constant', false);
this.secure = !!get(config, 'secure', false);
this.multipleArguments = !!get(config, 'multiple_arguments', false);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,11 @@ export const allowedValuesScriptField = {
required: true
};

export const maxCharsField = {
name: 'Max Characters',
type: 'int'
};

export const allowedValuesFromScriptField = {
name: 'Load from script',
withoutValue: true,
Expand Down
10 changes: 8 additions & 2 deletions web-src/src/common/components/textfield.vue
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@
}
if (!empty) {
var typeError = getValidByTypeError(value, this.config.type, this.config.min, this.config.max);
var typeError = getValidByTypeError(value, this.config.type, this.config.min, this.config.max, this.config.max_length);
if (!isEmptyString(typeError)) {
return typeError;
}
Expand Down Expand Up @@ -149,7 +149,13 @@
}
}
function getValidByTypeError(value, type, min, max) {
function getValidByTypeError(value, type, min, max, max_length) {
if (type === 'text') {
if (value.length > max_length) {
return 'Max chars allowed: ' + max_length
}
}
if (type === 'int') {
const isInteger = /^(((-?[1-9])(\d*))|0)$/.test(value);
if (!isInteger) {
Expand Down

0 comments on commit 9f40c57

Please sign in to comment.