Răsfoiți Sursa

fix(settings): require re-2FA confirmation for sensitive setting changes (#5610)

* fix(settings): require server-side 2fa for sensitive changes

* fix(lint): group third-party imports separately from local (goimports)

golangci-lint goimports flagged setting.go and setting_security_test.go because xlzd/gotp and gorm.io/gorm were mixed into the github.com/mhsanaei/3x-ui local-prefix group. Move them into the third-party group so the local imports stand alone.
n0ctal 14 ore în urmă
părinte
comite
2b10808fbd

+ 9 - 4
frontend/src/api/queries/useAllSettings.ts

@@ -7,6 +7,8 @@ import { AllSetting } from '@/models/setting';
 import { AllSettingSchema, type AllSettingInput } from '@/schemas/setting';
 import { keys } from '@/api/queryKeys';
 
+type SettingSavePayload = Partial<AllSetting> & Record<string, unknown>;
+
 async function fetchAllSetting(): Promise<AllSettingInput | null> {
   const msg = await HttpUtil.post('/panel/api/setting/all', undefined, { silent: true });
   if (!msg?.success) throw new Error(msg?.msg || 'Failed to fetch settings');
@@ -42,19 +44,21 @@ export function useAllSettings() {
   }, []);
 
   const saveMut = useMutation({
-    mutationFn: async (next: AllSetting): Promise<Msg<unknown>> => {
-      const body = AllSettingSchema.partial().safeParse(next);
+    mutationFn: async (next: SettingSavePayload): Promise<Msg<unknown>> => {
+      const payload = { ...next };
+      const body = AllSettingSchema.partial().safeParse(payload);
       if (!body.success) {
         console.warn('[zod] setting/update body failed validation', body.error.issues);
       }
-      return HttpUtil.post('/panel/api/setting/update', body.success ? body.data : next);
+      return HttpUtil.post('/panel/api/setting/update', body.success ? { ...payload, ...body.data } : payload);
     },
     onSuccess: (msg) => {
       if (msg?.success) queryClient.invalidateQueries({ queryKey: keys.settings.all() });
     },
   });
 
-  const saveAll = useCallback(() => saveMut.mutateAsync(draft), [saveMut, draft]);
+  const saveAll = useCallback(() => saveMut.mutateAsync({ ...draft }), [saveMut, draft]);
+  const savePayload = useCallback((payload: SettingSavePayload) => saveMut.mutateAsync(payload), [saveMut]);
   const saveDisabled = useMemo(() => server.equals(draft), [server, draft]);
 
   return {
@@ -65,5 +69,6 @@ export function useAllSettings() {
     setSpinning: setExtraSpinning,
     saveDisabled,
     saveAll,
+    savePayload,
   };
 }

+ 21 - 9
frontend/src/pages/settings/SecurityTab.tsx

@@ -37,6 +37,7 @@ interface ApiTokenRow {
 interface SecurityTabProps {
   allSetting: AllSetting;
   updateSetting: (patch: Partial<AllSetting>) => void;
+  saveSetting: (payload: Partial<AllSetting> & Record<string, unknown>) => Promise<unknown>;
 }
 
 const UNIX_MILLISECONDS_THRESHOLD = 100_000_000_000;
@@ -65,7 +66,7 @@ const TFA_INITIAL: TfaState = {
   onConfirm: () => {},
 };
 
-export default function SecurityTab({ allSetting, updateSetting }: SecurityTabProps) {
+export default function SecurityTab({ allSetting, updateSetting, saveSetting }: SecurityTabProps) {
   const { t } = useTranslation();
   const { isMobile } = useMediaQuery();
   const [modal, modalContextHolder] = Modal.useModal();
@@ -99,10 +100,10 @@ export default function SecurityTab({ allSetting, updateSetting }: SecurityTabPr
     setUser((prev) => ({ ...prev, [key]: value }));
   }
 
-  const sendUpdateUser = useCallback(async () => {
+  const sendUpdateUser = useCallback(async (twoFactorCode = '') => {
     setUpdating(true);
     try {
-      const msg = await HttpUtil.post('/panel/api/setting/updateUser', user) as ApiMsg;
+      const msg = await HttpUtil.post('/panel/api/setting/updateUser', { ...user, twoFactorCode }) as ApiMsg;
       if (msg?.success) {
         await HttpUtil.post('/logout');
         const basePath = window.X_UI_BASE_PATH || '/';
@@ -118,9 +119,11 @@ export default function SecurityTab({ allSetting, updateSetting }: SecurityTabPr
       openTfa({
         title: t('pages.settings.security.twoFactorModalChangeCredentialsTitle'),
         description: t('pages.settings.security.twoFactorModalChangeCredentialsStep'),
-        token: allSetting.twoFactorToken,
+        token: '',
         type: 'confirm',
-        onConfirm: (ok: boolean) => { if (ok) sendUpdateUser(); },
+        onConfirm: (ok: boolean, code?: string) => {
+          if (ok) sendUpdateUser(code || '');
+        },
       });
     } else {
       sendUpdateUser();
@@ -224,12 +227,21 @@ export default function SecurityTab({ allSetting, updateSetting }: SecurityTabPr
       openTfa({
         title: t('pages.settings.security.twoFactorModalDeleteTitle'),
         description: t('pages.settings.security.twoFactorModalRemoveStep'),
-        token: allSetting.twoFactorToken,
+        token: '',
         type: 'confirm',
-        onConfirm: (ok: boolean) => {
+        onConfirm: async (ok: boolean, code?: string) => {
           if (!ok) return;
-          messageApi.success(t('pages.settings.security.twoFactorModalDeleteSuccess'));
-          updateSetting({ twoFactorEnable: false, twoFactorToken: '' });
+          const next = {
+            ...allSetting,
+            twoFactorEnable: false,
+            twoFactorToken: '',
+            twoFactorCode: code || '',
+          };
+          const msg = await saveSetting(next) as ApiMsg;
+          if (msg?.success) {
+            messageApi.success(t('pages.settings.security.twoFactorModalDeleteSuccess'));
+            updateSetting({ twoFactorEnable: false, twoFactorToken: '', hasTwoFactorToken: false });
+          }
         },
       });
     }

+ 2 - 1
frontend/src/pages/settings/SettingsPage.tsx

@@ -76,6 +76,7 @@ export default function SettingsPage() {
     setSpinning,
     saveDisabled,
     saveAll,
+    savePayload,
   } = useAllSettings();
 
   const [entryHost, setEntryHost] = useState('');
@@ -196,7 +197,7 @@ export default function SettingsPage() {
 
   const categoryBody = useMemo(() => {
     switch (activeSlug) {
-      case 'security': return <SecurityTab allSetting={allSetting} updateSetting={updateSetting} />;
+      case 'security': return <SecurityTab allSetting={allSetting} updateSetting={updateSetting} saveSetting={savePayload} />;
       case 'telegram': return <TelegramTab allSetting={allSetting} updateSetting={updateSetting} />;
       case 'email': return <EmailTab allSetting={allSetting} updateSetting={updateSetting} />;
       case 'subscription': return <SubscriptionGeneralTab allSetting={allSetting} updateSetting={updateSetting} />;

+ 1 - 1
frontend/src/test/api-token-date.test.tsx

@@ -26,7 +26,7 @@ describe('API token creation date', () => {
       ],
     });
 
-    render(<SecurityTab allSetting={{} as AllSetting} updateSetting={vi.fn()} />);
+    render(<SecurityTab allSetting={{} as AllSetting} updateSetting={vi.fn()} saveSetting={vi.fn()} />);
     fireEvent.click(screen.getByRole('tab', { name: /API Token/ }));
 
     expect(await screen.findByText('seconds-token')).toBeTruthy();

+ 23 - 6
internal/web/controller/setting.go

@@ -19,10 +19,16 @@ import (
 
 // updateUserForm represents the form for updating user credentials.
 type updateUserForm struct {
-	OldUsername string `json:"oldUsername" form:"oldUsername"`
-	OldPassword string `json:"oldPassword" form:"oldPassword"`
-	NewUsername string `json:"newUsername" form:"newUsername"`
-	NewPassword string `json:"newPassword" form:"newPassword"`
+	OldUsername   string `json:"oldUsername" form:"oldUsername"`
+	OldPassword   string `json:"oldPassword" form:"oldPassword"`
+	NewUsername   string `json:"newUsername" form:"newUsername"`
+	NewPassword   string `json:"newPassword" form:"newPassword"`
+	TwoFactorCode string `json:"twoFactorCode" form:"twoFactorCode"`
+}
+
+type updateSettingForm struct {
+	entity.AllSetting
+	TwoFactorCode string `json:"twoFactorCode" form:"twoFactorCode"`
 }
 
 // SettingController handles settings and user management operations.
@@ -82,23 +88,30 @@ func (a *SettingController) getDefaultSettings(c *gin.Context) {
 
 // updateSetting updates all settings with the provided data.
 func (a *SettingController) updateSetting(c *gin.Context) {
-	allSetting, ok := middleware.BindAndValidate[entity.AllSetting](c)
+	form, ok := middleware.BindAndValidate[updateSettingForm](c)
 	if !ok {
 		return
 	}
+	allSetting := &form.AllSetting
 	oldTwoFactor, twoFactorErr := a.settingService.GetTwoFactorEnable()
 	oldPanelOutbound, _ := a.settingService.GetPanelOutbound()
 	oldTgEnable, _ := a.settingService.GetTgbotEnabled()
 	oldTgToken, _ := a.settingService.GetTgBotToken()
 	oldTgChatId, _ := a.settingService.GetTgBotChatId()
 	oldTgAPIServer, _ := a.settingService.GetTgBotAPIServer()
+	if twoFactorErr == nil && oldTwoFactor && !allSetting.TwoFactorEnable {
+		if err := a.settingService.VerifyTwoFactorCode(form.TwoFactorCode); err != nil {
+			jsonMsg(c, I18nWeb(c, "pages.settings.toasts.modifySettings"), err)
+			return
+		}
+	}
 	err := a.settingService.UpdateAllSetting(allSetting)
 	if err == nil && twoFactorErr == nil && !oldTwoFactor && allSetting.TwoFactorEnable {
 		if bumpErr := a.userService.BumpLoginEpoch(); bumpErr != nil {
 			err = bumpErr
 		}
 	}
-	if err == nil && allSetting.PanelOutbound != oldPanelOutbound {
+	if err == nil && form.PanelOutbound != oldPanelOutbound {
 		// The egress bridge lives in the generated config; reconcile the
 		// running core. One SOCKS inbound plus one routing rule — both
 		// hot-appliable, so this normally does not restart Xray.
@@ -136,6 +149,10 @@ func (a *SettingController) updateUser(c *gin.Context) {
 		jsonMsg(c, I18nWeb(c, "pages.settings.toasts.modifyUserError"), errors.New(I18nWeb(c, "pages.settings.toasts.userPassMustBeNotEmpty")))
 		return
 	}
+	if err := a.settingService.VerifyTwoFactorCode(form.TwoFactorCode); err != nil {
+		jsonMsg(c, I18nWeb(c, "pages.settings.toasts.modifyUserError"), err)
+		return
+	}
 	err = a.userService.UpdateUser(user.Id, form.NewUsername, form.NewPassword)
 	if err == nil {
 		user.Username = form.NewUsername

+ 20 - 2
internal/web/service/setting.go

@@ -14,6 +14,8 @@ import (
 	"time"
 
 	"github.com/google/uuid"
+	"github.com/xlzd/gotp"
+	"gorm.io/gorm"
 
 	"github.com/mhsanaei/3x-ui/v3/internal/config"
 	"github.com/mhsanaei/3x-ui/v3/internal/database"
@@ -25,8 +27,6 @@ import (
 	"github.com/mhsanaei/3x-ui/v3/internal/util/reflect_util"
 	"github.com/mhsanaei/3x-ui/v3/internal/web/entity"
 	"github.com/mhsanaei/3x-ui/v3/internal/xray"
-
-	"gorm.io/gorm"
 )
 
 //go:embed config.json
@@ -568,6 +568,24 @@ func (s *SettingService) SetTwoFactorToken(value string) error {
 	return s.setString("twoFactorToken", value)
 }
 
+func (s *SettingService) VerifyTwoFactorCode(code string) error {
+	enabled, err := s.GetTwoFactorEnable()
+	if err != nil {
+		return err
+	}
+	if !enabled {
+		return nil
+	}
+	token, err := s.GetTwoFactorToken()
+	if err != nil {
+		return err
+	}
+	if strings.TrimSpace(token) == "" || !gotp.NewDefaultTOTP(token).Verify(strings.TrimSpace(code), time.Now().Unix()) {
+		return common.NewError("invalid two factor code")
+	}
+	return nil
+}
+
 func (s *SettingService) GetPort() (int, error) {
 	return s.getInt("webPort")
 }

+ 21 - 0
internal/web/service/setting_security_test.go

@@ -4,6 +4,8 @@ import (
 	"path/filepath"
 	"testing"
 
+	"github.com/xlzd/gotp"
+
 	"github.com/mhsanaei/3x-ui/v3/internal/database"
 	"github.com/mhsanaei/3x-ui/v3/internal/database/model"
 )
@@ -100,3 +102,22 @@ func TestSanitizePublicHTTPURLBlocksPrivateAddressUnlessAllowed(t *testing.T) {
 		t.Fatalf("allowPrivate result = %q, %v", got, err)
 	}
 }
+
+func TestVerifyTwoFactorCode(t *testing.T) {
+	setupSettingTestDB(t)
+	s := &SettingService{}
+	if err := s.saveSetting("twoFactorEnable", "true"); err != nil {
+		t.Fatal(err)
+	}
+	const token = "JBSWY3DPEHPK3PXP"
+	if err := s.saveSetting("twoFactorToken", token); err != nil {
+		t.Fatal(err)
+	}
+
+	if err := s.VerifyTwoFactorCode(gotp.NewDefaultTOTP(token).Now()); err != nil {
+		t.Fatalf("valid code rejected: %v", err)
+	}
+	if err := s.VerifyTwoFactorCode("000000"); err == nil {
+		t.Fatal("invalid code accepted")
+	}
+}