[15.0][FIX] vault: Error when reencripting

[CarlosRoca13]

To reproduce the error, the steps would be:

1. Access a Vault with a significant number of records, and some of them have null-valued keys.
2. Remove a permission from the vault and re-encrypt the keys.
3. When you save, an error will be thrown due to a required field being empty, and any keys that haven't been saved will be lost.

With the changes applied in this patch, the aforementioned error no longer occurs, and the keys that are in an inconsistent state no longer cause the others to become corrupted. In addition to fixing this, we have added the feature to disable the re-encryption assistant buttons to avoid the impression that nothing is happening.

cc @Tecnativa TT44183

Please @fkantelberg review this. This is the only option that has occurred to me to solve the encryption problem when there is any key in an inconsistent state in the Vault. If you believe there might be a better solution, it would be greatly appreciated.

[CarlosRoca13]

I apologize for not expressing myself properly. I have managed to reproduce this issue with a vault containing 162 vault.entries, each with 2 to 3 vault.fields.

What I was trying to say is that due to some saving issue, the value of a certain vault.field record has been lost, resulting in a null value during re-encryption. I have been able to reproduce this inconsistency by closing the tab before the vault finishes saving, or if there is an inconsistent vault.field while saving the re-encrypted vault.

With the change I have made, in the case of any corrupt vault.fields, the value "" would be returned, and it would not interfere with the saving of other records.

I'm not sure how we can prevent the premature closing of the tab...

[CarlosRoca13]

I will give you 2 gifs with both situations

[CarlosRoca13]

The first gif show you when we delete a right with a inconsistent vault.field

On the second, you'll see a reload in the middle of the save

[fkantelberg]

We can't prevent the tab from closing early but we can try to make the re-encryption atomic which should help preventing such invalid fields. Basically instead of using the js frontend to apply the changes we could use a special route to rewrite everything at once.

[fkantelberg]

I could reproduce most of it. It's fine until you press save because before the changes are only in the frontend and reloading doesn't change anything. The problem is that saving to the database takes too long and isn't running in parallel.

saveRecord in the vault_controller.js is basically the critical function. When you save the changes to the rights the new master keys are transferred to the database first afterwards the fields + files are updated in the code snippet. Reloading the tab during it causes that the master keys doesn't match the encrypted values anymore.

We can try the following:

• Use framework.blockUI and framework.unblockUI
• Push the changes to the fields, files and rights within a single transaction. We have to push the new master keys in the same transaction as the fields and don't rely on the frontend.

It should also affect the older versions.

[CarlosRoca13]

Hi @fkantelberg please review last changes :smile:

[fkantelberg]

@CarlosRoca13 seems like we both worked on it. Can you look into my PR and maybe adapt the changes from there?

Otherwise I would point to the same points in the review:

• blockUI should be in re-encrypt and should be catched with try ... finally => you don't need to disable the buttons
• Master keys AND the file/fields values should be pushed within the same transaction to fully eliminate the reload option.
• method vs controller doesn't really matter but the naming should be something with replace/reencrypt

[CarlosRoca13]

@fkantelberg

I was looking at your changes and they fit better than what I've been doing. I'm going to check if it fits in our client's instance and in any case take the right path.

Thank you very much

[CarlosRoca13]

Closed as it is being dealt with in this other PR https://github.com/OCA/server-auth/pull/540