Assigning permissions to multiple endpoints for a user and company

Question

origin code:

# add this class to make the code can be a reproducible example.In acutually environment all attribute could be True or False.
class Capabilities:
    def __init__(self) -> None:
        self.canEditCompany = True
        self.canDeleteCompany = True
        self.canShareCompany = True
        self.canAddFiscal = True
        self.canAddCategory = True
        self.canEditCategory = True
        self.canGetCategory = True
        self.canAddAccount = True
        self.canGetAccount = True
        self.canEditAccount = True
        self.canAddTransaction = True
        self.canAddScan = True
        self.canAddTax = True

def permission_control(user_email,company):
    capabilities = Capabilities()
    casbin_roles = []
    if capabilities.canEditCompany:
        casbin_roles.append((user_email, company, 'PUT'))
    if capabilities.canDeleteCompany:
        casbin_roles.append((user_email, company, 'Delete'))
    if capabilities.canShareCompany:
        casbin_roles.append((user_email, f'{company}/sharing', 'Post'))
    if capabilities.canAddFiscal:
        casbin_roles.append((user_email, f'{company}/fiscal_year', 'POST'))
    if capabilities.canAddCategory:
        casbin_roles.append((user_email, f'{company}/fiscals/*/categories', 'Post'))
    if capabilities.canEditCategory:
        casbin_roles.append((user_email, f'{company}/fiscals/*/categories/*', 'PUT'))
    if capabilities.canGetCategory:
        casbin_roles.append((user_email, f'{company}/fiscals/*/categories', 'GET'))
    if capabilities.canAddAccount:
        casbin_roles.append((user_email, f'{company}/fiscals/*/accounts', 'POST'))
    if capabilities.canGetAccount:
        casbin_roles.append((user_email, f'{company}/fiscals/*/accounts', 'GET'))
    if capabilities.canEditAccount:
        casbin_roles.append((user_email, f'{company}/fiscals/*/accounts/*', 'PUT'))
    if capabilities.canAddAccount:
        casbin_roles.append((user_email, f'{company}/fiscals/*/account', 'POST'))
    if capabilities.canAddTax:
        casbin_roles.append((user_email, f'{company}/tax', 'POST'))
    if capabilities.canAddScan:
        casbin_roles.append((user_email, f'{company}/scan', 'POST'))
    if capabilities.canAddTransaction:
        casbin_roles.append((user_email, f'{company}/accounts/*', 'POST'))
    return casbin_roles

my try:

import functools
def permission_control(user_email,company):

    def partial(attribute_name):
        return functools.partial(getattr,capabilities,attribute_name)
        
    capabilities = Capabilities()
    permissions = ["canEditCompany","canDeleteCompany","canShareCompany","canAddFiscal","canAddCategory","canEditCategory","canGetCategory","canAddAccount","canGetAccount","canEditAccount","canAddAccount","canAddTax","canAddScan","canAddTransaction"]
    urls = [(company, 'PUT'),(company, 'Delete'),(f'{company}/sharing', 'Post'),(f'{company}/fiscal_year', 'POST'),(f'{company}/fiscals/*/categories', 'Post'),(f'{company}/fiscals/*/categories/*', 'PUT'),(f'{company}/fiscals/*/categories', 'GET'),(f'{company}/fiscals/*/accounts', 'POST'),(f'{company}/fiscals/*/accounts', 'GET'),(f'{company}/fiscals/*/accounts/*', 'PUT'),(f'{company}/fiscals/*/account', 'POST'),(f'{company}/tax', 'POST'),(f'{company}/scan', 'POST'),(f'{company}/accounts/*', 'POST')]
    return [((user_email,) + url) for permission,url in zip(permissions,urls) if partial(permission)()]

question

• Any better, more pythonic way to achieve it?
• I think my code maybe even worse than the origin code, because it is hard to understand/maintain for my co-workers. Am I right?

Answer 1

I'd dumb it down a little: no lambdas, no partials, no formatting (you only have suffixes and nothing else). Make your HTTP methods all-caps for uniformity, and also you can use a generator instead of a list comprehension. For your Capabilities you can accept boolean kwargs. Pre-register all of the possible capabilities in a constant dictionary roughly similar to Alex's actions_dict but with stronger typing.

Also note permission_control is a noun where method names should be verbs, i.e. control_permissions or, as I've proposed, .evaluate()

Suggested

from dataclasses import dataclass
from typing import Tuple, Iterable

CapTuple = Tuple[
    str,  # email
    str,  # URL path
    str,  # method
]


@dataclass(frozen=True)
class Capability:
    name: str
    method: str
    path: str


CAPABILITIES = {
    name: Capability(name, method, path)
    for name, method, path in (
        ("canEditCompany",    'PUT',    ''),
        ("canDeleteCompany",  'DELETE', ''),
        ("canShareCompany",   'POST',   '/sharing'),
        ("canAddFiscal",      'POST',   '/fiscal_year'),
        ("canAddCategory",    'POST',   '/fiscals/*/categories'),
        ("canEditCategory",   'PUT',    '/fiscals/*/categories/*'),
        ("canGetCategory",    'GET',    '/fiscals/*/categories'),
        ("canAddAccount",     'POST',   '/fiscals/*/accounts'),
        ("canGetAccount",     'GET',    '/fiscals/*/accounts'),
        ("canEditAccount",    'PUT',    '/fiscals/*/accounts/*'),
        ("canAddAccount",     'POST',   '/fiscals/*/account'),
        ("canAddTax",         'POST',   '/tax'),
        ("canAddScan",        'POST',   '/scan'),
        ("canAddTransaction", 'POST',   '/accounts/*'),
    )
}


class Capabilities:
    def __init__(self, **kwargs: bool):
        self.allowed: Tuple[Capability] = tuple(
            CAPABILITIES[name]
            for name, allowed in kwargs.items()
            if allowed
        )

    def evaluate(self, user_email: str, company: str) -> Iterable[CapTuple]:
        for cap in self.allowed:
            yield user_email, company + cap.path, cap.method


def test() -> None:
    caps = Capabilities(canAddAccount=True, canEditAccount=True, canDeleteCompany=False)
    for user_email, path, method in caps.evaluate('me@here.com', 'here_inc'):
        print(user_email, path, method)


if __name__ == '__main__':
    test()

Answer 2

I might be inclined to do something like this:

class Capabilities:
    def __init__(self) -> None:
        self.canEditCompany = True
        self.canDeleteCompany = True
        self.canShareCompany = True
        self.canAddFiscal = True
        self.canAddCategory = True
        self.canEditCategory = True
        self.canGetCategory = True
        self.canAddAccount = True
        self.canGetAccount = True
        self.canEditAccount = True
        self.canAddTransaction = True
        self.canAddScan = True
        self.canAddTax = True

 actions_dict = {
    "canEditCompany":    lambda company: (company, 'PUT'),
    "canDeleteCompany":  lambda company: (company, 'Delete'),
    "canShareCompany":   lambda company: (f'{company}/sharing', 'Post'),
    "canAddFiscal":      lambda company: (f'{company}/fiscal_year', 'POST'),
    "canAddCategory":    lambda company: (f'{company}/fiscals/*/categories', 'Post'),
    "canEditCategory":   lambda company: (f'{company}/fiscals/*/categories/*', 'PUT'),
    "canGetCategory":    lambda company: (f'{company}/fiscals/*/categories', 'GET'),
    "canAddAccount":     lambda company: (f'{company}/fiscals/*/accounts', 'POST'),
    "canGetAccount":     lambda company: (f'{company}/fiscals/*/accounts', 'GET'),
    "canEditAccount":    lambda company: (f'{company}/fiscals/*/accounts/*', 'PUT'),
    "canAddAccount":     lambda company: (f'{company}/fiscals/*/account', 'POST'),
    "canAddTax":         lambda company: (f'{company}/tax', 'POST'),
    "canAddScan":        lambda company: (f'{company}/scan', 'POST'),
    "canAddTransaction": lambda company: (f'{company}/accounts/*', 'POST')
}


def permission_control(user_email, company):
    capabilities = Capabilities()
    return [(user_email, *action(company)) for capability, action in actions_dict.items() if getattr(capabilities, capability)]

I agree that your code was in some ways less readable, but I think you were heading in the right direction by taking out that awful if-tree, which definitely counts as an anti-pattern in my opinion. I think keeping the input and the result together in a dictionary is much more readable than keeping them in two separate lists, as you had them — and by using lambda functions, we can construct the dict in the global namespace so it doesn't have to be made afresh every time the function is called, which is what your code was doing with your two lists.

---

EDIT: Following a suggestion in the comments, an even better solution would be to use format-strings rather than lambdas:

class Capabilities:
    def __init__(self) -> None:
        self.canEditCompany = True
        self.canDeleteCompany = True
        self.canShareCompany = True
        self.canAddFiscal = True
        self.canAddCategory = True
        self.canEditCategory = True
        self.canGetCategory = True
        self.canAddAccount = True
        self.canGetAccount = True
        self.canEditAccount = True
        self.canAddTransaction = True
        self.canAddScan = True
        self.canAddTax = True

actions_dict = {
    "canEditCompany":    ('{}', 'PUT'),
    "canDeleteCompany":  ('{}', 'Delete'),
    "canShareCompany":   ('{}/sharing', 'Post'),
    "canAddFiscal":      ('{}/fiscal_year', 'POST'),
    "canAddCategory":    ('{}/fiscals/*/categories', 'Post'),
    "canEditCategory":   ('{}/fiscals/*/categories/*', 'PUT'),
    "canGetCategory":    ('{}/fiscals/*/categories', 'GET'),
    "canAddAccount":     ('{}/fiscals/*/accounts', 'POST'),
    "canGetAccount":     ('{}/fiscals/*/accounts', 'GET'),
    "canEditAccount":    ('{}/fiscals/*/accounts/*', 'PUT'),
    "canAddAccount":     ('{}/fiscals/*/account', 'POST'),
    "canAddTax":         ('{}/tax', 'POST'),
    "canAddScan":        ('{}/scan', 'POST'),
    "canAddTransaction": ('{}/accounts/*', 'POST')
}


def permission_control(user_email, company):
    capabilities = Capabilities()

    return [
        (user_email, url.format(company), method)
        for capability, (url, method) in actions_dict.items() 
        if getattr(capabilities, capability)
    ]

---

SECOND EDIT: As was pointed out in the comments, the answer with format strings could be improved even more by pre-binding the format method of the format strings. This is slightly more performant, and constrains the strings to a single purpose.

I've also changed the data structure from a str: tuple[str, str] dictionary to a list of NamedTuples. A dictionary no longer made sense at this stage. NamedTuples aren't strictly necessary as opposed to tuples, but I think using them makes the intention of the code clearer and improves readability.

from typing import NamedTuple, Callable, Literal

class Capabilities:
    def __init__(self) -> None:
        self.canEditCompany = True
        self.canDeleteCompany = True
        self.canShareCompany = True
        self.canAddFiscal = True
        self.canAddCategory = True
        self.canEditCategory = True
        self.canGetCategory = True
        self.canAddAccount = True
        self.canGetAccount = True
        self.canEditAccount = True
        self.canAddTransaction = True
        self.canAddScan = True
        self.canAddTax = True


class CapabilityTuple(NamedTuple):
    capability: str
    url_factory: Callable[[str], str]

    # Any reason why some of these aren't all-uppercase in your question?
    # HTTP methods are usually all-uppercase
    method: Literal['PUT', 'Delete', 'POST', 'GET', 'Post'] 


actions_list = [
    CapabilityTuple(capability, url.format, method) for  capability, url, method in (
        ("canEditCompany",    '{}',                        'PUT'),
        ("canDeleteCompany",  '{}',                        'Delete'),
        ("canShareCompany",   '{}/sharing',                'Post'),
        ("canAddFiscal",      '{}/fiscal_year',            'POST'),
        ("canAddCategory",    '{}/fiscals/*/categories',   'Post'),
        ("canEditCategory",   '{}/fiscals/*/categories/*', 'PUT'),
        ("canGetCategory",    '{}/fiscals/*/categories',   'GET'),
        ("canAddAccount",     '{}/fiscals/*/accounts',     'POST'),
        ("canGetAccount",     '{}/fiscals/*/accounts',     'GET'),
        ("canEditAccount",    '{}/fiscals/*/accounts/*',   'PUT'),
        ("canAddAccount",     '{}/fiscals/*/account',      'POST'),
        ("canAddTax",         '{}/tax',                    'POST'),
        ("canAddScan",        '{}/scan',                   'POST'),
        ("canAddTransaction", '{}/accounts/*',             'POST')
    )
]


def permission_control(user_email, company):
    capabilities = Capabilities()

    return [
        (user_email, url_factory(company), method)
        for capability, url_factory, method in actions_list
        if getattr(capabilities, capability)
    ]