feat(JwtAuthenticator): add passphrase support to jwt auth #773
Conversation
👋 Greetings, Airbyte Team Member!Here are some helpful tips and reminders for your convenience. Testing This CDK VersionYou can test this version of the CDK using the following: # Run the CLI from this branch:
uvx 'git+https://github.com/airbytehq/airbyte-python-cdk.git@pnilan/jwt/add-asymmetrical-key-support#egg=airbyte-python-cdk[dev]' --help
# Update a connector to use the CDK from this branch ref:
cd airbyte-integrations/connectors/source-example
poe use-cdk-branch pnilan/jwt/add-asymmetrical-key-supportHelpful ResourcesPR Slash CommandsAirbyte Maintainers can execute the following slash commands on your PR:
|
There was a problem hiding this comment.
Pull Request Overview
Adds support for passphrase-protected private keys in JWT authentication and introduces a random UUID generator macro. This enables secure handling of encrypted RSA private keys commonly used with RSxxx algorithms and provides UUID generation functionality for upcoming connectors.
- Adds passphrase parameter to JwtAuthenticator for encrypted private key support
- Implements cryptographic key loading with passphrase decryption in the JWT authentication flow
- Introduces random_uuid() macro for generating UUID4 values
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| airbyte_cdk/sources/declarative/models/declarative_component_schema.py | Adds passphrase field to JwtAuthenticator model |
| airbyte_cdk/sources/declarative/declarative_component_schema.yaml | Updates schema definition with passphrase configuration |
| airbyte_cdk/sources/declarative/parsers/model_to_component_factory.py | Passes passphrase parameter to JwtAuthenticator constructor |
| airbyte_cdk/sources/declarative/auth/jwt.py | Implements passphrase-based private key decryption logic |
| airbyte_cdk/sources/declarative/interpolation/macros.py | Adds random_uuid() macro function |
| unit_tests/sources/declarative/auth/test_jwt.py | Adds comprehensive tests for passphrase functionality |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughAdds optional passphrase support for PEM-encrypted private keys in JwtAuthenticator, propagates the field through schema, model, and factory, and loads encrypted PEMs for signing. Also adds a new interpolation macro Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Config
participant Factory as ModelToComponentFactory
participant Auth as JwtAuthenticator
participant Crypto as cryptography.serialization
Config->>Factory: create_jwt_authenticator(model with secret + optional passphrase)
Factory->>Auth: JwtAuthenticator(secret=..., passphrase=..., ...)
Note right of Auth: Token creation / signing flow
Auth->>Auth: _get_secret_key()
alt passphrase provided
Auth->>Crypto: load_pem_private_key(pem_bytes, password=_passphrase)
Crypto-->>Auth: PrivateKey (e.g., RSAPrivateKey)
else no passphrase
Auth-->>Auth: Use secret (raw or base64-decoded bytes/str)
end
Auth->>Auth: jwt.encode(payload, key, algorithm)
Auth-->>Config: signed JWT
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Would you like a short changelog entry or a brief README note for the new Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (9)
unit_tests/sources/declarative/auth/test_jwt.py (5)
11-13: Drop deprecated default_backend imports?cryptography backends are deprecated and ignored. Would you remove these imports to future‑proof the tests, wdyt?
-from cryptography.hazmat.backends import default_backend from cryptography.hazmat.primitives import serialization from cryptography.hazmat.primitives.asymmetric import rsa
195-197: Remove deprecated backend=default_backend() arg?Backends are deprecated. rsa.generate_private_key no longer needs the backend. Shall we drop it here, wdyt?
- private_key = rsa.generate_private_key( - public_exponent=65537, key_size=2048, backend=default_backend() - ) + private_key = rsa.generate_private_key(public_exponent=65537, key_size=2048)
231-232: Same here: drop backend arg?Consistent with the other test, can we remove the deprecated backend parameter, wdyt?
- private_key = rsa.generate_private_key( - public_exponent=65537, key_size=2048, backend=default_backend() - ) + private_key = rsa.generate_private_key(public_exponent=65537, key_size=2048)
256-257: And here too: remove backend?Same refactor applies here for consistency, wdyt?
- private_key = rsa.generate_private_key( - public_exponent=65537, key_size=2048, backend=default_backend() - ) + private_key = rsa.generate_private_key(public_exponent=65537, key_size=2048)
250-251: Tighten exception expectation to ValueError?load_pem_private_key raises ValueError for wrong/missing password. Narrowing makes the test stricter. Want to update it, wdyt?
- with pytest.raises(Exception): + with pytest.raises(ValueError): authenticator._get_secret_key()airbyte_cdk/sources/declarative/auth/jwt.py (2)
180-192: Optional: validate algorithm ↔ key type compatibility.Would you add a runtime guard ensuring, e.g., RS*/PS* use RSA keys, ES* use EC keys, and EdDSA uses Ed25519/Ed448? This would fail fast on misconfigurations and improve error messages, wdyt?
180-192: Minor wording nit in docstring (optional).“Signed the JWT” → “Signs the JWT”. Want to tweak it, wdyt?
airbyte_cdk/sources/declarative/declarative_component_schema.yaml (2)
1273-1278: Add interpolation_context and password format to passphrase.Shall we mark this as a config-interpolated secret and mask it in UIs?
- Add interpolation_context: ['config'] (consistent with secret_key)
- Add format: password (so tooling can mask it)
- Optional: clarify in description that it’s only applicable when using asymmetric algorithms and a PEM private key, to avoid confusion with HS*.
Proposed patch:
passphrase: title: Passphrase description: A passphrase/password used to encrypt the private key. Only provide a passphrase if required by the API for JWT authentication. The API will typically provide the passphrase when generating the public/private key pair. type: string + format: password + interpolation_context: + - config examples: - "{{ config['passphrase'] }}"Also, do you want to move this field up near secret_key to keep related fields co-located for better UX in the docs, wdyt?
4812-4915: Document the new random_uuid macro.Since the PR adds a random_uuid() macro, should we document it here so it shows up in Builder/UX help? Suggested entry right before filters:
- title: str_to_datetime description: Converts a string to a datetime object with UTC timezone. arguments: s: The string to convert. return_type: datetime.datetime examples: - "{{ str_to_datetime('2022-01-14') }}" - "{{ str_to_datetime('2022-01-01 13:45:30') }}" - "{{ str_to_datetime('2022-01-01T13:45:30+00:00') }}" - "{{ str_to_datetime('2022-01-01T13:45:30.123456Z') }}" + - title: random_uuid + description: Returns a random UUID (version 4) as a string. + arguments: {} + return_type: str + examples: + - "'{{ random_uuid() }}' -> '550e8400-e29b-41d4-a716-446655440000'"Wdyt?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
airbyte_cdk/sources/declarative/auth/jwt.py(4 hunks)airbyte_cdk/sources/declarative/declarative_component_schema.yaml(1 hunks)airbyte_cdk/sources/declarative/interpolation/macros.py(2 hunks)airbyte_cdk/sources/declarative/models/declarative_component_schema.py(1 hunks)airbyte_cdk/sources/declarative/parsers/model_to_component_factory.py(1 hunks)unit_tests/sources/declarative/auth/test_jwt.py(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
unit_tests/sources/declarative/auth/test_jwt.py (2)
airbyte_cdk/sources/declarative/auth/jwt.py (3)
JwtAuthenticator(44-214)_get_secret_key(161-178)_get_signed_token(180-192)airbyte_cdk/sources/declarative/models/declarative_component_schema.py (1)
JwtAuthenticator(400-457)
airbyte_cdk/sources/declarative/auth/jwt.py (1)
airbyte_cdk/sources/declarative/interpolation/interpolated_string.py (1)
InterpolatedString(13-79)
🪛 GitHub Actions: Linters
airbyte_cdk/sources/declarative/auth/jwt.py
[error] 187-187: Argument "key" has incompatible type "DHPrivateKey | Ed25519PrivateKey | Ed448PrivateKey | RSAPrivateKey | DSAPrivateKey | EllipticCurvePrivateKey | X25519PrivateKey | X448PrivateKey | str | bytes"; expected "RSAPrivateKey | EllipticCurvePrivateKey | Ed25519PrivateKey | Ed448PrivateKey | PyJWK | str | bytes" [arg-type]
[error] 1-1: Found 1 error in 1 file (checked 445 source files)
[error] 1-1: Process completed with exit code 1.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)
- GitHub Check: Check: destination-motherduck
- GitHub Check: Check: source-pokeapi
- GitHub Check: Check: source-intercom
- GitHub Check: Check: source-hardcoded-records
- GitHub Check: Check: source-shopify
- GitHub Check: Pytest (All, Python 3.10, Ubuntu)
- GitHub Check: Pytest (All, Python 3.13, Ubuntu)
- GitHub Check: Pytest (All, Python 3.12, Ubuntu)
- GitHub Check: Pytest (All, Python 3.11, Ubuntu)
- GitHub Check: Pytest (Fast)
- GitHub Check: Manifest Server Docker Image Build
- GitHub Check: SDM Docker Image Build
🔇 Additional comments (7)
airbyte_cdk/sources/declarative/parsers/model_to_component_factory.py (1)
2688-2708: Passphrase propagation looks solid.Forwarding the new
passphrasefield into the runtime authenticator completes the pipe without disturbing existing defaults—looks good to me.airbyte_cdk/sources/declarative/models/declarative_component_schema.py (1)
451-456: Schema update aligns with runtime change.Adding
passphraseto the declarative schema keeps the manifest model in sync with the authenticator’s capabilities—nicely done.unit_tests/sources/declarative/auth/test_jwt.py (2)
192-227: LGTM: solid coverage for passphrase-protected key path.Good generation, encryption, and equality checks on key components. Nice.
253-288: LGTM: end-to-end signing/verification with RS256.Great to validate the full flow and payload claims.
airbyte_cdk/sources/declarative/auth/jwt.py (3)
80-81: New passphrase field: nice addition.The public surface aligns with the schema/model changes and is optional. Looks good.
Would you confirm model_to_component_factory forwards passphrase in all JwtAuthenticator creation sites?
110-114: _passphrase interpolation wiring looks correct.Using InterpolatedString keeps parity with other fields. LGTM.
185-191: Re-run linters to verify jwt.encode key type alignment
Narrowing _get_secret_key toPrivateKeyTypes | str | bytesaligns with jwt.encode’skeyparameter – does this resolve the mypy/pyright error? wdyt?
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
airbyte_cdk/sources/declarative/auth/jwt.py(4 hunks)airbyte_cdk/sources/declarative/interpolation/macros.py(3 hunks)unit_tests/sources/declarative/interpolation/test_macros.py(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
airbyte_cdk/sources/declarative/auth/jwt.py (3)
airbyte_cdk/sources/declarative/auth/declarative_authenticator.py (1)
DeclarativeAuthenticator(14-29)airbyte_cdk/sources/declarative/interpolation/interpolated_mapping.py (1)
InterpolatedMapping(14-56)airbyte_cdk/sources/declarative/interpolation/interpolated_string.py (1)
InterpolatedString(13-79)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)
- GitHub Check: Check: source-hardcoded-records
- GitHub Check: Check: source-shopify
- GitHub Check: Check: source-pokeapi
- GitHub Check: Check: source-intercom
- GitHub Check: Check: destination-motherduck
- GitHub Check: Pytest (Fast)
- GitHub Check: SDM Docker Image Build
- GitHub Check: Pytest (All, Python 3.11, Ubuntu)
- GitHub Check: Pytest (All, Python 3.13, Ubuntu)
- GitHub Check: Pytest (All, Python 3.12, Ubuntu)
- GitHub Check: Manifest Server Docker Image Build
- GitHub Check: Pytest (All, Python 3.10, Ubuntu)
🔇 Additional comments (2)
airbyte_cdk/sources/declarative/interpolation/macros.py (1)
221-235: generate_uuid registered cleanlyI appreciate that you wired
generate_uuidinto_macros_list, so manifests can actually use it—this should make the upcoming connector happy. 👍unit_tests/sources/declarative/interpolation/test_macros.py (1)
282-303: Thorough UUID macro coverageThanks for the comprehensive assertions (type, version, uniqueness, validation); they give me confidence the macro behaves as intended.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
airbyte_cdk/sources/declarative/auth/jwt.py(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
airbyte_cdk/sources/declarative/auth/jwt.py (3)
airbyte_cdk/sources/declarative/auth/declarative_authenticator.py (1)
DeclarativeAuthenticator(14-29)airbyte_cdk/sources/declarative/interpolation/interpolated_mapping.py (1)
InterpolatedMapping(14-56)airbyte_cdk/sources/declarative/interpolation/interpolated_string.py (1)
InterpolatedString(13-79)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)
- GitHub Check: Check: source-intercom
- GitHub Check: Check: destination-motherduck
- GitHub Check: Check: source-shopify
- GitHub Check: Check: source-pokeapi
- GitHub Check: Check: source-hardcoded-records
- GitHub Check: Pytest (Fast)
- GitHub Check: Pytest (All, Python 3.13, Ubuntu)
- GitHub Check: SDM Docker Image Build
- GitHub Check: Manifest Server Docker Image Build
- GitHub Check: Pytest (All, Python 3.12, Ubuntu)
- GitHub Check: Pytest (All, Python 3.11, Ubuntu)
- GitHub Check: Pytest (All, Python 3.10, Ubuntu)
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
airbyte_cdk/sources/declarative/auth/jwt.py (1)
118-122: Cache passphrase-protected keys after first decrypt?Right now every call to
_get_secret_key()rerunsserialization.load_pem_private_key, so passphrase-protected keys get decrypted on every token generation. For connectors that sign on each request that PBKDF step can become a noticeable CPU hit. Could we stash the decrypted key after the first successful load and reuse it, e.g. by initializing a cache slot in__post_init__and returning it when present, wdyt?self._passphrase = ( InterpolatedString.create(self.passphrase, parameters=parameters) if self.passphrase else None ) + self._cached_private_key: Optional[JwtKeyTypes] = None @@ if self._passphrase: + if self._cached_private_key is not None: + return self._cached_private_key passphrase_value = self._passphrase.eval(self.config, json_loads=json.loads) if passphrase_value: private_key = serialization.load_pem_private_key( secret_key.encode(), password=passphrase_value.encode(), ) - return cast(JwtKeyTypes, private_key) + self._cached_private_key = cast(JwtKeyTypes, private_key) + return self._cached_private_keyAlso applies to: 175-183
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
airbyte_cdk/sources/declarative/auth/jwt.py(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
airbyte_cdk/sources/declarative/auth/jwt.py (3)
airbyte_cdk/sources/declarative/auth/declarative_authenticator.py (1)
DeclarativeAuthenticator(14-29)airbyte_cdk/sources/declarative/interpolation/interpolated_mapping.py (1)
InterpolatedMapping(14-56)airbyte_cdk/sources/declarative/interpolation/interpolated_string.py (1)
InterpolatedString(13-79)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)
- GitHub Check: Check: destination-motherduck
- GitHub Check: Check: source-pokeapi
- GitHub Check: Check: source-shopify
- GitHub Check: Check: source-hardcoded-records
- GitHub Check: Check: source-intercom
- GitHub Check: Pytest (All, Python 3.10, Ubuntu)
- GitHub Check: Pytest (All, Python 3.13, Ubuntu)
- GitHub Check: Pytest (All, Python 3.12, Ubuntu)
- GitHub Check: Pytest (Fast)
- GitHub Check: Pytest (All, Python 3.11, Ubuntu)
- GitHub Check: Manifest Server Docker Image Build
- GitHub Check: SDM Docker Image Build
What
generate_uuid(). This is necessary for an upcoming connector that requires a random UUID for every generated request to verify each request. This uuid generator usesuuid.uuid4()under the hood as uuid7 is not available by default until Python 3.14. We will update macro naming and generation methods later.Review Order
Summary by CodeRabbit
New Features
Tests