Skip to content

Commit

Permalink
fix: updating a user pool updates cached options
Browse files Browse the repository at this point in the history
  • Loading branch information
jagregory committed Feb 18, 2022
1 parent ae88ba7 commit dc2b10e
Show file tree
Hide file tree
Showing 27 changed files with 125 additions and 73 deletions.
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
.cognito
.eslintcache
.idea
lib
node_modules
Expand Down
4 changes: 2 additions & 2 deletions integration-tests/cognitoService.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,11 +85,11 @@ describe("Cognito Service", () => {
expect(fs.existsSync(`${dataDirectory}/test-pool-1.json`)).toBe(true);
expect(fs.existsSync(`${dataDirectory}/test-pool-2.json`)).toBe(true);

await cognitoService.deleteUserPool(TestContext, up1.config);
await cognitoService.deleteUserPool(TestContext, up1.options);

expect(fs.existsSync(`${dataDirectory}/test-pool-1.json`)).not.toBe(true);

await cognitoService.deleteUserPool(TestContext, up2.config);
await cognitoService.deleteUserPool(TestContext, up2.options);

expect(fs.existsSync(`${dataDirectory}/test-pool-2.json`)).not.toBe(true);
});
Expand Down
6 changes: 3 additions & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
"format": "prettier --write src/**/*.ts integration-tests/**/*.ts",
"integration-test": "jest --config integration-tests/jest.config.js",
"integration-test:watch": "jest --config integration-tests/jest.config.js --watch",
"lint": "eslint src/**/*.ts && tsc --noEmit",
"lint": "eslint --cache src/**/*.ts && tsc --noEmit",
"start": "COGNITO_LOCAL_DEVMODE=1 ts-node src/bin/start.ts",
"start:debug": "COGNITO_LOCAL_DEVMODE=1 DEBUG=1 node -r ts-node/register --inspect=9230 --enable-source-maps src/bin/start.ts",
"start:watch": "nodemon",
Expand Down Expand Up @@ -87,8 +87,8 @@
},
"lint-staged": {
"*.ts": [
"eslint --fix",
"tsc --esModuleInterop --resolveJsonModule --noEmit ./setupTests.ts",
"eslint --fix --cache",
"tsc --target es2019 --moduleResolution node --esModuleInterop --resolveJsonModule --noEmit ./setupTests.ts",
"prettier --write"
],
"README.md": "markdown-toc -i --bullets=- --maxdepth=3"
Expand Down
4 changes: 2 additions & 2 deletions src/__tests__/mockUserPoolService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ export const newMockUserPoolService = (
Id: "test",
}
): jest.Mocked<UserPoolService> => ({
config,
options: config,
deleteAppClient: jest.fn(),
deleteGroup: jest.fn(),
deleteUser: jest.fn(),
Expand All @@ -20,7 +20,7 @@ export const newMockUserPoolService = (
saveGroup: jest.fn(),
saveUser: jest.fn(),
storeRefreshToken: jest.fn(),
update: jest.fn(),
updateOptions: jest.fn(),
});

export const newMockUserPoolServiceFactory = (
Expand Down
4 changes: 2 additions & 2 deletions src/services/cognitoService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -320,7 +320,7 @@ export class CognitoServiceImpl implements CognitoService {
)
);

return service.config;
return service.options;
}

public async deleteUserPool(ctx: Context, userPool: UserPool): Promise<void> {
Expand Down Expand Up @@ -403,7 +403,7 @@ export class CognitoServiceImpl implements CognitoService {
path.basename(x.name, path.extname(x.name))
);

return userPool.config;
return userPool.options;
})
);
}
Expand Down
43 changes: 43 additions & 0 deletions src/services/userPoolService.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -650,4 +650,47 @@ describe("User Pool Service", () => {
expect(groups[0].GroupName).toEqual("theGroupName");
});
});

describe("updateOptions", () => {
it("updates the options in the datastore", async () => {
const ds = newMockDataStore();
const userPool = new UserPoolServiceImpl(
mockClientsDataStore,
clock,
ds,
{
Id: "test",
}
);

await userPool.updateOptions(TestContext, {
Id: "test",
MfaConfiguration: "ON",
});

expect(ds.set).toHaveBeenCalledWith(TestContext, "Options", {
Id: "test",
MfaConfiguration: "ON",
});
});

it("updates the cached options", async () => {
const ds = newMockDataStore();
const userPool = new UserPoolServiceImpl(
mockClientsDataStore,
clock,
ds,
{
Id: "test",
}
);

await userPool.updateOptions(TestContext, {
Id: "test",
MfaConfiguration: "ON",
});

expect(userPool.options.MfaConfiguration).toEqual("ON");
});
});
});
25 changes: 17 additions & 8 deletions src/services/userPoolService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ export type UserPool = UserPoolType & {
};

export interface UserPoolService {
readonly config: UserPool;
readonly options: UserPool;

saveAppClient(ctx: Context, appClient: AppClient): Promise<void>;
deleteAppClient(ctx: Context, appClient: AppClient): Promise<void>;
Expand All @@ -150,7 +150,7 @@ export interface UserPoolService {
): Promise<User | null>;
listGroups(ctx: Context): Promise<readonly Group[]>;
listUsers(ctx: Context): Promise<readonly User[]>;
update(ctx: Context, userPool: UserPool): Promise<void>;
updateOptions(ctx: Context, userPool: UserPool): Promise<void>;
removeUserFromGroup(ctx: Context, group: Group, user: User): Promise<void>;
saveGroup(ctx: Context, group: Group): Promise<void>;
saveUser(ctx: Context, user: User): Promise<void>;
Expand All @@ -174,7 +174,11 @@ export class UserPoolServiceImpl implements UserPoolService {
private readonly clock: Clock;
private readonly dataStore: DataStore;

public readonly config: UserPool;
private _options: UserPool;

public get options(): UserPool {
return this._options;
}

public constructor(
clientsDataStore: DataStore,
Expand All @@ -183,7 +187,7 @@ export class UserPoolServiceImpl implements UserPoolService {
config: UserPool
) {
this.clientsDataStore = clientsDataStore;
this.config = config;
this._options = config;
this.clock = clock;
this.dataStore = dataStore;
}
Expand Down Expand Up @@ -245,9 +249,10 @@ export class UserPoolServiceImpl implements UserPoolService {
): Promise<User | null> {
ctx.logger.debug({ username }, "UserPoolServiceImpl.getUserByUsername");

const aliasEmailEnabled = this.config.UsernameAttributes?.includes("email");
const aliasEmailEnabled =
this.options.UsernameAttributes?.includes("email");
const aliasPhoneNumberEnabled =
this.config.UsernameAttributes?.includes("phone_number");
this.options.UsernameAttributes?.includes("phone_number");

const userByUsername = await this.dataStore.get<User>(ctx, [
"Users",
Expand Down Expand Up @@ -315,9 +320,13 @@ export class UserPoolServiceImpl implements UserPoolService {
return Object.values(users);
}

public async update(ctx: Context, userPool: UserPool): Promise<void> {
ctx.logger.debug({ userPoolId: userPool.Id }, "UserPoolServiceImpl.update");
public async updateOptions(ctx: Context, userPool: UserPool): Promise<void> {
ctx.logger.debug(
{ userPoolId: userPool.Id },
"UserPoolServiceImpl.updateOptions"
);
await this.dataStore.set(ctx, "Options", userPool);
this._options = userPool;
}

public async saveUser(ctx: Context, user: User): Promise<void> {
Expand Down
2 changes: 1 addition & 1 deletion src/targets/adminCreateUser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ const deliverWelcomeMessage = async (
ctx,
"AdminCreateUser",
null,
userPool.config.Id,
userPool.options.Id,
user,
temporaryPassword,
req.ClientMetadata,
Expand Down
6 changes: 3 additions & 3 deletions src/targets/adminInitiateAuth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ const adminUserPasswordAuthFlow = async (
user = await services.triggers.userMigration(ctx, {
clientMetadata: {},
validationData: {},
userPoolId: userPool.config.Id,
userPoolId: userPool.options.Id,
clientId: req.ClientId,
username: req.AuthParameters.USERNAME,
password: req.AuthParameters.PASSWORD,
Expand All @@ -74,7 +74,7 @@ const adminUserPasswordAuthFlow = async (
ctx,
user,
req.ClientId,
userPool.config.Id,
userPool.options.Id,
req.ClientMetadata,
"Authentication"
);
Expand Down Expand Up @@ -127,7 +127,7 @@ const refreshTokenAuthFlow = async (
ctx,
user,
req.ClientId,
userPool.config.Id,
userPool.options.Id,
req.ClientMetadata,
"RefreshTokens"
);
Expand Down
12 changes: 6 additions & 6 deletions src/targets/adminUpdateUserAttributes.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ describe("AdminUpdateUserAttributes target", () => {
const user = TDB.user();

mockUserPoolService.getUserByUsername.mockResolvedValue(user);
mockUserPoolService.config.SchemaAttributes = [
mockUserPoolService.options.SchemaAttributes = [
{
Name: "custom:example",
Mutable: true,
Expand Down Expand Up @@ -85,7 +85,7 @@ describe("AdminUpdateUserAttributes target", () => {
${"phone_number_verified without an phone_number attribute"} | ${"phone_number_verified"} | ${"Phone Number is required to verify/un-verify a phone number"}
`("req.UserAttributes contains $desc", ({ attribute, expectedError }) => {
beforeEach(() => {
mockUserPoolService.config.SchemaAttributes = [
mockUserPoolService.options.SchemaAttributes = [
{
Name: "email_verified",
Mutable: true,
Expand Down Expand Up @@ -149,7 +149,7 @@ describe("AdminUpdateUserAttributes target", () => {

describe("user pool has auto verified attributes enabled", () => {
beforeEach(() => {
mockUserPoolService.config.AutoVerifiedAttributes = ["email"];
mockUserPoolService.options.AutoVerifiedAttributes = ["email"];
});

describe.each`
Expand All @@ -167,7 +167,7 @@ describe("AdminUpdateUserAttributes target", () => {
});

mockUserPoolService.getUserByUsername.mockResolvedValue(user);
mockUserPoolService.config.SchemaAttributes = [
mockUserPoolService.options.SchemaAttributes = [
{ Name: "example", Mutable: true },
];

Expand Down Expand Up @@ -254,7 +254,7 @@ describe("AdminUpdateUserAttributes target", () => {

describe("user pool does not have auto verified attributes", () => {
beforeEach(() => {
mockUserPoolService.config.AutoVerifiedAttributes = [];
mockUserPoolService.options.AutoVerifiedAttributes = [];
});

describe.each`
Expand All @@ -272,7 +272,7 @@ describe("AdminUpdateUserAttributes target", () => {
});

mockUserPoolService.getUserByUsername.mockResolvedValue(user);
mockUserPoolService.config.SchemaAttributes = [
mockUserPoolService.options.SchemaAttributes = [
{ Name: "example", Mutable: true },
];

Expand Down
8 changes: 4 additions & 4 deletions src/targets/adminUpdateUserAttributes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ const sendAttributeVerificationCode = async (
code: string
) => {
const deliveryDetails = selectAppropriateDeliveryMethod(
userPool.config.AutoVerifiedAttributes ?? [],
userPool.options.AutoVerifiedAttributes ?? [],
user
);
if (!deliveryDetails) {
Expand All @@ -39,7 +39,7 @@ const sendAttributeVerificationCode = async (
ctx,
"UpdateUserAttribute",
null,
userPool.config.Id,
userPool.options.Id,
user,
code,
req.ClientMetadata,
Expand Down Expand Up @@ -78,7 +78,7 @@ export const AdminUpdateUserAttributes =
// or before we started explicitly saving the defaults. Fallback on the AWS defaults in
// this case, otherwise checks against the schema for default attributes like email will
// fail.
userPool.config.SchemaAttributes ??
userPool.options.SchemaAttributes ??
USER_POOL_AWS_DEFAULTS.SchemaAttributes ??
[]
)
Expand All @@ -95,7 +95,7 @@ export const AdminUpdateUserAttributes =
// deliberately only check the affected user attributes, not the combined attributes
// e.g. a user with email_verified=false that you don't touch the email attributes won't get notified
if (
userPool.config.AutoVerifiedAttributes?.length &&
userPool.options.AutoVerifiedAttributes?.length &&
hasUnverifiedContactAttributes(userAttributesToSet)
) {
const code = otp();
Expand Down
2 changes: 1 addition & 1 deletion src/targets/confirmForgotPassword.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ export const ConfirmForgotPassword =
clientMetadata: req.ClientMetadata,
source: "PostConfirmation_ConfirmForgotPassword",
username: updatedUser.Username,
userPoolId: userPool.config.Id,
userPoolId: userPool.options.Id,

// not sure whether this is a one off for PostConfirmation, or whether we should be adding cognito:user_status
// into every place we send attributes to lambdas
Expand Down
2 changes: 1 addition & 1 deletion src/targets/confirmSignUp.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ export const ConfirmSignUp =
clientMetadata: req.ClientMetadata,
source: "PostConfirmation_ConfirmSignUp",
username: updatedUser.Username,
userPoolId: userPool.config.Id,
userPoolId: userPool.options.Id,

// not sure whether this is a one off for PostConfirmation, or whether we should be adding cognito:user_status
// into every place we send attributes to lambdas
Expand Down
1 change: 0 additions & 1 deletion src/targets/createUserPoolClient.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import { newMockCognitoService } from "../__tests__/mockCognitoService";
import { newMockUserPoolService } from "../__tests__/mockUserPoolService";
import { TestContext } from "../__tests__/testContext";
import { UserPoolService } from "../services";
import { AppClient } from "../services/appClient";
import {
CreateUserPoolClient,
CreateUserPoolClientTarget,
Expand Down
2 changes: 1 addition & 1 deletion src/targets/deleteUserPool.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ export const DeleteUserPool =
throw new ResourceNotFoundError();
}

await cognito.deleteUserPool(ctx, userPool.config);
await cognito.deleteUserPool(ctx, userPool.options);

return {};
};
2 changes: 1 addition & 1 deletion src/targets/describeUserPool.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,6 @@ export const DescribeUserPool =
}

return {
UserPool: userPoolToResponseObject(userPool.config),
UserPool: userPoolToResponseObject(userPool.options),
};
};
2 changes: 1 addition & 1 deletion src/targets/forgotPassword.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ export const ForgotPassword =
ctx,
"ForgotPassword",
req.ClientId,
userPool.config.Id,
userPool.options.Id,
user,
code,
req.ClientMetadata,
Expand Down
4 changes: 2 additions & 2 deletions src/targets/getUserAttributeVerificationCode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ const sendAttributeVerificationCode = async (
code: string
) => {
const deliveryDetails = selectAppropriateDeliveryMethod(
userPool.config.AutoVerifiedAttributes ?? [],
userPool.options.AutoVerifiedAttributes ?? [],
user
);
if (!deliveryDetails) {
Expand All @@ -34,7 +34,7 @@ const sendAttributeVerificationCode = async (
ctx,
"VerifyUserAttribute",
null,
userPool.config.Id,
userPool.options.Id,
user,
code,
req.ClientMetadata,
Expand Down
Loading

0 comments on commit dc2b10e

Please sign in to comment.