RSPEED-2466: Replace verbose rh-identity error messages with opaque responses

[major]

Description

The rh-identity auth module returned error messages that revealed the full expected JSON structure of the x-rh-identity header (field names, nesting, valid identity types, required entitlements). An attacker could reconstruct the schema in ~9 requests by iterating on malformed headers.

All 11 verbose error messages now return generic responses ("Invalid identity data" / "Insufficient entitlements"), with detailed diagnostics moved to logger.warning() for ops troubleshooting.

Type of change

• Bug fix

Tools used to create PR

• Assisted-by: Claude
• Generated by: N/A

Related Tickets & Documents

• Closes RSPEED-2466

Checklist before requesting a review

• I have performed a self-review of my code.
• PR has passed all pre-merge test jobs.
• If it is a core feature, I have added thorough tests.

Testing

• uv run pytest tests/unit/authentication/test_rh_identity.py — 50/50 pass
• uv run make verify — pylint 10/10, pyright 0 errors, mypy clean, all linters pass

Summary by CodeRabbit

• Bug Fixes

  • Standardized authentication error messages: field-specific messages replaced with generic "Invalid identity data" and entitlements errors now return "Insufficient entitlements" while preserving HTTP status codes; added warning logs for validation issues.

• Tests

  • Updated unit and end-to-end tests to expect the new, standardized error messages and to assert warning logs.

[coderabbitai]

Walkthrough

Replaces detailed identity- and entitlement-specific HTTP error texts with standardized messages ("Invalid identity data", "Insufficient entitlements") and adds warning logs capturing detailed validation info; updates unit and e2e tests to expect the new messages.

Changes

Cohort / File(s)	Summary
Identity validation logic src/authentication/rh_identity.py	Replaced many specific 400/403 error details with generic messages ("Invalid identity data", "Insufficient entitlements") and added warning logs that include field/entitlement details prior to raising HTTP exceptions.
Unit tests tests/unit/authentication/test_rh_identity.py	Updated expectations to the new generic error messages and added logger warning mocks/assertions for identity and entitlement validation failures.
End-to-end feature tests/e2e/features/authorized_rh_identity.feature	Adjusted scenario expectations to match new standardized error messages for identity and entitlement failures (status codes unchanged).

Sequence Diagram(s)

(omitted — changes are message standardization and logging, not new multi-component control flow)

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• LCORE-261: Add RH identity header auth #814 — changes touch the same RH identity authentication logic in src/authentication/rh_identity.py.
• test(e2e): add comprehensive e2e tests for rh-identity authentication #1045 — also modifies RH identity validation/error messaging in src/authentication/rh_identity.py.

Suggested labels

ok-to-test

Suggested reviewers

• tisnik
• radofuchs

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately reflects the main objective: replacing verbose error messages with generic responses in the rh-identity module.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

tests/unit/authentication/test_rh_identity.py (1)

236-303: Consider asserting that logger.warning() fires on every validation failure path.

The warning logs are the only mechanism providing diagnostic detail after this change — their absence would silently degrade operational observability with no HTTP-visible symptom. None of the failure-path tests currently verify that a warning is emitted. Adding caplog or a mocker.patch assertion would create a regression guard for the key behavioral change this PR introduces.

Example using caplog:

def test_validation_failures(
    self, missing_field: dict, expected_error: str, caplog: pytest.LogCaptureFixture
) -> None:
    with caplog.at_level(logging.WARNING, logger="authentication.rh_identity"):
        with pytest.raises(HTTPException) as exc_info:
            RHIdentityData(missing_field)
    assert exc_info.value.status_code == 400
    assert expected_error in str(exc_info.value.detail)
    assert any("Identity validation failed" in r.message for r in caplog.records)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/unit/authentication/test_rh_identity.py` around lines 236 - 303, Update
the two tests (test_validation_failures and test_unsupported_identity_type) to
assert that a warning is logged when RHIdentityData validation fails: wrap the
RHIdentityData instantiation in a caplog.at_level(logging.WARNING,
logger="authentication.rh_identity") context (or alternatively patch
logger.warning via mocker.patch) and after the HTTPException assertion assert
that caplog recorded a message containing the validation-warning text (e.g.,
"Identity validation failed") or that logger.warning was called; this ensures
every failure path emits the expected warning from RHIdentityData.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/authentication/rh_identity.py`:
- Around line 102-105: The logger.warning call currently interpolates the
user-controlled identity_type (derived from identity["type"]) and is vulnerable
to log injection; update the logging in the identity validation path (the
logger.warning call referencing identity_type) to stop logging raw user input —
either log a static message like "Identity validation failed: unsupported
identity type received" or sanitize identity_type before logging by
removing/control characters (e.g., strip newlines and other control chars or
escape them) and then log the sanitized value. Ensure changes are made where
identity_type is defined/used in this module so all warning messages follow the
safe pattern.

---

Nitpick comments:
In `@tests/unit/authentication/test_rh_identity.py`:
- Around line 236-303: Update the two tests (test_validation_failures and
test_unsupported_identity_type) to assert that a warning is logged when
RHIdentityData validation fails: wrap the RHIdentityData instantiation in a
caplog.at_level(logging.WARNING, logger="authentication.rh_identity") context
(or alternatively patch logger.warning via mocker.patch) and after the
HTTPException assertion assert that caplog recorded a message containing the
validation-warning text (e.g., "Identity validation failed") or that
logger.warning was called; this ensures every failure path emits the expected
warning from RHIdentityData.

[tisnik]

LGTM

[tisnik]

Please fix e2e too (it's simple)

[major]

@tisnik This should fix it! 🤞

[coderabbitai]

🧹 Nitpick comments (1)

tests/unit/authentication/test_rh_identity.py (1)

207-234: Consider asserting the new validate_entitlements warning in test_validate_entitlements.

The PR adds a logger.warning(...) call in validate_entitlements (Lines 180–183 of rh_identity.py), but neither test_validate_entitlements nor test_entitlement_validation (Lines 428–449) verify that it fires. This is consistent with how test_validation_failures and test_unsupported_identity_type were updated to verify _validate_structure warnings.

♻️ Proposed addition to test_validate_entitlements (should_raise=True branch)

 def test_validate_entitlements(
     self,
     user_identity_data: dict,
     required_entitlements: Optional[list[str]],
     should_raise: bool,
     expected_error: Optional[str],
+    mocker: MockerFixture,
 ) -> None:
+    mock_warning = mocker.patch("authentication.rh_identity.logger.warning")
     rh_identity = RHIdentityData(user_identity_data, required_entitlements)

     if should_raise:
         with pytest.raises(HTTPException) as exc_info:
             rh_identity.validate_entitlements()
         assert exc_info.value.status_code == 403
         assert expected_error is not None
         assert expected_error in str(exc_info.value.detail)
+        mock_warning.assert_called_once()
+        assert "Entitlement validation failed" in mock_warning.call_args[0][0]
     else:
         rh_identity.validate_entitlements()

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/unit/authentication/test_rh_identity.py` around lines 207 - 234, Update
test_validate_entitlements to assert the new logger.warning emitted by
RHIdentityData.validate_entitlements: when should_raise is True, wrap the call
in the same logging capture used in
test_validation_failures/test_unsupported_identity_type (e.g., caplog) and
assert a WARNING-level record was emitted containing the expected warning text
(or part of it) from validate_entitlements; keep the existing HTTPException
assertions and follow the same pattern used in the other tests to locate and
assert the warning.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@tests/unit/authentication/test_rh_identity.py`:
- Around line 207-234: Update test_validate_entitlements to assert the new
logger.warning emitted by RHIdentityData.validate_entitlements: when
should_raise is True, wrap the call in the same logging capture used in
test_validation_failures/test_unsupported_identity_type (e.g., caplog) and
assert a WARNING-level record was emitted containing the expected warning text
(or part of it) from validate_entitlements; keep the existing HTTPException
assertions and follow the same pattern used in the other tests to locate and
assert the warning.