feat: evaluate alg based on the pem key#844
Merged
Merged
Conversation
Comment on lines
+88
to
+89
| elif isinstance(key, ec.EllipticCurvePrivateKey): | ||
| self.algorithm = "ES256" # or choose based on curve if you support ES384/ES512 |
Collaborator
There was a problem hiding this comment.
should we add a check based on curve sizes changes?
Collaborator
There was a problem hiding this comment.
something of this sort so we can check for the specific curve?
from cryptography.hazmat.primitives.asymmetric import rsa, ec, ed25519, ed448
if isinstance(key, rsa.RSAPrivateKey):
self.algorithm = "RS256"
elif isinstance(key, ec.EllipticCurvePrivateKey):
curve = key.curve
if isinstance(curve, ec.SECP256R1):
self.algorithm = "ES256"
elif isinstance(curve, ec.SECP384R1):
self.algorithm = "ES384"
elif isinstance(curve, ec.SECP521R1):
self.algorithm = "ES512"
else:
raise ValueError(f"Unsupported EC curve: {curve.name}")
elif isinstance(key, (ed25519.Ed25519PrivateKey, ed448.Ed448PrivateKey)):
self.algorithm = "EdDSA"
else:
raise ValueError(f"Unsupported private key type: {type(key)}")
edwinjosechittilappilly
approved these changes
Jan 26, 2026
edwinjosechittilappilly
left a comment
edwinjosechittilappilly
left a comment
Collaborator
There was a problem hiding this comment.
Ideal case this should cover the most of the cases.
Few NIT suggestions added for future.
edwinjosechittilappilly
changed the title
Updated the SessionManager to select the JWT algorithm based on the specific elliptic curve used in the EC private key, supporting ES256, ES384, and ES512. Raises an error for unsupported curves.
Changed the test-integration workflow to run on ubuntu-latest instead of a self-hosted ARM64 runner. Added a step to install docker-compose to ensure it is available in the CI environment.
Changed the test-integration workflow to run on a self-hosted ARM64 runner with specific labels instead of ubuntu-latest. This enables testing in an environment closer to production or with required hardware resources.
Refactor and optimize JWT authentication testing against OpenSearch for automated CI test targets by introducing reusable test function and eliminating code duplication. Changes: - Add reusable test_jwt_opensearch function (lines 34-64) * Improved error handling with --fail-with-body curl flag * Enhanced output formatting with colored messages * Proper cleanup using temp files - Add standalone test-os-jwt target for independent JWT testing * Documented in help system under "Diagnostic Tests" - Remove duplicate JWT diagnostic code from test-ci (lines 691-698) - Remove duplicate JWT diagnostic code from test-ci-local (lines 778-787) - Update both targets to call reusable function: $(call test_jwt_opensearch) - Update PHONY targets list and help documentation Closes #887
Collaborator
CI Integration Tests FailureProblemsJWT OpenSearch Test Scenario
Solution
Further OptimizationsSummaryRefactor and optimize JWT authentication testing against OpenSearch Changes
|
mpawlow
approved these changes
Jan 30, 2026
mpawlow
left a comment
mpawlow
left a comment
Collaborator
There was a problem hiding this comment.
Functional Review
- Added an additional commit to address the JWT OpenSearch test scenario issues
- Integration CI Tests all pass now
- LGTM / Approved
Collaborator
|
Thanks Mike! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request enhances the JWT signing configuration logic in
src/session_manager.pyto support additional asymmetric key types and improves key type detection. The main changes include support for EC and EdDSA keys, more robust PEM detection, and clearer logging.JWT signing enhancements:
ES256) and EdDSA (EdDSA) private keys in addition to existing RSA (RS256) and symmetric (HS256) keys when loading from the environment variableJWT_SIGNING_KEY.rsa,ec,ed25519, anded448key types fromcryptography.hazmat.primitives.asymmetric.