Skip to content
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

Use API-level proto-to-id helper for audit fields #2692

Conversation

azdagron
Copy link
Member

A few call sites were missed during the the recent change to introduce back-compat on "ensure leading slash" behavior where the audit fields are being populated.

This could cause an API that was accepted to omit the SPIFFE ID or parent ID fields from the audit log.

This change fixes those call sites to use the proper helpers from the API. The IDFromProto method is exported used instead of the more stringent TrustDomain*FromProto methods since audit logging doesn't need that extra validation.

A few call sites were missed during the the recent change to introduce
back-compat on "ensure leading slash" behavior where the audit fields
are being populated.

This could cause an API that was accepted to omit the SPIFFE ID or
parent ID fields from the audit log.

This change fixes those call sites to use the proper helpers from the
API. The IDFromProto method is exported used instead of the more
stringent TrustDomain*FromProto methods since audit logging doesn't need
that extra validation.

Signed-off-by: Andrew Harding <aharding@vmware.com>
Copy link
Member

@evan2645 evan2645 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good catch @azdagron thanks for raising it

🟧 🌃

@azdagron azdagron merged commit e443636 into spiffe:main Jan 21, 2022
@azdagron azdagron mentioned this pull request Jan 21, 2022
@azdagron azdagron added this to the 1.2.0 milestone Jan 21, 2022
@azdagron azdagron deleted the fix-audit-field-logging-for-paths-without-leading-slash branch January 21, 2022 21:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants