Skip to content

Commit 6c5f6da

Browse files
authored
My form list update with correct list (#1900)
* My form list update with correct list * spec update * Updated for group users submission * updated for error check * unit test case fix
1 parent 4566bd2 commit 6c5f6da

5 files changed

Lines changed: 79 additions & 20 deletions

File tree

app/src/components/tenantService.js

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ const config = require('config');
22
const axios = require('axios');
33
const jwtService = require('./jwtService');
44
const idpService = require('./idpService');
5+
const log = require('./log')(module.filename);
56
const SERVICE = 'TenantService';
67
const endpoint = config.get('cstar.endpoint');
78
const listUserTenantsPath = config.get('cstar.listUserTenantsPath');
@@ -144,8 +145,14 @@ class TenantService {
144145
const formGroups = await FormGroup.query().where({ formId }).select('groupId');
145146
const associatedGroupIds = formGroups.map((fg) => fg.groupId);
146147

147-
// Source of truth
148-
const allTenantGroups = await this.getGroupsForCurrentTenant(req);
148+
// Source of truth — degrade gracefully if CSTAR is unavailable so a
149+
// transient outage doesn't produce an error alert that persists across navigation.
150+
let allTenantGroups = [];
151+
try {
152+
allTenantGroups = await this.getGroupsForCurrentTenant(req);
153+
} catch (err) {
154+
log.error(`${SERVICE}: failed to fetch tenant groups for ${req.currentUser.tenantId}, degrading to empty group list`, err);
155+
}
149156

150157
// Associated and present
151158
const associatedGroups = allTenantGroups

app/src/forms/auth/service.js

Lines changed: 29 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,12 @@ const service = {
126126
.modify('filterActive', params.active)
127127
.modify('filterTenantId', userInfo.tenantId);
128128

129-
const userGroups = await tenantService.getUserTenantGroupsAndRoles({ currentUser: userInfo, headers }, userInfo.tenantId);
129+
let userGroups = [];
130+
try {
131+
userGroups = await tenantService.getUserTenantGroupsAndRoles({ currentUser: userInfo, headers }, userInfo.tenantId);
132+
} catch (err) {
133+
log.error(`Failed to fetch tenant groups/roles for tenant ${userInfo.tenantId}`, err);
134+
}
130135
const allRoles = await Role.query().withGraphFetched('permissions');
131136

132137
for (const item of items) {
@@ -137,19 +142,31 @@ const service = {
137142
} else {
138143
// if user has an id, then we fetch whatever forms match the query params
139144
items = await UserFormAccess.query().modify('filterUserId', userInfo.id).modify('filterFormId', params.formId).modify('filterActive', params.active);
140-
const userGroupsByTenant = new Map();
141-
const allRoles = await Role.query().withGraphFetched('permissions');
142-
for (const item of items) {
143-
if (item && item.tenantId && Array.isArray(item.idps) && item.idps.length === 0) {
144-
// Group-only tenanted forms need tenant context to look up roles; skip if no headers
145-
if (!headers) continue;
146145

147-
if (!userGroupsByTenant.has(item.tenantId)) {
148-
const userGroups = await tenantService.getUserTenantGroupsAndRoles({ currentUser: userInfo, headers }, item.tenantId);
149-
userGroupsByTenant.set(item.tenantId, userGroups);
146+
// For single-form permission checks (params.formId set, e.g. hasFormPermissions
147+
// middleware on /form/submit, /user/draft, /user/view), resolve group-based roles
148+
// using the form's own tenantId from form_tenant — not from the request header,
149+
// since those routes never send x-tenant-id by design. This lets legitimate group
150+
// members access the form regardless of which tenant is currently selected in the UI.
151+
//
152+
// For list-all calls (no formId, e.g. "My Forms"), skip resolution: group-only
153+
// forms must only appear when that tenant is actively selected (branch above).
154+
if (params.formId && headers) {
155+
const userGroupsByTenant = new Map();
156+
const allRoles = await Role.query().withGraphFetched('permissions');
157+
for (const item of items) {
158+
if (item && item.tenantId && Array.isArray(item.idps) && item.idps.length === 0) {
159+
if (!userGroupsByTenant.has(item.tenantId)) {
160+
let userGroups = [];
161+
try {
162+
userGroups = await tenantService.getUserTenantGroupsAndRoles({ currentUser: userInfo, headers }, item.tenantId);
163+
} catch (err) {
164+
log.error(`Failed to fetch tenant groups/roles for form ${item.formId} (tenant ${item.tenantId})`, err);
165+
}
166+
userGroupsByTenant.set(item.tenantId, userGroups);
167+
}
168+
await service.populateItemWithTenantRoles(item, userGroupsByTenant.get(item.tenantId), allRoles);
150169
}
151-
152-
await service.populateItemWithTenantRoles(item, userGroupsByTenant.get(item.tenantId), allRoles);
153170
}
154171
}
155172

app/src/forms/rbac/service.js

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ const { Roles } = require('../common/constants');
55
const { queryUtils } = require('../common/utils');
66
const authService = require('../auth/service');
77
const idpService = require('../../components/idpService');
8+
const log = require('../../components/log')(module.filename);
89

910
const service = {
1011
list: async () => {
@@ -93,7 +94,8 @@ const service = {
9394
);
9495
const filteredForms = authService.filterForms(currentUser, forms, accessLevels);
9596
return filteredForms;
96-
} catch {
97+
} catch (err) {
98+
log.error('Failed to get current user forms', err);
9799
return [];
98100
}
99101
},

app/tests/unit/components/tenantService.spec.js

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -479,7 +479,7 @@ describe('TenantService', () => {
479479
await expect(tenantService.getFormGroups(req, formId)).rejects.toThrow('DB error');
480480
});
481481

482-
it('should throw error when getGroupsForCurrentTenant fails', async () => {
482+
it('should degrade gracefully when getGroupsForCurrentTenant fails (CSTAR unavailable)', async () => {
483483
FormTenant.query.mockReturnValue({
484484
where: jest.fn().mockReturnValue({
485485
first: jest.fn().mockResolvedValue({ formId, tenantId: tenantId }),
@@ -488,13 +488,24 @@ describe('TenantService', () => {
488488

489489
FormGroup.query.mockReturnValue({
490490
where: jest.fn().mockReturnValue({
491-
select: jest.fn().mockResolvedValue([]),
491+
select: jest.fn().mockResolvedValue([{ groupId: 'group-1' }]),
492492
}),
493493
});
494494

495495
mockAxios.onGet(apiUrl).networkError();
496496

497-
await expect(tenantService.getFormGroups(req, formId)).rejects.toThrow();
497+
const result = await tenantService.getFormGroups(req, formId);
498+
expect(result.associatedGroups).toEqual([]);
499+
expect(result.availableGroups).toEqual([]);
500+
expect(result.missingGroups).toEqual([
501+
{
502+
id: 'group-1',
503+
name: 'Group no longer available',
504+
description: 'This group may have been deleted from the tenant',
505+
isAssociated: true,
506+
isDeleted: true,
507+
},
508+
]);
498509
});
499510
});
500511

app/tests/unit/forms/auth/authService.spec.js

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -198,22 +198,44 @@ describe('getUserForms', () => {
198198
expect(tenantSpy).not.toHaveBeenCalled();
199199
});
200200

201-
it('personal path: tenanted group-only form populates tenant roles when headers are present', async () => {
201+
it('personal path: group-only form without formId param does not resolve tenant roles (list-all / My Forms case)', async () => {
202202
const userInfo = { id: 'user-1' };
203203
const headers = { authorization: 'Bearer token' };
204204
const items = [{ formId: 'group-form', tenantId: 'tenant-1', idps: [], roles: [], permissions: [] }];
205205

206206
jest.spyOn(queryUtils, 'defaultActiveOnly').mockReturnValue({ active: true });
207207
const queryObj = makeQueryObj(items);
208208
jest.spyOn(UserFormAccess, 'query').mockReturnValue(queryObj);
209+
const tenantSpy = jest.spyOn(tenantService, 'getUserTenantGroupsAndRoles');
210+
jest.spyOn(service, 'filterForms').mockReturnValue([]);
211+
212+
// No formId in params → list-all path (My Forms). Group-only forms must stay
213+
// unresolved so they are excluded outside of an actively selected tenant context.
214+
const result = await service.getUserForms(userInfo, {}, headers);
215+
216+
expect(tenantSpy).not.toHaveBeenCalled();
217+
expect(result).toEqual([]);
218+
});
219+
220+
it('personal path: group-only form WITH formId param resolves tenant roles via form own tenantId (single-form permission check)', async () => {
221+
const userInfo = { id: 'user-1' };
222+
const headers = { authorization: 'Bearer token' };
223+
const items = [{ formId: 'group-form', tenantId: 'tenant-1', idps: [], roles: [], permissions: [] }];
224+
225+
jest.spyOn(queryUtils, 'defaultActiveOnly').mockReturnValue({ formId: 'group-form', active: true });
226+
const queryObj = makeQueryObj(items);
227+
jest.spyOn(UserFormAccess, 'query').mockReturnValue(queryObj);
209228
jest.spyOn(Role, 'query').mockReturnValue({
210229
withGraphFetched: jest.fn().mockResolvedValue([{ code: 'submission_reviewer', permissions: [{ code: 'submission_read' }] }]),
211230
});
212231
jest.spyOn(FormGroup, 'query').mockReturnValue({ modify: jest.fn().mockResolvedValue([{ groupId: 'group-1' }]) });
213232
jest.spyOn(tenantService, 'getUserTenantGroupsAndRoles').mockResolvedValue([{ id: 'group-1', roles: ['submission_reviewer'] }]);
214233
jest.spyOn(service, 'filterForms').mockReturnValue(['group-form']);
215234

216-
const result = await service.getUserForms(userInfo, {}, headers);
235+
// formId in params → single-form permission check (e.g. hasFormPermissions on
236+
// /form/submit or /user/draft which never send x-tenant-id). Roles must be
237+
// resolved using the form's own tenantId so legitimate group members get access.
238+
const result = await service.getUserForms(userInfo, { formId: 'group-form' }, headers);
217239

218240
expect(tenantService.getUserTenantGroupsAndRoles).toHaveBeenCalledWith({ currentUser: userInfo, headers }, 'tenant-1');
219241
expect(result).toEqual(['group-form']);

0 commit comments

Comments
 (0)