From 1d0787acbff00a1de4218933b55bb6a452ba6918 Mon Sep 17 00:00:00 2001 From: Freddy Montes Date: Tue, 12 May 2026 15:29:27 -0600 Subject: [PATCH] Fix OSGi resolution, YAML load, and stale per-host cache on recent dotCMS - Drop imports for non-exported packages (com.dotcms.api.system.event.message, common.Assert, javax.annotation.Nonnull) - Loosen Import-Package version ranges and disable uses: directive in bundle plugin - Load Dotuserproxy.yml via Bundle.getEntry instead of getResourceAsStream - Remove per-host lazyUserProxyMap; rely on dotCMS AppsCache to avoid stale config after save Co-Authored-By: Claude Opus 4.7 (1M context) --- pom.xml | 3 +- .../interceptor/UserProxyInterceptor.java | 49 +++++++++++++------ .../listener/UserProxyAppListener.java | 27 ++-------- .../com/dotcms/userproxy/osgi/Activator.java | 2 +- .../dotcms/userproxy/osgi/FileMoverUtil.java | 27 +++++----- 5 files changed, 57 insertions(+), 51 deletions(-) diff --git a/pom.xml b/pom.xml index 9133293..3dcce24 100644 --- a/pom.xml +++ b/pom.xml @@ -72,7 +72,8 @@ https://www.dotcms.com/ com.dotcms.userproxy.osgi.Activator .,{maven-dependencies} - * + *;version="0.0.0" + <_nouses>true com.dotcms.userproxy *;scope=compile|runtime lib diff --git a/src/main/java/com/dotcms/userproxy/interceptor/UserProxyInterceptor.java b/src/main/java/com/dotcms/userproxy/interceptor/UserProxyInterceptor.java index 5cb4b87..7ad7521 100644 --- a/src/main/java/com/dotcms/userproxy/interceptor/UserProxyInterceptor.java +++ b/src/main/java/com/dotcms/userproxy/interceptor/UserProxyInterceptor.java @@ -1,12 +1,8 @@ package com.dotcms.userproxy.interceptor; import java.util.Arrays; -import java.util.Collections; import java.util.List; -import java.util.Map; import java.util.Optional; -import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.atomic.AtomicReference; import java.util.regex.Pattern; import javax.servlet.http.HttpServletRequest; @@ -21,11 +17,11 @@ import com.dotmarketing.beans.Host; import com.dotmarketing.business.APILocator; import com.dotmarketing.business.web.WebAPILocator; +import com.dotmarketing.util.Logger; import com.liferay.portal.model.User; import com.liferay.portal.util.PortalUtil; import com.liferay.portal.util.WebKeys; -import io.vavr.Lazy; import io.vavr.control.Try; /** @@ -35,14 +31,7 @@ */ public class UserProxyInterceptor implements WebInterceptor { - private static final ConcurrentHashMap> lazyUserProxyMap = new ConcurrentHashMap<>(); - public UserProxyInterceptor() { - resetLazyUserProxyMap(); - } - - public static void resetLazyUserProxyMap() { - lazyUserProxyMap.clear(); } @Override @@ -54,32 +43,64 @@ public String[] getFilters() { public Result intercept(final HttpServletRequest request, final HttpServletResponse response) { if (hasExistingAuth(request)) { + Logger.debug(this, () -> "UserProxy skip: request already has auth. method=" + + request.getMethod() + " uri=" + request.getRequestURI()); return Result.NEXT; } Host host = WebAPILocator.getHostWebAPI().getCurrentHostNoThrow(request); + if (host == null) { + Logger.warn(this, "UserProxy could not resolve current host. method=" + + request.getMethod() + " uri=" + request.getRequestURI()); + return Result.NEXT; + } - List entries = lazyUserProxyMap.computeIfAbsent(host.getIdentifier(), - h -> UserProxyEntryMapper.buildListForHost(h)); + List entries = UserProxyEntryMapper.buildListForHost(host.getIdentifier()); + + Logger.info(this, "UserProxy intercept: method=" + request.getMethod() + + " uri=" + request.getRequestURI() + + " remoteAddr=" + request.getRemoteAddr() + + " hostId=" + host.getIdentifier() + + " entries=" + entries.size()); if (entries.isEmpty()) { + Logger.warn(this, "UserProxy found no entries for hostId=" + host.getIdentifier() + + " uri=" + request.getRequestURI()); return Result.NEXT; } // break on first match for (UserProxyEntry entry : entries) { if (entry.matches(request)) { + Logger.info(this, "UserProxy matched entry for method=" + request.getMethod() + + " uri=" + request.getRequestURI() + + " allowedMethods=" + entry.getMethods() + + " patterns=" + Arrays.toString(entry.getUrls())); + final Optional token = APILocator.getApiTokenAPI() .fromJwt(new String(entry.getUserToken()), request.getRemoteAddr()); + Logger.info(this, "UserProxy token lookup result for uri=" + + request.getRequestURI() + ": tokenPresent=" + token.isPresent()); + User user = Try.of(() -> token.get().getActiveUser().get()).getOrNull(); if (user != null) { + Logger.info(this, "UserProxy authenticated userId=" + user.getUserId() + + " for uri=" + request.getRequestURI()); request.setAttribute(WebKeys.USER, user); request.setAttribute(WebKeys.USER_ID, user.getUserId()); break; } + + Logger.warn(this, "UserProxy match did not resolve an active user for uri=" + + request.getRequestURI() + " remoteAddr=" + request.getRemoteAddr()); + } else { + Logger.debug(this, () -> "UserProxy entry did not match. method=" + + request.getMethod() + " uri=" + request.getRequestURI() + + " allowedMethods=" + entry.getMethods() + + " patterns=" + Arrays.toString(entry.getUrls())); } } diff --git a/src/main/java/com/dotcms/userproxy/listener/UserProxyAppListener.java b/src/main/java/com/dotcms/userproxy/listener/UserProxyAppListener.java index 5fd977b..e41a0d2 100644 --- a/src/main/java/com/dotcms/userproxy/listener/UserProxyAppListener.java +++ b/src/main/java/com/dotcms/userproxy/listener/UserProxyAppListener.java @@ -1,13 +1,9 @@ package com.dotcms.userproxy.listener; -import com.dotcms.api.system.event.message.MessageSeverity; -import com.dotcms.api.system.event.message.SystemMessageEventUtil; -import com.dotcms.api.system.event.message.builder.SystemMessageBuilder; import com.dotcms.security.apps.AppSecretSavedEvent; import com.dotcms.system.event.local.model.EventSubscriber; import com.dotcms.system.event.local.model.KeyFilterable; -import com.dotcms.userproxy.interceptor.UserProxyInterceptor; import com.dotcms.userproxy.model.UserProxyEntry; import com.dotcms.userproxy.model.UserProxyEntryMapper; import com.dotcms.userproxy.util.AppKey; @@ -44,8 +40,7 @@ public void notify(final AppSecretSavedEvent event) { Logger.info(this, "Missing event, aborting"); return; } - Logger.info(this, "UserProxyAppListener updated, clearing UserProxy map"); - UserProxyInterceptor.resetLazyUserProxyMap(); + Logger.info(this, "UserProxyAppListener received AppSecretSavedEvent"); String jsonConfig = event.getAppSecrets().getSecrets().get(AppKey.APP_CONFIG_KEY.appValue).getString(); @@ -61,25 +56,9 @@ public void notify(final AppSecretSavedEvent event) { return; } List entries = UserProxyEntryMapper.parseJsonToEntries(jsonConfig); - if (!entries.isEmpty()) { - return; + if (entries.isEmpty()) { + Logger.warn(this, "No valid User Proxy configuration found for users: " + users); } - final SystemMessageBuilder systemMessageBuilder = new SystemMessageBuilder(); - String velocityMessage = "No valid User Proxy configuration found."; - - MessageSeverity severity = (false) - ? MessageSeverity.INFO - : MessageSeverity.ERROR; - - systemMessageBuilder.setMessage(velocityMessage) - .setLife(7 * 1000) - .setSeverity(severity).create(); - - SystemMessageEventUtil.getInstance().pushMessage(systemMessageBuilder.create(), users); - - - - diff --git a/src/main/java/com/dotcms/userproxy/osgi/Activator.java b/src/main/java/com/dotcms/userproxy/osgi/Activator.java index 9d01862..ee37d1e 100644 --- a/src/main/java/com/dotcms/userproxy/osgi/Activator.java +++ b/src/main/java/com/dotcms/userproxy/osgi/Activator.java @@ -34,7 +34,7 @@ public void start(final org.osgi.framework.BundleContext context) throws IOExcep // Adding APP yaml Logger.info(Activator.class.getName(), "Copying UserProxy APP"); - new FileMoverUtil().copyAppYml(); + new FileMoverUtil().copyAppYml(context); // set up app listener diff --git a/src/main/java/com/dotcms/userproxy/osgi/FileMoverUtil.java b/src/main/java/com/dotcms/userproxy/osgi/FileMoverUtil.java index 982a729..75ce31d 100644 --- a/src/main/java/com/dotcms/userproxy/osgi/FileMoverUtil.java +++ b/src/main/java/com/dotcms/userproxy/osgi/FileMoverUtil.java @@ -12,19 +12,19 @@ import com.dotmarketing.util.ConfigUtils; import com.dotmarketing.util.Logger; import com.dotmarketing.util.UtilMethods; -import common.Assert; import io.vavr.control.Try; import java.io.File; import java.io.IOException; import java.io.InputStream; +import java.net.URL; import java.nio.file.Files; import java.util.ArrayList; import java.util.List; import java.util.Map; import java.util.jar.JarEntry; import java.util.stream.Collectors; -import javax.annotation.Nonnull; import org.apache.commons.io.IOUtils; +import org.osgi.framework.BundleContext; public class FileMoverUtil { @@ -40,7 +40,7 @@ public class FileMoverUtil { }; - public static List listFilesInPackage(@Nonnull String packagePathStr) { + public static List listFilesInPackage(String packagePathStr) { String packagePath = stripPackagePath(packagePathStr); List jarEntries = new ArrayList<>(); @@ -66,7 +66,7 @@ public static List listFilesInPackage(@Nonnull String packagePathStr) return jarEntries; } - public static Map loadGQLQueryMap(@Nonnull String packagePathStr) { + public static Map loadGQLQueryMap(String packagePathStr) { return Map.of(); } @@ -78,7 +78,7 @@ public static Map loadGQLQueryMap(@Nonnull String packagePathStr * * @param packagePathInJar - the directory path in the jar to copy */ - void copyFromJar(@Nonnull String packagePathInJar) { + void copyFromJar(String packagePathInJar) { this.copyFromJar(packagePathInJar, Try.of(() -> APILocator.getHostAPI().findDefaultHost(APILocator.systemUser(), false)) .getOrElseThrow(DotRuntimeException::new)); @@ -102,7 +102,7 @@ static String stripPackagePath(String packagePathInJar) { * @param packagePathInJar - the directory path in the jar to copy * @param site - the site to copy to */ - void copyFromJar(@Nonnull String packagePathInJar, @Nonnull Host site) { + void copyFromJar(String packagePathInJar, Host site) { String strippedPackagePath = stripPackagePath(packagePathInJar); @@ -173,8 +173,9 @@ void copyFromJar(@Nonnull String packagePathInJar, @Nonnull Host site) { dotfile.setProperty(Contentlet.DISABLE_WORKFLOW, true); APILocator.getContentletAPI().publish(dotfile, APILocator.systemUser(), false); - Assert.verify(dotfile != null && dotfile.getIdentifier() != null, - "Unable to create file asset: " + fileName); + if (dotfile == null || dotfile.getIdentifier() == null) { + throw new DotRuntimeException("Unable to create file asset: " + fileName); + } tmpFile.delete(); tmpDir.delete(); @@ -191,11 +192,15 @@ void copyFromJar(@Nonnull String packagePathInJar, @Nonnull Host site) { * * @throws IOException */ - public void copyAppYml() throws IOException { + public void copyAppYml(BundleContext context) throws IOException { Logger.info(this.getClass().getName(), "copying YAML File:" + installedAppYaml); - try (final InputStream in = this.getClass() - .getResourceAsStream("/" + AppKey.USER_PROXY_APP_VALUE.appValue + ".yml")) { + final String entryName = AppKey.USER_PROXY_APP_VALUE.appValue + ".yml"; + final URL entry = context.getBundle().getEntry("/" + entryName); + if (entry == null) { + throw new IOException("Bundle entry not found: /" + entryName); + } + try (final InputStream in = entry.openStream()) { IOUtils.copy(in, Files.newOutputStream(installedAppYaml.toPath())); } CacheLocator.getAppsCache().clearCache();