spetz commented on code in PR #2656:
URL: https://github.com/apache/iggy/pull/2656#discussion_r2777306837
##########
core/integration/tests/server/a2a_jwt/generate_jwks.py:
##########
@@ -0,0 +1,75 @@
+#!/usr/bin/env python3
Review Comment:
Why is there python under rust integration tests?
##########
core/server/src/http/jwt/jwt_manager.rs:
##########
@@ -232,26 +248,92 @@ impl JwtManager {
.error(|e: &IggyError| {
format!("{COMPONENT} (error: {e}) - failed to save revoked
access token: {id}")
})?;
- self.generate(jwt_claims.claims.sub)
+ let user_id = jwt_claims
+ .claims
+ .sub
+ .parse::<u32>()
+ .map_err(|_| IggyError::InvalidAccessToken)?;
+ self.generate(user_id)
}
- pub fn decode(
+ pub async fn decode(
&self,
token: &str,
algorithm: Algorithm,
) -> Result<TokenData<JwtClaims>, IggyError> {
let validation = self.validations.get(&algorithm);
- if validation.is_none() {
- return Err(IggyError::InvalidJwtAlgorithm(
- Self::map_algorithm_to_string(algorithm),
- ));
- }
+ let kid = jsonwebtoken::decode_header(token).ok().and_then(|h| h.kid);
- let validation = validation.unwrap();
- match jsonwebtoken::decode::<JwtClaims>(token, &self.validator.key,
validation) {
- Ok(claims) => Ok(claims),
- _ => Err(IggyError::Unauthenticated),
- }
+ // try to decode using JWKS if it's a trusted issuer
+ let insecure = match
jsonwebtoken::dangerous::insecure_decode::<JwtClaims>(token) {
+ Ok(claims) => claims,
+ Err(_) => {
+ error!("Failed to decode JWT insecurely");
+ return self.decode_with_fallback(token, validation, algorithm);
+ }
+ };
+
+ /* debug!(
+ "JWT decoded insecurely, issuer: {}, kid: {:?}",
+ insecure.claims.iss, kid
+ ); */
+
+ let config = match self.trusted_issuer.get(&insecure.claims.iss) {
+ Some(config) => config,
+ None => {
+ debug!("No trusted issuer found for: {}", insecure.claims.iss);
+ return self.decode_with_fallback(token, validation, algorithm);
+ }
+ };
+
+ // debug!("Found trusted issuer config: {}", config.issuer);
+
+ let kid_str = match kid.as_deref() {
+ Some(kid) => kid,
+ None => {
+ error!("No kid found in JWT header");
+ return self.decode_with_fallback(token, validation, algorithm);
+ }
+ };
+
+ let decoding_key = match self
+ .jwks_client
+ .get_key(&config.issuer, &config.jwks_url, kid_str)
+ .await
+ {
+ Some(key) => key,
+ None => {
+ error!("Failed to get decoding key from JWKS for kid: {}",
kid_str);
+ return self.decode_with_fallback(token, validation, algorithm);
+ }
+ };
+
+ // debug!("Got decoding key from JWKS for kid: {}", kid_str);
Review Comment:
Also this one, and any others similar to this one (commented out line of
code).
##########
core/server/src/http/jwt/jwt_manager.rs:
##########
@@ -232,26 +248,92 @@ impl JwtManager {
.error(|e: &IggyError| {
format!("{COMPONENT} (error: {e}) - failed to save revoked
access token: {id}")
})?;
- self.generate(jwt_claims.claims.sub)
+ let user_id = jwt_claims
+ .claims
+ .sub
+ .parse::<u32>()
+ .map_err(|_| IggyError::InvalidAccessToken)?;
+ self.generate(user_id)
}
- pub fn decode(
+ pub async fn decode(
&self,
token: &str,
algorithm: Algorithm,
) -> Result<TokenData<JwtClaims>, IggyError> {
let validation = self.validations.get(&algorithm);
- if validation.is_none() {
- return Err(IggyError::InvalidJwtAlgorithm(
- Self::map_algorithm_to_string(algorithm),
- ));
- }
+ let kid = jsonwebtoken::decode_header(token).ok().and_then(|h| h.kid);
- let validation = validation.unwrap();
- match jsonwebtoken::decode::<JwtClaims>(token, &self.validator.key,
validation) {
- Ok(claims) => Ok(claims),
- _ => Err(IggyError::Unauthenticated),
- }
+ // try to decode using JWKS if it's a trusted issuer
+ let insecure = match
jsonwebtoken::dangerous::insecure_decode::<JwtClaims>(token) {
+ Ok(claims) => claims,
+ Err(_) => {
+ error!("Failed to decode JWT insecurely");
+ return self.decode_with_fallback(token, validation, algorithm);
+ }
+ };
+
+ /* debug!(
+ "JWT decoded insecurely, issuer: {}, kid: {:?}",
+ insecure.claims.iss, kid
+ ); */
+
+ let config = match self.trusted_issuer.get(&insecure.claims.iss) {
+ Some(config) => config,
+ None => {
+ debug!("No trusted issuer found for: {}", insecure.claims.iss);
+ return self.decode_with_fallback(token, validation, algorithm);
+ }
+ };
+
+ // debug!("Found trusted issuer config: {}", config.issuer);
Review Comment:
Remove such comments.
##########
core/server/src/http/jwt/middleware.rs:
##########
@@ -92,10 +89,15 @@ pub async fn jwt_auth(
}
let request_details =
request.extensions().get::<RequestDetails>().unwrap();
+ let user_id = jwt_claims
Review Comment:
Sub now uses `string` instead of `u32` yet we expect it to be parsed to
numeric value - in such a case, why it's been changed to `string`?
##########
core/integration/tests/server/a2a_jwt/test_private_key.pem:
##########
@@ -0,0 +1,28 @@
+-----BEGIN PRIVATE KEY-----
+MIIEvQIBADANBgkqhkiG9w0BAQEFAASCBKcwggSjAgEAAoIBAQC/TvzTyulqS+s3
Review Comment:
We have the example certs under `core/certs`.
##########
core/integration/tests/server/a2a_jwt/jwt_tests.rs:
##########
@@ -0,0 +1,217 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+use integration::iggy_harness;
+use jsonwebtoken::{Algorithm, EncodingKey, Header, encode};
+use reqwest::Client;
+use serde::{Deserialize, Serialize};
+
+const TEST_ISSUER: &str = "https://test-issuer.com";;
+const TEST_AUDIENCE: &str = "iggy";
+const TEST_KEY_ID: &str = "test-key-1";
+
+/// Test claims structure for JWT tokens
+#[derive(Debug, Serialize, Deserialize)]
+struct TestClaims {
+ jti: String,
+ iss: String,
+ aud: String,
+ sub: String,
+ exp: u64,
+ iat: u64,
+ nbf: u64,
+}
+
+/// Get current timestamp in seconds since Unix epoch
+fn now_timestamp() -> i64 {
+ std::time::SystemTime::now()
+ .duration_since(std::time::UNIX_EPOCH)
+ .unwrap()
+ .as_secs() as i64
+}
+
+/// Creates a valid JWT token with specified expiration time
+fn create_valid_jwt(exp_seconds: i64) -> String {
+ let now = now_timestamp();
+ let claims = TestClaims {
+ jti: uuid::Uuid::now_v7().to_string(),
+ iss: TEST_ISSUER.to_string(),
+ aud: TEST_AUDIENCE.to_string(),
+ sub: "0".to_string(),
+ exp: (now + exp_seconds) as u64,
+ iat: now as u64,
+ nbf: now as u64,
+ };
+
+ let mut header = Header::new(Algorithm::RS256);
+ header.kid = Some(TEST_KEY_ID.to_string());
+ let private_key = include_bytes!("test_private_key.pem");
+ let encoding_key = EncodingKey::from_rsa_pem(private_key).unwrap();
+
+ encode(&header, &claims, &encoding_key).unwrap()
+}
+
+/// Creates an expired JWT token (expired 1 hour ago)
+fn create_expired_jwt() -> String {
+ let now = now_timestamp();
+ let claims = TestClaims {
+ jti: uuid::Uuid::now_v7().to_string(),
+ iss: TEST_ISSUER.to_string(),
+ aud: TEST_AUDIENCE.to_string(),
+ sub: "0".to_string(),
+ exp: (now - 3600) as u64,
+ iat: (now - 7200) as u64,
+ nbf: (now - 7200) as u64,
+ };
+
+ let mut header = Header::new(Algorithm::RS256);
+ header.kid = Some(TEST_KEY_ID.to_string());
+ let private_key = include_bytes!("test_private_key.pem");
+ let encoding_key = EncodingKey::from_rsa_pem(private_key).unwrap();
+
+ encode(&header, &claims, &encoding_key).unwrap()
+}
+
+/// Creates a JWT token with unknown issuer
+fn create_unknown_issuer_jwt() -> String {
+ let now = now_timestamp();
+ let claims = TestClaims {
+ jti: uuid::Uuid::now_v7().to_string(),
+ iss: "https://unknown-issuer.com".to_string(),
+ aud: TEST_AUDIENCE.to_string(),
+ sub: "0".to_string(),
+ exp: (now + 3600) as u64,
+ iat: now as u64,
+ nbf: now as u64,
+ };
+
+ let mut header = Header::new(Algorithm::RS256);
+ header.kid = Some(TEST_KEY_ID.to_string());
+ let private_key = include_bytes!("test_private_key.pem");
+ let encoding_key = EncodingKey::from_rsa_pem(private_key).unwrap();
+
+ encode(&header, &claims, &encoding_key).unwrap()
+}
+
+/// Test that valid A2A JWT token allows access to API
+#[iggy_harness(
+ server(config_path = "tests/server/a2a_jwt/config.toml"),
+ jwks_server(store_path = "tests/server/a2a_jwt/wiremock/__files/jwks.json")
+)]
+async fn test_a2a_jwt_valid_token(harness: &TestHarness) {
+ let server = harness
+ .all_servers()
+ .first()
+ .expect("server should be available");
+ let http_addr = server
+ .http_addr()
+ .expect("http address should be available");
+
+ let client = Client::new();
+ let token = create_valid_jwt(3600);
+
+ let response = client
+ .get(format!("http://{}/streams";, http_addr))
+ .header("Authorization", format!("Bearer {}", token))
+ .send()
+ .await
+ .unwrap();
+
+ assert_eq!(response.status(), 200);
+}
+
+/// Test that expired A2A JWT token is rejected
+#[iggy_harness(
+ server(config_path = "tests/server/a2a_jwt/config.toml"),
+ jwks_server(store_path = "tests/server/a2a_jwt/wiremock/__files/jwks.json")
+)]
+async fn test_a2a_jwt_expired_token(harness: &TestHarness) {
+ let server = harness
+ .all_servers()
+ .first()
+ .expect("server should be available");
+ let http_addr = server
+ .http_addr()
+ .expect("http address should be available");
+
+ let client = Client::new();
Review Comment:
Why not use Iggy HTTP client?
##########
core/server/src/http/jwt/jwt_manager.rs:
##########
@@ -232,26 +248,92 @@ impl JwtManager {
.error(|e: &IggyError| {
format!("{COMPONENT} (error: {e}) - failed to save revoked
access token: {id}")
})?;
- self.generate(jwt_claims.claims.sub)
+ let user_id = jwt_claims
+ .claims
+ .sub
+ .parse::<u32>()
+ .map_err(|_| IggyError::InvalidAccessToken)?;
+ self.generate(user_id)
}
- pub fn decode(
+ pub async fn decode(
&self,
token: &str,
algorithm: Algorithm,
) -> Result<TokenData<JwtClaims>, IggyError> {
let validation = self.validations.get(&algorithm);
- if validation.is_none() {
- return Err(IggyError::InvalidJwtAlgorithm(
- Self::map_algorithm_to_string(algorithm),
- ));
- }
+ let kid = jsonwebtoken::decode_header(token).ok().and_then(|h| h.kid);
- let validation = validation.unwrap();
- match jsonwebtoken::decode::<JwtClaims>(token, &self.validator.key,
validation) {
- Ok(claims) => Ok(claims),
- _ => Err(IggyError::Unauthenticated),
- }
+ // try to decode using JWKS if it's a trusted issuer
+ let insecure = match
jsonwebtoken::dangerous::insecure_decode::<JwtClaims>(token) {
+ Ok(claims) => claims,
+ Err(_) => {
+ error!("Failed to decode JWT insecurely");
+ return self.decode_with_fallback(token, validation, algorithm);
+ }
+ };
+
+ /* debug!(
+ "JWT decoded insecurely, issuer: {}, kid: {:?}",
+ insecure.claims.iss, kid
+ ); */
+
+ let config = match self.trusted_issuer.get(&insecure.claims.iss) {
+ Some(config) => config,
+ None => {
+ debug!("No trusted issuer found for: {}", insecure.claims.iss);
+ return self.decode_with_fallback(token, validation, algorithm);
+ }
+ };
+
+ // debug!("Found trusted issuer config: {}", config.issuer);
+
+ let kid_str = match kid.as_deref() {
+ Some(kid) => kid,
+ None => {
+ error!("No kid found in JWT header");
+ return self.decode_with_fallback(token, validation, algorithm);
+ }
+ };
+
+ let decoding_key = match self
+ .jwks_client
+ .get_key(&config.issuer, &config.jwks_url, kid_str)
+ .await
+ {
+ Some(key) => key,
+ None => {
+ error!("Failed to get decoding key from JWKS for kid: {}",
kid_str);
+ return self.decode_with_fallback(token, validation, algorithm);
+ }
+ };
+
+ // debug!("Got decoding key from JWKS for kid: {}", kid_str);
+
+ let mut validation = Validation::new(algorithm);
+ validation.set_issuer(std::slice::from_ref(&config.issuer));
+ validation.set_audience(std::slice::from_ref(&config.audience));
+ debug!("Validation configured, attempting to decode JWT");
+
+ jsonwebtoken::decode::<JwtClaims>(token, &decoding_key,
&validation).map_err(|e| {
+ error!("Failed to decode JWT: {}", e);
+ IggyError::Unauthenticated
+ })
+ }
+
+ /// fallback to standard JWT validation if JWKS validation fails
+ fn decode_with_fallback(
+ &self,
+ token: &str,
+ validation: Option<&Validation>,
+ algorithm: Algorithm,
+ ) -> Result<TokenData<JwtClaims>, IggyError> {
+ let validation = validation.ok_or_else(|| {
+
IggyError::InvalidJwtAlgorithm(Self::map_algorithm_to_string(algorithm))
+ })?;
+
+ jsonwebtoken::decode::<JwtClaims>(token, &self.validator.key,
validation)
Review Comment:
Maybe let's log with error! macro here and don't discard it in `map_err`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
