Context based validation or modification

[cwheel]

I've been playing around with wundergraph for a bit and I'm really liking it, but I'm a bit stuck on devising a method of operation authorization. I'd ideally like to be able to inspect instantiated models prior to inserting or updating and either allow the update, or, reject the update in consideration of the context.

I've been poking around in the source and it doesn't seem like there's any straight forward way to do this at the moment. I think(?) the easiest way of doing something like this might be to allow the implementation of a trait like:

impl<M, Ctx> WundergraphShouldInsert for MyInsertable<M, Ctx> {
    pub fn should_insert(model: &M, context: &Ctx) -> bool;
    // or
    pub fn should_insert(model: &M, context: &Ctx) -> Option<&M>;
}

In the case of inserting, it seems like this could probably be checked in the handle_insert call prior to actually asking Diesel to insert.

I'd also like something for reading/deleting, but I think the easiest cases to start with are probably insert/update. If this seems like something that you'd be interested in having Wundergraph support, I'd be happy to take a crack at implementing this. If I can already do this somehow and I'm missing it, I'd love to hear.

Thanks!

[weiznich]

The current design idea here is as following:

You either want something simple that just works, or you want to do something custom. The first option is implemented via #[derive(Insertable)] (+ some internal wundergraph trait impls for the corresponding mutation traits.) The second options requires you implementing HandleInsert and HandleBatchInsert for your insertable struct. Here is an example implementation. Those two custom would allow you to do basically everything, including complex insert statements, inspecting the generated query and doing access controls.

For the general story: Corresponding traits are available for update and delete mutations. For queries the story is a bit more complex. One the one hand there is QueryModifier which allows you to modify/intercept queries that are auto generated via #[derive(WundergraphEntity)] at the "cost" of having a custom context type, on the other hand it's possible to manually implement LoadingHandler which allows maximum control but is probably not that easy.

That all writing I think it could be a useful addition to have something like QueryModifier just also work with the mutations. I would like to have something that works with all mutations at once, not only with Insertable, but on the other hand you should have the control to implement it differently for different kinds of mutations. I will try to think about a concrete design here in the next few day, but if you have some suggestions for the general use case I'm open for suggestions.

[cwheel]

Thanks for all of the information, I didn't realize I could just implement HandleInsert/HandleBatchInsert (and similar for update/delete). I also didn't realize there was already a QueryModifier. I'll give both of those a shot.

As for a more general solution, I'm not sure that I have any particular ideas at the moment. I do agree though that it'd be nice to have a generic way to implement this logic across mutation types.

[weiznich]

So the documentation on this topic certainly needs to be improved. If you have any suggestions where this could be mentioned feel free to share them here.

[cwheel]

I wouldn't mind contributing to the example project or adding an authorization example to the readme when/if I get this working.

I've been playing around with QueryModifier, but I'm not quite sure of what you mean in the documentation by:

Add a specialized implementation for each type that implements LoadingHandler

Since QueryModifier is supposed to be implemented for the context, how can there exist multiple implementations of it on a singular context? If you happen to have an example of using QueryModifier with explicit models somewhere, I'd love to see it.

Thanks!

[weiznich]

I do not have an example for that available in public somewhere, but the basic idea is to do that by providing specialiced impls based on the generic paramater L in that trait definition. So instead of having one impl<L, DB> QueryModifier<L, DB> for MyContext impl somewhere globally you need to write for each of your entities one impl like impl<DB> QueryModifier<ConcreteEntityType1, DB> for MyContext where ConcreteEntityType1 is some type deriving WundergraphEntity/implementing LoadingHandler. The disadvantage of this method is that you really need to cover all of your concrete entity types with separate impls because of rusts coherence rules. (You cannot simply have a wildcard impl and only provide a specialized impl for a single type)

[cwheel]

That's very similar to what I was trying before, but I end up with a trait error:

the trait 'wundergraph::query_builder::selection::LoadingHandler<DB, Context<diesel::PgConnection>>' is not implemented for 'models::Post'

Simplified example:

#[derive(
    Clone,
    Debug,
    Identifiable,
    Queryable,
    WundergraphEntity,
)]
#[table_name = "posts"]
pub struct Post {
    pub id: i32,
}

impl<DB> QueryModifier<models::Post, DB> for Context<diesel::PgConnection>
where
    DB: Backend + ApplyOffset + 'static,
    Self: WundergraphContext,
{
    fn modify_query<'a>(
        &self,
        _select: &LookAheadSelection<'_, WundergraphScalarValue>,
        query: BoxedQuery<'a, models::Post, DB, Self>,
    ) -> wundergraph::error::Result<BoxedQuery<'a, models::Post, DB, Self>> {
        Ok(query)
    }
}

I'm guessing something is going wrong with resolving the implementation due to the explicitly defined context type? Thanks!

[weiznich]

As you are using a concrete connection type there you should also use a concrete type for the diesel backend in place of DB.

Something like this should work:

impl QueryModifier<models::Post, diesel::pg::Pg> for Context<diesel::PgConnection>
{
    fn modify_query<'a>(
        &self,
        _select: &LookAheadSelection<'_, WundergraphScalarValue>,
        query: BoxedQuery<'a, models::Post, diesel::pg::Pg, Self>,
    ) -> wundergraph::error::Result<BoxedQuery<'a, models::Post, diesel::pg::Pg, Self>> {
        Ok(query)
    }
}

[cwheel]

Oh yep, should have specified that. I've got it working now, thanks! Let me play around with this a bit and I'd be happy to provide some updated documentation for this sort of stuff if you'd like.