Skip to content

Commit c81e755

Browse files
security: add group principal tests for Schema Registry ACLs
- Fix missing group mapping in from_string_view<principal_type> - Add C++ tests for group principal parsing and authorization - Add Python integration tests for Schema Registry group ACL operations This ensures group-based ACLs work correctly for enterprise customers with the group_based_authorization feature enabled. Signed-off-by: Michael Boquard <michael@redpanda.com>
1 parent 6895e77 commit c81e755

3 files changed

Lines changed: 487 additions & 0 deletions

File tree

src/v/security/acl.cc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -405,6 +405,7 @@ from_string_view<principal_type>(std::string_view str) {
405405
to_string_view(principal_type::ephemeral_user),
406406
principal_type::ephemeral_user)
407407
.match(to_string_view(principal_type::role), principal_type::role)
408+
.match(to_string_view(principal_type::group), principal_type::group)
408409
.default_match(std::nullopt);
409410
}
410411

src/v/security/tests/authorizer_test.cc

Lines changed: 170 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3204,4 +3204,174 @@ TEST(AUTHORIZER_TEST, authz_sr_context_subject_patterns) {
32043204
}
32053205
}
32063206

3207+
// Tests for Group principal parsing and validation
3208+
TEST(AUTHORIZER_TEST, parse_group_principal_from_string_view) {
3209+
// Test the fixed from_string_view<principal_type> function
3210+
auto user_type = from_string_view<principal_type>("user");
3211+
ASSERT_TRUE(user_type.has_value());
3212+
EXPECT_EQ(*user_type, principal_type::user);
3213+
3214+
auto group_type = from_string_view<principal_type>("group");
3215+
ASSERT_TRUE(group_type.has_value());
3216+
EXPECT_EQ(*group_type, principal_type::group);
3217+
3218+
auto role_type = from_string_view<principal_type>("role");
3219+
ASSERT_TRUE(role_type.has_value());
3220+
EXPECT_EQ(*role_type, principal_type::role);
3221+
3222+
auto ephemeral_type = from_string_view<principal_type>("ephemeral user");
3223+
ASSERT_TRUE(ephemeral_type.has_value());
3224+
EXPECT_EQ(*ephemeral_type, principal_type::ephemeral_user);
3225+
3226+
// Test invalid type
3227+
auto invalid_type = from_string_view<principal_type>("invalid");
3228+
EXPECT_FALSE(invalid_type.has_value());
3229+
}
3230+
3231+
TEST(AUTHORIZER_TEST, parse_group_principal_from_string) {
3232+
// Test basic Group principal parsing
3233+
auto group1 = acl_principal::from_string("Group:developers");
3234+
EXPECT_EQ(group1.type(), principal_type::group);
3235+
EXPECT_EQ(group1.name(), "developers");
3236+
3237+
// Test Group with hyphens
3238+
auto group2 = acl_principal::from_string("Group:my-team");
3239+
EXPECT_EQ(group2.type(), principal_type::group);
3240+
EXPECT_EQ(group2.name(), "my-team");
3241+
3242+
// Test Group with underscores
3243+
auto group3 = acl_principal::from_string("Group:team_123");
3244+
EXPECT_EQ(group3.type(), principal_type::group);
3245+
EXPECT_EQ(group3.name(), "team_123");
3246+
3247+
// Test Group with mixed case
3248+
auto group4 = acl_principal::from_string("Group:TeamLead");
3249+
EXPECT_EQ(group4.type(), principal_type::group);
3250+
EXPECT_EQ(group4.name(), "TeamLead");
3251+
3252+
// Test that wildcard groups throw exception (only users can be wildcards)
3253+
EXPECT_THROW(acl_principal::from_string("Group:*"), acl_conversion_error);
3254+
3255+
// Test empty group name throws
3256+
EXPECT_THROW(acl_principal::from_string("Group:"), std::exception);
3257+
3258+
// Verify User principal with wildcard still works
3259+
auto wildcard_user = acl_principal::from_string("User:*");
3260+
EXPECT_EQ(wildcard_user.type(), principal_type::user);
3261+
EXPECT_TRUE(wildcard_user.wildcard());
3262+
3263+
// Verify User principal without wildcard
3264+
auto user1 = acl_principal::from_string("User:alice");
3265+
EXPECT_EQ(user1.type(), principal_type::user);
3266+
EXPECT_EQ(user1.name(), "alice");
3267+
EXPECT_FALSE(user1.wildcard());
3268+
}
3269+
3270+
TEST(AUTHORIZER_TEST, authorize_with_group_principal_acls) {
3271+
// Create test principals
3272+
auto user = acl_principal(principal_type::user, "alice");
3273+
auto group = acl_principal(principal_type::group, "developers");
3274+
auto host = acl_host("192.168.1.1");
3275+
const model::topic topic("dev-topic");
3276+
3277+
// Create ACL for Group:developers
3278+
acl_entry allow_read(
3279+
group, acl_wildcard_host, acl_operation::read, acl_permission::allow);
3280+
3281+
// Create resource and binding
3282+
resource_pattern resource(
3283+
resource_type::topic, topic(), pattern_type::literal);
3284+
std::vector<acl_binding> bindings;
3285+
bindings.emplace_back(resource, allow_read);
3286+
3287+
// Setup authorizer
3288+
role_store roles;
3289+
auto auth = make_test_instance(authorizer::allow_empty_matches::no, &roles);
3290+
auth.add_bindings(bindings);
3291+
3292+
// Test authorization with user who is in the group
3293+
auto result = auth.authorized(
3294+
topic,
3295+
acl_operation::read,
3296+
user,
3297+
host,
3298+
security::superuser_required::no,
3299+
{group});
3300+
3301+
// Verify authorization succeeded
3302+
EXPECT_TRUE(bool(result));
3303+
EXPECT_EQ(result.acl, allow_read);
3304+
EXPECT_EQ(result.group, group);
3305+
EXPECT_EQ(result.principal, user);
3306+
3307+
// Test authorization without group membership fails
3308+
auto result_no_group = auth.authorized(
3309+
topic,
3310+
acl_operation::read,
3311+
user,
3312+
host,
3313+
security::superuser_required::no,
3314+
{});
3315+
3316+
EXPECT_FALSE(bool(result_no_group));
3317+
}
3318+
3319+
TEST(AUTHORIZER_TEST, group_principal_acl_deny_precedence) {
3320+
// Create test principals
3321+
auto user = acl_principal(principal_type::user, "bob");
3322+
auto blocked_group = acl_principal(principal_type::group, "blocked");
3323+
auto host = acl_host("192.168.1.1");
3324+
const model::topic topic("sensitive-topic");
3325+
3326+
// Create DENY ACL for Group:blocked
3327+
acl_entry deny_write(
3328+
blocked_group,
3329+
acl_wildcard_host,
3330+
acl_operation::write,
3331+
acl_permission::deny);
3332+
3333+
// Create ALLOW ACL for User:bob
3334+
acl_entry allow_write(
3335+
user, acl_wildcard_host, acl_operation::write, acl_permission::allow);
3336+
3337+
// Create resource and bindings
3338+
resource_pattern resource(
3339+
resource_type::topic, topic(), pattern_type::literal);
3340+
std::vector<acl_binding> bindings;
3341+
bindings.emplace_back(resource, deny_write);
3342+
bindings.emplace_back(resource, allow_write);
3343+
3344+
// Setup authorizer
3345+
role_store roles;
3346+
auto auth = make_test_instance(authorizer::allow_empty_matches::no, &roles);
3347+
auth.add_bindings(bindings);
3348+
3349+
// Test: User bob is in blocked group - DENY should win
3350+
auto result_with_group = auth.authorized(
3351+
topic,
3352+
acl_operation::write,
3353+
user,
3354+
host,
3355+
security::superuser_required::no,
3356+
{blocked_group});
3357+
3358+
// DENY via group should take precedence over user ALLOW
3359+
EXPECT_FALSE(bool(result_with_group));
3360+
EXPECT_EQ(result_with_group.acl, deny_write);
3361+
EXPECT_EQ(result_with_group.group, blocked_group);
3362+
3363+
// Test: User bob without blocked group - ALLOW should work
3364+
auto result_without_group = auth.authorized(
3365+
topic,
3366+
acl_operation::write,
3367+
user,
3368+
host,
3369+
security::superuser_required::no,
3370+
{});
3371+
3372+
EXPECT_TRUE(bool(result_without_group));
3373+
EXPECT_EQ(result_without_group.acl, allow_write);
3374+
EXPECT_FALSE(result_without_group.group.has_value());
3375+
}
3376+
32073377
} // namespace security

0 commit comments

Comments
 (0)