Allow null cipher name to fix collection assignment#6934
Allow null cipher name to fix collection assignment#6934xjohnyknox wants to merge 1 commit intodani-garcia:mainfrom
Conversation
|
Should this not be fixed in the API call to prevent ciphers from being saved if they are missing a required field? |
That is what i think too. |
5e398ff to
75efa32
Compare
|
Good point, updated the approach. Now instead of silently accepting null names, all cipher creation and update paths validate that |
|
I think this PR is not needed, and you are (Or maybe AI is) only making it worse by making The server already doesn't accept empty cipher names, so nothing needs to be fixed here. There was a period where we did not verified this correctly, and during that period those items slipped through, and I really do not hope that KeyGuard still allows to create ciphers without a name, if so, then i think it's better that @AChep would resolve that issue on the client-side too. So the only issue is that there might be some leftover vault items which do not have a name, and that the server panics on that is ok i think. The user needs to update the cipher so that it will have a name, and that should fix it. I'm planning on closing this ticket, unless someone has other ideas on what to do with this? |
|
Added a fix on my end, thanks @BlackDex for mentioning me! |
|
@AChep no problem! I also tested this with a Bitwarden server, and it also does not accept a a cipher without a name. And, as long as we do not except it, that should prevent any future issues :) But thanks for mentioning it and providing a possible solution! |
Summary
When a cipher is created without a name (e.g. via alternative clients like Keyguard), assigning it to a collection via the web vault fails with:
The root cause is that
CipherData.nameis typed asString, so serde rejectsnullduring deserialization — causing the entire request (including the collection assignment) to fail.Changes
CipherData.namefromStringtoOption<String>unwrap_or_default(), keeping theCiphermodel and DB schema unchangedThis follows the same pattern already used by other optional fields in
CipherData(e.g.notes,key,fields).Test plan
Fixes #6933