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
Specify grants for users in users.xml
#49381
Conversation
This is an automated comment for commit ec011b9 with description of existing statuses. It's updated for the latest CI running
|
@@ -332,7 +332,7 @@ bool ParserGrantQuery::parseImpl(Pos & pos, ASTPtr & node, Expected & expected) | |||
parseOnCluster(pos, expected, cluster); | |||
|
|||
std::shared_ptr<ASTRolesOrUsersSet> grantees; | |||
if (!parseToGrantees(pos, expected, is_revoke, grantees)) | |||
if (!parseToGrantees(pos, expected, is_revoke, grantees) && !allow_no_grantees) | |||
return false; |
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.
Probably it's better to move this logic to UsersConfigAccessStorage next to the checks about roles and throw an exception with a user-friendly error message.
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.
This doesn't throw an exception if grantees are specified. Instead, this bool option just allows the parser not to fail if no grantees were provided. And yes, I definitely forgot to check if grantees are empty like I did with roles
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.
Please add a comment about what that allow_no_grantees
means, because it doesn't seem obvious.
for (const String & dictionary : *dictionaries) | ||
user->access.grantWithGrantOption(AccessFlags::allDictionaryFlags(), IDictionary::NO_DATABASE_TAG, dictionary); | ||
} | ||
if (grant_queries && databases) |
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.
It seems dictionaries
, access_management
, named_collection_control
, show_named_collections_secrets
should be also added to this condition.
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Adding the
grants
field in the users.xml file, which allows specifying grants for users.Documentation entry for user-facing changes
Example