-
Notifications
You must be signed in to change notification settings - Fork 14
AMM-118: Add time-based account lockout with auto-unlock #75
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
base: main
Are you sure you want to change the base?
AMM-118: Add time-based account lockout with auto-unlock #75
Conversation
📝 WalkthroughWalkthroughA database migration script adds a new Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/main/resources/db/migration/dbiemr/V35__AMM118_Account_Lockout_Timestamp.sql (1)
1-2: Consider adding an index onlock_timestampfor query performance.The migration adds the column correctly with proper idempotency (
IF NOT EXISTS). However, if the application needs to query for locked or expired accounts (e.g.,SELECT FROM m_user WHERE lock_timestamp < NOW()), the lack of an index could cause full table scans and performance degradation on larger datasets.📋 Proposed enhancement to add an index
ALTER TABLE m_user ADD COLUMN IF NOT EXISTS lock_timestamp TIMESTAMP NULL DEFAULT NULL; + +CREATE INDEX IF NOT EXISTS idx_m_user_lock_timestamp +ON m_user(lock_timestamp);Only add the index if the application actually queries by
lock_timestamp.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/resources/db/migration/dbiemr/V35__AMM118_Account_Lockout_Timestamp.sql
🔇 Additional comments (1)
src/main/resources/db/migration/dbiemr/V35__AMM118_Account_Lockout_Timestamp.sql (1)
1-2: Verify that auto-unlock logic is implemented in application code.The PR description mentions an "auto-unlock" feature, but this migration only adds the storage column. Ensure that the corresponding application code (job scheduler, service layer, or middleware) implements the logic to automatically unlock accounts when the lockout period expires.



Fixes PSMRI/AMRIT#118
📋 Description
Adds database migration to support time-based account lockout feature. Creates
lock_timestampcolumn inm_usertable to track when accounts were locked due to failed login attempts.✅ Type of Change
ℹ️ Additional Information
Changes:
lock_timestampTIMESTAMP column (nullable, default NULL)Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.