chore: refactor tests (#244)

* chore: refactor data structures * chore: test watchSubscriptions in integration tests * chore: allow running integration tests in parallel * chore: minimal deployment test * fix: flaky registry tests * chore: work on new test * chore: random nonce * chore: refactor magic strings * chore: properly parse did.json response * chore: further eliminate magic strings * chore: refactor magic strings * chore: refactor * chore: refactor * chore: refactor message construction * chore: refactor * chore: wip test * chore: improve test reliability * chore: complete Keys Server test * chore: unregister identity key * chore: remove redundant sleep * chore: refactor * chore: refactoring * chore: use versioned dependency
reown-com · Jan 5, 2024 · a3832ad · a3832ad
1 parent 5d1f595
commit a3832ad
Show file tree

Hide file tree

Showing 22 changed files with 1,704 additions and 316 deletions.
diff --git a/.github/workflows/sub-ci.yml b/.github/workflows/sub-ci.yml
@@ -79,4 +79,4 @@ jobs:
         uses: WalletConnect/actions-rs/cargo@1.1.0
         with:
           command: test
-          args: --test integration -- --test-threads=1
+          args: --test integration
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Cargo.toml b/Cargo.toml
@@ -74,8 +74,8 @@ derive_more = "0.99.17"
 futures = "0.3.26"
 dashmap = "5.4.0"
 
-relay_rpc = { git = "https://github.com/WalletConnect/WalletConnectRust.git", tag = "v0.23.2", features = ["cacao"] }
-relay_client = { git = "https://github.com/WalletConnect/WalletConnectRust.git", tag = "v0.23.2" }
+relay_rpc = { git = "https://github.com/WalletConnect/WalletConnectRust.git", tag = "v0.23.3", features = ["cacao"] }
+relay_client = { git = "https://github.com/WalletConnect/WalletConnectRust.git", tag = "v0.23.3" }
 x25519-dalek = { version = "2.0.0", features = ["static_secrets"] }
 hkdf = "0.12.3"
 sha2 = "0.10.6"

diff --git a/justfile b/justfile
@@ -29,7 +29,7 @@ test-all:
 
 test-integration:
   @echo '==> Testing integration'
-  RUST_BACKTRACE=1 ANSI_LOGS=true cargo test --test integration -- --test-threads=1 # --test-threads=1 to only run 1 integration test at a time since they drop the entire database schema
+  RUST_BACKTRACE=1 ANSI_LOGS=true cargo test --test integration
 
 # Clean build artifacts
 clean:

diff --git a/src/auth.rs b/src/auth.rs
@@ -7,21 +7,26 @@ use {
     },
     base64::Engine,
     chrono::{DateTime, Duration as CDuration, Utc},
+    core::fmt,
     ed25519_dalek::{Signer, SigningKey},
     hyper::StatusCode,
     relay_rpc::{
         auth::{
             cacao::{Cacao, CacaoError},
-            did::{DID_DELIMITER, DID_METHOD_KEY, DID_PREFIX},
+            did::{
+                combine_did_data, extract_did_data, DidError, DID_DELIMITER, DID_METHOD_KEY,
+                DID_PREFIX,
+            },
         },
-        domain::DecodedClientId,
+        domain::{ClientIdDecodingError, DecodedClientId},
         jwt::{JwtHeader, JWT_HEADER_ALG, JWT_HEADER_TYP},
     },
     reqwest::Response,
     serde::{de::DeserializeOwned, Deserialize, Serialize},
     serde_json::Value,
     std::{
         collections::HashSet,
+        fmt::{Display, Formatter},
         sync::Arc,
         time::{Duration, Instant},
     },
@@ -52,6 +57,13 @@ pub struct SharedClaims {
     pub aud: String,
     /// description of action intent
     pub act: String,
+    /// major version of the API level being used as a string
+    #[serde(default = "default_mjv")]
+    pub mjv: String,
+}
+
+fn default_mjv() -> String {
+    "0".to_owned()
 }
 
 pub fn add_ttl(now: DateTime<Utc>, ttl: Duration) -> DateTime<Utc> {
@@ -73,7 +85,7 @@ pub struct WatchSubscriptionsRequestAuth {
     pub sub: String,
     /// did:web of app domain to watch, or `null` for all domains
     #[serde(default)]
-    pub app: Option<String>,
+    pub app: Option<DidWeb>,
 }
 
 impl GetSharedClaims for WatchSubscriptionsRequestAuth {
@@ -154,7 +166,7 @@ pub struct SubscriptionRequestAuth {
     /// did:pkh
     pub sub: String,
     /// did:web of app domain
-    pub app: String,
+    pub app: DidWeb,
     /// space-delimited scope of notification types authorized by the user
     pub scp: String,
 }
@@ -173,7 +185,9 @@ pub struct SubscriptionResponseAuth {
     /// publicKey
     pub sub: String,
     /// did:web of app domain
-    pub app: String,
+    pub app: DidWeb,
+    /// array of Notify Server Subscriptions
+    pub sbs: Vec<NotifyServerSubscription>,
 }
 
 impl GetSharedClaims for SubscriptionResponseAuth {
@@ -191,7 +205,7 @@ pub struct SubscriptionUpdateRequestAuth {
     /// did:pkh
     pub sub: String,
     /// did:web of app domain
-    pub app: String,
+    pub app: DidWeb,
     /// space-delimited scope of notification types authorized by the user
     pub scp: String,
 }
@@ -209,7 +223,9 @@ pub struct SubscriptionUpdateResponseAuth {
     /// did:pkh
     pub sub: String,
     /// did:web of app domain
-    pub app: String,
+    pub app: DidWeb,
+    /// array of Notify Server Subscriptions
+    pub sbs: Vec<NotifyServerSubscription>,
 }
 
 impl GetSharedClaims for SubscriptionUpdateResponseAuth {
@@ -227,7 +243,7 @@ pub struct SubscriptionDeleteRequestAuth {
     /// did:pkh
     pub sub: String,
     /// did:web of app domain
-    pub app: String,
+    pub app: DidWeb,
 }
 
 impl GetSharedClaims for SubscriptionDeleteRequestAuth {
@@ -243,7 +259,9 @@ pub struct SubscriptionDeleteResponseAuth {
     /// did:pkh
     pub sub: String,
     /// did:web of app domain
-    pub app: String,
+    pub app: DidWeb,
+    /// array of Notify Server Subscriptions
+    pub sbs: Vec<NotifyServerSubscription>,
 }
 
 impl GetSharedClaims for SubscriptionDeleteResponseAuth {
@@ -384,8 +402,8 @@ pub enum AuthError {
     #[error("Keyserver returned successful response, but without a value")]
     KeyserverResponseMissingValue,
 
-    #[error("JWT iss not did:key")]
-    JwtIssNotDidKey,
+    #[error("JWT iss not did:key: {0}")]
+    JwtIssNotDidKey(ClientIdDecodingError),
 
     #[error("CACAO verification failed: {0}")]
     CacaoValidation(CacaoError),
@@ -396,8 +414,8 @@ pub enum AuthError {
     #[error("CACAO doesn't contain matching iss: {0}")]
     CacaoMissingIdentityKey(CacaoError),
 
-    #[error("CACAO iss is not a did:pkh")]
-    CacaoIssNotDidPkh,
+    #[error("CACAO iss is not a did:pkh: {0}")]
+    CacaoIssNotDidPkh(DidError),
 
     #[error("CACAO has wrong iss")]
     CacaoWrongIdentityKey,
@@ -437,6 +455,8 @@ pub enum AuthorizedApp {
     Unlimited,
 }
 
+pub const KEYS_SERVER_STATUS_SUCCESS: &str = "SUCCESS";
+
 async fn keys_server_request(url: Url) -> Result<Cacao> {
     info!("Timing: Requesting to keys server");
     let response = reqwest::get(url).await?;
@@ -452,7 +472,7 @@ async fn keys_server_request(url: Url) -> Result<Cacao> {
 
     let keyserver_response = response.json::<KeyServerResponse>().await?;
 
-    if keyserver_response.status != "SUCCESS" {
+    if keyserver_response.status != KEYS_SERVER_STATUS_SUCCESS {
         Err(AuthError::KeyserverNotSuccess {
             status: keyserver_response.status,
             error: keyserver_response.error,
@@ -517,18 +537,22 @@ impl KeysServerResponseSource {
     }
 }
 
+pub const KEYS_SERVER_IDENTITY_ENDPOINT: &str = "/identity";
+pub const KEYS_SERVER_IDENTITY_ENDPOINT_PUBLIC_KEY_QUERY: &str = "publicKey";
+
 pub async fn verify_identity(
     iss: &str,
     ksu: &str,
     sub: &str,
     redis: Option<&Arc<Redis>>,
     metrics: Option<&Metrics>,
 ) -> Result<Authorization> {
-    let mut url = Url::parse(ksu)?.join("/identity")?;
-    let pubkey = iss
-        .strip_prefix("did:key:")
-        .ok_or(AuthError::JwtIssNotDidKey)?;
-    url.set_query(Some(&format!("publicKey={pubkey}")));
+    let mut url = Url::parse(ksu)?.join(KEYS_SERVER_IDENTITY_ENDPOINT)?;
+    let pubkey = DecodedClientId::try_from_did_key(iss)
+        .map_err(AuthError::JwtIssNotDidKey)?
+        .to_string();
+    url.query_pairs_mut()
+        .append_pair(KEYS_SERVER_IDENTITY_ENDPOINT_PUBLIC_KEY_QUERY, &pubkey);
 
     let start = Instant::now();
     let (cacao, source) = keys_server_request_cached(url, redis).await?;
@@ -559,12 +583,7 @@ pub async fn verify_identity(
         Err(AuthError::CacaoAccountMismatch)?;
     }
 
-    let account = cacao
-        .p
-        .iss
-        .strip_prefix("did:pkh:")
-        .ok_or(AuthError::CacaoIssNotDidPkh)?
-        .into();
+    let account = AccountId::from_did_pkh(&cacao.p.iss).map_err(AuthError::CacaoIssNotDidPkh)?;
 
     if let Some(nbf) = cacao.p.nbf {
         let nbf = DateTime::parse_from_rfc3339(&nbf)?;
@@ -608,15 +627,15 @@ fn parse_cacao_statement(statement: &str, domain: &str) -> Result<AuthorizedApp>
 }
 
 #[derive(Debug, Clone, Serialize, Deserialize)]
-struct KeyServerResponse {
-    status: String,
-    error: Option<Value>,
-    value: Option<CacaoValue>,
+pub struct KeyServerResponse {
+    pub status: String,
+    pub error: Option<Value>,
+    pub value: Option<CacaoValue>,
 }
 
 #[derive(Debug, Clone, Serialize, Deserialize)]
-struct CacaoValue {
-    cacao: relay_rpc::auth::cacao::Cacao,
+pub struct CacaoValue {
+    pub cacao: relay_rpc::auth::cacao::Cacao,
 }
 
 pub fn encode_authentication_private_key(authentication_key: &SigningKey) -> String {
@@ -635,6 +654,53 @@ pub fn encode_subscribe_public_key(subscribe_key: &StaticSecret) -> String {
     hex::encode(PublicKey::from(subscribe_key))
 }
 
+const DID_METHOD_WEB: &str = "web";
+
+#[derive(Debug, Clone, PartialEq, Eq, Hash)]
+pub struct DidWeb {
+    domain: String,
+}
+
+impl DidWeb {
+    pub fn from(did_web: &str) -> std::result::Result<Self, DidError> {
+        let domain = extract_did_data(did_web, DID_METHOD_WEB)?;
+        Ok(Self::from_domain(domain.to_owned()))
+    }
+
+    pub fn from_domain(domain: String) -> Self {
+        // TODO domain validation?
+        Self { domain }
+    }
+
+    pub fn domain(self) -> String {
+        self.domain
+    }
+}
+
+impl Display for DidWeb {
+    fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
+        write!(f, "{}", combine_did_data(DID_METHOD_WEB, &self.domain))
+    }
+}
+
+impl Serialize for DidWeb {
+    fn serialize<S: serde::Serializer>(
+        &self,
+        serializer: S,
+    ) -> std::result::Result<S::Ok, S::Error> {
+        serializer.serialize_str(&self.to_string())
+    }
+}
+
+impl<'a> Deserialize<'a> for DidWeb {
+    fn deserialize<D: serde::Deserializer<'a>>(
+        deserializer: D,
+    ) -> std::result::Result<Self, D::Error> {
+        let did_web = String::deserialize(deserializer)?;
+        Self::from(&did_web).map_err(serde::de::Error::custom)
+    }
+}
+
 #[cfg(test)]
 mod test {
     use super::*;

diff --git a/src/error.rs b/src/error.rs
@@ -6,7 +6,10 @@ use {
     axum::{response::IntoResponse, Json},
     data_encoding::DecodeError,
     hyper::StatusCode,
-    relay_rpc::domain::{ClientIdDecodingError, ProjectId, Topic},
+    relay_rpc::{
+        auth::did::DidError,
+        domain::{ClientIdDecodingError, ProjectId, Topic},
+    },
     serde_json::json,
     std::string::FromUtf8Error,
     tracing::{error, info, warn},
@@ -163,8 +166,8 @@ pub enum Error {
     #[error("The requested app does not match the project's app domain")]
     AppDoesNotMatch,
 
-    #[error("`app` invalid, not a did:web")]
-    AppNotDidWeb,
+    #[error("`app` invalid, not a did:web: {0}")]
+    AppNotDidWeb(DidError),
 
     #[error(transparent)]
     AppNotAuthorized(#[from] CheckAppAuthorizationError),

diff --git a/src/model/types/account_id.rs b/src/model/types/account_id.rs
@@ -1,4 +1,12 @@
-use {relay_rpc::new_type, sha2::Digest, sha3::Keccak256, std::sync::Arc};
+use {
+    relay_rpc::{
+        auth::did::{combine_did_data, extract_did_data, DidError},
+        new_type,
+    },
+    sha2::Digest,
+    sha3::Keccak256,
+    std::sync::Arc,
+};
 
 new_type!(
     #[doc = "A CAIP-10 account ID."]
@@ -18,6 +26,18 @@ impl From<&str> for AccountId {
     }
 }
 
+const DID_METHOD_PKH: &str = "pkh";
+
+impl AccountId {
+    pub fn from_did_pkh(did: &str) -> Result<Self, DidError> {
+        Ok(extract_did_data(did, DID_METHOD_PKH)?.into())
+    }
+
+    pub fn to_did_pkh(&self) -> String {
+        combine_did_data(DID_METHOD_PKH, self.as_ref())
+    }
+}
+
 fn ensure_erc_55(s: &str) -> String {
     if s.starts_with("eip155:") {
         let ox = "0x";
@@ -98,4 +118,18 @@ mod test {
         test("0xdbF03B407c01E7cD3CBea99509d93f8DDDC8C6FB");
         test("0xD1220A0cf47c7B9Be7A2E6BA89F429762e7b9aDb");
     }
+
+    #[test]
+    fn to_did_pkh() {
+        let address = "0x1234567890123456789012345678901234567890";
+        let account_id = AccountId::from(address);
+        assert_eq!(account_id.to_did_pkh(), format!("did:pkh:{address}"));
+    }
+
+    #[test]
+    fn from_did_pkh() {
+        let address = "0x1234567890123456789012345678901234567890";
+        let account_id = AccountId::from_did_pkh(&format!("did:pkh:{address}")).unwrap();
+        assert_eq!(account_id.as_ref(), address);
+    }
 }