nextcloud / user_sql

🔒 App for authenticating Nextcloud users using SQL
GNU Affero General Public License v3.0
66 stars 33 forks source link

Error NC29 #193

Open RealKoenisch opened 4 months ago

RealKoenisch commented 4 months ago

{"reqId":"wwrjPQE2Tzll6lg4zzJD","level":3,"time":"2024-04-24T19:20:38+00:00","remoteAddr":"94.219.34.169","user":"XXXX","app":"user_sql","method":"GET","url":"/settings/admin/logging","message":"Could not execute the query: SELECT COUNT(u.username) AS count FROM aase6_users u WHERE u.username LIKE :search ","userAgent":"Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/124.0.0.0 Safari/537.36","version":"29.0.0.19","exception":{"Exception":"Doctrine\DBAL\Exception\TableNotFoundException","Message":"An exception occurred while executing a query: SQLSTATE[42S02]: Base table or view not found: 1146 Table 'owncloud.aase6_users' doesn't exist","Code":1146,"Trace":

The table is aase6_user. Ther could be something wrong with the parameters or binding of parameter (in PDO/Statment.php passing $params to statement is depreceated.

kainhofer commented 4 months ago

I have the same problem, but I'm trying to access a view in a different mysql database (dbname=xxx_admidio) on the same server. I did a little debugging and it seems that even though the user_sql passes in the correct dbname in the $parameters to the $connectionFactory->getConnection call in lib/Query/DataQuery.php, the connection is created to the default nextcloud database...

If I add the following code after the getConnection call:

$result = $this->connection->fetchOne("SELECT DATABASE();");
$this->logger->warning("MySQL: Connected to DB ".$result);

The log output is: "MySQL: Connected to DB xxx_nextcloud" rather than the correct database xxx_admidio

kainhofer commented 4 months ago

After some more debugging, I seem to understand how the problem appeared: In the commit https://github.com/nextcloud/server/commit/79c4986354da2c7dd102de21174114e7687daf98, the Nextcloud server code switched from using \Doctrine\DBAL\Connection for the DB connections to Doctrine\DBAL\Connections\PrimaryReadReplicaConnection.

Apparently PrimaryReadReplicaConnection uses a different array structure for its arguments (connection params are stored in the ['primary'] sub-array instead of first-level array elements). Unfortunately, the code in ConnectionFactory::getConnection (https://github.com/nextcloud/server/blob/147426c3ca7183ad065293bd9b600e10adde4abf/lib/private/DB/ConnectionFactory.php#L129) merges in the connection parameters as first-level arguments (as was required for the old Connection class) rather than into the ['primary'] sub-array, which is now needed for the PrimaryReadReplicaConnection.

So, while the old code needed

Array(
    'host' => "localhost",
    'password' => "myPassword",
    'user' => "DBUsername",
    'dbname' => "DBUserPassword"
)

the new Primary ReadReplicaConnection now needs them in the primary sub-array:

Array(
    'primary' => Array(
        'host' => "localhost",
        'password' => "myPassword",
        'user' => "DBUsername",
        'dbname' => "DBUserPassword"
    )
)

For a difference of the params, I found this nice page: https://blog.adamcameron.me/2023/01/php-primaryreadreplicaconnection.html

Without that page, I would still be debugging...

I suppose the correct fix would be to properly fix the ConnectionFactory to override the primary connection data instead of merging the params at top-level, where they are never used. I'll try to submit a issue with the nextcloud server repo.

kainhofer commented 4 months ago

As a quick fix (ONLY for NC29!!!), one can adjust the user_sql code with the following patch:

diff --git a/lib/Query/DataQuery.php b/lib/Query/DataQuery.php
index 1a7e2ae..51f3917 100644
--- a/lib/Query/DataQuery.php
+++ b/lib/Query/DataQuery.php
@@ -168,7 +168,7 @@ class DataQuery
         }

         $this->connection = $connectionFactory->getConnection(
-            $this->properties[DB::DRIVER], $parameters
+           $this->properties[DB::DRIVER], array("primary" => $parameters)
         );

         $this->logger->debug(

However, this (1) depends on the specific Nextcloud Server version, (2) brings in the logic from the underlying ConnectionFactory implementation to third-party apps (i.e. it's not transparent) for no good reason and (3) might break at any time the nextcloud server chooses to fix this issue.

RealKoenisch commented 4 months ago

Positiv: I tried your small patch - it works in NC-29.

FischFischer commented 3 months ago

Where / at which file did you do your changes?

RealKoenisch commented 3 months ago

Where / at which file did you do your changes?

The changes are in the fork from rotdrop. Not Master but cafevdb/stable 29 https://github.com/rotdrop/user_sql/tree/production/cafevdb/stable29

The Changes are made in "lib/Query/DataQuery.php"

The git is out of date - is there someone who can implement the changes to this version?

KR

d-jsb commented 2 months ago

Any news on this? While the patches shown above allow at least to login again. Groups are not working for me with NC29 All file shares are gone for the users as they are group based in my installation :( Is user_sql officially supported by nextcloud and if yes, why don't they check if it breaks when touching the server code? :/

StefanSander3 commented 1 week ago

Hey @RealKoenisch,

what is the state with this issue (see last comment) and this app in general. For some time now we can not use our nextcloud instance.