[LFXV2-257] Heimdall Middleware - forward body config#49
Conversation
- Update lfx-platform Helm chart to version 0.2.7, add JSON content type validation to Heimdall middleware, and introduce new middleware options for body forwarding. Jira Ticket: https://linuxfoundation.atlassian.net/browse/LFXV2-257 Signed-off-by: Mauricio Zanetti Salomao <mauriciozanetti86@gmail.com>
mauriciozanettisalomao
requested review from
a team and
emsearcy
as code owners
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughBumps Helm chart to 0.2.7, adds a Traefik Middleware Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant C as Client
participant T as Traefik
participant M as Middleware (heimdall-forward-body)
participant H as Heimdall (forwardAuth)
participant S as Backend
C->>T: HTTP request (may include body)
T->>M: Apply middleware
M->>H: forwardAuth request (headers + body)
H-->>M: Auth decision (allow / deny)
alt allowed
M-->>T: allow
T->>S: Proxy original request (body preserved)
S-->>T: Response
T-->>C: Response
else denied
M-->>T: deny (401/403)
T-->>C: 401/403
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
✨ Finishing Touches🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Pull Request Overview
This pull request enhances the Heimdall authentication middleware in the LFX Platform Helm chart to support forwarding request bodies for authentication decisions. This improvement enables Heimdall to inspect request bodies when making authorization decisions, which is particularly important for endpoints where authorization depends on data in the request payload.
- Adds a new Heimdall middleware with body forwarding capability for comprehensive authentication
- Introduces JSON content-type validation in Heimdall policies to enforce proper content formatting
- Updates chart version from 0.2.5 to 0.2.7
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| charts/lfx-platform/values.yaml | Adds JSON content-type validation rule to Heimdall policies |
| charts/lfx-platform/templates/heimdall/middleware.yaml | Creates new middleware with body forwarding and documents usage scenarios |
| charts/lfx-platform/Chart.yaml | Updates chart version to 0.2.7 |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Signed-off-by: Mauricio Zanetti Salomao <mauriciozanetti86@gmail.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
charts/lfx-platform/templates/heimdall/middleware.yaml (1)
7-7: Fix trailing spaces flagged by linter.Remove trailing spaces on comment lines.
-# This is the default middleware that should be used in most cases, particularly +# This is the default middleware that should be used in most cases, particularly -# Note: For routes handling very large payloads (like file uploads), consider using +# Note: For routes handling very large payloads (like file uploads), consider usingAlso applies to: 9-9
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
charts/lfx-platform/Chart.yaml(1 hunks)charts/lfx-platform/templates/heimdall/middleware.yaml(1 hunks)charts/lfx-platform/values.yaml(1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
charts/lfx-platform/templates/heimdall/middleware.yaml
[error] 7-7: trailing spaces
(trailing-spaces)
[error] 9-9: trailing spaces
(trailing-spaces)
[error] 11-11: syntax error: expected '', but found ''
(syntax)
🔇 Additional comments (3)
charts/lfx-platform/Chart.yaml (1)
8-8: Version bump looks correct.Chart version reflects a feature addition. No other metadata changes needed.
charts/lfx-platform/values.yaml (1)
195-201: Authorizer is defined but not wired into any rule.Only listing it under mechanisms doesn’t enforce it. Default rule still uses
deny_all; no rules referencejson_content_type.Please ensure body-bearing routes (POST/PUT/PATCH) add
authorizer: json_content_typebefore the OpenFGA check in their rule pipelines. If you want, I can draft the specific ruleset changes.Also applies to: 259-264
charts/lfx-platform/templates/heimdall/middleware.yaml (1)
23-36: No action required: only the/whoamiroute attaches theheimdallmiddleware (appropriate for a body-less GET), and no existing HTTPRoute requiresheimdall-forward-body.
…ng capability Jira Ticket: https://linuxfoundation.atlassian.net/browse/LFXV2-257 Signed-off-by: Mauricio Zanetti Salomao <mauriciozanetti86@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
charts/lfx-platform/Chart.yaml(1 hunks)
| description: LFX Platform v2 Helm chart | ||
| type: application | ||
| version: 0.2.6 | ||
| version: 0.2.7 |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Bump chart version to at least 0.3.0 (behavior change, not a patch).
The PR adds middleware and enforces JSON via Heimdall authorizers—this alters runtime behavior and can break existing clients. Under SemVer (even pre-1.0), features and behavior changes warrant a minor bump; breaking defaults may justify a major. Patch (0.2.7) is too low.
Apply:
-version: 0.2.7
+version: 0.3.0Also:
- Add/adjust CHANGELOG and upgrade notes (call out JSON Content-Type enforcement).
- Confirm whether the new authorizer is opt-in; if default-on and breaking, consider 1.0.0.
🏁 Script executed:
#!/bin/bash
set -e
# Search for Heimdall references in the codebase
rg -n 'Heimdall' || echo "No Heimdall reference found"
# Search for authorizer usage
rg -n 'authorizer' || echo "No authorizer reference found"
# Search for JSON enforcement
rg -n 'JSON' || echo "No JSON reference found"
# Search for middleware directory or usages
rg -n 'middleware' || echo "No middleware reference found"Length of output: 3257
#!/bin/bash
set -e
# Show the first 50 lines of Chart.yaml to verify version and metadata
sed -n '1,50p' charts/lfx-platform/Chart.yaml🏁 Script executed:
#!/bin/bash
set -e
# Display Chart.yaml header to confirm current version
sed -n '1,50p' charts/lfx-platform/Chart.yaml
# Display values.yaml around authorizers to check defaults
sed -n '160,220p' charts/lfx-platform/values.yaml
sed -n '240,300p' charts/lfx-platform/values.yamlLength of output: 5479
Bump version in Chart.yaml to 0.3.0
- Update CHANGELOG and UPGRADE notes to document the new JSON
Content-Typeenforcement - If the JSON authorizer is default-on and breaks existing clients, consider a 1.0.0 release
🤖 Prompt for AI Agents
charts/lfx-platform/Chart.yaml lines 8-8: bump the chart version from 0.2.7 to
0.3.0 and update release notes; change the version field to 0.3.0, add entries
to CHANGELOG and UPGRADE notes documenting the new enforcement of JSON
Content-Type and migration steps for clients, and if the JSON authorizer will be
enabled by default and breaks existing clients, instead plan a major 1.0.0
release (or document how to opt-out) so consumers are not unexpectedly broken.
There was a problem hiding this comment.
For now, I'll leave it as is.
There was a problem hiding this comment.
@mauriciozanettisalomao Understood! I'll respect your decision to keep the version at 0.2.7. You know your project's versioning strategy best.
✏️ Learnings added
Learnt from: mauriciozanettisalomao
PR: linuxfoundation/lfx-v2-helm#49
File: charts/lfx-platform/Chart.yaml:8-8
Timestamp: 2025-08-29T16:53:12.674Z
Learning: The maintainer mauriciozanettisalomao prefers to keep chart version increments conservative, choosing 0.2.7 over suggested 0.3.0 even for changes that could be considered behavior-altering (like new middleware and JSON Content-Type enforcement in Heimdall).
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
charts/lfx-platform/templates/heimdall/middleware.yaml (1)
17-21: On forwarding request headers: no need to add authRequestHeaders.Traefik forwards all request headers to the auth server by default;
authRequestHeadersis only for whitelisting when you want to filter. Your current config is sufficient. (doc.traefik.io, v2.doc.traefik.io)
🧹 Nitpick comments (1)
charts/lfx-platform/templates/heimdall/middleware.yaml (1)
7-10: Strip trailing spaces in comments.Minor lint fix; cleans up yamllint warnings.
-# This is the default middleware that should be used in most cases, particularly +# This is the default middleware that should be used in most cases, particularly -# Note: For routes handling very large payloads (like file uploads), consider using +# Note: For routes handling very large payloads (like file uploads), consider using
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
charts/lfx-platform/templates/heimdall/middleware.yaml(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: mauriciozanettisalomao
PR: linuxfoundation/lfx-v2-helm#49
File: charts/lfx-platform/Chart.yaml:8-8
Timestamp: 2025-08-29T16:53:12.674Z
Learning: The maintainer mauriciozanettisalomao prefers to keep chart version increments conservative, choosing 0.2.7 over suggested 0.3.0 even for changes that could be considered behavior-altering (like new middleware and JSON Content-Type enforcement in Heimdall).
📚 Learning: 2025-08-29T16:53:12.674Z
Learnt from: mauriciozanettisalomao
PR: linuxfoundation/lfx-v2-helm#49
File: charts/lfx-platform/Chart.yaml:8-8
Timestamp: 2025-08-29T16:53:12.674Z
Learning: The maintainer mauriciozanettisalomao prefers to keep chart version increments conservative, choosing 0.2.7 over suggested 0.3.0 even for changes that could be considered behavior-altering (like new middleware and JSON Content-Type enforcement in Heimdall).
Applied to files:
charts/lfx-platform/templates/heimdall/middleware.yaml
🪛 YAMLlint (1.37.1)
charts/lfx-platform/templates/heimdall/middleware.yaml
[error] 7-7: trailing spaces
(trailing-spaces)
[error] 9-9: trailing spaces
(trailing-spaces)
[error] 18-18: syntax error: expected , but found ''
(syntax)
🔇 Additional comments (4)
charts/lfx-platform/templates/heimdall/middleware.yaml (4)
5-5: Doc marker now correctly scoped within the Helm conditional.Prevents emitting a stray YAML document when the condition is false. LGTM.
17-21: Verify rendered YAML for middleware.yaml
Helm-render the template (e.g.helm template lfx charts/lfx-platform --namespace lfx --show-only charts/lfx-platform/templates/heimdall/middleware.yaml) and lint the output to confirm there are no syntax errors around theforwardAuthblock (lines 17–21).
19-21: Confirm Authorization header propagation
authResponseHeaders(lines 19–21 and 34–36) forwardsAuthorizationfrom Heimdall, overwriting any client‐supplied credentials.- If Heimdall issues its own bearer to upstreams, retaining
Authorizationis fine; otherwise remove it and forward dedicated headers (e.g.X-Heimdall-Subject,X-Heimdall-Roles) instead.
22-36: Route selection guidance: choose the appropriate Heimdall middlewareEnsure HTTPRoute definitions in charts/lfx-platform/templates/*httproute.yaml bind to
heimdall(no body forward) for large uploads or streaming to avoid unnecessary buffering/latency, and useheimdall-forward-bodyonly on endpoints whose policies require body inspection. Currently no HTTPRoute referencesheimdall-forward-body; please review and update routes accordingly.
3 participants
Overview
This pull request introduces improvements to the Heimdall authentication middleware in the Helm chart for the LFX Platform. The main changes include adding a new middleware for forwarding request bodies, providing an alternative for large payloads, and enforcing JSON content type in Heimdall policies. The chart version is also updated.
Solution based on the following PoC:
Test Summary
Project Services used as entrypoint
Test Scenario 1: Creating a Child Project with Valid Permissions
Actor: project_admin_1 (has writer access to the root project)
Parent Project: Root project (UID: efc80205-b0b3-4943-9395-2b634985a142)
Action: Create a child project
Expected Result: Success
Actual Result: Success (201 Created)
Child Project UID: cbef1ed5-17dc-4a50-84e2-6cddd70f6878
Evidence
Permission Check Request:
Permission Check Response:
{"allowed":true,"resolution":""}Project Creation Request:
Project Creation Response:
{"uid":"cbef1ed5-17dc-4a50-84e2-6cddd70f6878","slug":"test-project-slug-1","description":"project foo is a project about bar","name":"Foo Foundation","public":true,"parent_uid":"efc80205-b0b3-4943-9395-2b634985a142","stage":"Formation - Exploratory","category":"Active","funding_model":["Crowdfunding"],"charter_url":"https://example.com/charter.pdf","legal_entity_type":"Subproject","legal_entity_name":"Example Foundation LLC","legal_parent_uid":"7cad5a8d-19d0-41a4-81a6-043453daf9ee","entity_dissolution_date":"2021-12-31","entity_formation_document_url":"https://example.com/formation.pdf","autojoin_enabled":false,"formation_date":"2021-01-01","logo_url":"https://example.com/logo.png","repository_url":"https://example.com/project","website_url":"https://example.com","created_at":"2025-08-29T14:56:45Z","updated_at":"2025-08-29T14:56:45Z","mission_statement":"The mission of the project is to build a sustainable ecosystem around open source projects to accelerate technology development and industry adoption.","announcement_date":"2021-01-01","writers":["project_admin_2"],"meeting_coordinators":["user123","user456"]}Test Scenario 2: Writer Permission Propagation
Actor: project_admin_2 (added as a writer in the first child project)
Parent Project: First child project (UID: cbef1ed5-17dc-4a50-84e2-6cddd70f6878)
Action: Create a second-level project
Expected Result: Success
Actual Result: Success (201 Created)
Second-Level Project UID: 82b16a63-8c80-4e2b-ad3c-1a1a956fb32c
Evidence
Permission Check Request:
Permission Check Response:
{"allowed":true,"resolution":""}Project Creation Request:
Project Creation Response:
{"uid":"82b16a63-8c80-4e2b-ad3c-1a1a956fb32c","slug":"test-project-level-2","description":"Second level project created by project_admin_2","name":"Second Level Project","public":true,"parent_uid":"cbef1ed5-17dc-4a50-84e2-6cddd70f6878","stage":"Formation - Exploratory","category":"Active","funding_model":["Crowdfunding"],"charter_url":"https://example.com/charter2.pdf","legal_entity_type":"Subproject","legal_entity_name":"Example Foundation LLC Level 2","legal_parent_uid":"7cad5a8d-19d0-41a4-81a6-043453daf9ee","entity_dissolution_date":"2022-12-31","entity_formation_document_url":"https://example.com/formation2.pdf","autojoin_enabled":false,"formation_date":"2022-01-01","logo_url":"https://example.com/logo2.png","repository_url":"https://example.com/project2","website_url":"https://example2.com","created_at":"2025-08-29T14:58:07Z","updated_at":"2025-08-29T14:58:07Z","mission_statement":"This is a second-level project created by project_admin_2 to test permission inheritance","announcement_date":"2022-01-01","writers":["committee_member_1"],"meeting_coordinators":["user789"]}Test Scenario 3: Unauthorized Creation Attempt
Actor: committee_member_2 (no writer access to the second-level project)
Parent Project: Second-level project (UID: 82b16a63-8c80-4e2b-ad3c-1a1a956fb32c)
Action: Create a third-level project
Expected Result: Failure
Actual Result: Failure (403 Forbidden)
Evidence
Permission Check Request:
Permission Check Response:
{"allowed":false,"resolution":""}Project Creation Request:
Project Creation Response:
Conclusion
The permission checks for the POST /projects endpoint are working correctly:
The system correctly validates the
parent_uidfield in the request body against the user's permissions before allowing project creation.