Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PostgreSQL : duplicate key value violates unique constraint error message #6343

Closed
cyayon opened this issue Sep 2, 2017 · 39 comments · Fixed by #21037
Closed

PostgreSQL : duplicate key value violates unique constraint error message #6343

cyayon opened this issue Sep 2, 2017 · 39 comments · Fixed by #21037
Labels
1. to develop Accepted and waiting to be taken care of bug

Comments

@cyayon
Copy link

cyayon commented Sep 2, 2017

How to use GitHub

  • Please use the 👍 reaction to show that you are affected by the same issue.
  • Please don't comment if you have no relevant information to add. It's just extra noise for everyone subscribed to this issue.
  • Subscribe to receive notifications on status change and new comments.

Steps to reproduce

just logon on nextcloud and check syslog/postgresql log messages

Server configuration

Nextcloud version : 12.0.2
Operating system and version : archlinux (current)
Apache or nginx version : nginx 1.12.1
PHP version : 7.1.7
PostgreSQL : 9.6.3

Hi all !

I encounter a postgresql error message in my syslog everytime i login on my nextcloud instance :

ERROR: duplicate key value violates unique constraint "oc_credentials_pkey"
DETAIL: Key (“user”, identifier)=(myuser, password::logincredentials/credentials) already exists.
STATEMENT: INSERT INTO “oc_credentials” (“user”, “identifier”, “credentials”) VALUES($1, $2, $3)

No problem for login/logout and using nextcloud, only this error message.

Thanks !

@MorrisJobke
Copy link
Member

Could you try this one here as well: #366 (comment)

Thanks.

@cyayon
Copy link
Author

cyayon commented Sep 3, 2017

Hi,

I just tried redis-cli FLUSHALL command (return OK), but the PostrgreSQL error message continue to appears after each logon ...

@MorrisJobke
Copy link
Member

cc @icewind1991 @nickvergessen

@MorrisJobke MorrisJobke added 0. Needs triage Pending check for reproducibility or if it fits our roadmap bug and removed needs info labels Sep 3, 2017
@ghost
Copy link

ghost commented Sep 4, 2017

@cyayon Are you using LDAP Auth?

@cyayon
Copy link
Author

cyayon commented Sep 4, 2017

Hi,

No, i am using simple locale authentication.

@Gottox
Copy link

Gottox commented Oct 24, 2017

Same issue for me:

Nextcloud version : 12.0.3
Operating system and version : Voidlinux
Apache or nginx version : nginx 1.12.1
PHP version : 7.1.7
PostgreSQL : 9.6.5

Local auth, no redis cache.

@enoch85
Copy link
Member

enoch85 commented Oct 25, 2017

I'm also affected.

2017-10-25 20:26:07 CEST [2992-1] user@nextcloud_db ERROR:  duplicate key value violates unique constraint "oc_credentials_pkey"
2017-10-25 20:26:07 CEST [2992-2] user@nextcloud_db DETAIL:  Key ("user", identifier)=(beate, password::logincredentials/credentials) already exists.
2017-10-25 20:26:07 CEST [2992-3] user@nextcloud_db STATEMENT:  INSERT INTO "oc_credentials" ("user", "identifier", "credentials") VALUES($1, $2, $3)
2017-10-25 20:27:23 CEST [3035-1] user@nextcloud_db ERROR:  duplicate key value violates unique constraint "oc_credentials_pkey"
2017-10-25 20:27:23 CEST [3035-2] user@nextcloud_db DETAIL:  Key ("user", identifier)=(Dario, password::logincredentials/credentials) already exists.
2017-10-25 20:27:23 CEST [3035-3] user@nextcloud_db STATEMENT:  INSERT INTO "oc_credentials" ("user", "identifier", "credentials") VALUES($1, $2, $3)
2017-10-25 20:28:34 CEST [3080-1] user@nextcloud_db ERROR:  duplicate key value violates unique constraint "oc_credentials_pkey"
2017-10-25 20:28:34 CEST [3080-2] user@nextcloud_db DETAIL:  Key ("user", identifier)=(Dario, password::logincredentials/credentials) already exists.
2017-10-25 20:28:34 CEST [3080-3] user@nextcloud_db STATEMENT:  INSERT INTO "oc_credentials" ("user", "identifier", "credentials") VALUES($1, $2, $3)
2017-10-25 20:30:39 CEST [3213-1] user@nextcloud_db ERROR:  duplicate key value violates unique constraint "oc_credentials_pkey"
2017-10-25 20:30:39 CEST [3213-2] user@nextcloud_db DETAIL:  Key ("user", identifier)=(Dario, password::logincredentials/credentials) already exists.
2017-10-25 20:30:39 CEST [3213-3] user@nextcloud_db STATEMENT:  INSERT INTO "oc_credentials" ("user", "identifier", "credentials") VALUES($1, $2, $3)
2017-10-25 20:35:04 CEST [3364-1] user@nextcloud_db ERROR:  duplicate key value violates unique constraint "oc_credentials_pkey"
2017-10-25 20:35:04 CEST [3364-2] user@nextcloud_db DETAIL:  Key ("user", identifier)=(Dario, password::logincredentials/credentials) already exists.
2017-10-25 20:35:04 CEST [3364-3] user@nextcloud_db STATEMENT:  INSERT INTO "oc_credentials" ("user", "identifier", "credentials") VALUES($1, $2, $3)

PostgreSQL 9.6.5
PHP 7.0
APache 2.4
Nextcloud 12.0.3

This is even after I did FLUSALL on Redis. Think I forgot to try to login again the last time I tried.

@aventrax
Copy link

aventrax commented Nov 4, 2017

Yes, same here.

@nickvergessen
Copy link
Member

Well the issue is actually:

} catch (ConstraintViolationException $e) {

Maybe we should change that to "insert if not exists" instead. to avoid logging the error (which we catch php-wise afterwards).

@nickvergessen nickvergessen removed 0. Needs triage Pending check for reproducibility or if it fits our roadmap feature: external storage labels Nov 13, 2017
@enoch85
Copy link
Member

enoch85 commented Nov 13, 2017

Yes please.

@ghost
Copy link

ghost commented Feb 15, 2018

Still having this issue in NC 13...

@Drezil
Copy link

Drezil commented Feb 16, 2018

Having the same issue.

Why are you trying to insert and only then try to update? The normal case would be that a user already exists in the database.... IF you want to do it that way it would make sense to do it the other way around (i.e. first try to update and if that fails insert) ... although this still looks to me like horrible style..

(NC13 from git with stable13-branch and postgres 9.4)

@ghost
Copy link

ghost commented Feb 26, 2018

Let me advocate for the UPSERT once more, then I'll shut up about it 🙃

It does exactly that:

Maybe we should change that to "insert if not exists" instead

It's implemented in pgsql >= 9.5 and would do the job on the DB. It could eliminate the need for the entire catch (ConstraintViolationException $e) block.

For me the concept of

try 
  insert
catch
  update

sounds error prone. A little hacky, if you will.

@snw35
Copy link

snw35 commented May 4, 2018

I've found this issue after seeing this constantly in my logs:

ERROR: duplicate key value violates unique constraint "oc_credentials_pkey"
DETAIL: Key ("user", identifier)=(Username, password::logincredentials/credentials) already exists.
STATEMENT: INSERT INTO "oc_credentials" ("user", "identifier", "credentials") VALUES($1, $2, $3)

Please, please do not have this as normal/expected behaviour. I thought something was genuinely wrong with my install and have spent a fair bit of time on and off trying to diagnose it.

It looks from #366 and the above posts that both the cause and solution are known?

Nextcloud 13.0.2
Postgres 10.3
PHP 7.1.6

I love Nextcloud and you work - hope this can be fixed and an error-free install can have error-free logs.

@nextcloud-bot nextcloud-bot added the stale Ticket or PR with no recent activity label Jun 20, 2018
@ghost
Copy link

ghost commented Sep 6, 2018

Still having this issue in NC 14...

Edit: config++
Nextcloud 14.0.0
Postgres 10.5
PHP 7.2.9
nginx 1.14.0
Archlinux

@nextcloud-bot nextcloud-bot removed the stale Ticket or PR with no recent activity label Sep 6, 2018
@ghost
Copy link

ghost commented Sep 8, 2018

Shouldn't this be flagged as a security issue? Having nextcloud fill up systemd logs makes it difficult to read them and look for issues...

For the moment I raised postgres log level to "critical", but then i wont know if the other services that use postgres run properly. :/

@koehn
Copy link

koehn commented Sep 8, 2018 via email

@ovk
Copy link

ovk commented Mar 31, 2019

Any updates on this issue? Any plans to get it fixed? Any possible workarounds?

As it was mentioned above, this is definitely a security issue as constantly flooding the logs with error messages makes it very difficult to audit.

@tycho
Copy link

tycho commented Jul 15, 2019

Adding a comment so this issue doesn't get auto-marked "stale" again. Issue is still present in 16.0.3.

@arno01
Copy link

arno01 commented Oct 18, 2019

The same with the Nextcloud 16.0.5.
Each time the user logs in via Web or the Client, the postgresql throws these 3 lines:

postgres12_1    | 2019-10-18 13:22:14.000 UTC [840] ERROR:  duplicate key value violates unique constraint "oc_credentials_pkey"
postgres12_1    | 2019-10-18 13:22:14.000 UTC [840] DETAIL:  Key ("user", identifier)=(cooper, password::logincredentials/credentials) already exists.
postgres12_1    | 2019-10-18 13:22:14.000 UTC [840] STATEMENT:  INSERT INTO "oc_credentials" ("user", "identifier", "credentials") VALUES($1, $2, $3)

Additional info

Strange part is that SELECT user FROM oc_credentials isn't returning the users which I see in the oc_credentials:

nextcloud=# SELECT * FROM oc_credentials;
     user      |               identifier               | credentials
---------------+----------------------------------------+-------------
 tony          | password::global                       | REDACTED
 tony          | password::logincredentials/credentials | REDACTED
 cooper        | password::logincredentials/credentials | REDACTED
(3 rows)

nextcloud=# SELECT user FROM oc_credentials;
 user  
-------
 admin
 admin
 admin
(3 rows)
nextcloud=# \d+ oc_credentials
                                        Table "public.oc_credentials"
   Column    |         Type          | Collation | Nullable | Default | Storage  | Stats target | Description 
-------------+-----------------------+-----------+----------+---------+----------+--------------+-------------
 user        | character varying(64) |           | not null |         | extended |              | 
 identifier  | character varying(64) |           | not null |         | extended |              | 
 credentials | text                  |           |          |         | extended |              | 
Indexes:
    "oc_credentials_pkey" PRIMARY KEY, btree ("user", identifier)
    "credentials_user" btree ("user")
Access method: heap

This is how the table was created:

CREATE TABLE public.oc_credentials (
    "user" character varying(64) NOT NULL,
    identifier character varying(64) NOT NULL,
    credentials text
);

ALTER TABLE public.oc_credentials OWNER TO nextcloud;

ALTER TABLE ONLY public.oc_credentials
    ADD CONSTRAINT oc_credentials_pkey PRIMARY KEY ("user", identifier);

COPY public.oc_credentials ("user", identifier, credentials) FROM stdin;
...redacted...
\.

CREATE INDEX credentials_user ON public.oc_credentials USING btree ("user");

@ovk
Copy link

ovk commented Oct 20, 2019

Unfortunately, the same happens with Nextcloud 17.0. Tried with Postgres 11.4 and 12.0.

Are there any plans to address this issue? Any help needed?

@kesselb

This comment has been minimized.

@arsylum
Copy link

arsylum commented Nov 27, 2019

^ Testing it since > 1 week, no side effects as far as I can tell, no more postgres error spam in syslog 👍 maybe a proper review from someone who knows this part of the codebase won't hurt, but I think this should go upstream

@kesselb

This comment has been minimized.

@lukasjuhrich
Copy link

@kesselb Perhaps it's time for a pull request? It doesn't seem like anyone will react to your comment (tbf, there are 2k open issues and you didn't mention anybody in your last reply).

@SSpecken
Copy link

@kesselb Your patch solved our issues with PostgreSQL/LDAP/oc_credentials.

@skjnldsv
Copy link
Member

@kesselb so what is the status of this issue in the end? :)

@kesselb
Copy link
Contributor

kesselb commented Apr 10, 2020

I added some more background about that issue here: #19494 (comment)

My patch might fix the issue or not. At least it seems to work for some people. I'm not interested to dig any further because MariaDB is my preferred RDBMS.

One benefit from the patch is that the credentials are not written again on every login (for every RDBMS).

@J0WI
Copy link
Contributor

J0WI commented Apr 10, 2020

Would you like to open a PR with your patch?

@kesselb
Copy link
Contributor

kesselb commented Apr 10, 2020

Would you like to open a PR with your patch?

I don't have time to look into the adjustments for the tests.

@kesselb
Copy link
Contributor

kesselb commented Apr 12, 2020

I marked the patches as outdated. They both assume that credentials are always an array. That's at least not true if you use the ldap_contacts_backend app and probably for others apps as well.

@ghost

This comment has been minimized.

@ghost ghost added the stale Ticket or PR with no recent activity label May 12, 2020
@sophie-h

This comment has been minimized.

@ghost ghost removed the stale Ticket or PR with no recent activity label May 12, 2020
@kesselb kesselb added 1. to develop Accepted and waiting to be taken care of and removed needs info 0. Needs triage Pending check for reproducibility or if it fits our roadmap labels May 12, 2020
@fwgx

This comment has been minimized.

@krono
Copy link

krono commented Aug 24, 2020

Is this really fixed?
(I'm not running the code yet, sorry)
From what I read in #21037, this ensures that the credentials are written much less often, which is very helpful.

If I understand correctly, once you have a need for the credentials, they'll be touched very often, again.

Wouln'd it rather help if either CredentialsManager::store would use Connection::insertIgnoreConflict instead of setValues OR that Connection::setValues uses insertIgnoreConflict directly if $updatePreconditionValues is empty?

@enoch85
Copy link
Member

enoch85 commented Mar 5, 2021

cc @blizzz

What @krono said. --^

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1. to develop Accepted and waiting to be taken care of bug
Projects
None yet