v3.78

lib/Password.php

Line 109/110:

// allow plugins to add additional password rules
$errors = \Plugin::filter('login_newPasswordErrors', $errors, $passwordText);

Prior to this, $error is an array e.g.

$errors[] = t("Password cannot start with $hashPrefix");

So, if the login_newPasswordErrors returns anything, it overwrites any $errors[] that may have also been triggered.

I've applied this, and it allows both the core password errors AND anything added by the filter to be combined into the one error array:

$extraErrors = \Plugin::filter('login_newPasswordErrors', $errors, $passwordText);     
$errors = array_merge($errors, $extraErrors);

For context (i.e. because maybe I'm using this addFilter wrong..!) my login_newPasswordErrors filter is set up like this:

addFilter('login_newPasswordErrors', 'customPasswordErrors', null, 4);

and the customPasswordErrors function returns an array.

Cheers

Rob

In /cmsAdmin/lib/menus/general_server_info.php

Line 307:

$dbHaveSSL         = mysql_get_query("SHOW VARIABLES WHERE Variable_name IN ('have_ssl')")['Value']; // Disabled, Yes, No

Can return:

Warning: Trying to access array offset on null

In some infrastructure / networking cases..

I've changed it locally to:

$dbHaveSSLresult = mysql_get_query("SHOW VARIABLES WHERE Variable_name = 'have_ssl'");   

$dbHaveSSL = $dbHaveSSLresult['Value'] ?? null;

to prevent the Warning.

Can it be patched into the next version? :-)

Cheers

Not sure on your end goal with $recordNum, but that code will still pass fbclid (and those others) to your $recordNum, which could potentially still cause problems down the line.

Without seeing what you then do with $recordNum, I'm guessing it's used in a mysql_get/select to get the page record?

Cheers

/home/luxurychartergroup/luxurychartergroup.com/fr/yacht-type.php -> line 32

/home/luxurychartergroup/luxurychartergroup.com/404.php -> line 2

It looks like your site is grabbing the GET variable from the URL (in this case fbclid) and then using that to find a page/record in your database. It can't, so 404s.

My guess is your code is something like:

If there's a GET variable, use that to find the page

...but you're doing it for ANY GET variable, so it's using fbclid instead of whatever correct variable you use to get the correct page.

Proof of Concept:

https://www.luxurychartergroup.com/fr/yacht-type.php/sailing-1/?foo=bar

This also errors and 404s.

You need to harden your code to correctly define which GET variable you want to use, rather than any. If you show those lines, it'll likely be an easy fix.

Cheers

Rob

Does this release include the inclusion of columnEncryptionKey into the Env variables? And, the return of the login_isValidLogin hook (or similar) so we can add third party 2FA?

Cheers

Rob

Thanks - and yup, this has fixed it.

Going from:

NULL DEFAULT \'0000-00-00 00:00:00\'

to:

NULL DEFAULT CURRENT_TIMESTAMP

Thanks :-)

Both PROD and DEV are inside Docker containers, so maybe that's contributing to this?

phpinfo():

Configuration File (php.ini) Path: /usr/local/etc/php

php --ini:

Configuration File (php.ini) Path: /usr/local/etc/php
Loaded Configuration File:         (none)
Scan for additional .ini files in: /usr/local/etc/php/conf.d
Additional .ini files parsed:      /usr/local/etc/php/conf.d/docker-php-ext-gd.ini,
/usr/local/etc/php/conf.d/docker-php-ext-mysqli.ini,
/usr/local/etc/php/conf.d/docker-php-ext-sodium.ini,
/usr/local/etc/php/conf.d/docker-php-ext-zip.ini,
/usr/local/etc/php/conf.d/uploads.ini

So php --ini inside the container doesn't seem to report a loaded config file, but that's ok (I think) as we haven't seen any problems with this config for years now...! Pretty sure everything is well handled by the docker container base image. We are setting a few non-defaults in uploads.ini

Thoughts?

Cheers
Rob

Yeah I wondered that.

The MySQL type for the dateLogged field is, as reported on admin.php?menu=database&action=editTable&tableName=_error_log is

datetime NOT NULL DEFAULT 0000-00-00 00:00:00

and in the schema file:

  'dateLogged' => [
    'customColumnType' => 'TIMESTAMP DEFAULT CURRENT_TIMESTAMP',
    'label' => 'Date',
    'type' => 'date',
    'isSystemField' => 1,
    'defaultDate' => '',
    'defaultDateString' => '',
    'showTime' => 1,
    'showSeconds' => 1,
    'actualColumnType' => 'datetime NOT NULL DEFAULT \'0000-00-00 00:00:00\'',
  ],

And all the entries in the MySQL table have dateLogged (and updatedDate) as 0000-00-00 00:00:00.

I see in other schema files, system date fields are:

  'updatedDate' => [
    'type' => 'none',
    'label' => 'Last Updated',
    'isSystemField' => '1',
  ],

So is there perhaps an issue with our schema config for _error_log.schema.php?

This install was recently upgraded from a very old verison (a good year + old) to 3.76 and then 3.77.

Cheers

Rob

When loading /admin.php?menu=admin&action=general, it errors with:

absPath(): Argument #1 ($inputPath) must be of type string, bool given, called in /var/www/html/clinicAdmin/lib/menus/admin/general_server_info.php on line 425

as per attached screenshot.

Coming from:

general_server_info.php L425:

$loadedPhpIni  = absPath(php_ini_loaded_file(), \CMS::$rootDir);

file_functions L10:

function absPath(string $inputPath, string $baseDir = '.'): string {

Not sure if it's something specific to my server config causing this, but it's happening in both DEV and PROD.

Obviously not fatal, but annoying none the less as it triggers a Dev Log error email everytime someone accesses this page.

Cheers

Rob

Errors logged into _error_log are being inserted without a dateLogged, so they appear as "never" in the UI, as per attached screenshot.

Looking at

function errorDB_addErrorRecord($colsToValues):

and

function _errorlog_logErrorRecord

the dateLogged variable is not set anywhere within either of those.

Would likely make best sense to set it in the:

 //  create log record data
$colsToValues = [

errorlog_functions.php line 313

Cheers!

Rob

We used to use the plugin hook login_isValidLogin to add a third party 2FA to the CMS login process.

The hooked function would validate an extra input (the OTP code) on the login page (which itself is added by the login_content hook).

The login_content plugin hook still exists, so we can display the OTP field on the login screen but the login_isValidLogin is gone from the latest versions of CMS, and looking through the new User and Password classes, there doesn't seem to be any hook to add in this extra check.

Perhaps it'd go into the getLoginUserRecord or Password::isValid functions.

Is there a way to do this from the latest version (v3.76)? If not, what's the chances of having it added in the next update? :-)

Or, even better, have 2FA/OTP baked into CMSB?

Thanks
Rob

Really like the introduction of the .env.php files.

Can columnEncryptionKey (mysql) be added to the fuction getSettingsToEnvMap, so that all sensitive data is removed from the settings.dat.php files?

Ideally, also smtp_hostname and adminUrl

Cheers

Rob

Is there a built-in way to disable the Website/Code Backups option at a global (settings.dat.php) level? I can definitely see how it's handy for some, but with git and docker workflows, it's unnecessary and TBH, for security reasons, I don't want someone to be able to download the codebase just by having a CMS admin login.

I can remove it manually, but a built-in option would be great so I don't have to manually remove it again after any future upgrades.

Thanks

Rob

viewer_functions.php, line 568

Was:

  // create query
    $query = "SELECT COUNT(*)\n";
    $query .= "FROM `$TABLE_PREFIX{$options['tableName']}` as `{$options['tableName']}`\n";
    $query .= $LEFT_JOIN;
    $query .= "$where\n";
    $query .= (!empty($options['groupBy'])) ? " GROUP BY {$options['groupBy']}" : '';
    $query .= (!empty($options['having']))  ? " HAVING {$options['having']}" : '';
    $query .= (!empty($options['orderBy'])) ? " ORDER BY {$options['orderBy']}" : '';

Now:

// create query
 $query = "SELECT COUNT(*)\n";
    $query .= "FROM `$TABLE_PREFIX{$options['tableName']}` as `{$options['tableName']}`\n";
    $query .= $LEFT_JOIN;
    $query .= "$where\n";
    $query .= (!empty($options['groupBy'])) ? " GROUP BY {$options['tableName']} . {$options['groupBy']}" : '';
    $query .= (!empty($options['having']))  ? " HAVING {$options['having']}" : '';
    $query .= (!empty($options['orderBy'])) ? " ORDER BY {$options['tableName']} . {$options['orderBy']}" : '';

But this then can cause another error when a custom orderBy is defined already with a fully qualified field.

So I scrapped that idea and then I thought... arguably, SELECT COUNT doesn't need an orderBy as it will make no difference to the results of the COUNT, so just removed it entirely from this SELECT COUNT query.

groupBy can cause this same ambiguous error but removing this from the COUNT does cause different results so this may need a better fix. For now, I have fixed this by fully qualifying it in the getRecords as I only have a few places.

I have not tested having as I have never used that.

This works:

SELECT `primary_table`.*,
`secondary_table`.`num` AS `secondary_table.num`,
`secondary_table`.`createdDate` AS `secondary_table.createdDate`
FROM `primary_table` as `records`
LEFT JOIN `secondary_table` AS `secondary_table` ON (primary_table.num = secondary_table.linked_field)
ORDER BY createdDate

This doesn't work:

SELECT COUNT(*)
FROM `primary_table` as `records`
LEFT JOIN `secondary_table` AS `secondary_table` ON (primary_table.num = secondary_table.linked_field)
ORDER BY createdDate

and results in the error

column createdDate in order clause is ambiguous

And if I remove the COUNT entirely for testing

// comment out for testing
// $totalRecords = mysql_get_query($countQuery, true)[0];

from /lib/viewer_functions.php line 612, I do not get the error :-)

The previous CMS used the following for $totalRecords within function_getRecords_loadResults:

SELECT FOUND_ROWS()

The new CMS uses the following:

SELECT COUNT(*) FROM 

Previously it was querying the found rows from the original MySQL select query to get the count. Now, it's running its own separate COUNT query.

Looking at the full MySQL statements above, in the first SELECT, createdDate for the secondary_table is defined AS secondary_table.createdDate, removing the ambiguity. In the second SELECT COUNT, it is not defining each secondary_table field AS, and this is likely the cause for the ambiguity.

function _getRecords_getQuery includes // add fieldnames to SELECT resulting in the x AS y in the SELECT. (viewer_functions.php L398)

function _getRecords_getCountQuery however doesn't include this full SELECT AS

Has SELECT FOUND_ROWS() been replaced by SELECT COUNT(*) for efficiency reasons?

FYI all line numbers in ^^ are based on CMSB v3.65.

So is the SQL generated by the previous and current versions of CMSB the same?

Correct, which makes me think as well that it's MySQL config related.

And if so do you know if the MySQL/MariaDB might have been updated at the same time as you updated CMSB?

MySQL instance is unchanged. MySQL is running in it's own docker container which is untouched by the CMSB upgrade.

Do you know if you have a server that's using the same version of MySQL/MariaDB but on the older CMSB that's still working?  Or can you replicate the working condition on the same server with a temporary old cmsb install?  

Yes, LEFT JOINS work correctly if I switch back to the old CMSB version. I have the CMSB upgrade on a new branch so easy to switch back, and with all other environmental factors the same:

Old CMSB codebase: LEFT JOINs work

New CMSB codebase: LEFT JOINs result in ambiguous ORDER BY

I'm thinking either there's been some changes in MySQL/MariaDB that has them be more strict about requiring fully qualified table.column names in the ORDER BY for joins, or maybe some of the more stricter MySQL settings we're using are causing the error. 

I agree :-) I think it's more likely related to the stricter MySQL settings the new CMSB is using, as I've had no changes to my actual MySQL instance.

Are you able to replicate? If not, then I wonder if we need to compare MySQL server settings?

I'll do some more digging at my end too, but also happy to run some tests if you can point in whatever direction.

Cheers

Rob

Very similar to this issue, https://www.interactivetools.com/forum/forum-posts.php?postNum=2246572, after upgrading to v3.66 all getRecords calls I have which include a LEFT JOIN throw a MySQL Error:

MySQL Error(1052): Column 'createdDate' in order clause is ambiguous in...

In this case, createdDate is the default ordering field as defined in the schema settings. The same occurs if the default order is left as dragSortOrder.

This causes every getRecords which contains a leftJoin to error unless it has an explicit orderBy value defined at getRecords stage.

list($records, $Metadata) = getRecords(array(
    'tableName'   => 'primary_table',
    'leftJoin'   => array(
        "secondary_table" => "ON (primary_table.link_field = secondary_table.num)",
    ),
));

By explicitly applying orderBy to the getRecords function resolves the issue:

list($records, $Metadata) = getRecords(array(
    'tableName'   => 'primary_table',
   'orderBy'     => 'primary_table.createdDate',
    'leftJoin'   => array(
        "secondary_table" => "ON (primary_table.link_field = secondary_table.num)",
    ),
));

It was not an issue in previous versions of CMSB.

Full debugSQL:

SELECT `primary_table`.*,
`secondary_table`.`num` AS `secondary_table.num`,
`secondary_table`.`createdDate` AS `secondary_table.createdDate`,
`secondary_table`.`name` AS `secondary_table.name`
FROM `primary_table` as `primary_table`
LEFT JOIN `secondary_table` AS `secondary_table` ON (primary_table.link_field = secondary_table.num)

 ORDER BY createdDate DESC

The issue being

ORDER BY createdDate DESC

Because of the LEFT JOIN present, it needs to be fully qualified, e.g.:

ORDER BY primary_table.createdDate DESC

Thinking about a solution:

function getRecords {
if (!array_key_exists('orderBy', $options)) { $options['orderBy'] = _getOrderByFromUrl($schema) ?? $schema['listPageOrder']; }
...

$schema['listPageOrder'] needs to be fully qualified IF a LEFT JOIN is present.

The only current workaround is to explicitly apply the fully qualified field in the getRecords call using 'orderBy' but this impacts 100s of places of legacy code (because it wasn't an issue with previous version of CMSB) so ideally would love to see if fixed at getRecords level :-)

On CMSB v3.57, the ORDER BY createdDate DESC generated by getRecords is the same, i.e. not fully qualified when using a LEFT JOIN, yet works, so not entirely sure what's causing this error.

Thanks

Rob

I'm running MySQL Version 8.0.19 and the background tasks menu was throwing a fatal MySQL error:

admin.php?menu=admin&action=bgtasks

Notice: MySQL Error: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'function = "{functionName}" ORDER BY num DESC LIMIT 1' at line 1 - in mysql_functions.php on line 210 by mysql_get_query() in /{path}/admin/lib/common.php on line 278

This is caused by 'function' being a MySQL Reserved Word

Fix:

admin/lib/menus/admin/backgroundTasks.php

line 293

Was:

$latestLog = mysql_get('_cron_log', null, ' function = "' .mysql_escape($record['function']). '" ORDER BY num DESC');

Now:

$latestLog = mysql_get('_cron_log', null, ' `function` = "' .mysql_escape($record['function']). '" ORDER BY num DESC');

i.e. addition of backticks around function

So effectively going from:

SELECT * FROM `cmsb__cron_log` WHERE function = "{functionName}" ORDER BY num DESC LIMIT 1;

to

SELECT * FROM `cmsb__cron_log` WHERE 'function' = "{functionName}" ORDER BY num DESC LIMIT 1;

Also need to apply the same fix to:

admin/cron.php

Line 108:

Was:

  $jobLastLogRecord = mysql_get('_cron_log', null, ' function = "' .mysql_escape($cron['function']). '" ORDER BY num DESC');

Now:

$jobLastLogRecord = mysql_get('_cron_log', null, ' `function` = "' .mysql_escape($cron['function']). '" ORDER BY num DESC');

I've manually updated my file, but I thought I'd share here in case anyone else has the same issue, and it would be great if this could be patched in the next version.

Cheers

Rob