From b811dcff831762533ca876a4952652d54cb7c33a Mon Sep 17 00:00:00 2001 From: Russ Allbery Date: Tue, 7 Jan 2025 16:27:39 -0800 Subject: [PATCH] Stop escaping : in Ingress auth-url ingress-nginx 4.12.0 has added a restrictive regex filter on the auth-url annotation that rejects `%` characters and therefore all escaped characters in URLs. It does allow `:` unescaped, and leaving `:` unescaped in query parameters is safe according to the relevant RFC, so add `:` to the set of safe characters that do not need to be escaped.` --- changelog.d/20250107_161611_rra_DM_48326.md | 3 +++ src/gafaelfawr/services/kubernetes.py | 2 +- tests/data/kubernetes/output/ingresses.yaml | 12 ++++++------ tests/operator/ingress_test.py | 4 ++-- 4 files changed, 12 insertions(+), 9 deletions(-) create mode 100644 changelog.d/20250107_161611_rra_DM_48326.md diff --git a/changelog.d/20250107_161611_rra_DM_48326.md b/changelog.d/20250107_161611_rra_DM_48326.md new file mode 100644 index 000000000..f4289adfd --- /dev/null +++ b/changelog.d/20250107_161611_rra_DM_48326.md @@ -0,0 +1,3 @@ +### Bug fixes + +- Do not escape `:` characters in the `auth-url` `Ingress` annotation. ingress-nginx 4.12.0 has added a restrictive regex filter to acceptable URLs that disallows `%` and therefore all escaped characters. \ No newline at end of file diff --git a/src/gafaelfawr/services/kubernetes.py b/src/gafaelfawr/services/kubernetes.py index a794776d4..78c33d288 100644 --- a/src/gafaelfawr/services/kubernetes.py +++ b/src/gafaelfawr/services/kubernetes.py @@ -109,7 +109,7 @@ def _build_annotations(self, ingress: GafaelfawrIngress) -> dict[str, str]: auth_url = ( str(self._config.base_internal_url).rstrip("/") + "/ingress/auth?" - + urlencode(ingress.config.to_auth_query()) + + urlencode(ingress.config.to_auth_query(), safe=":/") ) base_url = ingress.config.base_url or str(self._config.base_url) snippet_key = "nginx.ingress.kubernetes.io/configuration-snippet" diff --git a/tests/data/kubernetes/output/ingresses.yaml b/tests/data/kubernetes/output/ingresses.yaml index 7a1111332..204962797 100644 --- a/tests/data/kubernetes/output/ingresses.yaml +++ b/tests/data/kubernetes/output/ingresses.yaml @@ -6,7 +6,7 @@ metadata: annotations: nginx.ingress.kubernetes.io/auth-method: GET nginx.ingress.kubernetes.io/auth-response-headers: "Authorization,Cookie,X-Auth-Request-Email,X-Auth-Request-Service,X-Auth-Request-Token,X-Auth-Request-User" - nginx.ingress.kubernetes.io/auth-url: "http://gafaelfawr.gafaelfawr.svc.cluster.local:8080/ingress/auth?scope=read%3Aall&service=tap" + nginx.ingress.kubernetes.io/auth-url: "http://gafaelfawr.gafaelfawr.svc.cluster.local:8080/ingress/auth?scope=read:all&service=tap" nginx.ingress.kubernetes.io/configuration-snippet: | {snippet} creationTimestamp: {any} @@ -50,7 +50,7 @@ metadata: nginx.ingress.kubernetes.io/auth-method: "GET" nginx.ingress.kubernetes.io/auth-response-headers: "Authorization,Cookie,X-Auth-Request-Email,X-Auth-Request-Service,X-Auth-Request-Token,X-Auth-Request-User" nginx.ingress.kubernetes.io/auth-signin: "https://foo.example.com/login" - nginx.ingress.kubernetes.io/auth-url: "http://gafaelfawr.gafaelfawr.svc.cluster.local:8080/ingress/auth?scope=read%3Aall&satisfy=any¬ebook=true&minimum_lifetime=600" + nginx.ingress.kubernetes.io/auth-url: "http://gafaelfawr.gafaelfawr.svc.cluster.local:8080/ingress/auth?scope=read:all&satisfy=any¬ebook=true&minimum_lifetime=600" nginx.ingress.kubernetes.io/configuration-snippet: | {snippet} creationTimestamp: {any} @@ -95,7 +95,7 @@ metadata: annotations: nginx.ingress.kubernetes.io/auth-method: GET nginx.ingress.kubernetes.io/auth-response-headers: "Authorization,Cookie,X-Auth-Request-Email,X-Auth-Request-Service,X-Auth-Request-Token,X-Auth-Request-User" - nginx.ingress.kubernetes.io/auth-url: "http://gafaelfawr.gafaelfawr.svc.cluster.local:8080/ingress/auth?scope=read%3Aall&scope=read%3Asome&delegate_to=some-service&delegate_scope=read%3Aall%2Cread%3Asome&auth_type=basic" + nginx.ingress.kubernetes.io/auth-url: "http://gafaelfawr.gafaelfawr.svc.cluster.local:8080/ingress/auth?scope=read:all&scope=read:some&delegate_to=some-service&delegate_scope=read:all%2Cread:some&auth_type=basic" nginx.ingress.kubernetes.io/configuration-snippet: | {snippet} creationTimestamp: {any} @@ -135,7 +135,7 @@ metadata: annotations: nginx.ingress.kubernetes.io/auth-method: GET nginx.ingress.kubernetes.io/auth-response-headers: "Authorization,Cookie,X-Auth-Request-Email,X-Auth-Request-Service,X-Auth-Request-Token,X-Auth-Request-User" - nginx.ingress.kubernetes.io/auth-url: "http://gafaelfawr.gafaelfawr.svc.cluster.local:8080/ingress/auth?scope=read%3Aall&delegate_to=some-service&delegate_scope=read%3Aall&use_authorization=true" + nginx.ingress.kubernetes.io/auth-url: "http://gafaelfawr.gafaelfawr.svc.cluster.local:8080/ingress/auth?scope=read:all&delegate_to=some-service&delegate_scope=read:all&use_authorization=true" nginx.ingress.kubernetes.io/configuration-snippet: | add_header "X-Foo" "bar"; {snippet} @@ -215,7 +215,7 @@ metadata: annotations: nginx.ingress.kubernetes.io/auth-method: GET nginx.ingress.kubernetes.io/auth-response-headers: "Authorization,Cookie,X-Auth-Request-Email,X-Auth-Request-Service,X-Auth-Request-Token,X-Auth-Request-User" - nginx.ingress.kubernetes.io/auth-url: "http://gafaelfawr.gafaelfawr.svc.cluster.local:8080/ingress/auth?scope=read%3Aall&username=some-user" + nginx.ingress.kubernetes.io/auth-url: "http://gafaelfawr.gafaelfawr.svc.cluster.local:8080/ingress/auth?scope=read:all&username=some-user" nginx.ingress.kubernetes.io/configuration-snippet: | {snippet} creationTimestamp: {any} @@ -255,7 +255,7 @@ metadata: annotations: nginx.ingress.kubernetes.io/auth-method: GET nginx.ingress.kubernetes.io/auth-response-headers: "Authorization,Cookie,X-Auth-Request-Email,X-Auth-Request-Service,X-Auth-Request-Token,X-Auth-Request-User" - nginx.ingress.kubernetes.io/auth-url: "http://gafaelfawr.gafaelfawr.svc.cluster.local:8080/ingress/auth?scope=read%3Aall&only_service=portal&only_service=vo-cutouts&service=uws" + nginx.ingress.kubernetes.io/auth-url: "http://gafaelfawr.gafaelfawr.svc.cluster.local:8080/ingress/auth?scope=read:all&only_service=portal&only_service=vo-cutouts&service=uws" nginx.ingress.kubernetes.io/configuration-snippet: | {snippet} creationTimestamp: {any} diff --git a/tests/operator/ingress_test.py b/tests/operator/ingress_test.py index 3002ed873..f6fe45630 100644 --- a/tests/operator/ingress_test.py +++ b/tests/operator/ingress_test.py @@ -80,7 +80,7 @@ async def test_replace( expected_url = ( "http://gafaelfawr.gafaelfawr.svc.cluster.local:8080/ingress/auth" - "?scope=read%3Aall&service=tap&auth_type=basic" + "?scope=read:all&service=tap&auth_type=basic" ) expected["metadata"]["annotations"][ "nginx.ingress.kubernetes.io/auth-url" @@ -111,7 +111,7 @@ async def test_replace( expected_url = ( "http://gafaelfawr.gafaelfawr.svc.cluster.local:8080/ingress/auth" - "?scope=read%3Aall&service=tap&auth_type=bearer" + "?scope=read:all&service=tap&auth_type=bearer" ) expected["metadata"]["annotations"][ "nginx.ingress.kubernetes.io/auth-url"