strawberry icon indicating copy to clipboard operation
strawberry copied to clipboard

Add support for authentication

Open patrick91 opened this issue 3 years ago • 20 comments

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.

  • [ ] We should be able to use different ways of authentication[1]
  • [ ] We should be able to have pluggable user types
  • [ ] We should be able to generate login, logout and signup mutations
  • [ ] We should be able to have pluggable input types and return types for these mutations[2]
  • [ ] We should be able to support django but also any other framework with not much code

[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:

  • https://github.com/PedroBern/django-graphql-auth
  • https://github.com/flavors/django-graphql-jwt

Upvote & Fund

  • We're using Polar.sh so you can upvote and help fund this issue.
  • We receive the funding once the issue is completed & confirmed by you.
  • Thank you in advance for helping prioritize & fund our backlog.
Fund with Polar

patrick91 avatar Apr 07 '21 22:04 patrick91

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.

kristofgilicze avatar Apr 08 '21 00:04 kristofgilicze

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.

jkimbo avatar Apr 08 '21 08:04 jkimbo

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?

patrick91 avatar Apr 08 '21 08:04 patrick91

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!

la4de avatar Apr 08 '21 09:04 la4de

@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?

patrick91 avatar Apr 08 '21 09:04 patrick91

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.

kristofgilicze avatar Apr 08 '21 09:04 kristofgilicze

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).

patrick91 avatar Apr 08 '21 09:04 patrick91

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

kristofgilicze avatar Apr 08 '21 10:04 kristofgilicze

@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.

joeydebreuk avatar Apr 08 '21 12:04 joeydebreuk

@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 :)

patrick91 avatar Apr 08 '21 12:04 patrick91

@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.

joeydebreuk avatar Apr 08 '21 13:04 joeydebreuk

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 😊

patrick91 avatar Apr 08 '21 13:04 patrick91

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.

taybenlor avatar May 19 '21 01:05 taybenlor

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

karolzlot avatar Aug 28 '21 15:08 karolzlot

@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.

jkimbo avatar Aug 29 '21 13:08 jkimbo

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.

karolzlot avatar Oct 07 '21 02:10 karolzlot

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

ifaizanali avatar Dec 26 '22 11:12 ifaizanali

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

yhdelgado avatar May 17 '23 03:05 yhdelgado