An issue that may cause errors in background tasks

[g123k]

Hi everyone,

I have digged a little bit in the code today, trying to find some explanations for this kind of error:
Exception: Could not save product - [{impact: {id: failure, name: Failure, Ic_name: Failure}, message: {id: invalid_user_id_and_password, name: Invalid user id and password, Ic_name: Invalid user id and password}}]

I'm pretty sure there is also an issue on the server BUT we may have a loophole in our implementation:

• Basically, we store the user ID / password
• When we schedule a background task, we save the user ID / password to not rely on the current value in the secured storage

Now, let's imagine this scenario:

• An action generates a background task, but can't be immediately sent
• During the meantime, the user changes this password
• All background tasks will fail indefinitely and there is no recovery method

I suggest changing a little bit the implementation to this one:

• In a background task, if the user ID is the same as the one in the secured storage, use the password from the secure storage
• In a background task, if the user ID is different from the current one, as we can't fix the situation, use the credentials from the secured storage instead.

I know that the second fix is not perfect (edit assigned to another user), but still better than data lost forever.
Another idea would be to use a generic user.

[monsieurtanuki]

In a background task, if the user ID is the same as the one in the secured storage, use the password from the secure storage
In a background task, if the user ID is different from the current one, as we can't fix the situation, use the credentials from the secured storage instead.

@g123k That means always use the secure storage credentials, right?

What if the user disconnects: we'll still try forever to run background tasks that will fail. Or assign it to our default user, smoothie-app.
And I'm afraid it won't solve the issue, as it happened even for users that didn't change their passwords, according to @NixedSec in #4637 (comment)

I have also not changed the password on this account via the app or website any time recently.

That said, it's somehow healthier not to store the password in background tasks, isn't it?

[NixedSec]

@monsieurtanuki I think I have managed to work out the issue, at least in my case. It appears that the "£" character being used within a password seems to cause issues. I tested several different characters and lengths with different accounts and using that within another account's password seemed to cause issues on that account. Though I am unsure why this should be an issue if the password is being hashed before checked.

[monsieurtanuki]

Very interesting, @NixedSec!

I'm not surprised that £ is problematic as it's not an ASCII character. Remember, we're only in 2023!

• when we create an account from the app (register), it's with a MULTIPART request, with the password as a "body" parameter
• when we log in (login2), it's with a POST request, with the password as a "body" parameter
• when we save a product, it's either with a POST request (saveProduct) or with a PATCH request (temporarySaveProductV3)
• when we save an image, it's with a MULTIPART request

Or it's a problem when we store securely the user/password.

@stephanegigandet Typically they say something like lowercase, uppercase, digits and a restricted set of special characters - are there similar restrictions for user and password in OFF?

[monsieurtanuki]

Aha!

I've changed my password in order to include a £ and have witnessed no problem in most cases:

• changing basic details
• uploading a new photo
• cropping an existing photo
• hunger games

BUT: I've encountered a problem with the new packagings feature, which is the only case to be saved with a PATCH request (temporarySaveProductV3, /api/v3/product/$barcode).

@stephanegigandet Any idea why the PATCH request would not accept a password with a £, whereas the /cgi/product_jqm2.pl POST works correctly?

patch param	value
uri	https://world.openfoodfacts.org/api/v3/product/9780000000001
header	{Accept: application/json, User-Agent: - Smoothie - OpenFoodFacts - 0.0.0+734 - android+sdk_gphone64_x86_64-userdebug 12 SE1A.220203.002.A1 8151367 dev-keys - https://world.openfoodfacts.org/ - , From: monsieurtanuki}
body	{"user_id":"monsieurtanuki","password":"something with a £","product":{"packagings":[{"shape":{"lc_name":"Bouteille"},"material":{"lc_name":"Verre"},"recycling":{"lc_name":"Recycler"},"number_of_units":1}]},"lc":"fr","tags_lc":"fr","cc":"fr","app_name":"Smoothie - OpenFoodFacts","app_version":"0.0.0+734","app_uuid":"6bd6aeef88464d03","app_platform":"android+sdk_gphone64_x86_64-userdebug 12 SE1A.220203.002.A1 8151367 dev-keys","comment":""}