Neoteroi / BlackSheep

Fast ASGI web framework for Python
https://www.neoteroi.dev/blacksheep/
MIT License
1.88k stars 78 forks source link

Dependency injection of Services in Auth handler. #187

Open YassineElbouchaibi opened 3 years ago

YassineElbouchaibi commented 3 years ago

๐Ÿš€ Feature Request

Dependency injection of Services in Auth handler and Middleware.

This is useful to access config, db session instance and many other things in these handlers.

Maybe there is already a way to do this ?

Currently I do it this way:

class AppAuthenticationHandler(AuthenticationHandler):
    def __init__(self, app: Application) -> None:
        super().__init__()
        self.app = app

    @property
    def settings(self) -> Settings:
        return self.app.service_provider.get("settings")

    @property
    def db_session(self) -> AsyncContextManager[AsyncSession]:
        return self.app.service_provider.get("db_session")  # type: ignore

    async def authenticate(self, context: Request) -> Optional[Identity]:
        ...
RobertoPrevato commented 3 years ago

Good idea, guardpost could be modified to be integrated with rodi, thus supporting DI in those handlers. I will think about this.

Beside that, I think the way you do it now makes sense. I only have two recommendations:

  1. I would keep the authentication handler abstracted from the instance of the web app; for example changing the constructor to accept the services you need, so that you can reuse the same code with other kinds of front-ends. Imagine the scenario where you implement also a CLI, as alternative client for your users, and the source code of the CLI doesn't need to depend on the source code of the web API.
  2. (side note) did you consider using JWTs instead of accessing the database at each request to verify user's authentication? One of the reasons why JWTs were designed was exactly to remove the need to authenticate users against a database

if you are interested in the idea of using JWTs, I can share some code that does that.

YassineElbouchaibi commented 3 years ago

Hi @RobertoPrevato, Thank you for your reply and suggestions. I like your suggestions!

I am definitely going to implement the first one as it makes a lot of sense.

For the second one, I already plan on using JWT, but we are not sure yet if we are going to be using something implemented directly into the main web service for auth, a separate auth service maintained internally or a third party auth service. Therefore I might not have control over the it's payload for multiple reasons or as much control as I would like. This is why I was planning on keeping that db_session in the class for now.

I am however still interested in seeing the code you are talking about as it could potentially give me ideas or help me better understand how it can be implemented/used.

Thank you!

RobertoPrevato commented 3 years ago

Hi @YassineElbouchaibi Sorry for taking one month to share my code for JWTs validation. In the last weeks I needed a vacation and to recharge my batteries.

I published an example here of JWT Bearer middleware: https://github.com/Neoteroi/BlackSheep-Examples/tree/main/jwt-validation

The code there implements validating JWTs using public RSA keys obtained from JWKS downloaded using OpenID discovery endpoints. The code should work with Auth0, Okta, and works well with Azure Active Directory and Azure Active Directory B2C. It would work also with a custom service that issues JWTs and should be easy to modify to handle other kinds of scenarios, like using symmetric encryption and secrets, instead of public RSA keys used for signature verification, and fetching JWTS from different sources in case of asymmetric encryption.

I took the occasion to clean up some code I already wrote for private projects. Soon I will pack portions of the example in a dedicated library. :)

RobertoPrevato commented 3 years ago

I just realized that support for dependency injection in middlewares was already implemented in BlackSheep when this issue was opened.

DI is not available in Authentication handlers, but on a second thought - they can be instantiated passing the services to their constructor or the application object, so at the moment I don't consider this feature high priority, compared to other things that can be done to improve the framework.

gh0st-work commented 1 year ago

Same here, dependency injection in middleware (funcs) & decorators is extremely needed. It is very necessary for multiple middleware that change data before the main function (in docs only after), and ofั decorators. In my case, there are custom auth and schema validation. And this is quite illogical and strangely done now, need to get steamed up to implement it.

Would like to see something like this: image image

I think it's not so difficult to implement, just call the function, and if it returns or wraps (inspect and ast will help though) another one, then recalculate injections and just throw the dependencies again in the new func. Also need to manage "AmbiguousMethodSignatureError" somehow.

The most realistic (easiest, MVP) implementation option (read sources, normalization is hard glued to Route) is not a recalculation of signature and binders, but just a function for the user (developer), such as "with_dependency_injections" or custom decorator helper that will just manage signature and binders & ensure response. Mb making Request field like container rodi.Container (request.container), if u need framework-level implementation. Mb will attach the example of implementation in following comment.

By the way, awesome project, thanks for your code.

gh0st-work commented 1 year ago

Minimum on the fly normalization of func (calculating of signature and binders) requires:

  1. Removing @functools.wraps from decorator (so, better implement on framework level)
  2. Adding to framework raw route.param_names, just to get it from initial injection, otherwise it calls JSONBinder & AmbiguousMethodSignatureError as result, same with not detected (by me) required one other arguments
  3. Coping normalization funcs (half of the normalization.py file) without route passing (not so many places, 2-10), up to _get_binders_for_function, where Route is optional
  4. Adding used & new info to Services

image

(Yes, it works)

Full fledged framework-level implementation requires:

  1. Application func normalize_handlers changes, bcuz it goes like @functools.wraps (all decorators requirements) are invisible and says JSONBinder for new types added inside decorator & AmbiguousMethodSignatureError
  2. Same point 2
  3. Creating custom class eg RouteData (Services._map?) for not Route functions in normalize binders funcs, for providing orig request info
  4. Encapsulating extra logic with custom decorator for user experience & mb for detecting all route and non-route required types, if neccesary
  5. Middlewares

Mb will pull some changes like this later.

RobertoPrevato commented 1 year ago

Hi @gh0st-work Thank for you input, I can work on adding support for dependency injections for authentication handlers. I don't have right now to read carefully all you wrote, but please don't create a PR now that can cause recalculation of signature and binders as you wrote, because I don't accept this idea. It's best to improve the underlying library used for authentication rather than recalculate signatures and binders (this requires reflection about code and would be bad for performance).

I have plans to change the library used for authentication (currently named guardpost) - I can make it more integrated with the web framework and the library used for DI.

gh0st-work commented 1 year ago

I know about guardpost, but this is not only about authentication, in fact about creating any decorators and middlewares that really change, process something (request, user input), we can say Turing completeness in terms of DI)

For example, I need not only authentication, but also data validation, let's say through the schema package, and actually, if you write a decorator (any) - all the magic disappears, moreover, everything breaks, because blacksheep ignores new types passed in.

Well, it's not about recalculation under the hood, but about recalculation at the user's (developer) request, in his own decorator, so simple function and really little changes in core (normalization.py).

In my project it's critical, because, i reuse logic)) For example, i specify schema params in decorator, if schema is not valid, i simply send failure response, if valid โ€“ passing to real function valid dict (in reality addict) inside of class/type (to use DI). And all that just in one decorator. Same my auth function also need some logic with my own jwt or api_key by choice, in mobile apps or some other limited software (automatization, bots) api_key is only choice, but it's not that secure for example for payments or admin route, so i can just set allow_api_key=False, that's it) I think normal usage of decorators โ€“ really cool and necessary feature. Just imagine custom logging in one decorator, sending requests to external apis, schema validation, auth, and really any stuff you want and then โ€“ back to processing request with your favorite framework โ€“ blacksheep. It's kinda sounds like an ad)) Dream feature))

I'm also familiar with pytest, I don't think I can "make trouble"))

RobertoPrevato commented 1 year ago

I don't doubt your proficiency in Python, I am not scare of you making trouble ๐Ÿ˜ƒ let me come back later to this conversation. Right now I am at work and cannot dedicate time here.

gh0st-work commented 1 year ago

Ok, just in time, I will write an example of implementation.

gh0st-work commented 1 year ago

Implementation example

Final version is extremelly easy and comfortable in understanding and coding: image

Logic of injections for only 50 lines, the key thing is understanding:

from blacksheep import Route, Request
from blacksheep.server.normalization import normalize_handler, ensure_response
from rodi import Services, class_name

class TempInject:

    def __init__(self, services: Services, *args, **kwargs):
        self.services = services

        self.injections = [*args, *kwargs.values()]
        self.injections_types = []
        self.injections_class_names = []
        for injection in self.injections:
            injection_type = injection.__class__
            self.injections_types.append(injection_type)

            raw_injection_class_name = class_name(injection_type)
            self.injections_class_names.append(raw_injection_class_name)

            self.services.set(injection_type, injection)

    def __enter__(self) -> Services:
        return self.services

    def __exit__(self, exc_type, exc_val, exc_tb):
        for injection_class_name in self.injections_class_names:
            del self.services._map[injection_class_name]
        for injection_type in self.injections_types:
            del self.services._map[injection_type]

async def with_deps_injection(
    orig_handler,
    request: Request,
    services: Services,
    *args,
    **kwargs
):
    route_like = Route('', orig_handler)  # For this (normal typing & beauty) are needed extra changes in core
    route_like.param_names = list(request.route_values.keys())

    with TempInject(services, *args, **kwargs) as temp_services:
        norm_handler = normalize_handler(
            route=route_like,
            services=temp_services
        )
        response = ensure_response(await norm_handler(request))

    return response

Also quite easy if you implement it into the framework itself, and even if blacksheep will manage functool.wraps. But result looks like magic)

RobertoPrevato commented 1 year ago

Hi @gh0st-work thank you for your code example.

+ @YassineElbouchaibi

For the next version of the web framework and related libraries, I worked in the last days to include support out of the box for dependency injection in authentication handlers and authorization requirements.

There are examples here:

I also added a protocol to support replacing my implementation of dependency injection with alternative libraries, if desired, to make the web framework more flexible. I simplified the library to handle authentication and make the imports much less verbose.

https://github.com/Neoteroi/BlackSheep/commit/890386557ddea84582d92947949ec6a8da833cf3

There will be some breaking changes for version 2, I am also renaming the packages to offer a more consistent experience, using PEP 420 under a common namespace.

https://github.com/Neoteroi/rodi

amirongit commented 3 months ago

sorry to bother; is it implemented now in V2?

@RobertoPrevato