Bugfix: user is not allowed by vitormattos · Pull Request #15187 · nextcloud/server

vitormattos · 2019-04-21T17:55:33Z

The created user using PostgreSQL database don't has permission to connect to database.

I changed the code to give permission

The created user don't has permission to connect to database. I changed the code to give permission Signed-off-by: Vitor Mattos <vitor@php.rio>

Signed-off-by: Vitor Mattos <vitor@php.rio>

vitormattos · 2019-04-22T15:28:00Z

I believe that continuous integration is in trouble. The status is that it is still running.

Another point is that the tests that broke in my PR are not influenced by my PR.

Signed-off-by: Vitor Mattos <vitor@php.rio>

vitormattos · 2019-04-23T11:25:32Z

@shyim done!

…tabase-user

nickvergessen

Fine by me, but can't test

vitormattos · 2019-05-06T18:02:19Z

Hello @nickvergessen!
The tests in the master are breaking, not because of the change I made.

rullzer · 2019-05-11T14:54:46Z

@icewind1991 mind to have a look and check?

vitormattos · 2019-06-03T13:36:40Z

Hello everybody!
This is my first PR to NC.
I want to send more but I need feedback on this PR. Do I need to do any other fixes?
@icewind1991 @rullzer @nickvergessen @shyim

J0WI · 2019-06-07T17:59:39Z

Can we plan this for the next release? nextcloud/docker#345 is a frequently discussed issue.

kesselb · 2019-06-07T18:27:35Z

@J0WI I don't like these create database / database user code. Would it be a problem for the docker images if we remove those? Nextcloud should use the database / database user provided.

kesselb

Make sense 👍

I think you could move the logic into another method. There is already a createDatabase method. If the database already exist we drop all the permission there. You may add the grant statment to this method.

charlag · 2019-06-07T18:56:31Z

In case it helps: I tried this little patch on my system and it finally worked.
Environment described here:
docker-library/postgres#584 (comment)

charlag · 2019-06-07T18:57:26Z

I would also say that using credentials other than provided is super confusing and I was blaming the postgres container for that.
Thanks for the patch.

J0WI · 2019-06-08T11:41:21Z

Would it be a problem for the docker images if we remove those? Nextcloud should use the database / database user provided.

That's fine for Docker. It can already be created in the database container.

Brightside56 · 2019-06-12T13:53:05Z

Oh... I see, power of opensource 👍

vitormattos · 2019-06-25T17:49:47Z

@rullzer have any pending for me to do in this PR?

lukasmrtvy · 2019-08-03T08:28:57Z

Merge?
Its really annoying , because its not possible to upgrade nextcloud ( change docker image version without persistant storage ) !
But You will hit this problem only when You are not using persistent storage ( -v nextcloud:/var/www/html),
entrypoint will always fallback to wizard, which will fail with:

Error while trying to create admin user: Failed to connect to the database: An exception occurred in driver: SQLSTATE[08006] [7] FATAL: permission denied for database "nextcloud"
DETAIL: User does not have CONNECT privilege.

I dont need to use volume ( of course DB is needed ) and this will not work, but it should work..

rullzer

Fine by me

welcome · 2019-08-08T07:03:53Z

Thanks for your first pull request and welcome to the community! Feel free to keep them coming! If you are looking for issues to tackle then have a look at this selection: https://github.com/nextcloud/server/issues?q=is%3Aopen+is%3Aissue+label%3A%22good+first+issue%22
Most developers hang out on IRC. So join #nextcloud-dev on Freenode for a chat!

J0WI · 2019-08-08T20:25:47Z

/backport to stable16

J0WI · 2019-08-08T20:25:53Z

/backport to stable15

J0WI · 2019-08-08T20:26:03Z

/backport to stable14

backportbot-nextcloud · 2019-08-08T20:27:09Z

The backport to stable16 failed. Please do this backport manually.

backportbot-nextcloud · 2019-08-08T20:27:17Z

The backport to stable15 failed. Please do this backport manually.

backportbot-nextcloud · 2019-08-08T20:27:22Z

The backport to stable14 failed. Please do this backport manually.

tianon · 2019-08-09T23:46:13Z

 $query = $connection->prepare("CREATE USER " . addslashes($this->dbUser) . " CREATEDB PASSWORD '" . addslashes($this->dbPassword) . "'");
 $query->execute();
+ if ($this->databaseExists($connection)) {
+ $query = $connection->prepare('GRANT CONNECT ON DATABASE ' . addslashes($this->dbName) . ' TO '.addslashes($this->dbUser));


Given that this is using ->prepare, shouldn't this be using ? and arguments instead of addslashes?
(https://www.php.net/addslashes -- "The addslashes() is sometimes incorrectly used to try to prevent SQL Injection. Instead, database-specific escaping functions and/or prepared statements should be used.")
I think this is one of the roots of what's being discussed over in docker-library/official-images#6252 (comment) (in PostgreSQL, - needs to be escaped, but addslashes won't do so, for example).

This reverts commit 4df6f79. The upstream bug has been fixed in nextcloud/server#15187

This reverts commit 4df6f79. The upstream bug has been fixed in nextcloud/server#15187 Signed-off-by: J0WI <J0WI@users.noreply.github.com>

…cloud#961) This reverts commit 4df6f79. The upstream bug has been fixed in nextcloud/server#15187 Signed-off-by: J0WI <J0WI@users.noreply.github.com>

skjnldsv · 2020-04-10T09:36:28Z

Please backport manually

nickvergessen · 2020-04-10T10:04:19Z

Not needed anymore as it is in 17+ and 16 goes EOL soon

This reverts commit 4df6f79. The upstream bug has been fixed in nextcloud/server#15187 Signed-off-by: J0WI <J0WI@users.noreply.github.com>

Bugfix: user is not allowed
824cc0a
The created user don't has permission to connect to database. I changed the code to give permission Signed-off-by: Vitor Mattos <vitor@php.rio>

vitormattos force-pushed the bugfix-create-database-user branch from 1157db1 to 824cc0aCompare April 21, 2019 18:01

Check if database exists and grand permission
4d187ce
Signed-off-by: Vitor Mattos <vitor@php.rio>

shyim reviewed Apr 22, 2019
View reviewed changes

Comment threadlib/private/Setup/PostgreSQL.php Outdated

solve code review
35465ca
Signed-off-by: Vitor Mattos <vitor@php.rio>

vitormattos added 2 commits April 26, 2019 10:36

Merge remote-tracking branch 'nextcloud/master' into bugfix-create-da…
09f7de0
…tabase-user

Merge remote-tracking branch 'nextcloud/master' into bugfix-create-da…
d12aaa2
…tabase-user

kesselb added 3. to review Waiting for reviews bug labels May 5, 2019

kesselb requested review from nickvergessen and rullzer May 5, 2019 16:05

nickvergessen approved these changes May 6, 2019
View reviewed changes

rullzer requested a review from icewind1991 May 11, 2019 14:54

J0WI added this to the Nextcloud 16.0.2 milestone Jun 7, 2019

kesselb reviewed Jun 7, 2019
View reviewed changes

kesselb mentioned this pull request Jun 17, 2019
Installation with Postgres Fails With Authentication Error #15986
Closed

rullzer mentioned this pull request Jun 24, 2019
16.0.2 RC1 #16064
Merged
5 tasks

J0WI mentioned this pull request Jun 26, 2019
Allow nextcloud to create the postgres db nextcloud/docker#784
Merged

MorrisJobke modified the milestones: Nextcloud 16.0.2, Nextcloud 16.0.3, Nextcloud 16.0.4 Jul 3, 2019

rullzer mentioned this pull request Aug 7, 2019
16.0.4 RC1 #16677
Merged
2 tasks

rullzer modified the milestones: Nextcloud 16.0.4, Nextcloud 17 Aug 8, 2019

rullzer approved these changes Aug 8, 2019
View reviewed changes

rullzer merged commit b42b26e into nextcloud:masterAug 8, 2019

backportbot-nextcloudBot added the backport-request label Aug 8, 2019

tianon reviewed Aug 9, 2019
View reviewed changes

tianon mentioned this pull request Aug 10, 2019
Replace usage of "addslashes" with prepared statements #16708
Closed

J0WI added a commit to J0WI/docker-nextcloud that referenced this pull request Jan 11, 2020
Remove workaround to create the postgres db (fixnextcloud#886)
347e5e5
This reverts commit 4df6f79. The upstream bug has been fixed in nextcloud/server#15187

J0WI mentioned this pull request Jan 11, 2020
Remove workaround to create the postgres db (fix #886) nextcloud/docker#961
Merged

J0WI added a commit to nextcloud/docker that referenced this pull request Jan 13, 2020
Remove workaround to create the postgres db (fix#886) (#961)
fbdb70e
This reverts commit 4df6f79. The upstream bug has been fixed in nextcloud/server#15187 Signed-off-by: J0WI <J0WI@users.noreply.github.com>

nickvergessen removed the backport-request label Apr 10, 2020

J0WI mentioned this pull request Jun 5, 2020
play-with-docker demo doesn't work nextcloud/docker#860
Closed

Uh oh!

Uh oh!

Conversation

vitormattos commented Apr 21, 2019

Uh oh!

vitormattos commented Apr 22, 2019

Uh oh!

Uh oh!

vitormattos commented Apr 23, 2019

Uh oh!

nickvergessen left a comment

Choose a reason for hiding this comment

Uh oh!

vitormattos commented May 6, 2019

Uh oh!

rullzer commented May 11, 2019

Uh oh!

vitormattos commented Jun 3, 2019

Uh oh!

J0WI commented Jun 7, 2019

Uh oh!

kesselb commented Jun 7, 2019

Uh oh!

kesselb left a comment

Choose a reason for hiding this comment

Uh oh!

charlag commented Jun 7, 2019

Uh oh!

charlag commented Jun 7, 2019

Uh oh!

J0WI commented Jun 8, 2019

Uh oh!

Brightside56 commented Jun 12, 2019

Uh oh!

vitormattos commented Jun 25, 2019

Uh oh!

lukasmrtvy commented Aug 3, 2019• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rullzer left a comment

Choose a reason for hiding this comment

Uh oh!

welcomeBot commented Aug 8, 2019

Uh oh!

J0WI commented Aug 8, 2019

Uh oh!

J0WI commented Aug 8, 2019

Uh oh!

J0WI commented Aug 8, 2019

Uh oh!

backportbot-nextcloudBot commented Aug 8, 2019

Uh oh!

backportbot-nextcloudBot commented Aug 8, 2019

Uh oh!

backportbot-nextcloudBot commented Aug 8, 2019

Uh oh!

tianonAug 9, 2019

Choose a reason for hiding this comment

Uh oh!

skjnldsv commented Apr 10, 2020

Uh oh!

nickvergessen commented Apr 10, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

12 participants

lukasmrtvy commented Aug 3, 2019•
edited
Loading