OIDC xds translation (#2191)

* OIDC xds translation Signed-off-by: huabing zhao <zhaohuabing@gmail.com> * refactor: merge shared codes Signed-off-by: huabing zhao <zhaohuabing@gmail.com> * put oauth2 filter before jwtauthFilter, so we can use jwtauthfilter to check the oauth2 tokens Signed-off-by: huabing zhao <zhaohuabing@gmail.com> * add comments to explain per-route configuration for filter types that have non-native per-route support Signed-off-by: huabing zhao <zhaohuabing@gmail.com> --------- Signed-off-by: huabing zhao <zhaohuabing@gmail.com>
envoyproxy · Nov 17, 2023 · 524f7e3 · 524f7e3
1 parent 057e41b
commit 524f7e3
Show file tree

Hide file tree

Showing 22 changed files with 1,114 additions and 184 deletions.
diff --git a/internal/gatewayapi/securitypolicy.go b/internal/gatewayapi/securitypolicy.go
@@ -8,8 +8,11 @@ package gatewayapi
 import (
 	"encoding/json"
 	"fmt"
+	"net"
 	"net/http"
+	"net/url"
 	"sort"
+	"strconv"
 	"strings"
 
 	v1 "k8s.io/api/core/v1"
@@ -416,6 +419,7 @@ func (t *Translator) buildOIDC(
 	var (
 		oidc         = policy.Spec.OIDC
 		clientSecret *v1.Secret
+		provider     *ir.OIDCProvider
 		err          error
 	)
 
@@ -438,11 +442,13 @@ func (t *Translator) buildOIDC(
 
 	// Discover the token and authorization endpoints from the issuer's
 	// well-known url if not explicitly specified
-	provider := oidc.Provider.DeepCopy()
-	if err := discoverEndpointsFromIssuer(provider); err != nil {
+	if provider, err = discoverEndpointsFromIssuer(&oidc.Provider); err != nil {
 		return nil, err
 	}
 
+	if err := validateTokenEndpoint(provider.TokenEndpoint); err != nil {
+		return nil, err
+	}
 	scopes := appendOpenidScopeIfNotExist(oidc.Scopes)
 
 	return &ir.OIDC{
@@ -479,16 +485,22 @@ type OpenIDConfig struct {
 
 // discoverEndpointsFromIssuer discovers the token and authorization endpoints from the issuer's well-known url
 // return error if failed to fetch the well-known configuration
-func discoverEndpointsFromIssuer(provider *egv1a1.OIDCProvider) error {
+func discoverEndpointsFromIssuer(provider *egv1a1.OIDCProvider) (*ir.OIDCProvider, error) {
 	if provider.TokenEndpoint == nil || provider.AuthorizationEndpoint == nil {
 		tokenEndpoint, authorizationEndpoint, err := fetchEndpointsFromIssuer(provider.Issuer)
 		if err != nil {
-			return fmt.Errorf("error fetching endpoints from issuer: %w", err)
+			return nil, fmt.Errorf("error fetching endpoints from issuer: %w", err)
 		}
-		provider.TokenEndpoint = &tokenEndpoint
-		provider.AuthorizationEndpoint = &authorizationEndpoint
+		return &ir.OIDCProvider{
+			TokenEndpoint:         tokenEndpoint,
+			AuthorizationEndpoint: authorizationEndpoint,
+		}, nil
 	}
-	return nil
+
+	return &ir.OIDCProvider{
+		TokenEndpoint:         *provider.TokenEndpoint,
+		AuthorizationEndpoint: *provider.AuthorizationEndpoint,
+	}, nil
 }
 
 func fetchEndpointsFromIssuer(issuerURL string) (string, string, error) {
@@ -508,3 +520,29 @@ func fetchEndpointsFromIssuer(issuerURL string) (string, string, error) {
 
 	return config.TokenEndpoint, config.AuthorizationEndpoint, nil
 }
+
+// validateTokenEndpoint validates the token endpoint URL
+func validateTokenEndpoint(tokenEndpoint string) error {
+	parsedURL, err := url.Parse(tokenEndpoint)
+	if err != nil {
+		return fmt.Errorf("error parsing token endpoint URL: %w", err)
+	}
+
+	if parsedURL.Scheme != "https" {
+		return fmt.Errorf("token endpoint URL scheme must be https: %s", tokenEndpoint)
+	}
+
+	if ip := net.ParseIP(parsedURL.Hostname()); ip != nil {
+		if v4 := ip.To4(); v4 != nil {
+			return fmt.Errorf("token endpoint URL must be a domain name: %s", tokenEndpoint)
+		}
+	}
+
+	if parsedURL.Port() != "" {
+		_, err = strconv.Atoi(parsedURL.Port())
+		if err != nil {
+			return fmt.Errorf("error parsing token endpoint URL port: %w", err)
+		}
+	}
+	return nil
+}
diff --git a/internal/gatewayapi/testdata/securitypolicy-with-oidc.out.yaml b/internal/gatewayapi/testdata/securitypolicy-with-oidc.out.yaml
@@ -286,7 +286,6 @@ xdsIR:
           clientSecret: Y2xpZW50MTpzZWNyZXQK
           provider:
             authorizationEndpoint: https://oauth.foo.com/oauth2/v2/auth
-            issuer: https://oauth.foo.com
             tokenEndpoint: https://oauth.foo.com/token
           scopes:
           - openid
@@ -314,7 +313,6 @@ xdsIR:
           clientSecret: Y2xpZW50MTpzZWNyZXQK
           provider:
             authorizationEndpoint: https://accounts.google.com/o/oauth2/v2/auth
-            issuer: https://accounts.google.com
             tokenEndpoint: https://oauth2.googleapis.com/token
           scopes:
           - openid
@@ -340,7 +338,6 @@ xdsIR:
           clientSecret: Y2xpZW50MTpzZWNyZXQK
           provider:
             authorizationEndpoint: https://oauth.bar.com/oauth2/v2/auth
-            issuer: https://oauth.bar.com
             tokenEndpoint: https://oauth.bar.com/token
           scopes:
           - openid
diff --git a/internal/ir/xds.go b/internal/ir/xds.go
@@ -335,7 +335,7 @@ type JWT struct {
 // +k8s:deepcopy-gen=true
 type OIDC struct {
 	// The OIDC Provider configuration.
-	Provider egv1a1.OIDCProvider `json:"provider" yaml:"provider"`
+	Provider OIDCProvider `json:"provider" yaml:"provider"`
 
 	// The OIDC client ID to be used in the
 	// [Authentication Request](https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest).
@@ -353,6 +353,14 @@ type OIDC struct {
 	Scopes []string `json:"scopes,omitempty" yaml:"scopes,omitempty"`
 }
 
+type OIDCProvider struct {
+	// The OIDC Provider's [authorization endpoint](https://openid.net/specs/openid-connect-core-1_0.html#AuthorizationEndpoint).
+	AuthorizationEndpoint string `json:"authorizationEndpoint,omitempty"`
+
+	// The OIDC Provider's [token endpoint](https://openid.net/specs/openid-connect-core-1_0.html#TokenEndpoint).
+	TokenEndpoint string `json:"tokenEndpoint,omitempty"`
+}
+
 // Validate the fields within the HTTPRoute structure
 func (h HTTPRoute) Validate() error {
 	var errs error

diff --git a/internal/ir/zz_generated.deepcopy.go b/internal/ir/zz_generated.deepcopy.go
diff --git a/internal/xds/translator/accesslog.go b/internal/xds/translator/accesslog.go
@@ -250,7 +250,7 @@ func processClusterForAccessLog(tCtx *types.ResourceVersionTable, al *ir.AccessL
 			name:         clusterName,
 			settings:     []*ir.DestinationSetting{ds},
 			tSocket:      nil,
-			endpointType: DefaultEndpointType,
+			endpointType: EndpointTypeDNS,
 		}); err != nil && !errors.Is(err, ErrXdsClusterExists) {
 			return err
 		}

diff --git a/internal/xds/translator/cluster.go b/internal/xds/translator/cluster.go
@@ -43,7 +43,7 @@ func buildXdsCluster(args *xdsClusterArgs) *clusterv3.Cluster {
 		cluster.TransportSocket = args.tSocket
 	}
 
-	if args.endpointType == Static {
+	if args.endpointType == EndpointTypeStatic {
 		cluster.ClusterDiscoveryType = &clusterv3.Cluster_Type{Type: clusterv3.Cluster_EDS}
 		cluster.EdsClusterConfig = &clusterv3.Cluster_EdsClusterConfig{
 			ServiceName: args.name,

diff --git a/internal/xds/translator/cluster_test.go b/internal/xds/translator/cluster_test.go
@@ -31,7 +31,7 @@ func TestBuildXdsCluster(t *testing.T) {
 	args := &xdsClusterArgs{
 		name:         bootstrapXdsCluster.Name,
 		tSocket:      bootstrapXdsCluster.TransportSocket,
-		endpointType: DefaultEndpointType,
+		endpointType: EndpointTypeDNS,
 	}
 	dynamicXdsCluster := buildXdsCluster(args)
 

diff --git a/internal/xds/translator/cors.go b/internal/xds/translator/cors.go
@@ -88,9 +88,9 @@ func listenerContainsCORS(irListener *ir.HTTPListener) bool {
 	return false
 }
 
-// patchRouteWithCORSConfig patches the provided route with the CORS config if
+// patchRouteWithCORS patches the provided route with the CORS config if
 // applicable.
-func patchRouteWithCORSConfig(route *routev3.Route, irRoute *ir.HTTPRoute) error {
+func patchRouteWithCORS(route *routev3.Route, irRoute *ir.HTTPRoute) error {
 	if route == nil {
 		return errors.New("xds route is nil")
 	}

diff --git a/internal/xds/translator/httpfilters.go b/internal/xds/translator/httpfilters.go
@@ -7,6 +7,7 @@ package translator
 
 import (
 	"sort"
+	"strings"
 
 	routev3 "github.com/envoyproxy/go-control-plane/envoy/config/route/v3"
 	hcmv3 "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/network/http_connection_manager/v3"
@@ -37,14 +38,16 @@ func newOrderedHTTPFilter(filter *hcmv3.HttpFilter) *OrderedHTTPFilter {
 	order := 50
 
 	// Set a rational order for all the filters.
-	switch filter.Name {
-	case wellknown.CORS:
+	switch {
+	case filter.Name == wellknown.CORS:
 		order = 1
-	case jwtAuthnFilter:
+	case isOAuth2Filter(filter):
 		order = 2
-	case wellknown.HTTPRateLimit:
+	case filter.Name == jwtAuthnFilter:
 		order = 3
-	case wellknown.Router:
+	case filter.Name == wellknown.HTTPRateLimit:
+		order = 4
+	case filter.Name == wellknown.Router:
 		order = 100
 	}
 
@@ -110,6 +113,11 @@ func (t *Translator) patchHCMWithFilters(
 		return err
 	}
 
+	// Add oauth2 filters, if needed.
+	if err := patchHCMWithOAuth2Filters(mgr, irListener); err != nil {
+		return err
+	}
+
 	// Add the router filter
 	mgr.HttpFilters = append(mgr.HttpFilters, xdsfilters.HTTPRouter)
 
@@ -118,8 +126,29 @@ func (t *Translator) patchHCMWithFilters(
 	return nil
 }
 
-// patchRouteWithFilters appends per-route filter configurations to the route.
-func patchRouteWithFilters(
+// patchRouteCfgWithPerRouteConfig appends per-route filter configurations to the
+// route config.
+// This is a generic way to add per-route filter configurations for all filters
+// that has none-native per-route configuration support.
+//   - For the filter type that without native per-route configuration support, EG
+//     adds a filter for each route in the HCM filter chain.
+//   - patchRouteCfgWithPerRouteConfig disables all the filters in the
+//     typedFilterConfig of the route config.
+//   - PatchRouteWithPerRouteConfig enables the corresponding oauth2 filter for each
+//     route in the typedFilterConfig of the route.
+//
+// The filter types that have non-native per-route support: oauth2, basic authn
+// Note: The filter types that have native per-route configuration support should
+// use their own native per-route configuration.
+func patchRouteCfgWithPerRouteConfig(
+	routeCfg *routev3.RouteConfiguration,
+	irListener *ir.HTTPListener) error {
+	// Only supports the oauth2 filter for now, other filters will be added later.
+	return patchRouteCfgWithOAuth2Filter(routeCfg, irListener)
+}
+
+// patchRouteWithPerRouteConfig appends per-route filter configurations to the route.
+func patchRouteWithPerRouteConfig(
 	route *routev3.Route,
 	irRoute *ir.HTTPRoute) error {
 	// TODO: Convert this into a generic interface for API Gateway features.
@@ -130,14 +159,26 @@ func patchRouteWithFilters(
 	}
 
 	// Add the cors per route config to the route, if needed.
-	if err := patchRouteWithCORSConfig(route, irRoute); err != nil {
+	if err := patchRouteWithCORS(route, irRoute); err != nil {
 		return err
 	}
 
 	// Add the jwt per route config to the route, if needed.
-	if err := patchRouteWithJWTConfig(route, irRoute); err != nil {
+	if err := patchRouteWithJWT(route, irRoute); err != nil {
+		return err
+	}
+
+	// Add the oauth2 per route config to the route, if needed.
+	if err := patchRouteWithOAuth2(route, irRoute); err != nil {
 		return err
 	}
 
 	return nil
 }
+
+// isOAuth2Filter returns true if the provided filter is an OAuth2 filter.
+func isOAuth2Filter(filter *hcmv3.HttpFilter) bool {
+	// Multiple oauth2 filters are added to the HCM filter chain, one for each
+	// route. The oauth2 filter name is prefixed with "envoy.filters.http.oauth2".
+	return strings.HasPrefix(filter.Name, oauth2Filter)
+}