From d8a3685f9164f5f97897adf623c0c3b4ae035607 Mon Sep 17 00:00:00 2001 From: Hinton Date: Mon, 26 Aug 2024 11:57:48 +0200 Subject: [PATCH 01/11] Improve sdk documentation --- docs/architecture/sdk/index.md | 40 ++++++++++++++---------- docs/architecture/sdk/server-bindings.md | 15 ++++++--- docs/architecture/sdk/versioning.md | 16 ++++++++++ docs/getting-started/sdk/index.md | 5 +-- 4 files changed, 53 insertions(+), 23 deletions(-) create mode 100644 docs/architecture/sdk/versioning.md diff --git a/docs/architecture/sdk/index.md b/docs/architecture/sdk/index.md index ad2ff4fa9..8fbc6e938 100644 --- a/docs/architecture/sdk/index.md +++ b/docs/architecture/sdk/index.md @@ -9,38 +9,38 @@ SDK for the Bitwarden [Password Manager][pm]. The SDK is written in Rust and pro multiple languages. The general end goal of the SDK is to own everything up to the presentational layers. This includes but is not limited to: API calls, data models, encryption, and business logic. -We have collected a list of various learning resources on -[Confluence](https://bitwarden.atlassian.net/wiki/spaces/DEV/pages/517898288/Rust+Learning+Resources). +We have compiled a list of resources for learning rust in a +[Confluence page](https://bitwarden.atlassian.net/wiki/spaces/DEV/pages/517898288/Rust+Learning+Resources). ## Architecture The Bitwarden SDK is structured as a single [Git repository](https://github.com/bitwarden/sdk) with -multiple internal crates. Review the repository README for more information about the different -crates. +multiple internal crates. Please review the README in the repository for up to date information +about the different crates. -We generally strive towards extracting features into separate crates to keep the core `bitwarden` +We generally strive towards extracting features into separate crates to keep the `bitwarden-core` crate as lean as possible. This has multiple benefits such as faster compile-time and clear ownership of features. ### `bitwarden` crate -The `bitwarden` crate is the main entry point for the SDK and is responsible for wiring up the sub -crates. It contains a `Client` struct which represents a single account instance in the SDK. The -`Client` struct implements multiple callable methods that may manipulate the state of the SDK. +The `bitwarden` crate represents the entry point for consumers of the SDK and is responsible for +providing a cohesive interface. The crate re-exports functionality of the internal crates and +contains very little logic itself. + +### `bitwarden-core` crate + +The `bitwarden-core` crate contains the underlying functionality of the SDK. This includes a +`Client` struct which represents a single account instance in the SDK. + +Other crates in the SDK depend on `bitwarden-core` and provides extensions to the `Client` struct to +implement specific domains. ## Language bindings The SDK is currently exposed with multiple language bindings. Currently we utilize a mix of hand written bindings for a C API, and programmatically generated bindings. -### C bindings - -Many language bindings utilize the `bitwarden-c` crate that exposes a C API. This is then combined -with hand written bindings for the specific language. Since manually writing FFI bindings is time -consuming and difficult we generally provide a JSON based API through the `bitwarden-json` crate -which allows the language bindings to just contain three FFI functions, `init`, `run_command` and -`free_memory`. - ### Mobile bindings We use [UniFFI](https://github.com/mozilla/uniffi-rs/) to generate bindings for the mobile @@ -63,5 +63,13 @@ The WebAssembly module is published on [npm](https://www.npmjs.com/package/@bitw prerelease builds are published on [GitHub Packages](https://github.com/bitwarden/sdk/pkgs/npm/sdk-wasm). +### C bindings + +Many language bindings utilize the `bitwarden-c` crate that exposes a C API. This is then combined +with hand written bindings for the specific language. Since manually writing FFI bindings is time +consuming and difficult we generally provide a JSON based API through the `bitwarden-json` crate +which allows the language bindings to just contain three FFI functions, `init`, `run_command` and +`free_memory`. + [sm]: https://bitwarden.com/products/secrets-manager/ [pm]: https://bitwarden.com/ diff --git a/docs/architecture/sdk/server-bindings.md b/docs/architecture/sdk/server-bindings.md index ce7a5dbe6..a34aee2ea 100644 --- a/docs/architecture/sdk/server-bindings.md +++ b/docs/architecture/sdk/server-bindings.md @@ -22,12 +22,14 @@ let response: SyncResponseModel = bitwarden_api_api::apis::sync_api::sync_get(&config.api, exclude_subdomains).await?; ``` -It's highly recommended to not expose the request and response models of the generated bindings and -instead use your own models. This decouples the server request / response models from the SDK -models, and allows for easier changes in the future. +You SHOULD not expose the request and response models of the auto generated bindings and SHOULD +instead define and use your own models. This ensures the server request / response models are +decoupled from the SDK models, which allows for easier changes in the future without breaking +backwards compatibility. -We recommend using either the `From` or `TryFrom` traits depending on if the conversion requires -error handling. Below are two examples of how this can be done: +We recommend using either the [`From`][from] or [`TryFrom`][tryfrom] [conversion traits][conversion] +depending on if the conversion requires error handling or not. Below are two examples of how this +can be done: ```rust impl TryFrom for LoginUri { @@ -57,3 +59,6 @@ impl From for UriMatchType { ``` [openapi]: https://github.com/OpenAPITools/openapi-generator +[from]: https://doc.rust-lang.org/std/convert/trait.From.html +[tryfrom]: https://doc.rust-lang.org/std/convert/trait.TryFrom.html +[conversion]: https://doc.rust-lang.org/std/convert/index.html diff --git a/docs/architecture/sdk/versioning.md b/docs/architecture/sdk/versioning.md new file mode 100644 index 000000000..26ce628aa --- /dev/null +++ b/docs/architecture/sdk/versioning.md @@ -0,0 +1,16 @@ +# Versioning & Breaking Changes + +The SDK strives towards maintaining backward compatibility for a reasonable time. This ensures that +client applications can safely upgrade to newer versions of the SDK without resolving a large number +of breaking changes. For a more in-depth explanation of what constitutes a breaking change in rust, +see the [SemVer Compatibility section](https://doc.rust-lang.org/cargo/reference/semver.html) in +[the Cargo book](https://doc.rust-lang.org/cargo/index.html). + +There may be certain functionality that is actively being developed and is not yet stable. In these +cases, they should be marked as such and gated behind a `unstable` feature flag. Consuming client +applications should actively avoid merging these changes into `main`. + +To track breaking changes, we use a [changelog file in the `bitwarden` crate][changelog]. This file +should be updated with noteworthy changes for each PR. + +[changelog]: https://github.com/bitwarden/sdk/blob/main/crates/bitwarden/CHANGELOG.md diff --git a/docs/getting-started/sdk/index.md b/docs/getting-started/sdk/index.md index f4478f1e0..beb871ab9 100644 --- a/docs/getting-started/sdk/index.md +++ b/docs/getting-started/sdk/index.md @@ -10,8 +10,9 @@ multiple languages. ## Requirements -- Latest stable version of Rust, preferably installed via [rustup](https://rustup.rs/). -- Node and npm. +- [Rust](https://www.rust-lang.org/tools/install) latest stable version - (preferably installed via + [rustup](https://rustup.rs/)) +- NodeJS and NPM. See the [Tools and Libraries](../tools/index.md) page for more information. From 249515e0f55ac790dc861144d872f4a316c396bd Mon Sep 17 00:00:00 2001 From: Hinton Date: Tue, 24 Sep 2024 13:22:40 +0200 Subject: [PATCH 02/11] Update contributing docs with local linking instructions --- docs/getting-started/sdk/index.md | 53 +++++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/docs/getting-started/sdk/index.md b/docs/getting-started/sdk/index.md index beb871ab9..c306b78e1 100644 --- a/docs/getting-started/sdk/index.md +++ b/docs/getting-started/sdk/index.md @@ -39,7 +39,60 @@ To build the SDK, run the following command: cargo build ``` +## Linking SDK to clients + +After modifying the SDK, it can be beneficial to test the changes in the client applications. To do +so you will need to update the SDK reference in the client applications. + +These instructions assumes you have a directory structure similar to: + +```text +sdk/ +clients/ +ios/ +android/ +``` + +### Web clients + +The web clients uses NPM to install the SDK as a dependency. NPM offers a dedicated command +[`link`][npm-link] which can be used to temporarily replace the packages with a local version. + +```bash +# Assuming sdk is installed next to clients. +# From the clients directory run: +npm link ../sdk/languages/sdk-client npm link ../sdk/languages/wasm +``` + +:::warning + +Running `npm ci` or `npm install` will replace the linked packages with the published version. + +::: + +### Mobile + +#### Android + +1. Build and publish the SDK to the local Maven repository: + + ```bash + ../sdk/languages/kotlin/publish-local.sh + ``` + +2. Set the project property `LOCAL_SDK` to true in the `gradle.properties` file. + +#### iOS + +Run the bootstrap script with the `LOCAL_SDK` environment variable set to true in order to use the +local SDK build: + +```bash +LOCAL_SDK=true ./Scripts/bootstrap.sh +``` + For more information on how to use the SDK, see the [repository](https://github.com/bitwarden/sdk). +[npm-link]: https://docs.npmjs.com/cli/v9/commands/npm-link [sm]: https://bitwarden.com/products/secrets-manager/ [pm]: https://bitwarden.com/ From a868455270fb57ff0d5850d27b73e29b9076bcdd Mon Sep 17 00:00:00 2001 From: Hinton Date: Tue, 24 Sep 2024 13:25:18 +0200 Subject: [PATCH 03/11] Fix property naming --- docs/getting-started/sdk/index.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/getting-started/sdk/index.md b/docs/getting-started/sdk/index.md index c306b78e1..3ffd0225b 100644 --- a/docs/getting-started/sdk/index.md +++ b/docs/getting-started/sdk/index.md @@ -80,7 +80,7 @@ Running `npm ci` or `npm install` will replace the linked packages with the publ ../sdk/languages/kotlin/publish-local.sh ``` -2. Set the project property `LOCAL_SDK` to true in the `gradle.properties` file. +2. Set the project property `localSdk` to true in the `gradle.properties` file. #### iOS From a3b4385e53a63a45bb90d9b4195fc26764bd88a6 Mon Sep 17 00:00:00 2001 From: Hinton Date: Thu, 26 Sep 2024 10:20:21 +0200 Subject: [PATCH 04/11] Update android instructions --- docs/getting-started/sdk/index.md | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/docs/getting-started/sdk/index.md b/docs/getting-started/sdk/index.md index 3ffd0225b..db17fbbc1 100644 --- a/docs/getting-started/sdk/index.md +++ b/docs/getting-started/sdk/index.md @@ -8,6 +8,9 @@ Bitwarden provides a public Software Development Kit (SDK) for [Secrets Manager] SDK for the Bitwarden [Password Manager][pm]. The SDK is written in Rust and provides bindings for multiple languages. +For more in-depth documentation please review the [SDK Architecture](../../architecture/sdk) page +and the project's [README](https://github.com/bitwarden/sdk). + ## Requirements - [Rust](https://www.rust-lang.org/tools/install) latest stable version - (preferably installed via @@ -59,9 +62,7 @@ The web clients uses NPM to install the SDK as a dependency. NPM offers a dedica [`link`][npm-link] which can be used to temporarily replace the packages with a local version. ```bash -# Assuming sdk is installed next to clients. -# From the clients directory run: -npm link ../sdk/languages/sdk-client npm link ../sdk/languages/wasm +npm link ../sdk/languages/sdk-client ../sdk/languages/wasm ``` :::warning @@ -80,7 +81,7 @@ Running `npm ci` or `npm install` will replace the linked packages with the publ ../sdk/languages/kotlin/publish-local.sh ``` -2. Set the project property `localSdk` to true in the `gradle.properties` file. +2. Set the user property `localSdk=true` in the `user.properties` file. #### iOS @@ -91,8 +92,6 @@ local SDK build: LOCAL_SDK=true ./Scripts/bootstrap.sh ``` -For more information on how to use the SDK, see the [repository](https://github.com/bitwarden/sdk). - [npm-link]: https://docs.npmjs.com/cli/v9/commands/npm-link [sm]: https://bitwarden.com/products/secrets-manager/ [pm]: https://bitwarden.com/ From 506bc5ac5fe0b9bafa9ce02972f1f478df026c7e Mon Sep 17 00:00:00 2001 From: Hinton Date: Thu, 26 Sep 2024 12:53:14 +0200 Subject: [PATCH 05/11] Document client struct --- docs/architecture/sdk/index.md | 44 ++++++++++++++++++++++++++++++---- 1 file changed, 40 insertions(+), 4 deletions(-) diff --git a/docs/architecture/sdk/index.md b/docs/architecture/sdk/index.md index 8fbc6e938..3b1b452a0 100644 --- a/docs/architecture/sdk/index.md +++ b/docs/architecture/sdk/index.md @@ -31,10 +31,46 @@ contains very little logic itself. ### `bitwarden-core` crate The `bitwarden-core` crate contains the underlying functionality of the SDK. This includes a -`Client` struct which represents a single account instance in the SDK. - -Other crates in the SDK depend on `bitwarden-core` and provides extensions to the `Client` struct to -implement specific domains. +`Client` struct. Other crates in the SDK depend on `bitwarden-core` and provides extensions to the +`Client` struct to implement specific domains. + +## Client struct + +The `Client` struct is the main entry point for the SDK and represents a single account instance. +Any action that needs to be performed on the account is generally done through the `Client` struct. +This allows the internal to be hidden from the consumer and provides a clear API. + +We can extend the `Client` struct using extension traits in feature crates. This allow the +underlying implementation to be internal to the crate with only the public API exposed through the +`Client` struct. Below is an example of a generator extension for the `Client` struct. + +```rust +pub struct ClientGenerator<'a> { + client: &'a Client, +} + +impl<'a> ClientGenerator<'a> { + fn new(client: &'a Client) -> Self { + Self { client } + } + + pub fn password(&self, input: PasswordGeneratorRequest) -> Result { + password(input) + } + +} + +// Extension which exposes `generator` method on the `Client` struct. +pub trait ClientGeneratorExt<'a> { + fn generator(&'a self) -> ClientGenerator<'a>; +} + +impl<'a> ClientGeneratorExt<'a> for Client { + fn generator(&'a self) -> ClientGenerator<'a> { + ClientGenerator::new(self) + } +} +``` ## Language bindings From 11b7e877ab1b2e81a3f9af22b19fbcb217e03c28 Mon Sep 17 00:00:00 2001 From: Hinton Date: Thu, 26 Sep 2024 13:06:30 +0200 Subject: [PATCH 06/11] Add link to api docs --- docs/architecture/sdk/index.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docs/architecture/sdk/index.md b/docs/architecture/sdk/index.md index 3b1b452a0..85b58fa70 100644 --- a/docs/architecture/sdk/index.md +++ b/docs/architecture/sdk/index.md @@ -11,6 +11,9 @@ layers. This includes but is not limited to: API calls, data models, encryption, We have compiled a list of resources for learning rust in a [Confluence page](https://bitwarden.atlassian.net/wiki/spaces/DEV/pages/517898288/Rust+Learning+Resources). +For API documentation view the latest +[API documentation](https://sdk-api-docs.bitwarden.com/bitwarden/index.html) which also includes +internal private items. ## Architecture From ff63e6b9c70f055b0c537222cf62f785688bb313 Mon Sep 17 00:00:00 2001 From: Hinton Date: Thu, 26 Sep 2024 13:17:02 +0200 Subject: [PATCH 07/11] Fix typo --- docs/architecture/sdk/index.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/architecture/sdk/index.md b/docs/architecture/sdk/index.md index 85b58fa70..072356edf 100644 --- a/docs/architecture/sdk/index.md +++ b/docs/architecture/sdk/index.md @@ -34,7 +34,7 @@ contains very little logic itself. ### `bitwarden-core` crate The `bitwarden-core` crate contains the underlying functionality of the SDK. This includes a -`Client` struct. Other crates in the SDK depend on `bitwarden-core` and provides extensions to the +`Client` struct. Other crates in the SDK depend on `bitwarden-core` and provide extensions to the `Client` struct to implement specific domains. ## Client struct From d254815e541543f1c9bc960a5b74e186b6c77b40 Mon Sep 17 00:00:00 2001 From: Oscar Hinton Date: Fri, 27 Sep 2024 10:38:49 +0200 Subject: [PATCH 08/11] Apply suggestions from code review Co-authored-by: Matt Bishop --- docs/architecture/sdk/index.md | 6 +++--- docs/architecture/sdk/server-bindings.md | 2 +- docs/getting-started/sdk/index.md | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/docs/architecture/sdk/index.md b/docs/architecture/sdk/index.md index 072356edf..277df40c8 100644 --- a/docs/architecture/sdk/index.md +++ b/docs/architecture/sdk/index.md @@ -9,16 +9,16 @@ SDK for the Bitwarden [Password Manager][pm]. The SDK is written in Rust and pro multiple languages. The general end goal of the SDK is to own everything up to the presentational layers. This includes but is not limited to: API calls, data models, encryption, and business logic. -We have compiled a list of resources for learning rust in a +We have compiled a list of resources for learning Rust in a [Confluence page](https://bitwarden.atlassian.net/wiki/spaces/DEV/pages/517898288/Rust+Learning+Resources). For API documentation view the latest -[API documentation](https://sdk-api-docs.bitwarden.com/bitwarden/index.html) which also includes +[API documentation](https://sdk-api-docs.bitwarden.com/bitwarden/index.html) that also includes internal private items. ## Architecture The Bitwarden SDK is structured as a single [Git repository](https://github.com/bitwarden/sdk) with -multiple internal crates. Please review the README in the repository for up to date information +multiple internal crates. Please review the `README` in the repository for up to date information about the different crates. We generally strive towards extracting features into separate crates to keep the `bitwarden-core` diff --git a/docs/architecture/sdk/server-bindings.md b/docs/architecture/sdk/server-bindings.md index a34aee2ea..1c9b763bb 100644 --- a/docs/architecture/sdk/server-bindings.md +++ b/docs/architecture/sdk/server-bindings.md @@ -22,7 +22,7 @@ let response: SyncResponseModel = bitwarden_api_api::apis::sync_api::sync_get(&config.api, exclude_subdomains).await?; ``` -You SHOULD not expose the request and response models of the auto generated bindings and SHOULD +You *should not* expose the request and response models of the auto generated bindings and *should* instead define and use your own models. This ensures the server request / response models are decoupled from the SDK models, which allows for easier changes in the future without breaking backwards compatibility. diff --git a/docs/getting-started/sdk/index.md b/docs/getting-started/sdk/index.md index db17fbbc1..c3ba97d7d 100644 --- a/docs/getting-started/sdk/index.md +++ b/docs/getting-started/sdk/index.md @@ -9,7 +9,7 @@ SDK for the Bitwarden [Password Manager][pm]. The SDK is written in Rust and pro multiple languages. For more in-depth documentation please review the [SDK Architecture](../../architecture/sdk) page -and the project's [README](https://github.com/bitwarden/sdk). +and the project's [`README`](https://github.com/bitwarden/sdk). ## Requirements @@ -42,7 +42,7 @@ To build the SDK, run the following command: cargo build ``` -## Linking SDK to clients +## Linking the SDK to clients After modifying the SDK, it can be beneficial to test the changes in the client applications. To do so you will need to update the SDK reference in the client applications. From 6cf04beb05bb70100d0d3dd1b30d669591de4d88 Mon Sep 17 00:00:00 2001 From: Hinton Date: Fri, 27 Sep 2024 15:28:21 +0200 Subject: [PATCH 09/11] Format --- docs/architecture/sdk/server-bindings.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/architecture/sdk/server-bindings.md b/docs/architecture/sdk/server-bindings.md index 1c9b763bb..9a1aecf0a 100644 --- a/docs/architecture/sdk/server-bindings.md +++ b/docs/architecture/sdk/server-bindings.md @@ -22,7 +22,7 @@ let response: SyncResponseModel = bitwarden_api_api::apis::sync_api::sync_get(&config.api, exclude_subdomains).await?; ``` -You *should not* expose the request and response models of the auto generated bindings and *should* +You _should not_ expose the request and response models of the auto generated bindings and _should_ instead define and use your own models. This ensures the server request / response models are decoupled from the SDK models, which allows for easier changes in the future without breaking backwards compatibility. From 8b5c81cb96273980a9867df63d2d3a9b4bf5b4b6 Mon Sep 17 00:00:00 2001 From: Hinton Date: Fri, 4 Oct 2024 09:28:19 +0200 Subject: [PATCH 10/11] Resolve review feedback --- docs/architecture/sdk/index.md | 2 +- docs/architecture/sdk/versioning.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/architecture/sdk/index.md b/docs/architecture/sdk/index.md index 277df40c8..ea2779caa 100644 --- a/docs/architecture/sdk/index.md +++ b/docs/architecture/sdk/index.md @@ -41,7 +41,7 @@ The `bitwarden-core` crate contains the underlying functionality of the SDK. Thi The `Client` struct is the main entry point for the SDK and represents a single account instance. Any action that needs to be performed on the account is generally done through the `Client` struct. -This allows the internal to be hidden from the consumer and provides a clear API. +This allows the internals to be hidden from the consumer and provides a clear API. We can extend the `Client` struct using extension traits in feature crates. This allow the underlying implementation to be internal to the crate with only the public API exposed through the diff --git a/docs/architecture/sdk/versioning.md b/docs/architecture/sdk/versioning.md index 26ce628aa..7c10bf8bc 100644 --- a/docs/architecture/sdk/versioning.md +++ b/docs/architecture/sdk/versioning.md @@ -8,7 +8,7 @@ see the [SemVer Compatibility section](https://doc.rust-lang.org/cargo/reference There may be certain functionality that is actively being developed and is not yet stable. In these cases, they should be marked as such and gated behind a `unstable` feature flag. Consuming client -applications should actively avoid merging these changes into `main`. +applications should avoid merging changes that depend on these features into `main`. To track breaking changes, we use a [changelog file in the `bitwarden` crate][changelog]. This file should be updated with noteworthy changes for each PR. From 09a4f289963da3c1c9d7b7ee9f236d7cdc34a217 Mon Sep 17 00:00:00 2001 From: Hinton Date: Fri, 4 Oct 2024 10:41:28 +0200 Subject: [PATCH 11/11] Fix link command --- docs/getting-started/sdk/index.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/getting-started/sdk/index.md b/docs/getting-started/sdk/index.md index c3ba97d7d..6982c6f00 100644 --- a/docs/getting-started/sdk/index.md +++ b/docs/getting-started/sdk/index.md @@ -62,7 +62,7 @@ The web clients uses NPM to install the SDK as a dependency. NPM offers a dedica [`link`][npm-link] which can be used to temporarily replace the packages with a local version. ```bash -npm link ../sdk/languages/sdk-client ../sdk/languages/wasm +npm link ../sdk/languages/sdk-internal ``` :::warning