From 2ffbf436f4c230dbe2f8c25e7168449c5c490fad Mon Sep 17 00:00:00 2001 From: Aumetra Weisman Date: Wed, 15 Jan 2025 22:22:52 +0100 Subject: [PATCH] initial success tests --- Cargo.lock | 11 +++ Cargo.toml | 1 + kitsune/src/http/handler/oauth/authorize.rs | 13 ++-- lib/komainu/Cargo.toml | 2 + lib/komainu/src/code_grant.rs | 36 ++++++--- lib/komainu/tests/code_grant.rs | 4 - lib/komainu/tests/flows.rs | 76 +++++++++++++++++++ .../tests/snapshots/flows__success-2.snap | 11 +++ .../tests/snapshots/flows__success.snap | 11 +++ 9 files changed, 145 insertions(+), 20 deletions(-) delete mode 100644 lib/komainu/tests/code_grant.rs create mode 100644 lib/komainu/tests/flows.rs create mode 100644 lib/komainu/tests/snapshots/flows__success-2.snap create mode 100644 lib/komainu/tests/snapshots/flows__success.snap diff --git a/Cargo.lock b/Cargo.lock index ef38e5b90..5f543234f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2758,6 +2758,16 @@ version = "0.4.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9171a2ea8a68358193d15dd5d70c1c10a2afc3e7e4c5bc92bc9f025cebd7359c" +[[package]] +name = "http-serde" +version = "2.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0f056c8559e3757392c8d091e796416e4649d8e49e88b8d76df6c002f05027fd" +dependencies = [ + "http", + "serde", +] + [[package]] name = "http-signatures" version = "0.0.1-pre.6" @@ -4061,6 +4071,7 @@ dependencies = [ "http", "http-body", "http-body-util", + "http-serde", "indexmap 2.7.0", "insta", "itertools 0.14.0", diff --git a/Cargo.toml b/Cargo.toml index 563c74b91..51dbed477 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -169,6 +169,7 @@ hickory-resolver = { version = "0.25.0-alpha.4", default-features = false, featu http = "1.2.0" http-body = "1.0.1" http-body-util = "0.1.2" +http-serde = "2.1.1" httpdate = "1.0.3" human-size = { version = "0.4.3", features = ["serde"] } hyper = "1.5.2" diff --git a/kitsune/src/http/handler/oauth/authorize.rs b/kitsune/src/http/handler/oauth/authorize.rs index d36772951..7394a18b2 100644 --- a/kitsune/src/http/handler/oauth/authorize.rs +++ b/kitsune/src/http/handler/oauth/authorize.rs @@ -205,11 +205,14 @@ pub async fn get( .await })?; - let mut scopes = authorizer - .scope() - .iter() - .filter_map(|scope| OAuthScope::from_str(scope).ok()) - .collect::>(); + let mut scopes = if let Some(scope) = authorizer.scope() { + scope + .iter() + .filter_map(|scope| OAuthScope::from_str(scope).ok()) + .collect() + } else { + Vec::new() + }; if scopes.is_empty() { // default to read scope if no scopes are defined diff --git a/lib/komainu/Cargo.toml b/lib/komainu/Cargo.toml index 6608e9621..005a278bc 100644 --- a/lib/komainu/Cargo.toml +++ b/lib/komainu/Cargo.toml @@ -21,6 +21,7 @@ http.workspace = true http-body.workspace = true http-body-util.workspace = true indexmap.workspace = true +insta.workspace = true itertools.workspace = true memchr.workspace = true serde.workspace = true @@ -38,6 +39,7 @@ url.workspace = true divan.workspace = true futures-test.workspace = true headers.workspace = true +http-serde.workspace = true insta.workspace = true rand.workspace = true rstest.workspace = true diff --git a/lib/komainu/src/code_grant.rs b/lib/komainu/src/code_grant.rs index 007422e08..b7916efd0 100644 --- a/lib/komainu/src/code_grant.rs +++ b/lib/komainu/src/code_grant.rs @@ -2,7 +2,7 @@ use crate::{ error::Error, flow::pkce, params::ParamStorage, scope::Scope, AuthInstruction, Client, ClientExtractor, }; -use std::{borrow::Cow, future::Future, ops::Deref, str::FromStr}; +use std::{borrow::Cow, future::Future, str::FromStr}; use strum::{AsRefStr, Display}; use thiserror::Error; @@ -63,7 +63,7 @@ where ) -> Result, GrantError> { let client_id = req.query.get("client_id").or_invalid_request()?; let response_type = req.query.get("response_type").or_invalid_request()?; - let scope = req.query.get("scope").map_or("", Deref::deref); + let scope = req.query.get("scope"); let state = req.query.get("state").map(|state| &**state); let client = self.client_extractor.extract(client_id, None).await?; @@ -79,13 +79,27 @@ where return Err(GrantError::AccessDenied); } - let request_scopes = scope.parse().unwrap(); + let request_scopes = if let Some(scope) = scope { + let scope = match Scope::from_str(scope) { + Ok(val) => val, + // Infallible so we have to do this shit to signal to the compiler "hey, this actually can't ever happen". + // Thanks for not stabilizing the never type. + // + // I'm so close to hacking the actual never type in here, I swear. + Err(err) => match err {}, + }; - // Check whether the client can actually perform the grant - if !client.scopes.can_perform(&request_scopes) { - debug!(?client_id, client_scopes = ?client.scopes, ?request_scopes, "client can't issue the requested scopes"); - // apparently clients do that. weird smh. - //return Err(GrantError::AccessDenied); + Some(scope) + } else { + None + }; + + if let Some(ref request_scopes) = request_scopes { + // Check whether the client can actually perform the grant + if !client.scopes.can_perform(request_scopes) { + debug!(?client_id, client_scopes = ?client.scopes, ?request_scopes, "client can't issue the requested scopes"); + return Err(GrantError::AccessDenied); + } } let pkce_payload = if let Some(challenge) = req.query.get("code_challenge") { @@ -144,7 +158,7 @@ pub struct Authorizer<'a, I> { issuer: &'a I, client: Client<'a>, pkce_payload: Option>, - scope: Scope, + scope: Option, query: &'a ParamStorage, Cow<'a, str>>, state: Option<&'a str>, } @@ -159,8 +173,8 @@ where } #[must_use] - pub fn scope(&self) -> &Scope { - &self.scope + pub fn scope(&self) -> Option<&Scope> { + self.scope.as_ref() } #[must_use] diff --git a/lib/komainu/tests/code_grant.rs b/lib/komainu/tests/code_grant.rs deleted file mode 100644 index 127e9744e..000000000 --- a/lib/komainu/tests/code_grant.rs +++ /dev/null @@ -1,4 +0,0 @@ -mod fixtures; - -#[futures_test::test] -async fn success() {} diff --git a/lib/komainu/tests/flows.rs b/lib/komainu/tests/flows.rs new file mode 100644 index 000000000..f871c06c9 --- /dev/null +++ b/lib/komainu/tests/flows.rs @@ -0,0 +1,76 @@ +use self::fixtures::Fixture; +use bytes::Bytes; +use http_body_util::Empty; +use komainu::scope::Scope; +use serde::Serialize; +use std::str::FromStr; + +mod fixtures; + +#[derive(Serialize)] +struct SerdeResponse { + body: Option, + #[serde(with = "http_serde::header_map")] + headers: http::HeaderMap, + #[serde(with = "http_serde::status_code")] + status: http::StatusCode, +} + +impl From> for SerdeResponse { + #[inline] + fn from(value: http::Response<()>) -> Self { + let (parts, _body) = value.into_parts(); + + Self { + body: None, + headers: parts.headers, + status: parts.status, + } + } +} + +impl From> for SerdeResponse { + #[inline] + fn from(value: http::Response) -> Self { + let (parts, body) = value.into_parts(); + let body = String::from_utf8(body.to_vec()).unwrap(); + + let mut response: Self = http::Response::from_parts(parts, ()).into(); + response.body = Some(body); + response + } +} + +#[futures_test::test] +async fn success() { + let fixture = Fixture::generate(); + + let uri = http::Uri::builder() + .scheme("http") + .authority("komainu.example") + .path_and_query("/oauth/authorize?response_type=code&client_id=client_1") + .build() + .unwrap(); + + let req = http::Request::builder() + .uri(uri) + .body(Empty::::new()) + .unwrap(); + let req = komainu::Request::read_from(req).await.unwrap(); + + let acceptor = { + let handle = fixture.code_grant.extract_raw(&req).await.unwrap(); + handle + .accept("user id".into(), &Scope::from_str("read").unwrap()) + .await + .unwrap() + }; + + let deny = { + let handle = fixture.code_grant.extract_raw(&req).await.unwrap(); + handle.deny() + }; + + insta::assert_json_snapshot!(SerdeResponse::from(acceptor.into_response())); + insta::assert_json_snapshot!(SerdeResponse::from(deny)); +} diff --git a/lib/komainu/tests/snapshots/flows__success-2.snap b/lib/komainu/tests/snapshots/flows__success-2.snap new file mode 100644 index 000000000..6ac81973b --- /dev/null +++ b/lib/komainu/tests/snapshots/flows__success-2.snap @@ -0,0 +1,11 @@ +--- +source: lib/komainu/tests/flows.rs +expression: "SerdeResponse::from(deny)" +--- +{ + "body": null, + "headers": { + "location": "http://client_1.example/?error=access_denied" + }, + "status": 302 +} diff --git a/lib/komainu/tests/snapshots/flows__success.snap b/lib/komainu/tests/snapshots/flows__success.snap new file mode 100644 index 000000000..19b68bda7 --- /dev/null +++ b/lib/komainu/tests/snapshots/flows__success.snap @@ -0,0 +1,11 @@ +--- +source: lib/komainu/tests/flows.rs +expression: "SerdeResponse::from(acceptor.into_response())" +--- +{ + "body": null, + "headers": { + "location": "http://client_1.example/?code=sn7A6s3FVFsptwK6" + }, + "status": 302 +}