Fix desktop app integration bugs#10
Merged
Merged
Conversation
The 1Password shared library requires `serviceAccountToken` to always be present in the client config (even as empty string) and `account_name` to be included when using desktop app auth. Previously both were incorrectly omitted from serialization. The shared library also returns response `payload` as a raw JSON byte array rather than a base64 string. The deserializer now accepts both formats.
This file contains hidden or 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
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.
Summary
Fixes two bugs discovered during a smoke test against the 1Password desktop app that prevented the desktop integration from working:
ClientConfig serialization -- The 1Password shared library requires
serviceAccountTokento always be present in the config payload (even as an empty string""), andaccount_nameto be included when set. Previously,serviceAccountTokenwas omitted when empty viaskip_serializing_if, andaccount_namewas unconditionally skipped viaskip_serializing. Both fields are now serialized correctly.Response payload deserialization -- The shared library returns
payloadas a raw JSON byte array ([65,110,...]), not a base64 string. Thebase64_payloaddeserializer has been replaced with aflexible_payloaddeserializer that handles both formats (raw byte array and base64 string), ensuring compatibility across library versions.Test plan
cargo fmt --checkpassescargo clippy -- -D warningspasses (default features)cargo clippy --features desktop -- -D warningspassescargo testpasses (23 tests, default features)cargo test --features desktoppasses (26 tests, including 3 new/renamed desktop tests)desktop_config_includes_account_nametest confirmsserviceAccountTokenis""andaccount_nameis"myaccount"service_account_config_omits_account_nametest confirmsserviceAccountTokenis"ops_test"andaccount_nameis absentresponse_payload_deserializes_from_byte_arraytest confirms raw byte array[104,101,108,108,111]deserializes tob"hello"response_payload_deserializes_from_base64test still passes (backward compat)Note
Medium Risk
Changes request/response wire formats for the desktop shared-library integration (config serialization and response payload decoding), which can affect interoperability and error handling across library versions.
Overview
Fixes desktop shared-library interoperability by adjusting
ClientConfigJSON serialization:serviceAccountTokenis now always emitted (even when empty) andaccount_nameis included when set.Updates shared-library
Responsedecoding to acceptpayloadas either base64 text or a raw JSON byte array via a newflexible_payloaddeserializer, and updates/adds tests to cover both formats.Reviewed by Cursor Bugbot for commit 4391436. Bugbot is set up for automated code reviews on this repo. Configure here.