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

Add User Migration trigger during amplify add auth? #4577

Open
jqdurham opened this issue Jun 16, 2020 · 20 comments
Open

Add User Migration trigger during amplify add auth? #4577

jqdurham opened this issue Jun 16, 2020 · 20 comments
Labels
auth Issues tied to the auth category of the CLI feature-request Request a new feature p3

Comments

@jqdurham
Copy link

Note: If your question is regarding the AWS Amplify Console service, please log it in the
official AWS Amplify Console forum

Which Category is your question related to?
auth

Amplify CLI Version
4.21.2

You can use amplify -v to check the amplify cli version on your system

What AWS Services are you utilizing?
Cognito, Lambda

Provide additional details e.g. code snippets

How do I configure a User Migration trigger during the setup of a user pool via Amplify? I'm able to create many other triggers (PostAuthentication, PreSignUp, etc.) but not UserMigration.

@attilah
Copy link
Contributor

attilah commented Jun 16, 2020

@jqdurham the user migration trigger is not supported today by the CLI. Since user migration is always requires custom code and custom logic, what the CLI could do is generate a placeholder Lambda function and wire it up to the user pool.

Would that satisfy your requirements?

@attilah attilah added auth Issues tied to the auth category of the CLI feature-request Request a new feature enhancement and removed feature-request Request a new feature labels Jun 16, 2020
@jqdurham
Copy link
Author

@jqdurham the user migration trigger is not supported today by the CLI. Since user migration is always requires custom code and custom logic, what the CLI could do is generate a placeholder Lambda function and wire it up to the user pool.

Would that satisfy your requirements?

Indeed it would.

@hisham
Copy link
Contributor

hisham commented Jun 27, 2020

From my experience pretty much all cognito triggers require custom logic. And I wish amplify would just set me up with one lambda to handle all cognito triggers instead of doing one lambda per trigger, juggling multiple lambdas and navigating the nested questions of amplify auth update is time consuming. Just some feedback. Perhaps I will get the big picture as amplify cli’s cognito triggers functionality matures more.

To support user migration trigger for now i piggy backed on the pre token lambda trigger and made that the user migration trigger as well by modifying the auth cloud formation.

@dukuo
Copy link

dukuo commented Jul 19, 2020

Just to be clear: Is is possible to at least create a dummy scaffolding of a lambda function and automatically assign it to the user migration trigger of the Cognito User Pool created with amplify add auth? From what I've seen so far it is possible to manually go through the AWS resources and wire everything up myself but that would be the Amplify way of doing things as far as I'm concerned.

@houmark
Copy link

houmark commented Jul 25, 2020

Status on this @attilah? Just adding a placeholder function for user migration would be nice.

Making it possible to select an existing function for each trigger would be nice, so that it's possible to combine similar logic triggers in one function, as others have mentioned.

If the documentation have example code for each trigger, a link to the docs can just be added in the CLI once you combine functions and this way the developer can copy and paste in the start code if they wish. I guess the reason why the CLI wants to create a new function for each trigger is because it wants to inject the template lambda code, but I'd rather do that manually and have less functions.

@ianmartorell
Copy link

+1 to adding a placeholder lambda function for the UserMigration trigger through the CLI. Aren't all other triggers basically placeholder functions as well?

@houmark Adding existing functions as triggers would be nice, but wouldn't you then have to manually check the event.triggerSource instead of just assuming the function is running for that trigger?

Something I don't quite understand is why the placeholder lambdas have an index.js that loops through the modules parameter. Why not just add the code to index.js? It seems like Amplify is promoting using one lambda for many purposes, but I can't even change the modules manually or the CLI will recreate the custom.js file. And with the generated index.js, when the first module calls callback the lambda will finish anyway. 😕 It all seems a bit strange to me.

@houmark
Copy link

houmark commented Jul 28, 2020

Yeah, I don't understand the modules looping either, it's just over-complexifying stuff that should be a simple boiler template and if it is, thus easy to understand for the end developer, and then the default file created could include some sample code or commented information about triggerSource and a link to documentation about it for Cognito triggers.

Personally I'd prefer using ONE function for all triggers, and then switch the functionality based on the triggerSource. Lambda Layers is still crazy buggy, and since many of these functions will do the same, the overhead in redundant code by creating multiple functions is kinda annoying. Having the option to choose an existing function would be nice also, but I could live with any implementation as long as all triggers are possible to add.

Currently, I'm doing it totally manual. I created a normal function and then in the Cognito UI, I have manually selected the function. This has the risk of the UI being overwritten by a push or a configuration being forgotten, but one gotta do what one gotta do to make stuff work.

How some triggers were added and others left out I don't understand, but it seems typical with doing things half-ass in Amplify land.

@jqdurham
Copy link
Author

jqdurham commented Jul 28, 2020 via email

@houmark
Copy link

houmark commented Jul 28, 2020

For what it's worth, I don't understand the desire to have a single function handle multiple events. Do one thing and do it well. Lambdas are no different.

Well, we have user migration, pre and post authentication, and pre and post signup events which does similar stuff using AppSync lookups in our database for example. The code required to run these AppSync requests are the same, and by having 6 functions, we have that same code 6 places vs. 1 place if they all resided in the same function.

If Lamda Layers were working well, we could have that shared code in one Layer and multiple functions would be cool, but since Lambda Layers seems to be very very buggy by looking at the number of bug reports and severity of bugs, we have not yet started using that feature.

Another reason to keep it as one function is warm up time. If one function is utilized more often, there's less chance it'll go cold and has to start up which adds to the execution time and memoization of for example lookups to the Parameter Store would also be utilized more.

Recently a change to functions and their access in Amplify was changed and broke for some existing users, requiring a manual update to access for all functions. That was a manual task, which becomes tedious and takes more time, by having to update more functions.

Need more? ;)

@jqdurham
Copy link
Author

jqdurham commented Nov 5, 2020

We have internal packages that handle common operations across our lambdas. We can reuse code without having to keep the code that differentiates the lambdas in a single file. It feels weird to still be preaching SOLID.

We're back considering Amplify again even though we know we'll have to abandon the CLI after the initial setup, unless this can somehow magically materialize in the next week or so.

@ianmartorell
Copy link

ianmartorell commented Nov 8, 2020

@jqdurham What I did to create a UserMigration trigger was creating another trigger that the CLI currently supports, and then I manually renamed all the files and newly added references to the trigger in all CloudFormation templates. It's not that much of a hassle and it gets recognized properly by the Amplify CLI afterwards. The function gets assigned to the trigger in Cognito and all that.

@madakaheri
Copy link

@ianmartorell I thought it was a good idea, but when I auth update again, the generated resources disappeared.

@siegerts siegerts added feature-request Request a new feature and removed enhancement labels Sep 3, 2021
@houmark
Copy link

houmark commented Nov 24, 2021

Status on this one @siegerts? The UserMigration trigger seems to be the only Cognito trigger not supported in the CLI and it seems trivial to add it around here: https://github.com/aws-amplify/amplify-cli/tree/master/packages/amplify-category-auth/provider-utils/awscloudformation/triggers

@pikanji
Copy link

pikanji commented Nov 24, 2021

I have automated adding UserMigration trigger by committing manual modification to auth-trigger-cloudformation-template.json, and it had been working. However, since CLI version 7, this modification is removed and I couldn't find any workaround. Is there any way to set up UserMigration trigger automatically for CI? Because of this, I cannot upgrade Amplify CLI to version 7.

@juniarz
Copy link
Contributor

juniarz commented Nov 24, 2021

CI? Because of this, I cannot upgrade Amplify CLI to version 7.

+1

@hanachan1026
Copy link

Because of this, I cannot upgrade Amplify CLI to version 7.

+1

@pikanji
Copy link

pikanji commented Dec 23, 2021

@hanachan1026 found out the way to solve my problem.
Using custom category (introduced in v7?), we could successfully let amplify set up UserMigration trigger with our custom cloud formation template. We added custom category and moved our customization to auth-trigger-cloudformation-template.json to the template file under custom category directory.

@josefaidt josefaidt added the p3 label Mar 14, 2022
@josefaidt
Copy link
Contributor

note: it appears there are two items here:

  • look into simplifying function logic, using a single Lambda based on comments above
  • add UserMigration trigger support with the auth category

@CrshOverride
Copy link

@pikanji How were you able to create the trigger via a custom resource when the User Pool and Lambda functions are managed outside of the Custom stack?

I just tried the same thing myself and found that cognito.UserPool.fromUserPoolArn returns IUserPool which doesn't actually have the addTrigger method (even tried forcing a cast which failed as well).

@josefaidt Any idea of when this might be supported?

@brinehart
Copy link

brinehart commented Nov 26, 2024

Will this ever be added to Amplify? This has been pending since 2020 and we still could really use it. It's kind of annoying to have to manually re-assign a lifecycle trigger every time we change something in Amplify.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auth Issues tied to the auth category of the CLI feature-request Request a new feature p3
Projects
None yet
Development

No branches or pull requests