Skip to content

Commit

Permalink
[Fix](auth) fix revoke role operation cause fe down (#23852)
Browse files Browse the repository at this point in the history
If there 3 above fe nodes,
the following opeartions will cause all FE nodes down.

DROP USER revoke_test_user
DROP ROLE revoke_test_role
DROP DATABASE IF EXISTS revoke_test_db
CREATE DATABASE revoke_test_db
CREATE ROLE revoke_test_role
CREATE USER revoke_test_user IDENTIFIED BY 'revoke_test_pwd'
GRANT SELECT_PRIV ON revoke_test_db.* TO ROLE 'revoke_test_role'
GRANT 'revoke_test_role' TO revoke_test_user
SHOW GRANTS FOR revoke_test_user
REVOKE 'revoke_test_role' from revoke_test_user
SHOW GRANTS FOR revoke_test_user
DROP USER revoke_test_user
DROP ROLE revoke_test_role
DROP DATABASE revoke_test_db
  • Loading branch information
neuyilan authored and xiaokang committed Sep 23, 2023
1 parent 4ece7aa commit ea0bfb2
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -711,7 +711,7 @@ public void replayRevoke(PrivInfo info) {
revokeInternal(info.getUserIdent(), info.getRole(), info.getWorkloadGroupPattern(), info.getPrivs(),
true /* err on non exist */, true /* is replay */);
} else {
revokeInternal(info.getUserIdent(), info.getRoles(), false);
revokeInternal(info.getUserIdent(), info.getRoles(), true /* is replay */);
}
} catch (DdlException e) {
LOG.error("should not happened", e);
Expand Down
49 changes: 49 additions & 0 deletions regression-test/suites/account_p0/test_revoke_role.groovy
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
// Licensed to the Apache Software Foundation (ASF) under one
// or more contributor license agreements. See the NOTICE file
// distributed with this work for additional information
// regarding copyright ownership. The ASF licenses this file
// to you under the Apache License, Version 2.0 (the
// "License"); you may not use this file except in compliance
// with the License. You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing,
// software distributed under the License is distributed on an
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
// KIND, either express or implied. See the License for the
// specific language governing permissions and limitations
// under the License.

suite("test_revoke_role", "account") {
def role= 'revoke_test_role'
def user = 'revoke_test_user'
def dbName = 'revoke_test_db'
def pwd = 'revoke_test_pwd'

try_sql("DROP ROLE ${role}")
try_sql("DROP USER ${user}")
try_sql("DROP ROLE ${role}")
sql """DROP DATABASE IF EXISTS ${dbName}"""
sql """CREATE DATABASE ${dbName}"""

sql """CREATE ROLE ${role}"""
sql """CREATE USER ${user} IDENTIFIED BY '${pwd}'"""

sql """GRANT SELECT_PRIV ON ${dbName}.* TO ROLE '${role}'"""
sql """GRANT '${role}' TO ${user}"""

def result = sql """ SHOW GRANTS FOR ${user} """
assertEquals(result.size(), 1)
assertTrue(result[0][5].contains("internal.default_cluster:${dbName}: Select_priv"))

sql """REVOKE '${role}' from ${user}"""
result = sql """ SHOW GRANTS FOR ${user} """
assertEquals(result.size(), 1)
assertFalse(result[0][5].contains("internal.default_cluster:${dbName}: Select_priv"))

sql """DROP USER ${user}"""
sql """DROP ROLE ${role}"""
sql """DROP DATABASE ${dbName}"""
}

0 comments on commit ea0bfb2

Please sign in to comment.