fix: remove intercept error from access token feature and update CRD

shawnhankim · shawnhankim · commit e5982adacf63 · 2023-03-16T19:41:29.000-07:00
diff --git a/deployments/common/crds/k8s.nginx.org_policies.yaml b/deployments/common/crds/k8s.nginx.org_policies.yaml
@@ -114,6 +114,8 @@ spec:
                   description: OIDC defines an Open ID Connect policy.
                   type: object
                   properties:
+                    accessTokenEnable:
+                      type: boolean
                     authEndpoint:
                       type: string
                     authExtraArgs:
@@ -134,10 +136,6 @@ spec:
                       type: string
                     zoneSyncLeeway:
                       type: integer
-                    accessTokenEnable:
-                      type: boolean
-                    interceptErrorEnable:
-                      type: boolean
                 rateLimit:
                   description: RateLimit defines a rate limit policy.
                   type: object
diff --git a/deployments/helm-chart/crds/k8s.nginx.org_policies.yaml b/deployments/helm-chart/crds/k8s.nginx.org_policies.yaml
@@ -114,6 +114,8 @@ spec:
                   description: OIDC defines an Open ID Connect policy.
                   type: object
                   properties:
+                    accessTokenEnable:
+                      type: boolean
                     authEndpoint:
                       type: string
                     authExtraArgs:
@@ -134,10 +136,6 @@ spec:
                       type: string
                     zoneSyncLeeway:
                       type: integer
-                    accessTokenEnable:
-                      type: boolean
-                    interceptErrorEnable:
-                      type: boolean
                 rateLimit:
                   description: RateLimit defines a rate limit policy.
                   type: object
diff --git a/docs/content/configuration/policy-resource.md b/docs/content/configuration/policy-resource.md
@@ -357,7 +357,6 @@ spec:
     tokenEndpoint: https://idp.example.com/openid-connect/token
     jwksURI: https://idp.example.com/openid-connect/certs
     accessTokenEnable: true
-    interceptErrorEnable: false
 ```
 
 NGINX Plus will pass the ID of an authenticated user to the backend in the HTTP header `username`.
@@ -389,8 +388,6 @@ The OIDC policy defines a few internal locations that can't be customized: `/_jw
 |``zoneSyncLeeway`` | Specifies the maximum timeout in milliseconds for synchronizing ID/access tokens and shared values between Ingress Controller pods. The default is ``200``. | ``int`` | No |
 |``accessTokenEnable`` | Option of whether Bearer token is used to authorize NGINX to access protected backend. | ``boolean`` | No |
 {{% /table %}}
-|``interceptErrorEnable`` | Option to intercept and redirect "401 Unauthorized" proxied responses to nginx for processing with the `error_page` directive to restart `@do_oidc_flow` if an access token can expire before ID token. | ``boolean`` | No |
-{{% /table %}}
 
 > **Note**: Only one OIDC policy can be referenced in a VirtualServer and its VirtualServerRoutes. However, the same policy can still be applied to different routes in the VirtualServer and VirtualServerRoutes.
 
diff --git a/examples/custom-resources/oidc/oidc.yaml b/examples/custom-resources/oidc/oidc.yaml
@@ -11,4 +11,3 @@ spec:
     jwksURI: http://keycloak.default.svc.cluster.local:8080/auth/realms/master/protocol/openid-connect/certs
     scope: openid+profile+email
     accessTokenEnable: true
-    interceptErrorEnable: false
diff --git a/internal/configs/version2/http.go b/internal/configs/version2/http.go
@@ -111,17 +111,16 @@ type EgressMTLS struct {
 
 // OIDC holds OIDC configuration data.
 type OIDC struct {
-	AuthEndpoint         string
-	ClientID             string
-	ClientSecret         string
-	JwksURI              string
-	Scope                string
-	TokenEndpoint        string
-	RedirectURI          string
-	ZoneSyncLeeway       int
-	AuthExtraArgs        string
-	AccessTokenEnable    bool
-	InterceptErrorEnable bool
+	AuthEndpoint      string
+	ClientID          string
+	ClientSecret      string
+	JwksURI           string
+	Scope             string
+	TokenEndpoint     string
+	RedirectURI       string
+	ZoneSyncLeeway    int
+	AuthExtraArgs     string
+	AccessTokenEnable bool
 }
 
 // WAF defines WAF configuration.
diff --git a/internal/configs/version2/nginx-plus.virtualserver.tmpl b/internal/configs/version2/nginx-plus.virtualserver.tmpl
@@ -433,9 +433,6 @@ server {
             {{ if $oidc.AccessTokenEnable }}
         {{ $proxyOrGRPC }}_set_header Authorization "Bearer $access_token";
             {{ end }}
-            {{ if $oidc.InterceptErrorEnable }}
-        {{ $proxyOrGRPC }}_intercept_errors on;
-            {{ end }}
         {{ end }}
 
         {{ with $l.WAF }}
diff --git a/internal/configs/virtualserver.go b/internal/configs/virtualserver.go
@@ -1052,17 +1052,16 @@ func (p *policiesCfg) addOIDCConfig(
 		}
 
 		oidcPolCfg.oidc = &version2.OIDC{
-			AuthEndpoint:         oidc.AuthEndpoint,
-			AuthExtraArgs:        authExtraArgs,
-			TokenEndpoint:        oidc.TokenEndpoint,
-			JwksURI:              oidc.JWKSURI,
-			ClientID:             oidc.ClientID,
-			ClientSecret:         string(clientSecret),
-			Scope:                scope,
-			RedirectURI:          redirectURI,
-			ZoneSyncLeeway:       generateIntFromPointer(oidc.ZoneSyncLeeway, 200),
-			AccessTokenEnable:    oidc.AccessTokenEnable,
-			InterceptErrorEnable: oidc.InterceptErrorEnable,
+			AuthEndpoint:      oidc.AuthEndpoint,
+			AuthExtraArgs:     authExtraArgs,
+			TokenEndpoint:     oidc.TokenEndpoint,
+			JwksURI:           oidc.JWKSURI,
+			ClientID:          oidc.ClientID,
+			ClientSecret:      string(clientSecret),
+			Scope:             scope,
+			RedirectURI:       redirectURI,
+			ZoneSyncLeeway:    generateIntFromPointer(oidc.ZoneSyncLeeway, 200),
+			AccessTokenEnable: oidc.AccessTokenEnable,
 		}
 		oidcPolCfg.key = polKey
 	}
diff --git a/internal/configs/virtualserver_test.go b/internal/configs/virtualserver_test.go
@@ -3136,16 +3136,15 @@ func TestGeneratePolicies(t *testing.T) {
 					},
 					Spec: conf_v1.PolicySpec{
 						OIDC: &conf_v1.OIDC{
-							AuthEndpoint:         "http://example.com/auth",
-							TokenEndpoint:        "http://example.com/token",
-							JWKSURI:              "http://example.com/jwks",
-							ClientID:             "client-id",
-							ClientSecret:         "oidc-secret",
-							Scope:                "scope",
-							RedirectURI:          "/redirect",
-							ZoneSyncLeeway:       createPointerFromInt(20),
-							AccessTokenEnable:    true,
-							InterceptErrorEnable: false,
+							AuthEndpoint:      "http://example.com/auth",
+							TokenEndpoint:     "http://example.com/token",
+							JWKSURI:           "http://example.com/jwks",
+							ClientID:          "client-id",
+							ClientSecret:      "oidc-secret",
+							Scope:             "scope",
+							RedirectURI:       "/redirect",
+							ZoneSyncLeeway:    createPointerFromInt(20),
+							AccessTokenEnable: true,
 						},
 					},
 				},
@@ -4254,12 +4253,11 @@ func TestGeneratePoliciesFails(t *testing.T) {
 					},
 					Spec: conf_v1.PolicySpec{
 						OIDC: &conf_v1.OIDC{
-							ClientSecret:         "oidc-secret",
-							AuthEndpoint:         "http://foo.com/bar",
-							TokenEndpoint:        "http://foo.com/bar",
-							JWKSURI:              "http://foo.com/bar",
-							AccessTokenEnable:    true,
-							InterceptErrorEnable: false,
+							ClientSecret:      "oidc-secret",
+							AuthEndpoint:      "http://foo.com/bar",
+							TokenEndpoint:     "http://foo.com/bar",
+							JWKSURI:           "http://foo.com/bar",
+							AccessTokenEnable: true,
 						},
 					},
 				},
@@ -4302,13 +4300,12 @@ func TestGeneratePoliciesFails(t *testing.T) {
 					},
 					Spec: conf_v1.PolicySpec{
 						OIDC: &conf_v1.OIDC{
-							ClientID:             "foo",
-							ClientSecret:         "oidc-secret",
-							AuthEndpoint:         "https://foo.com/auth",
-							TokenEndpoint:        "https://foo.com/token",
-							JWKSURI:              "https://foo.com/certs",
-							AccessTokenEnable:    true,
-							InterceptErrorEnable: false,
+							ClientID:          "foo",
+							ClientSecret:      "oidc-secret",
+							AuthEndpoint:      "https://foo.com/auth",
+							TokenEndpoint:     "https://foo.com/token",
+							JWKSURI:           "https://foo.com/certs",
+							AccessTokenEnable: true,
 						},
 					},
 				},
@@ -4319,13 +4316,12 @@ func TestGeneratePoliciesFails(t *testing.T) {
 					},
 					Spec: conf_v1.PolicySpec{
 						OIDC: &conf_v1.OIDC{
-							ClientID:             "foo",
-							ClientSecret:         "oidc-secret",
-							AuthEndpoint:         "https://bar.com/auth",
-							TokenEndpoint:        "https://bar.com/token",
-							JWKSURI:              "https://bar.com/certs",
-							AccessTokenEnable:    true,
-							InterceptErrorEnable: false,
+							ClientID:          "foo",
+							ClientSecret:      "oidc-secret",
+							AuthEndpoint:      "https://bar.com/auth",
+							TokenEndpoint:     "https://bar.com/token",
+							JWKSURI:           "https://bar.com/certs",
+							AccessTokenEnable: true,
 						},
 					},
 				},
@@ -4345,16 +4341,15 @@ func TestGeneratePoliciesFails(t *testing.T) {
 			context: "route",
 			oidcPolCfg: &oidcPolicyCfg{
 				oidc: &version2.OIDC{
-					AuthEndpoint:         "https://foo.com/auth",
-					TokenEndpoint:        "https://foo.com/token",
-					JwksURI:              "https://foo.com/certs",
-					ClientID:             "foo",
-					ClientSecret:         "super_secret_123",
-					RedirectURI:          "/_codexch",
-					Scope:                "openid",
-					ZoneSyncLeeway:       0,
-					AccessTokenEnable:    true,
-					InterceptErrorEnable: false,
+					AuthEndpoint:      "https://foo.com/auth",
+					TokenEndpoint:     "https://foo.com/token",
+					JwksURI:           "https://foo.com/certs",
+					ClientID:          "foo",
+					ClientSecret:      "super_secret_123",
+					RedirectURI:       "/_codexch",
+					Scope:             "openid",
+					ZoneSyncLeeway:    0,
+					AccessTokenEnable: true,
 				},
 				key: "default/oidc-policy-1",
 			},
@@ -4370,15 +4365,14 @@ func TestGeneratePoliciesFails(t *testing.T) {
 			},
 			expectedOidc: &oidcPolicyCfg{
 				oidc: &version2.OIDC{
-					AuthEndpoint:         "https://foo.com/auth",
-					TokenEndpoint:        "https://foo.com/token",
-					JwksURI:              "https://foo.com/certs",
-					ClientID:             "foo",
-					ClientSecret:         "super_secret_123",
-					RedirectURI:          "/_codexch",
-					Scope:                "openid",
-					AccessTokenEnable:    true,
-					InterceptErrorEnable: false,
+					AuthEndpoint:      "https://foo.com/auth",
+					TokenEndpoint:     "https://foo.com/token",
+					JwksURI:           "https://foo.com/certs",
+					ClientID:          "foo",
+					ClientSecret:      "super_secret_123",
+					RedirectURI:       "/_codexch",
+					Scope:             "openid",
+					AccessTokenEnable: true,
 				},
 				key: "default/oidc-policy-1",
 			},
@@ -4403,13 +4397,12 @@ func TestGeneratePoliciesFails(t *testing.T) {
 					},
 					Spec: conf_v1.PolicySpec{
 						OIDC: &conf_v1.OIDC{
-							ClientSecret:         "oidc-secret",
-							AuthEndpoint:         "https://foo.com/auth",
-							TokenEndpoint:        "https://foo.com/token",
-							JWKSURI:              "https://foo.com/certs",
-							ClientID:             "foo",
-							AccessTokenEnable:    true,
-							InterceptErrorEnable: false,
+							ClientSecret:      "oidc-secret",
+							AuthEndpoint:      "https://foo.com/auth",
+							TokenEndpoint:     "https://foo.com/token",
+							JWKSURI:           "https://foo.com/certs",
+							ClientID:          "foo",
+							AccessTokenEnable: true,
 						},
 					},
 				},
@@ -4420,13 +4413,12 @@ func TestGeneratePoliciesFails(t *testing.T) {
 					},
 					Spec: conf_v1.PolicySpec{
 						OIDC: &conf_v1.OIDC{
-							ClientSecret:         "oidc-secret",
-							AuthEndpoint:         "https://bar.com/auth",
-							TokenEndpoint:        "https://bar.com/token",
-							JWKSURI:              "https://bar.com/certs",
-							ClientID:             "bar",
-							AccessTokenEnable:    true,
-							InterceptErrorEnable: false,
+							ClientSecret:      "oidc-secret",
+							AuthEndpoint:      "https://bar.com/auth",
+							TokenEndpoint:     "https://bar.com/token",
+							JWKSURI:           "https://bar.com/certs",
+							ClientID:          "bar",
+							AccessTokenEnable: true,
 						},
 					},
 				},
@@ -4454,16 +4446,15 @@ func TestGeneratePoliciesFails(t *testing.T) {
 			},
 			expectedOidc: &oidcPolicyCfg{
 				&version2.OIDC{
-					AuthEndpoint:         "https://foo.com/auth",
-					TokenEndpoint:        "https://foo.com/token",
-					JwksURI:              "https://foo.com/certs",
-					ClientID:             "foo",
-					ClientSecret:         "super_secret_123",
-					RedirectURI:          "/_codexch",
-					Scope:                "openid",
-					ZoneSyncLeeway:       200,
-					AccessTokenEnable:    true,
-					InterceptErrorEnable: false,
+					AuthEndpoint:      "https://foo.com/auth",
+					TokenEndpoint:     "https://foo.com/token",
+					JwksURI:           "https://foo.com/certs",
+					ClientID:          "foo",
+					ClientSecret:      "super_secret_123",
+					RedirectURI:       "/_codexch",
+					Scope:             "openid",
+					ZoneSyncLeeway:    200,
+					AccessTokenEnable: true,
 				},
 				"default/oidc-policy",
 			},
diff --git a/pkg/apis/configuration/v1/types.go b/pkg/apis/configuration/v1/types.go
@@ -475,17 +475,16 @@ type EgressMTLS struct {
 
 // OIDC defines an Open ID Connect policy.
 type OIDC struct {
-	AuthEndpoint         string   `json:"authEndpoint"`
-	TokenEndpoint        string   `json:"tokenEndpoint"`
-	JWKSURI              string   `json:"jwksURI"`
-	ClientID             string   `json:"clientID"`
-	ClientSecret         string   `json:"clientSecret"`
-	Scope                string   `json:"scope"`
-	RedirectURI          string   `json:"redirectURI"`
-	ZoneSyncLeeway       *int     `json:"zoneSyncLeeway"`
-	AuthExtraArgs        []string `json:"authExtraArgs"`
-	AccessTokenEnable    bool     `json:"accessTokenEnable"`
-	InterceptErrorEnable bool     `json:"interceptErrorEnable"`
+	AuthEndpoint      string   `json:"authEndpoint"`
+	TokenEndpoint     string   `json:"tokenEndpoint"`
+	JWKSURI           string   `json:"jwksURI"`
+	ClientID          string   `json:"clientID"`
+	ClientSecret      string   `json:"clientSecret"`
+	Scope             string   `json:"scope"`
+	RedirectURI       string   `json:"redirectURI"`
+	ZoneSyncLeeway    *int     `json:"zoneSyncLeeway"`
+	AuthExtraArgs     []string `json:"authExtraArgs"`
+	AccessTokenEnable bool     `json:"accessTokenEnable"`
 }
 
 // WAF defines an WAF policy.
diff --git a/pkg/apis/configuration/validation/policy_test.go b/pkg/apis/configuration/validation/policy_test.go
