Skip to content

Comments

#2304 Add namespace access control at the gateway level#2348

Open
hu-ahmed wants to merge 1 commit intoeclipse-ditto:masterfrom
beyonnex-io:enable-certain-jwt-claims
Open

#2304 Add namespace access control at the gateway level#2348
hu-ahmed wants to merge 1 commit intoeclipse-ditto:masterfrom
beyonnex-io:enable-certain-jwt-claims

Conversation

@hu-ahmed
Copy link
Contributor

@hu-ahmed hu-ahmed commented Feb 19, 2026

Resolves: #2304

@hu-ahmed hu-ahmed force-pushed the enable-certain-jwt-claims branch from 8d8ffdb to af17314 Compare February 19, 2026 06:24
@thjaeckle thjaeckle added this to the 3.9.0 milestone Feb 19, 2026
@hu-ahmed hu-ahmed force-pushed the enable-certain-jwt-claims branch from af17314 to 89f46cc Compare February 19, 2026 08:45
@thjaeckle thjaeckle changed the title #2277 Add namespace access control at the gateway level #2304 Add namespace access control at the gateway level Feb 19, 2026
Copy link
Member

@thjaeckle thjaeckle left a comment

Choose a reason for hiding this comment

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

Review of PR #2348 — Namespace Access Control at the Gateway Level

Thanks for this contribution! The architecture is solid — condition evaluation via the existing placeholder/expression resolver system, clean separation of pattern matching and validation, and proper integration at all gateway entry points (HTTP routes, WebSocket, SSE). All core requirements from #2304 are addressed.

Below are the findings from the review, ordered by severity.


Bug

1. Missing Bearer prefix check in NamespaceAccessValidatorFactory.extractJwtFromHeaders

In NamespaceAccessValidatorFactory.java lines 92–94:

private static Optional<JsonWebToken> extractJwtFromHeaders(final DittoHeaders dittoHeaders) {
    return Optional.ofNullable(dittoHeaders.get(DittoHeaderDefinition.AUTHORIZATION.getKey()))
            .map(ImmutableJsonWebToken::fromAuthorization);
}

If the authorization header contains a non-JWT value (e.g. Basic auth or a malformed value), ImmutableJsonWebToken.fromAuthorization will throw JwtInvalidException since it expects exactly "scheme token" format.

Compare with NamespaceAccessEnforcementDirective.extractJwtFromRequest which correctly filters:

.filter(value -> value.toLowerCase().startsWith("bearer "))

This factory method is used by ThingSearchRoute and StreamingSessionActor — both code paths would fail on non-Bearer auth.


Code Issues

2. @Nullable on Optional return type

NamespaceAccessEnforcementDirective.java line 301–302:

@Nullable
private static Optional<JsonWebToken> extractJwtFromRequest(final RequestContext ctx) {

@Nullable on an Optional return type is contradictory — an Optional should never be null. The annotation should be removed.

3. @since version should be 3.9.0

NamespaceNotAccessibleException.java line 107 has @since 3.8.0. Since this is in the gateway-api module (public API), the tag should be @since 3.9.0.


Design Suggestions

4. Fake ThingId construction for namespace validation

In ThingsRoute.java (lines 541–543, 340–341) and ThingSearchRoute.java:

final ThingId tempThingId = ThingId.of(namespace + ":temp");
namespaceAccessDirective.validateNamespaceAccessForEntityId(ctx, dittoHeaders, tempThingId);

Constructing synthetic ThingIds just to extract the namespace is a workaround. The validator already has isNamespaceAccessible(String namespace). It would be cleaner if the directive also offered a validateNamespaceAccess(RequestContext ctx, DittoHeaders dittoHeaders, String namespace) method directly, avoiding the need for fake entity IDs.

5. Performance: NamespacePatternMatcher re-created on every call

NamespaceAccessValidator.isNamespaceAccessible creates a new NamespacePatternMatcher (and re-compiles regex patterns from LikeHelper.convertToRegexSyntax + Pattern.compile) for each config entry on every single call. Since the config is immutable, the matchers could be pre-built once in the NamespaceAccessValidator constructor and stored as a List<NamespacePatternMatcher>.

Similarly, StreamingSessionActor.isNamespaceAccessible creates a new NamespaceAccessValidator (and thus a new ExpressionResolver) per signal. For high-throughput event streams this could add noticeable overhead. Consider caching the validator per session (it only needs to change when the JWT refreshes).

6. Inconsistent JWT extraction — dual code paths

ThingSearchRoute receives NamespaceAccessValidatorFactory directly, while ThingsRoute and PoliciesRoute receive NamespaceAccessEnforcementDirective. This makes sense functionally (search needs filterAllowedNamespaces/getApplicableNamespacePatterns, not entity-level blocking), but it means JWT extraction logic is duplicated:

  • NamespaceAccessEnforcementDirective.extractJwtFromRequest — correctly filters for "bearer " prefix
  • NamespaceAccessValidatorFactory.extractJwtFromHeaders — no Bearer check (see bug #1)

Consider unifying the JWT extraction into a single shared utility method to avoid this kind of divergence.


Functional Gap

7. Wildcard patterns not injected into search queries

The issue explicitly raises this as an OPEN question. The current getApplicableNamespacePatterns() silently returns Optional.empty() when only wildcard allowed-namespaces are configured. This means a search with no explicit namespaces parameter will not be restricted when the config uses patterns like "org.eclipse.*".

This is likely the most common configuration pattern, so search results would not be restricted by namespace access control at all in that case. Worth documenting this limitation explicitly (e.g. in the HOCON config comments), or addressing as a follow-up.


Additional Observations

8. Config supports list of rules (undocumented)

The implementation supports multiple namespace-access rules (list of objects with OR semantics across rules). The issue only describes a single config block. This is a reasonable extension, but the HOCON example in gateway.conf only shows the single-object format — consider adding a comment or example showing the list variant and explaining the OR semantics across rules.


Test Coverage

Good coverage overall (NamespacePatternMatcherTest: 8 tests, NamespaceAccessValidatorTest: 16 tests, DefaultNamespaceAccessConfigTest: 6 tests, StreamingSessionActorTest: 2 new tests). Some gaps worth considering:

  • No tests for the error case when extractJwtFromHeaders encounters non-Bearer auth (relates to bug #1)
  • No tests for ThingSearchRoute.applyNamespaceAccessControl behavior
  • No tests for SSE/WebSocket event filtering with namespace access control enabled

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

Limit the namespaces which can be accessed via gateway (HTTP / Websocket) by matching a placeholder, e.g. JWT claim

2 participants