Skip to content

Commit d289944

Browse files
authored
Fixed lock error messages and default values in LoginLockManager. Corrected table model unlock syntax. Resolved errors in security administrator unlock functionality. (apache#16579)
1 parent 32f287a commit d289944

File tree

6 files changed

+59
-65
lines changed

6 files changed

+59
-65
lines changed

integration-test/src/test/java/org/apache/iotdb/auth/it/IoTDBLoginLockManagerIT.java

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,9 @@ public void testUnlockManual() throws Exception {
101101
login("test", "wrong", new String[] {authFailedMsg}, 1);
102102
}
103103
// account was locked
104-
login("test", "test", new String[] {authFailedMsg}, 1);
104+
String lockedMsg =
105+
"ErrorCan't execute sql because822: Account is blocked due to consecutive failed logins.";
106+
login("test", "test", new String[] {lockedMsg}, 1);
105107
// unlock user-lock manual
106108
session.executeNonQueryStatement("ALTER USER test ACCOUNT UNLOCK");
107109
login("test", "test", new String[] {loginSuccessMsg}, 1);
@@ -111,7 +113,7 @@ public void testUnlockManual() throws Exception {
111113
login("test", "wrong", new String[] {authFailedMsg}, 1);
112114
}
113115
// account was locked
114-
login("test", "test", new String[] {authFailedMsg}, 1);
116+
login("test", "test", new String[] {lockedMsg}, 1);
115117
// unlock user-lock manual
116118
session.executeNonQueryStatement("ALTER USER test @ '127.0.0.1' ACCOUNT UNLOCK");
117119
login("test", "test", new String[] {loginSuccessMsg}, 1);

iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/consensus/request/ConfigPhysicalPlanType.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,7 @@ public enum ConfigPhysicalPlanType {
135135
CreateUserWithRawPassword((short) 638),
136136
UpdateUserMaxSession((short) 639),
137137
UpdateUserMinSession((short) 640),
138+
AccountUnlock((short) 641),
138139

139140
/** Table Author */
140141
RCreateUser((short) 641),
@@ -172,6 +173,7 @@ public enum ConfigPhysicalPlanType {
172173
RListRolePrivilege((short) 672),
173174
RUpdateUserMaxSession((short) 673),
174175
RUpdateUserMinSession((short) 674),
176+
RAccountUnlock((short) 675),
175177

176178
/** Function. */
177179
CreateFunction((short) 700),

iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/persistence/auth/AuthorPlanExecutor.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,8 @@ public TSStatus executeAuthorNonQuery(AuthorTreePlan authorPlan) {
146146
case DropRole:
147147
authorizer.deleteRole(roleName);
148148
break;
149+
case AccountUnlock:
150+
break;
149151
case GrantRole:
150152
for (int permission : permissions) {
151153
PrivilegeType priv = PrivilegeType.values()[permission];
@@ -246,6 +248,8 @@ public TSStatus executeRelationalAuthorNonQuery(AuthorRelationalPlan authorPlan)
246248
case RRenameUser:
247249
authorizer.renameUser(userName, newUsername);
248250
break;
251+
case RAccountUnlock:
252+
break;
249253
case RDropRole:
250254
authorizer.deleteRole(roleName);
251255
break;

iotdb-core/confignode/src/main/java/org/apache/iotdb/confignode/persistence/executor/ConfigPlanExecutor.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -461,6 +461,7 @@ public TSStatus executeNonQueryPlan(ConfigPhysicalPlan physicalPlan)
461461
case DropUser:
462462
case DropUserV2:
463463
case DropRole:
464+
case AccountUnlock:
464465
case GrantRole:
465466
case GrantUser:
466467
case GrantRoleToUser:
@@ -493,6 +494,7 @@ public TSStatus executeNonQueryPlan(ConfigPhysicalPlan physicalPlan)
493494
case RUpdateUserV2:
494495
case RUpdateUserMaxSession:
495496
case RUpdateUserMinSession:
497+
case RAccountUnlock:
496498
case RGrantUserRole:
497499
case RGrantRoleAny:
498500
case RGrantUserAny:

iotdb-core/datanode/src/main/java/org/apache/iotdb/db/auth/LoginLockManager.java

Lines changed: 11 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -68,30 +68,27 @@ public LoginLockManager() {
6868
public LoginLockManager(
6969
int failedLoginAttempts, int failedLoginAttemptsPerUser, int passwordLockTimeMinutes) {
7070
// Set and validate failedLoginAttempts (IP level)
71-
if (failedLoginAttempts == -1) {
71+
if (failedLoginAttempts <= 0) {
7272
this.failedLoginAttempts = -1; // Completely disable IP-level restrictions
73+
LOGGER.info("IP-level login attempts disabled (set to {})", failedLoginAttempts);
7374
} else {
74-
this.failedLoginAttempts = failedLoginAttempts >= 1 ? failedLoginAttempts : 5;
75+
this.failedLoginAttempts = failedLoginAttempts;
7576
}
7677

7778
// Set and validate failedLoginAttemptsPerUser (user level)
78-
if (failedLoginAttemptsPerUser == -1) {
79-
// If IP-level is enabled, user-level cannot be disabled
80-
if (this.failedLoginAttempts != -1) {
81-
this.failedLoginAttemptsPerUser = 1000; // Default user-level value
82-
LOGGER.error(
83-
"User-level login attempts cannot be disabled when IP-level is enabled. "
84-
+ "Setting user-level attempts to default (1000)");
85-
} else {
86-
this.failedLoginAttemptsPerUser = -1; // Both are disabled
87-
}
79+
if (failedLoginAttemptsPerUser <= 0) {
80+
this.failedLoginAttemptsPerUser = -1; // Disable user-level restrictions
81+
LOGGER.info("User-level login attempts disabled (set to {})", failedLoginAttemptsPerUser);
8882
} else {
89-
this.failedLoginAttemptsPerUser =
90-
failedLoginAttemptsPerUser >= 1 ? failedLoginAttemptsPerUser : 1000;
83+
this.failedLoginAttemptsPerUser = failedLoginAttemptsPerUser;
9184
}
9285

9386
// Set and validate passwordLockTimeMinutes (default 10, minimum 1)
9487
this.passwordLockTimeMinutes = passwordLockTimeMinutes >= 1 ? passwordLockTimeMinutes : 10;
88+
if (passwordLockTimeMinutes < 1) {
89+
LOGGER.warn(
90+
"Invalid lock time value ({}), reset to default (10 minutes)", passwordLockTimeMinutes);
91+
}
9592

9693
// Log final effective configuration
9794
LOGGER.info(

iotdb-core/datanode/src/test/java/org/apache/iotdb/db/auth/LoginLockManagerTest.java

Lines changed: 36 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -70,32 +70,42 @@ public void testAllConfigScenarios() {
7070
assertEquals(1000, getField(normal, "failedLoginAttemptsPerUser"));
7171
assertEquals(10, getField(normal, "passwordLockTimeMinutes"));
7272

73-
// 2. Test disabling both IP-level and user-level restrictions
74-
LoginLockManager disableBoth = new LoginLockManager(-1, -1, 10);
75-
assertEquals(-1, getField(disableBoth, "failedLoginAttempts"));
76-
assertEquals(-1, getField(disableBoth, "failedLoginAttemptsPerUser"));
77-
78-
// 3. Test enabling only user-level restriction
79-
LoginLockManager userOnly = new LoginLockManager(-1, 5, 10);
80-
assertEquals(-1, getField(userOnly, "failedLoginAttempts"));
81-
assertEquals(5, getField(userOnly, "failedLoginAttemptsPerUser"));
82-
83-
// 4. Test that enabling IP-level automatically enables user-level with default value
84-
LoginLockManager ipEnable = new LoginLockManager(3, -1, 10);
85-
assertEquals(3, getField(ipEnable, "failedLoginAttempts"));
86-
assertEquals(1000, getField(ipEnable, "failedLoginAttemptsPerUser"));
87-
88-
// 5. Test invalid IP attempts value falls back to default (5)
89-
LoginLockManager invalidIp = new LoginLockManager(0, -1, 10);
90-
assertEquals(5, getField(invalidIp, "failedLoginAttempts"));
91-
92-
// 6. Test invalid user attempts value falls back to default (1000)
93-
LoginLockManager invalidUser = new LoginLockManager(-1, 0, 10);
94-
assertEquals(1000, getField(invalidUser, "failedLoginAttemptsPerUser"));
95-
96-
// 7. Test invalid lock time value falls back to default (10 minutes)
97-
LoginLockManager invalidTime = new LoginLockManager(5, 1000, 0);
98-
assertEquals(10, getField(invalidTime, "passwordLockTimeMinutes"));
73+
// 2. Test disabling both IP-level and user-level restrictions (using -1 or 0)
74+
LoginLockManager disableBoth1 = new LoginLockManager(-1, -1, 10);
75+
assertEquals(-1, getField(disableBoth1, "failedLoginAttempts"));
76+
assertEquals(-1, getField(disableBoth1, "failedLoginAttemptsPerUser"));
77+
78+
LoginLockManager disableBoth2 = new LoginLockManager(0, 0, 10);
79+
assertEquals(-1, getField(disableBoth2, "failedLoginAttempts"));
80+
assertEquals(-1, getField(disableBoth2, "failedLoginAttemptsPerUser"));
81+
82+
// 3. Test mixed scenarios (IP enabled + user disabled, and vice versa)
83+
LoginLockManager ipEnabledUserDisabled = new LoginLockManager(3, 0, 10);
84+
assertEquals(3, getField(ipEnabledUserDisabled, "failedLoginAttempts"));
85+
assertEquals(-1, getField(ipEnabledUserDisabled, "failedLoginAttemptsPerUser"));
86+
87+
LoginLockManager ipDisabledUserEnabled = new LoginLockManager(-1, 5, 10);
88+
assertEquals(-1, getField(ipDisabledUserEnabled, "failedLoginAttempts"));
89+
assertEquals(5, getField(ipDisabledUserEnabled, "failedLoginAttemptsPerUser"));
90+
91+
// 4. Test invalid positive values fall back to defaults
92+
LoginLockManager invalidIp =
93+
new LoginLockManager(-5, 1000, 10); // Negative treated as disable (-1)
94+
assertEquals(-1, getField(invalidIp, "failedLoginAttempts"));
95+
96+
LoginLockManager invalidUser =
97+
new LoginLockManager(5, -2, 10); // Negative treated as disable (-1)
98+
assertEquals(-1, getField(invalidUser, "failedLoginAttemptsPerUser"));
99+
100+
// 5. Test lock time validation
101+
LoginLockManager zeroLockTime = new LoginLockManager(5, 1000, 0);
102+
assertEquals(10, getField(zeroLockTime, "passwordLockTimeMinutes"));
103+
104+
LoginLockManager negativeLockTime = new LoginLockManager(5, 1000, -5);
105+
assertEquals(10, getField(negativeLockTime, "passwordLockTimeMinutes"));
106+
107+
LoginLockManager customLockTime = new LoginLockManager(5, 1000, 30);
108+
assertEquals(30, getField(customLockTime, "passwordLockTimeMinutes"));
99109
}
100110

101111
private int getField(LoginLockManager manager, String fieldName) {
@@ -340,29 +350,6 @@ public void testFailuresOutsideWindowNotCounted() throws Exception {
340350
}
341351

342352
// ---------------- Configuration and Invalid Input ----------------
343-
344-
@Test
345-
public void testInvalidConfigurationDefaults() {
346-
{
347-
LoginLockManager invalidConfigManager = new LoginLockManager(-1, 0, -1);
348-
for (int i = 0; i < 1000; i++) {
349-
invalidConfigManager.recordFailure(TEST_USER_ID, TEST_IP);
350-
}
351-
assertTrue(
352-
"Should use default config when invalid values provided",
353-
invalidConfigManager.checkLock(TEST_USER_ID, TEST_IP));
354-
}
355-
{
356-
LoginLockManager invalidConfigManager = new LoginLockManager(-3, 0, -1);
357-
for (int i = 0; i < 5; i++) {
358-
invalidConfigManager.recordFailure(TEST_USER_ID, TEST_IP);
359-
}
360-
assertTrue(
361-
"Should use default config when invalid values provided",
362-
invalidConfigManager.checkLock(TEST_USER_ID, TEST_IP));
363-
}
364-
}
365-
366353
@Test
367354
public void testNullIpHandling() {
368355
lockManager.recordFailure(TEST_USER_ID, null);

0 commit comments

Comments
 (0)