Skip to content

Improved authentication system, UI, and security#648

Open
th3sabbir wants to merge 1 commit intojeremykenedy:masterfrom
th3sabbir:improvement/project-enhancement
Open

Improved authentication system, UI, and security#648
th3sabbir wants to merge 1 commit intojeremykenedy:masterfrom
th3sabbir:improvement/project-enhancement

Conversation

@th3sabbir
Copy link

This pull request includes several improvements to the Laravel Authentication project.

Changes made:

  • Improved validation for login and registration
  • Improved error messages
  • Refactored code for better readability
  • Improved project structure
  • Enhanced security checks
  • Improved UI (login and register pages)
  • Updated README.md with installation guide and project details

The project runs successfully on localhost and follows Laravel best practices.

These improvements make the project more professional and easier to use.

Copilot AI review requested due to automatic review settings February 27, 2026 09:21
Copy link

Copilot AI left a 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 modernizes the authentication UX and tightens several auth/security-related behaviors in a Laravel 12 “laravel-auth” app, including logout handling, stronger password policy, and updated installation docs.

Changes:

  • Revamps auth Blade views (login/register/password reset) with new styling and client-side helpers (password visibility + strength indicators).
  • Strengthens validation (strong password rules; stricter email validation) and improves logout security (session invalidation + CSRF regeneration).
  • Adds docs updates (INSTALLATION guide, .env.example guidance) and a users-table indexing migration.

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
resources/views/partials/nav.blade.php Replaces logout link + hidden form with a direct POST form/button in the dropdown.
resources/views/layouts/app.blade.php Adds meta tags, logout confirmation JS, and auto-dismiss logic for success alerts.
resources/views/auth/register.blade.php New register page layout + password strength UI + visibility toggles + recaptcha placement.
resources/views/auth/passwords/reset.blade.php New reset-password layout + strength UI + visibility toggles.
resources/views/auth/passwords/email.blade.php New “forgot password” layout and alert formatting.
resources/views/auth/login.blade.php New login layout + password visibility toggle + recaptcha placement and new flash alert block.
database/migrations/2026_02_27_000000_add_performance_indexes_to_users_table.php Adds indexes for activated, deleted_at, and a composite activated + deleted_at index with an existence check.
app/Models/User.php Adjusts hidden/casts and adds an active() scope and full_name accessor.
app/Http/Requests/UpdateUserPasswordRequest.php Upgrades password validation to Laravel’s Password rule (mixed case, numbers, symbols, uncompromised).
app/Http/Requests/Auth/RegisterRequest.php Introduces a new FormRequest for registration validation + normalization (currently not wired in).
app/Http/Controllers/Auth/RegisterController.php Strengthens registration validator rules (email:rfc,dns + strong Password rule) and switches to Str::random.
app/Http/Controllers/Auth/LoginController.php Refactors logout to accept Request, logs logout event, invalidates session, regenerates CSRF token, and adds throttling properties.
README.md Adds link to the new INSTALLATION guide in the TOC.
INSTALLATION.md Adds a detailed XAMPP/localhost install guide and environment variable reference.
.env.example Updates defaults and adds local dev guidance for APP_URL/DB/MAIL settings.
Comments suppressed due to low confidence (10)

resources/views/auth/register.blade.php:255

  • The confirm-password visibility toggle is also tabindex="-1" and lacks an accessible label, so it can't be reached by keyboard/screen-reader users. Make it focusable and add an aria-label/aria-pressed state.
                                <button type="button" class="password-toggle" id="togglePasswordConfirm" tabindex="-1">
                                    <i class="fa fa-eye" id="togglePasswordConfirmIcon"></i>
                                </button>

app/Http/Requests/Auth/RegisterRequest.php:30

  • RegisterRequest is added but is not referenced anywhere in the codebase (no controller method type-hints it, and RegisterController still relies on validator() from RegistersUsers). Either wire this FormRequest into the registration flow or remove it to avoid dead/duplicated validation logic.
class RegisterRequest extends FormRequest
{
    /**
     * Determine if the user is authorized to make this request.
     */
    public function authorize(): bool
    {
        return true; // Guest access is enforced by the guest middleware on the route.
    }

    /**
     * Get the validation rules that apply to the request.
     *
     * @return array<string, mixed>
     */
    public function rules(): array
    {

resources/views/auth/passwords/reset.blade.php:158

  • The confirm-password visibility toggle is tabindex="-1" and lacks an accessible label, so it can't be reached by keyboard/screen-reader users. Make it focusable and add an aria-label/aria-pressed state.
                                <button type="button" class="password-toggle" id="togglePasswordConfirm" tabindex="-1">
                                    <i class="fa fa-eye" id="togglePasswordConfirmIcon"></i>
                                </button>

resources/views/layouts/app.blade.php:10

  • <meta name="robots" content="noindex, nofollow"> will prevent indexing for all environments, including production. If the intent is to avoid indexing only in non-production, make this conditional (e.g., based on app()->environment('production')) or remove it from the default layout.
        {{-- Security headers via meta --}}
        <meta name="referrer" content="strict-origin-when-cross-origin">
        <meta name="robots" content="noindex, nofollow">

app/Http/Controllers/Auth/LoginController.php:89

  • This logout now flashes status as a human-readable message, but the shared partials/form-status layout expects success for success alerts (or status + message for status-box style). As a result, the logout success message likely won't render on the / page. Consider flashing success (or status + message in the existing format) to match how the app displays flash messages.
        return redirect($this->redirectAfterLogout)
            ->with('status', __('You have been successfully logged out.'));

app/Http/Controllers/Auth/LoginController.php:90

  • Test coverage: logout() now invalidates the session/regenerates the CSRF token and logs a structured event. There are Feature tests for authentication, but none asserting logout behavior (redirect target + session invalidation + flash message). Please add a Feature test for POST /logout covering these outcomes.
    public function logout(Request $request)
    {
        $user = Auth::user();

        if ($user) {
            Log::info('User logged out.', [
                'user_id' => $user->id,
                'email'   => $user->email,
                'ip'      => $request->ip(),
            ]);
        }

        Auth::logout();

        $request->session()->invalidate();
        $request->session()->regenerateToken();

        return redirect($this->redirectAfterLogout)
            ->with('status', __('You have been successfully logged out.'));
    }

database/migrations/2026_02_27_000000_add_performance_indexes_to_users_table.php:66

  • This migration’s index-existence check queries information_schema.STATISTICS, which is MySQL/MariaDB-specific. The repository config ships with other database drivers (sqlite/pgsql/sqlsrv), so running migrations on a non-MySQL connection will fail. Consider guarding by driver ($conn->getDriverName()) or using a DB-agnostic approach (e.g., Doctrine schema manager via doctrine/dbal, which is already required).
    private function indexExists(string $table, string $indexName): bool
    {
        $conn    = Schema::getConnection();
        $dbName  = $conn->getDatabaseName();
        $indexes = $conn->select(
            "SELECT INDEX_NAME FROM information_schema.STATISTICS
             WHERE TABLE_SCHEMA = ? AND TABLE_NAME = ? AND INDEX_NAME = ?",
            [$dbName, $table, $indexName]
        );

        return count($indexes) > 0;
    }

resources/views/auth/register.blade.php:209

  • The password visibility toggle button is removed from the tab order (tabindex="-1") and has no accessible label, making it unusable for keyboard/screen-reader users. Keep it focusable and add an aria-label/aria-pressed state (similar to the login view).
                                <button type="button" class="password-toggle" id="togglePassword" tabindex="-1">
                                    <i class="fa fa-eye" id="togglePasswordIcon"></i>
                                </button>

resources/views/auth/passwords/reset.blade.php:112

  • The password visibility toggle button is removed from the tab order (tabindex="-1") and has no accessible label, making it unusable for keyboard/screen-reader users. Keep it focusable and add an aria-label/aria-pressed state.
                                <button type="button" class="password-toggle" id="togglePassword" tabindex="-1">
                                    <i class="fa fa-eye" id="togglePasswordIcon"></i>
                                </button>

resources/views/auth/passwords/email.blade.php:50

  • The dismiss button for the success alert is missing an accessible name and the Bootstrap-recommended markup (e.g., aria-label="Close" and a <span aria-hidden="true">&times;</span>). Add the aria attributes to make the alert dismiss control screen-reader friendly.
                        <div class="alert alert-success alert-dismissible fade show" role="alert">
                            <i class="fa fa-check-circle mr-2"></i>{{ session('status') }}
                            <button type="button" class="close" data-dismiss="alert">&times;</button>
                        </div>
                    @endif

                    <form method="POST" action="{{ route('password.email') }}" novalidate>
                        @csrf

                        <div class="form-group mb-4">
                            <label for="email" class="font-weight-600 text-muted small text-uppercase">
                                {{ __('Email Address') }}

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

var logoutForms = document.querySelectorAll('.logout-form');
logoutForms.forEach(function (form) {
form.addEventListener('submit', function (e) {
if (!confirm('{{ __("Are you sure you want to sign out?") }}')) {
Copy link

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The confirm() message is injected into a JavaScript single-quoted string via {{ __() }}. If the translation ever contains an apostrophe or line break, this will break the script (and can create an injection vector). Use @json(__('Are you sure you want to sign out?')) (or Js::from(...)) when embedding translated strings into JS.

Suggested change
if (!confirm('{{ __("Are you sure you want to sign out?") }}')) {
if (!confirm(@json(__('Are you sure you want to sign out?')))) {

Copilot uses AI. Check for mistakes.
Comment on lines +75 to +76
'first_name' => trim($this->first_name ?? ''),
'last_name' => trim($this->last_name ?? ''),
Copy link

Copilot AI Feb 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

prepareForValidation() forces first_name/last_name to empty strings when missing. With ConvertEmptyStringsToNull middleware, these would normally become null and pass the nullable rule; forcing '' will cause alpha_dash to fail for optional fields. Preserve null for optional inputs (only trim when the value is not null).

Suggested change
'first_name' => trim($this->first_name ?? ''),
'last_name' => trim($this->last_name ?? ''),
'first_name' => $this->first_name === null ? null : trim($this->first_name),
'last_name' => $this->last_name === null ? null : trim($this->last_name),

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants