From 1b62b4b6253668f3fb0396ef7e6f3ca5aeec1a0d Mon Sep 17 00:00:00 2001 From: forehalo Date: Wed, 12 Mar 2025 03:18:24 +0000 Subject: [PATCH] feat(server): support making doc private in workspace (#10744) --- .../__snapshots__/actions.spec.ts.md | 34 ++++++++++++++++ .../__snapshots__/actions.spec.ts.snap | Bin 1398 -> 1467 bytes .../core/permission/__tests__/actions.spec.ts | 3 +- .../src/core/permission/__tests__/doc.spec.ts | 38 ++++++++++++++++++ .../backend/server/src/core/permission/doc.ts | 13 ++++-- .../server/src/core/permission/types.ts | 27 +++---------- .../backend/server/src/models/common/role.ts | 4 ++ 7 files changed, 93 insertions(+), 26 deletions(-) diff --git a/packages/backend/server/src/core/permission/__tests__/__snapshots__/actions.spec.ts.md b/packages/backend/server/src/core/permission/__tests__/__snapshots__/actions.spec.ts.md index 6e5bfd57cc..88767c3c0e 100644 --- a/packages/backend/server/src/core/permission/__tests__/__snapshots__/actions.spec.ts.md +++ b/packages/backend/server/src/core/permission/__tests__/__snapshots__/actions.spec.ts.md @@ -6,6 +6,10 @@ Generated by [AVA](https://avajs.dev). ## should be able to fixup doc role from workspace role and doc role +> WorkspaceRole: External, DocRole: None + + 'None' + > WorkspaceRole: External, DocRole: External 'External' @@ -26,6 +30,10 @@ Generated by [AVA](https://avajs.dev). 'Editor' +> WorkspaceRole: Collaborator, DocRole: None + + 'None' + > WorkspaceRole: Collaborator, DocRole: External 'External' @@ -46,6 +54,10 @@ Generated by [AVA](https://avajs.dev). 'Owner' +> WorkspaceRole: Admin, DocRole: None + + 'Manager' + > WorkspaceRole: Admin, DocRole: External 'Manager' @@ -66,6 +78,10 @@ Generated by [AVA](https://avajs.dev). 'Owner' +> WorkspaceRole: Owner, DocRole: None + + 'Owner' + > WorkspaceRole: Owner, DocRole: External 'Owner' @@ -190,6 +206,24 @@ Generated by [AVA](https://avajs.dev). ## should be able to get correct permissions from DocRole +> DocRole: None + + { + 'Doc.Copy': false, + 'Doc.Delete': false, + 'Doc.Duplicate': false, + 'Doc.Properties.Read': false, + 'Doc.Properties.Update': false, + 'Doc.Publish': false, + 'Doc.Read': false, + 'Doc.Restore': false, + 'Doc.TransferOwner': false, + 'Doc.Trash': false, + 'Doc.Update': false, + 'Doc.Users.Manage': false, + 'Doc.Users.Read': false, + } + > DocRole: External { diff --git a/packages/backend/server/src/core/permission/__tests__/__snapshots__/actions.spec.ts.snap b/packages/backend/server/src/core/permission/__tests__/__snapshots__/actions.spec.ts.snap index 8d00ba8292e3fcc3cf2a53ca09cf316264a32867..e319b6c8d554b342c6f9a42894330a2889678da0 100644 GIT binary patch literal 1467 zcmV;s1w{HmRzVPTzXVXBNWLHT_Q6y@KpHfU?(9%>w2nfm# z5toA3o+PGT&tiL(O~rw#zyXO1CnO}KMj!z0H) zgzVXTvKTLex9+u>cb?VogAaIx$#ErF! zJ8h*O$5FjAq;=vNAyYTDzI+7WCjfT=oFKr91bB}CpAq0k0{lgQF%7t+0lo%oYQP^F za9;z?=)kfLe5eCIY<>Sz2PVjHg$&PC-=|hcNat(0Tm~5hnFJZf1komm4pOR+)1824 zqKb8bFjAXfCf?2nr#4{|_ZD~wn=?FP(W*--57&_i+HV8$~9dyAE7B)Od#xo%-`lc7G zP(Ql%#pshMdMTjAFOV3VSz0VZ>yiW9|3)bIg6NY_@K~8p@Hn|p@W_Es93Y|K(R~pL zMF{&S6ywQxWtvlp>`lQyEe2}Q549+EEoT>?xbXU<7W{~ZpcaY{_C+leRWDJCP;?u5 zxK$$e(m%Q?p(w50t3)pde3F(O)UE~?hj*P_Zk9vc+ZZi|XX9QvZ_x%*HQ zRbBkBbWl}?rmA?{L8vNE{OJ6m=v6e&Hd;nc&2;&9(4u=u(+VBtrMHqdZztB(ZzAjJ zX8`vAoFf1wz*hwLeXBT$23*ttrUBn+z+DZP)`3MGxTOQX=)gT4I6DMf8v?!<+8gVt z)PD1zu!B{qkP}&dzp$TG%DjtJ+}X;RPU3UrjFP%uIpf40m~0P~R+wTp)TWqhwp?$l zH83iK?OJ5AfjLf@VuiV$nQTgtb*7j{X`%Bg5Mjqz%b7{(v&qt}vfE^_$z3-- z!nQ!ZkRy=Yuk6aCQTtZZyFX8y{l5X6B*1F~Xb|9Q0^A|Me*`$K0k3L6bGN-)Q6zj9 zFLmC?%c$%s%006Z}yGZ~5 literal 1398 zcmV-+1&R7WRzVPTzXVXBNWLHTFC=&GnD5WJAq>3UAElnkaKtWAH z#HFp*o+PG@XR*CXQgNUvy&!Qx+>nqOfdt&Ra6?>+ID*6xl_N(Et+8X=9_QcH~ZFimg_!qu1B}GXzay4-MA2~vZn7^b!yplpIR}qmc5N;!*ZEp zh0Leca>&-KbrxQa8n#2z|Ji}t26$0mH=Q*VN$1lhU2=(eBZ9K(2iNSr}UpGM_6_XO41d#VX=emNS4-& z@8xoQ9eXj~L0Cz0o6(&vM_1SmWpw<`UU1jEK)MY|s^>LS#SQ3*sC4RI&=8f)`Ewec zLb4Sse5#XY(o`#m`26rh#YFc&Q%tjhhEFn2v)3f^JZX}%hVgoL!&srQ<*+cMPHZ)3 zxaLKX$AT#B939+wulOX5>ir?39oGn%y0QJ?V*oz_xCh`E0bV7*H3ED}fFB6(CjrI` z;Jg8Z2C!uSzZ<~625`~@mQ3Iy6Zn4n^B*QKL56E&c&`2dbz(x=AFI_W$SBAp$T%j5 zF-c62N{yUo2RxG$n-|5&36EydbS5~X%8i=LN4?>*dNlL47sVx^t)fYx;~aE~hc1O) zEQz3&V=uCX=QGLl69rsI?U>FoM~tlI$XV)B*~Cxfu(@!>4!j#wv81PS^t<-v8VzD) zUZ)H2kTqx+do;=d-^~cs0-h0$kS{?z!(zjg#x$0Xkkc0OI8Wo)3s#CjD1vUIONKDN z8Ax`#5W+%e2hlPOld~^IAJ5T?5iNf}QgC5usSK@4ZaV)Pq2LRmPeQ?CWkSK@h(P4R<-KFJ`87H~faXZr+ z<7`21#;yQx0o(%cHGsPS{sHhT0nTr0%~b;2B*1qB_=Ny}6QE{jHD#1r{KzbGTBmYT zoSP?Gk5!F$GyyalzUN3UjT0?QsrsGbkgBWb(FEJ9`(CsvMkH&KPj)nY9JMTFF+r_` zDAAOj$Z)bik}Rs$QcVEb+wWKo>-vKHp~SCze9Ee;T@1=R3t0}8zJ($iY6n9$o7Tu^4NMAQw|<6fpvKlvtWaxk z$fgwe9Ey3Aj)!KiHU3A52-`M@W+tU6BHM12HzJEoZjta2?g->78iDM5flwxm#y6tg z{aI$i{srJT0p21&lK@{4;4T3k5a5IXylDU%z4mTJk?>u-(tc%4-_0xSx8=%qb(IRr z`@1xzu0CJMU(-@Hd_^2xtzSC3 { for (const workspaceRole of workspaceRoles) { for (const docRole of docRoles) { + const fixedDocRole = fixupDocRole(workspaceRole, docRole); t.snapshot( - DocRole[fixupDocRole(workspaceRole, docRole)!], + fixedDocRole === null ? null : DocRole[fixedDocRole], `WorkspaceRole: ${WorkspaceRole[workspaceRole]}, DocRole: ${DocRole[docRole]}` ); } diff --git a/packages/backend/server/src/core/permission/__tests__/doc.spec.ts b/packages/backend/server/src/core/permission/__tests__/doc.spec.ts index f9e1eb9591..1359c6ef69 100644 --- a/packages/backend/server/src/core/permission/__tests__/doc.spec.ts +++ b/packages/backend/server/src/core/permission/__tests__/doc.spec.ts @@ -111,6 +111,44 @@ test('should return [External] if doc is public', async t => { t.is(role, DocRole.External); }); +test('should return null if doc role is [None]', async t => { + await models.doc.setDefaultRole(ws.id, 'doc1', DocRole.None); + + await models.workspaceUser.set( + ws.id, + user.id, + WorkspaceRole.Collaborator, + WorkspaceMemberStatus.Accepted + ); + + const role = await ac.getRole({ + workspaceId: ws.id, + docId: 'doc1', + userId: user.id, + }); + + t.is(role, null); +}); + +test('should return [External] if doc role is [None] but doc is public', async t => { + await models.doc.setDefaultRole(ws.id, 'doc1', DocRole.None); + await models.workspaceUser.set( + ws.id, + user.id, + WorkspaceRole.Collaborator, + WorkspaceMemberStatus.Accepted + ); + await models.doc.publish(ws.id, 'doc1'); + + const role = await ac.getRole({ + workspaceId: ws.id, + docId: 'doc1', + userId: 'random-user-id', + }); + + t.is(role, DocRole.External); +}); + test('should return mapped permissions', async t => { const { permissions } = await ac.role({ workspaceId: ws.id, diff --git a/packages/backend/server/src/core/permission/doc.ts b/packages/backend/server/src/core/permission/doc.ts index 1d2723c705..8e478c9f89 100644 --- a/packages/backend/server/src/core/permission/doc.ts +++ b/packages/backend/server/src/core/permission/doc.ts @@ -81,8 +81,12 @@ export class DocAccessController extends AccessController<'doc'> { ); // if user is in workspace but doc role is not set, fallback to default doc role - if (workspaceRole && workspaceRole !== WorkspaceRole.External) { - docRole = defaultDocRole.workspace; + if (workspaceRole !== null && workspaceRole !== WorkspaceRole.External) { + docRole = + defaultDocRole.external !== null + ? // edgecase: when doc role set to [None] for workspace member, but doc is public, we should fallback to external role + Math.max(defaultDocRole.workspace, defaultDocRole.external) + : defaultDocRole.workspace; } else { // else fallback to external doc role docRole = defaultDocRole.external; @@ -92,7 +96,10 @@ export class DocAccessController extends AccessController<'doc'> { // we need to fixup doc role to make sure it's not miss set // for example: workspace owner will have doc owner role // workspace external will not have role higher than editor - return fixupDocRole(workspaceRole, docRole); + const role = fixupDocRole(workspaceRole, docRole); + + // never return [None] + return role === DocRole.None ? null : role; } private async defaultDocRole(workspaceId: string, docId: string) { diff --git a/packages/backend/server/src/core/permission/types.ts b/packages/backend/server/src/core/permission/types.ts index ddec8648d7..8934ddb374 100644 --- a/packages/backend/server/src/core/permission/types.ts +++ b/packages/backend/server/src/core/permission/types.ts @@ -222,7 +222,7 @@ export function mapDocRoleToPermissions(docRole: DocRole | null) { {} as Record ); - if (docRole === null) { + if (docRole === null || docRole === DocRole.None) { return permissions; } @@ -249,7 +249,10 @@ export function fixupDocRole( workspaceRole: WorkspaceRole | null, docRole: DocRole | null ): DocRole | null { - if (workspaceRole === null && docRole === null) { + if ( + workspaceRole === null && + (docRole === null || docRole === DocRole.None) + ) { return null; } @@ -343,26 +346,6 @@ export function docActionRequiredRole(action: DocAction): DocRole { ); } -/** - * Useful when a workspace member doesn't have a specified role in the doc, but want to check the permission of the action - */ -export function docActionRequiredWorkspaceRole( - action: DocAction -): WorkspaceRole { - const docRole = docActionRequiredRole(action); - - switch (docRole) { - case DocRole.Owner: - return WorkspaceRole.Owner; - case DocRole.Manager: - return WorkspaceRole.Admin; - case DocRole.Editor: - case DocRole.Reader: - case DocRole.External: - return WorkspaceRole.Collaborator; - } -} - export function workspaceActionRequiredRole( action: WorkspaceAction ): WorkspaceRole { diff --git a/packages/backend/server/src/models/common/role.ts b/packages/backend/server/src/models/common/role.ts index 0239ffa4fd..f2cb868709 100644 --- a/packages/backend/server/src/models/common/role.ts +++ b/packages/backend/server/src/models/common/role.ts @@ -1,4 +1,8 @@ export enum DocRole { + /** + * `None` equals to `role = null`, it only exists to give a value that API can use + */ + None = -(1 << 15), External = 0, Reader = 10, Editor = 20,