Detect if route is not outermost / wrong "order" of decorators in Flask

Question

Since the @route decorator has to register the view with the current callback given to the decorator, it has to be the outermost decorator to receive the correct function to invoke when handling a request.

This creates a possible situation where a view has been decorated, but since the decorators are in the wrong order, the decorated function is not invoked. If used for decorating views that require the user to be logged in, have a certain role or have a specific flag, the check will be left out silently.

Our current fix is to have the standard action be to deny access to the resource, then requiring a decorator to allow access. In that case, if the decorator isn't invoked when the request is being handled, the request will fail.

But there are use-cases where this becomes cumbersome since it requires you to decorate all views, except for those few that should be exempt. For a pure hierarchical layout this may work, but for checking single flags the structure can get complicated.

Is there a proper way to detect that we're being invoked in a useful place in the decoratory hierarchy? I.e. can we detect that there hasn't already been a route decorator applied to function we get to wrap?

# wrapped in wrong order - @require_administrator should be after @app.route
@require_administrator
@app.route('/users', methods=['GET'])

Implemented as:

def require_administrator(func):
    @functools.wraps(func)
    def has_administrator(*args, **kwargs):
        if not getattr(g, 'user') or not g.user.is_administrator:
            abort(403)

        return func(*args, **kwargs)

    return has_administrator

Here I'd like to detect if my custom decorator is being wrapped after @app.route, and thus, never will be invoked when the request is handled.

Using functools.wraps replaces the wrapped function with the new one in all ways, so looking at __name__ of the function to be wrapped will fail. This also happens at each step of the decorator wrapping process.

I've tried looking at both traceback and inspect, but haven't found any decent way of determining if the sequence is correct.

Update

My currently best solution is to check the called function name against the set of registered endpoints. But since a Route() decorator can change the name of the endpoint, I'll have to support that for my decorator as well in that case, and it'll silently pass if a different function has used the same endpoint name as the current function.

It also have to iterate the set of registered endpoints since I weren't able to find a simple way to check if just the endpoint name exists (possibly more efficient by attempting to build an URL with it and catch the exception).

def require_administrator_checked(func):
    for rule in app.url_map.iter_rules():
        if func.__name__ == rule.endpoint:
            raise DecoratorOrderError(f"Wrapped endpoint '{rule.endpoint}' has already been registered - wrong order of decorators?")

    # as above ..

Answer 1

Update 2: See my other answer for a more reusable, less hack-y solution.

Update: Here is an decidedly less hack-y solution. However, it requires you to use a custom function instead of app.route. It takes an arbitrary number of decorators, and applies them in the order given, and then makes sure that app.route is called as the final function. This requires that you use only this decorator for every function.

def safe_route(rule, app, *decorators, **options):
    def _route(func):
        for decorator in decorators:
            func = decorator(func)
        return app.route(rule, **options)(func)
    return _route

You can then use it like this:

def require_administrator(func):
    @functools.wraps(func)
    def has_administrator(*args, **kwargs):
        print("Would check admin now")

        return func(*args, **kwargs)

    return has_administrator

@safe_route("/", app, require_administrator, methods=["GET"])
def test2():
    return "foo"

test2()
print(test2.__name__)

This prints:

Would check admin now
foo
test2

So if all supplied decorators use functools.wraps, this also conserves the test2 name.

Old answer: If you are OK with an admittedly hack-y solution, you can roll your own inspection by reading the file line by line. Here is a very rough function that does this. You can refine this quite a bit, e.g. at the moment it relies on the app being called "app", function definitions having at least one empty line before them (normal PEP-8 behavior, but still might be an issue), ...

Here is the complete code I used to test it.

import flask
import functools
from itertools import groupby


class DecoratorOrderError(TypeError):
    pass


app = flask.Flask(__name__)


def require_administrator(func):
    @functools.wraps(func)
    def has_administrator(*args, **kwargs):
        print("Would check admin now")

        return func(*args, **kwargs)

    return has_administrator


@require_administrator  # Will raise a custom exception
@app.route("/", methods=["GET"])
def test():
    return "ok"


def check_route_is_topmost_decorator():
    # Read own source
    with open(__file__) as f:
        content = [line.strip() for line in f.readlines()]

    # Split source code on line breaks
    split_by_lines = [list(group) for k, group in groupby(content, lambda x: x == "") if not k]

    # Find consecutive decorators
    decorator_groups = dict()
    for line_group in split_by_lines:
        decorators = []
        for line in line_group:
            if line.startswith("@"):
                decorators.append(line)
            elif decorators:
                decorator_groups[line] = decorators
                break
            else:
                break

    # Check if app.route is the last one (if it exists)
    for func_def, decorators in decorator_groups.items():
        is_route = [dec.startswith("@app.route") for dec in decorators]
        if sum(is_route) > 1 or (sum(is_route) == 1 and not decorators[0].startswith("@app.route")):
            raise DecoratorOrderError(f"@app.route is not the topmost decorator for '{func_def}'")


check_route_is_topmost_decorator()

This snippet will give you the following error:

Traceback (most recent call last):
  File "/home/vXYZ/test_sso.py", line 51, in <module>
    check_route_is_topmost_decorator()
  File "/home/vXYZ/test_sso.py", line 48, in check_route_is_topmost_decorator
    raise DecoratorOrderError(f"@app.route is not the topmost decorator for '{func_def}'")
__main__.DecoratorOrderError: @app.route is not the topmost decorator for 'def test():'

If you switch the order of the decorator for the test() function, it simply does nothing.

One downside is that you have to call this method explicitly in every file. I don't exactly know how reliable this is, I admit it is pretty ugly, and I won't take any responsibility if it breaks, but it's a start! I am sure there must be a better way.

Answer 2

I am adding another answer, because now I have something that is the least amount of hacky (read: I am using inspect to read to source code of a given function instead of reading the whole file myself), works across modules, and can be reused for any other decorators that should always be the last one. You also do not have to use a different syntax for app.route as in the update of my other answer.

Here is how to do this (Warning: This is quite a closure-inception):

import flask
import inspect


class DecoratorOrderError(TypeError):
    pass


def assert_last_decorator(final_decorator):
    """
    Converts a decorator so that an exception is raised when it is not the last    decorator to be used on a function.
    This only works for decorator syntax, not if somebody explicitly uses the decorator, e.g.
    final_decorator = some_other_decorator(final_decorator) will still work without an exception.

    :param final_decorator: The decorator that should be made final.
    :return: The same decorator, but it checks that it is the last one before calling the inner function.
    """
    def check_decorator_order(func):
        # Use inspect to read the code of the function
        code, _ = inspect.getsourcelines(func)
        decorators = []
        for line in code:
            if line.startswith("@"):
                decorators.append(line)
            else:
                break

        # Remove the "@", function calls, and any object calls, such as "app.route". We just want the name of the decorator function (e.g. "route")
        decorator_names_only = [dec.replace("@", "").split("(")[0].split(".")[-1] for dec in decorators]
        is_final_decorator = [final_decorator.__name__ == name for name in decorator_names_only]
        num_finals = sum(is_final_decorator)

        if num_finals > 1 or (num_finals == 1 and not is_final_decorator[0]):
            raise DecoratorOrderError(f"'{final_decorator.__name__}' is not the topmost decorator of function '{func.__name__}'")

        return func

    def handle_arguments(*args, **kwargs):
        # Used to pass the arguments to the final decorator

        def handle_function(f):
            # Which function should be decorated by the final decorator?
            return final_decorator(*args, **kwargs)(check_decorator_order(f))

        return handle_function

    return handle_arguments

You can now replace the app.route function with this function, applied to the app.route function. This is important and has to be done before any use of the app.route decorator, so I suggest to just do it when creating the app.

app = flask.Flask(__name__)
app.route = assert_last_decorator(app.route)


def require_administrator(func):
    @functools.wraps(func)
    def has_administrator(*args, **kwargs):
        print("Would check admin now")

        return func(*args, **kwargs)

    return has_administrator


@app.route("/good", methods=["GET"])  # Works
@require_administrator
def test_good():
    return "ok"

@require_administrator
@app.route("/bad", methods=["GET"])  # Raises an Exception
def test_bad():
    return "not ok"

I believe this is pretty much what you wanted in your question.