Skip to content
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

privilege: fix privilege check of GRANT ROLE #13896

Merged
merged 8 commits into from Dec 5, 2019

Conversation

imtbkcat
Copy link

@imtbkcat imtbkcat commented Dec 4, 2019

What problem does this PR solve?

In MySQL, GRANT roles TO users require ROLE_ADMIN or SUPER privilege, But TiDB use GrantPriv to check GRANT ROLE.

What is changed and how it works?

fix this bug and add test.

Check List

Tests

  • Unit test

Code changes

  • None

Side effects

  • None

Related changes

  • Need to cherry-pick to the release branch

Release note

  • fix privilege check rule for GRANT ROLE

@imtbkcat imtbkcat requested a review from a team as a code owner December 4, 2019 08:56
@ghost ghost requested review from alivxxx and lzmhhh123 and removed request for a team December 4, 2019 08:56
@codecov
Copy link

codecov bot commented Dec 4, 2019

Codecov Report

Merging #13896 into master will not change coverage.
The diff coverage is n/a.

@@             Coverage Diff             @@
##             master     #13896   +/-   ##
===========================================
  Coverage   80.2017%   80.2017%           
===========================================
  Files           480        480           
  Lines        119187     119187           
===========================================
  Hits          95590      95590           
  Misses        16069      16069           
  Partials       7528       7528

Copy link
Contributor

@alivxxx alivxxx left a comment

Choose a reason for hiding this comment

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

LGTM

@imtbkcat imtbkcat added the status/LGT1 Indicates that a PR has LGTM 1. label Dec 5, 2019
@tiancaiamao
Copy link
Contributor

LGTM

@tiancaiamao tiancaiamao added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Dec 5, 2019
@tiancaiamao
Copy link
Contributor

/merge

@imtbkcat imtbkcat added the status/can-merge Indicates a PR has been approved by a committer. label Dec 5, 2019
@sre-bot
Copy link
Contributor

sre-bot commented Dec 5, 2019

/run-all-tests

@sre-bot
Copy link
Contributor

sre-bot commented Dec 5, 2019

@imtbkcat merge failed.

@imtbkcat
Copy link
Author

imtbkcat commented Dec 5, 2019

/run-all-tests tidb-test=pr/963

@imtbkcat
Copy link
Author

imtbkcat commented Dec 5, 2019

/merge

@sre-bot
Copy link
Contributor

sre-bot commented Dec 5, 2019

/run-all-tests

@sre-bot
Copy link
Contributor

sre-bot commented Dec 5, 2019

@imtbkcat merge failed.

@ngaut ngaut merged commit ea1662a into pingcap:master Dec 5, 2019
@sre-bot
Copy link
Contributor

sre-bot commented Dec 5, 2019

cherry pick to release-3.0 in PR #13932

XiaTianliang pushed a commit to XiaTianliang/tidb that referenced this pull request Dec 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/privilege status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2. type/bug-fix This PR fixes a bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants