security/tests: add GBAC group claim edge case and nested group tests by nguyen-andrew · Pull Request #29747 · redpanda-data/redpanda

nguyen-andrew · 2026-03-03T22:32:26Z

This PR adds automated test coverage for GBAC (Group-Based Access Control) scenarios that were previously only manually verified. It covers edge cases in OIDC group claim parsing, group name ACL matching, and nested group path behavior.

The scenarios that these cover include:

Test ID	File
GBAC-GRP-PATH-040	oidc_principal_mapping_test.cc (test_get_group_claim_object_type)
GBAC-GRP-BAD-010	oidc_principal_mapping_test.cc (test_get_group_claim_semicolon_delimited)
GBAC-GRP-BAD-030	Covered by same test as GRP-PATH-040
GBAC-GRP-FMT-040	oidc_principal_mapping_test.cc (test_get_group_claim_csv_empty_entries)
GBAC-GRP-NAME-020	oidc_principal_mapping_test.cc (test_get_group_claim_unicode)
GBAC-GRP-NAME-030	oidc_principal_mapping_test.cc (test_get_group_claim_special_chars)
GBAC-GRP-NAME-040	oidc_principal_mapping_test.cc (test_get_group_claim_comma_in_array_element)
GBAC-GRP-NAME-050	oidc_principal_mapping_test.cc (test_get_group_claim_comma_in_csv_is_split)
GBAC-GRP-NAME-070	oidc_principal_mapping_test.cc (test_get_group_claim_duplicates)
GBAC-GRP-NAME-080	oidc_principal_mapping_test.cc (test_get_group_claim_whitespace_chars) + authorizer_test.cc (group_authz_whitespace_chars_in_name)
GBAC-GRP-NAME-010	authorizer_test.cc (group_authz_case_sensitive)
GBAC-NEST-SFX-030	oidc_principal_mapping_test.cc (test_apply_nested_group_policy_suffix_collision)
GBAC-NEST-NONE-020	redpanda_oauth_test.py (test_nested_group_behavior_none_requires_full_path)

Backports Required

none - not a bug fix

Release Notes

none

Unit tests cover group claim parsing edge cases (object-type claims, semicolon-delimited strings, empty CSV entries, unicode, special characters, duplicates, control chars) and authorizer group name matching (case sensitivity, whitespace). A new ducktape test verifies that nested_group_behavior=none requires the full Keycloak group path to match the ACL principal exactly.

Copilot

Pull request overview

Adds automated coverage for GBAC (Group-Based Access Control) edge cases that were previously manually validated, focusing on OIDC group-claim parsing, group name matching behavior, and nested group path handling.

Changes:

Add a new end-to-end test verifying nested_group_behavior=none requires the full group path to match ACL principals.
Add a Keycloak admin helper for adding a service account user to a group by group-id (supports nested-group setup).
Expand C++ unit tests for OIDC group-claim parsing edge cases and authorizer group principal matching behavior.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File	Description
`tests/rptest/tests/redpanda_oauth_test.py`	Adds ACL propagation helper and an E2E test for nested group behavior when full paths are required.
`tests/rptest/services/keycloak.py`	Adds a Keycloak admin utility to add the service account user to a group via group-id.
`src/v/security/tests/oidc_principal_mapping_test.cc`	Adds unit tests covering additional group claim parsing edge cases and nested suffix collision behavior.
`src/v/security/tests/authorizer_test.cc`	Adds tests validating case-sensitive group matching and whitespace characters in group names.

vbotbuildovich · 2026-03-03T23:58:31Z

CI test results

test results on build#81359

test_class	test_method	test_arguments	test_kind	job_url	test_status	passed	reason	test_history
PartitionBalancerTest	test_recovery_mode_rebalance_finish	null	integration	https://buildkite.com/redpanda/redpanda/builds/81359#019cb5de-f997-4c54-b27a-9182a3160e4a	FLAKY	10/11	Test PASSES after retries.No significant increase in flaky rate(baseline=0.0000, p0=1.0000, reject_threshold=0.0100. adj_baseline=0.1000, p1=0.3487, trust_threshold=0.5000)	https://redpanda.metabaseapp.com/dashboard/87-tests?tab=142-dt-individual-test-history&test_class=PartitionBalancerTest&test_method=test_recovery_mode_rebalance_finish
RedpandaNodeOperationsSmokeTest	test_node_ops_smoke_test	{"cloud_storage_type": 1, "mixed_versions": true}	integration	https://buildkite.com/redpanda/redpanda/builds/81359#019cb5df-6312-424b-9cad-1373c6f6693d	FLAKY	10/11	Test PASSES after retries.No significant increase in flaky rate(baseline=0.0388, p0=1.0000, reject_threshold=0.0100. adj_baseline=0.1118, p1=0.3055, trust_threshold=0.5000)	https://redpanda.metabaseapp.com/dashboard/87-tests?tab=142-dt-individual-test-history&test_class=RedpandaNodeOperationsSmokeTest&test_method=test_node_ops_smoke_test

pgellert · 2026-03-04T11:45:33Z

src/v/security/tests/oidc_principal_mapping_test.cc

+        "groups": "admin,,users"
+    })");
+    BOOST_REQUIRE(jwt.has_value());
+
+    json::Pointer group_pointer("/groups");
+    auto result = security::oidc::detail::get_group_claim(
+      group_pointer, jwt.assume_value());
+
+    BOOST_REQUIRE(result.has_value());
+    auto groups = std::move(result).value();
+    // absl::StrSplit keeps empty entries — "admin,,users" produces
+    // ["admin", "", "users"]. Empty group names are not filtered.
+    BOOST_REQUIRE_EQUAL(groups.size(), 3);
+    BOOST_CHECK_EQUAL(groups[0], "admin");
+    BOOST_CHECK_EQUAL(groups[1], "");
+    BOOST_CHECK_EQUAL(groups[2], "users");


What does "Ignored or denied (documented behavior)" from the test wiki mean here? From what I can tell it is not ignored / denied here, no?

Right, this is another good point of discussion. The current implementation does not filter out groups (e.g. empty groups or duplicate groups, as mentioned in the other comment thread) - it's produces a list of groups that is 1-1 to the groups that we receive in the token. Generally, I think that's simpler both in terms of implementation and operations (e.g. if a user is having issues with GBAC, they can call the ResolveOidcIdentity/whoami endpoint and see their list of groups and know that it corresponds 1-1 to the groups that the IdP gave them in their bearer token).

In terms of empty groups in the authz path, I believe they'll be ignored/no-ops since we cannot create ACLs with empty names.

What do ya'll think about keeping this behavior and updating the wiki to reflect? @cjayani

@nguyen-andrew Yeah makes sense, some of the expected results on the tests wiki were assumptions we made due to unavailability of the required implementation details at the time. Let's update the tests wiki to reflect the actual behavior

pgellert · 2026-03-04T11:48:49Z

src/v/security/tests/oidc_principal_mapping_test.cc

+    // Implementation does not deduplicate
+    BOOST_REQUIRE_EQUAL(groups.size(), 3);
+    BOOST_CHECK_EQUAL(groups[0], "admin");
+    BOOST_CHECK_EQUAL(groups[1], "admin");
+    BOOST_CHECK_EQUAL(groups[2], "users");


Should these be deduplicated based on the wiki?

I wonder if the wiki should be updated instead (to say that it does not deduplicate). From an authz standpoint, I don't think duplicates matter. If a group is duplicated and also has an acl match, the first duplicate will trigger the match. If a group is duplicated and does not have an acl match, neither the first nor any of the duplicates will trigger the match. What do you think @cjayani ?

Yeah agreed, let's update the wiki.

pgellert · 2026-03-04T11:52:14Z

src/v/security/tests/oidc_principal_mapping_test.cc

+// Multiple nested groups collide to same suffix
+BOOST_AUTO_TEST_CASE(test_apply_nested_group_policy_suffix_collision) {
+    auto p1 = security::oidc::detail::apply_nested_group_policy(
+      "deptA/admin", security::oidc::nested_group_behavior::suffix);
+    auto p2 = security::oidc::detail::apply_nested_group_policy(
+      "deptB/admin", security::oidc::nested_group_behavior::suffix);
+
+    // Both resolve to "admin"
+    BOOST_CHECK_EQUAL(p1.name(), "admin");
+    BOOST_CHECK_EQUAL(p2.name(), "admin");
+    BOOST_CHECK_EQUAL(p1.name(), p2.name());
+}


nit: Should this test be at the security::oidc::detail::get_group_claim API level or some higher level?

nguyen-andrew requested review from a team and cjayani March 3, 2026 22:32

nguyen-andrew self-assigned this Mar 3, 2026

nguyen-andrew requested review from Copilot and pgellert and removed request for a team March 3, 2026 22:32

github-actions bot added the area/redpanda label Mar 3, 2026

Copilot started reviewing on behalf of nguyen-andrew March 3, 2026 22:32 View session

Copilot AI reviewed Mar 3, 2026

View reviewed changes

pgellert reviewed Mar 4, 2026

View reviewed changes

cjayani added the claude-review Adding this label to a PR will trigger a workflow to review the code using claude. label Mar 4, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

security/tests: add GBAC group claim edge case and nested group tests#29747

security/tests: add GBAC group claim edge case and nested group tests#29747
nguyen-andrew wants to merge 1 commit intoredpanda-data:devfrom
nguyen-andrew:gbac/testing

nguyen-andrew commented Mar 3, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

vbotbuildovich commented Mar 3, 2026

Uh oh!

pgellert Mar 4, 2026

Uh oh!

nguyen-andrew Mar 4, 2026

Uh oh!

cjayani Mar 4, 2026

Uh oh!

pgellert Mar 4, 2026

Uh oh!

nguyen-andrew Mar 4, 2026

Uh oh!

cjayani Mar 4, 2026

Uh oh!

pgellert Mar 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

nguyen-andrew commented Mar 3, 2026

Backports Required

Release Notes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

vbotbuildovich commented Mar 3, 2026

CI test results

Uh oh!

pgellert Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

nguyen-andrew Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

cjayani Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

pgellert Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

nguyen-andrew Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

cjayani Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

pgellert Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants