From ff48b190c0acf1fd881dddb7ea44110bc3f5aa1a Mon Sep 17 00:00:00 2001
From: Spencer Ferris <3319370+spencewenski@users.noreply.github.com>
Date: Sun, 11 Aug 2024 17:53:35 -0700
Subject: [PATCH] fix: Do not simply use bearer token from cookie for auth
 (#326)

This approach is/may be vulnerable to a CSRF attack. More care/research
is needed before taking this approach. For now, it's safer to simply use
the authorization header.
---
 src/config/auth/mod.rs                        |  4 ++
 src/middleware/http/auth/jwt/mod.rs           | 43 +------------------
 ...arer_token_from_cookies@invalid_token.snap |  5 ---
 ...bearer_token_from_cookies@valid_token.snap |  7 ---
 4 files changed, 5 insertions(+), 54 deletions(-)
 delete mode 100644 src/middleware/http/auth/jwt/snapshots/roadster__middleware__http__auth__jwt__tests__bearer_token_from_cookies@invalid_token.snap
 delete mode 100644 src/middleware/http/auth/jwt/snapshots/roadster__middleware__http__auth__jwt__tests__bearer_token_from_cookies@valid_token.snap

diff --git a/src/config/auth/mod.rs b/src/config/auth/mod.rs
index d54ec241..c41d9367 100644
--- a/src/config/auth/mod.rs
+++ b/src/config/auth/mod.rs
@@ -18,6 +18,10 @@ pub struct Jwt {
     /// Name of the cookie used to pass the JWT access token. If not set, will use
     /// [`AUTHORIZATION`] as the cookie name.
     #[serde(default = "Jwt::default_cookie_name")]
+    #[deprecated(
+        since = "0.5.19",
+        note = "Using jwt from cookie is/may be a CSRF vulnerability. This functionality is removed for now and this config field is not used."
+    )]
     pub cookie_name: String,
 
     pub secret: String,
diff --git a/src/middleware/http/auth/jwt/mod.rs b/src/middleware/http/auth/jwt/mod.rs
index 735c3ea6..cfe52d68 100644
--- a/src/middleware/http/auth/jwt/mod.rs
+++ b/src/middleware/http/auth/jwt/mod.rs
@@ -15,9 +15,8 @@ use async_trait::async_trait;
 use axum::extract::{FromRef, FromRequestParts};
 use axum::http::request::Parts;
 use axum::RequestPartsExt;
-use axum_extra::extract::CookieJar;
 use axum_extra::headers::authorization::Bearer;
-use axum_extra::headers::{Authorization, HeaderValue};
+use axum_extra::headers::Authorization;
 use axum_extra::TypedHeader;
 use itertools::Itertools;
 use jsonwebtoken::{decode, DecodingKey, Header, TokenData, Validation};
@@ -60,12 +59,6 @@ where
             .await
             .ok()
             .map(|auth_header| auth_header.0.token().to_string());
-        let token = if token.is_some() {
-            token
-        } else {
-            let cookies = parts.extract::<CookieJar>().await.ok();
-            bearer_token_from_cookies(&context, cookies)
-        };
 
         let token = if let Some(token) = token {
             token
@@ -87,22 +80,6 @@ where
     }
 }
 
-fn bearer_token_from_cookies(context: &AppContext, cookies: Option<CookieJar>) -> Option<String> {
-    let cookie_name = &context.config().auth.jwt.cookie_name;
-    cookies
-        .as_ref()
-        .and_then(|cookies| cookies.get(cookie_name))
-        .map(|cookie| cookie.value())
-        .and_then(|token| HeaderValue::from_str(token).ok())
-        .and_then(|header_value| {
-            <Authorization<Bearer> as axum_extra::headers::Header>::decode(
-                &mut [&header_value].into_iter(),
-            )
-            .ok()
-        })
-        .map(|auth_header| auth_header.token().to_string())
-}
-
 fn decode_auth_token<T1, T2, C>(
     token: &str,
     jwt_secret: &str,
@@ -225,8 +202,6 @@ mod tests {
     use super::*;
     use crate::testing::snapshot::TestCase;
     use crate::util::serde::Wrapper;
-    use axum::http::header::AUTHORIZATION;
-    use axum_extra::extract::cookie::Cookie;
     use insta::assert_debug_snapshot;
     use rstest::{fixture, rstest};
     use serde_json::from_str;
@@ -238,22 +213,6 @@ mod tests {
         Default::default()
     }
 
-    #[rstest]
-    #[case::valid_token("Bearer foo")]
-    #[case::invalid_token("foo")]
-    fn bearer_token_from_cookies(_case: TestCase, #[case] cookie_value: &str) {
-        let context = AppContext::test(None, None, None).unwrap();
-
-        let cookies = CookieJar::new().add(Cookie::new(
-            AUTHORIZATION.to_string(),
-            cookie_value.to_string(),
-        ));
-
-        let token = super::bearer_token_from_cookies(&context, Some(cookies));
-
-        assert_debug_snapshot!(token);
-    }
-
     #[test]
     #[cfg_attr(coverage_nightly, coverage(off))]
     fn deserialize_subject_as_uri() {
diff --git a/src/middleware/http/auth/jwt/snapshots/roadster__middleware__http__auth__jwt__tests__bearer_token_from_cookies@invalid_token.snap b/src/middleware/http/auth/jwt/snapshots/roadster__middleware__http__auth__jwt__tests__bearer_token_from_cookies@invalid_token.snap
deleted file mode 100644
index 947b0212..00000000
--- a/src/middleware/http/auth/jwt/snapshots/roadster__middleware__http__auth__jwt__tests__bearer_token_from_cookies@invalid_token.snap
+++ /dev/null
@@ -1,5 +0,0 @@
----
-source: src/middleware/http/auth/jwt/mod.rs
-expression: token
----
-None
diff --git a/src/middleware/http/auth/jwt/snapshots/roadster__middleware__http__auth__jwt__tests__bearer_token_from_cookies@valid_token.snap b/src/middleware/http/auth/jwt/snapshots/roadster__middleware__http__auth__jwt__tests__bearer_token_from_cookies@valid_token.snap
deleted file mode 100644
index 97bd52a1..00000000
--- a/src/middleware/http/auth/jwt/snapshots/roadster__middleware__http__auth__jwt__tests__bearer_token_from_cookies@valid_token.snap
+++ /dev/null
@@ -1,7 +0,0 @@
----
-source: src/middleware/http/auth/jwt/mod.rs
-expression: token
----
-Some(
-    "foo",
-)