sebadob/rauthy

Reset password throws an database error and fetch users throws an internal server error

kerkmann opened this issue · 13 comments

Since yesterday the version 0.21.1 and 0.22.1 is throwing a database error when I try to reset my password. ^^"

2024-04-22T09:23:14.856435Z ERROR rauthy_common::error_response: Database Error: ColumnDecode { index: "\"failed_login_attempts\"", source: "mismatched types; Rust type `core::option::Option<i64>` (as SQL type `INT8`) is not compatible with SQL type `INT4`" }

It's also throwing an internal server error on the /auth/v1/users endpoint (from the administrator perspective).
It's throwing the same database erorr, the client response is:

{
    "timestamp": 1713778066,
    "error": "Internal",
    "message": "Internal error, please try again"
}

Posgres version is 16.

I can't reproduce this error and this very column has been inserted with the very first database migration ever and never been touched since then. I also tried against PG15 and 16, increasing this counter manually, fetching password reset, everything works fine.

Have you messed with the DB manually?

This column should be an INT4, not an INT8

grafik

This query cannot be type checked at compile time, because of being able to work with postgres and sqlite at the same time (SQLite integers are dynamic), but this has never failed on me so far.

Nope, I've never played with the DB. I just deployed it on Kubernetes and never touched it ^^"

I'll try to connect to the database (try to figure out how in kubernetes) and take a look into the database. Give me one sec.

Wait a sec, I misinterpreted the error message above.
From the error, the DB type is an INT4 (like it should), but it complains about the i64 in rust. Yes, i64 would be an INT8, but it has never been an issue to map an INT4 from the DB to an i64 in Rust, because this value is guaranteed to always fit in.
Weird is as well, that I can't reproduce this error and never had such an issue. This setup works for years by now.
The fix would be pretty simple by just changing the DB type to an INT8 with a migration (please don't do it manually).
This would just be a small waste of space though and my bigger problem is, that I am unable to produce this error. Can you find out how exactly you ended up there?

I "fixed" this in a test branch by changing the column type in postgres to an INT8 to exactly match the i64 in Rust.
However, I would really like to know where this is coming from. INT8 is a waste of space for that column.

rauthy=# \d+ rauthy
Did not find any relation named "rauthy".
rauthy=# \l
                                                   List of databases
   Name    | Owner | Encoding | Locale Provider |  Collate   |   Ctype    | ICU Locale | ICU Rules | Access privileges
-----------+-------+----------+-----------------+------------+------------+------------+-----------+-------------------
 postgres  | root  | UTF8     | libc            | en_US.utf8 | en_US.utf8 |            |           |
 rauthy    | root  | UTF8     | libc            | en_US.utf8 | en_US.utf8 |            |           |
 template0 | root  | UTF8     | libc            | en_US.utf8 | en_US.utf8 |            |           | =c/root          +
           |       |          |                 |            |            |            |           | root=CTc/root
 template1 | root  | UTF8     | libc            | en_US.utf8 | en_US.utf8 |            |           | =c/root          +
           |       |          |                 |            |            |            |           | root=CTc/root
(4 rows)
rauthy=# \d+ users
                                                               Table "public.users"
        Column         |       Type        | Collation | Nullable |         Default         | Storage  | Compression | Stats target | Description
-----------------------+-------------------+-----------+----------+-------------------------+----------+-------------+--------------+-------------
 id                    | character varying |           | not null |                         | extended |             |              |
 email                 | character varying |           | not null |                         | extended |             |              |
 given_name            | character varying |           | not null |                         | extended |             |              |
 family_name           | character varying |           | not null |                         | extended |             |              |
 password              | character varying |           |          |                         | extended |             |              |
 roles                 | character varying |           | not null |                         | extended |             |              |
 groups                | character varying |           |          |                         | extended |             |              |
 enabled               | boolean           |           | not null |                         | plain    |             |              |
 email_verified        | boolean           |           | not null |                         | plain    |             |              |
 password_expires      | bigint            |           |          |                         | plain    |             |              |
 created_at            | bigint            |           | not null |                         | plain    |             |              |
 last_login            | bigint            |           |          |                         | plain    |             |              |
 last_failed_login     | bigint            |           |          |                         | plain    |             |              |
 failed_login_attempts | integer           |           |          |                         | plain    |             |              |
 language              | character varying |           | not null | 'en'::character varying | extended |             |              |
 webauthn_user_id      | character varying |           |          |                         | extended |             |              |
 user_expires          | bigint            |           |          |                         | plain    |             |              |
 auth_provider_id      | character varying |           |          |                         | extended |             |              |
 federation_uid        | character varying |           |          |                         | extended |             |              |
Indexes:
    "users_pk" PRIMARY KEY, btree (id)
    "users_email" UNIQUE CONSTRAINT, btree (email)
    "users_federation_key" UNIQUE CONSTRAINT, btree (auth_provider_id, federation_uid)
    "users_pk2" UNIQUE CONSTRAINT, btree (webauthn_user_id)
Foreign-key constraints:
    "users_auth_providers_id_fk" FOREIGN KEY (auth_provider_id) REFERENCES auth_providers(id) ON UPDATE CASCADE ON DELETE SET NULL
Referenced by:
    TABLE "magic_links" CONSTRAINT "magic_links_user_id_fkey" FOREIGN KEY (user_id) REFERENCES users(id) ON UPDATE CASCADE ON DELETE CASCADE
    TABLE "passkeys" CONSTRAINT "passkeys_users_id_fk" FOREIGN KEY (user_id) REFERENCES users(id) ON UPDATE CASCADE ON DELETE CASCADE
    TABLE "passkeys" CONSTRAINT "passkeys_users_webauthn_user_id_fk" FOREIGN KEY (passkey_user_id) REFERENCES users(webauthn_user_id) ON UPDATE CASCADE ON DELETE CASCADE
    TABLE "recent_passwords" CONSTRAINT "recent_passwords_user_id_fkey" FOREIGN KEY (user_id) REFERENCES users(id) ON UPDATE CASCADE ON DELETE CASCADE
    TABLE "refresh_tokens" CONSTRAINT "refresh_tokens_user_id_fkey" FOREIGN KEY (user_id) REFERENCES users(id) ON UPDATE CASCADE ON DELETE CASCADE
    TABLE "sessions" CONSTRAINT "sessions_user_id_fkey" FOREIGN KEY (user_id) REFERENCES users(id) ON UPDATE CASCADE ON DELETE CASCADE
    TABLE "user_attr_values" CONSTRAINT "user_attr_values_users_id_fk" FOREIGN KEY (user_id) REFERENCES users(id) ON UPDATE CASCADE ON DELETE CASCADE
    TABLE "users_values" CONSTRAINT "users_values_users_id_fk" FOREIGN KEY (id) REFERENCES users(id) ON UPDATE CASCADE ON DELETE CASCADE
    TABLE "webids" CONSTRAINT "webids_users_id_fk" FOREIGN KEY (user_id) REFERENCES users(id) ON UPDATE CASCADE ON DELETE CASCADE
Access method: heap

I am not sure if that helps, it's just showing failed_login_attemps as integer, which is strange.

Edit: int4 it basically integer based on the postgres 16 docs

it's just showing failed_login_attemps as integer, which is strange.

No, this is to be expected. It should be an int4.

The error above is the other way around, which is the strange part, because an INT4 will alway fit into an i64.

Edit: int4 it basically integer based on the postgres 16 docs

You have INT 2, 4, 8 and integer is just a "common name" that postgres maps to INT4, correct.

Nevertheless, the linked PR does a migration on this column and change it to an INT8 (there is a "friendly name" in postgres called bigint, which is the same under the hood).
This wastes 4 bytes of space in that column, but on the other hand I was able to change the user lookup queries to be compile-time checked again. So we loose 4 bytes of storage per user but gain compile-time checked queries, I think this is a good trade-off. :)

I will merge the PR and build a nightly image for testing.

Thanks! I'll deploy the nightly image and give you feedback. :)

I have no idea how you ended up with that error.
Even after changing from the unchecked to compile-time checked queries, I did not get any error or even warning.

Can you please test with the nightly below?
It would be good, if you do this against a non-prod DB, work on a copy or do a backup before. If this does not fix the error as expected, we would need to either remove or adjust the new DB migration.

ghcr.io/sebadob/rauthy:0.22.2-20240424

Yes! This is working! Awesome! But it's strange that this error occured. ^^"

Yes! This is working! Awesome! But it's strange that this error occured. ^^"

Thanks for the feedback.

Yes I have absolutely no idea how this came up. I have another Rauthy instance on Postgres running since almost 2 years now, which has multiple failed logins and password resets daily, and this never threw any error at all.