Repository navigation
feat(auth): apply deletion_protection to the Cognito user pool - #599
Conversation
The root deletion_protection variable was not passed to the auth module, so the user pool was never protected. Every workspace is keyed by a user's Cognito sub, so a deleted pool cannot be recovered. Thread the existing variable through; the default stays false.
| module "auth" { | ||
| source = "./modules/auth" | ||
|
|
||
| deletion_protection = var.deletion_protection |
There was a problem hiding this comment.
[P2] Include Cognito in the disable-before-destroy workflow
The root deletion_protection default is true, so this wiring makes Cognito ACTIVE on ordinary/default deployments. However, scripts/destroy.sh prepares only its explicit Neptune/DynamoDB PROTECTION_TARGETS; it omits module.auth.aws_cognito_user_pool.main. The subsequent destroy with -var=deletion_protection=false does not first update the pool, and the locked AWS provider calls DeleteUserPool while protection remains ACTIVE. Both scripts/destroy.sh dev and installer-managed teardown can therefore fail partway through.
Please add the pool to the preparation targets and protected-resource coverage, preserving the existing state-membership filter for partial teardowns. Also extend scripts/inspect-terraform-plan.mjs: adding the target alone still fails because its preparation allowlist accepts only DynamoDB/Neptune boolean true→false. Narrowly allow an in-place Cognito ACTIVE→INACTIVE update with no other field changes; keep unrelated updates, creation, deletion and replacement rejected.
Regression acceptance: default-protected non-production teardown, already-INACTIVE/no-op, pool absent after partial teardown, production refusal, and unsafe preparation-plan rejection. Please also correct the PR description's no-change-at-default statement to reflect the root default.
Verified with actual-root mock plans, the unmodified destroy helper plus a recording Terraform fixture (baseline succeeds; protected state fails), and the actual inspector rejecting a clean Cognito disable plan. No cloud deletion was performed. AWS documents that active protection rejects deletion: https://docs.aws.amazon.com/cognito-user-identity-pools/latest/APIReference/API_DeleteUserPool.html.
There was a problem hiding this comment.
Thanks, you're right, and the description was wrong about the default; it has been corrected. Fixed in 92f264c:
destroy.shaddsmodule.auth.aws_cognito_user_pool.maintoPROTECTION_TARGETS, still filtered by state membership.inspect-terraform-plan.mjsaccepts an in-place Cognitodeletion_protectionchange fromACTIVEtoINACTIVEonly. Unrelated field changes, enabling, boolean values, replacement, deletion and creation stay rejected.terraform-hardening.test.mjsnow requires the protected pool in the target list.- New
release-tools.test.mjscases cover the inspector accept/reject matrix, the already-INACTIVEno-op, the pool being targeted (and cleared before destroy) when it is in state, and not being targeted when absent. Production refusal stays covered by the existing destroy tests. The three new tests fail without the script changes.
The root deletion_protection variable defaults to true, so wiring it to the user pool makes the pool ACTIVE on default deployments. destroy.sh only cleared protection on Neptune and DynamoDB, so the destroy then failed on DeleteUserPool. Add the pool to the teardown preparation targets (still filtered by state membership for partial teardowns), and let the preparation plan inspector accept an in-place Cognito deletion_protection ACTIVE -> INACTIVE update with no other change. Creation, deletion, replacement and unrelated updates stay rejected.
JWThewes
left a comment
There was a problem hiding this comment.
Thanks for contributing
Issue #, if available: None. Follow-up to #461, which added deletion protection to the data stores but not the Cognito user pool
Description of changes:
Problem
The root module already has a
deletion_protectionvariable, and other stateful resources use it, but it isn't passed to the auth module. The Cognito user pool is therefore never protected. Every workspace is keyed by a user's Cognitosub, so an accidental pool deletion or replacement can't be recovered.Solution
deletion_protectionvariable tomodules/auth, defaulting tofalse.deletion_protection = "ACTIVE"/"INACTIVE"onaws_cognito_user_pool.main.scripts/destroy.shnow includes the user pool in its protection-preparation targets, still filtered by state membership for partial teardowns.scripts/inspect-terraform-plan.mjs --deletion-protection-onlynow accepts an in-place Cognitodeletion_protectionchange fromACTIVEtoINACTIVEwith nothing else changing. Creation, deletion, replacement, enabling and unrelated updates are still rejected.Testing
release-tools.test.mjscases:ACTIVEtoINACTIVEupdate and an already-INACTIVEno-opdestroy.shtargets the pool when it's in state, and clears protection before the destroyterraform-hardening.test.mjsnow requires the teardown target list to include the protected user pool.node --testonterraform-hardening.test.mjsandrelease-tools.test.mjs: 64 tests pass. All three new tests fail without the script changes. The otherscripts/testfiles pass (39 tests).terraform fmt -check,terraform validate -no-testsand everyterraform testsuite from the Terraform workflow pass on Terraform 1.15.0.Impact
deletion_protectiondefault istrue, so deployments that keep the default will see an in-place update turning on user pool deletion protection. Deployments that set it tofalsesee no change.scripts/destroy.shand installer-managed teardown clear the pool's protection before destroying it, as they already do for Neptune and DynamoDB. A bareterraform destroyneedsdeletion_protection=falseapplied first.By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.