Hotfix 4.4.4 - FinTS bank import review fixes

.env.example

@@ -13,10 +13,14 @@ APP_MAINTENANCE_STORE=database
 
 BCRYPT_ROUNDS=12
 
-LOG_CHANNEL=stack
-LOG_STACK=single
+LOG_CHANNEL=daily
+# Only used when LOG_CHANNEL=stack.
+LOG_STACK=daily
 LOG_DEPRECATIONS_CHANNEL=null
+# In production use "warning" — "debug" fills the disk with request noise.
 LOG_LEVEL=debug
+# Days of rotated log files to keep (daily channel).
+LOG_DAILY_DAYS=30
 
 DB_CONNECTION=mariadb
 DB_HOST=127.0.0.1
@@ -31,7 +35,11 @@ FILESYSTEM_DISK=local
 QUEUE_CONNECTION=sync
 SESSION_DRIVER=file
 SESSION_LIFETIME=120
-SESSION_ENCRYPT=false
+# With the file driver the session is a PHP-serialized file under
+# storage/framework/sessions/. During a FinTS dialog it holds the online-banking PIN and the
+# bank's session state - FintsConnectionHandler keeps the password in the session on purpose,
+# so that it never reaches the database - which would otherwise sit there in cleartext.
+SESSION_ENCRYPT=true
 SESSION_PATH=/
 SESSION_DOMAIN=null
 

README.md

@@ -65,7 +65,7 @@ above with the right PHP/Composer/Node toolchain (versioned `php8.4`, local
 `composer.phar`, nvm) and put the app into maintenance mode during updates:
 
 - `stufis-setup` — first-time setup *and* idempotent toolchain refresh
-- `stufis-update [tag|branch]` — deploy a release tag (or branch tip): self-update toolchain, migrate and rebuild a running instance
+- `stufis-update [tag|branch]` — deploy a release tag (or branch tip): self-update toolchain, migrate, refresh the FinTS bank list and rebuild a running instance
 - `stufis-rebuild` — re-warm production caches and rebuild assets (no pull, no dependency install)
 - `stufis-down` / `stufis-up` — toggle maintenance mode (`stufis-down` prints a secret bypass URL)
 

app/Console/Commands/UpdateFintsInstitutesCommand.php

@@ -0,0 +1,267 @@
+<?php
+
+namespace App\Console\Commands;
+
+use App\Models\FintsInstitute;
+use App\Support\Fints\InstituteListParser;
+use Illuminate\Console\Command;
+use Illuminate\Http\Client\ConnectionException;
+use Illuminate\Support\Carbon;
+use Illuminate\Support\Collection;
+use Illuminate\Support\Facades\Date;
+use Illuminate\Support\Facades\DB;
+use Illuminate\Support\Facades\Http;
+
+class UpdateFintsInstitutesCommand extends Command
+{
+    /**
+     * The name and signature of the console command.
+     *
+     * @var string
+     */
+    protected $signature = 'stufis:fints-institutes-update
+        {--url= : Override the configured bank list source}
+        {--file= : Read a local blz.properties instead of downloading}
+        {--dry-run : Report what would change without writing}
+        {--prune : Delete institutes that no longer exist upstream}
+        {--min-entries=1000 : Refuse lists shorter than this as implausible}';
+
+    /**
+     * The console command description.
+     *
+     * @var string
+     */
+    protected $description = 'Pulls the public FinTS bank list (hbci4java blz.properties) into the fints_institutes table';
+
+    /**
+     * Rows per upsert. The full list is ~4000 institutes.
+     */
+    private const int CHUNK_SIZE = 500;
+
+    /**
+     * Execute the console command.
+     */
+    public function handle(): int
+    {
+        $source = $this->option('file') ?: ($this->option('url') ?: config('stufis.fints.institute_list_url'));
+
+        $contents = $this->option('file')
+            ? $this->readFile((string) $this->option('file'))
+            : $this->download((string) $source);
+
+        if ($contents === null) {
+            return self::FAILURE;
+        }
+
+        $parser = new InstituteListParser;
+        $upstream = $parser->parse($contents);
+
+        $this->line(sprintf('Gelesen: %d Institute aus %s', count($upstream), $source));
+        if ($parser->skipped > 0) {
+            $this->comment(sprintf('%d unbrauchbare Zeilen übersprungen.', $parser->skipped));
+        }
+        if ($parser->insecureEndpoints > 0) {
+            $this->warn(sprintf(
+                '%d PIN/TAN-Adressen verworfen, weil sie nicht mit https:// beginnen.',
+                $parser->insecureEndpoints,
+            ));
+        }
+
+        // A truncated download or an HTML error page would otherwise wipe the table.
+        $minEntries = (int) $this->option('min-entries');
+        if (count($upstream) < $minEntries) {
+            $this->error(sprintf(
+                'Nur %d Institute gefunden, erwartet mindestens %d - Quelle sieht unvollständig aus, Abbruch.',
+                count($upstream),
+                $minEntries,
+            ));
+
+            return self::FAILURE;
+        }
+
+        $existing = FintsInstitute::query()
+            ->get(['blz', ...FintsInstitute::SYNCED_FIELDS])
+            ->keyBy('blz');
+
+        $new = [];
+        $changed = [];
+        foreach ($upstream as $blz => $institute) {
+            $current = $existing->get($blz);
+
+            if ($current === null) {
+                $new[$blz] = $institute;
+
+                continue;
+            }
+
+            foreach (FintsInstitute::SYNCED_FIELDS as $field) {
+                if ($current->{$field} !== $institute[$field]) {
+                    $changed[$blz] = $institute;
+
+                    break;
+                }
+            }
+        }
+
+        $vanished = $existing->keys()->diff(array_keys($upstream));
+
+        $this->newLine();
+        $this->table(['', 'Institute'], [
+            ['neu', count($new)],
+            ['geändert', count($changed)],
+            ['unverändert', count($upstream) - count($new) - count($changed)],
+            ['nicht mehr in der Liste', $vanished->count()],
+        ]);
+
+        $this->reportChanges($changed, $existing);
+
+        if ($this->option('dry-run')) {
+            $this->comment('--dry-run: nichts geschrieben.');
+
+            return self::SUCCESS;
+        }
+
+        $syncedAt = Date::now();
+        $this->persist($upstream, $syncedAt);
+
+        if ($vanished->isNotEmpty()) {
+            $this->handleVanished($vanished);
+        }
+
+        $this->info(sprintf('Bankenliste aktualisiert (Stand %s).', $syncedAt->format('d.m.Y H:i')));
+
+        return self::SUCCESS;
+    }
+
+    private function readFile(string $path): ?string
+    {
+        if (! is_readable($path)) {
+            $this->error("Datei nicht lesbar: $path");
+
+            return null;
+        }
+
+        return (string) file_get_contents($path);
+    }
+
+    private function download(string $url): ?string
+    {
+        if ($url === '') {
+            $this->error('Keine Quelle konfiguriert (stufis.fints.institute_list_url).');
+
+            return null;
+        }
+
+        $this->line("Lade $url ...");
+
+        try {
+            $response = Http::timeout(30)->retry(2, 500, throw: false)->get($url);
+        } catch (ConnectionException $e) {
+            $this->error('Download fehlgeschlagen: '.$e->getMessage());
+
+            return null;
+        }
+
+        if (! $response->successful()) {
+            $this->error(sprintf('Download fehlgeschlagen: HTTP %d', $response->status()));
+
+            return null;
+        }
+
+        return $response->body();
+    }
+
+    /**
+     * @param  array<string, array<string, string|null>>  $upstream
+     */
+    private function persist(array $upstream, Carbon $syncedAt): void
+    {
+        $rows = [];
+        foreach ($upstream as $blz => $institute) {
+            $rows[] = [
+                'blz' => $blz,
+                ...$institute,
+                'synced_at' => $syncedAt,
+                'created_at' => $syncedAt,
+                'updated_at' => $syncedAt,
+            ];
+        }
+
+        DB::transaction(function () use ($rows): void {
+            foreach (array_chunk($rows, self::CHUNK_SIZE) as $chunk) {
+                FintsInstitute::query()->upsert(
+                    $chunk,
+                    ['blz'],
+                    [...FintsInstitute::SYNCED_FIELDS, 'synced_at', 'updated_at'],
+                );
+            }
+        });
+    }
+
+    /**
+     * @param  Collection<int, string>  $vanished
+     */
+    private function handleVanished(Collection $vanished): void
+    {
+        if (! $this->option('prune')) {
+            $this->comment(sprintf(
+                '%d Institute sind nicht mehr in der Liste, bleiben aber erhalten (--prune zum Löschen).',
+                $vanished->count(),
+            ));
+
+            return;
+        }
+
+        // A BLZ someone has a bank access for must survive, list or no list: konto_credentials
+        // references it, so deleting would either fail on the foreign key or take the access
+        // with it. Reported rather than swallowed - a bank leaving the list while we still
+        // bank there is worth a look.
+        $inUse = DB::table('konto_credentials')
+            ->whereIn('blz', $vanished->all())
+            ->pluck('blz')
+            ->unique();
+
+        if ($inUse->isNotEmpty()) {
+            $this->warn(sprintf(
+                'Nicht gelöscht, weil Bankzugänge darauf verweisen: %s',
+                $inUse->implode(', '),
+            ));
+        }
+
+        // Delete exactly the BLZs we found to be absent, rather than everything with an
+        // older synced_at: that column has second precision, so two runs within the same
+        // second would silently prune nothing.
+        $deleted = 0;
+        foreach ($vanished->diff($inUse)->chunk(self::CHUNK_SIZE) as $chunk) {
+            $deleted += FintsInstitute::query()->whereIn('blz', $chunk->all())->delete();
+        }
+
+        $this->comment(sprintf('%d veraltete Institute gelöscht.', $deleted));
+    }
+
+    /**
+     * @param  array<string, array<string, string|null>>  $changed
+     * @param  Collection<string, FintsInstitute>  $existing
+     */
+    private function reportChanges(array $changed, Collection $existing): void
+    {
+        if ($changed === []) {
+            return;
+        }
+
+        // The interesting drift is the endpoint moving, not a bank renaming itself.
+        $movedEndpoints = [];
+        foreach ($changed as $blz => $institute) {
+            $before = $existing->get($blz)?->pin_tan_address;
+            if ($before !== $institute['pin_tan_address']) {
+                $movedEndpoints[] = [$blz, $institute['name'], $before ?? '-', $institute['pin_tan_address'] ?? '-'];
+            }
+        }
+
+        if ($movedEndpoints !== []) {
+            $this->newLine();
+            $this->line('Geänderte PIN/TAN-Endpunkte:');
+            $this->table(['BLZ', 'Bank', 'vorher', 'nachher'], $movedEndpoints);
+        }
+    }
+}

app/Exceptions/LegacyDownloadException.php

@@ -0,0 +1,21 @@
+<?php
+
+namespace App\Exceptions;
+
+use Exception;
+use Symfony\Component\HttpFoundation\Response;
+
+/**
+ * Carries a finished file download out of a legacy Renderer.
+ *
+ * A legacy page renders into an output buffer that LegacyController hands to the app layout,
+ * so a handler cannot simply echo binary content - it would end up inside the HTML. Throwing
+ * unwinds to the controller, which drops the buffer and returns the response untouched.
+ */
+class LegacyDownloadException extends Exception
+{
+    public function __construct(public Response $response)
+    {
+        parent::__construct();
+    }
+}

app/Http/Controllers/Legacy/LegacyController.php

@@ -2,6 +2,7 @@
 
 namespace App\Http\Controllers\Legacy;
 
+use App\Exceptions\LegacyDownloadException;
 use App\Exceptions\LegacyJsonException;
 use App\Exceptions\LegacyRedirectException;
 use App\Http\Controllers\Controller;
@@ -22,6 +23,13 @@ public function bootstrap(): void
 
     public function render(Request $request)
     {
+        // Remembered so the catch blocks can unwind to exactly here. A legacy page renders
+        // inside two nested buffers (this one and the one HTMLPageRenderer opens per
+        // Renderer), and a single ob_get_clean() left the inner one open: PHP then flushed
+        // it at shutdown, appending legacy HTML to the response, and it swallowed the output
+        // of anything running afterwards in the same process.
+        $bufferLevel = ob_get_level();
+
         try {
             ob_start();
             $this->bootstrap();
@@ -39,18 +47,38 @@ public function render(Request $request)
                 // 'sectionTabs' => $this->resolveSectionTabs($request),
             ]);
         } catch (LegacyRedirectException $e) {
+            // Whatever the page printed before deciding to redirect is of no use.
+            $this->discardBufferedOutput($bufferLevel);
+
             return $e->redirect;
         } catch (LegacyJsonException $e) {
-            ob_get_clean(); // throw away all output
+            $this->discardBufferedOutput($bufferLevel);
 
             return response()->json($e->content);
+        } catch (LegacyDownloadException $e) {
+            // A download must not be wrapped in the app layout - drop whatever the page
+            // buffered and hand the file response straight back.
+            $this->discardBufferedOutput($bufferLevel);
+
+            return $e->response;
         } catch (\Exception $exception) {
             // get rid of the already printed html
-            ob_get_clean();
+            $this->discardBufferedOutput($bufferLevel);
             throw $exception;
         }
     }
 
+    /**
+     * Drops every output buffer opened since $downToLevel, leaving that level intact - it
+     * belongs to whoever called us (PHPUnit opens one per test, for instance).
+     */
+    private function discardBufferedOutput(int $downToLevel): void
+    {
+        while (ob_get_level() > $downToLevel) {
+            ob_end_clean();
+        }
+    }
+
     /**
      * The section sub-navigation tabs (Übersicht/TODO/Buchungen) used to be rendered
      * inside the legacy iframe; they are now lifted into the Laravel shell's tab bar.