feat: run PreAuthentication at managed login - #1177
Conversation
A local user signing in at the hosted domain ran no authentication trigger, where the same user signing in through `InitiateAuth` ran both, so a handler that logs or blocks a sign-in passed its API tests and waved every managed login sign-in through. `PreAuthentication` now runs there under the `PreAuthentication_Authentication` source real Cognito reports from `/login`, for a password and for a passkey alike. It runs once the user is known and before the password is checked, which is where the API sign-ins run it, so a sign-in the pool is about to refuse reaches the handler too. A passkey runs it on the request that asks for the credential rather than on the one presenting it, matching the `USER_AUTH` sign-in, and only an asked request holds a session to present against, so it runs once either way. A browser coming back on the managed login session it already holds runs nothing, which is what the trigger's own docs say of a renewed session. A handler that throws is drawn on the sign-in form the way a wrong password is. `PostAuthentication` stays unfired and the docs say why. Resolves #1173
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
Included review availability: 0 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 2 reviews per hour. 📝 WalkthroughWalkthroughHosted managed-login password and passkey sign-ins now run ChangesManaged-login authentication triggers
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to This change runs the pre-authentication handler for managed password and passkey sign-ins while preserving existing session behavior; no actionable merge-blocking risk remains after normal checks and review. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Full details: Description checkExplanation The description explains the behavior, trigger source, password and passkey flows, renewal behavior, handler errors, documentation, fixture change, and linked issue. It does not include the template checkboxes, but the substantive description is complete. Full details: Linked Issues checkExplanation The changes satisfy issue Full details: Docstring CoverageExplanation Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 6 files. (1 skipped: 1 unsupported.) ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
@coderabbitai full review |
✅ Action performedFull review finished. |
1 participant
A local user signing in at the hosted domain ran no authentication trigger, where the same user signing in through
InitiateAuthran both, so a handler that logs or blocks a sign-in passed its API tests and waved every managed login sign-in through.PreAuthenticationnow runs there under thePreAuthentication_Authenticationsource real Cognito reports from/login, for a password and for a passkey alike. It runs once the user is known and before the password is checked, which is where the API sign-ins run it, so a sign-in the pool is about to refuse reaches the handler too. A passkey runs it on the request that asks for the credential rather than on the one presenting it, matching theUSER_AUTHsign-in, and only an asked request holds a session to present against, so it runs once either way. A browser coming back on the managed login session it already holds runs nothing, which is what the trigger's own docs say of a renewed session. A handler that throws is drawn on the sign-in form the way a wrong password is.PostAuthenticationstays unfired and the docs say why.One thing worth knowing about the test fixture.
simCognitoWithHostedPasskeysets the pool'sSignInPolicywithUpdateUserPool, and an update replaces a pool's settings whole, so the request was taking the pool'sLambdaConfigback off it. That is real Cognito behaviour rather than a simulator bug, and the fixture now carries the triggers through the update with a comment saying why.Resolves #1173
Summary by CodeRabbit