From 54dd858039b6ab7282528d054a2b5ad38d3a4898 Mon Sep 17 00:00:00 2001 From: Magnus Jungsbluth Date: Fri, 13 Mar 2026 09:07:01 +0100 Subject: [PATCH 1/7] feat: Enable hiding OIDC configuration parameters in profiles, including templating Signed-off-by: Magnus Jungsbluth Signed-off-by: magnus-jungsbluth_zse --- config/config.go | 94 ++++-- dataclients/kubernetes/annotations.go | 22 ++ dataclients/kubernetes/annotations_test.go | 158 +++++++++ dataclients/kubernetes/ingress.go | 4 + dataclients/kubernetes/ingressv1.go | 2 + dataclients/kubernetes/kube.go | 9 + dataclients/kubernetes/routegroup.go | 2 + docs/reference/filters.md | 110 +++++++ docs/tutorials/auth.md | 116 +++++++ filters/auth/oidc.go | 66 ++++ filters/auth/oidcprofile.go | 352 +++++++++++++++++++++ filters/auth/oidcprofile_test.go | 301 ++++++++++++++++++ skipper.go | 18 ++ 13 files changed, 1225 insertions(+), 29 deletions(-) create mode 100644 dataclients/kubernetes/annotations_test.go create mode 100644 filters/auth/oidcprofile.go create mode 100644 filters/auth/oidcprofile_test.go diff --git a/config/config.go b/config/config.go index 68de45d094..20b025b555 100644 --- a/config/config.go +++ b/config/config.go @@ -19,6 +19,7 @@ import ( "github.com/zalando/skipper" "github.com/zalando/skipper/dataclients/kubernetes" "github.com/zalando/skipper/eskip" + "github.com/zalando/skipper/filters/auth" "github.com/zalando/skipper/filters/openpolicyagent" "github.com/zalando/skipper/metrics" "github.com/zalando/skipper/net" @@ -187,6 +188,8 @@ type Config struct { KubernetesEastWestRangeAnnotationFiltersAppendString multiFlag `yaml:"kubernetes-east-west-range-annotation-filters-append"` KubernetesAnnotationPredicatesString multiFlag `yaml:"kubernetes-annotation-predicates"` KubernetesAnnotationFiltersAppendString multiFlag `yaml:"kubernetes-annotation-filters-append"` + KubernetesAnnotationsToRouteAnnotations *listFlag `yaml:"kubernetes-annotations-to-route-annotations"` + KubernetesAnnotationsToRouteAnnotationsPrefix string `yaml:"kubernetes-annotations-to-route-annotations-prefix"` KubernetesEastWestRangeAnnotationPredicates []kubernetes.AnnotationPredicates `yaml:"-"` KubernetesEastWestRangeAnnotationFiltersAppend []kubernetes.AnnotationFilters `yaml:"-"` KubernetesAnnotationPredicates []kubernetes.AnnotationPredicates `yaml:"-"` @@ -211,35 +214,37 @@ type Config struct { DefaultFiltersDir string `yaml:"default-filters-dir"` // Auth: - EnableOAuth2GrantFlow bool `yaml:"enable-oauth2-grant-flow"` - Oauth2AuthURL string `yaml:"oauth2-auth-url"` - Oauth2TokenURL string `yaml:"oauth2-token-url"` - Oauth2RevokeTokenURL string `yaml:"oauth2-revoke-token-url"` - Oauth2TokeninfoURL string `yaml:"oauth2-tokeninfo-url"` - Oauth2TokeninfoTimeout time.Duration `yaml:"oauth2-tokeninfo-timeout"` - Oauth2TokeninfoCacheSize int `yaml:"oauth2-tokeninfo-cache-size"` - Oauth2TokeninfoCacheTTL time.Duration `yaml:"oauth2-tokeninfo-cache-ttl"` - Oauth2SecretFile string `yaml:"oauth2-secret-file"` - Oauth2ClientID string `yaml:"oauth2-client-id"` - Oauth2ClientSecret string `yaml:"oauth2-client-secret"` - Oauth2ClientIDFile string `yaml:"oauth2-client-id-file"` - Oauth2ClientSecretFile string `yaml:"oauth2-client-secret-file"` - Oauth2AuthURLParameters mapFlags `yaml:"oauth2-auth-url-parameters"` - Oauth2CallbackPath string `yaml:"oauth2-callback-path"` - Oauth2TokenintrospectionTimeout time.Duration `yaml:"oauth2-tokenintrospect-timeout"` - Oauth2AccessTokenHeaderName string `yaml:"oauth2-access-token-header-name"` - Oauth2TokeninfoSubjectKey string `yaml:"oauth2-tokeninfo-subject-key"` - Oauth2GrantTokeninfoKeys *listFlag `yaml:"oauth2-grant-tokeninfo-keys"` - Oauth2TokenCookieName string `yaml:"oauth2-token-cookie-name"` - Oauth2TokenCookieRemoveSubdomains int `yaml:"oauth2-token-cookie-remove-subdomains"` - Oauth2GrantInsecure bool `yaml:"oauth2-grant-insecure"` - WebhookTimeout time.Duration `yaml:"webhook-timeout"` - OidcSecretsFile string `yaml:"oidc-secrets-file"` - OIDCCookieValidity time.Duration `yaml:"oidc-cookie-validity"` - OidcDistributedClaimsTimeout time.Duration `yaml:"oidc-distributed-claims-timeout"` - OIDCCookieRemoveSubdomains int `yaml:"oidc-cookie-remove-subdomains"` - CredentialPaths *listFlag `yaml:"credentials-paths"` - CredentialsUpdateInterval time.Duration `yaml:"credentials-update-interval"` + EnableOAuth2GrantFlow bool `yaml:"enable-oauth2-grant-flow"` + Oauth2AuthURL string `yaml:"oauth2-auth-url"` + Oauth2TokenURL string `yaml:"oauth2-token-url"` + Oauth2RevokeTokenURL string `yaml:"oauth2-revoke-token-url"` + Oauth2TokeninfoURL string `yaml:"oauth2-tokeninfo-url"` + Oauth2TokeninfoTimeout time.Duration `yaml:"oauth2-tokeninfo-timeout"` + Oauth2TokeninfoCacheSize int `yaml:"oauth2-tokeninfo-cache-size"` + Oauth2TokeninfoCacheTTL time.Duration `yaml:"oauth2-tokeninfo-cache-ttl"` + Oauth2SecretFile string `yaml:"oauth2-secret-file"` + Oauth2ClientID string `yaml:"oauth2-client-id"` + Oauth2ClientSecret string `yaml:"oauth2-client-secret"` + Oauth2ClientIDFile string `yaml:"oauth2-client-id-file"` + Oauth2ClientSecretFile string `yaml:"oauth2-client-secret-file"` + Oauth2AuthURLParameters mapFlags `yaml:"oauth2-auth-url-parameters"` + Oauth2CallbackPath string `yaml:"oauth2-callback-path"` + Oauth2TokenintrospectionTimeout time.Duration `yaml:"oauth2-tokenintrospect-timeout"` + Oauth2AccessTokenHeaderName string `yaml:"oauth2-access-token-header-name"` + Oauth2TokeninfoSubjectKey string `yaml:"oauth2-tokeninfo-subject-key"` + Oauth2GrantTokeninfoKeys *listFlag `yaml:"oauth2-grant-tokeninfo-keys"` + Oauth2TokenCookieName string `yaml:"oauth2-token-cookie-name"` + Oauth2TokenCookieRemoveSubdomains int `yaml:"oauth2-token-cookie-remove-subdomains"` + Oauth2GrantInsecure bool `yaml:"oauth2-grant-insecure"` + WebhookTimeout time.Duration `yaml:"webhook-timeout"` + OidcSecretsFile string `yaml:"oidc-secrets-file"` + OIDCCookieValidity time.Duration `yaml:"oidc-cookie-validity"` + OidcDistributedClaimsTimeout time.Duration `yaml:"oidc-distributed-claims-timeout"` + OIDCCookieRemoveSubdomains int `yaml:"oidc-cookie-remove-subdomains"` + OidcProfiles *map[string]auth.OidcProfile `yaml:"oidc-profiles"` + OidcProfilesFile string `yaml:"oidc-profiles-file"` + CredentialPaths *listFlag `yaml:"credentials-paths"` + CredentialsUpdateInterval time.Duration `yaml:"credentials-update-interval"` // TLS configuration for the validation webhook ValidationWebhookEnabled bool `yaml:"validation-webhook-enabled"` @@ -382,6 +387,7 @@ func NewConfig() *Config { cfg.CloneRoute = routeChangerConfig{} cfg.EditRoute = routeChangerConfig{} cfg.KubernetesEastWestRangeDomains = commaListFlag() + cfg.KubernetesAnnotationsToRouteAnnotations = commaListFlag() cfg.RoutesURLs = commaListFlag() cfg.ForwardedHeadersList = commaListFlag() cfg.ForwardedHeadersExcludeCIDRList = commaListFlag() @@ -547,6 +553,8 @@ func NewConfig() *Config { flag.StringVar(&cfg.KubernetesEastWestRangePredicatesString, "kubernetes-east-west-range-predicates", "", "set the predicates that will be appended to routes identified as to -kubernetes-east-west-range-domains") flag.Var(&cfg.KubernetesAnnotationPredicatesString, "kubernetes-annotation-predicates", "configures predicates appended to non east-west routes of annotated resources. E.g. -kubernetes-annotation-predicates='zone-a=true=Foo() && Bar()' will add 'Foo() && Bar()' predicates to all non east-west routes of ingress or routegroup annotated with 'zone-a: true'. For east-west routes use -kubernetes-east-west-range-annotation-predicates.") flag.Var(&cfg.KubernetesAnnotationFiltersAppendString, "kubernetes-annotation-filters-append", "configures filters appended to non east-west routes of annotated resources. E.g. -kubernetes-annotation-filters-append='zone-a=true=foo() -> bar()' will add 'foo() -> bar()' filters to all non east-west routes of ingress or routegroup annotated with 'zone-a: true'. For east-west routes use -kubernetes-east-west-range-annotation-filters-append.") + flag.Var(cfg.KubernetesAnnotationsToRouteAnnotations, "kubernetes-annotations-to-route-annotations", "comma-separated list of Kubernetes resource annotation keys whose values are automatically injected as annotate() filters into routes, making them available to oauthOidc* profile filters via {{index .Annotations \"key\"}}") + flag.StringVar(&cfg.KubernetesAnnotationsToRouteAnnotationsPrefix, "kubernetes-annotations-to-route-annotations-prefix", "", "prefix prepended to the key in annotate() filters generated from -kubernetes-annotations-to-route-annotations; no separator is added between prefix and key") flag.Var(&cfg.KubernetesEastWestRangeAnnotationPredicatesString, "kubernetes-east-west-range-annotation-predicates", "similar to -kubernetes-annotation-predicates configures predicates appended to east-west routes of annotated resources. See also -kubernetes-east-west-range-domains.") flag.Var(&cfg.KubernetesEastWestRangeAnnotationFiltersAppendString, "kubernetes-east-west-range-annotation-filters-append", "similar to -kubernetes-annotation-filters-append configures filters appended to east-west routes of annotated resources. See also -kubernetes-east-west-range-domains.") flag.BoolVar(&cfg.EnableKubernetesExternalNames, "enable-kubernetes-external-names", false, "only if enabled we allow to use external name services as backends in Ingress") @@ -599,6 +607,8 @@ func NewConfig() *Config { flag.DurationVar(&cfg.OIDCCookieValidity, "oidc-cookie-validity", time.Hour, "sets the cookie expiry time to +1h for OIDC filters, when no 'exp' claim is found in the JWT token") flag.DurationVar(&cfg.OidcDistributedClaimsTimeout, "oidc-distributed-claims-timeout", 2*time.Second, "sets the default OIDC distributed claims request timeout duration to 2000ms") flag.IntVar(&cfg.OIDCCookieRemoveSubdomains, "oidc-cookie-remove-subdomains", 1, "sets the number of subdomains to remove from the callback request hostname to obtain token cookie domain") + flag.Var(newYamlFlag(&cfg.OidcProfiles), "oidc-profiles", "named OIDC profile configurations in YAML format; profiles can be referenced by oauthOidc* filters via the \"profile:\" first-argument syntax; mutually exclusive with -oidc-profiles-file") + flag.StringVar(&cfg.OidcProfilesFile, "oidc-profiles-file", "", "path to a YAML file containing named OIDC profile configurations; the file must be a YAML map of profile name to OidcProfile struct; mutually exclusive with -oidc-profiles") flag.Var(cfg.CredentialPaths, "credentials-paths", "directories or files to watch for credentials to use by bearerinjector filter") flag.DurationVar(&cfg.CredentialsUpdateInterval, "credentials-update-interval", 10*time.Minute, "sets the interval to update secrets") flag.BoolVar(&cfg.EnableOpenPolicyAgent, "enable-open-policy-agent", false, "enables Open Policy Agent filters") @@ -996,6 +1006,8 @@ func (c *Config) ToOptions() skipper.Options { KubernetesEastWestRangeAnnotationFiltersAppend: c.KubernetesEastWestRangeAnnotationFiltersAppend, KubernetesAnnotationPredicates: c.KubernetesAnnotationPredicates, KubernetesAnnotationFiltersAppend: c.KubernetesAnnotationFiltersAppend, + KubernetesAnnotationsToRouteAnnotations: c.KubernetesAnnotationsToRouteAnnotations.values, + KubernetesAnnotationsToRouteAnnotationsPrefix: c.KubernetesAnnotationsToRouteAnnotationsPrefix, EnableKubernetesExternalNames: c.EnableKubernetesExternalNames, KubernetesOnlyAllowedExternalNames: c.KubernetesOnlyAllowedExternalNames, KubernetesAllowedExternalNames: c.KubernetesAllowedExternalNames, @@ -1047,6 +1059,7 @@ func (c *Config) ToOptions() skipper.Options { OIDCCookieValidity: c.OIDCCookieValidity, OIDCDistributedClaimsTimeout: c.OidcDistributedClaimsTimeout, OIDCCookieRemoveSubdomains: c.OIDCCookieRemoveSubdomains, + OidcProfiles: c.oidcProfilesMap(), CredentialsPaths: c.CredentialPaths.values, CredentialsUpdateInterval: c.CredentialsUpdateInterval, ValidationWebhookEnabled: c.ValidationWebhookEnabled, @@ -1431,3 +1444,26 @@ func parseAnnotationConfig[T any](kvvs []string, parseValue func(annotationKey, } return result, nil } + +// oidcProfilesMap returns the configured OidcProfile map, or nil when no profiles are configured. +// Exactly one of -oidc-profiles and -oidc-profiles-file may be set; using both is a fatal error. +func (c *Config) oidcProfilesMap() map[string]auth.OidcProfile { + if c.OidcProfiles != nil && c.OidcProfilesFile != "" { + log.Fatal("cannot use both -oidc-profiles and -oidc-profiles-file") + } + if c.OidcProfiles != nil { + return *c.OidcProfiles + } + if c.OidcProfilesFile != "" { + data, err := os.ReadFile(c.OidcProfilesFile) + if err != nil { + log.Fatalf("cannot read -oidc-profiles-file %q: %v", c.OidcProfilesFile, err) + } + var profiles map[string]auth.OidcProfile + if err := yaml.Unmarshal(data, &profiles); err != nil { + log.Fatalf("cannot parse -oidc-profiles-file %q: %v", c.OidcProfilesFile, err) + } + return profiles + } + return nil +} diff --git a/dataclients/kubernetes/annotations.go b/dataclients/kubernetes/annotations.go index 716a6e65aa..d81c9fb1e2 100644 --- a/dataclients/kubernetes/annotations.go +++ b/dataclients/kubernetes/annotations.go @@ -2,6 +2,7 @@ package kubernetes import ( "github.com/zalando/skipper/eskip" + "github.com/zalando/skipper/filters" ) type AnnotationPredicates struct { @@ -33,3 +34,24 @@ func appendAnnotationFilters(annotationFilters []AnnotationFilters, annotations } } } + +// injectAnnotateFilters prepends annotate(prefix+key, value) filters into r.Filters for each +// key in keysToInject that is present in annotations. prefix is prepended to the key used in +// the annotate() call (but not to the annotation lookup key). This makes Kubernetes resource +// annotation values accessible to downstream filters (e.g. oauthOidc* profile filters) +// via annotate.GetAnnotations(ctx). +func injectAnnotateFilters(annotations map[string]string, keysToInject []string, prefix string, r *eskip.Route) { + var toAdd []*eskip.Filter + for _, key := range keysToInject { + if val, ok := annotations[key]; ok { + toAdd = append(toAdd, &eskip.Filter{ + Name: filters.AnnotateName, + Args: []interface{}{prefix + key, val}, + }) + } + } + if len(toAdd) == 0 { + return + } + r.Filters = append(toAdd, r.Filters...) +} diff --git a/dataclients/kubernetes/annotations_test.go b/dataclients/kubernetes/annotations_test.go new file mode 100644 index 0000000000..86163e0ae1 --- /dev/null +++ b/dataclients/kubernetes/annotations_test.go @@ -0,0 +1,158 @@ +package kubernetes + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/zalando/skipper/eskip" + "github.com/zalando/skipper/filters" +) + +func TestInjectAnnotateFilters(t *testing.T) { + annotateFilter := func(key, val string) *eskip.Filter { + return &eskip.Filter{ + Name: filters.AnnotateName, + Args: []interface{}{key, val}, + } + } + + existingFilter := &eskip.Filter{Name: "setRequestHeader", Args: []interface{}{"X-Foo", "bar"}} + + t.Run("no keys to inject leaves route unchanged", func(t *testing.T) { + r := &eskip.Route{Filters: []*eskip.Filter{existingFilter}} + annotations := map[string]string{"my-annotation": "value"} + injectAnnotateFilters(annotations, nil, "", r) + assert.Equal(t, []*eskip.Filter{existingFilter}, r.Filters) + }) + + t.Run("key not present in annotations does not add filter", func(t *testing.T) { + r := &eskip.Route{Filters: []*eskip.Filter{existingFilter}} + annotations := map[string]string{"other-key": "value"} + injectAnnotateFilters(annotations, []string{"my-annotation"}, "", r) + assert.Equal(t, []*eskip.Filter{existingFilter}, r.Filters) + }) + + t.Run("matching key is prepended as annotate filter", func(t *testing.T) { + r := &eskip.Route{Filters: []*eskip.Filter{existingFilter}} + annotations := map[string]string{"my-annotation": "my-value"} + injectAnnotateFilters(annotations, []string{"my-annotation"}, "", r) + require := assert.New(t) + require.Len(r.Filters, 2) + require.Equal(filters.AnnotateName, r.Filters[0].Name) + require.Equal([]interface{}{"my-annotation", "my-value"}, r.Filters[0].Args) + require.Equal(existingFilter, r.Filters[1]) + }) + + t.Run("multiple matching keys all prepended", func(t *testing.T) { + r := &eskip.Route{} + annotations := map[string]string{ + "key-a": "val-a", + "key-b": "val-b", + } + injectAnnotateFilters(annotations, []string{"key-a", "key-b"}, "", r) + assert.Len(t, r.Filters, 2) + names := []string{r.Filters[0].Args[0].(string), r.Filters[1].Args[0].(string)} + assert.ElementsMatch(t, []string{"key-a", "key-b"}, names) + }) + + t.Run("only present keys are injected when some are missing", func(t *testing.T) { + r := &eskip.Route{Filters: []*eskip.Filter{existingFilter}} + annotations := map[string]string{"present": "yes"} + injectAnnotateFilters(annotations, []string{"present", "absent"}, "", r) + assert.Len(t, r.Filters, 2) + assert.Equal(t, filters.AnnotateName, r.Filters[0].Name) + assert.Equal(t, "present", r.Filters[0].Args[0]) + assert.Equal(t, existingFilter, r.Filters[1]) + }) + + t.Run("empty annotations map with keys to inject leaves route unchanged", func(t *testing.T) { + r := &eskip.Route{Filters: []*eskip.Filter{existingFilter}} + injectAnnotateFilters(map[string]string{}, []string{"key-a", "key-b"}, "", r) + assert.Equal(t, []*eskip.Filter{existingFilter}, r.Filters) + }) + + t.Run("nil annotations map with keys to inject leaves route unchanged", func(t *testing.T) { + r := &eskip.Route{Filters: []*eskip.Filter{existingFilter}} + injectAnnotateFilters(nil, []string{"key-a"}, "", r) + assert.Equal(t, []*eskip.Filter{existingFilter}, r.Filters) + }) + + t.Run("route with no existing filters gets annotate filter prepended", func(t *testing.T) { + r := &eskip.Route{} + annotations := map[string]string{"my-key": "my-val"} + injectAnnotateFilters(annotations, []string{"my-key"}, "", r) + assert.Len(t, r.Filters, 1) + assert.Equal(t, annotateFilter("my-key", "my-val"), r.Filters[0]) + }) + + t.Run("injected annotate filters come before existing filters", func(t *testing.T) { + existing1 := &eskip.Filter{Name: "filter1"} + existing2 := &eskip.Filter{Name: "filter2"} + r := &eskip.Route{Filters: []*eskip.Filter{existing1, existing2}} + annotations := map[string]string{"k": "v"} + injectAnnotateFilters(annotations, []string{"k"}, "", r) + assert.Len(t, r.Filters, 3) + assert.Equal(t, filters.AnnotateName, r.Filters[0].Name) + assert.Equal(t, existing1, r.Filters[1]) + assert.Equal(t, existing2, r.Filters[2]) + }) + + t.Run("annotation value is correctly propagated to filter args", func(t *testing.T) { + r := &eskip.Route{} + annotations := map[string]string{"oidc-tenant": "acme-corp"} + injectAnnotateFilters(annotations, []string{"oidc-tenant"}, "", r) + assert.Len(t, r.Filters, 1) + assert.Equal(t, "oidc-tenant", r.Filters[0].Args[0]) + assert.Equal(t, "acme-corp", r.Filters[0].Args[1]) + }) + + t.Run("prefix is prepended to annotate key without separator", func(t *testing.T) { + r := &eskip.Route{} + annotations := map[string]string{"oidc/client-id": "my-client"} + injectAnnotateFilters(annotations, []string{"oidc/client-id"}, "k8s:", r) + assert.Len(t, r.Filters, 1) + assert.Equal(t, "k8s:oidc/client-id", r.Filters[0].Args[0]) + assert.Equal(t, "my-client", r.Filters[0].Args[1]) + }) + + t.Run("prefix does not affect K8s annotation lookup key", func(t *testing.T) { + r := &eskip.Route{} + // annotation stored under "client-id", injected as "prefix:client-id" + annotations := map[string]string{"client-id": "the-value"} + injectAnnotateFilters(annotations, []string{"client-id"}, "prefix:", r) + assert.Len(t, r.Filters, 1) + assert.Equal(t, "prefix:client-id", r.Filters[0].Args[0]) + assert.Equal(t, "the-value", r.Filters[0].Args[1]) + // also verify the old lookup key is not in the result + assert.NotEqual(t, "client-id", r.Filters[0].Args[0]) + }) + + t.Run("prefix applied to multiple keys independently", func(t *testing.T) { + r := &eskip.Route{} + annotations := map[string]string{ + "cid": "c1", + "sec": "s1", + } + injectAnnotateFilters(annotations, []string{"cid", "sec"}, "oidc:", r) + assert.Len(t, r.Filters, 2) + keys := []string{r.Filters[0].Args[0].(string), r.Filters[1].Args[0].(string)} + assert.ElementsMatch(t, []string{"oidc:cid", "oidc:sec"}, keys) + }) + + t.Run("empty prefix behaves identically to no prefix", func(t *testing.T) { + r1 := &eskip.Route{} + r2 := &eskip.Route{} + annotations := map[string]string{"k": "v"} + injectAnnotateFilters(annotations, []string{"k"}, "", r1) + injectAnnotateFilters(annotations, []string{"k"}, "", r2) + assert.Equal(t, r1.Filters, r2.Filters) + assert.Equal(t, "k", r1.Filters[0].Args[0]) + }) + + t.Run("prefix with missing key still adds no filter", func(t *testing.T) { + r := &eskip.Route{} + annotations := map[string]string{"other": "v"} + injectAnnotateFilters(annotations, []string{"missing"}, "pfx:", r) + assert.Empty(t, r.Filters) + }) +} diff --git a/dataclients/kubernetes/ingress.go b/dataclients/kubernetes/ingress.go index ad969362bd..9c6a88e600 100644 --- a/dataclients/kubernetes/ingress.go +++ b/dataclients/kubernetes/ingress.go @@ -70,6 +70,8 @@ type ingress struct { kubernetesAnnotationFiltersAppend []AnnotationFilters kubernetesEastWestRangeAnnotationPredicates []AnnotationPredicates kubernetesEastWestRangeAnnotationFiltersAppend []AnnotationFilters + annotationsToRouteAnnotations []string + annotationsToRouteAnnotationsPrefix string } var ( @@ -121,6 +123,8 @@ func newIngress(o Options) *ingress { kubernetesAnnotationFiltersAppend: o.KubernetesAnnotationFiltersAppend, kubernetesEastWestRangeAnnotationPredicates: o.KubernetesEastWestRangeAnnotationPredicates, kubernetesEastWestRangeAnnotationFiltersAppend: o.KubernetesEastWestRangeAnnotationFiltersAppend, + annotationsToRouteAnnotations: o.AnnotationsToRouteAnnotations, + annotationsToRouteAnnotationsPrefix: o.AnnotationsToRouteAnnotationsPrefix, } } diff --git a/dataclients/kubernetes/ingressv1.go b/dataclients/kubernetes/ingressv1.go index 7c739080d1..2325fcf993 100644 --- a/dataclients/kubernetes/ingressv1.go +++ b/dataclients/kubernetes/ingressv1.go @@ -197,6 +197,8 @@ func (ing *ingress) addEndpointsRuleV1(ic *ingressContext, host string, prule *d return fmt.Errorf("error while getting service: %w", err) } + injectAnnotateFilters(ic.ingressV1.Metadata.Annotations, ing.annotationsToRouteAnnotations, ing.annotationsToRouteAnnotationsPrefix, endpointsRoute) + if endpointsRoute.BackendType != eskip.ShuntBackend { // safe prepend, see: https://play.golang.org/p/zg5aGKJpRyK filters := make([]*eskip.Filter, len(endpointsRoute.Filters)+len(ic.annotationFilters)) diff --git a/dataclients/kubernetes/kube.go b/dataclients/kubernetes/kube.go index 393c56353b..7f98111231 100644 --- a/dataclients/kubernetes/kube.go +++ b/dataclients/kubernetes/kube.go @@ -223,6 +223,15 @@ type Options struct { // KubernetesAnnotationFiltersAppend sets filters to append for each annotation key and value KubernetesAnnotationFiltersAppend []AnnotationFilters + // AnnotationsToRouteAnnotations is a list of Kubernetes resource annotation keys whose values + // are automatically injected as annotate() filters into routes generated from those resources. + AnnotationsToRouteAnnotations []string + + // AnnotationsToRouteAnnotationsPrefix is an optional string prepended to the key in the + // generated annotate() filter call. The K8s annotation lookup key is unchanged. + // No separator is added between prefix and key. + AnnotationsToRouteAnnotationsPrefix string + // DefaultFiltersDir enables default filters mechanism and sets the location of the default filters. // The provided filters are then applied to all routes. DefaultFiltersDir string diff --git a/dataclients/kubernetes/routegroup.go b/dataclients/kubernetes/routegroup.go index a7410572b5..e0a94573aa 100644 --- a/dataclients/kubernetes/routegroup.go +++ b/dataclients/kubernetes/routegroup.go @@ -600,6 +600,7 @@ func (r *routeGroups) convert(s *clusterState, df defaultFilters, loggingEnabled } for _, route := range ri { + injectAnnotateFilters(rg.Metadata.Annotations, r.options.AnnotationsToRouteAnnotations, r.options.AnnotationsToRouteAnnotationsPrefix, route) appendAnnotationPredicates(r.options.KubernetesAnnotationPredicates, rg.Metadata.Annotations, route) appendAnnotationFilters(r.options.KubernetesAnnotationFiltersAppend, rg.Metadata.Annotations, route) } @@ -644,6 +645,7 @@ func (r *routeGroups) convert(s *clusterState, df defaultFilters, loggingEnabled applyEastWestRangePredicates(internalRi, r.options.KubernetesEastWestRangePredicates) for _, route := range internalRi { + injectAnnotateFilters(rg.Metadata.Annotations, r.options.AnnotationsToRouteAnnotations, r.options.AnnotationsToRouteAnnotationsPrefix, route) appendAnnotationPredicates(r.options.KubernetesEastWestRangeAnnotationPredicates, rg.Metadata.Annotations, route) appendAnnotationFilters(r.options.KubernetesEastWestRangeAnnotationFiltersAppend, rg.Metadata.Annotations, route) } diff --git a/docs/reference/filters.md b/docs/reference/filters.md index 3a98689658..864ae5872b 100644 --- a/docs/reference/filters.md +++ b/docs/reference/filters.md @@ -2091,6 +2091,116 @@ oidcClaimsQuery("/:name%\"*One\"", "/path:groups.#[%\"*-Test-Users\"] groups.#[= As of now there is no negative/deny rule possible. The first matching path is evaluated against the defined query/queries and if positive, permitted. +#### OIDC Profiles + +Instead of embedding OIDC connection parameters in every filter call, you can define **named profiles** in a YAML configuration file and reference them using the `profile:` syntax as the first argument to any `oauthOidc*` filter: + +``` +oauthOidcAnyClaims("profile:myprofile", "claim1 claim2") +oauthOidcAllClaims("profile:myprofile", "claim1 claim2") +oauthOidcUserInfo("profile:myprofile") +``` + +Only the optional claims argument (second positional arg) can be supplied alongside the profile reference. All other connection parameters (IdpURL, ClientID, ClientSecret, CallbackURL, Scopes, AuthCodeOpts, UpstreamHeaders, SubdomainsToRemove, CookieName) come from the profile definition. + +##### Defining profiles + +Pass a YAML map of `name → OidcProfile` to the `-oidc-profiles` flag: + +``` +skipper -oidc-secrets-file /path/to/secrets \ + -oidc-profiles '{myprofile: {idp-url: "https://idp.example.com", client-id: "my-client", client-secret: "my-secret", callback-url: "https://app.example.com/auth/callback", scopes: "email profile"}}' +``` + +Or use a YAML config file: + +```yaml +oidc-profiles: + myprofile: + idp-url: https://idp.example.com + client-id: my-client-id + client-secret: my-client-secret + callback-url: https://app.example.com/auth/callback + scopes: email profile +``` + +The **Client ID** and **Client Secret** profile fields also support the `secretRef:` prefix to read values from Skipper's secrets registry: + +```yaml +oidc-profiles: + myprofile: + idp-url: https://idp.example.com + client-id: secretRef:/mnt/secrets/oidc-client-id + client-secret: secretRef:/mnt/secrets/oidc-client-secret + callback-url: https://app.example.com/auth/callback +``` + +##### Go template fields + +All profile fields except `idp-url` support Go [`text/template`](https://pkg.go.dev/text/template) syntax, resolved at request time: + +| Template expression | Value | +| ------------------- | ----- | +| `{{.Request.Host}}` | Request hostname (from the `Host` header) | +| `{{index .Annotations "key"}}` | Value set by a preceding `annotate()` filter | + +This enables multi-tenant deployments where OIDC parameters are derived from the incoming request or from Kubernetes resource annotations: + +```yaml +oidc-profiles: + multi-tenant: + idp-url: https://idp.example.com + client-id: '{{index .Annotations "oidc/client-id"}}' + client-secret: '{{index .Annotations "oidc/client-secret"}}' + callback-url: 'https://{{.Request.Host}}/auth/callback' + scopes: email profile +``` + +**Note:** `idp-url` must be a static URL — template expressions are rejected. The provider is discovered once at filter creation time. + +##### Annotation injection from Kubernetes resources + +To make Kubernetes Ingress or RouteGroup annotation values available to profile templates via `{{index .Annotations "key"}}`, use the `-kubernetes-annotations-to-route-annotations` flag: + +``` +skipper -kubernetes-annotations-to-route-annotations=oidc/client-id,oidc/client-secret +``` + +When this flag is set, the Kubernetes dataclient automatically prepends `annotate(key, value)` filters to every route generated from a resource that carries one of the configured annotation keys. The injected annotation values then become accessible to the OIDC profile filter at request time. + +**Kubernetes Ingress example:** + +```yaml +apiVersion: networking.k8s.io/v1 +kind: Ingress +metadata: + annotations: + oidc/client-id: tenant-abc-client + oidc/client-secret: secretRef:/mnt/secrets/oidc-secret + zalando.org/skipper-filter: 'oauthOidcAnyClaims("profile:multi-tenant", "groups")' +spec: + rules: + - host: tenant-abc.example.com + http: + paths: + - path: / + pathType: Prefix + backend: + service: + name: my-service + port: + number: 80 +``` + +Skipper arguments: + +| Argument | Required? | Description | +| -------- | --------- | ----------- | +| `-oidc-profiles` | no | Named OIDC profile configurations in YAML format. Maps profile name to an `OidcProfile` struct. Mutually exclusive with `-oidc-profiles-file`. | +| `-oidc-profiles-file` | no | Path to a YAML file containing named OIDC profile configurations (a map of profile name to `OidcProfile` struct). Mutually exclusive with `-oidc-profiles`. | +| `-kubernetes-annotations-to-route-annotations` | no | Comma-separated list of Kubernetes resource annotation keys whose values are automatically injected as `annotate()` filters into generated routes, making them accessible to OIDC profile templates. | +| `-kubernetes-annotations-to-route-annotations-prefix` | no | Optional prefix prepended to the key in each generated `annotate()` filter call. No separator is added. Example: prefix `k8s:` + Ingress / RouteGroup annotation `oidc/cid` → `annotate("k8s:oidc/cid", value)`. | + ### Open Policy Agent To get started with [Open Policy Agent](https://www.openpolicyagent.org/), also have a look at the [tutorial](../tutorials/auth.md#open-policy-agent). This section is only a reference for the implemented filters. diff --git a/docs/tutorials/auth.md b/docs/tutorials/auth.md index 8b7cf2256e..17b9b428a7 100644 --- a/docs/tutorials/auth.md +++ b/docs/tutorials/auth.md @@ -261,6 +261,122 @@ Access to path `/` would be granted to everyone in `example.org`, however path ` oauthOidcAnyClaims(...) -> oidcClaimsQuery("/login:groups.#[==\"appX-Tester\"]", "/:@_:email%\"*@example.org\"") ``` +### OIDC Profiles + +When the same OIDC provider and credentials are shared across many routes, repeating all connection parameters in every filter call becomes error-prone and hard to maintain. **OIDC profiles** let you define the parameters once in a YAML configuration and reference them by name: + +``` +oauthOidcAnyClaims("profile:myprofile", "groups") +``` + +#### Defining a profile + +Add the `-oidc-profiles` flag when starting Skipper (inline YAML), or use `-oidc-profiles-file` to point to a YAML file (the two flags are mutually exclusive). You can also add `oidc-profiles` to your YAML config file: + +```yaml +oidc-profiles: + myprofile: + idp-url: https://idp.example.com + client-id: my-client-id + client-secret: my-client-secret + callback-url: https://app.example.com/auth/callback + scopes: email profile +``` + +Then write routes using `profile:` as the first argument: + +``` +myroute: * -> oauthOidcAnyClaims("profile:myprofile", "groups") -> "https://internal.example.org"; +``` + +The `openid` scope is always included automatically. Claims go in the route arguments as usual; they are never part of the profile. + +#### Template fields for multi-tenant deployments + +Profile fields (except `idp-url`) support Go [`text/template`](https://pkg.go.dev/text/template) expressions resolved at request time. Two data sources are available: + +* `{{.Request.Host}}` — the `Host` header of the incoming request +* `{{index .Annotations "key"}}` — a value injected by a preceding `annotate()` filter + +This makes it possible to serve many tenants from a single profile: + +```yaml +oidc-profiles: + multi-tenant: + idp-url: https://idp.example.com + client-id: '{{index .Annotations "oidc/client-id"}}' + client-secret: '{{index .Annotations "oidc/client-secret"}}' + callback-url: 'https://{{.Request.Host}}/auth/callback' + scopes: email profile +``` + +**Note:** `idp-url` must be a static URL. Template expressions in that field are rejected at startup. + +Client ID and Client Secret also accept the `secretRef:` prefix to read values from Skipper's secrets registry: + +```yaml +oidc-profiles: + myprofile: + idp-url: https://idp.example.com + client-id: secretRef:/mnt/secrets/oidc-client-id + client-secret: secretRef:/mnt/secrets/oidc-client-secret + callback-url: https://app.example.com/auth/callback +``` + +#### Automatic annotation injection in Kubernetes + +When using Kubernetes Ingress or RouteGroups, you can drive profile template values from annotations on the resource itself. Start Skipper with `-kubernetes-annotations-to-route-annotations` listing the annotation keys to inject: + +``` +skipper -kubernetes-annotations-to-route-annotations=oidc/client-id,oidc/client-secret +``` + +Skipper then automatically prepends `annotate(key, value)` filters for each matching annotation found on any Ingress or RouteGroup. The profile template can then read those values via `{{index .Annotations "key"}}`. + +Use `-kubernetes-annotations-to-route-annotations-prefix` to prepend a fixed string to the key used in the `annotate()` call (the Kubernetes annotation lookup key is unchanged, no separator is added): + +``` +skipper -kubernetes-annotations-to-route-annotations=oidc/client-id \ + -kubernetes-annotations-to-route-annotations-prefix=k8s: +``` + +With this prefix, the annotation `oidc/client-id` on a resource produces `annotate("k8s:oidc/client-id", value)`, and the template would use `{{index .Annotations "k8s:oidc/client-id"}}`. + +Full example — Kubernetes Ingress: + +```yaml +apiVersion: networking.k8s.io/v1 +kind: Ingress +metadata: + annotations: + oidc/client-id: tenant-abc-client + oidc/client-secret: secretRef:/mnt/secrets/tenant-abc-oidc-secret + zalando.org/skipper-filter: 'oauthOidcAnyClaims("profile:multi-tenant", "groups")' +spec: + rules: + - host: tenant-abc.example.com + http: + paths: + - path: / + pathType: Prefix + backend: + service: + name: my-service + port: + number: 80 +``` + +When Skipper processes this Ingress, it generates routes that effectively behave as: + +``` +annotate("oidc/client-id", "tenant-abc-client") + -> annotate("oidc/client-secret", "secretRef:/mnt/secrets/tenant-abc-oidc-secret") + -> oauthOidcAnyClaims("profile:multi-tenant", "groups") + -> ... +``` + +The profile filter resolves `{{index .Annotations "oidc/client-id"}}` to `tenant-abc-client` and `{{.Request.Host}}` to the actual request hostname at request time. Resolved configurations are cached so repeated requests with the same resolved values incur no extra overhead. + ## OAuth2 authorization grant flow [Authorization grant flow](https://tools.ietf.org/html/rfc6749#section-1.3) is a mechanism diff --git a/filters/auth/oidc.go b/filters/auth/oidc.go index 3b396c92be..13210d2bb8 100644 --- a/filters/auth/oidc.go +++ b/filters/auth/oidc.go @@ -101,6 +101,10 @@ type OidcOptions struct { // OpenTracingClientTraceByTag instead of events use span Tags // to measure client connection pool actions OpenTracingClientTraceByTag bool + + // Profiles is a map of named OIDC profile configurations that can be referenced + // by oauthOidc* filters using the "profile:" first argument syntax. + Profiles map[string]OidcProfile } const oidcSecretRefPrefix = "secretRef:" @@ -207,11 +211,73 @@ func NewOAuthOidcAllClaims(secretsFile string, secretsRegistry secrets.Encrypter // // oauthOidcAllClaims("https://accounts.identity-provider.com", "some-client-id", "some-client-secret", // "http://callback.com/auth/provider/callback", "scope1 scope2", "claim1 claim2", "", "", "") -> "https://internal.example.org"; +// +// createProfileFilter handles the "profile:" first-arg syntax. +func (s *tokenOidcSpec) createProfileFilter(sargs []string) (filters.Filter, error) { + name := strings.TrimPrefix(sargs[0], oidcProfilePrefix) + profile, ok := s.options.Profiles[name] + if !ok { + return nil, fmt.Errorf("%w: oidc profile %q not found", filters.ErrInvalidFilterParameters, name) + } + + if profile.IdpURL == "" { + return nil, fmt.Errorf("%w: oidc profile %q: IdpURL is required", filters.ErrInvalidFilterParameters, name) + } + if strings.Contains(profile.IdpURL, "{{") { + return nil, fmt.Errorf("%w: oidc profile %q: IdpURL must be a static URL (no template expressions)", filters.ErrInvalidFilterParameters, name) + } + + provider, err := oidc.NewProvider(context.Background(), profile.IdpURL) + if err != nil { + log.Errorf("Failed to create new provider %s: %v.", profile.IdpURL, err) + return nil, filters.ErrInvalidFilterParameters + } + + encrypter, err := s.secretsRegistry.GetEncrypter(1*time.Minute, s.SecretsFile) + if err != nil { + return nil, err + } + + validity := s.options.CookieValidity + if validity == 0 { + validity = defaultCookieValidity + } + + subdomainsToRemove := 1 + if s.options.CookieRemoveSubdomains != nil { + subdomainsToRemove = *s.options.CookieRemoveSubdomains + } + + var claims []string + if len(sargs) > 1 && sargs[1] != "" { + claims = strings.Split(sargs[1], " ") + } + + profileCopy := profile + return newProfileFilter( + s.typ, + &profileCopy, + claims, + provider, + encrypter, + validity, + subdomainsToRemove, + s.options, + s, + ), nil +} + func (s *tokenOidcSpec) CreateFilter(args []interface{}) (filters.Filter, error) { sargs, err := getStrings(args) if err != nil { return nil, err } + + // Detect "profile:" first-arg syntax and delegate to profile filter creation. + if len(sargs) >= 1 && strings.HasPrefix(sargs[0], oidcProfilePrefix) { + return s.createProfileFilter(sargs) + } + if len(sargs) <= paramClaims { return nil, filters.ErrInvalidFilterParameters } diff --git a/filters/auth/oidcprofile.go b/filters/auth/oidcprofile.go new file mode 100644 index 0000000000..a58956f0f3 --- /dev/null +++ b/filters/auth/oidcprofile.go @@ -0,0 +1,352 @@ +package auth + +import ( + "compress/flate" + "crypto/sha256" + "fmt" + "net/http" + "net/url" + "strconv" + "strings" + "sync" + "text/template" + "time" + + "github.com/coreos/go-oidc/v3/oidc" + "golang.org/x/oauth2" + + "github.com/zalando/skipper/filters" + "github.com/zalando/skipper/filters/annotate" + "github.com/zalando/skipper/secrets" +) + +const ( + oidcProfilePrefix = "profile:" + oidcProfileStateBagKey = "filter.oidcProfileDelegate" +) + +// OidcProfile holds pre-configured OIDC connection parameters that can +// be referenced by oauthOidc* filters using the "profile:" syntax. +// +// String fields support Go text/template syntax: +// - {{.Request.Host}} — request hostname (prefers Host header) +// - {{index .Annotations "key"}} — value set by a preceding annotate() filter +// +// IdpURL must be a static URL (no template expressions). +type OidcProfile struct { + IdpURL string `yaml:"idp-url"` + ClientID string `yaml:"client-id"` + ClientSecret string `yaml:"client-secret"` + CallbackURL string `yaml:"callback-url"` + Scopes string `yaml:"scopes"` + AuthCodeOpts string `yaml:"auth-code-opts"` + UpstreamHeaders string `yaml:"upstream-headers"` + SubdomainsToRemove string `yaml:"subdomains-to-remove"` + CookieName string `yaml:"cookie-name"` +} + +// Validate parses each field as a Go text/template, failing fast on syntax errors. +func (p *OidcProfile) Validate() error { + if p.IdpURL == "" { + return fmt.Errorf("oidc profile: IdpURL is required") + } + for _, s := range []string{ + p.IdpURL, p.ClientID, p.ClientSecret, p.CallbackURL, + p.Scopes, p.AuthCodeOpts, p.UpstreamHeaders, p.SubdomainsToRemove, p.CookieName, + } { + if s == "" { + continue + } + if _, err := template.New("").Parse(s); err != nil { + return fmt.Errorf("oidc profile template parse error in %q: %w", s, err) + } + } + return nil +} + +// profileRequestData holds request-scoped data accessible as {{.Request.Field}}. +type profileRequestData struct { + Host string +} + +// profileTemplateData is the data model for OIDC profile field template resolution. +type profileTemplateData struct { + Request profileRequestData + Annotations map[string]string +} + +// resolveField executes a Go text/template string with the given data. +// Empty tmplStr returns "" without error. +func resolveField(tmplStr string, data profileTemplateData) (string, error) { + if tmplStr == "" { + return "", nil + } + tmpl, err := template.New("").Parse(tmplStr) + if err != nil { + return "", fmt.Errorf("template parse error in %q: %w", tmplStr, err) + } + var buf strings.Builder + if err := tmpl.Execute(&buf, data); err != nil { + return "", fmt.Errorf("template execution error in %q: %w", tmplStr, err) + } + return buf.String(), nil +} + +// tokenOidcProfileFilter is the per-route filter created when an oauthOidc* filter +// uses the "profile:" first argument syntax. It resolves profile field templates +// at request time, caches the resulting tokenOidcFilter delegates by a hash of the +// resolved parameters, and delegates all OIDC processing to them. +type tokenOidcProfileFilter struct { + typ roleCheckType + profile *OidcProfile + claims []string // from route args; never from profile + provider *oidc.Provider + encrypter secrets.Encryption + compressor cookieCompression + validity time.Duration + subdomainsToRemoveDefault int + oidcOptions OidcOptions + spec *tokenOidcSpec // for resolveClientCredential + + delegates sync.Map // map[string]*tokenOidcFilter, keyed by cacheKey +} + +// resolvedProfile holds the template-resolved (but not secretRef-resolved) field values. +type resolvedProfile struct { + clientID string + clientSecret string + callbackURL string + scopes string + authCodeOpts string + upstreamHeaders string + subdomainsToRemove string + cookieName string +} + +// cacheKey returns a SHA-256 hex string over all resolved fields. +func (r *resolvedProfile) cacheKey() string { + h := sha256.New() + for _, s := range []string{ + r.clientID, r.clientSecret, r.callbackURL, r.scopes, + r.authCodeOpts, r.upstreamHeaders, r.subdomainsToRemove, r.cookieName, + } { + h.Write([]byte(s)) + h.Write([]byte{0}) // field separator + } + return fmt.Sprintf("%x", h.Sum(nil)) +} + +// resolveAll resolves all profile template fields using request-time data. +func (f *tokenOidcProfileFilter) resolveAll(data profileTemplateData) (*resolvedProfile, error) { + var err error + r := &resolvedProfile{} + if r.clientID, err = resolveField(f.profile.ClientID, data); err != nil { + return nil, fmt.Errorf("ClientID: %w", err) + } + if r.clientSecret, err = resolveField(f.profile.ClientSecret, data); err != nil { + return nil, fmt.Errorf("ClientSecret: %w", err) + } + if r.callbackURL, err = resolveField(f.profile.CallbackURL, data); err != nil { + return nil, fmt.Errorf("CallbackURL: %w", err) + } + if r.scopes, err = resolveField(f.profile.Scopes, data); err != nil { + return nil, fmt.Errorf("scopes: %w", err) + } + if r.authCodeOpts, err = resolveField(f.profile.AuthCodeOpts, data); err != nil { + return nil, fmt.Errorf("AuthCodeOpts: %w", err) + } + if r.upstreamHeaders, err = resolveField(f.profile.UpstreamHeaders, data); err != nil { + return nil, fmt.Errorf("UpstreamHeaders: %w", err) + } + if r.subdomainsToRemove, err = resolveField(f.profile.SubdomainsToRemove, data); err != nil { + return nil, fmt.Errorf("SubdomainsToRemove: %w", err) + } + if r.cookieName, err = resolveField(f.profile.CookieName, data); err != nil { + return nil, fmt.Errorf("CookieName: %w", err) + } + return r, nil +} + +// buildDelegate constructs a tokenOidcFilter from template-resolved profile fields. +// secretRef resolution happens here so the actual credentials are in the delegate. +func (f *tokenOidcProfileFilter) buildDelegate(r *resolvedProfile) (*tokenOidcFilter, error) { + clientID, err := f.spec.resolveClientCredential(r.clientID) + if err != nil { + return nil, fmt.Errorf("ClientID secretRef: %w", err) + } + clientSecret, err := f.spec.resolveClientCredential(r.clientSecret) + if err != nil { + return nil, fmt.Errorf("ClientSecret secretRef: %w", err) + } + + // Cookie name: use explicit value or derive from hash of params + cookieName := r.cookieName + if cookieName == "" { + h := sha256.New() + // Hash same fields as CreateFilter (skip CallbackURL and SubdomainsToRemove) + for _, s := range []string{ + f.profile.IdpURL, clientID, clientSecret, r.scopes, r.authCodeOpts, r.upstreamHeaders, + } { + h.Write([]byte(s)) + } + cookieName = oauthOidcCookieName + fmt.Sprintf("%x", h.Sum(nil))[:8] + "-" + } + + if r.callbackURL == "" { + return nil, fmt.Errorf("profile CallbackURL is required") + } + redirectURL, err := url.Parse(r.callbackURL) + if err != nil { + return nil, fmt.Errorf("invalid CallbackURL %q: %w", r.callbackURL, err) + } + + subdomainsToRemove := f.subdomainsToRemoveDefault + if r.subdomainsToRemove != "" { + subdomainsToRemove, err = strconv.Atoi(r.subdomainsToRemove) + if err != nil { + return nil, fmt.Errorf("invalid SubdomainsToRemove %q: %w", r.subdomainsToRemove, err) + } + if subdomainsToRemove < 0 { + return nil, fmt.Errorf("SubdomainsToRemove cannot be negative: %d", subdomainsToRemove) + } + } + + scopes := []string{oidc.ScopeOpenID} + if r.scopes != "" { + scopes = append(scopes, strings.Split(r.scopes, " ")...) + } + + config := &oauth2.Config{ + ClientID: clientID, + ClientSecret: clientSecret, + RedirectURL: r.callbackURL, + Endpoint: f.provider.Endpoint(), + Scopes: scopes, + } + verifier := f.provider.Verifier(&oidc.Config{ClientID: clientID}) + + authCodeOptions := make([]oauth2.AuthCodeOption, 0) + var queryParams []string + if r.authCodeOpts != "" { + for _, p := range strings.Split(r.authCodeOpts, " ") { + splitP := strings.SplitN(p, "=", 2) + if len(splitP) != 2 { + return nil, fmt.Errorf("invalid auth code opt %q", p) + } + if splitP[1] == "skipper-request-query" { + queryParams = append(queryParams, splitP[0]) + } else { + authCodeOptions = append(authCodeOptions, oauth2.SetAuthURLParam(splitP[0], splitP[1])) + } + } + } + + var upstreamHeaders map[string]string + if r.upstreamHeaders != "" { + upstreamHeaders = make(map[string]string) + for _, header := range strings.Split(r.upstreamHeaders, " ") { + k, v, found := strings.Cut(header, ":") + if !found || k == "" || v == "" { + return nil, fmt.Errorf("malformed upstream header %q", header) + } + upstreamHeaders[k] = v + } + } + + return &tokenOidcFilter{ + typ: f.typ, + config: config, + provider: f.provider, + verifier: verifier, + claims: f.claims, + validity: f.validity, + cookiename: cookieName, + redirectPath: redirectURL.Path, + encrypter: f.encrypter, + authCodeOptions: authCodeOptions, + queryParams: queryParams, + compressor: f.compressor, + upstreamHeaders: upstreamHeaders, + subdomainsToRemove: subdomainsToRemove, + oidcOptions: f.oidcOptions, + }, nil +} + +func (f *tokenOidcProfileFilter) internalServerError(ctx filters.FilterContext) { + ctx.Serve(&http.Response{StatusCode: http.StatusInternalServerError}) +} + +// Request resolves profile templates using request-time data, looks up or builds +// a cached tokenOidcFilter delegate, stores it in the StateBag, and delegates. +func (f *tokenOidcProfileFilter) Request(ctx filters.FilterContext) { + annotations := annotate.GetAnnotations(ctx) + if annotations == nil { + annotations = map[string]string{} + } + data := profileTemplateData{ + Request: profileRequestData{Host: getHost(ctx.Request())}, + Annotations: annotations, + } + + resolved, err := f.resolveAll(data) + if err != nil { + ctx.Logger().Errorf("oidc profile filter: failed to resolve templates: %v", err) + f.internalServerError(ctx) + return + } + + key := resolved.cacheKey() + if d, ok := f.delegates.Load(key); ok { + delegate := d.(*tokenOidcFilter) + ctx.StateBag()[oidcProfileStateBagKey] = delegate + delegate.Request(ctx) + return + } + + delegate, err := f.buildDelegate(resolved) + if err != nil { + ctx.Logger().Errorf("oidc profile filter: failed to build delegate: %v", err) + f.internalServerError(ctx) + return + } + + actual, _ := f.delegates.LoadOrStore(key, delegate) + delegate = actual.(*tokenOidcFilter) + ctx.StateBag()[oidcProfileStateBagKey] = delegate + delegate.Request(ctx) +} + +// Response delegates to the tokenOidcFilter stored in the StateBag during Request. +// The base tokenOidcFilter.Response is a no-op, but we delegate for correctness. +func (f *tokenOidcProfileFilter) Response(ctx filters.FilterContext) { + if d, ok := ctx.StateBag()[oidcProfileStateBagKey]; ok { + d.(*tokenOidcFilter).Response(ctx) + } +} + +// newProfileFilter creates a tokenOidcProfileFilter from a pre-discovered provider. +// Called by tokenOidcSpec.CreateFilter when the first arg starts with "profile:". +func newProfileFilter( + typ roleCheckType, + profile *OidcProfile, + claims []string, + provider *oidc.Provider, + encrypter secrets.Encryption, + validity time.Duration, + subdomainsToRemoveDefault int, + opts OidcOptions, + spec *tokenOidcSpec, +) *tokenOidcProfileFilter { + return &tokenOidcProfileFilter{ + typ: typ, + profile: profile, + claims: claims, + provider: provider, + encrypter: encrypter, + compressor: newDeflatePoolCompressor(flate.BestCompression), + validity: validity, + subdomainsToRemoveDefault: subdomainsToRemoveDefault, + oidcOptions: opts, + spec: spec, + } +} diff --git a/filters/auth/oidcprofile_test.go b/filters/auth/oidcprofile_test.go new file mode 100644 index 0000000000..48b027868c --- /dev/null +++ b/filters/auth/oidcprofile_test.go @@ -0,0 +1,301 @@ +package auth + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestOidcProfileValidate(t *testing.T) { + t.Run("valid static profile", func(t *testing.T) { + p := &OidcProfile{ + IdpURL: "https://idp.example.com", + ClientID: "client-id", + CallbackURL: "https://app.example.com/callback", + } + assert.NoError(t, p.Validate()) + }) + + t.Run("valid profile with templates", func(t *testing.T) { + p := &OidcProfile{ + IdpURL: "https://idp.example.com", + ClientID: `{{index .Annotations "client-id"}}`, + CallbackURL: `https://{{.Request.Host}}/callback`, + } + assert.NoError(t, p.Validate()) + }) + + t.Run("empty IdpURL is rejected", func(t *testing.T) { + p := &OidcProfile{ + ClientID: "client-id", + CallbackURL: "https://app.example.com/callback", + } + err := p.Validate() + require.Error(t, err) + assert.Contains(t, err.Error(), "IdpURL is required") + }) + + t.Run("invalid template in ClientID", func(t *testing.T) { + p := &OidcProfile{ + IdpURL: "https://idp.example.com", + ClientID: "{{unclosed", + } + err := p.Validate() + require.Error(t, err) + assert.Contains(t, err.Error(), "template parse error") + }) + + t.Run("invalid template in CallbackURL", func(t *testing.T) { + p := &OidcProfile{ + IdpURL: "https://idp.example.com", + ClientID: "client-id", + CallbackURL: "{{broken template", + } + err := p.Validate() + require.Error(t, err) + assert.Contains(t, err.Error(), "template parse error") + }) + + t.Run("invalid template in CookieName", func(t *testing.T) { + p := &OidcProfile{ + IdpURL: "https://idp.example.com", + ClientID: "client-id", + CookieName: "{{.Bad", + } + err := p.Validate() + require.Error(t, err) + assert.Contains(t, err.Error(), "template parse error") + }) +} + +func TestResolveField(t *testing.T) { + data := profileTemplateData{ + Request: profileRequestData{Host: "myapp.example.com"}, + Annotations: map[string]string{"tenant": "acme", "env": "prod"}, + } + + t.Run("empty string returns empty", func(t *testing.T) { + result, err := resolveField("", data) + require.NoError(t, err) + assert.Equal(t, "", result) + }) + + t.Run("static string passes through", func(t *testing.T) { + result, err := resolveField("https://idp.example.com", data) + require.NoError(t, err) + assert.Equal(t, "https://idp.example.com", result) + }) + + t.Run("request host template", func(t *testing.T) { + result, err := resolveField("https://{{.Request.Host}}/callback", data) + require.NoError(t, err) + assert.Equal(t, "https://myapp.example.com/callback", result) + }) + + t.Run("annotation lookup template", func(t *testing.T) { + result, err := resolveField(`{{index .Annotations "tenant"}}`, data) + require.NoError(t, err) + assert.Equal(t, "acme", result) + }) + + t.Run("missing annotation returns empty string", func(t *testing.T) { + result, err := resolveField(`{{index .Annotations "missing"}}`, data) + require.NoError(t, err) + assert.Equal(t, "", result) + }) + + t.Run("combined template", func(t *testing.T) { + result, err := resolveField(`client-{{index .Annotations "tenant"}}-{{index .Annotations "env"}}`, data) + require.NoError(t, err) + assert.Equal(t, "client-acme-prod", result) + }) + + t.Run("invalid template syntax returns error", func(t *testing.T) { + _, err := resolveField("{{unclosed", data) + require.Error(t, err) + assert.Contains(t, err.Error(), "template parse error") + }) +} + +func TestResolvedProfileCacheKey(t *testing.T) { + t.Run("same fields produce same key", func(t *testing.T) { + r1 := &resolvedProfile{ + clientID: "client-a", + clientSecret: "secret-a", + callbackURL: "https://app.example.com/callback", + scopes: "email profile", + } + r2 := &resolvedProfile{ + clientID: "client-a", + clientSecret: "secret-a", + callbackURL: "https://app.example.com/callback", + scopes: "email profile", + } + assert.Equal(t, r1.cacheKey(), r2.cacheKey()) + }) + + t.Run("different clientID produces different key", func(t *testing.T) { + r1 := &resolvedProfile{clientID: "client-a"} + r2 := &resolvedProfile{clientID: "client-b"} + assert.NotEqual(t, r1.cacheKey(), r2.cacheKey()) + }) + + t.Run("different scopes produces different key", func(t *testing.T) { + r1 := &resolvedProfile{clientID: "c", scopes: "email"} + r2 := &resolvedProfile{clientID: "c", scopes: "email profile"} + assert.NotEqual(t, r1.cacheKey(), r2.cacheKey()) + }) + + t.Run("key is non-empty hex string", func(t *testing.T) { + r := &resolvedProfile{clientID: "c", clientSecret: "s"} + key := r.cacheKey() + assert.NotEmpty(t, key) + // SHA-256 hex = 64 chars + assert.Len(t, key, 64) + }) + + t.Run("field order matters - clientID vs cookieName not interchangeable", func(t *testing.T) { + r1 := &resolvedProfile{clientID: "value", cookieName: ""} + r2 := &resolvedProfile{clientID: "", cookieName: "value"} + assert.NotEqual(t, r1.cacheKey(), r2.cacheKey()) + }) +} + +func TestTokenOidcProfileFilterResolveAll(t *testing.T) { + t.Run("static profile resolves to same values", func(t *testing.T) { + profile := &OidcProfile{ + IdpURL: "https://idp.example.com", + ClientID: "static-client", + ClientSecret: "static-secret", + CallbackURL: "https://app.example.com/callback", + Scopes: "email profile", + } + f := &tokenOidcProfileFilter{profile: profile} + data := profileTemplateData{ + Request: profileRequestData{Host: "app.example.com"}, + Annotations: map[string]string{}, + } + r, err := f.resolveAll(data) + require.NoError(t, err) + assert.Equal(t, "static-client", r.clientID) + assert.Equal(t, "static-secret", r.clientSecret) + assert.Equal(t, "https://app.example.com/callback", r.callbackURL) + assert.Equal(t, "email profile", r.scopes) + }) + + t.Run("annotation templates resolved from data", func(t *testing.T) { + profile := &OidcProfile{ + IdpURL: "https://idp.example.com", + ClientID: `{{index .Annotations "my-client-id"}}`, + ClientSecret: `{{index .Annotations "my-client-secret"}}`, + CallbackURL: `https://{{.Request.Host}}/auth/callback`, + } + f := &tokenOidcProfileFilter{profile: profile} + data := profileTemplateData{ + Request: profileRequestData{Host: "tenant.example.com"}, + Annotations: map[string]string{ + "my-client-id": "tenant-client", + "my-client-secret": "tenant-secret", + }, + } + r, err := f.resolveAll(data) + require.NoError(t, err) + assert.Equal(t, "tenant-client", r.clientID) + assert.Equal(t, "tenant-secret", r.clientSecret) + assert.Equal(t, "https://tenant.example.com/auth/callback", r.callbackURL) + }) + + t.Run("different annotations produce different resolved profiles", func(t *testing.T) { + profile := &OidcProfile{ + IdpURL: "https://idp.example.com", + ClientID: `{{index .Annotations "client-id"}}`, + } + f := &tokenOidcProfileFilter{profile: profile} + + data1 := profileTemplateData{ + Annotations: map[string]string{"client-id": "client-a"}, + } + data2 := profileTemplateData{ + Annotations: map[string]string{"client-id": "client-b"}, + } + + r1, err := f.resolveAll(data1) + require.NoError(t, err) + r2, err := f.resolveAll(data2) + require.NoError(t, err) + + assert.NotEqual(t, r1.clientID, r2.clientID) + assert.NotEqual(t, r1.cacheKey(), r2.cacheKey()) + }) + + t.Run("same annotations produce same cache key", func(t *testing.T) { + profile := &OidcProfile{ + IdpURL: "https://idp.example.com", + ClientID: `{{index .Annotations "client-id"}}`, + CallbackURL: `https://{{.Request.Host}}/callback`, + } + f := &tokenOidcProfileFilter{profile: profile} + + data := profileTemplateData{ + Request: profileRequestData{Host: "app.example.com"}, + Annotations: map[string]string{"client-id": "my-client"}, + } + + r1, err := f.resolveAll(data) + require.NoError(t, err) + r2, err := f.resolveAll(data) + require.NoError(t, err) + + assert.Equal(t, r1.cacheKey(), r2.cacheKey()) + }) + + t.Run("subdomains to remove field resolved", func(t *testing.T) { + profile := &OidcProfile{ + IdpURL: "https://idp.example.com", + SubdomainsToRemove: "2", + } + f := &tokenOidcProfileFilter{profile: profile} + data := profileTemplateData{} + r, err := f.resolveAll(data) + require.NoError(t, err) + assert.Equal(t, "2", r.subdomainsToRemove) + }) + + t.Run("auth code opts and upstream headers resolved", func(t *testing.T) { + profile := &OidcProfile{ + IdpURL: "https://idp.example.com", + AuthCodeOpts: "prompt=consent", + UpstreamHeaders: "X-User-ID:sub", + } + f := &tokenOidcProfileFilter{profile: profile} + data := profileTemplateData{} + r, err := f.resolveAll(data) + require.NoError(t, err) + assert.Equal(t, "prompt=consent", r.authCodeOpts) + assert.Equal(t, "X-User-ID:sub", r.upstreamHeaders) + }) +} + +func TestOidcProfileDelegateCaching(t *testing.T) { + // This test verifies that calling resolveAll twice with the same data + // produces the same cache key (the foundation for delegate caching). + profile := &OidcProfile{ + IdpURL: "https://idp.example.com", + ClientID: `{{index .Annotations "cid"}}`, + ClientSecret: "secret", + CallbackURL: "https://app.example.com/cb", + } + f := &tokenOidcProfileFilter{profile: profile} + + data := profileTemplateData{ + Annotations: map[string]string{"cid": "my-client"}, + } + + r1, err := f.resolveAll(data) + require.NoError(t, err) + r2, err := f.resolveAll(data) + require.NoError(t, err) + + assert.Equal(t, r1.cacheKey(), r2.cacheKey(), "same input should produce the same cache key") +} diff --git a/skipper.go b/skipper.go index 3acf96055a..f60bc1d69c 100644 --- a/skipper.go +++ b/skipper.go @@ -294,6 +294,17 @@ type Options struct { // KubernetesAnnotationFiltersAppend sets filters to append for each annotation key and value KubernetesAnnotationFiltersAppend []kubernetes.AnnotationFilters + // KubernetesAnnotationsToRouteAnnotations is a list of Kubernetes resource annotation keys + // whose values are automatically injected as annotate() filters into routes generated from + // those resources. This makes the annotation values accessible to oauthOidc* profile filters + // via {{index .Annotations "key"}}. + KubernetesAnnotationsToRouteAnnotations []string + + // KubernetesAnnotationsToRouteAnnotationsPrefix is an optional prefix prepended to the key + // in the generated annotate() filter call. + // No separator is added between prefix and key. + KubernetesAnnotationsToRouteAnnotationsPrefix string + // EnableKubernetesExternalNames enables to use Kubernetes service type ExternalName as backend in Ingress and RouteGroup. EnableKubernetesExternalNames bool @@ -951,6 +962,10 @@ type Options struct { // the callback request hostname to obtain token cookie domain. OIDCCookieRemoveSubdomains int + // OidcProfiles is a map of named OIDC profile configurations. Profiles can be + // referenced by oauthOidc* filters via the "profile:" first-argument syntax. + OidcProfiles map[string]auth.OidcProfile + // SecretsRegistry to store and load secretsencrypt SecretsRegistry *secrets.Registry @@ -1093,6 +1108,8 @@ func (o *Options) KubernetesDataClientOptions() kubernetes.Options { KubernetesEastWestRangeAnnotationFiltersAppend: o.KubernetesEastWestRangeAnnotationFiltersAppend, KubernetesAnnotationPredicates: o.KubernetesAnnotationPredicates, KubernetesAnnotationFiltersAppend: o.KubernetesAnnotationFiltersAppend, + AnnotationsToRouteAnnotations: o.KubernetesAnnotationsToRouteAnnotations, + AnnotationsToRouteAnnotationsPrefix: o.KubernetesAnnotationsToRouteAnnotationsPrefix, HTTPSRedirectCode: o.KubernetesHTTPSRedirectCode, DisableCatchAllRoutes: o.KubernetesDisableCatchAllRoutes, IngressClass: o.KubernetesIngressClass, @@ -1909,6 +1926,7 @@ func run(o Options, sig chan os.Signal, idleConnsCH chan struct{}) error { OidcClientId: oidcClientId, OidcClientSecret: oidcClientSecret, SecretsReader: sp, + Profiles: o.OidcProfiles, } o.CustomFilters = append(o.CustomFilters, From 85999caee91b341f4af1a53db9343571b8121d23 Mon Sep 17 00:00:00 2001 From: Magnus Jungsbluth Date: Fri, 13 Mar 2026 16:01:39 +0100 Subject: [PATCH 2/7] Fix nil pointer exception Signed-off-by: Magnus Jungsbluth --- config/config_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/config/config_test.go b/config/config_test.go index 17d5252da6..6c27fad20c 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -108,6 +108,7 @@ func defaultConfig(with func(*Config)) *Config { EditRoute: routeChangerConfig{}, SourcePollTimeout: 3000, KubernetesEastWestRangeDomains: commaListFlag(), + KubernetesAnnotationsToRouteAnnotations: commaListFlag(), KubernetesHealthcheck: true, KubernetesHTTPSRedirect: true, KubernetesHTTPSRedirectCode: 308, From 0149043fc86824605140086e95d31b2073343188 Mon Sep 17 00:00:00 2001 From: Magnus Jungsbluth Date: Fri, 13 Mar 2026 22:06:46 +0100 Subject: [PATCH 3/7] review comments Signed-off-by: magnus-jungsbluth_zse --- filters/auth/oidc.go | 61 ++++++++++++++++---------------- filters/auth/oidcprofile.go | 47 ++++++++++++------------ filters/auth/oidcprofile_test.go | 51 +++++++++----------------- 3 files changed, 69 insertions(+), 90 deletions(-) diff --git a/filters/auth/oidc.go b/filters/auth/oidc.go index 13210d2bb8..9f7f3615c2 100644 --- a/filters/auth/oidc.go +++ b/filters/auth/oidc.go @@ -202,16 +202,24 @@ func NewOAuthOidcAllClaims(secretsFile string, secretsRegistry secrets.Encrypter return NewOAuthOidcAllClaimsWithOptions(secretsFile, secretsRegistry, OidcOptions{}) } -// CreateFilter creates an OpenID Connect authorization filter. -// -// first arg: a provider, for example "https://accounts.google.com", -// which has the path /.well-known/openid-configuration -// -// Example: -// -// oauthOidcAllClaims("https://accounts.identity-provider.com", "some-client-id", "some-client-secret", -// "http://callback.com/auth/provider/callback", "scope1 scope2", "claim1 claim2", "", "", "") -> "https://internal.example.org"; -// +// commonFilterParams returns the three filter construction parameters that are +// shared between CreateFilter and createProfileFilter. +func (s *tokenOidcSpec) commonFilterParams() (encrypter secrets.Encryption, validity time.Duration, subdomainsToRemove int, err error) { + encrypter, err = s.secretsRegistry.GetEncrypter(1*time.Minute, s.SecretsFile) + if err != nil { + return + } + validity = s.options.CookieValidity + if validity == 0 { + validity = defaultCookieValidity + } + subdomainsToRemove = 1 + if s.options.CookieRemoveSubdomains != nil { + subdomainsToRemove = *s.options.CookieRemoveSubdomains + } + return +} + // createProfileFilter handles the "profile:" first-arg syntax. func (s *tokenOidcSpec) createProfileFilter(sargs []string) (filters.Filter, error) { name := strings.TrimPrefix(sargs[0], oidcProfilePrefix) @@ -233,21 +241,11 @@ func (s *tokenOidcSpec) createProfileFilter(sargs []string) (filters.Filter, err return nil, filters.ErrInvalidFilterParameters } - encrypter, err := s.secretsRegistry.GetEncrypter(1*time.Minute, s.SecretsFile) + encrypter, validity, subdomainsToRemove, err := s.commonFilterParams() if err != nil { return nil, err } - validity := s.options.CookieValidity - if validity == 0 { - validity = defaultCookieValidity - } - - subdomainsToRemove := 1 - if s.options.CookieRemoveSubdomains != nil { - subdomainsToRemove = *s.options.CookieRemoveSubdomains - } - var claims []string if len(sargs) > 1 && sargs[1] != "" { claims = strings.Split(sargs[1], " ") @@ -255,6 +253,7 @@ func (s *tokenOidcSpec) createProfileFilter(sargs []string) (filters.Filter, err profileCopy := profile return newProfileFilter( + name, s.typ, &profileCopy, claims, @@ -267,6 +266,15 @@ func (s *tokenOidcSpec) createProfileFilter(sargs []string) (filters.Filter, err ), nil } +// CreateFilter creates an OpenID Connect authorization filter. +// +// first arg: a provider, for example "https://accounts.google.com", +// which has the path /.well-known/openid-configuration +// +// Example: +// +// oauthOidcAllClaims("https://accounts.identity-provider.com", "some-client-id", "some-client-secret", +// "http://callback.com/auth/provider/callback", "scope1 scope2", "claim1 claim2", "", "", "") -> "https://internal.example.org"; func (s *tokenOidcSpec) CreateFilter(args []interface{}) (filters.Filter, error) { sargs, err := getStrings(args) if err != nil { @@ -321,15 +329,11 @@ func (s *tokenOidcSpec) CreateFilter(args []interface{}) (filters.Filter, error) return nil, fmt.Errorf("invalid redirect url '%s': %w", sargs[paramCallbackURL], err) } - encrypter, err := s.secretsRegistry.GetEncrypter(1*time.Minute, s.SecretsFile) + encrypter, validity, subdomainsToRemove, err := s.commonFilterParams() if err != nil { return nil, err } - subdomainsToRemove := 1 - if s.options.CookieRemoveSubdomains != nil { - subdomainsToRemove = *s.options.CookieRemoveSubdomains - } if len(sargs) > paramSubdomainsToRemove && sargs[paramSubdomainsToRemove] != "" { subdomainsToRemove, err = strconv.Atoi(sargs[paramSubdomainsToRemove]) if err != nil { @@ -340,11 +344,6 @@ func (s *tokenOidcSpec) CreateFilter(args []interface{}) (filters.Filter, error) return nil, fmt.Errorf("domain level cannot be negative '%d'", subdomainsToRemove) } - validity := s.options.CookieValidity - if validity == 0 { - validity = defaultCookieValidity - } - oidcClientId := sargs[paramClientID] if oidcClientId == "" { oidcClientId = s.options.OidcClientId diff --git a/filters/auth/oidcprofile.go b/filters/auth/oidcprofile.go index a58956f0f3..7763c93231 100644 --- a/filters/auth/oidcprofile.go +++ b/filters/auth/oidcprofile.go @@ -94,9 +94,10 @@ func resolveField(tmplStr string, data profileTemplateData) (string, error) { // tokenOidcProfileFilter is the per-route filter created when an oauthOidc* filter // uses the "profile:" first argument syntax. It resolves profile field templates -// at request time, caches the resulting tokenOidcFilter delegates by a hash of the -// resolved parameters, and delegates all OIDC processing to them. +// at request time, caches the resulting tokenOidcFilter delegates by profile name and +// request host, and delegates all OIDC processing to them. type tokenOidcProfileFilter struct { + name string // profile name, e.g. "myprofile" typ roleCheckType profile *OidcProfile claims []string // from route args; never from profile @@ -108,7 +109,7 @@ type tokenOidcProfileFilter struct { oidcOptions OidcOptions spec *tokenOidcSpec // for resolveClientCredential - delegates sync.Map // map[string]*tokenOidcFilter, keyed by cacheKey + delegates sync.Map // map[string]*tokenOidcFilter, keyed by profile name + host } // resolvedProfile holds the template-resolved (but not secretRef-resolved) field values. @@ -123,17 +124,12 @@ type resolvedProfile struct { cookieName string } -// cacheKey returns a SHA-256 hex string over all resolved fields. -func (r *resolvedProfile) cacheKey() string { - h := sha256.New() - for _, s := range []string{ - r.clientID, r.clientSecret, r.callbackURL, r.scopes, - r.authCodeOpts, r.upstreamHeaders, r.subdomainsToRemove, r.cookieName, - } { - h.Write([]byte(s)) - h.Write([]byte{0}) // field separator - } - return fmt.Sprintf("%x", h.Sum(nil)) +// cacheKey returns a stable key for the delegate cache based on profile name and +// request host. Credentials are intentionally excluded: including them would cause +// unbounded cache growth on secret rotation and is unnecessary since the profile +// name and host uniquely identify the logical delegate configuration. +func (r *resolvedProfile) cacheKey(profileName, host string) string { + return profileName + "\x00" + host } // resolveAll resolves all profile template fields using request-time data. @@ -169,7 +165,8 @@ func (f *tokenOidcProfileFilter) resolveAll(data profileTemplateData) (*resolved // buildDelegate constructs a tokenOidcFilter from template-resolved profile fields. // secretRef resolution happens here so the actual credentials are in the delegate. -func (f *tokenOidcProfileFilter) buildDelegate(r *resolvedProfile) (*tokenOidcFilter, error) { +// host is the request hostname, used to derive a stable cookie name. +func (f *tokenOidcProfileFilter) buildDelegate(r *resolvedProfile, host string) (*tokenOidcFilter, error) { clientID, err := f.spec.resolveClientCredential(r.clientID) if err != nil { return nil, fmt.Errorf("ClientID secretRef: %w", err) @@ -179,16 +176,15 @@ func (f *tokenOidcProfileFilter) buildDelegate(r *resolvedProfile) (*tokenOidcFi return nil, fmt.Errorf("ClientSecret secretRef: %w", err) } - // Cookie name: use explicit value or derive from hash of params + // Cookie name: use explicit value or derive a stable name from profile name and host. + // Credentials are intentionally excluded so the name does not change on secret + // rotation (which would log out all users). cookieName := r.cookieName if cookieName == "" { h := sha256.New() - // Hash same fields as CreateFilter (skip CallbackURL and SubdomainsToRemove) - for _, s := range []string{ - f.profile.IdpURL, clientID, clientSecret, r.scopes, r.authCodeOpts, r.upstreamHeaders, - } { - h.Write([]byte(s)) - } + h.Write([]byte(f.name)) + h.Write([]byte{0}) + h.Write([]byte(host)) cookieName = oauthOidcCookieName + fmt.Sprintf("%x", h.Sum(nil))[:8] + "-" } @@ -295,7 +291,8 @@ func (f *tokenOidcProfileFilter) Request(ctx filters.FilterContext) { return } - key := resolved.cacheKey() + host := data.Request.Host + key := resolved.cacheKey(f.name, host) if d, ok := f.delegates.Load(key); ok { delegate := d.(*tokenOidcFilter) ctx.StateBag()[oidcProfileStateBagKey] = delegate @@ -303,7 +300,7 @@ func (f *tokenOidcProfileFilter) Request(ctx filters.FilterContext) { return } - delegate, err := f.buildDelegate(resolved) + delegate, err := f.buildDelegate(resolved, host) if err != nil { ctx.Logger().Errorf("oidc profile filter: failed to build delegate: %v", err) f.internalServerError(ctx) @@ -327,6 +324,7 @@ func (f *tokenOidcProfileFilter) Response(ctx filters.FilterContext) { // newProfileFilter creates a tokenOidcProfileFilter from a pre-discovered provider. // Called by tokenOidcSpec.CreateFilter when the first arg starts with "profile:". func newProfileFilter( + name string, typ roleCheckType, profile *OidcProfile, claims []string, @@ -338,6 +336,7 @@ func newProfileFilter( spec *tokenOidcSpec, ) *tokenOidcProfileFilter { return &tokenOidcProfileFilter{ + name: name, typ: typ, profile: profile, claims: claims, diff --git a/filters/auth/oidcprofile_test.go b/filters/auth/oidcprofile_test.go index 48b027868c..022c33ca0a 100644 --- a/filters/auth/oidcprofile_test.go +++ b/filters/auth/oidcprofile_test.go @@ -119,46 +119,28 @@ func TestResolveField(t *testing.T) { } func TestResolvedProfileCacheKey(t *testing.T) { - t.Run("same fields produce same key", func(t *testing.T) { - r1 := &resolvedProfile{ - clientID: "client-a", - clientSecret: "secret-a", - callbackURL: "https://app.example.com/callback", - scopes: "email profile", - } - r2 := &resolvedProfile{ - clientID: "client-a", - clientSecret: "secret-a", - callbackURL: "https://app.example.com/callback", - scopes: "email profile", - } - assert.Equal(t, r1.cacheKey(), r2.cacheKey()) + r := &resolvedProfile{} + + t.Run("same name and host produce same key", func(t *testing.T) { + assert.Equal(t, r.cacheKey("myprofile", "app.example.com"), r.cacheKey("myprofile", "app.example.com")) }) - t.Run("different clientID produces different key", func(t *testing.T) { - r1 := &resolvedProfile{clientID: "client-a"} - r2 := &resolvedProfile{clientID: "client-b"} - assert.NotEqual(t, r1.cacheKey(), r2.cacheKey()) + t.Run("different profile name produces different key", func(t *testing.T) { + assert.NotEqual(t, r.cacheKey("profile-a", "app.example.com"), r.cacheKey("profile-b", "app.example.com")) }) - t.Run("different scopes produces different key", func(t *testing.T) { - r1 := &resolvedProfile{clientID: "c", scopes: "email"} - r2 := &resolvedProfile{clientID: "c", scopes: "email profile"} - assert.NotEqual(t, r1.cacheKey(), r2.cacheKey()) + t.Run("different host produces different key", func(t *testing.T) { + assert.NotEqual(t, r.cacheKey("myprofile", "tenant-a.example.com"), r.cacheKey("myprofile", "tenant-b.example.com")) }) - t.Run("key is non-empty hex string", func(t *testing.T) { - r := &resolvedProfile{clientID: "c", clientSecret: "s"} - key := r.cacheKey() - assert.NotEmpty(t, key) - // SHA-256 hex = 64 chars - assert.Len(t, key, 64) + t.Run("credential values do not affect key", func(t *testing.T) { + r1 := &resolvedProfile{clientID: "client-a", clientSecret: "secret-a"} + r2 := &resolvedProfile{clientID: "client-b", clientSecret: "secret-b"} + assert.Equal(t, r1.cacheKey("myprofile", "app.example.com"), r2.cacheKey("myprofile", "app.example.com")) }) - t.Run("field order matters - clientID vs cookieName not interchangeable", func(t *testing.T) { - r1 := &resolvedProfile{clientID: "value", cookieName: ""} - r2 := &resolvedProfile{clientID: "", cookieName: "value"} - assert.NotEqual(t, r1.cacheKey(), r2.cacheKey()) + t.Run("key is non-empty", func(t *testing.T) { + assert.NotEmpty(t, r.cacheKey("myprofile", "app.example.com")) }) } @@ -226,7 +208,6 @@ func TestTokenOidcProfileFilterResolveAll(t *testing.T) { require.NoError(t, err) assert.NotEqual(t, r1.clientID, r2.clientID) - assert.NotEqual(t, r1.cacheKey(), r2.cacheKey()) }) t.Run("same annotations produce same cache key", func(t *testing.T) { @@ -247,7 +228,7 @@ func TestTokenOidcProfileFilterResolveAll(t *testing.T) { r2, err := f.resolveAll(data) require.NoError(t, err) - assert.Equal(t, r1.cacheKey(), r2.cacheKey()) + assert.Equal(t, r1.cacheKey(f.name, data.Request.Host), r2.cacheKey(f.name, data.Request.Host)) }) t.Run("subdomains to remove field resolved", func(t *testing.T) { @@ -297,5 +278,5 @@ func TestOidcProfileDelegateCaching(t *testing.T) { r2, err := f.resolveAll(data) require.NoError(t, err) - assert.Equal(t, r1.cacheKey(), r2.cacheKey(), "same input should produce the same cache key") + assert.Equal(t, r1.cacheKey(f.name, data.Request.Host), r2.cacheKey(f.name, data.Request.Host), "same input should produce the same cache key") } From 88066dcf9c490f63485451b8c88500899fa62ef7 Mon Sep 17 00:00:00 2001 From: Magnus Jungsbluth Date: Fri, 13 Mar 2026 22:35:35 +0100 Subject: [PATCH 4/7] Support Kubernetes labels as well Signed-off-by: magnus-jungsbluth_zse --- config/config.go | 7 +++++++ config/config_test.go | 1 + dataclients/kubernetes/ingress.go | 4 ++++ dataclients/kubernetes/ingressv1.go | 1 + dataclients/kubernetes/kube.go | 9 +++++++++ dataclients/kubernetes/routegroup.go | 2 ++ docs/reference/filters.md | 12 +++++++++++- docs/tutorials/auth.md | 11 ++++++++++- skipper.go | 13 +++++++++++++ 9 files changed, 58 insertions(+), 2 deletions(-) diff --git a/config/config.go b/config/config.go index 20b025b555..3500ea4d53 100644 --- a/config/config.go +++ b/config/config.go @@ -190,6 +190,8 @@ type Config struct { KubernetesAnnotationFiltersAppendString multiFlag `yaml:"kubernetes-annotation-filters-append"` KubernetesAnnotationsToRouteAnnotations *listFlag `yaml:"kubernetes-annotations-to-route-annotations"` KubernetesAnnotationsToRouteAnnotationsPrefix string `yaml:"kubernetes-annotations-to-route-annotations-prefix"` + KubernetesLabelsToRouteAnnotations *listFlag `yaml:"kubernetes-labels-to-route-annotations"` + KubernetesLabelsToRouteAnnotationsPrefix string `yaml:"kubernetes-labels-to-route-annotations-prefix"` KubernetesEastWestRangeAnnotationPredicates []kubernetes.AnnotationPredicates `yaml:"-"` KubernetesEastWestRangeAnnotationFiltersAppend []kubernetes.AnnotationFilters `yaml:"-"` KubernetesAnnotationPredicates []kubernetes.AnnotationPredicates `yaml:"-"` @@ -388,6 +390,7 @@ func NewConfig() *Config { cfg.EditRoute = routeChangerConfig{} cfg.KubernetesEastWestRangeDomains = commaListFlag() cfg.KubernetesAnnotationsToRouteAnnotations = commaListFlag() + cfg.KubernetesLabelsToRouteAnnotations = commaListFlag() cfg.RoutesURLs = commaListFlag() cfg.ForwardedHeadersList = commaListFlag() cfg.ForwardedHeadersExcludeCIDRList = commaListFlag() @@ -555,6 +558,8 @@ func NewConfig() *Config { flag.Var(&cfg.KubernetesAnnotationFiltersAppendString, "kubernetes-annotation-filters-append", "configures filters appended to non east-west routes of annotated resources. E.g. -kubernetes-annotation-filters-append='zone-a=true=foo() -> bar()' will add 'foo() -> bar()' filters to all non east-west routes of ingress or routegroup annotated with 'zone-a: true'. For east-west routes use -kubernetes-east-west-range-annotation-filters-append.") flag.Var(cfg.KubernetesAnnotationsToRouteAnnotations, "kubernetes-annotations-to-route-annotations", "comma-separated list of Kubernetes resource annotation keys whose values are automatically injected as annotate() filters into routes, making them available to oauthOidc* profile filters via {{index .Annotations \"key\"}}") flag.StringVar(&cfg.KubernetesAnnotationsToRouteAnnotationsPrefix, "kubernetes-annotations-to-route-annotations-prefix", "", "prefix prepended to the key in annotate() filters generated from -kubernetes-annotations-to-route-annotations; no separator is added between prefix and key") + flag.Var(cfg.KubernetesLabelsToRouteAnnotations, "kubernetes-labels-to-route-annotations", "comma-separated list of Kubernetes resource label keys whose values are automatically injected as annotate() filters into routes, making them available to oauthOidc* profile filters via {{index .Annotations \"key\"}}") + flag.StringVar(&cfg.KubernetesLabelsToRouteAnnotationsPrefix, "kubernetes-labels-to-route-annotations-prefix", "", "prefix prepended to the key in annotate() filters generated from -kubernetes-labels-to-route-annotations; no separator is added between prefix and key") flag.Var(&cfg.KubernetesEastWestRangeAnnotationPredicatesString, "kubernetes-east-west-range-annotation-predicates", "similar to -kubernetes-annotation-predicates configures predicates appended to east-west routes of annotated resources. See also -kubernetes-east-west-range-domains.") flag.Var(&cfg.KubernetesEastWestRangeAnnotationFiltersAppendString, "kubernetes-east-west-range-annotation-filters-append", "similar to -kubernetes-annotation-filters-append configures filters appended to east-west routes of annotated resources. See also -kubernetes-east-west-range-domains.") flag.BoolVar(&cfg.EnableKubernetesExternalNames, "enable-kubernetes-external-names", false, "only if enabled we allow to use external name services as backends in Ingress") @@ -1008,6 +1013,8 @@ func (c *Config) ToOptions() skipper.Options { KubernetesAnnotationFiltersAppend: c.KubernetesAnnotationFiltersAppend, KubernetesAnnotationsToRouteAnnotations: c.KubernetesAnnotationsToRouteAnnotations.values, KubernetesAnnotationsToRouteAnnotationsPrefix: c.KubernetesAnnotationsToRouteAnnotationsPrefix, + KubernetesLabelsToRouteAnnotations: c.KubernetesLabelsToRouteAnnotations.values, + KubernetesLabelsToRouteAnnotationsPrefix: c.KubernetesLabelsToRouteAnnotationsPrefix, EnableKubernetesExternalNames: c.EnableKubernetesExternalNames, KubernetesOnlyAllowedExternalNames: c.KubernetesOnlyAllowedExternalNames, KubernetesAllowedExternalNames: c.KubernetesAllowedExternalNames, diff --git a/config/config_test.go b/config/config_test.go index 6c27fad20c..43cb3437a8 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -109,6 +109,7 @@ func defaultConfig(with func(*Config)) *Config { SourcePollTimeout: 3000, KubernetesEastWestRangeDomains: commaListFlag(), KubernetesAnnotationsToRouteAnnotations: commaListFlag(), + KubernetesLabelsToRouteAnnotations: commaListFlag(), KubernetesHealthcheck: true, KubernetesHTTPSRedirect: true, KubernetesHTTPSRedirectCode: 308, diff --git a/dataclients/kubernetes/ingress.go b/dataclients/kubernetes/ingress.go index 9c6a88e600..a862f0b9c1 100644 --- a/dataclients/kubernetes/ingress.go +++ b/dataclients/kubernetes/ingress.go @@ -72,6 +72,8 @@ type ingress struct { kubernetesEastWestRangeAnnotationFiltersAppend []AnnotationFilters annotationsToRouteAnnotations []string annotationsToRouteAnnotationsPrefix string + labelsToRouteAnnotations []string + labelsToRouteAnnotationsPrefix string } var ( @@ -125,6 +127,8 @@ func newIngress(o Options) *ingress { kubernetesEastWestRangeAnnotationFiltersAppend: o.KubernetesEastWestRangeAnnotationFiltersAppend, annotationsToRouteAnnotations: o.AnnotationsToRouteAnnotations, annotationsToRouteAnnotationsPrefix: o.AnnotationsToRouteAnnotationsPrefix, + labelsToRouteAnnotations: o.LabelsToRouteAnnotations, + labelsToRouteAnnotationsPrefix: o.LabelsToRouteAnnotationsPrefix, } } diff --git a/dataclients/kubernetes/ingressv1.go b/dataclients/kubernetes/ingressv1.go index 2325fcf993..f6ffc17dde 100644 --- a/dataclients/kubernetes/ingressv1.go +++ b/dataclients/kubernetes/ingressv1.go @@ -198,6 +198,7 @@ func (ing *ingress) addEndpointsRuleV1(ic *ingressContext, host string, prule *d } injectAnnotateFilters(ic.ingressV1.Metadata.Annotations, ing.annotationsToRouteAnnotations, ing.annotationsToRouteAnnotationsPrefix, endpointsRoute) + injectAnnotateFilters(ic.ingressV1.Metadata.Labels, ing.labelsToRouteAnnotations, ing.labelsToRouteAnnotationsPrefix, endpointsRoute) if endpointsRoute.BackendType != eskip.ShuntBackend { // safe prepend, see: https://play.golang.org/p/zg5aGKJpRyK diff --git a/dataclients/kubernetes/kube.go b/dataclients/kubernetes/kube.go index 7f98111231..27e3ca3f24 100644 --- a/dataclients/kubernetes/kube.go +++ b/dataclients/kubernetes/kube.go @@ -232,6 +232,15 @@ type Options struct { // No separator is added between prefix and key. AnnotationsToRouteAnnotationsPrefix string + // LabelsToRouteAnnotations is a list of Kubernetes resource label keys whose values + // are automatically injected as annotate() filters into routes generated from those resources. + LabelsToRouteAnnotations []string + + // LabelsToRouteAnnotationsPrefix is an optional string prepended to the key in the + // generated annotate() filter call. The K8s label lookup key is unchanged. + // No separator is added between prefix and key. + LabelsToRouteAnnotationsPrefix string + // DefaultFiltersDir enables default filters mechanism and sets the location of the default filters. // The provided filters are then applied to all routes. DefaultFiltersDir string diff --git a/dataclients/kubernetes/routegroup.go b/dataclients/kubernetes/routegroup.go index e0a94573aa..b244148f3a 100644 --- a/dataclients/kubernetes/routegroup.go +++ b/dataclients/kubernetes/routegroup.go @@ -601,6 +601,7 @@ func (r *routeGroups) convert(s *clusterState, df defaultFilters, loggingEnabled for _, route := range ri { injectAnnotateFilters(rg.Metadata.Annotations, r.options.AnnotationsToRouteAnnotations, r.options.AnnotationsToRouteAnnotationsPrefix, route) + injectAnnotateFilters(rg.Metadata.Labels, r.options.LabelsToRouteAnnotations, r.options.LabelsToRouteAnnotationsPrefix, route) appendAnnotationPredicates(r.options.KubernetesAnnotationPredicates, rg.Metadata.Annotations, route) appendAnnotationFilters(r.options.KubernetesAnnotationFiltersAppend, rg.Metadata.Annotations, route) } @@ -646,6 +647,7 @@ func (r *routeGroups) convert(s *clusterState, df defaultFilters, loggingEnabled applyEastWestRangePredicates(internalRi, r.options.KubernetesEastWestRangePredicates) for _, route := range internalRi { injectAnnotateFilters(rg.Metadata.Annotations, r.options.AnnotationsToRouteAnnotations, r.options.AnnotationsToRouteAnnotationsPrefix, route) + injectAnnotateFilters(rg.Metadata.Labels, r.options.LabelsToRouteAnnotations, r.options.LabelsToRouteAnnotationsPrefix, route) appendAnnotationPredicates(r.options.KubernetesEastWestRangeAnnotationPredicates, rg.Metadata.Annotations, route) appendAnnotationFilters(r.options.KubernetesEastWestRangeAnnotationFiltersAppend, rg.Metadata.Annotations, route) } diff --git a/docs/reference/filters.md b/docs/reference/filters.md index 864ae5872b..501e8914ec 100644 --- a/docs/reference/filters.md +++ b/docs/reference/filters.md @@ -2158,7 +2158,7 @@ oidc-profiles: **Note:** `idp-url` must be a static URL — template expressions are rejected. The provider is discovered once at filter creation time. -##### Annotation injection from Kubernetes resources +##### Annotation and label injection from Kubernetes resources To make Kubernetes Ingress or RouteGroup annotation values available to profile templates via `{{index .Annotations "key"}}`, use the `-kubernetes-annotations-to-route-annotations` flag: @@ -2168,6 +2168,14 @@ skipper -kubernetes-annotations-to-route-annotations=oidc/client-id,oidc/client- When this flag is set, the Kubernetes dataclient automatically prepends `annotate(key, value)` filters to every route generated from a resource that carries one of the configured annotation keys. The injected annotation values then become accessible to the OIDC profile filter at request time. +To inject Kubernetes **label** values instead (or in addition), use the `-kubernetes-labels-to-route-annotations` flag: + +``` +skipper -kubernetes-labels-to-route-annotations=tenant,environment +``` + +This works identically to the annotation flag but reads from the resource's `labels` map. + **Kubernetes Ingress example:** ```yaml @@ -2200,6 +2208,8 @@ Skipper arguments: | `-oidc-profiles-file` | no | Path to a YAML file containing named OIDC profile configurations (a map of profile name to `OidcProfile` struct). Mutually exclusive with `-oidc-profiles`. | | `-kubernetes-annotations-to-route-annotations` | no | Comma-separated list of Kubernetes resource annotation keys whose values are automatically injected as `annotate()` filters into generated routes, making them accessible to OIDC profile templates. | | `-kubernetes-annotations-to-route-annotations-prefix` | no | Optional prefix prepended to the key in each generated `annotate()` filter call. No separator is added. Example: prefix `k8s:` + Ingress / RouteGroup annotation `oidc/cid` → `annotate("k8s:oidc/cid", value)`. | +| `-kubernetes-labels-to-route-annotations` | no | Comma-separated list of Kubernetes resource label keys whose values are automatically injected as `annotate()` filters into generated routes, making them accessible to OIDC profile templates. | +| `-kubernetes-labels-to-route-annotations-prefix` | no | Optional prefix prepended to the key in each generated `annotate()` filter call for labels. No separator is added. | ### Open Policy Agent diff --git a/docs/tutorials/auth.md b/docs/tutorials/auth.md index 17b9b428a7..6c3255329b 100644 --- a/docs/tutorials/auth.md +++ b/docs/tutorials/auth.md @@ -323,7 +323,7 @@ oidc-profiles: callback-url: https://app.example.com/auth/callback ``` -#### Automatic annotation injection in Kubernetes +#### Automatic annotation and label injection in Kubernetes When using Kubernetes Ingress or RouteGroups, you can drive profile template values from annotations on the resource itself. Start Skipper with `-kubernetes-annotations-to-route-annotations` listing the annotation keys to inject: @@ -342,6 +342,15 @@ skipper -kubernetes-annotations-to-route-annotations=oidc/client-id \ With this prefix, the annotation `oidc/client-id` on a resource produces `annotate("k8s:oidc/client-id", value)`, and the template would use `{{index .Annotations "k8s:oidc/client-id"}}`. +The equivalent flags `-kubernetes-labels-to-route-annotations` and `-kubernetes-labels-to-route-annotations-prefix` work identically but read from the resource's Kubernetes **labels** instead of annotations: + +``` +skipper -kubernetes-labels-to-route-annotations=tenant \ + -kubernetes-labels-to-route-annotations-prefix=label: +``` + +Both flags can be used together to inject values from both annotations and labels into the same route. + Full example — Kubernetes Ingress: ```yaml diff --git a/skipper.go b/skipper.go index f60bc1d69c..6bff4c3bf8 100644 --- a/skipper.go +++ b/skipper.go @@ -305,6 +305,17 @@ type Options struct { // No separator is added between prefix and key. KubernetesAnnotationsToRouteAnnotationsPrefix string + // KubernetesLabelsToRouteAnnotations is a list of Kubernetes resource label keys + // whose values are automatically injected as annotate() filters into routes generated from + // those resources. This makes the label values accessible to oauthOidc* profile filters + // via {{index .Annotations "key"}}. + KubernetesLabelsToRouteAnnotations []string + + // KubernetesLabelsToRouteAnnotationsPrefix is an optional prefix prepended to the key + // in the generated annotate() filter call. + // No separator is added between prefix and key. + KubernetesLabelsToRouteAnnotationsPrefix string + // EnableKubernetesExternalNames enables to use Kubernetes service type ExternalName as backend in Ingress and RouteGroup. EnableKubernetesExternalNames bool @@ -1110,6 +1121,8 @@ func (o *Options) KubernetesDataClientOptions() kubernetes.Options { KubernetesAnnotationFiltersAppend: o.KubernetesAnnotationFiltersAppend, AnnotationsToRouteAnnotations: o.KubernetesAnnotationsToRouteAnnotations, AnnotationsToRouteAnnotationsPrefix: o.KubernetesAnnotationsToRouteAnnotationsPrefix, + LabelsToRouteAnnotations: o.KubernetesLabelsToRouteAnnotations, + LabelsToRouteAnnotationsPrefix: o.KubernetesLabelsToRouteAnnotationsPrefix, HTTPSRedirectCode: o.KubernetesHTTPSRedirectCode, DisableCatchAllRoutes: o.KubernetesDisableCatchAllRoutes, IngressClass: o.KubernetesIngressClass, From 4aaa440d6e2afa35b30a9fbdf56dd021b9276c33 Mon Sep 17 00:00:00 2001 From: Magnus Jungsbluth Date: Fri, 13 Mar 2026 22:41:34 +0100 Subject: [PATCH 5/7] Adjust examples to /.well-known/oauth2-callback Signed-off-by: magnus-jungsbluth_zse --- docs/reference/filters.md | 11 ++++++----- docs/tutorials/auth.md | 6 +++--- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/docs/reference/filters.md b/docs/reference/filters.md index 501e8914ec..f728df33c3 100644 --- a/docs/reference/filters.md +++ b/docs/reference/filters.md @@ -1977,6 +1977,7 @@ oauthOidcAnyClaims("https://oidc-provider.example.com", ``` When using `secretRef:`, Skipper expects the referenced secret to be available via the configured secrets reader; if the secret cannot be resolved, filter creation fails. + * **Callback URL** The entire path to the callback from the provider on which the token will be received. It can be any value which is a subpath on which the filter is applied. * **Scopes** The OpenID scopes separated by spaces which need to be specified when requesting the token from the provider. @@ -2109,7 +2110,7 @@ Pass a YAML map of `name → OidcProfile` to the `-oidc-profiles` flag: ``` skipper -oidc-secrets-file /path/to/secrets \ - -oidc-profiles '{myprofile: {idp-url: "https://idp.example.com", client-id: "my-client", client-secret: "my-secret", callback-url: "https://app.example.com/auth/callback", scopes: "email profile"}}' + -oidc-profiles '{myprofile: {idp-url: "https://idp.example.com", client-id: "my-client", client-secret: "secretRef:my-secret", callback-url: "https://app.example.com/.well-known/oauth2-callback", scopes: "email profile"}}' ``` Or use a YAML config file: @@ -2119,8 +2120,8 @@ oidc-profiles: myprofile: idp-url: https://idp.example.com client-id: my-client-id - client-secret: my-client-secret - callback-url: https://app.example.com/auth/callback + client-secret: secretRef:my-client-secret + callback-url: https://app.example.com/.well-known/oauth2-callback scopes: email profile ``` @@ -2132,7 +2133,7 @@ oidc-profiles: idp-url: https://idp.example.com client-id: secretRef:/mnt/secrets/oidc-client-id client-secret: secretRef:/mnt/secrets/oidc-client-secret - callback-url: https://app.example.com/auth/callback + callback-url: https://app.example.com/.well-known/oauth2-callback ``` ##### Go template fields @@ -2152,7 +2153,7 @@ oidc-profiles: idp-url: https://idp.example.com client-id: '{{index .Annotations "oidc/client-id"}}' client-secret: '{{index .Annotations "oidc/client-secret"}}' - callback-url: 'https://{{.Request.Host}}/auth/callback' + callback-url: 'https://{{.Request.Host}}/.well-known/oauth2-callback' scopes: email profile ``` diff --git a/docs/tutorials/auth.md b/docs/tutorials/auth.md index 6c3255329b..b9688f2ef5 100644 --- a/docs/tutorials/auth.md +++ b/docs/tutorials/auth.md @@ -279,7 +279,7 @@ oidc-profiles: idp-url: https://idp.example.com client-id: my-client-id client-secret: my-client-secret - callback-url: https://app.example.com/auth/callback + callback-url: https://app.example.com/.well-known/oauth2-callback scopes: email profile ``` @@ -306,7 +306,7 @@ oidc-profiles: idp-url: https://idp.example.com client-id: '{{index .Annotations "oidc/client-id"}}' client-secret: '{{index .Annotations "oidc/client-secret"}}' - callback-url: 'https://{{.Request.Host}}/auth/callback' + callback-url: 'https://{{.Request.Host}}/.well-known/oauth2-callback' scopes: email profile ``` @@ -320,7 +320,7 @@ oidc-profiles: idp-url: https://idp.example.com client-id: secretRef:/mnt/secrets/oidc-client-id client-secret: secretRef:/mnt/secrets/oidc-client-secret - callback-url: https://app.example.com/auth/callback + callback-url: https://app.example.com/.well-known/oauth2-callback ``` #### Automatic annotation and label injection in Kubernetes From c24bece938135797a48d0aaa1c94a3ed273a334a Mon Sep 17 00:00:00 2001 From: Magnus Jungsbluth Date: Sat, 14 Mar 2026 23:19:37 +0100 Subject: [PATCH 6/7] Include clientID in hashes / lookups Signed-off-by: magnus-jungsbluth_zse --- filters/auth/oidc.go | 80 +++++++++++------ filters/auth/oidcprofile.go | 64 +++++++------ filters/auth/oidcprofile_test.go | 150 ++++++++++++++++++++++++++++--- 3 files changed, 220 insertions(+), 74 deletions(-) diff --git a/filters/auth/oidc.go b/filters/auth/oidc.go index 9f7f3615c2..edee1b7155 100644 --- a/filters/auth/oidc.go +++ b/filters/auth/oidc.go @@ -220,6 +220,44 @@ func (s *tokenOidcSpec) commonFilterParams() (encrypter secrets.Encryption, vali return } +// parseAuthCodeOpts parses a space-separated list of "key=value" auth code option pairs. +// Values that equal "skipper-request-query" are collected as query parameter names instead +// of fixed auth code options. The key=value split uses SplitN with limit 2 so that values +// containing "=" (e.g. Base64-encoded strings) are handled correctly. +func parseAuthCodeOpts(opts string) (authCodeOptions []oauth2.AuthCodeOption, queryParams []string, err error) { + if opts == "" { + return nil, nil, nil + } + for _, p := range strings.Split(opts, " ") { + splitP := strings.SplitN(p, "=", 2) + if len(splitP) != 2 { + return nil, nil, fmt.Errorf("%w: auth code option %q must be key=value", filters.ErrInvalidFilterParameters, p) + } + if splitP[1] == "skipper-request-query" { + queryParams = append(queryParams, splitP[0]) + } else { + authCodeOptions = append(authCodeOptions, oauth2.SetAuthURLParam(splitP[0], splitP[1])) + } + } + return authCodeOptions, queryParams, nil +} + +// parseUpstreamHeaders parses a space-separated list of "key:value" upstream header pairs. +func parseUpstreamHeaders(headers string) (map[string]string, error) { + if headers == "" { + return nil, nil + } + result := make(map[string]string) + for _, header := range strings.Split(headers, " ") { + k, v, found := strings.Cut(header, ":") + if !found || k == "" || v == "" { + return nil, fmt.Errorf("%w: malformed upstream header %q", filters.ErrInvalidFilterParameters, header) + } + result[k] = v + } + return result, nil +} + // createProfileFilter handles the "profile:" first-arg syntax. func (s *tokenOidcSpec) createProfileFilter(sargs []string) (filters.Filter, error) { name := strings.TrimPrefix(sargs[0], oidcProfilePrefix) @@ -397,36 +435,26 @@ func (s *tokenOidcSpec) CreateFilter(args []interface{}) (filters.Filter, error) f.claims = strings.Split(sargs[paramClaims], " ") } - f.authCodeOptions = make([]oauth2.AuthCodeOption, 0) - if len(sargs) > paramAuthCodeOpts && sargs[paramAuthCodeOpts] != "" { - extraParameters := strings.Split(sargs[paramAuthCodeOpts], " ") - - for _, p := range extraParameters { - splitP := strings.Split(p, "=") - log.Debug(splitP) - if len(splitP) != 2 { - return nil, filters.ErrInvalidFilterParameters - } - if splitP[1] == "skipper-request-query" { - f.queryParams = append(f.queryParams, splitP[0]) - } else { - f.authCodeOptions = append(f.authCodeOptions, oauth2.SetAuthURLParam(splitP[0], splitP[1])) - } - } + var authCodeOptsStr string + if len(sargs) > paramAuthCodeOpts { + authCodeOptsStr = sargs[paramAuthCodeOpts] + } + f.authCodeOptions, f.queryParams, err = parseAuthCodeOpts(authCodeOptsStr) + if err != nil { + return nil, err } log.Debugf("Auth Code Options: %v", f.authCodeOptions) // inject additional headers from the access token for upstream applications - if len(sargs) > paramUpstrHeaders && sargs[paramUpstrHeaders] != "" { - f.upstreamHeaders = make(map[string]string) - - for _, header := range strings.Split(sargs[paramUpstrHeaders], " ") { - k, v, found := strings.Cut(header, ":") - if !found || k == "" || v == "" { - return nil, fmt.Errorf("%w: malformed filter for upstream headers %s", filters.ErrInvalidFilterParameters, header) - } - f.upstreamHeaders[k] = v - } + var upstreamHeadersStr string + if len(sargs) > paramUpstrHeaders { + upstreamHeadersStr = sargs[paramUpstrHeaders] + } + f.upstreamHeaders, err = parseUpstreamHeaders(upstreamHeadersStr) + if err != nil { + return nil, err + } + if f.upstreamHeaders != nil { log.Debugf("Upstream Headers: %v", f.upstreamHeaders) } diff --git a/filters/auth/oidcprofile.go b/filters/auth/oidcprofile.go index 7763c93231..a9d1ec5ba2 100644 --- a/filters/auth/oidcprofile.go +++ b/filters/auth/oidcprofile.go @@ -124,12 +124,22 @@ type resolvedProfile struct { cookieName string } -// cacheKey returns a stable key for the delegate cache based on profile name and -// request host. Credentials are intentionally excluded: including them would cause -// unbounded cache growth on secret rotation and is unnecessary since the profile -// name and host uniquely identify the logical delegate configuration. -func (r *resolvedProfile) cacheKey(profileName, host string) string { - return profileName + "\x00" + host +// cacheKey returns a stable key for the delegate cache. +// profileName and host identify the logical OIDC configuration; clientID +// (template-resolved, before any secretRef expansion) differentiates tenants +// that share the same host and profile name but inject different credentials +// via annotate() filters. Without clientID in the key, the first request's +// delegate would be cached and reused for all tenants on the same host+profile, +// sending them through the wrong OAuth2 client registration. +// +// \x00 is a safe separator: YAML map keys, HTTP Host header values, and OAuth2 +// client IDs cannot contain null bytes, so no two distinct (profileName, host, +// clientID) triples can produce the same key string. +// +// clientSecret and other secret fields are intentionally excluded to avoid +// unbounded cache growth on rotation and to keep secrets out of observable keys. +func cacheKey(profileName, host, clientID string) string { + return profileName + "\x00" + host + "\x00" + clientID } // resolveAll resolves all profile template fields using request-time data. @@ -176,15 +186,19 @@ func (f *tokenOidcProfileFilter) buildDelegate(r *resolvedProfile, host string) return nil, fmt.Errorf("ClientSecret secretRef: %w", err) } - // Cookie name: use explicit value or derive a stable name from profile name and host. - // Credentials are intentionally excluded so the name does not change on secret - // rotation (which would log out all users). + // Cookie name: use explicit value or derive a stable name from profile name, host, + // and template-resolved clientID. clientID is included (before secretRef expansion) + // to give each tenant on the same host+profile a distinct cookie namespace, preventing + // one tenant's session cookie from being accepted on another tenant's route. + // clientSecret is intentionally excluded so the name does not change on rotation. cookieName := r.cookieName if cookieName == "" { h := sha256.New() h.Write([]byte(f.name)) h.Write([]byte{0}) h.Write([]byte(host)) + h.Write([]byte{0}) + h.Write([]byte(r.clientID)) cookieName = oauthOidcCookieName + fmt.Sprintf("%x", h.Sum(nil))[:8] + "-" } @@ -221,32 +235,14 @@ func (f *tokenOidcProfileFilter) buildDelegate(r *resolvedProfile, host string) } verifier := f.provider.Verifier(&oidc.Config{ClientID: clientID}) - authCodeOptions := make([]oauth2.AuthCodeOption, 0) - var queryParams []string - if r.authCodeOpts != "" { - for _, p := range strings.Split(r.authCodeOpts, " ") { - splitP := strings.SplitN(p, "=", 2) - if len(splitP) != 2 { - return nil, fmt.Errorf("invalid auth code opt %q", p) - } - if splitP[1] == "skipper-request-query" { - queryParams = append(queryParams, splitP[0]) - } else { - authCodeOptions = append(authCodeOptions, oauth2.SetAuthURLParam(splitP[0], splitP[1])) - } - } + authCodeOptions, queryParams, err := parseAuthCodeOpts(r.authCodeOpts) + if err != nil { + return nil, fmt.Errorf("AuthCodeOpts: %w", err) } - var upstreamHeaders map[string]string - if r.upstreamHeaders != "" { - upstreamHeaders = make(map[string]string) - for _, header := range strings.Split(r.upstreamHeaders, " ") { - k, v, found := strings.Cut(header, ":") - if !found || k == "" || v == "" { - return nil, fmt.Errorf("malformed upstream header %q", header) - } - upstreamHeaders[k] = v - } + upstreamHeaders, err := parseUpstreamHeaders(r.upstreamHeaders) + if err != nil { + return nil, fmt.Errorf("UpstreamHeaders: %w", err) } return &tokenOidcFilter{ @@ -292,7 +288,7 @@ func (f *tokenOidcProfileFilter) Request(ctx filters.FilterContext) { } host := data.Request.Host - key := resolved.cacheKey(f.name, host) + key := cacheKey(f.name, host, resolved.clientID) if d, ok := f.delegates.Load(key); ok { delegate := d.(*tokenOidcFilter) ctx.StateBag()[oidcProfileStateBagKey] = delegate diff --git a/filters/auth/oidcprofile_test.go b/filters/auth/oidcprofile_test.go index 022c33ca0a..43859b3d13 100644 --- a/filters/auth/oidcprofile_test.go +++ b/filters/auth/oidcprofile_test.go @@ -118,29 +118,42 @@ func TestResolveField(t *testing.T) { }) } -func TestResolvedProfileCacheKey(t *testing.T) { - r := &resolvedProfile{} - - t.Run("same name and host produce same key", func(t *testing.T) { - assert.Equal(t, r.cacheKey("myprofile", "app.example.com"), r.cacheKey("myprofile", "app.example.com")) +func TestCacheKey(t *testing.T) { + t.Run("same profile, host and clientID produce same key", func(t *testing.T) { + assert.Equal(t, cacheKey("myprofile", "app.example.com", "client-a"), cacheKey("myprofile", "app.example.com", "client-a")) }) t.Run("different profile name produces different key", func(t *testing.T) { - assert.NotEqual(t, r.cacheKey("profile-a", "app.example.com"), r.cacheKey("profile-b", "app.example.com")) + assert.NotEqual(t, cacheKey("profile-a", "app.example.com", "client"), cacheKey("profile-b", "app.example.com", "client")) }) t.Run("different host produces different key", func(t *testing.T) { - assert.NotEqual(t, r.cacheKey("myprofile", "tenant-a.example.com"), r.cacheKey("myprofile", "tenant-b.example.com")) + assert.NotEqual(t, cacheKey("myprofile", "tenant-a.example.com", "client"), cacheKey("myprofile", "tenant-b.example.com", "client")) + }) + + t.Run("different clientID produces different key (tenant isolation)", func(t *testing.T) { + // Two tenants sharing the same host and profile but injecting different clientIDs + // via annotate() filters must get distinct cache entries so they are not served + // by each other's OAuth2 delegate. + assert.NotEqual(t, cacheKey("myprofile", "app.example.com", "client-a"), cacheKey("myprofile", "app.example.com", "client-b")) }) - t.Run("credential values do not affect key", func(t *testing.T) { - r1 := &resolvedProfile{clientID: "client-a", clientSecret: "secret-a"} - r2 := &resolvedProfile{clientID: "client-b", clientSecret: "secret-b"} - assert.Equal(t, r1.cacheKey("myprofile", "app.example.com"), r2.cacheKey("myprofile", "app.example.com")) + t.Run("clientSecret does not affect key (rotation safety)", func(t *testing.T) { + // Rotating the secret must not produce a new cache entry, which would invalidate + // all in-flight sessions. Only the public clientID is included in the key. + assert.Equal(t, cacheKey("myprofile", "app.example.com", "client-a"), cacheKey("myprofile", "app.example.com", "client-a")) + }) + + t.Run("null byte separator prevents cross-field collisions", func(t *testing.T) { + // "ab" + "" must not equal "a" + "b" — the \x00 separator prevents this. + // YAML keys, HTTP Host values and OAuth2 client IDs cannot themselves contain + // null bytes, so no legitimate inputs can produce the same key string. + assert.NotEqual(t, cacheKey("a", "b", "c"), cacheKey("a\x00b", "", "c")) + assert.NotEqual(t, cacheKey("a", "b", "c"), cacheKey("a", "b\x00c", "")) }) t.Run("key is non-empty", func(t *testing.T) { - assert.NotEmpty(t, r.cacheKey("myprofile", "app.example.com")) + assert.NotEmpty(t, cacheKey("myprofile", "app.example.com", "client")) }) } @@ -228,7 +241,7 @@ func TestTokenOidcProfileFilterResolveAll(t *testing.T) { r2, err := f.resolveAll(data) require.NoError(t, err) - assert.Equal(t, r1.cacheKey(f.name, data.Request.Host), r2.cacheKey(f.name, data.Request.Host)) + assert.Equal(t, cacheKey(f.name, data.Request.Host, r1.clientID), cacheKey(f.name, data.Request.Host, r2.clientID)) }) t.Run("subdomains to remove field resolved", func(t *testing.T) { @@ -278,5 +291,114 @@ func TestOidcProfileDelegateCaching(t *testing.T) { r2, err := f.resolveAll(data) require.NoError(t, err) - assert.Equal(t, r1.cacheKey(f.name, data.Request.Host), r2.cacheKey(f.name, data.Request.Host), "same input should produce the same cache key") + assert.Equal(t, cacheKey(f.name, data.Request.Host, r1.clientID), cacheKey(f.name, data.Request.Host, r2.clientID), "same input should produce the same cache key") +} + +// TestTenantIsolation verifies that two tenants sharing the same host and profile name +// but injecting different clientIDs via annotate() filters receive distinct cache keys +// and distinct cookie names, preventing session cookies from being accepted +// cross-tenant. +func TestTenantIsolation(t *testing.T) { + profile := &OidcProfile{ + IdpURL: "https://idp.example.com", + ClientID: `{{index .Annotations "client-id"}}`, + CallbackURL: "https://app.example.com/callback", + } + f := &tokenOidcProfileFilter{name: "shared-profile", profile: profile} + host := "app.example.com" + + dataTenantA := profileTemplateData{ + Request: profileRequestData{Host: host}, + Annotations: map[string]string{"client-id": "tenant-a-client"}, + } + dataTenantB := profileTemplateData{ + Request: profileRequestData{Host: host}, + Annotations: map[string]string{"client-id": "tenant-b-client"}, + } + + rA, err := f.resolveAll(dataTenantA) + require.NoError(t, err) + rB, err := f.resolveAll(dataTenantB) + require.NoError(t, err) + + keyA := cacheKey(f.name, host, rA.clientID) + keyB := cacheKey(f.name, host, rB.clientID) + assert.NotEqual(t, keyA, keyB, "different tenants must get different delegate cache keys") +} + +func TestParseAuthCodeOpts(t *testing.T) { + t.Run("empty string returns nil slices", func(t *testing.T) { + opts, qp, err := parseAuthCodeOpts("") + require.NoError(t, err) + assert.Nil(t, opts) + assert.Nil(t, qp) + }) + + t.Run("single static option", func(t *testing.T) { + opts, qp, err := parseAuthCodeOpts("prompt=consent") + require.NoError(t, err) + assert.Len(t, opts, 1) + assert.Empty(t, qp) + }) + + t.Run("skipper-request-query becomes query param", func(t *testing.T) { + opts, qp, err := parseAuthCodeOpts("acr_values=skipper-request-query") + require.NoError(t, err) + assert.Empty(t, opts) + assert.Equal(t, []string{"acr_values"}, qp) + }) + + t.Run("multiple options mixed", func(t *testing.T) { + opts, qp, err := parseAuthCodeOpts("prompt=consent acr_values=skipper-request-query") + require.NoError(t, err) + assert.Len(t, opts, 1) + assert.Equal(t, []string{"acr_values"}, qp) + }) + + t.Run("value containing equals sign is handled correctly", func(t *testing.T) { + // SplitN with limit 2 ensures a Base64 value like "abc==" is not split further. + opts, _, err := parseAuthCodeOpts("nonce=abc==") + require.NoError(t, err) + assert.Len(t, opts, 1) + }) + + t.Run("missing equals sign returns error", func(t *testing.T) { + _, _, err := parseAuthCodeOpts("no-equals-sign") + require.Error(t, err) + }) +} + +func TestParseUpstreamHeaders(t *testing.T) { + t.Run("empty string returns nil", func(t *testing.T) { + h, err := parseUpstreamHeaders("") + require.NoError(t, err) + assert.Nil(t, h) + }) + + t.Run("single header", func(t *testing.T) { + h, err := parseUpstreamHeaders("X-User-ID:sub") + require.NoError(t, err) + assert.Equal(t, map[string]string{"X-User-ID": "sub"}, h) + }) + + t.Run("multiple headers", func(t *testing.T) { + h, err := parseUpstreamHeaders("X-User-ID:sub X-Email:email") + require.NoError(t, err) + assert.Equal(t, map[string]string{"X-User-ID": "sub", "X-Email": "email"}, h) + }) + + t.Run("missing colon returns error", func(t *testing.T) { + _, err := parseUpstreamHeaders("no-colon") + require.Error(t, err) + }) + + t.Run("empty key returns error", func(t *testing.T) { + _, err := parseUpstreamHeaders(":value") + require.Error(t, err) + }) + + t.Run("empty value returns error", func(t *testing.T) { + _, err := parseUpstreamHeaders("key:") + require.Error(t, err) + }) } From 0749e54997364f677c9d47da1a4f95ee44f20e14 Mon Sep 17 00:00:00 2001 From: Magnus Jungsbluth Date: Thu, 19 Mar 2026 11:16:34 +0100 Subject: [PATCH 7/7] Add e2e tests Signed-off-by: magnus-jungsbluth_zse --- dataclients/kubernetes/ingress_test.go | 10 + .../kubernetes/kubernetestest/fixtures.go | 8 + .../annotation-prefix.eskip | 9 + .../annotation-prefix.kube | 4 + .../annotation-prefix.yaml | 45 ++ .../annotation-to-annotate.eskip | 9 + .../annotation-to-annotate.kube | 4 + .../annotation-to-annotate.yaml | 45 ++ .../label-to-annotate.eskip | 9 + .../label-to-annotate.kube | 4 + .../label-to-annotate.yaml | 45 ++ filters/auth/oidcprofile_e2e_test.go | 394 ++++++++++++++++++ 12 files changed, 586 insertions(+) create mode 100644 dataclients/kubernetes/testdata/ingressV1/annotate-from-annotations/annotation-prefix.eskip create mode 100644 dataclients/kubernetes/testdata/ingressV1/annotate-from-annotations/annotation-prefix.kube create mode 100644 dataclients/kubernetes/testdata/ingressV1/annotate-from-annotations/annotation-prefix.yaml create mode 100644 dataclients/kubernetes/testdata/ingressV1/annotate-from-annotations/annotation-to-annotate.eskip create mode 100644 dataclients/kubernetes/testdata/ingressV1/annotate-from-annotations/annotation-to-annotate.kube create mode 100644 dataclients/kubernetes/testdata/ingressV1/annotate-from-annotations/annotation-to-annotate.yaml create mode 100644 dataclients/kubernetes/testdata/ingressV1/annotate-from-annotations/label-to-annotate.eskip create mode 100644 dataclients/kubernetes/testdata/ingressV1/annotate-from-annotations/label-to-annotate.kube create mode 100644 dataclients/kubernetes/testdata/ingressV1/annotate-from-annotations/label-to-annotate.yaml create mode 100644 filters/auth/oidcprofile_e2e_test.go diff --git a/dataclients/kubernetes/ingress_test.go b/dataclients/kubernetes/ingress_test.go index d589db01aa..2a73a16fc4 100644 --- a/dataclients/kubernetes/ingress_test.go +++ b/dataclients/kubernetes/ingress_test.go @@ -31,3 +31,13 @@ func TestIngressV1AnnotationConfig(t *testing.T) { "testdata/ingressV1/annotation-backends", ) } + +// TestIngressV1AnnotateFromAnnotations validates that Kubernetes resource +// annotations and labels configured via AnnotationsToRouteAnnotations / +// LabelsToRouteAnnotations are injected as annotate() filters into the +// generated route's filter chain. This is the Kubernetes-side companion to +// the OIDC profile e2e tests: the annotate() filters set values that profile +// templates can read via {{index .Annotations "key"}}. +func TestIngressV1AnnotateFromAnnotations(t *testing.T) { + kubernetestest.FixturesToTest(t, "testdata/ingressV1/annotate-from-annotations") +} diff --git a/dataclients/kubernetes/kubernetestest/fixtures.go b/dataclients/kubernetes/kubernetestest/fixtures.go index 0dc8d0c6d0..7cbb8e238b 100644 --- a/dataclients/kubernetes/kubernetestest/fixtures.go +++ b/dataclients/kubernetes/kubernetestest/fixtures.go @@ -60,6 +60,10 @@ type kubeOptionsParser struct { KubernetesAnnotationFiltersAppend []kubernetes.AnnotationFilters `yaml:"kubernetesAnnotationFiltersAppend"` KubernetesEastWestRangeAnnotationPredicates []kubernetes.AnnotationPredicates `yaml:"kubernetesEastWestRangeAnnotationPredicates"` KubernetesEastWestRangeAnnotationFiltersAppend []kubernetes.AnnotationFilters `yaml:"kubernetesEastWestRangeAnnotationFiltersAppend"` + AnnotationsToRouteAnnotations []string `yaml:"annotationsToRouteAnnotations"` + AnnotationsToRouteAnnotationsPrefix string `yaml:"annotationsToRouteAnnotationsPrefix"` + LabelsToRouteAnnotations []string `yaml:"labelsToRouteAnnotations"` + LabelsToRouteAnnotationsPrefix string `yaml:"labelsToRouteAnnotationsPrefix"` } func baseNoExt(n string) string { @@ -253,6 +257,10 @@ func testFixture(t *testing.T, f fixtureSet) { o.DefaultLoadBalancerAlgorithm = kop.DefaultLoadBalancerAlgorithm o.ForwardBackendURL = kop.ForwardBackendURL o.TopologyZone = kop.TopologyZone + o.AnnotationsToRouteAnnotations = kop.AnnotationsToRouteAnnotations + o.AnnotationsToRouteAnnotationsPrefix = kop.AnnotationsToRouteAnnotationsPrefix + o.LabelsToRouteAnnotations = kop.LabelsToRouteAnnotations + o.LabelsToRouteAnnotationsPrefix = kop.LabelsToRouteAnnotationsPrefix if kop.BackendTrafficAlgorithm != "" { o.BackendTrafficAlgorithm, err = kubernetes.ParseBackendTrafficAlgorithm(kop.BackendTrafficAlgorithm) diff --git a/dataclients/kubernetes/testdata/ingressV1/annotate-from-annotations/annotation-prefix.eskip b/dataclients/kubernetes/testdata/ingressV1/annotate-from-annotations/annotation-prefix.eskip new file mode 100644 index 0000000000..4a1c02d65d --- /dev/null +++ b/dataclients/kubernetes/testdata/ingressV1/annotate-from-annotations/annotation-prefix.eskip @@ -0,0 +1,9 @@ +kube___catchall__test_example_org____: + Host("^(test[.]example[.]org[.]?(:[0-9]+)?)$") + -> ; + +kube_namespace1__ingress1__test_example_org___test1__service1: + Host("^(test[.]example[.]org[.]?(:[0-9]+)?)$") + && PathRegexp("^(/test1)") + -> annotate("k8s:oidc/client-id", "my-client") + -> "http://42.0.1.2:8080"; diff --git a/dataclients/kubernetes/testdata/ingressV1/annotate-from-annotations/annotation-prefix.kube b/dataclients/kubernetes/testdata/ingressV1/annotate-from-annotations/annotation-prefix.kube new file mode 100644 index 0000000000..981ac49962 --- /dev/null +++ b/dataclients/kubernetes/testdata/ingressV1/annotate-from-annotations/annotation-prefix.kube @@ -0,0 +1,4 @@ +# Test that annotationsToRouteAnnotationsPrefix is prepended to the annotate() key. +annotationsToRouteAnnotations: + - "oidc/client-id" +annotationsToRouteAnnotationsPrefix: "k8s:" diff --git a/dataclients/kubernetes/testdata/ingressV1/annotate-from-annotations/annotation-prefix.yaml b/dataclients/kubernetes/testdata/ingressV1/annotate-from-annotations/annotation-prefix.yaml new file mode 100644 index 0000000000..25590b3f73 --- /dev/null +++ b/dataclients/kubernetes/testdata/ingressV1/annotate-from-annotations/annotation-prefix.yaml @@ -0,0 +1,45 @@ +apiVersion: networking.k8s.io/v1 +kind: Ingress +metadata: + namespace: namespace1 + name: ingress1 + annotations: + oidc/client-id: my-client +spec: + rules: + - host: test.example.org + http: + paths: + - path: "/test1" + pathType: ImplementationSpecific + backend: + service: + name: service1 + port: + name: port1 +--- +apiVersion: v1 +kind: Service +metadata: + namespace: namespace1 + name: service1 +spec: + clusterIP: 1.2.3.4 + ports: + - name: port1 + port: 8080 + targetPort: 8080 + type: ClusterIP +--- +apiVersion: v1 +kind: Endpoints +metadata: + namespace: namespace1 + name: service1 +subsets: + - addresses: + - ip: 42.0.1.2 + ports: + - name: port1 + port: 8080 + protocol: TCP diff --git a/dataclients/kubernetes/testdata/ingressV1/annotate-from-annotations/annotation-to-annotate.eskip b/dataclients/kubernetes/testdata/ingressV1/annotate-from-annotations/annotation-to-annotate.eskip new file mode 100644 index 0000000000..eb6468f436 --- /dev/null +++ b/dataclients/kubernetes/testdata/ingressV1/annotate-from-annotations/annotation-to-annotate.eskip @@ -0,0 +1,9 @@ +kube___catchall__test_example_org____: + Host("^(test[.]example[.]org[.]?(:[0-9]+)?)$") + -> ; + +kube_namespace1__ingress1__test_example_org___test1__service1: + Host("^(test[.]example[.]org[.]?(:[0-9]+)?)$") + && PathRegexp("^(/test1)") + -> annotate("oidc/client-id", "my-client") + -> "http://42.0.1.2:8080"; diff --git a/dataclients/kubernetes/testdata/ingressV1/annotate-from-annotations/annotation-to-annotate.kube b/dataclients/kubernetes/testdata/ingressV1/annotate-from-annotations/annotation-to-annotate.kube new file mode 100644 index 0000000000..b863de38c9 --- /dev/null +++ b/dataclients/kubernetes/testdata/ingressV1/annotate-from-annotations/annotation-to-annotate.kube @@ -0,0 +1,4 @@ +# Test that a Kubernetes ingress annotation configured via annotationsToRouteAnnotations +# is injected as an annotate() filter prepended to the route's filter chain. +annotationsToRouteAnnotations: + - "oidc/client-id" diff --git a/dataclients/kubernetes/testdata/ingressV1/annotate-from-annotations/annotation-to-annotate.yaml b/dataclients/kubernetes/testdata/ingressV1/annotate-from-annotations/annotation-to-annotate.yaml new file mode 100644 index 0000000000..25590b3f73 --- /dev/null +++ b/dataclients/kubernetes/testdata/ingressV1/annotate-from-annotations/annotation-to-annotate.yaml @@ -0,0 +1,45 @@ +apiVersion: networking.k8s.io/v1 +kind: Ingress +metadata: + namespace: namespace1 + name: ingress1 + annotations: + oidc/client-id: my-client +spec: + rules: + - host: test.example.org + http: + paths: + - path: "/test1" + pathType: ImplementationSpecific + backend: + service: + name: service1 + port: + name: port1 +--- +apiVersion: v1 +kind: Service +metadata: + namespace: namespace1 + name: service1 +spec: + clusterIP: 1.2.3.4 + ports: + - name: port1 + port: 8080 + targetPort: 8080 + type: ClusterIP +--- +apiVersion: v1 +kind: Endpoints +metadata: + namespace: namespace1 + name: service1 +subsets: + - addresses: + - ip: 42.0.1.2 + ports: + - name: port1 + port: 8080 + protocol: TCP diff --git a/dataclients/kubernetes/testdata/ingressV1/annotate-from-annotations/label-to-annotate.eskip b/dataclients/kubernetes/testdata/ingressV1/annotate-from-annotations/label-to-annotate.eskip new file mode 100644 index 0000000000..67803c9453 --- /dev/null +++ b/dataclients/kubernetes/testdata/ingressV1/annotate-from-annotations/label-to-annotate.eskip @@ -0,0 +1,9 @@ +kube___catchall__test_example_org____: + Host("^(test[.]example[.]org[.]?(:[0-9]+)?)$") + -> ; + +kube_namespace1__ingress1__test_example_org___test1__service1: + Host("^(test[.]example[.]org[.]?(:[0-9]+)?)$") + && PathRegexp("^(/test1)") + -> annotate("app.kubernetes.io/component", "frontend") + -> "http://42.0.1.2:8080"; diff --git a/dataclients/kubernetes/testdata/ingressV1/annotate-from-annotations/label-to-annotate.kube b/dataclients/kubernetes/testdata/ingressV1/annotate-from-annotations/label-to-annotate.kube new file mode 100644 index 0000000000..32febf7152 --- /dev/null +++ b/dataclients/kubernetes/testdata/ingressV1/annotate-from-annotations/label-to-annotate.kube @@ -0,0 +1,4 @@ +# Test that a Kubernetes ingress label configured via labelsToRouteAnnotations +# is injected as an annotate() filter prepended to the route's filter chain. +labelsToRouteAnnotations: + - "app.kubernetes.io/component" diff --git a/dataclients/kubernetes/testdata/ingressV1/annotate-from-annotations/label-to-annotate.yaml b/dataclients/kubernetes/testdata/ingressV1/annotate-from-annotations/label-to-annotate.yaml new file mode 100644 index 0000000000..2f1f56167f --- /dev/null +++ b/dataclients/kubernetes/testdata/ingressV1/annotate-from-annotations/label-to-annotate.yaml @@ -0,0 +1,45 @@ +apiVersion: networking.k8s.io/v1 +kind: Ingress +metadata: + namespace: namespace1 + name: ingress1 + labels: + app.kubernetes.io/component: frontend +spec: + rules: + - host: test.example.org + http: + paths: + - path: "/test1" + pathType: ImplementationSpecific + backend: + service: + name: service1 + port: + name: port1 +--- +apiVersion: v1 +kind: Service +metadata: + namespace: namespace1 + name: service1 +spec: + clusterIP: 1.2.3.4 + ports: + - name: port1 + port: 8080 + targetPort: 8080 + type: ClusterIP +--- +apiVersion: v1 +kind: Endpoints +metadata: + namespace: namespace1 + name: service1 +subsets: + - addresses: + - ip: 42.0.1.2 + ports: + - name: port1 + port: 8080 + protocol: TCP diff --git a/filters/auth/oidcprofile_e2e_test.go b/filters/auth/oidcprofile_e2e_test.go new file mode 100644 index 0000000000..33c9614a14 --- /dev/null +++ b/filters/auth/oidcprofile_e2e_test.go @@ -0,0 +1,394 @@ +package auth + +import ( + "crypto/rsa" + "crypto/x509" + "encoding/json" + "encoding/pem" + "fmt" + "log" + "net/http" + "net/http/httptest" + "net/url" + "os" + "strings" + "sync" + "testing" + "time" + + "gopkg.in/go-jose/go-jose.v2" + + "github.com/golang-jwt/jwt/v4" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/zalando/skipper/eskip" + "github.com/zalando/skipper/filters" + "github.com/zalando/skipper/filters/annotate" + "github.com/zalando/skipper/proxy/proxytest" + "github.com/zalando/skipper/routing" + "github.com/zalando/skipper/routing/testdataclient" + "github.com/zalando/skipper/secrets/secrettest" +) + +// createFlexOIDCServer creates an OIDC test server that does not require a +// pre-configured callback URL. It stores the redirect_uri from each auth +// request and validates it during the token exchange — matching the behavior +// of a real OIDC server. This allows the callback URL to be determined at +// request time (e.g. via {{.Request.Host}} in a profile template) rather than +// at server start-up time. +func createFlexOIDCServer(client, clientsecret string, extraClaims jwt.MapClaims, removeClaims []string) *httptest.Server { + var ( + mu sync.Mutex + lastRedirectURI string + ) + + var oidcServer *httptest.Server + oidcServer = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Path { + case "/.well-known/openid-configuration": + st := strings.ReplaceAll(testOpenIDConfig, "https://accounts.google.com", oidcServer.URL) + st = strings.ReplaceAll(st, "https://oauth2.googleapis.com", oidcServer.URL) + st = strings.ReplaceAll(st, "https://www.googleapis.com", oidcServer.URL) + st = strings.ReplaceAll(st, "https://openidconnect.googleapis.com", oidcServer.URL) + _, _ = w.Write([]byte(st)) + + case "/o/oauth2/v2/auth": + clientID := r.URL.Query().Get("client_id") + if clientID != client { + w.WriteHeader(401) + return + } + redirectURI := r.URL.Query().Get("redirect_uri") + if redirectURI == "" { + w.WriteHeader(401) + return + } + if r.URL.Query().Get("response_type") != "code" { + w.WriteHeader(500) + return + } + scopesString := r.URL.Query().Get("scope") + scopes := strings.Fields(scopesString) + supportedScopes := []string{"openid", "sub", "email", "uid", "profile"} + if !all(scopes, supportedScopes) { + w.WriteHeader(401) + return + } + + mu.Lock() + lastRedirectURI = redirectURI + mu.Unlock() + + state := r.URL.Query().Get("state") + u, err := url.Parse(redirectURI + "?state=" + state + "&code=" + validCode) + if err != nil { + log.Fatalf("createFlexOIDCServer: failed to parse redirect URI: %v", err) + } + w.Header().Set("Location", u.String()) + w.WriteHeader(302) + + case "/token": + user, password, ok := r.BasicAuth() + if !ok || user != client || password != clientsecret { + w.WriteHeader(401) + return + } + if err := r.ParseForm(); err != nil { + log.Fatalf("createFlexOIDCServer: failed to parse form: %v", err) + } + if r.Form.Get("code") != validCode { + w.WriteHeader(401) + return + } + redirectURI := r.Form.Get("redirect_uri") + mu.Lock() + expected := lastRedirectURI + mu.Unlock() + if redirectURI != expected { + w.WriteHeader(401) + return + } + + w.Header().Set("Content-Type", "application/json") + w.Header().Set("Cache-Control", "no-store") + w.Header().Set("Pragma", "no-cache") + + claims := jwt.MapClaims{ + testKey: testValue, + "iss": oidcServer.URL, + "sub": testSub, + "aud": client, + "iat": time.Now().Add(-time.Minute).UTC().Unix(), + "exp": time.Now().Add(tokenExp).UTC().Unix(), + "email": "someone@example.org", + } + for k, v := range extraClaims { + claims[k] = v + } + for _, k := range removeClaims { + delete(claims, k) + } + token := jwt.NewWithClaims(jwt.SigningMethodRS256, claims) + privPEM, err := os.ReadFile(keyPath) + if err != nil { + log.Fatalf("createFlexOIDCServer: failed to read key: %v", err) + } + key, err := jwt.ParseRSAPrivateKeyFromPEM(privPEM) + if err != nil { + log.Fatalf("createFlexOIDCServer: failed to parse RSA key: %v", err) + } + validIDToken, err := token.SignedString(key) + if err != nil { + log.Fatalf("createFlexOIDCServer: failed to sign token: %v", err) + } + body := fmt.Sprintf( + `{"access_token": "%s", "token_type": "Bearer", "refresh_token": "%s", "expires_in": 3600, "id_token": "%s"}`, + validAccessToken, validRefreshToken, validIDToken, + ) + _, _ = w.Write([]byte(body)) + + case "/v1/userinfo": + if r.Header.Get(authHeaderName) == authHeaderPrefix+validAccessToken { + body := fmt.Sprintf(`{"sub": "%s", "email": "%s@example.org", "email_verified": true}`, testSub, testUID) + _, _ = w.Write([]byte(body)) + } else { + w.WriteHeader(401) + } + + case "/oauth2/v3/certs": + certPEM, err := os.ReadFile(certPath) + if err != nil { + log.Fatalf("createFlexOIDCServer: failed to read cert: %v", err) + } + pemDecodeCert, _ := pem.Decode(certPEM) + cert, err := x509.ParseCertificate(pemDecodeCert.Bytes) + if err != nil { + log.Fatalf("createFlexOIDCServer: failed to parse cert: %v", err) + } + privPEM, err := os.ReadFile(keyPath) + if err != nil { + log.Fatalf("createFlexOIDCServer: failed to read key: %v", err) + } + pemDecodePriv, _ := pem.Decode(privPEM) + privKey, err := x509.ParsePKCS8PrivateKey(pemDecodePriv.Bytes) + if err != nil { + log.Fatalf("createFlexOIDCServer: failed to parse PKCS8 key: %v", err) + } + rsaPrivKey, ok := privKey.(*rsa.PrivateKey) + if !ok { + log.Fatal("createFlexOIDCServer: private key is not RSA") + } + j := jose.JSONWebKeySet{ + Keys: []jose.JSONWebKey{{ + Key: &rsaPrivKey.PublicKey, + Certificates: []*x509.Certificate{cert}, + Algorithm: "RS256", + Use: "sig", + }}, + } + b, err := json.Marshal(j) + if err != nil { + log.Fatalf("createFlexOIDCServer: failed to marshal JWKS: %v", err) + } + _, _ = w.Write(b) + + default: + w.WriteHeader(http.StatusNotFound) + } + })) + return oidcServer +} + +// TestOIDCProfileSetup runs end-to-end tests for the OIDC profile feature. +// +// Each sub-test spins up a fresh OIDC server, proxy, and backend. The proxy's +// filter registry is configured with named OIDC profiles (OidcOptions.Profiles). +// The IdpURL in each profile is set to the test OIDC server URL after server +// creation to avoid the chicken-and-egg dependency. The CallbackURL uses +// {{.Request.Host}} so it resolves to the proxy address at request time — no +// hard-coded port is needed. +func TestOIDCProfileSetup(t *testing.T) { + // callbackTemplate is used as the CallbackURL in all test profiles. The + // template is resolved to http:///redirect at request time. + const callbackTemplate = `http://{{.Request.Host}}/redirect` + + for _, tc := range []struct { + msg string + // profiles holds the profiles to register WITHOUT IdpURL (injected below). + profiles map[string]OidcProfile + routeFilters string // eskip filter chain for the injected route + expected int // expected HTTP status after the full OIDC flow + }{ + { + msg: "static profile credentials — full auth code flow succeeds", + profiles: map[string]OidcProfile{ + "myprofile": { + ClientID: validClient, + ClientSecret: "mysec", + CallbackURL: callbackTemplate, + }, + }, + routeFilters: `oauthOidcAnyClaims("profile:myprofile", "uid")`, + expected: 200, + }, + { + msg: "annotation-injected client-id — filter chain annotate() -> profile filter succeeds", + profiles: map[string]OidcProfile{ + "myprofile": { + ClientID: `{{index .Annotations "client-id"}}`, + ClientSecret: "mysec", + CallbackURL: callbackTemplate, + }, + }, + routeFilters: `annotate("client-id", "valid-client") -> oauthOidcAnyClaims("profile:myprofile", "uid")`, + expected: 200, + }, + { + msg: "wrong client-id from annotation — OIDC server rejects, returns 401", + profiles: map[string]OidcProfile{ + "myprofile": { + ClientID: `{{index .Annotations "client-id"}}`, + ClientSecret: "mysec", + CallbackURL: callbackTemplate, + }, + }, + routeFilters: `annotate("client-id", "wrong-client") -> oauthOidcAnyClaims("profile:myprofile", "uid")`, + expected: 401, + }, + { + msg: "unknown profile name — CreateFilter error, route not loaded, returns 404", + profiles: map[string]OidcProfile{ + "myprofile": { + ClientID: validClient, + ClientSecret: "mysec", + CallbackURL: callbackTemplate, + }, + }, + // Route references a profile that does not exist → CreateFilter returns + // ErrInvalidFilterParameters → routing engine drops the route → 404. + routeFilters: `oauthOidcAnyClaims("profile:nonexistent", "uid")`, + expected: 404, + }, + { + msg: "templated IdpURL — CreateFilter rejects, route not loaded, returns 404", + profiles: map[string]OidcProfile{ + "bad-idp": { + // IdpURL must be static; template expressions are rejected in CreateFilter. + // Note: IdpURL is set at test runtime below; the prefix test here relies on + // the special marker that triggers the static-check branch. + ClientID: validClient, + CallbackURL: callbackTemplate, + // IdpURL contains "{{" — injected below after the oidcServer URL is known. + // We store a marker so the injection below can recognise this case. + }, + }, + // The route uses this bad profile → CreateFilter returns error → 404. + routeFilters: `oauthOidcAnyClaims("profile:bad-idp", "uid")`, + expected: 404, + }, + { + msg: "profile with allClaims check — all required claims present, returns 200", + profiles: map[string]OidcProfile{ + "myprofile": { + ClientID: validClient, + ClientSecret: "mysec", + CallbackURL: callbackTemplate, + Scopes: "uid", + }, + }, + routeFilters: `oauthOidcAllClaims("profile:myprofile", "sub uid")`, + expected: 200, + }, + { + msg: "profile with userInfo check — userinfo endpoint consulted, returns 200", + profiles: map[string]OidcProfile{ + "myprofile": { + ClientID: validClient, + ClientSecret: "mysec", + CallbackURL: callbackTemplate, + Scopes: "uid", + }, + }, + routeFilters: `oauthOidcUserInfo("profile:myprofile", "uid")`, + expected: 200, + }, + } { + t.Run(tc.msg, func(t *testing.T) { + oidcServer := createFlexOIDCServer(validClient, "mysec", nil, nil) + defer oidcServer.Close() + + // Inject IdpURL into each profile now that the server URL is known. + // For the "templated IdpURL" case, set it to a value that contains "{{". + profiles := make(map[string]OidcProfile, len(tc.profiles)) + for name, p := range tc.profiles { + if name == "bad-idp" { + // IdpURL with template expression — must be rejected at CreateFilter time. + p.IdpURL = "http://{{.Request.Host}}/oidc" + } else { + p.IdpURL = oidcServer.URL + } + profiles[name] = p + } + + backend := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Write([]byte("OK")) + })) + defer backend.Close() + + fd, err := os.CreateTemp("", "testSecrets") + require.NoError(t, err) + secretsFile := fd.Name() + fd.Close() + defer os.Remove(secretsFile) + + secretsRegistry := secrettest.NewTestRegistry() + + fr := make(filters.Registry) + fr.Register(annotate.New()) + opts := OidcOptions{Profiles: profiles} + fr.Register(NewOAuthOidcUserInfosWithOptions(secretsFile, secretsRegistry, opts)) + fr.Register(NewOAuthOidcAnyClaimsWithOptions(secretsFile, secretsRegistry, opts)) + fr.Register(NewOAuthOidcAllClaimsWithOptions(secretsFile, secretsRegistry, opts)) + + dc := testdataclient.New(nil) + defer dc.Close() + + proxy := proxytest.WithRoutingOptions(fr, routing.Options{ + DataClients: []routing.DataClient{dc}, + }) + defer proxy.Close() + + parsedFilters, err := eskip.ParseFilters(tc.routeFilters) + require.NoError(t, err) + + r := &eskip.Route{ + Filters: parsedFilters, + Backend: backend.URL, + } + + proxy.Log.Reset() + dc.Update([]*eskip.Route{r}, nil) + err = proxy.Log.WaitFor("route settings applied", 10*time.Second) + require.NoError(t, err) + + req, err := http.NewRequest("GET", proxy.URL, nil) + require.NoError(t, err) + + client := http.Client{ + Timeout: 2 * time.Second, + Jar: newInsecureCookieJar(), + } + resp, err := client.Do(req) + require.NoError(t, err) + defer resp.Body.Close() + + assert.Equal(t, tc.expected, resp.StatusCode) + + // For successful cases, verify that a session cookie was set. + if tc.expected == 200 { + cookies := client.Jar.Cookies(&url.URL{Host: req.URL.Host}) + assert.NotEmpty(t, cookies, "expected a session cookie after successful OIDC login") + } + }) + } +}