Skip to content

Add support for user names in config containing a dot#86633

Merged
azat merged 5 commits intoClickHouse:masterfrom
mkmkme:mkmkme/name-dots
Sep 9, 2025
Merged

Add support for user names in config containing a dot#86633
azat merged 5 commits intoClickHouse:masterfrom
mkmkme:mkmkme/name-dots

Conversation

@mkmkme
Copy link
Contributor

@mkmkme mkmkme commented Sep 3, 2025 •
edited
Loading

For correct config parsing, the dots in the user names have been escaped for some time [1]. This caused a confusion for some users. This commit adds the "un-escaping" to the user name handling.

Fixes #82026

[1] #58354

Changelog category (leave one):

  • Improvement

Changelog entry (a user-readable short description of the changes that goes into CHANGELOG.md):

Fixes handling of users with a dot in the name when added via config file.

Documentation entry for user-facing changes

  • Documentation is written (mandatory for new features)

@mkmkme
Add support for user names in config containing a dot
f8b0cc7 
For correct config parsing, the dots in the user names have been escaped
for some time [1]. This caused a confusion for some users. This commit
adds optional de-escaping to the config parsing process. In order to
keep the behaviour backwards-compatible, it's been hidden behind the new
config entry `<escape_dot_in_user_name>` which is turned on by default.

Fixes ClickHouse#82026

[1] ClickHouse#58354
@mkmkme
Copy link
Contributor Author

mkmkme commented Sep 3, 2025

If this needs further testing, please tell me what should I cover additionally. Also please tell me if I overlooked anything.

@bharatnc bharatnc added the can be tested Allows running workflows for external contributors label Sep 3, 2025
@clickhouse-gh
Copy link
Contributor

clickhouse-gh bot commented Sep 3, 2025 •
edited
Loading

Workflow [PR], commit [34daf9c]

Summary: ❌

job_name test_name status info comment
Stateless tests (amd_msan, parallel, 2/2) failure
03100_lwu_42_bytes_limits FAIL
Integration tests (amd_asan, old analyzer, 5/6) error

@clickhouse-gh clickhouse-gh bot added the pr-improvement Pull request with some product improvements label Sep 3, 2025
@mkmkme
Copy link
Contributor Author

mkmkme commented Sep 4, 2025

On a second thought, I am not sure whether this behaviour should be able to change via setting.

Before that commit, you still could have a user with a dot in the username. However, the behaviour was inconsistent. If you have a user foo.bar, the behaviour would differ on how you created such a user:

  1. An entry in the users.xml -> the dot in the name is escaped (foo\.bar)
  2. CREATE USER 'foo.bar' -> the dot in the name is not escaped (foo.bar)

So I wonder if the setting is needed or I should make this behaviour the default and non-changeable.

@mkmkme
Copy link
Contributor Author

mkmkme commented Sep 4, 2025

@azat you were the author of the PR that changed this behaviour as well. Would you be able to have a look at it? Thanks!

@azat azat self-assigned this Sep 4, 2025
azat
azat reviewed Sep 9, 2025
View reviewed changes
Copy link
Member

@azat azat left a comment

Choose a reason for hiding this comment

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

Thanks for fixing this!

src/Access/UsersConfigAccessStorage.cpp Outdated
user->setName(user_name);
String unescaped_user_name = user_name;
if (!escape_dot_in_user_name)
Poco::replaceInPlace(unescaped_user_name, "\\.", ".");
Copy link
Member

@azat azat Sep 9, 2025

Choose a reason for hiding this comment

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

Can we simply do this always, without any options? This looks safe for users.

Copy link
Member

@azat azat Sep 9, 2025

Choose a reason for hiding this comment

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

Oh, you already raised the question

So I wonder if the setting is needed or I should make this behaviour the default and non-changeable.

Yes, let's remove the configuration directive.

Copy link
Contributor Author

@mkmkme mkmkme Sep 9, 2025

Choose a reason for hiding this comment

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

Thanks for the feedback! I've removed the option making this change unconditional. Also, I updated the changelog entry to reflect the new behaviour. Please have a look.

azat
azat reviewed Sep 9, 2025
View reviewed changes
@azat azat enabled auto-merge September 9, 2025 12:23
auto-merge was automatically disabled September 9, 2025 13:28

Head branch was pushed to by a user without write access

@mkmkme
Copy link
Contributor Author

mkmkme commented Sep 9, 2025

Pushed a fix for the test and also renamed the test itself. Please re-add it to the merge queue if it looks good to you

@azat azat enabled auto-merge September 9, 2025 13:51
@azat azat added this pull request to the merge queue Sep 9, 2025
Merged via the queue into ClickHouse:master with commit 6b451c9 Sep 9, 2025
119 of 122 checks passed
@mkmkme mkmkme deleted the mkmkme/name-dots branch September 9, 2025 19:40
@robot-clickhouse-ci-2 robot-clickhouse-ci-2 added the pr-synced-to-cloud The PR is synced to the cloud repo label Sep 9, 2025
mkmkme pushed a commit to Altinity/ClickHouse that referenced this pull request Oct 17, 2025
@azat @mkmkme
Merge pull request ClickHouse#86633 from mkmkme/mkmkme/name-dots
11cf148 
Add support for user names in config containing a dot
@mkmkme mkmkme mentioned this pull request Oct 17, 2025
Merged
25 tasks
zvonand added a commit to Altinity/ClickHouse that referenced this pull request Oct 23, 2025
@zvonand
Merge pull request #1089 from Altinity/backport/24.8.14/86633
3b3df79 
24.8.14 Backport of ClickHouse#86633 - Add support for user names in config containing a dot
@zvonand zvonand mentioned this pull request Oct 23, 2025
Merged
mkmkme pushed a commit to Altinity/ClickHouse that referenced this pull request Nov 4, 2025
@azat @mkmkme
Merge pull request ClickHouse#86633 from mkmkme/mkmkme/name-dots
74940c9 
Add support for user names in config containing a dot
@mkmkme mkmkme mentioned this pull request Nov 4, 2025
Merged
25 tasks
zvonand pushed a commit to Altinity/ClickHouse that referenced this pull request Dec 24, 2025
@azat @zvonand
Merge pull request ClickHouse#86633 from mkmkme/mkmkme/name-dots
668316c 
Add support for user names in config containing a dot
@zvonand zvonand mentioned this pull request Dec 24, 2025
Merged
25 tasks
zvonand added a commit to Altinity/ClickHouse that referenced this pull request Dec 25, 2025
@zvonand
Merge pull request #1253 from Altinity/backports/25.8.13/86633
a78c905 
25.8.13 Backport of ClickHouse#86633 - Add support for user names in config containing a dot
zvonand pushed a commit to Altinity/ClickHouse that referenced this pull request Jan 15, 2026
@azat @zvonand
Merge pull request ClickHouse#86633 from mkmkme/mkmkme/name-dots
80eeeca 
Add support for user names in config containing a dot
@zvonand zvonand mentioned this pull request Jan 15, 2026
Closed
25 tasks
zvonand pushed a commit to Altinity/ClickHouse that referenced this pull request Jan 27, 2026
@azat @zvonand
Merge pull request ClickHouse#86633 from mkmkme/mkmkme/name-dots
b297adb 
Add support for user names in config containing a dot
@zvonand zvonand mentioned this pull request Jan 27, 2026
Merged
25 tasks
zvonand added a commit to Altinity/ClickHouse that referenced this pull request Jan 28, 2026
@zvonand
Merge pull request #1342 from Altinity/backports/25.8.15/86633
e8f10d5 
25.8.15 Stable Backport of ClickHouse#86633 - Add support for user names in config containing a dot
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@azat azat azat approved these changes

Assignees

@azat azat

Labels

can be tested Allows running workflows for external contributors pr-improvement Pull request with some product improvements pr-synced-to-cloud The PR is synced to the cloud repo

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

User with dot in users.xml

4 participants

@mkmkme @azat @bharatnc @robot-clickhouse-ci-2