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

[bugfix] security: fix check grant god when FLAGS_enable_authorize is… #4840

Merged
merged 8 commits into from
Nov 17, 2022

Conversation

codesigner
Copy link
Contributor

@codesigner codesigner commented Nov 9, 2022

God role should not be granted when enable_authorize is false

2 check was lift up before the FLAGS_enable_authorize check

 // Check 1. Reject any user grant or revoke role to GOD,
  if (targetRole == meta::cpp2::RoleType::GOD) {
    return Status::PermissionError("No permission to grant/revoke god user.");
  }

  // Check 2. Cloud auth user cannot grant role
  if (FLAGS_auth_type == "cloud") {
    return Status::PermissionError("Cloud authenticate user can't write role.");
  }

Not sure if auth_type cloud should also do this, comment if you know

What type of PR is this?

  • bug
  • feature
  • enhancement

What problem(s) does this PR solve?

Issue(s) number:

fix #4684

Description:

How do you solve it?

Special notes for your reviewer, ex. impact of this fix, design document, etc:

Checklist:

Tests:

  • Unit test(positive and negative cases)
  • Function test
  • Performance test
  • N/A

Affects:

  • Documentation affected (Please add the label if documentation needs to be modified.)
  • Incompatibility (If it breaks the compatibility, please describe it and add the label.)
  • If it's needed to cherry-pick (If cherry-pick to some branches is required, please label the destination version(s).)
  • Performance impacted: Consumes more CPU/Memory

Release notes:

Please confirm whether to be reflected in release notes and how to describe:

ex. Fixed the bug .....

@Shylock-Hg
Copy link
Contributor

Plz add test case.

@codesigner
Copy link
Contributor Author

Plz add test case.

Is there a way to set FLAGS_enable_authorize when run TCK?

@Shylock-Hg
Copy link
Contributor

Plz add test case.

Is there a way to set FLAGS_enable_authorize when run TCK?

Could disable one runner, https://github.com/vesoft-inc/nebula/blob/master/tests/common/nebula_service.py

@codesigner
Copy link
Contributor Author

disable

OK, let me try it

@Aiee
Copy link
Contributor

Aiee commented Nov 9, 2022

@MegaByte875 Please verify this change won't affect any current behavior in cloud products.

@MegaByte875
Copy link

@Aiee It's ok, no problem.

@Sophie-Xie Sophie-Xie merged commit 9796c33 into vesoft-inc:master Nov 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-for-testing PR: ready for the CI test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Shouldn't the cluster have only one account with the God role?
6 participants