-
Notifications
You must be signed in to change notification settings - Fork 14
feat: Add webhook decoder #80
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
WalkthroughAdds webhook validation helpers (decoder, event/source enums) and extends JWKS caching/parsing to accept a namespaced JWKS URL for trusted validation and cache isolation. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Decoder as WebhookDecoder
participant Utils as Utils::parseJWT
participant Storage
participant JWKS as JWKS Endpoint
Client->>Decoder: decodeWebhook(token, domain)
Decoder->>Decoder: normalize domain, build jwksUrl
Decoder->>Utils: parseJWT(token, jwksUrl)
Utils->>Storage: getCachedJwks(jwksUrl)
alt cache miss / refresh
Utils->>JWKS: fetch JWKS from jwksUrl
JWKS-->>Utils: return keys
Utils->>Storage: setCachedJwks(keys, ttl, jwksUrl)
end
Utils-->>Decoder: return payload or error
alt payload returned
Decoder-->>Client: return payload (array)
else invalid / error
Decoder-->>Client: return null
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Important Action Needed: IP Allowlist UpdateIf your organization protects your Git platform with IP whitelisting, please add the new CodeRabbit IP address to your allowlist:
Reviews will stop working after February 8, 2026 if the new IP is not added to your allowlist. 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@lib/Webhooks/WebhookDecoder.php`:
- Around line 24-33: In decodeWebhook, validate and parse the incoming $domain
before building $jwksUrl: use a URL/host parser (e.g., parse_url or filter_var)
to ensure the scheme is present and equals "https" and that the host is a valid
hostname (reject IPs/private ranges if required), and only then construct
$normalizedDomain and $jwksUrl; if validation fails, return null. Reference:
function decodeWebhook, variables $domain, $normalizedDomain, and $jwksUrl.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@lib/Webhooks/WebhookDecoder.php`:
- Around line 39-45: The code mutates global JWKS state via
Storage::getInstance()->setJwksUrl($jwksUrl) which causes race conditions;
instead remove that global mutation and pass the JWKS URL directly into the JWT
parsing call (e.g., change Utils::parseJWT($token) to Utils::parseJWT($token,
$jwksUrl) or an equivalent per-call parameter) from WebhookDecoder::...; update
Utils::parseJWT to accept and use the jwksUrl parameter (and preserve existing
exception handling) so each parseJWT call uses the provided jwksUrl and no
shared Storage state is modified.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/Sdk/Utils/Utils.php (1)
121-137: Fix inconsistent indentation in the error recovery block.Lines 125 and 130 have excessive leading whitespace that breaks the code's visual consistency.
Proposed fix
} catch (Exception $e) { // If parsing fails with cached JWKS, try to refresh from server if ($jwks !== null) { try { - Storage::getInstance()->clearCachedJwks($jwks_url); + Storage::getInstance()->clearCachedJwks($jwks_url); $jwks_json = file_get_contents($jwks_url); $jwks = json_decode($jwks_json, true); if ($jwks && isset($jwks['keys'])) { - Storage::getInstance()->setCachedJwks($jwks, 3600, $jwks_url); + Storage::getInstance()->setCachedJwks($jwks, 3600, $jwks_url); return json_decode(json_encode(JWT::decode($token, JWK::parseKeySet($jwks))), true); } } catch (Exception $refreshException) {
|
will look through this today |
|
Hey @Koosha-Owji, just finished running through this. Overall quality looks good and I’m happy with the approach. Just wanted to check a couple things with you as well. Would you be open to adding a lightweight domain guard to prevent untrusted domains being used to fetch JWKS? Also, can you confirm the customer requirement is strictly signature validation + decode, or do they expect additional claim checks, iss/aud/typ? |
|
Hello @KeeganBeuthin Yes my understanding it they are looking for the same functionality as the https://github.com/kinde-oss/webhook |
|
@KeeganBeuthin I have addressed your comments. |
Explain your changes
Fixes #77 — add PHP equivalent of @kinde/webhooks for webhook JWT validation/decoding, exposing event type constants and a decoder that reuses existing JWKS fetch/cache.
Checklist
🛟 If you need help, consider asking for advice over in the Kinde community.