fix(ACL): use acl for export, add GoG admin resolvers#7420
fix(ACL): use acl for export, add GoG admin resolvers#7420NamanJain8 merged 3 commits intoibrahim/multitenancyfrom
Conversation
abhimanyusinghgaur
left a comment
There was a problem hiding this comment.
Reviewed 6 of 7 files at r1.
Reviewable status: 6 of 7 files reviewed, 4 unresolved discussions (waiting on @manishrjain, @NamanJain8, @pawanrawal, and @vvbalaji-dgraph)
dgraph/cmd/alpha/http.go, line 647 at r1 (raw file):
// TODO(Pawan): Are we passing the correct namespace over here?
yes, for admin server this needs to be done ATM.
edgraph/access_ee.go, line 1046 at r1 (raw file):
ctx = x.AttachJWTNamespace(ctx)
why do we need to do this if we are anyways using ExtractJWTNamespace later? i.e., we aren't using the namespace directly present in the context metadata, we are only using the one from JWT, so why need to attach the namespace from JWT to metadata?
graphql/admin/admin.go, line 324 at r1 (raw file):
// guardianOfTheGalaxyQueryMWs are the middlewares which should be applied to queries served by // admin server unless some exceptional behaviour is required
comment can be updated
same below for GoGMutationMWs
graphql/resolve/middlewares.go, line 136 at r1 (raw file):
// GuardianOfTheGalaxyAuthMW4Query blocks the resolution of resolverFunc if there is no Guardian // auth present in context, otherwise it lets the resolverFunc resolve the query.
... of resolverFunc if the context doesn't have Guardian auth for Galaxy namespace, otherwise ...
same for comments on mutation MW snd the helper function resolveGuardianOfTheGalaxyAuth
NamanJain8
left a comment
There was a problem hiding this comment.
Reviewable status: 2 of 5 files reviewed, 4 unresolved discussions (waiting on @abhimanyusinghgaur, @manishrjain, @pawanrawal, and @vvbalaji-dgraph)
dgraph/cmd/alpha/http.go, line 647 at r1 (raw file):
Previously, abhimanyusinghgaur (Abhimanyu Singh Gaur) wrote…
// TODO(Pawan): Are we passing the correct namespace over here?yes, for admin server this needs to be done ATM.
Okay.
edgraph/access_ee.go, line 1046 at r1 (raw file):
Previously, abhimanyusinghgaur (Abhimanyu Singh Gaur) wrote…
ctx = x.AttachJWTNamespace(ctx)why do we need to do this if we are anyways using
ExtractJWTNamespacelater? i.e., we aren't using the namespace directly present in the context metadata, we are only using the one from JWT, so why need to attach the namespace from JWT to metadata?
Thanks. We don't need it here.
graphql/admin/admin.go, line 324 at r1 (raw file):
Previously, abhimanyusinghgaur (Abhimanyu Singh Gaur) wrote…
// guardianOfTheGalaxyQueryMWs are the middlewares which should be applied to queries served by // admin server unless some exceptional behaviour is requiredcomment can be updated
same below for GoGMutationMWs
Done.
graphql/resolve/middlewares.go, line 136 at r1 (raw file):
Previously, abhimanyusinghgaur (Abhimanyu Singh Gaur) wrote…
// GuardianOfTheGalaxyAuthMW4Query blocks the resolution of resolverFunc if there is no Guardian // auth present in context, otherwise it lets the resolverFunc resolve the query.... of resolverFunc if the context doesn't have Guardian auth for Galaxy namespace, otherwise ...same for comments on mutation MW snd the helper function
resolveGuardianOfTheGalaxyAuth
Done.
This change is