Skip to content

Commit

Permalink
auth: guard the /auth/key endpoint (#1487)
Browse files Browse the repository at this point in the history
* auth: /auth/key requires an admin secret...

...to convert basic API keys into JWTs

* auth: guard auth/key endpoint by admin key

* gateway: authorize /auth/key requests with an admin secret

* circleci: add gateway's convert key to jwt secret env variable

* gateway: simplify secret naming

* auth: simplify tests

* tests(auth): simplify get jwt from API key helper

* tests(auth): rename parameter

* tests(auth): add a missing parameter

* auth: simplify returining statement
  • Loading branch information
iulianbarbu committed Dec 21, 2023
1 parent fa0fce6 commit bc81eca
Show file tree
Hide file tree
Showing 13 changed files with 133 additions and 46 deletions.
6 changes: 6 additions & 0 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -430,6 +430,9 @@ jobs:
control-db-postgres-uri:
description: "Control database URI, used by the control plane components"
type: string
gateway-admin-key:
description: "Admin API key that authorizes gateway requests to auth service, for key to jwt conversion."
type: string
steps:
- checkout
- run:
Expand Down Expand Up @@ -459,6 +462,7 @@ jobs:
STRIPE_SECRET_KEY=${<< parameters.stripe-secret-key >>} \
AUTH_JWTSIGNING_PRIVATE_KEY=${<< parameters.jwt-signing-private-key >>} \
CONTROL_DB_POSTGRES_URI=${<< parameters.control-db-postgres-uri >>} \
GATEWAY_ADMIN_KEY=${<< parameters.gateway-admin-key >>} \
make deploy
- when:
condition:
Expand Down Expand Up @@ -854,6 +858,7 @@ workflows:
stripe-secret-key: DEV_STRIPE_SECRET_KEY
jwt-signing-private-key: DEV_AUTH_JWTSIGNING_PRIVATE_KEY
control-db-postgres-uri: DEV_CONTROL_DB_POSTGRES_URI
gateway-admin-key: DEV_GATEWAY_ADMIN_KEY
requires:
- build-and-push-unstable
- approve-deploy-images-unstable
Expand Down Expand Up @@ -935,6 +940,7 @@ workflows:
stripe-secret-key: PROD_STRIPE_SECRET_KEY
jwt-signing-private-key: PROD_AUTH_JWTSIGNING_PRIVATE_KEY
control-db-postgres-uri: PROD_CONTROL_DB_POSTGRES_URI
gateway-admin-key: PROD_GATEWAY_ADMIN_KEY
ssh-fingerprint: 6a:c5:33:fe:5b:c9:06:df:99:64:ca:17:0d:32:18:2e
ssh-config-script: production-ssh-config.sh
ssh-host: shuttle.prod.internal
Expand Down
2 changes: 2 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ CARGO_PROFILE?=debug
RUST_LOG?=nbuild_core=warn,shuttle=debug,info
DEV_SUFFIX=-dev
DEPLOYS_API_KEY?=gateway4deployes
GATEWAY_ADMIN_KEY?=dh9z58jttoes3qvt

# this should use the same version as our prod RDS database
CONTROL_DB_POSTGRES_TAG?=15
Expand Down Expand Up @@ -137,6 +138,7 @@ DOCKER_COMPOSE_ENV=\
MONGO_INITDB_ROOT_PASSWORD=$(MONGO_INITDB_ROOT_PASSWORD)\
STRIPE_SECRET_KEY=$(STRIPE_SECRET_KEY)\
AUTH_JWTSIGNING_PRIVATE_KEY=$(AUTH_JWTSIGNING_PRIVATE_KEY)\
GATEWAY_ADMIN_KEY=$(GATEWAY_ADMIN_KEY)\
DD_ENV=$(DD_ENV)\
USE_TLS=$(USE_TLS)\
COMPOSE_PROFILES=$(COMPOSE_PROFILES)\
Expand Down
1 change: 1 addition & 0 deletions auth/src/api/handlers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@ pub(crate) async fn convert_cookie(

/// Convert a valid API-key bearer token to a JWT.
pub(crate) async fn convert_key(
_: Admin,
State(RouterState {
key_manager,
user_manager,
Expand Down
29 changes: 23 additions & 6 deletions auth/src/user.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,12 @@ use std::{fmt::Formatter, io::ErrorKind, str::FromStr};
use async_trait::async_trait;
use axum::{
extract::{FromRef, FromRequestParts},
headers::{authorization::Bearer, Authorization},
headers::{authorization::Bearer, Authorization, HeaderMapExt},
http::request::Parts,
TypedHeader,
};
use serde::{Deserialize, Deserializer, Serialize};
use shuttle_common::{claims::AccountTier, secrets::Secret, ApiKey};
use shuttle_common::{backends::headers::XShuttleAdminSecret, claims::AccountTier, ApiKey, Secret};
use sqlx::{postgres::PgRow, query, FromRow, PgPool, Row};
use tracing::{debug, error, trace, Span};

Expand Down Expand Up @@ -387,11 +387,28 @@ where

async fn from_request_parts(parts: &mut Parts, state: &S) -> Result<Self, Self::Rejection> {
let user = User::from_request_parts(parts, state).await?;

if user.is_admin() {
Ok(Self { user })
} else {
Err(Error::Forbidden)
return Ok(Self { user });
}

match parts.headers.typed_try_get::<XShuttleAdminSecret>() {
Ok(Some(secret)) => {
let user_manager = UserManagerState::from_ref(state);
// For this particular case, we expect the secret to be an admin API key.
let key = ApiKey::parse(&secret.0).map_err(|_| Error::Unauthorized)?;
let admin_user = user_manager
.get_user_by_key(key)
.await
.map_err(|_| Error::Unauthorized)?;
if admin_user.is_admin() {
Ok(Self { user: admin_user })
} else {
Err(Error::Unauthorized)
}
}
Ok(_) => Err(Error::Unauthorized),
// Returning forbidden for the cases where we don't understand why we can not authorize.
Err(_) => Err(Error::Forbidden),
}
}
}
Expand Down
53 changes: 35 additions & 18 deletions auth/tests/api/auth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,58 +11,75 @@ mod needs_docker {
async fn convert_api_key_to_jwt() {
let app = app().await;

// Create test user
let response = app.post_user("test-user", "basic").await;

// Create test basic user
let response = app.post_user("test-user-basic", "basic").await;
assert_eq!(response.status(), StatusCode::OK);

// Extract the API key from the response so we can use it in a future request.
let body = hyper::body::to_bytes(response.into_body()).await.unwrap();
let user: Value = serde_json::from_slice(&body).unwrap();
let api_key = user["key"].as_str().unwrap();
let basic_user_key = user["key"].as_str().unwrap();

// GET /auth/key without bearer token.
let request = Request::builder()
.uri("/auth/key")
.body(Body::empty())
.unwrap();

let response = app.send_request(request).await;

assert_eq!(response.status(), StatusCode::UNAUTHORIZED);

// GET /auth/key with invalid bearer token.
// GET /auth/key with basic tier user API key.
let request = Request::builder()
.uri("/auth/key")
.header(AUTHORIZATION, "Bearer ndh9z58jttoefake")
.header(AUTHORIZATION, format!("Bearer {basic_user_key}"))
.body(Body::empty())
.unwrap();

let response = app.send_request(request).await;

assert_eq!(response.status(), StatusCode::UNAUTHORIZED);

// GET /auth/key with valid bearer token.
let response = app.get_jwt_from_api_key(api_key).await;
// GET /auth/key with an admin user key.
let response = app.get_jwt_from_api_key(ADMIN_KEY, None).await;
assert_eq!(response.status(), StatusCode::OK);

// Decode the JWT into a Claim.
let claim = app.claim_from_response(response).await;

// Verify the claim subject and tier matches the test user we created at the start of the test.
assert_eq!(claim.sub, "test-user");
assert_eq!(claim.tier, AccountTier::Basic);
assert_eq!(claim.sub, "admin");
assert_eq!(claim.tier, AccountTier::Admin);
assert_eq!(claim.limits.project_limit(), 3);

// GET /auth/key with an admin user bearer token.
let response = app.get_jwt_from_api_key(ADMIN_KEY).await;
// GET /auth/key with a basic user key that has an XShuttleAdminSecret header with a basic user key.
let response = app
.get_jwt_from_api_key(basic_user_key, Some(basic_user_key))
.await;
assert_eq!(response.status(), StatusCode::UNAUTHORIZED);

// GET /auth/key with an admin user key that has an XShuttleAdminSecret header with a basic user key.
let response = app
.get_jwt_from_api_key(ADMIN_KEY, Some(basic_user_key))
.await;
assert_eq!(response.status(), StatusCode::OK);

// Decode the JWT into a Claim.
let claim = app.claim_from_response(response).await;

// Verify the claim subject and tier matches the admin user.
// Verify the claim subject and tier matches the test user we created at the start of the test.
assert_eq!(claim.sub, "admin");
assert_eq!(claim.tier, AccountTier::Admin);
assert_eq!(claim.limits.project_limit(), 3);

// GET /auth/key with a basic user key that has an XShuttleAdminSecret header with an admin user key.
let response = app
.get_jwt_from_api_key(basic_user_key, Some(ADMIN_KEY))
.await;
assert_eq!(response.status(), StatusCode::OK);

// Decode the JWT into a Claim.
let claim = app.claim_from_response(response).await;

// Verify the claim subject and tier matches the admin user.
assert_eq!(claim.sub, "test-user-basic");
assert_eq!(claim.tier, AccountTier::Basic);
assert_eq!(claim.limits.project_limit(), 3);
}
}
25 changes: 19 additions & 6 deletions auth/tests/api/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,10 @@ use hyper::{
use once_cell::sync::Lazy;
use serde_json::Value;
use shuttle_auth::{pgpool_init, ApiBuilder};
use shuttle_common::claims::{AccountTier, Claim};
use shuttle_common::{
backends::headers::X_SHUTTLE_ADMIN_SECRET,
claims::{AccountTier, Claim},
};
use shuttle_common_tests::postgres::DockerInstance;
use sqlx::query;
use std::{
Expand Down Expand Up @@ -108,12 +111,22 @@ impl TestApp {
self.send_request(request).await
}

pub async fn get_jwt_from_api_key(&self, api_key: &str) -> Response {
let request = Request::builder()
/// If we don't provide a valid admin key, then the`user_api_key` parameter
/// should be of an admin user.
pub async fn get_jwt_from_api_key(
&self,
user_api_key: &str,
admin_api_key: Option<&str>,
) -> Response {
let mut request_builder = Request::builder()
.uri("/auth/key")
.header(AUTHORIZATION, format!("Bearer {api_key}"))
.body(Body::empty())
.unwrap();
.header(AUTHORIZATION, format!("Bearer {user_api_key}"));

if let Some(key) = admin_api_key {
request_builder = request_builder.header(X_SHUTTLE_ADMIN_SECRET.to_string(), key)
}

let request = request_builder.body(Body::empty()).unwrap();
self.send_request(request).await
}

Expand Down
2 changes: 1 addition & 1 deletion common/src/backends/headers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use http::HeaderValue;

pub static X_SHUTTLE_ADMIN_SECRET: HeaderName = HeaderName::from_static("x-shuttle-admin-secret");

/// Typed header for sending admin secrets to deployers
/// Typed header for sending admin secrets to Shuttle components
pub struct XShuttleAdminSecret(pub String);

impl Header for XShuttleAdminSecret {
Expand Down
1 change: 1 addition & 0 deletions docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,7 @@ services:
- "--builder-host=builder"
- "--proxy-fqdn=${APPS_FQDN}"
- "--use-tls=${USE_TLS}"
- "--admin-key=${GATEWAY_ADMIN_KEY}"
healthcheck:
test: ["CMD", "curl", "-f", "http://localhost:8001"]
interval: 1m
Expand Down
41 changes: 33 additions & 8 deletions gateway/src/api/auth_layer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@ use hyper_reverse_proxy::ReverseProxy;
use once_cell::sync::Lazy;
use opentelemetry::global;
use opentelemetry_http::HeaderInjector;
use shuttle_common::backends::{auth::ConvertResponse, cache::CacheManagement};
use shuttle_common::backends::{
auth::ConvertResponse, cache::CacheManagement, headers::XShuttleAdminSecret,
};
use tower::{Layer, Service};
use tracing::{error, trace, Span};
use tracing_opentelemetry::OpenTelemetrySpanExt;
Expand All @@ -36,16 +38,19 @@ const CACHE_MINUTES: u64 = 5;
#[derive(Clone)]
pub struct ShuttleAuthLayer {
auth_uri: Uri,
gateway_admin_key: String,
cache_manager: Arc<Box<dyn CacheManagement<Value = String>>>,
}

impl ShuttleAuthLayer {
pub fn new(
auth_uri: Uri,
gateway_admin_key: String,
cache_manager: Arc<Box<dyn CacheManagement<Value = String>>>,
) -> Self {
Self {
auth_uri,
gateway_admin_key,
cache_manager,
}
}
Expand All @@ -57,6 +62,7 @@ impl<S> Layer<S> for ShuttleAuthLayer {
fn layer(&self, inner: S) -> Self::Service {
ShuttleAuthService {
inner,
gateway_admin_key: self.gateway_admin_key.clone(),
auth_uri: self.auth_uri.clone(),
cache_manager: self.cache_manager.clone(),
}
Expand All @@ -67,6 +73,7 @@ impl<S> Layer<S> for ShuttleAuthLayer {
pub struct ShuttleAuthService<S> {
inner: S,
auth_uri: Uri,
gateway_admin_key: String,
cache_manager: Arc<Box<dyn CacheManagement<Value = String>>>,
}

Expand Down Expand Up @@ -117,7 +124,9 @@ where

// If /users/reset-api-key is called, invalidate the cached JWT.
if req.uri().path() == "/users/reset-api-key" {
if let Some((cache_key, _)) = cache_key_and_token_req(&req) {
if let Some((cache_key, _)) =
cache_key_and_token_req(&req, self.gateway_admin_key.as_str())
{
self.cache_manager.invalidate(&cache_key);
};
}
Expand Down Expand Up @@ -172,7 +181,9 @@ where

Box::pin(async move {
// Only if there is something to upgrade
if let Some((cache_key, token_request)) = cache_key_and_token_req(&req) {
if let Some((cache_key, token_request)) =
cache_key_and_token_req(&req, this.gateway_admin_key.as_str())
{
let target_url = this.auth_uri.to_string();

// Check if the token is cached.
Expand Down Expand Up @@ -268,34 +279,48 @@ where
}
}

fn cache_key_and_token_req(req: &Request<Body>) -> Option<(String, Request<Body>)> {
fn cache_key_and_token_req(
req: &Request<Body>,
gateway_admin_key: &str,
) -> Option<(String, Request<Body>)> {
req.headers()
.typed_get::<Authorization<Bearer>>()
.map(|bearer| {
let cache_key = bearer.token().trim().to_string();
let token_request = make_token_request("/auth/key", bearer);
let token_request = make_token_request("/auth/key", bearer, Some(gateway_admin_key));
(cache_key, token_request)
})
.or_else(|| {
req.headers().typed_get::<Cookie>().and_then(|cookie| {
cookie.get("shuttle.sid").map(|id| {
let cache_key = id.to_string();
let token_request = make_token_request("/auth/session", cookie.clone());
let token_request = make_token_request("/auth/session", cookie.clone(), None);
(cache_key, token_request)
})
})
})
}

fn make_token_request(uri: &str, header: impl Header) -> Request<Body> {
fn make_token_request(
uri: &str,
header: impl Header,
gateway_admin_key: Option<&str>,
) -> Request<Body> {
let mut token_request = Request::builder().uri(uri);
token_request
.headers_mut()
.expect("manual request to be valid")
.typed_insert(header);

let cx = Span::current().context();
if let Some(key) = gateway_admin_key {
trace!("XShuttleAdminSecret header inserted for token request");
token_request
.headers_mut()
.expect("manual request to be valid")
.typed_insert(XShuttleAdminSecret(key.to_string()));
}

let cx = Span::current().context();
global::get_text_map_propagator(|propagator| {
propagator.inject_context(
&cx,
Expand Down
Loading

0 comments on commit bc81eca

Please sign in to comment.