Refactor OAuth plugin domain scope handling to use a centralized method for enabling checks

Damans227 · Damans227 · commit 01fb9a83f603 · 2026-05-19T13:50:00.000-04:00
diff --git a/plugins/user-authenticators/oauth2/src/main/java/org/apache/cloudstack/oauth2/OAuth2AuthManager.java b/plugins/user-authenticators/oauth2/src/main/java/org/apache/cloudstack/oauth2/OAuth2AuthManager.java
@@ -18,6 +18,7 @@
 //
 package org.apache.cloudstack.oauth2;
 
+import com.cloud.domain.Domain;
 import com.cloud.utils.component.PluggableService;
 import org.apache.cloudstack.api.auth.PluggableAPIAuthenticator;
 import org.apache.cloudstack.auth.UserOAuth2Authenticator;
@@ -64,4 +65,19 @@ public interface OAuth2AuthManager extends PluggableAPIAuthenticator, PluggableS
     OauthProviderVO updateOauthProvider(UpdateOAuthProviderCmd cmd);
 
     Long resolveDomainId(Map<String, Object[]> params);
+
+    /**
+     * Resolves whether the OAuth plugin is enabled for the given domain scope.
+     * A null domain or the ROOT domain is treated as the global scope, since the
+     * ROOT domain has no domain-level override and inherits the global value;
+     * any other domain is checked strictly at its own domain scope (no inheritance).
+     * @param domainId domain id, or null for global
+     * @return true if OAuth is enabled for that scope
+     */
+    static boolean isPluginEnabledForDomain(Long domainId) {
+        if (domainId == null || domainId == Domain.ROOT_DOMAIN) {
+            return Boolean.TRUE.equals(OAuth2IsPluginEnabled.value());
+        }
+        return Boolean.TRUE.equals(OAuth2IsPluginEnabled.valueInScope(ConfigKey.Scope.Domain, domainId, true));
+    }
 }
diff --git a/plugins/user-authenticators/oauth2/src/main/java/org/apache/cloudstack/oauth2/OAuth2AuthManagerImpl.java b/plugins/user-authenticators/oauth2/src/main/java/org/apache/cloudstack/oauth2/OAuth2AuthManagerImpl.java
@@ -79,10 +79,7 @@ public boolean start() {
     }
 
     protected boolean isOAuthPluginEnabled(Long domainId) {
-        if (domainId == null) {
-            return Boolean.TRUE.equals(OAuth2IsPluginEnabled.value());
-        }
-        return Boolean.TRUE.equals(OAuth2IsPluginEnabled.valueInScope(ConfigKey.Scope.Domain, domainId, true));
+        return OAuth2AuthManager.isPluginEnabledForDomain(domainId);
     }
 
     @Override
diff --git a/plugins/user-authenticators/oauth2/src/main/java/org/apache/cloudstack/oauth2/OAuth2UserAuthenticator.java b/plugins/user-authenticators/oauth2/src/main/java/org/apache/cloudstack/oauth2/OAuth2UserAuthenticator.java
@@ -27,14 +27,11 @@
 import org.apache.cloudstack.api.ApiConstants;
 import org.apache.cloudstack.auth.UserAuthenticator;
 import org.apache.cloudstack.auth.UserOAuth2Authenticator;
-import org.apache.cloudstack.framework.config.ConfigKey;
 
 import javax.inject.Inject;
 import java.util.Map;
 import java.util.Objects;
 
-import static org.apache.cloudstack.oauth2.OAuth2AuthManager.OAuth2IsPluginEnabled;
-
 public class OAuth2UserAuthenticator extends AdapterBase implements UserAuthenticator {
 
     @Inject
@@ -92,9 +89,6 @@ public String encode(String password) {
     }
 
     protected boolean isOAuthPluginEnabled(Long domainId) {
-        if (domainId == null) {
-            return Boolean.TRUE.equals(OAuth2IsPluginEnabled.value());
-        }
-        return Boolean.TRUE.equals(OAuth2IsPluginEnabled.valueInScope(ConfigKey.Scope.Domain, domainId, true));
+        return OAuth2AuthManager.isPluginEnabledForDomain(domainId);
     }
 }
diff --git a/plugins/user-authenticators/oauth2/src/main/java/org/apache/cloudstack/oauth2/api/command/ListOAuthProvidersCmd.java b/plugins/user-authenticators/oauth2/src/main/java/org/apache/cloudstack/oauth2/api/command/ListOAuthProvidersCmd.java
@@ -38,7 +38,6 @@
 import org.apache.cloudstack.api.response.DomainResponse;
 import org.apache.cloudstack.api.response.ListResponse;
 import org.apache.cloudstack.auth.UserOAuth2Authenticator;
-import org.apache.cloudstack.framework.config.ConfigKey;
 import org.apache.cloudstack.oauth2.OAuth2AuthManager;
 import org.apache.cloudstack.oauth2.api.response.OauthProviderResponse;
 import org.apache.cloudstack.oauth2.vo.OauthProviderVO;
@@ -145,9 +144,7 @@ public String authenticate(String command, Map<String, Object[]> params, HttpSes
             Domain domain = result.getDomainId() != null ? ApiDBUtils.findDomainById(result.getDomainId()) : null;
             OauthProviderResponse r = new OauthProviderResponse(result.getUuid(), result.getProvider(),
                     result.getDescription(), result.getClientId(), result.getSecretKey(), result.getRedirectUri(), domain);
-            boolean oauthEnabled = result.getDomainId() == null
-                    ? Boolean.TRUE.equals(OAuth2AuthManager.OAuth2IsPluginEnabled.value())
-                    : Boolean.TRUE.equals(OAuth2AuthManager.OAuth2IsPluginEnabled.valueInScope(ConfigKey.Scope.Domain, result.getDomainId(), true));
+            boolean oauthEnabled = OAuth2AuthManager.isPluginEnabledForDomain(result.getDomainId());
             if (oauthEnabled && authenticatorPluginNames.contains(result.getProvider()) && result.isEnabled()) {
                 r.setEnabled(true);
             } else {
@@ -162,7 +159,7 @@ public String authenticate(String command, Map<String, Object[]> params, HttpSes
             List<OauthProviderVO> allProviders = _oauth2mgr.listOauthProviders(null, null, null);
             for (OauthProviderVO domainProvider : allProviders) {
                 if (domainProvider.getDomainId() != null && domainProvider.isEnabled()
-                        && Boolean.TRUE.equals(OAuth2AuthManager.OAuth2IsPluginEnabled.valueInScope(ConfigKey.Scope.Domain, domainProvider.getDomainId(), true))
+                        && OAuth2AuthManager.isPluginEnabledForDomain(domainProvider.getDomainId())
                         && authenticatorPluginNames.contains(domainProvider.getProvider())) {
                     totalEnabledCount++;
                 }
diff --git a/plugins/user-authenticators/oauth2/src/main/java/org/apache/cloudstack/oauth2/api/command/OauthLoginAPIAuthenticatorCmd.java b/plugins/user-authenticators/oauth2/src/main/java/org/apache/cloudstack/oauth2/api/command/OauthLoginAPIAuthenticatorCmd.java
@@ -50,9 +50,7 @@
 import java.util.Map;
 import java.net.InetAddress;
 
-import org.apache.cloudstack.framework.config.ConfigKey;
-
-import static org.apache.cloudstack.oauth2.OAuth2AuthManager.OAuth2IsPluginEnabled;
+import org.apache.cloudstack.oauth2.OAuth2AuthManager;
 
 @APICommand(name = "oauthlogin", description = "Logs a user into the CloudStack after successful verification of OAuth secret code from the particular provider." +
         "A successful login attempt will generate a JSESSIONID cookie value that can be passed in subsequent Query command calls until the \"logout\" command has been issued or the session has expired.",
@@ -146,9 +144,7 @@ public String authenticate(String command, Map<String, Object[]> params, HttpSes
                 domainId = userDomain.getId();
             }
 
-            boolean oauthEnabled = domainId == null
-                    ? Boolean.TRUE.equals(OAuth2IsPluginEnabled.value())
-                    : Boolean.TRUE.equals(OAuth2IsPluginEnabled.valueInScope(ConfigKey.Scope.Domain, domainId, true));
+            boolean oauthEnabled = OAuth2AuthManager.isPluginEnabledForDomain(domainId);
             if (!oauthEnabled) {
                 logger.debug(String.format("OAuth is not enabled %s, users cannot login using OAuth", domainId == null ? "globally" : "in domain " + domainId));
                 throw new CloudAuthenticationException(String.format(
diff --git a/plugins/user-authenticators/oauth2/src/main/java/org/apache/cloudstack/oauth2/api/command/UpdateOAuthProviderCmd.java b/plugins/user-authenticators/oauth2/src/main/java/org/apache/cloudstack/oauth2/api/command/UpdateOAuthProviderCmd.java
@@ -20,7 +20,6 @@
 import com.cloud.domain.Domain;
 import org.apache.cloudstack.api.ApiCommandResourceType;
 import org.apache.cloudstack.auth.UserOAuth2Authenticator;
-import org.apache.cloudstack.framework.config.ConfigKey;
 import org.apache.cloudstack.oauth2.OAuth2AuthManager;
 import org.apache.cloudstack.oauth2.api.response.OauthProviderResponse;
 import org.apache.cloudstack.oauth2.vo.OauthProviderVO;
@@ -144,9 +143,7 @@ public void execute() {
                 String name = authenticator.getName();
                 authenticatorPluginNames.add(name);
             }
-            boolean oauthEnabled = result.getDomainId() == null
-                    ? Boolean.TRUE.equals(OAuth2AuthManager.OAuth2IsPluginEnabled.value())
-                    : Boolean.TRUE.equals(OAuth2AuthManager.OAuth2IsPluginEnabled.valueInScope(ConfigKey.Scope.Domain, result.getDomainId(), true));
+            boolean oauthEnabled = OAuth2AuthManager.isPluginEnabledForDomain(result.getDomainId());
             if (oauthEnabled && authenticatorPluginNames.contains(result.getProvider()) && result.isEnabled()) {
                 r.setEnabled(true);
             } else {

Original file line number	Diff line number	Diff line change
`@@ -79,10 +79,7 @@ public boolean start() {`
`79`	`79`	`}`
`80`	`80`
`81`	`81`	`protected boolean isOAuthPluginEnabled(Long domainId) {`
`82`		`- if (domainId == null) {`
`83`		`- return Boolean.TRUE.equals(OAuth2IsPluginEnabled.value());`
`84`		`- }`
`85`		`- return Boolean.TRUE.equals(OAuth2IsPluginEnabled.valueInScope(ConfigKey.Scope.Domain, domainId, true));`
	`82`	`+ return OAuth2AuthManager.isPluginEnabledForDomain(domainId);`
`86`	`83`	`}`
`87`	`84`
`88`	`85`	`@Override`