diff --git a/src/class-convertkit-log.php b/src/class-convertkit-log.php index 4e71ba6..7dd5510 100644 --- a/src/class-convertkit-log.php +++ b/src/class-convertkit-log.php @@ -20,7 +20,7 @@ class ConvertKit_Log { * * @var string */ - private $path; + private $path = ''; /** * The path and filename of the log file. @@ -29,7 +29,7 @@ class ConvertKit_Log { * * @var string */ - private $log_file; + private $log_file = ''; /** * Constructor. Defines the log file location. @@ -40,16 +40,20 @@ class ConvertKit_Log { */ public function __construct( $path ) { - // Define location of log file. - $this->path = trailingslashit( $path . '/log' ); - $this->log_file = $this->path . 'log.txt'; + // If legacy log files exist in the Plugin's directory, delete them now. + $this->maybe_delete_legacy_log_files( $path ); - // Initialize WP_Filesystem. - require_once ABSPATH . 'wp-admin/includes/file.php'; - WP_Filesystem(); + // Fetch the uploads directory. + $upload_dir = wp_upload_dir(); - // If a historic log file exists, delete it now. - $this->maybe_delete_historic_log_file( $path ); + // Bail if the uploads directory is unavailable. + if ( ! empty( $upload_dir['error'] ) || empty( $upload_dir['basedir'] ) ) { + return; + } + + // Define location of log file. + $this->path = trailingslashit( $upload_dir['basedir'] ) . 'kit-logs/'; + $this->log_file = $this->path . $this->get_log_file_name( $path ); // If the secure log directory does not exist, create it now. $this->maybe_create_secure_log_directory(); @@ -57,21 +61,38 @@ public function __construct( $path ) { } /** - * Deletes a log.txt file for the given 'old' log file path location, - * which does not have .htaccess or index.html protection. + * Deletes log files stored in the Plugin's directory by earlier versions of + * this class. + * + * Deletes: + * - `log.txt`, used prior to 1.4.2, which has no .htaccess or index.html protection, + * - `log` directory and its contents, used from 1.4.2 to 2.6.0. * * @since 1.4.2 * - * @param string $old_path Path to possible historic log file. + * @param string $path Path to the Plugin. */ - private function maybe_delete_historic_log_file( $old_path ) { + private function maybe_delete_legacy_log_files( $path ) { - // Bail if file doesn't exist. - if ( ! file_exists( trailingslashit( $old_path ) . 'log.txt' ) ) { - return; + // If a log.txt file exists in the Plugin's directory (i.e. from 1.4.2 or earlier), delete it. + $legacy_file = trailingslashit( $path ) . 'log.txt'; + if ( file_exists( $legacy_file ) ) { + wp_delete_file( $legacy_file ); } - wp_delete_file( trailingslashit( $old_path ) . 'log.txt' ); + // If a log directory exists in the Plugin's directory (i.e. from 1.4.2 to 2.6.0), delete it and its contents. + $legacy_path = trailingslashit( $path ) . 'log'; + if ( is_dir( $legacy_path ) ) { + // Delete the files this class created in the log directory. + foreach ( array( 'log.txt', '.htaccess', 'index.html' ) as $file ) { + if ( file_exists( trailingslashit( $legacy_path ) . $file ) ) { + wp_delete_file( trailingslashit( $legacy_path ) . $file ); + } + } + + // Delete the log directory. + rmdir( trailingslashit( $path ) . 'log' ); // phpcs:ignore WordPress.WP.AlternativeFunctions.file_system_operations_rmdir + } } @@ -79,19 +100,27 @@ private function maybe_delete_historic_log_file( $old_path ) { * Creates a directory to store the log file, with .htaccess and index.html * files to protect the log file, as WooCommerce does. * + * Disables logging if the directory could not be created, or isn't writable. + * * @since 1.4.2 */ private function maybe_create_secure_log_directory() { - // Initialize WordPress file system. - global $wp_filesystem; - // Create directory. wp_mkdir_p( $this->path ); + // Disable logging if the directory doesn't exist or isn't writable. + if ( ! is_dir( $this->path ) || ! wp_is_writable( $this->path ) ) { + $this->path = ''; + $this->log_file = ''; + return; + } + // Define files to protect the directory. - $wp_filesystem->put_contents( $this->path . '.htaccess', 'deny from all' ); - $wp_filesystem->put_contents( $this->path . 'index.html', '' ); + // phpcs:disable WordPress.WP.AlternativeFunctions.file_system_operations_file_put_contents + file_put_contents( $this->path . '.htaccess', 'deny from all' ); + file_put_contents( $this->path . 'index.html', '' ); + // phpcs:enable WordPress.WP.AlternativeFunctions.file_system_operations_file_put_contents } @@ -117,6 +146,11 @@ public function get_filename() { */ public function exists() { + // Bail if logging is disabled. + if ( ! $this->log_file ) { + return false; + } + return file_exists( $this->get_filename() ); } @@ -130,15 +164,14 @@ public function exists() { */ public function add( $entry ) { - // Initialize WordPress file system. - global $wp_filesystem; + // Bail if logging is disabled. + if ( ! $this->log_file ) { + return; + } // Prefix the entry with a date and time. $entry = '(' . gmdate( 'Y-m-d H:i:s' ) . ') ' . $entry . "\n"; - // Get any existing log file contents. - $contents = $wp_filesystem->get_contents( $this->get_filename() ); - // Mask email addresses that may be contained within the entry. $entry = preg_replace_callback( '^[_a-z0-9-]+(\.[_a-z0-9-]+)*@[a-z0-9-]+(\.[a-z0-9-]+)*(\.[a-z]{2,3})^', @@ -149,10 +182,8 @@ function ( $matches ) { ); // Append entry. - $contents .= $entry; - - // Write contents. - $wp_filesystem->put_contents( $this->get_filename(), $contents ); + // phpcs:ignore WordPress.WP.AlternativeFunctions.file_system_operations_file_put_contents + file_put_contents( $this->get_filename(), $entry, FILE_APPEND ); } @@ -166,16 +197,14 @@ function ( $matches ) { */ public function read( $number_of_lines = 500 ) { - // Initialize WordPress file system. - global $wp_filesystem; - // Bail if the log file does not exist. if ( ! $this->exists() ) { return ''; } // Open log file. - $log = $wp_filesystem->get_contents_array( $this->get_filename() ); + // phpcs:ignore WordPress.WP.AlternativeFunctions.file_system_operations_file + $log = file( $this->get_filename() ); // Bail if the log file is empty. if ( ! is_array( $log ) || ! count( $log ) ) { @@ -194,10 +223,13 @@ public function read( $number_of_lines = 500 ) { */ public function clear() { - // Initialize WordPress file system. - global $wp_filesystem; + // Bail if logging is disabled. + if ( ! $this->log_file ) { + return; + } - $wp_filesystem->put_contents( $this->get_filename(), '' ); + // phpcs:ignore WordPress.WP.AlternativeFunctions.file_system_operations_file_put_contents + file_put_contents( $this->get_filename(), '' ); } @@ -208,8 +240,29 @@ public function clear() { */ public function delete() { + // Bail if logging is disabled. + if ( ! $this->log_file ) { + return; + } + wp_delete_file( $this->get_filename() ); } + /** + * Returns the log file's name for the Plugin at the given path. + * + * @since 2.6.1 + * + * @param string $path Path to the Plugin. + * @return string + */ + private function get_log_file_name( $path ) { + + $slug = sanitize_key( basename( untrailingslashit( $path ) ) ); + + return $slug . '-' . wp_hash( $slug ) . '.log'; + + } + } diff --git a/tests/Integration/APITest.php b/tests/Integration/APITest.php index bc6c76e..026d51c 100644 --- a/tests/Integration/APITest.php +++ b/tests/Integration/APITest.php @@ -217,8 +217,10 @@ protected function assertLastResponseStatusCode(int $expected): void } /** - * Test that a log directory and file are created in the expected location, with .htaccess - * and index.html protection, and that the name and email addresses are masked. + * Test that a log directory and file are created in the uploads directory, with .htaccess + * and index.html protection, that log files stored in the Plugin's directory by earlier + * versions of the ConvertKit_Log class are deleted, and that the name and email addresses + * are masked. * * @since 1.4.2 */ @@ -227,8 +229,14 @@ public function testLog() // Define location for log file. define( 'CONVERTKIT_PLUGIN_PATH', $_ENV['WORDPRESS_ROOT_DIR'] . '/wp-content/uploads' ); - // Create a log.txt file. - $this->tester->writeToFile(CONVERTKIT_PLUGIN_PATH . '/log.txt', 'historical log file'); + // Create a log.txt file in the Plugin's directory, as versions prior to 1.4.2 did. + $this->tester->writeToFile(CONVERTKIT_PLUGIN_PATH . '/log.txt', 'legacy log file'); + + // Create a log directory in the Plugin's directory, as versions 1.4.2 to 2.6.0 did. + wp_mkdir_p(CONVERTKIT_PLUGIN_PATH . '/log'); + $this->tester->writeToFile(CONVERTKIT_PLUGIN_PATH . '/log/log.txt', 'legacy log file'); + $this->tester->writeToFile(CONVERTKIT_PLUGIN_PATH . '/log/.htaccess', 'deny from all'); + $this->tester->writeToFile(CONVERTKIT_PLUGIN_PATH . '/log/index.html', ''); // Initialize API with logging enabled. $api = new \ConvertKit_API_V4( @@ -250,17 +258,25 @@ public function testLog() ); $api->profile($_ENV['CONVERTKIT_API_SIGNED_SUBSCRIBER_ID']); - // Confirm the historical log.txt file has been deleted. + // Confirm the legacy log.txt file (Libraries 1.4.2 and older) and + // log directory (Libraries 1.4.2 to 2.6.0) have been deleted from the + // Plugin's directory. $this->assertFileDoesNotExist(CONVERTKIT_PLUGIN_PATH . '/log.txt'); + $this->assertDirectoryDoesNotExist(CONVERTKIT_PLUGIN_PATH . '/log'); + + // Fetch the log file's location in the uploads directory. + $log = new \ConvertKit_Log(CONVERTKIT_PLUGIN_PATH); + $logFile = $log->get_filename(); + $logPath = dirname($logFile); - // Confirm the .htaccess and index.html files exist. - $this->assertDirectoryExists(CONVERTKIT_PLUGIN_PATH . '/log'); - $this->assertFileExists(CONVERTKIT_PLUGIN_PATH . '/log/.htaccess'); - $this->assertFileExists(CONVERTKIT_PLUGIN_PATH . '/log/index.html'); - $this->assertFileExists(CONVERTKIT_PLUGIN_PATH . '/log/log.txt'); + // Confirm the log directory, its .htaccess and index.html files, and the log file exist. + $this->assertDirectoryExists($logPath); + $this->assertFileExists($logPath . '/.htaccess'); + $this->assertFileExists($logPath . '/index.html'); + $this->assertFileExists($logFile); // Confirm the contents of the log file have masked the email address, name and signed subscriber ID. - $this->tester->openFile(CONVERTKIT_PLUGIN_PATH . '/log/log.txt'); + $this->tester->openFile($logFile); $this->tester->seeInThisFile('API: POST subscribers: {"email_address":"o****@n********.c**","first_name":"******Name","state":"active","fields":{"last_name":"Last"}}'); $this->tester->seeInThisFile('API: GET profile/*****************************************'); $this->tester->dontSeeInThisFile($_ENV['CONVERTKIT_API_SUBSCRIBER_EMAIL']); @@ -268,8 +284,8 @@ public function testLog() $this->tester->dontSeeInThisFile($_ENV['CONVERTKIT_API_SIGNED_SUBSCRIBER_ID']); // Cleanup test. - $this->tester->cleanDir(CONVERTKIT_PLUGIN_PATH . '/log'); - $this->tester->deleteDir(CONVERTKIT_PLUGIN_PATH . '/log'); + $this->tester->cleanDir($logPath); + $this->tester->deleteDir($logPath); } /** diff --git a/tests/Integration/LogTest.php b/tests/Integration/LogTest.php new file mode 100644 index 0000000..ad4a427 --- /dev/null +++ b/tests/Integration/LogTest.php @@ -0,0 +1,295 @@ +deleteDirectory($filePath); + continue; + } + + unlink($filePath); // phpcs:ignore WordPress.WP.AlternativeFunctions.unlink_unlink + } + + rmdir($path); // phpcs:ignore WordPress.WP.AlternativeFunctions.file_system_operations_rmdir + } + + /** + * Performs actions before each test. + * + * @since 2.6.1 + */ + public function setUp(): void + { + parent::setUp(); + + // Include class from /src to test. + require_once 'src/class-convertkit-log.php'; + + // Store the WP_Filesystem global, as one test replaces it. + $this->wpFilesystem = $GLOBALS['wp_filesystem'] ?? null; + + // Create a directory to mimic a Plugin's directory. + $this->plugin_path = trailingslashit(WP_CONTENT_DIR) . 'plugins/convertkit-log-test'; + wp_mkdir_p($this->plugin_path); + } + + /** + * Performs actions after each test. + * + * @since 2.6.1 + */ + public function tearDown(): void + { + // Restore the WP_Filesystem global. + $GLOBALS['wp_filesystem'] = $this->wpFilesystem; // phpcs:ignore WordPress.WP.GlobalVariablesOverride.Prohibited + + // Delete the log file. + if ($this->log) { + $this->log->delete(); + unset($this->log); + } + + // Delete the directory mimicking a Plugin's directory. + $this->deleteDirectory($this->plugin_path); + + parent::tearDown(); + } + + /** + * Test that the log file is stored in the uploads directory, and not in the + * Plugin's directory. + * + * Storing the log file in the Plugin's directory results in + * `wp plugin verify-checksums` reporting the Plugin as modified. + * + * @since 2.6.1 + */ + public function testLogFileIsStoredInUploadsDirectory() + { + $this->log = new \ConvertKit_Log($this->plugin_path); + $this->log->add('Log entry'); + + // Confirm the log file is in the uploads directory. + $uploadDir = wp_upload_dir(); + $this->assertStringStartsWith( + trailingslashit($uploadDir['basedir']) . 'kit-logs/', + $this->log->get_filename() + ); + $this->assertTrue($this->log->exists()); + + // Confirm the log file's name includes the Plugin's directory name, so that + // each Kit Plugin writes to its own log file. + $this->assertStringContainsString('convertkit-log-test-', basename($this->log->get_filename())); + $this->assertStringEndsWith('.log', $this->log->get_filename()); + + // Confirm no log file or directory was created in the Plugin's directory. + $this->assertFileDoesNotExist(trailingslashit($this->plugin_path) . 'log.txt'); + $this->assertDirectoryDoesNotExist(trailingslashit($this->plugin_path) . 'log'); + } + + /** + * Test that the uploads directory containing the log file has .htaccess and + * index.html files, to prevent listing and access on Apache. + * + * @since 2.6.1 + */ + public function testLogDirectoryIsProtected() + { + $this->log = new \ConvertKit_Log($this->plugin_path); + + $uploadDir = wp_upload_dir(); + $this->assertFileExists(trailingslashit($uploadDir['basedir']) . 'kit-logs/.htaccess'); + $this->assertFileExists(trailingslashit($uploadDir['basedir']) . 'kit-logs/index.html'); + } + + /** + * Test that a log.txt file, as created by versions prior to 1.4.2, is deleted + * from the Plugin's directory. + * + * @since 2.6.1 + */ + public function testLegacyLogFileIsDeleted() + { + // Create a log.txt file, as versions prior to 1.4.2 did. + $legacyLogFile = trailingslashit($this->plugin_path) . 'log.txt'; + file_put_contents($legacyLogFile, 'Log entry'); // phpcs:ignore WordPress.WP.AlternativeFunctions + $this->assertFileExists($legacyLogFile); + + $this->log = new \ConvertKit_Log($this->plugin_path); + + // Confirm the legacy log file was deleted. + $this->assertFileDoesNotExist($legacyLogFile); + } + + /** + * Test that the log directory, as created by versions 1.4.2 to 2.6.0, is deleted + * from the Plugin's directory. + * + * @since 2.6.1 + */ + public function testLegacyLogDirectoryIsDeleted() + { + // Create a log directory, as versions 1.4.2 to 2.6.0 did. + $legacyLogPath = trailingslashit($this->plugin_path) . 'log'; + wp_mkdir_p($legacyLogPath); + foreach ([ 'log.txt', '.htaccess', 'index.html' ] as $file) { + file_put_contents(trailingslashit($legacyLogPath) . $file, ''); // phpcs:ignore WordPress.WP.AlternativeFunctions + } + $this->assertDirectoryExists($legacyLogPath); + + $this->log = new \ConvertKit_Log($this->plugin_path); + + // Confirm the legacy log directory and its contents were deleted. + $this->assertDirectoryDoesNotExist($legacyLogPath); + } + + /** + * Test that entries can be added to, read from, cleared and deleted from the log file. + * + * @since 2.6.1 + */ + public function testAddReadClearAndDelete() + { + $this->log = new \ConvertKit_Log($this->plugin_path); + + // Add. + $this->log->add('Log entry'); + $this->assertStringContainsString('Log entry', $this->log->read()); + + // Clear. + $this->log->clear(); + $this->assertStringNotContainsString('Log entry', $this->log->read()); + $this->assertTrue($this->log->exists()); + + // Delete. + $this->log->delete(); + $this->assertFalse($this->log->exists()); + } + + /** + * Test that the log is written, and legacy log files deleted, when WordPress' + * WP_Filesystem is unusable. + * + * On hosts where WordPress selects the FTP transport and no FTP credentials are + * defined, $wp_filesystem is a WP_Filesystem_FTPext instance that failed to + * connect. Calling any of its methods passes a null connection to PHP's ftp_* + * functions, resulting in a fatal error. This class must therefore never call + * WP_Filesystem. + * + * @since 2.6.1 + */ + public function testLogIsWrittenWhenWPFilesystemIsUnusable() + { + // Replace the WP_Filesystem global with an object that fails the test if any + // of its methods are called. + // phpcs:ignore WordPress.WP.GlobalVariablesOverride.Prohibited + $GLOBALS['wp_filesystem'] = new class() { + /** + * Fails the test if any WP_Filesystem method is called. + * + * @since 2.6.1 + * + * @param string $name Method name. + * @param array $arguments Method arguments. + * + * @throws \RuntimeException If a WP_Filesystem method is called. + */ + public function __call($name, $arguments) + { + throw new \RuntimeException(esc_html('ConvertKit_Log must not call WP_Filesystem::' . $name . '()')); + } + }; + + // Create a legacy log directory, to confirm it is deleted without calling + // WP_Filesystem. + $legacyLogPath = trailingslashit($this->plugin_path) . 'log'; + wp_mkdir_p($legacyLogPath); + file_put_contents(trailingslashit($legacyLogPath) . 'log.txt', ''); // phpcs:ignore WordPress.WP.AlternativeFunctions + + $this->log = new \ConvertKit_Log($this->plugin_path); + $this->log->add('Log entry'); + + // Confirm the legacy log directory was deleted. + $this->assertDirectoryDoesNotExist($legacyLogPath); + + // Confirm the log was written and can be read. + $this->assertTrue($this->log->exists()); + $this->assertStringContainsString('Log entry', $this->log->read()); + } + + /** + * Test that email addresses are masked in log entries. + * + * @since 2.6.1 + */ + public function testEmailAddressesAreMasked() + { + $this->log = new \ConvertKit_Log($this->plugin_path); + $this->log->add('Subscribing user@convertkit.com'); + + $this->assertStringNotContainsString('user@convertkit.com', $this->log->read()); + } +}