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

[feature]/event hook - Schema Directives #39

Closed
imiskolee opened this issue Mar 6, 2018 · 8 comments
Closed

[feature]/event hook - Schema Directives #39

imiskolee opened this issue Mar 6, 2018 · 8 comments
Labels
enhancement New feature or request parser Blocked on parser

Comments

@imiskolee
Copy link

how to make authorzation layer when i generated a graphql server.
i think we need a event call before call each query/mutation.

for:

  1. authorization layer (Before* event)
  2. access log (After* event)
  3. common control logic

its like:

type Event interface{
  BeforeQuery(ctx context.Context)
  BeforeEachQuery(ctx context.Context,opName string,opParams map[string]interface{})
  BeforeEachMutation(ctx context.Context,opName string,opParams map[string]interface{})
  AfterQuery(ctx context.Context)
  AfterEachQuery(ctx context.Context,opName string,opParams map[string]interface{},output interface{})
  AfterEachMutation(ctx context.Context,opName string,opParams map[string]interface{},output interface{})
} 
@ghost
Copy link

ghost commented Mar 6, 2018 via email

@vektah
Copy link
Collaborator

vektah commented Mar 6, 2018

I'm not a fan of map[string]interface{}. I think what is really needed here is schema directives

for example:

directive @hasRole(role: String) on FIELD | FIELD_DEFINITION

type Mutation {
    updateAskingPrice(id: ID!, newPrice: Float!): Vehicle! @hasRole(role: "MANAGER")
}

could generate resolver methods

func (r *Resolvers) Directive_Vehicle_hasRole(ctx context.Context, next func(ctx context.Context) *Vehicle, role string) *Vehicle {
    if !UserForCtx(ctx).HasRole(role) {
        return nil
    }
    return next(ctx)
}

@vektah vektah added the enhancement New feature or request label Mar 8, 2018
@andrewmunro
Copy link

Agree with @vektah, directives are the way to go!

I'm also following progress on graphql/graphql-spec#300 to expose directives via introspection, that way the client can know what they have access to.

@imiskolee
Copy link
Author

imiskolee commented Mar 16, 2018

yes. i think directive better than.

i has some case for directive:

  1. control for each mutation operator.
  2. control for each field
  3. mixed directive control(sorted of define)
  4. change raw request
  5. change raw response

@vektah who work on it ?

@vektah vektah changed the title [feature]/event hook [feature]/event hook - Schema Directives Apr 1, 2018
@vektah
Copy link
Collaborator

vektah commented Apr 1, 2018

The blocker on this is that the parser gqlgen uses (https://github.com/graph-gophers/graphql-go/tree/master/internal/schema) doesn't currently support schema directives, only query directives.

@tonyghita has recently started maintaining it again and has been pushing the parser forward so I am reluctant to make too many changes here until we see which way he takes it. I think there was talk in the gophers slack around bringing in a new lexer which would be a pretty large change.

Good directive support is also required to validate against https://github.com/graphql-cats/graphql-cats

@andrewmunro
Copy link

@vektah Are we any further forward on this? The parser library hasn't received any updates in a few months now 😥

@vektah
Copy link
Collaborator

vektah commented May 15, 2018

This has kinda progressed on two fronts,

Firstly, most of the hooks described above can be achieved using middleware, I haven't documented it yet but take a look at the opentracing for some ideas.

Secondly, work on a new parser is well under way, currently working through setting up the validation logic in a way thats maintainable going forward. There was a bit of related chatter in #100

@vektah
Copy link
Collaborator

vektah commented Jun 29, 2018

Closed in favour of #156

@vektah vektah closed this as completed Jun 29, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request parser Blocked on parser
Projects
None yet
Development

No branches or pull requests

3 participants