Add authn for graphql and http admin endpoints #5162

abhimanyusinghgaur · 2020-04-10T06:17:41Z

Description.

This PR adds authentication to following endpoints:

/admin/backup (http & graphql)
/admin/config/lru_mb (http [GET & PUT] & graphql [query & mutation])
/admin/draining (http & graphql)
/admin/export (http & graphql)
/admin/shutdown (http & graphql)
/admin/restore (graphql only)
/admin/listBackups (graphql only)

Now, all the above http endpoints and their corresponding graphql versions have following kinds of auth:

IP White-listing, if --whitelist flag is passed to alpha
Poor-man's auth, if --auth_token flag is passed to alpha
Guardian only access, if ACL is enabled

This PR also adds query for config in graphql admin, as it was missing earlier.

In addition to above points:

All the /admin endpoints apply Poor-man's auth check at http level itself, while other auth checks are routed through graphql resolvers.
GraphQL Resolvers for health/state and the ones related to ACL User/Group have IP whitelisting middleware applied, while dgraph handles Guardian auth for them.
/alter has the existing behaviour of checking only Poor-man's and Guardian auth.
GraphQL Resolvers related to schema don't apply IP whitelisting as to keep them in sync with /alter. They do apply Guardian auth.
Any GraphQL admin introspection queries don't require IP whitelisting or Guardian auth.

GitHub Issue or Jira number.

Fixes #4758.

Other components or 3rd party tools affected (or regression areas).

No

Affected releases.

> 20.03.0

Changelog tags.

added authentication for admin endpoints

Please indicate if this is a breaking change.

No

Please indicate if this is an enterprise feature.

No

Please indicate if documentation needs to be updated.

Yes

Please indicate if end to end testing is needed.

Yes, for the whitelisting and poor-man's ACL part.

This change is

Docs Preview:

pawanrawal

Nice work, this mostly looks good. I am wondering if we can avoid duplication by checking poor man's auth and guardian access in the functions that are finally called by both GraphQL and HTTP.
Also needs a bunch of negative test cases which show access is restricted.

Reviewed 11 of 11 files at r1.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @abhimanyusinghgaur)

graphql/admin/config.go, line 58 at r1 (raw file):

	query *gql.GraphQuery,
	mutations []*dgoapi.Mutation) (map[string]string, map[string]interface{}, error) {
	err := edgraph.AuthorizeGuardians(ctx)

Is the poor man's auth applied here or in any of the GraphQL endpoints?

graphql/admin/export.go, line 59 at r1 (raw file):

	query *gql.GraphQuery,
	mutations []*dgoapi.Mutation) (map[string]string, map[string]interface{}, error) {
	err := edgraph.AuthorizeGuardians(ctx)

How about the poor man's auth here?

manishrjain

I think the auth stuff is getting copied in many different places. Instead, it should be done in one place, before the corresponding handler gets called.

Reviewable status: 10 of 15 files reviewed, 2 unresolved discussions (waiting on @abhimanyusinghgaur, @manishrjain, and @pawanrawal)

…dmin-auth # Conflicts: # ee/acl/acl_test.go # graphql/admin/backup.go # graphql/admin/config.go # graphql/admin/draining.go # graphql/admin/export.go # graphql/admin/shutdown.go

…dmin-auth # Conflicts: # ee/acl/acl_test.go # testutil/graphql.go

…dmin-auth

abhimanyusinghgaur

Added test cases for both positive (wherever possible) and negative cases.
Using middlewares now for Guardian auth in graphql; Poormans's and whitelisting are applied directly in http handler for /admin. So, no duplication now.

Reviewable status: 3 of 13 files reviewed, 2 unresolved discussions (waiting on @pawanrawal)

graphql/admin/config.go, line 58 at r1 (raw file):

Previously, pawanrawal (Pawan Rawal) wrote…

Is the poor man's auth applied here or in any of the GraphQL endpoints?

Using middlewares now for Guardian auth in graphql, Poormans's and whitelisting are applied directly in http handler for /admin

graphql/admin/export.go, line 59 at r1 (raw file):

Previously, pawanrawal (Pawan Rawal) wrote…

How about the poor man's auth here?

Using middlewares now for Guardian auth in graphql, Poormans's and whitelisting are applied directly in http handler for /admin

gja

Small changes requested

gja · 2020-04-20T05:28:05Z

dgraph/cmd/alpha/admin.go

 // handlerInit does some standard checks. Returns false if something is wrong.
-func handlerInit(w http.ResponseWriter, r *http.Request, allowedMethods map[string]bool) bool {
+func handlerInit(w http.ResponseWriter, r *http.Request, allowedMethods map[string]bool,
+	allowOnlyGuardians bool) bool {


should we be clubbing allowedMethods and allowOnlyGuardians into a type?

Also, can we rename handlerInit to something like checkAuth?

gja · 2020-04-20T05:35:21Z

dgraph/cmd/alpha/admin.go

 // handlerInit does some standard checks. Returns false if something is wrong.
-func handlerInit(w http.ResponseWriter, r *http.Request, allowedMethods map[string]bool) bool {
+func handlerInit(w http.ResponseWriter, r *http.Request, allowedMethods map[string]bool,
+	allowOnlyGuardians bool) bool {
 	if _, ok := allowedMethods[r.Method]; !ok {


can't we move these allowedMethods type checks into some sort of router or something? I know we do use httprouter, why aren't 405s moved over there?

gja · 2020-04-20T05:35:55Z

dgraph/cmd/alpha/admin.go

 	if _, ok := allowedMethods[r.Method]; !ok {
 		x.SetStatus(w, x.ErrorInvalidMethod, "Invalid method")
+		w.WriteHeader(http.StatusMethodNotAllowed)


Isn't this a bit strange? Middlewares should either do the return true/false pattern, or they should call next() on success. This feels like it's mixing both

pawanrawal

Seems like poorman's auth and IP whitelisting are applied to all endpoints except for /alter. Should /alter have Poorman's auth?

And then guardian ACL check is applied to all endpoints. One question is between the consistency of checks between /alter and updateGQLSchema. updateGQLSchema has IP whitelisting whereas /alter doesn't. With the way things are structured right now, I guess it would be hard to remove IP whitelisting just from updateGQLSchema to keep both endpoints consistent?

Reviewed 12 of 15 files at r2.
Reviewable status: all files reviewed, 7 unresolved discussions (waiting on @abhimanyusinghgaur)

dgraph/cmd/alpha/http.go, line 584 at r2 (raw file):

	}

	if !checkIpIsWhitelisted(w, r) || !checkPoormansAcl(w, r) {

Since IP whitelisting is not applied to schema alter above, we should keep it consistent for the graphql schema alter as well. Is the poor man's auth applied to schema alter.

graphql/admin/admin.go, line 404 at r2 (raw file):

	adminMutationResolvers := map[string]resolve.MutationResolverFunc{
		"backup":   resolve.CommonMutationMiddlewares.Then(resolveBackup),

Instead of this then pattern here, would have liked for this just to be an slice of functions which are applied in order.

graphql/resolve/middlewares.go, line 70 at r2 (raw file):

// resolveAuth returns a Resolved with error if the context doesn't contain any Guardian auth,
// otherwise it returns nil
func resolveAuth(ctx context.Context, f schema.Field) *Resolved {

This doesn't seem to apply poor man's auth. Is that applied earlier along with IP whitelisting?

graphql/resolve/middlewares.go, line 71 at r2 (raw file):

// otherwise it returns nil
func resolveAuth(ctx context.Context, f schema.Field) *Resolved {
	if err := edgraph.AuthorizeGuardians(ctx); err != nil {

I am assuming that AuthorizeGuardians returns true for oss builds?

…dmin-auth

abhimanyusinghgaur

Reviewable status: 5 of 22 files reviewed, 7 unresolved discussions (waiting on @gja and @pawanrawal)

dgraph/cmd/alpha/admin.go, line 55 at r2 (raw file):

Previously, gja (Tejas Dinkar) wrote…

Also, can we rename handlerInit to something like checkAuth?

Done.

dgraph/cmd/alpha/admin.go, line 56 at r2 (raw file):

Previously, gja (Tejas Dinkar) wrote…

can't we move these allowedMethods type checks into some sort of router or something? I know we do use httprouter, why aren't 405s moved over there?

I couldn't find the use of httprouter in our codebase.
For now, I am letting adminAuthHandler do these checks for admin endpoints.

dgraph/cmd/alpha/admin.go, line 58 at r2 (raw file):

Previously, gja (Tejas Dinkar) wrote…

Isn't this a bit strange? Middlewares should either do the return true/false pattern, or they should call next() on success. This feels like it's mixing both

Done.

dgraph/cmd/alpha/http.go, line 584 at r2 (raw file):

Previously, pawanrawal (Pawan Rawal) wrote…

Since IP whitelisting is not applied to schema alter above, we should keep it consistent for the graphql schema alter as well. Is the poor man's auth applied to schema alter.

Done.

graphql/admin/admin.go, line 404 at r2 (raw file):

Previously, pawanrawal (Pawan Rawal) wrote…

Instead of this then pattern here, would have liked for this just to be an slice of functions which are applied in order.

Done.

graphql/resolve/middlewares.go, line 70 at r2 (raw file):

Previously, pawanrawal (Pawan Rawal) wrote…

This doesn't seem to apply poor man's auth. Is that applied earlier along with IP whitelisting?

Poor-man's auth is handled at http level by adminAuthHandler for /admin

graphql/resolve/middlewares.go, line 71 at r2 (raw file):

Previously, pawanrawal (Pawan Rawal) wrote…

I am assuming that AuthorizeGuardians returns true for oss builds?

Yes, it returns nil error for oss builds.

…dmin-auth

MichaelJCompton

I haven't reviewed the actual auth that is applied at each point. This is a breaking change so for that, I think we should take the behaviour to Manish+Daniel in a demo to check through all the options and make sure it's making expected changes. I'd say we should hold off tipping this into master until that has gone through.

Looks in general to be a nice simplification/generalisation of how these auth rules are applied as well as adding features, so looks good.

I had a bunch of questions about the GraphQL middleware. I quite like the pattern of wrapping resolvers in other resolvers that apply things like auth. I expect we'll add more to this: like logging in admin, maybe query complexity in /graphql, and also maybe open up custom logic to allow users to give custom pre processing.

In thinking about the answers to the questions and the future stuff we might add, like logging in admin (which we'll add soon) make sure the pattern is general enough that we can play with it a bit and not have to rip it apart as soon as we add more things.

Reviewed 1 of 18 files at r3.
Reviewable status: 5 of 22 files reviewed, 12 unresolved discussions (waiting on @abhimanyusinghgaur, @gja, and @pawanrawal)

ee/acl/acl_test.go, line 1927 at r3 (raw file):

	assertNonGuardianFailure(t, "export", true, params)

	// assert non-ACL error for guardians

I don't quite get these cases in each of these tests ... why do we need to test this? Aren't there existing tests that show that this works for guardians? So what's really being tested here?

graphql/admin/admin.go, line 414 at r3 (raw file):

		"backup":   resolve.AdminMutationMWs.Then(resolve.MutationResolverFunc(resolveBackup)),
		"config":   resolve.AdminMutationMWs.Then(resolve.MutationResolverFunc(resolveConfig)),
		"draining": resolve.AdminMutationMWs.Then(resolve.MutationResolverFunc(resolveDraining)),

The 'Then' thing looks like a cute pattern, but is it really needed here? It doesn't look like we use it for anything other than applying the same two middlewares, so what benefit do we get from the generic pattern?

Having it require the complexity of writing it, plus writing tests for it. I'd say think carefully if the pattern is required and if we are using it now, or intend to use it really soon.

graphql/admin/admin.go, line 621 at r3 (raw file):

			}).
		// for queries and mutations related to User/Group, dgraph handles Guardian auth,
		// so need to apply GuardianAuth Middleware

does it matter though? We already have the resolve.AdminMutationMWs.Then pattern ... do we need to not apply the guardian middleware?

If we don't, it would be ok to remove the extra fns, and just apply the standard ones?

Also, if this is really required to only call the one middleware, you have this 'Then' pattern, so what would be the reason to have a specific IpWhitelistingMW4Mutation function and not use 'Then' on a list of middleware that just has the ip whitelisting? Looks to me like it adds a specific function that's already handled by the general case.

graphql/resolve/middlewares.go, line 17 at r3 (raw file):

 */

package resolve

does this belong in 'resolve' looks like this and the test belong in admin

graphql/resolve/middlewares.go, line 144 at r3 (raw file):

// GuardianAuthMW4Query blocks the resolution of resolverFunc if there is no Guardian auth
// present in context, otherwise it lets the resolverFunc resolve the query.
func GuardianAuthMW4Query(resolver QueryResolver) QueryResolver {

it's a bit sad that we need versions for query and for mutation ... that's a bit of a structural error in how the resolvers work for each case

abhimanyusinghgaur · 2020-04-30T09:41:28Z

dgraph/cmd/alpha/run.go

-	http.HandleFunc("/admin/export", exportHandler)
-	http.HandleFunc("/admin/config/lru_mb", memoryLimitHandler)
+	http.Handle("/admin/shutdown", adminAuthHandler(http.HandlerFunc(shutDownHandler),
+		adminAuthOptions{allowedMethods: map[string]bool{


change the type for allowedMethods to a alias type

gja · 2020-04-30T09:56:19Z

graphql/admin/admin.go

-		"login":    resolveLogin,
-		"restore":  resolveRestore,
-		"shutdown": resolveShutdown,
+	adminMutationResolvers := map[string]resolve.MutationResolver{


Please make this configuration into a map (or even a configuration file).

"backup": applyMiddleWare(resolveBackup, middleware["backups"])

middleware := { "backup": &MiddlewareOptions{ poorman: true, ipwhitelist: true } } http.handle("/backup", AdminAuthHandler(handleBackup, middleware["backups"])) foo.AddMutationResolver(AdminAuthResolver(someResolver(qtx, etc...), middleware["backups"]))

…dmin-auth # Conflicts: # graphql/admin/config.go # testutil/graphql.go

gja

LGTM

gja · 2020-05-15T10:49:42Z

ee/acl/acl_test.go

@@ -1821,6 +1801,219 @@ func TestHealthForAcl(t *testing.T) {
 	}
 }

+func TestAclForAdminEndpoints(t *testing.T) {
+	tcases := []struct {


move the constant out of the function

abhimanyusinghgaur

Reviewable status: 3 of 24 files reviewed, 15 unresolved discussions (waiting on @gja, @MichaelJCompton, and @pawanrawal)

ee/acl/acl_test.go, line 1927 at r3 (raw file):

Previously, MichaelJCompton (Michael Compton) wrote…

I don't quite get these cases in each of these tests ... why do we need to test this? Aren't there existing tests that show that this works for guardians? So what's really being tested here?

There were no such existing tests. We have limited the admin endpoint access to Guardians only in this PR. So, what I am checking here is two things:

When non-guardians try to access, they always get permission denied error
When guardians try to access, they get either success or some other error not related to permission

ee/acl/acl_test.go, line 1805 at r4 (raw file):

Previously, gja (Tejas Dinkar) wrote…

move the constant out of the function

Done.

graphql/admin/admin.go, line 411 at r3 (raw file):

Previously, gja (Tejas Dinkar) wrote…

middleware := {
	"backup": &MiddlewareOptions{
		poorman: true,
		ipwhitelist: true
	}
}

http.handle("/backup", AdminAuthHandler(handleBackup, middleware["backups"]))
foo.AddMutationResolver(AdminAuthResolver(someResolver(qtx, etc...), middleware["backups"]))

Done.

graphql/admin/admin.go, line 414 at r3 (raw file):

Previously, MichaelJCompton (Michael Compton) wrote…

The 'Then' thing looks like a cute pattern, but is it really needed here? It doesn't look like we use it for anything other than applying the same two middlewares, so what benefit do we get from the generic pattern?

Having it require the complexity of writing it, plus writing tests for it. I'd say think carefully if the pattern is required and if we are using it now, or intend to use it really soon.

Now middlewares are applied while querying the resolver for a query/mutation. Then is being used at that time only. Having it makes it easier to read through the code in resolver.go.

graphql/admin/admin.go, line 621 at r3 (raw file):

Previously, MichaelJCompton (Michael Compton) wrote…

does it matter though? We already have the resolve.AdminMutationMWs.Then pattern ... do we need to not apply the guardian middleware?

If we don't, it would be ok to remove the extra fns, and just apply the standard ones?

Also, if this is really required to only call the one middleware, you have this 'Then' pattern, so what would be the reason to have a specific IpWhitelistingMW4Mutation function and not use 'Then' on a list of middleware that just has the ip whitelisting? Looks to me like it adds a specific function that's already handled by the general case.

Refactored middlewares as a configuration now.

graphql/resolve/middlewares.go, line 71 at r2 (raw file):

Previously, abhimanyusinghgaur (Abhimanyu Singh Gaur) wrote…

Yes, it returns nil error for oss builds.

Done.

graphql/resolve/middlewares.go, line 17 at r3 (raw file):

Previously, MichaelJCompton (Michael Compton) wrote…

does this belong in 'resolve' looks like this and the test belong in admin

Its being user in resolver.go

graphql/resolve/middlewares.go, line 144 at r3 (raw file):

Previously, MichaelJCompton (Michael Compton) wrote…

it's a bit sad that we need versions for query and for mutation ... that's a bit of a structural error in how the resolvers work for each case

Done.

…dmin-auth

pawanrawal

Reviewed 7 of 18 files at r3, 12 of 14 files at r4, 2 of 2 files at r5.
Reviewable status: all files reviewed, 14 unresolved discussions (waiting on @gja, @MichaelJCompton, @pawanrawal, and @vvbalaji-dgraph)

…dmin-auth

Fixes dgraph-io#4758. This PR adds authentication to following endpoints: /admin/backup (http & graphql) /admin/config/lru_mb (http [GET & PUT] & graphql [query & mutation]) /admin/draining (http & graphql) /admin/export (http & graphql) /admin/shutdown (http & graphql) /admin/restore (graphql only) /admin/listBackups (graphql only) Now, all the above http endpoints and their corresponding graphql versions have following kinds of auth: IP White-listing, if --whitelist flag is passed to alpha Poor-man's auth, if --auth_token flag is passed to alpha Guardian only access, if ACL is enabled This PR also adds query for config in graphql admin, as it was missing earlier. In addition to above points: All the /admin endpoints apply Poor-man's auth check at http level itself, while other auth checks are routed through graphql resolvers. GraphQL Resolvers for health/state and the ones related to ACL User/Group have IP whitelisting middleware applied, while dgraph handles Guardian auth for them. /alter has the existing behaviour of checking only Poor-man's and Guardian auth. GraphQL Resolvers related to schema don't apply IP whitelisting as to keep them in sync with /alter. They do apply Guardian auth. Any GraphQL admin introspection queries don't require IP whitelisting or Guardian auth.

abhimanyusinghgaur added 2 commits April 8, 2020 13:42

add auth for admin endpoints

91fc1ca

add poor man's auth

c78becc

abhimanyusinghgaur requested a review from pawanrawal April 13, 2020 09:18

pawanrawal reviewed Apr 13, 2020

View reviewed changes

abhimanyusinghgaur added the area/enterprise/acl Related to Access Control Lists label Apr 14, 2020

add tests

563ecde

abhimanyusinghgaur marked this pull request as ready for review April 15, 2020 10:18

abhimanyusinghgaur requested review from manishrjain and a team as code owners April 15, 2020 10:18

update auth for /alter and /admin/schema

1dc12d1

abhimanyusinghgaur changed the title ~~graphql: Add authn for graphql and http admin endpoints~~ [BREAKING] Add authn for graphql and http admin endpoints Apr 15, 2020

remove whitelisting check for /alter

7006028

manishrjain reviewed Apr 15, 2020

View reviewed changes

abhimanyusinghgaur changed the title ~~[BREAKING] Add authn for graphql and http admin endpoints~~ Add authn for graphql and http admin endpoints Apr 16, 2020

abhimanyusinghgaur added 6 commits April 17, 2020 09:01

Merge branch 'master' of github.com:dgraph-io/dgraph into abhimanyu/a…

603fd25

…dmin-auth # Conflicts: # ee/acl/acl_test.go # graphql/admin/backup.go # graphql/admin/config.go # graphql/admin/draining.go # graphql/admin/export.go # graphql/admin/shutdown.go

Merge branch 'master' of github.com:dgraph-io/dgraph into abhimanyu/a…

5629249

…dmin-auth # Conflicts: # ee/acl/acl_test.go # testutil/graphql.go

Merge branch 'master' of github.com:dgraph-io/dgraph into abhimanyu/a…

deeb2bb

…dmin-auth

add middlewares

08772df

Merge branch 'master' of github.com:dgraph-io/dgraph into abhimanyu/a…

4009e29

…dmin-auth

fix test

9ea03fd

abhimanyusinghgaur commented Apr 20, 2020

View reviewed changes

gja suggested changes Apr 20, 2020

View reviewed changes

pawanrawal suggested changes Apr 20, 2020

View reviewed changes

abhimanyusinghgaur added 7 commits April 22, 2020 18:54

Merge branch 'master' of github.com:dgraph-io/dgraph into abhimanyu/a…

262b944

…dmin-auth

Merge branch 'master' of github.com:dgraph-io/dgraph into abhimanyu/a…

9af6def

…dmin-auth

incomplete changes

1b64682

final changes

46330f9

add test

3497cbb

fix tests

fa2b651

add more tests

1a471ba

abhimanyusinghgaur commented Apr 29, 2020

View reviewed changes

Merge branch 'master' of github.com:dgraph-io/dgraph into abhimanyu/a…

0b92b7f

…dmin-auth

MichaelJCompton suggested changes Apr 30, 2020

View reviewed changes

abhimanyusinghgaur commented Apr 30, 2020

View reviewed changes

gja reviewed Apr 30, 2020

View reviewed changes

abhimanyusinghgaur added 5 commits May 14, 2020 01:43

Merge branch 'master' of github.com:dgraph-io/dgraph into abhimanyu/a…

076e68b

…dmin-auth # Conflicts: # graphql/admin/config.go # testutil/graphql.go

manage middlewares as configuration

636999f

route admin http requests through graphql admin server

0d75284

add test

75a4d8b

restructure tests

b6e6863

gja approved these changes May 15, 2020

View reviewed changes

final touches

4f513fb

abhimanyusinghgaur commented May 15, 2020

View reviewed changes

fix tests

ed81244

abhimanyusinghgaur requested a review from vvbalaji-dgraph as a code owner May 15, 2020 11:56

abhimanyusinghgaur added 2 commits May 15, 2020 18:28

fix test

1d80239

Merge branch 'master' of github.com:dgraph-io/dgraph into abhimanyu/a…

3677c15

…dmin-auth

pawanrawal approved these changes May 18, 2020

View reviewed changes

abhimanyusinghgaur added 2 commits May 18, 2020 19:28

Merge branch 'master' of github.com:dgraph-io/dgraph into abhimanyu/a…

aa4e3cc

…dmin-auth

Merge branch 'master' of github.com:dgraph-io/dgraph into abhimanyu/a…

8fe2324

…dmin-auth

abhimanyusinghgaur added the area/graphql Issues related to GraphQL support on Dgraph. label May 19, 2020

abhimanyusinghgaur merged commit 8992238 into master May 19, 2020

abhimanyusinghgaur deleted the abhimanyu/admin-auth branch May 19, 2020 12:00

danielmai changed the title ~~Add authn for graphql and http admin endpoints~~ [BREAKING] Add authn for graphql and http admin endpoints Oct 26, 2020

danielmai changed the title ~~[BREAKING] Add authn for graphql and http admin endpoints~~ Add authn for graphql and http admin endpoints Oct 26, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add authn for graphql and http admin endpoints #5162

Add authn for graphql and http admin endpoints #5162

abhimanyusinghgaur commented Apr 10, 2020 •

edited by dgraph-bot

Loading

pawanrawal left a comment

manishrjain left a comment

abhimanyusinghgaur left a comment

gja left a comment

gja Apr 20, 2020

gja Apr 20, 2020

gja Apr 20, 2020

gja Apr 20, 2020

pawanrawal left a comment

abhimanyusinghgaur left a comment

MichaelJCompton left a comment

abhimanyusinghgaur Apr 30, 2020

gja Apr 30, 2020

gja Apr 30, 2020

gja left a comment

gja May 15, 2020

abhimanyusinghgaur left a comment

pawanrawal left a comment

Add authn for graphql and http admin endpoints #5162

Add authn for graphql and http admin endpoints #5162

Conversation

abhimanyusinghgaur commented Apr 10, 2020 • edited by dgraph-bot Loading

Description.

GitHub Issue or Jira number.

Other components or 3rd party tools affected (or regression areas).

Affected releases.

Changelog tags.

Please indicate if this is a breaking change.

Please indicate if this is an enterprise feature.

Please indicate if documentation needs to be updated.

Please indicate if end to end testing is needed.

pawanrawal left a comment

Choose a reason for hiding this comment

manishrjain left a comment

Choose a reason for hiding this comment

abhimanyusinghgaur left a comment

Choose a reason for hiding this comment

gja left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pawanrawal left a comment

Choose a reason for hiding this comment

abhimanyusinghgaur left a comment

Choose a reason for hiding this comment

MichaelJCompton left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gja left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

abhimanyusinghgaur left a comment

Choose a reason for hiding this comment

pawanrawal left a comment

Choose a reason for hiding this comment

abhimanyusinghgaur commented Apr 10, 2020 •

edited by dgraph-bot

Loading