google / tarpc

An RPC framework for Rust with a focus on ease of use.
MIT License
3.16k stars 195 forks source link

Metadata? #152

Open Boscop opened 7 years ago

Boscop commented 7 years ago

It would be useful to have metadata like in gRPC or HTTP Headers for authentication data that should be sent with every request. (And then on top of that it would be useful to have a capability for something like BeforeMiddleware that can check if a client is authenticated before a request is processed. But then it's not pure RPC anymore?)

shaladdle commented 7 years ago

Thanks for filing this!

We haven't thought about auth much yet. For now you can, of course, add the auth information as an argument (though I realize it's less convenient).

I think having some kind of user metadata like gRPC seems good, as does having some kind of BeforeMiddleware (and probably After as well). I spoke with @tikue a bit and his concern is that we don't clutter the existing API if people don't care about this kind of thing. That seems easy to satisfy though.

But then it's not pure RPC anymore?

What do you mean by this?

Boscop commented 7 years ago

But then it's not pure RPC anymore?

What do you mean by this?

I mean, if middlewares come into the picture, there is no 1-to-1 correspondence between caller and callee like with normal function calls.

I've been thinking about middlewares some more and came to the conclusion that it would probably complicate things too much. If a BeforeMiddleware can reject the call (like for auth), it would have to be reflected in the function's return type. If it can only modify function args, it's not that useful... The big advantage of RPC vs a normal JSON/REST API is type safety between server and client, so it's probably better to reflect the possible failure modes in the return type of the functions explicitly. But metadata like in gRPC could still be useful for things like auth.

tikue commented 7 years ago

It could be reflected in the type signature like this:


// Given this service:
service! {
    rpc foo() -> Foo | Bar;
    rpc baz() -> Baz | Error;
}

// ...this is generated:
trait Before {
    type BeforeError: Into<Bar> + Into<Error>;

    /// Returning Ok continues to the rpc handler.
    /// Returning Err propagates to the user by converting to the appropriate error type.
    fn before(&self) -> Result<(), Self::BeforeError>;
}
compressed commented 7 years ago

I had considered this idea too, especially for authentication, but I decided to let types to do the work for me. I'm currently doing something like this:

service! {
    rpc baz(untrusted_baz: UntrustedBaz) -> Baz | Error;
}

fn baz(&self, untrusted_baz: UntrustedBaz) -> Result<Baz, Error> {
    let baz: Baz = untrusted_baz.try_into()?; // do your authentication/validation etc. here
    // do whatever you need to do with baz now
}
tikue commented 7 years ago

That's a neat pattern! I've got a branch on my personal repo that has experimental support for task-local contexts. It seems pretty cool, but requiring being on-task is kind of an awkward limitation presently.

I honestly don't know which I prefer. I do agree that providing credentials explicitly to each request feels boilerplatey. But I also worry about any kind of local data because it's always error prone, e.g. when spawning new tasks or threads you need to manually propagate the context.

tikue commented 5 years ago

I'm pretty sure this will be a lot easier to accomplish now that services are essentially just FnOnce(Request) -> Response + Clone.

I'm imagining something like...(this is a strawman example, but hopefully you can get the idea)

/// Returns a serve fn that only allows authenticated users to be served.
fn serve_authenticated<Req, Resp>(
    allowed_users: Arc<HashSet<String>>,
    serve: impl FnOnce(Req) -> impl Future<Output = Result<Resp, ServerError>> + Clone,
) -> impl FnOnce(RequestWithMetadata<Req>) -> impl Future<Output = Result<Resp, ServerError>> + Clone {
    async move |request| {
        if allowed_users.contains(&request.metadata.user) {
            await!(serve(request.request))
        } else {
            Err(UnauthenticatedError)
        }
    }
}
tikue commented 5 years ago

Another option (for auth, specifically) is to plug it in at the transport layer, so that it's completely transparent to the user. Effectively, it'd transform a Transport<RequestWithMetadata<Request>, Response> to a Transport<Request, Response> by doing whatever it needs to with the metadata, and then passing along only the request body.