eleven / messenger Public

home: github anchors list and remove; account chooser on add/prove (#1168)

The anchor namespace over the oidc tables is oidcAnchorKinds (the
verifier providers + github) — the listing and removal spoke only
oidcProviderNames, so Paul's merged github anchor was on his account but
invisible and un-removable. Regression test pins the loop (list + the
last-method 409 on its removal route). Plus prompt=select_account on
Google/Microsoft for the add and combine-prove departures — the flows
where a DIFFERENT account is likely meant (GitHub has no such param;
silent reauth is their behaviour).
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Paul Campbell df22c85ea803ca133e5180555ce1f3a15ca855cc parent b7e3ed4
4 files changed, +75 −12
  • home/anchors.go +6 −5
  • home/github_test.go +52 −0
  • home/oidc.go +8 −0
  • home/web/static/home.js +9 −7
diff --git a/home/anchors.go b/home/anchors.go
index 23f072a..68852ec 100644
--- a/home/anchors.go
+++ b/home/anchors.go
@@ -56,10 +56,11 @@ func (s *Server) listAnchorsHandler(w http.ResponseWriter, r *http.Request) {
{"siwa", s.store.listSiwaAnchors},
{"email", s.store.listEmailAnchors},
}
- // The OIDC provider kinds come from the fixed table (oidcProviderNames),
- // not the enabled set: anchors of a currently-disabled door still list
- // (and remove below), exactly as siwa anchors do with SiwA off.
- for _, name := range oidcProviderNames {
+ // The OIDC-table kinds come from the fixed set (oidcAnchorKinds — the
+ // verifier providers plus github), not the enabled doors: anchors of a
+ // currently-disabled door still list (and remove below), exactly as
+ // siwa anchors do with SiwA off.
+ for _, name := range oidcAnchorKinds {
parts = append(parts, anchorSource{name, func(uid string) ([]Anchor, error) {
return s.store.listOidcAnchors(uid, name)
}})
@@ -98,7 +99,7 @@ func (s *Server) deleteAnchorHandler(w http.ResponseWriter, r *http.Request) {
deleted, last, err = s.store.deleteSiwaAnchor(uid, id)
case "email":
deleted, last, err = s.store.deleteEmailAnchor(uid, id)
- case "google", "microsoft": // the OIDC provider kinds (oidcProviderNames)
+ case "google", "microsoft", "github": // the OIDC-table kinds (oidcAnchorKinds)
deleted, last, err = s.store.deleteOidcAnchor(uid, kind, id)
default:
http.Error(w, "unknown sign-in method kind", http.StatusBadRequest)
diff --git a/home/github_test.go b/home/github_test.go
index 51957a0..2927279 100644
--- a/home/github_test.go
+++ b/home/github_test.go
@@ -128,3 +128,55 @@ func TestGithubReturnTrampoline(t *testing.T) {
t.Fatalf("native state location: %q", loc)
}
}
+
+// The regression Paul hit live: the github anchor stored fine (and merges
+// moved it) but the listing/removal surfaces spoke only oidcProviderNames,
+// so it was invisible and un-removable. Pin the whole loop: sign in, LIST
+// the anchor, and confirm the removal ROUTE recognizes the kind (the
+// last-method 409 — not the unknown-kind 400).
+func TestGithubAnchorListsAndRoutesRemoval(t *testing.T) {
+ ts, s := githubServer(t, 9001)
+ var signin struct {
+ Token string `json:"token"`
+ Proof string `json:"proof"`
+ }
+ resp := githubVerifyReq(t, ts, nil, map[string]any{"code": "good"})
+ if resp.StatusCode != http.StatusOK {
+ t.Fatalf("sign-in: got %d", resp.StatusCode)
+ }
+ if err := json.NewDecoder(resp.Body).Decode(&signin); err != nil {
+ t.Fatal(err)
+ }
+
+ resp = do(t, "GET", ts.URL+"/api/anchors", signin.Token, nil, nil)
+ if resp.StatusCode != http.StatusOK {
+ t.Fatalf("list: got %d", resp.StatusCode)
+ }
+ var listing struct {
+ Anchors []struct {
+ Kind string `json:"kind"`
+ ID string `json:"id"`
+ } `json:"anchors"`
+ }
+ if err := json.NewDecoder(resp.Body).Decode(&listing); err != nil {
+ t.Fatal(err)
+ }
+ ghID := ""
+ for _, a := range listing.Anchors {
+ if a.Kind == "github" {
+ ghID = a.ID
+ }
+ }
+ if ghID == "" {
+ t.Fatalf("github anchor missing from listing: %+v", listing.Anchors)
+ }
+ _ = s
+
+ // Its removal must ROUTE (the one method on the account → the
+ // last-method 409, never the unknown-kind 400).
+ resp = do(t, "DELETE", ts.URL+"/api/anchors/github/"+ghID, signin.Token,
+ map[string]string{"X-Ceremony-Proof": signin.Proof}, nil)
+ if resp.StatusCode != http.StatusConflict {
+ t.Fatalf("remove route: got %d, want 409 (last method)", resp.StatusCode)
+ }
+}
diff --git a/home/oidc.go b/home/oidc.go
index 74f197f..3ea3c91 100644
--- a/home/oidc.go
+++ b/home/oidc.go
@@ -73,6 +73,14 @@ const (
// own routes and its own siwa_anchors rows.
var oidcProviderNames = []string{"google", "microsoft"}
+// oidcAnchorKinds is the ANCHOR namespace over the oidc tables — the
+// verifier-table providers plus GitHub, whose door is the code-exchange
+// path (github.go) but whose anchors store identically. The listing and
+// removal surfaces speak this set, so a github anchor is visible and
+// removable like any other (its invisibility after Paul's first combine
+// was exactly this list missing github).
+var oidcAnchorKinds = append(append([]string{}, oidcProviderNames...), "github")
+
var oidcHTTP = &http.Client{Timeout: 10 * time.Second}
// oidcProvider is one table row: everything that differs between the doors.
diff --git a/home/web/static/home.js b/home/web/static/home.js
index f92212d..d97f805 100644
--- a/home/web/static/home.js
+++ b/home/web/static/home.js
@@ -639,26 +639,28 @@ const OIDC_WEB = {
verify: "/api/oidc/microsoft/verify",
// The "common" endpoint admits any tenant plus personal accounts —
// oidc.go's checkIssuer handles the per-tenant iss.
- authorize: (st, nonce) => "https://login.microsoftonline.com/common/oauth2/v2.0/authorize" +
+ authorize: (st, nonce, sel) => "https://login.microsoftonline.com/common/oauth2/v2.0/authorize" +
"?client_id=" + encodeURIComponent(MICROSOFT_CLIENT_ID) +
"&redirect_uri=" + encodeURIComponent(location.origin + "/oidc/return") +
"&response_type=id_token" +
"&response_mode=form_post" +
"&scope=openid" +
"&nonce=" + encodeURIComponent(nonce) +
- "&state=" + encodeURIComponent(st),
+ "&state=" + encodeURIComponent(st) +
+ (sel ? "&prompt=select_account" : ""),
},
google: {
display: "Google",
verify: "/api/oidc/google/verify",
- authorize: (st, nonce) => "https://accounts.google.com/o/oauth2/v2/auth" +
+ authorize: (st, nonce, sel) => "https://accounts.google.com/o/oauth2/v2/auth" +
"?client_id=" + encodeURIComponent(GOOGLE_CLIENT_ID) +
"&redirect_uri=" + encodeURIComponent(location.origin + "/oidc/return") +
"&response_type=id_token" +
"&response_mode=form_post" +
"&scope=openid" +
"&nonce=" + encodeURIComponent(nonce) +
- "&state=" + encodeURIComponent(st),
+ "&state=" + encodeURIComponent(st) +
+ (sel ? "&prompt=select_account" : ""),
},
};
@@ -686,7 +688,7 @@ function startOidcSignIn(provider, e, opts = {}) {
else sessionStorage.removeItem("home_methods_intent");
} catch { /* no storage — the return leg lands as a plain sign-in */ }
sessionStorage.setItem("home_oidc_state", JSON.stringify({ state: st, provider, nonce }));
- location.href = OIDC_WEB[provider].authorize(st, nonce);
+ location.href = OIDC_WEB[provider].authorize(st, nonce, !!opts.select);
}
// finishOidc — finishSiwa's twin against /api/oidc/{provider}/verify: the
@@ -1853,7 +1855,7 @@ function startAddOidc(provider, e) {
const src = e && e.currentTarget;
const go = (proof) => startOidcSignIn(provider,
{ currentTarget: src && src.isConnected ? src : document.createElement("button") },
- { linkProof: { proof, expires: ceremonyProof ? ceremonyProof.expires : Date.now() + PROOF_TTL_MS } });
+ { select: true, linkProof: { proof, expires: ceremonyProof ? ceremonyProof.expires : Date.now() + PROOF_TTL_MS } });
const p = freshProof();
if (p) { go(p); return; }
pendingIntent = { kind: "add-" + provider };
@@ -3736,7 +3738,7 @@ $("otherways-google").addEventListener("click", (e) => startOidcSignIn("google",
$("otherways-microsoft").addEventListener("click", (e) => startOidcSignIn("microsoft", e));
$("otherways-github").addEventListener("click", (e) => startOidcSignIn("github", e));
$("combine-oidc-go").addEventListener("click", (e) => {
- if (combine && OIDC_WEB[combine.kind]) startOidcSignIn(combine.kind, e, { intent: "merge-" + combine.kind });
+ if (combine && OIDC_WEB[combine.kind]) startOidcSignIn(combine.kind, e, { intent: "merge-" + combine.kind, select: true });
});
$("otherways-email").addEventListener("click", openEmailDoor);
$("otherways-passkey").addEventListener("click", () => signIn().catch(onSignInError));