One Login, Two Roles — and the Seven Bugs In Between

Merging two account types into one identity: why it was needed, why admin was deliberately left out, and what shipping it actually surfaced.

I'm the sole engineer on Coach Yu, a two-person company. Every decision below — including the mistakes — is mine.

The problem I was trying to solve

Coach Yu has always modelled athletes and coaches as two separate species of account: separate Mongo collections (users and coaches), separate passkeys, separate JWTs, separate everything. That was fine right up until someone who is both — a coach who also trains as an athlete on the platform — tried to log in.

Passkeys are tied to a device's keychain, scoped to the site (the "relying party"), not to a role. When that dual-role person hit the login page, their browser happily offered up two saved passkeys for coach-yu.com — one registered as an athlete, one as a coach — with no way to tell them apart. Worse, the two accounts didn't know about each other at all. Same person, same domain, two disconnected identities that happened to share an email.

The fix isn't a UI trick. It's a data model change: one identity should be able to hold more than one role.

Why not just "add a role field and call it done"?

Because admin exists, and admin is not like the others.

Admin login requires two independently-registered passkeys, checked one after another — a real two-factor boundary, not cosmetic. The moment you let an identity carry multiple roles, you have to ask: does that boundary survive? If an admin's account could also be an athlete account, does resetting one somehow touch the other? Does a shared credential array let an athlete's ordinary passkey accidentally satisfy half of admin's two-factor check?

I considered a couple of alternatives before ruling on this. A join table linking separate users/coaches/admins documents by a shared identity ID would have avoided touching either schema, but it just relocates the same sharp edges — which document's credentials are authoritative for login? — into a third place instead of resolving them. A shared credential-verification service in front of all three account types was the other option: cleaner in principle, but a real subsystem to design and operate for a problem that, for two of the three roles, doesn't need solving at all.

The answer was that admin doesn't survive a merge safely, not without redesigning its whole security model — which nobody asked for and isn't worth the risk. So admin was pulled out of scope early and stays fully separate: its own collection, its own claims type, its own middleware, untouched. The unification is coach + athlete only.

That boundary is now written down explicitly in the code — cmd/migrate-roles's own top comment says not to reintroduce it — precisely so nobody "helpfully" merges admin back in six months from now without re-deriving all of this reasoning.

What I built

A one-time migration tool (cmd/migrate-roles) that merges a coaches document into a users document by matching email, adding "coach" to a new Roles array and attaching a CoachProfile sub-document for anything coach-specific. It's deliberately paranoid: dry-run by default, a hard-coded kill switch that has to be manually flipped for a real write, an automatic backup before anything is touched, and a -verify mode that diffs the result against the source data afterward instead of trusting its own success counters.

Then a dual-read/dual-write layer (coach_lookup.go) so every coach-facing handler — register, login, roster management, invites, passkey reset — works correctly whether a given coach has been migrated yet or is still sitting in the old collection. This is the expand–contract pattern: both shapes are supported simultaneously, nothing breaks mid-migration, and the old shape only gets retired once every reader and writer has been moved off it.

Part one: the bugs I went looking for

Building this in stages and then testing it against real dev data — rather than trusting the code — surfaced two bugs that no amount of code review would have caught.

Bug 1: a passkey that couldn't log in

The first version of the merge appended a coach's passkey into the same shared credential list the athlete side uses. That seemed reasonable: share one identity, share its credentials.

It's wrong. A passkey's internal "user handle" is stamped in at registration and never changes, and the login library checks that handle against the account's one WebAuthn ID. Once a coach's passkey and an athlete's passkey sit in the same list under one ID, at most one of them can ever match. The other 401s, forever, with an error that means nothing to whoever's stuck looking at it: "userHandle and User ID do not match".

I watched this happen live on the dev deployment before I understood why. The fix was to stop sharing — a coach's WebAuthn identity now lives in its own namespaced spot, exactly like admin's already did, for exactly the same underlying reason.

Bug 2: an ID that looked fine and wasn't

When the migration created a brand-new merged account instead of updating an existing one, it never set that document's _id explicitly — so MongoDB quietly generated one. Every other part of the codebase that touches a coach's data looks it up by _id as a plain Go string:

// coachID here is a hex string, decoded from the ObjectID Mongo generated
// on insert. It reads correctly. It is not the same BSON type as _id.
store.users.CountDocuments(ctx, bson.M{"_id": coachID, "roles": "coach"})

The document's real _id is a BSON ObjectID; coachID is a Go string. They print identically — same hex digits — but MongoDB compares _id by exact BSON type, not string value, so the query matches nothing. No error, no panic, just zero results, silently, forever. The same filter is used for updates, which is where it actually hurt: finishing registration, updating a login counter, resetting a passkey — every write for an account created this way failed quietly, until I happened to click "reset passkey" in the admin panel and got a 500 with no explanation.

These two are why the test suite now runs an integration lane against a real MongoDB — a type mismatch like this is invisible to anything mocked. If I'm honest about sequencing, I'd have built that lane before the migration tool, not after. Both bugs are exactly the class of thing it now exists to catch on the first run rather than the fifth.

Part two: the bugs that shipping found for me

At this point the migration was code-complete and verified, but the real /login page still pointed at the old athlete-only flow. The unified version existed at a test URL nobody visited. Actually shipping it was three small changes: point /login at the unified flow, delete the test route, remove the "prototype" wording.

I also fixed the thing that made the merge worth doing in the first place. If one identity holds two passkeys, the OS picker needs to tell them apart, or I'd have rebuilt the original problem with extra steps. WebAuthnDisplayName() now appends the role list for multi-role accounts, and a coach's credential reads Coach — {username}#XXXX, with a stable four-digit tag derived deterministically from the coach's ID. No schema change; the ID was already there.

Then I deployed, and coaches reached the coach workspace for the first time. Within a day I'd found five more bugs, none of them new. They'd been sitting there for weeks.

A broken feature is also a very effective mask. Everything downstream of it is untested by definition, and stays untested for exactly as long as the thing in front stays broken. Fixing it doesn't ship one change — it ships every unverified assumption behind it, all at once.

Bug 3: an artifact that wasn't what I tested

/workspace returned a 500. So did /app and /athletes. All three serve the React console's index.html directly, and the file wasn't in the container.

The repo has a Dockerfile with a multi-stage build: a Node/Vite stage that compiles the frontend into static/console/, then a Go stage that copies it in. But the Cloud Build trigger for production wasn't using it. It had silently fallen back to Google Cloud Buildpacks, which sees a Go module, runs the equivalent of go build ., and produces a perfectly functional binary with no frontend in it at all.

Nothing in the code was wrong. Every test passed. The container I deployed was simply not the container my build instructions describe — which is worse than a code bug, because there's nothing to read. The source is correct, the build is correct, and the artifact is wrong.

The fix wasn't a commit. It was a build trigger pointed at cloudbuild.yaml, confirmed by watching the frontend stage actually appear in the build log rather than trusting that it would.

And here's the part that stuck with me: this had always been broken. Production had never once contained a working console. Nobody noticed because no coach had ever gotten far enough to request one of those three routes — bug 4 stopped them at the athlete flow first. One bug standing in front of another, and the only way to discover the second was to fix the first.

Bug 4: I built an abstraction and then skipped it

The whole migration rests on that dual-read/write shim. Every coach-facing handler goes through coach_lookup.go, so nothing cares whether a given coach has been migrated yet.

Every handler except the new one I'd just written.

POST /auth/identify — the endpoint the unified login page calls to work out which roles an email has — queried users and nothing else. A coach who hadn't been migrated came back with an empty roles array. The frontend treats empty as ["athlete"], which is correct behaviour given the convention, and routed them into the athlete-only flow with no route to the coach passkey step. They could see a login page. They could not log in.

The fix was one fallback call to findCoachByFilter, the function that already existed for precisely this.

This is the most instructive of the seven, because the abstraction that would have prevented it was mine, was correct, and was already in use in a dozen other places. Building the right seam doesn't help if you write the next handler without going through it. There are still three older handlers touching store.coaches directly, flagged and not yet converted — and this bug is a preview of what each of them can do.

Bug 5: the right helper existed and had a warning on it

Coaches who did log in successfully got bounced straight back to the login screen.

/coach/verify and /manage/verify both used extractBearerToken, which reads the Authorization header and then, if there isn't one, falls back to the athlete access_token cookie. If you had any leftover athlete cookie in the same browser — from testing the athlete flow, or from genuinely being both — that fallback fired before coach_token was ever considered. Verify validated an athlete token against a coach endpoint, returned 401, and the frontend did the reasonable thing and sent you back to log in. Which worked. Which then bounced you again.

The correct helper, tokenFromRequestOrCookie(r, "coach_token"), already existed. Both auth middlewares already used it. Its doc comment already described this exact shadowing scenario as the reason it exists.

I had written the warning and then not read it.

Bug 6: one identity, one cookie

Smaller, and mostly an annoyance rather than a failure: logging in as a coach never set the athlete cookie, and vice versa — even though both cookies authenticate the same, already-passkey-verified identity. Holding both roles meant logging in twice to see both views. Fixed by issuing the sibling role's cookie at every login-completion path when Roles includes it, plus a role switcher in the UI.

Bug 7: a convention that only holds while it's empty

This is the one worth the most.

User.Roles is a string array, and an empty array means implicitly athlete-only. That convention is documented in types_auth.go and honoured consistently by every role check in the codebase. Most accounts predate roles entirely, so most have never had that field written at all.

Coach Yu also has a self-service merge: a coach whose athlete account was registered under a different email can link the two themselves by proving ownership with a real passkey login on the athlete account. That path did this:

// Athlete's doc has never had `roles` written: the field is absent,
// which every role check in the codebase reads as "athlete".
{"$addToSet": bson.M{"roles": "coach"}}
// Now roles == ["coach"]. The athlete role didn't get removed.
// It was never there. It was an absence that meant something,
// and it stopped meaning it the moment the array became non-empty.

The account came out as a coach and nothing else. No error, no failed write — the operation did exactly what it said. The convention just silently stopped applying, because "empty means athlete" and "contains coach" are the same array in two different states, and only one of them is empty.

I found it because a real production account completed the merge and then never showed the role-switcher button. The fix is trivial once you see it: $addToSet both roles via $each.

The part worth noticing isn't the fix. It's that I then went looking for the same shape elsewhere and found it twice more — in the batch migration tool's merge-into-existing-user path, and in the live path used when an admin invites a coach whose email already has an account. Same root cause, same silent failure, two more places it could have happened. Neither had fired yet.

A bug found is a pattern to search for. Fixing only the instance in front of you leaves the other two waiting.

What it cost, and a comparison I didn't plan

The already-merged account couldn't be repaired by re-running anything. The self-service merge deletes the old coach document once it has successfully repointed every reference to it — invites, notes, program ownership. That ordering is deliberate: there's no Mongo transaction available here, so the destructive step goes last and only runs if everything before it succeeded. If it dies halfway, nothing irreplaceable is gone.

That protects against partial failure. It does nothing about successful-but-wrong. The merge worked. The merge was also incorrect. And because it had worked, the source data was gone, and the only route back was a hand-written update against production.

What makes this interesting is that elsewhere in the same codebase I made the opposite call. Coach training programs migrate through versioned schema changes, and the original v1 document is retained permanently — so if a migration step turns out to be buggy, any document can be rebuilt by replaying from source. I get replay there because I kept the input.

Same engineer, two subsystems, opposite decisions about retention. One handed me a recovery path; the other handed me a manual database edit. I don't think the merge is wrong to delete — carrying orphaned coach documents forever has its own costs — but I'd now want that decision made explicitly, with the recovery story written down, rather than falling out of "the reference is repointed, so the old row is garbage."

What I'd do differently

Build the integration test lane first. Bugs 1 and 2 are precisely what it exists to catch, and it arrived after them rather than before.

Test the flows, not just the handlers. The suite covers each endpoint's validation and error branching well. Bugs 4 through 7 all lived in the seams: identify → role detection → passkey step → verify → cookie. Individually correct handlers, wrong end to end.

Have a staging environment that builds the way production builds. Bug 3 was possible because "it works locally" and "it works in the deployed container" were answering different questions. Local used the Dockerfile. Production silently didn't.

Audit the bypasses before shipping, not after. I knew three handlers still talked to the old collection directly. I then shipped a fourth new one that did too, by accident, and it broke login. Grepping for store.coaches takes about ten seconds.

And more generally: treat fixing a blocking bug as a deploy of everything behind it. Every route those coaches couldn't reach was untested in production by definition. That wasn't five unlucky coincidences — it was one predictable consequence, and framing it that way beforehand would have had me going looking rather than waiting to be told.

Still outstanding: three handlers to route through the shim, a re-run of the migration tool to repair any accounts caught by the role bug, and the old coaches collection to retire once all of that has baked. That last step doesn't start until nothing reads it — which is currently not true.

None of that is exotic. It's mostly finishing what was started, carefully, in the same style that caught the bugs above: change something, then go verify it actually did what it claims, against real data, not just against what the code appears to say.