From 5ab967319cf9efac679b6fb670309a9adadf6eb7 Mon Sep 17 00:00:00 2001 From: DerekCrannaford <43056286+DerekCrannaford@users.noreply.github.com> Date: Mon, 22 May 2023 09:14:48 +1000 Subject: [PATCH 1/5] fix: drop encryption migration support (#191) * Bump config and block out legacy code * Fix revealed typo * Restore dropped var * Clarify deprecatino in comments --------- Co-authored-by: Derek Crannaford --- system/config.php | 3 +- .../actions/migration/migrationmessage.php | 5 +- ...190718121212-AdminSecurityAesToOpenssl.php | 551 +++++++++--------- 3 files changed, 292 insertions(+), 267 deletions(-) diff --git a/system/config.php b/system/config.php index c364d890a..96cde5f4e 100755 --- a/system/config.php +++ b/system/config.php @@ -32,9 +32,10 @@ /** * Otherwise, SSL will be used with KEY & IV from config, * if system has been upgraded per this migration: + * Config::set('system.encryptionMigration', 'AdminSecurityAesToOpenssl'); + * OR simply by key, for systems > PHP8 where no forward path from AES can be migrated! */ -Config::set('system.encryptionMigration', 'AdminSecurityAesToOpenssl'); // Adds ability to disable help and search Config::set('system.help_enabled', true); diff --git a/system/modules/admin/actions/migration/migrationmessage.php b/system/modules/admin/actions/migration/migrationmessage.php index e69d7c57b..a78516277 100644 --- a/system/modules/admin/actions/migration/migrationmessage.php +++ b/system/modules/admin/actions/migration/migrationmessage.php @@ -14,7 +14,8 @@ function migrationmessage_GET(Web $w) // Pretext Page File $w->ctx("migration_module", Request::string('module')); - $w->ctx("migration_filename", Request::string('filename')); + $migration_filename = Request::string('filename'); + $w->ctx("migration_filename", $migration_filename ); $migration_path = Request::string('path'); @@ -28,7 +29,7 @@ function migrationmessage_GET(Web $w) $migration_preText = $migration->preText(); $w->ctx("migration_preText", $migration_preText); } else { - $w->error("Migration Class not found for message", "/admin-migrations#batch"); + $w->error("Migration Class not found for message", "/admin-migration#batch"); } } } diff --git a/system/modules/admin/install/migrations/20190718121212-AdminSecurityAesToOpenssl.php b/system/modules/admin/install/migrations/20190718121212-AdminSecurityAesToOpenssl.php index 0c0992c4b..10fcc2df3 100644 --- a/system/modules/admin/install/migrations/20190718121212-AdminSecurityAesToOpenssl.php +++ b/system/modules/admin/install/migrations/20190718121212-AdminSecurityAesToOpenssl.php @@ -1,294 +1,317 @@ w)->error($err); - throw new Exception($err); - } - - $encryption_key = Config::get('system.encryption.key'); - //$encryption_iv = Config::get('system.encryption.iv'); - - if (empty($encryption_key)) { // || empty($encryption_iv)) { - $err = 'Encryption key/iv is not set'; - LogService::getInstance($this->w)->error($err); - throw new Exception($err); - } - - /* DB table_name and ObjectName don't always match - * (e. g. table name: channel_email_option, object name: EmailChannelOption), - * therefore it is not always possible to get a DB object from table name. - * to do: make them match each other? (e. g. channel_email_option -> ChannelEmailOption)*/ - $table = $this->w->db->query("select table_name, column_name from information_schema.columns - where table_schema='$db' and column_name like 's\_%';")->fetchAll(); - - if (empty($table)) { - return; - } - - $this->w->db->startTransaction(); - - try { - foreach ($table as $row) { - foreach ($row as $key => $val) { - if (is_numeric($key)) { - unset($row[$key]); - } - } - - $passwordSalt = null; - - $tableName = $row['table_name']; - $columnName = $row['column_name']; - - if ($tableName == "channel_email_option") { - $passwordSalt = hash("md5", $this->w->moduleConf("channels", "__password")); - } else if ($tableName == "report_connection") { - $passwordSalt = hash("md5", $this->w->moduleConf("report", "__password")); - } else { - // Config::set('system.password_salt', md5('override this in your project config')); - $passwordSalt = Config::get('system.password_salt'); - } - - $tbl = $this->w->db->query("select id, $columnName from $tableName")->fetchAll(); - foreach ($tbl as $r) { - foreach ($r as $k => $v) { - if (is_numeric($k)) { - unset($r[$k]); - } - } - - $decrypted = null; - $encrypted = null; - - if ($up) { - $decrypted = AESdecrypt($r[$columnName], $passwordSalt); - $encrypted = SystemSSLencrypt($decrypted); - } else { - $decrypted = SystemSSLdecrypt($r[$columnName]); - $encrypted = AESencrypt($decrypted, $passwordSalt); - } - - $this->w->db->update($tableName, [$columnName => $encrypted])->where('id', $r['id'])->execute(); - } - } - - $this->w->db->commitTransaction(); - } catch (Exception $e) { - $this->w->db->rollbackTransaction(); - throw $e; - } - +/*** + * Deprecated at 202305 / v5.0 / PHP 8.1 + * This migration nolonger viable, because: + * - it errors out depending on case sensitivity settings of MySQL DB + * - the error state is intermittent depending on sequence of module installation vs migration execution + * - it can't run anyway, from any recent merged branches, because "system->functions->AES_encryption" code has been purged * + */ + + +class AdminSecurityAesToOpenssl extends CmfiveMigration +{ + + // private function migrate($up = true) + // { + + // /* STEPS: + // Ensuring the client is on PHP 7.0 + // Ensuring the migration has not already run in the past + // When the migration runs, it must decrypt all DbObject fields starting with "s_" using PHPAES, and reencrypt using openssl + // Encryption should be verified by decrypting using openssl and verifying that the original content is preserved + // The entire encryption operation should be atomic, if one entry fails to re-encrypt for any reason, the entire operation must rollback + // If it is successful, we would then need to do a few spot checks in modules that use the encryption service like channels and the hosting module + // */ + + // // get only tables with encrypted values + + // $db = Config::get("database.database"); + // if (empty($db)) { + // $err = 'Database config not set'; + // LogService::getInstance($this->w)->error($err); + // throw new Exception($err); + // } + + // $encryption_key = Config::get('system.encryption.key'); + // //$encryption_iv = Config::get('system.encryption.iv'); + + // if (empty($encryption_key)) { // || empty($encryption_iv)) { + // $err = 'Encryption key/iv is not set'; + // LogService::getInstance($this->w)->error($err); + // throw new Exception($err); + // } + + // /* DB table_name and ObjectName don't always match + // * (e. g. table name: channel_email_option, object name: EmailChannelOption), + // * therefore it is not always possible to get a DB object from table name. + // * to do: make them match each other? (e. g. channel_email_option -> ChannelEmailOption)*/ + // $table = $this->w->db->query("select table_name, column_name from information_schema.columns + // where table_schema='$db' and column_name like 's\_%';")->fetchAll(); + + // if (empty($table)) { + // return; + // } + + // $this->w->db->startTransaction(); + + // try { + // foreach ($table as $row) { + // foreach ($row as $key => $val) { + // if (is_numeric($key)) { + // unset($row[$key]); + // } + // } + + // $passwordSalt = null; + + // $tableName = $row['table_name']; + // $columnName = $row['column_name']; + + // if ($tableName == "channel_email_option") { + // $passwordSalt = hash("md5", $this->w->moduleConf("channels", "__password")); + // } else if ($tableName == "report_connection") { + // $passwordSalt = hash("md5", $this->w->moduleConf("report", "__password")); + // } else { + // // Config::set('system.password_salt', md5('override this in your project config')); + // $passwordSalt = Config::get('system.password_salt'); + // } + + // $tbl = $this->w->db->query("select id, $columnName from $tableName")->fetchAll(); + // foreach ($tbl as $r) { + // foreach ($r as $k => $v) { + // if (is_numeric($k)) { + // unset($r[$k]); + // } + // } + + // $decrypted = null; + // $encrypted = null; + + // if ($up) { + // $decrypted = AESdecrypt($r[$columnName], $passwordSalt); + // $encrypted = SystemSSLencrypt($decrypted); + // } else { + // $decrypted = SystemSSLdecrypt($r[$columnName]); + // $encrypted = AESencrypt($decrypted, $passwordSalt); + // } + + // $this->w->db->update($tableName, [$columnName => $encrypted])->where('id', $r['id'])->execute(); + // } + // } + + // $this->w->db->commitTransaction(); + // } catch (Exception $e) { + // $this->w->db->rollbackTransaction(); + // throw $e; + // } + // } + + public function up() + { + + // if ($this->checkBlankContemporyInstall()) { + // return $this->w->migrating = true; + // } + + // if (!($this->checkMigrationClass()['pass'] // config must name this migration + // && $this->checkMigrationStatus()['pass'] // named migration(this) must never have run + // && $this->checkPHPversion()['pass'] // <5.3 and SSL won't work! >7.0 AES wont work! + // && $this->checkSSLKeys()['pass'])) { // no SSL without a key + // $err = "System is not suitable for " . get_class($this) . " migration"; + // LogService::getInstance($this->w)->error($err); + // throw new Exception($err); + // } + // $this->w->migrating = true; + // $this->migrate(); + // // finally, if no EXCEPTION --> migration named in config will be in table under 'ADMIN' + // // DbObject --> detects migration entry & uses SSL wrappers from functions.PHP } - public function up() { - - if($this->checkBlankContemporyInstall()) { - return $this->w->migrating = true; - } - - if(!( $this->checkMigrationClass()['pass'] // config must name this migration - && $this->checkMigrationStatus()['pass'] // named migration(this) must never have run - && $this->checkPHPversion()['pass'] // <5.3 and SSL won't work! >7.0 AES wont work! - && $this->checkSSLKeys()['pass'] )) { // no SSL without a key - $err = "System is not suitable for ".get_class($this)." migration"; - LogService::getInstance($this->w)->error($err); - throw new Exception($err); - } - $this->w->migrating = true; - $this->migrate(); - // finally, if no EXCEPTION --> migration named in config will be in table under 'ADMIN' - // DbObject --> detects migration entry & uses SSL wrappers from functions.PHP + public function down() + { + + // if ($this->checkBlankContemporyInstall()) { + // return $this->w->migrating = true; + // } + + // if (($this->checkMigrationStatus()['pass'] || (!$this->checkPHPversion()['pass']))) { + // // this MUST throw exception, or migration will be registered as rollback, + // // regardless that encryption was not reversed + // // BECAUSE : down() cannot return failure! + // $err = "System cannot rollback " . get_class($this) . " migration"; + // LogService::getInstance($this->w)->error($err); + // throw new Exception($err); + // } else { + // $this->w->migrating = true; + // $this->migrate(false); + // } } - public function down() { - - if($this->checkBlankContemporyInstall()) { - return $this->w->migrating = true; - } - - if(($this->checkMigrationStatus()['pass'] || (!$this->checkPHPversion()['pass']))) { - // this MUST throw exception, or migration will be registered as rollback, - // regardless that encryption was not reversed - // BECAUSE : down() cannot return failure! - $err = "System cannot rollback ".get_class($this)." migration"; - LogService::getInstance($this->w)->error($err); - throw new Exception($err); - } else { - $this->w->migrating = true; - $this->migrate(false); - } - } - private function checkPHPversion() { + // private function checkMigrationStatus() + // { + // $result = $this->w->db->query("select id from migration + // where module = 'admin' and classname = '" . Config::get('system.encryptionMigration') . "' ; ")->fetchAll(); + + // $checked = [ + // 'pass' => false, + // 'info' => "" + // ]; + // if (!empty($result)) { + // $checked['info'] = "Migration has been run."; + // } else { + // $checked['pass'] = true; + // $checked['info'] = "Migration not previously run."; + // } + + // return $checked; + // } + + + // private function checkMigrationClass() + // { + // $systemClass = Config::get('system.encryptionMigration', ""); + + // $checked = [ + // 'pass' => false, + // 'info' => "" + // ]; + + // if ($systemClass == get_class($this)) { + // $checked['pass'] = true; + // $checked['info'] = "Migration class is configured."; + // } else { + // $checked['info'] = "System config does not recognise this encryption migration."; + // } + + // return $checked; + // } + + // private function checkBlankContemporyInstall() + // { + + // // can we ever run SSL? + // if (PHP_VERSION_ID < "050300") { + // return false; + // } + + // // is the DB empty of encryption? + // $db = Config::get("database.database"); + // if (empty($db)) { + // $err = 'Database config not set'; + // LogService::getInstance($this->w)->error($err); + // throw new Exception($err); + // } + + // $table = $this->w->db->query("select table_name, column_name from information_schema.columns + // where table_schema='$db' and column_name like 's\_%';")->fetchAll(); + + // if (empty($table)) { + // return true; + // } + + // try { + // foreach ($table as $row) { + // foreach ($row as $key => $val) { + // if (is_numeric($key)) { + // unset($row[$key]); + // } + // } + // $tableName = $row['table_name']; + // $columnName = $row['column_name']; + // $tbl = $this->w->db->query("select id, $columnName from $tableName")->fetchAll(); + // if (!empty($tbl)) { + // return false; + // } + // } + // } catch (Exception $e) { + // throw $e; + // } + + // return true; + // } + + private function checkPHPversion() + { $serial = PHP_VERSION_ID; - $version = substr(PHP_VERSION_ID,-4,2); - $version = str_replace(substr(PHP_VERSION_ID,-4,4),"",PHP_VERSION_ID).".".$version; + $version = substr(PHP_VERSION_ID, -4, 2); + $version = str_replace(substr(PHP_VERSION_ID, -4, 4), "", PHP_VERSION_ID) . "." . $version; - $checked = [ - 'pass' => false , - 'version' => $version , - 'info' => "" ]; + $checked = [ + 'pass' => false, + 'version' => $version, + 'info' => "" + ]; - if(($serial>="050300")&&($serial<"070100")) { - $checked['pass'] = true; - $checked['info'] = 'Migration will promote AES to SSL for PHP ver.5.3->7.0'; - } - if($serial<"050300") { - $checked['info'] = 'Migration will not function for PHP ver. before 5.3'; - } - if($serial>="070100") { - $checked['info'] = 'Migration will not function for PHP ver. after 7.1'; + if ($serial <= "080000") { + $checked['info'] = 'Migration can not function for PHP ver. before 8.0'; + } else { + $checked['info'] = 'Migration is not indicated for PHP ver. after 8.0'; } - return $checked; - } - - private function checkMigrationStatus() { - $result = $this->w->db->query("select id from migration - where module = 'admin' and classname = '".Config::get('system.encryptionMigration')."' ; ")->fetchAll(); - - $checked = [ - 'pass' => false , - 'info' => "" ]; - if (!empty($result)) { - $checked['info'] = "Migration has been run."; - } else { - $checked['pass'] = true; - $checked['info'] = "Migration not previously run."; - } - - return $checked; + return $checked; } - private function checkSSLKeys() { - $encryption_key = Config::get('system.encryption.key',null); - //$encryption_iv = Config::get('system.encryption.iv',null); - - $checked = [ - 'pass' => false , - 'info' => "" ]; - - if (!empty($encryption_key)) { // || empty($encryption_iv))) { - $checked['pass'] = true; - $checked['info'] = "SSL Key exists."; - } else { - $checked['info'] = "No SSL Key found."; - } + private function checkSSLKeys() + { + $encryption_key = Config::get('system.encryption.key', null); + //$encryption_iv = Config::get('system.encryption.iv',null); - return $checked; - } - - private function checkMigrationClass() { - $systemClass = Config::get('system.encryptionMigration',""); + $checked = [ + 'pass' => false, + 'info' => "" + ]; - $checked = [ - 'pass' => false , - 'info' => "" ]; - - if ($systemClass==get_class($this)) { + if (!empty($encryption_key)) { // || empty($encryption_iv))) { $checked['pass'] = true; - $checked['info'] = "Migration class is configured."; + $checked['info'] = "SSL Key exists."; } else { - $checked['info'] = "System config does not recognise this encryption migration."; + $checked['info'] = "No SSL Key found."; } return $checked; } - private function checkBlankContemporyInstall() { - - // can we ever run SSL? - if(PHP_VERSION_ID<"050300") { - return false; - } - - // is the DB empty of encryption? - $db = Config::get("database.database"); - if (empty($db)) { - $err = 'Database config not set'; - LogService::getInstance($this->w)->error($err); - throw new Exception($err); - } - - $table = $this->w->db->query("select table_name, column_name from information_schema.columns - where table_schema='$db' and column_name like 's\_%';")->fetchAll(); - - if (empty($table)) { - return true; - } - - try { - foreach ($table as $row) { - foreach ($row as $key => $val) { - if (is_numeric($key)) { - unset($row[$key]); - } - } - $tableName = $row['table_name']; - $columnName = $row['column_name']; - $tbl = $this->w->db->query("select id, $columnName from $tableName")->fetchAll(); - if(!empty($tbl)) { return false; } - } - } catch (Exception $e) { - throw $e; - } - - return true; + public function preText() + { + // $configured = $this->checkMigrationClass(); + // if (!$configured['pass']) { + // return "Encryption changes require migration named in config."; + // } else { + // return "Detected PHP ver." . $this->checkPHPversion()['version'] + // . ", " . $this->checkSSLKeys()['info']; + // } + return "Detected PHP ver." . $this->checkPHPversion()['version'] + . ", " . $this->checkSSLKeys()['info']; } - public function preText() - { - $configured = $this->checkMigrationClass(); - if(!$configured['pass']) { - return "Encryption changes require migration named in config."; - } else { - return "Detected PHP ver.".$this->checkPHPversion()['version'] - .", ".$this->checkSSLKeys()['info']; - } - } + public function postText() + { + // if ($this->checkBlankContemporyInstall()) { + // return "Encryption will be SSL."; + // } - public function postText() - { - if($this->checkBlankContemporyInstall()) { - return "Encryption will be SSL."; - } + // return "Encryption will " . ( + // ($this->checkMigrationClass()['pass'] + // && $this->checkPHPversion()['pass'] + // && $this->checkSSLKeys()['pass']) + // ? "be SSL." : "not change."); - return "Encryption will ".( - ($this->checkMigrationClass()['pass'] - && $this->checkPHPversion()['pass'] - && $this->checkSSLKeys()['pass']) - ?"be SSL.":"not change." ); - } - - public function description() - { - if($this->checkBlankContemporyInstall()) { - return "New install does not need migration."; + return "Encryption will be SSL. ".$this->checkPHPversion()['info']; } - $configured = $this->checkMigrationClass(); - if(!$configured['pass']) { - return $configured['info']; - } else { - return $this->checkPHPversion()['info'].", ".$this->checkMigrationStatus()['info']; - } - } + public function description() + { + // if ($this->checkBlankContemporyInstall()) { + // return "New install does not need migration."; + // } + // $configured = $this->checkMigrationClass(); + // if (!$configured['pass']) { + // return $configured['info']; + // } else { + // return $this->checkPHPversion()['info'] . ", " . $this->checkMigrationStatus()['info']; + // } + return "Only SSL encryption is supported for PHP8, no migration is available."; + } } From aed9d104e01e0f031d36c1c7ad661def1b3c4edb Mon Sep 17 00:00:00 2001 From: 2piJareem <122758853+2piJareem@users.noreply.github.com> Date: Tue, 4 Jul 2023 22:38:16 +0000 Subject: [PATCH 2/5] Coalesce Null to array when Checking CSRF Token --- system/web.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/system/web.php b/system/web.php index b87b7463e..3e2393e8e 100755 --- a/system/web.php +++ b/system/web.php @@ -732,7 +732,7 @@ public function start($init_database = true) if (Config::get('system.csrf.enabled') === true) { $allowed = Config::get('system.csrf.protected'); if (!empty($allowed[$this->_module]) || (!empty($this->_submodule) && !empty($allowed[$this->_module . '-' . $this->_submodule]))) { - if (in_array($this->_action, $allowed[$this->_module]) || (!empty($this->_submodule) && in_array($this->_action, $allowed[$this->_module . '-' . $this->_submodule]))) { + if (in_array($this->_action, $allowed[$this->_module] ?? []) || (!empty($this->_submodule) && in_array($this->_action, $allowed[$this->_module . '-' . $this->_submodule] ?? []))) { // If we get here then we are configured to enforce CSRF checking LogService::getInstance($this)->debug("Checking CSRF"); try { From f1ac100b588f3ef6276a1de77b1d14b7af47c47a Mon Sep 17 00:00:00 2001 From: DerekCrannaford <43056286+DerekCrannaford@users.noreply.github.com> Date: Fri, 11 Aug 2023 13:21:30 +1000 Subject: [PATCH 3/5] Expose type ref for browser datetimelocal (#176) Co-authored-by: D.Crannaford --- system/classes/html/form/inputfield/DatetimeLocal.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/system/classes/html/form/inputfield/DatetimeLocal.php b/system/classes/html/form/inputfield/DatetimeLocal.php index 082aec449..b7897ce0f 100644 --- a/system/classes/html/form/inputfield/DatetimeLocal.php +++ b/system/classes/html/form/inputfield/DatetimeLocal.php @@ -2,10 +2,12 @@ /** * A helper InputField class for datetime-local, extends the datetime class for - * the time being as browser support for datetime-local is very poor. + * the time being (as)/(in case?) browser support for datetime-local is very poor. * * @author Adam Buckley */ class DatetimeLocal extends \Html\Form\InputField\Datetime { + + public $type = "datetime-local"; } From dab0bbb8f3a5a657573d659347f1d98d4eb07d9c Mon Sep 17 00:00:00 2001 From: DerekCrannaford <43056286+DerekCrannaford@users.noreply.github.com> Date: Sun, 13 Aug 2023 10:44:10 +1000 Subject: [PATCH 4/5] Fix: Stable File Migration (#177) * First cut at arbitray count and catch * Make default be all files moved --------- Co-authored-by: D.Crannaford --- .../file/actions/admin/moveToAdapter.php | 24 ++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/system/modules/file/actions/admin/moveToAdapter.php b/system/modules/file/actions/admin/moveToAdapter.php index 3cc4f6af0..60cf5b91b 100644 --- a/system/modules/file/actions/admin/moveToAdapter.php +++ b/system/modules/file/actions/admin/moveToAdapter.php @@ -4,6 +4,11 @@ function moveToAdapter_GET(Web $w) { $from_adapter = Request::string('from_adapter'); $to_adapter = Request::string('to_adapter'); + $max_count = Request::string('max_count'); + + if (empty($max_count)) { + $max_count = -1; + } if (!empty(Config::get('file.adapters.' . $to_adapter)) && Config::get('file.adapters.' . $to_adapter . '.active') === true) { if (empty(Config::get('file.adapters.' . $from_adapter))) { @@ -12,14 +17,27 @@ function moveToAdapter_GET(Web $w) // From index $count = 0; + $skipped = 0; $attachments = FileService::getInstance($w)->getAttachmentsForAdapter($from_adapter); if (!empty($attachments)) { foreach ($attachments as $attachment) { - $attachment->moveToAdapter($to_adapter); - $count++; + if ($max_count >= 0 && $count >= $max_count) { + break; + } + try { + $attachment->moveToAdapter($to_adapter); + $count++; + } catch (Exception $e) { + LogService::getInstance($w)->error($e->getMessage()); + $skipped++; + } } } - $w->msg($count . ' attachment' . ($count == 1 ? '' : 's') . ' moved from "' . $from_adapter . '" to "' . $to_adapter . '"', '/file-admin'); + $message = ($count . ' attachment' . ($count == 1 ? '' : 's') . ' moved from "' . $from_adapter . '" to "' . $to_adapter . '"'); + if ($skipped > 0) { + $message = $message . ('
' . $skipped . ' attachment' . ($skipped == 1 ? '' : 's') . ' skipped, check logs.'); + } + $w->msg($message, '/file-admin'); } else { $w->error('Target adapter "' . $to_adapter . '" is either not found or not active', '/file-admin'); } From 94086c2dbbcebea8db122f171aedb4ada288f929 Mon Sep 17 00:00:00 2001 From: DerekCrannaford <43056286+DerekCrannaford@users.noreply.github.com> Date: Mon, 14 Aug 2023 09:46:40 +1000 Subject: [PATCH 5/5] Feature: api backend on php8 (#194) * WIP with stubs for NONRELEASE * progress on the cmfive token model. Incomplete * Dane WIP, most of the way through getting user roles from a policy need to fix up a few issues, and then add group roles as well * getRoles working for the temporary tokens * moving to another hook for roles, but not working at this stage * working tokens and role system * added output templates, changed endpoints to give json responses * Better token concept may be viable * Oath base classes and generic actions * oauth flow, Cognito model, SplashScreen on Success * Cognito implemented with validation and user data * Suspend and embed stable test code * Part out oauth models * Avoid null roles exception * Factor token policies not visible as user roles * Deprecated magic methods * Ver8 logging corrections * Thumbs patched * Patch html fails on strlen * Patch html fails on substr * Patch missing auth token error * Patch typo for missing auth token * Patched toSlug for str vs null * Supressed dead and demo code * Heavy cleanup, defer examples/tests as external * Cleanup requested on PR * Cleaned comments --------- Co-authored-by: Derek Crannaford Co-authored-by: Dane Evans <98690514+Dane-2pi@users.noreply.github.com> --- system/config.php | 2 + system/functions.php | 2 +- system/html.php | 16 +-- system/modules/auth/models/AuthService.php | 52 ++++++++- system/modules/file/models/Attachment.php | 4 +- system/modules/tokens/config.php | 10 ++ .../tokens/models/ApiOutputService.php | 101 ++++++++++++++++++ system/modules/tokens/models/TokensPolicy.php | 93 ++++++++++++++++ .../modules/tokens/models/TokensService.php | 65 +++++++++++ system/modules/tokens/tokens.hooks.php | 37 +++++++ system/modules/tokens/tokens.roles.php | 15 +++ system/web.php | 2 +- 12 files changed, 384 insertions(+), 15 deletions(-) create mode 100644 system/modules/tokens/config.php create mode 100644 system/modules/tokens/models/ApiOutputService.php create mode 100644 system/modules/tokens/models/TokensPolicy.php create mode 100644 system/modules/tokens/models/TokensService.php create mode 100644 system/modules/tokens/tokens.hooks.php create mode 100644 system/modules/tokens/tokens.roles.php diff --git a/system/config.php b/system/config.php index 96cde5f4e..51900044f 100755 --- a/system/config.php +++ b/system/config.php @@ -94,3 +94,5 @@ 'secret' => '', ], ]); + +Config::set('system.use_api', true); diff --git a/system/functions.php b/system/functions.php index bbd7a413e..e7b8dbc86 100755 --- a/system/functions.php +++ b/system/functions.php @@ -222,7 +222,7 @@ function defaultVal($val, $default = null) */ function toSlug($title) { - return strtolower(str_replace([' ', '_', ',', '.', '/'], '-', $title)); + return strtolower(str_replace([' ', '_', ',', '.', '/'], '-', ($title ?? ""))); } /** diff --git a/system/html.php b/system/html.php index a844f2f47..f6e3d9436 100755 --- a/system/html.php +++ b/system/html.php @@ -196,7 +196,7 @@ public static function ab($href, $title, $class = "", $id = "", $confirm = "") /** * Create an a link styled as a button that pops up a reveal dialog * */ - public static function abox($href, $title, $class = null, $id = null, $confirm = null) + public static function abox($href, $title, $class = "", $id = "", $confirm = "") { $classParam = ' button tiny '; if (strlen($class) > 0) { @@ -365,8 +365,8 @@ public static function form($data, $action = null, $method = "POST", $submitTitl $readonly = ""; // handle disabled fields - if (substr($name, 0, 1) == '-') { - $name = substr($name, 1); + if (substr(($name ?? ""), 0, 1) == '-') { + $name = substr(($name ?? ""), 1); $readonly = " readonly='true' "; } // Add title field @@ -665,8 +665,8 @@ public static function multiColForm($data, $action = null, $method = "POST", $su $buffer .= ($type !== "hidden" ? "
" : ""); // handle disabled fields - if (substr($name, 0, 1) == '-') { - $name = substr($name, 1); + if (substr(($name ?? ""), 0, 1) == '-') { + $name = substr(($name ?? ""), 1); $readonly = " readonly='true' "; } @@ -969,7 +969,7 @@ public static function autocomplete($name, $options, $value = null, $class = nul } } // Remove trailing comma - $source = substr($source, 0, -1); + $source = substr(($source ?? ""), 0, -1); $source .= "]"; } else { $source = "'" . $options . "'"; @@ -1335,8 +1335,8 @@ public static function filter($legend, $data, $action = null, $method = "POST", } // handle disabled fields - if (substr($name, 0, 1) == '-') { - $name = substr($name, 1); + if (substr(($name ?? ""), 0, 1) == '-') { + $name = substr(($name ?? ""), 1); $readonly = " readonly='true' "; } diff --git a/system/modules/auth/models/AuthService.php b/system/modules/auth/models/AuthService.php index 0f50b6aaf..d9d74df0d 100755 --- a/system/modules/auth/models/AuthService.php +++ b/system/modules/auth/models/AuthService.php @@ -75,7 +75,6 @@ public function login($login, $password, $client_timezone, $skip_session = false } public function externalLogin($login, $password, $skip_session = false) { - $user = $this->getUserForLogin($login); if (empty($user->id) || ($user->encryptPassword($password) !== $user->password) || $user->is_external == 0) { return null; @@ -257,7 +256,7 @@ public function getContacts() * * @return array[Lookup] */ - public function getTitles() : array + public function getTitles(): array { return LookupService::getInstance($this->w)->getLookupByType("title"); } @@ -328,11 +327,51 @@ public function allowed($path, $url = null) return false; } + // Whitelisted action, or white-bread login session if ((function_exists("anonymous_allowed") && anonymous_allowed($this->w, $path)) || ($this->user() && $this->user()->allowed($path))) { self::$_cache[$key] = $url ? $url : true; return self::$_cache[$key]; } + // API token handling: + // If I have an authentication header: and it has a token -> else fallthrough to original logic + // ie: expecting [...curl...etc...] -H "Authorization: Bearer {token}" + /* + Note! If under Apache & HTTP_AUTHORIZATION is dropped, prove site HTPPS and then patch access: + RewriteEngine On + RewriteCond %{HTTP:Authorization} ^(.+)$ + RewriteRule .* - [E=HTTP_AUTHORIZATION:%{HTTP:Authorization}] + */ + + if (empty($this->user()) && (Config::get('system.use_api') === true) && !empty($_SERVER['HTTP_AUTHORIZATION'])) { + $speculativeToken = TokensService::getInstance($this->w)->getTokenFromAuthorisationHeader($_SERVER['HTTP_AUTHORIZATION']); + if (!empty($speculativeToken)) { + // call for a module to assert the token is valid + $hook_results = $this->w->callHook("auth", "get_auth_token_validation", $speculativeToken); + } + + // if the token is invalid( jwt fails checks, len == 0 or somesuch) then we stop and don't continue + if (empty($speculativeToken) || empty($hook_results)) { + LogService::getInstance($this->w)->error("Key invalid: '" . ($_SERVER['HTTP_AUTHORIZATION'] ?? "!NONE!") . "' was provided"); + ApiOutputService::getInstance($this->w)->apiRefuseMessage($path,"Token not valid"); + self::$_cache[$key] = false; + return false; + } + foreach ($hook_results as $module => $validatingToken) { + if (is_a($validatingToken, "TokensPolicy") && $validatingToken->tokensAllowed($path)) { + self::$_cache[$key] = $url ? $url : true; + return self::$_cache[$key]; + } else { + LogService::getInstance($this->w)->info('Handler ' . $module . ' did not provide Auth'); + } + } + ApiOutputService::getInstance($this->w)->apiRefuseMessage($path.":[".$speculativeToken."]", "Token not authenticated"); + self::$_cache[$key] = false; + return false; + } + + + // Allow forced user-login if any module will vouch for web server asserted identity if (empty($this->user()) && (Config::get('system.use_passthrough_authentication') === true) && !empty($_SERVER['AUTH_USER'])) { // Get the username $username = explode('\\', $_SERVER["AUTH_USER"]); @@ -346,6 +385,13 @@ public function allowed($path, $url = null) $this->forceLogin($user->id); if ($user->allowed($path)) { self::$_cache[$key] = $url ? $url : true; + // Observed during work for token handler: + // Here, we have forced login, + // But do we mean for it to still bounce 1x through auth/login as redirect? + // In standing core releases, a _cache[key] 'return' is omitted here + // = noting it was required by new tokens model! + // Possibly this block should also have return thus: + // return self::$_cache[$key]; } } else { LogService::getInstance($this->w)->info($module . ' did not provide passthrough user for:' . $username); @@ -362,7 +408,7 @@ public function allowed($path, $url = null) * * @return array of strings */ - public function getAllRoles() : array + public function getAllRoles(): array { $this->_loadRoles(); if (!$this->_roles) { diff --git a/system/modules/file/models/Attachment.php b/system/modules/file/models/Attachment.php index 4240d6978..d66c82141 100755 --- a/system/modules/file/models/Attachment.php +++ b/system/modules/file/models/Attachment.php @@ -496,8 +496,8 @@ public function updateAttachment($requestkey) if ($this->isImage()) { // Generate thumbnail and cache require_once 'phpthumb/ThumbLib.inc.php'; - $width = $this->w->request("w", FileService::$_thumb_width); - $height = $this->w->request("h", FileService::$_thumb_height); + $width = Request::int('w', FileService::$_thumb_width); + $height = Request::int('h', FileService::$_thumb_height); $thumb = PhpThumbFactory::create($this->getContent(), [], true); $thumb->adaptiveResize($width, $height); diff --git a/system/modules/tokens/config.php b/system/modules/tokens/config.php new file mode 100644 index 000000000..581d291c5 --- /dev/null +++ b/system/modules/tokens/config.php @@ -0,0 +1,10 @@ + true, + 'path' => 'system/modules', + 'topmenu' => false, + 'hooks' => [ + 'auth', + 'tokens' + ], +)); diff --git a/system/modules/tokens/models/ApiOutputService.php b/system/modules/tokens/models/ApiOutputService.php new file mode 100644 index 000000000..4ae8136f6 --- /dev/null +++ b/system/modules/tokens/models/ApiOutputService.php @@ -0,0 +1,101 @@ +setLayout(null); + // } + + public function apiReturnCmfiveStyledHtml($w, $response) + { + $w->setLayout("layout-modal"); + $w->enqueueScript( + [ + "name" => "main.js", "uri" => "/system/templates/js/main.js", "weight" => 995 + ] + ); + $w->enqueueStyle( + [ + "name" => "foundation.css", "uri" => "/system/templates/js/foundation-5.5.0/css/foundation.css", "weight" => 1005 + ] + ); + // $w->enqueueStyle(array("name" => "style.css", "uri" => "/system/templates/css/style.css", "weight" => 1000)); + // $w->outputStyles(); + // $w->outputScripts(); + $w->out($response); + echo $w->_buffer; + exit(0); + } + + public function apiReturnJsonResponse($response) + { + $this->w->setLayout(null); + http_response_code($response['status']); + // mark header for return content type JSON + if (substr($response['status'], 0, 1) == "2") { + header('Content-Type: application/json'); + } else { + LogService::getInstance($this->w)->info("API request rejected: " . $response['referer']); + // Don't need, already have set response code! + // header($_SERVER["SERVER_PROTOCOL"] . " " . $response['status'] . " " . $response['payload'][0]); + } + + echo json_encode(['response' => $response]); + exit(0); + } + + // JSON simple message + public function apiKeyedResponse($payload = [], $message = "Success", $status_code = "200") + { + $success = [ + 'status' => $status_code, + 'message' => $message, + 'payload' => $payload + ]; + $this->apiReturnJsonResponse($success); + } + + // JSON simple message + public function apiSimpleResponse($detail = "", $message = "Success", $status_code = "200") + { + $payload[] = $detail; + $success = [ + 'status' => $status_code, + 'message' => $message, + 'payload' => $payload + ]; + $this->apiReturnJsonResponse($success); + } + + // JSON nice fail message + public function apiFailMessage($source = "", $detail = "", $message = "Failure", $status_code = "500") + { + $payload[] = $detail; + $errors = [ + 'status' => $status_code, + 'referer' => $source, + 'message' => $message, + 'payload' => $payload + ]; + $this->apiReturnJsonResponse($errors); + } + + // JSON nice refuse message + public function apiRefuseMessage($source = "", $detail = "", $message = "Unauthorised", $status_code = "403") + { + $payload[] = $detail; + $errors = [ + 'status' => $status_code, + 'referer' => $source, + 'message' => $message, + 'payload' => $payload + ]; + + $this->apiReturnJsonResponse($errors); + } +} diff --git a/system/modules/tokens/models/TokensPolicy.php b/system/modules/tokens/models/TokensPolicy.php new file mode 100644 index 000000000..a8ba3aa6c --- /dev/null +++ b/system/modules/tokens/models/TokensPolicy.php @@ -0,0 +1,93 @@ +roles->allowed + - TokensPolicy GetBearersRoles assists with collation + - APP can interpret profile identifier freely (eg: as user_id, group_policy, code-baked app_actions etc) + */ + + public function getBearersRoles(): array + { + // Consider remaining stateless here... + // we have persisted no concept of the token beyond: + // - it was asserted valid + // - the VALIDATOR identified itself (for stubbed purposes, this is CMFIVE) + // - the VALIDATOR confirmed the implementation of an APP + // - the VALIDATOR asserted a ROLE PROFILE (for simplest purposes, this could be a CMFIVE USER) + + // So, from here, let's hit another hook, for validators/app_service models to populate roles from profile: + + // broadcast this policy + // listener will self identify as _validator and _app_id + // listener will interpret profile identifier (eg as user_id, group_policy, code-baked app_actions etc) + $hook_results = $this->w->callHook("tokens", "get_roles_from_policy", $this); + + if (empty($hook_results)) { + LogService::getInstance($this->w)->error("No Roles for policy:" . $this->_role_profile); + } + + // listener will return conventional/conformed roles list + // which we clean up in case of merged results typical of hooks: + $roles = []; + foreach ($hook_results as $_roles) { + $roles = array_merge($roles, $_roles ?? []); + } + $roles = array_unique($roles); + + return $roles; + } + + /** + * Check whether a bearer is allowed to navigate to a certain url + * in the system. + * + * This will execute all the functions associated to the bearer's roles + * until one function returns true. + * + * @param Web $w + * @param string $path + * @return true if one role function returned true + */ + public function tokensAllowed($path): bool + { + $roles = $this->getBearersRoles() ?? []; + + foreach ($roles as $rn) { + $rolefunc = "role_" . $rn . "_allowed"; + $policyfunc = "token_policy_" . $rn . "_allowed"; + if (!function_exists($rolefunc) && !function_exists($policyfunc)) { + LogService::getInstance($this->w)->error("Role or policy'" . $rn . "' does not exist!"); + continue; + } + // These will be visible as user role selectors + // Ticked user can navigate to, if logged in! + if (function_exists($rolefunc) && $rolefunc($this->w, $path)) { + return true; + } + // These have no visible roles + // Only a token policy can grant page access + if (function_exists($policyfunc) && $policyfunc($this->w, $path)) { + return true; + } + } + + return false; + } +} diff --git a/system/modules/tokens/models/TokensService.php b/system/modules/tokens/models/TokensService.php new file mode 100644 index 000000000..2fbcd0023 --- /dev/null +++ b/system/modules/tokens/models/TokensService.php @@ -0,0 +1,65 @@ + "json parse failed"]; + } + + // This will only work for a vanilla HS256 token! + // Key-pair hashed tokens (Cognito etc) will neeed their own check methods + + public function getJwtSignatureCheck($jwt, $asBase64 = false): bool + { + $parts = explode(".", $jwt); + + $header = json_decode(base64_decode($parts[0] ?? ""), true); + $alg = $header['alg'] ?? ""; + + if (empty($parts[2]) || !$alg == "HS256") { + return false; + } + + $signature = hash('sha256', $parts[0] . "." . ($parts[1] ?? "")); + if ($asBase64) { + $signature = $this->getBase64URL($signature); + } + + return ($signature == ($parts[2] ?? null)); + } + + public function getJwtPayload($jwt): array + { + $parts = explode(".", $jwt); + return json_decode(base64_decode($parts[1] ?? ""), true); + } + + public function getAppFromJwtPayload($jwt): string + { + return $this->getJwtPayload($jwt)['client_id'] ?? ""; + } + + function getBase64URL($plainText): string + { + $base64 = base64_encode($plainText); + $base64 = trim($base64, "="); + $base64url = strtr($base64, '+/', '-_'); + return $base64url; + } +} diff --git a/system/modules/tokens/tokens.hooks.php b/system/modules/tokens/tokens.hooks.php new file mode 100644 index 000000000..bbf49a874 --- /dev/null +++ b/system/modules/tokens/tokens.hooks.php @@ -0,0 +1,37 @@ +roles->allowed + - TokensPolicy GetBearersRoles assists with collation + - APP can interpret profile identifier freely (eg: as user_id, group_policy, code-baked app_actions etc) + */ + +/* + +// Your AUTH MODEL supporting API APP as token provider should catch this hook: + +function [MySmartAuthModule]_auth_get_auth_token_validation(Web $w, $jwt) +{ + return [MySmartAuthModule]::getInstance($w)->getResultOfAppliedTokenCheck($jwt); +} + + +// Your API APP as role provider should catch this hook: + +function [MyUsefulAPIModule]_tokens_get_roles_from_policy(Web $w, $policy) +{ + if ($policy->_validator == "MyAuthWasResponsible" && $policy->_app_id == "MyAppShouldHandleThis") { + return [MyUsefulAPIModule]::getInstance($w)->getAppropriateRolesByTokenResolvedToUserPolicy($policy); + } +} + +*/ diff --git a/system/modules/tokens/tokens.roles.php b/system/modules/tokens/tokens.roles.php new file mode 100644 index 000000000..f3d32380c --- /dev/null +++ b/system/modules/tokens/tokens.roles.php @@ -0,0 +1,15 @@ +checkUrl($path, "MyUsefulAPIModule", "MyUsefulAPIModuleActions", "MyAction") +// || $w->checkUrl($path, "MyUsefulAPIModule", "MyUsefulAPIModuleActions", "MyOtherAction") +// || $w->checkUrl($path, "MyUsefulAPIModule", "MyUsefulAPIModuleActions", "MyAlternateAction"); +// } + + diff --git a/system/web.php b/system/web.php index b87b7463e..ee8a3ac00 100755 --- a/system/web.php +++ b/system/web.php @@ -157,7 +157,7 @@ private function getUrlSchema() private function getHostname() { - return $this->getRequestHeader('HTTP_HOST'); + return $this->getRequestHeader("HTTP_X_FORWARDED_HOST", $this->getRequestHeader('HTTP_HOST')); } private function getRequestHeader($header, $default = '')