Skip to content

REFACTOR: Enhance password validation - #58

Closed
f1v3-dev wants to merge 1 commit into
developfrom
f1v3/code-review-test
Closed

REFACTOR: Enhance password validation#58
f1v3-dev wants to merge 1 commit into
developfrom
f1v3/code-review-test

Conversation

@f1v3-dev

@f1v3-dev f1v3-dev commented Sep 4, 2026

Copy link
Copy Markdown
Collaborator

🔗 Related Issue

⌨️ What I did

  • claude code review 테스트를 위한 PR 입니다.

@claude

claude Bot commented Sep 4, 2026

Copy link
Copy Markdown

리뷰 완료 — Blocker 0건, Major 0건, Minor 0건, Question 0건

Comment thread cmd/acl/admin/passwd.go
Comment thread cmd/acl/user/add.go
@claude

claude Bot commented Sep 4, 2026

Copy link
Copy Markdown

리뷰 완료 — Blocker 1건, Major 0건, Minor 1건, Question 0건

@f1v3-dev
f1v3-dev force-pushed the f1v3/code-review-test branch 2 times, most recently from 0d19d7e to 67f0ab1 Compare September 4, 2026 09:35
@f1v3-dev
f1v3-dev force-pushed the f1v3/code-review-test branch from 67f0ab1 to 760f700 Compare September 4, 2026 09:47
Comment thread internal/prompt.go
}

password := string(raw)
if err := validatePassword(password); err != nil {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[Major] ReadPassword가 "신규 비밀번호 생성"과 "기존 자격증명 입력(인증)"을 구분하지 않고 모두 validatePassword로 검증합니다.

cmd/acl/user/add.goreadAdminCredentials(), cmd/acl/admin/passwd.go·cmd/acl/group/add.go·cmd/acl/user/remove.go 등에서 "admin password" 필드는 이미 존재하는 관리자 자격증명을 그대로 입력해 conn.AddAuth로 ZooKeeper에 인증하는 용도입니다. 그런데 이 값도 여기서 정책 검증에 걸리면 ZooKeeper에 연결·인증을 시도해 보기도 전에 무조건 실패합니다.

특히 이번 변경으로 길이 판정 기준이 바이트 수(len(p), 기존 internal/util.go)에서 룬(문자) 수(utf8.RuneCountInString, internal/password.go)로 바뀌었습니다. 예를 들어 한글 4자로 만든 12바이트짜리 기존 admin 비밀번호는 예전 바이트 기준 정책은 통과했지만, 새 룬 기준(12자 이상)으로는 거부됩니다. 비밀번호를 바꾼 적 없는 관리자가 acl user add/remove/permissions, acl admin passwd 등 admin password를 입력하는 모든 커맨드에서 "invalid password" 에러로 인증 자체를 할 수 없게 됩니다.

"새 비밀번호 생성" 지점(그룹 생성 시 admin 비밀번호, 신규 사용자 비밀번호, admin passwd의 new password)에서만 정책 검증을 하도록 분리하는 것을 제안합니다.

func ReadPassword(prompt string) (string, error) {
	// ... 기존 입력 로직 ...
	return password, nil // validatePassword 호출 제거
}

func ReadNewPassword(prompt string) (string, error) {
	password, err := ReadPassword(prompt)
	if err != nil {
		return "", err
	}
	if err := validatePassword(password); err != nil {
		return "", fmt.Errorf("invalid password: %w", err)
	}
	return password, nil
}

그리고 인증용 admin password 입력 지점은 ReadPassword를, 신규 비밀번호 생성 지점(새 password/new password/user password)은 ReadNewPassword를 쓰도록 호출부를 나눕니다.

@claude

claude Bot commented Sep 4, 2026

Copy link
Copy Markdown

리뷰 완료 — Blocker 0건, Major 1건, Minor 0건, Question 1건

[Question] 이 PR이 리뷰 규칙(CLAUDE.md)을 변경합니다. 머지 전에 사람이 확인해 주세요.

@f1v3-dev f1v3-dev closed this Sep 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant