Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions objectstore-server/docs/architecture.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,9 @@ A request flows through several layers before reaching the storage service:
2. **Extractors**: path parameters are parsed into an
[`ObjectId`](objectstore_service::id::ObjectId) or
[`ObjectContext`](objectstore_service::id::ObjectContext). The auth token is
read from the `X-Os-Auth` header (preferred) or the standard
`Authorization` header (fallback), then validated and decoded into an
read from the `X-Os-Auth` header (preferred), the `X-Os-Auth` query
parameter, or the standard `Authorization` header (fallback), then
validated and decoded into an
[`AuthContext`](auth::AuthContext).
The optional `x-downstream-service` header is extracted for killswitch
matching.
Expand Down
5 changes: 3 additions & 2 deletions objectstore-server/src/auth/key_directory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,9 @@ impl TryFrom<&AuthZVerificationKey> for PublicKeyConfig {

/// Directory of keys that may be used to verify a request's auth token.
///
/// The auth token is read from the `X-Os-Auth` header (preferred) or the
/// standard `Authorization` header (fallback). This directory contains a map keyed
/// The auth token is read from the `X-Os-Auth` header (preferred), the
/// `X-Os-Auth` query parameter, or the standard `Authorization` header
/// (fallback). This directory contains a map keyed
/// on a key's ID. When verifying a JWT, the `kid` field should be read from the
/// JWT header and used to index into this directory to select the appropriate key.
#[derive(Debug)]
Expand Down
8 changes: 5 additions & 3 deletions objectstore-server/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -343,8 +343,9 @@ pub struct AuthZ {

/// Keys that may be used to verify a request's auth token.
///
/// The auth token is read from the `X-Os-Auth` header (preferred)
/// or the standard `Authorization` header (fallback). This field is a
/// The auth token is read from the `X-Os-Auth` header (preferred),
/// the `X-Os-Auth` query parameter, or the standard `Authorization`
/// header (fallback). This field is a
/// container keyed on a key's ID. When verifying a JWT, the `kid` field
/// should be read from the JWT header and used to index into this map to
/// select the appropriate key.
Expand Down Expand Up @@ -445,7 +446,8 @@ pub struct Config {
/// Content-based authorization configuration.
///
/// Controls the verification and enforcement of content-based access control based on the
/// JWT in a request's `X-Os-Auth` or `Authorization` header.
/// JWT in a request's `X-Os-Auth` header, `X-Os-Auth` query parameter, or `Authorization`
/// header.
pub auth: AuthZ,

/// A list of matchers for requests to discard without processing.
Expand Down
58 changes: 56 additions & 2 deletions objectstore-server/src/extractors/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,19 +13,32 @@ const BEARER_PREFIX: &str = "Bearer ";
/// this header.
const OBJECTSTORE_AUTH_HEADER: &str = "x-os-auth";

/// Query parameter name for Objectstore authentication. Used as a fallback when
/// headers cannot be set (e.g. browser-initiated requests). The value should be
/// the raw JWT without a `Bearer` prefix.
const OBJECTSTORE_AUTH_QUERY_PARAM: &str = "X-Os-Auth";

impl FromRequestParts<ServiceState> for AuthAwareService {
type Rejection = ApiError;

async fn from_request_parts(
parts: &mut Parts,
state: &ServiceState,
) -> Result<Self, Self::Rejection> {
// Precedence: X-Os-Auth header > X-Os-Auth query param > Authorization header.
let encoded_token = parts
.headers
.get(OBJECTSTORE_AUTH_HEADER)
.or_else(|| parts.headers.get(header::AUTHORIZATION))
.and_then(|v| v.to_str().ok())
.and_then(strip_bearer);
.and_then(strip_bearer)
.or_else(|| extract_query_param_value(parts.uri.query(), OBJECTSTORE_AUTH_QUERY_PARAM))
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Auth token in query string leaks to logs/traces

Medium Severity

Passing the JWT via the X-Os-Auth query parameter means the token will appear in tracing spans and Sentry transactions. The existing make_http_span function records uri = %request.uri() which includes the full query string, and SentryHttpLayer with enable_transaction() also captures request URIs. Unlike headers, which are not included in these spans, query-parameter tokens will be persisted in logs and Sentry. The codebase already redacts sensitive values elsewhere (e.g., DecodingKey debug output, config secrets), but no scrubbing exists for URIs.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 5316ff6. Configure here.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is okay? These tokens have a max validity anyways.

.or_else(|| {
parts
.headers
.get(header::AUTHORIZATION)
.and_then(|v| v.to_str().ok())
.and_then(strip_bearer)
});

let enforce = state.config.auth.enforce;
// Attempt to decode / verify the JWT, logging failure
Expand Down Expand Up @@ -57,6 +70,13 @@ fn strip_bearer(header_value: &str) -> Option<&str> {
}
}

fn extract_query_param_value<'a>(query: Option<&'a str>, param: &str) -> Option<&'a str> {
query?.split('&').find_map(|pair| {
let (key, value) = pair.split_once('=')?;
(key == param && !value.is_empty()).then_some(value)
})
}

#[cfg(test)]
mod tests {
use super::*;
Expand All @@ -75,4 +95,38 @@ mod tests {
// No character boundary at end of expected prefix
assert_eq!(strip_bearer("Bearer⚠️tokenvalue"), None);
}

#[test]
fn test_extract_query_param_value() {
assert_eq!(extract_query_param_value(None, "X-Os-Auth"), None);
assert_eq!(extract_query_param_value(Some(""), "X-Os-Auth"), None);
assert_eq!(
extract_query_param_value(Some("other=val"), "X-Os-Auth"),
None
);

assert_eq!(
extract_query_param_value(Some("X-Os-Auth=mytoken"), "X-Os-Auth"),
Some("mytoken")
);
assert_eq!(
extract_query_param_value(Some("foo=bar&X-Os-Auth=mytoken"), "X-Os-Auth"),
Some("mytoken")
);
assert_eq!(
extract_query_param_value(Some("X-Os-Auth=mytoken&foo=bar"), "X-Os-Auth"),
Some("mytoken")
);

// Empty value
assert_eq!(
extract_query_param_value(Some("X-Os-Auth="), "X-Os-Auth"),
None
);
// No `=` sign
assert_eq!(
extract_query_param_value(Some("X-Os-Auth"), "X-Os-Auth"),
None
);
}
}
Loading