From 65966e4cfd0da65343b5cbaaa42aceca4e6b39a2 Mon Sep 17 00:00:00 2001 From: Paul Fitzpatrick Date: Fri, 19 Apr 2024 13:59:32 -0400 Subject: [PATCH] (core) use visibility information when sharing referenced columns with forms Summary: This tightens down the set of referenced columns made available to forms for dropdowns. Previous access to columns was computed at the level of shared tables. Now it is calculated at the level of shared sections. That means that we can now refrain from following hidden references, and make the referred data unavailable to forms, since they should not need it. Test Plan: extended test Reviewers: jarek, dsagal Reviewed By: jarek, dsagal Subscribers: dsagal Differential Revision: https://phab.getgrist.com/D4234 --- app/common/ACLRulesReader.ts | 48 +++++++++++++++++------ app/common/DocActions.ts | 9 ++++- app/common/UserAPI.ts | 18 ++++++++- test/fixtures/docs/FilmsWithImages.grist | Bin 92160 -> 93184 bytes 4 files changed, 60 insertions(+), 15 deletions(-) diff --git a/app/common/ACLRulesReader.ts b/app/common/ACLRulesReader.ts index ed4154ce..741a719e 100644 --- a/app/common/ACLRulesReader.ts +++ b/app/common/ACLRulesReader.ts @@ -109,6 +109,12 @@ export interface ACLRulesReaderOptions { addShareRules?: boolean; } +interface ShareContext { + shareRef: number; + sections: MetaRowRecord<"_grist_Views_section">[]; + columns: MetaRowRecord<"_grist_Tables_column">[]; +} + /** * Helper class for reading ACL rules from DocData. */ @@ -204,6 +210,19 @@ export class ACLRulesReader { } ); + const sectionIds = new Set(sections.map(section => section.id)); + const fields = this.docData.getMetaTable('_grist_Views_section_field').getRecords().filter( + field => { + return sectionIds.has(field.parentId); + } + ); + const columnIds = new Set(fields.map(field => field.colRef)); + const columns = this.docData.getMetaTable('_grist_Tables_column').getRecords().filter( + column => { + return columnIds.has(column.id); + } + ); + const tableRefs = new Set(sections.map(section => section.tableRef)); const tables = this.docData.getMetaTable('_grist_Tables').getRecords().filter( table => tableRefs.has(table.id) @@ -211,13 +230,12 @@ export class ACLRulesReader { // For tables associated with forms, allow creation of records, // and reading of referenced columns. - // TODO: should probably be limiting to a set of columns associated - // with section - but for form widget that could potentially be very - // confusing since it may not be easy to see that certain columns - // haven't been made visible for it? For now, just working at table - // level. + // TODO: tighten access control on creation since it may be broader + // than users expect - hidden columns could be written. for (const table of tables) { - this._shareTableForForm(table, share.id); + this._shareTableForForm(table, { + shareRef: share.id, sections, columns, + }); } } @@ -248,10 +266,12 @@ export class ACLRulesReader { * Allow creating records in a table. */ private _shareTableForForm(table: MetaRowRecord<'_grist_Tables'>, - shareRef: number) { + shareContext: ShareContext) { + const { shareRef } = shareContext; const resource = this._findOrAddResource({ tableId: table.tableId, - colIds: '*', + colIds: '*', // At creation, allow all columns to be + // initialized. }); let aclFormula = `user.ShareRef == ${shareRef}`; let aclFormulaParsed = JSON.stringify([ @@ -277,19 +297,21 @@ export class ACLRulesReader { resource, aclFormula, aclFormulaParsed, permissionsText: '+R', })); - this._shareTableReferencesForForm(table, shareRef); + this._shareTableReferencesForForm(table, shareContext); } /** * Give read access to referenced columns. */ private _shareTableReferencesForForm(table: MetaRowRecord<'_grist_Tables'>, - shareRef: number) { + shareContext: ShareContext) { + const { shareRef } = shareContext; + const tables = this.docData.getMetaTable('_grist_Tables'); const columns = this.docData.getMetaTable('_grist_Tables_column'); - const tableColumns = columns.filterRecords({ - parentId: table.id, - }).filter(c => c.type.startsWith('Ref:') || c.type.startsWith('RefList:')); + const tableColumns = shareContext.columns.filter(c => + c.parentId === table.id && + (c.type.startsWith('Ref:') || c.type.startsWith('RefList:'))); for (const column of tableColumns) { const visibleColRef = column.visibleCol; // This could be blank in tests, not sure about real life. diff --git a/app/common/DocActions.ts b/app/common/DocActions.ts index b89246e7..8770f67d 100644 --- a/app/common/DocActions.ts +++ b/app/common/DocActions.ts @@ -133,8 +133,15 @@ export interface TableRecordValues { records: TableRecordValue[]; } -export interface TableRecordValue { +export interface TableRecordValuesWithoutIds { + records: TableRecordValueWithoutId[]; +} + +export interface TableRecordValue extends TableRecordValueWithoutId { id: number | string; +} + +export interface TableRecordValueWithoutId { fields: { [colId: string]: CellValue }; diff --git a/app/common/UserAPI.ts b/app/common/UserAPI.ts index 9b1cccdb..df4d0788 100644 --- a/app/common/UserAPI.ts +++ b/app/common/UserAPI.ts @@ -5,7 +5,8 @@ import {BaseAPI, IOptions} from 'app/common/BaseAPI'; import {BillingAPI, BillingAPIImpl} from 'app/common/BillingAPI'; import {BrowserSettings} from 'app/common/BrowserSettings'; import {ICustomWidget} from 'app/common/CustomWidget'; -import {BulkColValues, TableColValues, TableRecordValue, TableRecordValues, UserAction} from 'app/common/DocActions'; +import {BulkColValues, TableColValues, TableRecordValue, TableRecordValues, + TableRecordValuesWithoutIds, UserAction} from 'app/common/DocActions'; import {DocCreationInfo, OpenDocMode} from 'app/common/DocListAPI'; import {OrgUsageSummary} from 'app/common/DocUsage'; import {Product} from 'app/common/Features'; @@ -441,6 +442,10 @@ interface GetRowsParams { immediate?: boolean; } +interface SqlResult extends TableRecordValuesWithoutIds { + statement: string; +} + /** * Collect endpoints related to the content of a single document that we've been thinking * of as the (restful) "Doc API". A few endpoints that could be here are not, for historical @@ -452,6 +457,7 @@ export interface DocAPI { // opening a document are irrelevant. getRows(tableId: string, options?: GetRowsParams): Promise; getRecords(tableId: string, options?: GetRowsParams): Promise; + sql(sql: string, args?: any[]): Promise; updateRows(tableId: string, changes: TableColValues): Promise; addRows(tableId: string, additions: BulkColValues): Promise; removeRows(tableId: string, removals: number[]): Promise; @@ -925,6 +931,16 @@ export class DocAPIImpl extends BaseAPI implements DocAPI { return response.records; } + public async sql(sql: string, args?: any[]): Promise { + return this.requestJson(`${this._url}/sql`, { + body: JSON.stringify({ + sql, + ...(args ? { args } : {}), + }), + method: 'POST', + }); + } + public async updateRows(tableId: string, changes: TableColValues): Promise { return this.requestJson(`${this._url}/tables/${tableId}/data`, { body: JSON.stringify(changes), diff --git a/test/fixtures/docs/FilmsWithImages.grist b/test/fixtures/docs/FilmsWithImages.grist index 3881b6cf3a87d56acc2ae6382e43f5fa217bc9d8..2fdb578616d165f9ffa17eb5716c7aa7034347fc 100644 GIT binary patch delta 2829 zcmaJ@eNa`$72mVF&wF{i4;~1Y4Bm~!j-q_^o`rJ+#gcgI}nxQMv{?=#FdOBMM08aIE}L* zv00Er%Shx|NF;9b&)7YrWSwr|mjRyDubUJaz1px;FeK&rq;$g$>Eecx=3w`6wATqM zWvyR5{aB*#rzi>7!swq^@uzTy(7SMlM(Zy52ABt7mZW56!jM;Z z-Kh{II^1WZ`A&DBzT-OKkQII+z6H~sD+JCqm`-PWZB-Oi@UjZMyq%9Y-A zHBHXibu~3;@~OejfWI%~@9p_7KxSsP*pZh57U4~d*MFLq3ui1`mN~gX=m#`GSn_&A z9&ZF$yX}P>a2POc{t9>%Vlcpo+WRYDAJ`+>!E)HGeb4}>+BC6t5Td+Obu#=&eYF*^!n){c?Sdqh4t+ve zU64Sd;S=~X{0_$99GrmH;AQBCKHbHg&~Gg<<5)qq;upir)@NskG|EzpV$`%%wk~~a zv@@G^$qbQNEsGyx?G|g%W2247S_@~0jL$AWQNn)bp<7$oboOvq7tKIXX30l!+PjVF zN!7PZ&y3;WO(pOyq|)z@R5}KB1&LX4xnQVSl-4Ifl4!0!oA!D8v%WVNQUev+`lfcL z$X~k*6eDzEc)sY;{OVem)GVVxl1%5+in7<2?_qPRQS_R;XQS zNcFj$72c-Grm8iSPn-vRvXRP0)|y4{5%vx{%672^=A^oVw*hYUf5F#9al6+*c%>YP z&&Nu)C~bGy5Yd`vLKQZSbBN*gblW0S zWpPv`p(;)!x24z%T3eR1W-!z#5!hSp$+kj-<=YVez zQX?Tdh2k&JuYNf<#&VG`LN}3%WQYu50uR9k@$0b?^H6-Uq~zql z-VTwuD-F^j1Nh>u9YTBYqH#95)$U%gdiN@DPg}S+zWZfx8j`-I|7SAW#2%u*EYKIf zG)|#epZq4}akLh%u~!V2ry)ewlUL|*bh=l&R0{>7PD|9{>tMeaJy->qqFz(PrGs%o z9@}i>bGdsJkmk9wl%)0 zHPwezd^~#CW<;8?3Bo?YJ97eK{0k=D6Y17y;3NrxTRQ@(j~+Q>@mN^}j_?AZUx3%i z1r*Y-<&%X~h`FO>uu!y*7HQX>2M-%)b8DBXU>+>MM4O;RSHri#rJbmTbeL}hMfZ zdyZM?HTna(9dmI971zukfg}z`cYrJ=qKhn?Tq5KW>!jD<2*&lCd0<_dq-1BqpsK}J zf$OoC9WpLrpTDO^6_f7xtii<@lH&2e@WN2Xm%7yUEdhTp)LyQ3bp>bETZ}<+L#;Wg zp+pl^Fvk?=eWA)HUayRcF|uuMZ^-RjQCC+}>8%wvE8^&s&Db-c?R~0&)Am<^$eIHo z@`g&xC(f$cipq`7r|qA$%?LT`YCi{%^;zfH1Ig)mYvEati73lo?lMD)VeiB)c z&QBp*tn(isTcYzHBU`HT^T>*IeqmCA5;6DQxAgtvK?cw1{4F#r)A?m&%XR(^+RJqQ z9!g%F{{h(wo&V`-l<^AtyPLwIGj}JMsTAF=)NXQ!)0KbTVX#W)_m7#ODtwYcBbX}D zT#Zh4gm-h;%i#GimjOFo6Xr31UDk&AY=hT@d7{DV!+egx*M{*9L*5YP=>~5MSI{_~V(LP6;5hc@({c1LuPU|GrRy$5hX#T)7m}(w19cCmmHX|V&dhQZ+`$z7~ zcg}sB^E>C>d+zTWowJP2SjIAZag4DJ`tG$E>WF&-(m#Rr;sX8)Z{j@u75^?hV(+>61-^|;oH~hhYe}@YNwmh0aHo?F$Wq{feZ+7SYk3ZJ zl4*k4jFFoQA-UxG6Xc$o-^b+}8P&29M`N9|i1;?cJNQq0+e~m=#C9iISO(~IL&3nn z{$a}+Zec#qn*&2n3=Z{o4Ri;F(FPV~#OQUwzW!k+CNU0v27X@7+$!4tH%#J&QCbS$ zZ9Cr0@uE8k@D)z=kde1$K$#F?Ac zT?D>6{x=Sx>zo~A@CiF89VG*DJTD*YYGdYXULn|yCTZI0F_;ZG4lY+3EvH0pidoYK~gw(6%2HoK1-MU!N8yTEujjVfoyvi2piPenSS~%J+Gwajk&l@r! zF>-Wqr55vAgB|R-N9&!DFh3|Jll72rf4QgrF36Q57EI%5Qfn$UJ^K4#rtSK~68{?2 zOf`egp_*S})%^G4itO0_|BuKBk-tDT0GE)2M zRJp7sM4j|VY`rFx`qWx_1Mb?9z`!=E&SdcMT3elku<|2VdEME4QCO-Qg|!*U&`^do zDp6{+ee@h_vR4NQ(+#81n^7DUi7e?keXKVm$Dd4*FZYJvYmuwn5Ih7K^4Gyz#5ym3dkNM? zW=}ua;eiI`1cT!o7Ww2P0Uy0@epI5f^d$ccO zn+^S)oQ}er8!U|XyvvaB4)!iYQ!@E6s50{N;b^|RI+_JuDx?=|p=Z)HHThi8>ZG|i z`WVReP#JjSSm?A2^`@E4M}DVcJ1(0ZkppJgk1lighPm!~81C&F3hY;xWAnxI4R&|Qzx_Dd<>UjD+5}@0{0dC4H!v0BRbHy`E7CVr19|d^ zsnTPURgfjOOui)#ytos5@((YLSuvG$_V17vGQ9H8OB=y2&%LzEqOSFp{MOlG$R#5~ zPMNNqI5dn+20OuC%# zLv!rIVT4js+@h5-Q!J1wH^m|uRGQ*0jUG0|7o;|w7cdtC#o_rF@N&q1%>gTDsi4T! zlQ-Ou7&hWz0ih-=lHjxrwuWDI!5*+|A%kry(!uZ{H#~>1JuI?;(CfmYfDm9uSZq+b zJ}gR<-We7ZN;ibXW~Cd$qDJYau-LBjqhV1G%l3_rIEmO}iPnh}Pf-P}a{kgP=Y#eY zoSz6Ev&09S4_L#PO!vK?NNLjiJw69c^Wmjd(77GYF^(aIA)KWRu{g8*uJu#4J zICW>HuIu7H)$Q!>>Iu+ja<&r+SAU#vn_)3O4!4P_Zh)LE@k4y%Da4hYkh{xLt~+DbdM(eQ(Gbh*70Q;HyOUd{{lBDjdzI3&XzwYBYAwH7;+*-_2I;r zwcTYk9#o&*um1nQKVbX=I)Q847D|uNjH)U)vO~J*m(0uE;Gobhv$p(#7;hF~v5&3@ J_MUWz{{cwCHd6oq