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

fix(GraphQL): fix @auth rules evaluation in case of null variables in custom claims. #7380

Merged
merged 20 commits into from
Feb 9, 2021

Conversation

minhaj-shakeel
Copy link
Contributor

@minhaj-shakeel minhaj-shakeel commented Jan 29, 2021

Fixes GRAPHQL-988.


This change is Reviewable

@github-actions github-actions bot added the area/graphql Issues related to GraphQL support on Dgraph. label Jan 29, 2021
Copy link
Contributor

@pawanrawal pawanrawal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 6 of 6 files at r1.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @minhaj-shakeel)


graphql/e2e/auth/auth_test.go, line 498 at r1 (raw file):

}

func TestAuthRulesWithNullValuesInJWT(t *testing.T) {

add a case for deep value as well


graphql/resolve/auth_query_test.yaml, line 1078 at r1 (raw file):

      queryUser {
        username
        tickets {

Tickets should not be returned if auth rule is negative here because of null USER value?

Copy link
Contributor Author

@minhaj-shakeel minhaj-shakeel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


graphql/e2e/auth/auth_test.go, line 498 at r1 (raw file):

Previously, pawanrawal (Pawan Rawal) wrote…

add a case for deep value as well

Done.


graphql/resolve/auth_query_test.yaml, line 1078 at r1 (raw file):

Previously, pawanrawal (Pawan Rawal) wrote…

Tickets should not be returned if auth rule is negative here because of null USER value?

Tickets are not being returned as expected. The given query will only return username and an empty array for tickets.

Copy link
Contributor

@pawanrawal pawanrawal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 5 of 5 files at r2.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @minhaj-shakeel)

@minhaj-shakeel minhaj-shakeel merged commit f437724 into master Feb 9, 2021
minhaj-shakeel added a commit that referenced this pull request Feb 9, 2021
… custom claims. (#7380)

Fixes GRAPHQL-988.

(cherry picked from commit f437724)
@minhaj-shakeel minhaj-shakeel deleted the minhaj/fix-query-rewriting branch April 22, 2021 04:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/graphql Issues related to GraphQL support on Dgraph.
Development

Successfully merging this pull request may close these issues.

2 participants