aws-amplify / amplify-backend

Home to all tools related to Amplify's code-first DX (Gen 2) for building fullstack apps on AWS
Apache License 2.0
186 stars 64 forks source link

[Amplify Gen2] please add a `custom sender lambda trigger` as a defineAuth's attribute #1607

Open ggj0418 opened 5 months ago

ggj0418 commented 5 months ago

Environment information

System:
  OS: Windows 11 10.0.22631
  CPU: (22) x64 Intel(R) Core(TM) Ultra 7 155H
  Memory: 11.66 GB / 31.53 GB
Binaries:
  Node: 20.11.0 - C:\Program Files\nodejs\node.EXE
  Yarn: 1.22.21 - ~\AppData\Roaming\npm\yarn.CMD
  npm: 10.8.0 - ~\WebstormProjects\amplify-next-pages-template\node_modules\.bin\npm.CMD
  pnpm: undefined - undefined
NPM Packages:
  @aws-amplify/backend: 1.0.2
  @aws-amplify/backend-cli: 1.0.3
  aws-amplify: 6.2.0
  aws-cdk: 2.140.0
  aws-cdk-lib: 2.140.0
  typescript: 5.4.5
AWS environment variables:
  AWS_NODEJS_CONNECTION_REUSE_ENABLED = 1
  AWS_SDK_LOAD_CONFIG = 1
  AWS_STS_REGIONAL_ENDPOINTS = regional
No CDK environment variables

Description

these values are the defineAuth method's option attributes.

triggers?: Partial<Record<'createAuthChallenge' | 'customMessage' | 'defineAuthChallenge' | 'postAuthentication' | 'postConfirmation' | 'preAuthentication' | 'preSignUp' | 'preTokenGeneration' | 'userMigration' | 'verifyAuthChallengeResponse', ConstructFactory<...>>> | undefined

there is no custom sender lambda value.

To reach that requirement, i had to custom the userPool on a phase of amplify backend cdk.

The sample code is here.

// backend.ts
const backend = defineBackend({
  sendVerificationCodeFunction,
  auth,
  data,
  storage
})

const { cfnUserPool } = backend.auth.resources.cfnResources
const existedSendVerificationCodeFunction = backend.sendVerificationCodeFunction.resources.lambda
existedSendVerificationCodeFunction.grantInvoke(
  new aws_iam.ServicePrincipal('cognito-idp.amazonaws.com')
)
const key = aws_kms.Key.fromKeyArn(
  cfnUserPool,
  `${KeyId}`,
  `${KeyArn}`
)
key.grantDecrypt(existedSendVerificationCodeFunction)

cfnUserPool.addPropertyOverride('LambdaConfig', {
  CustomSMSSender: {
    LambdaArn: existedSendVerificationCodeFunction.functionArn,
    LambdaVersion: 'V1_0'
  },
  KMSKeyID: key.keyArn
})

and this is the function's handler code

// handler.ts
export const handler = async (event: CustomSMSSenderTriggerEvent) => {
  try {
    const { decrypt } = buildClient(CommitmentPolicy.REQUIRE_ENCRYPT_ALLOW_DECRYPT)
    const generatorKeyId: string = `${KeyAlias}`
    const keyIds: string[] = [`${KeyArn}`]

    const keyringInput: KmsKeyringNodeInput = { generatorKeyId, keyIds }
    const keyring = new KmsKeyringNode(keyringInput)

    const request = parseEvent(event)
    console.log('Request:', JSON.stringify(request, null, 2))

    let plainTextCode: string | undefined
    if (request.code) {
      const { plaintext, messageHeader } = await decrypt(keyring, b64.toByteArray(request.code))
      plainTextCode = plaintext.toString('utf-8')
    }

    // do something with that plainTextCode

    return {
      statusCode: 200,
      headers: {
        'Access-Control-Allow-Origin': '*',
        'Access-Control-Allow-Headers': '*'
      },
      body: JSON.stringify({ success: true })
    }
  } catch (error) {
    console.error('Error sending message:', error)
    return {
      statusCode: 500,
      headers: {
        'Access-Control-Allow-Origin': '*',
        'Access-Control-Allow-Headers': '*'
      },
      body: JSON.stringify({ success: false })
    }
  }
}

I hope the amplify backend supply this feature officially

jamilsaadeh97 commented 3 weeks ago

I was looking into contributing this feature into the repo and I saw #2087 merged yesterday, so I would like to figure some things out before diving deeper if that's okay.

The feature introduced this new type:

export type CustomEmailSender = {
     handler: IFunction;
     kmsKeyArn?: string;
 };

If I want to implement CustomSmsSender using the same strategy as CustomEmailSender, it would have the same fields(a handler and a KMS key) but CloudFormation expects the KMS key to be inside the LambdaConfig, not a specific sender. My question is, should we have two KMS keys or the current structure needs to be refactored in order to also have CustomSmsSender trigger?

Thanks for any information

jamilsaadeh97 commented 3 weeks ago

I also find it a bit confusing that CustomEmailSender is to be defined in senders:{} instead of triggers:{} since this and CustomSmsSender are lambda triggers and defined as such in all the documentation in AWS

ggj0418 commented 1 week ago

@jamilsaadeh97

Thank you for raising this point! After reviewing the current structure and CloudFormation's requirements, I believe refactoring the KMS key configuration to reside at the LambdaConfig level would be a more scalable and maintainable solution. Here's my reasoning:

Why refactor to a shared kmsKeyArn in LambdaConfig?

  1. Alignment with CloudFormation:

    • CloudFormation already expects the kmsKeyArn to be part of the LambdaConfig, not specific to each sender. By following this convention, we can avoid potential misconfigurations and ensure compatibility with CloudFormation's structure.
  2. Simplified Configuration:

    • A shared KMS key in LambdaConfig reduces redundancy and makes it easier to manage configurations. This is especially beneficial if the key is meant to serve both CustomEmailSender and CustomSmsSender use cases.
  3. Flexibility for Future Extensions:

    • By placing the kmsKeyArn in LambdaConfig, we create a centralized configuration point. If new senders or triggers are added in the future, they can reuse the same structure without additional changes.

Proposed Changes:

Example:

Here’s an example of how the updated types might look:


export type LambdaConfig = {
    handler: IFunction;
    kmsKeyArn?: string; // Shared KMS key for all senders
};

export type CustomEmailSender = {
    handler: IFunction;
};

export type CustomSmsSender = {
    handler: IFunction;
};