Skip to content

LCORE-694: add skip_for_health_probes support to jwk-token and api-key-token auth modules #1788

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

Merged
tisnik merged 1 commit into from
May 26, 2026
+39 −3
Merged

LCORE-694: add skip_for_health_probes support to jwk-token and api-key-token auth modules #1788

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
20 changes: 19 additions & 1 deletion src/authentication/api_key_token.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,9 @@

from fastapi import HTTPException, Request, status

from authentication.interface import AuthInterface, AuthTuple
from authentication.interface import NO_AUTH_TUPLE, AuthInterface, AuthTuple
from authentication.utils import extract_user_token
from configuration import configuration
from constants import (
DEFAULT_USER_NAME,
DEFAULT_USER_UID,
Expand All @@ -23,6 +24,19 @@
logger = get_logger(__name__)


def _should_skip_auth(request: Request) -> bool:
"""Check if auth should be skipped for health probe or metrics endpoints."""
auth_config = configuration.authentication_configuration
if auth_config.skip_for_health_probes and request.url.path in (
"/readiness",
"/liveness",
):
return True
if auth_config.skip_for_metrics and request.url.path in ("/metrics",):
return True
return False
Comment on lines +27 to +37
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot May 25, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial | ⚡ Quick win

Duplicate helper function across auth modules.

This _should_skip_auth function is identical to the one in jwk_token.py (and likely mirrors logic in k8s.py/rh_identity.py). Consider extracting it to authentication/utils.py to centralize the skip-auth logic and avoid maintaining identical code in multiple modules.

Additionally, per coding guidelines, the docstring should follow Google conventions with Parameters and Returns sections.

♻️ Proposed refactor: extract to utils.py

Add to src/authentication/utils.py:

def should_skip_auth(request: Request, auth_config: AuthenticationConfiguration) -> bool:
    """Check if authentication should be skipped for health probe or metrics endpoints.

    Parameters:
        request: The incoming FastAPI request.
        auth_config: The authentication configuration containing skip flags.

    Returns:
        True if authentication should be bypassed, False otherwise.
    """
    if auth_config.skip_for_health_probes and request.url.path in ("/readiness", "/liveness"):
        return True
    if auth_config.skip_for_metrics and request.url.path in ("/metrics",):
        return True
    return False

Then import and use in both auth modules:

-def _should_skip_auth(request: Request) -> bool:
-    """Check if auth should be skipped for health probe or metrics endpoints."""
-    auth_config = configuration.authentication_configuration
-    if auth_config.skip_for_health_probes and request.url.path in ("/readiness", "/liveness"):
-        return True
-    if auth_config.skip_for_metrics and request.url.path in ("/metrics",):
-        return True
-    return False
+from authentication.utils import should_skip_auth
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/authentication/api_key_token.py` around lines 27 - 34, Extract the
duplicated _should_skip_auth logic into a single helper should_skip_auth in a
new src/authentication/utils.py, changing its signature to
should_skip_auth(request: Request, auth_config: AuthenticationConfiguration) and
move the health/metrics checks there; update api_key_token._should_skip_auth and
jwk_token._should_skip_auth to import and call should_skip_auth(request,
configuration.authentication_configuration) (or remove the duplicate functions
and call the util directly), and replace the docstring with a Google-style
docstring including Parameters and Returns as shown in the proposed refactor;
ensure no behavior change and remove the duplicated helper from other auth
modules (k8s.py, rh_identity.py) to centralize logic.

Copy link
Copy Markdown
Contributor

@asimurka asimurka May 26, 2026

Choose a reason for hiding this comment

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

@Jos-Jerus this is actually a good point by code rabbit. It would be nice to extract the function to a shared module and reuse it.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot May 26, 2026

Choose a reason for hiding this comment

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

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!



class APIKeyTokenAuthDependency(
AuthInterface
): # pylint: disable=too-few-public-methods
Expand Down Expand Up @@ -59,6 +73,10 @@ async def __call__(self, request: Request) -> AuthTuple:
HTTPException: If the bearer token is missing or
doesn't match the configured API key (HTTP 401).
"""
if not request.headers.get("Authorization"):
if _should_skip_auth(request):
return NO_AUTH_TUPLE

# try to extract user token from request
user_token = extract_user_token(request.headers)
Comment thread
coderabbitai[bot] marked this conversation as resolved.

Expand Down
22 changes: 20 additions & 2 deletions src/authentication/jwk_token.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,9 @@
from cachetools import TTLCache
from fastapi import HTTPException, Request

from authentication.interface import AuthInterface, AuthTuple
from authentication.interface import NO_AUTH_TUPLE, AuthInterface, AuthTuple
from authentication.utils import extract_user_token
from configuration import configuration
from constants import (
DEFAULT_VIRTUAL_PATH,
)
Expand Down Expand Up @@ -141,6 +142,19 @@ def _internal(header: dict[str, Any], _payload: dict[str, Any]) -> Key:
return _internal


def _should_skip_auth(request: Request) -> bool:
"""Check if auth should be skipped for health probe or metrics endpoints."""
auth_config = configuration.authentication_configuration
if auth_config.skip_for_health_probes and request.url.path in (
"/readiness",
"/liveness",
):
return True
if auth_config.skip_for_metrics and request.url.path in ("/metrics",):
return True
return False


class JwkTokenAuthDependency(AuthInterface): # pylint: disable=too-few-public-methods
"""JWK AuthDependency class for JWK-based JWT authentication."""

Expand All @@ -166,7 +180,9 @@ def __init__(
self.config: JwkConfiguration = config
self.skip_userid_check = False

async def __call__(self, request: Request) -> AuthTuple:
async def __call__( # pylint: disable=too-many-statements
self, request: Request
) -> AuthTuple:
"""Authenticate the JWT in the headers against the keys from the JWK url.

When the Authorization header is missing, this method raises
Expand All @@ -190,6 +206,8 @@ async def __call__(self, request: Request) -> AuthTuple:
authentication; all error paths raise HTTPException.
"""
if not request.headers.get("Authorization"):
if _should_skip_auth(request):
return NO_AUTH_TUPLE
response = UnauthorizedResponse(cause="No Authorization header found")
raise HTTPException(**response.model_dump())

Expand Down
Loading