Skip to content

Commit 2fb7e81

Browse files
arhumanarhuman
committed
feat(auth): map one verified email to one org, expose member id on seam
First OIDC login now links a new provider identity into an org already owned by the login's verified email instead of minting a duplicate org-of-one, so the same human via GitHub and Google keeps a single org and plan. The target org is chosen deterministically (paid over free, then oldest); only a genuinely new email creates a fresh org. githubIdentity is tightened to accept only a primary+verified address: the unverified public-profile fallback is removed and a failed emails lookup is now fatal, so cross-provider account linking never runs on an unverified email. Adds RouteContext.SessionMemberID to the extension seam (mirroring SessionOrg) so a composing build can resolve the caller's own member row without importing internal auth. See docs/adr/0024-one-verified-email-one-org.md.
1 parent 173ee45 commit 2fb7e81

9 files changed

Lines changed: 175 additions & 38 deletions

File tree

‎docs/adr/0024-one-verified-email-one-org.md‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
---
2+
status: accepted
3+
---
4+
5+
# One verified email maps to one org
6+
7+
First-login resolution keyed solely on the OIDC `oidc_subject` (`github:<id>`,
8+
`google:<sub>`), and each miss created a fresh org-of-one. Signing in through a
9+
second provider therefore produced a second `oidc_subject`, a second org, and a
10+
second plan for the same human — observed in production as one person holding both a
11+
`pro/active` and a `free/none` org. Plans are per-org, so this split a paying customer
12+
across two tenants. This ADR records the fix.
13+
14+
## Decision
15+
16+
- **Link a new identity into an org that already belongs to its verified email.** In
17+
`upsertMemberFromOIDC`, when no member matches the `oidc_subject` (first login for
18+
this identity), we look up an org already owned by the login's email before minting
19+
a new one. On a match we insert a new admin member carrying the new
20+
`oidc_subject` into that org and report `isNew=false` (an existing org, so no
21+
reveal-once key interstitial). Only with no match do we create the org-of-one, as
22+
before (`isNew=true`). The target org is chosen deterministically: a paid org over a
23+
free one, then the oldest, so a person converges on their revenue-bearing tenant.
24+
- **Only ever link on a provider-verified email.** Linking by email is safe precisely
25+
because the email is provider-verified: `googleIdentity` already required
26+
`email_verified`, and `githubIdentity` is tightened here to accept only a
27+
`primary && verified` address from `/user/emails` — the previous fallback to the
28+
(possibly unverified) public profile email is removed, and a failed emails lookup is
29+
now a hard error rather than a silent downgrade. An unverified address must never
30+
reach a `members` row, or a stranger could claim another person's org by registering
31+
their email at a second provider.
32+
- **A linked identity is a second `members` row, not a merged one.** The same human
33+
ends up with two member rows (one per provider) in one org. This keeps the change to
34+
a single query branch and no schema change. It has one cost: each identity counts as
35+
a seat.
36+
37+
## Consequences
38+
39+
- The duplicate-per-provider path is closed going forward. Orgs that were *already*
40+
split before this change are not healed retroactively (both identities already have
41+
member rows, so neither hits the link path); they are merged once, by hand, via
42+
`maping-enterprise/ops/merge-dup-org.sql`.
43+
- A person who links a second provider consumes a second seat in their org. If this
44+
proves annoying at a low free-tier seat cap, the clean fix is a `member_identities`
45+
table (one member, many OIDC identities) so an identity is not a seat; that is
46+
deliberately deferred, not built here.
47+
- A GitHub account with no verified primary email can no longer sign in at all (it
48+
previously could, via the public profile email). This is the intended safety
49+
posture now that email drives account linking.

‎docs/adr/README.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,3 +28,4 @@ records keep their number and are marked here rather than deleted.
2828
| [0021](0021-diagnosis-engine.md) | Diagnosis engine on the endpoint detail page |
2929
| [0022](0022-relicense-server-mit.md) | Relicense the server to MIT (supersedes 0004) |
3030
| [0023](0023-in-app-documentation.md) | In-app documentation |
31+
| [0024](0024-one-verified-email-one-org.md) | One verified email maps to one org (account linking across providers) |

‎server/app/assemble.go‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -134,14 +134,22 @@ func assembleMux(d builtDeps, o options, log *slog.Logger) (http.Handler, *atomi
134134
// verified-org reader (both nil when auth is off), so a composing build mounts
135135
// its own authenticated routes without importing the internal auth package.
136136
var (
137-
gate func(http.Handler) http.Handler
138-
sessionOrg func(*http.Request) (string, bool)
137+
gate func(http.Handler) http.Handler
138+
sessionOrg func(*http.Request) (string, bool)
139+
sessionMemberID func(*http.Request) (string, bool)
139140
)
140141
if authLayer != nil {
141142
gate = authLayer.Middleware
142143
sessionOrg = auth.TenantFromContext
144+
sessionMemberID = func(r *http.Request) (string, bool) {
145+
s, ok := auth.FromContext(r.Context())
146+
if !ok {
147+
return "", false
148+
}
149+
return s.MemberID, true
150+
}
143151
}
144-
mountExtensions(mux, o.routes, d.pool, gate, sessionOrg, webHandler.RenderShellPage, docsHandler.Render, log)
152+
mountExtensions(mux, o.routes, d.pool, gate, sessionOrg, sessionMemberID, webHandler.RenderShellPage, docsHandler.Render, log)
145153

146154
// Background loops (extension jobs) share one context, cancelled on shutdown so
147155
// every goroutine exits before the pool closes.

‎server/app/extensions.go‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,12 @@ type RouteContext struct {
3535
// caller's org (never the request body), keeping the session context key
3636
// private to auth. Nil when auth is off.
3737
SessionOrg func(*http.Request) (orgID string, ok bool)
38+
// SessionMemberID reads the caller's verified member id from a request that has
39+
// passed through Gate. Like SessionOrg it reads the private auth session context
40+
// key an extension cannot reach directly, so an extension can resolve the caller's
41+
// own member row (e.g. their email) without trusting request input. Nil when auth
42+
// is off.
43+
SessionMemberID func(*http.Request) (memberID string, ok bool)
3844
// RenderShell writes a full dashboard page (sidebar + top bar chrome) wrapping
3945
// the given content, so an extension's own page looks native to the dashboard
4046
// instead of a detached page. content is trusted HTML the caller produced from a
@@ -198,9 +204,9 @@ func WithAccountLink(href string) Option {
198204
// core routes are mounted. pool is nil in static dev mode; gate and sessionOrg are
199205
// nil when auth is off, so a registrar needing authenticated routes checks them
200206
// and mounts nothing when absent.
201-
func mountExtensions(mux *http.ServeMux, routes []RouteRegistrar, pool *pgxpool.Pool, gate func(http.Handler) http.Handler, sessionOrg func(*http.Request) (string, bool), renderShell func(http.ResponseWriter, *http.Request, string, template.HTML), renderDoc func(http.ResponseWriter, *http.Request, string, template.HTML), log *slog.Logger) {
207+
func mountExtensions(mux *http.ServeMux, routes []RouteRegistrar, pool *pgxpool.Pool, gate func(http.Handler) http.Handler, sessionOrg func(*http.Request) (string, bool), sessionMemberID func(*http.Request) (string, bool), renderShell func(http.ResponseWriter, *http.Request, string, template.HTML), renderDoc func(http.ResponseWriter, *http.Request, string, template.HTML), log *slog.Logger) {
202208
for _, r := range routes {
203-
r(RouteContext{Mux: mux, Pool: pool, Log: log, Gate: gate, SessionOrg: sessionOrg, RenderShell: renderShell, RenderDoc: renderDoc})
209+
r(RouteContext{Mux: mux, Pool: pool, Log: log, Gate: gate, SessionOrg: sessionOrg, SessionMemberID: sessionMemberID, RenderShell: renderShell, RenderDoc: renderDoc})
204210
}
205211
if len(routes) > 0 {
206212
log.Info("extension routes mounted", slog.Int("count", len(routes)))

‎server/app/extensions_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ func TestMountExtensionsMountsExtraRoutes(t *testing.T) {
6363
_, _ = w.Write([]byte("pong"))
6464
})
6565
},
66-
}, nil, nil, nil, nil, renderDoc, testLogger())
66+
}, nil, nil, nil, nil, nil, renderDoc, testLogger())
6767

6868
require.True(t, called, "registrar must be invoked")
6969
rec := httptest.NewRecorder()

‎server/internal/auth/userinfo.go‎

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -86,8 +86,7 @@ func getJSON(ctx context.Context, client *http.Client, url string, tok *oauth2.T
8686
// githubIdentity reads the GitHub user id and a verified primary email.
8787
func githubIdentity(ctx context.Context, client *http.Client, _ *oauth2.Config, tok *oauth2.Token) (identity, error) {
8888
var user struct {
89-
ID int64 `json:"id"`
90-
Email string `json:"email"`
89+
ID int64 `json:"id"`
9190
}
9291
if err := getJSON(ctx, client, "https://api.github.com/user", tok, &user); err != nil {
9392
return identity{}, err
@@ -96,24 +95,27 @@ func githubIdentity(ctx context.Context, client *http.Client, _ *oauth2.Config,
9695
return identity{}, fmt.Errorf("auth: github user has no id")
9796
}
9897

99-
email := user.Email
100-
// The public profile email may be null; fetch the verified primary from the
101-
// emails endpoint.
98+
// Only ever trust a primary+verified address from the emails endpoint: the
99+
// public profile email (user.email) may be unverified or null, and the identity
100+
// is linked to an account by email downstream, so an unverified value must never
101+
// reach a member row.
102102
var emails []struct {
103103
Email string `json:"email"`
104104
Primary bool `json:"primary"`
105105
Verified bool `json:"verified"`
106106
}
107-
if err := getJSON(ctx, client, "https://api.github.com/user/emails", tok, &emails); err == nil {
108-
for _, e := range emails {
109-
if e.Primary && e.Verified {
110-
email = e.Email
111-
break
112-
}
107+
if err := getJSON(ctx, client, "https://api.github.com/user/emails", tok, &emails); err != nil {
108+
return identity{}, fmt.Errorf("auth: github emails: %w", err)
109+
}
110+
email := ""
111+
for _, e := range emails {
112+
if e.Primary && e.Verified {
113+
email = e.Email
114+
break
113115
}
114116
}
115117
if email == "" {
116-
return identity{}, fmt.Errorf("auth: github: no verified email")
118+
return identity{}, fmt.Errorf("auth: github: no verified primary email")
117119
}
118120
return identity{Subject: fmt.Sprintf("github:%d", user.ID), Email: email}, nil
119121
}

‎server/internal/auth/userinfo_test.go‎

Lines changed: 20 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -173,15 +173,17 @@ func TestGithubIdentity_HappyPath(t *testing.T) {
173173
}
174174
}
175175

176-
// TestGithubIdentity_FallsBackToProfileEmail verifies that when the /user/emails
177-
// endpoint returns no verified primary, the public profile email is used.
178-
func TestGithubIdentity_FallsBackToProfileEmail(t *testing.T) {
176+
// TestGithubIdentity_RejectsUnverifiedProfileEmail verifies that when /user/emails
177+
// has no verified primary, the login is REJECTED rather than falling back to the
178+
// (possibly unverified) public profile email. The identity is linked to an account
179+
// by email downstream, so an unverified address must never reach a member row.
180+
func TestGithubIdentity_RejectsUnverifiedProfileEmail(t *testing.T) {
179181
mux := http.NewServeMux()
180182
mux.HandleFunc("/user", func(w http.ResponseWriter, _ *http.Request) {
181183
w.Header().Set("Content-Type", "application/json")
182184
json.NewEncoder(w).Encode(map[string]any{"id": 42, "email": "profile@example.com"})
183185
})
184-
// /user/emails returns unverified entries — no match, so fall back.
186+
// /user/emails returns only unverified entries — no verified primary to trust.
185187
mux.HandleFunc("/user/emails", func(w http.ResponseWriter, _ *http.Request) {
186188
w.Header().Set("Content-Type", "application/json")
187189
json.NewEncoder(w).Encode([]map[string]any{
@@ -194,12 +196,12 @@ func TestGithubIdentity_FallsBackToProfileEmail(t *testing.T) {
194196
client := srv.Client()
195197
client.Transport = rewriteTransport{base: srv.URL, inner: srv.Client().Transport}
196198

197-
id, err := githubIdentity(context.Background(), client, nil, fakeToken())
198-
if err != nil {
199-
t.Fatalf("githubIdentity error: %v", err)
199+
_, err := githubIdentity(context.Background(), client, nil, fakeToken())
200+
if err == nil {
201+
t.Fatal("expected error when no verified primary email is available")
200202
}
201-
if id.Email != "profile@example.com" {
202-
t.Errorf("email = %q, want profile@example.com (profile fallback)", id.Email)
203+
if !strings.Contains(err.Error(), "no verified primary email") {
204+
t.Errorf("error %q should mention 'no verified primary email'", err.Error())
203205
}
204206
}
205207

@@ -226,17 +228,18 @@ func TestGithubIdentity_MissingID(t *testing.T) {
226228
}
227229
}
228230

229-
// TestGithubIdentity_NoEmail verifies that a missing email (zero profile email,
230-
// no verified primary) is rejected.
231-
func TestGithubIdentity_NoEmail(t *testing.T) {
231+
// TestGithubIdentity_EmailsEndpointFailure verifies that when the /user/emails
232+
// lookup fails, the login is rejected: since the verified primary is now the ONLY
233+
// trusted email source (no profile fallback), a failed lookup cannot yield an
234+
// identity.
235+
func TestGithubIdentity_EmailsEndpointFailure(t *testing.T) {
232236
mux := http.NewServeMux()
233237
mux.HandleFunc("/user", func(w http.ResponseWriter, _ *http.Request) {
234238
w.Header().Set("Content-Type", "application/json")
235-
// email is empty string in the profile
236239
json.NewEncoder(w).Encode(map[string]any{"id": 7})
237240
})
238241
mux.HandleFunc("/user/emails", func(w http.ResponseWriter, _ *http.Request) {
239-
// Return a non-200 to simulate the emails endpoint failing
242+
// Non-200 simulates the emails endpoint failing.
240243
http.Error(w, "not found", http.StatusNotFound)
241244
})
242245
srv := httptest.NewServer(mux)
@@ -247,10 +250,10 @@ func TestGithubIdentity_NoEmail(t *testing.T) {
247250

248251
_, err := githubIdentity(context.Background(), client, nil, fakeToken())
249252
if err == nil {
250-
t.Fatal("expected error for missing email")
253+
t.Fatal("expected error when the github emails lookup fails")
251254
}
252-
if !strings.Contains(err.Error(), "no verified email") {
253-
t.Errorf("error %q should mention 'no verified email'", err.Error())
255+
if !strings.Contains(err.Error(), "github emails") {
256+
t.Errorf("error %q should mention the failed 'github emails' lookup", err.Error())
254257
}
255258
}
256259

‎server/internal/control/member.go‎

Lines changed: 40 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,46 @@ func upsertMemberFromOIDC(ctx context.Context, tb txBeginner, oidcSubject, email
5656
return "", "", "", false, fmt.Errorf("control.UpsertMemberFromOIDC: lookup: %w", err)
5757
}
5858

59-
// First login: create an org-of-one and its admin member atomically. Name
59+
// First login for this identity: link into an org already owned by this verified
60+
// email, or mint a fresh org-of-one. Both paths commit tx.
61+
return firstLoginOrLink(ctx, tx, oidcSubject, email)
62+
}
63+
64+
// firstLoginOrLink handles the no-existing-member path inside tx. It first tries to
65+
// link the identity into an org that already belongs to this verified email — the
66+
// same human signing in through a second provider (github: vs google:) — preferring a
67+
// paid org over free and otherwise the oldest, so a person keeps a single org/plan
68+
// instead of a duplicate per provider. A match inserts a new admin member there and
69+
// reports isNew=false (an existing org, so no reveal-once key interstitial). With no
70+
// such org it creates an org-of-one and its admin member, reporting isNew=true. email
71+
// is provider-verified (auth.userinfo), which is what makes cross-provider linking
72+
// safe. It commits tx on success.
73+
func firstLoginOrLink(ctx context.Context, tx pgx.Tx, oidcSubject, email string) (orgID, memberID, role string, isNew bool, err error) {
74+
err = tx.QueryRow(ctx,
75+
`SELECT id::text FROM orgs
76+
WHERE id IN (SELECT org_id FROM members WHERE lower(email) = lower($1))
77+
ORDER BY (plan <> 'free') DESC, created_at ASC
78+
LIMIT 1`, email,
79+
).Scan(&orgID)
80+
switch {
81+
case err == nil:
82+
role = "admin"
83+
if err = tx.QueryRow(ctx,
84+
`INSERT INTO members (org_id, email, role, oidc_subject)
85+
VALUES ($1, $2, 'admin', $3) RETURNING id::text`,
86+
orgID, email, oidcSubject,
87+
).Scan(&memberID); err != nil {
88+
return "", "", "", false, fmt.Errorf("control.UpsertMemberFromOIDC: link member: %w", err)
89+
}
90+
if err = tx.Commit(ctx); err != nil {
91+
return "", "", "", false, fmt.Errorf("control.UpsertMemberFromOIDC: commit link: %w", err)
92+
}
93+
return orgID, memberID, role, false, nil
94+
case !errors.Is(err, pgx.ErrNoRows):
95+
return "", "", "", false, fmt.Errorf("control.UpsertMemberFromOIDC: link lookup: %w", err)
96+
}
97+
98+
// No existing org for this email: create an org-of-one and its admin member. Name
6099
// the org after the email so the dashboard has a human label.
61100
if err = tx.QueryRow(ctx,
62101
`INSERT INTO orgs (name, plan) VALUES ($1, 'free') RETURNING id::text`, email,
@@ -71,7 +110,6 @@ func upsertMemberFromOIDC(ctx context.Context, tb txBeginner, oidcSubject, email
71110
).Scan(&memberID); err != nil {
72111
return "", "", "", false, fmt.Errorf("control.UpsertMemberFromOIDC: create member: %w", err)
73112
}
74-
75113
if err = tx.Commit(ctx); err != nil {
76114
return "", "", "", false, fmt.Errorf("control.UpsertMemberFromOIDC: commit create: %w", err)
77115
}

‎server/internal/control/member_test.go‎

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,8 +65,10 @@ func TestUpsertMemberFromOIDCExisting(t *testing.T) {
6565
}
6666

6767
func TestUpsertMemberFromOIDCFirstLoginCreatesAdminOrgOfOne(t *testing.T) {
68-
// 1) member lookup -> no rows; 2) INSERT org RETURNING id; 3) INSERT member RETURNING id.
68+
// 1) member lookup by subject -> no rows; 2) email -> existing org lookup -> no
69+
// rows (novel email); 3) INSERT org RETURNING id; 4) INSERT member RETURNING id.
6970
tx := &fakeTx{rows: []fakeRow{
71+
{err: pgx.ErrNoRows},
7072
{err: pgx.ErrNoRows},
7173
{values: []any{"org-new"}},
7274
{values: []any{"member-new"}},
@@ -89,6 +91,34 @@ func TestUpsertMemberFromOIDCFirstLoginCreatesAdminOrgOfOne(t *testing.T) {
8991
}
9092
}
9193

94+
func TestUpsertMemberFromOIDCFirstLoginLinksExistingEmailOrg(t *testing.T) {
95+
// A second provider identity for a verified email that already owns an org must
96+
// link into that org instead of creating a new one.
97+
// 1) member lookup by subject -> no rows; 2) email -> existing org "org-1";
98+
// 3) INSERT member into org-1 RETURNING id. No org is created.
99+
tx := &fakeTx{rows: []fakeRow{
100+
{err: pgx.ErrNoRows},
101+
{values: []any{"org-1"}},
102+
{values: []any{"member-2"}},
103+
}}
104+
org, mem, role, isNew, err := upsertMemberFromOIDC(context.Background(), &fakeTxBeginner{tx: tx}, "google:sub", "dup@user.dev")
105+
if err != nil {
106+
t.Fatalf("upsertMemberFromOIDC: %v", err)
107+
}
108+
if org != "org-1" || mem != "member-2" {
109+
t.Errorf("got org=%q member=%q, want org-1/member-2", org, mem)
110+
}
111+
if role != "admin" {
112+
t.Errorf("linked member should be admin, got %q", role)
113+
}
114+
if isNew {
115+
t.Error("linking to an existing org must report isNew=false (no fresh-key interstitial)")
116+
}
117+
if !tx.committed {
118+
t.Error("link path must commit")
119+
}
120+
}
121+
92122
func TestDevOrgAdminReusesExistingMember(t *testing.T) {
93123
// 1) org-by-name -> org id; 2) member-by-subject -> member id (exists).
94124
q := &scriptedQuerier{rows: []fakeRow{

0 commit comments

Comments
 (0)