Skip to content

Fix logging to file and LDAP authentication #884

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
christiankreidl wants to merge 15 commits into from
Closed

Fix logging to file and LDAP authentication #884

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
0cd1b03
fix: use lower-case log_level to fix case-sensitivity bug in string c…
christiankreidl Nov 12, 2025
58a0ba9
fix: add prefix to ConfigKeys key value and edit example config accor…
christiankreidl Nov 12, 2025
95c1bbb
fix: use ConfigKey instead of hard-coded name
christiankreidl Nov 12, 2025
8d404c0
fix: informational log message changed to more appropriate level (DEB…
christiankreidl Nov 12, 2025
803c866
fix: use ConfigKey instead of hard-coded name
christiankreidl Nov 12, 2025
c986428
fix: informational log message changed to more appropriate level (DEB…
christiankreidl Nov 12, 2025
b8a896b
fix(ldap): Rename debug configuration key for consistency
lucs7 Nov 14, 2025
4e9d65e
fix(auth): Updated plugin configuration keys into nested structures
lucs7 Nov 14, 2025
d3e21f1
test(auth): Add comprehensive tests for authentication plugin configu…
lucs7 Nov 14, 2025
d9160bb
fix(config): Preserve unknown subkeys in original structure for valid…
lucs7 Nov 14, 2025
d4866ab
fix(ldap): Update default search filter to be optional with improved …
lucs7 Nov 14, 2025
ad60cfb
docs(auth): Enhance description for Admin Username in Active Director…
lucs7 Nov 14, 2025
903b198
docs(auth): Enhance description for properties in plugins
lucs7 Nov 14, 2025
6ea9259
Merge pull request #1 from lucs7/fix/ldap-config
christiankreidl Nov 18, 2025
35ba693
Merge branch 'develop' into fix-ldap
christiankreidl Nov 18, 2025
File filter

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions lib/Common/Logging/Log.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ private function __construct()
$this->logger = new Logger('app');
$this->sqlLogger = new Logger('sql');

$log_level = Configuration::Instance()->GetKey(ConfigKeys::LOGGING_LEVEL);
$log_level = strtolower(Configuration::Instance()->GetKey(ConfigKeys::LOGGING_LEVEL));

$log_folder = null;
$log_sql = false;
Expand Down Expand Up @@ -103,7 +103,7 @@ public static function Debug($message, $args = [])
*/
public static function Error($message, $args = [])
{
$log_level = Configuration::Instance()->GetKey(ConfigKeys::LOGGING_LEVEL);
$log_level = strtolower(Configuration::Instance()->GetKey(ConfigKeys::LOGGING_LEVEL));
if ($log_level == 'none') {
return;
}
Expand Down Expand Up @@ -148,7 +148,7 @@ public static function Sql($message, $args = [])
}
public static function DebugEnabled()
{
$log_level = Configuration::Instance()->GetKey(ConfigKeys::LOGGING_LEVEL);
$log_level = strtolower(Configuration::Instance()->GetKey(ConfigKeys::LOGGING_LEVEL));
return $log_level != 'none';
}
}
Expand Down
7 changes: 4 additions & 3 deletions lib/Config/Configuration.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -351,8 +351,9 @@ private function RewriteLegacyKeys(array $config): array
if (!call_user_func([$this->_configKeysClass, 'findByKey'], $fullKey)) {
error_log("[CONFIG] Deprecated config key '$fullKey' used. It maps to '$finalKey'. Support for legacy keys will be removed in a future release.");
}

continue;
} else {
// Unknown subkey - preserve in original structure for validation
$rewritten[$key][$subKey] = $subValue;
}
}

Expand Down Expand Up @@ -415,7 +416,7 @@ private function ValidateConfig(array $data, string $path = ''): array
}

if (isset($configDef['choices']) && !array_key_exists($value, $configDef['choices'])) {
error_log("[CONFIG] Invalid value '$value' for '{$fullKey}'. Should be one of the following options: [" . implode(', ', array_map( fn($key, $value) => "{$key} => {$value}", array_keys($configDef['choices']), $configDef['choices'])) . "]");
error_log("[CONFIG] Invalid value '$value' for '{$fullKey}'. Should be one of the following options: [" . implode(', ', array_map(fn($key, $value) => "{$key} => {$value}", array_keys($configDef['choices']), $configDef['choices'])) . "]");
$validated[$key] = $configDef['default'];
continue;
}
Expand Down
10 changes: 8 additions & 2 deletions lib/Config/PluginConfigKeys.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,14 @@ public static function all(): array
public static function findByKey(string $key): ?array
{
foreach (static::all() as $config) {
if (($config['key'] ?? null) === $key) {
return $config;
$configKey = $config['key'] ?? null;
$section = $config['section'] ?? null;

// If this config has a section, ONLY match the section-prefixed key
if ($section) {
if ("{$section}.{$configKey}" === $key) {
return $config;
}
Copy link

Copilot AI Nov 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The findByKey method is incomplete. When a config doesn't have a section, the method should still check if the key matches (without section prefix) and return the config. Currently, if $section is null or empty, the method will never return anything, causing all non-sectioned keys to not be found.

Add an else clause to handle configs without sections:

} else { // Config without section - match key directly if ($configKey === $key) { return $config; } }
Suggested change
}
}
} else {
// Config without section - match key directly
if ($configKey === $key) {
return $config;
}
Copilot uses AI. Check for mistakes.
}
}

Expand Down
58 changes: 30 additions & 28 deletions plugins/Authentication/ActiveDirectory/ActiveDirectory.config.dist.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,46 +2,48 @@

return [
'settings' => [
// comma separated list of ldap servers such as domaincontroller1,controller2
'domain.controllers' => 'mydomain,local',
'activedirectory' => [
// comma separated list of ldap servers such as domaincontroller1,controller2
'domain.controllers' => 'mydomain,local',

// default ldap port 389 or 636 for ssl.
'port' => 389,
// default ldap port 389 or 636 for ssl.
'port' => 389,

// admin user - bind to ldap service with an authorized account user/password
'username' => '',
// admin user - bind to ldap service with an authorized account user/password
'username' => '',

// admin password - corresponding password
'password' => '',
// admin password - corresponding password
'password' => '',

// The base dn for your domain. This is generally the same as your account suffix, but broken up and prefixed with DC=. Your base dn can be located in the extended attributes in Active Directory Users and Computers MMC.
'basedn' => 'ou=uidauthent,o=domain.com',
// The base dn for your domain. This is generally the same as your account suffix, but broken up and prefixed with DC=. Your base dn can be located in the extended attributes in Active Directory Users and Computers MMC.
'basedn' => 'ou=uidauthent,o=domain.com',

// LDAP protocol version
'version' => 3,
// LDAP protocol version
'version' => 3,

// 'true' if 636 was used.
'use.ssl' => 'false',
// 'true' if 636 was used.
'use.ssl' => 'false',

// The full account suffix for your domain. Example: @uidauthent.domain.com.
'account.suffix' => '',
// The full account suffix for your domain. Example: @uidauthent.domain.com.
'account.suffix' => '',

// if ldap auth fails, authenticate against LibreBooking database
'database.auth.when.ldap.user.not.found' => false,
// if ldap auth fails, authenticate against LibreBooking database
'database.auth.when.ldap.user.not.found' => false,

// mapping of required attributes to attribute names in your directory
'attribute.mapping' => 'sn=sn,givenname=givenname,mail=mail,telephonenumber=telephonenumber,physicaldeliveryofficename=physicaldeliveryofficename,title=title',
// mapping of required attributes to attribute names in your directory
'attribute.mapping' => 'sn=sn,givenname=givenname,mail=mail,telephonenumber=telephonenumber,physicaldeliveryofficename=physicaldeliveryofficename,title=title',

// Required groups (empty if not necessary) User only needs to belong to at least one listed (eg. Group1,Group2)
'required.groups' => '',
// Required groups (empty if not necessary) User only needs to belong to at least one listed (eg. Group1,Group2)
'required.groups' => '',

// Whether or not groups should be synced into LibreBooking. When true then be sure that the attribute.mapping config value contains a correct map for groups
'sync.groups' => false,
// Whether or not groups should be synced into LibreBooking. When true then be sure that the attribute.mapping config value contains a correct map for groups
'sync.groups' => false,

// Whether or not to use single sign on
'use.sso' => false,
// Whether or not to use single sign on
'use.sso' => false,

// If the username is an email address or contains the domain, clean it
'prevent.clean.username' => false,
// If the username is an email address or contains the domain, clean it
'prevent.clean.username' => false,
],
],
];
32 changes: 16 additions & 16 deletions plugins/Authentication/ActiveDirectory/ActiveDirectoryConfigKeys.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ class ActiveDirectoryConfigKeys extends PluginConfigKeys
{
public const CONFIG_ID = 'activeDirectory';
public const DOMAIN_CONTROLLERS = [
'key' => 'domain.controllers',
'key' => 'activedirectory.domain.controllers',
'type' => 'string',
'default' => '',
'label' => 'Domain Controllers',
Expand All @@ -15,7 +15,7 @@ class ActiveDirectoryConfigKeys extends PluginConfigKeys
];

public const PORT = [
'key' => 'port',
'key' => 'activedirectory.port',
'type' => 'integer',
'default' => 389,
'label' => 'LDAP Port',
Expand All @@ -24,16 +24,16 @@ class ActiveDirectoryConfigKeys extends PluginConfigKeys
];

public const USERNAME = [
'key' => 'username',
'key' => 'activedirectory.username',
'type' => 'string',
'default' => '',
'label' => 'Admin Username',
'description' => 'Username for binding to LDAP service',
'description' => 'Username for binding to LDAP service. Will have account.suffix appended automatically',
'section' => 'activedirectory'
];

public const PASSWORD = [
'key' => 'password',
'key' => 'activedirectory.password',
'type' => 'string',
'default' => '',
'label' => 'Admin Password',
Expand All @@ -43,7 +43,7 @@ class ActiveDirectoryConfigKeys extends PluginConfigKeys
];

public const BASEDN = [
'key' => 'basedn',
'key' => 'activedirectory.basedn',
'type' => 'string',
'default' => '',
'label' => 'Base DN',
Expand All @@ -52,7 +52,7 @@ class ActiveDirectoryConfigKeys extends PluginConfigKeys
];

public const USE_SSL = [
'key' => 'use.ssl',
'key' => 'activedirectory.use.ssl',
'type' => 'boolean',
'default' => false,
'label' => 'Use SSL',
Expand All @@ -61,7 +61,7 @@ class ActiveDirectoryConfigKeys extends PluginConfigKeys
];

public const VERSION = [
'key' => 'version',
'key' => 'activedirectory.version',
'type' => 'integer',
'default' => 3,
'label' => 'LDAP Protocol Version',
Expand All @@ -70,7 +70,7 @@ class ActiveDirectoryConfigKeys extends PluginConfigKeys
];

public const ACCOUNT_SUFFIX = [
'key' => 'account.suffix',
'key' => 'activedirectory.account.suffix',
'type' => 'string',
'default' => '',
'label' => 'Account Suffix',
Expand All @@ -79,7 +79,7 @@ class ActiveDirectoryConfigKeys extends PluginConfigKeys
];

public const SECTION_AD = [
'key' => 'ad',
'key' => 'activedirectory.ad',
'type' => 'string',
'default' => '',
'label' => 'Active Directory Section',
Expand All @@ -88,7 +88,7 @@ class ActiveDirectoryConfigKeys extends PluginConfigKeys
'is_hidden' => true
];
public const RETRY_AGAINST_DATABASE = [
'key' => 'database.auth.when.ldap.user.not.found',
'key' => 'activedirectory.database.auth.when.ldap.user.not.found',
'type' => 'boolean',
'default' => false,
'label' => 'Retry Against Database',
Expand All @@ -97,7 +97,7 @@ class ActiveDirectoryConfigKeys extends PluginConfigKeys
];

public const ATTRIBUTE_MAPPING = [
'key' => 'attribute.mapping',
'key' => 'activedirectory.attribute.mapping',
'type' => 'string',
'default' => 'sn=sn,givenname=givenname,mail=mail,telephonenumber=telephonenumber,physicaldeliveryofficename=physicaldeliveryofficename,title=title',
'label' => 'Attribute Mapping',
Expand All @@ -106,7 +106,7 @@ class ActiveDirectoryConfigKeys extends PluginConfigKeys
];

public const REQUIRED_GROUPS = [
'key' => 'required.groups',
'key' => 'activedirectory.required.groups',
'type' => 'string',
'default' => '',
'label' => 'Required Groups',
Expand All @@ -115,7 +115,7 @@ class ActiveDirectoryConfigKeys extends PluginConfigKeys
];

public const SYNC_GROUPS = [
'key' => 'sync.groups',
'key' => 'activedirectory.sync.groups',
'type' => 'boolean',
'default' => false,
'label' => 'Sync Groups',
Expand All @@ -124,7 +124,7 @@ class ActiveDirectoryConfigKeys extends PluginConfigKeys
];

public const USE_SSO = [
'key' => 'use.sso',
'key' => 'activedirectory.use.sso',
'type' => 'boolean',
'default' => false,
'label' => 'Use SSO',
Expand All @@ -133,7 +133,7 @@ class ActiveDirectoryConfigKeys extends PluginConfigKeys
];

public const PREVENT_CLEAN_USERNAME = [
'key' => 'prevent.clean.username',
'key' => 'activedirectory.prevent.clean.username',
'type' => 'boolean',
'default' => false,
'label' => 'Prevent Clean Username',
Expand Down
42 changes: 22 additions & 20 deletions plugins/Authentication/CAS/CAS.config.dist.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,34 +2,36 @@

return [
'settings' => [
// '1.0' = CAS_VERSION_1_0, '2.0' = CAS_VERSION_2_0, 'S1' = SAML_VERSION_1_1
'cas.version' => 'S1',
'cas' => [
// '1.0' = CAS_VERSION_1_0, '2.0' = CAS_VERSION_2_0, 'S1' = SAML_VERSION_1_1
'version' => 'S1',

// the hostname of the CAS server
'cas.server.hostname' => 'localhost',
// the hostname of the CAS server
'server.hostname' => 'localhost',

// the port the CAS server is running on
'cas.port' => 443,
// the port the CAS server is running on
'port' => 443,

// the URI the CAS server is responding on
'cas.server.uri' => '',
// the URI the CAS server is responding on
'server.uri' => '',

// Allow phpCAS to change the session_id
'cas.change.session.id' => false,
// Allow phpCAS to change the session_id
'change.session.id' => false,

// Email suffix to use when storing CAS user account. IE, email addresses will be saved to LibreBooking as username@yourdomain.com
'email.suffix' => '@yourdomain.com',
// Email suffix to use when storing CAS user account. IE, email addresses will be saved to LibreBooking as username@yourdomain.com
'email.suffix' => '@yourdomain.com',

// Comma separated list of servers to use for logout. Leave blank to not use cas logout servers
'cas.logout.servers' => '',
// Comma separated list of servers to use for logout. Leave blank to not use cas logout servers
'logout.servers' => '',

// Path to certificate to use for CAS. Leave blank if no certificate should be used
'cas.certificates' => '',
// Path to certificate to use for CAS. Leave blank if no certificate should be used
'certificates' => '',

// bookedAttribute=CASAttribute
'cas.attribute.mapping' => 'givenName=givenName,surName=surname,email=mail,groups=Role',
// bookedAttribute=CASAttribute
'attribute.mapping' => 'givenName=givenName,surName=surname,email=mail,groups=Role',

'cas.debug.enabled' => false,
'cas.debug.file' => '/tmp/phpcas.log',
'debug.enabled' => false,
'debug.file' => '/tmp/phpcas.log',
],
],
];
Loading
Loading