fix!: harden audit routes and unify login gate (BREAKING — release as v0.5.0)

php/README.md

@@ -11,6 +11,35 @@ Includes:
 - login audit logging: an owned `auth_audit_log` table, a default database logger, binary IP storage, optional read endpoints, and opt-in retention (see "Login audit logging")
 - policy and audit contracts for app-specific behavior
 
+## Upgrading to v0.5.0 (breaking)
+
+This release adds a required method to the `AuthUserPolicy` contract:
+
+```php
+public function canLogin(Authenticatable $user, Request $request): bool;
+```
+
+It is the single gate for account-state checks (active, approved, not disabled)
+and is now enforced by the new `RequireActiveUser` middleware on the package's
+audit routes, so a role-only admin gate can no longer let a pending or disabled
+account through.
+
+Because the contract gained a required method, this is a **breaking change** and
+must be released as **v0.5.0** (not a 0.4.x patch) so consumers opt in. Consuming
+apps that **implement `AuthUserPolicy` directly** must add `canLogin()` when they
+upgrade — typically delegating to their model:
+
+```php
+public function canLogin(Authenticatable $user, Request $request): bool
+{
+    return $user instanceof User && $user->canLogin() && $user->hasVerifiedEmail();
+}
+```
+
+Apps that extend `DefaultAuthUserPolicy` inherit a working `canLogin()` (it
+duck-types `$user->canLogin()`, falls back to `is_disabled`, defaults to `true`)
+and need no change.
+
 ## Install
 
 ```sh
@@ -178,6 +207,70 @@ Bind `BWH\Auth\Contracts\AuthUserPolicy` when an app needs custom login gates or
 
 Bind `BWH\Auth\Contracts\AuthAuditLogger` only when an app wants to override the built-in audit behavior (for example, to mirror events into its own broader audit table). Most apps should instead use the database driver described below.
 
+### canLogin() — the single gate for account state
+
+`AuthUserPolicy::canLogin()` is the **single source of truth** for "is this account allowed to proceed through any login flow." The default implementation duck-types `$user->canLogin()` and falls back to checking `$user->is_disabled`. Apps with additional account-state columns (e.g. `approved_at`, `email_verified_at`, or a role whitelist) must bind a custom policy and encode **all** conditions in `canLogin()`:
+
+```php
+// app/Auth/AppUserPolicy.php
+class AppUserPolicy extends DefaultAuthUserPolicy
+{
+    public function canLogin(Authenticatable $user, Request $request): bool
+    {
+        return $user->approved_at !== null
+            && ! $user->is_disabled;
+    }
+}
+
+// AppServiceProvider::register()
+$this->app->bind(AuthUserPolicy::class, AppUserPolicy::class);
+```
+
+The package calls `canLogin()` automatically from:
+
+- `RequireActiveUser` middleware, applied to all package audit-log routes
+- `canPasskeyLogin()` in the default policy (passkey auth delegates here)
+- The 2FA `completeLogin()` path delegates through `redirectAfterLogin()`; if the user should be
+  blocked at that point, `canLogin()` must return false so the redirect sends them away from the app
+
+Apps must also call `canLogin()` from their own:
+
+1. **Primary password-login controller** — before `Auth::attempt()` or after resolving the user.
+2. **Email-verification callback** — after marking the email verified, call `canLogin()` and use
+   `redirectAfterLogin()` (not a hardcoded path) so a just-verified but still-pending user goes to
+   the pending page rather than into the app. Hardcoding `/pending` in the verification handler
+   causes approved users who verified their email to be falsely shown the pending page.
+
+### Protecting admin gates against pending/disabled accounts
+
+When setting `audit.admin_ability`, the Gate ability definition must verify **both** admin role and active-account state. The package applies `RequireActiveUser` on top, but your Gate definition should be correct independently (it may be called from other locations):
+
+```php
+// AppServiceProvider::boot()
+Gate::define('admin-only', function (User $user) {
+    // WRONG: only checks role — a pending admin bypasses account-state checks
+    // return $user->is_admin;
+
+    // CORRECT: role AND account state
+    return $user->is_admin
+        && $user->approved_at !== null
+        && ! $user->is_disabled;
+});
+```
+
+### Backfilling approved_at after adding an approval column
+
+If you add an `approved_at` (nullable, null = pending) column to your existing `users` table, every pre-existing row will be null after migration, instantly locking out all current users — including the primary admin. Before deploying the migration to production, add a backfill step in your migration (or a separate migration) to grandfather existing rows:
+
+```php
+// In your migration's up() method, after adding the column:
+DB::table('users')
+    ->whereNull('approved_at')
+    ->update(['approved_at' => now()]);
+```
+
+Alternatively, make null mean "approved" and use a different sentinel (e.g. a `pending` boolean), but document the convention clearly.
+
 ## Login audit logging
 
 The package can own a single append-only audit log for authentication events, so consuming apps no longer hand-roll their own login-audit table and writer.

php/config/bherila-auth.php

@@ -24,6 +24,11 @@
         // null = retain forever (no pruning). Set a positive integer to enable `model:prune`.
         'retention_days' => env('BHERILA_AUTH_AUDIT_RETENTION_DAYS'),
         // Gate ability required for the cross-user admin endpoint; null disables that route.
+        // IMPORTANT: the ability must verify that the user is active/approved AND is an admin.
+        // The package enforces its own RequireActiveUser check on top of this gate, but the
+        // gate should still verify account state independently so your Gate definition is
+        // correct even when called from other locations. Example: check both ->is_admin and
+        // ->approved_at, not just the role.
         'admin_ability' => env('BHERILA_AUTH_AUDIT_ADMIN_ABILITY'),
     ],
 

php/routes/audit.php

@@ -1,9 +1,13 @@
 <?php
 
 use BWH\Auth\Http\Controllers\AuthAuditController;
+use BWH\Auth\Http\Middleware\RequireActiveUser;
 use Illuminate\Support\Facades\Route;
 
-Route::middleware('auth')->group(function () {
+// The RequireActiveUser middleware ensures a pending or disabled account cannot reach
+// these endpoints even when the surrounding app gate only checks role without verifying
+// account state (see bherila-auth.audit.admin_ability).
+Route::middleware(['auth', RequireActiveUser::class])->group(function () {
     Route::get('/auth/audit-log', [AuthAuditController::class, 'index'])->name('bherila-auth.audit.index');
     Route::post('/auth/audit-log/{id}/suspicious', [AuthAuditController::class, 'markSuspicious'])->name('bherila-auth.audit.suspicious');
     Route::get('/auth/audit-log/all', [AuthAuditController::class, 'all'])->name('bherila-auth.audit.all');

php/src/Contracts/AuthUserPolicy.php

@@ -7,6 +7,18 @@
 
 interface AuthUserPolicy
 {
+    /**
+     * Whether the user is allowed to complete any login (active, not disabled, approved, etc.).
+     *
+     * The package's own middleware ({@see \BWH\Auth\Http\Middleware\RequireActiveUser}) calls
+     * this before serving audit-log endpoints. Implementing apps must also call it from their
+     * primary password-login controller and from any post-email-verification redirect.
+     *
+     * The default implementation duck-types `$user->canLogin()`, then falls back to checking
+     * `$user->is_disabled`. If neither exists the user is assumed to be allowed.
+     */
+    public function canLogin(Authenticatable $user, Request $request): bool;
+
     public function canPasskeyLogin(Authenticatable $user, Request $request): bool;
 
     public function redirectAfterLogin(Authenticatable $user, Request $request): string;

php/src/Http/Middleware/RequireActiveUser.php

@@ -0,0 +1,47 @@
+<?php
+
+namespace BWH\Auth\Http\Middleware;
+
+use BWH\Auth\Contracts\AuthUserPolicy;
+use Closure;
+use Illuminate\Http\Request;
+use Symfony\Component\HttpFoundation\Response;
+
+/**
+ * Abort with 403 if the authenticated user is not allowed to log in.
+ *
+ * Applied automatically to the package's own audit-log routes so that a pending
+ * or disabled account cannot reach those endpoints even if the surrounding gate
+ * ability only checks admin role without verifying account state.
+ *
+ * Apps may also apply this middleware to their own authenticated routes:
+ *
+ * ```php
+ * Route::middleware(['auth', \BWH\Auth\Http\Middleware\RequireActiveUser::class])
+ *     ->group(function () { ... });
+ * ```
+ *
+ * The check delegates to {@see \BWH\Auth\Contracts\AuthUserPolicy::canLogin()}, which
+ * by default duck-types `$user->canLogin()` and falls back to `$user->is_disabled`.
+ * Bind a custom AuthUserPolicy to add approved-at, email-verified, or other checks.
+ */
+class RequireActiveUser
+{
+    public function __construct(private readonly AuthUserPolicy $policy) {}
+
+    public function handle(Request $request, Closure $next): Response
+    {
+        $user = $request->user();
+
+        if ($user === null) {
+            // Let the auth middleware handle unauthenticated requests.
+            return $next($request);
+        }
+
+        if (! $this->policy->canLogin($user, $request)) {
+            abort(403, 'Your account is not active.');
+        }
+
+        return $next($request);
+    }
+}

php/src/Services/DefaultAuthUserPolicy.php

@@ -8,7 +8,19 @@
 
 class DefaultAuthUserPolicy implements AuthUserPolicy
 {
-    public function canPasskeyLogin(Authenticatable $user, Request $request): bool
+    /**
+     * Whether the user is allowed to log in.
+     *
+     * Duck-types `$user->canLogin()` first (covers approved/active/disabled checks in one call),
+     * then falls back to `$user->is_disabled`, then defaults to `true` for apps that have no
+     * such columns.
+     *
+     * Apps with an `approved_at` column or a multi-step onboarding state should bind a custom
+     * {@see \BWH\Auth\Contracts\AuthUserPolicy} implementation that encodes those rules here,
+     * and call `canLogin()` from their primary password-login controller and from any
+     * post-email-verification redirect so all entry points share the same gate.
+     */
+    public function canLogin(Authenticatable $user, Request $request): bool
     {
         if (method_exists($user, 'canLogin')) {
             return (bool) $user->canLogin();
@@ -21,6 +33,17 @@ public function canPasskeyLogin(Authenticatable $user, Request $request): bool
         return true;
     }
 
+    /**
+     * Whether the user may authenticate via a passkey.
+     *
+     * Delegates to {@see canLogin()} — if the user is not allowed to log in at all, passkey
+     * login is also denied.
+     */
+    public function canPasskeyLogin(Authenticatable $user, Request $request): bool
+    {
+        return $this->canLogin($user, $request);
+    }
+
     public function redirectAfterLogin(Authenticatable $user, Request $request): string
     {
         if (method_exists($user, 'getLoginRedirectUrl')) {

php/tests/Feature/RequireActiveUserTest.php

@@ -0,0 +1,116 @@
+<?php
+
+namespace BWH\Auth\Tests\Feature;
+
+use BWH\Auth\Contracts\AuthUserPolicy;
+use BWH\Auth\Http\Middleware\RequireActiveUser;
+use BWH\Auth\Services\DefaultAuthUserPolicy;
+use BWH\Auth\Tests\Fixtures\User;
+use BWH\Auth\Tests\TestCase;
+use Illuminate\Contracts\Auth\Authenticatable;
+use Illuminate\Http\Request;
+
+class RequireActiveUserTest extends TestCase
+{
+    // --- DefaultAuthUserPolicy::canLogin() ---
+
+    public function test_default_policy_allows_user_with_no_special_columns(): void
+    {
+        $user = User::create(['name' => 'Test', 'email' => 'a@example.com', 'password' => bcrypt('x')]);
+        $policy = new DefaultAuthUserPolicy;
+
+        $this->assertTrue($policy->canLogin($user, Request::create('/')));
+    }
+
+    public function test_default_policy_delegates_to_can_login_method_when_present(): void
+    {
+        $user = new class extends User
+        {
+            public function canLogin(): bool
+            {
+                return false;
+            }
+        };
+        $user->forceFill(['name' => 'Test', 'email' => 'b@example.com', 'password' => bcrypt('x')]);
+        $user->exists = true;
+
+        $policy = new DefaultAuthUserPolicy;
+        $this->assertFalse($policy->canLogin($user, Request::create('/')));
+    }
+
+    public function test_default_policy_uses_is_disabled_fallback(): void
+    {
+        $user = new class extends User
+        {
+            public bool $is_disabled = true;
+        };
+        $user->forceFill(['name' => 'Test', 'email' => 'c@example.com', 'password' => bcrypt('x')]);
+        $user->exists = true;
+
+        $policy = new DefaultAuthUserPolicy;
+        $this->assertFalse($policy->canLogin($user, Request::create('/')));
+    }
+
+    public function test_default_policy_can_passkey_login_delegates_to_can_login(): void
+    {
+        $user = new class extends User
+        {
+            public function canLogin(): bool
+            {
+                return false;
+            }
+        };
+        $user->forceFill(['name' => 'Test', 'email' => 'd@example.com', 'password' => bcrypt('x')]);
+        $user->exists = true;
+
+        $policy = new DefaultAuthUserPolicy;
+        $this->assertFalse($policy->canPasskeyLogin($user, Request::create('/')));
+    }
+
+    // --- RequireActiveUser middleware ---
+
+    public function test_middleware_passes_through_when_user_can_login(): void
+    {
+        $user = User::create(['name' => 'Test', 'email' => 'e@example.com', 'password' => bcrypt('x')]);
+
+        $request = Request::create('/');
+        $request->setUserResolver(fn () => $user);
+
+        $middleware = new RequireActiveUser(app(AuthUserPolicy::class));
+        $response = $middleware->handle($request, fn ($r) => response('ok'));
+
+        $this->assertSame('ok', $response->getContent());
+    }
+
+    public function test_middleware_aborts_403_when_user_cannot_login(): void
+    {
+        $policy = new class extends DefaultAuthUserPolicy
+        {
+            public function canLogin(Authenticatable $user, Request $request): bool
+            {
+                return false;
+            }
+        };
+
+        $user = User::create(['name' => 'Test', 'email' => 'f@example.com', 'password' => bcrypt('x')]);
+
+        $request = Request::create('/');
+        $request->setUserResolver(fn () => $user);
+
+        $this->expectException(\Symfony\Component\HttpKernel\Exception\HttpException::class);
+
+        $middleware = new RequireActiveUser($policy);
+        $middleware->handle($request, fn ($r) => response('ok'));
+    }
+
+    public function test_middleware_passes_through_when_no_authenticated_user(): void
+    {
+        $request = Request::create('/');
+        $request->setUserResolver(fn () => null);
+
+        $middleware = new RequireActiveUser(app(AuthUserPolicy::class));
+        $response = $middleware->handle($request, fn ($r) => response('ok'));
+
+        $this->assertSame('ok', $response->getContent());
+    }
+}