Skip to content

refactor(test): restructure OIDC policy tests and add redirect bug coverage#1011

Open
crstrn13 wants to merge 3 commits into
Kuadrant:mainfrom
crstrn13:refactor/oidc-tests
Open

refactor(test): restructure OIDC policy tests and add redirect bug coverage#1011
crstrn13 wants to merge 3 commits into
Kuadrant:mainfrom
crstrn13:refactor/oidc-tests

Conversation

@crstrn13

@crstrn13 crstrn13 commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Description

Restructures the OIDC policy test suite for clarity, reduces duplication, and adds new test coverage for the OIDCPolicy bug fixes from kuadrant-operator#2032.

Changes

Test restructuring:

  • Removed the OIDCTestClient wrapper class (testsuite/oidc/test_client.py) — tests now create Keycloak clients directly via ClientConfig + KeycloakOpenID, removing an unnecessary abstraction layer
  • Moved ClientConfig dataclass to testsuite/oidc/keycloak/objects.py alongside other Keycloak object wrappers
  • Extracted the shared auth fixture to the OIDC conftest (was duplicated across 4 test files)
  • Split monolithic test functions into focused, single-concern tests across dedicated modules: confidential client, public client, service client, JWT validation, and redirect handling

New test coverage (kuadrant-operator#2032):

  • test_oidc_policy_redirect.py — validates fixes for:
    • Redirect URI includes non-standard listener port (tests with port 8001)
    • Target cookie preserves query string parameters after OIDC auth redirect
    • Authenticated requests with query parameters succeed
  • test_oidc_policy_custom_redirect.py — validates custom redirectURI:
    • When provider.redirectURI is set, the authorize redirect uses the custom callback URL
    • The custom callback path replaces the default /auth/callback

Infrastructure:

  • LoadBalancerServiceExposer hostname now includes cluster.project to avoid collisions when multiple namespaces share a domain

Known gaps

  • OPA cookie parser fix for = in JWT base64 values (operator Bug 2) — this is only exercisable via the full OIDC authorization code flow (callback route), which requires programmatic Keycloak login. The operator has unit tests for this fix.

Verification

  • Ran full OIDC test suite against a Kind cluster with Kuadrant deployed
  • All linting checks pass (pylint, mypy, black)

crstrn13 added 2 commits June 12, 2026 16:23
Signed-off-by: Alexander Cristurean <acristur@redhat.com>
Signed-off-by: Alexander Cristurean <acristur@redhat.com>
@coderabbitai

coderabbitai Bot commented Jul 2, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 53380b27-ba53-43d2-aa2b-b2765adf0196

📥 Commits

Reviewing files that changed from the base of the PR and between 1185d1a and 36f2d11.

📒 Files selected for processing (10)
  • testsuite/gateway/exposers.py
  • testsuite/oidc/keycloak/objects.py
  • testsuite/oidc/test_client.py
  • testsuite/tests/singlecluster/extensions/oidc_policy/conftest.py
  • testsuite/tests/singlecluster/extensions/oidc_policy/test_oidc_policy_confidential_client.py
  • testsuite/tests/singlecluster/extensions/oidc_policy/test_oidc_policy_custom_redirect.py
  • testsuite/tests/singlecluster/extensions/oidc_policy/test_oidc_policy_jwt_validation.py
  • testsuite/tests/singlecluster/extensions/oidc_policy/test_oidc_policy_public_client.py
  • testsuite/tests/singlecluster/extensions/oidc_policy/test_oidc_policy_redirect.py
  • testsuite/tests/singlecluster/extensions/oidc_policy/test_oidc_policy_service_client.py
💤 Files with no reviewable changes (1)
  • testsuite/oidc/test_client.py
👮 Files not reviewed due to content moderation or server errors (9)
  • testsuite/tests/singlecluster/extensions/oidc_policy/test_oidc_policy_custom_redirect.py
  • testsuite/tests/singlecluster/extensions/oidc_policy/test_oidc_policy_service_client.py
  • testsuite/tests/singlecluster/extensions/oidc_policy/test_oidc_policy_confidential_client.py
  • testsuite/tests/singlecluster/extensions/oidc_policy/test_oidc_policy_public_client.py
  • testsuite/oidc/keycloak/objects.py
  • testsuite/tests/singlecluster/extensions/oidc_policy/test_oidc_policy_jwt_validation.py
  • testsuite/tests/singlecluster/extensions/oidc_policy/conftest.py
  • testsuite/gateway/exposers.py
  • testsuite/tests/singlecluster/extensions/oidc_policy/test_oidc_policy_redirect.py

📝 Walkthrough

[!WARNING]

Walkthrough skipped

File diffs could not be summarized.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@crstrn13 crstrn13 marked this pull request as draft July 2, 2026 11:06
Signed-off-by: Alexander Cristurean <acristur@redhat.com>
@crstrn13 crstrn13 force-pushed the refactor/oidc-tests branch from 05c67ae to 36f2d11 Compare July 2, 2026 11:13
@crstrn13 crstrn13 marked this pull request as ready for review July 2, 2026 11:19
@crstrn13 crstrn13 self-assigned this Jul 2, 2026

def expose_hostname(self, name, gateway: Gateway) -> Hostname:
hostname = f"{name}.{self.base_domain}"
hostname = f"{name}-{self.cluster.project}.{self.base_domain}"

@crstrn13 crstrn13 Jul 2, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

OpenShiftExposer creates Routes and the resulting hostname is {name}.{cluster.apps_url}, which naturally includes the project context since Routes are namespace-scoped. On Kind, the LoadBalancerServiceExposer was just doing {name}.test.com — no project/namespace in the hostname at all. Adding cluster.project makes the Kind hostname format mirror what OpenShift produces, so tests don't break when switching between the two environments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant