diff --git a/accounts_cli.go b/accounts_cli.go index ef29c8c5..ae848c52 100644 --- a/accounts_cli.go +++ b/accounts_cli.go @@ -329,7 +329,9 @@ func addAccount(agentID string) error { } switch st.State { case "done": - if st.Using { + if st.Again { + fmt.Println(green.Render("✓"), st.User, "is already listed — its sign-in was renewed") + } else if st.Using { fmt.Println(green.Render("✓"), agentID, "is signed in as", st.User) } else { fmt.Println(green.Render("✓"), "added", st.User, muted.Render("· use it: magpie accounts switch "+agentID+" "+st.User)) diff --git a/internal/gui/assets/app.js b/internal/gui/assets/app.js index 82dfb7dc..e1320f3b 100644 --- a/internal/gui/assets/app.js +++ b/internal/gui/assets/app.js @@ -5511,6 +5511,8 @@ async function signedIn(st) { const who = st.user || subOf(st.agent)?.name || st.agent; // signed in but listed nowhere (#155): say so rather than "added" if (!p) status(t("{user} signed in, but magpie can't list it — please report this", { user: who }), "err"); + // an account listed already is said to be, not added (#413) + else if (st.again) status(t("{user} is already listed — its sign-in was renewed", { user: who }), "ok"); else status(st.using ? t("Signed in as {user}", { user: who }) : t("{user} added — switch to it any time", { user: who }), "ok"); state = await api("state"); renderAgents(); diff --git a/internal/gui/assets/i18n.js b/internal/gui/assets/i18n.js index b36f7e7e..29163e0d 100644 --- a/internal/gui/assets/i18n.js +++ b/internal/gui/assets/i18n.js @@ -497,6 +497,7 @@ const I18N = { "Try again": "重试", "Signed in as {user}": "已登录 {user}", "{user} added — switch to it any time": "已添加 {user}，随时可以切换", + "{user} is already listed — its sign-in was renewed": "{user}：该账号已在列表中，已更新登录", "Import instead…": "改为导入…", "Bring in accounts exported from Antigravity Cockpit, Antigravity Manager or CLIProxyAPI": "导入从 Antigravity Cockpit、Antigravity Manager 或 CLIProxyAPI 导出的账号", "Import accounts from a file…": "从文件导入账号…", diff --git a/internal/gui/tests/README.md b/internal/gui/tests/README.md index 0970c1b8..9e2ab14a 100644 --- a/internal/gui/tests/README.md +++ b/internal/gui/tests/README.md @@ -781,10 +781,17 @@ has room" (额度恢复后切回首选); with nothing to go back to, the account is on is just "First". The Routing note says magpie moves back. The page doesn't move; English and Chinese, Chromium and WebKit. +`signin-again.test.cjs` checks a sign-in to an account magpie lists +already (#413: WorkBuddy's page offers the account WorkBuddy is signed in +to). Adding another WorkBuddy account comes back done with `again`: the +status says it is already listed and its sign-in renewed (该账号已在列表中， +已更新登录), not "added"; a new account still says added. The page doesn't +move; English and Chinese, Chromium and WebKit. + With Node.js and Playwright available: ```sh -node --test internal/gui/tests/menu-scroll.test.cjs internal/gui/tests/click-scroll.test.cjs internal/gui/tests/panel-fold.test.cjs internal/gui/tests/rollup-short.test.cjs internal/gui/tests/panel-routing.test.cjs internal/gui/tests/gateway-fold.test.cjs internal/gui/tests/routing-kind.test.cjs internal/gui/tests/balance-fix.test.cjs internal/gui/tests/cli-update.test.cjs internal/gui/tests/login-import.test.cjs internal/gui/tests/agy-launch.test.cjs internal/gui/tests/currency.test.cjs internal/gui/tests/usage-ledger.test.cjs internal/gui/tests/update-check.test.cjs internal/gui/tests/omarchy.test.cjs internal/gui/tests/brand.test.cjs internal/gui/tests/library-loading.test.cjs internal/gui/tests/provider-typed.test.cjs node --test internal/gui/tests/signin-callback.test.cjs internal/gui/tests/routing-steady.test.cjs internal/gui/tests/old-webkit.test.cjs internal/gui/tests/session-terminal.test.cjs internal/gui/tests/text-size.test.cjs internal/gui/tests/zcode-site.test.cjs internal/gui/tests/rail-tip.test.cjs internal/gui/tests/sync-update.test.cjs internal/gui/tests/plugin-signin.test.cjs internal/gui/tests/plugin-market.test.cjs internal/gui/tests/s3-sync.test.cjs internal/gui/tests/routing-wb-refused.test.cjs internal/gui/tests/routing-blocked.test.cjs internal/gui/tests/routing-shape.test.cjs internal/gui/tests/market-have.test.cjs internal/gui/tests/sessions-today.test.cjs internal/gui/tests/routing-side-calls.test.cjs internal/gui/tests/import-all-skills.test.cjs internal/gui/tests/routing-flood.test.cjs internal/gui/tests/routing-busy.test.cjs internal/gui/tests/own-skill.test.cjs internal/gui/tests/routing-manual.test.cjs internal/gui/tests/tray-usages.test.cjs internal/gui/tests/provider-remove.test.cjs internal/gui/tests/signin-link.test.cjs internal/gui/tests/tray-inuse.test.cjs internal/gui/tests/codex-auto-reset.test.cjs internal/gui/tests/routing-auto-reset.test.cjs internal/gui/tests/claude-usage-asked.test.cjs internal/gui/tests/claude-risk.test.cjs internal/gui/tests/library-toggle-status.test.cjs internal/gui/tests/library-all.test.cjs internal/gui/tests/routing-reroute-title.test.cjs internal/gui/tests/plain-names.test.cjs internal/gui/tests/panel-effort.test.cjs internal/gui/tests/agent-models-all-hidden.test.cjs internal/gui/tests/omp-roles.test.cjs internal/gui/tests/claude-effort.test.cjs internal/gui/tests/remote-magpie.test.cjs internal/gui/tests/qoder-cn-tile.test.cjs internal/gui/tests/provider-levels-scroll.test.cjs internal/gui/tests/signin-paste-codex.test.cjs internal/gui/tests/usage-alert.test.cjs internal/gui/tests/quota-families.test.cjs internal/gui/tests/provider-search.test.cjs internal/gui/tests/webdav-settings-refresh.test.cjs internal/gui/tests/lan-docker.test.cjs internal/gui/tests/plugin-key-hint.test.cjs internal/gui/tests/update-hide.test.cjs internal/gui/tests/tray-logos.test.cjs internal/gui/tests/routing-proxy.test.cjs internal/gui/tests/sse-body.test.cjs internal/gui/tests/remote-group-served.test.cjs internal/gui/tests/plugin-move-overflow.test.cjs internal/gui/tests/routing-time.test.cjs internal/gui/tests/unlisted-models.test.cjs internal/gui/tests/plugin-updates.test.cjs internal/gui/tests/request-archive.test.cjs internal/gui/tests/group-member-fast.test.cjs internal/gui/tests/providers-off-fold.test.cjs internal/gui/tests/account-return.test.cjs internal/gui/tests/usage-ledger-detail.test.cjs internal/gui/tests/usage-ledger-chart.test.cjs internal/gui/tests/panel-usage.test.cjs internal/gui/tests/usage-refresh.test.cjs internal/gui/tests/usage-ledger-content.test.cjs internal/gui/tests/claude-direct.test.cjs +node --test internal/gui/tests/menu-scroll.test.cjs internal/gui/tests/click-scroll.test.cjs internal/gui/tests/panel-fold.test.cjs internal/gui/tests/rollup-short.test.cjs internal/gui/tests/panel-routing.test.cjs internal/gui/tests/gateway-fold.test.cjs internal/gui/tests/routing-kind.test.cjs internal/gui/tests/balance-fix.test.cjs internal/gui/tests/cli-update.test.cjs internal/gui/tests/login-import.test.cjs internal/gui/tests/agy-launch.test.cjs internal/gui/tests/currency.test.cjs internal/gui/tests/usage-ledger.test.cjs internal/gui/tests/update-check.test.cjs internal/gui/tests/omarchy.test.cjs internal/gui/tests/brand.test.cjs internal/gui/tests/library-loading.test.cjs internal/gui/tests/provider-typed.test.cjs node --test internal/gui/tests/signin-callback.test.cjs internal/gui/tests/routing-steady.test.cjs internal/gui/tests/old-webkit.test.cjs internal/gui/tests/session-terminal.test.cjs internal/gui/tests/text-size.test.cjs internal/gui/tests/zcode-site.test.cjs internal/gui/tests/rail-tip.test.cjs internal/gui/tests/sync-update.test.cjs internal/gui/tests/plugin-signin.test.cjs internal/gui/tests/plugin-market.test.cjs internal/gui/tests/s3-sync.test.cjs internal/gui/tests/routing-wb-refused.test.cjs internal/gui/tests/routing-blocked.test.cjs internal/gui/tests/routing-shape.test.cjs internal/gui/tests/market-have.test.cjs internal/gui/tests/sessions-today.test.cjs internal/gui/tests/routing-side-calls.test.cjs internal/gui/tests/import-all-skills.test.cjs internal/gui/tests/routing-flood.test.cjs internal/gui/tests/routing-busy.test.cjs internal/gui/tests/own-skill.test.cjs internal/gui/tests/routing-manual.test.cjs internal/gui/tests/tray-usages.test.cjs internal/gui/tests/provider-remove.test.cjs internal/gui/tests/signin-link.test.cjs internal/gui/tests/tray-inuse.test.cjs internal/gui/tests/codex-auto-reset.test.cjs internal/gui/tests/routing-auto-reset.test.cjs internal/gui/tests/claude-usage-asked.test.cjs internal/gui/tests/claude-risk.test.cjs internal/gui/tests/library-toggle-status.test.cjs internal/gui/tests/library-all.test.cjs internal/gui/tests/routing-reroute-title.test.cjs internal/gui/tests/plain-names.test.cjs internal/gui/tests/panel-effort.test.cjs internal/gui/tests/agent-models-all-hidden.test.cjs internal/gui/tests/omp-roles.test.cjs internal/gui/tests/claude-effort.test.cjs internal/gui/tests/remote-magpie.test.cjs internal/gui/tests/qoder-cn-tile.test.cjs internal/gui/tests/provider-levels-scroll.test.cjs internal/gui/tests/signin-paste-codex.test.cjs internal/gui/tests/usage-alert.test.cjs internal/gui/tests/quota-families.test.cjs internal/gui/tests/provider-search.test.cjs internal/gui/tests/webdav-settings-refresh.test.cjs internal/gui/tests/lan-docker.test.cjs internal/gui/tests/plugin-key-hint.test.cjs internal/gui/tests/update-hide.test.cjs internal/gui/tests/tray-logos.test.cjs internal/gui/tests/routing-proxy.test.cjs internal/gui/tests/sse-body.test.cjs internal/gui/tests/remote-group-served.test.cjs internal/gui/tests/plugin-move-overflow.test.cjs internal/gui/tests/routing-time.test.cjs internal/gui/tests/unlisted-models.test.cjs internal/gui/tests/plugin-updates.test.cjs internal/gui/tests/request-archive.test.cjs internal/gui/tests/group-member-fast.test.cjs internal/gui/tests/providers-off-fold.test.cjs internal/gui/tests/account-return.test.cjs internal/gui/tests/usage-ledger-detail.test.cjs internal/gui/tests/usage-ledger-chart.test.cjs internal/gui/tests/panel-usage.test.cjs internal/gui/tests/usage-refresh.test.cjs internal/gui/tests/usage-ledger-content.test.cjs internal/gui/tests/claude-direct.test.cjs internal/gui/tests/signin-again.test.cjs ``` If Playwright is installed outside the repository, set `NODE_PATH` to the diff --git a/internal/gui/tests/signin-again.test.cjs b/internal/gui/tests/signin-again.test.cjs new file mode 100644 index 00000000..1bb133cf --- /dev/null +++ b/internal/gui/tests/signin-again.test.cjs @@ -0,0 +1,86 @@ +// Run with Node's test runner and Playwright on the module path; see README.md. +// A sign-in to an account magpie lists already is said to be one (#413: +// WorkBuddy's page offers the account WorkBuddy is signed in to, and adding +// it read as if a new one came in, or as nothing at all). Add another +// WorkBuddy account, the sign-in comes back done with again: the status +// says the account is already listed and its sign-in renewed, not "added"; +// a new one still says added. No click moves the page. English and +// Chinese, Chromium and WebKit; no backend, the API is faked here. +const assert = require("node:assert/strict"); +const fs = require("node:fs/promises"); +const path = require("node:path"); +const { test } = require("node:test"); +const { chromium, webkit } = require("playwright"); + +const assets = path.resolve(__dirname, "../assets"); +const account = { + id: "workbuddy", name: "WorkBuddy (CodeBuddy)", icon: "workbuddy-color", chat: "", responses: "", anthropic: "", catalog: "", + models: [{ id: "glm-5", name: "GLM-5", on: true }], agents: [], fallback: [], headers: {}, keyList: [], + account: { agent: "workbuddy", agentName: "WorkBuddy", user: "Alice", logins: [{ user: "Alice", active: true, on: true, own: true }, { user: "Bob", on: true }] }, +}; + +function serve(lang, done) { + const state = { agents: [], profiles: [], settings: { lang, theme: "light" } }; + return async (route) => { + const url = new URL(route.request().url()); + const json = (data) => route.fulfill({ json: data }); + if (url.pathname === "/boot.js") return route.fulfill({ contentType: "text/javascript", body: `window.bootPrefs = {lang:"${lang}",theme:"light",web:true};` }); + if (url.pathname === "/wails/runtime.js") return route.fulfill({ contentType: "text/javascript", body: "export const Window = {};" }); + if (url.pathname === "/api/state") return json(state); + if (url.pathname === "/api/providers") return json({ providers: [account], presets: [], excluded: [], gateway: { running: true, window: true } }); + if (url.pathname === "/api/plugins") return json({ plugins: [] }); + if (url.pathname === "/api/groups") return json({ groups: [], pools: [] }); + if (url.pathname === "/api/gateway/trace") return json({ routes: [] }); + if (url.pathname === "/api/usage/quotas") return json([]); + if (url.pathname === "/api/signin") return json({ id: "wb1", agent: "workbuddy", state: "waiting", url: "https://www.codebuddy.cn/login?x=1" }); + if (url.pathname === "/api/signin/wb1") return json({ id: "wb1", agent: "workbuddy", state: "done", ...done.shift() }); + if (url.pathname.startsWith("/api/")) return json({}); + const file = path.join(assets, url.pathname === "/" ? "index.html" : url.pathname); + const contentType = { ".html": "text/html", ".js": "text/javascript", ".css": "text/css", ".svg": "image/svg+xml", ".png": "image/png" }[path.extname(file)]; + try { await route.fulfill({ body: await fs.readFile(file), contentType }); } catch { await route.fulfill({ status: 404, body: "" }); } + }; +} + +const want = { + en: { again: "Bob is already listed — its sign-in was renewed", added: "Carol added — switch to it any time" }, + zh: { again: "Bob：该账号已在列表中，已更新登录", added: "已添加 Carol，随时可以切换" }, +}; + +const cases = [ + { name: "an account listed already is said to be", done: { user: "Bob", again: true }, say: "again" }, + { name: "a new account is said to be added", done: { user: "Carol" }, say: "added" }, +]; + +for (const engine of (process.env.BROWSER ? [process.env.BROWSER] : ["chromium", "webkit"])) { + for (const lang of ["en", "zh"]) { + for (const c of cases) { + test(`${engine} ${lang}: ${c.name}`, async (t) => { + const browser = await (engine === "webkit" ? webkit.launch() : chromium.launch({ channel: "chromium" })); + const page = await (await browser.newContext({ viewport: { width: 900, height: 760 }, reducedMotion: "reduce" })).newPage(); + t.after(async () => { + if (process.env.ARTIFACT_DIR) { + await fs.mkdir(process.env.ARTIFACT_DIR, { recursive: true }); + await page.screenshot({ path: path.join(process.env.ARTIFACT_DIR, `${engine}-${lang}-${c.say}-signin-again.png`) }); + } + await browser.close(); + }); + page.setDefaultTimeout(5000); + const errors = []; + page.on("pageerror", (e) => errors.push(e.message)); + await page.route("**/*", serve(lang, [c.done])); + await page.goto("http://magpie.test/?view=providers"); + await page.locator(".row.provider", { hasText: "WorkBuddy" }).click(); + const add = page.locator(".editor .accts .acc.add").first(); + await add.waitFor(); + const scrolls = () => page.evaluate(() => [window.scrollY, ...[...document.querySelectorAll("*")].filter((e) => e.scrollTop).map((e) => e.scrollTop)]); + const before = await scrolls(); + await add.click(); + const say = want[lang][c.say]; + await page.waitForFunction((s) => document.querySelector("#status").textContent.includes(s), say); + assert.equal((await page.locator("#status").textContent()).trim(), say); + assert.deepEqual(await scrolls(), before, "nothing scrolled"); + assert.deepEqual(errors, []); + }); + } + } +} diff --git a/internal/plugin/host.js b/internal/plugin/host.js index 1d2ccf28..135fcaf5 100644 --- a/internal/plugin/host.js +++ b/internal/plugin/host.js @@ -311,9 +311,16 @@ function secretOf(a) { return a?.type === "oauth" ? a.refresh ?? a.access ?? "" : a?.key ?? "" } +// uidOf is the vendor's id a plugin keeps beside an account's name +// (WorkBuddy's uid): two accounts of one name are told apart by it (#413). +function uidOf(a) { + return typeof a?.uid === "string" ? a.uid : "" +} + // settle keeps a sign-in just saved at key once: one to an account already -// signed in (the same account id, else the same secret) replaces that one's -// and goes. It gives where it is kept. +// signed in (the same account id and, where both have one, the same uid, +// else the same secret) replaces that one's and goes. It gives where it is +// kept. function settle(provider, key) { const all = readAuth() const now = all[key] @@ -323,7 +330,8 @@ function settle(provider, key) { for (const k of accountsOf(all, provider)) { if (k === key) continue const was = all[k] - if ((who && whoOf(was) === who) || (!who && !whoOf(was) && secret && secretOf(was) === secret)) { + const other = uidOf(now) && uidOf(was) && uidOf(now) !== uidOf(was) + if ((who && whoOf(was) === who && !other) || (!who && !whoOf(was) && secret && secretOf(was) === secret)) { all[k] = now delete all[key] writeAuth(all) diff --git a/internal/plugin/plugin_uid_test.go b/internal/plugin/plugin_uid_test.go new file mode 100644 index 00000000..2954d00d --- /dev/null +++ b/internal/plugin/plugin_uid_test.go @@ -0,0 +1,55 @@ +package plugin + +import ( + "context" + "encoding/json" + "os" + "path/filepath" + "testing" + "time" +) + +// Two accounts of one name that the plugin tells apart by a uid (WorkBuddy's +// nickname and uid) are both kept: the second took the first one's place +// (#413). The same uid again replaces its own sign-in. +func TestPluginSameNameOtherUID(t *testing.T) { + sandbox(t) + ctx, cancel := context.WithTimeout(context.Background(), time.Minute) + defer cancel() + abs, _ := filepath.Abs("testdata/fake/index.js") + if _, err := Add(ctx, abs); err != nil { + t.Fatal(err) + } + signIn := func(team string) Saved { + t.Helper() + a, err := Authorize(ctx, "fakeco", 1, map[string]string{"where": "work", "team": team}, NewAccount) + if err != nil { + t.Fatal(err) + } + s, err := Finish(ctx, a.Session, "good") + if err != nil { + t.Fatal(err) + } + return s + } + saved := func() map[string]map[string]any { + t.Helper() + var m map[string]map[string]any + b, _ := os.ReadFile(AuthPath()) + json.Unmarshal(b, &m) + return m + } + first, second := signIn("pat/u1"), signIn("pat/u2") + if first.Account == second.Account { + t.Fatalf("the second took the first's place: %+v %+v", first, second) + } + if m := saved(); len(m) != 2 || m[first.Account]["uid"] != "u1" || m[second.Account]["uid"] != "u2" { + t.Fatalf("saved %v", m) + } + if again := signIn("pat/u1"); again != first { + t.Fatalf("u1 again as %+v, not %+v", again, first) + } + if m := saved(); len(m) != 2 { + t.Fatalf("after u1 again %v", m) + } +} diff --git a/internal/plugin/testdata/fake/index.js b/internal/plugin/testdata/fake/index.js index d635048c..c9a587dc 100644 --- a/internal/plugin/testdata/fake/index.js +++ b/internal/plugin/testdata/fake/index.js @@ -3,6 +3,13 @@ // built-in's plugin does. const ID = process.env.FAKE_ID || "fakeco" +// fakeWho is who a team signs in as: team@fake, and a team named name/uid +// is name@fake with that uid, as WorkBuddy's plugin keeps one +function fakeWho(team) { + const [name, uid] = (team ?? "me").split("/") + return uid ? { accountId: name + "@fake", uid } : { accountId: name + "@fake" } +} + export const FakePlugin = async ({ client }) => ({ config: async (cfg) => { cfg.provider = cfg.provider ?? {} @@ -36,7 +43,7 @@ export const FakePlugin = async ({ client }) => ({ method: "code", callback: async (code) => code === "good" - ? { type: "success", refresh: "r-" + (inputs.team ?? "none"), access: "stale", expires: 0, accountId: (inputs.team ?? "me") + "@fake" } + ? { type: "success", refresh: "r-" + (inputs.team ?? "none"), access: "stale", expires: 0, ...fakeWho(inputs.team) } : code === "expired" ? { type: "failed", error: "the sign-in page expired" } : { type: "failed" }, diff --git a/internal/provider/signin.go b/internal/provider/signin.go index 8953ae6e..038213a2 100644 --- a/internal/provider/signin.go +++ b/internal/provider/signin.go @@ -58,7 +58,10 @@ type SignInState struct { User string `json:"user,omitempty"` // the account, once done Plan string `json:"plan,omitempty"` // Using bool `json:"using,omitempty"` // the agent was signed in to it too - Error string `json:"error,omitempty"` + // Again is an account listed already: its sign-in was renewed rather + // than a new account added (#413) + Again bool `json:"again,omitempty"` + Error string `json:"error,omitempty"` } type signInFlow struct { diff --git a/internal/provider/workbuddy.go b/internal/provider/workbuddy.go index 63d15d8f..f4e2c140 100644 --- a/internal/provider/workbuddy.go +++ b/internal/provider/workbuddy.go @@ -39,6 +39,7 @@ import ( "path/filepath" "regexp" "runtime" + "slices" "strconv" "strings" "sync" @@ -812,24 +813,85 @@ func startWorkBuddySignIn(s *signInFlow, w *wbSite) error { return } c.UID = acc.UID - who := wbWho(acc.Nickname, acc.PhoneNumber, acc.UID) - auth, _ := json.Marshal(c) - ownUser, _, hasOwn := wbOwn(w) - if !hasOwn { - ownUser = "" - } - if err := addSideLogin(savedLogin{Agent: w.id, User: who, Auth: auth}, ownUser, func(savedLogin) {}); err != nil { + who, again, using, err := wbKeepSignIn(w, c, wbWho(acc.Nickname, acc.PhoneNumber, acc.UID)) + if err != nil { fail(err.Error()) return } wbTokens.Lock() wbTokens.m[w.id+"|"+c.UID] = c wbTokens.Unlock() - s.finish(SignInState{State: "done", User: who, Using: hasOwn && strings.EqualFold(ownUser, who)}) + s.finish(SignInState{State: "done", User: who, Using: using, Again: again}) }() return nil } +// wbKeepSignIn keeps an account magpie just signed in, told from the +// others by its WorkBuddy uid, never by its name: two accounts can share a +// nickname, and the second took the first one's place (#413). The same +// account again renews its sign-in where it is listed (again). The one +// WorkBuddy itself is signed in to becomes magpie's own sign-in of it, so +// it stays listed once WorkBuddy signs in to another: WorkBuddy's page +// offers the account the app is signed in to, and accounts added one by +// one that way each pushed the last off the list (#413). using says +// WorkBuddy is signed in to it too. +func wbKeepSignIn(w *wbSite, c wbCreds, name string) (who string, again, using bool, err error) { + ownUser, own, hasOwn := wbOwn(w) + auth, err := json.Marshal(c) + if err != nil { + return "", false, false, err + } + using = hasOwn && own.UID == c.UID + loginsMu.Lock() + defer loginsMu.Unlock() + ls := readLogins() + // uidOf is a saved account's uid, "" where it isn't known: WorkBuddy's + // own while WorkBuddy can't be read, a sign-in that can't be read + uidOf := func(l savedLogin) string { + if l.own() { + if hasOwn && strings.EqualFold(l.User, ownUser) { + return own.UID + } + return "" + } + sc, _ := wbSavedCreds(l) + return sc.UID + } + same := slices.IndexFunc(ls, func(l savedLogin) bool { return l.Agent == w.id && uidOf(l) == c.UID }) + if same < 0 { + // one of the same name whose uid isn't known is taken to be it, as + // before (#155); one whose uid is another's is another account + same = slices.IndexFunc(ls, func(l savedLogin) bool { + return l.Agent == w.id && strings.EqualFold(l.User, name) && uidOf(l) == "" + }) + } + now := time.Now().UTC().Truncate(time.Second) + if same >= 0 { + l := &ls[same] + again = l.Hidden == "" + if l.own() { + // WorkBuddy's own is first unless another was put first: it + // stays first as magpie's + if !slices.ContainsFunc(ls, func(m savedLogin) bool { return m.Agent == w.id && m.First }) { + l.First = true + } + l.On = true + } + l.Auth, l.Seen, l.Lapsed, l.Hidden = auth, now, "", "" + return l.User, again, using, writeLogins(ls) + } + // another account of the same name is told apart by the end of its uid + who = name + " (" + c.UID + ")" + tail := c.UID[max(0, len(c.UID)-4):] + for _, n := range []string{name, name + " (" + tail + ")"} { + if !slices.ContainsFunc(ls, func(l savedLogin) bool { return l.Agent == w.id && strings.EqualFold(l.User, n) }) { + who = n + break + } + } + return who, false, using, writeLogins(append(ls, savedLogin{Agent: w.id, User: who, Auth: auth, Seen: now, On: true})) +} + // wbPoll asks path every second until it answers with data, giving up at // the deadline. A retry code (the answer isn't ready) waits and asks again; // any other error ends the sign-in through fail. diff --git a/internal/provider/workbuddy_accounts_test.go b/internal/provider/workbuddy_accounts_test.go new file mode 100644 index 00000000..44bbd25c --- /dev/null +++ b/internal/provider/workbuddy_accounts_test.go @@ -0,0 +1,173 @@ +package provider + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "sync" + "testing" + "time" +) + +// wbFakeWho is the account wbFakeAccounts' WorkBuddy signs in to when it +// is asked, its tokens named after the uid. +type wbFakeWho struct { + mu sync.Mutex + uid, nick string +} + +func (f *wbFakeWho) set(uid, nick string) { + f.mu.Lock() + f.uid, f.nick = uid, nick + f.mu.Unlock() +} + +func wbFakeAccounts(t *testing.T) *wbFakeWho { + t.Helper() + who := &wbFakeWho{} + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + ok := func(data any) { json.NewEncoder(w).Encode(map[string]any{"code": 0, "data": data}) } + who.mu.Lock() + uid, nick := who.uid, who.nick + who.mu.Unlock() + switch r.URL.Path { + case "/v2/plugin/auth/state": + ok(map[string]any{"state": "s1", "authUrl": "https://login.codebuddy.cn/oauth?client_id=c"}) + case "/v2/plugin/auth/token": + ok(map[string]any{"accessToken": "access-" + uid, "refreshToken": "refresh-" + uid, + "expiresIn": 3600, "refreshExpiresIn": 7200, "domain": "www.codebuddy.cn", "tokenType": "Bearer"}) + case "/v2/plugin/login/account": + ok(map[string]any{"uid": uid, "nickname": nick}) + default: + w.WriteHeader(404) + } + })) + t.Cleanup(srv.Close) + oldEnd, oldInt := wbEndpoint, wbPollInterval + wbEndpoint, wbPollInterval = srv.URL, 5*time.Millisecond + t.Cleanup(func() { wbEndpoint, wbPollInterval = oldEnd, oldInt }) + wbTokens.Lock() + wbTokens.m = map[string]wbCreds{} + wbTokens.Unlock() + return who +} + +// wbDesktopAs signs WorkBuddy itself in to uid, nick. +func wbDesktopAs(t *testing.T, home, uid, nick string) { + t.Helper() + writeFile(t, wbAuthFile(home), map[string]any{ + "account": map[string]any{"uid": uid, "nickname": nick}, + "auth": map[string]any{"accessToken": "desktop-" + uid, "refreshToken": "r", + "expiresAt": time.Now().Add(24 * time.Hour).UnixMilli(), "domain": "www.codebuddy.cn"}, + }) +} + +func wbUsers(ls []Login) []string { + var out []string + for _, l := range ls { + out = append(out, l.User) + } + return out +} + +func wbAccessOf(t *testing.T, user string) string { + t.Helper() + for _, a := range wbLogins(wbCN) { + if a.User == user { + return a.creds.Access + } + } + t.Fatalf("no account %q", user) + return "" +} + +// Another account of the same nickname as WorkBuddy's own is added beside +// it: it was taken for WorkBuddy's own and let go, the sign-in said +// "signed in" and no account was added (#413). +func TestWorkBuddyAddSameNicknameAsOwn(t *testing.T) { + home := signIn(t) + fake := wbFakeAccounts(t) + wbDesktopAs(t, home, "u1", "Me") + Logins("workbuddy") + fake.set("u2", "Me") + st := wbSignInNow(t) + if st.State != "done" || st.Using || st.Again || st.User != "Me (u2)" { + t.Fatalf("sign-in: %+v", st) + } + ls := Logins("workbuddy") + if len(ls) != 2 || ls[0].User != "Me" || !ls[0].Active || ls[1].User != "Me (u2)" { + t.Fatalf("logins: %v", wbUsers(ls)) + } + if a := wbAccessOf(t, "Me"); a != "desktop-u1" { + t.Fatalf("WorkBuddy's own signs with %q", a) + } + if a := wbAccessOf(t, "Me (u2)"); a != "access-u2" { + t.Fatalf("the added one signs with %q", a) + } +} + +// Two accounts magpie signed in that share a nickname are both kept: the +// second replaced the first (#413). +func TestWorkBuddyAddSameNicknameTwice(t *testing.T) { + signIn(t) + fake := wbFakeAccounts(t) + fake.set("u2", "Me") + if st := wbSignInNow(t); st.State != "done" || st.User != "Me" || st.Again { + t.Fatalf("first: %+v", st) + } + fake.set("u3abcdef", "Me") + if st := wbSignInNow(t); st.State != "done" || st.User != "Me (cdef)" || st.Again { + t.Fatalf("second: %+v", st) + } + ls := Logins("workbuddy") + if len(ls) != 2 || wbAccessOf(t, "Me") != "access-u2" || wbAccessOf(t, "Me (cdef)") != "access-u3abcdef" { + t.Fatalf("logins: %v", wbUsers(ls)) + } + + // the first again: renewed where it is, and said so + fake.set("u2", "Me") + if st := wbSignInNow(t); st.State != "done" || st.User != "Me" || !st.Again { + t.Fatalf("again: %+v", st) + } + if ls := Logins("workbuddy"); len(ls) != 2 { + t.Fatalf("logins after again: %v", wbUsers(ls)) + } +} + +// Accounts added one by one through WorkBuddy — WorkBuddy signed in to +// each, then picked on its page from magpie — each pushed the last off: +// the one WorkBuddy is signed in to was only read from WorkBuddy, so it +// went once WorkBuddy signed in to the next (#413). Signed in from magpie, +// it is magpie's and stays. +func TestWorkBuddyAddWhatWorkBuddyIsSignedInTo(t *testing.T) { + home := signIn(t) + fake := wbFakeAccounts(t) + wbDesktopAs(t, home, "u1", "Alice") + Logins("workbuddy") + fake.set("u1", "Alice") + st := wbSignInNow(t) + if st.State != "done" || st.User != "Alice" || !st.Using || !st.Again { + t.Fatalf("sign-in: %+v", st) + } + if ls := Logins("workbuddy"); len(ls) != 1 || ls[0].User != "Alice" || !ls[0].Active { + t.Fatalf("logins: %v", wbUsers(ls)) + } + + wbDesktopAs(t, home, "u2", "Bob") + ls := Logins("workbuddy") + if len(ls) != 2 || ls[0].User != "Alice" || !ls[0].Active || ls[1].User != "Bob" { + t.Fatalf("after WorkBuddy signed in to Bob: %v", wbUsers(ls)) + } + if a := wbAccessOf(t, "Alice"); a != "access-u1" { + t.Fatalf("Alice signs with %q", a) + } + + fake.set("u2", "Bob") + if st := wbSignInNow(t); st.State != "done" || st.User != "Bob" || !st.Using || !st.Again { + t.Fatalf("Bob: %+v", st) + } + wbDesktopAs(t, home, "u3", "Carol") + if ls := Logins("workbuddy"); len(ls) != 3 || ls[0].User != "Alice" || ls[1].User != "Bob" || ls[2].User != "Carol" { + t.Fatalf("after WorkBuddy signed in to Carol: %v", wbUsers(ls)) + } +}