Update Key Rotation web-vault v2024.3.x

[BlackDex]

Key rotation was changed since 2024.1.x.
Multiple other items were added to be rotated like password-reset and emergency-access data to be part of just one POST instead of having multiple.

See: dani-garcia/bw_web_builds#157

[BlackDex]

There seems to be a bug in 2024.3.0 (and probably the latest 2024.2.x also).

There is a phantom folder without an id, that causes an issue. This also happens on a Self-Hosted Bitwarden.

[stefan0xC]

There seems to be a bug in 2024.3.0 (and probably the latest 2024.2.x also).

There is a phantom folder without an id, that causes an issue. This also happens on a Self-Hosted Bitwarden.

What do you mean?

[BlackDex]

There seems to be a bug in 2024.3.0 (and probably the latest 2024.2.x also).
There is a phantom folder without an id, that causes an issue. This also happens on a Self-Hosted Bitwarden.

What do you mean?

That during key rotation there is a folder in the folders array without an id, but with a name. Which isn't allowed.
It also breaks on there self-hosted instance, and probably also on the cloud environment. I have not yet tested that.

[stefan0xC]

Yep. Looking into it I think that it seems like the empty folder is sent by the client as it is added to the folderViews but not filtered out anymore (since bitwarden/clients#6881)
Cf. https://github.com/bitwarden/clients/blob/web-v2023.12.0/apps/web/src/app/auth/settings/change-password.component.ts#L235-L242 vs. https://github.com/bitwarden/clients/blob/web-v2024.3.0/apps/web/src/app/auth/key-rotation/user-key-rotation.service.ts#L118-L130

Note to reproduce: it only affects you if you add a folder (so rotating the keys while having no folders at all still works).

[stefan0xC]

A workaround would be that we change

vaultwarden/src/api/core/accounts.rs

Lines 440 to 441 in 2d98aa3

	struct UpdateFolderData {
	Id: String,

to Option<String> and skip the processing of it accordingly

vaultwarden/src/api/core/accounts.rs

Lines 474 to 475 in 2d98aa3

	for folder_data in data.Folders {
	let mut saved_folder = match Folder::find_by_uuid(&folder_data.Id, &mut conn).await {

[BlackDex]

A workaround would be that we change

vaultwarden/src/api/core/accounts.rs

Lines 440 to 441 in 2d98aa3

	struct UpdateFolderData {
	Id: String,

to Option<String> and skip the processing of it accordingly

vaultwarden/src/api/core/accounts.rs

Lines 474 to 475 in 2d98aa3

	for folder_data in data.Folders {
	let mut saved_folder = match Folder::find_by_uuid(&folder_data.Id, &mut conn).await {

I'm working on that indeed. But bumping into other strange issues.
Like they started sending org ciphers too, which we probably do not want to update i think.
So we need to skip those too.

Edit:
It also breaks on there Self-Hosted images. Strangely it doesn't on the cloud, even with the same versions.

[stefan0xC]

It also breaks on there Self-Hosted images. Strangely it doesn't on the cloud, even with the same versions.

Not familiar with bitwarden self-hosted. Might be related to whether you have enabled the key-rotation-improvements feature flag? At least the validation seems to differ when it's enabled.

[BlackDex]

It also breaks on there Self-Hosted images. Strangely it doesn't on the cloud, even with the same versions.

Not familiar with bitwarden self-hosted. Might be related to whether you have enabled the key-rotation-improvements feature flag? At least the validation seems to differ when it's enabled.

Indeed, that is enabled on Cloud, but not on the Self-Hosted Bitwarden, that might be the difference indeed.

[BlackDex]

Ah, that flag does make it better for us. But it doesn't change the layout.
I think it already sends it in this format because of a version check maybe?

Not sure since we had 2023.9.1 there, and i just now changed it to 2024.2.0, but that didn't make a difference really.
The flag does. It prevents some other calls to be done afterwards :) which are handled already with the new format.

[BlackDex]

Hmm. Maybe the org ciphers are needed for the flexible collections feature maybe?
Not sure how those work actually.

[Gerardv514]

Curious if the title of this Pr is a typo. Should v2023.3 be v2024.3?

[BlackDex]

Technically this PR works. But i want to check for some more optimizations.

[Gerardv514]

Change title on commit to 2024.3 as well?