Ensure client metadata hashing is stable

This is done by using the indexmap crate to preserve insertion order for
localized fields.

Author: sandhose

Cargo.lock

@@ -184,6 +184,11 @@ version = "0.27.5"
 features = ["http1", "http2"]
 default-features = false
 
+# HashMap which preserves insertion order
+[workspace.dependencies.indexmap]
+version = "2.8.0"
+features = ["serde"]
+
 # Snapshot testing
 [workspace.dependencies.insta]
 version = "1.42.2"

Cargo.toml

@@ -73,7 +73,7 @@ camino.workspace = true
 chrono.workspace = true
 elliptic-curve.workspace = true
 governor.workspace = true
-indexmap = "2.8.0"
+indexmap.workspace = true
 pkcs8.workspace = true
 psl = "2.1.96"
 time = "0.3.41"

crates/handlers/Cargo.toml

@@ -21,9 +21,11 @@ serde_with = { version = "3.12.0", features = ["chrono"] }
 chrono.workspace = true
 sha2 = "0.10.8"
 thiserror.workspace = true
+indexmap.workspace = true
 
 mas-iana.workspace = true
 mas-jose.workspace = true
 
 [dev-dependencies]
 assert_matches = "1.5.0"
+insta.workspace = true

crates/oauth2-types/Cargo.toml

@@ -4,9 +4,10 @@
 // SPDX-License-Identifier: AGPL-3.0-only
 // Please see LICENSE in the repository root for full details.
 
-use std::{borrow::Cow, collections::HashMap};
+use std::borrow::Cow;
 
 use chrono::Duration;
+use indexmap::IndexMap;
 use language_tags::LanguageTag;
 use mas_iana::{
     jose::{JsonWebEncryptionAlg, JsonWebEncryptionEnc, JsonWebSignatureAlg},
@@ -45,18 +46,18 @@ impl<T> Localized<T> {
     }
 
     fn deserialize(
-        map: &mut HashMap<String, HashMap<Option<LanguageTag>, Value>>,
+        map: &mut IndexMap<String, IndexMap<Option<LanguageTag>, Value>>,
         field_name: &'static str,
     ) -> Result<Option<Self>, serde_json::Error>
     where
         T: DeserializeOwned,
     {
-        let Some(map) = map.remove(field_name) else {
+        let Some(map) = map.shift_remove(field_name) else {
             return Ok(None);
         };
 
         let mut non_localized = None;
-        let mut localized = HashMap::with_capacity(map.len() - 1);
+        let mut localized = IndexMap::with_capacity(map.len() - 1);
 
         for (k, v) in map {
             let value = serde_json::from_value(v)?;
@@ -347,8 +348,8 @@ impl<'de> Deserialize<'de> for ClientMetadataLocalizedFields {
     where
         D: serde::Deserializer<'de>,
     {
-        let map = HashMap::<Cow<'de, str>, Value>::deserialize(deserializer)?;
-        let mut new_map: HashMap<String, HashMap<Option<LanguageTag>, Value>> = HashMap::new();
+        let map = IndexMap::<Cow<'de, str>, Value>::deserialize(deserializer)?;
+        let mut new_map: IndexMap<String, IndexMap<Option<LanguageTag>, Value>> = IndexMap::new();
 
         for (k, v) in map {
             let (prefix, lang) = if let Some((prefix, lang)) = k.split_once('#') {
@@ -389,6 +390,8 @@ impl<'de> Deserialize<'de> for ClientMetadataLocalizedFields {
 
 #[cfg(test)]
 mod tests {
+    use insta::assert_yaml_snapshot;
+
     use super::*;
 
     #[test]
@@ -461,16 +464,28 @@ mod tests {
         .validate()
         .unwrap();
 
-        assert_eq!(
-            serde_json::to_value(metadata).unwrap(),
-            serde_json::json!({
-                "redirect_uris": ["http://localhost/oidc"],
-                "client_name": "Postbox",
-                "client_name#fr": "Boîte à lettres",
-                "client_uri": "https://localhost/",
-                "client_uri#fr": "https://localhost/fr",
-                "client_uri#de": "https://localhost/de",
-            })
-        );
+        assert_yaml_snapshot!(metadata, @r###"
+        redirect_uris:
+          - "http://localhost/oidc"
+        client_name: Postbox
+        "client_name#fr": Boîte à lettres
+        client_uri: "https://localhost/"
+        "client_uri#fr": "https://localhost/fr"
+        "client_uri#de": "https://localhost/de"
+        "###);
+
+        // Do a roundtrip, we should get the same metadata back with the same order
+        let metadata: ClientMetadata =
+            serde_json::from_value(serde_json::to_value(metadata).unwrap()).unwrap();
+        let metadata = metadata.validate().unwrap();
+        assert_yaml_snapshot!(metadata, @r###"
+        redirect_uris:
+          - "http://localhost/oidc"
+        client_name: Postbox
+        "client_name#fr": Boîte à lettres
+        client_uri: "https://localhost/"
+        "client_uri#fr": "https://localhost/fr"
+        "client_uri#de": "https://localhost/de"
+        "###);
     }
 }

crates/oauth2-types/src/registration/client_metadata_serde.rs

@@ -8,9 +8,10 @@
 //!
 //! [Dynamic Client Registration]: https://openid.net/specs/openid-connect-registration-1_0.html
 
-use std::{collections::HashMap, ops::Deref};
+use std::ops::Deref;
 
 use chrono::{DateTime, Duration, Utc};
+use indexmap::IndexMap;
 use language_tags::LanguageTag;
 use mas_iana::{
     jose::{JsonWebEncryptionAlg, JsonWebEncryptionEnc, JsonWebSignatureAlg},
@@ -58,7 +59,7 @@ pub const DEFAULT_ENCRYPTION_ENC_ALGORITHM: &JsonWebEncryptionEnc =
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub struct Localized<T> {
     non_localized: T,
-    localized: HashMap<LanguageTag, T>,
+    localized: IndexMap<LanguageTag, T>,
 }
 
 impl<T> Localized<T> {
@@ -104,8 +105,8 @@ impl<T> Localized<T> {
     }
 }
 
-impl<T> From<(T, HashMap<LanguageTag, T>)> for Localized<T> {
-    fn from(t: (T, HashMap<LanguageTag, T>)) -> Self {
+impl<T> From<(T, IndexMap<LanguageTag, T>)> for Localized<T> {
+    fn from(t: (T, IndexMap<LanguageTag, T>)) -> Self {
         Localized {
             non_localized: t.0,
             localized: t.1,