Change API and structs to camelCase #4386
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dani-garcia
force-pushed
the
remove_upcase
branch
from
4074a8a to
909e0d7
Compare
dani-garcia
force-pushed
the
remove_upcase
branch
5 times, most recently
from
f40f45a to
c54f75e
Compare
dani-garcia
force-pushed
the
remove_upcase
branch
from
c54f75e to
0f5cb4c
Compare
BlackDex
requested changes
May 25, 2024
|
Also, card vault types have |
|
Also, not sure, but isn't it more useful to use |
|
On this PR, and web 2024.05 confirming users to orgs does not work (I'm not sure if this is due to the PR, or due to the web vault changing), but since this is the same kind of issue it makes sense to solve here. Specifically, the body the web vault sends contains "key:", while the server expects "Key:" on the post body to |
|
Also, for native app compatibility, there seem to be some keys missing for organization profile responses, like causing the native app(s) [only tested for iOS] to crash for accounts in an organization. |
|
Also, somehow all custom fields in the sync response are null on this branch, (making custom fields inaccessible on all clients). |
dani-garcia
force-pushed
the
remove_upcase
branch
from
663c8f8 to
d5148bc
Compare
|
@quexten Thanks for the review! I fixed the invalid case in @BlackDex I didn't even notice |
|
Native apps are available in beta now, so we should try getting this merged soon: |
|
Fixed in the latest main, uploads should work in the next beta update. |
|
Ill see if i can do some more testing soon. |
BlackDex
requested changes
Jun 19, 2024
There was a problem hiding this comment.
I just did some testing with my local accounts i use for debugging, and these are my findings so far.
- The admin UI doesn't work, the user and org overview are broken because the handlebar templates are not updated.
- For some reason in the Android Emulator i can't login at all, but that is not something we can fix i think.
On my physical phone (Pixel 7) it also doesn't work, i can login, but the sync fails on the client side.
- Sends have a
textobject which have the keysHiddenandTextthose are still capital-case
"text": {
"Hidden": false,
"Text": "2.j2Z/rN...TbjMvCQ==|/Kk0bfdc...68kv8A==|W1qbVban...AVJYYJRPy/WbK5iQeEQJIWwJh1k="
}- The same for Sends
file:
"file": {
"FileName": "2.KcN9y...GuollclQ==|XxaO+KyT4fjIIYRb...MNxZ8VeHAAz4=|hruuM...PU4A=",
"Id": "25587de08f158fe7ecfef24d22fc600c596efaf53bc5d7e23125e75c033a49bb",
"Size": 10737,
"SizeName": "10.49 KB"
},Other than that, i have not yet found anything which could be wrong any futher
|
Small update, Those sends were added via the PascalCase version of Vaultwarden. |
|
Asking google will be the right place as it's the android debug tool hehe. |
|
Also @P3RF3CTION same for you, try to see what For me i can login and sync with a test account which has broken entries in the database and are fixed during sync. |
|
@BlackDex allready cleared the app, thats not the Problem |
|
@BlackDex I maybe forgot to mention, I am running on premise no docker thats why i mentioned Ubuntu 24.04. And it must be a feature/adjustment in the latest Bitwarden App since it is not running anymore since the latest Update while the Vaultwarden Version 1.32.7 and Web-Vault was not updated but is already on 2025-01. To mention Reinstallation and clearing in andorid does not solve the issue.
|
|
@BlackDex here is the logcat Logcat |
|
@celevra, unfortunately this is not enough. The main reason is that not everything is tagged with I did some testing and checking my self, best would be to use something like this grep -A20 -iE '(Process:.*bitwarden|android.runtime.JavaProxyThrowable)' |
|
doesn't find anyhting: |
|
and what about: adb logcat | grep -A20 -iE '(Process:.*bitwarden|android.runtime.JavaProxyThrowable)'And then try to sync. |
|
@BlackDex no nothing, but this is the log from "bitwarden allready started und just hit refresh until error". |
|
To provide an update to this Topic after installing the previous APK for Android of Bitwarden the Issue is resolved https://github.com/bitwarden/android/tree/v2024.11.7. So the latest patch notes of the bitwarden app might contain any hint or change that is leading to the error. It is definitely something from the bitwarden app which vaultwarden does not support right now. This means the version of https://github.com/bitwarden/android/releases/tag/v2025.1.0 and also https://github.com/bitwarden/android/releases/tag/v2024.12.0 have a breaking change since both provide the same error. |
|
That is probably not the best way. And the latest version released today should provide a working sync as far as i know. Best would be to try and get the logs of the app. adb logcat --pid=$(adb shell pidof -s com.x8bit.bitwarden) |
|
@BlackDex now on 1.33.0 |
|
And you tried to do a sync which generated an error message during this log output? |
|
Yes, two times |
|
Weird. Not seeing anything useful. And run the same command but then for dev like. adb logcat --pid=$(adb shell pidof -s com.x8bit.bitwarden.dev)Hopefully that provides more details. |
|
there is now a lot of output inclusive passwords, usernames and so on. So not safe to post here |
I can confirm updating to 1.33.0 resolves the issue with the latest Android app. |
|
@celevra , yes, that is useful, now i only need to know where that specific date is coming from! So somewhere in the sync data that date is located. I would need to know where that date is located. |
|
@BlackDex |
|
On which kind of elementen? Can you provide more json context? |
|
The strange thing is, that string should be impossible. |
|
And, after providing more context. You can probably fix this your self by search for the uuid of that cipher, and change the date to have 6 digets instead of 9, and if that is the only one it should fix your sync. |
|
@BlackDex don't know if it helps, i came from enpass, than onepassword and than bitwarden so there passwords have gone through 2 conversions. |
|
Thanks, that helps. |
|
Yea, that might be a good thing to do in any case |
|
I also have a fix implemented, doing some extra testing. |
|
Fix is located here #5477 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR changes all the project's structs and API inputs and outputs from the old
PascalCaseformat to the newcamelCaseformat. At the moment the clients support both but that won't be a thing forever.I haven't had the time to fully test this, but at least the basic things work. There may be some things missing, and I still need to review what we store in the db to make sure that there aren't any backwards compat issues.
This might also cause problems with other pending PRs so we can wait to merge it until they are dealt with.
Some fields that have special casing:
OTPin/accounts/verify-otp, as that's what the clients send. That said we also checkotpKeysandNfcin yubikey, this is for backwards compat with older values stored on the server, now we serialize new versions tokeysandnfc.AttestationObjectandclientDataJsonin webauthn, as that's what the clients send. This is a bigger annoyance as it complicates the use of the webauthn crate.camelCase.Fixes #4656