Overhaul deserialization of claims and client metadata

Previously, there were two issues with the deserialization of claims and
dynamic client registration metadata:
  1. Explicit `null` JSON values provided for optional fields resulted in
     deserialization errors. The spec states that identity providers
     "SHOULD NOT" pass `null` values for optional fields (see
     https://openid.net/specs/openid-connect-core-1_0.html#UserInfoResponse).
     Rather, those fields should be omitted altogether. However, it
     makes sense for this crate to deserialize both representations as
     `None`.
  2. When deserialization errors occurred while deserializing specific
     fields, the error message did not include the name of the field.
     This crate uses `serde_path_to_error` to provide helpful error
     messages, but the custom deserializer used to support localized
     claims has no way to inform `serde_path_to_error` about the key
     of the JSON map field being deserialized. Instead, this change
     explicitly prefixes deserialization errors with the key to
     provide equivalent diagnostic information to the user.

Fixes #184.

src/claims.rs

@@ -1,4 +1,4 @@
-use crate::helpers::{timestamp_to_utc, utc_to_seconds, Boolean, FlattenFilter, Timestamp};
+use crate::helpers::{Boolean, DeserializeMapField, FlattenFilter, Timestamp};
 use crate::types::localized::split_language_tag_key;
 use crate::{
     AddressCountry, AddressLocality, AddressPostalCode, AddressRegion, EndUserBirthday,
@@ -214,31 +214,128 @@ where
             where
                 V: MapAccess<'de>,
             {
-                deserialize_fields! {
-                    map {
-                        [sub]
-                        [LanguageTag(name)]
-                        [LanguageTag(given_name)]
-                        [LanguageTag(family_name)]
-                        [LanguageTag(middle_name)]
-                        [LanguageTag(nickname)]
-                        [Option(preferred_username)]
-                        [LanguageTag(profile)]
-                        [LanguageTag(picture)]
-                        [LanguageTag(website)]
-                        [Option(email)]
-                        [Option(Boolean(email_verified))]
-                        [Option(gender)]
-                        [Option(birthday)]
-                        [Option(birthdate)]
-                        [Option(zoneinfo)]
-                        [Option(locale)]
-                        [Option(phone_number)]
-                        [Option(Boolean(phone_number_verified))]
-                        [Option(address)]
-                        [Option(DateTime(Seconds(updated_at)))]
-                    }
+                // NB: The non-localized fields are actually Option<Option<_>> here so that we can
+                // distinguish between omitted fields and fields explicitly set to `null`. The
+                // latter is necessary so that we can detect duplicate fields (e.g., if a key is
+                // present both with a null value and a non-null value, that's an error).
+                let mut sub = None;
+                let mut name = None;
+                let mut given_name = None;
+                let mut family_name = None;
+                let mut middle_name = None;
+                let mut nickname = None;
+                let mut preferred_username = None;
+                let mut profile = None;
+                let mut picture = None;
+                let mut website = None;
+                let mut email = None;
+                let mut email_verified = None;
+                let mut gender = None;
+                let mut birthday = None;
+                let mut birthdate = None;
+                let mut zoneinfo = None;
+                let mut locale = None;
+                let mut phone_number = None;
+                let mut phone_number_verified = None;
+                let mut address = None;
+                let mut updated_at = None;
+
+                macro_rules! field_case {
+                    ($field:ident, $typ:ty, $language_tag:ident) => {{
+                        $field = Some(<$typ>::deserialize_map_field(
+                            &mut map,
+                            stringify!($field),
+                            $language_tag,
+                            $field,
+                        )?);
+                    }};
+                }
+
+                while let Some(key) = map.next_key::<String>()? {
+                    let (field_name, language_tag) = split_language_tag_key(&key);
+                    match field_name {
+                        "sub" => field_case!(sub, SubjectIdentifier, language_tag),
+                        "name" => field_case!(name, LocalizedClaim<Option<_>>, language_tag),
+                        "given_name" => {
+                            field_case!(given_name, LocalizedClaim<Option<_>>, language_tag)
+                        }
+                        "family_name" => {
+                            field_case!(family_name, LocalizedClaim<Option<_>>, language_tag)
+                        }
+                        "middle_name" => {
+                            field_case!(middle_name, LocalizedClaim<Option<_>>, language_tag)
+                        }
+                        "nickname" => {
+                            field_case!(nickname, LocalizedClaim<Option<_>>, language_tag)
+                        }
+                        "preferred_username" => {
+                            field_case!(preferred_username, Option<_>, language_tag)
+                        }
+                        "profile" => {
+                            field_case!(profile, LocalizedClaim<Option<_>>, language_tag)
+                        }
+                        "picture" => {
+                            field_case!(picture, LocalizedClaim<Option<_>>, language_tag)
+                        }
+                        "website" => {
+                            field_case!(website, LocalizedClaim<Option<_>>, language_tag)
+                        }
+                        "email" => field_case!(email, Option<_>, language_tag),
+                        "email_verified" => {
+                            field_case!(email_verified, Option<Boolean>, language_tag)
+                        }
+                        "gender" => field_case!(gender, Option<_>, language_tag),
+                        "birthday" => field_case!(birthday, Option<_>, language_tag),
+                        "birthdate" => field_case!(birthdate, Option<_>, language_tag),
+                        "zoneinfo" => field_case!(zoneinfo, Option<_>, language_tag),
+                        "locale" => field_case!(locale, Option<_>, language_tag),
+                        "phone_number" => field_case!(phone_number, Option<_>, language_tag),
+                        "phone_number_verified" => {
+                            field_case!(phone_number_verified, Option<Boolean>, language_tag)
+                        }
+                        "address" => field_case!(address, Option<_>, language_tag),
+                        "updated_at" => field_case!(updated_at, Option<Timestamp>, language_tag),
+                        // Ignore unknown fields.
+                        _ => {
+                            map.next_value::<serde::de::IgnoredAny>()?;
+                            continue;
+                        }
+                    };
                 }
+
+                Ok(StandardClaims {
+                    sub: sub.ok_or_else(|| serde::de::Error::missing_field("sub"))?,
+                    name: name.and_then(LocalizedClaim::flatten_or_none),
+                    given_name: given_name.and_then(LocalizedClaim::flatten_or_none),
+                    family_name: family_name.and_then(LocalizedClaim::flatten_or_none),
+                    middle_name: middle_name.and_then(LocalizedClaim::flatten_or_none),
+                    nickname: nickname.and_then(LocalizedClaim::flatten_or_none),
+                    preferred_username: preferred_username.flatten(),
+                    profile: profile.and_then(LocalizedClaim::flatten_or_none),
+                    picture: picture.and_then(LocalizedClaim::flatten_or_none),
+                    website: website.and_then(LocalizedClaim::flatten_or_none),
+                    email: email.flatten(),
+                    email_verified: email_verified.flatten().map(Boolean::into_inner),
+                    gender: gender.flatten(),
+                    birthday: birthday.flatten(),
+                    birthdate: birthdate.flatten(),
+                    zoneinfo: zoneinfo.flatten(),
+                    locale: locale.flatten(),
+                    phone_number: phone_number.flatten(),
+                    phone_number_verified: phone_number_verified.flatten().map(Boolean::into_inner),
+                    address: address.flatten(),
+                    updated_at: updated_at
+                        .flatten()
+                        .map(|sec| {
+                            sec.to_utc().map_err(|_| {
+                                serde::de::Error::custom(format!(
+                                    "failed to parse `{sec}` as UTC datetime (in seconds) for key \
+                                     `updated_at`"
+                                ))
+                            })
+                        })
+                        .transpose()?,
+                })
             }
         }
         deserializer.deserialize_map(ClaimsVisitor(PhantomData))
@@ -280,3 +377,106 @@ where
         }
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use crate::core::CoreGenderClaim;
+    use crate::StandardClaims;
+
+    // The spec states (https://openid.net/specs/openid-connect-core-1_0.html#UserInfoResponse):
+    //   "If a Claim is not returned, that Claim Name SHOULD be omitted from the JSON object
+    //   representing the Claims; it SHOULD NOT be present with a null or empty string value."
+    // However, we still aim to support identity providers that disregard this suggestion.
+    #[test]
+    fn test_null_optional_claims() {
+        let claims = serde_json::from_str::<StandardClaims<CoreGenderClaim>>(
+            r#"{
+                "sub": "24400320",
+                "name": null,
+                "given_name": null,
+                "family_name": null,
+                "middle_name": null,
+                "nickname": null,
+                "preferred_username": null,
+                "profile": null,
+                "picture": null,
+                "website": null,
+                "email": null,
+                "email_verified": null,
+                "gender": null,
+                "birthday": null,
+                "birthdate": null,
+                "zoneinfo": null,
+                "locale": null,
+                "phone_number": null,
+                "phone_number_verified": null,
+                "address": null,
+                "updated_at": null
+            }"#,
+        )
+        .expect("should deserialize successfully");
+
+        assert_eq!(claims.subject().as_str(), "24400320");
+        assert_eq!(claims.name(), None);
+    }
+
+    fn expect_err_prefix(
+        result: Result<StandardClaims<CoreGenderClaim>, serde_json::Error>,
+        expected_prefix: &str,
+    ) {
+        let err_str = result.expect_err("deserialization should fail").to_string();
+        assert!(
+            err_str.starts_with(expected_prefix),
+            "error message should begin with `{}`: {}",
+            expected_prefix,
+            err_str,
+        )
+    }
+
+    #[test]
+    fn test_duplicate_claims() {
+        expect_err_prefix(
+            serde_json::from_str(
+                r#"{
+                    "sub": "24400320",
+                    "sub": "24400321"
+                }"#,
+            ),
+            "duplicate field `sub` at line",
+        );
+
+        expect_err_prefix(
+            serde_json::from_str(
+                r#"{
+                    "name": null,
+                    "sub": "24400320",
+                    "name": "foo",
+                }"#,
+            ),
+            "duplicate field `name` at line",
+        );
+
+        expect_err_prefix(
+            serde_json::from_str(
+                r#"{
+                    "name#en": null,
+                    "sub": "24400320",
+                    "name#en": "foo",
+                }"#,
+            ),
+            "duplicate field `name#en` at line",
+        );
+    }
+
+    #[test]
+    fn test_err_field_name() {
+        expect_err_prefix(
+            serde_json::from_str(
+                r#"{
+                    "sub": 24400320
+                }"#,
+            ),
+            "sub: invalid type: integer `24400320`, expected a string at line",
+        );
+    }
+}