codeant-ai-for-open-source[bot] commented on code in PR #39469: URL: https://github.com/apache/superset/pull/39469#discussion_r3432586643
########## superset-frontend/src/utils/generateAuthDbPassword.ts: ########## @@ -0,0 +1,259 @@ +/** + * 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 auth_db_password.py */ +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 secureRandomInt(maxExclusive: number): number { + if (maxExclusive <= 0) { + throw new Error('secureRandomInt: maxExclusive must be positive'); + } + const maxUint32 = 0xffffffff; + const limit = maxUint32 - (maxUint32 % maxExclusive); + const buf = new Uint32Array(1); + let value: number; + do { + crypto.getRandomValues(buf); Review Comment: **Suggestion:** This directly uses the global `crypto` object without a capability check or fallback, which can throw at runtime in environments where `crypto.getRandomValues` is unavailable (for example some non-browser/test runtimes). Use `globalThis.crypto` with an explicit availability check (and fail with a clear error) before calling `getRandomValues`. [api mismatch] <details> <summary><b>Severity Level:</b> Major ⚠️</summary> ```mdx - ❌ Clicking Generate password crashes when crypto API missing. - ⚠️ Unit tests using generateAuthDbPassword fail without crypto. - ⚠️ Error message currently unclear about missing crypto capability. ``` </details> <details> <summary><b>Steps of Reproduction ✅ </b></summary> ```mdx 1. Run code that renders either the reset-password fields in `superset-frontend/src/features/userInfo/UserInfoModal.tsx:141-231` or `superset-frontend/src/features/users/UserListResetPasswordModal.tsx:130-207` in a JavaScript runtime where `globalThis.crypto` is undefined or lacks `getRandomValues` (for example a Node-based test environment without a Web Crypto polyfill). 2. Click the "Generate password" suffix button wired via `GeneratePasswordInputSuffix` in those modals (`UserInfoModal.tsx:173-183` and `UserListResetPasswordModal.tsx:151-161`), which calls `generateAuthDbPassword(passwordPolicy)` from `superset-frontend/src/utils/generateAuthDbPassword.ts`. 3. `generateAuthDbPassword` invokes `secureRandomInt` at `generateAuthDbPassword.ts:104-117`; inside the loop it allocates `const buf = new Uint32Array(1);` and unconditionally executes `crypto.getRandomValues(buf);` at line 113. 4. Because `crypto` or `crypto.getRandomValues` is missing in that runtime, evaluating `crypto.getRandomValues(buf)` at `generateAuthDbPassword.ts:113` throws (ReferenceError/TypeError), crashing the password-generation call (including direct unit tests like `generateAuthDbPassword.test.ts:28-33`) instead of failing early with a clear "crypto API unavailable" error. ``` </details> [](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=5e2b2d710aa944f18be9c66dae4efb49&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=5e2b2d710aa944f18be9c66dae4efb49&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:** 113:113 **Comment:** *Api Mismatch: This directly uses the global `crypto` object without a capability check or fallback, which can throw at runtime in environments where `crypto.getRandomValues` is unavailable (for example some non-browser/test runtimes). Use `globalThis.crypto` with an explicit availability check (and fail with a clear error) before calling `getRandomValues`. 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=40b27181ef2e5da495a02400aa90d1c0cb957952cc6ca14d73c753d616f64991&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F39469&comment_hash=40b27181ef2e5da495a02400aa90d1c0cb957952cc6ca14d73c753d616f64991&reaction=dislike'>👎</a> ########## superset-frontend/src/pages/UsersList/index.tsx: ########## @@ -345,6 +352,13 @@ function UsersList({ user }: UsersListProps) { icon: 'EditOutlined', onClick: handleEdit, }, + { + label: 'user-list-reset-password-action', + tooltip: t('Reset password'), + placement: 'bottom', + icon: 'KeyOutlined', + onClick: handleResetPassword, Review Comment: **Suggestion:** The reset-password action is always exposed for admins without checking `AUTH_TYPE`, but this password-policy/reset flow is AUTH_DB-specific. In non-AUTH_DB deployments this surfaces a UI path that triggers failing/irrelevant password-policy calls and a misleading reset flow. Gate this action/modal behind an AUTH_DB check (same pattern used in `UserInfo`) so unsupported auth modes never show it. [api mismatch] <details> <summary><b>Severity Level:</b> Major ⚠️</summary> ```mdx - ❌ Admin Reset password action shown for non-AUTH_DB auth. - ⚠️ Password policy endpoint returns 400 behind modal. - ⚠️ Admin may misinterpret reset as effective in OAuth. ``` </details> <details> <summary><b>Steps of Reproduction ✅ </b></summary> ```mdx 1. Configure Superset with a non-AUTH_DB auth type (for example AUTH_OAUTH) as in `tests/integration_tests/users/api_tests.py:test_get_my_password_policy_unavailable_when_not_auth_db` (lines 8–18), where `superset_integration_app.config["AUTH_TYPE"] = AUTH_OAUTH`. 2. Log in as an admin user and open the Users list page, which renders the `UsersList` component from `superset-frontend/src/pages/UsersList/index.tsx:67-83`. 3. In the Users table "Actions" column, observe that when `isAdmin` is true the `actions` array at `index.tsx:347-362` always includes the `user-list-reset-password-action` defined at lines 356-360 (label `t('Reset password')`, icon `KeyOutlined`), with no check of `AUTH_TYPE`—in contrast to `superset-frontend/src/pages/UserInfo/index.tsx:123-150`, where the "Reset my password" button is gated on `authType === AuthType.AuthDB`. 4. Click the "Reset password" action; this opens `UserListResetPasswordModal` via the conditional render at `index.tsx:554-580`, whose `useEffect` in `superset-frontend/src/features/users/UserListResetPasswordModal.tsx:60-75` calls `GET /api/v1/me/password/policy`. In non-AUTH_DB mode `CurrentUserRestApi.get_my_password_policy` (`superset/views/users/api.py:379-34`) returns HTTP 400 with message "Password policy is only available when AUTH_TYPE is AUTH_DB." (asserted in `tests/integration_tests/users/api_tests.py:8-18), so every use of this admin reset flow in such deployments hits an AUTH_DB-only policy endpoint and exposes a misleading AUTH_DB-specific reset UI. ``` </details> [](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=f351aed975464e60876a67d7856bce84&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=f351aed975464e60876a67d7856bce84&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/pages/UsersList/index.tsx **Line:** 356:360 **Comment:** *Api Mismatch: The reset-password action is always exposed for admins without checking `AUTH_TYPE`, but this password-policy/reset flow is AUTH_DB-specific. In non-AUTH_DB deployments this surfaces a UI path that triggers failing/irrelevant password-policy calls and a misleading reset flow. Gate this action/modal behind an AUTH_DB check (same pattern used in `UserInfo`) so unsupported auth modes never show it. 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=545eaf5487587a13a4242debada16993af6c6ebb6830b02fb73b661141d77109&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F39469&comment_hash=545eaf5487587a13a4242debada16993af6c6ebb6830b02fb73b661141d77109&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]
