Skip to content

Commit 43cfd1a

Browse files
committed
accept codex review comment
1 parent 7dbfe64 commit 43cfd1a

File tree

2 files changed

+183
-85
lines changed

2 files changed

+183
-85
lines changed

fe/fe-core/src/main/java/org/apache/doris/service/FrontendServiceImpl.java

Lines changed: 99 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1237,9 +1237,10 @@ private void checkSingleTablePasswordAndPrivs(String user, String passwd, String
12371237
* @param tbl the table name
12381238
* @param clientIp the client IP address
12391239
* @param predicate the required privilege
1240+
* @return true if password verification can be skipped, false otherwise
12401241
* @throws AuthenticationException if authentication or authorization fails
12411242
*/
1242-
private void checkCertBasedAuthAndPrivs(String user, TCertBasedAuth certAuth, String db, String tbl,
1243+
private boolean checkCertBasedAuthAndPrivs(String user, TCertBasedAuth certAuth, String db, String tbl,
12431244
String clientIp, PrivPredicate predicate) throws AuthenticationException {
12441245
final String fullUserName = ClusterNamespace.getNameFromFullName(user);
12451246

@@ -1275,6 +1276,11 @@ private void checkCertBasedAuthAndPrivs(String user, TCertBasedAuth certAuth, St
12751276
"Access denied; you need (at least one of) the (" + predicate.toString()
12761277
+ ") privilege(s) for this operation");
12771278
}
1279+
1280+
// Determine if password verification can be skipped:
1281+
// 1. The user has TLS requirements configured (e.g., REQUIRE SAN)
1282+
// 2. The system is configured to skip password verification
1283+
return userIdentity.hasTlsRequirements() && certVerifier.shouldSkipPasswordVerification();
12781284
}
12791285

12801286
private void checkDbPasswordAndPrivs(String user, String passwd, String db, String clientIp,
@@ -1370,10 +1376,13 @@ private TLoadTxnBeginResult loadTxnBeginImpl(TLoadTxnBeginRequest request, Strin
13701376
} else if (Strings.isNullOrEmpty(request.getToken())) {
13711377
// Check if certificate-based authentication is provided
13721378
if (request.isSetCertBasedAuth() && request.getCertBasedAuth().isSetSan()) {
1373-
checkCertBasedAuthAndPrivs(request.getUser(), request.getCertBasedAuth(),
1379+
boolean canSkipPassword = checkCertBasedAuthAndPrivs(request.getUser(), request.getCertBasedAuth(),
13741380
request.getDb(), request.getTbl(), request.getUserIp(), PrivPredicate.LOAD);
1381+
if (!canSkipPassword) {
1382+
checkSingleTablePasswordAndPrivs(request.getUser(), request.getPasswd(), request.getDb(),
1383+
request.getTbl(), request.getUserIp(), PrivPredicate.LOAD);
1384+
}
13751385
} else {
1376-
// Fall back to password-based authentication
13771386
checkSingleTablePasswordAndPrivs(request.getUser(), request.getPasswd(), request.getDb(),
13781387
request.getTbl(), request.getUserIp(), PrivPredicate.LOAD);
13791388
}
@@ -1638,21 +1647,38 @@ private void loadTxnPreCommitImpl(TLoadTxnCommitRequest request) throws UserExce
16381647
// Check if certificate-based authentication is provided
16391648
boolean useCertAuth = request.isSetCertBasedAuth() && request.getCertBasedAuth().isSetSan();
16401649

1641-
// refactoring it
16421650
if (CollectionUtils.isNotEmpty(request.getTbls())) {
1643-
for (String tbl : request.getTbls()) {
1644-
if (useCertAuth) {
1651+
if (useCertAuth) {
1652+
// Check first table to determine if password can be skipped
1653+
String firstTbl = request.getTbls().get(0);
1654+
boolean canSkipPassword = checkCertBasedAuthAndPrivs(request.getUser(), request.getCertBasedAuth(),
1655+
request.getDb(), firstTbl, request.getUserIp(), PrivPredicate.LOAD);
1656+
// Check remaining tables with cert auth (privileges already checked in checkCertBasedAuthAndPrivs)
1657+
for (int i = 1; i < request.getTbls().size(); i++) {
16451658
checkCertBasedAuthAndPrivs(request.getUser(), request.getCertBasedAuth(),
1646-
request.getDb(), tbl, request.getUserIp(), PrivPredicate.LOAD);
1647-
} else {
1659+
request.getDb(), request.getTbls().get(i), request.getUserIp(), PrivPredicate.LOAD);
1660+
}
1661+
if (!canSkipPassword) {
1662+
// Still need password verification for all tables
1663+
for (String tbl : request.getTbls()) {
1664+
checkSingleTablePasswordAndPrivs(request.getUser(), request.getPasswd(), request.getDb(),
1665+
tbl, request.getUserIp(), PrivPredicate.LOAD);
1666+
}
1667+
}
1668+
} else {
1669+
for (String tbl : request.getTbls()) {
16481670
checkSingleTablePasswordAndPrivs(request.getUser(), request.getPasswd(), request.getDb(),
16491671
tbl, request.getUserIp(), PrivPredicate.LOAD);
16501672
}
16511673
}
16521674
} else {
16531675
if (useCertAuth) {
1654-
checkCertBasedAuthAndPrivs(request.getUser(), request.getCertBasedAuth(),
1676+
boolean canSkipPassword = checkCertBasedAuthAndPrivs(request.getUser(), request.getCertBasedAuth(),
16551677
request.getDb(), request.getTbl(), request.getUserIp(), PrivPredicate.LOAD);
1678+
if (!canSkipPassword) {
1679+
checkSingleTablePasswordAndPrivs(request.getUser(), request.getPasswd(), request.getDb(),
1680+
request.getTbl(), request.getUserIp(), PrivPredicate.LOAD);
1681+
}
16561682
} else {
16571683
checkSingleTablePasswordAndPrivs(request.getUser(), request.getPasswd(), request.getDb(),
16581684
request.getTbl(), request.getUserIp(), PrivPredicate.LOAD);
@@ -1759,12 +1785,24 @@ private void loadTxn2PCImpl(TLoadTxn2PCRequest request) throws UserException {
17591785
// Check if certificate-based authentication is provided
17601786
boolean useCertAuth = request.isSetCertBasedAuth() && request.getCertBasedAuth().isSetSan();
17611787

1762-
for (Table table : tableList) {
1763-
// check auth
1764-
if (useCertAuth) {
1788+
if (useCertAuth && !tableList.isEmpty()) {
1789+
// Check first table to determine if password can be skipped
1790+
boolean canSkipPassword = checkCertBasedAuthAndPrivs(request.getUser(), request.getCertBasedAuth(),
1791+
request.getDb(), tableList.get(0).getName(), request.getUserIp(), PrivPredicate.LOAD);
1792+
// Check remaining tables with cert auth
1793+
for (int i = 1; i < tableList.size(); i++) {
17651794
checkCertBasedAuthAndPrivs(request.getUser(), request.getCertBasedAuth(),
1766-
request.getDb(), table.getName(), request.getUserIp(), PrivPredicate.LOAD);
1767-
} else {
1795+
request.getDb(), tableList.get(i).getName(), request.getUserIp(), PrivPredicate.LOAD);
1796+
}
1797+
if (!canSkipPassword) {
1798+
// Still need password verification for all tables
1799+
for (Table table : tableList) {
1800+
checkSingleTablePasswordAndPrivs(request.getUser(), request.getPasswd(), request.getDb(),
1801+
table.getName(), request.getUserIp(), PrivPredicate.LOAD);
1802+
}
1803+
}
1804+
} else {
1805+
for (Table table : tableList) {
17681806
checkSingleTablePasswordAndPrivs(request.getUser(), request.getPasswd(), request.getDb(),
17691807
table.getName(), request.getUserIp(), PrivPredicate.LOAD);
17701808
}
@@ -1836,18 +1874,32 @@ private boolean loadTxnCommitImpl(TLoadTxnCommitRequest request) throws UserExce
18361874

18371875
if (CollectionUtils.isNotEmpty(request.getTbls())) {
18381876
if (useCertAuth) {
1839-
for (String tbl : request.getTbls()) {
1877+
// Check first table to determine if password can be skipped
1878+
String firstTbl = request.getTbls().get(0);
1879+
boolean canSkipPassword = checkCertBasedAuthAndPrivs(request.getUser(), request.getCertBasedAuth(),
1880+
request.getDb(), firstTbl, request.getUserIp(), PrivPredicate.LOAD);
1881+
// Check remaining tables with cert auth
1882+
for (int i = 1; i < request.getTbls().size(); i++) {
18401883
checkCertBasedAuthAndPrivs(request.getUser(), request.getCertBasedAuth(),
1841-
request.getDb(), tbl, request.getUserIp(), PrivPredicate.LOAD);
1884+
request.getDb(), request.getTbls().get(i), request.getUserIp(), PrivPredicate.LOAD);
1885+
}
1886+
if (!canSkipPassword) {
1887+
// Still need password verification for all tables
1888+
checkPasswordAndPrivs(request.getUser(), request.getPasswd(), request.getDb(),
1889+
request.getTbls(), request.getUserIp(), PrivPredicate.LOAD);
18421890
}
18431891
} else {
18441892
checkPasswordAndPrivs(request.getUser(), request.getPasswd(), request.getDb(),
18451893
request.getTbls(), request.getUserIp(), PrivPredicate.LOAD);
18461894
}
18471895
} else {
18481896
if (useCertAuth) {
1849-
checkCertBasedAuthAndPrivs(request.getUser(), request.getCertBasedAuth(),
1897+
boolean canSkipPassword = checkCertBasedAuthAndPrivs(request.getUser(), request.getCertBasedAuth(),
18501898
request.getDb(), request.getTbl(), request.getUserIp(), PrivPredicate.LOAD);
1899+
if (!canSkipPassword) {
1900+
checkSingleTablePasswordAndPrivs(request.getUser(), request.getPasswd(), request.getDb(),
1901+
request.getTbl(), request.getUserIp(), PrivPredicate.LOAD);
1902+
}
18511903
} else {
18521904
checkSingleTablePasswordAndPrivs(request.getUser(), request.getPasswd(), request.getDb(),
18531905
request.getTbl(), request.getUserIp(), PrivPredicate.LOAD);
@@ -2073,19 +2125,37 @@ private void loadTxnRollbackImpl(TLoadTxnRollbackRequest request) throws UserExc
20732125

20742126
// multi table load
20752127
if (CollectionUtils.isNotEmpty(request.getTbls())) {
2076-
for (String tbl : request.getTbls()) {
2077-
if (useCertAuth) {
2128+
if (useCertAuth) {
2129+
// Check first table to determine if password can be skipped
2130+
String firstTbl = request.getTbls().get(0);
2131+
boolean canSkipPassword = checkCertBasedAuthAndPrivs(request.getUser(), request.getCertBasedAuth(),
2132+
request.getDb(), firstTbl, request.getUserIp(), PrivPredicate.LOAD);
2133+
// Check remaining tables with cert auth
2134+
for (int i = 1; i < request.getTbls().size(); i++) {
20782135
checkCertBasedAuthAndPrivs(request.getUser(), request.getCertBasedAuth(),
2079-
request.getDb(), tbl, request.getUserIp(), PrivPredicate.LOAD);
2080-
} else {
2136+
request.getDb(), request.getTbls().get(i), request.getUserIp(), PrivPredicate.LOAD);
2137+
}
2138+
if (!canSkipPassword) {
2139+
// Still need password verification for all tables
2140+
for (String tbl : request.getTbls()) {
2141+
checkSingleTablePasswordAndPrivs(request.getUser(), request.getPasswd(), request.getDb(),
2142+
tbl, request.getUserIp(), PrivPredicate.LOAD);
2143+
}
2144+
}
2145+
} else {
2146+
for (String tbl : request.getTbls()) {
20812147
checkSingleTablePasswordAndPrivs(request.getUser(), request.getPasswd(), request.getDb(),
20822148
tbl, request.getUserIp(), PrivPredicate.LOAD);
20832149
}
20842150
}
20852151
} else {
20862152
if (useCertAuth) {
2087-
checkCertBasedAuthAndPrivs(request.getUser(), request.getCertBasedAuth(),
2153+
boolean canSkipPassword = checkCertBasedAuthAndPrivs(request.getUser(), request.getCertBasedAuth(),
20882154
request.getDb(), request.getTbl(), request.getUserIp(), PrivPredicate.LOAD);
2155+
if (!canSkipPassword) {
2156+
checkSingleTablePasswordAndPrivs(request.getUser(), request.getPasswd(), request.getDb(),
2157+
request.getTbl(), request.getUserIp(), PrivPredicate.LOAD);
2158+
}
20892159
} else {
20902160
checkSingleTablePasswordAndPrivs(request.getUser(), request.getPasswd(), request.getDb(),
20912161
request.getTbl(), request.getUserIp(), PrivPredicate.LOAD);
@@ -2366,8 +2436,14 @@ private void httpStreamPutImpl(TStreamLoadPutRequest request, TStreamLoadPutResu
23662436
} else if (Strings.isNullOrEmpty(request.getToken())) {
23672437
// Check if certificate-based authentication is provided
23682438
if (request.isSetCertBasedAuth() && request.getCertBasedAuth().isSetSan()) {
2369-
checkCertBasedAuthAndPrivs(request.getUser(), request.getCertBasedAuth(),
2439+
boolean canSkipPassword = checkCertBasedAuthAndPrivs(request.getUser(), request.getCertBasedAuth(),
23702440
request.getDb(), request.getTbl(), request.getUserIp(), PrivPredicate.LOAD);
2441+
if (!canSkipPassword) {
2442+
// User doesn't have TLS requirements or password skipping is disabled,
2443+
// still need to verify password
2444+
checkSingleTablePasswordAndPrivs(request.getUser(), request.getPasswd(), request.getDb(),
2445+
request.getTbl(), request.getUserIp(), PrivPredicate.LOAD);
2446+
}
23712447
} else {
23722448
checkSingleTablePasswordAndPrivs(request.getUser(), request.getPasswd(), request.getDb(),
23732449
request.getTbl(), request.getUserIp(), PrivPredicate.LOAD);

0 commit comments

Comments
 (0)