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/oauthLogin.c src/hg/hgLogin/oauthLogin.c
index 801cf1b9d52..811ad168121 100644
--- src/hg/hgLogin/oauthLogin.c
+++ src/hg/hgLogin/oauthLogin.c
@@ -38,30 +38,41 @@
 
 static char *provCfg(char *name, char *field)
 /* Return hg.conf login.oauth.<name>.<field>, falling back to the older login.<name>.<field>. */
 {
 char key[256];
 safef(key, sizeof(key), "login.oauth.%s.%s", name, field);
 char *val = cfgOption(key);
 if (isEmpty(val))
     {
     safef(key, sizeof(key), "login.%s.%s", name, field);
     val = cfgOption(key);
     }
 return val;
 }
 
+static char *cfgTrim(char *name, char *field)
+/* Like provCfg, but with surrounding whitespace removed and NULL for empty.  A stray trailing
+ * space in an hg.conf value (e.g. on an issuer or endpoint) would otherwise corrupt the URLs
+ * built from it. */
+{
+char *val = provCfg(name, field);
+if (isEmpty(val))
+    return NULL;
+return trimSpaces(cloneString(val));
+}
+
 static void fillBuiltinDefaults(struct oauthProvider *p)
 /* For well-known provider names, fill in type/label/endpoints/scopes that were not set
  * explicitly in hg.conf. */
 {
 if (sameWord(p->name, "google"))
     {
     if (isEmpty(p->type))        p->type = "oidc";
     if (isEmpty(p->label))       p->label = "Google";
     if (isEmpty(p->authUrl))     p->authUrl = "https://accounts.google.com/o/oauth2/v2/auth";
     if (isEmpty(p->tokenUrl))    p->tokenUrl = "https://oauth2.googleapis.com/token";
     if (isEmpty(p->userinfoUrl)) p->userinfoUrl = "https://openidconnect.googleapis.com/v1/userinfo";
     if (isEmpty(p->scopes))      p->scopes = "openid email profile";
     }
 else if (sameWord(p->name, "orcid"))
     {
@@ -77,39 +88,39 @@
     if (isEmpty(p->type))        p->type = "github";
     if (isEmpty(p->label))       p->label = "GitHub";
     if (isEmpty(p->authUrl))     p->authUrl = "https://github.com/login/oauth/authorize";
     if (isEmpty(p->tokenUrl))    p->tokenUrl = "https://github.com/login/oauth/access_token";
     if (isEmpty(p->userinfoUrl)) p->userinfoUrl = "https://api.github.com/user";
     if (isEmpty(p->scopes))      p->scopes = "read:user user:email";
     }
 }
 
 static struct oauthProvider *newProvider(char *name)
 /* Build a provider from its hg.conf block, or NULL if clientId/clientSecret are missing. */
 {
 struct oauthProvider *p;
 AllocVar(p);
 p->name = cloneString(name);
-p->label = cloneString(provCfg(name, "label"));
-p->type = cloneString(provCfg(name, "type"));
-p->clientId = cloneString(provCfg(name, "clientId"));
-p->clientSecret = cloneString(provCfg(name, "clientSecret"));
-p->authUrl = cloneString(provCfg(name, "authUrl"));
-p->tokenUrl = cloneString(provCfg(name, "tokenUrl"));
-p->userinfoUrl = cloneString(provCfg(name, "userinfoUrl"));
-p->scopes = cloneString(provCfg(name, "scopes"));
-p->issuer = cloneString(provCfg(name, "issuer"));
+p->label = cfgTrim(name, "label");
+p->type = cfgTrim(name, "type");
+p->clientId = cfgTrim(name, "clientId");
+p->clientSecret = cfgTrim(name, "clientSecret");
+p->authUrl = cfgTrim(name, "authUrl");
+p->tokenUrl = cfgTrim(name, "tokenUrl");
+p->userinfoUrl = cfgTrim(name, "userinfoUrl");
+p->scopes = cfgTrim(name, "scopes");
+p->issuer = cfgTrim(name, "issuer");
 fillBuiltinDefaults(p);
 if (isEmpty(p->type))
     p->type = "oidc";
 if (isEmpty(p->label))
     p->label = p->name;
 if (isEmpty(p->scopes))
     p->scopes = "openid email profile";
 if (isEmpty(p->clientId) || isEmpty(p->clientSecret))
     return NULL;
 return p;
 }
 
 static void addProviderName(struct slName **pList, char *name)
 /* Append name to the list if not already present and not blank. */
 {
@@ -286,31 +297,35 @@
 }
 
 static void ensureEndpoints(struct oauthProvider *p)
 /* For an OIDC provider configured with only an issuer, fetch the discovery document once and
  * fill in any endpoints that were not set explicitly. */
 {
 if (p->discovered || !sameWord(p->type, "oidc") || isEmpty(p->issuer))
     return;
 p->discovered = TRUE;
 if (isNotEmpty(p->authUrl) && isNotEmpty(p->tokenUrl) && isNotEmpty(p->userinfoUrl))
     return;
 char url[1024];
 safef(url, sizeof(url), "%s/.well-known/openid-configuration", p->issuer);
 struct jsonElement *j = jsonParseSafe(httpRequest(url, "GET", "Accept: application/json\r\n", NULL));
 if (j == NULL)
+    {
+    fprintf(stderr, "hgLogin oauth: OIDC discovery failed for provider '%s' at %s "
+        "(check login.oauth.%s.issuer)\n", p->name, url, p->name);
     return;
+    }
 if (isEmpty(p->authUrl))
     p->authUrl = cloneString(jsonOptionalStringField(j, "authorization_endpoint", NULL));
 if (isEmpty(p->tokenUrl))
     p->tokenUrl = cloneString(jsonOptionalStringField(j, "token_endpoint", NULL));
 if (isEmpty(p->userinfoUrl))
     p->userinfoUrl = cloneString(jsonOptionalStringField(j, "userinfo_endpoint", NULL));
 }
 
 char *oauthLoginUrl(char *name, char *redirectUri, char *state)
 /* Return the provider's authorization URL to redirect the browser to, or NULL. */
 {
 struct oauthProvider *p = providerByName(name);
 if (p == NULL)
     return NULL;
 ensureEndpoints(p);