? ? Pending

User tests: Successful: Unsuccessful:

avatar richard67
richard67
1 Jun 2020

Pull Request for Issue # .

Summary of Changes

This Pull Request (PR) solves following issues with database checks when doing a new installation of Joomla 4 (Joomla 3 doesn't have these issues):

  1. When using "MySQL (PDO)" or "PostgreSQL (PDO)" as database type and chosing to create a new database by specifying a user who has privileges for that and a database name for which no database exists, and you are using a database server which does not fulfill the version requirement by the CMS, the version check is done AFTER the new database is created.
    That means you end with the correct error message about the version number and can correct this by specifying another database server, which is fine, but in the old server which you have tried first there is a new, empty database now, and that's not so fine.
  2. When using a database server not equal to "localhost" or "127.0.0.1" or "::1", there is an additional security check with use of a temporary text file so you have to proof ownership of your webspace before you continue. Depending on the database type it can happen that when you enter invalid user name or password, or you use a valid user who doesn't have the privilege to create a new database and a not existing database name, then this additional security check is not executed because the database connection is tested before that.
  3. Because the database connection is validated in two places in J4 there is already a lot of long duplicated code, and correcting the above two issues would produce even more of it. Therefore this PR refactors the checks by moving them into functions in the database helper class, and the setup and the database model use these functions.

With this PR, all checks of database connection parameters before connecting, the connection test and then the checks after connection (version number and encryption support and status) are done at all necessary places in both setup model and database model. This might look redundant at the first look, but it makes sure that during a normal setup we detect problems at the earliest possible stage and still are waterproof for the case that some CLI installer directly uses the database model.

In Joomla 3, the first issue is not an issue because there we don't have separate version requirements by the CMS which are more restrictive than those of the database drivers, and these are already checked at all places.

The second and third issues are not relevant for Joomla 3 because there are no database connection parameters checks in the setup model, they are all at one place in the database model, and creating a new database works only for MySQLi anyway.

Testing Instructions

Requirements

It needs tests with MySQL and with PostgreSQL databases, details see below.

Testers please report back which kind of database you have used for the test. If you have both, please test both.

We can better count then if we have enough tests for both kinds of databases.

For the tests "Test 1" and "Test 2" you need a database server with a version which does not fulfill the CMS' requirement on the minimum server version, which are:

  • 5.6 for MySQL database
  • 10.0 for MariaDB database
  • 11.0 for PostgreSQL database

In case if you don't have such an old database server, instructions how to patch the CMS requirement are given at the beginning of each of these two tests.

In addition it needs a PDO database client for tests "Test 1" and "Test 2" so you can use "MySQL (PDO)" or "PostgreSQL (PDO)".

For the tests "Test 3" and "Test 4" you either need a database server on a remote host, or if you don't have that and have only a local database, make sure that your computer can be accessed with TCP/IP using something else than "localhost" or "127.0.0.1" or "::1", i.e. with some host name. You can add an entry to your local hosts file ("/etc/hosts" on Linux and "C:\Windows\System32\drivers\etc\hosts" on Windows) so that e.g. "myfancyhostname" resolves to "127.0.0.1", and use "myfancyhostname" for the installation.

"Test 3", "Test 4" and "Test 5" can be done with any kind of database server and client.

Test 1: Installation attempt with database creation and failed minimum version check without this PR applied

  1. On a clean current 4.0-dev branch or latest nightly 4.0 build or a 4.0 Beta 1, modify the CMS' database server version requirement depending on your database type if you don't have an old database server which doesn't fulfill the requirements anyway.
    You can do that by setting the variable for your database type to a value higher than what you really have here at this place:
    https://github.com/joomla/joomla-cms/blob/4.0-dev/installation/src/Helper/DatabaseHelper.php#L23-L48
  2. Start a new installation.
  3. When coming to the database details, select as database type one of the PDO drivers depending on your database type, e.g. "MySQL (PDO)" or "PostgreSQL (PDO)". Don't use "MySQLi".
  4. Fill in username and password for a user who has the premission to create a new database on the database server you want to use. On local standard installations of MySQL or MariaDB this is normally a user named "root" and on PostgreSQL databases "postgres".
  5. Fill in a database name for which no database exists, and complete all other neccessary field.
  6. Use the "Install Joomla" button.
    Result: An alert is shown, telling that the database server version is not sufficient, e.g. for MySQL:
    j4-installation-tweaks-1_1
  7. Using a tool like e.g. phpMyAdmin or phpPgAdmin, depending on your database type, check if there is an empty database with the same name as specified during the installation.

Result: There is an empty database with this name.

Test 2: Installation attempt with database creation and failed minimum version check with this PR applied

  1. On a clean current 4.0-dev branch or latest nightly 4.0 build or a 4.0 Beta 1 not being modified as described in step 1 of the previous test "Test 1", apply the patch of this PR.
  2. Now modify the CMS' database server version requirement depending on your database type if you don't have an old database server which doesn't fulfill the requirements anyway.
    You can do that by setting the variable for your database type to a value higher than what you really have here at this place:
    /**
    * The minimum database server version for MariaDB databases as required by the CMS.
    * This is not necessarily equal to what the database driver requires.
    *
    * @var string
    * @since 4.0.0
    */
    protected static $dbMinimumMariaDb = '10.1';
    /**
    * The minimum database server version for MySQL databases as required by the CMS.
    * This is not necessarily equal to what the database driver requires.
    *
    * @var string
    * @since 4.0.0
    */
    protected static $dbMinimumMySql = '5.6';
    /**
    * The minimum database server version for PostgreSQL databases as required by the CMS.
    * This is not necessarily equal to what the database driver requires.
    *
    * @var string
    * @since 4.0.0
    */
    protected static $dbMinimumPostgreSql = '11.0';
  3. Execute steps 2 to 5 of the previous test "Test 1", either using again a new database name or, if using the same, having removed that database before starting the installation.

Result: There is no empty database with this name.

Test 3: Installation attempt with remote database host and wrong database credentials without this PR applied

  1. On a clean current 4.0-dev branch or latest nightly 4.0 build or a 4.0 Beta 1 without the patch of this PR applied, start a new installation.
  2. When coming to the database details fill in a database name for which an empty database exists.
  3. Fill in a valid host name not equal to "localhost" or "127.0.0.1" or "::1" (see requirements above).
  4. Fill in an invalid user name or password.
  5. Fill in everything else right.
  6. Use the "Install Joomla" button.
    Result: An alert shows an error message due to failed database user authentication.
  7. Correct user name and password so all is correct.
  8. Use again the "Install Joomla" button.
    Result: You see a warning alert with information about where to find documentation for the remote database extra security check and an information alert telling what to do.
  9. Proceed as adviced in the info alert.
  10. Use again the "Install Joomla" button.
    Result: The installation finish with success.

Test 4: Installation attempt with remote database host and wrong database credentials with this PR applied

Note: If you execute this test after the previous one you have to make sure all session data is cleared, i.e. either delete the session cookie or close the browser window and open a new one.

  1. On a clean current 4.0-dev branch or latest nightly 4.0 build or a 4.0 Beta 1, apply the patch of this PR.
  2. Start a new installation.
  3. Execute steps 2 to 6 of the previous test "Test 3".
    Result: You see a warning alert with information about where to find documentation for the remote database extra security check and an information alert telling what to do.
  4. Proceed as adviced in the info alert.
  5. Use again the "Install Joomla" button.
    Result: An alert shows an error message due to failed database user authentication.
  6. Correct user name and password so all is correct.
  7. Use again the "Install Joomla" button.
    Result: The installation finish with success.

Test 5: Free test - Try to fool the installations database checks with this PR applied

As the title of this test says: Try to fool the installation's database checks on a current 4.0-dev or latest 4.0 nightly build with the patch of this PR applied.

If possible, test with all available database types.

Enter first wrong things, then correct and continue, or enter again other wrong things, like wrong username or password or a not existing database without the user having permissions to create one, or invalid database names e.g. with not allowed special characters, whatever comes into your mind.

Especially in case of the MySQLi driver, error messages are sometimes not very englighting, but there are some in any case, and you always come back to the form so you can correct your entries and try again.

Make sure that and the end when you have corrected everything in the form after the nth attempt so that all is right, you can finish the installation with success.

Expected result

  1. When using "MySQL (PDO)" or "PostgreSQL (PDO)" as database type and chosing to create a new database by specifying a user who has privileges for that and a database name for which no database exists, and you are using a database server which does not fulfill the version requirement by the CMS, the version check is done BEFORE the new database is created.
  2. When using a database host name not equal to "localhost" or "127.0.0.1" or "::1", the additional security check with use of a temporary text file to proof the acess to the webspace is performed after all checks of input data but BEFORE the database connection is tested.

Actual result

  1. When using "MySQL (PDO)" or "PostgreSQL (PDO)" as database type and chosing to create a new database by specifying a user who has privileges for that and a database name for which no database exists, and you are using a database server which does not fulfill the version requirement by the CMS, the version check is done AFTER the new database is created.
  2. When using a database host name not equal to "localhost" or "127.0.0.1" or "::1", the additional security check with use of a temporary text file to proof the acess to the webspace is performed after all checks of input data and AFTER the database connection is tested.

Documentation Changes Required

None.

Votes

# of Users Experiencing Issue
1/1
Average Importance Score
5.00

avatar richard67 richard67 - open - 1 Jun 2020
avatar richard67 richard67 - change - 1 Jun 2020
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 1 Jun 2020
Category Installation
avatar Quy
Quy - comment - 1 Jun 2020

OK to close PR #25144?

avatar richard67
richard67 - comment - 1 Jun 2020

@Quy If you mean because this PR here could make that other PR obsolete: No, these 2 PRs are independent. This one here only makes sure there is an error raised for PostgreSQL if the table is not lowercase.

avatar richard67 richard67 - change - 1 Jun 2020
Title
[4.0] Refactor and strengthen database checks at installation
[4.0] Refactor and harden database checks at installation
avatar richard67 richard67 - edited - 1 Jun 2020
avatar richard67 richard67 - change - 1 Jun 2020
The description was changed
avatar richard67 richard67 - edited - 1 Jun 2020
avatar richard67 richard67 - change - 1 Jun 2020
The description was changed
avatar richard67 richard67 - edited - 1 Jun 2020
avatar richard67 richard67 - change - 1 Jun 2020
The description was changed
avatar richard67 richard67 - edited - 1 Jun 2020
avatar richard67 richard67 - change - 1 Jun 2020
The description was changed
avatar richard67 richard67 - edited - 1 Jun 2020
avatar richard67 richard67 - change - 1 Jun 2020
The description was changed
avatar richard67 richard67 - edited - 1 Jun 2020
avatar richard67 richard67 - change - 1 Jun 2020
The description was changed
avatar richard67 richard67 - edited - 1 Jun 2020
avatar richard67 richard67 - change - 1 Jun 2020
The description was changed
avatar richard67 richard67 - edited - 1 Jun 2020
avatar richard67 richard67 - change - 1 Jun 2020
The description was changed
avatar richard67 richard67 - edited - 1 Jun 2020
avatar richard67 richard67 - change - 1 Jun 2020
The description was changed
avatar richard67 richard67 - edited - 1 Jun 2020
avatar richard67 richard67 - change - 1 Jun 2020
The description was changed
avatar richard67 richard67 - edited - 1 Jun 2020
avatar richard67 richard67 - change - 1 Jun 2020
The description was changed
avatar richard67 richard67 - edited - 1 Jun 2020
avatar richard67 richard67 - change - 1 Jun 2020
The description was changed
avatar richard67 richard67 - edited - 1 Jun 2020
avatar richard67 richard67 - change - 1 Jun 2020
The description was changed
avatar richard67 richard67 - edited - 1 Jun 2020
avatar richard67 richard67 - change - 1 Jun 2020
The description was changed
avatar richard67 richard67 - edited - 1 Jun 2020
avatar richard67 richard67 - change - 1 Jun 2020
The description was changed
avatar richard67 richard67 - edited - 1 Jun 2020
avatar richard67 richard67 - change - 1 Jun 2020
Labels Added: ?
avatar richard67 richard67 - change - 1 Jun 2020
The description was changed
avatar richard67 richard67 - edited - 1 Jun 2020
avatar richard67 richard67 - change - 1 Jun 2020
The description was changed
avatar richard67 richard67 - edited - 1 Jun 2020
avatar richard67 richard67 - change - 1 Jun 2020
The description was changed
avatar richard67 richard67 - edited - 1 Jun 2020
avatar richard67 richard67 - change - 1 Jun 2020
The description was changed
avatar richard67 richard67 - edited - 1 Jun 2020
avatar richard67 richard67 - change - 1 Jun 2020
The description was changed
avatar richard67 richard67 - edited - 1 Jun 2020
avatar richard67 richard67 - change - 1 Jun 2020
The description was changed
avatar richard67 richard67 - edited - 1 Jun 2020
avatar richard67 richard67 - change - 1 Jun 2020
The description was changed
avatar richard67 richard67 - edited - 1 Jun 2020
avatar richard67 richard67 - change - 1 Jun 2020
The description was changed
avatar richard67 richard67 - edited - 1 Jun 2020
avatar richard67 richard67 - change - 1 Jun 2020
The description was changed
avatar richard67 richard67 - edited - 1 Jun 2020
avatar richard67 richard67 - change - 1 Jun 2020
The description was changed
avatar richard67 richard67 - edited - 1 Jun 2020
avatar SniperSister
SniperSister - comment - 2 Jun 2020

@wilsonge added a Release Blocker label because of the security impact

avatar richard67
richard67 - comment - 2 Jun 2020

@alikon Another one which needs PostgreSQL tests, too (as well as MySQL). Could you test this, too? Thanks in advance.

avatar richard67
richard67 - comment - 7 Jun 2020

@SniperSister @zero-24 Since people seem to be reluctant to test this PR: Maybe we can delegate testing to the SST?

avatar fancyFranci
fancyFranci - comment - 7 Jun 2020

I followed your instructions and in general: Yes it is working. The database is created after everything else. The security step comes before the failing authentication.

But I have another problem. When I click "Install Joomla", the message "Error The installation process failed." appears. After a second click: "Error JINVALID_TOKEN_NOTICE" and finally a third click brings up "Error You need MariaDB 10.5 or higher to continue the installation. Your version is: 10.4.6-MariaDB". At least without the patch. I don't receive the last message with the patch. Only the first message again.

Is my test successful now and this problem is out of scope for this patch? I freshly cloned and installed everything before testing this patch.

dbserver: mysql
dbversion: 10.4.6-MariaDB
phpversion: 7.3.9
server: Apache/2.4.41 (Win64) OpenSSL/1.1.1c PHP/7.3.9
version: Joomla! 4.0.0-beta2-dev Development [ Mañana ] 30-May-2020 21:13 GMT
useragent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/83.0.4103.61 Safari/537.36


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/29362.

avatar richard67
richard67 - comment - 7 Jun 2020

@fancyFranci I would say it is out of scope of this PR. No idea what's happening. Only thing maybe could be that if you do a test of this PR soon after a previous test of this PR, still using the same broswer session, the installation may use old session data stored in the session cookie. Do you get this "Error JINVALID_TOKEN_NOTICE" if you delete all cookies from the domain (or localhost) you use for testing, or close browser windows between the tests (that should be enough)?

avatar fancyFranci
fancyFranci - comment - 7 Jun 2020

The "JINVALID_TOKEN_NOTICE" message appears when I'm not leaving the form/session/browser. Just click "Install Joomla" again, after getting "The installation process failed". It is confusing, that I get the correct (and much more helpful) message, after clicking the button a third time (same window, same session). Unfortunately only without your patch.

I would really like to give you a successful test, but I can't do that, with the missing (helpful) error message :/ I hope it is a problem on my system only. At least Test 3 + 4 are working!


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/29362.

avatar richard67
richard67 - comment - 7 Jun 2020

@fancyFranci For the tests 1 and 2, did you use a user who has the permission to create a database? On normal local installations this is "root" for MySQL or MariaDB, and "postgres" for PostgreSQL databases. Can you install J4 with such a user with creating a new database, i.e. specify not existing database during installation, when this PR is not applied and the hack for the version number requirement is not made, i.e. a normal 4.0-dev branch or nightly or Beta 1?

avatar richard67
richard67 - comment - 7 Jun 2020

@fancyFranci P.S.: When you have made an installation and now delete configuration.php to make a new one, then you should close your browser window, otherwise you get this annoying "JINVALID_TOKEN_NOTICE". This has nothing to do with this PR and it not really an issue, since normally nobody installs Joomla and then deleted configuration.php to install it again. So this is really unrelated to this PR, regardless if in this case there is a useful error message or not with or without my PR. The question is: Does my PR work if you use a new browser session for Test 2 after you have made Test 1, also in a new session?

avatar richard67
richard67 - comment - 7 Jun 2020

@wilsonge Can you confirm my 2 last comments before this one?

avatar PhilETaylor PhilETaylor - test_item - 7 Jun 2020 - Tested successfully
avatar PhilETaylor
PhilETaylor - comment - 7 Jun 2020

I have tested this item successfully on 28965e0

This PR fixes the issues reported to the @joomla/security JSST by me post beta1 //@SniperSister


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/29362.

avatar richard67
richard67 - comment - 7 Jun 2020

@PhilETaylor Thanks for testing.

@fancyFranci Thanks for testing, even if the result is not (yet) what I expect. But maybe you could have a look again? Anyway, even if not, I really appreciate your efforts. I know it's not little work.

avatar richard67 richard67 - change - 7 Jun 2020
Labels Added: ?
avatar richard67
richard67 - comment - 7 Jun 2020

@PhilETaylor Sorry for the inconvenience. Could you test again with the last change? It should not make a difference, so maybe even a code review is sufficient. Then set your test result again in the issue tracker? Thanks in advance.

@Quy Done. Will you do a test?

avatar richard67
richard67 - comment - 10 Jun 2020

@PhilETaylor Sorry for the inconvenience. Could you test again? Thanks in advance.

avatar richard67
richard67 - comment - 10 Jun 2020

@fancyFranci Could you repeat your test? I know it's time consuming. But there was a change in J4 ( #29556 ) which could possibily help here with the problems you had in tests 1 and 2. And clear the session cookie or close the browser window at the beginning of test 1 and test 2. Thanks in advance.

avatar PhilETaylor PhilETaylor - test_item - 11 Jun 2020 - Tested successfully
avatar PhilETaylor
PhilETaylor - comment - 11 Jun 2020

@PhilETaylor Sorry for the inconvenience. Could you test again? Thanks in advance.

I have tested this item successfully on dae1a3b

I have only tested the security aspect to resolve the issue I reported to the JSST.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/29362.
avatar wilsonge wilsonge - change - 11 Jun 2020
Status Pending Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2020-06-11 20:49:26
Closed_By wilsonge
avatar wilsonge wilsonge - close - 11 Jun 2020
avatar wilsonge wilsonge - merge - 11 Jun 2020
avatar wilsonge
wilsonge - comment - 11 Jun 2020

Thanks!

Add a Comment

Login with GitHub to post a comment