fix(jwt-authenticator): handle PEM private keys with escaped newlines #855
Conversation
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
Fixes an issue where JWT authentication fails when PEM-formatted private keys contain escaped newlines (\n) instead of actual newline characters. This commonly occurs when keys are stored in configuration systems like Airbyte Cloud. The fix adds normalization logic in JwtAuthenticator._get_secret_key() that: - Detects PEM-style keys (containing '-----BEGIN' and 'KEY-----') - Converts escaped newlines to actual newlines before JWT signing - Is guarded to only affect PEM keys, leaving other secret types unchanged This resolves the same issue fixed for the Okta connector in airbytehq/airbyte#69831, but at the CDK level so all declarative/Builder connectors using JWT authentication with private keys benefit from the fix. Includes a unit test that verifies JWT signing works with escaped newlines. Co-Authored-By: syed.khadeer@airbyte.io <cloud-support@airbyte.io>
Contributor
Original prompt from syed.khadeer@airbyte.io |
Contributor
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
👋 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@devin/1764219353-fix-jwt-pem-escaped-newlines#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 devin/1764219353-fix-jwt-pem-escaped-newlinesHelpful ResourcesPR Slash CommandsAirbyte Maintainers can execute the following slash commands on your PR:
|
Co-Authored-By: syed.khadeer@airbyte.io <cloud-support@airbyte.io>
2 tasks
Contributor
📝 WalkthroughWalkthroughNormalize escaped newline sequences in PEM-style secret keys used by the JWT authenticator before any passphrase or base64 fallback processing; add a unit test verifying signing with an escaped-newline PEM key. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client as Caller
participant Auth as JwtAuthenticator
participant Crypto as KeyLoader / CryptoLib
Client->>Auth: request_signed_token(secret, algorithm, payload)
note right of Auth `#EFEFEF`: Secret preparation
Auth->>Auth: inspect secret for "-----BEGIN" and "KEY-----"
alt PEM-like secret
Auth->>Auth: replace "\\n" → "\n" %% escaped-newline normalization
end
Auth->>Auth: apply passphrase handling / base64 fallback
Auth->>Crypto: load private key
Crypto-->>Auth: private key object
Auth->>Crypto: sign payload (algorithm)
Crypto-->>Auth: signed JWT
Auth-->>Client: return signed JWT
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (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)
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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
unit_tests/sources/declarative/auth/test_jwt.py (1)
396-436: Solid test coverage for the escaped newlines feature!The test properly validates the end-to-end flow: generating a key, escaping newlines, signing, and verifying. The structure mirrors the existing passphrase test nicely.
One optional enhancement: you could add a more focused unit test that directly calls
_get_secret_key()with an escaped PEM key and verifies the result is the properly loaded key object (similar totest_get_secret_keyon lines 125-134). This would test the normalization logic more directly, wdyt? But the current integration-style test is definitely acceptable.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
airbyte_cdk/sources/declarative/auth/jwt.py(1 hunks)unit_tests/sources/declarative/auth/test_jwt.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
unit_tests/sources/declarative/auth/test_jwt.py (1)
airbyte_cdk/sources/declarative/auth/jwt.py (1)
_get_signed_token(206-218)
🪛 GitHub Actions: Linters
airbyte_cdk/sources/declarative/auth/jwt.py
[error] 185-196: ruff format check failed. 1 file would be reformatted. Run 'poetry run ruff format .' to apply formatting changes.
⏰ 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). (14)
- GitHub Check: Check: source-shopify
- GitHub Check: Check: source-intercom
- GitHub Check: Check: source-pokeapi
- GitHub Check: Check: source-hardcoded-records
- GitHub Check: Check: destination-motherduck
- GitHub Check: Pytest (Fast)
- GitHub Check: Manifest Server Docker Image Build
- GitHub Check: SDM Docker Image Build
- GitHub Check: Pytest (All, Python 3.12, Ubuntu)
- GitHub Check: Pytest (All, Python 3.13, Ubuntu)
- GitHub Check: Pytest (All, Python 3.10, Ubuntu)
- GitHub Check: Pytest (All, Python 3.11, Ubuntu)
- GitHub Check: Analyze (python)
- GitHub Check: Analyze (python)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
Fixes a critical issue where JWT authentication fails when PEM-formatted private keys contain escaped newlines (
\\n) instead of actual newline characters. This commonly occurs when keys are stored in configuration systems like Airbyte Cloud.The Problem:
JwtAuthenticatorwith RS256 private keys fail withInvalidKeyError: Could not parse the provided public key\ncharacters instead of actual newlinesjwt.encode()cannot parse PEM keys in this formatThe Solution:
JwtAuthenticator._get_secret_key()that detects PEM-style keys and converts escaped newlines to actual newlines"-----BEGIN"and"KEY-----") to avoid affecting HMAC secretsRelated:
Review & Testing Checklist for Human
Risk Level: 🟡 Yellow - Fix is straightforward but needs real-world validation
"-----BEGIN"and"KEY-----") could have false positives with non-PEM secrets\\n(e.g.,\r\n, double-escaping)?Test Plan
Local testing (already done):
Recommended Cloud testing:
Notes
Summary by CodeRabbit
Bug Fixes
Tests
✏️ Tip: You can customize this high-level summary in your review settings.