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

[PM-16908] Validate WASM BitwardenClient API is async #173

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .github/workflows/build-wasm-internal.yml
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,9 @@ jobs:
registry-url: "https://npm.pkg.github.com"
cache: "npm"

- name: NPM setup
run: npm ci

- name: Install dependencies
run: npm i -g binaryen

Expand Down
3 changes: 3 additions & 0 deletions crates/bitwarden-wasm-internal/build.sh
Original file line number Diff line number Diff line change
Expand Up @@ -45,3 +45,6 @@ npx terser ./crates/bitwarden-wasm-internal/npm/bitwarden_wasm_internal_bg.wasm.

# Remove unneeded files
rm -rf ./crates/bitwarden-wasm-internal/npm/mvp

# Validate the generated types and generate the subclient map
node ./crates/bitwarden-wasm-internal/validate_wasm_types.js
6 changes: 3 additions & 3 deletions crates/bitwarden-wasm-internal/src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,15 @@
}

/// Test method, echoes back the input
pub fn echo(&self, msg: String) -> String {
pub async fn echo(&self, msg: String) -> String {

Check warning on line 23 in crates/bitwarden-wasm-internal/src/client.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden-wasm-internal/src/client.rs#L23

Added line #L23 was not covered by tests
msg
}

pub fn version(&self) -> String {
pub async fn version(&self) -> String {

Check warning on line 27 in crates/bitwarden-wasm-internal/src/client.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden-wasm-internal/src/client.rs#L27

Added line #L27 was not covered by tests
env!("SDK_VERSION").to_owned()
}

pub fn throw(&self, msg: String) -> Result<(), TestError> {
pub async fn throw(&self, msg: String) -> Result<(), TestError> {

Check warning on line 31 in crates/bitwarden-wasm-internal/src/client.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden-wasm-internal/src/client.rs#L31

Added line #L31 was not covered by tests
Err(TestError(msg))
}

Expand Down
7 changes: 5 additions & 2 deletions crates/bitwarden-wasm-internal/src/crypto.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,14 +42,17 @@

/// Generates a new key pair and encrypts the private key with the provided user key.
/// Crypto initialization not required.
pub fn make_key_pair(&self, user_key: String) -> Result<MakeKeyPairResponse, CryptoError> {
pub async fn make_key_pair(
&self,
user_key: String,
) -> Result<MakeKeyPairResponse, CryptoError> {

Check warning on line 48 in crates/bitwarden-wasm-internal/src/crypto.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden-wasm-internal/src/crypto.rs#L45-L48

Added lines #L45 - L48 were not covered by tests
self.0.crypto().make_key_pair(user_key)
}

/// Verifies a user's asymmetric keys by decrypting the private key with the provided user
/// key. Returns if the private key is decryptable and if it is a valid matching key.
/// Crypto initialization not required.
pub fn verify_asymmetric_keys(
pub async fn verify_asymmetric_keys(

Check warning on line 55 in crates/bitwarden-wasm-internal/src/crypto.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden-wasm-internal/src/crypto.rs#L55

Added line #L55 was not covered by tests
&self,
request: VerifyAsymmetricKeysRequest,
) -> Result<VerifyAsymmetricKeysResponse, CryptoError> {
Expand Down
4 changes: 4 additions & 0 deletions crates/bitwarden-wasm-internal/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
// We define the WASM API as async so that we can use the same API for the IPC remote API.
// This means that some methods will need to be async even though they do no async work.
#![allow(clippy::unused_async)]

mod client;
mod crypto;
mod custom_types;
Expand Down
2 changes: 1 addition & 1 deletion crates/bitwarden-wasm-internal/src/vault/folders.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
#[wasm_bindgen]
impl ClientFolders {
/// Decrypt folder
pub fn decrypt(&self, folder: Folder) -> Result<FolderView, DecryptError> {
pub async fn decrypt(&self, folder: Folder) -> Result<FolderView, DecryptError> {

Check warning on line 19 in crates/bitwarden-wasm-internal/src/vault/folders.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden-wasm-internal/src/vault/folders.rs#L19

Added line #L19 was not covered by tests
self.0.vault().folders().decrypt(folder)
}
}
2 changes: 1 addition & 1 deletion crates/bitwarden-wasm-internal/src/vault/totp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
/// # Returns
/// - `Ok(TotpResponse)` containing the generated code and period
/// - `Err(TotpError)` if code generation fails
pub fn generate_totp(
pub async fn generate_totp(

Check warning on line 31 in crates/bitwarden-wasm-internal/src/vault/totp.rs

View check run for this annotation

Codecov / codecov/patch

crates/bitwarden-wasm-internal/src/vault/totp.rs#L31

Added line #L31 was not covered by tests
&self,
key: String,
time_ms: Option<f64>,
Expand Down
127 changes: 127 additions & 0 deletions crates/bitwarden-wasm-internal/validate_wasm_types.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,127 @@
// @ts-check
const fs = require("fs");
const path = require("path");
const ts = require("typescript");

// The root SDK client, and the main entry point for the remote IPC-based client
const ROOT_CLIENT = "BitwardenClient";

const SKIP_METHODS = [
// This methods is generated by the `wasm-bindgen` macro and is not async
"free",
];

// Read the types definition file and create an AST
const jsFilename = path.resolve(__dirname, "npm/bitwarden_wasm_internal_bg.js");
const tsFilename = path.resolve(__dirname, "npm/bitwarden_wasm_internal.d.ts");
const jsCode = fs.readFileSync(jsFilename, "utf-8");
const tsCode = fs.readFileSync(tsFilename, "utf-8");
const ast = ts.createSourceFile(tsFilename, tsCode, ts.ScriptTarget.Latest);

/** @type {Set<string>} */
const allClasses = new Set();

/** @type {Record<string, Record<string, string>>} */
const allTransitions = {};

/** @type {{className: string, methodName: string}[]} */
const syncMethods = [];

// First collect all the classes
ast.forEachChild((child) => {
if (ts.isClassDeclaration(child) && child.name) {
allClasses.add(child.name.text);
}
});

// Then create the transitions table and validate that all functions are async
ast.forEachChild((child) => {
if (ts.isClassDeclaration(child) && child.name) {
const className = child.name.text;
child.members.forEach((member) => {
if (ts.isMethodDeclaration(member) && ts.isIdentifier(member.name)) {
const methodName = member.name.text;
if (SKIP_METHODS.includes(methodName)) {
return;
}

// Check if the return type is a reference type (class/promise)
if (
member.type &&
ts.isTypeReferenceNode(member.type) &&
ts.isIdentifier(member.type.typeName)
) {
const returnType = member.type.typeName.text;
// If it's a class that we define, add it to the transitions table.
if (allClasses.has(returnType)) {
allTransitions[className] ??= {};
allTransitions[className][returnType] = methodName;
return;
}

// If it's a Promise, return early so it's not added to the syncMethods list.
if (returnType === "Promise") {
return;
}
}

// Check if the method is using the async keyword
if (!member.modifiers?.some((modifier) => modifier.kind === ts.SyntaxKind.AsyncKeyword)) {
syncMethods.push({ className, methodName });
}
}
});
}
});

// Generate the sub-clients table by following all the transitions from the root client.
// Also keep track of all the clients that are seen, so we can ensure that all their methods are sync.
/**
* @param {string} clientName
* @param {Record<string, any>} output
* @param {Set<string>} seenClients
*/
function addSubClients(clientName, output, seenClients) {
seenClients.add(clientName);
for (const [subClient, func] of Object.entries(allTransitions[clientName] ?? {})) {
seenClients.add(subClient);
output[func] ??= {};
addSubClients(subClient, output[func], seenClients);
}
}
const subClients = {};
const seenClients = new Set();
addSubClients(ROOT_CLIENT, subClients, seenClients);

// Save the sub-clients table to the types file
const SEPARATOR = "/* The following code is generated by the validate_wasm_types.js script */";
fs.writeFileSync(
jsFilename,
`${jsCode.split(SEPARATOR)[0]}
${SEPARATOR}
export const SUB_CLIENT_METHODS = ${JSON.stringify(subClients, null, 2)};
`,
);
fs.writeFileSync(
tsFilename,
`${tsCode.split(SEPARATOR)[0]}
${SEPARATOR}
export declare const SUB_CLIENT_METHODS: Record<string, any>;
`,
);

// Report any errors and exit with a non-zero code
let hasSyncMethods = false;
for (const { className, methodName } of syncMethods) {
if (seenClients.has(className)) {
console.error(`[ERROR] Method ${className}.${methodName} is not async`);
hasSyncMethods = true;
}
}

if (hasSyncMethods) {
console.error(
"Some methods in the SDK clients are not async. To support being used through an IPC channel, all methods must be async.",
);
process.exit(1);
}
16 changes: 15 additions & 1 deletion package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
},
"devDependencies": {
"@openapitools/openapi-generator-cli": "2.15.3",
"prettier": "3.4.2"
"prettier": "3.4.2",
"typescript": "^5.7.3"
}
}
Loading