-
Notifications
You must be signed in to change notification settings - Fork 0
feat: when login, add passsword leaked warning for the user to change… #50
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?
Conversation
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 adds password breach detection functionality that warns users when their password has been found in data breaches using the HaveIBeenPwned API. The warning is displayed after successful login to encourage users to change compromised passwords.
- Integrates HaveIBeenPwned API to check for password breaches during login
- Displays a warning alert when a compromised password is detected
- Implements secure password checking using SHA-1 hash prefix methodology
Reviewed Changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 4 comments.
File | Description |
---|---|
resources/views/livewire/auth/login.blade.php | Adds password breach checking logic and flashes warning message |
resources/views/components/layouts/public.blade.php | Displays breach warning alert with auto-dismiss functionality |
Comments suppressed due to low confidence (1)
resources/views/livewire/auth/login.blade.php:1
- The password breach warning alert is always shown regardless of whether there's a breach warning in the session. Add a condition to only show this alert when
session('password_breach_warning')
exists.
<?php
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
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.
Remove the excessive blank lines (lines 80-83) before the isPasswordPwned
method to maintain consistent code formatting.
Copilot uses AI. Check for mistakes.
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.
Remove the excessive blank lines (lines 104-112) at the end of the class to maintain consistent code formatting.
Copilot uses AI. Check for mistakes.
foreach (explode("\n", $response->body()) as $line) { | ||
[$hashSuffix, $count] = explode(':', $line); | ||
if ($suffix === trim($hashSuffix)) { | ||
return true; | ||
} | ||
} |
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.
The code doesn't handle empty lines or malformed responses from the API. Add validation to ensure $line
contains a colon before using explode(':', $line)
to prevent potential errors.
Copilot uses AI. Check for mistakes.
|
||
|
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.
Remove the trailing whitespace on line 127 and the unnecessary blank line 128 to maintain clean formatting.
Copilot uses AI. Check for mistakes.
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
Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
if ($this->isPasswordPwned($this->password)) { | ||
// Flash a warning message after successful login | ||
session()->flash('password_breach_warning', ' Your password has appeared in a data breach. For your safety, please change it soon.'); |
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.
The error message has an extra space at the beginning. Remove the leading space before 'Your password'.
session()->flash('password_breach_warning', ' Your password has appeared in a data breach. For your safety, please change it soon.'); | |
session()->flash('password_breach_warning', 'Your password has appeared in a data breach. For your safety, please change it soon.'); |
Copilot uses AI. Check for mistakes.
protected function isPasswordPwned(string $password): bool |
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.
Inconsistent indentation. The method signature should align with other methods in the class (no leading spaces).
protected function isPasswordPwned(string $password): bool | |
protected function isPasswordPwned(string $password): bool |
Copilot uses AI. Check for mistakes.
$prefix = substr($sha1, 0, 5); | ||
$suffix = substr($sha1, 5); | ||
$response = Http::get("https://api.pwnedpasswords.com/range/{$prefix}"); |
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.
The API call is synchronous and could cause login delays if the service is slow. Consider adding a timeout or making this check asynchronous.
$response = Http::get("https://api.pwnedpasswords.com/range/{$prefix}"); | |
try { | |
$response = Http::timeout(2)->get("https://api.pwnedpasswords.com/range/{$prefix}"); | |
} catch (\Exception $e) { | |
return false; // fail-safe on timeout or connection error | |
} |
Copilot uses AI. Check for mistakes.
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.
Excessive blank lines after the method. Remove unnecessary empty lines to maintain consistent code formatting.
Copilot uses AI. Check for mistakes.
|
||
|
||
<!-- Password Breach Warning Alert --> | ||
<!-- Password Breach Warning Alert --> |
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.
Duplicate comment. The comment on line 174 and 175 are redundant.
<!-- Password Breach Warning Alert --> |
Copilot uses AI. Check for mistakes.
<div class="bg-base-100 rounded-lg w-full max-w-sm p-6 relative shadow-lg"> | ||
|
||
<!-- Close button --> | ||
<!-- Close button --> |
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.
Duplicate comment. Remove one of the redundant 'Close button' comments.
<!-- Close button --> |
Copilot uses AI. Check for mistakes.
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
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
$prefix = substr($sha1, 0, 5); | ||
$suffix = substr($sha1, 5); | ||
try { |
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.
Inconsistent indentation - this line should be aligned with the function body using 4 spaces, not 3.
try { | |
try { |
Copilot uses AI. Check for mistakes.
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.
Remove the excessive blank lines at the end of the function. Use consistent spacing with at most one blank line.
Copilot uses AI. Check for mistakes.
|
||
<!-- Password Breach Warning Alert --> | ||
<!-- Password Breach Warning Alert --> | ||
@if ( session('password_breach_warning')) |
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.
Remove the unnecessary space after the opening parenthesis. Should be @if (session('password_breach_warning'))
.
@if ( session('password_breach_warning')) | |
@if (session('password_breach_warning')) |
Copilot uses AI. Check for mistakes.
… password