From 4c30b53a7c40327828f7a90d229a1359da9e35c4 Mon Sep 17 00:00:00 2001 From: Shengliang Guan Date: Mon, 9 May 2022 16:03:31 +0800 Subject: [PATCH 1/2] feat: make grant revoke work --- include/common/tmsg.h | 10 +- source/dnode/mnode/impl/inc/mndAuth.h | 2 +- source/dnode/mnode/impl/src/mndAuth.c | 17 +--- source/dnode/mnode/impl/src/mndUser.c | 135 ++++++++++++++++---------- 4 files changed, 93 insertions(+), 71 deletions(-) diff --git a/include/common/tmsg.h b/include/common/tmsg.h index 72418148b0..8e6e73d649 100644 --- a/include/common/tmsg.h +++ b/include/common/tmsg.h @@ -131,12 +131,10 @@ typedef enum _mgmt_table { #define TSDB_ALTER_USER_SUPERUSER 0x2 #define TSDB_ALTER_USER_ADD_READ_DB 0x3 #define TSDB_ALTER_USER_REMOVE_READ_DB 0x4 -#define TSDB_ALTER_USER_CLEAR_READ_DB 0x5 -#define TSDB_ALTER_USER_ADD_WRITE_DB 0x6 -#define TSDB_ALTER_USER_REMOVE_WRITE_DB 0x7 -#define TSDB_ALTER_USER_CLEAR_WRITE_DB 0x8 -#define TSDB_ALTER_USER_ADD_ALL_DB 0x9 -#define TSDB_ALTER_USER_REMOVE_ALL_DB 0xA +#define TSDB_ALTER_USER_ADD_WRITE_DB 0x5 +#define TSDB_ALTER_USER_REMOVE_WRITE_DB 0x6 +#define TSDB_ALTER_USER_ADD_ALL_DB 0x7 +#define TSDB_ALTER_USER_REMOVE_ALL_DB 0x8 #define TSDB_ALTER_USER_PRIVILEGES 0x2 diff --git a/source/dnode/mnode/impl/inc/mndAuth.h b/source/dnode/mnode/impl/inc/mndAuth.h index 890879912f..de59a11cd7 100644 --- a/source/dnode/mnode/impl/inc/mndAuth.h +++ b/source/dnode/mnode/impl/inc/mndAuth.h @@ -26,7 +26,7 @@ int32_t mndInitAuth(SMnode *pMnode); void mndCleanupAuth(SMnode *pMnode); int32_t mndCheckCreateUserAuth(SUserObj *pOperUser); -int32_t mndCheckAlterUserAuth(SUserObj *pOperUser, SUserObj *pUser, SDbObj *pDb, SAlterUserReq *pAlter); +int32_t mndCheckAlterUserAuth(SUserObj *pOperUser, SUserObj *pUser, SAlterUserReq *pAlter); int32_t mndCheckDropUserAuth(SUserObj *pOperUser); int32_t mndCheckNodeAuth(SUserObj *pOperUser); diff --git a/source/dnode/mnode/impl/src/mndAuth.c b/source/dnode/mnode/impl/src/mndAuth.c index 8e5ec40c47..1d89241dd5 100644 --- a/source/dnode/mnode/impl/src/mndAuth.c +++ b/source/dnode/mnode/impl/src/mndAuth.c @@ -79,14 +79,12 @@ int32_t mndCheckCreateUserAuth(SUserObj *pOperUser) { return -1; } -int32_t mndCheckAlterUserAuth(SUserObj *pOperUser, SUserObj *pUser, SDbObj *pDb, SAlterUserReq *pAlter) { +int32_t mndCheckAlterUserAuth(SUserObj *pOperUser, SUserObj *pUser, SAlterUserReq *pAlter) { if (pAlter->alterType == TSDB_ALTER_USER_PASSWD) { if (pOperUser->superUser || strcmp(pUser->user, pOperUser->user) == 0) { return 0; } - } - - if (pAlter->alterType == TSDB_ALTER_USER_SUPERUSER) { + } else if (pAlter->alterType == TSDB_ALTER_USER_SUPERUSER) { if (strcmp(pUser->user, TSDB_DEFAULT_USER) == 0) { terrno = TSDB_CODE_MND_NO_RIGHTS; return -1; @@ -95,21 +93,12 @@ int32_t mndCheckAlterUserAuth(SUserObj *pOperUser, SUserObj *pUser, SDbObj *pDb, if (pOperUser->superUser) { return 0; } - } - - if (pAlter->alterType == TSDB_ALTER_USER_CLEAR_WRITE_DB || pAlter->alterType == TSDB_ALTER_USER_CLEAR_READ_DB) { + } else { if (pOperUser->superUser) { return 0; } } - if (pAlter->alterType == TSDB_ALTER_USER_ADD_READ_DB || pAlter->alterType == TSDB_ALTER_USER_REMOVE_READ_DB || - pAlter->alterType == TSDB_ALTER_USER_ADD_WRITE_DB || pAlter->alterType == TSDB_ALTER_USER_REMOVE_WRITE_DB) { - if (pOperUser->superUser || strcmp(pUser->user, pDb->createUser) == 0) { - return 0; - } - } - terrno = TSDB_CODE_MND_NO_RIGHTS; return -1; } diff --git a/source/dnode/mnode/impl/src/mndUser.c b/source/dnode/mnode/impl/src/mndUser.c index d0af17ff5c..1706820bdc 100644 --- a/source/dnode/mnode/impl/src/mndUser.c +++ b/source/dnode/mnode/impl/src/mndUser.c @@ -394,6 +394,8 @@ static SHashObj *mndDupDbHash(SHashObj *pOld) { static int32_t mndProcessAlterUserReq(SNodeMsg *pReq) { SMnode *pMnode = pReq->pNode; + SSdb *pSdb = pMnode->pSdb; + void *pIter = NULL; int32_t code = -1; SUserObj *pUser = NULL; SUserObj *pOperUser = NULL; @@ -429,7 +431,13 @@ static int32_t mndProcessAlterUserReq(SNodeMsg *pReq) { goto _OVER; } + if (mndCheckAlterUserAuth(pOperUser, pUser, &alterReq) != 0) { + goto _OVER; + } + memcpy(&newUser, pUser, sizeof(SUserObj)); + newUser.authVersion++; + newUser.updateTime = taosGetTimestampMs(); taosRLockLatch(&pUser->lock); newUser.readDbs = mndDupDbHash(pUser->readDbs); @@ -440,63 +448,90 @@ static int32_t mndProcessAlterUserReq(SNodeMsg *pReq) { goto _OVER; } - int32_t len = strlen(alterReq.dbname) + 1; - SDbObj *pDb = mndAcquireDb(pMnode, alterReq.dbname); - mndReleaseDb(pMnode, pDb); - if (alterReq.alterType == TSDB_ALTER_USER_PASSWD) { char pass[TSDB_PASSWORD_LEN + 1] = {0}; taosEncryptPass_c((uint8_t *)alterReq.pass, strlen(alterReq.pass), pass); memcpy(newUser.pass, pass, TSDB_PASSWORD_LEN); - } else if (alterReq.alterType == TSDB_ALTER_USER_SUPERUSER) { - newUser.superUser = alterReq.superUser; - } else if (alterReq.alterType == TSDB_ALTER_USER_ADD_READ_DB) { - if (pDb == NULL) { - terrno = TSDB_CODE_MND_DB_NOT_EXIST; - goto _OVER; - } - if (taosHashPut(newUser.readDbs, alterReq.dbname, len, alterReq.dbname, TSDB_DB_FNAME_LEN) != 0) { - terrno = TSDB_CODE_OUT_OF_MEMORY; - goto _OVER; - } - newUser.authVersion++; - } else if (alterReq.alterType == TSDB_ALTER_USER_REMOVE_READ_DB) { - if (taosHashRemove(newUser.readDbs, alterReq.dbname, len) != 0) { - terrno = TSDB_CODE_MND_DB_NOT_EXIST; - goto _OVER; - } - newUser.authVersion++; - } else if (alterReq.alterType == TSDB_ALTER_USER_CLEAR_READ_DB) { - taosHashClear(newUser.readDbs); - newUser.authVersion++; - } else if (alterReq.alterType == TSDB_ALTER_USER_ADD_WRITE_DB) { - if (pDb == NULL) { - terrno = TSDB_CODE_MND_DB_NOT_EXIST; - goto _OVER; - } - if (taosHashPut(newUser.writeDbs, alterReq.dbname, len, alterReq.dbname, TSDB_DB_FNAME_LEN) != 0) { - terrno = TSDB_CODE_OUT_OF_MEMORY; - goto _OVER; - } - newUser.authVersion++; - } else if (alterReq.alterType == TSDB_ALTER_USER_REMOVE_WRITE_DB) { - if (taosHashRemove(newUser.writeDbs, alterReq.dbname, len) != 0) { - terrno = TSDB_CODE_MND_DB_NOT_EXIST; - goto _OVER; - } - newUser.authVersion++; - } else if (alterReq.alterType == TSDB_ALTER_USER_CLEAR_WRITE_DB) { - taosHashClear(newUser.writeDbs); - newUser.authVersion++; - } else { - terrno = TSDB_CODE_MND_INVALID_ALTER_OPER; - goto _OVER; } - newUser.updateTime = taosGetTimestampMs(); + if (alterReq.alterType == TSDB_ALTER_USER_SUPERUSER) { + newUser.superUser = alterReq.superUser; + } - if (mndCheckAlterUserAuth(pOperUser, pUser, pDb, &alterReq) != 0) { - goto _OVER; + if (alterReq.alterType == TSDB_ALTER_USER_ADD_READ_DB || alterReq.alterType == TSDB_ALTER_USER_ADD_ALL_DB) { + if (strcmp(alterReq.dbname, "*") != 0) { + int32_t len = strlen(alterReq.dbname) + 1; + SDbObj *pDb = mndAcquireDb(pMnode, alterReq.dbname); + if (pDb == NULL) { + mndReleaseDb(pMnode, pDb); + goto _OVER; + } + if (taosHashPut(newUser.readDbs, alterReq.dbname, len, alterReq.dbname, TSDB_DB_FNAME_LEN) != 0) { + mndReleaseDb(pMnode, pDb); + goto _OVER; + } + } else { + while (1) { + SDbObj *pDb = NULL; + pIter = sdbFetch(pSdb, SDB_DB, pIter, (void **)&pDb); + if (pIter == NULL) break; + int32_t len = strlen(pDb->name) + 1; + taosHashPut(newUser.readDbs, pDb->name, len, pDb->name, TSDB_DB_FNAME_LEN); + sdbRelease(pSdb, pDb); + } + } + } + + if (alterReq.alterType == TSDB_ALTER_USER_ADD_WRITE_DB || alterReq.alterType == TSDB_ALTER_USER_ADD_ALL_DB) { + if (strcmp(alterReq.dbname, "*") != 0) { + int32_t len = strlen(alterReq.dbname) + 1; + SDbObj *pDb = mndAcquireDb(pMnode, alterReq.dbname); + if (pDb == NULL) { + mndReleaseDb(pMnode, pDb); + goto _OVER; + } + if (taosHashPut(newUser.writeDbs, alterReq.dbname, len, alterReq.dbname, TSDB_DB_FNAME_LEN) != 0) { + mndReleaseDb(pMnode, pDb); + goto _OVER; + } + } else { + while (1) { + SDbObj *pDb = NULL; + pIter = sdbFetch(pSdb, SDB_DB, pIter, (void **)&pDb); + if (pIter == NULL) break; + int32_t len = strlen(pDb->name) + 1; + taosHashPut(newUser.writeDbs, pDb->name, len, pDb->name, TSDB_DB_FNAME_LEN); + sdbRelease(pSdb, pDb); + } + } + } + + if (alterReq.alterType == TSDB_ALTER_USER_REMOVE_READ_DB || alterReq.alterType == TSDB_ALTER_USER_REMOVE_ALL_DB) { + if (strcmp(alterReq.dbname, "*") != 0) { + int32_t len = strlen(alterReq.dbname) + 1; + SDbObj *pDb = mndAcquireDb(pMnode, alterReq.dbname); + if (pDb == NULL) { + mndReleaseDb(pMnode, pDb); + goto _OVER; + } + taosHashRemove(newUser.readDbs, alterReq.dbname, len); + } else { + taosHashClear(newUser.readDbs); + } + } + + if (alterReq.alterType == TSDB_ALTER_USER_REMOVE_WRITE_DB || alterReq.alterType == TSDB_ALTER_USER_REMOVE_ALL_DB) { + if (strcmp(alterReq.dbname, "*") != 0) { + int32_t len = strlen(alterReq.dbname) + 1; + SDbObj *pDb = mndAcquireDb(pMnode, alterReq.dbname); + if (pDb == NULL) { + mndReleaseDb(pMnode, pDb); + goto _OVER; + } + taosHashRemove(newUser.writeDbs, alterReq.dbname, len); + } else { + taosHashClear(newUser.writeDbs); + } } code = mndAlterUser(pMnode, pUser, &newUser, pReq); From 2186d655e2dc194dfaf7bf8bf1c011770d95329b Mon Sep 17 00:00:00 2001 From: Shengliang Guan Date: Mon, 9 May 2022 16:03:50 +0800 Subject: [PATCH 2/2] test: add test case for grant revoke user --- source/dnode/mnode/impl/test/user/user.cpp | 6 +- tests/script/tsim/user/privilege1.sim | 71 ++++++++++++++++++++++ 2 files changed, 75 insertions(+), 2 deletions(-) create mode 100644 tests/script/tsim/user/privilege1.sim diff --git a/source/dnode/mnode/impl/test/user/user.cpp b/source/dnode/mnode/impl/test/user/user.cpp index ee961e9a27..1e03d8ff4a 100644 --- a/source/dnode/mnode/impl/test/user/user.cpp +++ b/source/dnode/mnode/impl/test/user/user.cpp @@ -238,9 +238,10 @@ TEST_F(MndTestUser, 03_Alter_User) { { SAlterUserReq alterReq = {0}; - alterReq.alterType = TSDB_ALTER_USER_CLEAR_WRITE_DB; + alterReq.alterType = TSDB_ALTER_USER_REMOVE_ALL_DB; strcpy(alterReq.user, "u3"); strcpy(alterReq.pass, "1"); + strcpy(alterReq.dbname, "*"); int32_t contLen = tSerializeSAlterUserReq(NULL, 0, &alterReq); void* pReq = rpcMallocCont(contLen); @@ -253,9 +254,10 @@ TEST_F(MndTestUser, 03_Alter_User) { { SAlterUserReq alterReq = {0}; - alterReq.alterType = TSDB_ALTER_USER_CLEAR_READ_DB; + alterReq.alterType = TSDB_ALTER_USER_REMOVE_ALL_DB; strcpy(alterReq.user, "u3"); strcpy(alterReq.pass, "1"); + strcpy(alterReq.dbname, "*"); int32_t contLen = tSerializeSAlterUserReq(NULL, 0, &alterReq); void* pReq = rpcMallocCont(contLen); diff --git a/tests/script/tsim/user/privilege1.sim b/tests/script/tsim/user/privilege1.sim new file mode 100644 index 0000000000..a7c5d9d13d --- /dev/null +++ b/tests/script/tsim/user/privilege1.sim @@ -0,0 +1,71 @@ +system sh/stop_dnodes.sh +system sh/deploy.sh -n dnode1 -i 1 +system sh/exec.sh -n dnode1 -s start +sql connect + +print =============== show users +sql create database d1 vgroups 1; +sql create database d2 vgroups 1; +sql create database d3 vgroups 1; +sql show databases +if $rows != 5 then + return -1 +endi + +print =============== create users +sql create user user1 PASS 'user1' +sql create user user2 PASS 'user2' +sql show users +if $rows != 3 then + return -1 +endi + +print =============== test read +sql_error GRANT read ON d1.* to a; +sql_error GRANT read ON d0.* to user1; + +sql GRANT read ON d1.* to user1; +sql GRANT read ON d2.* to user1; +sql GRANT read ON *.* to user1; + +sql REVOKE read ON d1.* from user1; +sql REVOKE read ON d2.* from user1; +sql REVOKE read ON *.* from user1; + +print =============== test write +sql_error GRANT write ON d1.* to a; +sql_error GRANT write ON d0.* to user1; + +sql GRANT write ON d1.* to user1; +sql GRANT write ON d2.* to user1; +sql GRANT write ON *.* to user1; + +sql REVOKE write ON d1.* from user1; +sql REVOKE write ON d2.* from user1; +sql REVOKE write ON *.* from user1; + +print =============== test all +sql_error GRANT all ON d1.* to a; +sql_error GRANT all ON d0.* to user1; + +sql GRANT all ON d1.* to user1; +sql GRANT all ON d2.* to user1; +sql GRANT all ON *.* to user1; + +sql REVOKE all ON d1.* from user1; +sql REVOKE all ON d2.* from user1; +sql REVOKE all ON *.* from user1; + +print =============== test read write +sql_error GRANT read,write ON d1.* to a; +sql_error GRANT read,write ON d0.* to user1; + +sql GRANT read,write ON d1.* to user1; +sql GRANT read,write ON d2.* to user1; +sql GRANT read,write ON *.* to user1; + +sql REVOKE read,write ON d1.* from user1; +sql REVOKE read,write ON d2.* from user1; +sql REVOKE read,write ON *.* from user1; + +system sh/exec.sh -n dnode1 -s stop -x SIGINT