Skip to content

[AAP-76188]fix: enforce TLS certificate validation for JWT and resource server - #1658

Open
wfealdel wants to merge 3 commits into
ansible:mainfrom
wfealdel:AAP-76188/prevent-token-forgery-and-auth-bypass
Open

[AAP-76188]fix: enforce TLS certificate validation for JWT and resource server#1658
wfealdel wants to merge 3 commits into
ansible:mainfrom
wfealdel:AAP-76188/prevent-token-forgery-and-auth-bypass

Conversation

@wfealdel

@wfealdel wfealdel commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

What is being changed?

The production defaults for ANSIBLE_BASE_JWT_VALIDATE_CERT and RESOURCE_SERVER__VALIDATE_HTTPS are changed from False to True in defaults.py. A startup warning is added in post_load.py that logs a SECURITY WARNING when either setting is disabled in production (non-debug) mode. Deployment documentation is updated with TLS certificate verification guidance including CA bundle configuration.

https://redhat.atlassian.net/browse/AAP-76188

Why is this change needed?

In production AAP deployments, JWT is the sole authentication mechanism. Defaulting TLS certificate verification to False exposes critical authentication channels to man-in-the-middle attacks and JWT token forgery (CTRL-010, AAP-76188).

How does this change address the issue?

  • Flips both defaults to True, enforcing secure-by-default in production
  • Logs prominent warnings at startup when verification is disabled, directing operators to configure CA bundles
  • Documents CA bundle configuration for private PKI environments in docs/deployment.md
  • Development mode (EDA_MODE=development) retains False defaults via development_defaults.py

Does this change introduce any new dependencies, blockers or breaking changes?

Breaking change: production deployments that relied on the False defaults and lack proper CA configuration will need to either configure a CA bundle or explicitly set EDA_ANSIBLE_BASE_JWT_VALIDATE_CERT=False / EDA_RESOURCE_SERVER__VALIDATE_HTTPS=False. Docker compose files for dev/stage already set these explicitly and are unaffected.

How it can be tested?

  • pytest tests/unit/test_settings.py -v — 4 new tests verify defaults are True, warnings fire in production, no warnings in debug mode
  • pytest tests/unit/test_settings_bootstrap.py -v — existing bootstrap tests pass unchanged
  • Manual: start EDA with EDA_MODE=production and EDA_ANSIBLE_BASE_JWT_VALIDATE_CERT=False — both SECURITY WARNING messages appear in logs

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Security

    • HTTPS and JWT certificate validation are now enabled by default for production connections.
    • Disabling certificate verification in production now generates a startup warning.
    • Development mode continues to support self-signed certificates by default.
  • Documentation

    • Added Docker Compose guidance for private certificate authorities and secure TLS configuration.

@wfealdel
wfealdel requested a review from a team as a code owner August 24, 2026 20:21
@coderabbitai

coderabbitai Bot commented Aug 24, 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 Plus

Run ID: 85050719-87ce-416e-9c36-d58113c89d56

📥 Commits

Reviewing files that changed from the base of the PR and between 867792e and 5b7c7df.

📒 Files selected for processing (5)
  • docs/deployment.md
  • src/aap_eda/settings/defaults.py
  • src/aap_eda/settings/post_load.py
  • src/aap_eda/utils/logging.py
  • tests/unit/test_settings.py

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

TLS certificate verification now defaults to enabled for JWT and resource-server connections. Production startup warns when verification is disabled. Startup logs and deployment documentation describe the settings and private CA options.

Changes

TLS verification configuration

Layer / File(s) Summary
Secure defaults and production warnings
src/aap_eda/settings/defaults.py, src/aap_eda/settings/post_load.py, tests/unit/test_settings.py
JWT and resource-server certificate validation default to enabled. Production startup logs warnings when either validation setting is disabled. Tests cover production, debug, and enabled configurations.
Startup visibility and deployment guidance
src/aap_eda/utils/logging.py, docs/deployment.md
Startup logging includes ANSIBLE_BASE_JWT_VALIDATE_CERT. Deployment guidance documents private CA configuration, insecure verification disabling, and development behavior.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 5b7c7

The change enforces TLS certificate validation by default in production while retaining explicit development and opt-out configurations; no actionable merge-blocking risk remains after normal checks and review.

Suggested reviewers: mkanoor

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 22.22% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 9 functions across 4 files. (1 skipped: 1… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the main change: enforcing TLS certificate validation for JWT and resource-server connections.
Description check ✅ Passed The description covers what, why, implementation, breaking changes, dependencies, testing, and the related issue link.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 22.22% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 9 functions across 4 files. (1 skipped: 1 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@wfealdel
wfealdel marked this pull request as draft August 24, 2026 20:22
@wfealdel
wfealdel force-pushed the AAP-76188/prevent-token-forgery-and-auth-bypass branch from 5c6faa0 to 8903c7c Compare August 24, 2026 20:29
@wfealdel

Copy link
Copy Markdown
Contributor Author

/run-atf-tests

@wfealdel

Copy link
Copy Markdown
Contributor Author

/run-e2e

@codecov-commenter

codecov-commenter commented Aug 24, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.39%. Comparing base (02470ba) to head (09022c6).

@@           Coverage Diff           @@
##             main    #1658   +/-   ##
=======================================
  Coverage   93.39%   93.39%           
=======================================
  Files         247      247           
  Lines       11698    11706    +8     
=======================================
+ Hits        10925    10933    +8     
  Misses        773      773           
Flag Coverage Δ
unit-int-tests-3.11 93.39% <100.00%> (+<0.01%) ⬆️
unit-int-tests-3.12 93.39% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
src/aap_eda/settings/defaults.py 100.00% <100.00%> (ø)
src/aap_eda/settings/post_load.py 95.83% <100.00%> (+0.18%) ⬆️
src/aap_eda/utils/logging.py 94.28% <ø> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@aap-pde-ci-bot

Copy link
Copy Markdown

✅ Test Results - PASSED

Summary

Metric Count
Total Tests 66
✅ Passed 50
❌ Failed 0
⚠️ Errors 0
⏭️ Skipped 16
⏱️ Duration 334.27s

Pass Rate: 75.8%

…ce server

Change ANSIBLE_BASE_JWT_VALIDATE_CERT and RESOURCE_SERVER__VALIDATE_HTTPS
defaults from False to True in production settings to prevent JWT token
forgery and authentication bypass via MITM attacks. Development mode
retains False defaults via development_defaults.py. A startup warning
is logged when either setting is disabled in non-debug mode.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@wfealdel
wfealdel force-pushed the AAP-76188/prevent-token-forgery-and-auth-bypass branch from 8903c7c to e2e9d1c Compare August 25, 2026 14:23
@wfealdel
wfealdel marked this pull request as ready for review August 26, 2026 19:58
@ptoscano

Copy link
Copy Markdown
Contributor

/run-e2e

@sonarqubecloud

Copy link
Copy Markdown

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.

4 participants