hubcio commented on code in PR #2974:
URL: https://github.com/apache/iggy/pull/2974#discussion_r2965666996
##########
core/server/src/http/http_server.rs:
##########
@@ -268,12 +269,30 @@ async fn build_app_state(
tokens_path = shard.config.system.get_state_tokens_path();
}
- let jwt_manager = JwtManager::from_config(persister, &tokens_path,
&config.jwt);
- if let Err(e) = jwt_manager {
- panic!("Failed to initialize JWT manager: {e}");
+ let mut jwt_config = config.jwt.clone();
+ let encoding_empty = jwt_config.encoding_secret.is_empty();
+ let decoding_empty = jwt_config.decoding_secret.is_empty();
+ if encoding_empty || decoding_empty {
Review Comment:
when only one of encoding/decoding is configured and the other is left
empty, the `||` guard fires and generates a random secret - but it only fills
the empty side. for HMAC (HS256/384/512) encoding and decoding must use the
same key, so the server starts fine but every token validation returns
`Unauthenticated`.
the both-empty and both-set cases work correctly. only the mixed case is
broken - e.g. user sets `encoding_secret = "my_key"` but leaves
`decoding_secret = ""`.
fix options:
- reject startup when exactly one is empty (fail fast with a clear error)
- copy the configured secret to the empty slot for HMAC algorithms
##########
core/server/src/http/http_server.rs:
##########
@@ -268,12 +269,30 @@ async fn build_app_state(
tokens_path = shard.config.system.get_state_tokens_path();
}
- let jwt_manager = JwtManager::from_config(persister, &tokens_path,
&config.jwt);
- if let Err(e) = jwt_manager {
- panic!("Failed to initialize JWT manager: {e}");
+ let mut jwt_config = config.jwt.clone();
+ let encoding_empty = jwt_config.encoding_secret.is_empty();
+ let decoding_empty = jwt_config.decoding_secret.is_empty();
+ if encoding_empty || decoding_empty {
+ let secret = crypto::generate_secret(32..64);
+ let redacted = secret.chars().take(3).collect::<String>();
+ if encoding_empty {
+ jwt_config.encoding_secret = secret.clone();
Review Comment:
`secret.clone()` is only needed when both branches fire (both empty). when
only encoding is empty, the original `secret` is never used again since the `if
decoding_empty` block won't execute. not a real issue since this is
startup-only code - just a minor observation.
--
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]
