Skip to content

Fix occ maintenance:install database connect failure - #19303

Merged
nickvergessen merged 4 commits into
masterfrom
test/debug-install
Feb 6, 2020
Merged

Fix occ maintenance:install database connect failure#19303
nickvergessen merged 4 commits into
masterfrom
test/debug-install

Conversation

@nickvergessen

@nickvergessennickvergessen commented Feb 5, 2020

Copy link
Copy Markdown
Member

@nickvergessennickvergessen added the 2. developing Work in progress label Feb 5, 2020
@nickvergessennickvergessen added this to the Nextcloud 19 milestone Feb 5, 2020
@nickvergessennickvergessen self-assigned this Feb 5, 2020
@skjnldsv

Copy link
Copy Markdown
Member

Those gotta be the best commits I've ever seen 😁
image

Provide the auth method for MySQL 8.0+
Signed-off-by: Joas Schilling <coding@schilljs.com>
Signed-off-by: Joas Schilling <coding@schilljs.com>
More gebuging
Signed-off-by: Joas Schilling <coding@schilljs.com>
@nickvergessen
nickvergessen marked this pull request as ready for review February 5, 2020 12:56
@nickvergessennickvergessen added 3. to review Waiting for reviews bug feature: install and update and removed 2. developing Work in progress labels Feb 5, 2020
…orrectly
Signed-off-by: Joas Schilling <coding@schilljs.com>
@kesselb

This comment has been minimized.

@skjnldsvskjnldsv changed the title Test/debug installFix occ maintenance:install database connect failureFeb 5, 2020
@skjnldsv

Copy link
Copy Markdown
Member

Failures unrelated

@skjnldsv

This comment has been minimized.

@skjnldsv

This comment has been minimized.

@skjnldsv

This comment has been minimized.

@nickvergessen

nickvergessen commented Feb 5, 2020

Copy link
Copy Markdown
MemberAuthor

Marking offtopic hides the comments only, or aborts the backport? The commits 1+4 should be backported until 16. 2+3 could be skipped, as they just make debugging easier as it makes handling of exceptions more explicit, but they don't fix a primary error.

@skjnldsv

Copy link
Copy Markdown
Member

Marking offtopic hides the comments only, or aborts the backport?

only hide :)
This was for visibility 😉

@MichaIng

Copy link
Copy Markdown
Member

Okay this fixes the authentication plugin situation currently, but e.g. requires maintenance when new MySQL version come up or MariaDB switches to a different default authentication method, when a password is supplied that way.

An alternative that would be more future prove:

$query = "CREATE USER '$name'@'localhost' IDENTIFIED WITH mysql_native_password;"
$connection->executeUpdate($query);
$query = "ALTER USER '$name'@'localhost' IDENTIFIED BY '$password';"
$connection->executeUpdate($query);

@nickvergessen

Copy link
Copy Markdown
MemberAuthor

I checked that and it didn't work on mysql 8 and mariadb 10.4 with the same Syntax. Password needs to be hashed manually for mariadb etc. So i think that's the better story for now

@skjnldsvskjnldsv added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Feb 6, 2020
@nickvergessen
nickvergessen merged commit c5c3178 into masterFeb 6, 2020
@nickvergessen
nickvergessen deleted the test/debug-install branch February 6, 2020 18:36
@backportbot-nextcloud

Copy link
Copy Markdown

backport to stable18 in #19326

@nickvergessen

Copy link
Copy Markdown
MemberAuthor

/backport to stable17

@nickvergessen

Copy link
Copy Markdown
MemberAuthor

/backport to stable16

@backportbot-nextcloud

Copy link
Copy Markdown

backport to stable17 in #19327

@backportbot-nextcloud

Copy link
Copy Markdown

backport to stable16 in #19328

@MichaIng

Copy link
Copy Markdown
Member

@nickvergessen

I checked that and it didn't work on mysql 8 and mariadb 10.4 with the same Syntax. Password needs to be hashed manually for mariadb etc. So i think that's the better story for now

Strange, works perfectly fine here with MariaDB:

2020-02-06 23:28:45 root@VM-Buster:~$ mariadb
Welcome to the MariaDB monitor. Commands end with ; or \g.
Your MariaDB connection id is 47
Server version: 10.3.18-MariaDB-0+deb10u1 Debian 10
Copyright (c) 2000, 2018, Oracle, MariaDB Corporation Ab and others.
Type 'help;' or '\h' for help. Type '\c' to clear the current input statement.
MariaDB [(none)]> select user,host,plugin,password from mysql.user;
+------+-----------+-------------+----------+
| user | host | plugin | password |
+------+-----------+-------------+----------+
| root | localhost | unix_socket | |
+------+-----------+-------------+----------+
1 row in set (0.000 sec)
MariaDB [(none)]> create user test@localhost identified with mysql_native_password;
Query OK, 0 rows affected (0.001 sec)
MariaDB [(none)]> select user,host,plugin,password from mysql.user;
+------+-----------+-------------+----------+
| user | host | plugin | password |
+------+-----------+-------------+----------+
| root | localhost | unix_socket | |
| test | localhost | | |
+------+-----------+-------------+----------+
2 rows in set (0.000 sec)
MariaDB [(none)]> alter user test@localhost identified by 'test';
Query OK, 0 rows affected (0.001 sec)
MariaDB [(none)]> select user,host,plugin,password from mysql.user;
+------+-----------+-------------+-------------------------------------------+
| user | host | plugin | password |
+------+-----------+-------------+-------------------------------------------+
| root | localhost | unix_socket | |
| test | localhost | | *94BDCEBE19083CE2A1F959FD02F964C7AF4CFC29 |
+------+-----------+-------------+-------------------------------------------+
2 rows in set (0.000 sec)

Also the docs state clearly that plain test password must be used here: https://mariadb.com/kb/en/alter-user/
Hashed password is possible/required when using:

... IDENTIFIED BY PASSWORD 'password_hash'

Such a solution would solve the issue with Nextcloud 16 as well: #19328 (comment)

Sadly Debian does not ship MariaDB even on Bullseye+Sid and no MySQL, hence I cannot test with these.

@nickvergessen

Copy link
Copy Markdown
MemberAuthor

Well yes it works like that, but that doesn't work with mysql8. And if you specify the plugin it requires the hash in mariadb. Anyway the PR we use now fixes it for all supported dbs... Good enough

@MichaIng

Copy link
Copy Markdown
Member

@nickvergessen

And if you specify the plugin it requires the hash in mariadb.

That must be either wrong or new in MariaDB 10.4, as with 10.3 it's not true, as of the two different syntax for plain and hashed password, see above.

but that doesn't work with mysql8

Okay, no chance then. A pain that syntax differs on such basic SQL commands 😢.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

4. to releaseReady to be released and/or waiting for tests to finishbugfeature: install and update

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[17.0.2] "occ maintenance:install" uses "oc_<admin>" before creating it mysql port syntax in config

5 participants

@nickvergessen@skjnldsv@kesselb@MichaIng@juliusknorr