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

Fix of TypeError when returning None in AuthBase.__call__ from async code #1386 #1387

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

mmkhitaryan
Copy link

The root cause

The function _run_authentication is made to run AuthBase.__call__ and execute what if returned. But for an example, the HttpBearer returns None in cases when a token is misplaced in the request:

    def __call__(self, request: HttpRequest) -> Optional[Any]:
        headers = request.headers
        auth_value = headers.get(self.header)
        if not auth_value:
            return None
        parts = auth_value.split(" ")

        if parts[0].lower() != self.openapi_scheme:
            if settings.DEBUG:
                logger.error(f"Unexpected auth - '{auth_value}'")
            return None
        token = " ".join(parts[1:])
        return self.authenticate(request, token)

Even though None is returned, the function _run_authentication tries to call async_to_sync without checking if the coroutine is None.

Because of the call to async_to_sync with None, instead of a coroutine, this TypeError is raised:

Traceback (most recent call last):
  File "C:\Users\Marat\Desktop\csmbackend\.venv\Lib\site-packages\ninja\operation.py", line 190, in _run_authentication
    result = async_to_sync(callback)(request)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\Users\Marat\Desktop\csmbackend\.venv\Lib\site-packages\asgiref\sync.py", line 254, in __call__
    return call_result.result()
           ^^^^^^^^^^^^^^^^^^^^
  File "C:\Users\Marat\AppData\Local\Programs\Python\Python312\Lib\concurrent\futures\_base.py", line 449, in result
    return self.__get_result()
           ^^^^^^^^^^^^^^^^^^^
  File "C:\Users\Marat\AppData\Local\Programs\Python\Python312\Lib\concurrent\futures\_base.py", line 401, in __get_result
    raise self._exception
  File "C:\Users\Marat\Desktop\csmbackend\.venv\Lib\site-packages\asgiref\sync.py", line 331, in main_wrap
    result = await self.awaitable(*args, **kwargs)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
TypeError: object NoneType can't be used in 'await' expression

Fix

Because we know that the coroutine is async:

if is_async_callable(callback) or getattr(callback, "is_async", False):

We can assume that calling it without putting the coroutine into the eventloop won't trigger the coroutine body. So we can safely call the coroutine without putting it into the eventloop, and check if the result is a coroutine, not a function:

if inspect.iscoroutine(initial_result):
    # If it's a coroutine, use async_to_sync to resolve it
    resolved_result = async_to_sync(callback)(request)
else:
    # If it's not a coroutine, use the initial result
    resolved_result = initial_result

That way we won't call async_to_sync with None, thus triggering TypeError.

@mmkhitaryan mmkhitaryan changed the title Fix of TypeError when returning None in AuthBase.__call__ from async code Fix of TypeError when returning None in AuthBase.__call__ from async code #1386 Jan 9, 2025
@vitalik
Copy link
Owner

vitalik commented Jan 12, 2025

Hi @mmkhitaryan

Thank you for PR

a few issues I see

  1. missing 100% coverage

  2. I thinik it can be simplified to

    def _run_authentication(self, request: HttpRequest) -> Optional[HttpResponse]:
        for callback in self.auth_callbacks:
            try:
                if is_async_callable(callback) or getattr(callback, "is_async", False):
                    result = callback(request)
                    if inspect.iscoroutine(result):
                        result = async_to_sync(callback)(request)
                else:
                    result = callback(request)
            except Exception as exc:
                return self.api.on_exception(request, exc)

            if result:
                request.auth = result  # type: ignore
                return None
        return self.api.on_exception(request, AuthenticationError())

keep in mind that we check bool authentication result (not only None) if result:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants