client-api: Split uiaa::UserIdentifier::ThirdParty

crates/ruma-client-api/CHANGELOG.md

@@ -4,6 +4,7 @@ Breaking changes:
 
 * Remove `PartialEq` implementations for a number of types
   * If the lack of such an `impl` causes problems, please open a GitHub issue
+* Split `uiaa::UserIdentifier::ThirdParty` into two separate variants
 
 Improvements:
 

crates/ruma-client-api/src/session/login.rs

@@ -414,10 +414,7 @@ pub mod v3 {
         #[test]
         #[cfg(feature = "client")]
         fn serialize_login_request_body() {
-            use ruma_common::{
+            use ruma_common::api::{MatrixVersion, OutgoingRequest, SendAccessToken};
-                api::{MatrixVersion, OutgoingRequest, SendAccessToken},
-                thirdparty::Medium,
-            };
             use serde_json::Value as JsonValue;
 
             use super::{LoginInfo, Password, Request, Token};
@@ -449,10 +446,7 @@ pub mod v3 {
 
             let req: http::Request<Vec<u8>> = Request {
                 login_info: LoginInfo::Password(Password {
-                    identifier: UserIdentifier::ThirdPartyId {
+                    identifier: UserIdentifier::Email { address: "hello@example.com" },
-                        address: "hello@example.com",
-                        medium: Medium::Email,
-                    },
                     password: "deadbeef",
                 }),
                 device_id: None,

crates/ruma-client-api/src/uiaa.rs

@@ -562,38 +562,67 @@ pub struct IncomingCustomAuthData {
 }
 
 /// Identification information for the user.
-#[derive(Clone, Debug, PartialEq, Eq, Incoming, Serialize)]
+#[derive(Clone, Debug, PartialEq, Eq, Incoming)]
-#[serde(from = "user_serde::IncomingUserIdentifier", into = "user_serde::UserIdentifier<'_>")]
+#[incoming_derive(!Deserialize)]
 #[allow(clippy::exhaustive_enums)]
 pub enum UserIdentifier<'a> {
     /// Either a fully qualified Matrix user ID, or just the localpart (as part of the 'identifier'
     /// field).
     UserIdOrLocalpart(&'a str),
 
-    /// Third party identifier (as part of the 'identifier' field).
+    /// An email address.
-    ThirdPartyId {
+    Email {
-        /// Third party identifier for the user.
+        /// The email address.
         address: &'a str,
-
-        /// The medium of the identifier.
-        medium: Medium,
     },
 
-    /// Same as third-party identification with medium == msisdn, but with a non-canonicalised
+    /// A phone number in the MSISDN format.
-    /// phone number.
+    Msisdn {
+        /// The phone number according to the [E.164] numbering plan.
+        ///
+        /// [E.164]: https://www.itu.int/rec/T-REC-E.164-201011-I/en
+        number: &'a str,
+    },
+
+    /// A phone number as a separate country code and phone number.
+    ///
+    /// The homeserver will be responsible for canonicalizing this to the MSISDN format.
     PhoneNumber {
         /// The country that the phone number is from.
+        ///
+        /// This is a two-letter uppercase [ISO-3166-1 alpha-2] country code.
+        ///
+        /// [ISO-3166-1 alpha-2]: https://www.iso.org/iso-3166-country-codes.html
         country: &'a str,
 
         /// The phone number.
         phone: &'a str,
     },
+
+    #[doc(hidden)]
+    _CustomThirdParty(CustomThirdPartyId<'a>),
 }
 
 impl<'a> UserIdentifier<'a> {
-    /// Creates a [`UserIdentifier::ThirdPartyId`] from an email address.
+    /// Creates a new `UserIdentifier` from the given third party identifier.
-    pub fn email(address: &'a str) -> Self {
+    pub fn third_party_id(medium: &'a Medium, address: &'a str) -> Self {
-        Self::ThirdPartyId { address, medium: Medium::Email }
+        match medium {
+            Medium::Email => Self::Email { address },
+            Medium::Msisdn => Self::Msisdn { number: address },
+            _ => Self::_CustomThirdParty(CustomThirdPartyId { medium, address }),
+        }
+    }
+
+    /// Get this `UserIdentifier` as a third party identifier if it is one.
+    pub fn as_third_party_id(&self) -> Option<(&'a Medium, &'a str)> {
+        match self {
+            Self::Email { address } => Some((&Medium::Email, address)),
+            Self::Msisdn { number } => Some((&Medium::Msisdn, number)),
+            Self::_CustomThirdParty(CustomThirdPartyId { medium, address }) => {
+                Some((medium, address))
+            }
+            _ => None,
+        }
     }
 }
 
@@ -613,14 +642,33 @@ impl IncomingUserIdentifier {
     pub(crate) fn to_outgoing(&self) -> UserIdentifier<'_> {
         match self {
             Self::UserIdOrLocalpart(id) => UserIdentifier::UserIdOrLocalpart(id),
-            Self::ThirdPartyId { address, medium } => {
+            Self::Email { address } => UserIdentifier::Email { address },
-                UserIdentifier::ThirdPartyId { address, medium: medium.clone() }
+            Self::Msisdn { number } => UserIdentifier::Msisdn { number },
-            }
             Self::PhoneNumber { country, phone } => UserIdentifier::PhoneNumber { country, phone },
+            Self::_CustomThirdParty(id) => UserIdentifier::_CustomThirdParty(CustomThirdPartyId {
+                medium: &id.medium,
+                address: &id.address,
+            }),
         }
     }
 }
 
+#[doc(hidden)]
+#[derive(Clone, Debug, PartialEq, Eq, Serialize)]
+#[non_exhaustive]
+pub struct CustomThirdPartyId<'a> {
+    medium: &'a Medium,
+    address: &'a str,
+}
+
+#[doc(hidden)]
+#[derive(Clone, Debug, PartialEq, Eq, Deserialize)]
+#[non_exhaustive]
+pub struct IncomingCustomThirdPartyId {
+    medium: Medium,
+    address: String,
+}
+
 /// Credentials for third-party authentication (e.g. email / phone number).
 #[derive(Clone, Debug, Deserialize, Serialize)]
 #[cfg_attr(not(feature = "unstable-exhaustive-types"), non_exhaustive)]

crates/ruma-client-api/src/uiaa/user_serde.rs

@@ -1,46 +1,219 @@
-//! Helper module for the Serialize / Deserialize impl's for the User struct
+//! Helper module for the Serialize / Deserialize impl's for the UserIdentifier struct
 //! in the parent module.
 
-use ruma_common::{serde::Incoming, thirdparty::Medium};
+use ruma_common::{serde::from_raw_json_value, thirdparty::Medium};
-use serde::Serialize;
+use serde::{de, ser::SerializeStruct, Deserialize, Deserializer, Serialize};
+use serde_json::value::RawValue as RawJsonValue;
 
-// The following structs could just be used in place of the one in the parent module, but
+use super::{
-// that one is arguably much easier to deal with.
+    CustomThirdPartyId, IncomingCustomThirdPartyId, IncomingUserIdentifier, UserIdentifier,
-#[derive(Clone, Debug, PartialEq, Eq, Incoming, Serialize)]
+};
-#[serde(tag = "type")]
+
-pub(crate) enum UserIdentifier<'a> {
+impl<'a> Serialize for UserIdentifier<'a> {
+    fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
+    where
+        S: serde::Serializer,
+    {
+        let mut id;
+        match self {
+            Self::UserIdOrLocalpart(user) => {
+                id = serializer.serialize_struct("UserIdentifier", 2)?;
+                id.serialize_field("type", "m.id.user")?;
+                id.serialize_field("user", user)?;
+            }
+            Self::PhoneNumber { country, phone } => {
+                id = serializer.serialize_struct("UserIdentifier", 3)?;
+                id.serialize_field("type", "m.id.phone")?;
+                id.serialize_field("country", country)?;
+                id.serialize_field("phone", phone)?;
+            }
+            Self::Email { address } => {
+                id = serializer.serialize_struct("UserIdentifier", 3)?;
+                id.serialize_field("type", "m.id.thirdparty")?;
+                id.serialize_field("medium", &Medium::Email)?;
+                id.serialize_field("address", address)?;
+            }
+            Self::Msisdn { number } => {
+                id = serializer.serialize_struct("UserIdentifier", 3)?;
+                id.serialize_field("type", "m.id.thirdparty")?;
+                id.serialize_field("medium", &Medium::Msisdn)?;
+                id.serialize_field("address", number)?;
+            }
+            Self::_CustomThirdParty(CustomThirdPartyId { medium, address }) => {
+                id = serializer.serialize_struct("UserIdentifier", 3)?;
+                id.serialize_field("type", "m.id.thirdparty")?;
+                id.serialize_field("medium", &medium)?;
+                id.serialize_field("address", address)?;
+            }
+        }
+        id.end()
+    }
+}
+
+impl<'de> Deserialize<'de> for IncomingUserIdentifier {
+    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
+    where
+        D: Deserializer<'de>,
+    {
+        let json = Box::<RawJsonValue>::deserialize(deserializer)?;
+
+        #[derive(Deserialize)]
+        #[serde(tag = "type")]
+        enum ExtractType {
             #[serde(rename = "m.id.user")]
-    UserIdOrLocalpart { user: &'a str },
+            User,
-    #[serde(rename = "m.id.thirdparty")]
-    ThirdPartyId { medium: Medium, address: &'a str },
             #[serde(rename = "m.id.phone")]
-    PhoneNumber { country: &'a str, phone: &'a str },
+            Phone,
-}
+            #[serde(rename = "m.id.thirdparty")]
+            ThirdParty,
+        }
 
-impl<'a> From<super::UserIdentifier<'a>> for UserIdentifier<'a> {
+        #[derive(Deserialize)]
-    fn from(id: super::UserIdentifier<'a>) -> Self {
+        struct UserIdOrLocalpart {
-        use UserIdentifier as SerdeId;
+            user: String,
+        }
 
-        use super::UserIdentifier as SuperId;
+        #[derive(Deserialize)]
+        struct ThirdPartyId {
+            medium: Medium,
+            address: String,
+        }
 
-        match id {
+        #[derive(Deserialize)]
-            SuperId::UserIdOrLocalpart(user) => SerdeId::UserIdOrLocalpart { user },
+        struct PhoneNumber {
-            SuperId::ThirdPartyId { address, medium } => SerdeId::ThirdPartyId { address, medium },
+            country: String,
-            SuperId::PhoneNumber { country, phone } => SerdeId::PhoneNumber { country, phone },
+            phone: String,
+        }
+
+        let id_type = serde_json::from_str::<ExtractType>(json.get()).map_err(de::Error::custom)?;
+
+        match id_type {
+            ExtractType::User => from_raw_json_value(&json)
+                .map(|user_id: UserIdOrLocalpart| Self::UserIdOrLocalpart(user_id.user)),
+            ExtractType::Phone => from_raw_json_value(&json)
+                .map(|nb: PhoneNumber| Self::PhoneNumber { country: nb.country, phone: nb.phone }),
+            ExtractType::ThirdParty => {
+                let ThirdPartyId { medium, address } = from_raw_json_value(&json)?;
+                match medium {
+                    Medium::Email => Ok(Self::Email { address }),
+                    Medium::Msisdn => Ok(Self::Msisdn { number: address }),
+                    _ => {
+                        Ok(Self::_CustomThirdParty(IncomingCustomThirdPartyId { medium, address }))
+                    }
+                }
+            }
         }
     }
 }
 
-impl From<IncomingUserIdentifier> for super::IncomingUserIdentifier {
+#[cfg(test)]
-    fn from(id: IncomingUserIdentifier) -> super::IncomingUserIdentifier {
+mod tests {
-        use IncomingUserIdentifier as SerdeId;
+    use crate::uiaa::{IncomingUserIdentifier, UserIdentifier};
+    use assert_matches::assert_matches;
+    use serde_json::{from_value as from_json_value, json, to_value as to_json_value};
 
-        use super::IncomingUserIdentifier as SuperId;
+    #[test]
+    fn serialize() {
+        assert_eq!(
+            to_json_value(UserIdentifier::UserIdOrLocalpart("@user:notareal.hs")).unwrap(),
+            json!({
+                "type": "m.id.user",
+                "user": "@user:notareal.hs",
+            })
+        );
 
-        match id {
+        assert_eq!(
-            SerdeId::UserIdOrLocalpart { user } => SuperId::UserIdOrLocalpart(user),
+            to_json_value(UserIdentifier::PhoneNumber { country: "33", phone: "0102030405" })
-            SerdeId::ThirdPartyId { address, medium } => SuperId::ThirdPartyId { address, medium },
+                .unwrap(),
-            SerdeId::PhoneNumber { country, phone } => SuperId::PhoneNumber { country, phone },
+            json!({
+                "type": "m.id.phone",
+                "country": "33",
+                "phone": "0102030405",
+            })
+        );
+
+        assert_eq!(
+            to_json_value(UserIdentifier::Email { address: "me@myprovider.net" }).unwrap(),
+            json!({
+                "type": "m.id.thirdparty",
+                "medium": "email",
+                "address": "me@myprovider.net",
+            })
+        );
+
+        assert_eq!(
+            to_json_value(UserIdentifier::Msisdn { number: "330102030405" }).unwrap(),
+            json!({
+                "type": "m.id.thirdparty",
+                "medium": "msisdn",
+                "address": "330102030405",
+            })
+        );
+
+        assert_eq!(
+            to_json_value(UserIdentifier::third_party_id(&"robot".into(), "01001110")).unwrap(),
+            json!({
+                "type": "m.id.thirdparty",
+                "medium": "robot",
+                "address": "01001110",
+            })
+        );
     }
+
+    #[test]
+    fn deserialize() {
+        let json = json!({
+            "type": "m.id.user",
+            "user": "@user:notareal.hs",
+        });
+        let user = assert_matches!(
+            from_json_value(json),
+            Ok(IncomingUserIdentifier::UserIdOrLocalpart(user)) => user
+        );
+        assert_eq!(user, "@user:notareal.hs");
+
+        let json = json!({
+            "type": "m.id.phone",
+            "country": "33",
+            "phone": "0102030405",
+        });
+        let (country, phone) = assert_matches!(
+            from_json_value(json),
+            Ok(IncomingUserIdentifier::PhoneNumber { country, phone }) => (country, phone)
+        );
+        assert_eq!(country, "33");
+        assert_eq!(phone, "0102030405");
+
+        let json = json!({
+            "type": "m.id.thirdparty",
+            "medium": "email",
+            "address": "me@myprovider.net",
+        });
+        let address = assert_matches!(
+            from_json_value(json),
+            Ok(IncomingUserIdentifier::Email { address }) => address
+        );
+        assert_eq!(address, "me@myprovider.net");
+
+        let json = json!({
+            "type": "m.id.thirdparty",
+            "medium": "msisdn",
+            "address": "330102030405",
+        });
+        let number = assert_matches!(
+            from_json_value(json),
+            Ok(IncomingUserIdentifier::Msisdn { number }) => number
+        );
+        assert_eq!(number, "330102030405");
+
+        let json = json!({
+            "type": "m.id.thirdparty",
+            "medium": "robot",
+            "address": "01110010",
+        });
+        let id = from_json_value::<IncomingUserIdentifier>(json).unwrap();
+        let (medium, address) = id.to_outgoing().as_third_party_id().unwrap();
+        assert_eq!(medium.as_str(), "robot");
+        assert_eq!(address, "01110010");
     }
 }