From 8549035d9b66fb2543c54bcd1219dab85ce83cd3 Mon Sep 17 00:00:00 2001 From: Philip Lykke Carlsen Date: Thu, 9 Sep 2021 08:40:59 +0200 Subject: [PATCH] RFC: (Table) Permissions in MySQL This PR introduces an RFC for permissions in MySQL. (solves #2097) [Rendered](https://github.com/hasura/graphql-engine-mono/blob/plc/rfc/mysql-permissions/rfcs/permissions-mysql.md) https://github.com/hasura/graphql-engine-mono/pull/2183 Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> GitOrigin-RevId: e63fb9ddfd3a8752e28536818193b04403635f0d --- rfcs/permissions-mysql.md | 222 ++++++++++++++++++ .../Inherited roles permissions.png | Bin 0 -> 7781 bytes 2 files changed, 222 insertions(+) create mode 100644 rfcs/permissions-mysql.md create mode 100644 rfcs/permissions-mysql/Inherited roles permissions.png diff --git a/rfcs/permissions-mysql.md b/rfcs/permissions-mysql.md new file mode 100644 index 00000000000..d9f494fc61f --- /dev/null +++ b/rfcs/permissions-mysql.md @@ -0,0 +1,222 @@ +# Table Permissions in MySQL + +## Metadata +``` +--- +authors: Philip Lykke Carlsen +discussion: + https://github.com/hasura/graphql-engine-mono/pull/2183 +state: published +--- +``` + +## Description + +We want to support the [role-based access control +feature](https://hasura.io/docs/latest/graphql/core/auth/authorization/index.html) +on MySQL in the same fasihion that it works on Postgres currently. + +The role-based access control feature, often referred to simply as "Permissions" +allows Hasura users to restrict what data is returned by queries and admitted by +mutations. Several flavors of permissions exist: + +**_Column Permissions_** censor the columns that cliens in a given role have access +to (either in Queries or Mutations), by means of an explicit list of columns +exposed. + +**_Row Permissions_** censor table rows returned or affected, on the basis of a +boolean-returning SQL expression, which is allowed to reference the columns of +the table as well as session variables. These are sometimes also known as +_Filter Permissions_ in the server code. Both _Queries_ and _Mutations_ may be +subjected to _Row Permissions_. + +The query semantics of _Column Permissions_ and _Row Permissions_ is that, +in the context of a role: + +> _For all intents and purposes, the dataset that a query ranges over includes + only those columns and rows that the **Column Permissions** and **Row + Permissions** of that role allow._ + +For example: +* A user is unable to delete rows that her role's delete-permissions do not + include. It is not an error to try; from the point of view of the delete + mutation the rows indicated by the query are just not there. +* A query for an aggregation (say, the average `age` in a `persons` table) will + only consider those rows that her role's select-permissions include. + +The **_Aggregation Permission_** (a single boolean) decides if the query root +fields that relate to aggregations should appear in the schema. + +**_Limit Permissions_** (an integer) apply only to queries and limit the +maximum number of rows any query may yield. Importantly, an active _Limit +Permission_ does not influence the row domain of an aggregation query, only the +maximum number of rows produced by the query, see [[1]](#footnote-1-def). + +Last, **_Inherited Roles_** may compose permissions, see [User +docs](https://hasura.io/docs/latest/graphql/core/auth/authorization/inherited-roles.html#select-permissions). +An _Inherited Role_ is an authorization role defined in terms of other +pre-existing roles. The permissions that an _Inherited Role_ grants are _not_ +just the point-wise union of each of the parent roles' permission syntax, but +rather the _union of the query datasets_ that each parent role permits, in the +sense described above. + +This introduces a complication: In isolation, a role's _Column Permissions_ and +_Row Permissions_ describe a "rectangular" dataset, with columns along one side +and rows along the other. For two or more roles however, when we union the +datasets they permit we do not necessarily end up with a rectangular dataset: + +![Inherited roles diagram](permissions-mysql/Inherited%20roles%20permissions.png) + +Our data universe however only permits "rectangular" data. In order to +accomodate the complexity resulting from _Inherited Roles_ we make columns that +are particular to a single parent role nullable. For example, in the diagram +above we would return `null` for (`Row 5`, `Column B`) and (`Row 2`, `Column +D`). + +## What does this concretely look like + +When this is implemented, it should be possible to set permissions on MySQL +tables in exactly the same fashion as is possible on Postgres tables, and +queries and mutations should respect those permissions. + +At the time of this writing, this means every tracked MySQL tables has a +_Permissions_ tab in the Console, which allows a user to set permissions on: + +* Rows and Columns + * For each of the CRUD actions + * Using all the predicates supported as boolean operators in `_where: {..}` + arguments in queries to MySQL tables. +* Limit and Aggregation + +The tests in +[`server/tests-py/test_graphql_queries.py#L575`](https://github.com/hasura/graphql-engine-mono/blob/dfba245a4dbe1a71b1e3cc7c92914fc0a919c2b0/server/tests-py/test_graphql_queries.py#L575) +pertaining to permissions should be generalised to multiple backends and made to +pass for MySQL. + +## How are we going to implement it + +The GraphQL-Engine applies permissions at three points of processing: + +1. When building the schema, where _Column Permissions_ may cause fields to be + censored from the schema. +2. When parsing an incoming GraphQL query into HGE IR, where _Column Permissions_ + again influence the grammar parsed, and _Row Permissions_ influence the IR + generated such that relevant permissions are included. +3. When SQL is generated from the IR, where the translation needs to take the IR + node fields containing permissions into account. + +Since parser/schema generation is a single unified abstraction in +GraphQL-Engine, all a backend needs to do to support permissions is a suitable +implementation of type class methods `MonadSchema.buildTableQueryFields`, +`buildTableInsertMutationFields` etc.. + +`buildTableQueryFields` et al. are given as inputs a representation of the +permissions for a table (in the context of some role), which for _Column +Permissions_ list the exposed columns and for _Row Permissions_ contain +backend-specific Boolean Expression IR fragments, which are supposed to end up +in parser outputs. + +There are already backend-generic implementations of these methods in +`Hasura.GraphQL.Schema.Build` which we may use unless a product requirement +surfaces that require us to deviate from the (de facto) standard table schema. + +The inputting and storing of permissions in metadata is handled completely +generically by the core infrastructure referencing only the backend-defined +notions of column names and boolean expressions and how to (de-)serialize them. +The only work that is required here is to expose API endpoints for the various +CRUD-actions on permissions. + +### Development plan for Queries + +1. Implement the `instance MonadSchema 'MySQL` using the backend-generic default + implementations. + +2. Enabling the API for manipulating permissions amounts to is adding + `tablePermissionsCommands @MySQL` to the `metadataV1CommandParsers` + implementation of the `BackendAPI MySQL` instance. + +3. For SQL generation of a _Query_, the case that translates an `AnnSelectG` + [[2]](#footnote-2-def). Any applicable _Row + Permissions_ and _Limit Permissions_ are found in + the field `_asnPerm` and need to translate into `WHERE` and `LIMIT` clauses + respectably. + +4. Also for SQL generation of a _Query_, the case that translates an + `AnnColumnField` [[3]](#footnote-3-def) + needs to observe the field `_acfCaseBoolExpression`, which decides + whether the column value should be nullified, as resulting from + inherited roles. + +### Notes for Mutations + +A GQL _Mutation_ however may result in either of `INSERT`, `UPDATE`, or `DELETE` +statements. Of these, `INSERT` has no obvious point in which to include a permissions +predicate over the rows inserted. + +As a consequence of this we need to translate an insert-mutation into a MySQL +transaction, where performing the mutation and checking permissions on the +affected rows is split over multiple statements. [[4]](#footnote-4-def) + +One suggested way to do this could be making a temporary table having the +permissions as `CHECK`-constraints, inserting the new rows into this table +(which fails if the permissions are not satisfied) and copying them over to the +table actually targeted by the mutation. + +## Future + +This document is a product of its time, brought into existence by the +contemporary need to elaborate on how permissons work because the development +work on MySQL needs to incorporate them. + +An insight resulting from discussing this subject is that it would be more +appropriate to treat permissions not as a distinct topic of a dedicated RFC +document, but rather as associated concepts of the RFCs of the objects they +apply to, i.e. variants of _Queries_ and _Mutations_. + +As it were, _permissions_ do not exist in a vacuum. In order to talk about them +we need to also talk about what they apply to. As such it makes for a more +elegant exposition to talk about permissons as associated aspects of the subject +they act on. + +It it therefore expected that this document be superseded by dedicated RFCs on +the subjects of _Queries_, _Mutations_. + +## Questions + +How does the feature of _Inherited Roles_ interact with the permissions-support +in a backend? + +> The permissions that result from Inherited Roles are completely resolved into +> base permissions before being handed over to schema building. So Inherited +> Roles have no interaction with backend code. + +Do _Limit Permissions_ only apply to root-fields or also to array relationships? + +> Yet unanswered. + +## Footnotes + +[1][^](#footnote-1-ref): For example, + +``` +query { + articles_aggregate { + count + nodes { .. } + } +} +``` + +If the select permission on some role `r` specifies a limit of `5` and there are +a total of `10` rows accessible to `r` (as per active _Row Permissions_), the +`count` in the above query should return `10` while `nodes` should only return +`5`. I.e, the _Limit Permission_ should only be applied when returning rows and +not when computing aggregate data. + +[2][^](#footnote-2-ref): See [`server/src-lib/Hasura/RQL/IR/Select.hs`]( https://github.com/hasura/graphql-engine-mono/blob/dfba245a4dbe1a71b1e3cc7c92914fc0a919c2b0/server/src-lib/Hasura/RQL/IR/Select.hs#L67). + +[3][^](#footnote-3-ref): See [`server/src-lib/Hasura/RQL/IR/Select.hs`]( https://github.com/hasura/graphql-engine-mono/blob/dfba245a4dbe1a71b1e3cc7c92914fc0a919c2b0/server/src-lib/Hasura/RQL/IR/Select.hs#L305). Haddocks contain descriptions of use. + +[4][^](#footnote-4-ref): In PostgreSQL we exploit that `INSERT` supports a +`RETURNING` clause that lets us extract information from the affected rows. +MySQL does not support this. diff --git a/rfcs/permissions-mysql/Inherited roles permissions.png b/rfcs/permissions-mysql/Inherited roles permissions.png new file mode 100644 index 0000000000000000000000000000000000000000..38086987dfbc62ce0257768aab225911169f632c GIT binary patch literal 7781 zcmb7J2{@Gd+a_fzTF^ox<=8`qP_|(t$-YiuW)iZ6VvuD}wm6{($ufqrn~`P8-g3kc zMwZDkRLIyymNAU&dq+B_^Zl3a`mW2&>z()a{?_NYpZmF=7*k_C9&SNy78VvB{Yw|j zSy*-W+3j5EvI4E z+VO^(Q)O-#prSlc8Tg+cdwrK;uyo} zV>Wy(Kk;GqlM)L)-ea=xZl_D^X)%~X*NxBf&0H_bRDhRr>wJdkTlK=oFtQ?9Q3Iy2 zq5l0%WsQv$c`lfd#n3O}A@7FbHm{b!A9AI!q&?&UE~h_<+!Q>}5@8r-*s}Y8YBuX# z;0oNbVAup7ZvV*q`7OHwtHMb>5x$7{GFL4TzU@0LN*McZz;Ddk2woShohAy}6i%V- zKEe!vH}n4PG2Fq?!67_HGB&56wvKn4y{g14zj(#O*oIYs%g=E)k8*;DVOrvjyBs0nVMJD*OXAx%yEsAiY&+`$-X`nXrwZ<`aSMny z%nivn**4!h#e23oP2#A;!ahX(r0P1}f~I4TH(8{1Dv7V%u_w89MT1A@qE+5(Ve&pD zu_Qjn9QBwnwUh_qLKgTVMM-f*BM%LYD@~NAGz%AQvFYlE_%yPm9rqEhZ8bW3k#Y_8 zPME&yYE*84O3$kd{jPH+z25u!yFR~v=bssY4U}!@w#H2pT=x^kp6!qAb-yIh#=-Eb zYB$eFBIpzB`pTU?VG@JYgWAWhyo3KnIxDaw?&$p$WxmC$p^KI3Hc`qk=@UunZg2@F zX7XTUINZ{r)~&XDn%QXO*N4L_1Y8ZWJlZ^4iv{sd>T?mEuk#Mxdw%0|?|z=@#qn}= z{Mer{HKke&?$Eg*2ra5Qv5DlDT`8F=oGu+xG#B7W%fQ-fgpUXbfb=}`1F`zjBk^Y? z31fdMTn#Wi-Gk@Zvmuq1aY6hY{-y(MiuNq={^l)+xO2jGml1(t##i{$Mg)>I#9k-I&jpSBQgBvtC z&*>M|27iq9pLHixo|>(FW$`Vxh)qW!YqnOCCb*pM=xv@>VuNWrb)M#)^^l?nVs969G>&^t?(LSGu|qXq+Us(q zi~TjDW?-aeJT`eHjOztD%Gp&&*4_eJrE4;SjKB=)&DZ129a+weBvtFFW41>|5Y)HK$-p-vl*n8iBWJ4$Px#Qr@^=~Z`2~Qp0a)U~I zK814jb=t zXtT<3jW4F`Z6*sP6CSih5muS$uD`oeTJrPj_xS)d7DvtI>o=%WPdS_$rAtrAOw z7)J%f_|d#Jv<_4QcS7pwJJXIE=qK$HcwNHhCD>Nb%NymQXs!Cm$+q1XH5~{C05QOy zmbkM7fPL({w?RgVhqV8;m(K_2Jd#%chbMfiAQw;}ND13k*mMq1*W4rjd%z;APs$7rWNSMTCi#QVf#wATsVsW1 zYEIBC(;r(GL0#r1BcDi?8Sbek40WHnYMnhC{04hmY{AHO*1k|3eh6=G8$V6(trcvi zkN8bBgTC)N2(0?YUUQO5xnVSocMo3_Ox=!}mAM@KwBv-8L3+QS%V0I&>K8@5o%#~T z^PkG%0~Tu1RI#|q;WNf#Hjfq#VRR_xBzJAcJjKnUk)9LWur6Px8kh(AH`(M!z+>BP z(xm#o^-NXbUUMfKNnA$h&E~+Xe2P%UN|SraZPIv=N)fz2yMXuyMH)L{KPiuk<(3+8 zloIu;TKZ!v8X#&Pvja`jBE7#FZlpXf18c18IfZ= zTnL#p>2iB==-m9+ovfKMu$qyY{>J14~k=E6-bChry$s?}ucEUWYlKW)#(p$nKT z5J_1?*fuB@2nQ#KdA2{ocT2>MIh+|i2!wmySiqup0hoSrKlAXIVV)V}52!My z#5A>O5o8ZTpoC+t{=0~-S+t}U<+Ji$7-&p8@@%O;SrRjK`r~z0gNHdMd4#%j=(M;f?VQaQ543A-n?eY z3eiKqZtvvRq5zD9T>!$d1i(Dg`u9m|QGhqjX=8+>@qQx#t}T^zk(qbJn^%QW9cY6M z$GaceKPewFCEU9F>2>z5h$v?pL2fcsxXZSY5x*G<+X^xE3R!Eoh?NVO z6M%tlVAgO3=&mLTeB6W_O=N{|0E<0TGZl0Z1sbd?YCeX?NrK#7T(RDEFWa?W+^e1D zmy4Car5BDeE=xQut-0F8W0D)c_UO2psp_elR~#^ETN#_~0c(0Aq#N}c?ucH!MWgZL ziKX;Ky48Y}$JR$w;B2JN)4E19F-~cfAllk!j;kT(ZqaAuDN{kk>EpQu6H6U~7Rg(y zoXo>!fxNs-@xu}YV9>n5-evud=2u->hO0hUqZ2)hOO4mKOHBKbIX(V8a#L1n`2!P{ zNpO+kWY!mR5-nv;SQUtFLVd58j5-zL9E=afY9kLJEd?Y zO`29Q{mEOzs;%{V3-TQyRCyAeo2&<3brki+)d!oM*q9W6oewn;jSfA3t$I$CIkS={ zfQig%jN5#R^{{}K*W39KxnZ(Sgh-qY>GvB*H$tS0Q(?BN=fr9M&-DtP{2{TeB< z1eF&slP(&;`_nM2C~mM~lia79%St}0!|oLI?yj#Ns~HTH=0(TojegewfX2;__4jHn zug)aho@!gD5loKY$-h8pSbjBJy&fx`x@F|0X|oFJCLl!Sq8&Kq?7E-^dK!fMA0@@pf>&|KeAtr*C1ykz_fKumw0VSP0)8@jgZiC9e znFl?-{-K*NlThx+gtC8i2m}{8cOF&tYL6csh9xyWty{mpAm2{) zu3zbKUg{HSB#l7p8VGdi)!RHHU@vg}@1d$v>S1NOa57LV{_{rH(Yb=~@$kL8#F09z4v2L8z>B0SGh zWDv_uW>DJcetgT@uW#j~4iU!{0Q|=mo5RlsrAi z*Fh|+t4_H)E;;VkO^FvlszE8SSX+V>bWlRyzzK6Qh@@bPx{N;NBQqJ{{9plPuwR_>wYH-muo!17Fyqaibv zvI}gRhkqM{#0kCDTC&?C#P-jQT00Qpc4%e&btrPoa*d9^S@OgCmm3N8iN!)S$>%yW zqdUBJRdu%L0^Ing8GKk7ltcnv5+19)XNe27?TbH;gk{?te~uTeE`t1FoDlRjdjVc) z`f0>W-f*UXY3onsn`N2Y(FLD;sDjp82syIKC2^Fyw`)^i6y=p9!B z9Ir+;Dz=pD4;NIBh1xr36F`h{i4D`fsONVs{koOAF^c>*#^X!7Pgz~@pnsgtEIS{G zmmRD;sQQbQ!yr&amo$akH;{SDXR-lRca+AZrUaym(O!coIl-G{-q|ff4+XqDh$7`< z$XFDde5L-i*{8N1zXQMa;sH?siF2Q}(DqWGS6)gHC{RI^$>_xdcUuQtkxodB@c5K6 z8t6^NUR>ND<@62f^}Kf#k(hD+c>g35$bR~Y9zWnKQ70YDdPnySLLc)gWrB-Rlq-~o z_+_Ksy$>#qwU&srnWRt8UWA#zRVRatHuWXNVyPB(BEeSvik24 zzU)xNzxqkNri!__hO(gHOE|}`lY6O8vD}Wx3IhX3B6p_cA9i96S5ZDX|MZg@x+{>J z+z^XMsj_Ns&30--1j66411K50V>pxdNyPt3*9%SZ0~co8PN>lOB}xG6_zT)R1Zo0% zOOHHJNEbRR7&#b_T0AO4O~6BEV&gO1w&3L}Pm&;@{6J&*u6ERSfYJWFu`O)OJ9Yu0 z#1fYX;26Ib9JSmhoozzlV^Uy;9n6b3mH?!he{Ua?rk?>?YN3e|%#hQ}^ipOQC51$@ zX=qtUc&tr=jj-jqMjws^6AmOIOd*g%;A7>mZ|-`5n#Desh;`*STMQFHW5_nhdoCv!-!oHx40uc3qB2?CZ3<{_7?DCe|mgWUR{4f!#`LM zo&&@O=JF$`7eWc7r9ZbAQJbz%jitf~esFamI2f6avsrGcb6RLH9@aq zHR)O#LlXk(>pAeD^(V|wbO^|3t*M_>Wz8dd{p3L>2IbWO*~pgdEjvIgFt#R`wVpj1 zp*yE#_HP#a&oCcA0N91YK#HMfcuir2Wv z(XHtEY&|dyDeC^lIrJG~eV!3}#1u?*p3=1VfZnQE=p=y`OR_@$V9u!mqGHH@CGXy_ zBh~fMX&FphMNfH(<@{_-xr;S$H93^WtI1m(yQ7ex=Ucdb5R!%h&QpqXARUG9d z6x>tmyD<;*&`ekUx;)u=q@9~QBG*`?H*N7EMC``u;K2HP!V+bp0$poP6{rELui1%u zCul4NZG0&mDO~&B9d{fq<@G%fE5Kmn-;hC`mqBup?~;heq8{k>lFIuVa*zd)@ZOKs zXkqduSj9^IjNl`gij>n}59~`JUyT5=A&3OE@j5EuU3zIXcB@2JP=>msGxu=^kl#6p&9GJ0hL@fmSpa!e|_`?d{ba~)q4|zxyIY(LLC`Jv1 z+D}(bHiM1Rn?rnjwo(^ff;TTt2xrVLPJ<@|RU@beWRSWjkMw_}7q~K|eCvGFp1Au) zj~Vd;y7g`omTMVqANM*|DI}}VipSdBU7f`2*;vB4F=_#rpzmTEOJC<6Bi7>@CP@%_ zMnh1*Nw2L^g!*QjM!q&qeetGf2x4oM27}OpGJ~##_%v;pbh7UzMMPLUG*y9Xp7_{A)UK z`zv8yz2836@2Ov6tG{}HUsXaf8l_MeL3}KbCz!0LossG7;rbfjn{Vuxe(p&#I(T9l ziUyCAG&F%%Zpp!LAF!C96+K4NH?@KFHwNuW$o(4YQ!{xn$s}SXiLaBt#(Q=4n}fda z+D+M;wTRcX3@li!Y3)-|>eAOnxhbXbTF)G2AZ3g6JZX1B%w=)#l;809)Jp7H?yNv< zj%fZsW9sS6=_6C;dVK<8mNz~p>0>=9twp#&MWgti_ZvHJcuov%2zdFQW9uwz+^F6tYm?yB#1OTu;jCJ_w*ARqAMFIqGqr8B?vSF1&=8~0!Hsn%OlU)B zuTt;`5xlkDI0?vs_5kQHVXkW%aygh52~Y)qP&;P&u>O!Sn2%3x+iIAm{d-Y>^|Sm) z2DZ~1mK{W-bAzfv_DuIilx2kj#Sx(++mv7SZ^~jf>#5bC*^fI$1$O(e-J0hE`@UF{(F^gN-9&I1>{QL_yB>H zPc=FDN`GFRqb(kw{pp#SRsW9mCl1aGsHzE557j)Mo@cH#k!xGGyzxJb@8l~2@lFO`LmS6zW{~@5~@rog<(oR^eYfEiQ=+t0x zOPAk@pbYXygZ$vg_{C|vMV1}WK40w@JsgyDsTH_*#%{4*I6D!fcC+F!P&a1L*EPO?g