-
Notifications
You must be signed in to change notification settings - Fork 707
CORE-14880: Migrate "OIDC" Admin API v1 Security Endpoints to v2 ConnectRPC API #28973
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
CORE-14880: Migrate "OIDC" Admin API v1 Security Endpoints to v2 ConnectRPC API #28973
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR migrates OIDC-related Admin API endpoints from v1 to v2 using ConnectRPC, introducing the new security service for Admin API v2. The implementation provides functional parity with v1 endpoints while adding proper cluster-wide coordination and error aggregation.
Key changes:
- Introduces v2 SecurityService with three OIDC RPCs: identity resolution, cluster-wide key refresh, and session revocation
- Implements cluster-wide broadcasting for key refresh and session revocation operations with aggregated error handling
- Adds comprehensive integration tests validating v2 endpoints alongside existing v1 tests
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/rptest/tests/redpanda_oauth_test.py | Adds v2 endpoint tests (resolve identity, refresh keys, revoke sessions) and refactors metrics helpers for reuse |
| src/v/redpanda/application.cc | Registers the new security service in the admin server |
| src/v/redpanda/admin/services/security.h | Defines security service interface with OIDC RPC methods |
| src/v/redpanda/admin/services/security.cc | Implements security service with cluster-wide operation broadcasting and error aggregation |
| src/v/redpanda/admin/services/BUILD | Adds build target for security service library |
| src/v/redpanda/BUILD | Adds security service dependency to redpanda application |
| proto/redpanda/core/admin/v2/security.proto | Defines protobuf schema for security service RPCs |
| proto/redpanda/core/admin/v2/BUILD | Adds build target for security proto library |
| security::oidc::authenticator auth{_controller->get_oidc_service().local()}; | ||
| auto res = auth.authenticate( | ||
| auth_result->get_password().substr(authz_bearer_prefix.length())); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: This seems redundant to re-authenticate the token (which shoudl have already been authenticated) to just get the expiration timeout. I wonder if the you could either expand the auth_result to also contain that information or maybe expose something in security that can do a simple parse of the provided token.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Definitely agree, maybe we can expand auth_result to include an auth_payload or something and use that instead instead of having to decode the token again - I'd like to explore that in a follow-on PR if that's okay.
4e7fbc2 to
46c6bfc
Compare
|
Force push to address PR comments:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated 6 comments.
tests/rptest/clients/admin/proto/redpanda/core/admin/v2/security_pb2_connect.py
Show resolved
Hide resolved
tests/rptest/clients/admin/proto/redpanda/core/admin/v2/security_pb2_connect.py
Show resolved
Hide resolved
Retry command for Build#77928please wait until all jobs are finished before running the slash command |
46c6bfc to
2577d2f
Compare
|
Force push to rebase on latest dev. |
2577d2f to
b7104cf
Compare
|
Force push - needed to re-generate ducktape protos with updated proto documentation. |
michael-redpanda
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks really nice!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.
| from . import broker_pb2 | ||
| from . import cluster_pb2 | ||
| from . import internal | ||
| from . import kafka_connections_pb2 | ||
| from . import security_pb2 | ||
| from . import shadow_link_pb2 |
Copilot
AI
Dec 16, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The internal import was removed. If this module exists and is used elsewhere in the codebase, this removal could break imports. However, since this appears to be part of a refactoring to add the security module and the file is a .pyi stub file, this may be intentional cleanup of an unused import.
rockwotj
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Love how this has been simplified since the first pass. Just one additional nit on top of Mike's feedback.
b7104cf to
3130548
Compare
Creating a new proto file for security-related messages in Admin API v2. This new proto initially includes definitions for OIDC-related services and messages.
This commit adds the skeleton implementation of the OIDC security service for the Redpanda Admin API v2. The service includes unimplemented stubs for the following methods: - resolve_oidc_identity - refresh_oidc_keys - revoke_oidc_sessions
3130548 to
1024d21
Compare
|
Generate Ducktape protobuf clients to test the new Admin API v2 OIDC security endpoints.
1024d21 to
f8d54f5
Compare
|
Force push: accidentally forgot to include changes to |
Add OIDC identity resolution to Admin API v2, updates application wiring, and extends oauth tests to validate the new functionality.
Implement refresh_oidc_keys RPC to refresh OIDC keys by calling any broker. The receiving broker executes the refresh on all local shards and proxies/broadcasts the request to all other nodes. Brokers receiving proxied calls only execute locally and do not rebroadcast to other nodes. The originating node aggregates failures across all nodes/shards and returns an aggregated error if any occurred, otherwise success. Add tests ensuring a single-node call refreshes OIDC keys across the cluster.
Implement RevokeOidcSessions RPC to refresh OIDC keys and revoke all active OIDC sessions across the cluster. The receiving broker refreshes OIDC keys on all local shards, revokes OIDC SASL sessions with kafka::server, and broadcasts the request to all other nodes in the cluster, with each broker performing local key refresh and session revocation. The originating node aggregates errors across all brokers and returns an aggregated error if any node fails, otherwise success. Add comprehensive test coverage validating session revocation with various authentication scenarios. This enables administrators to immediately invalidate all OIDC sessions when security policies change or credentials are compromised.
f8d54f5 to
17b7d07
Compare
|
Force push to use |
rockwotj
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, will leave the final approval to Mike
michael-redpanda
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm!
This PR introduces the security service for Redpanda Admin API v2. Starting with the OIDC-related RPCs, this v2 service initially aims to achieve functional parity with the v1 API by providing the following:
The implementation has the following characteristics:
Backports Required
Release Notes
Features