From 7f5e967583b6b4ff81e04705d550fefa009e9ca4 Mon Sep 17 00:00:00 2001
From: Sebastiano Tocci <seba.tocci@gmail.com>
Date: Wed, 2 Aug 2023 01:50:57 +0200
Subject: [PATCH] fixed serialization of oauth2 token scope (#1930)

---
 proto/Cargo.toml             |  1 +
 proto/src/oauth2.rs          |  8 ++++++--
 server/lib/src/idm/oauth2.rs | 14 ++++++++++++++
 3 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/proto/Cargo.toml b/proto/Cargo.toml
index ca75ecb28..c63b9fcca 100644
--- a/proto/Cargo.toml
+++ b/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"] }
diff --git a/proto/src/oauth2.rs b/proto/src/oauth2.rs
index 96b4466fb..ceaf4137c 100644
--- a/proto/src/oauth2.rs
+++ b/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>>,
     },
 }
diff --git a/server/lib/src/idm/oauth2.rs b/server/lib/src/idm/oauth2.rs
index 9b102887e..9b1d21a43 100644
--- a/server/lib/src/idm/oauth2.rs
+++ b/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());
+    }
 }