Skip to content

Commit 2db7d16

Browse files
uneq95kushalshit27
andauthored
fix: pagination bug skipping last page in do-while loops (#1277)
The do-while pagination pattern was incorrectly checking hasNextPage() after calling getNextPage(), causing the last page of results to be skipped. Affected functions: - getConnectionEnabledClients() in connections.ts - role permissions listing in roles.ts - getOrganizationEnabledConnections() in organizations.ts - getOrganizationClientGrants() in organizations.ts - getAllOrganizationDiscoveryDomains() in organizations.ts - getAllFlowConnections() in flowVaultConnections.ts Added multi-page pagination tests to verify the fix. Fixes #1272 Co-authored-by: Kushal <43465488+kushalshit27@users.noreply.github.com>
1 parent 2a1b2a5 commit 2db7d16

8 files changed

Lines changed: 223 additions & 23 deletions

File tree

src/tools/auth0/handlers/connections.ts

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -162,17 +162,22 @@ export const getConnectionEnabledClients = async (
162162

163163
let enabledClients = await auth0Client.connections.clients.get(connectionId);
164164

165-
do {
166-
if (enabledClients && enabledClients.data?.length > 0) {
167-
enabledClients.data.forEach((client) => {
168-
if (client?.client_id) {
169-
enabledClientsFormatted.push(client.client_id);
170-
}
171-
});
165+
// Process first page
166+
enabledClients.data?.forEach((client) => {
167+
if (client?.client_id) {
168+
enabledClientsFormatted.push(client.client_id);
172169
}
170+
});
173171

172+
// Fetch remaining pages
173+
while (enabledClients.hasNextPage()) {
174174
enabledClients = await enabledClients.getNextPage();
175-
} while (enabledClients.hasNextPage());
175+
enabledClients.data?.forEach((client) => {
176+
if (client?.client_id) {
177+
enabledClientsFormatted.push(client.client_id);
178+
}
179+
});
180+
}
176181

177182
return enabledClientsFormatted;
178183
} catch (error) {

src/tools/auth0/handlers/flowVaultConnections.ts

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,15 @@ export const getAllFlowConnections = async (
3030
const allFlowConnections: Management.FlowsVaultConnectionSummary[] = [];
3131

3232
let vaultConnections = await auth0Client.flows.vault.connections.list();
33-
do {
34-
allFlowConnections.push(...vaultConnections.data);
33+
34+
// Process first page
35+
allFlowConnections.push(...vaultConnections.data);
36+
37+
// Fetch remaining pages
38+
while (vaultConnections.hasNextPage()) {
3539
vaultConnections = await vaultConnections.getNextPage();
36-
} while (vaultConnections.hasNextPage());
40+
allFlowConnections.push(...vaultConnections.data);
41+
}
3742

3843
return allFlowConnections;
3944
};

src/tools/auth0/handlers/organizations.ts

Lines changed: 24 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -560,10 +560,15 @@ export default class OrganizationsHandler extends DefaultHandler {
560560
let organizationConnections = await this.client.organizations.enabledConnections.list(
561561
organizationId
562562
);
563-
do {
564-
allOrganizationConnections.push(...organizationConnections.data);
563+
564+
// Process first page
565+
allOrganizationConnections.push(...organizationConnections.data);
566+
567+
// Fetch remaining pages
568+
while (organizationConnections.hasNextPage()) {
565569
organizationConnections = await organizationConnections.getNextPage();
566-
} while (organizationConnections.hasNextPage());
570+
allOrganizationConnections.push(...organizationConnections.data);
571+
}
567572

568573
return allOrganizationConnections;
569574
}
@@ -576,10 +581,15 @@ export default class OrganizationsHandler extends DefaultHandler {
576581
let organizationClientGrants = await this.client.organizations.clientGrants.list(
577582
organizationId
578583
);
579-
do {
580-
allOrganizationClientGrants.push(...organizationClientGrants.data);
584+
585+
// Process first page
586+
allOrganizationClientGrants.push(...organizationClientGrants.data);
587+
588+
// Fetch remaining pages
589+
while (organizationClientGrants.hasNextPage()) {
581590
organizationClientGrants = await organizationClientGrants.getNextPage();
582-
} while (organizationClientGrants.hasNextPage());
591+
allOrganizationClientGrants.push(...organizationClientGrants.data);
592+
}
583593

584594
return allOrganizationClientGrants;
585595
}
@@ -614,10 +624,15 @@ export default class OrganizationsHandler extends DefaultHandler {
614624
let orgDiscoveryDomain = await this.client.organizations.discoveryDomains.list(
615625
organizationId
616626
);
617-
do {
618-
allDiscoveryDomains.push(...orgDiscoveryDomain.data);
627+
628+
// Process first page
629+
allDiscoveryDomains.push(...orgDiscoveryDomain.data);
630+
631+
// Fetch remaining pages
632+
while (orgDiscoveryDomain.hasNextPage()) {
619633
orgDiscoveryDomain = await orgDiscoveryDomain.getNextPage();
620-
} while (orgDiscoveryDomain.hasNextPage());
634+
allDiscoveryDomains.push(...orgDiscoveryDomain.data);
635+
}
621636

622637
return allDiscoveryDomains;
623638
} catch (err) {

src/tools/auth0/handlers/roles.ts

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -181,10 +181,15 @@ export default class RolesHandler extends DefaultHandler {
181181

182182
const rolesId = roles[index].id as string;
183183
let permissions = await this.client.roles.permissions.list(rolesId, { per_page: 100 });
184-
do {
185-
allPermission.push(...permissions.data);
184+
185+
// Process first page
186+
allPermission.push(...permissions.data);
187+
188+
// Fetch remaining pages
189+
while (permissions.hasNextPage()) {
186190
permissions = await permissions.getNextPage();
187-
} while (permissions.hasNextPage());
191+
allPermission.push(...permissions.data);
192+
}
188193

189194
const strippedPerms = await Promise.all(
190195
allPermission.map(async (permission) => {

test/tools/auth0/handlers/connections.tests.js

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -924,6 +924,40 @@ describe('#connections enabled clients functionality', () => {
924924

925925
expect(result).to.deep.equal([]);
926926
});
927+
928+
it('should handle multi-page pagination correctly', async () => {
929+
const connectionId = 'con_123';
930+
931+
// Simulate 3 pages of results
932+
const page3 = {
933+
data: [{ client_id: 'client_7' }, { client_id: 'client_8' }],
934+
hasNextPage: () => false,
935+
getNextPage: async () => ({ data: [], hasNextPage: () => false }),
936+
};
937+
938+
const page2 = {
939+
data: [{ client_id: 'client_4' }, { client_id: 'client_5' }, { client_id: 'client_6' }],
940+
hasNextPage: () => true,
941+
getNextPage: async () => page3,
942+
};
943+
944+
const page1 = {
945+
data: [{ client_id: 'client_1' }, { client_id: 'client_2' }, { client_id: 'client_3' }],
946+
hasNextPage: () => true,
947+
getNextPage: async () => page2,
948+
};
949+
950+
mockAuth0Client.connections.clients.get.resolves(page1);
951+
952+
const result = await getConnectionEnabledClients(mockAuth0Client, connectionId);
953+
954+
// Should include ALL clients from ALL 3 pages
955+
expect(result).to.deep.equal([
956+
'client_1', 'client_2', 'client_3', // Page 1
957+
'client_4', 'client_5', 'client_6', // Page 2
958+
'client_7', 'client_8', // Page 3
959+
]);
960+
});
927961
});
928962

929963
describe('#updateConnectionEnabledClients', () => {

test/tools/auth0/handlers/flowVaultConnections.tests.js

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,39 @@ describe('#flowVaultConnections handler', () => {
126126
expect(data).to.deep.equal([sampleFlowVaultConnections]);
127127
});
128128

129+
it('should handle multi-page pagination for flowVaultConnections', async () => {
130+
// Simulate 3 pages of flow vault connections
131+
const page1 = [
132+
{ id: 'ac_1', name: 'Connection 1', app_id: 'HTTP' },
133+
{ id: 'ac_2', name: 'Connection 2', app_id: 'HTTP' },
134+
];
135+
const page2 = [
136+
{ id: 'ac_3', name: 'Connection 3', app_id: 'HTTP' },
137+
];
138+
const page3 = [
139+
{ id: 'ac_4', name: 'Connection 4', app_id: 'HTTP' },
140+
{ id: 'ac_5', name: 'Connection 5', app_id: 'HTTP' },
141+
];
142+
143+
const auth0 = {
144+
flows: {
145+
vault: {
146+
connections: {
147+
list: (params) => mockPagedData(params, 'connections', page1, [page2, page3]),
148+
},
149+
},
150+
},
151+
pool,
152+
};
153+
154+
const handler = new flowVaultConnections.default({ client: pageClient(auth0), config });
155+
const data = await handler.getType();
156+
157+
// Should include connections from ALL 3 pages
158+
expect(data).to.have.length(5);
159+
expect(data.map((c) => c.id)).to.deep.equal(['ac_1', 'ac_2', 'ac_3', 'ac_4', 'ac_5']);
160+
});
161+
129162
it('should update flowVaultConnections', async () => {
130163
const auth0 = {
131164
flows: {

test/tools/auth0/handlers/organizations.tests.js

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -338,6 +338,59 @@ describe('#organizations handler', () => {
338338
]);
339339
});
340340

341+
it('should handle multi-page pagination for enabled connections', async () => {
342+
// Simulate 3 pages of enabled connections
343+
const connectionsPage1 = [
344+
{ connection_id: 'con_1', connection: { name: 'conn1' } },
345+
{ connection_id: 'con_2', connection: { name: 'conn2' } },
346+
];
347+
const connectionsPage2 = [
348+
{ connection_id: 'con_3', connection: { name: 'conn3' } },
349+
];
350+
const connectionsPage3 = [
351+
{ connection_id: 'con_4', connection: { name: 'conn4' } },
352+
{ connection_id: 'con_5', connection: { name: 'conn5' } },
353+
];
354+
355+
const auth0 = {
356+
organizations: {
357+
list: (params) => Promise.resolve(mockPagedData(params, 'organizations', [sampleOrg])),
358+
enabledConnections: {
359+
list: () =>
360+
mockPagedData(
361+
{},
362+
'enabled_connections',
363+
connectionsPage1,
364+
[connectionsPage2, connectionsPage3]
365+
),
366+
},
367+
clientGrants: {
368+
list: () => mockPagedData({}, 'client_grants', []),
369+
},
370+
discoveryDomains: {
371+
list: () => mockPagedData({}, 'discovery_domains', []),
372+
},
373+
},
374+
clients: {
375+
list: (params) => mockPagedData(params, 'clients', sampleClients),
376+
},
377+
pool,
378+
};
379+
380+
const handler = new organizations.default({ client: pageClient(auth0), config });
381+
const data = await handler.getType();
382+
383+
// Should include connections from ALL 3 pages
384+
expect(data[0].connections).to.have.length(5);
385+
expect(data[0].connections.map((c) => c.connection_id)).to.deep.equal([
386+
'con_1',
387+
'con_2',
388+
'con_3',
389+
'con_4',
390+
'con_5',
391+
]);
392+
});
393+
341394
it('should get all organizations', async function () {
342395
const organizationsPage1 = Array.from({ length: 3 }, (v, i) => ({
343396
id: 'org_' + i,

test/tools/auth0/handlers/roles.tests.js

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,56 @@ describe('#roles handler', () => {
143143
]);
144144
});
145145

146+
it('should handle multi-page pagination for role permissions', async () => {
147+
// Simulate 3 pages of permissions
148+
const page1Permissions = [
149+
{ permission_name: 'read:users', resource_server_identifier: 'api1' },
150+
{ permission_name: 'write:users', resource_server_identifier: 'api1' },
151+
];
152+
const page2Permissions = [
153+
{ permission_name: 'read:orders', resource_server_identifier: 'api2' },
154+
{ permission_name: 'write:orders', resource_server_identifier: 'api2' },
155+
];
156+
const page3Permissions = [
157+
{ permission_name: 'delete:all', resource_server_identifier: 'api3' },
158+
];
159+
160+
const auth0 = {
161+
roles: {
162+
list: (params) =>
163+
mockPagedData({ ...params, include_totals: true }, 'roles', [
164+
{
165+
name: 'adminRole',
166+
id: 'role_123',
167+
description: 'Admin role with multi-page permissions',
168+
},
169+
]),
170+
permissions: {
171+
list: (roleId, params) =>
172+
mockPagedData(
173+
{ ...params, include_totals: true },
174+
'permissions',
175+
page1Permissions,
176+
[page2Permissions, page3Permissions]
177+
),
178+
},
179+
},
180+
pool,
181+
};
182+
183+
const handler = new roles.default({ client: pageClient(auth0), config });
184+
const data = await handler.getType();
185+
186+
// Should include permissions from ALL 3 pages
187+
expect(data[0].permissions).to.deep.equal([
188+
{ permission_name: 'read:users', resource_server_identifier: 'api1' },
189+
{ permission_name: 'write:users', resource_server_identifier: 'api1' },
190+
{ permission_name: 'read:orders', resource_server_identifier: 'api2' },
191+
{ permission_name: 'write:orders', resource_server_identifier: 'api2' },
192+
{ permission_name: 'delete:all', resource_server_identifier: 'api3' },
193+
]);
194+
});
195+
146196
it('should return an empty array for 501 status code', async () => {
147197
const auth0 = {
148198
roles: {

0 commit comments

Comments
 (0)