Skip to content

Commit

Permalink
fix: Do not simply use bearer token from cookie for auth (#326)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
spencewenski authored Aug 12, 2024
1 parent 9b054a6 commit ff48b19
Show file tree
Hide file tree
Showing 4 changed files with 5 additions and 54 deletions.
4 changes: 4 additions & 0 deletions src/config/auth/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
43 changes: 1 addition & 42 deletions src/middleware/http/auth/jwt/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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
Expand All @@ -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,
Expand Down Expand Up @@ -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;
Expand All @@ -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() {
Expand Down

This file was deleted.

This file was deleted.

0 comments on commit ff48b19

Please sign in to comment.