Skip to content

Commit

Permalink
feat: Allow jwt from cookie, but only if it's explicitly requested (#329
Browse files Browse the repository at this point in the history
)

Getting the JWT from the cookie seems to be acceptable, and is useful
for GET requests (e.g., initial page load when using SSR). However, for
POST requests it should be accompanied with some CSRF protection
mechanism. At the moment, the CSRF validation will need to be performed
by the consuming application.
  • Loading branch information
spencewenski authored Aug 12, 2024
1 parent 89798eb commit b20d890
Show file tree
Hide file tree
Showing 10 changed files with 86 additions and 21 deletions.
32 changes: 17 additions & 15 deletions src/config/auth/mod.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
use crate::util::serde::UriOrString;
use axum::http::header::AUTHORIZATION;
use serde_derive::{Deserialize, Serialize};
use validator::Validate;

Expand All @@ -15,14 +14,16 @@ pub struct Auth {
#[serde(rename_all = "kebab-case")]
#[non_exhaustive]
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,
/// Name of the cookie used to pass the JWT access token. If provided, the default
/// [`Jwt`][crate::middleware::http::auth::jwt::Jwt] will extract the access token from the
/// provided cookie name if it wasn't present in the [`axum::http::header::AUTHORIZATION`]
/// request header. If not provided, the extractor will only consider the request header.
///
/// Warning: Providing this field opens up an application to CSRF vulnerabilities unless the
/// application has the proper protections in place. See the following for more information:
/// - <https://owasp.org/www-community/attacks/csrf>
/// - <https://cheatsheetseries.owasp.org/cheatsheets/Cross-Site_Request_Forgery_Prevention_Cheat_Sheet.html>
pub cookie_name: Option<String>,

pub secret: String,

Expand All @@ -31,12 +32,6 @@ pub struct Jwt {
pub claims: JwtClaims,
}

impl Jwt {
fn default_cookie_name() -> String {
AUTHORIZATION.as_str().to_string()
}
}

#[derive(Debug, Clone, Default, Validate, Serialize, Deserialize)]
#[serde(rename_all = "kebab-case", default)]
#[non_exhaustive]
Expand Down Expand Up @@ -94,6 +89,13 @@ mod tests {
required-claims = ["baz"]
"#
)]
#[case(
r#"
[jwt]
secret = "foo"
cookie-name = "authorization"
"#
)]
#[cfg_attr(coverage_nightly, coverage(off))]
fn auth(_case: TestCase, #[case] config: &str) {
let auth: Auth = toml::from_str(config).unwrap();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ source: src/config/auth/mod.rs
expression: auth
---
[jwt]
cookie-name = 'authorization'
secret = 'foo'

[jwt.claims]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ source: src/config/auth/mod.rs
expression: auth
---
[jwt]
cookie-name = 'authorization'
secret = 'foo'

[jwt.claims]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ source: src/config/auth/mod.rs
expression: auth
---
[jwt]
cookie-name = 'authorization'
secret = 'foo'

[jwt.claims]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ source: src/config/auth/mod.rs
expression: auth
---
[jwt]
cookie-name = 'authorization'
secret = 'foo'

[jwt.claims]
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
---
source: src/config/auth/mod.rs
expression: auth
---
[jwt]
cookie-name = 'authorization'
secret = 'foo'

[jwt.claims]
audience = []
required-claims = []
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,6 @@ timeout = true
max-duration = 60
disable-argument-coercion = false
[auth.jwt]
cookie-name = 'authorization'
secret = 'secret-test'

[auth.jwt.claims]
Expand Down
47 changes: 46 additions & 1 deletion src/middleware/http/auth/jwt/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@ use crate::util::serde::{deserialize_from_str, serialize_to_str};
use async_trait::async_trait;
use axum::extract::{FromRef, FromRequestParts};
use axum::http::request::Parts;
use axum::http::HeaderValue;
use axum::RequestPartsExt;
use axum_extra::extract::CookieJar;
use axum_extra::headers::authorization::Bearer;
use axum_extra::headers::Authorization;
use axum_extra::TypedHeader;
Expand Down Expand Up @@ -59,11 +61,24 @@ where
.await
.ok()
.map(|auth_header| auth_header.0.token().to_string());
let token = if token.is_some() {
token
} else if let Some(cookie_name) = context.config().auth.jwt.cookie_name.as_ref() {
parts
.extract::<CookieJar>()
.await
.ok()
.and_then(|cookies| bearer_token_from_cookies(cookie_name, cookies))
} else {
None
};

let token = if let Some(token) = token {
token
} else {
return Err(HttpError::unauthorized().into());
return Err(HttpError::unauthorized()
.error("Authorization token not found.")
.into());
};

let token: TokenData<C> = decode_auth_token(
Expand All @@ -80,6 +95,20 @@ where
}
}

fn bearer_token_from_cookies(cookie_name: &str, cookies: CookieJar) -> Option<String> {
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 @@ -202,6 +231,8 @@ 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 @@ -213,6 +244,20 @@ 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 cookies = CookieJar::new().add(Cookie::new(
AUTHORIZATION.as_str(),
cookie_value.to_string(),
));

let token = super::bearer_token_from_cookies(AUTHORIZATION.as_str(), cookies);

assert_debug_snapshot!(token);
}

#[test]
#[cfg_attr(coverage_nightly, coverage(off))]
fn deserialize_subject_as_uri() {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
source: src/middleware/http/auth/jwt/mod.rs
expression: token
---
None
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
source: src/middleware/http/auth/jwt/mod.rs
expression: token
---
Some(
"foo",
)

0 comments on commit b20d890

Please sign in to comment.