-
Notifications
You must be signed in to change notification settings - Fork 5.9k
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.
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
privileges: add SkipWithGrant check for RBAC methods #10681
Conversation
Codecov Report
@@ Coverage Diff @@
## master #10681 +/- ##
===========================================
Coverage ? 79.6625%
===========================================
Files ? 414
Lines ? 87656
Branches ? 0
===========================================
Hits ? 69829
Misses ? 12650
Partials ? 5177 |
privilege/privileges/privileges.go
Outdated
@@ -185,6 +185,10 @@ func (p *UserPrivileges) UserPrivilegesTable() [][]types.Datum { | |||
|
|||
// ShowGrants implements privilege.Manager ShowGrants interface. | |||
func (p *UserPrivileges) ShowGrants(ctx sessionctx.Context, user *auth.UserIdentity, roles []*auth.RoleIdentity) (grants []string, err error) { | |||
if SkipWithGrant { | |||
grants, err = nil, errNonexistingGrant.GenWithStackByArgs() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why not
grants, err = nil, errNonexistingGrant.GenWithStackByArgs() | |
return nil, errNonexistingGrant.GenWithStackByArgs() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what will this error look like?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PTAL @jackysp
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you add some test cases?
privilege/privileges/privileges.go
Outdated
@@ -185,6 +185,9 @@ func (p *UserPrivileges) UserPrivilegesTable() [][]types.Datum { | |||
|
|||
// ShowGrants implements privilege.Manager ShowGrants interface. | |||
func (p *UserPrivileges) ShowGrants(ctx sessionctx.Context, user *auth.UserIdentity, roles []*auth.RoleIdentity) (grants []string, err error) { | |||
if SkipWithGrant { | |||
return nil, errNonexistingGrant.GenWithStackByArgs() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What will this error look like?
LGTM |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
/run-all-tests |
Please cherry-pick this PR to release 3.0. |
/run-unit-test |
/run-integration-ddl-test |
What problem does this PR solve?
RBAC methods in privileges package did not check
SkipWithGrant
. This cause panic when enable--skip-grant-table
.What is changed and how it works?
add check for
SkipWithGrant
, whenSkipWithGrant
is enabled, calling for RBAC method will be ignored.Check List
Tests
Code changes