fix: pin remembered-idp cookie expiry to add() result

write() previously recomputed cookieExpiresAt() independently of the
value returned by add(), risking a drift between the logged/returned
expiry and the actual Set-Cookie expiry. write() now accepts an
optional explicit expires so callers can pass the value already
computed by add(). Also documents the Secure/SameSite=None coupling
on write()/clear().

Author: kayjoosten

library/EngineBlock/Corto/Module/Service/ContinueToIdp.php

@@ -153,7 +153,7 @@ private function _persistRememberedChoice(ServiceProvider $sp, $idp, $rememberCh
         }
 
         $result = $rememberedIdpCookie->add($entries, $sp->entityId, $idp->entityId);
-        $rememberedIdpCookie->write($result['entries']);
+        $rememberedIdpCookie->write($result['entries'], $result['expires']);
 
         $log->info(sprintf(
             'WAYF-remember-my-choice set for SP %s and IdP %s and expiry %d',

src/OpenConext/EngineBlock/Service/Wayf/RememberedIdpCookie.php

@@ -184,13 +184,21 @@ public function cookieExpiresAt(): int
         return $this->timeProvider->time() + $this->lifetime;
     }
 
-    /** @param array<string, array{idp: string, expires: int}> $entries */
-    public function write(array $entries): bool
+    /**
+     * @param array<string, array{idp: string, expires: int}> $entries
+     * @param int|null $expires Pass the expiry already computed by add() so the logged/returned
+     *     value and the actual cookie expiry never drift apart. Defaults to a freshly computed
+     *     expiry (e.g. when merely refreshing the cookie's lifetime after normalization).
+     */
+    public function write(array $entries, ?int $expires = null): bool
     {
+        // SameSite=None requires the Secure attribute or browsers silently drop the cookie; this
+        // is only safe because 'cookie.secure' defaults to (and, per SAML's HTTPS requirement,
+        // should always remain) true.
         return $this->cookieService->setCookieWithSameSite(
             self::NAME,
             $this->encode($entries),
-            $this->cookieExpiresAt(),
+            $expires ?? $this->cookieExpiresAt(),
             $this->cookiePath,
             $this->cookieDomain,
             $this->secure,
@@ -201,6 +209,7 @@ public function write(array $entries): bool
 
     public function clear(): bool
     {
+        // See the note on write() about the Secure/SameSite=None coupling.
         return $this->cookieService->clearCookieWithSameSite(
             self::NAME,
             $this->cookiePath,

tests/unit/OpenConext/EngineBlock/Service/Wayf/RememberedIdpCookieTest.php

@@ -250,6 +250,31 @@ public function testWriteAndClearUseSameSiteAwareCookieOperations(): void
         );
     }
 
+    public function testWriteUsesExplicitlyProvidedExpiryWhenGiven(): void
+    {
+        $cookieService = Phake::mock(CookieService::class);
+        Phake::when($cookieService)->setCookieWithSameSite(Phake::anyParameters())->thenReturn(true);
+
+        $cookie = $this->buildCookie(cookieService: $cookieService);
+        $entries = [
+            'https://sp.example.org' => ['idp' => 'https://idp.example.org', 'expires' => self::NOW + self::LIFETIME],
+        ];
+        $explicitExpiry = self::NOW + self::LIFETIME + 1234;
+
+        $this->assertTrue($cookie->write($entries, $explicitExpiry));
+
+        Phake::verify($cookieService)->setCookieWithSameSite(
+            RememberedIdpCookie::NAME,
+            $cookie->encode($entries),
+            $explicitExpiry,
+            self::COOKIE_PATH,
+            self::COOKIE_DOMAIN,
+            true,
+            true,
+            'None'
+        );
+    }
+
     public function testLoadValidEntriesLogsWarningAndClearsCookieOnInvalidInput(): void
     {
         $cookieService = Phake::mock(CookieService::class);