4998072e98b35a6fc5c694f1ae56fcd01fefe3bb max Tue Sep 15 05:29:18 2026 -0700 hgLogin: say why a social sign-in is asking for an email confirmation Landing on "a confirmation email has been sent to you" straight after clicking a provider button is confusing on its own: the user asked to sign in, not to fill in a form, and is told to go and read their mail. The page now says first what happened, in one of two ways, because the two cases that reach it are different. A first sign-in through a provider that released no address says that no account uses the address they typed yet, so one is being made for it. An existing account whose address has never been confirmed names that account instead, since telling its owner that no account exists would be untrue. An account that already carries the address the provider just gave us no longer has to confirm anything: that is the same assurance a new signup through the same provider gets, so the account is activated and the user signed in. Accounts left unactivated by the earlier code, which asked for an address and took it on trust, settle themselves on the owner's next sign-in this way. A plain email signup sets none of the new cart variables and that page reads as before. They are html-encoded on output and dropped from an incoming request, so a supplied copy cannot put text on the page. diff --git src/hg/hgLogin/hgLogin.c src/hg/hgLogin/hgLogin.c index a7d1aa73d01..d33f49ce175 100644 --- src/hg/hgLogin/hgLogin.c +++ src/hg/hgLogin/hgLogin.c @@ -392,39 +392,72 @@ /* redirect to hgLogin page with given parameter string */ { jsInlineF( "window.location ='%s?%s';\n" , hgLoginUrl, paramStr); } void displayActMailSuccess() /* display Activate mail success box */ { char *returnURL = getReturnToUrlForAttr(); hPrintf( "
" "\n" "

%s

", brwName); +/* Arriving here straight after a social sign-in is confusing on its own: the user asked to sign + * in, not to fill in a form, and gets told to go and read their mail. Say first what happened + * and which address it turns on. These are set only by the two flows that send a user here + * from a provider sign-in; a plain email signup sets none of them and the page reads as before. + * Everything below is either config or an address, both of which can reach the cart from a + * request, so encode all of it. */ +char *provider = cartUsualString(cart, "hgLogin_actMailProvider", ""); +char *address = cartUsualString(cart, "hgLogin_actMailTo", ""); +char *existingUser = cartUsualString(cart, "hgLogin_actMailUser", ""); +if (isNotEmpty(provider) && isNotEmpty(address)) + { + char *encProvider = htmlEncode(provider); + char *encAddress = htmlEncode(address); + if (isEmpty(existingUser)) + hPrintf("

You signed in with %s, and %s did not tell us an email address, so we " + "asked you for one. No %s account uses %s yet, so we are making a new " + "account for it. Confirming the address is the last step.

", + encProvider, encProvider, brwName, encAddress); + else + { + char *encUser = htmlEncode(existingUser); + hPrintf("

Your %s sign-in belongs to the %s account %s, but the address on " + "that account, %s, has never been confirmed. Confirm it once and %s will " + "sign you straight in from then on.

", + encProvider, brwName, encUser, encAddress, encProvider); + freeMem(encUser); + } + freeMem(encProvider); + freeMem(encAddress); + } hPrintf( "

A confirmation email has been sent to you. \n" "Please click the confirmation link in the email to activate your account.

" "

You may have to look in your spam folder for an email from genome-www@soe.ucsc.edu, " "especially if you use Microsoft Outlook or Hotmail.

" "\n" "

Return

", returnURL); cartRemove(cart, "hgLogin_email"); cartRemove(cart, "hgLogin_userName"); +cartRemove(cart, "hgLogin_actMailProvider"); +cartRemove(cart, "hgLogin_actMailTo"); +cartRemove(cart, "hgLogin_actMailUser"); } void sendActMailOut(char *email, char *subject, char *msg) /* send mail to email address */ { int result; result = mailViaPipeBounce(email, subject, msg, returnAddr); if (result == -1) { hPrintf( "

%s

", brwName); hPrintf( "

" @@ -2561,30 +2594,35 @@ pending.provider = provider; pending.subject = subject; pending.email = email; linkIdentity(conn, idx, &pending); clearPendingIdentity(); if (activateNow) { loginAndReturn(conn, user, idx); return; } /* Unconfirmed address: send the confirmation mail and say so, rather than signing the user in * and leaving a mail nobody has any reason to open. Activating is what makes the address usable * for signing in by email link and for linking a later social login, so it is worth a click. */ setupNewAccount(conn, email, user); +/* Tell the confirmation page what to explain. No user name here: this is a brand new account, + * which is the one thing that page cannot work out for itself. */ +cartSetString(cart, "hgLogin_actMailProvider", oauthProviderLabel(provider)); +cartSetString(cart, "hgLogin_actMailTo", email); +cartRemove(cart, "hgLogin_actMailUser"); cartRemove(cart, "hgLogin_email"); cartRemove(cart, "hgLogin_userName"); redirectToLoginPage("hgLogin.do.displayActMailSuccess=1"); } void chooseAccountPage(struct sqlConnection *conn) /* Ask the user which of several accounts sharing an email address to sign in to. Used by * two flows: OAuth (oauth_pending_* in the cart -> the chosen account is linked to the social * identity) and the passwordless email link (emailLogin_* in the cart -> just sign in). */ { char *provider = cartUsualString(cart, "oauth_pending_provider", ""); boolean emailMode = isEmpty(provider); if (emailMode && !emailLinkEnabled()) { // The email-link chooser must not run where passwordless login is switched off. @@ -2789,45 +2827,65 @@ if (n > 1) { setPendingIdentity(id); gbMembersFreeList(&matches); chooseAccountPage(conn); return; } struct gbMembers *linked = memberForIdentity(conn, id); if (linked != NULL) { linkIdentity(conn, linked->idx, id); /* The provider identity is proven, but the address on the account may not be: when the * provider released none (ORCID) the user typed it themselves, and completeAccount left the - * account unactivated until the mailed link is opened. Signing in here would make that mail - * pointless -- the user would simply click the provider button again and never confirm -- so - * send them back to their inbox instead. Mail a fresh link each time, because the first one - * expires after seven days and this is the only way to activate such an account. An install - * that cannot send mail never creates an unactivated account here, but guard anyway rather - * than leave the user with nothing to click. */ - if (!sameString(linked->accountActivated, "Y") && !sameWord(returnAddr, "NOEMAIL") - && isNotEmpty(linked->email)) + * account unactivated until the mailed link is opened. */ + if (!sameString(linked->accountActivated, "Y")) + { + if (isNotEmpty(id->email) && sameWord(id->email, linked->email)) { + /* ...but this time the provider handed us that very address, which is the same + * assurance a new signup through this provider gets (see oauthProviderEmail). So + * confirm it here and let the user in, rather than sending them to fetch a link + * proving something we have just been told. This also settles accounts left + * unactivated by an earlier release that asked for an address and took it on + * trust. */ + char query[512]; + sqlSafef(query, sizeof(query), + "UPDATE gbMembers SET accountActivated='Y', dateActivated=NOW(), " + "emailToken='', emailTokenExpires='' WHERE idx=%u", linked->idx); + sqlUpdate(conn, query); + } + else if (!sameWord(returnAddr, "NOEMAIL") && isNotEmpty(linked->email)) + { + /* The address on the account is still nobody's word but the user's, so signing in + * would make the confirmation mail pointless: they would click the provider button + * again and never confirm. Send them to their inbox, with a fresh link each time, + * because the first expires after seven days and for an account with no password + * this is the only way to activate it. An install that cannot send mail no longer + * creates such an account, but guard rather than leave the user nothing to click. */ setupNewAccount(conn, linked->email, linked->userName); + cartSetString(cart, "hgLogin_actMailProvider", oauthProviderLabel(id->provider)); + cartSetString(cart, "hgLogin_actMailTo", linked->email); + cartSetString(cart, "hgLogin_actMailUser", linked->userName); gbMembersFree(&linked); gbMembersFreeList(&matches); displayActMailSuccess(); return; } + } loginAndReturn(conn, linked->userName, linked->idx); gbMembersFree(&linked); gbMembersFreeList(&matches); return; } if (n == 1) { linkIdentity(conn, matches->idx, id); loginAndReturn(conn, matches->userName, matches->idx); gbMembersFreeList(&matches); return; } gbMembersFreeList(&matches); @@ -3079,30 +3137,31 @@ /* The cart variables holding the state of a login in flight are written by hgLogin and by * nothing else: the nonce and provider of an OAuth round trip, the pending identity behind * the account chooser, and the verified address behind the email-link chooser. The cart * takes CGI variables verbatim (loadCgiOverHash in hg/lib/cart.c), so a copy arriving with * the request would stand in for the copy we stored. Drop those before anything reads them; * a flow whose state is dropped fails closed and the user starts it again. Note that * excludeVars would not do this job: it governs what is saved at the end of a request, not * what is read during it. */ { static char *serverOwned[] = { "oauth_state", "oauth_provider", "oauth_pending_provider", "oauth_pending_subject", "oauth_pending_email", "oauth_pending_email_verified", "oauth_pending_name", "oauth_pending_time", "oauth_pending_sig", "emailLogin_email", "emailLogin_tokenMd5", + "hgLogin_actMailProvider", "hgLogin_actMailTo", "hgLogin_actMailUser", }; int i; for (i = 0; i < ArraySize(serverOwned); i++) if (cgiVarExists(serverOwned[i])) cartRemove(cart, serverOwned[i]); } void doMiddle(struct cart *theCart) /* Write the middle parts of the HTML page. * This routine sets up some globals and then * dispatches to the appropriate page-maker. */ { struct sqlConnection *conn = hConnectCentral(); // on mirrors, try to add the field 'recovEmail' to gbMembers. This may or may not work, depending on their config