From cb3ba66931c36fb0eabe290345cd7232b9dee002 Mon Sep 17 00:00:00 2001 From: "hanyu.liang" Date: Mon, 13 Apr 2026 01:03:54 -0700 Subject: [PATCH] [identity]: fix tenant context loss and add userId filter 1. ExternalTenantResourceTracker: add TaskContext fallback in both receive-side (notifyResourceOwnershipCreated) and send-side (beforeSendMessage) interceptors to recover ExternalTenantContext when ThreadLocal is lost across thdf.chainSubmit thread-pool handoff. 2. ExternalTenantResourceTracker: store tenant context in TaskContext alongside ThreadLocal in beforeDeliveryMessage interceptor so FlowChain workers inherit it via HasThreadContextAspect. 3. ExternalTenantZQLExtension: add optional userId filter to the generated SQL subquery for user-level resource isolation within a tenant. Resolves: ZCF-1147 Change-Id: I4dc60e51251fbc97841edd999a53fb7495bd63d1 --- .../ExternalTenantResourceTracker.java | 54 +++++++++++++++++-- .../identity/ExternalTenantZQLExtension.java | 17 ++++-- 2 files changed, 63 insertions(+), 8 deletions(-) diff --git a/identity/src/main/java/org/zstack/identity/ExternalTenantResourceTracker.java b/identity/src/main/java/org/zstack/identity/ExternalTenantResourceTracker.java index cbb6b1bff69..b71bc97a017 100644 --- a/identity/src/main/java/org/zstack/identity/ExternalTenantResourceTracker.java +++ b/identity/src/main/java/org/zstack/identity/ExternalTenantResourceTracker.java @@ -16,6 +16,8 @@ import org.zstack.header.message.Message; import org.zstack.header.vo.ResourceVO; +import org.zstack.utils.TaskContext; + import javax.persistence.EntityManager; import java.util.*; @@ -46,6 +48,38 @@ public class ExternalTenantResourceTracker implements */ private static final String HEADER_EXTERNAL_TENANT = "external-tenant-context"; + /** + * TaskContext key used to propagate ExternalTenantContext across thread-pool + * boundaries (e.g. thdf.chainSubmit). ZStack's HasThreadContextAspect + * automatically snapshots TaskContext when a ChainTask/Completion is created, + * and SetThreadContextAspect restores it before execution in the worker thread. + * By storing the tenant context here in addition to the ThreadLocal, we ensure + * cascade resources (Volume, NIC, CdRom) created inside FlowChains that run + * in a different thread still see the correct tenant context. + */ + static final String TASK_CONTEXT_EXTERNAL_TENANT = "__externalTenantContext__"; + + /** + * Resolve the current ExternalTenantContext using ThreadLocal first, then + * falling back to TaskContext. This covers the case where ThreadLocal is + * lost after a thread-pool handoff (e.g. thdf.chainSubmit) but TaskContext + * was propagated by HasThreadContextAspect/SetThreadContextAspect. + * + * Note: this method only reads — it does NOT restore the ThreadLocal, because + * the caller's thread-local state should be managed by SetThreadContextAspect + * at the thread entry point. + */ + private static ExternalTenantContext resolveCurrentContext() { + ExternalTenantContext ctx = ExternalTenantContext.getCurrent(); + if (ctx == null || ctx.getTenantId() == null) { + Object taskCtx = TaskContext.getTaskContextItem(TASK_CONTEXT_EXTERNAL_TENANT); + if (taskCtx instanceof ExternalTenantContext) { + ctx = (ExternalTenantContext) taskCtx; + } + } + return ctx; + } + @Override public boolean start() { for (ExternalTenantProvider p : pluginRgty.getExtensionList(ExternalTenantProvider.class)) { @@ -65,7 +99,7 @@ public boolean start() { bus.installBeforeSendMessageInterceptor(new AbstractBeforeSendMessageInterceptor() { @Override public void beforeSendMessage(Message msg) { - ExternalTenantContext ctx = ExternalTenantContext.getCurrent(); + ExternalTenantContext ctx = resolveCurrentContext(); if (ctx != null && ctx.getTenantId() != null) { msg.putHeaderEntry(HEADER_EXTERNAL_TENANT, new String[]{ctx.getSource(), ctx.getTenantId(), ctx.getUserId()}); @@ -90,19 +124,28 @@ public void beforeDeliveryMessage(Message msg) { SessionInventory session = ((APIMessage) msg).getSession(); if (session != null && session.hasExternalTenant()) { ExternalTenantContext ctx = session.getExternalTenantContext(); - ExternalTenantContext.setCurrent(ctx); + if (ctx != null) { + ExternalTenantContext.setCurrent(ctx); + TaskContext.putTaskContextItem(TASK_CONTEXT_EXTERNAL_TENANT, ctx); + } else { + ExternalTenantContext.clearCurrent(); + TaskContext.removeTaskContextItem(TASK_CONTEXT_EXTERNAL_TENANT); + } } else { ExternalTenantContext.clearCurrent(); + TaskContext.removeTaskContextItem(TASK_CONTEXT_EXTERNAL_TENANT); } } else { // Non-APIMessage: restore from dedicated message header (message-scoped) String[] tenantData = msg.getHeaderEntry(HEADER_EXTERNAL_TENANT); if (tenantData != null && tenantData.length >= 2) { String userId = tenantData.length >= 3 ? tenantData[2] : null; - ExternalTenantContext.setCurrent( - new ExternalTenantContext(tenantData[0], tenantData[1], userId)); + ExternalTenantContext ctx = new ExternalTenantContext(tenantData[0], tenantData[1], userId); + ExternalTenantContext.setCurrent(ctx); + TaskContext.putTaskContextItem(TASK_CONTEXT_EXTERNAL_TENANT, ctx); } else { ExternalTenantContext.clearCurrent(); + TaskContext.removeTaskContextItem(TASK_CONTEXT_EXTERNAL_TENANT); } } } @@ -119,7 +162,8 @@ public boolean stop() { @Override public void notifyResourceOwnershipCreated(AccountResourceRefVO ref, EntityManager entityManager) { - ExternalTenantContext ctx = ExternalTenantContext.getCurrent(); + ExternalTenantContext ctx = resolveCurrentContext(); + if (ctx == null || ctx.getTenantId() == null) { return; } diff --git a/identity/src/main/java/org/zstack/identity/ExternalTenantZQLExtension.java b/identity/src/main/java/org/zstack/identity/ExternalTenantZQLExtension.java index f62098fc77e..220d9ae8b79 100644 --- a/identity/src/main/java/org/zstack/identity/ExternalTenantZQLExtension.java +++ b/identity/src/main/java/org/zstack/identity/ExternalTenantZQLExtension.java @@ -70,14 +70,25 @@ public String restrictByExpr(ZQLExtensionContext context, ASTNode.RestrictExpr e String primaryKey = EntityMetadata.getPrimaryKeyField(src.inventoryAnnotation.mappingVOClass()).getName(); String inventoryAlias = src.simpleInventoryName(); - // Generate subquery, filter associated resources by source + tenantId + // Generate subquery, filter associated resources by source + tenantId (+ userId if present) + // Add userId filter only when present and valid; invalid userId is + // silently ignored (falls back to tenant-level isolation) rather than + // throwing SkipThisRestrictExprException which would remove the entire + // tenant filter — a security escalation. + String userId = tenantCtx.getUserId(); + String userFilter = ""; + if (userId != null && !userId.isEmpty() && SAFE_TENANT_VALUE.matcher(userId).matches()) { + userFilter = String.format(" AND etref.userId = '%s'", escapeSql(userId)); + } + return String.format( "(%s.%s IN (SELECT etref.resourceUuid FROM ExternalTenantResourceRefVO etref" + - " WHERE etref.source = '%s' AND etref.tenantId = '%s'))", + " WHERE etref.source = '%s' AND etref.tenantId = '%s'%s))", inventoryAlias, primaryKey, escapeSql(tenantCtx.getSource()), - escapeSql(tenantCtx.getTenantId()) + escapeSql(tenantCtx.getTenantId()), + userFilter ); }