kimai2_users title field length limit will cause error logging in via SAML if SAML provided title is too long.

[timlegge]

Describe the bug

kimai2_users title field length limit will cause error logging in via SAML if SAML provided title is too long.

Probably should truncate and add the row.

To Reproduce
Steps to reproduce the behavior:

1. Create user in SAML provider with a long tltle
2. Attempt to login
3. See error

request.CRITICAL: Uncaught PHP Exception RuntimeException: "Failed creating or hydrating user "[email protected]": An exception occurred while executing 'INSERT INTO kimai2_users (username, username_canonical, email, email_canonical, enabled, salt, password, last_login, confirmation_token, password_requested_at, roles, alias, registration_date, title, avatar, api_token, auth) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)' with params ["SQLSTATE[22001]: String data, right truncated: 1406 Data too long for column 'title' at row 1 at /opt/kimai/src/Saml/Controller/SamlController.php:52)"} []

Logfile

Screenshots
Login says Something is wrong

Desktop/Smartphone
N/A

Additional context
Pre 1.8 with SAML support Commit

[kevinpapst]

Hm, well the title field was not supposed to hold arbitrary data.

What do you propose?
Cutting off the title?
Adding some parsing logic to the SAML mapping config, to truncate a string at position X?

[timlegge]

In our case I had the user's title truncated on the AD side.

First maybe a documentation entry on the length of the fields. Then I would be fine with truncating the data to the field size.

I will look into the SAML provider to see whether there is an ability to truncate the field before it is sent to Kimai.

Tim

[kevinpapst]

There is a missing validator, thanks for the info! A SQL error shouldn't bubble up.

Lengths can be found here:
https://github.com/kevinpapst/kimai2/blob/master/src/Entity/User.php#L72

Maybe truncating needs to be added to the user class directly, which I don't really like (silently changing user data is not a good approach).

But in that case it might okay, as users can come from multi sources and are not necessarily created in Kimai.

[timlegge]

Agreed. The sources can't always be changed. Cutting it off with ... added to the end might be fine. Tim

…

On Mon, Mar 16, 2020 at 2:37 PM Kevin Papst ***@***.***> wrote: There is a missing validator, thanks for the info! A SQL error shouldn't bubble up. Lengths can be found here: https://github.com/kevinpapst/kimai2/blob/master/src/Entity/User.php#L72 Maybe truncating needs to be added to the user class directly, which I don't really like (silently changing user data is not a good approach). But in that case it might okay, as users can come from multi sources and are not necessarily created in Kimai. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#1562 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAH3N6Z3KNRXTSFZC3NWOITRHZPUNANCNFSM4LMOW2QQ> .

[adrianrudnik]

@kevinpapst Regarding the validation constraint: Is there any reason to have it at 50 except to keep the title short for display purposes? Same goes for the $alias which is varchar(60) but the validator triggers at 160 characters.

Maybe have a secure limit through entity constraints (something like 512/1k for varchar) if it comes through APIs or internals and use a different constraint inside forms, to indicate some sensible length towards the user?

[kevinpapst]

Honestly: I can't remember ... the user entity is from the very early days and wasn't really changed since then. Validators have to be added obviously, they are missing.
Having more eyes on Kimai will help to smooth out such points.

I don't think that there is a real benefit in increasing these field sizes (to more than 100 chars). The display space is limited and both title and alias are "UI goodies", not something that is really required.
If we allow 1024 chars, the UI should not break if they were really used.

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. If you use Kimai on a daily basis, please consider donating to support further development of Kimai.