f75ffeed34d2a51f88fb338562bb7e1187ead08d max Tue Aug 18 06:30:37 2026 -0700 hgLogin: match OAuth/OpenID login on recovery email too, not just primary, refs #37984 When a provider returns a verified email, resolveIdentity() and the account chooser only matched it against gbMembers.email, so a user whose provider email is stored as their recovery address (recovEmail) could not auto-link or sign into that account. Password login and the passwordless email link already match email OR recovEmail; this brings social login in line. The provider email is guaranteed non-empty before the query runs (the existing isNotEmpty/isEmpty guards), so a blank recovEmail='' row can never match the empty string. Updated all three queries: the resolveIdentity auto-link, the OAuth branch of chooseAccountPage, and the chooseAccount confirmation, so the chooser cannot offer a row the finalize step rejects. diff --git src/hg/hgLogin/hgLogin.c src/hg/hgLogin/hgLogin.c index 7009e166211..17e30c1f304 100644 --- src/hg/hgLogin/hgLogin.c +++ src/hg/hgLogin/hgLogin.c @@ -2078,33 +2078,35 @@ clearPendingIdentity(); displayLoginPage(conn); return; } char *encEmail = htmlEncode(email); // the address is displayed; never trust it raw (XSS) char query[512]; if (emailMode) // Only the accounts that hold the just-validated login token, matching what emailLogin saw. sqlSafef(query, sizeof(query), "SELECT * FROM gbMembers WHERE (email='%s' OR recovEmail='%s') AND loginToken='%s' " "AND loginToken<>'' AND loginTokenExpires > NOW() AND accountActivated='Y' ORDER BY idx", email, email, cartUsualString(cart, "emailLogin_tokenMd5", "")); else // Only activated accounts, matching what chooseAccount() and resolveIdentity() accept; // otherwise the page offers a row the action refuses, and shows the username of an - // unactivated row anyone could have created with this address. + // unactivated row anyone could have created with this address. Match both primary and + // recovery address, as resolveIdentity() does; email is non-empty here (checked above). sqlSafef(query, sizeof(query), - "SELECT * FROM gbMembers WHERE email='%s' AND accountActivated='Y' ORDER BY idx", email); + "SELECT * FROM gbMembers WHERE (email='%s' OR recovEmail='%s') AND accountActivated='Y' " + "ORDER BY idx", email, email); struct gbMembers *list = gbMembersLoadByQuery(conn, query), *m; hPrintf("
" "

%s

", brwName); hPrintf("

Choose an account

"); if (emailMode) hPrintf("

The email address %s is associated with more than one %s account. " "Select the account you would like to sign in to.

", encEmail, brwName); else hPrintf("

The email address %s is associated with more than one %s account. " "Select the account you would like to sign in to; your %s login will be linked to it.

", encEmail, brwName, oauthProviderLabel(provider)); hPrintf("%s", errMsg ? errMsg : ""); hPrintf("
", hgLoginUrl); hPrintf("
"); @@ -2187,34 +2189,37 @@ return; } /* OAuth mode. */ char *subject = cartUsualString(cart, "oauth_pending_subject", ""); char *email = cartUsualString(cart, "oauth_pending_email", ""); if (isEmpty(subject) || isEmpty(email) || !pendingIdentityValid()) { clearPendingIdentity(); freez(&errMsg); errMsg = cloneString("Your login session expired. Please sign in again."); displayLoginPage(conn); return; } /* Only an activated account counts: an unactivated row can hold any address someone typed - * without ever proving they own it (see resolveIdentity), so it must not receive a social link. */ + * without ever proving they own it (see resolveIdentity), so it must not receive a social link. + * Match both primary and recovery address, as chooseAccountPage() offers; email is non-empty + * here (checked above). */ sqlSafef(query, sizeof(query), - "SELECT * FROM gbMembers WHERE idx=%d AND email='%s' AND accountActivated='Y'", - chosenIdx, email); + "SELECT * FROM gbMembers WHERE idx=%d AND (email='%s' OR recovEmail='%s') " + "AND accountActivated='Y'", + chosenIdx, email, email); struct gbMembers *m = gbMembersLoadByQuery(conn, query); if (m == NULL) { freez(&errMsg); errMsg = cloneString("Please choose one of the listed accounts."); chooseAccountPage(conn); return; } struct oauthIdentity pending; ZeroVar(&pending); pending.provider = provider; pending.subject = subject; pending.email = email; linkIdentity(conn, m->idx, &pending); clearPendingIdentity(); @@ -2228,38 +2233,42 @@ * 1. If the provider gave a verified email matching MORE THAN ONE account, always let the * user pick which one -- even if this identity was linked before. Because login cookies * never expire, a user goes through OAuth very rarely, so an occasional pick is cheap * and it lets a person with several same-email accounts choose freely each time. * 2. Else if the (provider,subject) is already linked, log into that account. * 3. Else if the verified email matches exactly one account, auto-link and log in. * 4. Else send the user to the "choose a username" page to finish a new account. * (Providers that don't release an email, e.g. ORCID, never reach step 1 or 3 and rely on * the stored link from step 2.) */ { struct gbMembers *matches = NULL; int n = 0; if (id->emailVerified && isNotEmpty(id->email)) { char query[512]; - /* Match only activated accounts. gbMembers has no unique key on email, and the plain + /* Match the provider email against both the primary and the recovery address, the same as + * password and email-link login do (see sendUsername/emailLogin). The isNotEmpty() guard + * above keeps id->email out of the query, so a blank recovEm=='' row can never match. + * Match only activated accounts. gbMembers has no unique key on email, and the plain * signup form will create an unactivated row for any address a person types -- the * activation mail goes to the address's real owner, who ignores it. Without this filter * someone could pre-register a victim's address, and the victim's first social login would * then auto-link to (and sign in as) the attacker's account. */ sqlSafef(query, sizeof(query), - "SELECT * FROM gbMembers WHERE email='%s' AND accountActivated='Y' ORDER BY idx", - id->email); + "SELECT * FROM gbMembers WHERE (email='%s' OR recovEmail='%s') AND accountActivated='Y' " + "ORDER BY idx", + id->email, id->email); matches = gbMembersLoadByQuery(conn, query); n = slCount(matches); } if (n > 1) { setPendingIdentity(id); gbMembersFreeList(&matches); chooseAccountPage(conn); return; } struct gbMembers *linked = memberForIdentity(conn, id); if (linked != NULL) {