Add linter to CI and applied some fixes (#35)

* Apply clippy fixes

* Setup clippy CI

* Apply cargo fmt

* Fix for 'module_inception' clippy rule

This error resulted from the fact that we had a files that matched their folder's name.
E.g: We had crypto/src/key/key.rs. The module's name is `key`, and it contained a module named `key` within it.

To fix, I removed all of this inception. `key.rs`'s code now lives in `mod.rs`, same with `key_manager.rs`. Subsequently, this allows us to r
emove all of the global use statements like `pub use key::*;`!

This also brings consistency with the approach taken in the `tbdex-rs` repo, as discussed here: TBD54566975/tbdex-rs#31 (comment)

* Apply suggestions from code review

Co-authored-by: Adam Mika <88001738+amika-sq@users.noreply.github.com>

---------

Co-authored-by: Adam Mika <amika@squareup.com>
Co-authored-by: Adam Mika <88001738+amika-sq@users.noreply.github.com>

.github/workflows/ci.yml

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

crates/credentials/src/pex/v2/presentation_definition.rs

@@ -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()
+        })
     }
 }
 
@@ -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")
     }
 }

crates/crypto/src/key/mod.rs

@@ -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;
+}

crates/crypto/src/key/private_key.rs

@@ -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;
 
@@ -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)
     }

crates/crypto/src/key/public_key.rs

@@ -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)
     }
@@ -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() {
@@ -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();
         assert_eq!(verification_warnings.len(), 0);
     }
 
@@ -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());
     }
 }

crates/crypto/src/key_manager/key_store/in_memory_key_store.rs

@@ -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;
@@ -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());
@@ -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 {

crates/crypto/src/key_manager/key_store/mod.rs

@@ -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>;
+}

crates/crypto/src/key_manager/local_key_manager.rs

@@ -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;
@@ -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)
     }

crates/crypto/src/key_manager/mod.rs

@@ -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>;
+}