feat(GraphQL): Support auth with custom DQL #7775

minhaj-shakeel · 2021-05-03T14:54:13Z

Fixes GRAPHQL-1171.
This PR adds support for @auth on custom DQL queries.
This PR also adds the parsing step to the DQL query.
The Algorithm tries to figure out the queried type at the root of the query either from the root func or filters.
Suppose if the root func is func: type(Person) or func: has(Person.name) or func: eq(Person.age, 10) then
it infers that the queried type is Person.
However, there are certain DQL queries on which @auth rules are not added currently since the queried type is not clear.
for eg:

me(func: uid(0x1, 0x2)) {
 ...
}

or the query:

me(func: has(name@en) {
 ...
}

This change is

abhimanyusinghgaur

Reviewed 1 of 9 files at r1.
Reviewable status: 1 of 9 files reviewed, 8 unresolved discussions (waiting on @minhaj-shakeel and @pawanrawal)

graphql/dgraph/graphquery.go, line 217 at r1 (raw file):

writeFilterArguments(b, q.Func.Args, q.Func.Attr, q.Func.Name)

maybe just pass q.Func and use that in the function

graphql/dgraph/graphquery.go, line 223 at r1 (raw file):

}

func maybeQuoteArg(fn string, arg interface{}) string {

I think we have a similar function in resolve pkg. Can't we reuse that here?
If it's a package dependency issue, we can move that function to a higher level package in the dependency tree (schema pkg), or in the packagex.

graphql/e2e/auth/schema.graphql, line 991 at r1 (raw file):

$search: string

stale

graphql/e2e/auth/schema.graphql, line 997 at r1 (raw file):

orderdesc: val(age)

can this directly work here? as the map age is a map from owner uid to age value, while the root func here is iterating over the Issue uids. So, I think this might not produce the intended results after all.
This var query might be needed:

		iss as var(func: type(Issue)) @filter(has(Issue.owner)) {
			Issue.owner {
				age as User.age
			}
			ownerAge as max(val(age))
		}

Then use val(ownerAge) in the orderdesc.

graphql/resolve/custom_auth_query_test.yaml, line 26 at r1 (raw file):

    }

- name : "custom DQL query with var block RBAC rules true"

can we also add another test where there are multiple var blocks in the DQL, and all of them are used in the user's query?

graphql/resolve/custom_auth_query_test.yaml, line 75 at r1 (raw file):

iss as var(func: uid(0x1)) @filter((uid(0x2) AND has(Issue.owner)))

👍
why is the AND has(Issue.owner) here?

graphql/resolve/query_rewriter.go, line 641 at r1 (raw file):

Quoted 12 lines of code…

	args := query.Arguments()
	vars := make(map[string]string)
	for k, v := range args {
		// dgoapi.Request{}.Vars accepts only string values for variables,
		// so need to convert all variable values to string
		vStr, err := convertScalarToString(v)
		if err != nil {
			return nil, err
		}
		// the keys in dgoapi.Request{}.Vars are assumed to be prefixed with $
		vars["$"+k] = vStr
	}

we should put this in a function, this piece of code is being used at two places

graphql/resolve/query_rewriter.go, line 680 at r1 (raw file):

	typeName = extractTypeFromFilter(dgQuery.Filter)
	return typeName

return extractTypeFromFilter(dgQuery.Filter)

also, should we try to extract type from order?

minhaj-shakeel

Reviewable status: 0 of 10 files reviewed, 8 unresolved discussions (waiting on @abhimanyusinghgaur and @pawanrawal)

graphql/dgraph/graphquery.go, line 217 at r1 (raw file):