Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Permissions decorator running after pydantic validation #111

Open
simonjharris opened this issue Sep 22, 2023 · 4 comments
Open

Permissions decorator running after pydantic validation #111

simonjharris opened this issue Sep 22, 2023 · 4 comments
Labels
bug Something isn't working Stale

Comments

@simonjharris
Copy link

I'm looking into migrating from flask-pydantic to flask-openapi3, but one problem I'm having is around the authentication decorators we currently use. When using flask-openapi3 pydantic validation is taking place before the auth checks.

So when making an unauthenticated request I receive a 422 Unprocessable Entity response rather than 401 Unauthorized. If the request body is valid, the 401 response is returned, but an invalid request body yields a 422.

Is this expected behaviour?

example:

@user_admin_api.post(
    "/admin/users",
    summary="Create a user",
    responses={HTTPStatus.CREATED: UserIdResponse},
    tags=[user_admin_tag]
)
@permission_only("perm.users.create")
def create_user(body: UserCreationWrapper, user_id: str, **_):
    ...

Environment:

  • Python version: 3.9
  • Operating system: mac
  • Flask version: 2.0.3
  • flask-openapi3 version: 3.0.0rc1
@simonjharris simonjharris added the bug Something isn't working label Sep 22, 2023
@luolingchun
Copy link
Owner

luolingchun commented Sep 24, 2023

Can you write a detailed example?
Here's a decorator sample code for permission validation:

https://github.com/luolingchun/flask-api-demo/blob/master/src/app/api/user.py#L54

https://github.com/luolingchun/flask-api-demo/blob/master/src/app/api/admin.py#L31

https://github.com/luolingchun/flask-api-demo/blob/master/src/app/utils/jwt_tools.py#L52

By the way, user_id: str, **_ is a misuse of the attempt to support only a few parameters: path, query, form, body, header, cookie

Here is the help documentation: https://luolingchun.github.io/flask-openapi3/v2.x/Usage/Request/

@simonjharris
Copy link
Author

Here's a unit test MWE:

(This setup works with flask-pydantic)

import pytest
from pydantic import BaseModel

from flask_openapi3 import OpenAPI
from functools import wraps

app = OpenAPI(__name__)


class ForbiddenError(Exception):
    pass


class TestModel(BaseModel):
    field: str


@app.errorhandler(ForbiddenError)
def handle_forbidden_error(e):
    return "Forbidden", 403


@pytest.fixture
def client():
    client = app.test_client()

    yield client


def intercept():
    def middle(f):
        @wraps(f)
        def wrapper(*args, **kwargs):
            raise ForbiddenError()

        return wrapper

    return middle


@app.post("/forbidden")
@intercept()
def forbidden_route(body: TestModel):
    return "success", 200


def test_forbidden_error(client):
    r = client.post("/forbidden")
    assert r.status_code == 403
    assert r.text == "Forbidden"
test_interception_decorator.py::test_forbidden_error FAILED              [100%]
test_interception_decorator.py:49 (test_forbidden_error)
422 != 403

Expected :403
Actual   :422
<Click to see difference>

client = <FlaskClient <OpenAPI 'test_interception_decorator'>>

    def test_forbidden_error(client):
        r = client.post("/forbidden")
>       assert r.status_code == 403
E       assert 422 == 403
E        +  where 422 = <WrapperTestResponse streamed [422 UNPROCESSABLE ENTITY]>.status_code

test_interception_decorator.py:52: AssertionError

@luolingchun
Copy link
Owner

I compared flask-openapi3 and flask-pydantic and came to the following conclusion:

  1. flask-pydantic uses a standalone @validate() decorator, so you can choose the @intercept() execution order.
    # Execute @validate() first and then @intercept(), 
    # which will throw a validation error, just like flask-openapi3 here.
    @app.route("/", methods=["GET"])
    @validate()
    @intercept()
    def get(query: QueryModel):
        ...
    
    
    # Execute @intercept() first and then @validate(), @intercept() throws an exception.
    @app.route("/", methods=["GET"])
    @intercept()
    @validate()
    def get(query: QueryModel):
        ...
  2. flask-openapi3 does not have a @validate() decorator, so it must pass parameter validation before continuing execution.

@simonjharris
Copy link
Author

Thanks for your reply, I see your point here.

I did a bit more testing and noticed that if I remove the @wraps() within intercept, the test passes. I'm not sure if this expected behaviour.

@github-actions github-actions bot added the Stale label Dec 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Stale
Projects
None yet
Development

No branches or pull requests

2 participants