Skip to content

Fix authentication username check in versionCallback#103

Open
TDannhauer wants to merge 1 commit into
FRAMEWORK_6_0from
fix/ensure_EAS_Max_Protocol_Permission_is_applied
Open

Fix authentication username check in versionCallback#103
TDannhauer wants to merge 1 commit into
FRAMEWORK_6_0from
fix/ensure_EAS_Max_Protocol_Permission_is_applied

Conversation

@TDannhauer
Copy link
Copy Markdown
Contributor

Fix per-user ActiveSync EAS version permissions in versionCallback()
Summary
Per-user horde:activesync:version permissions did not raise the server’s advertised EAS ceiling when the global conf['activesync']['version'] was lower. Clients kept negotiating the global maximum (for example 14.1) even when a user or group was allowed a higher version (for example 16.0).

Problem
Horde_Core_ActiveSync_Driver::versionCallback() runs before authentication on each ActiveSync request. It should read the client username from Horde_ActiveSync_Credentials, resolve horde:activesync:version for that principal, and call Horde_ActiveSync::setSupportedVersion() when a valid permission is present.

In practice the override was skipped on authenticated requests. The server stayed on the global default, so MS-ASProtocolVersions and related negotiation did not reflect per-user permissions.

Root cause
versionCallback() used !empty($credentials->username) to detect a Basic-auth username. Horde_ActiveSync_Credentials exposes username via __get() and does not implement __isset(). For magic properties, PHP’s empty() does not invoke __get() in this situation, so a valid username was treated as missing.

The method then fell back to getUser(), which is still empty before authenticate() completes. The callback returned early and never applied the permission-based ceiling.

Solution
Read the credential username explicitly instead of using empty() on the magic property:

Treat false as “no username provided” (existing __get() sentinel).
Treat an empty string the same way.
Otherwise cast the username to string and continue with the existing permission resolution and setSupportedVersion() path.
No change to permission storage, global defaults, or client-side version selection.

@TDannhauer TDannhauer requested review from amulet1 and ralflang May 14, 2026 11:29
@ralflang
Copy link
Copy Markdown
Member

Any unit tests?

@TDannhauer
Copy link
Copy Markdown
Contributor Author

Good one, I forgot. Will provide some

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