fixed serialization of oauth2 token scope (#1930)

proto/Cargo.toml

@@ -22,6 +22,7 @@ num_enum = { workspace = true }
 scim_proto = { workspace = true }
 serde = { workspace = true, features = ["derive"] }
 serde_json = { workspace = true }
+serde_with = "3.1.0"
 time = { workspace = true, features = ["serde", "std"] }
 tracing = { workspace = true }
 url = { workspace = true, features = ["serde"] }

proto/src/oauth2.rs

@@ -2,6 +2,8 @@ use std::collections::{BTreeMap, BTreeSet};
 
 use base64urlsafedata::Base64UrlSafeData;
 use serde::{Deserialize, Serialize};
+use serde_with::formats::SpaceSeparator;
+use serde_with::{serde_as, skip_serializing_none, StringWithSeparator};
 use url::Url;
 
 #[derive(Serialize, Deserialize, Debug, PartialEq, Eq, Clone, Copy)]
@@ -77,6 +79,9 @@ pub enum AuthorisationResponse {
     Permitted,
 }
 
+#[serde_as]
+#[skip_serializing_none]
+// this is the equivalent of serde(skip_serializing_if = "Option::is_none") applied to ALL the options
 #[derive(Serialize, Deserialize, Debug)]
 #[serde(tag = "grant_type", rename_all = "snake_case")]
 pub enum GrantTypeReq {
@@ -85,12 +90,11 @@ pub enum GrantTypeReq {
         code: String,
         // Must be the same as the original redirect uri.
         redirect_uri: Url,
-        #[serde(skip_serializing_if = "Option::is_none")]
         code_verifier: Option<String>,
     },
     RefreshToken {
         refresh_token: String,
-        #[serde(skip_serializing_if = "Option::is_none")]
+        #[serde_as(as = "Option<StringWithSeparator::<SpaceSeparator, String>>")]
         scope: Option<BTreeSet<String>>,
     },
 }

server/lib/src/idm/oauth2.rs

@@ -4762,4 +4762,18 @@ mod tests {
 
         // Success!
     }
+    #[test] // I know this looks kinda dumb but at some point someone pointed out that our scope syntax wasn't compliant with rfc6749
+            //(https://datatracker.ietf.org/doc/html/rfc6749#section-3.3), so I'm just making sure that we don't break it again.
+    fn compliant_serialization_test() {
+        let token_req: Result<AccessTokenRequest, serde_json::Error> = serde_json::from_str(
+            r#"
+            {
+                "grant_type": "refresh_token",
+                "refresh_token": "some_dumb_refresh_token",
+                "scope": "invalid_scope vasd asd"
+            }
+        "#,
+        );
+        assert!(token_req.is_ok());
+    }
 }