*: support password validation options and variables #38953

CbcWestwolf · 2022-11-07T15:19:49Z

What problem does this PR solve?

Issue Number: close #38928 #38924

Problem Summary:

We need to improve the compatibility with MySQL in password validation.

What is changed and how it works?

Most of the feature is the same as MySQL's, except that:

Since TiDB does not have a mechanism like components in MySQL, we add a new global variable validate_password.enable to turn on/off the validation.
We introduce validate_password.dictionary instead of validate_password.dictionary_file, since it is hard to upload the dictionary file to the tidb-servers's local file systems.

Check List

Tests

Unit test
Integration test
Manual test (add detailed scripts or steps below)
No code

Side effects

Performance regression: Consumes more CPU
Performance regression: Consumes more Memory
Breaking backward compatibility

Documentation

Release note

Please refer to Release Notes Language Style Guide to write a quality release note.

1. support password validation when `create user`, `alter user` and `set password`
3. introduce system variables about password validation
4. support system function `validate_password_strength`

ti-chi-bot · 2022-11-07T15:19:51Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

bb7133
wjhuang2016

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

The full list of commands accepted by this bot can be found here.

Reviewer can indicate their review by submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.

CbcWestwolf · 2022-11-09T08:40:26Z

/cc wjhuang2016 xhebox

CbcWestwolf · 2022-11-09T08:43:46Z

/cc xiongjiwei

executor/simple.go

xiongjiwei · 2022-11-24T02:30:52Z

executor/simple.go

+	return false
+}
+
+func (e *SimpleExec) enableValidatePassword() bool {


Suggested change

func (e *SimpleExec) enableValidatePassword() bool {

func (e *SimpleExec) isValidatePasswordenabled() bool {

xiongjiwei · 2022-11-24T02:32:54Z

util/password-validation/password_validation.go

+	pwdLength := len(pwd)
+	if err != nil {
+		return false, err
+	}


Suggested change

pwdLength := len(pwd)

if err != nil {

return false, err

}

if err != nil {

return false, err

}

pwdLength := len(pwd)

xiongjiwei · 2022-11-24T02:34:58Z

util/password-validation/password_validation.go

+	if len(words) == 0 {
+		return true, nil
+	}
+	cache := make(map[string]interface{}, len(words))


Suggested change

cache := make(map[string]interface{}, len(words))

cache := make(map[string]struct{}, len(words))

xiongjiwei · 2022-11-24T02:35:11Z

util/password-validation/password_validation.go

+	for _, word := range words {
+		word = strings.ToLower(word)
+		if len(word) >= minPwdValidationLength && len(word) <= maxPwdValidationLength {
+			cache[word] = nil


Suggested change

cache[word] = nil

cache[word] = struct{}{}

Co-authored-by: xiongjiwei <[email protected]>

…validate_password

sessionctx/variable/sysvar.go

bb7133

LGTM

xiongjiwei · 2022-11-24T09:16:55Z

sessionctx/variable/noop.go

@@ -117,7 +115,6 @@ var noopSysVars = []*SysVar{
 	{Scope: ScopeNone, Name: "innodb_log_group_home_dir", Value: "./"},
 	{Scope: ScopeNone, Name: "performance_schema_events_statements_history_size", Value: "10"},
 	{Scope: ScopeGlobal, Name: GeneralLog, Value: Off, Type: TypeBool},
-	{Scope: ScopeGlobal, Name: "validate_password_dictionary_file", Value: ""},


this variable is still noop in tidb

xiongjiwei · 2022-11-24T09:20:54Z

sessionctx/variable/sysvar.go

+			PasswordValidtaionNumberCount.Store(int32(TidbOptInt64(val, 1)))
+			return nil
+		}, GetGlobal: func(_ context.Context, s *SessionVars) (string, error) {
+			return fmt.Sprintf("%d", PasswordValidtaionNumberCount.Load()), nil


strconv.FormatInt(PasswordValidtaionNumberCount.Load(), 10) has better performance

Modified :-)

xiongjiwei · 2022-11-24T10:28:40Z

/merge

ti-chi-bot · 2022-11-24T10:28:44Z

This pull request has been accepted and is ready to merge.

Commit hash: 9b7e323

CbcWestwolf · 2022-11-24T11:56:29Z

/run-unit-test

sre-bot · 2022-11-24T13:03:36Z

TiDB MergeCI notify

✅ Well Done! New fixed [2] after this pr merged.

CI Name	Result	Duration	Compare with Parent commit
idc-jenkins-ci-tidb/mybatis-test	🔴 failed 1, success 0, total 1	11 min	Existing failure
idc-jenkins-ci-tidb/integration-ddl-test	🔴 failed 1, success 5, total 6	4 min 59 sec	Existing failure
idc-jenkins-ci/integration-cdc-test	✅ all 39 tests passed	19 min	Fixed
idc-jenkins-ci-tidb/common-test	✅ all 11 tests passed	13 min	Fixed
idc-jenkins-ci-tidb/sqllogic-test-2	🟢 all 28 tests passed	32 min	Existing passed
idc-jenkins-ci-tidb/integration-common-test	🟢 all 17 tests passed	14 min	Existing passed
idc-jenkins-ci-tidb/tics-test	🟢 all 1 tests passed	5 min 39 sec	Existing passed
idc-jenkins-ci-tidb/sqllogic-test-1	🟢 all 26 tests passed	5 min 9 sec	Existing passed
idc-jenkins-ci-tidb/integration-compatibility-test	🟢 all 1 tests passed	2 min 40 sec	Existing passed
idc-jenkins-ci-tidb/plugin-test	🟢 build success, plugin test success	4min	Existing passed

Add system variables

1790641

CbcWestwolf changed the title ~~Add system variables~~ *: support password validation options and variables Nov 7, 2022

ti-chi-bot removed the do-not-merge/invalid-title label Nov 7, 2022

TODO: checkDictionary

9a3f177

ti-chi-bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Nov 8, 2022

CbcWestwolf added 2 commits November 8, 2022 19:49

Fix

20ff355

TODO: add UT

1779510

ti-chi-bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Nov 8, 2022

CbcWestwolf added 3 commits November 9, 2022 01:01

Fix

983023a

Add basic UT

21ad50c

Finish UT

58460d7

ti-chi-bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-none Denotes a PR that doesn't merit a release note. labels Nov 9, 2022

CbcWestwolf marked this pull request as ready for review November 9, 2022 08:39

CbcWestwolf requested a review from a team as a code owner November 9, 2022 08:39

ti-chi-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 9, 2022

ti-chi-bot requested review from wjhuang2016 and xhebox November 9, 2022 08:40

ti-chi-bot requested a review from xiongjiwei November 9, 2022 08:43

CbcWestwolf added 2 commits November 9, 2022 17:42

Update UT

44ec82e

Merge branch 'master' of github.com:pingcap/tidb into validate_password

fb503f6

wjhuang2016 approved these changes Nov 18, 2022

View reviewed changes

ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label Nov 18, 2022

CbcWestwolf added 5 commits November 23, 2022 13:33

Merge branch 'master' into validate_password

adf75e6

Merge branch 'master' of github.com:pingcap/tidb into validate_password

7811a1d

Update

ca8fc6e

Fix

e1024de

Fix

4667f3a

xiongjiwei reviewed Nov 24, 2022

View reviewed changes

CbcWestwolf and others added 3 commits November 24, 2022 10:44

Update executor/simple.go

df65a25

Co-authored-by: xiongjiwei <[email protected]>

Update

106f7c2

Merge branch 'validate_password' of github.com:CbcWestwolf/tidb into …

c0f2564

…validate_password

xiongjiwei reviewed Nov 24, 2022

View reviewed changes

sessionctx/variable/sysvar.go Show resolved Hide resolved

Update

e97d7d3

bb7133 approved these changes Nov 24, 2022

View reviewed changes

ti-chi-bot added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Nov 24, 2022

xiongjiwei reviewed Nov 24, 2022

View reviewed changes

Update

9b7e323

xiongjiwei approved these changes Nov 24, 2022

View reviewed changes

ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Nov 24, 2022

Merge branch 'master' into validate_password

cef0eb0

ti-chi-bot merged commit e205f93 into pingcap:master Nov 24, 2022

CbcWestwolf deleted the validate_password branch November 24, 2022 13:37

YangKeao mentioned this pull request Sep 12, 2024

Incompatible variables of password validation in mysql.global_variables after upgrading to v6.5 #55838

Closed

CbcWestwolf mentioned this pull request Sep 13, 2024

bootstrap: create sysvars for password validation during upgrade #56052

Closed

13 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

*: support password validation options and variables #38953

*: support password validation options and variables #38953

CbcWestwolf commented Nov 7, 2022 •

edited

Loading

ti-chi-bot commented Nov 7, 2022 •

edited

Loading

CbcWestwolf commented Nov 9, 2022

CbcWestwolf commented Nov 9, 2022

xiongjiwei Nov 24, 2022

xiongjiwei Nov 24, 2022

xiongjiwei Nov 24, 2022

xiongjiwei Nov 24, 2022

bb7133 left a comment

xiongjiwei Nov 24, 2022

xiongjiwei Nov 24, 2022

CbcWestwolf Nov 24, 2022

xiongjiwei commented Nov 24, 2022

ti-chi-bot commented Nov 24, 2022

CbcWestwolf commented Nov 24, 2022

sre-bot commented Nov 24, 2022

	func (e *SimpleExec) enableValidatePassword() bool {
	func (e *SimpleExec) isValidatePasswordenabled() bool {

	cache := make(map[string]interface{}, len(words))
	cache := make(map[string]struct{}, len(words))

*: support password validation options and variables #38953

*: support password validation options and variables #38953

Conversation

CbcWestwolf commented Nov 7, 2022 • edited Loading

What problem does this PR solve?

What is changed and how it works?

Check List

Release note

ti-chi-bot commented Nov 7, 2022 • edited Loading

CbcWestwolf commented Nov 9, 2022

CbcWestwolf commented Nov 9, 2022

xiongjiwei Nov 24, 2022

Choose a reason for hiding this comment

xiongjiwei Nov 24, 2022

Choose a reason for hiding this comment

xiongjiwei Nov 24, 2022

Choose a reason for hiding this comment

xiongjiwei Nov 24, 2022

Choose a reason for hiding this comment

bb7133 left a comment

Choose a reason for hiding this comment

xiongjiwei Nov 24, 2022

Choose a reason for hiding this comment

xiongjiwei Nov 24, 2022

Choose a reason for hiding this comment

CbcWestwolf Nov 24, 2022

Choose a reason for hiding this comment

xiongjiwei commented Nov 24, 2022

ti-chi-bot commented Nov 24, 2022

CbcWestwolf commented Nov 24, 2022

sre-bot commented Nov 24, 2022

TiDB MergeCI notify

CbcWestwolf commented Nov 7, 2022 •

edited

Loading

ti-chi-bot commented Nov 7, 2022 •

edited

Loading