fix #38749, postgresql GRANT user's permission after createDatabase, …

[whlsxl]

…ensure database exist when GRANT

• Resolves: #38749

[come-nc]

I’m not familiar enough with databases to review this

[kesselb]

I think we should remove the code to create database tables and users.

[whlsxl]

Thank you very much for your pull 👍

Could you please provide us some more information about your change?

I'm not familiar with PostgreSQL. Grant create does not work, before creating the database because the schema public does not exist?

Sorry, not familiar with PHP.

This is the docker-compose.yml to reproduce this issue.

version: "3.5"

services:
  postgres:
    container_name: postgres
    image: postgres:15.3-alpine
    restart: unless-stopped
    environment:
      - "POSTGRES_USER=postgres"
      - "POSTGRES_PASSWORD=U8PNOtHPefQ"
      - "POSTGRES_HOST_AUTH_METHOD=trust"
  nextcloud:
    container_name: nextcloud
    image: nextcloud:26.0.2-fpm-alpine
    restart: unless-stopped

After container up, enter the container docker exec -it nextcloud sh & install nextcloud with php occ maintenance:install --database pgsql --database-host postgres:5432 --database-name nextcloud --database-user postgres --database-pass U8PNOtHPefQ --admin-user admin --admin-pass U8PNOtHPefQ --data-dir /var/www/html/data/

got error: PostgreSQL username and/or password not valid

[whlsxl]

I traced this bug. The pull request https://github.com/nextcloud/server/pull/34645 is to fix the https://github.com/nextcloud/server/issues/34617, but I tried many ways, can't reproduce the problem.

I think we should remove the code to create database tables and users.

In my opinion, create users & database & tables are very good functions, because most users are newbies, they don't have the ability to manage the database.

But this function should be refactor, for example https://github.com/nextcloud/server/blob/63bf207ca7a18dd50ce3aeaea42e53f4ee400fc0/lib/private/Setup/PostgreSQL.php#L122-L143 if the database exists, the superuser should resign the owner to nextcloud database user and copy all the permission nextcloud database user.

I can do this job, but still try to figure out how to write the TEST & execute it.

[kesselb]

In my opinion, create users & database & tables are very good functions, because most users are newbies, they don't have the ability to manage the database.

I would prefer that newbies learn how to create a database and database user ;)

We have automated tests for PostgreSQL.

https://github.com/nextcloud/.github/blob/dc0440094e086858cb960e0054a29ca565c5fbbd/workflow-templates/phpunit-pgsql.yml#L49-L52

The notable difference:

• PostgreSQL 14 vs 15
• POSTGRES_DB (we ask the container to create a database named nextcloud)

That means in our automated tests, we do not run the code to create a database table. This code path is apparently broken.

Previous: Doctrine\DBAL\Exception: Failed to connect to the database: An exception occurred in the driver: SQLSTATE[08006] [7] FATAL:  database "nextcloud" does not exist
Trace: #0 /var/www/html/lib/private/Setup/PostgreSQL.php(114): OC\DB\Connection->connect()
#1 /var/www/html/lib/private/Setup.php(353): OC\Setup\PostgreSQL->setupDatabase('admin')

The stack trace is a bit misleading.

https://github.com/nextcloud/server/blob/eb7b682dc08f522212a86636994abc90ea926616/lib/private/Setup/PostgreSQL.php#L65

Here is the problem. connect() uses this->dbName by default. That means we tell doctrine/pdo to connect to a non-existing database.

$connectionMainDatabase = $this->connect([
    'dbname' => 'postgres'
]);

We need to overwrite the dbname (like for $connection a few lines above) and hope that the postgres table will be there.

[whlsxl]

This bug still exists, wish someone could review it.

[susnux]

Can you please adjust your commits to comply with conventional commits? Thank you :)

[whlsxl]

Can you please adjust your commits to comply with conventional commits? Thank you :)

Are you referring to the commit message? can you tell me more specifically?

[susnux]

Are you referring to the commit message? can you tell me more specifically?

Yes the commit message, we try to follow the conventional commits spec: https://www.conventionalcommits.org/

Some examples:

• fix: Something that was wrong
• fix(DB): Something that was wrong in database
• feat: Something new