-
Notifications
You must be signed in to change notification settings - Fork 0
feat: add login attempt counter #82
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
feat: add login attempt counter #82
Conversation
Change-Id: Icd5ceb7f886ffa918449c872047ce4f279ee9c81
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.
Pull Request Overview
This PR introduces a login attempt counter, exposes related session variables to the frontend, and updates backend logic to track and reset failed attempts.
- Add
max_login_failed_attemptsanduser_is_activesession values in the login view - Enhance React form to display dynamic error messages based on login attempt counts and lock status
- Persist and reset
login_failed_attemptin the user model and propagate values via the controller
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| resources/views/auth/login.blade.php | Inject new session variables (maxLoginFailedAttempts, user_is_active) into JS config |
| resources/js/login/login.js | Use loginAttempts, maxLoginFailedAttempts, and userIsActive to conditionally render errors |
| app/libs/Auth/Models/User.php | Initialize login_failed_attempt in activate() and verifyEmail() |
| app/libs/Auth/Factories/UserFactory.php | Remove unused import |
| app/Services/SecurityPolicies/LockUserCounterMeasure.php | Simplify null check on $user |
| app/Http/Controllers/UserController.php | Expose new config values (max_login_failed_attempts, user_is_active) in JSON responses |
Comments suppressed due to low confidence (4)
resources/views/auth/login.blade.php:70
- The config property uses snake_case (
user_is_active) while other properties use camelCase. Rename touserIsActivefor consistency with the rest of the JS config.
config.user_is_active = {{Session::get("user_is_active")}};
app/Http/Controllers/UserController.php:513
- The value for
user_verifiedis sometimes a boolean (true) and sometimes an integer (1). For consistency and clearer API design, use a boolean in all cases.
$response_data['user_verified'] = 1;
app/libs/Auth/Models/User.php:1857
- Setting
login_failed_attemptto 10 inactivate()may immediately lock the user again. Verify whether this should be reset to 0 instead to fully clear prior failures.
$this->login_failed_attempt = 10;
resources/js/login/login.js:819
- Server response keys are snake_case (
login_attempts,max_login_failed_attempts) but props are accessed as camelCase. This mismatch may cause undefined values; ensure prop names align with the data shape (either rename server keys or adjust prop accessors).
loginAttempts={this.props?.loginAttempts}
romanetar
left a 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.
@smarcet please review comments
app/libs/Auth/Models/User.php
Outdated
| if(!$this->active) { | ||
| $this->active = true; | ||
| $this->spam_type = self::SpamTypeHam; | ||
| $this->login_failed_attempt = 10; |
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.
Please extract this to a constant
romanetar
left a 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.
LGTM
Change-Id: Ib765702819565ec82e1bc60361aee27aa492350b
…78) * feat: propagate can_login to UI and give more feedback Signed-off-by: romanetar <[email protected]> * feat: add more feedback to UI on inactive and/or unverified accounts Signed-off-by: romanetar <[email protected]> * fix: login flow ui tweaks Signed-off-by: romanetar <[email protected]> * fix: PR review feedback Signed-off-by: romanetar <[email protected]> * feat: add login attempt counter (#82) * feat: add login attempt counter Change-Id: Icd5ceb7f886ffa918449c872047ce4f279ee9c81 * fix: remove hard coded test value Change-Id: Ib765702819565ec82e1bc60361aee27aa492350b --------- Signed-off-by: romanetar <[email protected]> Co-authored-by: sebastian marcet <[email protected]>
Change-Id: Icd5ceb7f886ffa918449c872047ce4f279ee9c81
ref https://tipit.avaza.com/project/view/376250#!tab=task-pane&task=3818781