search

matteo-convertino / otpmanager-nextcloud

Nextcloud app that allows you to manage your OTP (TOTP/HOTP) codes easily
GNU Affero General Public License v3.0
25 stars 5 forks source link

Error with long secret #17

Closed Elm89 closed 7 months ago

Elm89 commented 9 months ago

I happened to come across a 254 characters secret.

I was able to add the OTP in the iOS App without any problem, but the app then refused to sync the entry to the server.

I did a little digging and found this error in the nextcloud logs : [...]/DbalException.php","Line":71,"Previous":{"Exception":"Doctrine\\DBAL\\Exception\\DriverException","Message":"An exception occurred while executing a query: SQLSTATE[22001]: String data, right truncated: 1406 Data too long for column 'secret' at row 1",[...]

Just changed the DB with ALTER TABLE oc_otpmanager_accounts MODIFY secret varchar(512) and it now works great. I know nothing about this things so 512 might be a terrible idea.

I had the same "Data too long" problem with the "name" field with some Microsoft default names also so I did the same thing.

Hope you can fix it.

Thanks for your work, the app is great!

matteo-convertino commented 9 months ago

Thank you very much for pointing out this problem. Indeed at this time the secret key could not exceed 200 characters and the name 50 characters. This was a pretty stupid mistake, I don't even remember why I didn't put 256 characters directly for the secret key (I don't think there are any longer ones, have you ever come across any?). As for the name: how many characters long is it? In your opinion, would 64 or 100 characters be sufficient for both the name and the issuer?

Elm89 commented 9 months ago

I never came across a longer secret.

Regarding the name, a rapid test with a microsoft account produces a name with about 80 characters (could go over 100 since the username alone is limited to 64 on azureAD), something like Default Directory:thisisarandomnamemeanttobelong@myemailadress.onmicrosoft.com I don't think that much characters are needed on the client side to identify the OTP but if the DB limit is at 64 characters then the client apps would have to trim the name before saving it.

Once again I know nothing about software dev and my advices are basically worth nothing! If someone has more infos about this please help!

matteo-convertino commented 7 months ago

It should be fixed with the new release. Let me know if the problem persists.