fix(jira): Add a 'key_id' block list for JIRA installed webhook endpoint #87086
Conversation
github-actions
bot
added
the
Scope: Backend
Codecov ReportAll modified and coverable lines are covered by tests ✅ ✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## master #87086 +/- ##
========================================
Coverage 87.74% 87.75%
========================================
Files 9828 9835 +7
Lines 556309 556577 +268
Branches 21945 21945
========================================
+ Hits 488127 488406 +279
+ Misses 67752 67741 -11
Partials 430 430 |
| if key_id in INVALID_KEY_IDS: | ||
| lifecycle.record_halt(halt_reason="JWT contained invalid key_id (kid)") | ||
| return self.respond( | ||
| {"detail": "Invalid key id"}, status=status.HTTP_400_BAD_REQUEST | ||
| ) |
There was a problem hiding this comment.
i wonder if instead of this, we have a try/except to verify the JWT. if we encounter a ValueError while decoding AND the key is in invalid ids, we should record halt
the reason being we should still allow the decoding to happen as that is the expected behavior, we just don't want the exception to be counted as a failure
There was a problem hiding this comment.
Yeah this was another option I talked with @GabeVillalobos about, but we went this method as it was easiest and the installation webhook module will get rewritten soon(TM) as Jira Forge uses a different authentication method/doesn't need this JWT verification part.
Our Jira installation SLO was failing, because Atlassian sends bots/scanners to "test" their integrators and check if they're working. Some of these bots were hitting our installed webhook endpoint and providing JWTs with a bad
key_id(kid) (relevant Sentry issue with details). We would then reach out to Atlassian using the bad kid for the public key and we would get back some 403 Unauthorized HTML. The JWT module would then raise anValueErrorwhen we tried to verify the token using the 403 HTML and cause our metric/SLO to fail.We're just adding a blocklist, since 1. This module is going to get nuked soon with the Atlassian forge changes and 2. so we only filter out the atlassian bots since they all use the same kid.