fastapi Add WebSocket handling support for HTTP security dependencies

Related: https://github.com/tiangolo/fastapi/discussions/8983

Since https://github.com/encode/starlette/pull/1263 has been merged, that makes possible for us to add WebSocket handling support for current HTTP security dependencies.

This PR fixes exceptions like this:

Traceback (most recent call last):
  File "<string>", line 17, in <module>
  File "/docker/nb2-test/venv/lib/python3.8/site-packages/nonebot/__init__.py", line 309, in run
    get_driver().run(*args, **kwargs)
  File "/docker/nb2-test/venv/lib/python3.8/site-packages/nonebot/drivers/fastapi.py", line 198, in run
    uvicorn.run(
  File "/docker/nb2-test/venv/lib/python3.8/site-packages/uvicorn/main.py", line 578, in run
    server.run()
  File "/docker/nb2-test/venv/lib/python3.8/site-packages/uvicorn/server.py", line 61, in run
    return asyncio.run(self.serve(sockets=sockets))
  File "/usr/lib/python3.8/asyncio/runners.py", line 44, in run
    return loop.run_until_complete(main)
> File "/docker/nb2-test/venv/lib/python3.8/site-packages/uvicorn/protocols/websockets/websockets_impl.py", line 254, in run_asgi
    result = await self.app(self.scope, self.asgi_receive, self.asgi_send)
  File "/docker/nb2-test/venv/lib/python3.8/site-packages/uvicorn/middleware/proxy_headers.py", line 78, in __call__
    return await self.app(scope, receive, send)
  File "/docker/nb2-test/venv/lib/python3.8/site-packages/fastapi/applications.py", line 290, in __call__
    await super().__call__(scope, receive, send)
  File "/docker/nb2-test/venv/lib/python3.8/site-packages/starlette/applications.py", line 122, in __call__
    await self.middleware_stack(scope, receive, send)
  File "/docker/nb2-test/venv/lib/python3.8/site-packages/starlette/middleware/errors.py", line 149, in __call__
    await self.app(scope, receive, send)
  File "/docker/nb2-test/venv/lib/python3.8/site-packages/starlette/middleware/exceptions.py", line 79, in __call__
    raise exc
  File "/docker/nb2-test/venv/lib/python3.8/site-packages/starlette/middleware/exceptions.py", line 68, in __call__
    await self.app(scope, receive, sender)
  File "/docker/nb2-test/venv/lib/python3.8/site-packages/fastapi/middleware/asyncexitstack.py", line 20, in __call__
    raise e
  File "/docker/nb2-test/venv/lib/python3.8/site-packages/fastapi/middleware/asyncexitstack.py", line 17, in __call__
    await self.app(scope, receive, send)
  File "/docker/nb2-test/venv/lib/python3.8/site-packages/starlette/routing.py", line 718, in __call__
    await route.handle(scope, receive, send)
  File "/docker/nb2-test/venv/lib/python3.8/site-packages/starlette/routing.py", line 443, in handle
    await self.app(scope, receive, send)
  File "/docker/nb2-test/venv/lib/python3.8/site-packages/fastapi/applications.py", line 290, in __call__
    await super().__call__(scope, receive, send)
  File "/docker/nb2-test/venv/lib/python3.8/site-packages/starlette/applications.py", line 122, in __call__
    await self.middleware_stack(scope, receive, send)
  File "/docker/nb2-test/venv/lib/python3.8/site-packages/starlette/middleware/errors.py", line 149, in __call__
    await self.app(scope, receive, send)
  File "/docker/nb2-test/venv/lib/python3.8/site-packages/starlette/middleware/gzip.py", line 26, in __call__
    await self.app(scope, receive, send)
  File "/docker/nb2-test/venv/lib/python3.8/site-packages/starlette/middleware/base.py", line 26, in __call__
    await self.app(scope, receive, send)
  File "/docker/nb2-test/venv/lib/python3.8/site-packages/starlette/middleware/exceptions.py", line 79, in __call__
    raise exc
  File "/docker/nb2-test/venv/lib/python3.8/site-packages/starlette/middleware/exceptions.py", line 68, in __call__
    await self.app(scope, receive, sender)
  File "/docker/nb2-test/venv/lib/python3.8/site-packages/fastapi/middleware/asyncexitstack.py", line 20, in __call__
    raise e
  File "/docker/nb2-test/venv/lib/python3.8/site-packages/fastapi/middleware/asyncexitstack.py", line 17, in __call__
    await self.app(scope, receive, send)
  File "/docker/nb2-test/venv/lib/python3.8/site-packages/starlette/routing.py", line 718, in __call__
    await route.handle(scope, receive, send)
  File "/docker/nb2-test/venv/lib/python3.8/site-packages/starlette/routing.py", line 341, in handle
    await self.app(scope, receive, send)
  File "/docker/nb2-test/venv/lib/python3.8/site-packages/starlette/routing.py", line 82, in app
    await func(session)
  File "/docker/nb2-test/venv/lib/python3.8/site-packages/fastapi/routing.py", line 283, in app
    solved_result = await solve_dependencies(
  File "/docker/nb2-test/venv/lib/python3.8/site-packages/fastapi/dependencies/utils.py", line 593, in solve_dependencies
    solved_result = await solve_dependencies(
  File "/docker/nb2-test/venv/lib/python3.8/site-packages/fastapi/dependencies/utils.py", line 622, in solve_dependencies
    solved = await call(**sub_values)
TypeError: __call__() missing 1 required positional argument: 'request'

Aug 26 '23 07:08 mnixry

I think the https://github.com/encode/starlette/pull/1263 is not enough for this feature.

Aug 31 '23 06:08 Kludex

I think the encode/starlette#1263 is not enough for this feature.

Yes, in large extent, it is caused by the limitations of ASGI. send for WS connections or HTTP is completely different. For standard ASGI, the only thing we can do is disconnect before accept, which will result in a 403 error.

Although there are some ASGI extensions, allow us to custom denial response, but starlette does not support this feature right now: https://github.com/encode/starlette/pull/2041, and only little amount of ASGI servers has implemented this extension.

However, returning a 403 error has been better than raising a "request not found" exception on server side. Additionally, it makes more sense that these dependencies support both HTTP and WS requests.

Aug 31 '23 08:08 mnixry

Hypercorn supports the WSDR (WebSocket Denial Response), and Uvicorn has a PR to implement it already. And... There aren't many server implementations around. Also, I'm a maintainer of Starlette, and I'm keen to have support for WSDR.

I think it's better to wait Uvicorn and Starlette instead of getting this in.

Aug 31 '23 08:08 Kludex

Hypercorn supports the WSDR (WebSocket Denial Response), and Uvicorn has a PR to implement it already. And... There aren't many server implementations around. Also, I'm a maintainer of Starlette, and I'm keen to have support for WSDR.

I think it's better to wait Uvicorn and Starlette instead of getting this in.

Am I understand correctly, that disagreements are about using @handle_exc_for_ws to handle authentication errors? As I understand it will be handled in WSDR after FastAPI is updated to use Startlette 0.37.

Should we exclude @handle_exc_for_ws from this PR and merge it? It will allow using these security tools for websockets at least with auto_error=False. And it will fully work after FastAPI is updated to use Startlette 0.37.

Feb 23 '24 08:02 YuriiMotov

Referencing the discussion in https://github.com/tiangolo/fastapi/pull/11146#issuecomment-2031005586, it appears that FastAPI won't be upgrading to starlette >= 0.37 anytime soon. We might need to explore other options, or else we could be stuck without progress indefinitely.

Although the @handle_exc_for_ws decorator isn't the most elegant solution, finding an alternative approach hasn't been straightforward. Perhaps we could implement some modifications within the SecurityBase class.

Apr 02 '24 09:04 mnixry

FastAPI has upgraded Starlette to version >=0.37.2 a few hours ago #11266 . I'm looking forward to this PR being merged as it can solve many downstream issues.

Apr 02 '24 10:04 WSH032

FastAPI has upgraded Starlette to version >=0.37.2 a few hours ago #11266 . I'm looking forward to this PR being merged as it can solve many downstream issues.

Oh, thanks for bringing this to my attention! I hadn't noticed that PR before. I'll get this PR prepared soon.

Apr 02 '24 10:04 mnixry

Updated. Security dependencies are now functional for incoming WebSocket connections when auto_error is set to False.

However, I'm currently uncertain about how to integrate HTTPException to handle WebSocket connections when auto_error is set to True.

Apr 03 '24 06:04 mnixry

I wonder if the WebSocket Denial Response is supposed to be sent behind the scenes when HttpException(status_code=HTTP_403_FORBIDDEN,..) occurs, or should we use WebSocket.send_denial_response to send it?

When HTTPException is raised during a WebSocket connection, it won't be processed correctly and will default to the ExceptionMiddleware, resulting in an internal server error (status code 500). In the worst-case scenario, if ASGI doesn't support WebSocket Denial Response (WSDR), this will lead to a RuntimeError without sending any response, potentially causing the request to block indefinitely, which is clearly undesirable.

In other words, do we need something like this?

The code snippet you've provided has two critical flaws:

Utilizing send_denial_response doesn't stop the processing of the request, which means the dependencies will still be executed. This could lead to an authentication bypass vulnerability. Therefore, stopping the control flow through an Exception is a better solution.
The absence of detection for WSDR support can lead to a RuntimeError if the ASGI server doesn't support it.

To address these problems, introducing a custom middleware that specifically handles HTTPException for WebSocket connections might be necessary.

Apr 03 '24 11:04 mnixry