Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OIDC xds translation #2191

Merged
merged 4 commits into from
Nov 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 45 additions & 7 deletions internal/gatewayapi/securitypolicy.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,11 @@
import (
"encoding/json"
"fmt"
"net"
"net/http"
"net/url"
"sort"
"strconv"
"strings"

v1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -416,6 +419,7 @@
var (
oidc = policy.Spec.OIDC
clientSecret *v1.Secret
provider *ir.OIDCProvider
err error
)

Expand All @@ -438,11 +442,13 @@

// 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
}

Check warning on line 451 in internal/gatewayapi/securitypolicy.go

View check run for this annotation

Codecov / codecov/patch

internal/gatewayapi/securitypolicy.go#L450-L451

Added lines #L450 - L451 were not covered by tests
scopes := appendOpenidScopeIfNotExist(oidc.Scopes)

return &ir.OIDC{
Expand Down Expand Up @@ -479,16 +485,22 @@

// 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) {
Expand All @@ -508,3 +520,29 @@

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)
}

Check warning on line 529 in internal/gatewayapi/securitypolicy.go

View check run for this annotation

Codecov / codecov/patch

internal/gatewayapi/securitypolicy.go#L528-L529

Added lines #L528 - L529 were not covered by tests

if parsedURL.Scheme != "https" {
return fmt.Errorf("token endpoint URL scheme must be https: %s", tokenEndpoint)
}

Check warning on line 533 in internal/gatewayapi/securitypolicy.go

View check run for this annotation

Codecov / codecov/patch

internal/gatewayapi/securitypolicy.go#L532-L533

Added lines #L532 - L533 were not covered by tests

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)
}

Check warning on line 538 in internal/gatewayapi/securitypolicy.go

View check run for this annotation

Codecov / codecov/patch

internal/gatewayapi/securitypolicy.go#L536-L538

Added lines #L536 - L538 were not covered by tests
}

if parsedURL.Port() != "" {
_, err = strconv.Atoi(parsedURL.Port())
if err != nil {
return fmt.Errorf("error parsing token endpoint URL port: %w", err)
}

Check warning on line 545 in internal/gatewayapi/securitypolicy.go

View check run for this annotation

Codecov / codecov/patch

internal/gatewayapi/securitypolicy.go#L542-L545

Added lines #L542 - L545 were not covered by tests
}
return nil
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
10 changes: 9 additions & 1 deletion internal/ir/xds.go
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion internal/ir/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion internal/xds/translator/accesslog.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
2 changes: 1 addition & 1 deletion internal/xds/translator/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion internal/xds/translator/cluster_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ func TestBuildXdsCluster(t *testing.T) {
args := &xdsClusterArgs{
name: bootstrapXdsCluster.Name,
tSocket: bootstrapXdsCluster.TransportSocket,
endpointType: DefaultEndpointType,
endpointType: EndpointTypeDNS,
}
dynamicXdsCluster := buildXdsCluster(args)

Expand Down
4 changes: 2 additions & 2 deletions internal/xds/translator/cors.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
Expand Down
59 changes: 50 additions & 9 deletions internal/xds/translator/httpfilters.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

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"
Expand Down Expand Up @@ -37,14 +38,16 @@
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
}

Expand Down Expand Up @@ -110,6 +113,11 @@
return err
}

// Add oauth2 filters, if needed.
if err := patchHCMWithOAuth2Filters(mgr, irListener); err != nil {
return err
}

Check warning on line 119 in internal/xds/translator/httpfilters.go

View check run for this annotation

Codecov / codecov/patch

internal/xds/translator/httpfilters.go#L118-L119

Added lines #L118 - L119 were not covered by tests

// Add the router filter
mgr.HttpFilters = append(mgr.HttpFilters, xdsfilters.HTTPRouter)

Expand All @@ -118,8 +126,29 @@
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.
Expand All @@ -130,14 +159,26 @@
}

// 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
}

Check warning on line 169 in internal/xds/translator/httpfilters.go

View check run for this annotation

Codecov / codecov/patch

internal/xds/translator/httpfilters.go#L168-L169

Added lines #L168 - L169 were not covered by tests

// 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)
}
Loading