bb3f3f9c420a366835a93d0073a5db3668282349 max Mon Aug 10 09:50:47 2026 -0700 hgLogin: close the two remaining social-login account gaps, refs #38037 Fold the email-verified flag into the signed pending-identity string so it can no longer be flipped by adding &oauth_pending_email_verified=1 to the completeAccount request, which decided whether a new account was written activated (trusted for future auto-linking). Require accountActivated='Y' in the four passwordless email-link queries (sendEmailLink, emailLogin, and the two email-mode chooser queries), so the login link no longer signs anyone into an unactivated account that a stranger created with their address. Password login and the OAuth queries already did this. diff --git src/hg/hgLogin/hgLogin.c src/hg/hgLogin/hgLogin.c index 595cb879f59..2df0c4f7094 100644 --- src/hg/hgLogin/hgLogin.c +++ src/hg/hgLogin/hgLogin.c @@ -1808,86 +1808,89 @@ char query[512]; sqlSafef(query, sizeof(query), "SELECT idx FROM gbMemberIdentity WHERE provider='%s' AND subject='%s'", id->provider, id->subject); uint idx = (uint)sqlQuickLongLong(conn, query); if (idx == 0) return NULL; sqlSafef(query, sizeof(query), "SELECT * FROM gbMembers WHERE idx=%u", idx); return gbMembersLoadByQuery(conn, query); } /* A pending social identity is good for this many seconds -- long enough to choose a username * or an account, short enough that a leaked signature is quickly useless. */ #define OAUTH_PENDING_TTL 900 -static char *oauthPendingSig(char *provider, char *subject, char *email, char *timeStr) +static char *oauthPendingSig(char *provider, char *subject, char *email, char *emailVerified, + char *timeStr) /* HMAC-MD5 over a pending social identity, keyed by the secret login.cookieSalt. Only * resolveIdentity (which runs after a genuine provider verification) can produce a valid one, * so a pending identity injected through cart/CGI variables will not validate. The signature * also covers this browser's hguid (cart->userId, which -- unlike the hgsid -- survives the * provider redirect) and the time it was minted, so a signature that leaks into a saved or * shared session cannot be replayed by a different browser or after it expires (see * pendingIdentityValid). Result is allocd. */ { char *salt = cfgOption(CFG_LOGIN_COOKIE_SALT); if (isEmpty(salt)) errAbort("Signing in with an external identity provider requires %s in hg.conf, set to a " "secret random string. Without a secret we cannot sign the pending identity, and the " "account chooser would accept a forged one.", CFG_LOGIN_COOKIE_SALT); char buf[1024]; -safef(buf, sizeof(buf), "%s|%s|%s|%s|%s", +safef(buf, sizeof(buf), "%s|%s|%s|%s|%s|%s", emptyForNull(provider), emptyForNull(subject), emptyForNull(email), - emptyForNull(cart->userId), emptyForNull(timeStr)); + emptyForNull(emailVerified), emptyForNull(cart->userId), emptyForNull(timeStr)); return hmacMd5(salt, buf); } static boolean pendingIdentityValid() /* TRUE only if the pending-identity cart variables carry a signature we minted, for this * browser, within the last OAUTH_PENDING_TTL seconds. Guards the OAuth chooser and * completeAccount against forged, injected, replayed, or stale pending identities. */ { char *sig = cartUsualString(cart, "oauth_pending_sig", ""); char *timeStr = cartUsualString(cart, "oauth_pending_time", ""); if (isEmpty(sig) || isEmpty(timeStr)) return FALSE; if (clock1() - atol(timeStr) > OAUTH_PENDING_TTL) return FALSE; char *expected = oauthPendingSig(cartUsualString(cart, "oauth_pending_provider", ""), cartUsualString(cart, "oauth_pending_subject", ""), cartUsualString(cart, "oauth_pending_email", ""), + cartUsualString(cart, "oauth_pending_email_verified", ""), timeStr); boolean ok = sameString(sig, expected); freeMem(expected); return ok; } static void setPendingIdentity(struct oauthIdentity *id) /* Stash an authenticated-but-not-yet-linked identity in the cart so it survives a form * round-trip (the "choose a username" or "choose an account" page). The signature is what * proves, on the way back, that we really verified this identity, for this browser, recently. */ { char timeStr[32]; safef(timeStr, sizeof(timeStr), "%ld", clock1()); cartSetString(cart, "oauth_pending_provider", id->provider); cartSetString(cart, "oauth_pending_subject", id->subject); cartSetString(cart, "oauth_pending_email", emptyForNull(id->email)); -cartSetString(cart, "oauth_pending_email_verified", id->emailVerified ? "1" : "0"); +char *emailVerified = id->emailVerified ? "1" : "0"; +cartSetString(cart, "oauth_pending_email_verified", emailVerified); cartSetString(cart, "oauth_pending_name", emptyForNull(id->displayName)); cartSetString(cart, "oauth_pending_time", timeStr); cartSetString(cart, "oauth_pending_sig", - oauthPendingSig(id->provider, id->subject, emptyForNull(id->email), timeStr)); + oauthPendingSig(id->provider, id->subject, emptyForNull(id->email), emailVerified, timeStr)); } static void clearPendingIdentity() /* Remove the pending-identity cart variables. Call this on every path that finishes with the * pending identity -- success or definitive failure -- so a stale signature is not left behind * in the cart to be swept into a saved session. */ { cartRemove(cart, "oauth_pending_provider"); cartRemove(cart, "oauth_pending_subject"); cartRemove(cart, "oauth_pending_email"); cartRemove(cart, "oauth_pending_email_verified"); cartRemove(cart, "oauth_pending_name"); cartRemove(cart, "oauth_pending_time"); cartRemove(cart, "oauth_pending_sig"); } @@ -2057,31 +2060,31 @@ char *email = emailMode ? cartUsualString(cart, "emailLogin_email", "") : cartUsualString(cart, "oauth_pending_email", ""); if (isEmpty(email) || (!emailMode && !pendingIdentityValid())) { if (!emailMode) 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() ORDER BY idx", + "AND loginToken<>'' AND loginTokenExpires > NOW() AND accountActivated='Y' ORDER BY idx", email, email, cartUsualString(cart, "emailLogin_tokenMd5", "")); else sqlSafef(query, sizeof(query), "SELECT * FROM gbMembers WHERE email='%s' ORDER BY idx", email); struct gbMembers *list = gbMembersLoadByQuery(conn, query), *m; hPrintf("<div id=\"chooseAccountBox\" class=\"centeredContainer formBox\">" "<h2>%s</h2>", brwName); hPrintf("<h3>Choose an account</h3>"); if (emailMode) hPrintf("<p>The email address <b>%s</b> is associated with more than one %s account. " "Select the account you would like to sign in to.</p>", encEmail, brwName); else hPrintf("<p>The email address <b>%s</b> 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.</p>", @@ -2127,31 +2130,32 @@ { displayLoginPage(conn); return; } char *email = cartUsualString(cart, "emailLogin_email", ""); char *tokenMd5 = cartUsualString(cart, "emailLogin_tokenMd5", ""); if (isEmpty(email) || isEmpty(tokenMd5)) { freez(&errMsg); errMsg = cloneString("Your login link expired. Please request a new one."); displayLoginPage(conn); return; } sqlSafef(query, sizeof(query), "SELECT * FROM gbMembers WHERE idx=%d AND (email='%s' OR recovEmail='%s') " - "AND loginToken='%s' AND loginToken<>'' AND loginTokenExpires > NOW()", + "AND loginToken='%s' AND loginToken<>'' AND loginTokenExpires > NOW() " + "AND accountActivated='Y'", chosenIdx, email, email, tokenMd5); struct gbMembers *m = gbMembersLoadByQuery(conn, query); if (m == NULL) { freez(&errMsg); errMsg = cloneString("Please choose one of the listed accounts."); chooseAccountPage(conn); return; } /* Consume the token on every account that shared it (single use), then sign in. */ sqlSafef(query, sizeof(query), "UPDATE gbMembers SET loginToken='' WHERE (email='%s' OR recovEmail='%s') AND loginToken='%s'", email, email, tokenMd5); sqlUpdate(conn, query); cartRemove(cart, "emailLogin_email"); @@ -2405,31 +2409,32 @@ if (!emailLinkEnabled()) { displayLoginPage(conn); return; } char *email = cartUsualString(cart, "hgLogin_email", ""); if (isEmpty(email) || spc_email_isvalid(email) == 0) { freez(&errMsg); errMsg = cloneString("Please enter a valid email address."); emailLinkPage(conn); return; } char query[512]; sqlSafef(query, sizeof(query), - "SELECT * FROM gbMembers WHERE email='%s' OR recovEmail='%s'", email, email); + "SELECT * FROM gbMembers WHERE (email='%s' OR recovEmail='%s') AND accountActivated='Y'", + email, email); struct gbMembers *list = gbMembersLoadByQuery(conn, query), *m; if (list != NULL) { /* One token for the address, stored on every account that uses it, and one email. * The user proves they own the address by clicking; only then (in emailLogin) do we * reveal the accounts and let them choose, so we never disclose accounts to someone * who merely typed the address here. */ char *token = makeRandomKey(128+33); char *tokenMD5 = generateTokenMD5(token); for (m = list; m != NULL; m = m->next) { sqlSafef(query, sizeof(query), "UPDATE gbMembers SET loginToken='%s', " "loginTokenExpires=DATE_ADD(NOW(), INTERVAL 1 HOUR) WHERE idx=%u", tokenMD5, m->idx); @@ -2448,31 +2453,32 @@ /* Validate a one-time email login token. The token proves the user owns the address; if it * matches one account, log straight in; if it matches several accounts that share the * address, show the account chooser (the same one the OAuth flow uses). */ { if (!emailLinkEnabled()) { displayLoginPage(conn); return; } char *email = cgiUsualString("email", ""); char *token = cgiUsualString("token", ""); char *tokenMD5 = generateTokenMD5(token); char query[512]; sqlSafef(query, sizeof(query), "SELECT * FROM gbMembers WHERE (email='%s' OR recovEmail='%s') AND loginToken='%s' " - "AND loginToken<>'' AND loginTokenExpires > NOW() ORDER BY idx", email, email, tokenMD5); + "AND loginToken<>'' AND loginTokenExpires > NOW() AND accountActivated='Y' ORDER BY idx", + email, email, tokenMD5); struct gbMembers *list = gbMembersLoadByQuery(conn, query); int n = slCount(list); if (n == 0) { freez(&errMsg); errMsg = cloneString("This login link is invalid or has expired. Please request a new one."); displayLoginPage(conn); } else if (n == 1) { sqlSafef(query, sizeof(query), "UPDATE gbMembers SET loginToken='', lastUse=NOW() WHERE idx=%u", list->idx); sqlUpdate(conn, query); loginAndReturn(list->userName, list->idx); }