Skip to content

fix(core): keep Horde_Session::$begin readable after close() (imp#88)#197

Merged
ralflang merged 1 commit into
FRAMEWORK_6_0from
test/imp88-readonly-download-auth
Jul 10, 2026
Merged

fix(core): keep Horde_Session::$begin readable after close() (imp#88)#197
ralflang merged 1 commit into
FRAMEWORK_6_0from
test/imp88-readonly-download-auth

Conversation

@TDannhauer

Copy link
Copy Markdown
Contributor

Summary

  • Fix Horde_Session::$begin returning 0 after close(), which broke the session.max_time auth check for read-only service scripts.

Motivation

Reported in horde/imp#88: downloading an attachment in IMP returned a 500 (IMP_Exception: User is not authenticated.), even for a fully logged-in user. The attachment download service boots with a read-only session and calls close() right after bootstrap.

Horde_Session::__get('begin') returned 0 whenever the shim's _active flag was false - which close() sets, without clearing the underlying session data. Horde_Registry::checkExistingAuth() uses $session->begin for the session.max_time check:

if (!empty($conf['session']['max_time'])
    && (($conf['session']['max_time'] + $session->begin) < time())) {
    // -> auth check fails
}

With begin collapsed to 0, max_time + 0 is always less than the current Unix timestamp, so this check failed unconditionally for every closed, read-only request once session.max_time was configured - regardless of how recently the user actually authenticated.

Changes

  • Horde_Session::__get('begin') now reads through to the modern HordeSession unconditionally. Its data survives close() untouched, so this is safe for both the closed-but-was-active case and the never-started case (falls back to 0 via the existing null handling).
  • Added HordeSessionShimBeginTest covering both cases.

Test plan

  • vendor/bin/phpunit test/Unit/Session/HordeSessionShimBeginTest.php (2/2 passing)
  • Reporter confirmed on their own deployment that this fix resolves the attachment download 500 (see issue comments)

Read-only service scripts (the attachment download service, and
potentially other read-only endpoints) call close() right after
session bootstrap. close() flips the legacy Horde_Session shims
`_active` flag to false but never clears the underlying session
data - the modern HordeSession keeps it intact.

Horde_Session::__get(begin) short-circuited to 0 whenever `_active`
was false, so Registry::checkExistingAuth()s `session.max_time`
check (`max_time + $session->begin < time()`) incorrectly rejected
every closed, read-only request once session.max_time was
configured: max_time + 0 is always less than the current Unix
timestamp. That produced "User is not authenticated" on attachment
downloads even for a fully logged-in user.

Read through to the modern HordeSession unconditionally instead -
its data survives close() untouched. Add HordeSessionShimBeginTest
covering both the post-close and never-started cases.
@ralflang ralflang merged commit a955883 into FRAMEWORK_6_0 Jul 10, 2026
1 check failed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants