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.

Sign up for GitHub

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

Jump to bottom

[Audit Logging] Audit logging support in authorization engines. #32995

Merged
merged 12 commits into from May 3, 2023
Merged

[Audit Logging] Audit logging support in authorization engines. #32995

merged 12 commits into from May 3, 2023

Conversation

rockspore
Copy link
Contributor

  1. GrpcAuthorizationEngine creates the logger from the given config in its ctor.
  2. Evaluate() invokes audit logging when needed.

@rockspore rockspore added release notes: no Indicates if PR should not be in release notes and removed lang/core labels May 3, 2023
markdroth
markdroth reviewed May 3, 2023
View reviewed changes
Copy link
Member

@markdroth markdroth left a comment

Choose a reason for hiding this comment

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

This looks good overall, comments are minor.

Please let me know if you have any questions. Thanks!

src/core/ext/filters/rbac/rbac_service_config_parser.cc Show resolved Hide resolved
src/core/lib/security/authorization/grpc_authorization_engine.cc Outdated Show resolved Hide resolved
src/core/lib/security/authorization/grpc_authorization_engine.cc Outdated Show resolved Hide resolved
test/core/security/grpc_authorization_engine_test.cc Show resolved Hide resolved
test/core/security/grpc_authorization_engine_test.cc Outdated Show resolved Hide resolved
rockspore
rockspore commented May 3, 2023
View reviewed changes
Copy link
Contributor Author

@rockspore rockspore left a comment

Choose a reason for hiding this comment

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

Thanks for the quick review, Mark!

src/core/ext/filters/rbac/rbac_service_config_parser.cc Show resolved Hide resolved
src/core/lib/security/authorization/grpc_authorization_engine.cc Outdated Show resolved Hide resolved
src/core/lib/security/authorization/grpc_authorization_engine.cc Outdated Show resolved Hide resolved
test/core/security/grpc_authorization_engine_test.cc Outdated Show resolved Hide resolved
test/core/security/grpc_authorization_engine_test.cc Show resolved Hide resolved
gtcooke94
gtcooke94 approved these changes May 3, 2023
View reviewed changes
Copy link
Contributor

@gtcooke94 gtcooke94 left a comment

Choose a reason for hiding this comment

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

This LGTM

@rockspore rockspore requested a review from markdroth May 3, 2023 20:10
@rockspore rockspore changed the title [Audit Logging] Audit logging support in authorization engines [Audit Logging] Audit logging support in authorization engines. May 3, 2023
@rockspore rockspore merged commit abc82b9 into grpc:master May 3, 2023
63 of 64 checks passed
@rockspore rockspore deleted the authz-engine branch May 3, 2023 21:07
@copybara-service copybara-service bot added the imported Specifies if the PR has been imported to the internal repository label May 3, 2023
paulosjca pushed a commit to paulosjca/grpc that referenced this pull request May 4, 2023
@rockspore @paulosjca
[Audit Logging] Audit logging support in authorization engines. (grpc…
a1a37b7 
…#32995)

1. `GrpcAuthorizationEngine` creates the logger from the given config in
its ctor.
2. `Evaluate()` invokes audit logging when needed.

---------

Co-authored-by: rockspore <rockspore@users.noreply.github.com>
wanlin31 pushed a commit that referenced this pull request May 18, 2023
@rockspore @wanlin31
[Audit Logging] Audit logging support in authorization engines. (#32995)
e3ddead 
1. `GrpcAuthorizationEngine` creates the logger from the given config in
its ctor.
2. `Evaluate()` invokes audit logging when needed.

---------

Co-authored-by: rockspore <rockspore@users.noreply.github.com>
@yijiem yijiem added release notes: yes Indicates if PR needs to be in release notes and removed release notes: no Indicates if PR should not be in release notes labels May 31, 2023
@erm-g erm-g removed the release notes: yes Indicates if PR needs to be in release notes label Jun 12, 2023
@yijiem yijiem added the release notes: no Indicates if PR should not be in release notes label Jun 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gtcooke94 gtcooke94 gtcooke94 approved these changes

@markdroth markdroth markdroth approved these changes

@erm-g erm-g Awaiting requested review from erm-g

Assignees
No one assigned
Labels
bloat/none imported Specifies if the PR has been imported to the internal repository lang/core per-call-memory/neutral per-channel-memory/neutral release notes: no Indicates if PR should not be in release notes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@rockspore @gtcooke94 @markdroth @yijiem @erm-g