Skip to content

Commit 33c3967

Browse files
Damans227Daman Arora
Damans227
and
Daman Arora
authored
Add per-domain OAuth (Google, GitHub) provider support (#12702)
* Add domain_id to oauth_provider table and VO * Add domain-aware methods to OauthProviderDao * Add domainId parameter to OAuth provider API commands and response * Add domain support to OAuth2AuthManager * Add domain-aware OAuth verification * Add domain support to ListOAuthProvidersCmd and update related tests * fix domain path issue * Add domainId support to OAuth provider * Return domain name and UUID in OAuth provider API responses using ApiDBUtils * Refactor domain ID resolution in VerifyOAuthCodeAndGetUserCmd to improve code clarity * Enhance OAuth2 plugin support for domain-level configuration and authentication checks * Update OAuth2 tests and VerifyOAuthCodeAndGetUserCmdTest * Add method to find OAuth provider by domain with global fallback * Update OAuth provider configuration to use 'domain' instead of 'domainid' in columns and details * Refactor OAuth provider methods to support domain-level queries and enhance user verification * Add caching for access token retrieval in GithubOAuth2Provider * Refactor access token checks in GithubOAuth2Provider to use StringUtils for improved readability and consistency * Refactor null checks to use utility for improved readability and consistency * Update OAuth2UserAuthenticatorTest to include domainId in user verification method * Remove unnecessary blank line and unused imports in OAuth provider command classes * Refactor and cleanup * Remove unnecessary blank lines * Enhance RegisterOAuthProviderCmdTest with additional provider mock data * Remove startup gate from OAuth plugin initialization to support dynamic config toggling * Add strictScope to ConfigKey to disable global fallback for domain-scoped oauth2.enabled * Add domain-scoped provider filtering to listOauthProvider and centralize domain resolution in OAuth2AuthManager * Add External OAuth tab with domain-scoped provider selection to login page * code cleanup * test fix * Handle login page provider visibility * UI cleanup * UI Cleanup * Keep text color consistent * add unit tests * Add Multiple-domain OAuth tests * Refactor as per PR comments * Use idempotent DDL helpers for oauth_provider schema migration * Use global config check for global providers and extract oauthEnabled variable * Make strictScope return null when id is null * Rename verification methods to use 'verifySecretCodeAndFetchEmail' for consistency * Refactor domain handling in OAuth2AuthManagerImpl to use DomainService instead of DomainDao * Enhance domain ID descriptions in OAuth command classes for clarity * Add domain path handling to OAuth provider commands and improve descriptions * Update domain path description in VerifyOAuthCodeAndGetUserCmd to clarify behavior with Domain ID * Replace remove method with expunge in deleteOauthProvider and add corresponding unit test * Add external login label to Login.vue and update i18n locale handling * Fix stale value issue in updateConfiguration response handling in ConfigurationValue.vue * Enhance OAuth login error handling and add unit test for missing parameters * Add validation to reject enabling OAuth provider when plugin is disabled at domain scope * Add domain reassignment support to UpdateOAuthProviderCmd and enhance validation in OAuth2AuthManagerImpl * Add domain ID to OAuth provider arguments in config * Fix condition for OAuth verification URL handling in router * Add domain path to OauthProviderResponse and update UI config to display it * Update config to remove 'secretkey' from columns and details * Add secretkey to details in config and display in DetailsTab * Implement normalization of ROOT domain to null for global OAuth provider handling and add corresponding unit tests * Refactor OAuth plugin domain scope handling to use a centralized method for enabling checks * Add strict scope handling to ConfigKey and update OAuth2AuthManager usage * Implement domain removal listener to clean up OAuth providers on domain deletion * Enhance OAuth tab icons with disabled state styling for better UX * Add domain-specific provider prompt and update OAuth provider handling --------- Co-authored-by: Daman Arora <daman.arora@shapeblue.com>
1 parent 1400616 commit 33c3967

35 files changed

Lines changed: 1432 additions & 250 deletions

‎api/src/main/java/org/apache/cloudstack/auth/UserOAuth2Authenticator.java‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,8 +42,19 @@ public interface UserOAuth2Authenticator extends Adapter {
4242
* Verifies the code provided by provider and fetches email
4343
* @return returns email
4444
*/
45-
String verifyCodeAndFetchEmail(String secretCode);
45+
String verifySecretCodeAndFetchEmail(String secretCode);
4646

47+
/**
48+
* Verifies if the logged in user is valid for a specific domain
49+
* @return true if it's a valid user, otherwise false
50+
*/
51+
boolean verifyUser(String email, String secretCode, Long domainId);
52+
53+
/**
54+
* Verifies the secret code provided by provider and fetches email for a specific domain
55+
* @return email for the specified domain
56+
*/
57+
String verifySecretCodeAndFetchEmail(String secretCode, Long domainId);
4758

4859
/**
4960
* Fetches email using the accessToken

‎engine/schema/src/main/resources/META-INF/db/schema-42210to42300.sql‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,12 @@
1919
-- Schema upgrade from 4.22.1.0 to 4.23.0.0
2020
--;
2121

22+
CALL `cloud`.`IDEMPOTENT_ADD_COLUMN`('cloud.oauth_provider', 'domain_id', 'bigint unsigned DEFAULT NULL COMMENT "NULL for global provider, domain ID for domain-specific" AFTER `redirect_uri`');
23+
CALL `cloud`.`IDEMPOTENT_ADD_FOREIGN_KEY`('cloud.oauth_provider', 'fk_oauth_provider__domain_id', '(`domain_id`)', '`domain`(`id`)');
24+
CALL `cloud`.`IDEMPOTENT_ADD_KEY`('i_oauth_provider__domain_id', 'cloud.oauth_provider', '(`domain_id`)');
25+
26+
CALL `cloud`.`IDEMPOTENT_ADD_UNIQUE_KEY`('cloud.oauth_provider', 'uk_oauth_provider__provider_domain', '(`provider`, `domain_id`)');
27+
2228
CREATE TABLE `cloud`.`backup_offering_details` (
2329
`id` bigint unsigned NOT NULL auto_increment,
2430
`backup_offering_id` bigint unsigned NOT NULL COMMENT 'Backup offering id',

‎framework/config/src/main/java/org/apache/cloudstack/framework/config/ConfigKey.java‎

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -269,6 +269,17 @@ public String toString() {
269269

270270
private String _defaultValueIfEmpty = null;
271271

272+
private boolean _strictScope = false;
273+
274+
public boolean isStrictScope() {
275+
return _strictScope;
276+
}
277+
278+
public ConfigKey<T> withStrictScope() {
279+
this._strictScope = true;
280+
return this;
281+
}
282+
272283
public static void init(ConfigDepotImpl depot) {
273284
s_depot = depot;
274285
}
@@ -429,11 +440,18 @@ public T valueInDomain(Long domainId) {
429440
}
430441

431442
public T valueInScope(Scope scope, Long id) {
443+
return valueInScope(scope, id, false);
444+
}
445+
446+
public T valueInScope(Scope scope, Long id, boolean strictScope) {
432447
if (id == null) {
433-
return value();
448+
return strictScope ? null : value();
434449
}
435450
String value = s_depot != null ? s_depot.getConfigStringValue(_name, scope, id) : null;
436451
if (value == null) {
452+
if (strictScope) {
453+
return null;
454+
}
437455
return valueInGlobalOrAvailableParentScope(scope, id);
438456
}
439457
logger.trace("Scope({}) value for config ({}): {}", scope, _name, _value);

‎plugins/user-authenticators/oauth2/src/main/java/org/apache/cloudstack/oauth2/OAuth2AuthManager.java‎

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
//
1919
package org.apache.cloudstack.oauth2;
2020

21+
import com.cloud.domain.Domain;
2122
import com.cloud.utils.component.PluggableService;
2223
import org.apache.cloudstack.api.auth.PluggableAPIAuthenticator;
2324
import org.apache.cloudstack.auth.UserOAuth2Authenticator;
@@ -27,10 +28,15 @@
2728
import org.apache.cloudstack.oauth2.vo.OauthProviderVO;
2829

2930
import java.util.List;
31+
import java.util.Map;
3032

3133
public interface OAuth2AuthManager extends PluggableAPIAuthenticator, PluggableService {
34+
String GLOBAL_DOMAIN_FILTER = "-1";
35+
Long GLOBAL_DOMAIN_ID = -1L;
36+
3237
public static ConfigKey<Boolean> OAuth2IsPluginEnabled = new ConfigKey<Boolean>("Advanced", Boolean.class, "oauth2.enabled", "false",
33-
"Indicates whether OAuth plugin is enabled or not", false);
38+
"Indicates whether OAuth plugin is enabled or not. This can be configured at domain level.", true, ConfigKey.Scope.Domain)
39+
.withStrictScope();
3440
public static final ConfigKey<String> OAuth2Plugins = new ConfigKey<String>("Advanced", String.class, "oauth2.plugins", "google,github",
3541
"List of OAuth plugins", true);
3642
public static final ConfigKey<String> OAuth2PluginsExclude = new ConfigKey<String>("Advanced", String.class, "oauth2.plugins.exclude", "",
@@ -49,13 +55,30 @@ public interface OAuth2AuthManager extends PluggableAPIAuthenticator, PluggableS
4955
*/
5056
UserOAuth2Authenticator getUserOAuth2AuthenticationProvider(final String providerName);
5157

52-
String verifyCodeAndFetchEmail(String code, String provider);
58+
String verifySecretCodeAndFetchEmail(String code, String provider, Long domainId);
5359

5460
OauthProviderVO registerOauthProvider(RegisterOAuthProviderCmd cmd);
5561

56-
List<OauthProviderVO> listOauthProviders(String provider, String uuid);
62+
List<OauthProviderVO> listOauthProviders(String provider, String uuid, Long domainId);
5763

5864
boolean deleteOauthProvider(Long id);
5965

6066
OauthProviderVO updateOauthProvider(UpdateOAuthProviderCmd cmd);
67+
68+
Long resolveDomainId(Map<String, Object[]> params);
69+
70+
/**
71+
* Resolves whether the OAuth plugin is enabled for the given domain scope.
72+
* A null domain or the ROOT domain is treated as the global scope, since the
73+
* ROOT domain has no domain-level override and inherits the global value;
74+
* any other domain is checked strictly at its own domain scope (no inheritance).
75+
* @param domainId domain id, or null for global
76+
* @return true if OAuth is enabled for that scope
77+
*/
78+
static boolean isPluginEnabledForDomain(Long domainId) {
79+
if (domainId == null || domainId == Domain.ROOT_DOMAIN) {
80+
return Boolean.TRUE.equals(OAuth2IsPluginEnabled.value());
81+
}
82+
return Boolean.TRUE.equals(OAuth2IsPluginEnabled.valueInScope(ConfigKey.Scope.Domain, domainId, true));
83+
}
6184
}

‎plugins/user-authenticators/oauth2/src/main/java/org/apache/cloudstack/oauth2/OAuth2AuthManagerImpl.java‎

Lines changed: 143 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,11 @@
2323
import java.util.HashMap;
2424
import java.util.List;
2525
import java.util.Map;
26+
import java.util.Objects;
2627

2728
import javax.inject.Inject;
2829

30+
import org.apache.cloudstack.api.ApiConstants;
2931
import org.apache.cloudstack.auth.UserOAuth2Authenticator;
3032
import org.apache.cloudstack.framework.config.ConfigKey;
3133
import org.apache.cloudstack.framework.config.Configurable;
@@ -39,15 +41,28 @@
3941
import org.apache.cloudstack.oauth2.vo.OauthProviderVO;
4042
import org.apache.commons.lang3.StringUtils;
4143

44+
import org.apache.commons.lang3.ArrayUtils;
45+
46+
import com.cloud.domain.Domain;
47+
import com.cloud.domain.DomainVO;
48+
import com.cloud.user.DomainManager;
49+
import com.cloud.user.DomainService;
4250
import com.cloud.utils.component.Manager;
4351
import com.cloud.utils.component.ManagerBase;
4452
import com.cloud.utils.exception.CloudRuntimeException;
53+
import org.apache.cloudstack.framework.messagebus.MessageBus;
4554

4655
public class OAuth2AuthManagerImpl extends ManagerBase implements OAuth2AuthManager, Manager, Configurable {
4756

4857
@Inject
4958
protected OauthProviderDao _oauthProviderDao;
5059

60+
@Inject
61+
private DomainService _domainService;
62+
63+
@Inject
64+
private MessageBus _messageBus;
65+
5166
protected static Map<String, UserOAuth2Authenticator> userOAuth2AuthenticationProvidersMap = new HashMap<>();
5267

5368
private List<UserOAuth2Authenticator> userOAuth2AuthenticationProviders;
@@ -63,17 +78,29 @@ public List<Class<?>> getAuthCommands() {
6378

6479
@Override
6580
public boolean start() {
66-
if (isOAuthPluginEnabled()) {
67-
logger.info("OAUTH plugin loaded");
68-
initializeUserOAuth2AuthenticationProvidersMap();
69-
} else {
70-
logger.info("OAUTH plugin not enabled so not loading");
71-
}
81+
initializeUserOAuth2AuthenticationProvidersMap();
82+
addDomainRemovalListener();
83+
logger.info("OAUTH plugin loaded");
7284
return true;
7385
}
7486

75-
protected boolean isOAuthPluginEnabled() {
76-
return OAuth2IsPluginEnabled.value();
87+
private void addDomainRemovalListener() {
88+
_messageBus.subscribe(DomainManager.MESSAGE_PRE_REMOVE_DOMAIN_EVENT, (senderAddress, subject, args) -> {
89+
try {
90+
long domainId = ((DomainVO) args).getId();
91+
List<OauthProviderVO> providers = _oauthProviderDao.listByDomain(domainId);
92+
for (OauthProviderVO provider : providers) {
93+
_oauthProviderDao.expunge(provider.getId());
94+
logger.debug("Removed OAuth provider {} for deleted domain {}", provider.getProvider(), domainId);
95+
}
96+
} catch (Exception e) {
97+
logger.error("Failed to remove OAuth providers for deleted domain", e);
98+
}
99+
});
100+
}
101+
102+
protected boolean isOAuthPluginEnabled(Long domainId) {
103+
return OAuth2AuthManager.isPluginEnabledForDomain(domainId);
77104
}
78105

79106
@Override
@@ -124,9 +151,11 @@ protected void initializeUserOAuth2AuthenticationProvidersMap() {
124151
}
125152

126153
@Override
127-
public String verifyCodeAndFetchEmail(String code, String provider) {
154+
public String verifySecretCodeAndFetchEmail(String code, String provider, Long domainId) {
128155
UserOAuth2Authenticator authenticator = getUserOAuth2AuthenticationProvider(provider);
129-
return authenticator.verifyCodeAndFetchEmail(code);
156+
String email = authenticator.verifySecretCodeAndFetchEmail(code, domainId);
157+
158+
return email;
130159
}
131160

132161
@Override
@@ -136,27 +165,38 @@ public OauthProviderVO registerOauthProvider(RegisterOAuthProviderCmd cmd) {
136165
String clientId = StringUtils.trim(cmd.getClientId());
137166
String redirectUri = StringUtils.trim(cmd.getRedirectUri());
138167
String secretKey = StringUtils.trim(cmd.getSecretKey());
168+
Long domainId = normalizeGlobalScope(resolveDomainIdFromIdOrPath(cmd.getDomainId(), cmd.getDomainPath()));
139169
String authorizeUrl = StringUtils.trim(cmd.getAuthorizeUrl());
140170
String tokenUrl = StringUtils.trim(cmd.getTokenUrl());
141171

142-
if (!isOAuthPluginEnabled()) {
172+
if (!isOAuthPluginEnabled(domainId)) {
143173
throw new CloudRuntimeException("OAuth is not enabled, please enable to register");
144174
}
145-
OauthProviderVO providerVO = _oauthProviderDao.findByProvider(provider);
175+
176+
// Check for existing provider with same name and domain
177+
OauthProviderVO providerVO = _oauthProviderDao.findByProviderAndDomain(provider, domainId);
146178
if (providerVO != null) {
147-
throw new CloudRuntimeException(String.format("Provider with the name %s is already registered", provider));
179+
if (domainId == null) {
180+
throw new CloudRuntimeException(String.format("Global provider with the name %s is already registered", provider));
181+
} else {
182+
throw new CloudRuntimeException(String.format("Provider with the name %s is already registered for domain %d", provider, domainId));
183+
}
148184
}
149185

150-
return saveOauthProvider(provider, description, clientId, secretKey, redirectUri, authorizeUrl, tokenUrl);
186+
return saveOauthProvider(provider, description, clientId, secretKey, redirectUri, authorizeUrl, tokenUrl, domainId);
151187
}
152188

153189
@Override
154-
public List<OauthProviderVO> listOauthProviders(String provider, String uuid) {
190+
public List<OauthProviderVO> listOauthProviders(String provider, String uuid, Long domainId) {
155191
List<OauthProviderVO> providers;
156192
if (uuid != null) {
157193
providers = Collections.singletonList(_oauthProviderDao.findByUuid(uuid));
194+
} else if (StringUtils.isNotBlank(provider) && domainId != null) {
195+
providers = Collections.singletonList(_oauthProviderDao.findByProviderAndDomain(provider, domainId));
158196
} else if (StringUtils.isNotBlank(provider)) {
159-
providers = Collections.singletonList(_oauthProviderDao.findByProvider(provider));
197+
providers = Collections.singletonList(_oauthProviderDao.findByProviderAndDomain(provider, null));
198+
} else if (domainId != null) {
199+
providers = _oauthProviderDao.listByDomainIncludingGlobal(domainId);
160200
} else {
161201
providers = _oauthProviderDao.listAll();
162202
}
@@ -179,6 +219,30 @@ public OauthProviderVO updateOauthProvider(UpdateOAuthProviderCmd cmd) {
179219
throw new CloudRuntimeException("Provider with the given id is not there");
180220
}
181221

222+
Long targetDomainId = providerVO.getDomainId();
223+
if (cmd.getDomainId() != null || StringUtils.isNotEmpty(cmd.getDomainPath())) {
224+
Long resolved = resolveDomainIdFromIdOrPath(cmd.getDomainId(), cmd.getDomainPath());
225+
if (resolved == null) {
226+
throw new CloudRuntimeException("Unable to resolve the supplied domain. Provide a valid domain id or path.");
227+
}
228+
resolved = normalizeGlobalScope(resolved);
229+
if (!Objects.equals(resolved, providerVO.getDomainId())) {
230+
OauthProviderVO existing = _oauthProviderDao.findByProviderAndDomain(providerVO.getProvider(), resolved);
231+
if (existing != null) {
232+
throw new CloudRuntimeException(String.format(
233+
"Provider with the name %s is already registered for domain %s", providerVO.getProvider(),
234+
resolved == null ? "ROOT (global)" : resolved));
235+
}
236+
}
237+
targetDomainId = resolved;
238+
}
239+
240+
if (Boolean.TRUE.equals(enabled) && !isOAuthPluginEnabled(targetDomainId)) {
241+
throw new CloudRuntimeException(String.format(
242+
"OAuth plugin is not enabled %s. Enable oauth2.enabled at that scope before enabling this provider.",
243+
targetDomainId == null ? "globally" : "for this domain"));
244+
}
245+
182246
if (StringUtils.isNotEmpty(description)) {
183247
providerVO.setDescription(description);
184248
}
@@ -200,20 +264,22 @@ public OauthProviderVO updateOauthProvider(UpdateOAuthProviderCmd cmd) {
200264
if (enabled != null) {
201265
providerVO.setEnabled(enabled);
202266
}
267+
providerVO.setDomainId(targetDomainId);
203268

204269
_oauthProviderDao.update(id, providerVO);
205270

206271
return _oauthProviderDao.findById(id);
207272
}
208273

209-
private OauthProviderVO saveOauthProvider(String provider, String description, String clientId, String secretKey, String redirectUri, String authorizeUrl, String tokenUrl) {
274+
private OauthProviderVO saveOauthProvider(String provider, String description, String clientId, String secretKey, String redirectUri, String authorizeUrl, String tokenUrl, Long domainId) {
210275
final OauthProviderVO oauthProviderVO = new OauthProviderVO();
211276

212277
oauthProviderVO.setProvider(provider);
213278
oauthProviderVO.setDescription(description);
214279
oauthProviderVO.setClientId(clientId);
215280
oauthProviderVO.setSecretKey(secretKey);
216281
oauthProviderVO.setRedirectUri(redirectUri);
282+
oauthProviderVO.setDomainId(domainId);
217283
oauthProviderVO.setAuthorizeUrl(authorizeUrl);
218284
oauthProviderVO.setTokenUrl(tokenUrl);
219285
oauthProviderVO.setEnabled(true);
@@ -225,7 +291,66 @@ private OauthProviderVO saveOauthProvider(String provider, String description, S
225291

226292
@Override
227293
public boolean deleteOauthProvider(Long id) {
228-
return _oauthProviderDao.remove(id);
294+
return _oauthProviderDao.expunge(id);
295+
}
296+
297+
@Override
298+
public Long resolveDomainId(Map<String, Object[]> params) {
299+
final String[] domainIdArray = (String[])params.get(ApiConstants.DOMAIN_ID);
300+
if (ArrayUtils.isNotEmpty(domainIdArray)) {
301+
String domainUuid = domainIdArray[0];
302+
if (GLOBAL_DOMAIN_FILTER.equals(domainUuid)) {
303+
return GLOBAL_DOMAIN_ID;
304+
}
305+
Domain domain = _domainService.getDomain(domainUuid);
306+
if (Objects.nonNull(domain)) {
307+
return domain.getId();
308+
}
309+
}
310+
final String[] domainArray = (String[])params.get(ApiConstants.DOMAIN);
311+
if (ArrayUtils.isNotEmpty(domainArray)) {
312+
String path = normalizeDomainPath(domainArray[0]);
313+
if (StringUtils.isNotEmpty(path)) {
314+
Domain domain = _domainService.findDomainByIdOrPath(null, path);
315+
if (Objects.nonNull(domain)) {
316+
return domain.getId();
317+
}
318+
}
319+
}
320+
return null;
321+
}
322+
323+
protected Long resolveDomainIdFromIdOrPath(Long domainId, String domainPath) {
324+
if (domainId != null) {
325+
return domainId;
326+
}
327+
String path = normalizeDomainPath(domainPath);
328+
if (StringUtils.isNotEmpty(path)) {
329+
Domain domain = _domainService.findDomainByIdOrPath(null, path);
330+
if (Objects.nonNull(domain)) {
331+
return domain.getId();
332+
}
333+
}
334+
return null;
335+
}
336+
337+
// The ROOT domain is the top of the tree, so a provider scoped to it is equivalent
338+
// to a global provider; treat it as global so the global oauth2.enabled config applies.
339+
protected Long normalizeGlobalScope(Long domainId) {
340+
return (domainId != null && Domain.ROOT_DOMAIN == domainId) ? null : domainId;
341+
}
342+
343+
protected String normalizeDomainPath(String path) {
344+
if (StringUtils.isEmpty(path)) {
345+
return null;
346+
}
347+
if (!path.startsWith("/")) {
348+
path = "/" + path;
349+
}
350+
if (!path.endsWith("/")) {
351+
path += "/";
352+
}
353+
return path;
229354
}
230355

231356
@Override

0 commit comments

Comments
 (0)