Skip to content

Commit 0d62d88

Browse files
committed
fix(bb-auth-cognito): lifecycle slice self-sufficient for getUser group read
getUser is lifecycle-gated but reports group memberships via AdminListGroupsForUser, which was only in the 'groups' IAM slice. A pool with admin: { actions: ['lifecycle'] } would grant AdminGetUser but not AdminListGroupsForUser, so getUser 500'd with IAM AccessDenied only in the deployed AWS runtime (mock reads groups from memory; the admin:{} sandbox e2e grants everything — both hid it). - Add AdminListGroupsForUser to the lifecycle IAM slice (shared with groups). - Make getUser's group fetch best-effort: swallow AccessDenied and return groups: undefined rather than failing the whole read under a hand-narrowed policy. - Regression tests: CDK lifecycle-only self-sufficiency (AdminGetUser + AdminListGroupsForUser both granted) and a mock lifecycle-only getUser path. Caught in PR review; a self-inflicted gap from the earlier getUser groups fix.
1 parent 1be77a9 commit 0d62d88

4 files changed

Lines changed: 62 additions & 15 deletions

File tree

packages/bb-auth-cognito/src/admin.test.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -239,4 +239,18 @@ describe('auth.admin action-scope runtime gate (Gap 3)', () => {
239239
const all = new AuthCognito(ROOT, unique(), { admin: {} });
240240
await all.admin.createUser('hugo'); // no actions restriction → no throw
241241
});
242+
243+
test('lifecycle-only getUser still reports groups (no cross-action gate)', async () => {
244+
// getUser reports group memberships but is lifecycle-gated. A lifecycle-only
245+
// pool must NOT be blocked by the runtime action gate on the group read
246+
// (the CDK grant makes AdminListGroupsForUser available to lifecycle too).
247+
const auth = new AuthCognito(ROOT, unique(), { groups: ['admins'], admin: { actions: ['lifecycle'] } });
248+
await auth.admin.createUser('ivy');
249+
// addUserToGroup is a groups action → seed membership via the all-access
250+
// instance sharing the same on-disk state would differ; instead assert
251+
// getUser works and returns an (empty) groups array without throwing.
252+
const user = await auth.admin.getUser('ivy');
253+
assert.ok(user);
254+
assert.deepStrictEqual(user.groups, []);
255+
});
242256
});

packages/bb-auth-cognito/src/index.aws.ts

Lines changed: 20 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -842,22 +842,31 @@ export class AuthCognito<const O extends AuthCognitoOptions = AuthCognitoOptions
842842
}
843843
// AdminGetUser does not return group memberships, so fetch them
844844
// separately to populate AdminUser.groups (matches the mock,
845-
// which reads groups from its in-memory state).
846-
const groups: string[] = [];
847-
let nextToken: string | undefined;
848-
do {
849-
const g = await this.client.send(new AdminListGroupsForUserCommand({
850-
UserPoolId: this.adminUserPoolId(), Username: username, NextToken: nextToken,
851-
}));
852-
for (const grp of g.Groups ?? []) if (grp.GroupName) groups.push(grp.GroupName);
853-
nextToken = g.NextToken;
854-
} while (nextToken);
845+
// which reads groups from its in-memory state). The lifecycle
846+
// IAM slice grants AdminListGroupsForUser for exactly this; if a
847+
// hand-narrowed policy omits it, degrade to `groups: undefined`
848+
// rather than failing the whole read.
849+
let groups: string[] | undefined = [];
850+
try {
851+
let nextToken: string | undefined;
852+
do {
853+
const g = await this.client.send(new AdminListGroupsForUserCommand({
854+
UserPoolId: this.adminUserPoolId(), Username: username, NextToken: nextToken,
855+
}));
856+
for (const grp of g.Groups ?? []) if (grp.GroupName) groups!.push(grp.GroupName);
857+
nextToken = g.NextToken;
858+
} while (nextToken);
859+
} catch (e) {
860+
// Missing AdminListGroupsForUser grant → report groups as unknown.
861+
if (e instanceof Error && /AccessDenied|NotAuthorized/.test(e.name)) groups = undefined;
862+
else throw e;
863+
}
855864
return {
856865
username: resp.Username ?? username,
857866
userSub: attributes['sub'] ?? '',
858867
enabled: resp.Enabled ?? true,
859868
attributes,
860-
groups: groups as GroupOf<AuthCognitoOptions>[],
869+
groups: groups as GroupOf<AuthCognitoOptions>[] | undefined,
861870
};
862871
} catch (e) {
863872
if (e instanceof Error && e.name === AuthCognitoErrors.UserNotFound) return null;

packages/bb-auth-cognito/src/index.cdk.test.ts

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -568,9 +568,17 @@ const LIFECYCLE_ADMIN_ACTIONS = [
568568
'cognito-idp:AdminResetUserPassword',
569569
'cognito-idp:AdminSetUserPassword',
570570
'cognito-idp:AdminGetUser',
571+
// Shared with the group slice — getUser reports group memberships, so the
572+
// lifecycle slice must be self-sufficient for that read.
573+
'cognito-idp:AdminListGroupsForUser',
571574
'cognito-idp:ListUsers',
572575
'cognito-idp:AdminUserGlobalSignOut',
573576
];
577+
// Actions granted by BOTH slices — excluded from the "does not grant the other
578+
// slice's actions" cross-checks below.
579+
const SHARED_ADMIN_ACTIONS = ['cognito-idp:AdminListGroupsForUser'];
580+
const groupOnlyActions = GROUP_ADMIN_ACTIONS.filter((a) => !SHARED_ADMIN_ACTIONS.includes(a));
581+
const lifecycleOnlyActions = LIFECYCLE_ADMIN_ACTIONS.filter((a) => !SHARED_ADMIN_ACTIONS.includes(a));
574582

575583
describe('AuthCognito (CDK) — admin IAM grant', () => {
576584
test('no admin option → NO Admin* actions granted (least privilege)', () => {
@@ -593,21 +601,33 @@ describe('AuthCognito (CDK) — admin IAM grant', () => {
593601
}
594602
});
595603

596-
test("actions: ['groups'] → grants group actions only", () => {
604+
test("actions: ['groups'] → grants group actions, not lifecycle-only actions", () => {
597605
const template = synth((stack) => {
598606
new AuthCognito(scope(stack), 'auth', { groups: ['admins'], admin: { actions: ['groups'] } });
599607
});
600608
const actions = grantedActions(template);
601609
for (const a of GROUP_ADMIN_ACTIONS) assert.ok(actions.has(a), `missing group action ${a}`);
602-
for (const a of LIFECYCLE_ADMIN_ACTIONS) assert.ok(!actions.has(a), `unexpected lifecycle action ${a}`);
610+
for (const a of lifecycleOnlyActions) assert.ok(!actions.has(a), `unexpected lifecycle action ${a}`);
603611
});
604612

605-
test("actions: ['lifecycle'] → grants lifecycle actions only", () => {
613+
test("actions: ['lifecycle'] → grants lifecycle actions, not group-only actions", () => {
606614
const template = synth((stack) => {
607615
new AuthCognito(scope(stack), 'auth', { groups: ['admins'], admin: { actions: ['lifecycle'] } });
608616
});
609617
const actions = grantedActions(template);
610618
for (const a of LIFECYCLE_ADMIN_ACTIONS) assert.ok(actions.has(a), `missing lifecycle action ${a}`);
611-
for (const a of GROUP_ADMIN_ACTIONS) assert.ok(!actions.has(a), `unexpected group action ${a}`);
619+
for (const a of groupOnlyActions) assert.ok(!actions.has(a), `unexpected group action ${a}`);
620+
});
621+
622+
test("actions: ['lifecycle'] is self-sufficient for getUser's group read (regression)", () => {
623+
// getUser is lifecycle-gated but reports group memberships via
624+
// AdminListGroupsForUser. A lifecycle-only pool must therefore grant that
625+
// action, or getUser 500s with IAM AccessDenied in the deployed runtime.
626+
const template = synth((stack) => {
627+
new AuthCognito(scope(stack), 'auth', { groups: ['admins'], admin: { actions: ['lifecycle'] } });
628+
});
629+
const actions = grantedActions(template);
630+
assert.ok(actions.has('cognito-idp:AdminGetUser'), 'lifecycle must grant AdminGetUser');
631+
assert.ok(actions.has('cognito-idp:AdminListGroupsForUser'), 'lifecycle must grant AdminListGroupsForUser for getUser groups');
612632
});
613633
});

packages/bb-auth-cognito/src/index.cdk.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -388,6 +388,10 @@ function adminIamActions(actions?: readonly ('groups' | 'lifecycle')[]): string[
388388
'cognito-idp:AdminResetUserPassword',
389389
'cognito-idp:AdminSetUserPassword',
390390
'cognito-idp:AdminGetUser',
391+
// getUser also reports the user's group memberships (AdminGetUser does
392+
// not return them), so the lifecycle slice must be self-sufficient for
393+
// that read. Shared with the `groups` slice, which also grants it.
394+
'cognito-idp:AdminListGroupsForUser',
391395
'cognito-idp:ListUsers',
392396
'cognito-idp:AdminUserGlobalSignOut',
393397
];

0 commit comments

Comments
 (0)