Merge pull request #7301 from influxdata/auth-header

fix: Put authorization header in request extension
pull/24376/head
Marko Mikulicic 2023-03-22 04:27:09 +01:00 committed by GitHub
commit 8518ab502b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 96 additions and 7 deletions

11
Cargo.lock generated
View File

@ -3161,6 +3161,7 @@ dependencies = [
"serde",
"serde_json",
"serde_urlencoded",
"server_util",
"service_grpc_testing",
"snafu",
"tokio",
@ -4947,6 +4948,7 @@ dependencies = [
"iox_catalog",
"iox_tests",
"iox_time",
"ioxd_common",
"metric",
"mutable_batch",
"mutable_batch_lp",
@ -4963,6 +4965,7 @@ dependencies = [
"serde",
"serde_json",
"serde_urlencoded",
"server_util",
"service_grpc_catalog",
"service_grpc_namespace",
"service_grpc_object_store",
@ -5212,6 +5215,14 @@ dependencies = [
"serde",
]
[[package]]
name = "server_util"
version = "0.1.0"
dependencies = [
"http",
"workspace-hack",
]
[[package]]
name = "service_common"
version = "0.1.0"

View File

@ -64,6 +64,7 @@ members = [
"query_functions",
"router",
"schema",
"server_util",
"service_common",
"service_grpc_influxrpc",
"service_grpc_flight",

View File

@ -20,6 +20,7 @@ observability_deps = { path = "../observability_deps" }
# (honestly I thought that cargo dependencies were isolated on a per crate basis so I'm a bit surprised that pprof accidentally builds
# successfully just because another crate happens to depend on backtrace-rs)
pprof = { version = "0.11", default-features = false, features = ["flamegraph", "prost-codec"], optional = true }
server_util = { path = "../server_util" }
service_grpc_testing = { path = "../service_grpc_testing" }
trace = { path = "../trace" }
trace_exporters = { path = "../trace_exporters" }

View File

@ -7,6 +7,7 @@ use hyper::{
};
use observability_deps::tracing::{debug, error};
use serde::Deserialize;
use server_util::authorization::AuthorizationHeaderExtension;
use snafu::Snafu;
use tokio_util::sync::CancellationToken;
use tower::Layer;
@ -116,8 +117,12 @@ async fn route_request(
server_type: Arc<dyn ServerType>,
mut req: Request<Body>,
) -> Result<Response<Body>, Infallible> {
// we don't need the authorization header and we don't want to accidentally log it.
req.headers_mut().remove("authorization");
let auth = { req.headers().get(hyper::header::AUTHORIZATION).cloned() };
req.extensions_mut()
.insert(AuthorizationHeaderExtension::new(auth));
// we don't need the authorization header anymore and we don't want to accidentally log it.
req.headers_mut().remove(hyper::header::AUTHORIZATION);
debug!(request = ?req,"Processing request");
let method = req.method().clone();

View File

@ -19,6 +19,7 @@ hashbrown = { workspace = true }
hyper = "0.14"
iox_catalog = { path = "../iox_catalog" }
iox_time = { path = "../iox_time" }
ioxd_common = { path = "../ioxd_common" }
metric = { path = "../metric" }
mutable_batch = { path = "../mutable_batch" }
mutable_batch_lp = { path = "../mutable_batch_lp" }
@ -31,6 +32,7 @@ schema = { version = "0.1.0", path = "../schema" }
serde = "1.0"
serde_json = "1.0.94"
serde_urlencoded = "0.7"
server_util = { path = "../server_util" }
service_grpc_catalog = { path = "../service_grpc_catalog"}
service_grpc_namespace = { path = "../service_grpc_namespace"}
service_grpc_schema = { path = "../service_grpc_schema" }

View File

@ -15,6 +15,7 @@ use mutable_batch_lp::LinesConverter;
use observability_deps::tracing::*;
use predicate::delete_predicate::parse_delete_predicate;
use serde::Deserialize;
use server_util::authorization::AuthorizationHeaderExtension;
use std::{str::Utf8Error, sync::Arc, time::Instant};
use thiserror::Error;
use tokio::sync::{Semaphore, TryAcquireError};
@ -415,8 +416,9 @@ where
if let Some(authz) = &self.authz {
let token = req
.headers()
.get(hyper::header::AUTHORIZATION)
.extensions()
.get::<AuthorizationHeaderExtension>()
.and_then(|v| v.as_ref())
.and_then(|v| {
let s = v.as_ref();
if s.len() < b"Token ".len() {
@ -1439,7 +1441,9 @@ mod tests {
let request = Request::builder()
.uri("https://bananas.example/api/v2/write?org=bananas&bucket=test")
.method("POST")
.header("Authorization", "Token GOOD")
.extension(AuthorizationHeaderExtension::new(Some(
HeaderValue::from_str("Token GOOD").expect("ok"),
)))
.body(Body::from("platanos,tag1=A,tag2=B val=42i 123456"))
.unwrap();
@ -1449,7 +1453,9 @@ mod tests {
let request = Request::builder()
.uri("https://bananas.example/api/v2/write?org=bananas&bucket=test")
.method("POST")
.header("Authorization", "Token BAD")
.extension(AuthorizationHeaderExtension::new(Some(
HeaderValue::from_str("Token BAD").expect("ok"),
)))
.body(Body::from(""))
.unwrap();
@ -1468,7 +1474,9 @@ mod tests {
let request = Request::builder()
.uri("https://bananas.example/api/v2/write?org=bananas&bucket=test")
.method("POST")
.header("Authorization", "Token UGLY")
.extension(AuthorizationHeaderExtension::new(Some(
HeaderValue::from_str("Token UGLY").expect("ok"),
)))
.body(Body::from(""))
.unwrap();

12
server_util/Cargo.toml Normal file
View File

@ -0,0 +1,12 @@
[package]
name = "server_util"
description = "Shared code for IOx servers"
version.workspace = true
authors.workspace = true
edition.workspace = true
license.workspace = true
[dependencies]
http = "0.2.9"
workspace-hack = { version = "0.1", path = "../workspace-hack" }

View File

@ -0,0 +1,29 @@
//! Common authorization helpers
use http::HeaderValue;
/// We strip off the "authorization" header from the request, to prevent it from being accidentally logged
/// and we put it in an extension of the request. Extensions are typed and this is the typed wrapper that
/// holds an (optional) authorization header value.
pub struct AuthorizationHeaderExtension(Option<HeaderValue>);
impl AuthorizationHeaderExtension {
/// Construct new extension wrapper for a possible header value
pub fn new(header: Option<HeaderValue>) -> Self {
Self(header)
}
}
impl std::fmt::Debug for AuthorizationHeaderExtension {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
f.write_str("AuthorizationHeaderExtension(...)")
}
}
impl std::ops::Deref for AuthorizationHeaderExtension {
type Target = Option<HeaderValue>;
fn deref(&self) -> &Self::Target {
&self.0
}
}

20
server_util/src/lib.rs Normal file
View File

@ -0,0 +1,20 @@
//! Shared InfluxDB IOx API client functionality
#![deny(
rustdoc::broken_intra_doc_links,
rustdoc::bare_urls,
rust_2018_idioms,
missing_debug_implementations,
unreachable_pub
)]
#![warn(
missing_docs,
clippy::todo,
clippy::dbg_macro,
clippy::clone_on_ref_ptr,
clippy::future_not_send,
clippy::todo,
clippy::dbg_macro
)]
#![allow(clippy::missing_docs_in_private_items)]
pub mod authorization;