GraphQL skip/access on FIELD_DEFINITION, RBAC for fields?

Question

Let's imagine having a simple type in graphql

type Coupon {
    id: ID!
    code: String!
}

I'd love to show the id field only if a user has a determinate condition. I'll then create a directive

directive IncludeWhen(cond: String!) on FIELD_DEFINITION

and edit the code as

type Coupon {
    id: ID! @IncludeWhen(cond: "ADMIN")
    code: String!
}

I'll check in the directive resolver if the user has necessary privileges:

export class IncludeWhenDirective extends SchemaDirectiveVisitor {
    public visitFieldDefinition(field) {
        const { resolve = defaultFieldResolver } = field;

        const requiredCond = this.args.cond;

        field.resolve = async function (...args) {
            const context = args[2];
            const currentUser = context.currentUser;

            let result;

            if (currentUser.hasCond(requiredCond) {
                throw new AuthenticationError();
                result = await resolve.apply(this, args);
            }

            return result;
        };
    }
}

Well, this is wrong. Endpoint return an error like

Cannot return null for non-nullable field Coupon.ID

when queried from a user without required condition.

Is it possible to skip this field entirely? Maybe accessing the resolver and deleting the fields without breaking the whole thing up?

works for other non nullable fields? If field is guarded should not be request-able (nullable or not used in query) ... you should throw access error if condition not met ... client (app) shouldn't ask for this if user has no rights for that field — xadm
@xadm my idea is different: you can call, but i'm not showing (if not all requirements are meet). But in the end, I think with the actual implementation of the graphql tech is not possible. I'll resort to diffent types for different call — Valerio
If you can call then you have to return some (not null) value for not nullable (typed with !) field (you can't change type on demand, otherwise server will throw error 'null for not nullable'). - return FAKE - if fake (detect on client) not show?.But it can lead to client cache error if you use a const for id of items in array ... return random number above some limit? IMHO should be not requestable and client shoul be aware of that (not hard to implement). — xadm

Valerio Valerio · Accepted Answer · 2020-04-24T07:43:21

In the end, the whole question I made is wrong, not just the code.

The logic behind is flawed, I cannot manipulate the output in this way. As @xadm suggested, and I want to thank him/her for pointing me out the right way, I'll place a "guard" on the field. If the requirements aren't met, the whole query fails.

Even with introspection disabled, someone can try to "guess" what other fields are present on a type. Protecting the query by failing if someone is trying to access fields without required condition is the right way to do it.

For other cases, IE different output based on different conditions will be covered with a factory pattern for example.

GraphQL skip/access on FIELD_DEFINITION, RBAC for fields?

1 Answers