fix(dotauth): address code review findings across OAuth/OIDC layer

- Cache RemoteJWKSet instances by jwksUri to avoid per-validation JWKS fetches
- Bound ROLE_SYNC_LOCKS with Guava Cache (maxSize 10k) to prevent unbounded growth
- Swap auth check order in saveConfig so unauthenticated callers get 401 before 400
- Add requireAdmin gate to saveHeadlessConfig and clearHeadlessConfig
- Default to deny-all CORS when allowedOrigins is empty on the exchange endpoint
- Replace findAll with search API in listSites to skip archived/system hosts at query level
- Use sendRedirect instead of raw setHeader for provider logout redirect
- Add null guard on config in applyExtraRoles to prevent potential NPE

Author: swicken

core-web/libs/dotcms-models/src/lib/dot-auth.model.ts

@@ -59,6 +59,7 @@ export interface DotAuthConfigValues {
     extraRoles?: string;
     buildRolesStrategy?: string;
     callbackUrl?: string;
+    autoProvision?: boolean;
 }
 
 export type DotAuthSignatureValidation = 'none' | 'response' | 'assertion' | 'responseandassertion';

dotCMS/src/main/java/com/dotcms/auth/dotAuth/rest/DotAuthOAuthExchangeResource.java

@@ -215,23 +215,32 @@ public Response exchange(@Context final HttpServletRequest request,
             }
             final OAuthAppConfig config = cfgOpt.get();
 
-            // CORS origin check: reject requests from unlisted browser origins
+            // CORS origin check: reject requests from unlisted browser origins.
+            // When no allowedOrigins are configured, browser-originated requests are
+            // denied outright — the safe default for a token-exchange endpoint.
+            // Non-browser callers (no Origin header) pass through unaffected.
             final List<String> allowedOrigins = parseJsonStringList(config.allowedOriginsJson);
-            if (!allowedOrigins.isEmpty()) {
-                final String origin = request.getHeader("Origin");
-                if (UtilMethods.isSet(origin) && !allowedOrigins.contains(origin)) {
+            final String origin = request.getHeader("Origin");
+            if (UtilMethods.isSet(origin)) {
+                if (allowedOrigins.isEmpty()) {
+                    SecurityLogger.logInfo(DotAuthOAuthExchangeResource.class,
+                            "OAuth exchange rejected: no allowedOrigins configured; origin '"
+                                    + origin + "' from " + request.getRemoteAddr());
+                    return Response.status(Response.Status.FORBIDDEN)
+                            .entity(new ResponseEntityView<>(
+                                    "allowedOrigins must be configured for browser-based token exchange"))
+                            .build();
+                }
+                if (!allowedOrigins.contains(origin)) {
                     SecurityLogger.logInfo(DotAuthOAuthExchangeResource.class,
                             "OAuth exchange rejected: origin '" + origin
                                     + "' not in allowed origins from " + request.getRemoteAddr());
                     return Response.status(Response.Status.FORBIDDEN)
                             .entity(new ResponseEntityView<>("Origin not allowed"))
                             .build();
                 }
-                // Set CORS headers for allowed origins
-                if (UtilMethods.isSet(origin)) {
-                    response.setHeader("Access-Control-Allow-Origin", origin);
-                    response.setHeader("Access-Control-Allow-Credentials", "true");
-                }
+                response.setHeader("Access-Control-Allow-Origin", origin);
+                response.setHeader("Access-Control-Allow-Credentials", "true");
             }
 
             if (!config.isOidc()) {

dotCMS/src/main/java/com/dotcms/auth/dotAuth/rest/DotAuthResource.java

@@ -172,12 +172,12 @@ public final Response listSites(@Context final HttpServletRequest request,
             final boolean systemHeadless = systemKeys.contains(
                     DotAuthConstants.HEADLESS_APP_KEY.toLowerCase());
 
-            final List<Host> allHosts = APILocator.getHostAPI().findAll(user, false);
+            // Use the paginated search API instead of the deprecated findAll.
+            // An empty filter with limit <= 0 returns all live, non-system hosts.
+            final List<Host> allHosts = APILocator.getHostAPI()
+                    .search("", false, false, 0, 0, user, false);
             final List<DotAuthSitesView.SiteRowView> rows = new ArrayList<>();
             for (final Host host : allHosts) {
-                if (host == null || host.isSystemHost() || host.isArchived()) {
-                    continue;
-                }
                 final Set<String> hostKeys = appsByHost
                         .getOrDefault(host.getIdentifier().toLowerCase(), Set.of());
                 final Optional<DotAuthProtocol> hostProtocol = detectProtocol(hostKeys);
@@ -314,8 +314,8 @@ public final Response saveConfig(@Context final HttpServletRequest request,
             if (form == null) {
                 throw new IllegalArgumentException("Body required");
             }
-            form.checkValid();
             final User user = initUser(request, response);
+            form.checkValid();
             final Host host = resolveHost(hostId, user);
 
             // DotAuthConfigForm#constructor already defaults protocol to OAUTH when null,
@@ -427,6 +427,7 @@ public final Response saveHeadlessConfig(@Context final HttpServletRequest reque
                 throw new IllegalArgumentException("Body required");
             }
             final User user = initUser(request, response);
+            requireAdmin(user);
             final Host systemHost = APILocator.systemHost();
             appsAPI.saveSecrets(headlessHelper.buildSecrets(form), systemHost, user);
             SecurityLogger.logInfo(DotAuthResource.class,
@@ -456,6 +457,7 @@ public final Response clearHeadlessConfig(@Context final HttpServletRequest requ
                                               @Context final HttpServletResponse response) {
         try {
             final User user = initUser(request, response);
+            requireAdmin(user);
             final Host systemHost = APILocator.systemHost();
             appsAPI.deleteSecrets(DotAuthConstants.HEADLESS_APP_KEY, systemHost, user);
             SecurityLogger.logInfo(DotAuthResource.class,

dotCMS/src/main/java/com/dotcms/auth/providers/oauth/OAuthHelper.java

@@ -9,6 +9,8 @@
 import com.dotmarketing.exception.DotDataException;
 import com.dotmarketing.exception.DotRuntimeException;
 import com.dotmarketing.exception.DotSecurityException;
+import com.google.common.cache.Cache;
+import com.google.common.cache.CacheBuilder;
 import com.dotmarketing.util.Config;
 import com.dotmarketing.util.Logger;
 import com.dotmarketing.util.SecurityLogger;
@@ -26,7 +28,7 @@
 import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
-import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ExecutionException;
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 import javax.servlet.http.HttpSession;
@@ -54,7 +56,8 @@ public class OAuthHelper {
      * case is a redundant reapply of the same roles, which is idempotent. That
      * is an accepted cost versus the complexity of a distributed lock.
      */
-    private static final ConcurrentHashMap<String, Object> ROLE_SYNC_LOCKS = new ConcurrentHashMap<>();
+    private static final Cache<String, Object> ROLE_SYNC_LOCKS =
+            CacheBuilder.newBuilder().maximumSize(10_000).build();
 
     /**
      * Per-login role-sync strategy, mirroring {@code SAMLHelper}'s {@code build.roles}
@@ -319,7 +322,12 @@ private void applyBuildRolesStrategy(final User user,
         // same user never observe the "wiped but not yet reapplied" intermediate state.
         // Intrinsic-monitor lock on a per-user sentinel is enough — the block runs
         // DB-bound work for well under a second and we hold nothing else inside it.
-        final Object userLock = ROLE_SYNC_LOCKS.computeIfAbsent(user.getUserId(), id -> new Object());
+        final Object userLock;
+        try {
+            userLock = ROLE_SYNC_LOCKS.get(user.getUserId(), Object::new);
+        } catch (final ExecutionException e) {
+            throw new DotRuntimeException("Failed to acquire role sync lock", e);
+        }
         synchronized (userLock) {
             // Remove all existing roles before reapplying, unless the strategy is STATICADD
             // (additive / legacy behavior). Matches SAMLHelper.addRoles exactly.
@@ -427,7 +435,7 @@ private void addRoleIfPresent(final User user, final Role role) {
     }
 
     private void applyExtraRoles(final User user, final OAuthAppConfig config) {
-        if (config.extraRoles == null) {
+        if (config == null || config.extraRoles == null) {
             return;
         }
         for (final String roleKey : config.extraRoles) {

dotCMS/src/main/java/com/dotcms/auth/providers/oauth/OAuthWebInterceptor.java

@@ -324,7 +324,7 @@ private Result handleLogout(final HttpServletRequest request,
      */
     private Result doCoreLogout(final HttpServletRequest request,
                                 final HttpServletResponse response,
-                                final String providerLogoutUrl) {
+                                final String providerLogoutUrl) throws IOException {
         final User user = PortalUtil.getUser(request);
         Try.run(() -> APILocator.getLoginServiceAPI().doActionLogout(request, response))
                 .onFailure(e -> Logger.warn(this, "doActionLogout failed: " + e.getMessage()));
@@ -334,13 +334,7 @@ private Result doCoreLogout(final HttpServletRequest request,
                             + ") has logged out via OAuth from IP: " + request.getRemoteAddr());
         }
         if (!response.isCommitted()) {
-            if (UtilMethods.isSet(providerLogoutUrl)) {
-                response.setStatus(HttpServletResponse.SC_FOUND);
-                response.setHeader("Location", providerLogoutUrl);
-            } else {
-                response.setStatus(HttpServletResponse.SC_FOUND);
-                response.setHeader("Location", "/");
-            }
+            response.sendRedirect(UtilMethods.isSet(providerLogoutUrl) ? providerLogoutUrl : "/");
         }
         return Result.SKIP_NO_CHAIN;
     }

dotCMS/src/main/java/com/dotcms/auth/providers/oauth/provider/OIDCProvider.java

@@ -59,6 +59,8 @@ public class OIDCProvider implements OAuthProvider {
      */
     private static final ConcurrentHashMap<String, CachedDiscovery> DISCOVERY_CACHE = new ConcurrentHashMap<>();
 
+    private static final ConcurrentHashMap<String, RemoteJWKSet<SecurityContext>> JWKS_CACHE = new ConcurrentHashMap<>();
+
     /**
      * Safe {@code id_token} signing algorithms. Asymmetric only (RSA or EC) — symmetric
      * {@code HS*} algs rely on a shared secret, and {@code none} is an explicit attacker
@@ -362,13 +364,13 @@ public Map<String, Object> validateIdTokenAndExtractClaims(final String idToken,
         }
         try {
             final SignedJWT jwt = SignedJWT.parse(idToken);
-            final URL jwksUrl;
-            try {
-                jwksUrl = new URL(jwksUri);
-            } catch (final MalformedURLException e) {
-                throw new DotRuntimeException("OIDC jwks_uri is malformed: " + jwksUri, e);
-            }
-            final JWKSource<SecurityContext> keySource = new RemoteJWKSet<>(jwksUrl);
+            final JWKSource<SecurityContext> keySource = JWKS_CACHE.computeIfAbsent(jwksUri, uri -> {
+                try {
+                    return new RemoteJWKSet<>(new URL(uri));
+                } catch (final MalformedURLException e) {
+                    throw new DotRuntimeException("OIDC jwks_uri is malformed: " + uri, e);
+                }
+            });
             final JWSAlgorithm tokenAlg = jwt.getHeader().getAlgorithm() == null
                     ? null
                     : JWSAlgorithm.parse(jwt.getHeader().getAlgorithm().getName());