Empty session on callback is reported as "The received state has expired."

We get "The received state has expired." on the OIDC callback a couple of seconds after starting the login, so nothing has actually expired. What's really happening is that the session has no entry for the state that came back.

LoginController::code() reads the timestamp out of the session without checking whether it's there:

https://github.com/nextcloud/user_oidc/blob/main/lib/Controller/LoginController.php#L386-L393

$currentTimestamp = $this->timeFactory->getTime();
$sessionTimestamp = $this->session->get(self::TIMESTAMP . $sessionKeySuffix);
if ($currentTimestamp - $sessionTimestamp > self::LOGIN_FLOW_TIMEOUT) {
    // the state, nonce etc... were stored too long ago, the login flow has expired
    $this->cleanupSessionState($sessionKeySuffix);
    $message = $this->l10n->t('The received state has expired.');
    return $this->build403TemplateResponse($message, Http::STATUS_FORBIDDEN, [], false);
}

The key is oidc.timestamp-<state>, suffixed with the state from the query string. If the callback lands on a session that has no entry for that state, get() returns null, PHP turns null into 0 for the subtraction, and the check becomes <unix timestamp> > 300. That's always true, so you get the expiry page no matter how fast the callback arrived.

Reproducing it

You don't need a working IdP login for this, just a configured provider — the flow never gets as far as the IdP. Two requests, and the only thing that differs is whether the session cookie goes back:

UA='Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/26.0 Safari/605.1.15'
NC=https://<your-nextcloud>

# start the flow, keep the cookies, pull the state out of the authorization URL
curl -s -c jar -A "$UA" "$NC/apps/user_oidc/login/1" -o login.html
STATE=$(grep -o 'state=[A-Za-z0-9]*' login.html | head -1 | cut -d= -f2)

# same state, seconds later, WITHOUT the session cookie
curl -s -A "$UA" "$NC/apps/user_oidc/code?state=$STATE&code=dummycode"
#   403  "The received state has expired."

# identical, WITH the session cookie
curl -s -b jar -A "$UA" "$NC/apps/user_oidc/code?state=$STATE&code=dummycode"
#   403  "Failed to contact the OIDC provider token endpoint: Code not valid"

The second one gets through the timestamp check and the state comparison and only fails at the token exchange, which is what you'd want given the code is made up. The first claims expiry about two seconds into a five minute window.

Why it's a problem

The message points at the wrong thing. We spent a while looking for a timeout that wasn't there — we'd measured 1-11 seconds between /apps/user_oidc/login/1 and /apps/user_oidc/code against a 300 second limit — before working out that the sessions simply weren't coming back on the callback.

There's also no logging in this branch, so the server log stays empty through all of it. The check just below, $storedState !== $state, fails in this situation too and it does log a warning, but the timestamp check runs first and swallows it.

Which brings up a second thing: I think the two checks are in the wrong order. Whether a login flow expired only means anything once you know the state is one you issued. As it stands, an unknown, stale or replayed state also comes back as "expired".

And it's self-reinforcing, because cleanupSessionState() is called inside this branch and drops all six oidc.* keys for that suffix. Any retry carrying the same state is then guaranteed to hit it as well.

Possible fix

Check for null explicitly, and let the state comparison go first:

$sessionTimestamp = $this->session->get(self::TIMESTAMP . $sessionKeySuffix);
if ($sessionTimestamp === null) {
    $this->logger->warning('Login flow timestamp missing from session; the session had no entry for this state', ['state' => $state]);
    $this->cleanupSessionState($sessionKeySuffix);
    $message = $this->l10n->t('The session was lost during login. Please try again.');
    return $this->build403TemplateResponse($message, Http::STATUS_FORBIDDEN, [], false);
}

Happy to send a PR if that looks like the right direction.

Versions

• user_oidc 8.10.1, unchanged in 8.11.0 and on main as of today
• Nextcloud 34.0.1 (nextcloud:34.0.1-apache)
• Sessions in Redis via phpredis

What led us here

Our own trigger is Safari specific: the UA branch added in #358 still loses the session for us on Safari 26.6.2, while Chrome against the same server never fails. That's a separate problem and I'm not making any claim about it here. The reproduction above doesn't depend on the browser.

Start in lib/Controller/LoginController.php, in LoginController::code() around the timestamp and stored-state checks; use the two curl requests in the issue to compare callbacks with and without the session cookie. Done means a missing-session callback is distinguished from an expired flow, state validation is considered before expiry, and a normal session reaches the token-exchange failure shown by the reproduction.