Skip to content

feat(x509): add test for standalone Authorino x509.source.expression #939

New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Open
averevki wants to merge 2 commits into
base: main
Choose a base branch
from
Open

feat(x509): add test for standalone Authorino x509.source.expression #939

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ec04f1e
feat(x509): renamed authorino standalone tls tests to x509
averevki Apr 17, 2026
c9107ee
feat(x509): add test for standalone Authorino x509.source.expression …
averevki Apr 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion testsuite/kuadrant/policy/authorization/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,4 +189,4 @@ class X509Source:

xfccHeader: Optional[str] = None
clientCertHeader: Optional[str] = None
expression: Optional[CelExpression] = None
expression: Optional[str] = None
Copy link
Copy Markdown
Member

@azgabur azgabur Apr 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why change this to string? Also you forgot to change the documenation

Copy link
Copy Markdown
Contributor Author

@averevki averevki Apr 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because expression is not a separate key here, it is part of the authorino x509 identity spec

Copy link
Copy Markdown
Contributor Author

@averevki averevki Apr 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what documentation are you referring to?

0 ...ecluster/authorino/operator/tls/README.md → ...cluster/authorino/operator/x509/README.md
View file
Open in desktop
File renamed without changes.
0 ...luster/authorino/operator/tls/__init__.py → ...uster/authorino/operator/x509/__init__.py
View file
Open in desktop
File renamed without changes.
0 ...luster/authorino/operator/tls/conftest.py → ...uster/authorino/operator/x509/conftest.py
View file
Open in desktop
File renamed without changes.
0 ...r/authorino/operator/tls/mtls/__init__.py → ...rator/x509/gateway_validation/__init__.py
View file
Open in desktop
File renamed without changes.
6 changes: 3 additions & 3 deletions ...r/authorino/operator/tls/mtls/conftest.py → ...rator/x509/gateway_validation/conftest.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
"""Conftest for all mTLS tests"""
"""Conftest for all x509 identity tests"""

import pytest

Expand All @@ -9,8 +9,8 @@

@pytest.fixture(scope="module", autouse=True)
def authorization(authorization, blame, selector, cert_attributes):
"""Create AuthConfig with mtls identity and pattern matching rule"""
authorization.identity.add_mtls(blame("mtls"), selector=selector)
"""Create AuthConfig with x509 identity and pattern matching rule"""
authorization.identity.add_mtls(blame("x509"), selector=selector)

rule_organization = Pattern("auth.identity.Organization", "incl", cert_attributes["O"])
authorization.authorization.add_auth_rules(blame("redhat"), [rule_organization])
Expand Down
10 changes: 5 additions & 5 deletions ...operator/tls/mtls/test_mtls_attributes.py → ...ateway_validation/test_x509_attributes.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
"""Tests on mTLS authentication with multiple attributes"""
"""Tests on x509 identity authentication with multiple attributes"""

import pytest

Expand All @@ -16,15 +16,15 @@ def authorization(authorization, blame, cert_attributes):
return authorization


def test_mtls_multiple_attributes_success(envoy_authority, valid_cert, hostname):
"""Test successful mtls authentication with two matching attributes"""
def test_x509_multiple_attributes_success(envoy_authority, valid_cert, hostname):
"""Test successful x509 authentication with two matching attributes"""
with hostname.client(verify=envoy_authority, cert=valid_cert) as client:
response = client.get("/get")
assert response.status_code == 200


def test_mtls_multiple_attributes_fail(envoy_authority, custom_cert, hostname):
"""Test mtls authentication with one matched and one unmatched attributes"""
def test_x509_multiple_attributes_fail(envoy_authority, custom_cert, hostname):
"""Test x509 authentication with one matched and one unmatched attributes"""
Comment on lines +26 to +27
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Apr 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Fix the singular/plural mismatch in the docstring.

Line 27 should use singular “attribute” after “one unmatched”.

Suggested wording update 
-    """Test x509 authentication with one matched and one unmatched attributes"""
+    """Test x509 authentication with one matched and one unmatched attribute"""
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In
`@testsuite/tests/singlecluster/authorino/operator/x509/gateway_validation/test_x509_attributes.py`
around lines 26 - 27, Update the docstring for
test_x509_multiple_attributes_fail to fix the singular/plural mismatch: change
"one unmatched attributes" to "one unmatched attribute" so the sentence reads
that the test covers "one matched and one unmatched attribute"; edit the
docstring string inside the test_x509_multiple_attributes_fail function
accordingly.

with hostname.client(verify=envoy_authority, cert=custom_cert) as client:
response = client.get("/get")
assert response.status_code == 403
37 changes: 37 additions & 0 deletions ...te/tests/singlecluster/authorino/operator/x509/gateway_validation/test_x509_cel_source.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
"""Test x509 client certificate authentication with CEL expression on AuthConfig"""

import pytest

from testsuite.kuadrant.policy.authorization import X509Source

pytestmark = [pytest.mark.authorino, pytest.mark.standalone_only]


@pytest.fixture(scope="module", autouse=True)
def authorization(authorization, blame, selector):
"""AuthConfig with x509 identity using CEL expression for certificate source"""
authorization.identity.add_mtls(
blame("x509"), selector=selector, source=X509Source(expression="source.certificate")
)
return authorization


def test_x509_success(envoy_authority, valid_cert, hostname):
"""Test successful x509 authentication with CEL expression"""
with hostname.client(verify=envoy_authority, cert=valid_cert) as client:
response = client.get("/get")
assert response.status_code == 200


def test_x509_invalid_cert(envoy_authority, invalid_cert, hostname):
"""Test that a certificate signed by an untrusted CA is rejected"""
with hostname.client(verify=envoy_authority, cert=invalid_cert) as client:
result = client.get("/get")
assert result.has_unknown_ca_error()
Comment on lines +26 to +30
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Apr 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Inspect unknown-CA assertions versus explicit Authorino 401 x509 rejection coverage.

set -euo pipefail

echo "Result.has_unknown_ca_error definition and call sites:"
rg -n -C3 '\bdef\s+has_unknown_ca_error\b|\bhas_unknown_ca_error\s*\(' --iglob '*.py'

echo
echo "Existing x509 tests that assert explicit 401 rejection:"
rg -n -C4 'intermediate_cert_unlabeled|status_code\s*==\s*401' --iglob '*.py'

Repository: Kuadrant/testsuite

Length of output: 50374

🏁 Script executed:

#!/bin/bash
# Check the conftest.py for test_x509_cel_source.py to understand available fixtures and certificates

find . -path "*/singlecluster/authorino/operator/x509/gateway_validation/conftest.py" -type f | head -1 | xargs cat

Repository: Kuadrant/testsuite

Length of output: 2107

Assert Authorino-side 401 for wrong-CA coverage.

The test currently uses has_unknown_ca_error(), which validates a TLS-level certificate verification failure during the handshake itself. This means the request is rejected by Envoy before reaching Authorino, not exercising the x509 identity validation at the Authorino level. Per the PR objective ("certificate signed by wrong CA → 401 Unauthorized"), use a client certificate that Envoy accepts but Authorino rejects, then assert response.status_code == 401 instead. Existing tests (test_xfcc_forwarding.py, test_x509_trust_chain.py) demonstrate this pattern; the intermediate_cert_unlabeled fixture available in conftest.py is suitable for this purpose.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In
`@testsuite/tests/singlecluster/authorino/operator/x509/gateway_validation/test_x509_cel_source.py`
around lines 26 - 30, The test test_x509_invalid_cert currently asserts a TLS
handshake failure via has_unknown_ca_error(), but we need to exercise
Authorino's x509 rejection and assert a 401; replace the invalid_cert fixture
with the intermediate_cert_unlabeled fixture (which Envoy will accept but
Authorino should reject), keep using hostname.client(verify=envoy_authority,
cert=intermediate_cert_unlabeled) so the handshake succeeds, perform the GET and
assert response.status_code == 401 instead of calling has_unknown_ca_error().



def test_x509_no_cert(envoy_authority, hostname):
"""Test that a request without a client certificate is rejected"""
with hostname.client(verify=envoy_authority) as client:
result = client.get("/get")
assert result.has_cert_required_error()
12 changes: 6 additions & 6 deletions ...o/operator/tls/mtls/test_mtls_identity.py → .../gateway_validation/test_x509_identity.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
"""mTLS authentication tests"""
"""x509 identity authentication tests"""

from typing import Callable
import pytest
Expand All @@ -8,8 +8,8 @@
pytestmark = [pytest.mark.authorino, pytest.mark.standalone_only]


def test_mtls_success(envoy_authority, valid_cert, hostname):
"""Test successful mtls authentication"""
def test_x509_success(envoy_authority, valid_cert, hostname):
"""Test successful x509 authentication"""
with hostname.client(verify=envoy_authority, cert=valid_cert) as client:
response = client.get("/get")
assert response.status_code == 200
Expand All @@ -24,8 +24,8 @@ def test_mtls_success(envoy_authority, valid_cert, hostname):
pytest.param("invalid_authority", "valid_cert", Result.has_cert_verify_error, id="Unknown authority"),
],
)
def test_mtls_fail(request, cert_authority, certificate, check_error: Callable, hostname):
"""Test failed mtls verification"""
def test_x509_fail(request, cert_authority, certificate, check_error: Callable, hostname):
"""Test failed x509 verification"""
ca = request.getfixturevalue(cert_authority)
cert = request.getfixturevalue(certificate) if certificate else None

Expand All @@ -34,7 +34,7 @@ def test_mtls_fail(request, cert_authority, certificate, check_error: Callable,
assert check_error(result)


def test_mtls_unmatched_attributes(envoy_authority, custom_cert, hostname):
def test_x509_unmatched_attributes(envoy_authority, custom_cert, hostname):
"""Test certificate that signed by the trusted CA, though their attributes are unmatched"""
with hostname.client(verify=envoy_authority, cert=custom_cert) as client:
response = client.get("/get")
Expand Down
14 changes: 7 additions & 7 deletions ...perator/tls/mtls/test_mtls_trust_chain.py → ...teway_validation/test_x509_trust_chain.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
"""mTLS trust chain tests"""
"""x509 identity trust chain tests"""

import pytest

Expand All @@ -12,22 +12,22 @@ def create_intermediate_authority_secrets(create_secret, authorino_labels, certi
create_secret(certificates["intermediate_ca_unlabeled"], "interunld")


def test_mtls_trust_chain_success(envoy_authority, certificates, hostname):
"""Test mtls verification with certificate signed by intermediate authority in the trust chain"""
def test_x509_trust_chain_success(envoy_authority, certificates, hostname):
"""Test x509 verification with certificate signed by intermediate authority in the trust chain"""
with hostname.client(verify=envoy_authority, cert=certificates["intermediate_valid_cert"]) as client:
response = client.get("/get")
assert response.status_code == 200


def test_mtls_trust_chain_fail(envoy_authority, certificates, hostname):
"""Test mtls verification on intermediate certificate with unmatched attribute"""
def test_x509_trust_chain_fail(envoy_authority, certificates, hostname):
"""Test x509 verification on intermediate certificate with unmatched attribute"""
with hostname.client(verify=envoy_authority, cert=certificates["intermediate_custom_cert"]) as client:
response = client.get("/get")
assert response.status_code == 403


def test_mtls_trust_chain_rejected_cert(envoy_authority, certificates, hostname):
"""Test mtls verification with intermediate certificate accepted in Envoy, but rejected by Authorino"""
def test_x509_trust_chain_rejected_cert(envoy_authority, certificates, hostname):
"""Test x509 verification with intermediate certificate accepted in Envoy, but rejected by Authorino"""
with hostname.client(verify=envoy_authority, cert=certificates["intermediate_cert_unlabeled"]) as client:
response = client.get("/get")
assert response.status_code == 401
5 changes: 1 addition & 4 deletions ...er/authorino/operator/tls/test_webhook.py → ...r/authorino/operator/x509/test_webhook.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,7 @@ def specific_authorino_name(blame):

@pytest.fixture(scope="module")
def authorino_domain(cluster, specific_authorino_name):
"""
Hostname of the upstream certificate sent to be validated by APIcast
May be overwritten to configure different test cases
"""
"""Returns the Authorino service hostname used for webhook configuration"""
return f"{specific_authorino_name}-authorino-authorization.{cluster.project}.svc"


Expand Down
2 changes: 1 addition & 1 deletion ...luster/authorino/operator/tls/test_tls.py → ...ster/authorino/operator/x509/test_x509.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ def test_no_certificate(hostname, envoy_authority):


def test_invalid_certificate(envoy_authority, invalid_cert, auth, hostname):
"""Tests that certificate with different CA will be rejeceted"""
"""Tests that certificate with different CA will be rejected"""
with hostname.client(verify=envoy_authority, cert=invalid_cert) as client:
result = client.get("/get", auth=auth)
assert result.has_unknown_ca_error()
Loading