From d6a2ab4b61e0db754e84c3f59205b0b3f7f2357a Mon Sep 17 00:00:00 2001 From: ChengLei Shao Date: Wed, 6 Sep 2023 09:19:10 +0800 Subject: [PATCH] fix(acl): parse acl params of association collection (#2594) * fix(acl): parse params of association collection * fix: filter params in association acl --- packages/core/acl/src/acl.ts | 24 ++-- .../acl/src/server/__tests__/actions.test.ts | 119 ++++++++++++++++++ packages/plugins/acl/src/server/server.ts | 8 +- 3 files changed, 136 insertions(+), 15 deletions(-) diff --git a/packages/core/acl/src/acl.ts b/packages/core/acl/src/acl.ts index 47309d70c..3fe5452cb 100644 --- a/packages/core/acl/src/acl.ts +++ b/packages/core/acl/src/acl.ts @@ -321,20 +321,20 @@ export class ACL extends EventEmitter { this.snippetManager.register(snippet); } + filterParams(ctx, resourceName, params) { + if (params?.filter?.createdById) { + const collection = ctx.db.getCollection(resourceName); + if (!collection || !collection.getField('createdById')) { + return lodash.omit(params, 'filter.createdById'); + } + } + + return params; + } + protected addCoreMiddleware() { const acl = this; - const filterParams = (ctx, resourceName, params) => { - if (params?.filter?.createdById) { - const collection = ctx.db.getCollection(resourceName); - if (!collection || !collection.getField('createdById')) { - return lodash.omit(params, 'filter.createdById'); - } - } - - return params; - }; - this.middlewares.add( async (ctx, next) => { const resourcerAction: Action = ctx.action; @@ -354,7 +354,7 @@ export class ACL extends EventEmitter { ctx.log?.info && ctx.log.info('acl params', params); if (params && resourcerAction.mergeParams) { - const filteredParams = filterParams(ctx, resourceName, params); + const filteredParams = acl.filterParams(ctx, resourceName, params); const parsedParams = await acl.parseJsonTemplate(filteredParams, ctx); ctx.permission.parsedParams = parsedParams; diff --git a/packages/plugins/acl/src/server/__tests__/actions.test.ts b/packages/plugins/acl/src/server/__tests__/actions.test.ts index ebff31329..046434a95 100644 --- a/packages/plugins/acl/src/server/__tests__/actions.test.ts +++ b/packages/plugins/acl/src/server/__tests__/actions.test.ts @@ -26,6 +26,125 @@ describe('destroy action with acl', () => { await app.destroy(); }); + it('should load the association collection when the source collection does not have the createdById field', async () => { + const A = app.collection({ + name: 'a', + fields: [ + { type: 'string', name: 'title' }, + { type: 'belongsToMany', name: 'bs', target: 'b' }, + ], + }); + + const B = app.collection({ + name: 'b', + fields: [{ type: 'string', name: 'title' }], + }); + + await app.db.sync(); + + await A.repository.create({ + values: [ + { + title: 'a1', + bs: [ + { + title: 'b1', + }, + ], + }, + ], + }); + + const userRole = app.acl.define({ + role: 'user', + strategy: { + actions: ['view:own'], + }, + }); + + app.resourcer.use( + (ctx, next) => { + ctx.state.currentRole = 'user'; + ctx.state.currentUser = { + id: 1, + }; + return next(); + }, + { + before: 'acl', + }, + ); + + const a1 = await A.repository.findOne({ filter: { title: 'a1' } }); + + const response = await app.agent().resource('a.bs', a1.get('id')).list(); + expect(response.statusCode).toEqual(200); + }); + + it('should parse association acl params', async () => { + const Comment = app.db.collection({ + name: 'comments', + fields: [ + { type: 'string', name: 'content' }, + { type: 'belongsToMany', name: 'posts' }, + ], + }); + + await app.db.sync(); + + await Post.repository.create({ + values: [ + { + title: 'p1', + comments: [ + { + content: 'c11', + }, + { + content: 'c12', + }, + ], + }, + { + title: 'p2', + comments: [ + { + content: 'c21', + }, + { + content: 'c22', + }, + ], + }, + ], + }); + + const userRole = app.acl.define({ + role: 'user', + strategy: { + actions: ['view:own'], + }, + }); + + app.resourcer.use( + (ctx, next) => { + ctx.state.currentRole = 'user'; + ctx.state.currentUser = { + id: 1, + }; + return next(); + }, + { + before: 'acl', + }, + ); + + const p1 = await Post.repository.findOne({ filter: { title: 'p1' } }); + + const response = await app.agent().resource('posts.comments', p1.get('id')).list(); + expect(response.statusCode).toEqual(403); + }); + it('should throw error when user has no permission to destroy record', async () => { const userRole = app.acl.define({ role: 'user', diff --git a/packages/plugins/acl/src/server/server.ts b/packages/plugins/acl/src/server/server.ts index f90fe9ac8..df2e8ec13 100644 --- a/packages/plugins/acl/src/server/server.ts +++ b/packages/plugins/acl/src/server/server.ts @@ -570,7 +570,6 @@ export class PluginACL extends Plugin { const action = ctx.can({ resource: collectionName, action: actionName }); const availableAction = this.app.acl.getAvailableAction(actionName); - if (availableAction?.options?.onNewRecord) { if (action) { ctx.permission.skip = true; @@ -578,11 +577,14 @@ export class PluginACL extends Plugin { ctx.permission.can = false; } } else { - const filter = await parseJsonTemplate(action?.params?.filter || {}, ctx); + const filteredParams = this.app.acl.filterParams(ctx, collectionName, action?.params || {}); + const params = await parseJsonTemplate(filteredParams, ctx); + const sourceInstance = await ctx.db.getRepository(collectionName).findOne({ filterByTk: resourceOf, - filter, + filter: params.filter || {}, }); + if (!sourceInstance) { ctx.permission.can = false; }