Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: include only required userAttributes and generate identityPoolName in backend file #13994

Merged
merged 2 commits into from
Oct 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion packages/amplify-gen1-codegen-auth-adapter/API.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ export interface AuthSynthesizerOptions {
// (undocumented)
identityGroups?: GroupType[];
// (undocumented)
identityPoolName?: string;
// (undocumented)
identityProviders?: ProviderDescription[];
// (undocumented)
identityProvidersDetails?: IdentityProviderType[];
Expand All @@ -48,7 +50,7 @@ export interface AuthTriggerConnection {
export type AuthTriggerConnectionSourceMap = Partial<Record<keyof LambdaConfigType, string>>;

// @public (undocumented)
export const getAuthDefinition: ({ userPool, identityProviders, identityProvidersDetails, identityGroups, webClient, authTriggerConnections, guestLogin, mfaConfig, totpConfig, }: AuthSynthesizerOptions) => AuthDefinition;
export const getAuthDefinition: ({ userPool, identityPoolName, identityProviders, identityProvidersDetails, identityGroups, webClient, authTriggerConnections, guestLogin, mfaConfig, totpConfig, }: AuthSynthesizerOptions) => AuthDefinition;

// (No @packageDocumentation comment for this package)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -600,7 +600,7 @@ void describe('auth codegen', () => {
const result = getAuthDefinition({
userPool: { Name: 'test' },
});
assert.deepEqual(result.userPoolOverrides, { userPoolName: 'test', usernameAttributes: [] });
assert.deepEqual(result.userPoolOverrides, { userPoolName: 'test', usernameAttributes: undefined });
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ export type AuthTriggerConnectionSourceMap = Partial<Record<keyof LambdaConfigTy

export interface AuthSynthesizerOptions {
userPool: UserPoolType;
identityPoolName?: string;
identityProviders?: ProviderDescription[];
identityProvidersDetails?: IdentityProviderType[];
identityGroups?: GroupType[];
Expand Down Expand Up @@ -80,7 +81,7 @@ const getUserPoolOverrides = (userPool: UserPoolType): Partial<PolicyOverrides>
Object.assign(userPoolOverrides, userNamePolicy);
}
if (userPool.UsernameAttributes === undefined || userPool.UsernameAttributes.length === 0) {
userPoolOverrides.usernameAttributes = [];
userPoolOverrides.usernameAttributes = undefined;
}
return userPoolOverrides;
};
Expand Down Expand Up @@ -118,7 +119,7 @@ const getStandardUserAttributes = (
required: attribute.Required,
mutable: attribute.Mutable,
};
if (attribute.Name !== undefined && attribute.Name in mappedUserAttributeName) {
if (attribute.Name !== undefined && attribute.Name in mappedUserAttributeName && attribute.Required) {
return {
...standardAttributes,
[mappedUserAttributeName[attribute.Name as keyof typeof mappedUserAttributeName] as Attribute]: standardAttribute,
Expand Down Expand Up @@ -235,6 +236,7 @@ function filterAttributeMapping(
*/
export const getAuthDefinition = ({
userPool,
identityPoolName,
identityProviders,
identityProvidersDetails,
identityGroups,
Expand Down Expand Up @@ -354,6 +356,7 @@ export const getAuthDefinition = ({
userPoolOverrides,
lambdaTriggers: getAuthTriggers(userPool.LambdaConfig ?? {}, authTriggerConnections ?? {}),
guestLogin,
identityPoolName,
oAuthFlows: webClient?.AllowedOAuthFlows,
readAttributes: webClient?.ReadAttributes,
writeAttributes: webClient?.WriteAttributes,
Expand Down
2 changes: 2 additions & 0 deletions packages/amplify-gen2-codegen/API.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ export interface AuthDefinition {
// (undocumented)
guestLogin?: boolean;
// (undocumented)
identityPoolName?: string;
// (undocumented)
lambdaTriggers?: Partial<AuthLambdaTriggers>;
// (undocumented)
loginOptions?: LoginOptions;
Expand Down
1 change: 1 addition & 0 deletions packages/amplify-gen2-codegen/src/auth/source_builder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,7 @@ export interface AuthDefinition {
userPoolOverrides?: PolicyOverrides;
lambdaTriggers?: Partial<AuthLambdaTriggers>;
guestLogin?: boolean;
identityPoolName?: string;
oAuthFlows?: string[];
readAttributes?: string[];
writeAttributes?: string[];
Expand Down
30 changes: 29 additions & 1 deletion packages/amplify-gen2-codegen/src/backend/synthesizer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ describe('BackendRenderer', () => {
'Policies.PasswordPolicy.RequireUppercase': false,
'Policies.PasswordPolicy.TemporaryPasswordValidityDays': 10,
userPoolName: 'Test_Name',
userNameAttributes: undefined,
};
const mappedPolicyType: Record<string, string> = {
MinimumLength: 'minimumLength',
Expand Down Expand Up @@ -58,7 +59,9 @@ describe('BackendRenderer', () => {
} else if (typeof value === 'boolean') {
assert(output.includes(`cfnUserPool.policies = {passwordPolicy:{${policyKey}:${value}}}`));
}
} else if (value) {
} else if (value === undefined) {
assert(output.includes(`cfnUserPool.${key} = ${value}`));
} else {
assert(output.includes(`cfnUserPool.${key} = "${value}"`));
}
});
Expand Down Expand Up @@ -113,6 +116,31 @@ describe('BackendRenderer', () => {
assert(!output.includes('cfnIdentityPool'));
});
});
describe('Identity Pool Name', () => {
it('Renders cfnIdentityPool accessor if identityPoolName is defined', () => {
const renderer = new BackendSynthesizer();
const rendered = renderer.render({
auth: {
importFrom: './auth/resource.ts',
identityPoolName: 'Test_Name',
guestLogin: true,
},
});
const output = printNodeArray(rendered);
assert(output.includes('cfnIdentityPool'));
});
it('Does not render cfnIdentityPool accessor if identityPoolName is undefined and guestLogin is true', () => {
const renderer = new BackendSynthesizer();
const rendered = renderer.render({
auth: {
importFrom: './auth/resource.ts',
guestLogin: true,
},
});
const output = printNodeArray(rendered);
assert(!output.includes('cfnIdentityPool'));
});
});
describe('OAuth Flows', () => {
it('Renders cfnUserPoolClient accessor if oAuthFlows is defined', () => {
const renderer = new BackendSynthesizer();
Expand Down
20 changes: 15 additions & 5 deletions packages/amplify-gen2-codegen/src/backend/synthesizer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ export interface BackendRenderParameters {
importFrom: string;
userPoolOverrides?: PolicyOverrides;
guestLogin?: boolean;
identityPoolName?: string;
oAuthFlows?: string[];
readAttributes?: string[];
writeAttributes?: string[];
Expand Down Expand Up @@ -83,15 +84,15 @@ export class BackendSynthesizer {
private setPropertyValue(
objectIdentifier: Identifier,
propertyPath: string,
value: number | string | boolean | string[] | object,
value: number | string | boolean | string[] | object | undefined,
): ExpressionStatement {
const propertyAccessExpression = this.createPropertyAccessExpression(objectIdentifier, propertyPath);
const overrideValue = this.getOverrideValue(value);

return factory.createExpressionStatement(factory.createAssignment(propertyAccessExpression, overrideValue));
}

private getOverrideValue(value: number | string | boolean | string[] | object): Expression {
private getOverrideValue(value: number | string | boolean | string[] | object | undefined): Expression {
if (typeof value === 'number') {
return factory.createNumericLiteral(value);
} else if (typeof value === 'string') {
Expand All @@ -107,6 +108,8 @@ export class BackendSynthesizer {
properties.push(property);
}
return factory.createObjectLiteralExpression(properties, true);
} else if (value === undefined) {
return factory.createIdentifier('undefined');
}
throw new TypeError(`Unrecognized type: ${typeof value}`);
}
Expand Down Expand Up @@ -217,7 +220,7 @@ export class BackendSynthesizer {
if (value !== undefined && policyKey in mappedPolicyType) {
policies.passwordPolicy[mappedPolicyType[policyKey] as string] = value;
}
} else if (value) {
} else {
nodes.push(this.setPropertyValue(factory.createIdentifier('cfnUserPool'), overridePath, value));
}
}
Expand All @@ -230,12 +233,19 @@ export class BackendSynthesizer {
);
}

if (renderArgs.auth?.guestLogin === false) {
if (renderArgs.auth?.guestLogin === false || renderArgs.auth?.identityPoolName) {
const cfnIdentityPoolVariableStatement = this.createVariableStatement(
this.createVariableDeclaration('cfnIdentityPool', 'auth.resources.cfnResources.cfnIdentityPool'),
);
nodes.push(cfnIdentityPoolVariableStatement);
nodes.push(this.setPropertyValue(factory.createIdentifier('cfnIdentityPool'), 'allowUnauthenticatedIdentities', false));
if (renderArgs.auth?.identityPoolName) {
nodes.push(
this.setPropertyValue(factory.createIdentifier('cfnIdentityPool'), 'identityPoolName', renderArgs.auth.identityPoolName),
);
}
if (renderArgs.auth?.guestLogin === false) {
nodes.push(this.setPropertyValue(factory.createIdentifier('cfnIdentityPool'), 'allowUnauthenticatedIdentities', false));
}
}

if (renderArgs.auth?.oAuthFlows || renderArgs.auth?.readAttributes || renderArgs.auth?.writeAttributes) {
Expand Down
1 change: 1 addition & 0 deletions packages/amplify-gen2-codegen/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,7 @@ export const createGen2Renderer = ({
importFrom: './auth/resource',
userPoolOverrides: auth?.userPoolOverrides,
guestLogin: auth?.guestLogin,
identityPoolName: auth?.identityPoolName,
oAuthFlows: auth?.oAuthFlows,
readAttributes: auth?.readAttributes,
writeAttributes: auth?.writeAttributes,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ export class AppAuthDefinitionFetcher {
}),
);

const { AllowUnauthenticatedIdentities: guestLogin } = await this.cognitoIdentityPoolClient.send(
const { AllowUnauthenticatedIdentities: guestLogin, IdentityPoolName: identityPoolName } = await this.cognitoIdentityPoolClient.send(
new DescribeIdentityPoolCommand({
IdentityPoolId: resourcesByLogicalId['IdentityPool'].PhysicalResourceId,
}),
Expand All @@ -95,6 +95,7 @@ export class AppAuthDefinitionFetcher {
assert(userPool, 'User pool not found');
return getAuthDefinition({
userPool,
identityPoolName,
identityProviders,
identityProvidersDetails,
identityGroups,
Expand Down
Loading