[oidc-discovery-provider] Fix keys url (#5690)

* [oidc-discovery-provider] Fix keys url When jwt_issuer is specified, it is overriding the jwks key url in addition to the issuer property. This may cause the subsequent key retrieval to hit the wrong server, or fail if that server doesn't actually exist. Signed-off-by: Kevin Fox <[email protected]> * Update tests Signed-off-by: Kevin Fox <[email protected]> * Add advertised_url support Signed-off-by: Kevin Fox <[email protected]> * Simplify change Signed-off-by: Kevin Fox <[email protected]> * Revert domain name check Signed-off-by: Kevin Fox <[email protected]> * Add prefix support Signed-off-by: Kevin Fox <[email protected]> * Cleanup Signed-off-by: Kevin Fox <[email protected]> * Fix typo Signed-off-by: Kevin Fox <[email protected]> * Update names after feedback Signed-off-by: Kevin Fox <[email protected]> * Fix typo Signed-off-by: Kevin Fox <[email protected]> * Incorperate feedback Signed-off-by: Kevin Fox <[email protected]> * Incorperate feedback Signed-off-by: Kevin Fox <[email protected]> * Fix tests Signed-off-by: Kevin Fox <[email protected]> * Fix test Signed-off-by: Kevin Fox <[email protected]> * Incorperate feedback Signed-off-by: Kevin Fox <[email protected]> * Incorperate feedback Signed-off-by: Kevin Fox <[email protected]> * Incorperate feedback Signed-off-by: Kevin Fox <[email protected]> * Add test for compat behavior. Fix lint. Signed-off-by: Kevin Fox <[email protected]> * Fix lint Signed-off-by: Kevin Fox <[email protected]> * Update docs Signed-off-by: Kevin Fox <[email protected]> * Incorperate feedback Signed-off-by: Kevin Fox <[email protected]> * Fix lint issue Signed-off-by: Kevin Fox <[email protected]> --------- Signed-off-by: Kevin Fox <[email protected]> Co-authored-by: Marcos Yacob <[email protected]>
spiffe · Feb 6, 2025 · 03f3db9 · 03f3db9
1 parent f120f0a
commit 03f3db9
Show file tree

Hide file tree

Showing 5 changed files with 431 additions and 32 deletions.
diff --git a/support/oidc-discovery-provider/README.md b/support/oidc-discovery-provider/README.md
@@ -13,6 +13,9 @@ It provides the following endpoints:
 | `GET` | `/ready`                            | Returns http.OK (200) as soon as requests can be served. (disabled by default)                                          |
 | `GET` | `/live`                             | Returns http.OK (200) as soon as a keyset is available, otherwise http.InternalServerError (500). (disabled by default) |
 
+The endpoints can be moved to a different prefix by way of the `server_path_prefix` option. For example, setting server_path_prefix to `/instance/1` will make
+the OIDC discovery document served at `/instance/1/.well-known/openid-configuration` and keys at `/instance/1/keys`
+
 The provider by default relies on ACME to obtain TLS certificates that it uses to
 serve the documents securely.
 
@@ -49,6 +52,8 @@ The configuration file is **required** by the provider. It contains
 | `workload_api`          | section | required\[2\]      | Provides Workload API details.                                         |          |
 | `health_checks`         | section | optional           | Enable and configure health check endpoints                            |          |
 | `jwt_issuer`            | string  | optional           | Specifies the issuer for the OIDC provider configuration request       |          |
+| `jwks_uri`              | string  | optional           | Specifies the JWKS URI returned in the discovery document              |          |
+| `server_path_prefix`    | string  | optional           | If specified, all endpoints listened to will be prefixed by this value | `"/"`    |
 
 | experimental             | Type   | Required?          | Description                                          | Default |
 |--------------------------|--------|--------------------|------------------------------------------------------|---------|

diff --git a/support/oidc-discovery-provider/config.go b/support/oidc-discovery-provider/config.go
@@ -81,6 +81,15 @@ type Config struct {
 
 	// JWTIssuer specifies the issuer for the OIDC provider configuration request.
 	JWTIssuer string `hcl:"jwt_issuer"`
+
+	// JWKSURI specifies the absolute uri to the jwks keys document. Use this if you are fronting the
+	// discovery provider with a load balancer or reverse proxy
+	JWKSURI string `hcl:"jwks_uri"`
+
+	// ServerPathPrefix specifies the prefix to strip from the path of requests to route to the server.
+	// Example: if ServerPathPrefix is /foo then a request to http://127.0.0.1/foo/.well-known/openid-configuration and
+	// http://127.0.0.1/foo/keys will function with the server.
+	ServerPathPrefix string `hcl:"server_path_prefix"`
 }
 
 type ServingCertFileConfig struct {
@@ -308,6 +317,15 @@ func ParseConfig(hclConfig string) (_ *Config, err error) {
 			return nil, errors.New("the jwt_issuer url must contain a host")
 		}
 	}
+	if c.JWKSURI != "" {
+		jwksURI, err := url.Parse(c.JWKSURI)
+		if err != nil || jwksURI.Scheme == "" || jwksURI.Host == "" {
+			return nil, fmt.Errorf("the jwks_uri setting could not be parsed: %w", err)
+		}
+	}
+	if c.JWKSURI == "" && c.JWTIssuer != "" {
+		fmt.Printf("Warning: The jwt_issuer configuration will also affect the jwks_uri behavior when jwks_url is not set. This behaviour will be changed in 1.13.0.")
+	}
 	return c, nil
 }
 

diff --git a/support/oidc-discovery-provider/handler.go b/support/oidc-discovery-provider/handler.go
@@ -24,24 +24,40 @@ type Handler struct {
 	allowInsecureScheme bool
 	setKeyUse           bool
 	log                 logrus.FieldLogger
-	jwtIssuer           string
+	jwtIssuer           *url.URL
+	jwksURI             *url.URL
+	serverPathPrefix    string
 
 	http.Handler
 }
 
-func NewHandler(log logrus.FieldLogger, domainPolicy DomainPolicy, source JWKSSource, allowInsecureScheme bool, setKeyUse bool, jwtIssuer string) *Handler {
+func NewHandler(log logrus.FieldLogger, domainPolicy DomainPolicy, source JWKSSource, allowInsecureScheme bool, setKeyUse bool, jwtIssuer *url.URL, jwksURI *url.URL, serverPathPrefix string) *Handler {
+	if serverPathPrefix == "" {
+		serverPathPrefix = "/"
+	}
 	h := &Handler{
 		domainPolicy:        domainPolicy,
 		source:              source,
 		allowInsecureScheme: allowInsecureScheme,
 		setKeyUse:           setKeyUse,
 		log:                 log,
 		jwtIssuer:           jwtIssuer,
+		jwksURI:             jwksURI,
+		serverPathPrefix:    serverPathPrefix,
 	}
 
 	mux := http.NewServeMux()
-	mux.Handle("/.well-known/openid-configuration", handlers.ProxyHeaders(http.HandlerFunc(h.serveWellKnown)))
-	mux.Handle("/keys", http.HandlerFunc(h.serveKeys))
+	wkPath, err := url.JoinPath(serverPathPrefix, "/.well-known/openid-configuration")
+	if err != nil {
+		return nil
+	}
+	jwksPath, err := url.JoinPath(serverPathPrefix, "/keys")
+	if err != nil {
+		return nil
+	}
+
+	mux.Handle(wkPath, handlers.ProxyHeaders(http.HandlerFunc(h.serveWellKnown)))
+	mux.Handle(jwksPath, http.HandlerFunc(h.serveKeys))
 
 	h.Handler = mux
 	return h
@@ -53,34 +69,55 @@ func (h *Handler) serveWellKnown(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	var host string
-	var path string
-	var urlScheme string
-	if h.jwtIssuer != "" {
-		jwtIssuerURL, _ := url.Parse(h.jwtIssuer)
-		host = jwtIssuerURL.Host
-		path = jwtIssuerURL.Path
-		urlScheme = jwtIssuerURL.Scheme
-	} else {
-		host = r.Host
-		urlScheme = "https"
-		if h.allowInsecureScheme && r.TLS == nil && r.URL.Scheme != "https" {
-			urlScheme = "http"
-		}
+	urlScheme := "https"
+	if h.allowInsecureScheme && r.TLS == nil && r.URL.Scheme != "https" {
+		urlScheme = "http"
 	}
 
-	if err := h.verifyHost(host); err != nil {
-		http.Error(w, err.Error(), http.StatusBadRequest)
-		return
+	issuerURL := h.jwtIssuer
+	if h.jwtIssuer == nil {
+		issuerURL = &url.URL{
+			Scheme: urlScheme,
+			Host:   r.Host,
+		}
+		if h.serverPathPrefix != "/" {
+			issuerURL.Path = h.serverPathPrefix
+		}
 	}
 
-	issuerURL := url.URL{
-		Scheme: urlScheme,
-		Host:   host,
-		Path:   path,
+	var jwksURI *url.URL
+	switch {
+	case h.jwksURI != nil:
+		jwksURI = h.jwksURI
+	case h.jwtIssuer != nil:
+		// If jwksIsser is set but not jwksURI, fall back to 1.11.1 behavior until we can remove jwksIssuer leaking into jwksURI in 1.13.0
+		keysPath, err := url.JoinPath(h.jwtIssuer.Path, "keys")
+		if err != nil {
+			http.Error(w, err.Error(), http.StatusInternalServerError)
+			return
+		}
+		jwksURI = &url.URL{
+			Scheme: h.jwtIssuer.Scheme,
+			Host:   h.jwtIssuer.Host,
+			Path:   keysPath,
+		}
+	default:
+		keysPath, err := url.JoinPath(h.serverPathPrefix, "keys")
+		if err != nil {
+			http.Error(w, err.Error(), http.StatusInternalServerError)
+			return
+		}
+		jwksURI = &url.URL{
+			Scheme: urlScheme,
+			Host:   r.Host,
+			Path:   keysPath,
+		}
 	}
 
-	jwksURI := issuerURL.JoinPath("keys")
+	if err := h.verifyHost(r.Host); err != nil {
+		http.Error(w, err.Error(), http.StatusBadRequest)
+		return
+	}
 
 	doc := struct {
 		Issuer  string `json:"issuer"`