Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for authentication #830

Open
5 tasks
patrick91 opened this issue Apr 7, 2021 · 20 comments
Open
5 tasks

Add support for authentication #830

patrick91 opened this issue Apr 7, 2021 · 20 comments
Labels
discussion enhancement New feature or request help wanted Extra attention is needed

Comments

@patrick91
Copy link
Member

patrick91 commented Apr 7, 2021

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:

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 patrick91 added enhancement New feature or request help wanted Extra attention is needed discussion labels Apr 7, 2021
@kristofgilicze
Copy link
Contributor

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
Copy link
Member

jkimbo commented Apr 8, 2021

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
Copy link
Member Author

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
Copy link
Contributor

la4de commented Apr 8, 2021

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
Copy link
Member Author

@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
Copy link
Contributor

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
Copy link
Member Author

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
Copy link
Contributor

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
Copy link
Member

@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
Copy link
Member Author

@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
Copy link
Member

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

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
Copy link
Member Author

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
Copy link
Member

taybenlor commented May 19, 2021

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
Copy link

karolzlot commented Aug 28, 2021

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
Copy link
Member

jkimbo commented Aug 29, 2021

@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
Copy link

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
Copy link

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
Copy link

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

@elefher
Copy link

elefher commented May 21, 2024

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

@patrick91
Copy link
Member Author

@elefher what implementations would like to see the most?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

10 participants