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): Apply auth rules on type having @dgraph directive #5702

Merged
merged 4 commits into from
Jun 24, 2020

Conversation

arijitAD
Copy link

@arijitAD arijitAD commented Jun 22, 2020

Fixes GRAPHQL-525
Fixes #5700

Auth rules were not applied on type having @dgraph directive because they were stored in authRules map corresponding to Graphql type name and was fetched with Dgraph type name when rewriting the query.


This change is Reviewable

@github-actions github-actions bot added the area/graphql Issues related to GraphQL support on Dgraph. label Jun 22, 2020
@arijitAD arijitAD changed the title fix(GraphQL): Apply auth rules on type and field having @dgraph directive fix(GraphQL): Apply auth rules on type having @dgraph directive Jun 22, 2020
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.

:lgtm:

I remember you were saying that even the RBAC rules were not working. Can you add a test for that as well?

Reviewed 4 of 4 files at r1.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @arijitAD and @MichaelJCompton)


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

		Query: `
		mutation {
			addStudent(input : [{email : "` + emails[0] + `"}, {email : "` + emails[1] + `"}]) {

Can you use GraphQL variables here instead of string concatenation? That is usually the correct way to do these things.

Copy link
Author

@arijitAD arijitAD left a comment

Choose a reason for hiding this comment

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

Added RBAC test,

Reviewable status: 1 of 4 files reviewed, 1 unresolved discussion (waiting on @MichaelJCompton and @pawanrawal)


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

Previously, pawanrawal (Pawan Rawal) wrote…

Can you use GraphQL variables here instead of string concatenation? That is usually the correct way to do these things.

Done.

@arijitAD arijitAD merged commit 8562311 into master Jun 24, 2020
arijitAD pushed a commit that referenced this pull request Jul 7, 2020
Auth rules were not applied on type having @dgraph directive because they were stored in authRules map corresponding to Graphql type name and was fetched with Dgraph type name when rewriting the query.

(cherry picked from commit 8562311)
arijitAD pushed a commit that referenced this pull request Jul 9, 2020
… (#5863)

Auth rules were not applied on type having @dgraph directive because they were stored in authRules map corresponding to Graphql type name and was fetched with Dgraph type name when rewriting the query.

(cherry picked from commit 8562311)
dna2github pushed a commit to dna2fork/dgraph that referenced this pull request Jul 18, 2020
…rmodeinc#5702)

Auth rules were not applied on type having @dgraph directive because they were stored in authRules map corresponding to Graphql type name and was fetched with Dgraph type name when rewriting the query.
@joshua-goldstein joshua-goldstein deleted the arijitAD/auth_dgraph_pred branch August 11, 2022 20:48
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.

GraphQL: Auth rules on type with @dgraph pred is not applied
2 participants