Преглед изворни кода

Phase 2 hotfix: scalar-safe Request + local admin login

Bug: array-valued entries in $_SERVER/$_GET (e.g. HTTP_*[]= or query[]=
params that PHP parses into arrays, or mod_php populating nested
structures) tripped array-to-string conversion in Request::fromGlobals.
That warning printed to the response body before SessionGuard::start
could touch session settings, so every subsequent session_* call failed
with "headers already sent".

Fix in Request::fromGlobals:
- Keep $_GET / $_SERVER as arrays of mixed values (no blanket strval).
- Guard every scalar coercion with is_scalar (header loop,
  CONTENT_TYPE/CONTENT_LENGTH pickup, ip()).
- New queryString() / postString() helpers for single-value reads, each
  returning '' for missing or non-scalar values.

Belt-and-suspenders: wrap the front controller in ob_start so a stray
warning never poisons the response headers again.

Local admin login (new):
- .env.example documents LOCAL_ADMIN_EMAIL / LOCAL_ADMIN_PASSWORD /
  LOCAL_ADMIN_NAME. Setting email + password enables the flow.
- Auth\LocalAdmin — env reader + timing-safe verify (hash_equals on
  both email and password).
- UserRepository::upsertFromOidc gains a forceAdmin flag. When true,
  is_admin is forced to 1 on both INSERT and UPDATE, so the configured
  local admin keeps admin rights even if previously demoted. Default
  behavior for the OIDC flow is unchanged.
- AuthController::loginLocalForm (GET /auth/local) renders the form,
  pre-filling the configured email. loginLocal (POST /auth/local)
  checks the CSRF token, verifies credentials, upserts with
  oid = "local:<email>" + forceAdmin, writes CREATE/UPDATE +
  BOOTSTRAP_ADMIN (first user) + LOGIN audit rows inside one tx, and
  starts the session. Bad credentials trigger a LOGIN_FAILED audit
  row and a redirect back with ?error=1. Both routes return 404 when
  local admin is disabled.
- Home page offers both sign-in buttons when configured; shows a clear
  "no sign-in method configured" message when neither is.

Verified:
- php -l on every changed file.
- Request::fromGlobals no longer warns with array $_SERVER values.
- LocalAdmin verify: correct=true, bad-email=false, bad-pw=false.
- upsertFromOidc(forceAdmin=true) creates admin on insert and
  re-admins a demoted user on update.
- Default OIDC path does NOT re-promote a demoted user.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
achiappa пре 2 недеља
родитељ
комит
83493d0541

+ 11 - 0
.env.example

@@ -20,3 +20,14 @@ SESSION_PATH=/var/www/data/sessions
 
 # 'production' disables verbose error output. Anything else is treated as dev.
 APP_ENV=production
+
+# ---------------------------------------------------------------------------
+# Local admin (optional) — lets you sign in without Entra, e.g. during initial
+# setup or for a fully on-prem deployment. Set BOTH email and password to
+# enable; leave blank to disable. The password is compared in plain text
+# against this env value — so .env must be readable only by the app user.
+# The resulting user is stored with entra_oid = "local:<email>" and is_admin=1.
+# ---------------------------------------------------------------------------
+LOCAL_ADMIN_EMAIL=
+LOCAL_ADMIN_PASSWORD=
+LOCAL_ADMIN_NAME=Local Admin

+ 22 - 9
public/index.php

@@ -2,6 +2,7 @@
 
 declare(strict_types=1);
 
+use App\Auth\LocalAdmin;
 use App\Auth\OidcClient;
 use App\Auth\SessionGuard;
 use App\Controllers\AuthController;
@@ -14,6 +15,10 @@ use App\Http\View;
 use App\Repositories\UserRepository;
 use App\Services\AuditLogger;
 
+// Buffer output so a stray warning/notice can't send headers before
+// Response::send() gets a chance to set them. send() will flush.
+ob_start();
+
 define('APP_ROOT', dirname(__DIR__));
 
 // ---------------------------------------------------------------------------
@@ -67,7 +72,7 @@ try {
 $view   = new View(APP_ROOT . '/views');
 $users  = new UserRepository($pdo);
 $audit  = new AuditLogger($pdo);
-$auth   = new AuthController($pdo, $users, $audit);
+$auth   = new AuthController($pdo, $users, $audit, $view);
 
 // ---------------------------------------------------------------------------
 // Routing
@@ -81,14 +86,15 @@ $router->get('/', function (Request $req) use ($view, $pdo, $users, $appEnv): Re
     )->fetchColumn();
 
     return Response::html($view->render('home', [
-        'title'         => 'Sprint Planner',
-        'currentUser'   => $currentUser,
-        'schemaVersion' => $schemaVersion,
-        'dbPath'        => Connection::path(),
-        'appEnv'        => $appEnv,
-        'oidcConfigured' => OidcClient::isConfigured(),
-        'authError'     => isset($req->query['auth_error']),
-        'csrfToken'     => SessionGuard::csrfToken(),
+        'title'            => 'Sprint Planner',
+        'currentUser'      => $currentUser,
+        'schemaVersion'    => $schemaVersion,
+        'dbPath'           => Connection::path(),
+        'appEnv'           => $appEnv,
+        'oidcConfigured'   => OidcClient::isConfigured(),
+        'localAdminEnabled' => LocalAdmin::isEnabled(),
+        'authError'        => isset($req->query['auth_error']),
+        'csrfToken'        => SessionGuard::csrfToken(),
     ]));
 });
 
@@ -97,6 +103,8 @@ $router->get('/healthz', fn() => Response::text('ok'));
 $router->get('/auth/login',     $auth->login(...));
 $router->get('/auth/callback',  $auth->callback(...));
 $router->post('/auth/logout',   $auth->logout(...));
+$router->get('/auth/local',     $auth->loginLocalForm(...));
+$router->post('/auth/local',    $auth->loginLocal(...));
 
 // ---------------------------------------------------------------------------
 // Dispatch
@@ -104,3 +112,8 @@ $router->post('/auth/logout',   $auth->logout(...));
 $request  = Request::fromGlobals();
 $response = $router->dispatch($request);
 $response->send();
+
+// Flush the output buffer opened at the top.
+if (ob_get_level() > 0) {
+    @ob_end_flush();
+}

+ 67 - 0
src/Auth/LocalAdmin.php

@@ -0,0 +1,67 @@
+<?php
+
+declare(strict_types=1);
+
+namespace App\Auth;
+
+/**
+ * Env-configured admin fallback for environments where OIDC is not yet
+ * (or deliberately not) configured.
+ *
+ * Enabled iff BOTH env vars are set:
+ *     LOCAL_ADMIN_EMAIL
+ *     LOCAL_ADMIN_PASSWORD
+ *
+ * Optional:
+ *     LOCAL_ADMIN_NAME       display name (default "Local Admin")
+ *
+ * Password is compared verbatim against the env value using a timing-safe
+ * comparison. The corresponding user row is stored with
+ *     entra_oid    = "local:<email>"
+ *     is_admin     = 1
+ * so it will not collide with a real Entra user.
+ */
+final class LocalAdmin
+{
+    public const OID_PREFIX = 'local:';
+
+    public static function isEnabled(): bool
+    {
+        return self::email() !== '' && self::password() !== '';
+    }
+
+    public static function email(): string
+    {
+        $v = getenv('LOCAL_ADMIN_EMAIL');
+        return is_string($v) ? trim($v) : '';
+    }
+
+    public static function displayName(): string
+    {
+        $v = getenv('LOCAL_ADMIN_NAME');
+        $name = is_string($v) ? trim($v) : '';
+        return $name !== '' ? $name : 'Local Admin';
+    }
+
+    public static function oid(): string
+    {
+        return self::OID_PREFIX . self::email();
+    }
+
+    /** Timing-safe credential check. Returns false if local admin is disabled. */
+    public static function verify(string $email, string $password): bool
+    {
+        if (!self::isEnabled()) {
+            return false;
+        }
+        $emailMatch = hash_equals(self::email(), trim($email));
+        $pwMatch    = hash_equals(self::password(), $password);
+        return $emailMatch && $pwMatch;
+    }
+
+    private static function password(): string
+    {
+        $v = getenv('LOCAL_ADMIN_PASSWORD');
+        return is_string($v) ? $v : '';
+    }
+}

+ 105 - 0
src/Controllers/AuthController.php

@@ -4,10 +4,12 @@ declare(strict_types=1);
 
 namespace App\Controllers;
 
+use App\Auth\LocalAdmin;
 use App\Auth\OidcClient;
 use App\Auth\SessionGuard;
 use App\Http\Request;
 use App\Http\Response;
+use App\Http\View;
 use App\Repositories\UserRepository;
 use App\Services\AuditLogger;
 use PDO;
@@ -19,6 +21,7 @@ final class AuthController
         private readonly PDO            $pdo,
         private readonly UserRepository $users,
         private readonly AuditLogger    $audit,
+        private readonly View           $view,
     ) {
     }
 
@@ -170,6 +173,108 @@ final class AuthController
         return Response::redirect('/');
     }
 
+    /** GET /auth/local — render the local-admin login form. 404 when disabled. */
+    public function loginLocalForm(Request $req): Response
+    {
+        if (!LocalAdmin::isEnabled()) {
+            return Response::text('Not Found', 404);
+        }
+        SessionGuard::start();
+        $error = $req->queryString('error') === '1';
+        return Response::html($this->view->render('auth/local', [
+            'title'        => 'Local sign-in',
+            'currentUser'  => null,
+            'csrfToken'    => SessionGuard::csrfToken(),
+            'email'        => LocalAdmin::email(),
+            'error'        => $error,
+        ]));
+    }
+
+    /** POST /auth/local — verify credentials, upsert user, start session. */
+    public function loginLocal(Request $req): Response
+    {
+        if (!LocalAdmin::isEnabled()) {
+            return Response::text('Not Found', 404);
+        }
+        SessionGuard::start();
+
+        if (!SessionGuard::verifyCsrf($req)) {
+            return Response::text('CSRF token invalid', 403);
+        }
+
+        $email    = $req->postString('email');
+        $password = isset($req->post['password']) && is_scalar($req->post['password'])
+            ? (string) $req->post['password']
+            : '';
+
+        if (!LocalAdmin::verify($email, $password)) {
+            $this->logFailure($req, 'local_admin_credential_mismatch');
+            return Response::redirect('/auth/local?error=1');
+        }
+
+        $this->pdo->beginTransaction();
+        try {
+            $isFirstUser = $this->users->count() === 0;
+            $result      = $this->users->upsertFromOidc(
+                oid:            LocalAdmin::oid(),
+                email:          LocalAdmin::email(),
+                name:           LocalAdmin::displayName(),
+                promoteToAdmin: $isFirstUser,
+                forceAdmin:     true,
+            );
+            $user   = $result['user'];
+            $before = $result['before']?->toAuditSnapshot();
+
+            $action = $before === null ? 'CREATE' : 'UPDATE';
+            $this->audit->record(
+                action:     $action,
+                entityType: 'user',
+                entityId:   $user->id,
+                before:     $before,
+                after:      $user->toAuditSnapshot(),
+                userId:     $user->id,
+                userEmail:  $user->email,
+                ipAddress:  $req->ip(),
+                userAgent:  $req->userAgent(),
+            );
+
+            if ($isFirstUser) {
+                $this->audit->record(
+                    action:     'BOOTSTRAP_ADMIN',
+                    entityType: 'user',
+                    entityId:   $user->id,
+                    before:     null,
+                    after:      ['is_admin' => 1, 'via' => 'local'],
+                    userId:     $user->id,
+                    userEmail:  $user->email,
+                    ipAddress:  $req->ip(),
+                    userAgent:  $req->userAgent(),
+                );
+            }
+
+            $this->audit->record(
+                action:     'LOGIN',
+                entityType: 'user',
+                entityId:   $user->id,
+                before:     null,
+                after:      ['via' => 'local'],
+                userId:     $user->id,
+                userEmail:  $user->email,
+                ipAddress:  $req->ip(),
+                userAgent:  $req->userAgent(),
+            );
+
+            $this->pdo->commit();
+        } catch (Throwable $e) {
+            $this->pdo->rollBack();
+            $this->logFailure($req, 'local_admin_upsert_failed: ' . $e->getMessage());
+            return Response::redirect('/auth/local?error=1');
+        }
+
+        SessionGuard::login($user);
+        return Response::redirect('/');
+    }
+
     /** Write a LOGIN_FAILED audit row in its own tx; never throws. */
     private function logFailure(Request $req, string $reason): void
     {

+ 33 - 11
src/Http/Request.php

@@ -7,10 +7,10 @@ namespace App\Http;
 final class Request
 {
     /**
-     * @param array<string,string> $query
-     * @param array<string,mixed>  $post
-     * @param array<string,string> $headers  header names already lower-cased
-     * @param array<string,string> $server
+     * @param array<string,mixed>  $query   raw $_GET (may contain arrays for ?a[]=…)
+     * @param array<string,mixed>  $post    raw $_POST
+     * @param array<string,string> $headers header names already lower-cased
+     * @param array<string,mixed>  $server  raw $_SERVER (may contain arrays)
      */
     public function __construct(
         public readonly string $method,
@@ -34,27 +34,48 @@ final class Request
 
         $headers = [];
         foreach ($_SERVER as $k => $v) {
-            if (str_starts_with($k, 'HTTP_')) {
-                $name = strtolower(str_replace('_', '-', substr($k, 5)));
+            if (!is_scalar($v)) {
+                continue; // mod_php can occasionally put non-scalars here
+            }
+            if (str_starts_with((string) $k, 'HTTP_')) {
+                $name = strtolower(str_replace('_', '-', substr((string) $k, 5)));
                 $headers[$name] = (string) $v;
             }
         }
-        if (isset($_SERVER['CONTENT_TYPE']))   { $headers['content-type']   = (string) $_SERVER['CONTENT_TYPE']; }
-        if (isset($_SERVER['CONTENT_LENGTH'])) { $headers['content-length'] = (string) $_SERVER['CONTENT_LENGTH']; }
+        if (isset($_SERVER['CONTENT_TYPE']) && is_scalar($_SERVER['CONTENT_TYPE'])) {
+            $headers['content-type'] = (string) $_SERVER['CONTENT_TYPE'];
+        }
+        if (isset($_SERVER['CONTENT_LENGTH']) && is_scalar($_SERVER['CONTENT_LENGTH'])) {
+            $headers['content-length'] = (string) $_SERVER['CONTENT_LENGTH'];
+        }
 
         $raw = (string) (file_get_contents('php://input') ?: '');
 
         return new self(
             method:  $method,
             path:    $path,
-            query:   array_map(strval(...), $_GET),
+            query:   $_GET,
             post:    $_POST,
             rawBody: $raw,
             headers: $headers,
-            server:  array_map(strval(...), $_SERVER),
+            server:  $_SERVER,
         );
     }
 
+    /** Read a single-value query param as a trimmed string. Returns '' if missing or non-scalar. */
+    public function queryString(string $name): string
+    {
+        $v = $this->query[$name] ?? null;
+        return is_scalar($v) ? trim((string) $v) : '';
+    }
+
+    /** Read a single-value post param as a trimmed string. Returns '' if missing or non-scalar. */
+    public function postString(string $name): string
+    {
+        $v = $this->post[$name] ?? null;
+        return is_scalar($v) ? trim((string) $v) : '';
+    }
+
     public function header(string $name): ?string
     {
         return $this->headers[strtolower($name)] ?? null;
@@ -81,7 +102,8 @@ final class Request
 
     public function ip(): string
     {
-        return (string) ($this->server['REMOTE_ADDR'] ?? '');
+        $v = $this->server['REMOTE_ADDR'] ?? '';
+        return is_scalar($v) ? (string) $v : '';
     }
 
     public function userAgent(): string

+ 18 - 5
src/Repositories/UserRepository.php

@@ -40,6 +40,11 @@ final class UserRepository
      * the version *before* the call (null for a newly-created user) so the
      * caller can audit the change.
      *
+     * @param bool $promoteToAdmin  Set is_admin=1 on INSERT only.
+     * @param bool $forceAdmin      Additionally set is_admin=1 on UPDATE. Used
+     *                              for the local-admin login path so the
+     *                              configured admin keeps admin rights.
+     *
      * @return array{user: User, before: ?User}
      */
     public function upsertFromOidc(
@@ -47,6 +52,7 @@ final class UserRepository
         string $email,
         string $name,
         bool $promoteToAdmin,
+        bool $forceAdmin = false,
     ): array {
         $now      = gmdate('Y-m-d\TH:i:s\Z');
         $existing = $this->findByOid($oid);
@@ -60,7 +66,7 @@ final class UserRepository
                 $oid,
                 $email,
                 $name,
-                $promoteToAdmin ? 1 : 0,
+                ($promoteToAdmin || $forceAdmin) ? 1 : 0,
                 $now,
                 $now,
             ]);
@@ -72,10 +78,17 @@ final class UserRepository
             return ['user' => $user, 'before' => null];
         }
 
-        $stmt = $this->pdo->prepare(
-            'UPDATE users SET email = ?, display_name = ?, last_login_at = ? WHERE id = ?'
-        );
-        $stmt->execute([$email, $name, $now, $existing->id]);
+        if ($forceAdmin) {
+            $stmt = $this->pdo->prepare(
+                'UPDATE users SET email = ?, display_name = ?, last_login_at = ?, is_admin = 1 WHERE id = ?'
+            );
+            $stmt->execute([$email, $name, $now, $existing->id]);
+        } else {
+            $stmt = $this->pdo->prepare(
+                'UPDATE users SET email = ?, display_name = ?, last_login_at = ? WHERE id = ?'
+            );
+            $stmt->execute([$email, $name, $now, $existing->id]);
+        }
 
         $after = $this->find($existing->id) ?? $existing;
         return ['user' => $after, 'before' => $existing];

+ 52 - 0
views/auth/local.php

@@ -0,0 +1,52 @@
+<?php
+/** @var string $csrfToken */
+/** @var string $email */
+/** @var bool   $error */
+use function App\Http\e;
+?>
+<section class="max-w-md mx-auto mt-6">
+    <div class="rounded-lg border bg-white p-6">
+        <h1 class="text-xl font-semibold tracking-tight">Local admin sign-in</h1>
+        <p class="text-slate-600 text-sm mt-1">
+            Use this form only while Entra ID is not yet configured. Credentials
+            come from the <code>LOCAL_ADMIN_*</code> environment variables.
+        </p>
+
+        <?php if ($error): ?>
+            <div class="mt-4 rounded-md border border-red-200 bg-red-50 px-3 py-2 text-sm text-red-800">
+                Email or password did not match.
+            </div>
+        <?php endif; ?>
+
+        <form method="post" action="/auth/local" class="mt-4 space-y-3"
+              autocomplete="off">
+            <input type="hidden" name="_csrf" value="<?= e($csrfToken) ?>">
+
+            <label class="block">
+                <span class="text-sm text-slate-700">Email</span>
+                <input type="email" name="email" required
+                       value="<?= e($email) ?>"
+                       class="mt-1 block w-full rounded-md border-slate-300 shadow-sm
+                              px-3 py-2 border focus:outline-none focus:ring-2
+                              focus:ring-slate-400">
+            </label>
+
+            <label class="block">
+                <span class="text-sm text-slate-700">Password</span>
+                <input type="password" name="password" required autofocus
+                       class="mt-1 block w-full rounded-md border-slate-300 shadow-sm
+                              px-3 py-2 border focus:outline-none focus:ring-2
+                              focus:ring-slate-400">
+            </label>
+
+            <button type="submit"
+                    class="w-full rounded-md bg-slate-900 text-white px-4 py-2 text-sm font-medium hover:bg-slate-800">
+                Sign in
+            </button>
+        </form>
+
+        <p class="text-xs text-slate-500 mt-4">
+            <a href="/" class="hover:underline">← Back</a>
+        </p>
+    </div>
+</section>

+ 15 - 3
views/home.php

@@ -4,6 +4,7 @@
 /** @var string $appEnv */
 /** @var \App\Domain\User|null $currentUser */
 /** @var bool   $oidcConfigured */
+/** @var bool   $localAdminEnabled */
 /** @var bool   $authError */
 use function App\Http\e;
 ?>
@@ -21,15 +22,23 @@ use function App\Http\e;
                 Sign in with your Microsoft account to get started. The first person
                 to sign in becomes the admin automatically.
             </p>
-            <div class="mt-4">
+            <div class="mt-4 flex flex-wrap items-center gap-3">
                 <?php if ($oidcConfigured): ?>
                     <a href="/auth/login"
                        class="inline-flex items-center gap-2 rounded-md bg-slate-900 text-white px-4 py-2 text-sm font-medium hover:bg-slate-800">
                         Sign in with Microsoft
                     </a>
-                <?php else: ?>
+                <?php endif; ?>
+                <?php if ($localAdminEnabled): ?>
+                    <a href="/auth/local"
+                       class="inline-flex items-center gap-2 rounded-md border border-slate-300 bg-white text-slate-700 px-4 py-2 text-sm font-medium hover:bg-slate-100">
+                        Sign in as local admin
+                    </a>
+                <?php endif; ?>
+                <?php if (!$oidcConfigured && !$localAdminEnabled): ?>
                     <span class="inline-block rounded-md bg-slate-100 text-slate-600 px-3 py-2 text-sm">
-                        OIDC not configured — set <code>ENTRA_*</code> in <code>.env</code>.
+                        No sign-in method configured. Set <code>ENTRA_*</code> or
+                        <code>LOCAL_ADMIN_*</code> in <code>.env</code>.
                     </span>
                 <?php endif; ?>
             </div>
@@ -65,6 +74,9 @@ use function App\Http\e;
 
             <dt class="text-slate-500">OIDC</dt>
             <dd class="font-mono"><?= $oidcConfigured ? 'configured' : 'not configured' ?></dd>
+
+            <dt class="text-slate-500">Local admin</dt>
+            <dd class="font-mono"><?= $localAdminEnabled ? 'enabled' : 'disabled' ?></dd>
         </dl>
     </div>