Skip to content

Commit 966f8ca

Browse files
fix(auth): reject initial passwords in self-service password changes
1 parent e028a23 commit 966f8ca

17 files changed

Lines changed: 340 additions & 16 deletions

File tree

‎backend/apps/system/api/login.py‎

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -11,20 +11,14 @@
1111
from common.core.config import settings
1212
from common.core.schemas import Token
1313
from sqlbot_xpack.authentication.manage import logout as xpack_logout
14-
from sqlbot_xpack.config.arg_manage import get_group_args
14+
from apps.system.crud.password_policy import initial_pwd_disabled
1515

1616
from common.audit.models.log_model import OperationType, OperationModules
1717
from common.audit.schemas.logger_decorator import system_log, LogConfig
1818

1919
router = APIRouter(tags=["login"], prefix="/login")
2020

2121

22-
async def initial_pwd_disabled(session) -> bool:
23-
login_args = await get_group_args(session=session, flag='login')
24-
disabled_arg = next((a for a in login_args if a.pkey == 'login.initial_pwd_disabled'), None)
25-
return bool(disabled_arg and str(disabled_arg.pval).strip().lower() == 'true')
26-
27-
2822
@router.post("/access-token")
2923
@system_log(LogConfig(
3024
operation_type=OperationType.LOGIN,

‎backend/apps/system/api/user.py‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11
from collections import defaultdict
22
from typing import Optional
3-
from fastapi import APIRouter, File, Path, Query, UploadFile
3+
from fastapi import APIRouter, File, HTTPException, Path, Query, UploadFile
44
from sqlmodel import SQLModel, case, or_, select, delete as sqlmodel_delete
55
from apps.system.crud.user import check_account_exists, check_email_exists, check_email_format, check_pwd_format, get_db_user, single_delete, user_ws_options
6+
from apps.system.crud.password_policy import initial_pwd_disabled
67
from apps.system.crud.user_excel import batchUpload, downTemplate, download_error_file
78
from apps.system.models.system_model import UserWsModel, WorkspaceModel
89
from apps.system.models.user import UserModel
@@ -398,6 +399,8 @@ async def pwdUpdate(session: SessionDep, current_user: CurrentUser, trans: Trans
398399
db_user: UserModel = get_db_user(session=session, user_id=current_user.id)
399400
if not verify_md5pwd(editor.pwd, db_user.password):
400401
raise Exception(trans('i18n_error', key = trans('i18n_user.password')))
402+
if await initial_pwd_disabled(session) and new_pwd == settings.DEFAULT_PWD:
403+
raise HTTPException(status_code=400, detail=trans('i18n_login.new_pwd_is_initial'))
401404
db_user.password = md5pwd(new_pwd)
402405
session.add(db_user)
403406
return db_user
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
"""Shared initial-password policy for login and self-service password changes."""
2+
3+
from sqlbot_xpack.config.arg_manage import get_group_args
4+
5+
6+
async def initial_pwd_disabled(session) -> bool:
7+
login_args = await get_group_args(session=session, flag="login")
8+
disabled_arg = next(
9+
(a for a in login_args if a.pkey == "login.initial_pwd_disabled"), None
10+
)
11+
return bool(disabled_arg and str(disabled_arg.pval).strip().lower() == "true")
Lines changed: 169 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,169 @@
1+
"""Offline behavior tests of password handlers with external boundaries replaced.
2+
3+
Extract actual functions to avoid importing DB/xpack startup. These tests do not
4+
claim coverage of authentication middleware, decorators or real transactions.
5+
"""
6+
7+
import ast
8+
import asyncio
9+
import hashlib
10+
import json
11+
from pathlib import Path
12+
from types import SimpleNamespace
13+
from unittest.mock import AsyncMock, Mock
14+
15+
import pytest
16+
from fastapi import HTTPException
17+
18+
ROOT = Path(__file__).resolve().parents[2]
19+
20+
21+
def load_function(path, name, namespace):
22+
tree = ast.parse((ROOT / path).read_text())
23+
node = next(
24+
n
25+
for n in tree.body
26+
if isinstance(n, (ast.FunctionDef, ast.AsyncFunctionDef)) and n.name == name
27+
)
28+
node.decorator_list = []
29+
node.returns = None
30+
for arg in node.args.args:
31+
arg.annotation = None
32+
exec(
33+
compile(
34+
ast.fix_missing_locations(ast.Module(body=[node], type_ignores=[])),
35+
str(path),
36+
"exec",
37+
),
38+
namespace,
39+
)
40+
return namespace[name]
41+
42+
43+
def md5(value):
44+
return hashlib.md5(value.encode()).hexdigest()
45+
46+
47+
@pytest.fixture
48+
def context():
49+
user = SimpleNamespace(id=2, password=md5("OldPassword42!"))
50+
session = Mock()
51+
args = AsyncMock(
52+
return_value=[SimpleNamespace(pkey="login.initial_pwd_disabled", pval="true")]
53+
)
54+
ns = {
55+
"get_group_args": args,
56+
"HTTPException": HTTPException,
57+
"get_db_user": Mock(return_value=user),
58+
"md5pwd": md5,
59+
"verify_md5pwd": lambda value, hashed: md5(value) == hashed,
60+
"default_pwd": lambda: "InitialPassword42!",
61+
"settings": SimpleNamespace(DEFAULT_PWD="InitialPassword42!"),
62+
}
63+
import re
64+
65+
ns["re"] = re
66+
schema = ast.parse(
67+
(ROOT / "backend/apps/system/schemas/system_schema.py").read_text()
68+
)
69+
regex = next(
70+
n
71+
for n in schema.body
72+
if isinstance(n, ast.Assign)
73+
and any(isinstance(t, ast.Name) and t.id == "PWD_REGEX" for t in n.targets)
74+
)
75+
exec(
76+
compile(ast.Module(body=[regex], type_ignores=[]), "<password-regex>", "exec"),
77+
ns,
78+
)
79+
load_function("backend/apps/system/crud/user.py", "check_pwd_format", ns)
80+
policy_path = "backend/apps/system/crud/password_policy.py"
81+
if not (ROOT / policy_path).exists():
82+
policy_path = "backend/apps/system/api/login.py"
83+
load_function(policy_path, "initial_pwd_disabled", ns)
84+
handler = load_function("backend/apps/system/api/user.py", "pwdUpdate", ns)
85+
return user, session, args, ns, handler
86+
87+
88+
def update(context, new="InitialPassword42!", old="OldPassword42!", uid=2):
89+
_, session, _, _, handler = context
90+
return asyncio.run(
91+
handler(
92+
session,
93+
SimpleNamespace(id=uid),
94+
lambda message_key, **kwargs: message_key,
95+
SimpleNamespace(pwd=old, new_pwd=new),
96+
)
97+
)
98+
99+
100+
@pytest.mark.parametrize("uid", [1, 2])
101+
def test_disabled_rejects_initial_password_without_writing(context, uid):
102+
user, session, _, ns, _ = context
103+
before = user.password
104+
with pytest.raises(HTTPException) as exc:
105+
update(context, uid=uid)
106+
assert exc.value.status_code == 400
107+
assert exc.value.detail == "i18n_login.new_pwd_is_initial"
108+
assert user.password == before
109+
session.add.assert_not_called()
110+
ns["get_db_user"].assert_called_once_with(session=session, user_id=uid)
111+
112+
113+
@pytest.mark.parametrize("value", ["false", False, None])
114+
def test_disabled_off_allows_initial_password(context, value):
115+
user, session, args, _, _ = context
116+
args.return_value = [SimpleNamespace(pkey="login.initial_pwd_disabled", pval=value)]
117+
update(context)
118+
assert user.password == md5("InitialPassword42!")
119+
session.add.assert_called_once_with(user)
120+
121+
122+
def test_enabled_allows_other_password(context):
123+
user, session, _, _, _ = context
124+
update(context, new="DifferentPassword42!")
125+
assert user.password == md5("DifferentPassword42!")
126+
session.add.assert_called_once_with(user)
127+
128+
129+
@pytest.mark.parametrize(
130+
"old,new", [("wrong", "InitialPassword42!"), ("OldPassword42!", "bad")]
131+
)
132+
def test_invalid_credentials_or_format_never_write(context, old, new):
133+
user, session, _, _, _ = context
134+
before = user.password
135+
with pytest.raises(Exception) as exc:
136+
update(context, old=old, new=new)
137+
assert str(exc.value) == ("i18n_error" if old == "wrong" else "i18n_format_invalid")
138+
assert user.password == before
139+
session.add.assert_not_called()
140+
141+
142+
@pytest.mark.parametrize(
143+
"value,expected",
144+
[
145+
("true", True),
146+
(" TRUE ", True),
147+
(True, True),
148+
("false", False),
149+
(False, False),
150+
(None, False),
151+
],
152+
)
153+
def test_policy_config_values(context, value, expected):
154+
_, session, args, ns, _ = context
155+
args.return_value = [SimpleNamespace(pkey="login.initial_pwd_disabled", pval=value)]
156+
assert asyncio.run(ns["initial_pwd_disabled"](session)) is expected
157+
args.assert_awaited_once_with(session=session, flag="login")
158+
159+
160+
def test_missing_policy_defaults_off(context):
161+
_, session, args, ns, _ = context
162+
args.return_value = []
163+
assert asyncio.run(ns["initial_pwd_disabled"](session)) is False
164+
165+
166+
def test_error_translations_present():
167+
for locale in ["en", "zh-CN", "zh-TW", "ko-KR", "th-TH"]:
168+
messages = json.loads((ROOT / "backend/locales" / f"{locale}.json").read_text())
169+
assert messages["i18n_login"]["new_pwd_is_initial"]

‎frontend/src/components/layout/PwdForm.vue‎

Lines changed: 33 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,19 @@
11
<script lang="ts" setup>
2-
import { ref, reactive } from 'vue'
2+
import { ref, reactive, watch } from 'vue'
3+
import { isAxiosError } from 'axios'
34
import { useI18n } from 'vue-i18n'
45
import { userApi } from '@/api/auth'
56
const { t } = useI18n()
67
const pwdRef = ref()
8+
const submitError = ref('')
79
const pwdForm = reactive({
810
pwd: '',
911
new_pwd: '',
1012
confirm_pwd: '',
1113
})
14+
watch(pwdForm, () => {
15+
submitError.value = ''
16+
})
1217
const PWD_REGEX =
1318
/^(?=.*[a-z])(?=.*[A-Z])(?=.*\d)(?=.*[~!@#$%^&*()_+\-={}|:"<>?`\[\];',./])[A-Za-z\d~!@#$%^&*()_+\-={}|:"<>?`\[\];',./]{8,20}$/
1419
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
@@ -23,7 +28,8 @@ const validatePass = (rule: any, value: any, callback: any) => {
2328
}
2429
if (pwdForm.confirm_pwd !== '') {
2530
if (!pwdRef.value) return
26-
pwdRef.value.validateField('confirm_pwd')
31+
// Field validation already renders mismatch errors in the form.
32+
pwdRef.value.validateField('confirm_pwd').catch(() => undefined)
2733
}
2834
callback()
2935
}
@@ -62,13 +68,24 @@ const submit = () => {
6268
pwd: pwdForm.pwd,
6369
new_pwd: pwdForm.new_pwd,
6470
}
65-
userApi.pwd(param).then(() => {
66-
ElMessage({
67-
type: 'success',
68-
message: t('common.save_success'),
71+
submitError.value = ''
72+
userApi
73+
.pwd(param)
74+
.then(() => {
75+
ElMessage({
76+
type: 'success',
77+
message: t('common.save_success'),
78+
})
79+
emits('pwdSaved')
80+
})
81+
.catch((error: unknown) => {
82+
// The shared HTTP client keeps its toast and authentication handling.
83+
const message = isAxiosError(error) ? error.response?.data : undefined
84+
submitError.value =
85+
typeof message === 'string' && message.trim()
86+
? message
87+
: t('user.upgrade_pwd.save_error')
6988
})
70-
emits('pwdSaved')
71-
})
7289
}
7390
})
7491
}
@@ -114,6 +131,14 @@ defineExpose({
114131
clearable
115132
/>
116133
</el-form-item>
134+
<el-alert
135+
v-if="submitError"
136+
class="pwd-submit-error"
137+
:title="submitError"
138+
type="error"
139+
:closable="false"
140+
show-icon
141+
/>
117142
</el-form>
118143
</div>
119144
</template>

‎frontend/src/i18n/en.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -612,6 +612,7 @@
612612
"old_password": "Old password",
613613
"title": "User management",
614614
"upgrade_pwd": {
615+
"save_error": "Unable to change the password. Please try again later.",
615616
"title": "Change password",
616617
"old_pwd": "Old password",
617618
"new_pwd": "New password",

‎frontend/src/i18n/ko-KR.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -612,6 +612,7 @@
612612
"old_password": "기존 비밀번호",
613613
"title": "사용자 관리",
614614
"upgrade_pwd": {
615+
"save_error": "비밀번호를 변경하지 못했습니다. 잠시 후 다시 시도해 주세요.",
615616
"title": "비밀번호 변경",
616617
"old_pwd": "기존 비밀번호",
617618
"new_pwd": "새 비밀번호",

‎frontend/src/i18n/th-TH.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -612,6 +612,7 @@
612612
"old_password": "รหัสผ่านเดิม",
613613
"title": "การจัดการผู้ใช้",
614614
"upgrade_pwd": {
615+
"save_error": "เปลี่ยนรหัสผ่านไม่สำเร็จ โปรดลองอีกครั้งในภายหลัง",
615616
"title": "เปลี่ยนรหัสผ่าน",
616617
"old_pwd": "รหัสผ่านเดิม",
617618
"new_pwd": "รหัสผ่านใหม่",

‎frontend/src/i18n/zh-CN.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -612,6 +612,7 @@
612612
"old_password": "旧密码",
613613
"title": "用户管理",
614614
"upgrade_pwd": {
615+
"save_error": "密码修改失败,请稍后重试",
615616
"title": "修改密码",
616617
"old_pwd": "旧密码",
617618
"new_pwd": "新密码",

‎frontend/src/i18n/zh-TW.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -612,6 +612,7 @@
612612
"old_password": "舊密碼",
613613
"title": "使用者管理",
614614
"upgrade_pwd": {
615+
"save_error": "密碼修改失敗,請稍後重試",
615616
"title": "修改密碼",
616617
"old_pwd": "舊密碼",
617618
"new_pwd": "新密碼",

0 commit comments

Comments
 (0)