codeant-ai-for-open-source[bot] commented on code in PR #39469: URL: https://github.com/apache/superset/pull/39469#discussion_r3474407364
########## superset-frontend/src/utils/generateAuthDbPassword.ts: ########## @@ -0,0 +1,287 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import { t } from '@apache-superset/core/translation'; +/** + * Client-side generator aligned with default ``AUTH_DB_CONFIG`` / Python + * ``superset.utils.auth_db_password`` (minimum length, character classes, common list). + * Keep ``AUTH_DB_COMMON_PASSWORDS`` in sync with ``_COMMON_PASSWORDS`` in that module. + */ +export const AUTH_DB_PASSWORD_MIN_LENGTH = 12; +export interface AuthDbPasswordPolicy { + password_min_length: number; + password_require_uppercase: boolean; + password_require_lowercase: boolean; + password_require_digit: boolean; + password_require_special: boolean; + password_common_list_check: boolean; +} + +export const AUTH_DB_DEFAULT_PASSWORD_POLICY: AuthDbPasswordPolicy = { + password_min_length: AUTH_DB_PASSWORD_MIN_LENGTH, + password_require_uppercase: true, + password_require_lowercase: true, + password_require_digit: true, + password_require_special: true, + password_common_list_check: true, +}; + +/** + * Lowercased entries; keep in sync with ``_COMMON_PASSWORDS`` in + * ``superset/utils/auth_db_password.py``. Sync is enforced by + * ``tests/unit_tests/utils/test_auth_db_password.py``. + */ +export const AUTH_DB_COMMON_PASSWORDS = new Set( + [ + 'password', + 'password1', + 'password123', + '123456', + '12345678', + '123456789', + 'qwerty', + 'abc123', + 'monkey', + 'letmein', + 'trustno1', + 'dragon', + 'baseball', + 'iloveyou', + 'master', + 'sunshine', + 'ashley', + 'bailey', + 'shadow', + 'superman', + 'qazwsx', + 'michael', + 'football', + 'welcome', + 'jesus', + 'ninja', + 'mustang', + 'password1!', + 'admin', + 'admin123', + 'root', + 'toor', + 'guest', + 'p@ssw0rd', + 'Passw0rd', + 'Password1', + 'Password123', + 'Welcome1', + 'Qwerty123', + ].map(s => s.toLowerCase()), +); + +const UPPER = 'ABCDEFGHJKLMNPQRSTUVWXYZ'; +const LOWER = 'abcdefghijkmnopqrstuvwxyz'; +const DIGIT = '23456789'; +const SPECIAL = '!@#$%^&*-_=+'; +const ALPHANUM = UPPER + LOWER + DIGIT; + +export interface AuthDbPasswordPolicyChecks { + minLength: boolean; + uppercase: boolean; + lowercase: boolean; + digit: boolean; + special: boolean; + commonPassword: boolean; +} + +function getCodePointLength(text: string): number { + return Array.from(text).length; +} + +/** Mirrors ``int(cfg.get("password_min_length", default))`` in auth_db_password.py. */ +function resolveAuthDbPasswordMinLength( + passwordMinLength: number | undefined | null, +): number { + if (passwordMinLength === undefined || passwordMinLength === null) { + return AUTH_DB_PASSWORD_MIN_LENGTH; + } + const parsed = Number(passwordMinLength); + if (!Number.isFinite(parsed)) { + return AUTH_DB_PASSWORD_MIN_LENGTH; + } + return Math.trunc(parsed); +} + +function secureRandomInt(maxExclusive: number): number { + if (maxExclusive <= 0) { + throw new Error('secureRandomInt: maxExclusive must be positive'); + } + const cryptoObj = globalThis.crypto; + if (!cryptoObj?.getRandomValues) { + throw new Error( + 'secureRandomInt: a Web Crypto implementation with getRandomValues ' + + 'is required to generate passwords', + ); + } + const maxUint32 = 0xffffffff; + const limit = maxUint32 - (maxUint32 % maxExclusive); + const buf = new Uint32Array(1); + let value: number; + do { + cryptoObj.getRandomValues(buf); + value = buf[0]!; + } while (value >= limit); + return value % maxExclusive; +} + +function pick(pool: string): string { + return pool[secureRandomInt(pool.length)]!; +} + +function shuffleInPlace(chars: string[]): void { + for (let i = chars.length - 1; i > 0; i -= 1) { + const j = secureRandomInt(i + 1); + const t = chars[i]!; + chars[i] = chars[j]!; + chars[j] = t; + } +} + +function getRequiredCharacterPools(policy: AuthDbPasswordPolicy): string[] { + const pools: string[] = []; + if (policy.password_require_uppercase) { + pools.push(UPPER); + } + if (policy.password_require_lowercase) { + pools.push(LOWER); + } + if (policy.password_require_digit) { + pools.push(DIGIT); + } + if (policy.password_require_special) { + pools.push(SPECIAL); + } + return pools; +} + +function getGenerationPool(policy: AuthDbPasswordPolicy): string { + let pool = ''; + if (policy.password_require_uppercase) { + pool += UPPER; + } + if (policy.password_require_lowercase) { + pool += LOWER; + } + if (policy.password_require_digit) { + pool += DIGIT; + } + if (policy.password_require_special) { + pool += SPECIAL; + } + return pool || ALPHANUM + SPECIAL; +} + +/** Returns rule-by-rule checks for default AUTH_DB password policy. */ +export function getAuthDbPasswordPolicyChecks( + password: string, + policy: AuthDbPasswordPolicy = AUTH_DB_DEFAULT_PASSWORD_POLICY, +): AuthDbPasswordPolicyChecks { + const minLength = resolveAuthDbPasswordMinLength(policy.password_min_length); + return { + minLength: getCodePointLength(password) >= minLength, + uppercase: !policy.password_require_uppercase || /[A-Z]/.test(password), + lowercase: !policy.password_require_lowercase || /[a-z]/.test(password), + digit: !policy.password_require_digit || /\p{Nd}/u.test(password), + special: + !policy.password_require_special || /[^\p{L}\p{N}\s]/u.test(password), + commonPassword: + !policy.password_common_list_check || + !AUTH_DB_COMMON_PASSWORDS.has(password.toLowerCase().trim()), + }; +} + +function satisfiesAuthDbPasswordPolicy( + password: string, + policy: AuthDbPasswordPolicy, +): boolean { + const checks = getAuthDbPasswordPolicyChecks(password, policy); + return Object.values(checks).every(Boolean); +} + +/** True when the string satisfies default AUTH_DB rules (mirrors backend checks). */ +export function satisfiesDefaultAuthDbPasswordPolicy( + password: string, +): boolean { + return satisfiesAuthDbPasswordPolicy( + password, + AUTH_DB_DEFAULT_PASSWORD_POLICY, + ); +} + +/** Returns the first validation error for a password under the given policy. */ +export function getAuthDbPasswordPolicyError( + password: string, + policy: AuthDbPasswordPolicy = AUTH_DB_DEFAULT_PASSWORD_POLICY, +): string | null { + const checks = getAuthDbPasswordPolicyChecks(password, policy); + if (!checks.minLength) { + return t( + 'Password must be at least %s characters long.', + resolveAuthDbPasswordMinLength(policy.password_min_length), + ); + } + if (policy.password_require_uppercase && !checks.uppercase) { + return t('Password must contain at least one uppercase letter.'); + } + if (policy.password_require_lowercase && !checks.lowercase) { + return t('Password must contain at least one lowercase letter.'); + } + if (policy.password_require_digit && !checks.digit) { + return t('Password must contain at least one digit.'); + } + if (policy.password_require_special && !checks.special) { + return t( + 'Password must contain at least one special character (not a letter, digit, or space).', + ); + } + if (policy.password_common_list_check && !checks.commonPassword) { + return t('This password is too common. Choose a stronger password.'); + } + return null; +} + +/** + * Returns a random password that should pass ``validate_auth_db_password`` for the + * supplied policy (defaults to server ``AUTH_DB_CONFIG`` defaults). + */ +export function generateAuthDbPassword( + policy: AuthDbPasswordPolicy = AUTH_DB_DEFAULT_PASSWORD_POLICY, +): string { + const minLen = resolveAuthDbPasswordMinLength(policy.password_min_length); + const requiredPools = getRequiredCharacterPools(policy); + const generationPool = getGenerationPool(policy); + const maxAttempts = 64; + for (let attempt = 0; attempt < maxAttempts; attempt += 1) { + const chars: string[] = requiredPools.map(pool => pick(pool)); + while (getCodePointLength(chars.join('')) < minLen) { + chars.push(pick(generationPool)); + } Review Comment: **Suggestion:** Password generation can return an empty string when the policy allows zero minimum length and no required character classes, because no characters are seeded and the fill loop never runs. This breaks the password form flow (required field) and produces an unusable generated password. Ensure generation always emits at least one character (or enforce a generation floor independent of policy). [logic error] <details> <summary><b>Severity Level:</b> Major ⚠️</summary> ```mdx - ⚠️ Generated password may be empty under len-zero policy. - ⚠️ Password reset UI shows errors after using generator. - ⚠️ Admins cannot rely on generator for relaxed policies. ``` </details> <details> <summary><b>Steps of Reproduction ✅ </b></summary> ```mdx 1. Configure AUTH_DB to allow empty passwords by setting a zero minimum length and disabling all character class checks, e.g. in `superset_config.py` use the policy from the backend test `test_validate_auth_db_password_accepts_any_length_when_min_length_is_zero` at `superset/tests/unit_tests/utils/test_auth_db_password.py:102-112` (min length 0, all `password_require_*` flags false, `password_common_list_check` false); this is treated as valid by `validate_auth_db_password()` at `superset/utils/auth_db_password.py:188-229`. 2. Sign in with AUTH_DB and open the self-service password reset modal in the UI, which uses `UserInfoModal` at `superset-frontend/src/features/userInfo/UserInfoModal.tsx:45-52`; when the modal is shown in reset mode (`!isEditMode`), it fetches the active policy from `/api/v1/me/password/policy` (lines 57-69) and stores it in `passwordPolicy` state as an `AuthDbPasswordPolicy`. 3. In the reset modal, click the "Generate password" control rendered by `GeneratePasswordInputSuffix` at `superset-frontend/src/components/GeneratePasswordInputSuffix.tsx:29-37`, which calls `onGenerate={() => { const pwd = generateAuthDbPassword(passwordPolicy); ... }}` in `UserInfoModal` at `superset-frontend/src/features/userInfo/UserInfoModal.tsx:181-189`. 4. For the zero-minimum policy with all requirements disabled, `generateAuthDbPassword()` at `superset-frontend/src/utils/generateAuthDbPassword.ts:268-285` computes `minLen = resolveAuthDbPasswordMinLength(0) = 0`, `requiredPools = []`, and thus `chars` starts as `[]`; the `while (getCodePointLength(chars.join('')) < minLen)` loop at lines 276-279 never executes, so `password` is the empty string `''`. `form.setFieldsValue({ new_password: pwd, confirm_password: pwd })` at `UserInfoModal.tsx:183-188` sets both fields to empty, immediately violating the `required: true` rule on `new_password` at lines 159-163 and causing the "New password is required" validation error. The generator has produced an unusable empty password in a configuration that backend validation explicitly supports. ``` </details> [](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=1e05df562a1241d2b24836e78e9136fc&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=1e05df562a1241d2b24836e78e9136fc&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) *(Use Cmd/Ctrl + Click for best experience)* <details> <summary><b>Prompt for AI Agent 🤖 </b></summary> ```mdx This is a comment left during a code review. **Path:** superset-frontend/src/utils/generateAuthDbPassword.ts **Line:** 276:279 **Comment:** *Logic Error: Password generation can return an empty string when the policy allows zero minimum length and no required character classes, because no characters are seeded and the fill loop never runs. This breaks the password form flow (required field) and produces an unusable generated password. Ensure generation always emits at least one character (or enforce a generation floor independent of policy). Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise. Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix ``` </details> <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F39469&comment_hash=2cc96d1012057579c462dac30231511a78b3eb841e46bc8e4e22957f9b0bfc57&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F39469&comment_hash=2cc96d1012057579c462dac30231511a78b3eb841e46bc8e4e22957f9b0bfc57&reaction=dislike'>👎</a> -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
