-
-
Notifications
You must be signed in to change notification settings - Fork 462
Description
While auditing the code, I came across a use of DI in the AuthenticationBackend
class that I could not rationalize. In the examples the following is shown as an intended usage for this class.
def get_jwt_strategy() -> JWTStrategy[models.UP, models.ID]:
return JWTStrategy(secret=SECRET, lifetime_seconds=3600)
auth_backend = AuthenticationBackend(
name="jwt",
transport=bearer_transport,
get_strategy=get_jwt_strategy,
)
fastapi_users = FastAPIUsers[User, uuid.UUID](get_user_manager, [auth_backend])
Then in FastAPIUsers, there is a method get_auth_router
which in turn calls get_auth_router
from 'routers land'. This get_auth_router
from 'routers_land' needs an AuthenticationBackend
object let's say to make routes. Let's check the 'login' route function. I am showing code snippets only relevant for my rant 😄
@router.post(
"/login",
name=f"auth:{backend.name}.login",
responses=login_responses,
)
async def login(
request: Request,
credentials: OAuth2PasswordRequestForm = Depends(),
user_manager: BaseUserManager[models.UP, models.ID] = Depends(get_user_manager),
strategy: Strategy[models.UP, models.ID] = Depends(backend.get_strategy),
):
user = await user_manager.authenticate(credentials)
response = await backend.login(strategy, user)
await user_manager.on_after_login(user, request, response)
return response
Ok. At this point it would be reasonable to assume that backend
needs get_strategy
or it does something with it like create a Strategy
object. But then I got curious as to why there is a strategy object being passed to backend.login
, after all one would assume AuthenticationBackend
has its own internal Strategy
object provided to it via the DI i.e. get_strategy 😕 Then I went on to see how AuthenticationBackend
makes use of or NOT this Strategy
object. Lo and behold ...
class AuthenticationBackend(Generic[models.UP, models.ID]):
name: str
transport: Transport
def __init__(
self,
name: str,
transport: Transport,
get_strategy: DependencyCallable[Strategy[models.UP, models.ID]],
):
self.name = name
self.transport = transport
self.get_strategy = get_strategy
async def login(
self, strategy: Strategy[models.UP, models.ID], user: models.UP
) -> Response:
token = await strategy.write_token(user)
return await self.transport.get_login_response(token)
So then, this means, AuthenticationBackend
owns a dependency callable, that callers need to provide so that they are able to pass it's result i.e. a Strategy
object back to it's methods 😖 .
This being a dependency for a route function, it will repeatedly create the same Strategy
object whenever the route is accessed. But why is it being created repeatedly - isn't it intended to be static - I mean one is not expected to infer what sort of strategy object to create based on the contents of a request sent to the login
route right?
In natural language the AuthenticationBackend
class is saying, and I quote 😆
I need a
DependencyCallable
object to live. I need it so that I let you use the result of this callable which is aStrategy
object to call my methods with. Oh, and BTW I do not own the callable, neither do I actually need it. I just add a namespace to it so that you use the dot notation to access this callable.
😳
Am I missing something ? 😕 I could not understand the rationale for this pattern here. I would appreciate it if some one lets me know that this is deliberate and there is an intended use-case for it.