korbit-ai[bot] commented on code in PR #33620: URL: https://github.com/apache/superset/pull/33620#discussion_r2116591876
########## superset/views/users/schemas.py: ########## @@ -14,8 +14,17 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. -from marshmallow import Schema +from flask_appbuilder.security.sqla.apis.user.schema import User +from flask_appbuilder.security.sqla.apis.user.validator import ( + PasswordComplexityValidator, +) +from marshmallow import fields, Schema from marshmallow.fields import Boolean, Integer, String +from marshmallow.validate import Length + +first_name_description = "The current user's first name" +last_name_description = "The current user's last name" +password_description = "The current user's password for authentication" # noqa: S105 Review Comment: ### Redundant field descriptions <sub></sub> <details> <summary>Tell me more</summary> ###### What is the issue? The field descriptions are overly simplistic and don't provide any useful information beyond what's obvious from the field names. ###### Why this matters Self-evident descriptions add maintenance overhead without providing value to developers or API consumers. ###### Suggested change ∙ *Feature Preview* first_name_description = "User's given name, max 64 characters" last_name_description = "User's family name, max 64 characters" password_description = "Must meet system's password complexity requirements" # noqa: S105 ###### Provide feedback to improve future suggestions [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/fc3a95a1-8b15-499d-b5e2-51cc1274cc9a/upvote) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/fc3a95a1-8b15-499d-b5e2-51cc1274cc9a?what_not_true=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/fc3a95a1-8b15-499d-b5e2-51cc1274cc9a?what_out_of_scope=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/fc3a95a1-8b15-499d-b5e2-51cc1274cc9a?what_not_in_standard=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/fc3a95a1-8b15-499d-b5e2-51cc1274cc9a) </details> <sub> 💬 Looking for more details? Reply to this comment to chat with Korbit. </sub> <!--- korbi internal id:76360467-ac34-435e-8ed9-e4e85ec1fa6a --> [](76360467-ac34-435e-8ed9-e4e85ec1fa6a) ########## superset/views/user_info.py: ########## @@ -0,0 +1,34 @@ +# 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. +from flask_appbuilder import permission_name +from flask_appbuilder.api import expose +from flask_appbuilder.security.decorators import has_access + +from superset.superset_typing import FlaskResponse + +from .base import BaseSupersetView + + +class UserInfoView(BaseSupersetView): + route_base = "/" + class_permission_name = "user" + + @expose("/user_info/") + @has_access + @permission_name("read") + def list(self) -> FlaskResponse: + return super().render_app_template() Review Comment: ### Non-Descriptive Method Name <sub></sub> <details> <summary>Tell me more</summary> ###### What is the issue? The method name 'list' is generic and doesn't reflect its purpose of retrieving user information. ###### Why this matters Generic method names reduce code readability and make it harder to understand the intention of the endpoint without diving into implementation details. ###### Suggested change ∙ *Feature Preview* Rename the method to better reflect its purpose: ```python def get_user_info(self) -> FlaskResponse: return super().render_app_template() ``` ###### Provide feedback to improve future suggestions [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/802db4f4-b71c-4c37-b668-1abb578d8ea8/upvote) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/802db4f4-b71c-4c37-b668-1abb578d8ea8?what_not_true=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/802db4f4-b71c-4c37-b668-1abb578d8ea8?what_out_of_scope=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/802db4f4-b71c-4c37-b668-1abb578d8ea8?what_not_in_standard=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/802db4f4-b71c-4c37-b668-1abb578d8ea8) </details> <sub> 💬 Looking for more details? Reply to this comment to chat with Korbit. </sub> <!--- korbi internal id:10699ed7-5f1a-4ccf-b2e3-448db49a4f09 --> [](10699ed7-5f1a-4ccf-b2e3-448db49a4f09) ########## superset/views/users/schemas.py: ########## @@ -14,8 +14,17 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. -from marshmallow import Schema +from flask_appbuilder.security.sqla.apis.user.schema import User +from flask_appbuilder.security.sqla.apis.user.validator import ( + PasswordComplexityValidator, +) +from marshmallow import fields, Schema from marshmallow.fields import Boolean, Integer, String Review Comment: ### Inconsistent Marshmallow Field Imports <sub></sub> <details> <summary>Tell me more</summary> ###### What is the issue? Inconsistent import style for marshmallow fields - some fields are imported directly while others are accessed through the fields module. ###### Why this matters Mixed import styles make it harder to understand where symbols come from and can cause confusion when maintaining the code. It also makes it more difficult to spot potential naming conflicts. ###### Suggested change ∙ *Feature Preview* ```python from marshmallow import Schema from marshmallow.fields import Boolean, Integer, String, String as MarshmallowString # Then use either: first_name = MarshmallowString( required=False, metadata={"description": first_name_description}, validate=[Length(1, 64)], ) ``` Or alternatively: ```python from marshmallow import fields, Schema # Then use consistently: first_name = fields.String( required=False, metadata={"description": first_name_description}, validate=[Length(1, 64)], ) ``` ###### Provide feedback to improve future suggestions [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/b3602e6e-76d0-4938-b2d9-7bf9bd49c329/upvote) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/b3602e6e-76d0-4938-b2d9-7bf9bd49c329?what_not_true=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/b3602e6e-76d0-4938-b2d9-7bf9bd49c329?what_out_of_scope=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/b3602e6e-76d0-4938-b2d9-7bf9bd49c329?what_not_in_standard=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/b3602e6e-76d0-4938-b2d9-7bf9bd49c329) </details> <sub> 💬 Looking for more details? Reply to this comment to chat with Korbit. </sub> <!--- korbi internal id:222d6895-e1b8-469d-be11-34f0cd53beb4 --> [](222d6895-e1b8-469d-be11-34f0cd53beb4) ########## superset-frontend/src/features/userInfo/UserInfoModal.tsx: ########## @@ -0,0 +1,150 @@ +/** + * 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 { SupersetClient, t } from '@superset-ui/core'; +import { useToasts } from 'src/components/MessageToasts/withToasts'; +import FormModal from 'src/components/Modal/FormModal'; +import { FormItem } from 'src/components/Form'; +import { Input } from 'src/components/Input'; +import { User } from 'src/types/bootstrapTypes'; +import { BaseUserListModalProps, FormValues } from '../users/types'; + +export interface UserInfoModalProps extends BaseUserListModalProps { + isEditMode?: boolean; + user?: User; +} + +function UserInfoModal({ + show, + onHide, + onSave, + isEditMode, + user, +}: UserInfoModalProps) { + const { addDangerToast, addSuccessToast } = useToasts(); + + const requiredFields = isEditMode + ? ['first_name', 'last_name'] + : ['password', 'confirm_password']; + const initialValues = isEditMode + ? { + first_name: user?.firstName, + last_name: user?.lastName, + } + : {}; + const handleFormSubmit = async (values: FormValues) => { + try { + const { confirm_password, ...payload } = values; + await SupersetClient.put({ + endpoint: `/api/v1/me/`, + jsonPayload: { ...payload }, + }); Review Comment: ### Unverified secure transport for password transmission <sub></sub> <details> <summary>Tell me more</summary> ###### What is the issue? The password is being sent in the request payload without explicit HTTPS verification or transport layer security checks. ###### Why this matters If the connection is not secure, sensitive password data could be intercepted during transmission. This is particularly critical for password management functionality. ###### Suggested change ∙ *Feature Preview* Add transport security verification by ensuring the SupersetClient enforces HTTPS: ```typescript await SupersetClient.put({ endpoint: `/api/v1/me/`, jsonPayload: { ...payload }, forceSSL: true, // Add explicit HTTPS enforcement }); ``` ###### Provide feedback to improve future suggestions [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/1760bef3-c9be-496d-8c73-4b4ea278e095/upvote) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/1760bef3-c9be-496d-8c73-4b4ea278e095?what_not_true=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/1760bef3-c9be-496d-8c73-4b4ea278e095?what_out_of_scope=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/1760bef3-c9be-496d-8c73-4b4ea278e095?what_not_in_standard=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/1760bef3-c9be-496d-8c73-4b4ea278e095) </details> <sub> 💬 Looking for more details? Reply to this comment to chat with Korbit. </sub> <!--- korbi internal id:474032d4-de48-4e13-b624-09732e99091d --> [](474032d4-de48-4e13-b624-09732e99091d) ########## superset-frontend/src/features/userInfo/UserInfoModal.tsx: ########## @@ -0,0 +1,150 @@ +/** + * 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 { SupersetClient, t } from '@superset-ui/core'; +import { useToasts } from 'src/components/MessageToasts/withToasts'; +import FormModal from 'src/components/Modal/FormModal'; +import { FormItem } from 'src/components/Form'; +import { Input } from 'src/components/Input'; +import { User } from 'src/types/bootstrapTypes'; +import { BaseUserListModalProps, FormValues } from '../users/types'; + +export interface UserInfoModalProps extends BaseUserListModalProps { + isEditMode?: boolean; + user?: User; +} + +function UserInfoModal({ + show, + onHide, + onSave, + isEditMode, + user, +}: UserInfoModalProps) { + const { addDangerToast, addSuccessToast } = useToasts(); + + const requiredFields = isEditMode + ? ['first_name', 'last_name'] + : ['password', 'confirm_password']; + const initialValues = isEditMode + ? { + first_name: user?.firstName, + last_name: user?.lastName, + } + : {}; + const handleFormSubmit = async (values: FormValues) => { + try { + const { confirm_password, ...payload } = values; + await SupersetClient.put({ + endpoint: `/api/v1/me/`, + jsonPayload: { ...payload }, + }); + addSuccessToast( + isEditMode + ? t('The user was updated successfully') + : t('The password reset was successfull'), + ); + onSave(); + } catch (error) { + addDangerToast(t('Something went wrong while saving the user info')); + } + }; + + const EditModeFields = () => ( + <> + <FormItem + name="first_name" + label={t('First name')} + rules={[{ required: true, message: t('First name is required') }]} + > + <Input + name="first_name" + placeholder={t("Enter the user's first name")} + /> + </FormItem> + <FormItem + name="last_name" + label={t('Last name')} + rules={[{ required: true, message: t('Last name is required') }]} + > + <Input name="last_name" placeholder={t("Enter the user's last name")} /> + </FormItem> + </> + ); + + const ResetPasswordFields = () => ( + <> + <FormItem + name="password" + label={t('Password')} + rules={[{ required: true, message: t('Password is required') }]} + > + <Input.Password + name="password" + placeholder="Enter the user's password" Review Comment: ### Missing Translation Wrapper <sub></sub> <details> <summary>Tell me more</summary> ###### What is the issue? Untranslated string literal used directly in placeholder, while other strings use the translation function t(). ###### Why this matters Inconsistent use of translation function makes it harder to maintain internationalization and reduces code consistency. ###### Suggested change ∙ *Feature Preview* ```typescript placeholder={t("Enter the user's password")} ``` ###### Provide feedback to improve future suggestions [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/b1bce896-c808-471f-b34c-5fc9812e4561/upvote) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/b1bce896-c808-471f-b34c-5fc9812e4561?what_not_true=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/b1bce896-c808-471f-b34c-5fc9812e4561?what_out_of_scope=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/b1bce896-c808-471f-b34c-5fc9812e4561?what_not_in_standard=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/b1bce896-c808-471f-b34c-5fc9812e4561) </details> <sub> 💬 Looking for more details? Reply to this comment to chat with Korbit. </sub> <!--- korbi internal id:91d27f47-741b-4f00-9f24-07de55d7bd5a --> [](91d27f47-741b-4f00-9f24-07de55d7bd5a) ########## superset-frontend/src/views/routes.tsx: ########## @@ -137,6 +137,10 @@ const UsersList: LazyExoticComponent<any> = lazy( () => import(/* webpackChunkName: "UsersList" */ 'src/pages/UsersList'), ); + +const UserInfo = lazy( + () => import(/* webpackChunkName: "UserInfo" */ 'src/pages/UserInfo'), +); const ActionLogList: LazyExoticComponent<any> = lazy( () => import(/* webpackChunkName: "ActionLogList" */ 'src/pages/ActionLog'), ); Review Comment: ### Inconsistent Component Type Annotations <sub></sub> <details> <summary>Tell me more</summary> ###### What is the issue? Inconsistent type annotations across lazy-loaded components. Some components explicitly declare LazyExoticComponent<any> while others don't have type annotations. ###### Why this matters Inconsistent typing makes it harder to understand component expectations and may hide potential type-related issues. ###### Suggested change ∙ *Feature Preview* Add consistent type annotations to all lazy-loaded components: ```typescript const UsersList: LazyExoticComponent<any> = lazy( () => import('src/pages/UsersList'), ); const UserInfo: LazyExoticComponent<any> = lazy( () => import('src/pages/UserInfo'), ); const ActionLogList: LazyExoticComponent<any> = lazy( () => import('src/pages/ActionLog'), ); ``` ###### Provide feedback to improve future suggestions [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/a3497b96-6031-4cbf-9ae1-8ceceb9d5fb1/upvote) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/a3497b96-6031-4cbf-9ae1-8ceceb9d5fb1?what_not_true=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/a3497b96-6031-4cbf-9ae1-8ceceb9d5fb1?what_out_of_scope=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/a3497b96-6031-4cbf-9ae1-8ceceb9d5fb1?what_not_in_standard=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/a3497b96-6031-4cbf-9ae1-8ceceb9d5fb1) </details> <sub> 💬 Looking for more details? Reply to this comment to chat with Korbit. </sub> <!--- korbi internal id:a2af845e-21a7-4b8c-ba1b-26d7c295fc88 --> [](a2af845e-21a7-4b8c-ba1b-26d7c295fc88) ########## superset-frontend/src/views/routes.tsx: ########## @@ -243,6 +247,7 @@ export const routes: Routes = [ path: '/sqllab/', Component: SqlLab, }, + { path: '/user_info/', Component: UserInfo }, Review Comment: ### Missing Permission Check for User Info Route <sub></sub> <details> <summary>Tell me more</summary> ###### What is the issue? The user info route is accessible to all users regardless of their permissions, which contradicts typical user management security patterns. ###### Why this matters This could allow unauthorized users to access and potentially modify user information, creating a security vulnerability. ###### Suggested change ∙ *Feature Preview* Move the user info route inside the admin permission check block alongside other user management routes: ```typescript if (isAdmin) { routes.push( { path: '/roles/', Component: RolesList, }, { path: '/users/', Component: UsersList, }, { path: '/user_info/', Component: UserInfo, }, ); } ``` ###### Provide feedback to improve future suggestions [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/b0981a9d-c63c-498e-ad2e-bfef1d2d0c20/upvote) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/b0981a9d-c63c-498e-ad2e-bfef1d2d0c20?what_not_true=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/b0981a9d-c63c-498e-ad2e-bfef1d2d0c20?what_out_of_scope=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/b0981a9d-c63c-498e-ad2e-bfef1d2d0c20?what_not_in_standard=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/b0981a9d-c63c-498e-ad2e-bfef1d2d0c20) </details> <sub> 💬 Looking for more details? Reply to this comment to chat with Korbit. </sub> <!--- korbi internal id:d1f6d1cd-ed8a-484e-b469-5f0aa04b53ab --> [](d1f6d1cd-ed8a-484e-b469-5f0aa04b53ab) ########## superset-frontend/src/features/userInfo/UserInfoModal.tsx: ########## @@ -0,0 +1,150 @@ +/** + * 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 { SupersetClient, t } from '@superset-ui/core'; +import { useToasts } from 'src/components/MessageToasts/withToasts'; +import FormModal from 'src/components/Modal/FormModal'; +import { FormItem } from 'src/components/Form'; +import { Input } from 'src/components/Input'; +import { User } from 'src/types/bootstrapTypes'; +import { BaseUserListModalProps, FormValues } from '../users/types'; + +export interface UserInfoModalProps extends BaseUserListModalProps { + isEditMode?: boolean; + user?: User; +} + +function UserInfoModal({ + show, + onHide, + onSave, + isEditMode, + user, +}: UserInfoModalProps) { + const { addDangerToast, addSuccessToast } = useToasts(); + + const requiredFields = isEditMode + ? ['first_name', 'last_name'] + : ['password', 'confirm_password']; + const initialValues = isEditMode + ? { + first_name: user?.firstName, + last_name: user?.lastName, + } + : {}; + const handleFormSubmit = async (values: FormValues) => { + try { + const { confirm_password, ...payload } = values; + await SupersetClient.put({ + endpoint: `/api/v1/me/`, + jsonPayload: { ...payload }, + }); + addSuccessToast( + isEditMode + ? t('The user was updated successfully') + : t('The password reset was successfull'), + ); + onSave(); + } catch (error) { + addDangerToast(t('Something went wrong while saving the user info')); + } + }; + + const EditModeFields = () => ( + <> + <FormItem + name="first_name" + label={t('First name')} + rules={[{ required: true, message: t('First name is required') }]} + > + <Input + name="first_name" + placeholder={t("Enter the user's first name")} + /> + </FormItem> + <FormItem + name="last_name" + label={t('Last name')} + rules={[{ required: true, message: t('Last name is required') }]} + > + <Input name="last_name" placeholder={t("Enter the user's last name")} /> + </FormItem> + </> + ); + + const ResetPasswordFields = () => ( + <> + <FormItem + name="password" + label={t('Password')} + rules={[{ required: true, message: t('Password is required') }]} + > + <Input.Password + name="password" + placeholder="Enter the user's password" + /> + </FormItem> + <FormItem + name="confirm_password" + label={t('Confirm Password')} + dependencies={['password']} + rules={[ + { + required: true, + message: t('Please confirm your password'), + }, + ({ getFieldValue }) => ({ + validator(_, value) { + if (!value || getFieldValue('password') === value) { + return Promise.resolve(); + } + return Promise.reject(new Error(t('Passwords do not match!'))); + }, + }), + ]} + > Review Comment: ### Memoize Password Validator Function <sub></sub> <details> <summary>Tell me more</summary> ###### What is the issue? The password validation logic creates a new validator function on every render, which is unnecessary and impacts performance. ###### Why this matters Creating functions inside render causes new function instances on each render, leading to potential unnecessary re-renders of child components and increased memory allocation. ###### Suggested change ∙ *Feature Preview* Extract the validator function outside the component and memoize it with useCallback or define it as a standalone function: ```typescript const validatePasswordMatch = ({ getFieldValue }) => ({ validator(_, value) { if (!value || getFieldValue('password') === value) { return Promise.resolve(); } return Promise.reject(new Error(t('Passwords do not match!'))); }, }); // Then in the component: rules={[ { required: true, message: t('Please confirm your password'), }, validatePasswordMatch, ]} ``` ###### Provide feedback to improve future suggestions [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/ca698fc7-18f4-4389-b2d8-0f64ad8912e8/upvote) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/ca698fc7-18f4-4389-b2d8-0f64ad8912e8?what_not_true=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/ca698fc7-18f4-4389-b2d8-0f64ad8912e8?what_out_of_scope=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/ca698fc7-18f4-4389-b2d8-0f64ad8912e8?what_not_in_standard=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/ca698fc7-18f4-4389-b2d8-0f64ad8912e8) </details> <sub> 💬 Looking for more details? Reply to this comment to chat with Korbit. </sub> <!--- korbi internal id:31153b2b-dced-4187-824f-8d96bb465483 --> [](31153b2b-dced-4187-824f-8d96bb465483) ########## superset-frontend/src/features/userInfo/UserInfoModal.tsx: ########## @@ -0,0 +1,150 @@ +/** + * 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 { SupersetClient, t } from '@superset-ui/core'; +import { useToasts } from 'src/components/MessageToasts/withToasts'; +import FormModal from 'src/components/Modal/FormModal'; +import { FormItem } from 'src/components/Form'; +import { Input } from 'src/components/Input'; +import { User } from 'src/types/bootstrapTypes'; +import { BaseUserListModalProps, FormValues } from '../users/types'; + +export interface UserInfoModalProps extends BaseUserListModalProps { + isEditMode?: boolean; + user?: User; +} + +function UserInfoModal({ + show, + onHide, + onSave, + isEditMode, + user, +}: UserInfoModalProps) { + const { addDangerToast, addSuccessToast } = useToasts(); + + const requiredFields = isEditMode + ? ['first_name', 'last_name'] + : ['password', 'confirm_password']; + const initialValues = isEditMode + ? { + first_name: user?.firstName, + last_name: user?.lastName, + } + : {}; + const handleFormSubmit = async (values: FormValues) => { + try { + const { confirm_password, ...payload } = values; + await SupersetClient.put({ + endpoint: `/api/v1/me/`, + jsonPayload: { ...payload }, + }); + addSuccessToast( + isEditMode + ? t('The user was updated successfully') + : t('The password reset was successfull'), + ); + onSave(); + } catch (error) { + addDangerToast(t('Something went wrong while saving the user info')); + } + }; + + const EditModeFields = () => ( + <> + <FormItem + name="first_name" + label={t('First name')} + rules={[{ required: true, message: t('First name is required') }]} + > + <Input + name="first_name" + placeholder={t("Enter the user's first name")} + /> + </FormItem> + <FormItem + name="last_name" + label={t('Last name')} + rules={[{ required: true, message: t('Last name is required') }]} + > + <Input name="last_name" placeholder={t("Enter the user's last name")} /> + </FormItem> + </> + ); + + const ResetPasswordFields = () => ( + <> + <FormItem + name="password" + label={t('Password')} + rules={[{ required: true, message: t('Password is required') }]} + > + <Input.Password + name="password" + placeholder="Enter the user's password" + /> Review Comment: ### Missing autocomplete prevention on password field <sub></sub> <details> <summary>Tell me more</summary> ###### What is the issue? The password input field lacks explicit autocomplete prevention, which could allow browsers to save the password. ###### Why this matters Browsers might cache or save the password, potentially exposing it to other users of the same device or through browser vulnerabilities. ###### Suggested change ∙ *Feature Preview* Add autocomplete="new-password" to prevent password saving: ```typescript <Input.Password name="password" placeholder="Enter the user's password" autoComplete="new-password" /> ``` ###### Provide feedback to improve future suggestions [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/0c6471c3-7727-4d49-8496-b99ef4ea36ef/upvote) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/0c6471c3-7727-4d49-8496-b99ef4ea36ef?what_not_true=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/0c6471c3-7727-4d49-8496-b99ef4ea36ef?what_out_of_scope=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/0c6471c3-7727-4d49-8496-b99ef4ea36ef?what_not_in_standard=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/0c6471c3-7727-4d49-8496-b99ef4ea36ef) </details> <sub> 💬 Looking for more details? Reply to this comment to chat with Korbit. </sub> <!--- korbi internal id:bfa5b249-8f7f-4eb7-b23d-cb88db59434d --> [](bfa5b249-8f7f-4eb7-b23d-cb88db59434d) ########## superset-frontend/src/features/userInfo/UserInfoModal.tsx: ########## @@ -0,0 +1,150 @@ +/** + * 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 { SupersetClient, t } from '@superset-ui/core'; +import { useToasts } from 'src/components/MessageToasts/withToasts'; +import FormModal from 'src/components/Modal/FormModal'; +import { FormItem } from 'src/components/Form'; +import { Input } from 'src/components/Input'; +import { User } from 'src/types/bootstrapTypes'; +import { BaseUserListModalProps, FormValues } from '../users/types'; + +export interface UserInfoModalProps extends BaseUserListModalProps { + isEditMode?: boolean; + user?: User; +} + +function UserInfoModal({ + show, + onHide, + onSave, + isEditMode, + user, +}: UserInfoModalProps) { + const { addDangerToast, addSuccessToast } = useToasts(); + + const requiredFields = isEditMode + ? ['first_name', 'last_name'] + : ['password', 'confirm_password']; + const initialValues = isEditMode + ? { + first_name: user?.firstName, + last_name: user?.lastName, + } + : {}; + const handleFormSubmit = async (values: FormValues) => { + try { + const { confirm_password, ...payload } = values; + await SupersetClient.put({ + endpoint: `/api/v1/me/`, Review Comment: ### Incorrect API endpoint for user updates <sub></sub> <details> <summary>Tell me more</summary> ###### What is the issue? The API endpoint '/api/v1/me/' suggests this only updates the current user's information, but the component is designed to edit any user's information based on the 'user' prop. ###### Why this matters This mismatch could lead to incorrect user data updates or security issues where changes intended for one user affect another user's data. ###### Suggested change ∙ *Feature Preview* Use the appropriate endpoint for updating any user's information: ```typescript endpoint: `/api/v1/users/${user?.id}/` ``` ###### Provide feedback to improve future suggestions [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/ae55a899-d3de-48f8-a438-c240b6d1cfbd/upvote) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/ae55a899-d3de-48f8-a438-c240b6d1cfbd?what_not_true=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/ae55a899-d3de-48f8-a438-c240b6d1cfbd?what_out_of_scope=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/ae55a899-d3de-48f8-a438-c240b6d1cfbd?what_not_in_standard=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/ae55a899-d3de-48f8-a438-c240b6d1cfbd) </details> <sub> 💬 Looking for more details? Reply to this comment to chat with Korbit. </sub> <!--- korbi internal id:18e4c622-0e99-4bb2-b48c-628b00a0f46c --> [](18e4c622-0e99-4bb2-b48c-628b00a0f46c) -- 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]
