fix(personal): fail closed on ambiguous enterprise grants

This commit is contained in:
2026-07-12 16:01:52 +08:00
parent 8cf8da6899
commit c7756f86e2
6 changed files with 110 additions and 23 deletions
@@ -29,11 +29,12 @@ public class OrgSnapshotEnterpriseKnowledgeAccessPolicy implements EnterpriseKno
return Optional.empty();
}
try {
Optional<String> phone = userPhone(owner);
if (phone.isEmpty()) {
Optional<UserIdentity> userIdentity = userIdentity(owner);
if (userIdentity.isEmpty()) {
return denied(owner, "user_phone_missing");
}
Optional<OrganizationIdentity> organization = organization(owner.tenantId(), phone.orElseThrow());
Optional<OrganizationIdentity> organization = organization(owner.tenantId(),
userIdentity.orElseThrow().phone());
if (organization.isEmpty()) {
return denied(owner, "active_org_missing");
}
@@ -51,9 +52,9 @@ public class OrgSnapshotEnterpriseKnowledgeAccessPolicy implements EnterpriseKno
}
}
private Optional<String> userPhone(PersonalOwner owner) {
List<String> phones = jdbcTemplate.query("""
SELECT phonenumber
private Optional<UserIdentity> userIdentity(PersonalOwner owner) {
List<UserIdentity> users = jdbcTemplate.query("""
SELECT user_id, phonenumber
FROM sys_user
WHERE BINARY tenant_id = BINARY ?
AND user_id = ?
@@ -63,8 +64,26 @@ public class OrgSnapshotEnterpriseKnowledgeAccessPolicy implements EnterpriseKno
AND phonenumber <> ''
ORDER BY user_id
LIMIT 1
""", (rs, rowNum) -> rs.getString("phonenumber"), owner.tenantId(), owner.userId());
return phones.stream().map(String::trim).filter(value -> !value.isEmpty()).findFirst();
""", (rs, rowNum) -> new UserIdentity(rs.getLong("user_id"), trimmed(rs.getString("phonenumber"))),
owner.tenantId(), owner.userId());
if (users.size() != 1 || users.get(0).userId() != owner.userId() || users.get(0).phone().isBlank()) {
return Optional.empty();
}
UserIdentity identity = users.get(0);
List<Long> matchingUserIds = jdbcTemplate.query("""
SELECT user_id
FROM sys_user
WHERE BINARY tenant_id = BINARY ?
AND phonenumber = ?
AND status = '0'
AND del_flag = '0'
ORDER BY user_id
LIMIT 2
""", (rs, rowNum) -> rs.getLong("user_id"), owner.tenantId(), identity.phone());
if (matchingUserIds.size() != 1 || matchingUserIds.get(0) != owner.userId()) {
return Optional.empty();
}
return Optional.of(identity);
}
private Optional<OrganizationIdentity> organization(String tenantId, String phone) {
@@ -92,7 +111,7 @@ public class OrgSnapshotEnterpriseKnowledgeAccessPolicy implements EnterpriseKno
private List<Long> authorizedFragmentIds(String tenantId, OrganizationIdentity identity) {
String canonicalPosition = canonicalPosition(identity.positionName());
return jdbcTemplate.query("""
List<Long> fragmentIds = jdbcTemplate.query("""
SELECT DISTINCT f.id AS fragment_id
FROM aihr_knowledge_acl a
JOIN aihr_knowledge_info i
@@ -103,6 +122,7 @@ public class OrgSnapshotEnterpriseKnowledgeAccessPolicy implements EnterpriseKno
AND BINARY f.tenant_id = BINARY a.tenant_id
WHERE BINARY a.tenant_id = BINARY ?
AND a.enabled = 1
AND a.classification = 'INTERNAL'
AND (
a.access_scope = 'TENANT'
OR (a.access_scope = 'PROJECT' AND a.project_code = ?)
@@ -112,14 +132,23 @@ public class OrgSnapshotEnterpriseKnowledgeAccessPolicy implements EnterpriseKno
AND (a.position_level IS NULL OR a.position_level = '' OR a.position_level = ?))
)
ORDER BY f.id ASC
LIMIT 200
LIMIT 201
""", (rs, rowNum) -> rs.getLong("fragment_id"), tenantId, identity.projectCode(),
identity.projectCode(), canonicalPosition, identity.positionLevel()).stream()
.filter(id -> id != null && id > 0)
.distinct()
.sorted()
.limit(MAX_FRAGMENT_GRANTS)
.toList();
return boundedFragmentIds(tenantId, fragmentIds);
}
private List<Long> boundedFragmentIds(String tenantId, List<Long> fragmentIds) {
if (fragmentIds.size() > MAX_FRAGMENT_GRANTS) {
log.warn("enterprise_acl_denied tenant={} reason=fragment_limit_exceeded count={}",
tenantId, fragmentIds.size());
return List.of();
}
return fragmentIds;
}
private Optional<EnterpriseKnowledgeGrant> denied(PersonalOwner owner, String reason) {
@@ -152,4 +181,7 @@ public class OrgSnapshotEnterpriseKnowledgeAccessPolicy implements EnterpriseKno
return !projectCode.isBlank() && !positionName.isBlank();
}
}
private record UserIdentity(long userId, String phone) {
}
}
@@ -104,6 +104,18 @@ class OrgSnapshotEnterpriseKnowledgeAccessPolicyTest {
assertFalse(jdbc.sql.stream().anyMatch(value -> value.contains("FROM aihr_knowledge_acl")));
}
@Test
void duplicateActiveUserAccountsForPhoneFailClosedBeforeOrganizationLookup() {
RecordingJdbcTemplate jdbc = fixture();
jdbc.phoneUserIds = List.of(103L, 104L);
jdbc.fragmentIds = List.of(100101L);
assertTrue(new OrgSnapshotEnterpriseKnowledgeAccessPolicy(jdbc)
.authorize(new PersonalOwner("000000", 103L, null)).isEmpty());
assertFalse(jdbc.sql.stream().anyMatch(value -> value.contains("FROM aihr_org_snapshot")));
assertFalse(jdbc.sql.stream().anyMatch(value -> value.contains("FROM aihr_knowledge_acl")));
}
@Test
void aclQueryEnforcesTenantProjectPositionAndTenantScopes() {
RecordingJdbcTemplate jdbc = fixture();
@@ -115,6 +127,7 @@ class OrgSnapshotEnterpriseKnowledgeAccessPolicyTest {
String aclSql = jdbc.sql.stream().filter(value -> value.contains("FROM aihr_knowledge_acl"))
.findFirst().orElseThrow();
assertTrue(aclSql.contains("a.enabled = 1"));
assertTrue(aclSql.contains("a.classification = 'INTERNAL'"));
assertTrue(aclSql.contains("a.access_scope = 'TENANT'"));
assertTrue(aclSql.contains("a.access_scope = 'PROJECT' AND a.project_code = ?"));
assertTrue(aclSql.contains("a.access_scope = 'POSITION'"));
@@ -123,6 +136,21 @@ class OrgSnapshotEnterpriseKnowledgeAccessPolicyTest {
jdbc.args.get(jdbc.args.size() - 1));
}
@Test
void restrictedClassificationAndCrossTenantAclRowsFailClosed() {
RecordingJdbcTemplate restricted = fixture();
restricted.aclClassification = "RESTRICTED";
restricted.fragmentIds = List.of(100101L);
assertTrue(new OrgSnapshotEnterpriseKnowledgeAccessPolicy(restricted)
.authorize(new PersonalOwner("000000", 103L, null)).isEmpty());
RecordingJdbcTemplate crossTenant = fixture();
crossTenant.aclTenant = "999999";
crossTenant.fragmentIds = List.of(100101L);
assertTrue(new OrgSnapshotEnterpriseKnowledgeAccessPolicy(crossTenant)
.authorize(new PersonalOwner("000000", 103L, null)).isEmpty());
}
@Test
void positionAliasesAreResolvedOnlyOnServer() {
for (String position : List.of("生活顾问", "物业管家", "客服管家")) {
@@ -164,9 +192,18 @@ class OrgSnapshotEnterpriseKnowledgeAccessPolicyTest {
}
@Test
void fragmentGrantIsStableDistinctAndCappedAtTwoHundred() {
void moreThanTwoHundredFragmentsRejectsEntireGrant() {
RecordingJdbcTemplate jdbc = fixture();
List<Long> ids = new ArrayList<>(LongStream.rangeClosed(1, 250).boxed().toList());
jdbc.fragmentIds = LongStream.rangeClosed(1, 201).boxed().toList();
assertTrue(new OrgSnapshotEnterpriseKnowledgeAccessPolicy(jdbc)
.authorize(new PersonalOwner("000000", 103L, null)).isEmpty());
}
@Test
void twoHundredFragmentsAreAllowedWithStableDistinctOrdering() {
RecordingJdbcTemplate jdbc = fixture();
List<Long> ids = new ArrayList<>(LongStream.rangeClosed(1, 200).map(value -> 201 - value).boxed().toList());
ids.add(1L);
jdbc.fragmentIds = ids;
@@ -176,6 +213,7 @@ class OrgSnapshotEnterpriseKnowledgeAccessPolicyTest {
assertEquals(200, grant.allowedFragmentIds().size());
assertEquals(1L, grant.allowedFragmentIds().get(0));
assertEquals(200L, grant.allowedFragmentIds().get(199));
assertTrue(jdbc.sql.get(jdbc.sql.size() - 1).contains("LIMIT 201"));
}
@Test
@@ -203,9 +241,12 @@ class OrgSnapshotEnterpriseKnowledgeAccessPolicyTest {
private final List<List<Object>> args = new ArrayList<>();
private String phone;
private String expectedTenant;
private List<Long> phoneUserIds = List.of(103L);
private Map<String, String> organization;
private List<Map<String, String>> organizations;
private List<Long> fragmentIds = List.of();
private String aclTenant = "000000";
private String aclClassification = "INTERNAL";
private boolean fail;
@Override
@@ -219,7 +260,12 @@ class OrgSnapshotEnterpriseKnowledgeAccessPolicyTest {
if (expectedTenant != null && !expectedTenant.equals(args[0])) {
return List.of();
}
return phone == null ? List.of() : mapRows(rowMapper, List.of(Map.of("phonenumber", phone)));
if (sql.contains("AND phonenumber = ?")) {
return phoneUserIds.stream()
.map(id -> mapRow(rowMapper, Map.of("user_id", id))).toList();
}
return phone == null ? List.of() : mapRows(rowMapper,
List.of(Map.of("user_id", phoneUserIds.get(0), "phonenumber", phone)));
}
if (sql.contains("FROM aihr_org_snapshot")) {
if (organizations != null) {
@@ -228,6 +274,10 @@ class OrgSnapshotEnterpriseKnowledgeAccessPolicyTest {
return organization == null ? List.of() : mapRows(rowMapper, List.of(organization));
}
if (sql.contains("FROM aihr_knowledge_acl")) {
if (!sql.contains("a.classification = 'INTERNAL'")
|| !"INTERNAL".equals(aclClassification) || !args[0].equals(aclTenant)) {
return List.of();
}
return fragmentIds.stream().map(id -> mapRow(rowMapper, Map.of("fragment_id", id))).toList();
}
return List.of();
@@ -34,9 +34,13 @@ class PersonalSchemaContractTest {
assertTrue(acl.contains("key `idx_aihr_knowledge_acl_lookup` (`tenant_id`, `enabled`, `access_scope`)"));
assertTrue(acl.contains("unique key `uk_aihr_knowledge_acl_rule`"));
assertTrue(sql.contains("(11001, '000000', 1001, 'position'"));
assertTrue(sql.contains("(11002, '000000', 1002, 'position'"));
assertTrue(sql.contains("(11003, '000000', 1003, 'position'"));
int seedStart = sql.indexOf("insert into `aihr_knowledge_acl`");
assertTrue(seedStart > 0);
String aclSeed = sql.substring(seedStart, sql.indexOf(';', seedStart));
assertFalse(aclSeed.contains("(`id`, `tenant_id`"), "ACL seed must not reserve fixed primary keys");
assertTrue(aclSeed.contains("('000000', 1001, 'position'"));
assertTrue(aclSeed.contains("('000000', 1002, 'position'"));
assertTrue(aclSeed.contains("('000000', 1003, 'position'"));
assertTrue(sql.contains("'生活顾问', '一线', 'internal', 1"));
assertFalse(sql.contains("'tenant', null, null, null, 'internal', 1"),
"Seed SOP knowledge must not be tenant-wide");