diff --git a/app/group/handler.ts b/app/group/handler.ts index ae93e2725..057e3394a 100644 --- a/app/group/handler.ts +++ b/app/group/handler.ts @@ -15,7 +15,12 @@ export const get = { description: 'get group by id', tags: ['api'], handler: async (request: Hapi.Request) => { - return Resource.get(request.params.id, request.query); + const { id: loggedInUserId } = request?.auth?.credentials || { id: '' }; + return Resource.get( + request.params.id, + loggedInUserId, + request.query + ); } }; diff --git a/app/group/resource.ts b/app/group/resource.ts index 51926c8e7..7e6e6ee84 100644 --- a/app/group/resource.ts +++ b/app/group/resource.ts @@ -121,8 +121,8 @@ export const checkSubjectHasAccessToEditGroup = async ( // ? 2) Count all members of a group // ? 3) Find all users with specified user_role as well // ? 4) Check whether current logged in user is mapped with the group -export const list = async (filters: JSObj = {}, loggedInUserId: string) => { - const { user_role = '', ...resource } = filters; +export const list = async (filters: JSObj = {}, loggedInUserId = '') => { + const { user_role = '', group, ...attributes } = filters; const GET_GROUP_DOC = `JSON_AGG(DISTINCT groups.*) AS group_arr`; const MEMBER_COUNT = `SUM(CASE WHEN casbin_rule.ptype = 'g' THEN 1 ELSE 0 END) AS member_count`; @@ -151,10 +151,15 @@ export const list = async (filters: JSObj = {}, loggedInUserId: string) => { ) .groupBy('groups.id'); - if (!R.isEmpty(resource)) { + // ? this is to filter single group query if passed in filters + if (!R.isNil(group)) { + cursor.where(`groups.id = :groupId`, { groupId: group }); + } + + if (!R.isEmpty(attributes)) { const FILTER_BY_RESOURCE_ATTRIBUTES = `SUM(CASE WHEN casbin_rule.ptype = 'g2' AND casbin_rule.v1 like :attribute THEN 1 ELSE 0 END) > 0`; cursor.having(FILTER_BY_RESOURCE_ATTRIBUTES, { - attribute: toLikeQuery(resource) + attribute: toLikeQuery(attributes) }); } @@ -162,13 +167,17 @@ export const list = async (filters: JSObj = {}, loggedInUserId: string) => { return parseGroupListResult(rawResult); }; -export const get = async (groupId: string, filters?: JSObj) => { - const group = await Group.findOne(groupId); +export const get = async ( + groupId: string, + loggedInUserId = '', + filters: JSObj = {} +) => { + const groupList = await list({ group: groupId, ...filters }, loggedInUserId); + const group = R.head(groupList); if (!group) throw Boom.notFound('group not found'); - const subject = { group: group?.id }; + const subject = { group: R.path(['id'], group) }; const policies = await PolicyResource.getPoliciesBySubject(subject, filters); - const attributes = await PolicyResource.getAttributesForGroup(groupId); - return { ...group, policies, attributes }; + return { ...group, policies }; }; const getValidGroupname = async (payload: any) => { @@ -200,7 +209,7 @@ export const create = async (payload: any, loggedInUserId: string) => { policies, loggedInUserId ); - const updatedGroup = await get(groupId); + const updatedGroup = await get(groupId, loggedInUserId); return { ...updatedGroup, policyOperationResult }; }; @@ -210,7 +219,7 @@ export const update = async ( loggedInUserId: string ) => { const { policies = [], attributes = [], ...groupPayload } = payload; - const groupWithExtraKeys = await get(groupId); + const groupWithExtraKeys = await get(groupId, loggedInUserId); const group = R.omit(['policies', 'attributes'], groupWithExtraKeys); // ? We need to check this only if the attributes change @@ -227,6 +236,6 @@ export const update = async ( policies, loggedInUserId ); - const updatedGroup = await get(groupId); + const updatedGroup = await get(groupId, loggedInUserId); return { ...updatedGroup, policyOperationResult }; }; diff --git a/test/app/group/handler.ts b/test/app/group/handler.ts index 3ba305626..a7ffdad0f 100644 --- a/test/app/group/handler.ts +++ b/test/app/group/handler.ts @@ -92,13 +92,18 @@ lab.experiment('Group::Handler', () => { getStub.restore(); }); - lab.test('should return current profile', async () => { + lab.test('should return group by id', async () => { getStub.resolves(group); const response = await server.inject(request); - Sandbox.assert.calledWithExactly(getStub, group.id.toString(), { - test: '123' - }); + Sandbox.assert.calledWithExactly( + getStub, + group.id.toString(), + TEST_AUTH.credentials.id, + { + test: '123' + } + ); Code.expect(response.result).to.equal(group); Code.expect(response.statusCode).to.equal(200); }); diff --git a/test/app/group/resource.ts b/test/app/group/resource.ts index a2ed59334..08aafe1ef 100644 --- a/test/app/group/resource.ts +++ b/test/app/group/resource.ts @@ -38,22 +38,21 @@ lab.experiment('Group::resource', () => { const getPoliciesBySubjectStub = Sandbox.stub( PolicyResource, 'getPoliciesBySubject' - ).returns(policies); + ).resolves(policies); - const attributes = [{ entity: 'gojek' }]; - const getAttributesForGroupStub = Sandbox.stub( - PolicyResource, - 'getAttributesForGroup' - ).returns(attributes); + const groupList = [group]; + const listStub = Sandbox.stub(Resource, 'list').resolves(groupList); - const response = await Resource.get(group.id, {}); - Code.expect(response).to.equal({ ...group, policies, attributes }); + const response = await Resource.get(group.id); + Code.expect(response).to.equal({ ...group, policies }); Sandbox.assert.calledWithExactly( getPoliciesBySubjectStub, - { group: group.id }, + { + group: group.id + }, {} ); - Sandbox.assert.calledWithExactly(getAttributesForGroupStub, group.id); + Sandbox.assert.calledWithExactly(listStub, { group: group.id }, ''); } ); @@ -61,7 +60,9 @@ lab.experiment('Group::resource', () => { 'should return undefined response if group is not found', async () => { try { + const listStub = Sandbox.stub(Resource, 'list').resolves([]); await Resource.get(Faker.random.uuid()); + Sandbox.assert.calledOnce(listStub); } catch (e) { Code.expect(e.output.statusCode).to.equal(404); } @@ -140,7 +141,7 @@ lab.experiment('Group::resource', () => { payload.policies, loggedInUserId ); - Sandbox.assert.calledWithExactly(getStub, groupId); + Sandbox.assert.calledWithExactly(getStub, groupId, loggedInUserId); Code.expect(response).to.equal({ id: groupId, ...payload, @@ -154,7 +155,7 @@ lab.experiment('Group::resource', () => { Sandbox.restore(); }); - lab.test('should create group by id', async () => { + lab.test('should update group by id', async () => { const payload = { displayname: 'Data Engineering', metadata: { @@ -211,7 +212,7 @@ lab.experiment('Group::resource', () => { payload.policies, loggedInUserId ); - Sandbox.assert.calledWithExactly(getStub, groupId); + Sandbox.assert.calledWithExactly(getStub, groupId, loggedInUserId); Sandbox.assert.callCount(getStub, 2); Code.expect(response).to.equal({ id: groupId,