Skip to content

Commit

Permalink
privileges: show grants return incorrect results when granted with 2 …
Browse files Browse the repository at this point in the history 
…or more privileges (pingcap#31322)

close pingcap#30855
  • Loading branch information
@djshow832
djshow832 authored Jan 6, 2022
1 parent fac8bf3 commit e053cbb
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 29 deletions.
24 changes: 4 additions & 20 deletions privilege/privileges/cache.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1231,19 +1231,11 @@ func (p *MySQLPrivilege) showGrants(user, host string, roles []*auth.RoleIdentit
dbPrivTable := make(map[string]mysql.PrivilegeType)
for _, record := range p.DB {
if record.fullyMatch(user, host) {
if _, ok := dbPrivTable[record.DB]; ok {
dbPrivTable[record.DB] |= record.Privileges
} else {
dbPrivTable[record.DB] = record.Privileges
}
dbPrivTable[record.DB] |= record.Privileges
} else {
for _, r := range allRoles {
if record.baseRecord.match(r.Username, r.Hostname) {
if _, ok := dbPrivTable[record.DB]; ok {
dbPrivTable[record.DB] |= record.Privileges
} else {
dbPrivTable[record.DB] = record.Privileges
}
dbPrivTable[record.DB] |= record.Privileges
}
}
}
Expand Down Expand Up @@ -1273,19 +1265,11 @@ func (p *MySQLPrivilege) showGrants(user, host string, roles []*auth.RoleIdentit
for _, record := range p.TablesPriv {
recordKey := record.DB + "." + record.TableName
if user == record.User && host == record.Host {
if _, ok := dbPrivTable[record.DB]; ok {
tablePrivTable[recordKey] |= record.TablePriv
} else {
tablePrivTable[recordKey] = record.TablePriv
}
tablePrivTable[recordKey] |= record.TablePriv
} else {
for _, r := range allRoles {
if record.baseRecord.match(r.Username, r.Hostname) {
if _, ok := dbPrivTable[record.DB]; ok {
tablePrivTable[recordKey] |= record.TablePriv
} else {
tablePrivTable[recordKey] = record.TablePriv
}
tablePrivTable[recordKey] |= record.TablePriv
}
}
}
Expand Down
47 changes: 38 additions & 9 deletions privilege/privileges/privileges_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,7 @@ func TestShowGrants(t *testing.T) {
require.Len(t, gs, 2)
expected := []string{`GRANT ALL PRIVILEGES ON *.* TO 'show'@'localhost'`,
`GRANT SELECT ON test.* TO 'show'@'localhost'`}
require.True(t, testutil.CompareUnorderedStringSlice(gs, expected))
require.True(t, testutil.CompareUnorderedStringSlice(gs, expected), fmt.Sprintf("gs: %v, expected: %v", gs, expected))

mustExec(t, se, `GRANT Index ON test1.* TO 'show'@'localhost';`)
gs, err = pc.ShowGrants(se, &auth.UserIdentity{Username: "show", Hostname: "localhost"}, nil)
Expand All @@ -280,7 +280,17 @@ func TestShowGrants(t *testing.T) {
expected = []string{`GRANT ALL PRIVILEGES ON *.* TO 'show'@'localhost'`,
`GRANT SELECT ON test.* TO 'show'@'localhost'`,
`GRANT INDEX ON test1.* TO 'show'@'localhost'`}
require.True(t, testutil.CompareUnorderedStringSlice(gs, expected))
require.True(t, testutil.CompareUnorderedStringSlice(gs, expected), fmt.Sprintf("gs: %v, expected: %v", gs, expected))

// Add another db privilege to the same db and test again.
mustExec(t, se, `GRANT Delete ON test1.* TO 'show'@'localhost';`)
gs, err = pc.ShowGrants(se, &auth.UserIdentity{Username: "show", Hostname: "localhost"}, nil)
require.NoError(t, err)
require.Len(t, gs, 3)
expected = []string{`GRANT ALL PRIVILEGES ON *.* TO 'show'@'localhost'`,
`GRANT SELECT ON test.* TO 'show'@'localhost'`,
`GRANT DELETE,INDEX ON test1.* TO 'show'@'localhost'`}
require.True(t, testutil.CompareUnorderedStringSlice(gs, expected), fmt.Sprintf("gs: %v, expected: %v", gs, expected))

mustExec(t, se, `GRANT ALL ON test1.* TO 'show'@'localhost';`)
gs, err = pc.ShowGrants(se, &auth.UserIdentity{Username: "show", Hostname: "localhost"}, nil)
Expand All @@ -289,7 +299,7 @@ func TestShowGrants(t *testing.T) {
expected = []string{`GRANT ALL PRIVILEGES ON *.* TO 'show'@'localhost'`,
`GRANT SELECT ON test.* TO 'show'@'localhost'`,
`GRANT ALL PRIVILEGES ON test1.* TO 'show'@'localhost'`}
require.True(t, testutil.CompareUnorderedStringSlice(gs, expected))
require.True(t, testutil.CompareUnorderedStringSlice(gs, expected), fmt.Sprintf("gs: %v, expected: %v", gs, expected))

// Add table scope privileges
mustExec(t, se, `GRANT Update ON test.test TO 'show'@'localhost';`)
Expand All @@ -300,13 +310,33 @@ func TestShowGrants(t *testing.T) {
`GRANT SELECT ON test.* TO 'show'@'localhost'`,
`GRANT ALL PRIVILEGES ON test1.* TO 'show'@'localhost'`,
`GRANT UPDATE ON test.test TO 'show'@'localhost'`}
require.True(t, testutil.CompareUnorderedStringSlice(gs, expected))
require.True(t, testutil.CompareUnorderedStringSlice(gs, expected), fmt.Sprintf("gs: %v, expected: %v", gs, expected))

// Revoke the db privilege of `test` and test again. See issue #30855.
mustExec(t, se, `REVOKE SELECT ON test.* FROM 'show'@'localhost'`)
gs, err = pc.ShowGrants(se, &auth.UserIdentity{Username: "show", Hostname: "localhost"}, nil)
require.NoError(t, err)
require.NoError(t, err)
require.Len(t, gs, 3)
expected = []string{`GRANT ALL PRIVILEGES ON *.* TO 'show'@'localhost'`,
`GRANT ALL PRIVILEGES ON test1.* TO 'show'@'localhost'`,
`GRANT UPDATE ON test.test TO 'show'@'localhost'`}
require.True(t, testutil.CompareUnorderedStringSlice(gs, expected), fmt.Sprintf("gs: %v, expected: %v", gs, expected))

// Add another table privilege and test again.
mustExec(t, se, `GRANT Select ON test.test TO 'show'@'localhost';`)
gs, err = pc.ShowGrants(se, &auth.UserIdentity{Username: "show", Hostname: "localhost"}, nil)
require.NoError(t, err)
require.Len(t, gs, 3)
expected = []string{`GRANT ALL PRIVILEGES ON *.* TO 'show'@'localhost'`,
`GRANT ALL PRIVILEGES ON test1.* TO 'show'@'localhost'`,
`GRANT SELECT,UPDATE ON test.test TO 'show'@'localhost'`}
require.True(t, testutil.CompareUnorderedStringSlice(gs, expected), fmt.Sprintf("gs: %v, expected: %v", gs, expected))

// Expected behavior: Usage still exists after revoking all privileges
mustExec(t, se, `REVOKE ALL PRIVILEGES ON *.* FROM 'show'@'localhost'`)
mustExec(t, se, `REVOKE Select on test.* FROM 'show'@'localhost'`)
mustExec(t, se, `REVOKE ALL ON test1.* FROM 'show'@'localhost'`)
mustExec(t, se, `REVOKE UPDATE on test.test FROM 'show'@'localhost'`)
mustExec(t, se, `REVOKE UPDATE, SELECT on test.test FROM 'show'@'localhost'`)
gs, err = pc.ShowGrants(se, &auth.UserIdentity{Username: "show", Hostname: "localhost"}, nil)
require.NoError(t, err)
require.Len(t, gs, 1)
Expand Down Expand Up @@ -2506,17 +2536,16 @@ func TestShowGrantsForCurrentUserUsingRole(t *testing.T) {
"GRANT USAGE ON *.* TO 'joe'@'%'",
"GRANT SELECT ON test.* TO 'joe'@'%'",
"GRANT UPDATE ON role.* TO 'joe'@'%'",
"GRANT DELETE ON mysql.user TO 'joe'@'%'",
"GRANT SELECT,DELETE ON mysql.user TO 'joe'@'%'",
"GRANT 'admins'@'%', 'engineering'@'%', 'otherrole'@'%' TO 'joe'@'%'",
))
tk.MustQuery("SHOW GRANTS FOR joe USING otherrole;").Check(testkit.Rows(
"GRANT USAGE ON *.* TO 'joe'@'%'",
"GRANT SELECT ON test.* TO 'joe'@'%'",
"GRANT UPDATE ON role.* TO 'joe'@'%'",
"GRANT DELETE ON mysql.user TO 'joe'@'%'",
"GRANT SELECT,DELETE ON mysql.user TO 'joe'@'%'",
"GRANT 'admins'@'%', 'engineering'@'%', 'otherrole'@'%' TO 'joe'@'%'",
))

}

func TestGrantPlacementAdminDynamicPriv(t *testing.T) {
Expand Down

0 comments on commit e053cbb

Please sign in to comment.