Server: PostgreSQL : duplicate key value violates unique constraint error message

[cyayon]

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]

Could you try this one here as well: https://github.com/nextcloud/server/issues/366#issuecomment-319212033

Thanks.

[cyayon]

Hi,

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

[MorrisJobke]

cc @icewind1991 @nickvergessen

[ghost]

@cyayon Are you using LDAP Auth?

[cyayon]

Hi,

No, i am using simple locale authentication.

[Gottox]

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]

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]

Yes, same here.

[nickvergessen]

Well the issue is actually: https://github.com/nextcloud/server/blob/0eebff152a177dd59ed8773df26f1679f8a88e90/lib/private/DB/Connection.php#L289

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

[enoch85]

Yes please.

[ingoj]

Still having this issue in NC 13...

[Drezil]

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]

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

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]

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.

[ghost]

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

[ghost]

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]

Just use grep -v.

-- Brad

On Sep 8, 2018, at 7:30 AM, Llaurence [e-mail] wrote:

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. :/

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

[ovk]

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.

[kesselb]

Upsert for pqsql is part of nextcloud 16. Not sure if all code is using upsert. Check https://github.com/nextcloud/server/pull/13721 for more details.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity and it seems to be missing some essential informations. It will be closed if no further activity occurs. Thank you for your contributions.

[ovk]

Unfortunately, the issue is still there in Nextcloud 16.

[tycho]

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

[arno01]

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]

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]

Please use https://github.com/nextcloud/server/issues/6343#issuecomment-570063834. This patch will not handle password change correctly.

Index: lib/private/Security/CredentialsManager.php
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- lib/private/Security/CredentialsManager.php (revision 1d82bb55dbf9a8f12603570a7203ce80b818f443)
+++ lib/private/Security/CredentialsManager.php (date 1572036731105)
@@ -58,14 +58,20 @@
     * @param mixed $credentials
     */
    public function store($userId, $identifier, $credentials) {
-       $value = $this->crypto->encrypt(json_encode($credentials));
+       $oldCredentials = $this->retrieve($userId, $identifier);

-       $this->dbConnection->setValues(self::DB_TABLE, [
-           'user' => $userId,
-           'identifier' => $identifier,
-       ], [
-           'credentials' => $value,
-       ]);
+       if ($oldCredentials === $credentials) {
+           return;
+       }
+
+       $qb = $this->dbConnection->getQueryBuilder();
+       $qb->insert(self::DB_TABLE)
+           ->values([
+               'user' => $qb->createNamedParameter($userId),
+               'identifier' => $qb->createNamedParameter($identifier),
+               'credentials' => $qb->createNamedParameter($this->crypto->encrypt(json_encode($credentials))),
+           ])
+           ->execute();
    }

    /**

Should address most of the messages related to oc_credentials. Needs some testing ;)

[arsylum]

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

[kesselb]

Index: lib/private/Security/CredentialsManager.php
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- lib/private/Security/CredentialsManager.php (revision 9e7f52d225c00873300d5966c68c896caa09cde3)
+++ lib/private/Security/CredentialsManager.php (date 1577894037340)
@@ -59,14 +59,28 @@
     * @param mixed $credentials
     */
    public function store($userId, $identifier, $credentials) {
-       $value = $this->crypto->encrypt(json_encode($credentials));
+       $oldCredentials = (array)$this->retrieve($userId, $identifier);
+       if ([] === array_diff_assoc($credentials, $oldCredentials)) {
+           return;
+       }

-       $this->dbConnection->setValues(self::DB_TABLE, [
-           'user' => $userId,
-           'identifier' => $identifier,
-       ], [
-           'credentials' => $value,
-       ]);
+       $encryptedCredentials = $this->crypto->encrypt(json_encode($credentials));
+       $qb = $this->dbConnection->getQueryBuilder();
+
+       if ([] === $oldCredentials) {
+           $qb->insert(self::DB_TABLE)
+               ->values([
+                   'user' => $qb->createNamedParameter($userId),
+                   'identifier' => $qb->createNamedParameter($identifier),
+                   'credentials' => $qb->createNamedParameter($encryptedCredentials),
+               ]);
+       } else {
+           $qb->update(self::DB_TABLE)
+               ->set('credentials', $qb->createNamedParameter($encryptedCredentials))
+               ->where($qb->expr()->eq('user', $qb->createNamedParameter($userId)))
+               ->andWhere($qb->expr()->eq('identifier', $qb->createNamedParameter($identifier)));
+       }
+       $qb->execute();
    }

    /**

Could you please test this patch? The earlier version missed code to handle password change correctly. Thanks :+1:

[lukasjuhrich]

@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]

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

[skjnldsv]

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

[kesselb]

I added some more background about that issue here: https://github.com/nextcloud/server/issues/19494#issuecomment-612006756

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]

Would you like to open a PR with your patch?

[kesselb]

Would you like to open a PR with your patch?

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

[kesselb]

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.

[nextcloud-stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity and seems to be missing some essential information. It will be closed if no further activity occurs. Thank you for your contributions.

[sophie-h]

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

[fwgx]

Same issue on 18.0.4 running in Docker

[krono]

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?