From 3bd909314e5a4f50988614d9096128f08f9bdab8 Mon Sep 17 00:00:00 2001 From: Abhimanyu Singh Gaur <12651351+abhimanyusinghgaur@users.noreply.github.com> Date: Thu, 10 Dec 2020 16:18:42 +0530 Subject: [PATCH 1/5] remove unnecessary `dgraph.uid : uid` selections for auth queries --- graphql/resolve/auth_add_test.yaml | 156 ++++++++------------------ graphql/resolve/auth_delete_test.yaml | 52 +++------ graphql/resolve/auth_query_test.yaml | 134 ++++------------------ graphql/resolve/auth_test.go | 14 --- graphql/resolve/auth_update_test.yaml | 121 ++++++-------------- graphql/resolve/query_rewriter.go | 6 +- 6 files changed, 119 insertions(+), 364 deletions(-) diff --git a/graphql/resolve/auth_add_test.yaml b/graphql/resolve/auth_add_test.yaml index 99089184325..c952b33b8c9 100644 --- a/graphql/resolve/auth_add_test.yaml +++ b/graphql/resolve/auth_add_test.yaml @@ -162,11 +162,8 @@ inProject : Column.inProject { roles : Project.roles @filter(eq(Role.permission, "ADMIN")) { assignedTo : Role.assignedTo @filter(eq(User.username, "user1")) - dgraph.uid : uid } - dgraph.uid : uid } - dgraph.uid : uid } Ticket(func: uid(Ticket3)) @filter(uid(TicketAuth4)) { uid @@ -177,20 +174,16 @@ inProject : Column.inProject { roles : Project.roles @filter(eq(Role.permission, "EDIT")) { assignedTo : Role.assignedTo @filter(eq(User.username, "user1")) - dgraph.uid : uid } - dgraph.uid : uid } - dgraph.uid : uid } - dgraph.uid : uid } } authjson: | - { + { "Column": [ { "uid": "0x456" } ], - "Ticket": [ { "uid": "0x789" } ] - } + "Ticket": [ { "uid": "0x789" } ] + } - name: "Add multiple nodes of different types that fails auth" gqlquery: | @@ -231,11 +224,8 @@ inProject : Column.inProject { roles : Project.roles @filter(eq(Role.permission, "ADMIN")) { assignedTo : Role.assignedTo @filter(eq(User.username, "user1")) - dgraph.uid : uid } - dgraph.uid : uid } - dgraph.uid : uid } Ticket(func: uid(Ticket3)) @filter(uid(TicketAuth4)) { uid @@ -246,13 +236,9 @@ inProject : Column.inProject { roles : Project.roles @filter(eq(Role.permission, "EDIT")) { assignedTo : Role.assignedTo @filter(eq(User.username, "user1")) - dgraph.uid : uid } - dgraph.uid : uid } - dgraph.uid : uid } - dgraph.uid : uid } } authjson: | @@ -293,9 +279,9 @@ } } json: | - { + { "Project2": [ { "uid": "0x123" } ], - "Project5": [ { "uid": "0x123" } ] + "Project5": [ { "uid": "0x123" } ] } uids: | { "Column1": "0x456", "Ticket3": "0x789", "Column4": "0x459", "Ticket6": "0x799" } @@ -309,11 +295,8 @@ inProject : Column.inProject { roles : Project.roles @filter(eq(Role.permission, "ADMIN")) { assignedTo : Role.assignedTo @filter(eq(User.username, "user1")) - dgraph.uid : uid } - dgraph.uid : uid } - dgraph.uid : uid } Ticket(func: uid(Ticket3)) @filter(uid(TicketAuth4)) { uid @@ -324,20 +307,16 @@ inProject : Column.inProject { roles : Project.roles @filter(eq(Role.permission, "EDIT")) { assignedTo : Role.assignedTo @filter(eq(User.username, "user1")) - dgraph.uid : uid } - dgraph.uid : uid } - dgraph.uid : uid } - dgraph.uid : uid } } authjson: | - { + { "Column": [ { "uid": "0x456" }, { "uid": "0x459" } ], - "Ticket": [ { "uid": "0x789" }, { "uid": "0x799" } ] - } + "Ticket": [ { "uid": "0x789" }, { "uid": "0x799" } ] + } - name: "Add multiples of multiple nodes of different types that fails auth" gqlquery: | @@ -372,9 +351,9 @@ } } json: | - { + { "Project2": [ { "uid": "0x123" } ], - "Project5": [ { "uid": "0x123" } ] + "Project5": [ { "uid": "0x123" } ] } uids: | { "Column1": "0x456", "Ticket3": "0x789", "Column4": "0x459", "Ticket6": "0x799" } @@ -388,11 +367,8 @@ inProject : Column.inProject { roles : Project.roles @filter(eq(Role.permission, "ADMIN")) { assignedTo : Role.assignedTo @filter(eq(User.username, "user1")) - dgraph.uid : uid } - dgraph.uid : uid } - dgraph.uid : uid } Ticket(func: uid(Ticket3)) @filter(uid(TicketAuth4)) { uid @@ -403,25 +379,21 @@ inProject : Column.inProject { roles : Project.roles @filter(eq(Role.permission, "EDIT")) { assignedTo : Role.assignedTo @filter(eq(User.username, "user1")) - dgraph.uid : uid } - dgraph.uid : uid } - dgraph.uid : uid } - dgraph.uid : uid } } authjson: | - { + { "Column": [ { "uid": "0x456" } ], - "Ticket": [ { "uid": "0x789" }, { "uid": "0x799" } ] - } + "Ticket": [ { "uid": "0x789" }, { "uid": "0x799" } ] + } error: { "message": "mutation failed because authorization failed" } # See comments about additional deletes in add_mutation_test.yaml. -# Because of those additional deletes, for example, when we add a column and +# Because of those additional deletes, for example, when we add a column and # link it to an existing ticket, we remove that ticket from the column it was # attached to ... so we need authorization to update that column as well # as to add the new column. @@ -464,15 +436,12 @@ inProject : Column.inProject { roles : Project.roles @filter(eq(Role.permission, "ADMIN")) { assignedTo : Role.assignedTo @filter(eq(User.username, "user1")) - dgraph.uid : uid } - dgraph.uid : uid } - dgraph.uid : uid } } json: | - { + { "Project2": [ { "uid": "0x123" } ], "Ticket3": [ { "uid": "0x789" } ], "Column4": [ { "uid": "0x799" } ], @@ -490,18 +459,15 @@ inProject : Column.inProject { roles : Project.roles @filter(eq(Role.permission, "ADMIN")) { assignedTo : Role.assignedTo @filter(eq(User.username, "user1")) - dgraph.uid : uid } - dgraph.uid : uid } - dgraph.uid : uid } } authjson: | - { - "Column": [ { "uid": "0x456" } ] + { + "Column": [ { "uid": "0x456" } ] } - + - name: "Add with auth on additional delete that fails" gqlquery: | mutation addColumn($col: AddColumnInput!) { @@ -541,15 +507,12 @@ inProject : Column.inProject { roles : Project.roles @filter(eq(Role.permission, "ADMIN")) { assignedTo : Role.assignedTo @filter(eq(User.username, "user1")) - dgraph.uid : uid } - dgraph.uid : uid } - dgraph.uid : uid } } json: | - { + { "Project2": [ { "uid": "0x123" } ], "Ticket3": [ { "uid": "0x789" } ], "Column4": [ { "uid": "0x799" } ] @@ -566,17 +529,14 @@ inProject : Column.inProject { roles : Project.roles @filter(eq(Role.permission, "ADMIN")) { assignedTo : Role.assignedTo @filter(eq(User.username, "user1")) - dgraph.uid : uid } - dgraph.uid : uid } - dgraph.uid : uid } } authjson: | - { - "Column": [ { "uid": "0x456" } ] - } + { + "Column": [ { "uid": "0x456" } ] + } error: { "message": "couldn't rewrite query for mutation addColumn because authorization failed" } @@ -592,11 +552,11 @@ jwtvar: USER: "user1" variables: | - { + { "proj": { "name": "Project1", "pwd": "Password", - "columns": [ { + "columns": [ { "name": "a column", "tickets": [ { "id": "0x789" } ] } ] @@ -620,23 +580,20 @@ inProject : Column.inProject { roles : Project.roles @filter(eq(Role.permission, "ADMIN")) { assignedTo : Role.assignedTo @filter(eq(User.username, "user1")) - dgraph.uid : uid } - dgraph.uid : uid } - dgraph.uid : uid } } json: | - { + { "Ticket3": [ { "uid": "0x789" } ], "Column4": [ { "uid": "0x799" } ], "Column4.auth": [ { "uid": "0x799" } ] } uids: | - { + { "Project1": "0x123", - "Column2": "0x456" + "Column2": "0x456" } authquery: |- query { @@ -648,11 +605,8 @@ inProject : Column.inProject { roles : Project.roles @filter(eq(Role.permission, "ADMIN")) { assignedTo : Role.assignedTo @filter(eq(User.username, "user1")) - dgraph.uid : uid } - dgraph.uid : uid } - dgraph.uid : uid } Project(func: uid(Project3)) @filter(uid(ProjectAuth4)) { uid @@ -661,13 +615,11 @@ ProjectAuth4 as var(func: uid(Project3)) @cascade { roles : Project.roles @filter(eq(Role.permission, "ADMIN")) { assignedTo : Role.assignedTo @filter(eq(User.username, "user1")) - dgraph.uid : uid } - dgraph.uid : uid } } authjson: | - { + { "Column": [ { "uid": "0x456" } ], "Project": [ { "uid": "0x123" } ] } @@ -684,11 +636,11 @@ jwtvar: USER: "user1" variables: | - { + { "proj": { "name": "Project1", "pwd": "Password", - "columns": [ { + "columns": [ { "name": "a column", "tickets": [ { "id": "0x789" } ] } ] @@ -712,20 +664,17 @@ inProject : Column.inProject { roles : Project.roles @filter(eq(Role.permission, "ADMIN")) { assignedTo : Role.assignedTo @filter(eq(User.username, "user1")) - dgraph.uid : uid } - dgraph.uid : uid } - dgraph.uid : uid } } json: | - { + { "Ticket3": [ { "uid": "0x789" } ], "Column4": [ { "uid": "0x799" } ] } uids: | - { + { "Project1": "0x123", "Column2": "0x456" } @@ -739,11 +688,8 @@ inProject : Column.inProject { roles : Project.roles @filter(eq(Role.permission, "ADMIN")) { assignedTo : Role.assignedTo @filter(eq(User.username, "user1")) - dgraph.uid : uid } - dgraph.uid : uid } - dgraph.uid : uid } Project(func: uid(Project3)) @filter(uid(ProjectAuth4)) { uid @@ -752,16 +698,14 @@ ProjectAuth4 as var(func: uid(Project3)) @cascade { roles : Project.roles @filter(eq(Role.permission, "ADMIN")) { assignedTo : Role.assignedTo @filter(eq(User.username, "user1")) - dgraph.uid : uid } - dgraph.uid : uid } } authjson: | - { + { "Column": [ { "uid": "0x456" } ], "Project": [ { "uid": "0x123" } ] - } + } error: { "message": "couldn't rewrite query for mutation addProject because authorization failed" } @@ -871,9 +815,7 @@ ProjectAuth2 as var(func: uid(Project1)) @cascade { roles : Project.roles @filter(eq(Role.permission, "ADMIN")) { assignedTo : Role.assignedTo @filter(eq(User.username, "user1")) - dgraph.uid : uid } - dgraph.uid : uid } } authjson: | @@ -927,7 +869,6 @@ Issue1 as var(func: uid(0x789)) IssueAuth2 as var(func: uid(Issue1)) @cascade { owner : Issue.owner @filter(eq(User.username, "user1")) - dgraph.uid : uid } } authjson: | @@ -979,7 +920,6 @@ Issue1 as var(func: uid(0x789)) Issue2 as var(func: uid(Issue1)) @cascade { owner : Issue.owner @filter(eq(User.username, "user1")) - dgraph.uid : uid } } error: @@ -1043,11 +983,11 @@ } } } - jwtvar: + jwtvar: USER: "user1" ANS: "true" variables: | - { "question": + { "question": [{ "text": "A Question", "pwd": "password", @@ -1072,9 +1012,7 @@ dgraph.type author : Post.author @filter(eq(Author.name, "user1")) { name : Author.name - dgraph.uid : uid } - dgraph.uid : uid } } authjson: | @@ -1092,11 +1030,11 @@ } } } - jwtvar: + jwtvar: USER: "user1" ANS: "true" variables: | - { "question": + { "question": [{ "text": "A Question", "pwd": "password", @@ -1121,18 +1059,16 @@ dgraph.type author : Post.author @filter(eq(Author.name, "user1")) { name : Author.name - dgraph.uid : uid } - dgraph.uid : uid } } authjson: | {"Question": [ ], "Author": [ { "uid" : "0x456"} ] } - error: + error: { "message": "mutation failed because authorization failed"} - name: "Add type with having RBAC rule on interface successfully" - gqlquery: | + gqlquery: | mutation addFbPost($post: [AddFbPostInput!]!){ addFbPost(input: $post){ fbPost { @@ -1143,11 +1079,11 @@ } } } - jwtvar: + jwtvar: USER: "user1" ROLE: "ADMIN" variables: | - { "post": + { "post": [{ "text": "A Question", "pwd": "password", @@ -1168,16 +1104,14 @@ dgraph.type author : Post.author @filter(eq(Author.name, "user1")) { name : Author.name - dgraph.uid : uid } - dgraph.uid : uid } } authjson: | {"FbPost": [ {"uid": "0x123"}]} - name: "Add type with Having RBAC rule on interface failed" - gqlquery: | + gqlquery: | mutation addFbPost($post: [AddFbPostInput!]!){ addFbPost(input: $post){ fbPost{ @@ -1188,7 +1122,7 @@ } } } - jwtvar: + jwtvar: USER: "user1" ROLE: "USER" variables: | @@ -1203,6 +1137,6 @@ } uids: | {"FbPost1": "0x123", "Author1": "0x456" } - error: + error: {"message" : "mutation failed because authorization failed"} \ No newline at end of file diff --git a/graphql/resolve/auth_delete_test.yaml b/graphql/resolve/auth_delete_test.yaml index 9570b0897f0..b4212bec76c 100644 --- a/graphql/resolve/auth_delete_test.yaml +++ b/graphql/resolve/auth_delete_test.yaml @@ -132,13 +132,9 @@ inProject : Column.inProject { roles : Project.roles @filter(eq(Role.permission, "EDIT")) { assignedTo : Role.assignedTo @filter(eq(User.username, "user1")) - dgraph.uid : uid } - dgraph.uid : uid } - dgraph.uid : uid } - dgraph.uid : uid } } @@ -195,13 +191,9 @@ inProject : Column.inProject { roles : Project.roles @filter(eq(Role.permission, "EDIT")) { assignedTo : Role.assignedTo @filter(eq(User.username, "user1")) - dgraph.uid : uid } - dgraph.uid : uid } - dgraph.uid : uid } - dgraph.uid : uid } ticket(func: uid(Ticket5)) { title : Ticket.title @@ -228,13 +220,9 @@ inProject : Column.inProject { roles : Project.roles @filter(eq(Role.permission, "VIEW")) { assignedTo : Role.assignedTo @filter(eq(User.username, "user1")) - dgraph.uid : uid } - dgraph.uid : uid } - dgraph.uid : uid } - dgraph.uid : uid } var(func: uid(Ticket5)) { Column6 as Ticket.onColumn @@ -255,19 +243,14 @@ ProjectAuth12 as var(func: uid(Project7)) @cascade { roles : Project.roles @filter(eq(Role.permission, "VIEW")) { assignedTo : Role.assignedTo @filter(eq(User.username, "user1")) - dgraph.uid : uid } - dgraph.uid : uid } ColumnAuth14 as var(func: uid(Column6)) @cascade { inProject : Column.inProject { roles : Project.roles @filter(eq(Role.permission, "VIEW")) { assignedTo : Role.assignedTo @filter(eq(User.username, "user1")) - dgraph.uid : uid } - dgraph.uid : uid } - dgraph.uid : uid } } @@ -530,7 +513,7 @@ gqlquery: | mutation ($ids: [ID!]) { deleteIssue(filter: {id: $ids}) { - numUids + numUids } } variables: | @@ -554,7 +537,6 @@ Issue1 as var(func: uid(0x1, 0x2)) @filter(type(Issue)) IssueAuth2 as var(func: uid(Issue1)) @cascade { owner : Issue.owner @filter(eq(User.username, "user1")) - dgraph.uid : uid } } @@ -562,11 +544,11 @@ gqlquery: | mutation ($ids: [ID!]) { deleteIssue(filter: {id: $ids}) { - numUids + numUids } } variables: | - { + { "ids": ["0x1", "0x2"] } jwtvar: @@ -586,7 +568,7 @@ gqlquery: | mutation ($ids: [ID!]) { deleteRole(filter: {id: $ids}) { - numUids + numUids } } variables: | @@ -610,7 +592,7 @@ gqlquery: | mutation ($ids: [ID!]) { deleteRole(filter: {id: $ids}) { - numUids + numUids } } variables: | @@ -636,7 +618,7 @@ gqlquery: | mutation ($ids: [ID!]) { deletePost(filter: {id: $ids}) { - numUids + numUids } } variables: | @@ -672,18 +654,14 @@ dgraph.type author : Post.author @filter(eq(Author.name, "user1")) { name : Author.name - dgraph.uid : uid } - dgraph.uid : uid } Answer1 as var(func: type(Answer)) AnswerAuth4 as var(func: uid(Answer1)) @cascade { dgraph.type author : Post.author @filter(eq(Author.name, "user1")) { name : Author.name - dgraph.uid : uid } - dgraph.uid : uid } } @@ -691,7 +669,7 @@ gqlquery: | mutation ($ids: [ID!]) { deletePost(filter: {id: $ids}) { - numUids + numUids } } jwtvar: @@ -715,7 +693,7 @@ gqlquery: | mutation ($ids: [ID!]) { deleteA(filter: {id: $ids}) { - numUids + numUids } } variables: | @@ -741,7 +719,7 @@ gqlquery: | mutation ($ids: [ID!]) { deleteQuestion(filter: {id: $ids}) { - numUids + numUids } } variables: | @@ -776,9 +754,7 @@ dgraph.type author : Post.author @filter(eq(Author.name, "user1")) { name : Author.name - dgraph.uid : uid } - dgraph.uid : uid } } @@ -786,7 +762,7 @@ gqlquery: | mutation ($ids: [ID!]) { deleteQuestion(filter: {id: $ids}) { - numUids + numUids } } variables: | @@ -809,11 +785,11 @@ gqlquery: | mutation ($ids: [ID!]) { deleteFbPost(filter: {id: $ids}) { - numUids + numUids } } variables: | - { + { "ids": ["0x1", "0x2"] } jwtvar: @@ -833,7 +809,7 @@ gqlquery: | mutation ($ids: [ID!]) { deleteFbPost(filter: {id: $ids}) { - numUids + numUids } } variables: | @@ -865,8 +841,6 @@ dgraph.type author : Post.author @filter(eq(Author.name, "user1")) { name : Author.name - dgraph.uid : uid } - dgraph.uid : uid } } \ No newline at end of file diff --git a/graphql/resolve/auth_query_test.yaml b/graphql/resolve/auth_query_test.yaml index 52e54accfa5..502eecd8f4c 100644 --- a/graphql/resolve/auth_query_test.yaml +++ b/graphql/resolve/auth_query_test.yaml @@ -299,9 +299,7 @@ ProjectAuth5 as var(func: uid(Project4)) @cascade { roles : Project.roles @filter(eq(Role.permission, "VIEW")) { assignedTo : Role.assignedTo @filter(eq(User.username, "user1")) - dgraph.uid : uid } - dgraph.uid : uid } var(func: uid(ProjectRoot)) { Column1 as Project.columns @@ -311,11 +309,8 @@ inProject : Column.inProject { roles : Project.roles @filter(eq(Role.permission, "VIEW")) { assignedTo : Role.assignedTo @filter(eq(User.username, "user1")) - dgraph.uid : uid } - dgraph.uid : uid } - dgraph.uid : uid } } @@ -437,7 +432,6 @@ Issue3 as var(func: uid(Issue1)) @filter(uid(IssueAuth2)) IssueAuth2 as var(func: uid(Issue1)) @cascade { owner : Issue.owner @filter(eq(User.username, "user1")) - dgraph.uid : uid } } @@ -455,14 +449,14 @@ ROLE: "USER" USER: "user1" dgquery: |- - query { - queryUser(func: uid(UserRoot)) { - username : User.username - dgraph.uid : uid - } - UserRoot as var(func: uid(User2)) - User2 as var(func: type(User)) + query { + queryUser(func: uid(UserRoot)) { + username : User.username + dgraph.uid : uid } + UserRoot as var(func: uid(User2)) + User2 as var(func: type(User)) + } - name: "Auth with top level AND rbac true" gqlquery: | @@ -484,7 +478,6 @@ Issue1 as var(func: type(Issue)) IssueAuth2 as var(func: uid(Issue1)) @cascade { owner : Issue.owner @filter(eq(User.username, "user1")) - dgraph.uid : uid } } @@ -554,9 +547,9 @@ ROLE: "USER" USER: "user1" dgquery: |- - query { - queryLog() - } + query { + queryLog() + } - name: "Auth with top level AND rbac false" gqlquery: | @@ -569,9 +562,9 @@ ROLE: "USER" USER: "user1" dgquery: |- - query { - queryIssue() - } + query { + queryIssue() + } - name: "Aggregate Query on Auth with top level AND rbac false" gqlquery: | @@ -600,14 +593,14 @@ ROLE: "ADMIN" USER: "user1" dgquery: |- - query { - queryProject(func: uid(ProjectRoot)) { - name : Project.name - dgraph.uid : uid - } - ProjectRoot as var(func: uid(Project1)) - Project1 as var(func: type(Project)) - } + query { + queryProject(func: uid(ProjectRoot)) { + name : Project.name + dgraph.uid : uid + } + ProjectRoot as var(func: uid(Project1)) + Project1 as var(func: type(Project)) + } - name: "Aggregate on Auth with top level OR rbac true" gqlquery: | @@ -655,17 +648,15 @@ Group1 as var(func: type(Group)) GroupAuth2 as var(func: uid(Group1)) @cascade { users : Group.users @filter(eq(User.username, "user1")) - dgraph.uid : uid } GroupAuth3 as var(func: uid(Group1)) @cascade { createdBy : Group.createdBy @filter(eq(User.username, "user1")) - dgraph.uid : uid } } - name: "Auth with top level OR rbac false" gqlquery: | - query { + query { queryProject { name } @@ -684,9 +675,7 @@ ProjectAuth2 as var(func: uid(Project1)) @cascade { roles : Project.roles @filter(eq(Role.permission, "VIEW")) { assignedTo : Role.assignedTo @filter(eq(User.username, "user1")) - dgraph.uid : uid } - dgraph.uid : uid } } @@ -734,13 +723,9 @@ inProject : Column.inProject { roles : Project.roles @filter(eq(Role.permission, "VIEW")) { assignedTo : Role.assignedTo @filter(eq(User.username, "user1")) - dgraph.uid : uid } - dgraph.uid : uid } - dgraph.uid : uid } - dgraph.uid : uid } } @@ -751,7 +736,7 @@ username tickets { id - title + title } } } @@ -778,13 +763,9 @@ inProject : Column.inProject { roles : Project.roles @filter(eq(Role.permission, "VIEW")) { assignedTo : Role.assignedTo @filter(eq(User.username, "user1")) - dgraph.uid : uid } - dgraph.uid : uid } - dgraph.uid : uid } - dgraph.uid : uid } } @@ -795,7 +776,7 @@ username tickets(filter: { title: { anyofterms: "graphql" } }) { id - title + title } } } @@ -822,13 +803,9 @@ inProject : Column.inProject { roles : Project.roles @filter(eq(Role.permission, "VIEW")) { assignedTo : Role.assignedTo @filter(eq(User.username, "user1")) - dgraph.uid : uid } - dgraph.uid : uid } - dgraph.uid : uid } - dgraph.uid : uid } } @@ -853,13 +830,10 @@ MovieAuth3 as var(func: uid(Movie1)) @cascade { regionsAvailable : Movie.regionsAvailable { users : Region.users @filter(eq(User.username, "user1")) - dgraph.uid : uid } - dgraph.uid : uid } MovieAuth4 as var(func: uid(Movie1)) @cascade { regionsAvailable : Movie.regionsAvailable @filter(eq(Region.global, true)) - dgraph.uid : uid } } @@ -893,13 +867,10 @@ MovieAuth5 as var(func: uid(Movie3)) @cascade { regionsAvailable : Movie.regionsAvailable { users : Region.users @filter(eq(User.username, "user1")) - dgraph.uid : uid } - dgraph.uid : uid } MovieAuth6 as var(func: uid(Movie3)) @cascade { regionsAvailable : Movie.regionsAvailable @filter(eq(Region.global, true)) - dgraph.uid : uid } var(func: uid(MovieRoot)) { Region1 as Movie.regionsAvailable @@ -957,13 +928,10 @@ MovieAuth10 as var(func: uid(Movie8)) @cascade { regionsAvailable : Movie.regionsAvailable { users : Region.users @filter(eq(User.username, "user1")) - dgraph.uid : uid } - dgraph.uid : uid } MovieAuth11 as var(func: uid(Movie8)) @cascade { regionsAvailable : Movie.regionsAvailable @filter(eq(Region.global, true)) - dgraph.uid : uid } var(func: uid(MovieRoot)) { Region1 as Movie.regionsAvailable @@ -1001,13 +969,10 @@ MovieAuth3 as var(func: uid(Movie1)) @cascade { regionsAvailable : Movie.regionsAvailable { users : Region.users @filter(eq(User.username, "user1")) - dgraph.uid : uid } - dgraph.uid : uid } MovieAuth4 as var(func: uid(Movie1)) @cascade { regionsAvailable : Movie.regionsAvailable @filter(eq(Region.global, true)) - dgraph.uid : uid } } @@ -1037,13 +1002,10 @@ MovieAuth3 as var(func: uid(Movie1)) @cascade { regionsAvailable : Movie.regionsAvailable { users : Region.users @filter(eq(User.username, "user1")) - dgraph.uid : uid } - dgraph.uid : uid } MovieAuth4 as var(func: uid(Movie1)) @cascade { regionsAvailable : Movie.regionsAvailable @filter(eq(Region.global, true)) - dgraph.uid : uid } } @@ -1146,7 +1108,6 @@ MovieAuth2 as var(func: uid(Movie1)) @filter(eq(Movie.hidden, true)) @cascade MovieAuth3 as var(func: uid(Movie1)) @cascade { regionsAvailable : Movie.regionsAvailable @filter(eq(Region.global, true)) - dgraph.uid : uid } } @@ -1201,13 +1162,9 @@ inProject : Column.inProject { roles : Project.roles @filter(eq(Role.permission, "VIEW")) { assignedTo : Role.assignedTo @filter(eq(User.username, "user1")) - dgraph.uid : uid } - dgraph.uid : uid } - dgraph.uid : uid } - dgraph.uid : uid } } @@ -1261,13 +1218,9 @@ inProject : Column.inProject { roles : Project.roles @filter(eq(Role.permission, "VIEW")) { assignedTo : Role.assignedTo @filter(eq(User.username, "user1")) - dgraph.uid : uid } - dgraph.uid : uid } - dgraph.uid : uid } - dgraph.uid : uid } var(func: uid(UserRoot)) { IssueAggregateResult4 as User.issues @@ -1275,7 +1228,6 @@ Issue6 as var(func: uid(IssueAggregateResult4)) @filter(uid(IssueAuth5)) IssueAuth5 as var(func: uid(IssueAggregateResult4)) @cascade { owner : Issue.owner @filter(eq(User.username, "user1")) - dgraph.uid : uid } var(func: uid(UserRoot)) { Ticket7 as User.tickets @@ -1286,13 +1238,9 @@ inProject : Column.inProject { roles : Project.roles @filter(eq(Role.permission, "VIEW")) { assignedTo : Role.assignedTo @filter(eq(User.username, "user1")) - dgraph.uid : uid } - dgraph.uid : uid } - dgraph.uid : uid } - dgraph.uid : uid } } @@ -1328,7 +1276,6 @@ Issue3 as var(func: uid(IssueAggregateResult1)) @filter(uid(IssueAuth2)) IssueAuth2 as var(func: uid(IssueAggregateResult1)) @cascade { owner : Issue.owner @filter(eq(User.username, "user1")) - dgraph.uid : uid } } @@ -1382,9 +1329,7 @@ dgraph.type author : Post.author @filter(eq(Author.name, "Random")) { name : Author.name - dgraph.uid : uid } - dgraph.uid : uid } } @@ -1410,9 +1355,7 @@ dgraph.type author : Post.author @filter(eq(Author.name, "Random")) { name : Author.name - dgraph.uid : uid } - dgraph.uid : uid } } @@ -1439,9 +1382,7 @@ dgraph.type author : Post.author @filter(eq(Author.name, "Random")) { name : Author.name - dgraph.uid : uid } - dgraph.uid : uid } } @@ -1489,27 +1430,21 @@ dgraph.type author : Post.author @filter(eq(Author.name, "Random")) { name : Author.name - dgraph.uid : uid } - dgraph.uid : uid } FbPost1 as var(func: type(FbPost)) FbPostAuth4 as var(func: uid(FbPost1)) @cascade { dgraph.type author : Post.author @filter(eq(Author.name, "Random")) { name : Author.name - dgraph.uid : uid } - dgraph.uid : uid } Answer1 as var(func: type(Answer)) AnswerAuth5 as var(func: uid(Answer1)) @cascade { dgraph.type author : Post.author @filter(eq(Author.name, "Random")) { name : Author.name - dgraph.uid : uid } - dgraph.uid : uid } } @@ -1541,27 +1476,21 @@ dgraph.type author : Post.author @filter(eq(Author.name, "Random")) { name : Author.name - dgraph.uid : uid } - dgraph.uid : uid } FbPost1 as var(func: type(FbPost)) FbPostAuth4 as var(func: uid(FbPost1)) @cascade { dgraph.type author : Post.author @filter(eq(Author.name, "Random")) { name : Author.name - dgraph.uid : uid } - dgraph.uid : uid } Answer1 as var(func: type(Answer)) AnswerAuth5 as var(func: uid(Answer1)) @cascade { dgraph.type author : Post.author @filter(eq(Author.name, "Random")) { name : Author.name - dgraph.uid : uid } - dgraph.uid : uid } } @@ -1607,18 +1536,14 @@ dgraph.type author : Post.author @filter(eq(Author.name, "Random")) { name : Author.name - dgraph.uid : uid } - dgraph.uid : uid } Answer1 as var(func: type(Answer)) AnswerAuth4 as var(func: uid(Answer1)) @cascade { dgraph.type author : Post.author @filter(eq(Author.name, "Random")) { name : Author.name - dgraph.uid : uid } - dgraph.uid : uid } } @@ -1649,18 +1574,14 @@ dgraph.type author : Post.author @filter(eq(Author.name, "Random")) { name : Author.name - dgraph.uid : uid } - dgraph.uid : uid } Answer1 as var(func: type(Answer)) AnswerAuth4 as var(func: uid(Answer1)) @cascade { dgraph.type author : Post.author @filter(eq(Author.name, "Random")) { name : Author.name - dgraph.uid : uid } - dgraph.uid : uid } } @@ -1813,9 +1734,7 @@ ProjectAuth5 as var(func: uid(Project4)) @cascade { roles : Project.roles @filter(eq(Role.permission, "EDIT")) { assignedTo : Role.assignedTo @filter(eq(User.username, "user1")) - dgraph.uid : uid } - dgraph.uid : uid } var(func: uid(ProjectRoot)) { Column1 as Project.columns @@ -1825,11 +1744,8 @@ inProject : Column.inProject { roles : Project.roles @filter(eq(Role.permission, "VIEW")) { assignedTo : Role.assignedTo @filter(eq(User.username, "user1")) - dgraph.uid : uid } - dgraph.uid : uid } - dgraph.uid : uid } checkPwd(func: uid(ProjectRoot)) @filter(type(Project)) { pwd as checkpwd(Project.pwd, "something") @@ -1911,9 +1827,7 @@ FbPostAuth3 as var(func: uid(FbPost1)) @cascade { author : Post.author @filter(eq(Author.name, "ADMIN")) { name : Author.name - dgraph.uid : uid } - dgraph.uid : uid } Answer1 as var(func: type(Answer)) checkPwd(func: uid(PostRoot)) @filter(type(Post)) { diff --git a/graphql/resolve/auth_test.go b/graphql/resolve/auth_test.go index f413b8097d1..af888f172f7 100644 --- a/graphql/resolve/auth_test.go +++ b/graphql/resolve/auth_test.go @@ -447,13 +447,9 @@ func mutationQueryRewriting(t *testing.T, sch string, authMeta *testutil.AuthMet inProject : Column.inProject { roles : Project.roles @filter(eq(Role.permission, "VIEW")) { assignedTo : Role.assignedTo @filter(eq(User.username, "user1")) - dgraph.uid : uid } - dgraph.uid : uid } - dgraph.uid : uid } - dgraph.uid : uid } var(func: uid(TicketRoot)) { Column1 as Ticket.onColumn @@ -463,11 +459,8 @@ func mutationQueryRewriting(t *testing.T, sch string, authMeta *testutil.AuthMet inProject : Column.inProject { roles : Project.roles @filter(eq(Role.permission, "VIEW")) { assignedTo : Role.assignedTo @filter(eq(User.username, "user1")) - dgraph.uid : uid } - dgraph.uid : uid } - dgraph.uid : uid } }`, }, @@ -503,13 +496,9 @@ func mutationQueryRewriting(t *testing.T, sch string, authMeta *testutil.AuthMet inProject : Column.inProject { roles : Project.roles @filter(eq(Role.permission, "VIEW")) { assignedTo : Role.assignedTo @filter(eq(User.username, "user1")) - dgraph.uid : uid } - dgraph.uid : uid } - dgraph.uid : uid } - dgraph.uid : uid } var(func: uid(TicketRoot)) { Column1 as Ticket.onColumn @@ -519,11 +508,8 @@ func mutationQueryRewriting(t *testing.T, sch string, authMeta *testutil.AuthMet inProject : Column.inProject { roles : Project.roles @filter(eq(Role.permission, "VIEW")) { assignedTo : Role.assignedTo @filter(eq(User.username, "user1")) - dgraph.uid : uid } - dgraph.uid : uid } - dgraph.uid : uid } }`, }, diff --git a/graphql/resolve/auth_update_test.yaml b/graphql/resolve/auth_update_test.yaml index a39a87d2ee7..ef44dc320ae 100644 --- a/graphql/resolve/auth_update_test.yaml +++ b/graphql/resolve/auth_update_test.yaml @@ -40,10 +40,10 @@ USER: "user1" variables: | { "upd": - { + { "filter": { "colID": [ "0x123" ] }, - "set": { - "name": "new name", + "set": { + "name": "new name", "tickets": [ { "title": "a ticket" } ] } } @@ -59,11 +59,8 @@ inProject : Column.inProject { roles : Project.roles @filter(eq(Role.permission, "ADMIN")) { assignedTo : Role.assignedTo @filter(eq(User.username, "user1")) - dgraph.uid : uid } - dgraph.uid : uid } - dgraph.uid : uid } } uids: | @@ -81,19 +78,15 @@ inProject : Column.inProject { roles : Project.roles @filter(eq(Role.permission, "EDIT")) { assignedTo : Role.assignedTo @filter(eq(User.username, "user1")) - dgraph.uid : uid } - dgraph.uid : uid } - dgraph.uid : uid } - dgraph.uid : uid } } authjson: | - { - "Ticket": [ { "uid": "0x789" } ] - } + { + "Ticket": [ { "uid": "0x789" } ] + } - name: "Update a node that does a deep add and fails auth" @@ -109,10 +102,10 @@ USER: "user1" variables: | { "upd": - { + { "filter": { "colID": [ "0x123" ] }, - "set": { - "name": "new name", + "set": { + "name": "new name", "tickets": [ { "title": "a ticket" } ] } } @@ -128,11 +121,8 @@ inProject : Column.inProject { roles : Project.roles @filter(eq(Role.permission, "ADMIN")) { assignedTo : Role.assignedTo @filter(eq(User.username, "user1")) - dgraph.uid : uid } - dgraph.uid : uid } - dgraph.uid : uid } } uids: | @@ -150,22 +140,18 @@ inProject : Column.inProject { roles : Project.roles @filter(eq(Role.permission, "EDIT")) { assignedTo : Role.assignedTo @filter(eq(User.username, "user1")) - dgraph.uid : uid } - dgraph.uid : uid } - dgraph.uid : uid } - dgraph.uid : uid } } authjson: | - { } + { } error: { "message": "mutation failed because authorization failed" } # See comments about additional deletes in update_mutation_test.yaml. -# Because of those additional deletes, for example, when we update a column and +# Because of those additional deletes, for example, when we update a column and # link it to an existing ticket, we might remove that ticket from the column it was # attached to ... so we need authorization to update that column as well. - name: "update with auth on additional delete (updt list edge)" @@ -181,10 +167,10 @@ USER: "user1" variables: | { "upd": - { + { "filter": { "colID": [ "0x123" ] }, - "set": { - "name": "new name", + "set": { + "name": "new name", "tickets": [ { "id": "0x789" } ] } } @@ -200,11 +186,8 @@ inProject : Column.inProject { roles : Project.roles @filter(eq(Role.permission, "ADMIN")) { assignedTo : Role.assignedTo @filter(eq(User.username, "user1")) - dgraph.uid : uid } - dgraph.uid : uid } - dgraph.uid : uid } Ticket4 as Ticket4(func: uid(0x789)) @filter(type(Ticket)) { uid @@ -222,21 +205,18 @@ inProject : Column.inProject { roles : Project.roles @filter(eq(Role.permission, "ADMIN")) { assignedTo : Role.assignedTo @filter(eq(User.username, "user1")) - dgraph.uid : uid } - dgraph.uid : uid } - dgraph.uid : uid } } json: | - { + { "Column1": [ { "uid": "0x123" } ], "Ticket4": [ { "uid": "0x789" } ], "Column5": [ { "uid": "0x456" } ], "Column5.auth": [ { "uid": "0x456" } ] } - + - name: "update with auth on additional delete that fails (updt list edge)" gqlquery: | mutation updateColumn($upd: UpdateColumnInput!) { @@ -250,10 +230,10 @@ USER: "user1" variables: | { "upd": - { + { "filter": { "colID": [ "0x123" ] }, - "set": { - "name": "new name", + "set": { + "name": "new name", "tickets": [ { "id": "0x789" } ] } } @@ -269,11 +249,8 @@ inProject : Column.inProject { roles : Project.roles @filter(eq(Role.permission, "ADMIN")) { assignedTo : Role.assignedTo @filter(eq(User.username, "user1")) - dgraph.uid : uid } - dgraph.uid : uid } - dgraph.uid : uid } Ticket4 as Ticket4(func: uid(0x789)) @filter(type(Ticket)) { uid @@ -291,15 +268,12 @@ inProject : Column.inProject { roles : Project.roles @filter(eq(Role.permission, "ADMIN")) { assignedTo : Role.assignedTo @filter(eq(User.username, "user1")) - dgraph.uid : uid } - dgraph.uid : uid } - dgraph.uid : uid } } json: | - { + { "Column1": [ { "uid": "0x123" } ], "Ticket4": [ { "uid": "0x789" } ], "Column5": [ { "uid": "0x456" } ] @@ -320,10 +294,10 @@ USER: "user1" variables: | { "upd": - { + { "filter": { "id": [ "0x123" ] }, - "set": { - "title": "new title", + "set": { + "title": "new title", "onColumn": { "colID": "0x456" } } } @@ -340,13 +314,9 @@ inProject : Column.inProject { roles : Project.roles @filter(eq(Role.permission, "EDIT")) { assignedTo : Role.assignedTo @filter(eq(User.username, "user1")) - dgraph.uid : uid } - dgraph.uid : uid } - dgraph.uid : uid } - dgraph.uid : uid } Column4 as Column4(func: uid(0x456)) @filter(type(Column)) { uid @@ -364,20 +334,17 @@ inProject : Column.inProject { roles : Project.roles @filter(eq(Role.permission, "ADMIN")) { assignedTo : Role.assignedTo @filter(eq(User.username, "user1")) - dgraph.uid : uid } - dgraph.uid : uid } - dgraph.uid : uid } } json: | - { + { "Column4": [ { "uid": "0x456" } ], "Column5": [ { "uid": "0x499" } ], "Column5.auth": [ { "uid": "0x499" } ] } - + - name: "update with auth on additional delete that fails (updt single edge)" gqlquery: | mutation updateTicket($upd: UpdateTicketInput!) { @@ -391,10 +358,10 @@ USER: "user1" variables: | { "upd": - { + { "filter": { "id": [ "0x123" ] }, - "set": { - "title": "new title", + "set": { + "title": "new title", "onColumn": { "colID": "0x456" } } } @@ -411,13 +378,9 @@ inProject : Column.inProject { roles : Project.roles @filter(eq(Role.permission, "EDIT")) { assignedTo : Role.assignedTo @filter(eq(User.username, "user1")) - dgraph.uid : uid } - dgraph.uid : uid } - dgraph.uid : uid } - dgraph.uid : uid } Column4 as Column4(func: uid(0x456)) @filter(type(Column)) { uid @@ -435,15 +398,12 @@ inProject : Column.inProject { roles : Project.roles @filter(eq(Role.permission, "ADMIN")) { assignedTo : Role.assignedTo @filter(eq(User.username, "user1")) - dgraph.uid : uid } - dgraph.uid : uid } - dgraph.uid : uid } } json: | - { + { "Column4": [ { "uid": "0x456" } ], "Column5": [ { "uid": "0x499" } ] } @@ -539,9 +499,7 @@ ProjectAuth2 as var(func: uid(Project1)) @cascade { roles : Project.roles @filter(eq(Role.permission, "ADMIN")) { assignedTo : Role.assignedTo @filter(eq(User.username, "user1")) - dgraph.uid : uid } - dgraph.uid : uid } } @@ -605,7 +563,6 @@ Issue1 as var(func: uid(0x123)) @filter(type(Issue)) IssueAuth2 as var(func: uid(Issue1)) @cascade { owner : Issue.owner @filter(eq(User.username, "user1")) - dgraph.uid : uid } } @@ -700,7 +657,7 @@ } } } - jwtvar: + jwtvar: USER: "user1" ANS: "true" variables: | @@ -727,9 +684,7 @@ dgraph.type author : Post.author @filter(eq(Author.name, "user1")) { name : Author.name - dgraph.uid : uid } - dgraph.uid : uid } } @@ -766,7 +721,7 @@ } } } - jwtvar: + jwtvar: ROLE: "ADMIN" USER: "user1" variables: | @@ -790,9 +745,7 @@ dgraph.type author : Post.author @filter(eq(Author.name, "user1")) { name : Author.name - dgraph.uid : uid } - dgraph.uid : uid } } @@ -805,7 +758,7 @@ } } } - jwtvar: + jwtvar: ROLE: "USER" USER: "user1" variables: | @@ -861,27 +814,21 @@ dgraph.type author : Post.author @filter(eq(Author.name, "user1")) { name : Author.name - dgraph.uid : uid } - dgraph.uid : uid } FbPost1 as var(func: type(FbPost)) FbPostAuth4 as var(func: uid(FbPost1)) @cascade { dgraph.type author : Post.author @filter(eq(Author.name, "user1")) { name : Author.name - dgraph.uid : uid } - dgraph.uid : uid } Answer1 as var(func: type(Answer)) AnswerAuth5 as var(func: uid(Answer1)) @cascade { dgraph.type author : Post.author @filter(eq(Author.name, "user1")) { name : Author.name - dgraph.uid : uid } - dgraph.uid : uid } } @@ -919,18 +866,14 @@ dgraph.type author : Post.author @filter(eq(Author.name, "user1")) { name : Author.name - dgraph.uid : uid } - dgraph.uid : uid } Answer1 as var(func: type(Answer)) AnswerAuth3 as var(func: uid(Answer1)) @cascade { dgraph.type author : Post.author @filter(eq(Author.name, "user1")) { name : Author.name - dgraph.uid : uid } - dgraph.uid : uid } } diff --git a/graphql/resolve/query_rewriter.go b/graphql/resolve/query_rewriter.go index 129d0fbcd63..d4c271171c1 100644 --- a/graphql/resolve/query_rewriter.go +++ b/graphql/resolve/query_rewriter.go @@ -559,7 +559,11 @@ func rewriteAsQuery(field schema.Field, authRw *authRewriter) []*gql.GraphQuery addArgumentsToField(dgQuery[0], field) selectionAuth := addSelectionSetFrom(dgQuery[0], field, authRw) - addUID(dgQuery[0]) + // we don't need to query uid for auth queries, as they always have at least one field in their + // selection set. + if !authRw.writingAuth() { + addUID(dgQuery[0]) + } addCascadeDirective(dgQuery[0], field) dgQuery = authRw.addAuthQueries(field.Type(), dgQuery, rbac) From 40652c40750540f7ac49dfaee3f971b09dcfcf20 Mon Sep 17 00:00:00 2001 From: Abhimanyu Singh Gaur <12651351+abhimanyusinghgaur@users.noreply.github.com> Date: Thu, 10 Dec 2020 17:45:31 +0530 Subject: [PATCH 2/5] filter auth queries as early as possible --- graphql/resolve/auth_query_test.yaml | 32 ++++++++++++------------ graphql/resolve/query_rewriter.go | 37 ++++++++-------------------- 2 files changed, 26 insertions(+), 43 deletions(-) diff --git a/graphql/resolve/auth_query_test.yaml b/graphql/resolve/auth_query_test.yaml index 502eecd8f4c..015c78fa290 100644 --- a/graphql/resolve/auth_query_test.yaml +++ b/graphql/resolve/auth_query_test.yaml @@ -795,9 +795,9 @@ UserRoot as var(func: uid(User4)) User4 as var(func: type(User)) var(func: uid(UserRoot)) { - Ticket1 as User.tickets + Ticket1 as User.tickets @filter(anyofterms(Ticket.title, "graphql")) } - Ticket3 as var(func: uid(Ticket1)) @filter((anyofterms(Ticket.title, "graphql") AND uid(TicketAuth2))) + Ticket3 as var(func: uid(Ticket1)) @filter(uid(TicketAuth2)) TicketAuth2 as var(func: uid(Ticket1)) @cascade { onColumn : Ticket.onColumn { inProject : Column.inProject { @@ -873,9 +873,9 @@ regionsAvailable : Movie.regionsAvailable @filter(eq(Region.global, true)) } var(func: uid(MovieRoot)) { - Region1 as Movie.regionsAvailable + Region1 as Movie.regionsAvailable @filter(eq(Region.name, "Region123")) } - Region2 as var(func: uid(Region1)) @filter(eq(Region.name, "Region123")) + Region2 as var(func: uid(Region1)) } - name: "Auth deep query - 3 level" @@ -934,17 +934,17 @@ regionsAvailable : Movie.regionsAvailable @filter(eq(Region.global, true)) } var(func: uid(MovieRoot)) { - Region1 as Movie.regionsAvailable + Region1 as Movie.regionsAvailable @filter(eq(Region.name, "Region123")) } - Region7 as var(func: uid(Region1)) @filter(eq(Region.name, "Region123")) + Region7 as var(func: uid(Region1)) var(func: uid(Region1)) { - User2 as Region.users + User2 as Region.users @filter(eq(User.username, "User321")) } - User6 as var(func: uid(User2)) @filter(eq(User.username, "User321")) + User6 as var(func: uid(User2)) var(func: uid(User2)) { - UserSecret3 as User.secrets + UserSecret3 as User.secrets @filter(allofterms(UserSecret.aSecret, "Secret132")) } - UserSecret5 as var(func: uid(UserSecret3)) @filter((allofterms(UserSecret.aSecret, "Secret132") AND uid(UserSecretAuth4))) + UserSecret5 as var(func: uid(UserSecret3)) @filter(uid(UserSecretAuth4)) UserSecretAuth4 as var(func: uid(UserSecret3)) @filter(eq(UserSecret.ownedBy, "user1")) @cascade } @@ -1154,9 +1154,9 @@ UserRoot as var(func: uid(User4)) User4 as var(func: type(User)) var(func: uid(UserRoot)) { - TicketAggregateResult1 as User.tickets + TicketAggregateResult1 as User.tickets @filter(anyofterms(Ticket.title, "graphql")) } - Ticket3 as var(func: uid(TicketAggregateResult1)) @filter((anyofterms(Ticket.title, "graphql") AND uid(TicketAuth2))) + Ticket3 as var(func: uid(TicketAggregateResult1)) @filter(uid(TicketAuth2)) TicketAuth2 as var(func: uid(TicketAggregateResult1)) @cascade { onColumn : Ticket.onColumn { inProject : Column.inProject { @@ -1210,9 +1210,9 @@ UserRoot as var(func: uid(User10)) User10 as var(func: type(User)) var(func: uid(UserRoot)) { - TicketAggregateResult1 as User.tickets + TicketAggregateResult1 as User.tickets @filter(anyofterms(Ticket.title, "graphql")) } - Ticket3 as var(func: uid(TicketAggregateResult1)) @filter((anyofterms(Ticket.title, "graphql") AND uid(TicketAuth2))) + Ticket3 as var(func: uid(TicketAggregateResult1)) @filter(uid(TicketAuth2)) TicketAuth2 as var(func: uid(TicketAggregateResult1)) @cascade { onColumn : Ticket.onColumn { inProject : Column.inProject { @@ -1230,9 +1230,9 @@ owner : Issue.owner @filter(eq(User.username, "user1")) } var(func: uid(UserRoot)) { - Ticket7 as User.tickets + Ticket7 as User.tickets @filter(anyofterms(Ticket.title, "graphql2")) } - Ticket9 as var(func: uid(Ticket7)) @filter((anyofterms(Ticket.title, "graphql2") AND uid(TicketAuth8))) + Ticket9 as var(func: uid(Ticket7)) @filter(uid(TicketAuth8)) TicketAuth8 as var(func: uid(Ticket7)) @cascade { onColumn : Ticket.onColumn { inProject : Column.inProject { diff --git a/graphql/resolve/query_rewriter.go b/graphql/resolve/query_rewriter.go index d4c271171c1..18dd2fc6dab 100644 --- a/graphql/resolve/query_rewriter.go +++ b/graphql/resolve/query_rewriter.go @@ -977,7 +977,6 @@ func buildCommonAuthQueries( }, } - addFilterToField(selectionQry, f) return commonAuthQueryVars{ parentQry: parentQry, selectionQry: selectionQry, @@ -1114,27 +1113,19 @@ func buildAggregateFields( // possibly mainField. Auth filters are added to count aggregation fields and // mainField. Adding filters only for mainField is sufficient for other aggregate // functions as the aggregation functions use var from mainField. - for _, aggregateChild := range aggregateChildren { - if authFilter != nil { - if aggregateChild.Filter == nil { - aggregateChild.Filter = authFilter - } else { - aggregateChild.Filter = &gql.FilterTree{ - Op: "and", - Child: []*gql.FilterTree{aggregateChild.Filter, authFilter}, - } - } - } - } + // Adds auth queries. The variable authQueriesAppended ensures that auth queries are // appended only once. This also merges auth filters and any other filters of count // aggregation fields / mainField. if len(f.SelectionSet()) > 0 && !auth.isWritingAuth && auth.hasAuthRules { commonAuthQueryVars := buildCommonAuthQueries(f, auth, parentQryName) + // add child filter to parent query, auth filters to selection query and + // selection query as a filter to child + commonAuthQueryVars.selectionQry.Filter = authFilter var authQueriesAppended = false for _, aggregateChild := range aggregateChildren { - commonAuthQueryVars.selectionQry.Filter = aggregateChild.Filter if !authQueriesAppended { + commonAuthQueryVars.parentQry.Children[0].Filter = aggregateChild.Filter retAuthQueries = append(retAuthQueries, commonAuthQueryVars.parentQry, commonAuthQueryVars.selectionQry) authQueriesAppended = true } @@ -1319,27 +1310,19 @@ func addSelectionSetFrom( fieldAuth, authFilter = auth.rewriteAuthQueries(f.Type()) } - if authFilter != nil { - if child.Filter == nil { - child.Filter = authFilter - } else { - child.Filter = &gql.FilterTree{ - Op: "and", - Child: []*gql.FilterTree{child.Filter, authFilter}, - } - } - } - if len(f.SelectionSet()) > 0 && !auth.isWritingAuth && auth.hasAuthRules { commonAuthQueryVars := buildCommonAuthQueries(f, auth, parentQryName) - commonAuthQueryVars.selectionQry.Filter = child.Filter - authQueries = append(authQueries, commonAuthQueryVars.parentQry, commonAuthQueryVars.selectionQry) + // add child filter to parent query, auth filters to selection query and + // selection query as a filter to child + commonAuthQueryVars.parentQry.Children[0].Filter = child.Filter + commonAuthQueryVars.selectionQry.Filter = authFilter child.Filter = &gql.FilterTree{ Func: &gql.Function{ Name: "uid", Args: []gql.Arg{{Value: commonAuthQueryVars.filterVarName}}, }, } + authQueries = append(authQueries, commonAuthQueryVars.parentQry, commonAuthQueryVars.selectionQry) } authQueries = append(authQueries, selectionAuth...) authQueries = append(authQueries, fieldAuth...) From 225169f5f4a4f06046f82d60fc26d95fb62394dc Mon Sep 17 00:00:00 2001 From: Abhimanyu Singh Gaur <12651351+abhimanyusinghgaur@users.noreply.github.com> Date: Thu, 10 Dec 2020 17:56:35 +0530 Subject: [PATCH 3/5] deep-source --- graphql/resolve/query_rewriter.go | 5 ----- 1 file changed, 5 deletions(-) diff --git a/graphql/resolve/query_rewriter.go b/graphql/resolve/query_rewriter.go index 18dd2fc6dab..02b2a637006 100644 --- a/graphql/resolve/query_rewriter.go +++ b/graphql/resolve/query_rewriter.go @@ -415,11 +415,6 @@ func addArgumentsToField(dgQuery *gql.GraphQuery, field schema.Field) { addPagination(dgQuery, field) } -func addFilterToField(dgQuery *gql.GraphQuery, field schema.Field) { - filter, _ := field.ArgValue("filter").(map[string]interface{}) - _ = addFilter(dgQuery, field.Type(), filter) -} - func addTopLevelTypeFilter(query *gql.GraphQuery, field schema.Field) { addTypeFilter(query, field.Type()) } From f51e581cba647cafbc424a4a20ece71ab480453a Mon Sep 17 00:00:00 2001 From: Abhimanyu Singh Gaur <12651351+abhimanyusinghgaur@users.noreply.github.com> Date: Thu, 10 Dec 2020 18:34:02 +0530 Subject: [PATCH 4/5] fix with some tests --- graphql/resolve/auth_query_test.yaml | 44 ++++++++++---------- graphql/resolve/query_rewriter.go | 60 +++++++++++++++------------- 2 files changed, 54 insertions(+), 50 deletions(-) diff --git a/graphql/resolve/auth_query_test.yaml b/graphql/resolve/auth_query_test.yaml index 015c78fa290..23163ac5eb7 100644 --- a/graphql/resolve/auth_query_test.yaml +++ b/graphql/resolve/auth_query_test.yaml @@ -23,10 +23,10 @@ queryContact(func: uid(ContactRoot)) { id : uid nickName : Contact.nickName - adminTasks : Contact.adminTasks @filter(uid(AdminTask5)) { + adminTasks : Contact.adminTasks @filter(uid(AdminTask1)) { id : uid name : AdminTask.name - occurrences : AdminTask.occurrences @filter(uid(TaskOccurrence4)) { + occurrences : AdminTask.occurrences @filter(uid(TaskOccurrence3)) { due : TaskOccurrence.due comp : TaskOccurrence.comp dgraph.uid : uid @@ -36,14 +36,14 @@ ContactRoot as var(func: uid(Contact6)) Contact6 as var(func: type(Contact)) var(func: uid(ContactRoot)) { - AdminTask1 as Contact.adminTasks + AdminTask2 as Contact.adminTasks } - AdminTask5 as var(func: uid(AdminTask1)) + AdminTask1 as var(func: uid(AdminTask2)) var(func: uid(AdminTask1)) { - TaskOccurrence2 as AdminTask.occurrences + TaskOccurrence4 as AdminTask.occurrences } - TaskOccurrence4 as var(func: uid(TaskOccurrence2)) @filter(uid(TaskOccurrenceAuth3)) - TaskOccurrenceAuth3 as var(func: uid(TaskOccurrence2)) @filter(eq(TaskOccurrence.role, "ADMINISTRATOR")) @cascade + TaskOccurrence3 as var(func: uid(TaskOccurrence4)) @filter(uid(TaskOccurrenceAuth5)) + TaskOccurrenceAuth5 as var(func: uid(TaskOccurrence4)) @filter(eq(TaskOccurrence.role, "ADMINISTRATOR")) @cascade } - name: "Deep RBAC rule - Level 0 false" @@ -97,8 +97,8 @@ id : uid nickName : Contact.nickName } - ContactRoot as var(func: uid(Contact5)) - Contact5 as var(func: type(Contact)) + ContactRoot as var(func: uid(Contact6)) + Contact6 as var(func: type(Contact)) } - name: "Deep RBAC rule with cascade - Level 1 false" @@ -126,10 +126,10 @@ queryContact(func: uid(ContactRoot)) @cascade { id : uid nickName : Contact.nickName - adminTasks : Contact.adminTasks @filter(uid(AdminTask6)) { + adminTasks : Contact.adminTasks @filter(uid(AdminTask1)) { id : uid name : AdminTask.name - occurrences : AdminTask.occurrences @filter(uid(TaskOccurrence4)) { + occurrences : AdminTask.occurrences @filter(uid(TaskOccurrence3)) { due : TaskOccurrence.due comp : TaskOccurrence.comp dgraph.uid : uid @@ -139,15 +139,15 @@ ContactRoot as var(func: uid(Contact7)) Contact7 as var(func: type(Contact)) var(func: uid(ContactRoot)) { - AdminTask1 as Contact.adminTasks + AdminTask2 as Contact.adminTasks } - AdminTask6 as var(func: uid(AdminTask1)) @filter(uid(AdminTask5)) + AdminTask1 as var(func: uid(AdminTask2)) @filter(uid(AdminTask6)) var(func: uid(AdminTask1)) { - TaskOccurrence2 as AdminTask.occurrences + TaskOccurrence4 as AdminTask.occurrences } - TaskOccurrence4 as var(func: uid(TaskOccurrence2)) @filter(uid(TaskOccurrenceAuth3)) - TaskOccurrenceAuth3 as var(func: uid(TaskOccurrence2)) @filter(eq(TaskOccurrence.role, "ADMINISTRATOR")) @cascade - AdminTask5 as var(func: uid()) + TaskOccurrence3 as var(func: uid(TaskOccurrence4)) @filter(uid(TaskOccurrenceAuth5)) + TaskOccurrenceAuth5 as var(func: uid(TaskOccurrence4)) @filter(eq(TaskOccurrence.role, "ADMINISTRATOR")) @cascade + AdminTask6 as var(func: uid()) } - name: "Deep RBAC rule - Level 2 false" @@ -175,17 +175,17 @@ queryContact(func: uid(ContactRoot)) { id : uid nickName : Contact.nickName - adminTasks : Contact.adminTasks @filter(uid(AdminTask3)) { + adminTasks : Contact.adminTasks @filter(uid(AdminTask1)) { id : uid name : AdminTask.name } } - ContactRoot as var(func: uid(Contact4)) - Contact4 as var(func: type(Contact)) + ContactRoot as var(func: uid(Contact5)) + Contact5 as var(func: type(Contact)) var(func: uid(ContactRoot)) { - AdminTask1 as Contact.adminTasks + AdminTask2 as Contact.adminTasks } - AdminTask3 as var(func: uid(AdminTask1)) + AdminTask1 as var(func: uid(AdminTask2)) } - name: "Deep RBAC rule - Level 1 type without auth." diff --git a/graphql/resolve/query_rewriter.go b/graphql/resolve/query_rewriter.go index 02b2a637006..86b72742b8e 100644 --- a/graphql/resolve/query_rewriter.go +++ b/graphql/resolve/query_rewriter.go @@ -63,8 +63,6 @@ type commonAuthQueryVars struct { // Stores queries which aggregate filters and auth rules. Eg. // // User6 as var(func: uid(User2), orderasc: ...) @filter((eq(User.username, "User1") AND (...Auth Filter)))) selectionQry *gql.GraphQuery - // Contains name of the generated filterVarName - filterVarName string } // NewQueryRewriter returns a new QueryRewriter. @@ -743,7 +741,7 @@ func (authRw *authRewriter) addAuthQueries( Args: []gql.Arg{{Value: authRw.parentVarName}}, } - // The final query that includes the user's filter and auth processsing is thus like + // The final query that includes the user's filter and auth processing is thus like // // queryTodo(func: uid(Todo1)) @filter(uid(Todo2) AND uid(Todo3)) { ... } // Todo1 as var(func: ... ) @filter(...) @@ -943,39 +941,37 @@ func buildTypeFunc(typ string) *gql.Function { func buildCommonAuthQueries( f schema.Field, auth *authRewriter, - parentQryName string) commonAuthQueryVars { + parentSelectionName string) commonAuthQueryVars { // This adds the following query. // var(func: uid(Ticket)) { // User as Ticket.assignedTo // } - // where `Ticket` is the nodes selected at parent level and `User` is the nodes we - // need on the current level. + // where `Ticket` is the nodes selected at parent level after applying auth and `User` is the + // nodes we need on the current level. parentQry := &gql.GraphQuery{ Func: &gql.Function{ Name: "uid", - Args: []gql.Arg{{Value: auth.parentVarName}}, + Args: []gql.Arg{{Value: parentSelectionName}}, }, Attr: "var", - Children: []*gql.GraphQuery{{Attr: f.ConstructedForDgraphPredicate(), Var: parentQryName}}, + Children: []*gql.GraphQuery{{Attr: f.ConstructedForDgraphPredicate(), Var: auth.varName}}, } // This query aggregates all filters and auth rules and is used by root query to filter // the final nodes for the current level. // User6 as var(func: uid(User2), orderasc: ...) @filter((eq(User.username, "User1") AND (...Auth Filter)))) - filterVarName := auth.varGen.Next(f.ConstructedFor(), "", "", auth.isWritingAuth) selectionQry := &gql.GraphQuery{ - Var: filterVarName, + Var: auth.parentVarName, Attr: "var", Func: &gql.Function{ Name: "uid", - Args: []gql.Arg{{Value: parentQryName}}, + Args: []gql.Arg{{Value: auth.varName}}, }, } return commonAuthQueryVars{ - parentQry: parentQry, - selectionQry: selectionQry, - filterVarName: filterVarName, + parentQry: parentQry, + selectionQry: selectionQry, } } @@ -1095,10 +1091,10 @@ func buildAggregateFields( var parentVarName, parentQryName string if len(f.SelectionSet()) > 0 && !auth.isWritingAuth && auth.hasAuthRules { parentVarName = auth.parentVarName - parentQryName = auth.varGen.Next(f.Type(), "", "", auth.isWritingAuth) + parentQryName = auth.varName + auth.parentVarName = auth.varGen.Next(f.Type(), "", "", auth.isWritingAuth) + auth.varName = auth.varGen.Next(f.Type(), "", "", auth.isWritingAuth) } - auth.parentVarName = parentVarName - auth.varName = parentQryName var fieldAuth, retAuthQueries []*gql.GraphQuery var authFilter *gql.FilterTree if rbac == schema.Uncertain { @@ -1113,7 +1109,7 @@ func buildAggregateFields( // appended only once. This also merges auth filters and any other filters of count // aggregation fields / mainField. if len(f.SelectionSet()) > 0 && !auth.isWritingAuth && auth.hasAuthRules { - commonAuthQueryVars := buildCommonAuthQueries(f, auth, parentQryName) + commonAuthQueryVars := buildCommonAuthQueries(f, auth, parentVarName) // add child filter to parent query, auth filters to selection query and // selection query as a filter to child commonAuthQueryVars.selectionQry.Filter = authFilter @@ -1127,10 +1123,13 @@ func buildAggregateFields( aggregateChild.Filter = &gql.FilterTree{ Func: &gql.Function{ Name: "uid", - Args: []gql.Arg{{Value: commonAuthQueryVars.filterVarName}}, + Args: []gql.Arg{{Value: commonAuthQueryVars.selectionQry.Var}}, }, } } + // Restore the auth state after processing is done. + auth.parentVarName = parentVarName + auth.varName = parentQryName } // otherAggregation Children are appended to aggregationChildren to return them. // This step is performed at the end to ensure that auth and other filters are @@ -1247,9 +1246,9 @@ func addSelectionSetFrom( var parentVarName, parentQryName string if len(f.SelectionSet()) > 0 && !auth.isWritingAuth && auth.hasAuthRules { parentVarName = auth.parentVarName - parentQryName = auth.varGen.Next(f.Type(), "", "", auth.isWritingAuth) - auth.parentVarName = parentQryName - auth.varName = parentQryName + parentQryName = auth.varName + auth.parentVarName = auth.varGen.Next(f.Type(), "", "", auth.isWritingAuth) + auth.varName = auth.varGen.Next(f.Type(), "", "", auth.isWritingAuth) } var selectionAuth []*gql.GraphQuery @@ -1257,13 +1256,16 @@ func addSelectionSetFrom( selectionAuth = addSelectionSetFrom(child, f, auth) } - if len(f.SelectionSet()) > 0 && !auth.isWritingAuth && auth.hasAuthRules { - // Restore the state after processing is done. - auth.parentVarName = parentVarName - auth.varName = parentQryName + restoreAuthState := func() { + if len(f.SelectionSet()) > 0 && !auth.isWritingAuth && auth.hasAuthRules { + // Restore the auth state after processing is done. + auth.parentVarName = parentVarName + auth.varName = parentQryName + } } if f.Type().IsInbuiltOrEnumType() && (fieldSeenCount[f.DgraphAlias()] > 0) { + restoreAuthState() continue } fieldSeenCount[f.DgraphAlias()]++ @@ -1297,6 +1299,7 @@ func addSelectionSetFrom( } else if rbac == schema.Negative { // If RBAC rules are evaluated to Negative, we don't write queries for deeper levels. // Hence we don't need to do any further processing for this field. + restoreAuthState() continue } @@ -1306,7 +1309,7 @@ func addSelectionSetFrom( } if len(f.SelectionSet()) > 0 && !auth.isWritingAuth && auth.hasAuthRules { - commonAuthQueryVars := buildCommonAuthQueries(f, auth, parentQryName) + commonAuthQueryVars := buildCommonAuthQueries(f, auth, parentVarName) // add child filter to parent query, auth filters to selection query and // selection query as a filter to child commonAuthQueryVars.parentQry.Children[0].Filter = child.Filter @@ -1314,13 +1317,14 @@ func addSelectionSetFrom( child.Filter = &gql.FilterTree{ Func: &gql.Function{ Name: "uid", - Args: []gql.Arg{{Value: commonAuthQueryVars.filterVarName}}, + Args: []gql.Arg{{Value: commonAuthQueryVars.selectionQry.Var}}, }, } authQueries = append(authQueries, commonAuthQueryVars.parentQry, commonAuthQueryVars.selectionQry) } authQueries = append(authQueries, selectionAuth...) authQueries = append(authQueries, fieldAuth...) + restoreAuthState() } // Sort the required fields before adding them to q.Children so that the query produced after From c6bd2ccba34a48cf9d537e9a6400e96e8654ebac Mon Sep 17 00:00:00 2001 From: Abhimanyu Singh Gaur <12651351+abhimanyusinghgaur@users.noreply.github.com> Date: Fri, 11 Dec 2020 12:51:05 +0530 Subject: [PATCH 5/5] update comments --- graphql/resolve/query_rewriter.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/graphql/resolve/query_rewriter.go b/graphql/resolve/query_rewriter.go index 42b5ef5f46e..03f019f9a68 100644 --- a/graphql/resolve/query_rewriter.go +++ b/graphql/resolve/query_rewriter.go @@ -950,10 +950,10 @@ func buildCommonAuthQueries( auth *authRewriter, parentSelectionName string) commonAuthQueryVars { // This adds the following query. - // var(func: uid(Ticket)) { - // User as Ticket.assignedTo + // var(func: uid(Ticket1)) { + // User4 as Ticket.assignedTo // } - // where `Ticket` is the nodes selected at parent level after applying auth and `User` is the + // where `Ticket1` is the nodes selected at parent level after applying auth and `User4` is the // nodes we need on the current level. parentQry := &gql.GraphQuery{ Func: &gql.Function{ @@ -966,7 +966,7 @@ func buildCommonAuthQueries( // This query aggregates all filters and auth rules and is used by root query to filter // the final nodes for the current level. - // User6 as var(func: uid(User2), orderasc: ...) @filter((eq(User.username, "User1") AND (...Auth Filter)))) + // User3 as var(func: uid(User4)) @filter((eq(User.username, "User1") AND (...Auth Filter)))) selectionQry := &gql.GraphQuery{ Var: auth.parentVarName, Attr: "var",