Compare commits

...

2 Commits

Author SHA1 Message Date
ericwyuan
e911a7c1f0 feat(garmin): 绑定表单也能强制重试了,之前只有同步页有
用户想现在就试绑定,账号却处在昨天真实撞上的 SSO 冷却期里。发现「强制
重试」按钮只接在同步流程上——绑定表单命中同一个 429 时,除了看提示、等
到冷却过期,没有别的路。

- error/blocked 这两个状态本来就是绑定表单和同步区共用的,但重试按钮硬编码
  只会调 syncHistory(true)。加一个 retryAction 记住是哪个流程触发的失败,
  按钮据此调对应的重试
- startLogin 加 force 参数透传给后端已有的 /garmin/login force 支持;密码
  只在请求真正成功后才清空,所以失败重试不需要用户重新输入
- 强制重试仍然只在真正拿到 429 之后才出现,不是默认可点的选项——窗口内
  重试会延长冷却,这个闸门就是为了防这个

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-09-13 07:31:43 +08:00
ericwyuan
db7f182030 fix(garmin): 退出账号会把邮箱和令牌一起永久删掉,SSO 账号无处补救
用户刚才绑定失败:`Request failed with status code 400`。查下来是
`delete_token()`("退出 Garmin 账号"背后的函数)的注释说"邮箱留在 user 表,
下次只需要密码",但代码只有 `DELETE FROM garmin_tokens`——而 `garmin_email`
唯一的落脚点就是这张表。`users.garmin_email` 是本地密码登录时代的遗留列,
`get_remembered_email` 里写得很清楚:auth-hub 接管之后,没有任何绑定路径再
往那张表写过东西。也就是说现在**所有账号**(auth-hub SSO)退出一次,等于
永久忘记邮箱——注释描述的"安全网"对这些账号从来没生效过。

- `delete_token` 删除前把 `garmin_tokens.garmin_email` 复制一份到
  `users.garmin_email`,注释里承诺的行为终于是真的
- 顺带修了一条原有测试掩盖真相的问题:`test_disconnecting_drops_the_
  current_binding_but_not_the_legacy_value` 预先在 users 表塞了一条 legacy
  邮箱,退出后自然能读到——从来没测过 SSO 账号真正会遇到的情况(legacy 列
  本来就是空的)。新增两条测试覆盖这个和"连续退出两次不会把邮箱也搞丢"
- 生产账号的邮箱已经从退出前的 dump 备份里手工恢复,不用重新绑定就能补上

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-09-13 07:20:52 +08:00
4 changed files with 66 additions and 10 deletions

View File

@@ -496,10 +496,22 @@ def delete_token(user_id):
"""Forget the stored Garmin OAuth token. """Forget the stored Garmin OAuth token.
The next sync or login will have to re-authenticate and mint a fresh token. The next sync or login will have to re-authenticate and mint a fresh token.
garmin_email on the user record is left in place so re-login only needs the garmin_email is meant to survive this — the UI's own "只需一次" promise is
password. The cached client — built from the old token's session — is dropped that a disconnect still leaves the address remembered — but for every
in the same step so a stale session can't keep being reused. auth-hub-era account (i.e. all of them now) that address lives *only* on
the row this deletes: `users.garmin_email` is a legacy column nothing
since auth-hub has ever written to (see `get_remembered_email`). Without
copying it forward first, "退出 Garmin 账号" quietly erases the one thing
it promised to keep, and the next bind attempt 400s on a blank email field
with no visible connection to the disconnect that caused it.
The cached client — built from the old token's session — is dropped in
the same step so a stale session can't keep being reused.
""" """
row = query_one("SELECT garmin_email FROM garmin_tokens WHERE user_id = ?", [user_id])
email = (row or {}).get("garmin_email")
if email:
execute("UPDATE users SET garmin_email = ? WHERE id = ?", [email, user_id])
execute("DELETE FROM garmin_tokens WHERE user_id = ?", [user_id]) execute("DELETE FROM garmin_tokens WHERE user_id = ?", [user_id])
forget_client(user_id) forget_client(user_id)

View File

@@ -338,16 +338,41 @@ class TestRememberedEmail:
) )
assert garmin_svc.get_remembered_email(user["id"]) == "legacy@example.com" assert garmin_svc.get_remembered_email(user["id"]) == "legacy@example.com"
def test_disconnecting_drops_the_current_binding_but_not_the_legacy_value( def test_disconnecting_remembers_the_most_recent_email_not_a_stale_legacy_one(
self, db, user self, db, user
): ):
"""The current binding's email is copied forward on disconnect (see
`delete_token`), so it wins over whatever older address happened to be
sitting in the legacy column — the account last used `current@…`, and
that is what the next bind attempt should be offered."""
db.execute( db.execute(
"UPDATE users SET garmin_email = ? WHERE id = ?", "UPDATE users SET garmin_email = ? WHERE id = ?",
["legacy@example.com", user["id"]], ["legacy@example.com", user["id"]],
) )
garmin_svc.save_token(user["id"], "tok", "current@example.com") garmin_svc.save_token(user["id"], "tok", "current@example.com")
garmin_svc.delete_token(user["id"]) garmin_svc.delete_token(user["id"])
assert garmin_svc.get_remembered_email(user["id"]) == "legacy@example.com" assert garmin_svc.get_remembered_email(user["id"]) == "current@example.com"
def test_disconnecting_an_auth_hub_account_still_remembers_the_email(
self, db, user
):
"""The case the test above does not cover, and the one that actually
broke: an auth-hub account has no pre-existing legacy row — the only
copy of the email is the one `delete_token` is about to remove. Without
copying it forward first, 退出 Garmin 账号 silently breaks its own
"只需一次" promise, and the next bind attempt 400s on a blank email
with no visible link back to the disconnect that caused it.
"""
assert garmin_svc.get_remembered_email(user["id"]) == ""
garmin_svc.save_token(user["id"], "tok", "current@example.com")
garmin_svc.delete_token(user["id"])
assert garmin_svc.get_remembered_email(user["id"]) == "current@example.com"
def test_disconnecting_twice_does_not_forget_the_email(self, db, user):
garmin_svc.save_token(user["id"], "tok", "current@example.com")
garmin_svc.delete_token(user["id"])
garmin_svc.delete_token(user["id"]) # no token row left to read from
assert garmin_svc.get_remembered_email(user["id"]) == "current@example.com"
class TestMfaHandling: class TestMfaHandling:

View File

@@ -54,6 +54,11 @@ function SyncPage() {
const [loading, setLoading] = useState(false); const [loading, setLoading] = useState(false);
const [error, setError] = useState(''); const [error, setError] = useState('');
/* Which action 强制重试 should retry — the two flows share one error banner
but call different endpoints, and retrying the wrong one either does
nothing (sync with no token bound yet) or silently drops the email/
password the user just typed. */
const [retryAction, setRetryAction] = useState<'sync' | 'login' | null>(null);
/* Whether the last attempt was refused by the recorded cooldown. Gates the /* Whether the last attempt was refused by the recorded cooldown. Gates the
强制重试 button: offering it unconditionally would invite the very thing 强制重试 button: offering it unconditionally would invite the very thing
the cooldown prevents. */ the cooldown prevents. */
@@ -99,10 +104,14 @@ function SyncPage() {
}, []); }, []);
// --- Garmin login ------------------------------------------------------- // --- Garmin login -------------------------------------------------------
const startLogin = async (e: React.FormEvent) => { /* `force` is only ever true when the user clicks 强制重试 after a refusal —
e.preventDefault(); never the default path, because retrying inside Garmin's real cooldown
window is what extends it (see services/garmin.py's sso_cooldown). */
const startLogin = async (e?: React.FormEvent, force?: boolean) => {
e?.preventDefault();
setError(''); setError('');
setMessage(''); setMessage('');
setBlocked(false);
if (!email) { if (!email) {
setError('请输入 Garmin 邮箱'); setError('请输入 Garmin 邮箱');
return; return;
@@ -114,7 +123,7 @@ function SyncPage() {
setLoading(true); setLoading(true);
try { try {
const sid = await apiClient.startGarminLogin(password, email); const sid = await apiClient.startGarminLogin(password, email, force);
// The password is only ever needed for this one request. // The password is only ever needed for this one request.
setPassword(''); setPassword('');
setSession(sid); setSession(sid);
@@ -123,6 +132,10 @@ function SyncPage() {
beginPolling(sid); beginPolling(sid);
} catch (err: any) { } catch (err: any) {
setError(errorMessage(err, '登录失败')); setError(errorMessage(err, '登录失败'));
if (err?.response?.status === 429 && err?.response?.data?.status === 'rate_limited') {
setBlocked(true);
setRetryAction('login');
}
} finally { } finally {
setLoading(false); setLoading(false);
} }
@@ -246,6 +259,7 @@ function SyncPage() {
setError(''); setError('');
setMessage(''); setMessage('');
setBlocked(false); setBlocked(false);
setRetryAction('sync');
setLoading(true); setLoading(true);
try { try {
// 0 means "everything" and -1 "since the last sync"; the backend caps // 0 means "everything" and -1 "since the last sync"; the backend caps
@@ -327,7 +341,9 @@ function SyncPage() {
{blocked && ( {blocked && (
<button <button
className="sync-force" className="sync-force"
onClick={() => syncHistory(true)} onClick={() => (
retryAction === 'login' ? startLogin(undefined, true) : syncHistory(true)
)}
disabled={loading} disabled={loading}
> >

View File

@@ -658,10 +658,13 @@ class ApiClient {
* Start an interactive Garmin login. Returns a session id; the login runs * Start an interactive Garmin login. Returns a session id; the login runs
* in the background and parks if Garmin asks for a two-factor code. * in the background and parks if Garmin asks for a two-factor code.
*/ */
async startGarminLogin(garminPassword: string, garminEmail?: string) { async startGarminLogin(
garminPassword: string, garminEmail?: string, force?: boolean
) {
const { data } = await this.client.post<{ session: string }>('/garmin/login', { const { data } = await this.client.post<{ session: string }>('/garmin/login', {
garminPassword, garminPassword,
...(garminEmail ? { garminEmail } : {}), ...(garminEmail ? { garminEmail } : {}),
...(force ? { force: true } : {}),
}); });
return data.session; return data.session;
} }