Add missing audit events for metadata updates and verification status #3533
Conversation
|
The latest Buf updates on your PR. Results from workflow Buf CI / buf (pull_request).
|
| message PayloadOrganizationVerificationStatusChanged { | ||
| // old_verification_status is the organization verification status before the update. | ||
| string old_verification_status = 1; | ||
| // new_verification_status is the organization verification status after the update. | ||
| string new_verification_status = 2; | ||
| } |
There was a problem hiding this comment.
This is in a similar spot like this other conversation, the definition of these verification statuses live in registry-proto here, so we don't have a way to reference the right value here.
The options I see are:
- Log the string value of that
enuminregistry-protohere; that doesn't feel right. - Add an equivalent
enumin this pkg, and a parser in the audit log writer that converts from thatregistry-proto'senumto the one in here; it's duplication of types but that doesn't sound too bad I think. - Punt auditing this event until we move this pkg to be colocated in
registry-proto.
This same scenario happens with PayloadUserVerificationStatusChanged.
There was a problem hiding this comment.
I could be wrong, but it feels like this over-indexes on "audit logging" everything available. Do users really care what user changed the description and to what?
Where do we draw the line on what we audit and not?
There was a problem hiding this comment.
In general terms we're leaning on auditing any write operation. Even the original proposal was thinking how we could expand on read operations too, but we haven't gone that far yet.
This PR adds metadata (true it may be not super important, just URLs and descriptions), and verification status changes (they sound pretty important to me, and they're not being audited yet).
Let's chat about it.
There was a problem hiding this comment.
approving from my side but you both resolve this
|
Chatted internally with the team, closing this PR until/if we decide these events are worth auditing. |
No description provided.