From 8ae0c59a259e954d0677be3b9902da28b236df6a Mon Sep 17 00:00:00 2001
From: Philippe Loctaux
Date: Thu, 16 Mar 2023 00:05:54 +0100
Subject: [PATCH] ezidam, openid: check scopes, check response types before
getting app
---
Cargo.lock | 1 +
crates/ezidam/src/error.rs | 4 +++
crates/ezidam/src/routes/oauth.rs | 12 ++++++--
crates/openid/Cargo.toml | 1 +
crates/openid/src/lib.rs | 2 +-
crates/openid/src/response_types.rs | 48 +++++++++++++++++++++++++++++
crates/openid/src/scopes.rs | 33 ++++++++++++++++++++
7 files changed, 98 insertions(+), 3 deletions(-)
diff --git a/Cargo.lock b/Cargo.lock
index e3751d8..8677cc6 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -1964,6 +1964,7 @@ version = "0.0.0"
dependencies = [
"itertools",
"openidconnect",
+ "serde",
"serde_json",
"thiserror",
"url",
diff --git a/crates/ezidam/src/error.rs b/crates/ezidam/src/error.rs
index 70e0eb4..96ea2ef 100644
--- a/crates/ezidam/src/error.rs
+++ b/crates/ezidam/src/error.rs
@@ -43,4 +43,8 @@ impl Error {
pub fn not_found(value: M) -> Self {
Self::new(Status::NotFound, value)
}
+
+ pub fn bad_request(value: M) -> Self {
+ Self::new(Status::BadRequest, value)
+ }
}
diff --git a/crates/ezidam/src/routes/oauth.rs b/crates/ezidam/src/routes/oauth.rs
index ee35d6d..1d81552 100644
--- a/crates/ezidam/src/routes/oauth.rs
+++ b/crates/ezidam/src/routes/oauth.rs
@@ -28,7 +28,15 @@ async fn authorize_page(
flash: Option>,
auth_request: AuthenticationRequest<'_>,
) -> Result {
- // TODO: parse "scope" and "response_type" -> from openid local crate
+ // Check for valid response types
+ openid::parse_response_types(auth_request.response_type)
+ .ok_or_else(|| Error::bad_request("Bad response types"))?;
+
+ // Check for supported scopes
+ if !openid::SupportedScopes::check_supported_scopes(auth_request.scope) {
+ return Err(Error::bad_request("Invalid scopes"));
+ }
+
let mut transaction = db.begin().await?;
// TODO: wrap checking in function?
@@ -38,7 +46,7 @@ async fn authorize_page(
auth_request.redirect_uri,
)
.await?
- .ok_or_else(|| Error::not_found(auth_request.client_id))?;
+ .ok_or_else(|| Error::bad_request("Invalid application"))?;
let settings = Settings::get(&mut transaction).await?;
diff --git a/crates/openid/Cargo.toml b/crates/openid/Cargo.toml
index e4e5578..f924df9 100644
--- a/crates/openid/Cargo.toml
+++ b/crates/openid/Cargo.toml
@@ -6,6 +6,7 @@ edition = "2021"
[dependencies]
thiserror = { workspace = true }
url = { workspace = true }
+serde = { workspace = true }
serde_json = { workspace = true }
openidconnect = { version = "3.0.0-alpha.1", default-features = false }
itertools = "0.10.5"
\ No newline at end of file
diff --git a/crates/openid/src/lib.rs b/crates/openid/src/lib.rs
index 8383ca0..4c8d809 100644
--- a/crates/openid/src/lib.rs
+++ b/crates/openid/src/lib.rs
@@ -6,7 +6,7 @@ mod scopes;
/// Exports
pub use configuration::configuration;
pub use error::Error;
-pub use response_types::supported_response_types;
+pub use response_types::{parse_response_types, supported_response_types};
pub use scopes::SupportedScopes;
/// Type exports
diff --git a/crates/openid/src/response_types.rs b/crates/openid/src/response_types.rs
index 9dbc5ba..e2b22c1 100644
--- a/crates/openid/src/response_types.rs
+++ b/crates/openid/src/response_types.rs
@@ -9,3 +9,51 @@ pub fn supported_response_types() -> Vec> {
ResponseTypes::new(vec![CoreResponseType::Token, CoreResponseType::IdToken]), // Other flows including hybrid flows may also be specified here.
]
}
+
+pub fn parse_response_types(raw: &str) -> Option> {
+ if raw.is_empty() {
+ return None;
+ }
+
+ let list = raw
+ .split_whitespace()
+ .flat_map(|s| {
+ // Convert string to json to deserialize with serde_json
+ let value_json = format!("\"{}\"", s);
+ let deserializer = &mut serde_json::Deserializer::from_str(&value_json);
+ // Attempt to deserialize into CoreResponseType
+ serde::Deserialize::deserialize(deserializer).ok()
+ })
+ // If any fails, return None
+ .collect::>();
+
+ // If vec is empty or contains `Extension`, return None
+ if list.is_empty()
+ || list
+ .iter()
+ .any(|variant| matches!(variant, CoreResponseType::Extension(_)))
+ {
+ None
+ } else {
+ Some(list)
+ }
+}
+
+#[cfg(test)]
+mod tests {
+ use super::parse_response_types;
+
+ #[test]
+ fn check_valid() {
+ assert!(parse_response_types("code id_token").is_some());
+ assert!(parse_response_types("code token").is_some());
+ assert!(parse_response_types("code token id_token").is_some());
+ }
+
+ #[test]
+ fn check_invalid() {
+ assert!(parse_response_types("").is_none());
+ assert!(parse_response_types("test").is_none());
+ assert!(parse_response_types("abc def code ghi id_token").is_none());
+ }
+}
diff --git a/crates/openid/src/scopes.rs b/crates/openid/src/scopes.rs
index c35cfaf..6e7062e 100644
--- a/crates/openid/src/scopes.rs
+++ b/crates/openid/src/scopes.rs
@@ -1,5 +1,6 @@
use itertools::Itertools;
use openidconnect::Scope;
+use std::collections::HashSet;
pub struct SupportedScopes(pub Vec);
@@ -7,6 +8,16 @@ impl SupportedScopes {
pub fn url_format() -> String {
Self::default().0.iter().map(|s| s.as_str()).join(" ")
}
+ pub fn check_supported_scopes(scopes: &str) -> bool {
+ if scopes.is_empty() {
+ return false;
+ }
+
+ let list = Self::default();
+ let scope_set: &HashSet<_> = &list.0.iter().map(|s| s.as_str()).collect();
+ let requested_scopes: HashSet<_> = scopes.split_whitespace().collect();
+ requested_scopes.is_subset(scope_set)
+ }
}
impl Default for SupportedScopes {
@@ -18,3 +29,25 @@ impl Default for SupportedScopes {
])
}
}
+
+#[cfg(test)]
+mod tests {
+ use super::SupportedScopes;
+
+ #[test]
+ fn check_valid() {
+ assert!(SupportedScopes::check_supported_scopes("openid"));
+ assert!(SupportedScopes::check_supported_scopes("profile email"));
+ assert!(SupportedScopes::check_supported_scopes("email openid"));
+ }
+
+ #[test]
+ fn check_invalid() {
+ assert!(!SupportedScopes::check_supported_scopes(""));
+ assert!(!SupportedScopes::check_supported_scopes("openid abc"));
+ assert!(!SupportedScopes::check_supported_scopes("test"));
+ assert!(!SupportedScopes::check_supported_scopes(
+ "email testing wrong profile"
+ ));
+ }
+}