graphql-rust / juniper

GraphQL server library for Rust
Other
5.7k stars 421 forks source link

Is there an existing way to pass data from the schema to the graphql handler function? #1105

Closed kingledion closed 2 years ago

kingledion commented 2 years ago

Is your feature request related to a problem? Please describe. I would like to implement authentication (signup and login) in pure graphql, without ancillary login and signup endpoints. In order to do this, I need to be able to set the 401- Unauthorized for unauthenticated traffic to protected schemas, and the 403 - Forbidden for unauthorized traffic to protected schemas. Additionally, it would be nice to set 400 status for failed requests; out of the box Juniper will just send a 200 with the error message.

Furthermore, for the authentication problem, if my schema does result in a successful login or signup, I need some way to pass the authenticated user token from the schema back to the handler to apply it to the current session as a cookie.

Both problem are the same; how do you pass information from the schema back to the "business logic layer" of the handler.

Describe the solution you'd like Any way to pass information from the business logic layer of the handler.

Describe alternatives you've considered Right now I have implemented something myself using the context. The downside of this that adding mutable information to the context needlessly requires an RWLock around it. In my use case, a new context is created for each handler the information in the context is not shared (unlike the backing resources, such as a database reference in the context, which is shared).

Additional context I want to know if anyone else has a solution here before I propose one. I would probably just make the simplest possible version of my authentication scheme and use it as an example, at first. Some of the boilerplate authentication code could make it into the juniper core codebase if the example is well received, to eliminate the verbosity of my context using solution. There is something inelegant, after all, about having to manually set the http status code in the context before you return a FieldResult from the schema...

tyranron commented 2 years ago

@kingledion

Right now I have implemented something myself using the context.

Yes, this is the right way to go. You should use context argument for those kind of things.

The downside of this that adding mutable information to the context needlessly requires an RWLock around it. In my use case, a new context is created for each handler the information in the context is not shared (unlike the backing resources, such as a database reference in the context, which is shared).

It's not required needlessly. The fact that something in context has the lifespan of the request only, doesn't mean it shouldn't be protected by a Mutex/RwLock. In fact, if you're running on tokio multi-thread runtime, which does implement work stealing, your request resolution easily may be executed concurrently on different threads at the same time.

But, if you're using a runtime like actix-rt (the one underneath the actix-web) where you're absolutely sure that once request resolution has started in one thread it would never leave it, you may use send_wrapper crate to avoid unnecessary multi-thread synchronization. But you will need to use RefCell instead anyway, though, as the interior mutability is required, because even inside a single thread different handlers of the same request may be executed concurrently.

tyranron commented 2 years ago

There is something inelegant, after all, about having to manually set the http status code in the context before you return a FieldResult from the schema...

In our codebase we do this only for non-standard successful HTTP codes. For error codes we just return a custom error type, which implements both juniper::IntoFieldError and conversions into HTTP response of our web framework. In that conversions we set all the desired HTTP codes and HTTP headers for specific kinds of error.

kingledion commented 2 years ago

Thanks, I think I have had an imperfect understanding of how GraphQL is supposed to work. I'm going to take a look at these and work from there.

I don't think there is any further need for this feature request as is, until I have a better understanding of how I am going to go forward.