6
\$\begingroup\$

I threw together this Python function to check if a User has a certain Permission.

First of all, Permissions have this kind of format: category1.category2.some.task

and 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 we are looking for
                    break 
                return True
    #nothing matched, return False. Permission we're looking for not in the list
    return False


permissions = ["app.cat1.*", "app.cat2.do.something", "app.cat2.cat3.*"]
assert check_permission("app.cat2.do.something", permissions) == True
assert check_permission("app.cat2.do.something.special", permissions) == False
assert check_permission("app.cat1.some.task", permissions) == True
assert check_permission("app.cat2.cat4", permissions) == False
assert check_permission("app.cat2.cat3.do.something.special", permissions) == True
assert check_permission("something.completely.different", permissions) == False
\$\endgroup\$
3
\$\begingroup\$

This is completely unnecessarily complicated:

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 True

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:

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 False

The 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 True

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:

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 False

Finally, 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 False
| improve this answer | |
\$\endgroup\$
0
\$\begingroup\$
needed_permissions_list = list(filter(None, needed_permission.split(".")))

can be

needed_permissions_list = [p for p in needed_permission.split('.') if p]
| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ Thank you for your answer, however regarding the part "I think will only work if you can assume that the permissions list is sorted." I don't think you got how the code works :) The code is iterating over all permission and then splits each up in a list based on the "." \$\endgroup\$ – Stefan Dec 20 '14 at 9:14
  • \$\begingroup\$ Ah, you're right. \$\endgroup\$ – Reinderien Dec 20 '14 at 14:26

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.