Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

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

Make OAuth2PasswordBearer compatible with WebSocket objects #2587

Closed
WilliamDEdwards opened this issue Dec 31, 2020 · 11 comments
Closed

Make OAuth2PasswordBearer compatible with WebSocket objects #2587

WilliamDEdwards opened this issue Dec 31, 2020 · 11 comments
Labels
question Question or problem question-migrate

Comments

@WilliamDEdwards
Copy link

WilliamDEdwards commented Dec 31, 2020

I'm trying to implement WebSockets.

Just like with HTTP, I use a header called 'X-Authorization' for my JWT token:

websocket.WebSocketApp("ws://localhost:5000/api/v1/subscriptions/", header={"X-Authorization": f"Bearer {token}"})

I inject a dependency called get_current_user. This dependency uses OAuth2PasswordBearer (in line with documentation).

This results in the following error:

Traceback (most recent call last):
  File "/usr/local/lib/python3.9/site-packages/uvicorn/protocols/websockets/websockets_impl.py", line 154, in run_asgi
    result = await self.app(self.scope, self.asgi_receive, self.asgi_send)
  File "/usr/local/lib/python3.9/site-packages/uvicorn/middleware/proxy_headers.py", line 45, in __call__
    return await self.app(scope, receive, send)
  File "/usr/local/lib/python3.9/site-packages/fastapi/applications.py", line 199, in __call__
    await super().__call__(scope, receive, send)
  File "/usr/local/lib/python3.9/site-packages/starlette/applications.py", line 111, in __call__
    await self.middleware_stack(scope, receive, send)
  File "/usr/local/lib/python3.9/site-packages/starlette/middleware/errors.py", line 146, in __call__
    await self.app(scope, receive, send)
  File "/usr/local/lib/python3.9/site-packages/starlette/middleware/base.py", line 21, in __call__
    await self.app(scope, receive, send)
  File "/usr/local/lib/python3.9/site-packages/starlette/middleware/cors.py", line 70, in __call__
    await self.app(scope, receive, send)
  File "/usr/local/lib/python3.9/site-packages/starlette/middleware/sessions.py", line 75, in __call__
    await self.app(scope, receive, send_wrapper)
  File "/usr/local/lib/python3.9/site-packages/starlette/exceptions.py", line 58, in __call__
    await self.app(scope, receive, send)
  File "/usr/local/lib/python3.9/site-packages/starlette/routing.py", line 566, in __call__
    await route.handle(scope, receive, send)
  File "/usr/local/lib/python3.9/site-packages/starlette/routing.py", line 283, in handle
    await self.app(scope, receive, send)
  File "/usr/local/lib/python3.9/site-packages/starlette/routing.py", line 57, in app
    await func(session)
  File "/usr/local/lib/python3.9/site-packages/fastapi/routing.py", line 237, in app
    solved_result = await solve_dependencies(
  File "/usr/local/lib/python3.9/site-packages/fastapi/dependencies/utils.py", line 516, in solve_dependencies
    solved_result = await solve_dependencies(
  File "/usr/local/lib/python3.9/site-packages/fastapi/dependencies/utils.py", line 548, in solve_dependencies
    solved = await call(**sub_values)
TypeError: __call__() missing 1 required positional argument: 'request'

... because OAuth2PasswordBearer always looks at the Request object, which we don't have when using WebSockets:

authorization: str = request.headers.get("Authorization")

According to the documentation, WebSocket objects have Header as well, so shouldn't we allow for looking at the WebSocket object?

@WilliamDEdwards WilliamDEdwards added the question Question or problem label Dec 31, 2020
@Kludex
Copy link
Sponsor Collaborator

Kludex commented Dec 31, 2020

We don't have the Request object available there. Instead, we have the WebSocket object. Try to run this:

from fastapi.websockets import WebSocket
from fastapi import FastAPI

app = FastAPI()

@app.websocket("/ws")
async def websocket_endpoint(websocket: WebSocket):
    print(websocket.headers)
    await websocket.accept()
    while True:
        data = await websocket.receive_text()
        await websocket.send_text(f"Message text was: {data}")

For those who want to verify, run wtih: uvicorn <path>:app

And establish a connection: websocat ws://localhost:8000/ws

@WilliamDEdwards
Copy link
Author

We don't have the Request object available there. Instead, we have the WebSocket object. Try to run this:

from fastapi.websockets import WebSocket
from fastapi import FastAPI

app = FastAPI()

@app.websocket("/ws")
async def websocket_endpoint(websocket: WebSocket):
    print(websocket.headers)
    await websocket.accept()
    while True:
        data = await websocket.receive_text()
        await websocket.send_text(f"Message text was: {data}")

For those who want to verify, run wtih: uvicorn <path>:app

And establish a connection: websocat ws://localhost:8000/ws

I know, that's what I'm saying.

@Kludex
Copy link
Sponsor Collaborator

Kludex commented Dec 31, 2020

It's not possible to implement it for WebSockets, the reason is that HTTPExceptions are handled for http, not for websockets and the OAuth2 classes uses it underneath in case is not authorized. Hence, not possible to do it, in a trivial way at least.

What is possible to do is:

  1. Change the request: Request for authorization: str = Header(...) (or Optional[str] = Header(None)
  2. Create a wrapper around it, so it can handle the erased HTTPException in a "clean way".

A "clean way" could be just returning None and not sending back an acceptance e.g. (await websocket.accept()).

P.S.: As soon as I saw your last message I tried to implement it, then I faced the mentioned issue.

@WilliamDEdwards
Copy link
Author

Your efforts are highly appreciated!

I'm not very familiar with the internals of FastAPI, so I'm not sure which option is best for this use case. Should we keep this issue open for others to think along?

@Kludex
Copy link
Sponsor Collaborator

Kludex commented Jan 2, 2021

Sure! 🤓 👍

@falkben
Copy link
Sponsor Contributor

falkben commented Jan 2, 2021

Haven't used this myself, but you might have a look at: https://indominusbyte.github.io/fastapi-jwt-auth/advanced-usage/websocket/

@fatvlady
Copy link

fatvlady commented Jan 2, 2021

Hi, just encountered this issue too. I am not familiar with FastAPI internals, but maybe it's worth adding websocket support when auto_error is False?

@frankie567
Copy link
Contributor

Also interested in this 👀 I maintain an authentication library which rely internally on OAuth2PasswordBearer and APIKeyCookie.

I get questions from some users who don't understand why it's not working for websockets. It would be nice indeed if those security schemes could support websockets in some way.

I get that there is a gotcha with the HTTPException there. I'm not familiar with fastapi internals but here is an idea:

  • Instead of directly raise an HTTPException, the schemes could raise a generic exception "SecurityException"
  • I see that the dependency resolver is able to distinguish between Request and Websocket. So maybe we could catch those SecurityException there and accordingly raise an HTTPException or close the websocket.

I've not thought about it very much, so maybe it's totally wrong and it probably needs lot of changes in the codebase.

Would be happy to help if needed though 😄

@Kludex
Copy link
Sponsor Collaborator

Kludex commented Jan 11, 2021

Just found out this PR on Starlette: encode/starlette#527

@frankie567
Copy link
Contributor

Clearly, that would definitely help! But it seems there are some blockers there. Not sure, how we could help though 😅

@grossmj
Copy link

grossmj commented Aug 11, 2021

Any news about this? it would be great to be able to easily secure websocket routes

Repository owner locked and limited conversation to collaborators Feb 28, 2023
@tiangolo tiangolo converted this issue into discussion #8983 Feb 28, 2023

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
question Question or problem question-migrate
Projects
None yet
Development

No branches or pull requests

7 participants