Rework noauth #62
Rework noauth #62
Conversation
| @@ -17,7 +17,7 @@ | |||
| from fps.hooks import register_router # type: ignore | |||
|
|
|||
| from fps_auth.db import get_user_db # type: ignore | |||
| from fps_auth.routes import ( # type: ignore | |||
| from fps_auth.backends import ( # type: ignore | |||
| current_user, | |||
| cookie_authentication, | |||
| LoginCookieAuthentication, | |||
There was a problem hiding this comment.
I see that you are implementing a kind of login endpoint for token authentication mode. IMO, this should also be a custom authentication backend that would check for a static token in query params and return a static user.
There was a problem hiding this comment.
Actually the mechanism for token authentication is that when you access the root endpoint with the token as a query parameter, the server will set a cookie so that you don't have to pass the token again in next accesses (this is the default behavior for jupyter-server). I'm not sure how we could implement that differently. Maybe you have ideas?
There was a problem hiding this comment.
Oh I think I see what you mean, we should check for this token in query parameters for all endpoints, and thus have a special authentication back-end for this.
There was a problem hiding this comment.
Oh ok, if the goal is to set a cookie, then what you did is indeed a good solution.
I thought the token was required to be passed on each request (like a REST API), in which case, a custom auth backend would have been more appropriate.
There was a problem hiding this comment.
Yes but still, we should allow the token to be passed on any endpoint, not just the root endpoint. I tried playing with a TokenAuthentication class, but then I was faced with the problem of getting the token as a query parameter, and basically didn't know where to get it.
There was a problem hiding this comment.
The key is to use the security schemes of FastAPI: there is one for most common cases. In this case, APIKeyQuery seems a good candidate: name argument is the name of your query parameter to read the token from.
It's also important to set auto_error to False to prevent the scheme from raising a 403 when the value is not present: FastAPI Users needs this to try several backends before raising an error.
There was a problem hiding this comment.
I'll keep that for a next PR, thanks.
|
Thanks for the review @frankie567, I'll push another commit with the changes you suggested. |
davidbrochart
force-pushed
the
rework_noauth
branch
from
aac0abc
to
78f3d25
Compare
davidbrochart
force-pushed
the
rework_noauth
branch
from
78f3d25
to
7b1c9c7
Compare
davidbrochart
force-pushed
the
rework_noauth
branch
from
cae0096
to
a2c0b18
Compare
davidbrochart
force-pushed
the
rework_noauth
branch
from
a2c0b18
to
9431b94
Compare
Implement authentication backend for no-auth.
Not functional yet, cc @frankie567.