c465f5ce00f640497ff3f0b607552d7701bfa758 max Tue Aug 4 06:44:15 2026 -0700 hgLogin: show provider OAuth errors on the login page instead of falling through to the signup page; trim whitespace in oauth config values and log OIDC discovery failures; document CILogon/LS-AAI issuer URLs in mirrorManual. refs #37984 diff --git src/hg/hgLogin/hgLogin.c src/hg/hgLogin/hgLogin.c index 4ff08174b69..c5fcbd30535 100644 --- src/hg/hgLogin/hgLogin.c +++ src/hg/hgLogin/hgLogin.c @@ -2072,34 +2072,44 @@ displayLoginPage(conn); return; } jsInlineF("window.location = '%s';\n", url); } void oauthReturn(struct sqlConnection *conn) /* Handle the provider's redirect back to us: verify state, exchange the code for the * user's identity, and resolve/auto-link the account. */ { char *state = cgiUsualString("state", ""); char *savedState = cartUsualString(cart, "oauth_state", ""); char *provider = cartUsualString(cart, "oauth_provider", ""); cartRemove(cart, "oauth_state"); // one-time use -if (isNotEmpty(cgiUsualString("error", ""))) - { +char *errParam = cgiUsualString("error", ""); +if (isNotEmpty(errParam)) + { + // The provider redirected back with an OAuth error instead of a code (e.g. the user + // declined, or the client is misconfigured/unapproved). Show its message rather than + // silently falling through to another page. + char *desc = cgiUsualString("error_description", ""); + struct dyString *dy = dyStringNew(256); + dyStringAppend(dy, "Social login failed. "); + if (isNotEmpty(desc)) + dyStringPrintf(dy, "%s ", htmlEncode(desc)); + dyStringPrintf(dy, "(%s)", htmlEncode(errParam)); freez(&errMsg); - errMsg = cloneString("Social login was cancelled or denied."); + errMsg = dyStringCannibalize(&dy); displayLoginPage(conn); return; } if (isEmpty(state) || isEmpty(savedState) || differentString(state, savedState)) { freez(&errMsg); errMsg = cloneString("Your login session expired or was invalid. Please try again."); displayLoginPage(conn); return; } char *code = cgiUsualString("code", ""); struct oauthIdentity *id = oauthFetchIdentity(provider, code, hgLoginUrl); if (id == NULL) { freez(&errMsg); @@ -2281,33 +2291,36 @@ autoUpgradeTableAddColumn(conn, "gbMembers", "loginToken", "varchar(255)", FALSE, "NULL"); if (sqlFieldIndex(conn, "gbMembers", "loginTokenExpires") == -1) autoUpgradeTableAddColumn(conn, "gbMembers", "loginTokenExpires", "DATETIME", FALSE, "NULL"); // table linking accounts to social (Google/ORCID) identities; only needed where OAuth is set up if (oauthAnyProviderEnabled()) createIdentityTable(conn); cart = theCart; safecpy(brwName,sizeof(brwName), browserName()); safecpy(brwAddr,sizeof(brwAddr), browserAddr()); safecpy(signature,sizeof(signature), mailSignature()); safecpy(returnAddr,sizeof(returnAddr), mailReturnAddr()); pwdEyeIconEnabled = cfgOptionBooleanDefault(CFG_LOGIN_PWD_EYE_ICON, TRUE); -// A provider's OAuth redirect back to us carries 'code' and 'state' but none of our own -// hgLogin.do.* variables, so detect it up front. -if (cgiOptionalString("code") != NULL && cgiOptionalString("state") != NULL) +// A provider's OAuth redirect back to us carries 'code' (success) or 'error' (failure) but +// none of our own hgLogin.do.* variables, so detect it up front. We gate on an OAuth flow +// being in progress (oauth_provider set by oauthStart) so a stray code/error param can't +// trigger this. Error returns may omit 'code' and even 'state', so we must not require them. +if ((cgiOptionalString("code") != NULL || cgiOptionalString("error") != NULL) + && isNotEmpty(cartUsualString(cart, "oauth_provider", ""))) oauthReturn(conn); else if (cartVarExists(cart, "hgLogin.do.oauthStart")) oauthStart(conn); else if (cartVarExists(cart, "hgLogin.do.completeAccount")) completeAccount(conn); else if (cartVarExists(cart, "hgLogin.do.chooseAccount")) chooseAccount(conn); else if (cartVarExists(cart, "hgLogin.do.emailLinkPage")) emailLinkPage(conn); else if (cartVarExists(cart, "hgLogin.do.sendEmailLink")) sendEmailLink(conn); else if (cartVarExists(cart, "hgLogin.do.emailLogin")) emailLogin(conn); else if (cartVarExists(cart, "hgLogin.do.changePasswordPage")) changePasswordPage(conn);