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 user with % hostname can not show grants. #15825

Merged
merged 7 commits into from
Mar 30, 2020

Conversation

imtbkcat
Copy link

What problem does this PR solve?

#15524 didn't fix this case:

create user u1;
// login as u1
show grants;

Authhostname of u1 is %, hostname of u1 is 127.0.0.1. ShowGrants checks hostname in executor which cause show grants fail.

What is changed and how it works?

if user in ShowGrants is nil, copy session variable user to executor.

Related changes

  • Need to cherry-pick to the release branch

Check List

Tests

  • Unit test

Side effects

  • None

Release note

@imtbkcat imtbkcat requested a review from a team as a code owner March 30, 2020 04:42
@ghost ghost requested review from SunRunAway and removed request for a team March 30, 2020 04:42
@github-actions github-actions bot added the sig/execution SIG execution label Mar 30, 2020
@codecov
Copy link

codecov bot commented Mar 30, 2020

Codecov Report

Merging #15825 into master will not change coverage by %.
The diff coverage is n/a.

@@             Coverage Diff             @@
##             master     #15825   +/-   ##
===========================================
  Coverage   80.5380%   80.5380%           
===========================================
  Files           504        504           
  Lines        135238     135238           
===========================================
  Hits         108918     108918           
  Misses        17820      17820           
  Partials       8500       8500           

@imtbkcat imtbkcat removed the request for review from SunRunAway March 30, 2020 05:38
c.Assert(err, IsNil)
c.Assert(se2.Auth(&auth.UserIdentity{Username: "show_grants", Hostname: "127.0.0.1", AuthUsername: "show_grants", AuthHostname: "%"}, nil, nil), IsTrue)
tk2.Se = se2
err = tk2.QueryToErr("show grants")
Copy link
Member

Choose a reason for hiding this comment

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

why not use MustQuery?

Copy link
Author

Choose a reason for hiding this comment

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

fixed

@tiancaiamao
Copy link
Contributor

LGTM

@tiancaiamao tiancaiamao added the status/LGT1 Indicates that a PR has LGTM 1. label Mar 30, 2020
@imtbkcat
Copy link
Author

/run-all-tests

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

@jackysp
Copy link
Member

jackysp commented Mar 30, 2020

/merge

@sre-bot sre-bot added the status/can-merge Indicates a PR has been approved by a committer. label Mar 30, 2020
@sre-bot
Copy link
Contributor

sre-bot commented Mar 30, 2020

/run-all-tests

@sre-bot
Copy link
Contributor

sre-bot commented Mar 30, 2020

@imtbkcat merge failed.

@imtbkcat
Copy link
Author

/merge

@sre-bot
Copy link
Contributor

sre-bot commented Mar 30, 2020

/run-all-tests

@sre-bot
Copy link
Contributor

sre-bot commented Mar 30, 2020

@imtbkcat merge failed.

@imtbkcat
Copy link
Author

/merge

@sre-bot
Copy link
Contributor

sre-bot commented Mar 30, 2020

Your auto merge job has been accepted, waiting for 15837

@sre-bot
Copy link
Contributor

sre-bot commented Mar 30, 2020

/run-all-tests

@sre-bot
Copy link
Contributor

sre-bot commented Mar 30, 2020

@imtbkcat merge failed.

@imtbkcat
Copy link
Author

/run-unit-test

@imtbkcat imtbkcat merged commit 1142e65 into pingcap:master Mar 30, 2020
sre-bot pushed a commit to sre-bot/tidb that referenced this pull request Mar 30, 2020
Signed-off-by: sre-bot <sre-bot@pingcap.com>
imtbkcat pushed a commit to imtbkcat/tidb that referenced this pull request Mar 30, 2020
sre-bot pushed a commit to sre-bot/tidb that referenced this pull request Mar 30, 2020
Signed-off-by: sre-bot <sre-bot@pingcap.com>
sre-bot pushed a commit to sre-bot/tidb that referenced this pull request Mar 30, 2020
imtbkcat pushed a commit to imtbkcat/tidb that referenced this pull request Mar 31, 2020
@sre-bot
Copy link
Contributor

sre-bot commented Apr 7, 2020

It seems that, not for sure, we failed to cherry-pick this commit to release-3.0 release-3.1. Please comment '/run-cherry-picker' to try to trigger the cherry-picker if we did fail to cherry-pick this commit before. @imtbkcat PTAL.

@imtbkcat
Copy link
Author

imtbkcat commented Apr 8, 2020

/run-cherry-picker

@sre-bot
Copy link
Contributor

sre-bot commented Apr 8, 2020

cherry pick to release-3.0 failed

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/can-merge Indicates a PR has been approved by a committer. 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