Skip to content

Fix settings NO_PASSWORD authentication mode in users.xml.#11081

Merged
vitlibar merged 1 commit intoClickHouse:masterfrom
vitlibar:fix-no-password-mode
May 26, 2020
Merged

Fix settings NO_PASSWORD authentication mode in users.xml.#11081
vitlibar merged 1 commit intoClickHouse:masterfrom
vitlibar:fix-no-password-mode

Conversation

@vitlibar
Copy link
Copy Markdown
Member

@vitlibar vitlibar commented May 20, 2020

I hereby agree to the terms of the CLA available at: https://yandex.ru/legal/cla/?lang=en

Changelog category:

  • Non-significant (changelog entry is not required)

Fix settings NO_PASSWORD authentication mode in users.xml.

@vitlibar vitlibar force-pushed the fix-no-password-mode branch from 5f15360 to 10f45c3 Compare May 20, 2020 20:39
@blinkov blinkov added the pr-not-for-changelog This PR should not be mentioned in the changelog label May 20, 2020
@vitlibar vitlibar marked this pull request as ready for review May 21, 2020 13:21
@vitlibar vitlibar force-pushed the fix-no-password-mode branch from 10f45c3 to 3d56acb Compare May 21, 2020 14:46
@vitlibar vitlibar added pr-bugfix Pull request with bugfix, not backported by default and removed pr-not-for-changelog This PR should not be mentioned in the changelog labels May 21, 2020
@blinkov blinkov added the pr-not-for-changelog This PR should not be mentioned in the changelog label May 21, 2020
@akuzm
Copy link
Copy Markdown
Contributor

akuzm commented May 23, 2020

Looking at the code, it seems there is no explicit record in xml for NO PASSWORD auth, right? Maybe it would be better to require it. Imagine if someone edits the config and makes a typo in password tag, and it just silently switches to accepting any password. The correct password is also accepted, so the user would think that everything is configured correctly, but in fact the password isn't checked.

@vitlibar vitlibar force-pushed the fix-no-password-mode branch from 3d56acb to eeb4cbc Compare May 25, 2020 08:35
@vitlibar
Copy link
Copy Markdown
Member Author

vitlibar commented May 25, 2020

Looking at the code, it seems there is no explicit record in xml for NO PASSWORD auth, right? Maybe it would be better to require it. Imagine if someone edits the config and makes a typo in password tag, and it just silently switches to accepting any password. The correct password is also accepted, so the user would think that everything is configured correctly, but in fact the password isn't checked.

Good point. I've changed that, now NO_PASSWORD mode is set explicitly.

@vitlibar vitlibar merged commit 2c8a355 into ClickHouse:master May 26, 2020
@vitlibar vitlibar deleted the fix-no-password-mode branch May 26, 2020 11:20
KochetovNicolai pushed a commit that referenced this pull request Jun 8, 2020
@KochetovNicolai
Merge pull request #11081 from vitlibar/fix-no-password-mode
2832981 
Fix settings NO_PASSWORD authentication mode in users.xml.

(cherry picked from commit 2c8a355)
KochetovNicolai pushed a commit that referenced this pull request Jun 8, 2020
@KochetovNicolai
Merge pull request #11081 from vitlibar/fix-no-password-mode
e4bc577 
Fix settings NO_PASSWORD authentication mode in users.xml.

(cherry picked from commit 2c8a355)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

pr-bugfix Pull request with bugfix, not backported by default pr-not-for-changelog This PR should not be mentioned in the changelog

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@vitlibar @akuzm @blinkov @KochetovNicolai