Skip to content

Commit b056914

Browse files
committed
Implement normalization of ROOT domain to null for global OAuth provider handling and add corresponding unit tests
1 parent 3b2ef3f commit b056914

2 files changed

Lines changed: 37 additions & 3 deletions

File tree

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

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,7 @@ public OauthProviderVO registerOauthProvider(RegisterOAuthProviderCmd cmd) {
147147
String clientId = StringUtils.trim(cmd.getClientId());
148148
String redirectUri = StringUtils.trim(cmd.getRedirectUri());
149149
String secretKey = StringUtils.trim(cmd.getSecretKey());
150-
Long domainId = resolveDomainIdFromIdOrPath(cmd.getDomainId(), cmd.getDomainPath());
150+
Long domainId = normalizeGlobalScope(resolveDomainIdFromIdOrPath(cmd.getDomainId(), cmd.getDomainPath()));
151151

152152
if (!isOAuthPluginEnabled(domainId)) {
153153
throw new CloudRuntimeException("OAuth is not enabled, please enable to register");
@@ -203,11 +203,13 @@ public OauthProviderVO updateOauthProvider(UpdateOAuthProviderCmd cmd) {
203203
if (resolved == null) {
204204
throw new CloudRuntimeException("Unable to resolve the supplied domain. Provide a valid domain id or path.");
205205
}
206-
if (!resolved.equals(providerVO.getDomainId())) {
206+
resolved = normalizeGlobalScope(resolved);
207+
if (!Objects.equals(resolved, providerVO.getDomainId())) {
207208
OauthProviderVO existing = _oauthProviderDao.findByProviderAndDomain(providerVO.getProvider(), resolved);
208209
if (existing != null) {
209210
throw new CloudRuntimeException(String.format(
210-
"Provider with the name %s is already registered for domain %d", providerVO.getProvider(), resolved));
211+
"Provider with the name %s is already registered for domain %s", providerVO.getProvider(),
212+
resolved == null ? "ROOT (global)" : resolved));
211213
}
212214
}
213215
targetDomainId = resolved;
@@ -302,6 +304,12 @@ protected Long resolveDomainIdFromIdOrPath(Long domainId, String domainPath) {
302304
return null;
303305
}
304306

307+
// The ROOT domain is the top of the tree, so a provider scoped to it is equivalent
308+
// to a global provider; treat it as global so the global oauth2.enabled config applies.
309+
protected Long normalizeGlobalScope(Long domainId) {
310+
return (domainId != null && Domain.ROOT_DOMAIN == domainId) ? null : domainId;
311+
}
312+
305313
protected String normalizeDomainPath(String path) {
306314
if (StringUtils.isEmpty(path)) {
307315
return null;

plugins/user-authenticators/oauth2/src/test/java/org/apache/cloudstack/oauth2/OAuth2AuthManagerImplTest.java

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -208,6 +208,32 @@ public void testUpdateOauthProviderRejectsDuplicateAtTargetDomain() {
208208
}
209209
}
210210

211+
@Test
212+
public void testRegisterOauthProviderForRootDomainTreatedAsGlobal() {
213+
RegisterOAuthProviderCmd cmd = Mockito.mock(RegisterOAuthProviderCmd.class);
214+
when(cmd.getProvider()).thenReturn("github");
215+
when(cmd.getDomainId()).thenReturn(com.cloud.domain.Domain.ROOT_DOMAIN);
216+
when(cmd.getSecretKey()).thenReturn("secret");
217+
when(cmd.getClientId()).thenReturn("clientId");
218+
when(cmd.getRedirectUri()).thenReturn("https://redirect");
219+
220+
// global check must be consulted (domainId resolves to null), not the ROOT domain scope
221+
when(_authManager.isOAuthPluginEnabled(Mockito.isNull())).thenReturn(true);
222+
when(_oauthProviderDao.findByProviderAndDomain("github", null)).thenReturn(null);
223+
when(_oauthProviderDao.persist(Mockito.any(OauthProviderVO.class))).thenAnswer(i -> i.getArgument(0));
224+
225+
OauthProviderVO result = _authManager.registerOauthProvider(cmd);
226+
assertNull(result.getDomainId());
227+
Mockito.verify(_oauthProviderDao).findByProviderAndDomain("github", null);
228+
}
229+
230+
@Test
231+
public void testNormalizeGlobalScopeMapsRootToNull() {
232+
assertNull(_authManager.normalizeGlobalScope(com.cloud.domain.Domain.ROOT_DOMAIN));
233+
assertNull(_authManager.normalizeGlobalScope(null));
234+
assertEquals(Long.valueOf(42L), _authManager.normalizeGlobalScope(42L));
235+
}
236+
211237
@Test
212238
public void testUpdateOauthProviderRejectsEnableWhenPluginDisabledAtScope() {
213239
Long id = 7L;

0 commit comments

Comments
 (0)