strawberry-graphql / strawberry

A GraphQL library for Python that leverages type annotations 🍓
https://strawberry.rocks
MIT License
3.96k stars 528 forks source link

Add support for authentication #830

Open patrick91 opened 3 years ago

patrick91 commented 3 years ago

I've opened a discussion on this, but I think it might be worth making an issue related to authentication.

I'd say we can split this work into multiple chunks, but let's think about what we want in ideal world.

[1] JWT with cookies, JWT in header, plain cookies for sessions (basic django auth) [2] we shouldn't force users to use our preferred way of error handling

All of the above is up for discussion by the way :) haven't really found any other library that do authentication built-in. For Graphene we have these two:

Upvote & Fund

Fund with Polar

kristofgilicze commented 3 years ago

I can see why this make sense for the django integration. Common features working out-of-the-box (with a few lines of settings.py) is expected there. DRF style API would be pretty familiar to most django devs.

When using a minimal framework like Flask, this might feel a bit unusual since Flask has no built in auth. Sanic is the same, just a few example in the docs. So I believe this should be only for the django and the 'django-like' integrations.

Shipping a first-party strawberry-graphql/auth ext is also something to consider, though I would the prefer the built-in approach.

jkimbo commented 3 years ago

I agree with @kristofgilicze I think an authentication integration makes sense with Django because it's provided by the framework. Maybe we can build something into the Django integration @la4de ?

Something like that Sanic guide would be good for the other frameworks.

patrick91 commented 3 years ago

I wonder though, if we build support for django, wouldn't be "easy" to make it pluggable and support multiple frameworks. The mutations and queries would be the same everywhere, no?

The implementation would change, so maybe we can provide some hooks for that?

la4de commented 3 years ago

We have preliminary support already in strawberry-graphql-django package. It's still in very early stage but you can already generate graphql type from user model and login / logout with given auth mutations.

Current user is also accessible through info.context.request.user.

https://github.com/strawberry-graphql/strawberry-graphql-django#django-authentication-examples

Feel free to give feedback and share your ideas!

patrick91 commented 3 years ago

@la4de that's awesome, so maybe we can keep this in strawberry-django and then when it is mature we can think of making more generic, what do you all think?

kristofgilicze commented 3 years ago

core implementation should be reused between integrations.

The django way to configure stuff is in the settings.py. I think it is already a little weird that I set the schema on the view.

patrick91 commented 3 years ago

I think it is already a little weird that I set the schema on the view.

It's definitely different than django, but it allows to have multiple GraphQL views in one project (we do that at work for example).

kristofgilicze commented 3 years ago

If you have multiple views, then I agree the 'global' setting does not make sense.

I had a closer look at how DRF handles the auth classes.

... there is also an option to set it on a per-view basis.

What I described before: in the settings.py you actually set / override the default auth classes.

see: https://www.django-rest-framework.org/api-guide/authentication/#setting-the-authentication-scheme

joeydebreuk commented 3 years ago

@la4de that's awesome, so maybe we can keep this in strawberry-django and then when it is mature we can think of making more generic, what do you all think?

I think that it should remain framework specific. There are too many things to cover, too many different frameworks and too many different approaches. While within the context of a framework for a specific auth method, its really simple, you can see here.

I'd suggests to make it really simple for integrations (eg strawberry-graphql-django) and for users to:

  1. Add something like "auth" to "context"
  2. Use this context for permission classes.
patrick91 commented 3 years ago

@joeydebreuk but what about the mutations and queries? those won't change even if you use different frameworks (the internal implementation will, but not the way the mutations are used).

I feel like there might be some opportunity for reusing code somehow :) even django has pluggable auth backends :)

joeydebreuk commented 3 years ago

@patrick91 I feel like we had a similar discussion regarding adding framework specific code to strawberry. I suppose its a matter of philosophy and defining the scope of strawberry.

I'm sure you can add some abstraction on the strawberry level, but I don't see how it would add a lot of value. Just more complexity. And more docs to write.

We should be able to use different ways of authentication[1]

Already possible, and quite simple to do. Some documentation would help though.

We should be able to have pluggable user types

Why should strawberry be are of user types?

We should be able to generate login, logout and signup mutations

These mutations are very simple, I don't see the point in doing this.

We should be able to have pluggable input types and return types for these mutations[2]

Defining these inputs and returns will be most of the code one would need for a mutation in the first place

We should be able to support django but also any other framework with not much code

Maybe its an idea to just provide a guide for each framework?

even django has pluggable auth backends :)

Django has everything though :smile:

In short, I would say that adding a few guides for setting up strawberry for frameworks is better than adding framework-specific code to strawberry.

patrick91 commented 3 years ago

In short, I would say that adding a few guides for setting up strawberry for frameworks is better than adding framework-specific code to strawberry.

Could be! It's just that I see auth something used a lot and quite critical (auth should be implemented well and should secure), so it would be nice if we can help with that as a framework.

I'll see if I can come up with some examples of what I am thinking 😊

taybenlor commented 3 years ago

Weighing in here, I'm personally not at all interested in Django integrations so this discussion seems a bit too "heavyweight" for me. I'm providing a GraphQL alternative to a FastAPI endpoint, which is conceptually similar to people discussing Flask.

I'd like to be able to share some code between the two, but currently I'm most interested in Strawberry providing tools like hooks, helpers, and mixins that would make Authentication easier. Ideally I would be free to define my own mutations and permissions on queries, but with some deeper strawberry integration.

For example a solution to me might be for Strawberry to simply provide tools that work like the current Permission classes. Then make it my responsibility to provide login and signup mutations.

@strawberry.authenticated(any=[HeaderAuthentication, SimpleHTTPAuthentication, CookieAuthentication])
@strawberry.type
class SomeQuery:
   @strawberry.field
   def some_resolver(self, info) -> str:
       user = info.user

@strawberry.authenticated(all=[CookieAuthentication, AdminAuthentication])
@strawberry.type
class SomeAdminQuery:
    @strawberry.field
    def some_resolver(self, info) -> str:
        admin = info.user

In this scenario the any argument and the all arguments allow for authentication that can be structured as alternatives (any) or layered checks (all). The info object is expanded to have a configurable/settable property called user. The Authentication classes I've defined here would be ones I've implemented myself, but potentially extending a BaseAuthentication class which provides some helpers for accessing the request scope.

I'm not suggesting this be the exact implementation nor am I married to any particular approaches here. Just making suggestions that may be simpler right now as a way to progress this ticket without thinking about Django.

Edit: While I've written these at the class level, they may make more sense at the resolver level (the way permissions work). Or potentially at both, depending on whether you want global permission requirements.

karolzlot commented 3 years ago

It would be great if strawberry would be able to use something like FastAPI's Depends().

For example see Depends(deps.get_current_active_user) and Depends(deps.get_current_active_superuser) as demonstrated in these links (this is template project which uses FastAPI):

https://github.com/tiangolo/full-stack-fastapi-postgresql/blob/master/%7B%7Bcookiecutter.project_slug%7D%7D/backend/app/app/api/api_v1/endpoints/items.py

https://github.com/tiangolo/full-stack-fastapi-postgresql/blob/master/%7B%7Bcookiecutter.project_slug%7D%7D/backend/app/app/api/api_v1/endpoints/users.py

This way is super comfortable to use!

(after you go to the links above, you can click on those method names to see how they are defined in this project)


Maybe this information will be helpful in implementing it (but it is about graphene):

https://stackoverflow.com/questions/61930695/how-to-get-user-auth-info-in-fastapi-when-using-app-add-route-for-graphql

jkimbo commented 3 years ago

@karolzlot I personally like the dependency injection API that FastAPI has and I'd like to see something similar implemented in Strawberry. It's unlikely that we would be able to use FastAPI dependencies directly though because it's quite paradigm. For example in the get_current_active_user dependency that you linked it raises an HTTP error if there isn't a current user which is not something we would want to do in GraphQL.

karolzlot commented 3 years ago

I think it could be something like this:

Inside project insead of get_current_active_user two other functions are defined:

get_current_active_user_rest get_current_active_user_graphql

which differ by how they raise errors

They could inherit from one class or inherit from one another.

But I don't know Strawberry enough to be sure that this approach is good.

ifaizanali commented 1 year ago

Is there a way to use logical operators in strawberry permissions classes like: permission_classes = [IsAuthenticates, ((IsOwner and IsMember) or IsAdmin)] something like that ^^^^^

yhdelgado commented 1 year ago

Is there any advance on this topic? I'm seeking a practical solution to validate a third-party token on FastAPI+Strawberry.

elefher commented 4 months ago

is there any update on this topic? Some of the implementations mentioned above would be very useful

patrick91 commented 3 months ago

@elefher what implementations would like to see the most?