fix: address review feedback for auth server URL rewriting

- Handle url.Parse error in authorization redirect instead of ignoring
- Move rewriteMetadataURLs inside authorizationProxyEnabled guard so
  URLs are only rewritten when the proxy routes exist
- Restrict metadata URL rewriting to known endpoint fields (allowlist)
  to avoid rewriting issuer which would break token validation
- Use URL parsing for scheme/host comparison instead of string prefix
  matching to prevent false matches on similar hostnames
- Use url.JoinPath for JWKS URI construction and return errors for
  unparseable URIs instead of silently falling back
- Validate paths start with "/" before registering with ServeMux to
  prevent panics on relative URIs
- Add test case for relative URI rejection

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Author: salmon-21

oauth/authorization.go

@@ -26,7 +26,10 @@ func NewAuthorizationHandler(config *config.Config, meta map[string]any) (http.H
 		return nil, fmt.Errorf("could not parse authorization endpoint: %w", err)
 	} else {
 		// Rewrite the authorization endpoint to use the public host URL
-		publicURL, _ := url.Parse(config.Host.String())
+		publicURL, err := url.Parse(config.Host.String())
+		if err != nil {
+			return nil, fmt.Errorf("could not parse public host URL: %w", err)
+		}
 		publicURL.Path = authEndpointURL.Path
 
 		return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {

oauth/authorization_server_metadata.go

@@ -47,10 +47,12 @@ func NewAuthorizationServerMetadataHandler(config *config.Config) http.Handler {
 				metadata["authorization_endpoint"] = authorizationURI.String()
 				log.Get(r.Context()).Info("Adding authorization endpoint to authorization server metadata",
 					"url", metadata["authorization_endpoint"])
-			}
 
-			// Rewrite all URL fields pointing to internal auth server to public host
-			rewriteMetadataURLs(metadata, config.Authorization.Server, config.Host.String())
+				// Rewrite endpoint URL fields pointing to internal auth server
+				// to public host so external clients can reach them through the
+				// gateway's reverse proxy.
+				rewriteMetadataURLs(metadata, config.Authorization.Server, config.Host.String())
+			}
 
 			w.Header().Set("Content-Type", "application/json")
 			if err := json.NewEncoder(w).Encode(metadata); err != nil {
@@ -97,14 +99,48 @@ func GetMedatata(server string) (map[string]any, error) {
 	return nil, err
 }
 
-// rewriteMetadataURLs rewrites all string values in metadata that start with
-// the internal auth server URL to use the public host URL instead.
+// endpointURLFields lists the metadata fields that contain endpoint URLs
+// eligible for rewriting. Fields like "issuer" are intentionally excluded
+// because rewriting them would make the metadata inconsistent with the "iss"
+// claim in tokens issued by the auth server.
+var endpointURLFields = []string{
+	"authorization_endpoint",
+	"token_endpoint",
+	"jwks_uri",
+	"userinfo_endpoint",
+	"registration_endpoint",
+	"introspection_endpoint",
+	"revocation_endpoint",
+	"device_authorization_endpoint",
+	"end_session_endpoint",
+}
+
+// rewriteMetadataURLs rewrites known endpoint URL fields in metadata from the
+// internal auth server URL to the public host URL. Only fields whose
+// scheme and host match the internal server are rewritten.
 func rewriteMetadataURLs(metadata map[string]any, internalServer string, publicHost string) {
-	internal := strings.TrimRight(internalServer, "/")
-	public := strings.TrimRight(publicHost, "/")
-	for key, value := range metadata {
-		if s, ok := value.(string); ok && strings.HasPrefix(s, internal) {
-			metadata[key] = strings.Replace(s, internal, public, 1)
+	internalURL, err := url.Parse(strings.TrimRight(internalServer, "/"))
+	if err != nil || internalURL.Scheme == "" || internalURL.Host == "" {
+		return
+	}
+	publicURL, err := url.Parse(strings.TrimRight(publicHost, "/"))
+	if err != nil || publicURL.Scheme == "" || publicURL.Host == "" {
+		return
+	}
+
+	for _, key := range endpointURLFields {
+		s, ok := metadata[key].(string)
+		if !ok {
+			continue
+		}
+		u, err := url.Parse(s)
+		if err != nil || u.Scheme == "" || u.Host == "" {
+			continue
+		}
+		if u.Scheme == internalURL.Scheme && u.Host == internalURL.Host {
+			u.Scheme = publicURL.Scheme
+			u.Host = publicURL.Host
+			metadata[key] = u.String()
 		}
 	}
 }

oauth/oauth.go

@@ -55,9 +55,21 @@ func NewManager(ctx context.Context, config *config.Config) (*Manager, error) {
 	// circular dependency when the auth server's issuer is set to the
 	// public gateway URL (the metadata would advertise the gateway's own
 	// URL as jwks_uri, causing the gateway to fetch from itself).
-	internalJWKSURI := strings.TrimRight(config.Authorization.Server, "/") + "/keys"
-	if u, err := url.Parse(jwksURIRaw); err == nil {
-		internalJWKSURI = strings.TrimRight(config.Authorization.Server, "/") + u.Path
+	baseURL, err := url.Parse(config.Authorization.Server)
+	if err != nil {
+		return nil, fmt.Errorf("invalid authorization server URL %q: %w", config.Authorization.Server, err)
+	}
+	jwksURL, err := url.Parse(jwksURIRaw)
+	if err != nil {
+		return nil, fmt.Errorf("invalid jwks_uri %q: %w", jwksURIRaw, err)
+	}
+	jwksPath := jwksURL.Path
+	if jwksPath == "" || jwksPath == "/" {
+		jwksPath = "/keys"
+	}
+	internalJWKSURI, err := url.JoinPath(baseURL.String(), jwksPath)
+	if err != nil {
+		return nil, fmt.Errorf("failed to construct internal JWKS URI: %w", err)
 	}
 
 	if err := cache.Register(timeoutCtx, internalJWKSURI,
@@ -176,7 +188,7 @@ func authServerProxyPaths(meta map[string]any) []string {
 
 	for _, key := range endpointKeys {
 		if s, ok := meta[key].(string); ok {
-			if u, err := url.Parse(s); err == nil {
+			if u, err := url.Parse(s); err == nil && strings.HasPrefix(u.Path, "/") {
 				add(u.Path)
 			}
 		}
@@ -186,7 +198,7 @@ func authServerProxyPaths(meta map[string]any) []string {
 	// prefix pattern so that connector-specific sub-paths (e.g.
 	// /auth/github) are also proxied.
 	if s, ok := meta["authorization_endpoint"].(string); ok {
-		if u, err := url.Parse(s); err == nil {
+		if u, err := url.Parse(s); err == nil && strings.HasPrefix(u.Path, "/") {
 			add(u.Path)
 			add(strings.TrimRight(u.Path, "/") + "/")
 		}

oauth/oauth_test.go

@@ -60,6 +60,15 @@ func TestAuthServerProxyPaths(t *testing.T) {
 			},
 			contains: []string{"/keys", "/callback", "/approval"},
 		},
+		{
+			name: "skips relative URIs that would panic ServeMux",
+			meta: map[string]any{
+				"token_endpoint": "token",        // relative, no leading /
+				"jwks_uri":       "https://example.com/keys",
+			},
+			contains: []string{"/keys", "/callback", "/approval"},
+			excludes: []string{"token"},
+		},
 		{
 			name: "registers authorization endpoint prefix for sub-paths",
 			meta: map[string]any{