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

Add linter to CI and applied some fixes #35

Merged
merged 6 commits into from
Dec 5, 2023
Merged
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
13 changes: 12 additions & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,25 @@ on:

env:
CARGO_TERM_COLOR: always
# Make sure CI fails on all warnings, including Clippy lints
RUSTFLAGS: "-Dwarnings"

jobs:
clippy:
runs-on: ubuntu-latest
strategy:
matrix:
rust: [ stable, nightly ]
steps:
- uses: actions/checkout@v3
- name: Run Clippy
run: cargo clippy --workspace
test:
name: cargo test
strategy:
matrix:
os: [ ubuntu-latest, macos-latest, windows-latest ]
rust: [stable, nightly]
rust: [ stable, nightly ]
runs-on: ${{ matrix.os }}
steps:
- uses: actions/checkout@v4
Expand Down
19 changes: 8 additions & 11 deletions crates/credentials/src/pex/v2/presentation_definition.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,15 +69,12 @@ pub struct Field {

impl Field {
pub fn filter_schema(&self) -> Option<JSONSchema> {
self.filter
.as_ref()
.map(|json| {
JSONSchema::options()
.with_draft(Draft::Draft7)
.compile(json)
.ok()
})
.flatten()
self.filter.as_ref().and_then(|json| {
JSONSchema::options()
.with_draft(Draft::Draft7)
.compile(json)
.ok()
})
}
}

Expand Down Expand Up @@ -201,7 +198,7 @@ mod tests {

fn load_json(path: &str) -> String {
let path = Path::new(path);
let json = fs::read_to_string(path).expect("Unable to load json file");
json

fs::read_to_string(path).expect("Unable to load json file")
}
}
22 changes: 0 additions & 22 deletions crates/crypto/src/key/key.rs

This file was deleted.

29 changes: 23 additions & 6 deletions crates/crypto/src/key/mod.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,25 @@
mod key;
pub use key::*;
pub mod private_key;
pub mod public_key;

mod private_key;
pub use private_key::*;
use ssi_jwk::JWK;
use ssi_jws::Error as JWSError;

mod public_key;
pub use public_key::*;
/// Enum defining all supported cryptographic key types.
pub enum KeyType {
Secp256k1,
Secp256r1,
Ed25519,
}

#[derive(thiserror::Error, Debug)]
pub enum KeyError {
#[error(transparent)]
JWSError(#[from] JWSError),
#[error("Algorithm not found on JWK")]
AlgorithmNotFound,
}

/// Trait defining all common behavior for cryptographic keys.
pub trait Key {
fn jwk(&self) -> &JWK;
}
5 changes: 3 additions & 2 deletions crates/crypto/src/key/private_key.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use crate::key::{Key, KeyError, PublicKey};
use crate::key::public_key::PublicKey;
use crate::key::{Key, KeyError};
use ssi_jwk::JWK;
use ssi_jws::sign_bytes;

Expand All @@ -14,7 +15,7 @@ impl PrivateKey {
/// Sign a payload using the target [`PrivateKey`].
pub fn sign(&self, payload: &[u8]) -> Result<Vec<u8>, KeyError> {
let algorithm = self.0.get_algorithm().ok_or(KeyError::AlgorithmNotFound)?;
let signed_bytes = sign_bytes(algorithm, &payload, &self.0)?;
let signed_bytes = sign_bytes(algorithm, payload, &self.0)?;

Ok(signed_bytes)
}
Expand Down
9 changes: 4 additions & 5 deletions crates/crypto/src/key/public_key.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,7 @@ impl PublicKey {
) -> Result<VerificationWarnings, KeyError> {
let algorithm = self.0.get_algorithm().ok_or(KeyError::AlgorithmNotFound)?;

let verification_warnings =
verify_bytes_warnable(algorithm, &payload, &self.0, &signature)?;
let verification_warnings = verify_bytes_warnable(algorithm, payload, &self.0, signature)?;

Ok(verification_warnings)
}
Expand All @@ -30,7 +29,7 @@ impl Key for PublicKey {
#[cfg(test)]
mod tests {
use super::*;
use crate::key::PrivateKey;
use crate::key::private_key::PrivateKey;

#[test]
fn test_verify() {
Expand All @@ -39,7 +38,7 @@ mod tests {
let signature = private_key.sign(payload).unwrap();

let public_key = private_key.to_public();
let verification_warnings = public_key.verify(&payload, &signature).unwrap();
let verification_warnings = public_key.verify(payload, &signature).unwrap();
Copy link
Contributor

Choose a reason for hiding this comment

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

What provoked this change? Why is signature passed by reference but payload isn't?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

BTW this was the result of running cargo clippy --fix

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh I see, because payload was already initialized by reference whereas signature was not

assert_eq!(verification_warnings.len(), 0);
}

Expand All @@ -51,7 +50,7 @@ mod tests {

// public_key is unrelated to the private_key used to sign the payload, so it should fail
let public_key = PublicKey(JWK::generate_secp256k1().unwrap());
let verification_warnings = public_key.verify(&payload, &signature);
let verification_warnings = public_key.verify(payload, &signature);
assert!(verification_warnings.is_err());
}
}
37 changes: 0 additions & 37 deletions crates/crypto/src/key_manager/key_manager.rs

This file was deleted.

10 changes: 8 additions & 2 deletions crates/crypto/src/key_manager/key_store/in_memory_key_store.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use crate::key::PrivateKey;
use crate::key::private_key::PrivateKey;
use crate::key_manager::key_store::{KeyStore, KeyStoreError};
use std::collections::HashMap;
use std::sync::RwLock;
Expand All @@ -8,6 +8,12 @@ pub struct InMemoryKeyStore {
map: RwLock<HashMap<String, PrivateKey>>,
}

impl Default for InMemoryKeyStore {
fn default() -> Self {
Self::new()
}
}

impl InMemoryKeyStore {
pub fn new() -> Self {
let map = RwLock::new(HashMap::new());
Expand Down Expand Up @@ -41,7 +47,7 @@ impl KeyStore for InMemoryKeyStore {
#[cfg(test)]
mod tests {
use super::*;
use crate::key::PrivateKey;
use crate::key::private_key::PrivateKey;
use ssi_jwk::JWK;

fn new_private_key() -> PrivateKey {
Expand Down
15 changes: 0 additions & 15 deletions crates/crypto/src/key_manager/key_store/key_store.rs

This file was deleted.

20 changes: 16 additions & 4 deletions crates/crypto/src/key_manager/key_store/mod.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,17 @@
mod key_store;
pub use key_store::*;
pub mod in_memory_key_store;

mod in_memory_key_store;
pub use in_memory_key_store::*;
use crate::key::private_key::PrivateKey;

#[derive(thiserror::Error, Debug)]
pub enum KeyStoreError {
#[error("{0}")]
InternalKeyStoreError(String),
}

// Trait for storing and retrieving private keys.
//
// Implementations of this trait should be thread-safe and allow for concurrent access.
pub trait KeyStore: Send + Sync {
fn get(&self, key_alias: &str) -> Result<Option<PrivateKey>, KeyStoreError>;
fn insert(&self, key_alias: &str, private_key: PrivateKey) -> Result<(), KeyStoreError>;
}
9 changes: 6 additions & 3 deletions crates/crypto/src/key_manager/local_key_manager.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
use crate::key::{KeyType, PrivateKey, PublicKey};
use crate::key_manager::key_store::{InMemoryKeyStore, KeyStore};
use crate::key::private_key::PrivateKey;
use crate::key::public_key::PublicKey;
use crate::key::KeyType;
use crate::key_manager::key_store::in_memory_key_store::InMemoryKeyStore;
use crate::key_manager::key_store::KeyStore;
use crate::key_manager::{KeyManager, KeyManagerError};
use ssi_jwk::JWK;
use std::sync::Arc;
Expand Down Expand Up @@ -56,7 +59,7 @@ impl KeyManager for LocalKeyManager {
.get(key_alias)?
.ok_or(KeyManagerError::SigningKeyNotFound)?;

let signed_payload = private_key.sign(&payload.to_vec())?;
let signed_payload = private_key.sign(payload)?;

Ok(signed_payload)
}
Expand Down
44 changes: 39 additions & 5 deletions crates/crypto/src/key_manager/mod.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,41 @@
mod key_manager;
pub use key_manager::*;
pub mod key_store;
pub mod local_key_manager;

mod local_key_manager;
pub use local_key_manager::*;
use crate::key::public_key::PublicKey;
use crate::key::{KeyError, KeyType};
use crate::key_manager::key_store::KeyStoreError;
use ssi_jwk::Error as JWKError;

pub mod key_store;
#[derive(thiserror::Error, Debug)]
pub enum KeyManagerError {
#[error("Signing key not found in KeyManager")]
SigningKeyNotFound,
#[error(transparent)]
JWKError(#[from] JWKError),
#[error(transparent)]
KeyError(#[from] KeyError),
#[error(transparent)]
KeyStoreError(#[from] KeyStoreError),
}

/// A key management trait for generating, storing, and utilizing keys private keys and their
/// associated public keys.
///
/// Implementations of this trait might provide key management through various Key Management
/// Systems (KMS), such as AWS KMS, Google Cloud KMD, Hardware Security Modules (HSM), or simple
/// in-memory storage, each adhering to the same consistent API for usage within applications.
pub trait KeyManager: Send + Sync {
/// Generates and securely stores a private key based on the provided `key_type`,
/// returning a unique alias that can be utilized to reference the generated key for future
/// operations.
fn generate_private_key(&self, key_type: KeyType) -> Result<String, KeyManagerError>;

/// Returns the public key associated with the provided `key_alias`, if one exists.
fn get_public_key(&self, key_alias: &str) -> Result<Option<PublicKey>, KeyManagerError>;

/// Signs the provided payload using the private key identified by the provided `key_alias`.
fn sign(&self, key_alias: &str, payload: &[u8]) -> Result<Vec<u8>, KeyManagerError>;

/// Returns the key alias of a public key, as was originally returned by `generate_private_key`.
fn alias(&self, public_key: &PublicKey) -> Result<String, KeyManagerError>;
}
Loading