#2304 Add namespace access control at the gateway level

[hu-ahmed]

Resolves: #2304

[thjaeckle]

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

[thjaeckle]

Thanks @hu-ahmed for fixing the first round of review comments.

I added some more, mainly in regards to:

• We should not support "both" config variants (single namespace-access object OR array as content of namespace-access) .. also the Helm template does only support "single object"
  • Please only support the "array" / list option - to define multiple - and drop the "single entry" variant

And another thing I noticed (which was also missing in the issue description):
We also need to add a configuration if a single namespace-access conditions and allowed-namespaces and blocked-namespaces are defined for things or policies or for both.

So I would suggest to add:

resource-types = ["thing","policy"]

We already have that pattern also in entity-creation config.

[thjaeckle]

In Ditto codebase, we don't use @Nullable for return types - only for parameters.
Please change that to Optional<JsonWebToken>.
Previous to the last commit, but @Nullable and Optional were used.

[thjaeckle]

I would not support both - but only the "list of objects" way.
It is complex to support both, as we also would need to support both in the Helm values.

Please adjust that to only support the "list of objects".

[thjaeckle]

The Helm template only suggests how to use the "single object" config.
As also mentioned in another comment, please adjust the config so that only the "list of objects" is supported.

We don't need to support both config options.

[thjaeckle]

This must support array notation.
As mentioned in other comments already, we should not as well support the "single object" variant - but only the array variant.

[thjaeckle]

@hu-ahmed Could you also rebase from master, please? There were additional routes added in Policy endpoints