patternpythonMinor
Activity-based permission checking
Viewed 0 times
activitypermissioncheckingbased
Problem
I threw together this Python function to check if a User has a certain Permission.
First of all, Permissions have this kind of format:
and can also include wildcards like this:
All the permissions a User has are stored in a list, so for example:
You can then use the function below to check if a User has a certain Permission in his list:
```
def check_permission(needed_permission, permissions):
#normalise needed permissions ending with a "." dot, convert filter to list again!
needed_permissions_list = list(filter(None, needed_permission.split(".")))
#loop over the permissions the user has, check if each matches the needed one until we find one and return True
for permission in permissions:
#permission is the permission the user already has and we are working on
#permission_list is the list representation of the permission mentioned above
permission_list = permission.split(".")
#if the needed permission is exactly the same as the one we are working on, return True. E.g. needed: app., in list: app.
if permission_list == needed_permissions_list:
return True
#needed permission nearly matches, not the same because of wildcard. Return True
if permission_list[-1] == "*" and (permission_list[:-1] == needed_permissions_list):
return True
#zip the to lists together to iterate
zipped = zip(permission_list, needed_permissions_list)
#iterate over the two lists simultaneousley
for item, item2 in zipped:
#whoa, an item doesn't match, time for further checks
if item != item2:
if item == "*":
#it's not matching because it's a wildcard, so return True
return True
else:
#it's not a wildcard, so it's definetely not the permission
First of all, Permissions have this kind of format:
category1.category2.some.taskand can also include wildcards like this:
app.view.*All the permissions a User has are stored in a list, so for example:
["app.cat1.*", "app.cat2.do.something", "app.cat2.cat3.*"]You can then use the function below to check if a User has a certain Permission in his list:
```
def check_permission(needed_permission, permissions):
#normalise needed permissions ending with a "." dot, convert filter to list again!
needed_permissions_list = list(filter(None, needed_permission.split(".")))
#loop over the permissions the user has, check if each matches the needed one until we find one and return True
for permission in permissions:
#permission is the permission the user already has and we are working on
#permission_list is the list representation of the permission mentioned above
permission_list = permission.split(".")
#if the needed permission is exactly the same as the one we are working on, return True. E.g. needed: app., in list: app.
if permission_list == needed_permissions_list:
return True
#needed permission nearly matches, not the same because of wildcard. Return True
if permission_list[-1] == "*" and (permission_list[:-1] == needed_permissions_list):
return True
#zip the to lists together to iterate
zipped = zip(permission_list, needed_permissions_list)
#iterate over the two lists simultaneousley
for item, item2 in zipped:
#whoa, an item doesn't match, time for further checks
if item != item2:
if item == "*":
#it's not matching because it's a wildcard, so return True
return True
else:
#it's not a wildcard, so it's definetely not the permission
Solution
This is completely unnecessarily complicated:
This is equivalent:
The
so you can delete that line.
This is unnecessary:
This treats a special case, that the rest of the method automatically handles.
You can safely remove this part.
This is a shorter and simpler implementation:
The assertions are not written in a Pythonic way:
you should avoid comparisons with
This is the Pythonic way:
Your spec is not very clear.
For example you didn't mention if the pattern
Probably not valid, because your code doesn't handle the case (for example
Effectively, a pattern like
It's good to specify this clearly.
I don't really like this special treatment:
Because this check has an overlap with the logic in the for loop:
if the two lists are not identical,
the loop will repeat some of the checks.
It would be better to get rid of this overlap,
see this alternative implementation:
Finally,
another alternative implementation that also passes your assertions and somewhat simpler, taking a different approach without splitting to segments:
needed_permissions_list = list(filter(None, needed_permission.split(".")))This is equivalent:
needed_permissions_list = needed_permission.split(".")The
return True at the end of the if item != item2 is unreachable,so you can delete that line.
item2 is not a good name. needed_item would be better, so you don't mix up which is which. It's important to know which is which, because the needed item must not contain *.This is unnecessary:
if permission_list[-1] == "*" and (permission_list[:-1] == needed_permissions_list):
return TrueThis treats a special case, that the rest of the method automatically handles.
You can safely remove this part.
This is a shorter and simpler implementation:
def check_permission(needed_permission, permissions):
needed_permissions_list = needed_permission.split(".")
for permission in permissions:
permission_list = permission.split(".")
if permission_list == needed_permissions_list:
return True
zipped = zip(permission_list, needed_permissions_list)
for item, needed_item in zipped:
if item != needed_item:
if item == "*":
return True
else:
break
return FalseThe assertions are not written in a Pythonic way:
you should avoid comparisons with
True and False.This is the Pythonic way:
assert check_permission("app.cat2.do.something", permissions)
assert not check_permission("app.cat2.do.something.special", permissions)
assert check_permission("app.cat1.some.task", permissions)
assert not check_permission("app.cat2.cat4", permissions)
assert check_permission("app.cat2.cat3.do.something.special", permissions)
assert not check_permission("something.completely.different", permissions)Your spec is not very clear.
For example you didn't mention if the pattern
a.*.c is valid.Probably not valid, because your code doesn't handle the case (for example
a.b.x will yield true).Effectively, a pattern like
a..c is handled as if it was a..It's good to specify this clearly.
I don't really like this special treatment:
if permission_list == needed_permissions_list:
return TrueBecause this check has an overlap with the logic in the for loop:
if the two lists are not identical,
the loop will repeat some of the checks.
It would be better to get rid of this overlap,
see this alternative implementation:
def check_permission2(needed_permission, permissions):
needed_permissions_list = needed_permission.split(".")
for permission in permissions:
permission_list = permission.split(".")
zipped = zip(permission_list, needed_permissions_list)
for item, needed_item in zipped:
if item != needed_item:
if item == "*":
return True
else:
break
else:
return len(permission_list) == len(needed_permissions_list)
return FalseFinally,
another alternative implementation that also passes your assertions and somewhat simpler, taking a different approach without splitting to segments:
def check_permission(needed_permission, permissions):
for permission in permissions:
index_of_star = permission.find('*')
if index_of_star == -1:
if needed_permission == permission:
return True
else:
normalized = permission[:index_of_star]
if needed_permission.startswith(normalized):
return True
return FalseCode Snippets
needed_permissions_list = list(filter(None, needed_permission.split(".")))needed_permissions_list = needed_permission.split(".")if permission_list[-1] == "*" and (permission_list[:-1] == needed_permissions_list):
return Truedef check_permission(needed_permission, permissions):
needed_permissions_list = needed_permission.split(".")
for permission in permissions:
permission_list = permission.split(".")
if permission_list == needed_permissions_list:
return True
zipped = zip(permission_list, needed_permissions_list)
for item, needed_item in zipped:
if item != needed_item:
if item == "*":
return True
else:
break
return Falseassert check_permission("app.cat2.do.something", permissions)
assert not check_permission("app.cat2.do.something.special", permissions)
assert check_permission("app.cat1.some.task", permissions)
assert not check_permission("app.cat2.cat4", permissions)
assert check_permission("app.cat2.cat3.do.something.special", permissions)
assert not check_permission("something.completely.different", permissions)Context
StackExchange Code Review Q#74225, answer score: 3
Revisions (0)
No revisions yet.