diff --git a/charts/tests/__snapshots__/helmunit_test.snap b/charts/tests/__snapshots__/helmunit_test.snap index 3ebe121d45..78d45f7e8c 100755 --- a/charts/tests/__snapshots__/helmunit_test.snap +++ b/charts/tests/__snapshots__/helmunit_test.snap @@ -442,8 +442,6 @@ spec: - -ssl-dynamic-reload=true - -enable-telemetry-reporting=true - -weight-changes-dynamic-reload=false - - minReadySeconds: 0 /-/-/-/ # Source: nginx-ingress/templates/controller-ingress-class.yaml apiVersion: networking.k8s.io/v1 @@ -911,8 +909,6 @@ spec: - -ssl-dynamic-reload=true - -enable-telemetry-reporting=true - -weight-changes-dynamic-reload=false - - minReadySeconds: 0 /-/-/-/ # Source: nginx-ingress/templates/controller-ingress-class.yaml apiVersion: networking.k8s.io/v1 @@ -1449,8 +1445,6 @@ spec: - -weight-changes-dynamic-reload=false - -agent=true - -agent-instance-group=app-protect-waf-agentv2-nginx-ingress-controller - - minReadySeconds: 0 /-/-/-/ # Source: nginx-ingress/templates/controller-ingress-class.yaml apiVersion: networking.k8s.io/v1 @@ -1965,7 +1959,6 @@ spec: mountPath: /opt/app_protect/config - name: app-protect-bundles mountPath: /etc/app_protect/bundles - minReadySeconds: 0 /-/-/-/ # Source: nginx-ingress/templates/controller-ingress-class.yaml apiVersion: networking.k8s.io/v1 @@ -2547,7 +2540,6 @@ spec: mountPath: /opt/app_protect/config - name: app-protect-bundles mountPath: /etc/app_protect/bundles - minReadySeconds: 0 /-/-/-/ # Source: nginx-ingress/templates/controller-ingress-class.yaml apiVersion: networking.k8s.io/v1 @@ -2961,8 +2953,6 @@ spec: - -ssl-dynamic-reload=true - -enable-telemetry-reporting=true - -weight-changes-dynamic-reload=false - - minReadySeconds: 0 /-/-/-/ # Source: nginx-ingress/templates/controller-ingress-class.yaml apiVersion: networking.k8s.io/v1 @@ -3406,8 +3396,6 @@ spec: - -ssl-dynamic-reload=true - -enable-telemetry-reporting=true - -weight-changes-dynamic-reload=false - - minReadySeconds: 0 /-/-/-/ # Source: nginx-ingress/templates/controller-ingress-class.yaml apiVersion: networking.k8s.io/v1 @@ -3851,8 +3839,6 @@ spec: - -ssl-dynamic-reload=true - -enable-telemetry-reporting=true - -weight-changes-dynamic-reload=false - - minReadySeconds: 0 /-/-/-/ # Source: nginx-ingress/templates/controller-ingress-class.yaml apiVersion: networking.k8s.io/v1 @@ -4297,8 +4283,6 @@ spec: - -ssl-dynamic-reload=true - -enable-telemetry-reporting=true - -weight-changes-dynamic-reload=false - - minReadySeconds: 0 /-/-/-/ # Source: nginx-ingress/templates/controller-ingress-class.yaml apiVersion: networking.k8s.io/v1 @@ -4763,8 +4747,6 @@ spec: - -ssl-dynamic-reload=true - -enable-telemetry-reporting=true - -weight-changes-dynamic-reload=false - - minReadySeconds: 0 /-/-/-/ # Source: nginx-ingress/templates/controller-ingress-class.yaml apiVersion: networking.k8s.io/v1 @@ -5210,8 +5192,6 @@ spec: - -ssl-dynamic-reload=true - -enable-telemetry-reporting=true - -weight-changes-dynamic-reload=false - - minReadySeconds: 0 /-/-/-/ # Source: nginx-ingress/templates/controller-ingress-class.yaml apiVersion: networking.k8s.io/v1 @@ -5672,8 +5652,6 @@ spec: - -ssl-dynamic-reload=true - -enable-telemetry-reporting=true - -weight-changes-dynamic-reload=false - - minReadySeconds: 0 /-/-/-/ # Source: nginx-ingress/templates/controller-ingress-class.yaml apiVersion: networking.k8s.io/v1 @@ -6141,8 +6119,6 @@ spec: - -ssl-dynamic-reload=true - -enable-telemetry-reporting=true - -weight-changes-dynamic-reload=false - - minReadySeconds: 0 /-/-/-/ # Source: nginx-ingress/templates/controller-ingress-class.yaml apiVersion: networking.k8s.io/v1 @@ -6620,8 +6596,6 @@ spec: - -ssl-dynamic-reload=true - -enable-telemetry-reporting=true - -weight-changes-dynamic-reload=false - - minReadySeconds: 0 /-/-/-/ # Source: nginx-ingress/templates/controller-ingress-class.yaml apiVersion: networking.k8s.io/v1 @@ -7080,8 +7054,6 @@ spec: - -ssl-dynamic-reload=true - -enable-telemetry-reporting=true - -weight-changes-dynamic-reload=false - - minReadySeconds: 0 /-/-/-/ # Source: nginx-ingress/templates/controller-ingress-class.yaml apiVersion: networking.k8s.io/v1 @@ -7540,8 +7512,6 @@ spec: - -ssl-dynamic-reload=true - -enable-telemetry-reporting=true - -weight-changes-dynamic-reload=false - - minReadySeconds: 0 /-/-/-/ # Source: nginx-ingress/templates/controller-ingress-class.yaml apiVersion: networking.k8s.io/v1 @@ -8010,8 +7980,6 @@ spec: - -ssl-dynamic-reload=true - -enable-telemetry-reporting=true - -weight-changes-dynamic-reload=false - - minReadySeconds: 0 /-/-/-/ # Source: nginx-ingress/templates/controller-ingress-class.yaml apiVersion: networking.k8s.io/v1 diff --git a/config/crd/bases/k8s.nginx.org_policies.yaml b/config/crd/bases/k8s.nginx.org_policies.yaml index da36ddeb89..d44aab7672 100644 --- a/config/crd/bases/k8s.nginx.org_policies.yaml +++ b/config/crd/bases/k8s.nginx.org_policies.yaml @@ -163,6 +163,8 @@ spec: type: string jwksURI: type: string + pkceEnable: + type: boolean postLogoutRedirectURI: type: string redirectURI: diff --git a/deploy/crds.yaml b/deploy/crds.yaml index 9d59787abd..c079eb106b 100644 --- a/deploy/crds.yaml +++ b/deploy/crds.yaml @@ -325,6 +325,8 @@ spec: type: string jwksURI: type: string + pkceEnable: + type: boolean postLogoutRedirectURI: type: string redirectURI: diff --git a/examples/custom-resources/oidc/README.md b/examples/custom-resources/oidc/README.md index 9fd03fa70b..3a500ed13c 100644 --- a/examples/custom-resources/oidc/README.md +++ b/examples/custom-resources/oidc/README.md @@ -5,12 +5,24 @@ application using an OpenID Connect policy and [Keycloak](https://www.keycloak.o **Note**: The KeyCloak container does not support IPv6 environments. +**Note**: This example assumes that your default namespace is set to `default`. You can check this with + +```shell +kubectl config view --minify | grep namespace +``` + +If it's not empty, and anything other than `default`, you can set to `default` with the following command: + +```shell +kubectl config set-context --namespace default --current +``` + ## Prerequisites 1. Follow the [installation](https://docs.nginx.com/nginx-ingress-controller/installation/installation-with-manifests/) - instructions to deploy the Ingress Controller. This example requires that the HTTPS port of the Ingress Controller is - `443`. -1. Save the public IP address of the Ingress Controller into `/etc/hosts` of your machine: + instructions to deploy NGINX Ingress Controller. This example requires that the HTTPS port of the Ingress + Controller is `443`. +2. Save the public IP address of the Ingress Controller into `/etc/hosts` of your machine: ```text ... @@ -27,7 +39,7 @@ application using an OpenID Connect policy and [Keycloak](https://www.keycloak.o Create a secret with the TLS certificate and key that will be used for TLS termination of the web application and Keycloak: -```console +```shell kubectl apply -f tls-secret.yaml ``` @@ -35,7 +47,7 @@ kubectl apply -f tls-secret.yaml Create the application deployment and service: -```console +```shell kubectl apply -f webapp.yaml ``` @@ -43,13 +55,13 @@ kubectl apply -f webapp.yaml 1. Create the Keycloak deployment and service: - ```console + ```shell kubectl apply -f keycloak.yaml ``` -1. Create a VirtualServer resource for Keycloak: +2. Create a VirtualServer resource for Keycloak: - ```console + ```shell kubectl apply -f virtual-server-idp.yaml ``` @@ -59,27 +71,30 @@ To set up Keycloak: 1. Follow the steps in the "Configuring Keycloak" [section of the documentation](https://docs.nginx.com/nginx/deployment-guides/single-sign-on/keycloak/#configuring-keycloak): 1. To connect to Keycloak, use `https://keycloak.example.com`. - 1. Make sure to save the client secret for NGINX-Plus client to the `SECRET` shell variable: + 2. Make sure to save the client secret for NGINX-Plus client to the `SECRET` shell variable: - ```console + ```shell SECRET=value ``` -1. Alternatively, [execute the commands](./keycloak_setup.md). +2. Alternatively, [execute the commands](./keycloak_setup.md). ## Step 5 - Deploy the Client Secret +**Note**: If you're using PKCE, skip this step. PKCE clients do not have client secrets. Applying this will result +in a broken deployment. + 1. Encode the secret, obtained in the previous step: - ```console + ```shell echo -n $SECRET | base64 ``` -1. Edit `client-secret.yaml`, replacing `` with the encoded secret. +2. Edit `client-secret.yaml`, replacing `` with the encoded secret. -1. Create a secret with the name `oidc-secret` that will be used by the OIDC policy: +3. Create a secret with the name `oidc-secret` that will be used by the OIDC policy: - ```console + ```shell kubectl apply -f client-secret.yaml ``` @@ -96,7 +111,7 @@ Steps: 1. Apply the ConfigMap `nginx-config.yaml`, which contains `zone-sync` configuration parameter that enable zone synchronization and the resolver using the kube-dns service. - ```console + ```shell kubectl apply -f nginx-config.yaml ``` @@ -104,7 +119,7 @@ Steps: Create a policy with the name `oidc-policy` that references the secret from the previous step: -```console +```shell kubectl apply -f oidc.yaml ``` @@ -112,7 +127,7 @@ kubectl apply -f oidc.yaml Create a VirtualServer resource for the web application: -```console +```shell kubectl apply -f virtual-server.yaml ``` @@ -122,9 +137,9 @@ Note that the VirtualServer references the policy `oidc-policy` created in Step 1. Open a web browser and navigate to the URL of the web application: `https://webapp.example.com`. You will be redirected to Keycloak. -1. Log in with the username and password for the user you created in Keycloak, `nginx-user` and `test`. +2. Log in with the username and password for the user you created in Keycloak, `nginx-user` and `test`. ![keycloak](./keycloak.png) -1. Once logged in, you will be redirected to the web application and get a response from it. Notice the field `User ID` +3. Once logged in, you will be redirected to the web application and get a response from it. Notice the field `User ID` in the response, this will match the ID for your user in Keycloak. ![webapp](./webapp.png) ## Step 10 - Log Out @@ -132,5 +147,5 @@ in the response, this will match the ID for your user in Keycloak. ![webapp](./w 1. To log out, navigate to `https://webapp.example.com/logout`. Your session will be terminated, and you will be redirected to the default post logout URI `https://webapp.example.com/_logout`. ![logout](./logout.png) -1. To confirm that you have been logged out, navigate to `https://webapp.example.com`. You will be redirected to +2. To confirm that you have been logged out, navigate to `https://webapp.example.com`. You will be redirected to Keycloak to log in again. diff --git a/examples/custom-resources/oidc/keycloak_setup.md b/examples/custom-resources/oidc/keycloak_setup.md index 570778e365..278e0fe32c 100644 --- a/examples/custom-resources/oidc/keycloak_setup.md +++ b/examples/custom-resources/oidc/keycloak_setup.md @@ -17,39 +17,63 @@ Steps: 1. Save the address of Keycloak into a shell variable: - ```console + ```shell KEYCLOAK_ADDRESS=keycloak.example.com ``` -1. Retrieve the access token and store it into a shell variable: +2. Retrieve the access token and store it into a shell variable: - ```console + ```shell TOKEN=`curl -sS -k --data "username=admin&password=admin&grant_type=password&client_id=admin-cli" "https://${KEYCLOAK_ADDRESS}/realms/master/protocol/openid-connect/token" | jq -r .access_token` ``` Ensure the request was successful and the token is stored in the shell variable by running: - ```console + ```shell echo $TOKEN ``` ***Note***: The access token lifespan is very short. If it expires between commands, retrieve it again with the command above. -1. Create the user `nginx-user`: +3. Create the user `nginx-user`: - ```console + ```shell curl -sS -k -X POST -d '{ "username": "nginx-user", "enabled": true, "credentials":[{"type": "password", "value": "test", "temporary": false}]}' -H "Content-Type:application/json" -H "Authorization: bearer ${TOKEN}" https://${KEYCLOAK_ADDRESS}/admin/realms/master/users ``` -1. Create the client `nginx-plus` and retrieve the secret: - - ```console - SECRET=`curl -sS -k -X POST -d '{ "clientId": "nginx-plus", "redirectUris": ["https://webapp.example.com:443/_codexch"], "attributes": {"post.logout.redirect.uris": "https://webapp.example.com:443/*"}}' -H "Content-Type:application/json" -H "Authorization: bearer ${TOKEN}" https://${KEYCLOAK_ADDRESS}/realms/master/clients-registrations/default | jq -r .secret` - ``` - - If everything went well you should have the secret stored in $SECRET. To double check run: - - ```console - echo $SECRET - ``` +4. Create the client `nginx-plus`: + + - If you are not using PKCE, use the following command to create an OIDC client that does not use PKCE: + + ```shell + SECRET=`curl -sS -k -X POST -d '{ "clientId": "nginx-plus", "redirectUris": ["https://webapp.example.com:443/_codexch"], "attributes": {"post.logout.redirect.uris": "https://webapp.example.com:443/*"}}' -H "Content-Type:application/json" -H "Authorization: bearer ${TOKEN}" https://${KEYCLOAK_ADDRESS}/realms/master/clients-registrations/default | jq -r .secret` + ``` + + If everything went well, you should have the secret stored in $SECRET. To double-check, run: + + ```shell + echo $SECRET + ``` + + - Or if you are using PKCE with OIDC, use the following command to create the client: + + ```shell + curl -sS -k -H "Content-Type: application/json" -H "Authorization: Bearer ${TOKEN}" \ + --data '{ + "clientId": "nginx-plus", + "enabled": true, + "standardFlowEnabled": true, + "directAccessGrantsEnabled": false, + "publicClient": true, + "redirectUris": [ + "https://webapp.example.com:443/_codexch" + ], + "attributes": { + "pkce.code.challenge.method":"S256", + "post.logout.redirect.uris": "https://webapp.example.com:443/*" + }, + "protocol": "openid-connect" + }' \ + https://${KEYCLOAK_ADDRESS}/admin/realms/master/clients + ``` diff --git a/internal/configs/oidc/oidc_common.conf b/internal/configs/oidc/oidc_common.conf index 26ef21b05b..30d5d37a5d 100644 --- a/internal/configs/oidc/oidc_common.conf +++ b/internal/configs/oidc/oidc_common.conf @@ -20,7 +20,6 @@ proxy_cache_path /var/cache/nginx/jwk levels=1 keys_zone=jwk:64k max_size=1m; keyval_zone zone=oidc_id_tokens:1M timeout=1h sync; keyval_zone zone=oidc_access_tokens:1M timeout=1h sync; keyval_zone zone=refresh_tokens:1M timeout=8h sync; -#keyval_zone zone=oidc_pkce:128K timeout=90s sync; # Temporary storage for PKCE code verifier. keyval $cookie_auth_token $session_jwt zone=oidc_id_tokens; # Exchange cookie for ID token(JWT) keyval $cookie_auth_token $access_token zone=oidc_access_tokens; # Exchange cookie for access token @@ -28,7 +27,6 @@ keyval $cookie_auth_token $refresh_token zone=refresh_tokens; # Exchange coo keyval $request_id $new_session zone=oidc_id_tokens; # For initial session creation keyval $request_id $new_access_token zone=oidc_access_tokens; keyval $request_id $new_refresh zone=refresh_tokens; # '' -#keyval $pkce_id $pkce_code_verifier zone=oidc_pkce; auth_jwt_claim_set $jwt_audience aud; # In case aud is an array js_import oidc from oidc/openid_connect.js; diff --git a/internal/configs/oidc/oidc_pkce_supplements.conf b/internal/configs/oidc/oidc_pkce_supplements.conf new file mode 100644 index 0000000000..668b817f46 --- /dev/null +++ b/internal/configs/oidc/oidc_pkce_supplements.conf @@ -0,0 +1,8 @@ +# https://nginx.org/en/docs/http/ngx_http_keyval_module.html +# context: http + +# keyval_zone keyval_zone zone=name:size [state=file] [timeout=time] [type=string|ip|prefix] [sync]; +keyval_zone zone=oidc_pkce:128K timeout=90s sync; # Temporary storage for PKCE code verifier. + +# keyval key $variable zone +keyval $pkce_id $pkce_code_verifier zone=oidc_pkce; diff --git a/internal/configs/version2/__snapshots__/templates_test.snap b/internal/configs/version2/__snapshots__/templates_test.snap index d2d9ce4cdc..df3f494012 100644 --- a/internal/configs/version2/__snapshots__/templates_test.snap +++ b/internal/configs/version2/__snapshots__/templates_test.snap @@ -335,7 +335,7 @@ server { proxy_pass http://coffee-v2; health_check uri=/ port=50 interval=5s jitter=0s fails=1 passes=1 mandatory persistent keepalive_time=60s; - } + } location @hc-tea { grpc_connect_timeout ; @@ -344,7 +344,7 @@ server { grpc_pass grpc://tea-v3; health_check port=50 interval=5s jitter=0s fails=1 passes=1 type=grpc grpc_status=12 grpc_service=tea-servicev2; - } + } location @vs_cafe_cafe_vsr_tea_tea_tea__tea_error_page_0 { default_type "application/json"; @@ -761,7 +761,7 @@ server { proxy_pass http://coffee-v2; health_check uri=/ port=50 interval=5s jitter=0s fails=1 passes=1 mandatory persistent keepalive_time=60s; - } + } location @hc-tea { grpc_connect_timeout ; @@ -770,7 +770,7 @@ server { grpc_pass grpc://tea-v3; health_check port=50 interval=5s jitter=0s fails=1 passes=1 type=grpc grpc_status=12 grpc_service=tea-servicev2; - } + } location @vs_cafe_cafe_vsr_tea_tea_tea__tea_error_page_0 { default_type "application/json"; @@ -1316,6 +1316,54 @@ server { --- +[TestExecuteVirtualServerTemplateWithOIDCAndPKCEPolicyNGINXPlus - 1] + +include oidc/oidc_pkce_supplements.conf; + +server { + listen 80 proxy_protocol; + listen [::]:80 proxy_protocol; + + + server_name example.com; + status_zone example.com; + set $resource_type "virtualserver"; + set $resource_name ""; + set $resource_namespace ""; + include oidc/oidc.conf; + + set $oidc_pkce_enable 1; + set $oidc_client_auth_method "client_secret_post"; + set $oidc_logout_redirect ""; + set $oidc_hmac_key ""; + set $zone_sync_leeway 0; + + set $oidc_authz_endpoint ""; + set $oidc_authz_extra_args ""; + set $oidc_token_endpoint ""; + set $oidc_end_session_endpoint ""; + set $oidc_jwt_keyfile ""; + set $oidc_scopes ""; + set $oidc_client ""; + set $oidc_client_secret ""; + set $redir_location ""; + + server_tokens ""; + + + + + location / { + set $service ""; + status_zone ""; + + + set $default_connection_header close; + } +} + +--- + [TestExecuteVirtualServerTemplate_RendersOSSTemplateWithHTTP2Off - 1] server { @@ -1438,7 +1486,7 @@ server { return 204; } - + } @@ -3303,7 +3351,7 @@ server { return 204; } - + } @@ -3402,7 +3450,7 @@ server { proxy_pass http://coffee-v2; health_check uri=/ port=50 interval=5s jitter=0s fails=1 passes=1 mandatory persistent keepalive_time=60s; - } + } location @hc-tea { grpc_connect_timeout ; @@ -3411,7 +3459,7 @@ server { grpc_pass grpc://tea-v3; health_check port=50 interval=5s jitter=0s fails=1 passes=1 type=grpc grpc_status=12 grpc_service=tea-servicev2; - } + } location @vs_cafe_cafe_vsr_tea_tea_tea__tea_error_page_0 { default_type "application/json"; diff --git a/internal/configs/version2/http.go b/internal/configs/version2/http.go index 818546b557..3f27badf1c 100644 --- a/internal/configs/version2/http.go +++ b/internal/configs/version2/http.go @@ -153,6 +153,7 @@ type OIDC struct { ZoneSyncLeeway int AuthExtraArgs string AccessTokenEnable bool + PKCEEnable bool } // APIKey holds API key configuration. diff --git a/internal/configs/version2/nginx-plus.virtualserver.tmpl b/internal/configs/version2/nginx-plus.virtualserver.tmpl index f71a5aab74..8d04789e14 100644 --- a/internal/configs/version2/nginx-plus.virtualserver.tmpl +++ b/internal/configs/version2/nginx-plus.virtualserver.tmpl @@ -82,6 +82,10 @@ match {{ $m.Name }} { proxy_cache_path /var/cache/nginx/jwks_uri_{{$s.VSName}} levels=1 keys_zone=jwks_uri_{{$s.VSName}}:1m max_size=10m; {{- end }} +{{- if and $s.OIDC $s.OIDC.PKCEEnable }} +include oidc/oidc_pkce_supplements.conf; +{{- end }} + server { {{- if $s.Gunzip }} gunzip on; @@ -97,7 +101,7 @@ server { {{- with $oidc := $s.OIDC }} include oidc/oidc.conf; - set $oidc_pkce_enable 0; + set $oidc_pkce_enable {{ boolToInteger $oidc.PKCEEnable }}; set $oidc_client_auth_method "client_secret_post"; set $oidc_logout_redirect "{{ $oidc.PostLogoutRedirectURI }}"; set $oidc_hmac_key "{{ $s.VSName }}"; @@ -364,7 +368,7 @@ server { {{- if $hc.GRPCPass }} type=grpc{{- if $hc.GRPCStatus }} grpc_status={{ $hc.GRPCStatus }}{{- end -}} {{- if $hc.GRPCService }} grpc_service={{ $hc.GRPCService }}{{- end -}}{{ end -}}; - } + } {{- end }} {{- range $e := $s.ErrorPageLocations }} @@ -672,7 +676,7 @@ server { {{- with $ssl := $s.SSL }} {{ if $ssl.HTTP2 }} - location @grpc_deadline_exceeded { + location @grpc_deadline_exceeded { default_type application/grpc; add_header content-type application/grpc; add_header grpc-status 4; @@ -728,6 +732,6 @@ server { return 204; } - {{ end }} + {{ end }} {{ end }} } diff --git a/internal/configs/version2/nginx.virtualserver.tmpl b/internal/configs/version2/nginx.virtualserver.tmpl index 59a86b5e52..35376ecf88 100644 --- a/internal/configs/version2/nginx.virtualserver.tmpl +++ b/internal/configs/version2/nginx.virtualserver.tmpl @@ -424,7 +424,7 @@ server { {{- with $ssl := $s.SSL }} {{ if $ssl.HTTP2 }} - location @grpc_deadline_exceeded { + location @grpc_deadline_exceeded { default_type application/grpc; add_header content-type application/grpc; add_header grpc-status 4; @@ -480,6 +480,6 @@ server { return 204; } - {{ end }} + {{ end }} {{ end }} } diff --git a/internal/configs/version2/template_helper.go b/internal/configs/version2/template_helper.go index 667f158e94..a31003e928 100644 --- a/internal/configs/version2/template_helper.go +++ b/internal/configs/version2/template_helper.go @@ -231,6 +231,23 @@ func makeServerName(s StreamServer) string { return fmt.Sprintf("server_name \"%s\";", s.ServerName) } +// boolToInteger will do the following conversions: +// false -> 0 +// true -> 1 +// +// An example for where it's used is the OIDC PKCE Enable flag. +func boolToInteger(b bool) int { + // The compiler currently only optimizes this form. + // See issue 6011. + var i int + if b { + i = 1 + } else { + i = 0 + } + return i +} + var helperFunctions = template.FuncMap{ "headerListToCIMap": headerListToCIMap, "hasCIKey": hasCIKey, @@ -246,4 +263,5 @@ var helperFunctions = template.FuncMap{ "makeHeaderQueryValue": makeHeaderQueryValue, "makeTransportListener": makeTransportListener, "makeServerName": makeServerName, + "boolToInteger": boolToInteger, } diff --git a/internal/configs/version2/template_helper_test.go b/internal/configs/version2/template_helper_test.go index 58d166657d..cc9a53067b 100644 --- a/internal/configs/version2/template_helper_test.go +++ b/internal/configs/version2/template_helper_test.go @@ -815,3 +815,31 @@ func newReplaceAll(t *testing.T) *template.Template { } return tmpl } + +func TestBoolToInt(t *testing.T) { + cases := []struct { + name string + in bool + want int + }{ + { + name: "converts true to 1", + in: true, + want: 1, + }, + { + name: "converts false to 0", + in: false, + want: 0, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got := boolToInteger(tc.in) + if got != tc.want { + t.Errorf("boolToInteger(%v) returned %v but expected %v.", tc.in, got, tc.want) + } + }) + } +} diff --git a/internal/configs/version2/templates_test.go b/internal/configs/version2/templates_test.go index 53729954d3..ae02e5f264 100644 --- a/internal/configs/version2/templates_test.go +++ b/internal/configs/version2/templates_test.go @@ -809,6 +809,27 @@ func TestExecuteVirtualServerTemplate_WithCustomOIDCRedirectLocation(t *testing. if !bytes.Contains(got, []byte(expectedRedirVar)) { t.Errorf("Should set $redir_location to custom value: %s", expectedRedirVar) } +} + +func TestExecuteVirtualServerTemplateWithOIDCAndPKCEPolicyNGINXPlus(t *testing.T) { + t.Parallel() + + e := newTmplExecutorNGINXPlus(t) + got, err := e.ExecuteVirtualServerTemplate(&virtualServerCfgWithOIDCAndPKCETurnedOn) + if err != nil { + t.Error(err) + } + + want := "include oidc/oidc_pkce_supplements.conf" + want2 := "include oidc/oidc.conf;" + + if !bytes.Contains(got, []byte(want)) { + t.Errorf("want %q in generated template", want) + } + + if !bytes.Contains(got, []byte(want2)) { + t.Errorf("want %q in generated template", want2) + } snaps.MatchSnapshot(t, string(got)) t.Log(string(got)) @@ -2416,6 +2437,22 @@ var ( }, } + virtualServerCfgWithOIDCAndPKCETurnedOn = VirtualServerConfig{ + Server: Server{ + ServerName: "example.com", + StatusZone: "example.com", + ProxyProtocol: true, + OIDC: &OIDC{ + PKCEEnable: true, + }, + Locations: []Location{ + { + Path: "/", + }, + }, + }, + } + transportServerCfg = TransportServerConfig{ Upstreams: []StreamUpstream{ { diff --git a/internal/configs/virtualserver.go b/internal/configs/virtualserver.go index 1c4e11d5d6..c0bf3e5733 100644 --- a/internal/configs/virtualserver.go +++ b/internal/configs/virtualserver.go @@ -1371,24 +1371,35 @@ func (p *policiesCfg) addOIDCConfig( } } else { secretKey := fmt.Sprintf("%v/%v", polNamespace, oidc.ClientSecret) - secretRef := secretRefs[secretKey] + secretRef, ok := secretRefs[secretKey] + clientSecret := []byte("") - var secretType api_v1.SecretType - if secretRef.Secret != nil { - secretType = secretRef.Secret.Type - } - if secretType != "" && secretType != secrets.SecretTypeOIDC { - res.addWarningf("OIDC policy %s references a secret %s of a wrong type '%s', must be '%s'", polKey, secretKey, secretType, secrets.SecretTypeOIDC) - res.isError = true - return res - } else if secretRef.Error != nil { - res.addWarningf("OIDC policy %s references an invalid secret %s: %v", polKey, secretKey, secretRef.Error) + if ok { + var secretType api_v1.SecretType + if secretRef.Secret != nil { + secretType = secretRef.Secret.Type + } + if secretType != "" && secretType != secrets.SecretTypeOIDC { + res.addWarningf("OIDC policy %s references a secret %s of a wrong type '%s', must be '%s'", polKey, secretKey, secretType, secrets.SecretTypeOIDC) + res.isError = true + return res + } else if secretRef.Error != nil && !oidc.PKCEEnable { + res.addWarningf("OIDC policy %s references an invalid secret %s: %v", polKey, secretKey, secretRef.Error) + res.isError = true + return res + } else if oidc.PKCEEnable { + res.addWarningf("OIDC policy %s has a secret and PKCE enabled. Secrets can't be used with PKCE", polKey) + res.isError = true + return res + } + + clientSecret = secretRef.Secret.Data[ClientSecretKey] + } else if !oidc.PKCEEnable { + res.addWarningf("Client secret is required for OIDC policy %s when not using PKCE", polKey) res.isError = true return res } - clientSecret := secretRef.Secret.Data[ClientSecretKey] - redirectURI := oidc.RedirectURI if redirectURI == "" { redirectURI = "/_codexch" @@ -1419,6 +1430,7 @@ func (p *policiesCfg) addOIDCConfig( PostLogoutRedirectURI: postLogoutRedirectURI, ZoneSyncLeeway: generateIntFromPointer(oidc.ZoneSyncLeeway, 200), AccessTokenEnable: oidc.AccessTokenEnable, + PKCEEnable: oidc.PKCEEnable, } oidcPolCfg.key = polKey } diff --git a/internal/configs/virtualserver_test.go b/internal/configs/virtualserver_test.go index cd5170afdd..e183b55f15 100644 --- a/internal/configs/virtualserver_test.go +++ b/internal/configs/virtualserver_test.go @@ -10652,6 +10652,7 @@ func TestGeneratePoliciesFails(t *testing.T) { EndSessionEndpoint: "http://foo.com/bar", PostLogoutRedirectURI: "/_logout", AccessTokenEnable: true, + PKCEEnable: true, }, }, }, @@ -11138,6 +11139,101 @@ func TestGeneratePoliciesFails(t *testing.T) { expectedOidc: &oidcPolicyCfg{}, msg: "multi waf", }, + { + policyRefs: []conf_v1.PolicyReference{ + { + Name: "oidc-policy", + Namespace: "default", + }, + }, + policies: map[string]*conf_v1.Policy{ + "default/oidc-policy": { + ObjectMeta: meta_v1.ObjectMeta{ + Name: "oidc-policy", + Namespace: "default", + }, + 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", + EndSessionEndpoint: "https://foo.com/logout", + PostLogoutRedirectURI: "/_logout", + ClientID: "foo", + AccessTokenEnable: true, + PKCEEnable: true, + }, + }, + }, + }, + policyOpts: policyOptions{ + secretRefs: map[string]*secrets.SecretReference{ + "default/oidc-secret": { + Secret: &api_v1.Secret{ + Type: secrets.SecretTypeOIDC, + Data: map[string][]byte{ + "client-secret": []byte("super_secret_123"), + }, + }, + }, + }, + }, + context: "route", + expected: policiesCfg{ + ErrorReturn: &version2.Return{ + Code: 500, + }, + }, + expectedWarnings: Warnings{ + nil: { + `OIDC policy default/oidc-policy has a secret and PKCE enabled. Secrets can't be used with PKCE`, + }, + }, + expectedOidc: &oidcPolicyCfg{}, + msg: "oidc pkce yes secret yes", + }, + { + policyRefs: []conf_v1.PolicyReference{ + { + Name: "oidc-policy", + Namespace: "default", + }, + }, + policies: map[string]*conf_v1.Policy{ + "default/oidc-policy": { + ObjectMeta: meta_v1.ObjectMeta{ + Name: "oidc-policy", + Namespace: "default", + }, + Spec: conf_v1.PolicySpec{ + OIDC: &conf_v1.OIDC{ + AuthEndpoint: "https://foo.com/auth", + TokenEndpoint: "https://foo.com/token", + JWKSURI: "https://foo.com/certs", + EndSessionEndpoint: "https://foo.com/logout", + PostLogoutRedirectURI: "/_logout", + ClientID: "foo", + AccessTokenEnable: true, + PKCEEnable: false, + }, + }, + }, + }, + context: "route", + expected: policiesCfg{ + ErrorReturn: &version2.Return{ + Code: 500, + }, + }, + expectedWarnings: Warnings{ + nil: { + `Client secret is required for OIDC policy default/oidc-policy when not using PKCE`, + }, + }, + expectedOidc: &oidcPolicyCfg{}, + msg: "oidc pkce no secret no", + }, } for _, test := range tests { diff --git a/internal/k8s/controller.go b/internal/k8s/controller.go index 3435e07de1..dcdca18b94 100644 --- a/internal/k8s/controller.go +++ b/internal/k8s/controller.go @@ -2832,6 +2832,10 @@ func (lbc *LoadBalancerController) addOIDCSecretRefs(secretRefs map[string]*secr continue } + if pol.Spec.OIDC.PKCEEnable { + continue + } + secretKey := fmt.Sprintf("%v/%v", pol.Namespace, pol.Spec.OIDC.ClientSecret) secretRef := lbc.secretStore.GetSecret(secretKey) diff --git a/pkg/apis/configuration/v1/types.go b/pkg/apis/configuration/v1/types.go index 8f93dfbe96..5f379f31a7 100644 --- a/pkg/apis/configuration/v1/types.go +++ b/pkg/apis/configuration/v1/types.go @@ -685,6 +685,7 @@ type OIDC struct { ZoneSyncLeeway *int `json:"zoneSyncLeeway"` AuthExtraArgs []string `json:"authExtraArgs"` AccessTokenEnable bool `json:"accessTokenEnable"` + PKCEEnable bool `json:"pkceEnable"` } // WAF defines an WAF policy. diff --git a/pkg/apis/configuration/validation/policy.go b/pkg/apis/configuration/validation/policy.go index c687a43938..0e48dc2c84 100644 --- a/pkg/apis/configuration/validation/policy.go +++ b/pkg/apis/configuration/validation/policy.go @@ -265,15 +265,15 @@ func validateOIDC(oidc *v1.OIDC, fieldPath *field.Path) field.ErrorList { if oidc.ClientID == "" { return field.ErrorList{field.Required(fieldPath.Child("clientID"), "")} } - if oidc.ClientSecret == "" { - return field.ErrorList{field.Required(fieldPath.Child("clientSecret"), "")} - } if oidc.EndSessionEndpoint == "" && oidc.PostLogoutRedirectURI != "" { msg := "postLogoutRedirectURI can only be set when endSessionEndpoint is set" return field.ErrorList{field.Forbidden(fieldPath.Child("postLogoutRedirectURI"), msg)} } allErrs := field.ErrorList{} + + allErrs = append(allErrs, validatePKCE(oidc.PKCEEnable, oidc.ClientSecret, fieldPath.Child("clientSecret"))...) + if oidc.Scope != "" { allErrs = append(allErrs, validateOIDCScope(oidc.Scope, fieldPath.Child("scope"))...) } @@ -470,6 +470,26 @@ func validateOIDCScope(scope string, fieldPath *field.Path) field.ErrorList { return nil } +// validatePKCE checks for the duo of PKCEEnable and clientSecret settings. +// +// - yes PKCE and not empty client secret is bad, because PKCE does not use +// client secret +// - no PKCE and empty client secret is bad because standard OIDC uses client +// secrets +func validatePKCE(PKCEEnable bool, clientSecret string, + clientSecretPath *field.Path, +) field.ErrorList { + if !PKCEEnable && clientSecret == "" { + return field.ErrorList{field.Required(clientSecretPath, "clientSecret is required when PKCE is not used")} + } + + if PKCEEnable && clientSecret != "" { + return field.ErrorList{field.Forbidden(clientSecretPath, "clientSecret cannot be used when PKCE is used")} + } + + return nil +} + func validateURL(name string, fieldPath *field.Path) field.ErrorList { u, err := url.Parse(name) if err != nil { diff --git a/site/content/configuration/policy-resource.md b/site/content/configuration/policy-resource.md index 727509c481..552edc2526 100644 --- a/site/content/configuration/policy-resource.md +++ b/site/content/configuration/policy-resource.md @@ -649,6 +649,7 @@ spec: endSessionEndpoint: https://idp.example.com/openid-connect/logout postLogoutRedirectURI: / accessTokenEnable: true + pkceEnable: false ``` NGINX Plus will pass the ID of an authenticated user to the backend in the HTTP header `username`. @@ -678,7 +679,7 @@ The OIDC policy defines a few internal locations that can't be customized: `/_jw |Field | Description | Type | Required | | ---| ---| ---| --- | |``clientID`` | The client ID provided by your OpenID Connect provider. | ``string`` | Yes | -|``clientSecret`` | The name of the Kubernetes secret that stores the client secret provided by your OpenID Connect provider. It must be in the same namespace as the Policy resource. The secret must be of the type ``nginx.org/oidc``, and the secret under the key ``client-secret``, otherwise the secret will be rejected as invalid. | ``string`` | Yes | +|``clientSecret`` | The name of the Kubernetes secret that stores the client secret provided by your OpenID Connect provider. It must be in the same namespace as the Policy resource. The secret must be of the type ``nginx.org/oidc``, and the secret under the key ``client-secret``, otherwise the secret will be rejected as invalid. If PKCE is enabled, this should be not configured. | ``string`` | Yes | |``authEndpoint`` | URL for the authorization endpoint provided by your OpenID Connect provider. | ``string`` | Yes | |``authExtraArgs`` | A list of extra URL arguments to pass to the authorization endpoint provided by your OpenID Connect provider. Arguments must be URL encoded, multiple arguments may be included in the list, for example ``[ arg1=value1, arg2=value2 ]`` | ``string[]`` | No | |``tokenEndpoint`` | URL for the token endpoint provided by your OpenID Connect provider. | ``string`` | Yes | @@ -689,6 +690,7 @@ The OIDC policy defines a few internal locations that can't be customized: `/_jw |``postLogoutRedirectURI`` | URI to redirect to after the logout has been performed. Requires ``endSessionEndpoint``. The default is ``/_logout``. | ``string`` | No | |``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 | +|``pkceEnable`` | Switches Proof Key for Code Exchange on. The OpenID client needs to be in public mode. `clientSecret` is not used in this mode. | ``boolean`` | No | {{% /table %}} {{< note >}} diff --git a/site/content/includes/installation/create-common-resources.md b/site/content/includes/installation/create-common-resources.md index d30f555255..5ebea07176 100644 --- a/site/content/includes/installation/create-common-resources.md +++ b/site/content/includes/installation/create-common-resources.md @@ -18,6 +18,12 @@ In this section, you'll create resources that most NGINX Ingress Controller inst kubectl apply -f deployments/common/nginx-config.yaml ``` + If you're using NGINX Plus, you will also need to apply the management config map as well + + ```shell + kubectl apply -f deployments/common/plus-mgmt-configmap.yaml + ``` + 3. Create an `IngressClass` resource. NGINX Ingress Controller won't start without an `IngressClass` resource. ```shell diff --git a/tests/data/oidc/pkce.yaml b/tests/data/oidc/pkce.yaml new file mode 100644 index 0000000000..92af0c1305 --- /dev/null +++ b/tests/data/oidc/pkce.yaml @@ -0,0 +1,14 @@ +apiVersion: k8s.nginx.org/v1 +kind: Policy +metadata: + name: oidc-policy +spec: + oidc: + clientID: nginx-plus-pkce + authEndpoint: https://keycloak.example.com/realms/master/protocol/openid-connect/auth + tokenEndpoint: http://keycloak.default.svc.cluster.local:8080/realms/master/protocol/openid-connect/token + jwksURI: http://keycloak.default.svc.cluster.local:8080/realms/master/protocol/openid-connect/certs + endSessionEndpoint: https://keycloak.example.com/realms/master/protocol/openid-connect/logout + scope: openid+profile+email + accessTokenEnable: true + pkceEnable: true diff --git a/tests/suite/test_oidc.py b/tests/suite/test_oidc.py index 70f7aa2704..da4f55d7f2 100644 --- a/tests/suite/test_oidc.py +++ b/tests/suite/test_oidc.py @@ -14,6 +14,7 @@ create_secret_from_yaml, delete_common_app, delete_secret, + delete_service, replace_configmap_from_yaml, wait_before_test, wait_until_all_pods_are_ready, @@ -30,6 +31,7 @@ keycloak_vs_src = f"{TEST_DATA}/oidc/virtual-server-idp.yaml" oidc_secret_src = f"{TEST_DATA}/oidc/client-secret.yaml" oidc_pol_src = f"{TEST_DATA}/oidc/oidc.yaml" +pkce_pol_src = f"{TEST_DATA}/oidc/pkce.yaml" oidc_vs_src = f"{TEST_DATA}/oidc/virtual-server.yaml" orig_vs_src = f"{TEST_DATA}/virtual-server-tls/standard/virtual-server.yaml" cm_src = f"{TEST_DATA}/oidc/nginx-config.yaml" @@ -81,6 +83,23 @@ def keycloak_setup(request, kube_apis, test_namespace, ingress_controller_endpoi } response = requests.post(create_user_url, headers=headers, json=user_payload, verify=False) + # Create client "nginx-plus-pkce" for the pkce test + create_pkce_client_url = f"https://{ingress_controller_endpoint.public_ip}:{ingress_controller_endpoint.port_ssl}/admin/realms/master/clients" + pkce_client_payload = { + "clientId": "nginx-plus-pkce", + "redirectUris": ["https://virtual-server-tls.example.com:443/_codexch"], + "standardFlowEnabled": True, + "directAccessGrantsEnabled": False, + "publicClient": True, + "attributes": { + "post.logout.redirect.uris": "https://virtual-server-tls.example.com:443/*", + "pkce.code.challenge.method": "S256", + }, + "protocol": "openid-connect", + } + pkce_client_resp = requests.post(create_pkce_client_url, headers=headers, json=pkce_client_payload, verify=False) + pkce_client_resp.raise_for_status() + # Create client "nginx-plus" and get secret create_client_url = f"https://{ingress_controller_endpoint.public_ip}:{ingress_controller_endpoint.port_ssl}/realms/master/clients-registrations/default" client_payload = { @@ -128,6 +147,7 @@ def fin(): ) class TestOIDC: @pytest.mark.parametrize("configmap", [cm_src, cm_zs_src]) + @pytest.mark.parametrize("oidcYaml", [oidc_pol_src, pkce_pol_src]) def test_oidc( self, request, @@ -139,6 +159,7 @@ def test_oidc( virtual_server_setup, keycloak_setup, configmap, + oidcYaml, ): print(f"Create oidc secret") with open(oidc_secret_src) as f: @@ -147,7 +168,7 @@ def test_oidc( secret_name = create_secret(kube_apis.v1, test_namespace, secret_data) print(f"Create oidc policy") - with open(oidc_pol_src) as f: + with open(oidcYaml) as f: doc = yaml.safe_load(f) pol = doc["metadata"]["name"] doc["spec"]["oidc"]["tokenEndpoint"] = doc["spec"]["oidc"]["tokenEndpoint"].replace("default", test_namespace) @@ -188,6 +209,11 @@ def test_oidc( patch_virtual_server_from_yaml( kube_apis.custom_objects, virtual_server_setup.vs_name, orig_vs_src, test_namespace ) + if configmap == cm_src: + with open(svc_src) as f: + headless_svc = yaml.safe_load(f) + headless_name = headless_svc["metadata"]["name"] + delete_service(kube_apis.v1, headless_name, ingress_controller_prerequisites.namespace) def run_oidc(browser_type, ip_address, port):