Skip to content

Commit

Permalink
Fix(GraphQL): Fix bug with password query rewriting (#7011)
Browse files Browse the repository at this point in the history
* Fix bug with password query rewriting

* Remove query.ResponseName() from query_rewriter.go

(cherry picked from commit 46a39c6)
  • Loading branch information
vmrajas committed Nov 27, 2020
1 parent 161eddf commit 5c7605a
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 3 deletions.
28 changes: 28 additions & 0 deletions graphql/e2e/common/query.go
Original file line number Diff line number Diff line change
Expand Up @@ -3051,6 +3051,33 @@ func checkUser(t *testing.T, userObj, expectedObj *user) {
}
}

func checkUserPasswordWithAlias(t *testing.T, userObj, expectedObj *user) {
checkUserParams := &GraphQLParams{
Query: `query checkUserPassword($name: String!, $pwd: String!) {
verify : checkUserPassword(name: $name, password: $pwd) { name }
}`,
Variables: map[string]interface{}{
"name": userObj.Name,
"pwd": userObj.Password,
},
}

gqlResponse := checkUserParams.ExecuteAsPost(t, GraphqlURL)
RequireNoGQLErrors(t, gqlResponse)

var result struct {
CheckUserPasword *user `json:"verify,omitempty"`
}

err := json.Unmarshal([]byte(gqlResponse.Data), &result)
require.Nil(t, err)

opt := cmpopts.IgnoreFields(user{}, "Password")
if diff := cmp.Diff(expectedObj, result.CheckUserPasword, opt); diff != "" {
t.Errorf("result mismatch (-want +got):\n%s", diff)
}
}

func passwordTest(t *testing.T) {
newUser := &user{
Name: "Test User",
Expand Down Expand Up @@ -3095,6 +3122,7 @@ func passwordTest(t *testing.T) {
string(gqlResponse.Data))

checkUser(t, newUser, newUser)
checkUserPasswordWithAlias(t, newUser, newUser)
checkUser(t, &user{Name: "Test User", Password: "Wrong Pass"}, nil)

gqlResponse = postExecutor(t, GraphqlURL, updateUserParams)
Expand Down
6 changes: 3 additions & 3 deletions graphql/resolve/query_rewriter.go
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ func passwordQuery(m schema.Query, authRw *authRewriter) (*gql.GraphQuery, error
// or dgQuery may be empty and its children may contain check<Type>Password query.
// Find the exact dgQuery with the name check<Type>Password query.
mainQuery := dgQuery
for !strings.HasPrefix(mainQuery.Attr, m.ResponseName()) {
for !strings.HasPrefix(mainQuery.Attr, m.Name()) {
mainQuery = mainQuery.Children[0]
}

Expand Down Expand Up @@ -412,7 +412,7 @@ func rewriteAsGet(
// caught here but in case of interface, we need to check validity on each
// implementing type as Rules for the interface are made empty.
if rbac == schema.Negative {
return &gql.GraphQuery{Attr: query.ResponseName() + "()"}
return &gql.GraphQuery{Attr: query.Name() + "()"}
}

// For interface, empty query should be returned if Auth rules are
Expand All @@ -427,7 +427,7 @@ func rewriteAsGet(
}

if !implementingTypesHasFailedRules {
return &gql.GraphQuery{Attr: query.ResponseName() + "()"}
return &gql.GraphQuery{Attr: query.Name() + "()"}
}
}

Expand Down
19 changes: 19 additions & 0 deletions graphql/resolve/query_test.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2352,6 +2352,25 @@
}
}
-
name: "Password query with alias"
gqlquery: |
query {
verify : checkUserPassword(name: "user1", pwd: "Password") {
name
}
}
dgquery: |-
query {
checkUserPassword(func: eq(User.name, "user1")) @filter((eq(val(pwd), 1) AND type(User))) {
name : User.name
dgraph.uid : uid
}
checkPwd(func: eq(User.name, "user1")) @filter(type(User)) {
pwd as checkpwd(User.pwd, "Password")
}
}
- name: "Rewrite without custom fields"
gqlquery: |
query {
Expand Down

0 comments on commit 5c7605a

Please sign in to comment.