Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. Weโ€™ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve SDK documentation #415

Merged
merged 13 commits into from
Oct 7, 2024
79 changes: 63 additions & 16 deletions docs/architecture/sdk/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,38 +9,77 @@ 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.

<Bitwarden>We have collected a list of various learning resources on
[Confluence](https://bitwarden.atlassian.net/wiki/spaces/DEV/pages/517898288/Rust+Learning+Resources).</Bitwarden>
<Bitwarden>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).</Bitwarden>
For API documentation view the latest
[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. 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. Other crates in the SDK depend on `bitwarden-core` and provide 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 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
`Client` struct. Below is an example of a generator extension for the `Client` struct.

```rust
pub struct ClientGenerator<'a> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thought: I know this is an established format but to me ClientGenerator sounds weird, like it's something that generates clients. It's the same thing with the other client extensions, e.g. ClientVault which sounds like there are multiple vaults and this one belongs to the Client.

On the other hand ClientGeneratorExt sounds very reasonable, it's a Generator Extension for the Client.

I don't think I would've reacted to this if we would've just passed around the Client struct and simply applied Extensions to it, i.e. Client.password(...), but as it is now, we are actually creating secondary "sub-clientswhich themself have a reference to theClientinstance.Client.generator()` actively constructs a new type of "Client"

Maybe I'm just nitpicking I don't know, but I think we'll have problems referring to things in the future

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm open for alternative name schemas.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Hinton
Well my suggestion is to flip the name from ClientGenerator to GeneratorClient, not that big change considering how big my previous message was xD

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems fine to me. Mind creating a jira issue and tackling this mountain of a task :).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

client: &'a Client,
}

impl<'a> ClientGenerator<'a> {
fn new(client: &'a Client) -> Self {
Self { client }
}

pub fn password(&self, input: PasswordGeneratorRequest) -> Result<String, PasswordError> {
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

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
Expand All @@ -63,5 +102,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/
15 changes: 10 additions & 5 deletions docs/architecture/sdk/server-bindings.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
coroiu marked this conversation as resolved.
Show resolved Hide resolved
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<bitwarden_api_api::models::CipherLoginUriModel> for LoginUri {
Expand Down Expand Up @@ -57,3 +59,6 @@ impl From<bitwarden_api_api::models::UriMatchType> 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
16 changes: 16 additions & 0 deletions docs/architecture/sdk/versioning.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
# Versioning & Breaking Changes

The SDK strives towards maintaining backward compatibility for a reasonable time. This ensures that
dani-garcia marked this conversation as resolved.
Show resolved Hide resolved
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 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.

[changelog]: https://github.com/bitwarden/sdk/blob/main/crates/bitwarden/CHANGELOG.md
59 changes: 56 additions & 3 deletions docs/getting-started/sdk/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,14 @@ 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

- 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.

Expand All @@ -38,7 +42,56 @@ To build the SDK, run the following command:
cargo build
```

For more information on how to use the SDK, see the [repository](https://github.com/bitwarden/sdk).
## 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.

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
npm link ../sdk/languages/sdk-internal
```

:::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
```
coroiu marked this conversation as resolved.
Show resolved Hide resolved

2. Set the user property `localSdk=true` in the `user.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
```

[npm-link]: https://docs.npmjs.com/cli/v9/commands/npm-link
[sm]: https://bitwarden.com/products/secrets-manager/
[pm]: https://bitwarden.com/