feat(server): enable authentication to webhook endpoint#1015
feat(server): enable authentication to webhook endpoint#1015rene-oromtz wants to merge 11 commits intomainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Adds server-side enforcement and optional authentication for the agent events webhook forwarding, configured via the charm and validated in the server API to prevent sending events (and auth tokens) to unexpected destinations.
Changes:
- Add charm typed config (Pydantic) and new charm options for
webhook_endpointand optionalwebhook_auth. - Update
/v1/job/<job_id>/eventsto validatejob_id, verify job existence, enforce webhook scheme/host/path matching againstWEBHOOK_ENDPOINT, and optionally forwardWEBHOOK_AUTHas a Bearer token. - Expand unit tests for webhook forwarding, rejection cases, and auth header behavior.
Reviewed changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| server/tests/test_v1.py | Adds/updates server API unit tests around webhook endpoint enforcement and auth forwarding. |
| server/src/testflinger/database.py | Introduces job_exists() helper used by the events endpoint. |
| server/src/testflinger/api/v1.py | Implements webhook endpoint validation, auth header forwarding, and improved error handling for the events endpoint. |
| server/schemas/openapi.json | Updates OpenAPI description to reflect new webhook behavior/docs. |
| server/charm/uv.lock | Adds Pydantic to charm dependency lockfile. |
| server/charm/tests/unit/test_config.py | New unit tests for typed charm config validation. |
| server/charm/src/config.py | Adds TestflingerServerConfig Pydantic model and validators. |
| server/charm/src/charm.py | Switches charm to typed config and exports WEBHOOK_ENDPOINT / WEBHOOK_AUTH to workload env. |
| server/charm/pyproject.toml | Adds Pydantic dependency; minor formatting tweak. |
| server/charm/charmcraft.yaml | Adds charm config options for webhook endpoint and auth token. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1015 +/- ##
==========================================
+ Coverage 75.48% 75.61% +0.12%
==========================================
Files 112 113 +1
Lines 11082 11140 +58
Branches 941 951 +10
==========================================
+ Hits 8365 8423 +58
+ Misses 2505 2504 -1
- Partials 212 213 +1
*This pull request uses carry forward flags. Click here to find out more.
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 10 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| parsed_server_url = urlparse(webhook_url) | ||
| parsed_job_url = urlparse(job_webhook) | ||
| if ( | ||
| parsed_server_url.scheme != parsed_job_url.scheme | ||
| or parsed_server_url.netloc != parsed_job_url.netloc | ||
| ): | ||
| abort(HTTPStatus.FORBIDDEN, message="Unauthorized webhook URL") |
There was a problem hiding this comment.
The webhook host check compares urlparse(...).netloc directly. This is overly strict and can incorrectly reject equivalent URLs (e.g., hostname case differences, or an explicit default port like :443 vs no port). Consider comparing normalized components instead (e.g., scheme.lower(), hostname.lower(), and effective port with scheme defaults) so legitimate webhooks aren’t rejected.
edfa3b8 to
79ebfd9
Compare
Description
This PR add support for providing authentication to webhook endpoint via Charm definition.
This follows the pattern discussed and approved in MM to let TF be the source of auth with Test Observer
Resolved issues
Resolves CERTTF-985
Documentation
Web service API changes
@v1.post("/job/<job_id>/events") should remain backward compatibility on current production usage but now it is enforced that the specified webhook in job should match the one configured by server
Tests
Added additional unit tests on charm and server.
Pending to test in staging.