From 078184b7d65a1c46d2f75a3a807b8b12e98ccca0 Mon Sep 17 00:00:00 2001
From: Alexis Sellier <alexis@radicle.xyz>
Date: Sat, 3 Sep 2022 23:27:50 +0200
Subject: [PATCH] node: Fix encoding of identifiers

Signed-off-by: Alexis Sellier <self@cloudhead.io>
---
 node/src/crypto.rs    | 16 ++++++++++-
 node/src/identity.rs  | 66 ++++++++++++++++++++++++++++++++++++++++++-
 node/src/lib.rs       |  1 +
 node/src/serde_ext.rs | 25 ++++++++++++++++
 node/src/storage.rs   | 19 ++-----------
 5 files changed, 109 insertions(+), 18 deletions(-)
 create mode 100644 node/src/serde_ext.rs

diff --git a/node/src/crypto.rs b/node/src/crypto.rs
index 1d8367ac..ea46d528 100644
--- a/node/src/crypto.rs
+++ b/node/src/crypto.rs
@@ -50,7 +50,7 @@ where
 
 /// The public/verification key.
 #[derive(Serialize, Deserialize, Eq, Debug, Copy, Clone)]
-#[serde(transparent)]
+#[serde(into = "String", try_from = "String")]
 pub struct PublicKey(pub ed25519::VerificationKey);
 
 /// The private/signing key.
@@ -78,6 +78,12 @@ impl fmt::Display for PublicKey {
     }
 }
 
+impl From<PublicKey> for String {
+    fn from(other: PublicKey) -> Self {
+        other.encode()
+    }
+}
+
 impl PartialEq for PublicKey {
     fn eq(&self, other: &Self) -> bool {
         self.0 == other.0
@@ -110,6 +116,14 @@ impl FromStr for PublicKey {
     }
 }
 
+impl TryFrom<String> for PublicKey {
+    type Error = PublicKeyError;
+
+    fn try_from(value: String) -> Result<Self, Self::Error> {
+        Self::from_str(&value)
+    }
+}
+
 impl Deref for PublicKey {
     type Target = ed25519::VerificationKey;
 
diff --git a/node/src/identity.rs b/node/src/identity.rs
index 3374e154..6e6d3b37 100644
--- a/node/src/identity.rs
+++ b/node/src/identity.rs
@@ -9,6 +9,7 @@ use thiserror::Error;
 
 use crate::crypto::{self, Verified};
 use crate::hash;
+use crate::serde_ext;
 use crate::storage::Remotes;
 
 pub static IDENTITY_PATH: Lazy<&Path> = Lazy::new(|| Path::new("Radicle.toml"));
@@ -22,7 +23,7 @@ pub enum ProjIdError {
     InvalidDigest(#[from] hash::DecodeError),
 }
 
-#[derive(Serialize, Deserialize, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
+#[derive(Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
 pub struct ProjId(hash::Digest);
 
 impl fmt::Display for ProjId {
@@ -66,13 +67,50 @@ impl From<hash::Digest> for ProjId {
     }
 }
 
+impl serde::Serialize for ProjId {
+    fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
+    where
+        S: serde::Serializer,
+    {
+        serde_ext::string::serialize(self, serializer)
+    }
+}
+
+impl<'de> serde::Deserialize<'de> for ProjId {
+    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
+    where
+        D: serde::Deserializer<'de>,
+    {
+        serde_ext::string::deserialize(deserializer)
+    }
+}
+
+#[derive(Error, Debug)]
+pub enum DidError {
+    #[error("invalid did: {0}")]
+    Did(String),
+    #[error("invalid public key: {0}")]
+    PublicKey(#[from] crypto::PublicKeyError),
+}
+
 #[derive(Serialize, Deserialize, PartialEq, Eq, Hash, Debug, Clone)]
+#[serde(into = "String", try_from = "String")]
 pub struct Did(crypto::PublicKey);
 
 impl Did {
     pub fn encode(&self) -> String {
         format!("did:key:{}", self.0.encode())
     }
+
+    pub fn decode(input: &str) -> Result<Self, DidError> {
+        let key = input
+            .strip_prefix("did:key:")
+            .ok_or_else(|| DidError::Did(input.to_owned()))?;
+
+        crypto::PublicKey::from_str(key)
+            .map(Did)
+            .map_err(DidError::from)
+    }
 }
 
 impl From<crypto::PublicKey> for Did {
@@ -81,6 +119,20 @@ impl From<crypto::PublicKey> for Did {
     }
 }
 
+impl From<Did> for String {
+    fn from(other: Did) -> Self {
+        other.encode()
+    }
+}
+
+impl TryFrom<String> for Did {
+    type Error = DidError;
+
+    fn try_from(value: String) -> Result<Self, Self::Error> {
+        Self::decode(&value)
+    }
+}
+
 impl fmt::Display for Did {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         write!(f, "{}", self.encode())
@@ -163,4 +215,16 @@ mod test {
 
         assert_eq!(input, decoded);
     }
+
+    #[quickcheck]
+    fn prop_json_eq_str(user: UserId, proj: ProjId, did: Did) {
+        let json = serde_json::to_string(&user).unwrap();
+        assert_eq!(format!("\"{}\"", user), json);
+
+        let json = serde_json::to_string(&proj).unwrap();
+        assert_eq!(format!("\"{}\"", proj), json);
+
+        let json = serde_json::to_string(&did).unwrap();
+        assert_eq!(format!("\"{}\"", did), json);
+    }
 }
diff --git a/node/src/lib.rs b/node/src/lib.rs
index a02e24a9..810c5854 100644
--- a/node/src/lib.rs
+++ b/node/src/lib.rs
@@ -16,6 +16,7 @@ mod identity;
 mod logger;
 mod protocol;
 mod rad;
+mod serde_ext;
 mod storage;
 #[cfg(test)]
 mod test;
diff --git a/node/src/serde_ext.rs b/node/src/serde_ext.rs
new file mode 100644
index 00000000..16326eff
--- /dev/null
+++ b/node/src/serde_ext.rs
@@ -0,0 +1,25 @@
+pub mod string {
+    use std::fmt::Display;
+    use std::str::FromStr;
+
+    use serde::{de, Deserialize, Deserializer, Serializer};
+
+    pub fn serialize<T, S>(value: &T, serializer: S) -> Result<S::Ok, S::Error>
+    where
+        T: Display,
+        S: Serializer,
+    {
+        serializer.collect_str(value)
+    }
+
+    pub fn deserialize<'de, T, D>(deserializer: D) -> Result<T, D::Error>
+    where
+        T: FromStr,
+        T::Err: Display,
+        D: Deserializer<'de>,
+    {
+        String::deserialize(deserializer)?
+            .parse()
+            .map_err(de::Error::custom)
+    }
+}
diff --git a/node/src/storage.rs b/node/src/storage.rs
index 4b78ec4c..b0eef3d7 100644
--- a/node/src/storage.rs
+++ b/node/src/storage.rs
@@ -92,24 +92,11 @@ impl<V> IntoIterator for Remotes<V> {
     }
 }
 
-#[allow(clippy::from_over_into)]
-impl Into<HashMap<String, Remote<Unverified>>> for Remotes<Unverified> {
-    fn into(self) -> HashMap<String, Remote<Unverified>> {
+impl<V> From<Remotes<V>> for HashMap<RemoteId, Refs> {
+    fn from(other: Remotes<V>) -> Self {
         let mut remotes = HashMap::with_hasher(fastrand::Rng::new().into());
 
-        for (k, v) in self.0 {
-            remotes.insert(k.to_string(), v);
-        }
-        remotes
-    }
-}
-
-#[allow(clippy::from_over_into)]
-impl<V> Into<HashMap<RemoteId, Refs>> for Remotes<V> {
-    fn into(self) -> HashMap<RemoteId, Refs> {
-        let mut remotes = HashMap::with_hasher(fastrand::Rng::new().into());
-
-        for (k, v) in self.into_iter() {
+        for (k, v) in other.into_iter() {
             remotes.insert(k, v.refs.into());
         }
         remotes