HYPERFLEET-1165 - docs: add multi-tenant identity and authz design (DRAFT)#189
HYPERFLEET-1165 - docs: add multi-tenant identity and authz design (DRAFT)#189pnguyen44 wants to merge 1 commit into
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds a multi-tenant identity and authorization design covering OPA middleware hooks, multi-issuer JWT claim mapping, Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 11✅ Passed checks (11 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with 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.
Inline comments:
In `@hyperfleet/docs/glossary.md`:
- Around line 66-68: Update the tenant_id glossary entry to state that it is
server-owned and immutable: clients must not supply or modify it, updates must
preserve the existing tenant, and system identities must not create tenantless
or cross-tenant resources. Retain the existing creation and DAO filtering
description while making clear that filtering does not replace write-path
enforcement.
In `@hyperfleet/docs/multi-tenant-identity-authz-design.md`:
- Line 330: Update the TODO in the multi-tenant identity authorization design
document to include the tracked ticket ID for filling it after POC results are
available, using the project’s required ticket format.
- Line 299: Update the “Trust contract (if gateway-first is adopted later)”
section to remove shared internal network access as an acceptable authentication
mechanism. Require authenticated gateway-to-API mTLS or a cryptographically
signed tenant header, explicitly strip or reject client-supplied X-Tenant-ID
values, and reject direct API traffic whenever gateway-derived identity is
required.
- Around line 118-120: Update the authorization design and implementation around
OPA availability to require fail-closed behavior for all protected operations,
removing configurable fail-open access. Define explicit OPA request timeouts and
bounded retries, and restrict authorization bypasses to health endpoints only;
flag the security-boundary bypass in the relevant documentation.
- Around line 159-161: Update the “Claim-to-tenant resolution” design so raw
claim values cannot collide across issuers: require an issuer-qualified tenant
identity key or a mandatory raw-to-canonical tenant mapping with uniqueness
constraints. Replace the current assumption that raw claim values may be stored
directly as tenant_id, and document the selected resolution behavior for all
issuers.
- Around line 216-231: Update the Shared Sentinel with system identity design
and its corresponding mitigation section to require explicit validation for
system tokens: a dedicated internal audience, exact service-account
subject/workload binding, approved issuer and signing algorithm allowlists,
short token lifetimes, and rejection of direct external access. State that token
rotation is insufficient unless the API validates the projected service-account
token format and issuer.
- Around line 159-163: Update the Claim-to-tenant resolution and tenant-scoped
middleware design to explicitly reject tokens when the configured tenant_claim
is missing, empty, or malformed, returning the documented consistent 401/403
response. Ensure rejection occurs before any DAO access and never falls back to
an unscoped query; align the behavior with the existing missing-claim contract
in the upgrade guide.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 8e94c46e-9d58-46c1-97ab-035c05a37947
📒 Files selected for processing (2)
hyperfleet/docs/glossary.mdhyperfleet/docs/multi-tenant-identity-authz-design.md
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
openshift-hyperfleet/architecture(manual)openshift-hyperfleet/hyperfleet-api(manual)openshift-hyperfleet/hyperfleet-sentinel(manual)openshift-hyperfleet/hyperfleet-adapter(manual)openshift-hyperfleet/hyperfleet-broker(manual)
19e835f to
83cdb05
Compare
83cdb05 to
3466f0e
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with 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.
Inline comments:
In `@hyperfleet/docs/multi-tenant-identity-authz-design.md`:
- Around line 193-203: Update the Query Filtering recommendation and related
tenant-scoped identity design to require binding tenant_id on writes from
authenticated context, not caller input alone. Validate at HTTP handlers, CLI
parsers, and webhook receivers; reject caller-supplied mismatches, keep
tenant_id immutable for tenant-scoped identities, and allow changes only through
explicit system authorization.
- Around line 313-330: The recommended application-only Go middleware must
perform full JWT validation at the HTTP boundary, including signature, issuer,
audience, lifetime, and allowed algorithm checks; do not limit these checks to
the future gateway-first option or rely on signature-only validation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: aa76dc1e-4080-40ed-bc21-2b9edee50e05
📒 Files selected for processing (2)
hyperfleet/docs/glossary.mdhyperfleet/docs/multi-tenant-identity-authz-design.md
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
openshift-hyperfleet/architecture(manual)openshift-hyperfleet/hyperfleet-api(manual)openshift-hyperfleet/hyperfleet-sentinel(manual)openshift-hyperfleet/hyperfleet-adapter(manual)openshift-hyperfleet/hyperfleet-broker(manual)
🚧 Files skipped from review as they are similar to previous changes (1)
- hyperfleet/docs/glossary.md
| ### Query Filtering | ||
|
|
||
| Options: | ||
|
|
||
| | Approach | Pros | Cons | | ||
| |----------|------|------| | ||
| | Implicit `WHERE tenant_id = ?` on all queries | Safer (can't forget it), enforced at DAO layer | Less flexible, internal services (Sentinel, Adapter) need a bypass | | ||
| | Explicit query parameter | More flexible, caller decides scope | Easy to forget, creates a security hole if someone omits it | | ||
| | PostgreSQL Row-Level Security (RLS) | Database-level enforcement as safety net | Requires setting a session variable per request, adds complexity to connection pooling | | ||
|
|
||
| **Recommendation:** Use implicit filtering at the DAO layer. It prevents accidental cross-tenant data leaks because developers can't forget the WHERE clause. Internal services that need cross-tenant access (Sentinel, Adapter) would use a bypass mechanism (see Sentinel/Adapter Identity section). RLS can be layered on top as defense-in-depth but should not be the sole mechanism. |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Bind tenant_id on writes, not only reads.
DAO filtering prevents cross-tenant reads, but the design does not define how create/update requests protect resource ownership. A caller-supplied tenant_id could create or update another tenant’s resource. Derive it from authenticated context, reject mismatches, and make it immutable for tenant-scoped identities; permit changes only through explicit system authorization. CWE-639, CWE-915.
As per path instructions, validate input at system boundaries (HTTP handlers, CLI parsers, webhook receivers).
🤖 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 `@hyperfleet/docs/multi-tenant-identity-authz-design.md` around lines 193 -
203, Update the Query Filtering recommendation and related tenant-scoped
identity design to require binding tenant_id on writes from authenticated
context, not caller input alone. Validate at HTTP handlers, CLI parsers, and
webhook receivers; reject caller-supplied mismatches, keep tenant_id immutable
for tenant-scoped identities, and allow changes only through explicit system
authorization.
Source: Path instructions
| ### Option: Application-only (current approach) | ||
|
|
||
| Pros: | ||
|
|
||
| - Portable. Works anywhere without infrastructure dependencies. | ||
| - Single source of truth for auth logic (Go middleware). | ||
| - Testable without infrastructure: no gateway to simulate in unit/integration tests. | ||
|
|
||
| Cons: | ||
|
|
||
| - Every request (including invalid ones) reaches the application before being rejected. | ||
| - If HyperFleet adds more services in the future, each would reimplement the same auth logic. | ||
|
|
||
| ### Recommendation | ||
|
|
||
| **Application-only for now.** JWT validation and tenant extraction both happen in Go middleware. Only the API receives external traffic; Sentinel and Adapter are internal-only. There's no multi-service gateway benefit. The middleware approach is portable and doesn't depend on NetworkPolicy for security (the JWT signature itself prevents spoofing). | ||
|
|
||
| Gateway-layer validation can be added later as a performance optimization without changing application code (defense-in-depth, not replacement). |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Apply full JWT validation to the recommended application-only path.
The explicit signature, issuer, audience, lifetime, and algorithm checks are scoped to the future gateway-first option, while the current Go middleware is the trust boundary. Require the same checks in application middleware; signature-only validation can accept tokens intended for another audience or issuer. CWE-347, CWE-287.
As per path instructions, validate input at system boundaries (HTTP handlers, CLI parsers, webhook receivers).
🤖 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 `@hyperfleet/docs/multi-tenant-identity-authz-design.md` around lines 313 -
330, The recommended application-only Go middleware must perform full JWT
validation at the HTTP boundary, including signature, issuer, audience,
lifetime, and allowed algorithm checks; do not limit these checks to the future
gateway-first option or rely on signature-only validation.
Source: Path instructions
Summary
HYPERFLEET-1165
Design document for adding multi-tenant resource isolation to HyperFleet. Evaluates two paths: OPA-based authorization (Path A) and JWT claim mapping with DAO-layer filtering (Path B). Includes recommendations for Sentinel/Adapter identity bypass and gateway vs application-layer boundaries.