Skip to content

Fix/driver pass change #35254

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
yihaoDeng wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Fix/driver pass change #35254

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0d26ff3
feat: add password change detection and related error handling
yihaoDeng Apr 28, 2026
9c7d04f
Merge remote-tracking branch 'origin/main' into fix/driverPassChange
yihaoDeng Apr 29, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions include/util/taoserror.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,7 @@ int32_t taosGetErrSize();
#define TSDB_CODE_TSC_SESS_MAX_CALL_VNODE_LIMIT TAOS_DEF_ERROR_CODE(0, 0x023D)
#define TSDB_CODE_TSC_INVALID_TOKEN TAOS_DEF_ERROR_CODE(0, 0x023E)
#define TSDB_CODE_TSC_INSTANCE_API_RATE_LIMIT TAOS_DEF_ERROR_CODE(0, 0x023F)
#define TSDB_CODE_TSC_PASS_CHANGED TAOS_DEF_ERROR_CODE(0, 0x0240)
#define TSDB_CODE_TSC_INTERNAL_ERROR TAOS_DEF_ERROR_CODE(0, 0x02FF)

// mnode-common
Expand Down
1 change: 1 addition & 0 deletions source/client/inc/clientInt.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,7 @@ typedef struct SAppInfo {

typedef struct {
int32_t ver;
int8_t passChanged;
void* param;
__taos_notify_fn_t fp;
} STscNotifyInfo;
Expand Down
4 changes: 4 additions & 0 deletions source/client/src/clientEnv.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -580,6 +580,10 @@ int32_t createRequest(uint64_t connId, int32_t type, int64_t reqid, SRequestObj
releaseTscObj(connId);
TSC_ERR_JRET(TSDB_CODE_MND_USER_DISABLED);
}
if (atomic_load_8(&pTscObj->passInfo.passChanged)) {
releaseTscObj(connId);
TSC_ERR_JRET(TSDB_CODE_TSC_PASS_CHANGED);
}
SSyncQueryParam *interParam = taosMemoryCalloc(1, sizeof(SSyncQueryParam));
if (interParam == NULL) {
releaseTscObj(connId);
Expand Down
7 changes: 5 additions & 2 deletions source/client/src/clientHb.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ static int32_t hbUpdateUserAuthInfo(SAppHbMgr *pAppHbMgr, SUserAuthBatchRsp *bat
}

// update password version
if (pTscObj->passInfo.fp) {
{
SPassInfo *passInfo = &pTscObj->passInfo;
int32_t oldVer = 0;
do {
Expand All @@ -242,7 +242,10 @@ static int32_t hbUpdateUserAuthInfo(SAppHbMgr *pAppHbMgr, SUserAuthBatchRsp *bat
}
} while (atomic_val_compare_exchange_32(&passInfo->ver, oldVer, pRsp->passVer) != oldVer);
if (oldVer < pRsp->passVer) {
(*passInfo->fp)(passInfo->param, &pRsp->passVer, TAOS_NOTIFY_PASSVER);
atomic_store_8(&passInfo->passChanged, 1);
if (passInfo->fp) {
(*passInfo->fp)(passInfo->param, &pRsp->passVer, TAOS_NOTIFY_PASSVER);
}
tscDebug("update passVer of user %s from %d to %d, conn:%" PRIi64, pRsp->user, oldVer,
atomic_load_32(&passInfo->ver), pTscObj->id);
}
Comment on lines +245 to 251
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot Apr 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The pRsp pointer used here is declared outside the while loop (at line 131) and is not reset for each connection iteration. This creates a significant bug in multi-user scenarios: if a heartbeat response contains updates for multiple users, a connection for one user will incorrectly reuse the pRsp data from a previous user's connection processed in the same loop. This will lead to incorrect password change detection, session metric updates, and other auth-related logic for all subsequent connections in the loop. Although the declaration is outside this diff hunk, the new logic added here is directly affected by this issue.

Expand Down
1 change: 1 addition & 0 deletions source/util/src/terror.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,7 @@ TAOS_DEFINE_ERROR(TSDB_CODE_NOT_SUPPORTTED_IN_WINDOWS, "Operation not support
TAOS_DEFINE_ERROR(TSDB_CODE_TSC_INVALID_TOTP_CODE, "Invalid TOTP code")
TAOS_DEFINE_ERROR(TSDB_CODE_TSC_INVALID_TOKEN, "Invalid token")
TAOS_DEFINE_ERROR(TSDB_CODE_TSC_INSTANCE_API_RATE_LIMIT, "instance register/list API rate limit exceeded")
TAOS_DEFINE_ERROR(TSDB_CODE_TSC_PASS_CHANGED, "Password has been changed from another session, please reconnect")
Copy link

Copilot AI Apr 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The message says the password was changed "from another session", but the client-side detection is based solely on passVer changes and can also trigger when the same session/user changes the password. Consider rewording to a neutral/accurate message (e.g., "Password has been changed, please reconnect") or otherwise ensuring the condition really implies an external-session change.

Suggested change
TAOS_DEFINE_ERROR(TSDB_CODE_TSC_PASS_CHANGED, "Password has been changed from another session, please reconnect")
TAOS_DEFINE_ERROR(TSDB_CODE_TSC_PASS_CHANGED, "Password has been changed, please reconnect")

Copilot uses AI. Check for mistakes.

TAOS_DEFINE_ERROR(TSDB_CODE_TSC_SESS_PER_USER_LIMIT, "reached the maximum sessions per user limit")
TAOS_DEFINE_ERROR(TSDB_CODE_TSC_SESS_CONN_TIMEOUT, "reached the maximum connection timeout limit")
Expand Down
32 changes: 30 additions & 2 deletions test/cases/24-Users/test_user_basic.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -434,7 +434,33 @@ def do_user_manage(self):
# self.subscribe_topic() # PRIV_TODO: topic

print("do user manager ............. [passed]")


#
# ------------------- test_pass_change_detect ----------------
#
def do_pass_change_detect(self):
tdLog.info("=============== test: password change detection")
tdSql.execute("CREATE USER upasschk PASS 'AAbb1122'")
conn = taos.connect(user='upasschk', password='AAbb1122')
cursor = conn.cursor()
cursor.execute("SELECT SERVER_VERSION()")
tdLog.info("conn1 query before pass change: success")

tdSql.execute("ALTER USER upasschk PASS 'CCdd3344!!'")
tdLog.info("password changed via root conn, waiting for heartbeat...")
time.sleep(5)

try:
cursor.execute("SELECT SERVER_VERSION()")
tdLog.exit("query should have failed after password change but succeeded")
except Exception as e:
tdLog.info(f"query after pass change failed as expected: {e}")
Comment on lines +450 to +457
Copy link

Copilot AI Apr 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using a fixed sleep here adds a hard 5s to the suite and can still be timing-sensitive if heartbeat behavior changes. Prefer polling until the expected error is observed (or a bounded timeout), which both reduces runtime and makes the test more deterministic.

Suggested change
tdLog.info("password changed via root conn, waiting for heartbeat...")
time.sleep(5)
try:
cursor.execute("SELECT SERVER_VERSION()")
tdLog.exit("query should have failed after password change but succeeded")
except Exception as e:
tdLog.info(f"query after pass change failed as expected: {e}")
tdLog.info("password changed via root conn, waiting for old connection to be rejected...")
deadline = time.monotonic() + 5
last_error = None
while time.monotonic() < deadline:
try:
cursor.execute("SELECT SERVER_VERSION()")
except Exception as e:
last_error = e
break
time.sleep(0.1)
if last_error is None:
tdLog.exit("query should have failed after password change but succeeded")
else:
tdLog.info(f"query after pass change failed as expected: {last_error}")

Copilot uses AI. Check for mistakes.

cursor.close()
conn.close()
tdSql.execute("DROP USER upasschk")
print("do pass change detect ............. [passed]")

Comment on lines +443 to +463
Copy link

Copilot AI Apr 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test creates a user/connection and only drops/closes them on the success path. If any assertion/exception occurs before the end, the user may be left behind and impact subsequent test runs. Wrap the body in try/finally (and/or drop the user defensively) to guarantee cursor/conn close and user cleanup.

Suggested change
tdSql.execute("CREATE USER upasschk PASS 'AAbb1122'")
conn = taos.connect(user='upasschk', password='AAbb1122')
cursor = conn.cursor()
cursor.execute("SELECT SERVER_VERSION()")
tdLog.info("conn1 query before pass change: success")
tdSql.execute("ALTER USER upasschk PASS 'CCdd3344!!'")
tdLog.info("password changed via root conn, waiting for heartbeat...")
time.sleep(5)
try:
cursor.execute("SELECT SERVER_VERSION()")
tdLog.exit("query should have failed after password change but succeeded")
except Exception as e:
tdLog.info(f"query after pass change failed as expected: {e}")
cursor.close()
conn.close()
tdSql.execute("DROP USER upasschk")
print("do pass change detect ............. [passed]")
conn = None
cursor = None
tdSql.execute("CREATE USER upasschk PASS 'AAbb1122'")
try:
conn = taos.connect(user='upasschk', password='AAbb1122')
cursor = conn.cursor()
cursor.execute("SELECT SERVER_VERSION()")
tdLog.info("conn1 query before pass change: success")
tdSql.execute("ALTER USER upasschk PASS 'CCdd3344!!'")
tdLog.info("password changed via root conn, waiting for heartbeat...")
time.sleep(5)
try:
cursor.execute("SELECT SERVER_VERSION()")
tdLog.exit("query should have failed after password change but succeeded")
except Exception as e:
tdLog.info(f"query after pass change failed as expected: {e}")
print("do pass change detect ............. [passed]")
finally:
if cursor is not None:
cursor.close()
if conn is not None:
conn.close()
try:
tdSql.execute("DROP USER upasschk")
except Exception as e:
tdLog.info(f"cleanup drop user upasschk failed: {e}")

Copilot uses AI. Check for mistakes.
#
# ------------------- main ----------------
#
Expand All @@ -448,6 +474,7 @@ def test_user_basic(self):
5. Ensures proper error handling for invalid privilege values
6. Check create multi users and grant/revoke privilege for them
7. Subscribe topic with different user privileges
8. Password change detection: after password changed, all requests on old connection fail

Catalog:
- User
Expand All @@ -467,4 +494,5 @@ def test_user_basic(self):
tdSql.execute("alter all dnodes 'enableAdvancedSecurity' '1'")
self.do_user_basic()
self.do_user_privilege_multi_users()
self.do_user_manage()
self.do_user_manage()
self.do_pass_change_detect()
Loading