codeant-ai-for-open-source[bot] commented on code in PR #39469:
URL: https://github.com/apache/superset/pull/39469#discussion_r3596011015
##########
superset-frontend/src/features/userInfo/UserInfoModal.tsx:
##########
@@ -86,30 +159,90 @@ function UserInfoModal({
</>
);
- const ResetPasswordFields = () => (
+ const ResetPasswordFields = ({ form }: { form: FormInstance }) => (
<>
<FormItem
- name="password"
- label={t('Password')}
- rules={[{ required: true, message: t('Password is required') }]}
+ name="current_password"
+ label={t('Current password')}
+ rules={[{ required: true, message: t('Current password is required')
}]}
>
<Input.Password
- name="password"
- placeholder={t("Enter the user's password")}
+ name="current_password"
+ autoComplete="current-password"
+ placeholder={t('Enter your current password')}
/>
</FormItem>
+ <FormItem
+ name="new_password"
+ label={t('New password')}
+ rules={[
+ { required: true, message: t('New password is required') },
+ {
+ validator(_, value) {
+ const password = String(value ?? '');
+ if (!password) {
+ return Promise.resolve();
+ }
+ const errorMessage = getPasswordPolicyError(password);
+ return errorMessage
+ ? Promise.reject(new Error(errorMessage))
+ : Promise.resolve();
+ },
+ },
+ ]}
+ >
+ <Input.Password
+ name="new_password"
+ autoComplete="new-password"
+ placeholder={t('Enter a new password')}
+ suffix={
+ <GeneratePasswordInputSuffix
+ onGenerate={() => {
+ const pwd = generateAuthDbPassword(passwordPolicy);
+ form.setFieldsValue({
+ new_password: pwd,
+ confirm_password: pwd,
+ });
+ // setFieldsValue does not fire the form's change handlers, so
+ // validate the affected fields to recompute submit state.
+ form
+ .validateFields(['new_password', 'confirm_password'])
+ .catch(() => {});
+ }}
+ />
Review Comment:
**Suggestion:** The password generator call can throw (for example when
server policy is impossible to satisfy, such as bcrypt with a minimum length
above 72 bytes), but this click handler does not catch that exception. That
will surface as an uncaught runtime error and break the modal instead of
showing a user-facing validation/toast message. Catch generator failures in
this handler and report a friendly error without crashing the form. [possible
bug]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
❌ Password generator click can crash reset password modal.
⚠️ Users lose feedback when policy or crypto misconfigured.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open the password generator UI wiring in
`superset-frontend/src/features/userInfo/UserInfoModal.tsx:176-215`. On
lines 198-213, the
`Input.Password` suffix renders `GeneratePasswordInputSuffix` with
`onGenerate={() => {
const pwd = generateAuthDbPassword(passwordPolicy); ... }}` (lines 201-212).
2. Inspect the generator implementation at
`superset-frontend/src/utils/generateAuthDbPassword.ts:286-310`.
`generateAuthDbPassword()` (lines 290-309) calls helpers that can throw:
- `secureRandomInt()` (lines 139-148) throws if
`globalThis.crypto.getRandomValues` is
unavailable.
- The generator loop throws `new Error('generateAuthDbPassword: exhausted
retries')` at
line 309 if no password can satisfy the supplied `AuthDbPasswordPolicy`
(e.g., an
impossible combination of minimum length and bcrypt byte limit).
3. Follow the user-facing path: the UserInfo page at
`superset-frontend/src/pages/UserInfo/index.tsx:91-269` opens
`ChangePasswordModal` (lines
245-253), which renders `UserInfoModal` with `isEditMode={false}`. This
causes
`ResetPasswordFields` (lines 162-260) to be shown, including the generator
button wired to
the `onGenerate` handler above.
4. In an environment without Web Crypto (`globalThis.crypto.getRandomValues`
missing) or
under a misconfigured password policy that cannot be satisfied, clicking the
"Generate
password" button (rendered by `GeneratePasswordInputSuffix` in
`superset-frontend/src/components/GeneratePasswordInputSuffix.tsx:29-48`)
will propagate
the exception from `generateAuthDbPassword()` through `onGenerate` without
any try/catch.
React treats this as an uncaught error during the click event, which can
break the modal
or hit a global error boundary instead of displaying a toast or validation
error,
confirming the lack of defensive handling on this path.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=6429e61bdb484492864de314c8e70a48&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=6429e61bdb484492864de314c8e70a48&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/features/userInfo/UserInfoModal.tsx
**Line:** 201:212
**Comment:**
*Possible Bug: The password generator call can throw (for example when
server policy is impossible to satisfy, such as bcrypt with a minimum length
above 72 bytes), but this click handler does not catch that exception. That
will surface as an uncaught runtime error and break the modal instead of
showing a user-facing validation/toast message. Catch generator failures in
this handler and report a friendly error without crashing the form.
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=f88cecb1ba38705d024d3176bd87a949a90992b891536417f1bce05fd51a1e02&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F39469&comment_hash=f88cecb1ba38705d024d3176bd87a949a90992b891536417f1bce05fd51a1e02&reaction=dislike'>👎</a>
##########
superset-frontend/src/features/userInfo/UserInfoModal.tsx:
##########
@@ -36,10 +50,51 @@ function UserInfoModal({
user,
}: UserInfoModalProps) {
const { addDangerToast, addSuccessToast } = useToasts();
+ const [passwordPolicy, setPasswordPolicy] = useState<AuthDbPasswordPolicy>(
+ AUTH_DB_DEFAULT_PASSWORD_POLICY,
+ );
+
+ useEffect(() => {
+ if (!show || isEditMode) {
+ return;
+ }
+ let ignore = false;
+ SupersetClient.get({
+ endpoint: '/api/v1/me/password/policy',
+ })
+ .then(({ json }) => {
+ if (!ignore && json?.result) {
+ setPasswordPolicy(json.result as AuthDbPasswordPolicy);
+ }
+ })
+ .catch(() => {
+ // Keep default policy when endpoint is unavailable.
+ });
+ return () => {
+ ignore = true;
+ };
+ }, [show, isEditMode]);
+
+ const getPasswordPolicyError = (password: string): string | null =>
+ getAuthDbPasswordPolicyError(password, passwordPolicy);
+
+ const getSubmitErrorMessage = async (error: unknown): Promise<string> => {
+ const clientError = await getClientErrorObject(
+ error as Parameters<typeof getClientErrorObject>[0],
+ );
+ const raw = clientError.error ?? clientError.message;
+ if (typeof raw === 'string' && raw) {
+ return raw;
+ }
+ if (raw && typeof raw === 'object') {
+ return (Object.values(raw).flat() as string[]).join(' ');
+ }
+ return t('Something went wrong while saving the user info');
Review Comment:
**Suggestion:** This fallback error message is shown for both profile edit
and password-change submissions, but it explicitly says “saving the user info,”
which is incorrect for password reset failures and will mislead users. Use an
operation-appropriate fallback message for the password flow. [comment mismatch]
<details>
<summary><b>Severity Level:</b> Minor 🧹</summary>
```mdx
⚠️ Password reset errors misreported as user info issues.
⚠️ Can confuse users diagnosing password change failures.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Inspect the shared error-message helper in
`superset-frontend/src/features/userInfo/UserInfoModal.tsx:81-93`.
`getSubmitErrorMessage()` falls back at line 92 to `t('Something went wrong
while saving
the user info')` when neither `clientError.error` nor `clientError.message`
provides a
usable string.
2. Examine the form submission handler in the same file at
`handleFormSubmit()` on lines
104-137. For edit mode (lines 106-112), it PUTs `/api/v1/me/` to update
profile info; for
password reset (lines 113-133), it PUTs `/api/v1/me/password` to change the
password and
re-authenticates the session.
3. Note the shared catch block on lines 134-137: `catch (error) {
addDangerToast(await
getSubmitErrorMessage(error)); throw error; }`. This is used for both the
profile edit
flow and the password-change flow, meaning the fallback string from step 1
is shown on any
generic failure regardless of which endpoint failed.
4. Trigger a generic backend or network error on the password-change
endpoint (e.g., cause
a 500 or a timeout so that `getClientErrorObject()` yields no specific
message). The
password reset submission from the ChangePasswordModal (wired in
`superset-frontend/src/pages/UserInfo/index.tsx:245-253`) results in a toast
with the
fallback `Something went wrong while saving the user info`, which
inaccurately describes a
password-change failure as a user-info save issue and can mislead users
diagnosing
password problems.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=81b953e4df44428d9f1dc9364a217398&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=81b953e4df44428d9f1dc9364a217398&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/features/userInfo/UserInfoModal.tsx
**Line:** 92:92
**Comment:**
*Comment Mismatch: This fallback error message is shown for both
profile edit and password-change submissions, but it explicitly says “saving
the user info,” which is incorrect for password reset failures and will mislead
users. Use an operation-appropriate fallback message for the password flow.
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=376f8b7811269a2b97a22577bf0f2957684c792e32a468db300ea3fd6bbd3604&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F39469&comment_hash=376f8b7811269a2b97a22577bf0f2957684c792e32a468db300ea3fd6bbd3604&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]
