Replace webhook.Principal with auth.Identity#4316
Conversation
The webhook package defined its own Principal struct, but auth.Identity already represents the same concept with a superset of fields. Maintaining two parallel identity types creates sync burden and will require conversion logic in the middleware layer. Since the webhook package has no external consumers yet (v0.1.0), consolidate now before the API solidifies. Closes #4315 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
@Sanskarzz did this one as a follow up of your previous PR introducing the webhook fundamentals. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4316 +/- ##
==========================================
+ Coverage 69.07% 69.10% +0.02%
==========================================
Files 477 477
Lines 48074 48146 +72
==========================================
+ Hits 33207 33269 +62
- Misses 12284 12294 +10
Partials 2583 2583 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
@JAORMX, thanks for the ping! Great catch on the duplicate identity types. Unifying them into |
Address review feedback on the webhook wire format: auth.Identity carries Token, TokenType, and Metadata fields that have no business in external webhook payloads — even redacted. Extract the credential-free subset (Subject, Name, Email, Groups, Claims) into auth.PrincipalInfo and embed it in Identity. This gives structural safety: webhook.Request.Principal is now *auth.PrincipalInfo, so tokens can never enter the webhook payload. The JSON wire format uses "sub" with omitempty, matching the original webhook.Principal contract from #3840. Field access on Identity (e.g., identity.Subject) continues to work transparently via Go embedding — only struct literals needed updating. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
@jhrozek I changed the approach to add a subset of the principal to the auth package itself and make the Identity struct take that into use. I believe this is more re-usable. |
Summary
Principalstruct, butauth.Identityalready represents the same concept with a superset of fields. Maintaining two parallel identity types creates sync burden and will require conversion logic when the middleware layer wires them together.webhook.Principaland changeRequest.Principalto use*auth.Identitydirectly. Update all test fixtures accordingly.Closes #4315
Type of change
Test plan
task test)Changes
pkg/webhook/types.goPrincipalstruct, changeRequest.Principaltype to*auth.Identity, addpkg/authimportpkg/webhook/client_test.go&Principal{Sub: "user1"}with&auth.Identity{Subject: "user1"}(5 instances), addpkg/authimportSpecial notes for reviewers
pkg/authimports only stdlib."sub"to"subject"in the webhook payload. This is acceptable because the webhook API is pre-release (v0.1.0) with no external consumers yet.auth.Identityhas a customMarshalJSONthat redactsToken— this is a feature, not a problem, since tokens should never leak into webhook payloads.auth.Identity.MarshalJSONemits all fields (including empty ones) rather than usingomitempty. This is slightly more verbose but consistent with how identity is serialized elsewhere.Generated with Claude Code