From 2d362bb9f057161b8d969ee1e66ab3473fa0d052 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tommy=20Tr=C3=B8en?= Date: Thu, 14 Nov 2024 11:05:33 +0100 Subject: [PATCH] refactor: add identity_provider to IntrospectRequest * add more tests * change error message for KeyNotInJWKS Co-authored-by: tronghn Co-authored-by: kimtore --- hack/roundtrip-azure-cc.sh | 2 +- hack/roundtrip-azure-obo.sh | 2 +- hack/roundtrip-idporten.sh | 2 +- hack/roundtrip-maskinporten.sh | 2 +- hack/roundtrip-tokenx.sh | 2 +- src/app.rs | 111 ++++++++++++++++++++++++++------- src/claims.rs | 4 +- src/handlers.rs | 10 +-- src/identity_provider.rs | 44 ++++--------- src/jwks.rs | 10 +-- 10 files changed, 116 insertions(+), 73 deletions(-) diff --git a/hack/roundtrip-azure-cc.sh b/hack/roundtrip-azure-cc.sh index eff8176..2bc2adf 100755 --- a/hack/roundtrip-azure-cc.sh +++ b/hack/roundtrip-azure-cc.sh @@ -2,7 +2,7 @@ response=$(curl -s -X POST http://localhost:3000/api/v1/token -H "content-type: application/json" -d '{"target": "client-id", "identity_provider": "azuread"}') token=$(echo ${response} | jq -r .access_token) -validation=$(curl -s -X POST http://localhost:3000/api/v1/introspect -H "content-type: application/json" -d "{\"token\": \"${token}\"}") +validation=$(curl -s -X POST http://localhost:3000/api/v1/introspect -H "content-type: application/json" -d "{\"token\": \"${token}\", \"identity_provider\": \"azuread\"}") echo echo "JWT:" diff --git a/hack/roundtrip-azure-obo.sh b/hack/roundtrip-azure-obo.sh index 90b4eb1..1231298 100755 --- a/hack/roundtrip-azure-obo.sh +++ b/hack/roundtrip-azure-obo.sh @@ -6,7 +6,7 @@ user_token=$(echo ${user_token_response} | jq -r .access_token) response=$(curl -s -X POST http://localhost:3000/api/v1/token/exchange -H "content-type: application/json" -d '{"target": "client-id", "identity_provider": "azuread", "user_token": "'${user_token}'"}') token=$(echo ${response} | jq -r .access_token) -validation=$(curl -s -X POST http://localhost:3000/api/v1/introspect -H "content-type: application/json" -d "{\"token\": \"${token}\"}") +validation=$(curl -s -X POST http://localhost:3000/api/v1/introspect -H "content-type: application/json" -d "{\"token\": \"${token}\", \"identity_provider\": \"azuread\"}") echo echo "User token:" diff --git a/hack/roundtrip-idporten.sh b/hack/roundtrip-idporten.sh index 43d47b3..dd899d0 100755 --- a/hack/roundtrip-idporten.sh +++ b/hack/roundtrip-idporten.sh @@ -2,7 +2,7 @@ user_token_response=$(curl -s -X POST http://localhost:8080/idporten/token -d "grant_type=authorization_code&code=yolo&client_id=yolo&client_secret=bolo") user_token=$(echo ${user_token_response} | jq -r .access_token) -validation=$(curl -s -X POST http://localhost:3000/api/v1/introspect -H "content-type: application/json" -d "{\"token\": \"${user_token}\"}") +validation=$(curl -s -X POST http://localhost:3000/api/v1/introspect -H "content-type: application/json" -d "{\"token\": \"${user_token}\", \"identity_provider\": \"idporten\"}") echo echo "User token:" diff --git a/hack/roundtrip-maskinporten.sh b/hack/roundtrip-maskinporten.sh index 693b8bc..31d8cbd 100755 --- a/hack/roundtrip-maskinporten.sh +++ b/hack/roundtrip-maskinporten.sh @@ -1,7 +1,7 @@ #!/bin/bash -e response=$(curl -s -X POST http://localhost:3000/api/v1/token -H "content-type: application/json" -d '{"target": "my-target", "identity_provider": "maskinporten"}') token=$(echo ${response} | jq -r .access_token) -validation=$(curl -s -X POST http://localhost:3000/api/v1/introspect -H "content-type: application/json" -d "{\"token\": \"${token}\"}") +validation=$(curl -s -X POST http://localhost:3000/api/v1/introspect -H "content-type: application/json" -d "{\"token\": \"${token}\", \"identity_provider\": \"maskinporten\"}") echo echo "JWT:" diff --git a/hack/roundtrip-tokenx.sh b/hack/roundtrip-tokenx.sh index d337929..d6cb471 100755 --- a/hack/roundtrip-tokenx.sh +++ b/hack/roundtrip-tokenx.sh @@ -5,7 +5,7 @@ user_token=$(echo ${user_token_response} | jq -r .access_token) response=$(curl -s -X POST http://localhost:3000/api/v1/token/exchange -H "content-type: application/json" -d '{"target": "client-id", "identity_provider": "tokenx", "user_token": "'${user_token}'"}') token=$(echo ${response} | jq -r .access_token) -validation=$(curl -s -X POST http://localhost:3000/api/v1/introspect -H "content-type: application/json" -d "{\"token\": \"${token}\"}") +validation=$(curl -s -X POST http://localhost:3000/api/v1/introspect -H "content-type: application/json" -d "{\"token\": \"${token}\", \"identity_provider\": \"tokenx\"}") echo echo "User token:" diff --git a/src/app.rs b/src/app.rs index 51d7174..ca74746 100644 --- a/src/app.rs +++ b/src/app.rs @@ -1,18 +1,18 @@ -use std::time::Duration; -use axum::body::Bytes; -use axum::extract::MatchedPath; -use axum::http::{HeaderMap, Request}; -use axum::response::Response; use crate::config::Config; use crate::handlers::__path_introspect; use crate::handlers::__path_token; use crate::handlers::__path_token_exchange; use crate::handlers::{introspect, token, token_exchange, HandlerState}; +use axum::body::Bytes; +use axum::extract::MatchedPath; +use axum::http::{HeaderMap, Request}; +use axum::response::Response; use axum::Router; use log::info; use opentelemetry::propagation::TextMapPropagator; use opentelemetry_http::HeaderExtractor; use opentelemetry_sdk::propagation::TraceContextPropagator; +use std::time::Duration; use tokio::net::TcpListener; use tower_http::classify::ServerErrorsFailureClass; use tower_http::trace::TraceLayer; @@ -207,6 +207,7 @@ mod tests { test_introspect_token_is_not_a_jwt(&address).await; test_introspect_token_missing_issuer(&address).await; test_introspect_token_unrecognized_issuer(&address).await; + test_introspect_token_issuer_mismatch(&address, &identity_provider_address).await; test_introspect_token_missing_kid(&address, &identity_provider_address).await; test_introspect_token_missing_key_in_jwks(&address, &identity_provider_address).await; test_introspect_token_is_expired(&address, &identity_provider_address).await; @@ -246,22 +247,62 @@ mod tests { } async fn test_introspect_token_unrecognized_issuer(address: &str) { - let token = Token::sign(TokenClaims::from([("iss".into(), Value::String("snafu".into()))])); + let token = Token::sign_with_kid( + TokenClaims::from([ + ("iss".into(), Value::String("snafu".into())), + ("nbf".into(), (epoch_now_secs()).into()), + ("iat".into(), (epoch_now_secs()).into()), + ("exp".into(), (epoch_now_secs() + 120).into()), + ]), + &IdentityProvider::Maskinporten.to_string(), + ); test_well_formed_json_request( &format!("http://{}/api/v1/introspect", address), - IntrospectRequest { token }, - IntrospectResponse::new_invalid("unrecognized issuer: 'snafu'"), + IntrospectRequest { + token, + identity_provider: IdentityProvider::Maskinporten, + }, + IntrospectResponse::new_invalid("invalid token: InvalidIssuer"), + StatusCode::OK, + ) + .await; + } + + async fn test_introspect_token_issuer_mismatch(address: &str, identity_provider_address: &str) { + let iss = format!("http://{}/maskinporten", identity_provider_address); + let token = Token::sign_with_kid(TokenClaims::from([ + ("iss".into(), Value::String(iss)), + ("nbf".into(), (epoch_now_secs()).into()), + ("iat".into(), (epoch_now_secs()).into()), + ("exp".into(), (epoch_now_secs() + 120).into()), + ]), &IdentityProvider::Maskinporten.to_string()); + + test_well_formed_json_request( + &format!("http://{}/api/v1/introspect", address), + IntrospectRequest { + token, + identity_provider: IdentityProvider::AzureAD, + }, + IntrospectResponse::new_invalid("token can not be validated with this identity provider"), StatusCode::OK, ) .await; } async fn test_introspect_token_missing_issuer(address: &str) { - let token = Token::sign(TokenClaims::new()); + let token = Token::sign_with_kid(TokenClaims::from([ + ("nbf".into(), (epoch_now_secs()).into()), + ("iat".into(), (epoch_now_secs()).into()), + ("exp".into(), (epoch_now_secs() + 120).into()), + ]), &IdentityProvider::Maskinporten.to_string()); + test_well_formed_json_request( &format!("http://{}/api/v1/introspect", address), - IntrospectRequest { token }, - IntrospectResponse::new_invalid("token is invalid"), + IntrospectRequest { + token, + identity_provider: IdentityProvider::Maskinporten, + }, + IntrospectResponse::new_invalid("invalid token: Missing required claim: iss"), StatusCode::OK, ) .await; @@ -271,9 +312,10 @@ mod tests { test_well_formed_json_request( &format!("http://{}/api/v1/introspect", address), IntrospectRequest { - token: "this is not a token".to_string(), + token: "not a jwt".to_string(), + identity_provider: IdentityProvider::AzureAD, }, - IntrospectResponse::new_invalid("token is invalid"), + IntrospectResponse::new_invalid("invalid token header: InvalidToken"), StatusCode::OK, ) .await; @@ -283,7 +325,10 @@ mod tests { let token = Token::sign(TokenClaims::from([("iss".into(), format!("http://{}/maskinporten", identity_provider_address).into())])); test_well_formed_json_request( &format!("http://{}/api/v1/introspect", address), - IntrospectRequest { token }, + IntrospectRequest { + token, + identity_provider: IdentityProvider::AzureAD, + }, IntrospectResponse::new_invalid("missing key id from token header"), StatusCode::OK, ) @@ -295,8 +340,11 @@ mod tests { test_well_formed_json_request( &format!("http://{}/api/v1/introspect", address), - IntrospectRequest { token }, - IntrospectResponse::new_invalid("signing key with missing-key not in json web key set"), + IntrospectRequest { + token, + identity_provider: IdentityProvider::Maskinporten, + }, + IntrospectResponse::new_invalid("token can not be validated with this identity provider"), StatusCode::OK, ) .await; @@ -316,7 +364,10 @@ mod tests { test_well_formed_json_request( &format!("http://{}/api/v1/introspect", address), - IntrospectRequest { token }, + IntrospectRequest { + token, + identity_provider: IdentityProvider::Maskinporten, + }, IntrospectResponse::new_invalid("invalid token: ExpiredSignature"), StatusCode::OK, ) @@ -336,7 +387,10 @@ mod tests { test_well_formed_json_request( &format!("http://{}/api/v1/introspect", address), - IntrospectRequest { token }, + IntrospectRequest { + token, + identity_provider: IdentityProvider::Maskinporten, + }, IntrospectResponse::new_invalid("invalid token: ImmatureSignature"), StatusCode::OK, ) @@ -356,7 +410,10 @@ mod tests { test_well_formed_json_request( &format!("http://{}/api/v1/introspect", address), - IntrospectRequest { token }, + IntrospectRequest { + token, + identity_provider: IdentityProvider::Maskinporten, + }, IntrospectResponse::new_invalid("invalid token: ImmatureSignature"), StatusCode::OK, ) @@ -383,7 +440,10 @@ mod tests { test_well_formed_json_request( &format!("http://{}/api/v1/introspect", address), - IntrospectRequest { token: body.access_token.clone() }, + IntrospectRequest { + token: body.access_token.clone(), + identity_provider: IdentityProvider::AzureAD, + }, IntrospectResponse::new_invalid("invalid token: InvalidAudience"), StatusCode::OK, ) @@ -470,7 +530,10 @@ mod tests { let response = post_request( format!("http://{}/api/v1/introspect", address.clone().to_string()), - IntrospectRequest { token: body.access_token.clone() }, + IntrospectRequest { + token: body.access_token.clone(), + identity_provider, + }, request_format, ) .await @@ -532,7 +595,10 @@ mod tests { let response = post_request( format!("http://{}/api/v1/introspect", address.clone().to_string()), - IntrospectRequest { token: body.access_token.clone() }, + IntrospectRequest { + token: body.access_token.clone(), + identity_provider, + }, request_format, ) .await @@ -575,6 +641,7 @@ mod tests { format!("http://{}/api/v1/introspect", address.clone().to_string()), IntrospectRequest { token: user_token.access_token.clone(), + identity_provider: IdentityProvider::IDPorten, }, request_format, ) diff --git a/src/claims.rs b/src/claims.rs index 0d91d85..b0a6fbb 100644 --- a/src/claims.rs +++ b/src/claims.rs @@ -64,9 +64,7 @@ impl Assertion for ClientAssertion { } impl Assertion for () { - fn new(_token_endpoint: String, _client_id: String, _target: String) -> Self { - () - } + fn new(_token_endpoint: String, _client_id: String, _target: String) -> Self {} } pub fn epoch_now_secs() -> u64 { diff --git a/src/handlers.rs b/src/handlers.rs index 5956fee..f0eefe5 100644 --- a/src/handlers.rs +++ b/src/handlers.rs @@ -126,7 +126,8 @@ pub async fn token_exchange(State(state): State, JsonOrForm(reques description = "Introspect a token. This means to validate the token and returns its claims. The `active` is not part of the claims, but indicates whether the token is valid.", examples( ("Token introspection" = (value = json!(IntrospectRequest{ - token: "eyJraWQiOiJ0b2tlbngiLCJ0eXAiOiJKV1QiLCJhbGciOiJSUzI1NiJ9.eyJzdWIiOiJlMDE1NTQyYy0wZjgxLTQwZjUtYmJkOS03YzNkOTM2NjI5OGYiLCJhdWQiOiJteS10YXJnZXQiLCJuYmYiOjE3MzA5NzcyOTMsImF6cCI6InlvbG8iLCJpc3MiOiJodHRwOi8vbG9jYWxob3N0OjgwODAvdG9rZW54IiwiZXhwIjoxNzMwOTgwODkzLCJpYXQiOjE3MzA5NzcyOTMsImp0aSI6ImU3Y2JhZGMzLTZiZGEtNDljMC1hMTk2LWM0NzMyOGRhODgwZSIsInRpZCI6InRva2VueCJ9.SIme9o5YE6pZXT9IMAx5upV3V4ww_TnDlqZG203pkySPBd_VqNGBXzOKHeOasIDpXEMlf8Yc-1nKgySjGOT3c46PIHEUrhQFXF6s9OpJAYAwy7L2n2DIFfEOLt8EpwSpM5hWDwnGpSdvebWlmoaA3ImFEB5dtnxLrVG-7dYEEzZjMfBOKFWrPp03FTO4qKOJUqCZR0tmZRmcPzymPWFIMjP2FTj6iz9zai93dhQmdvNVMGL9HBXF6ewKf_CTlUIx9XpwI2M-dhlyH2PIxyhix7Amuff_mHuEHTuCAFqMfjon-F438uyZmgicyrvhoUGxV8W1PfZEiLIv0RBeWRJ9gw".to_string() + token: "eyJraWQiOiJ0b2tlbngiLCJ0eXAiOiJKV1QiLCJhbGciOiJSUzI1NiJ9.eyJzdWIiOiJlMDE1NTQyYy0wZjgxLTQwZjUtYmJkOS03YzNkOTM2NjI5OGYiLCJhdWQiOiJteS10YXJnZXQiLCJuYmYiOjE3MzA5NzcyOTMsImF6cCI6InlvbG8iLCJpc3MiOiJodHRwOi8vbG9jYWxob3N0OjgwODAvdG9rZW54IiwiZXhwIjoxNzMwOTgwODkzLCJpYXQiOjE3MzA5NzcyOTMsImp0aSI6ImU3Y2JhZGMzLTZiZGEtNDljMC1hMTk2LWM0NzMyOGRhODgwZSIsInRpZCI6InRva2VueCJ9.SIme9o5YE6pZXT9IMAx5upV3V4ww_TnDlqZG203pkySPBd_VqNGBXzOKHeOasIDpXEMlf8Yc-1nKgySjGOT3c46PIHEUrhQFXF6s9OpJAYAwy7L2n2DIFfEOLt8EpwSpM5hWDwnGpSdvebWlmoaA3ImFEB5dtnxLrVG-7dYEEzZjMfBOKFWrPp03FTO4qKOJUqCZR0tmZRmcPzymPWFIMjP2FTj6iz9zai93dhQmdvNVMGL9HBXF6ewKf_CTlUIx9XpwI2M-dhlyH2PIxyhix7Amuff_mHuEHTuCAFqMfjon-F438uyZmgicyrvhoUGxV8W1PfZEiLIv0RBeWRJ9gw".to_string(), + identity_provider: IdentityProvider::TokenX, }))) ), ), @@ -194,13 +195,12 @@ where Ok(Arc::new(RwLock::new(Box::new( Provider::::new( kind, - provider_cfg.issuer.clone(), provider_cfg.client_id.clone(), provider_cfg.token_endpoint.clone(), provider_cfg.client_jwk.clone(), jwks::Jwks::new(&provider_cfg.issuer.clone(), &provider_cfg.jwks_uri.clone(), audience).await?, ) - .ok_or(InitError::Jwk)?, + .ok_or(InitError::Jwk)?, )))) } @@ -288,8 +288,8 @@ impl FromRequest for JsonOrForm where S: Send + Sync, T: 'static, - Json: FromRequest, - Form: FromRequest, + Json: FromRequest, + Form: FromRequest, { type Rejection = ApiError; diff --git a/src/identity_provider.rs b/src/identity_provider.rs index 7407a2b..ecf6709 100644 --- a/src/identity_provider.rs +++ b/src/identity_provider.rs @@ -183,7 +183,7 @@ impl From for StatusCode { /// Identity providers for use with token fetch, exchange and introspection. /// /// Each identity provider is enabled when appropriately configured in `nais.yaml`. -#[derive(Deserialize, Serialize, ToSchema, Clone, Debug, PartialEq)] +#[derive(Deserialize, Serialize, ToSchema, Clone, Debug, PartialEq, Copy)] pub enum IdentityProvider { #[serde(rename = "azuread")] AzureAD, @@ -234,6 +234,7 @@ pub struct TokenExchangeRequest { #[derive(Serialize, Deserialize, ToSchema)] pub struct IntrospectRequest { pub token: String, + pub identity_provider: IdentityProvider, } impl IntrospectRequest { @@ -262,7 +263,6 @@ pub struct Provider { client_id: String, pub token_endpoint: Option, identity_provider_kind: IdentityProvider, - issuer: String, private_jwk: Option, client_assertion_header: Option, upstream_jwks: jwks::Jwks, @@ -275,7 +275,7 @@ where R: TokenRequestBuilder, A: Assertion, { - pub fn new(kind: IdentityProvider, issuer: String, client_id: String, token_endpoint: Option, private_jwk: Option, upstream_jwks: jwks::Jwks) -> Option { + pub fn new(kind: IdentityProvider, client_id: String, token_endpoint: Option, private_jwk: Option, upstream_jwks: jwks::Jwks) -> Option { let (client_private_jwk, client_assertion_header) = if let Some(private_jwk) = private_jwk { let client_private_jwk: jwk::JsonWebKey = private_jwk.parse().ok()?; let alg: jwt::Algorithm = client_private_jwk.algorithm?.into(); @@ -294,7 +294,6 @@ where token_endpoint, client_assertion_header, upstream_jwks, - issuer, identity_provider_kind: kind, private_jwk: client_private_jwk, _fake_request: Default::default(), @@ -318,7 +317,7 @@ where async fn get_token(&self, request: TokenRequest) -> Result { let token_request = TokenRequestBuilderParams { target: request.target.clone(), - assertion: self.create_assertion(request.target).ok_or(ApiError::TokenRequestUnsupported(self.identity_provider_kind.clone()))?, + assertion: self.create_assertion(request.target).ok_or(ApiError::TokenRequestUnsupported(self.identity_provider_kind))?, client_id: Some(self.client_id.clone()), user_token: None, }; @@ -328,7 +327,7 @@ where async fn exchange_token(&self, request: TokenExchangeRequest) -> Result { let token_request = TokenRequestBuilderParams { target: request.target.clone(), - assertion: self.create_assertion(request.target).ok_or(ApiError::TokenExchangeUnsupported(self.identity_provider_kind.clone()))?, + assertion: self.create_assertion(request.target).ok_or(ApiError::TokenExchangeUnsupported(self.identity_provider_kind))?, client_id: Some(self.client_id.clone()), user_token: Some(request.user_token), }; @@ -344,7 +343,7 @@ where let client = reqwest::Client::new(); let response = client - .post(self.token_endpoint.clone().ok_or(ApiError::TokenRequestUnsupported(self.identity_provider_kind.clone()))?) + .post(self.token_endpoint.clone().ok_or(ApiError::TokenRequestUnsupported(self.identity_provider_kind))?) .header("accept", "application/json") .form(¶ms) .send() @@ -397,13 +396,8 @@ where } fn should_handle_introspect_request(&self, request: &IntrospectRequest) -> bool { - match request.issuer() { - Some(iss) if iss == self.issuer => true, - Some(_) => false, - None => false, - } + self.identity_provider_kind == request.identity_provider } - // JWTBearer grant does not support exchanging tokens. } @@ -416,11 +410,7 @@ where } fn should_handle_introspect_request(&self, request: &IntrospectRequest) -> bool { - match request.issuer() { - Some(iss) if iss == self.issuer => true, - Some(_) => false, - None => false, - } + self.identity_provider_kind == request.identity_provider } // ClientCredentials grant does not support exchanging tokens. @@ -437,11 +427,7 @@ where } fn should_handle_introspect_request(&self, request: &IntrospectRequest) -> bool { - match request.issuer() { - Some(iss) if iss == self.issuer => true, - Some(_) => false, - None => false, - } + self.identity_provider_kind == request.identity_provider } } @@ -455,11 +441,7 @@ where self.identity_provider_kind == request.identity_provider } fn should_handle_introspect_request(&self, request: &IntrospectRequest) -> bool { - match request.issuer() { - Some(iss) if iss == self.issuer => true, - Some(_) => false, - None => false, - } + self.identity_provider_kind == request.identity_provider } } @@ -468,10 +450,6 @@ where A: Serialize + Assertion, { fn should_handle_introspect_request(&self, request: &IntrospectRequest) -> bool { - match request.issuer() { - Some(iss) if iss == self.issuer => true, - Some(_) => false, - None => false, - } + self.identity_provider_kind == request.identity_provider } } diff --git a/src/jwks.rs b/src/jwks.rs index 373767d..26bfb65 100644 --- a/src/jwks.rs +++ b/src/jwks.rs @@ -29,8 +29,8 @@ pub enum Error { MissingKeyID, #[error("missing key id from token header")] MissingKeyIDInTokenHeader, - #[error("signing key with {0} not in json web key set")] - KeyNotInJWKS(String), + #[error("token can not be validated with this identity provider")] + KeyNotInJWKS, #[error("invalid token header: {0}")] InvalidTokenHeader(jwt::errors::Error), #[error("invalid token: {0}")] @@ -59,11 +59,11 @@ impl Jwks { endpoint: endpoint.to_string(), issuer: issuer.to_string(), required_audience: required_audience.clone(), - validation: Self::validate_with(issuer.to_string(), required_audience), + validation: Self::validator(issuer.to_string(), required_audience), }) } - fn validate_with(issuer: String, audience: Option) -> Validation { + fn validator(issuer: String, audience: Option) -> Validation { let alg = jwt::Algorithm::RS256; let mut validation = Validation::new(alg); @@ -98,7 +98,7 @@ impl Jwks { let signing_key = match self.keys.get(&key_id) { None => { self.refresh().await?; - self.keys.get(&key_id).ok_or(Error::KeyNotInJWKS(key_id))? + self.keys.get(&key_id).ok_or(Error::KeyNotInJWKS)? } Some(key) => key, };