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

executor, privilege: fix some two bug of RBAC #11273

Merged
merged 9 commits into from
Jul 22, 2019
Merged

Conversation

imtbkcat
Copy link

What problem does this PR solve?

Fix two bug of RBAC.

bug 1: CURRENT_ROLE is not handle correctly in GRANT ROLE

mysql> create role r_1;
Query OK, 1 row affected (0.01 sec)

mysql> grant r_1 to current_user();
ERROR 1396 (HY000): Operation GRANT ROLE failed for @

bug 2: REVOKE ROLE doesn't delete information in mysql.default_role

mysql> REVOKE r1 FROM u1@localhost;
Query OK, 0 rows affected (0.01 sec)

mysql> select * from mysql.default_roles;
+-----------+------+-------------------+-------------------+
| HOST      | USER | DEFAULT_ROLE_HOST | DEFAULT_ROLE_USER |
+-----------+------+-------------------+-------------------+
| localhost | u1   | %                 | r1                |
+-----------+------+-------------------+-------------------+
1 row in set (0.00 sec)

What is changed and how it works?

  1. Add check for CURRENT_USER in GrantRoleExec, if User is CURRENT_USER, it will be set to current user.
  2. Delete information from mysql.default_role in RevokeRoleStmt.

Check List

Tests

  • Unit test

Code changes

Side effects

  • Increased code complexity

Related changes

  • Need to cherry-pick to the release branch

@codecov
Copy link

codecov bot commented Jul 16, 2019

Codecov Report

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

@@             Coverage Diff             @@
##             master     #11273   +/-   ##
===========================================
  Coverage   81.2607%   81.2607%           
===========================================
  Files           423        423           
  Lines         90233      90233           
===========================================
  Hits          73324      73324           
  Misses        11607      11607           
  Partials       5302       5302

@imtbkcat
Copy link
Author

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

@imtbkcat
Copy link
Author

/run-common-test tidb-test=pr/847

@imtbkcat
Copy link
Author

/run-common-test

@imtbkcat
Copy link
Author

/run-all-tests

@tiancaiamao
Copy link
Contributor

LGTM

@tiancaiamao tiancaiamao added the status/LGT1 Indicates that a PR has LGTM 1. label Jul 19, 2019
Copy link
Member

@jackysp jackysp left a comment

Choose a reason for hiding this comment

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

LGTM

@sre-bot
Copy link
Contributor

sre-bot commented Jul 22, 2019

cherry pick to release-3.0 in PR #11356

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/privilege sig/execution SIG execution status/LGT1 Indicates that a PR has LGTM 1. type/bugfix This PR fixes a bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants