Normalize App ID to String & Enhance JWT Issuer Verification #3272

x612skm · 2025-03-27T20:06:10Z

This PR fixes #3214 by ensuring self._app_id is always a string in AppAuth, preventing type mismatches in the JWT "iss" claim. It also strengthens JWT tests by adding issuer validation during decoding.

The AppAuth class previously stored self._app_id as-is (either int or str, per Union[int, str]), which could result in the JWT "iss" claim being an integer. Per the JWT specification (RFC 7519, Section 4.1.1), the iss (issuer) claim should be a string, and GitHub’s authentication flow expects it as such. This type mismatch caused potential failures in downstream processing or validation (e.g., with PyJWT or GitHub’s API), as reported in #3214. Additionally, the tests lacked proactive issuer checks, relying only on payload comparison, which missed an opportunity to catch errors earlier.

Changes

String Normalization in AppAuth (github/Auth.py): Changed self._app_id = app_id to self._app_id = str(app_id) in the initializer. This ensures the "iss" claim is consistently a string, regardless of whether the input app_id is an integer (common for GitHub App IDs) or a string.
Test Improvements (tests/Authentication.py): Updated testCreateJWT and testCreateJWTWithExpiration to expect a string "iss" and added issuer=str(APP_ID) to jwt.decode(). This leverages PyJWT’s built-in issuer validation (introduced in PyJWT 1.0.0+), aligning with its security features and failing fast on issuer mismatches.

Fixes
✅ Fixes type mismatch in JWT "iss" claim.
✅ Enhances test robustness by failing early on incorrect issuers.

Impact
Fixes the root cause of type-related errors in JWT generation, ensuring compliance with RFC 7519 and GitHub’s expectations.
Strengthens tests by mirroring real-world validation (e.g., GitHub rejecting tokens with invalid iss), reducing the risk of regressions.

Testing
Verified with pytest tests/Authentication.py—both testCreateJWT and testCreateJWTWithExpiration pass with the updated expectations and issuer verification.

Fixes: PyGithub/PyGithub#3214

Wrap self._app_id with str() in the JWT payload's "iss" field to ensure it is always a string, addressing a potential type mismatch. This fixes compatibility issues where downstream JWT processing expects a string issuer, as GitHub App IDs are typically integers.

Update the AppAuth initializer to set self._app_id = str(app_id) instead of app_id directly. This normalizes the GitHub App ID (which can be int or str per the Union[int, str] type hint) to a string at initialization, ensuring the JWT "iss" claim is always a string as expected by JWT standards and downstream processing.

Modify testCreateJWT and testCreateJWTWithExpiration to expect a string "iss" claim (str(APP_ID)) in the payload, matching the AppAuth initializer change. Add issuer=str(APP_ID) to jwt.decode() calls to leverage PyJWT's built-in issuer validation, improving test robustness by failing early on issuer mismatches.

x612skm · 2025-06-18T13:11:03Z

Can someone take a look at this?

EnricoMi

LGTM!

github-actions · 2025-07-30T21:16:47Z

Test Results

8 files ±0 8 suites ±0 4m 52s ⏱️ ±0s
1 008 tests ±0 1 008 ✅ ±0 0 💤 ±0 0 ❌ ±0
11 139 runs ±0 11 138 ✅ ±0 1 💤 ±0 0 ❌ ±0

Results for commit 9189c95. ± Comparison against base commit c7858c8.

…b#3272) This PR fixes PyGithub#3214 by ensuring self._app_id is always a string in AppAuth, preventing type mismatches in the JWT "iss" claim. It also strengthens JWT tests by adding issuer validation during decoding. The AppAuth class previously stored self._app_id as-is (either int or str, per Union[int, str]), which could result in the JWT "iss" claim being an integer. Per the JWT specification ([RFC 7519, Section 4.1.1](https://tools.ietf.org/html/rfc7519#section-4.1.1)), the iss (issuer) claim should be a string, and GitHub’s authentication flow expects it as such. This type mismatch caused potential failures in downstream processing or validation (e.g., with PyJWT or GitHub’s API), as reported in PyGithub#3214. Additionally, the tests lacked proactive issuer checks, relying only on payload comparison, which missed an opportunity to catch errors earlier. **Changes** - String Normalization in AppAuth (github/Auth.py): `Changed self._app_id = app_id to self._app_id = str(app_id)` in the initializer. This ensures the "iss" claim is consistently a string, regardless of whether the input app_id is an integer (common for GitHub App IDs) or a string. - Test Improvements (tests/Authentication.py): Updated testCreateJWT and testCreateJWTWithExpiration to expect a string "iss" and added `issuer=str(APP_ID)` to jwt.decode(). This leverages PyJWT’s built-in issuer validation (introduced in PyJWT 1.0.0+), aligning with its security features and failing fast on issuer mismatches. **Fixes** ✅ Fixes type mismatch in JWT "iss" claim. ✅ Enhances test robustness by failing early on incorrect issuers. **Impact** Fixes the root cause of type-related errors in JWT generation, ensuring compliance with RFC 7519 and GitHub’s expectations. Strengthens tests by mirroring real-world validation (e.g., GitHub rejecting tokens with invalid iss), reducing the risk of regressions. **Testing** Verified with pytest tests/Authentication.py—both testCreateJWT and testCreateJWTWithExpiration pass with the updated expectations and issuer verification. Fixes: [PyGithub#3214](PyGithub#3214) --------- Co-authored-by: Enrico Minack <github@enrico.minack.dev>

x612skm added 4 commits March 28, 2025 00:49

remove unwanted line gap

c378c32

EnricoMi approved these changes Jul 30, 2025

View reviewed changes

Merge branch 'main' into bug-dev-3214

9189c95

EnricoMi added this pull request to the merge queue Jul 30, 2025

Merged via the queue into PyGithub:main with commit 01196d6 Jul 30, 2025
22 checks passed

EnricoMi added the bugfix label Jul 31, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Normalize App ID to String & Enhance JWT Issuer Verification #3272

Normalize App ID to String & Enhance JWT Issuer Verification #3272

Uh oh!

x612skm commented Mar 27, 2025 •

edited

Loading

Uh oh!

x612skm commented Jun 18, 2025

Uh oh!

EnricoMi left a comment

Uh oh!

github-actions bot commented Jul 30, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Normalize App ID to String & Enhance JWT Issuer Verification #3272

Normalize App ID to String & Enhance JWT Issuer Verification #3272

Uh oh!

Conversation

x612skm commented Mar 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

x612skm commented Jun 18, 2025

Uh oh!

EnricoMi left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Jul 30, 2025

Test Results

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

x612skm commented Mar 27, 2025 •

edited

Loading