Creating a credentials.dat with temporary causes issues

[CLCWebsiteServices]

Hi,

Seems that the package you're using to create the backup-temp folder is creating this with a 0777 permission, thus preventing majority of MySQL installations to ignore the config file.

Here's the output from mine:

mysql: [Warning] World-writable config file '/var/www/static/storage/app/backup-temp/temp/credentials.dat' is ignored.
ERROR 1045 (28000): Access denied for user 'root'@'localhost' (using password: NO)

I also added a lot to ensure it was doing everything correctly prior to it & seems correct, however here's also a copy of the command it's trying to execute

{"cmd":"mysql --defaults-extra-file=\"/var/www/static/storage/app/backup-temp/temp/credentials.dat\" tenant_ZKaQ3K < /var/www/static/storage/app/backup-restore-temp/2023-10-17-04-21-54-e4558f5d-710d-4ff4-b1e5-c97b89bb27cc/db-dumps/mysql-tenant_ZKaQ3K.sql"} 

A simple fix for this, would be to chmod the file once it's created to 0644, I would also like to note that I'm doing this locally on Vagrant and using shared directories which could be the issue; however would persist with a few users.

[stefanzweifel]

Thanks for reporting. I honestly can't tell right know what the right permission settings would be for the storage-folder; have to do some light research on this as well.

We also currently use file_put_contents to store that file. Might have to switch to the Storage-facade. https://laravel.com/docs/10.x/filesystem#local-files-and-visibility

IMHO I would like to refactor this whole "credentials" part completely, but that's not something I will do in the next few days.

[stefanzweifel]

@CLCWebsiteServices Currently thinking about removing the whole approach of using a credentials/extra-file.

Could you maybe share when you're using this package to restore a backup? From your original comment, it looks like you're running this on a production server. What's your use case?

[CLCWebsiteServices]

Sorry for the long delay, I didn't receive a notification to reply!.

But yeah, we're currently running a multi-tenancy platform of which we wanted users to create manual backups & manually restore when they wish. albeit that's why we used this package but the issue we had was tenancy was using the Storage facade, whereas you was storing manually into storage folder, so couldn't access it directly.

Here's what we did instead of using the credentials file!

Inside my RestoreTenant Job, I done the following:

private function extendDbFactory(): void
{
    DbImporterFactory::extend('mysql', new MySql());
}

And then my MySql class looks like the following:

<?php

namespace App\Domains\Backup\Databases;

use Illuminate\Contracts\Encryption\DecryptException;
use Spatie\Backup\Exceptions\CannotCreateDbDumper;
use Spatie\Backup\Tasks\Backup\DbDumperFactory;
use Wnx\LaravelBackupRestore\Databases\DbImporter;

class MySql extends DbImporter
{
    /**
     * @throws CannotCreateDbDumper
     */
    public function getImportCommand(string $dumpFile, string $connection): string
    {
        $dumper = DbDumperFactory::createFromConnection($connection);
        $importToDatabase = $dumper->getDbName();

        $credentialsArray = [
            'host' => config('database.connections.mysql.host'),
            'port' => config('database.connections.mysql.port'),
            'user' => config('database.connections.mysql.username'),
            'password' => config('database.connections.mysql.password'),
        ];

        if (str($dumpFile)->endsWith('gz')) {
            $command = $this->getMySqlImportCommandForCompressedDump($dumpFile, $credentialsArray, $importToDatabase);
        } else {
            $command = $this->getMySqlImportCommandForUncompressedDump($dumpFile, $credentialsArray, $importToDatabase);
        }

        return $command;
    }

    public function getCliName(): string
    {
        return 'mysql';
    }

    private function getMySqlImportCommandForCompressedDump(string $dumpFile, array $credentialsArray, string $importToDatabase): string
    {
        return collect([
            "gunzip < {$dumpFile}",
            '|',
            'mysql',
            '-u', $credentialsArray['user'],
            "-p'".$this->decryptIfEncrypted($credentialsArray['password'])."'",
            '-P', $credentialsArray['port'],
            isset($credentialsArray['host']) ? '-h '.$credentialsArray['host'] : '',
            $importToDatabase,
        ])->filter()->implode(' ');
    }

    private function getMySqlImportCommandForUncompressedDump(string $dumpFile, array $credentialsArray, string $importToDatabase): string
    {
        return collect([
            'mysql',
            '-u', $credentialsArray['user'],
            "-p'".$this->decryptIfEncrypted($credentialsArray['password'])."'",
            '-P', $credentialsArray['port'],
            isset($credentialsArray['host']) ? '-h '.$credentialsArray['host'] : '',
            $importToDatabase,
            '<',
            $dumpFile,
        ])->filter()->implode(' ');
    }

    private function decryptIfEncrypted($value)
    {
        try {
            return decrypt($value);
        } catch (DecryptException $e) {
            return $value;
        }
    }
}

[stefanzweifel]

@CLCWebsiteServices Thanks for sharing your workaround.

Will probably update the package to drop the credentials file as well and use the -u, -p and -P flags directly. I think I've used the credentials.dat file, as I saw that the underlying Spatie package exposed this. Let's keep it simple and pass the credentials to the command directly.

[stefanzweifel]

I've updated the package to no longer use credentials file. Instead we pass username and password as arguments, like what you've done in your application. (#42)

A new version has been released: v1.1.3.

Note: I didn't implement the decryptIfEncrypted-method, as this caused a couple of issues in our test suite. So you might have to stick with your implementation for a while. I plan to rework the whole DbImporter-classes in v2 or even extract them to their own package.