Skip to content

Commit 785b888

Browse files
committed
Implement domain removal listener to clean up OAuth providers on domain deletion
1 parent e4c3b3d commit 785b888

4 files changed

Lines changed: 71 additions & 0 deletions

File tree

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

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,14 @@
1919
package org.apache.cloudstack.oauth2;
2020

2121
import com.cloud.domain.Domain;
22+
import com.cloud.domain.DomainVO;
23+
import com.cloud.user.DomainManager;
2224
import com.cloud.user.DomainService;
2325
import com.cloud.user.dao.UserDao;
2426
import com.cloud.utils.component.Manager;
2527
import com.cloud.utils.component.ManagerBase;
2628
import com.cloud.utils.exception.CloudRuntimeException;
29+
import org.apache.cloudstack.framework.messagebus.MessageBus;
2730
import org.apache.cloudstack.api.ApiConstants;
2831
import org.apache.cloudstack.auth.UserOAuth2Authenticator;
2932
import org.apache.cloudstack.framework.config.ConfigKey;
@@ -58,6 +61,9 @@ public class OAuth2AuthManagerImpl extends ManagerBase implements OAuth2AuthMana
5861
@Inject
5962
private DomainService _domainService;
6063

64+
@Inject
65+
private MessageBus _messageBus;
66+
6167
protected static Map<String, UserOAuth2Authenticator> userOAuth2AuthenticationProvidersMap = new HashMap<>();
6268

6369
private List<UserOAuth2Authenticator> userOAuth2AuthenticationProviders;
@@ -74,10 +80,26 @@ public List<Class<?>> getAuthCommands() {
7480
@Override
7581
public boolean start() {
7682
initializeUserOAuth2AuthenticationProvidersMap();
83+
addDomainRemovalListener();
7784
logger.info("OAUTH plugin loaded");
7885
return true;
7986
}
8087

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

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@ public interface OauthProviderDao extends GenericDao<OauthProviderVO, Long> {
2727

2828
public List<OauthProviderVO> listByDomainIncludingGlobal(Long domainId);
2929

30+
public List<OauthProviderVO> listByDomain(Long domainId);
31+
3032
public OauthProviderVO findByProviderAndDomainWithGlobalFallback(String provider, Long domainId);
3133

3234
}

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,13 @@ public List<OauthProviderVO> listByDomainIncludingGlobal(Long domainId) {
5454
return listBy(sc);
5555
}
5656

57+
@Override
58+
public List<OauthProviderVO> listByDomain(Long domainId) {
59+
SearchCriteria<OauthProviderVO> sc = createSearchCriteria();
60+
sc.addAnd("domainId", SearchCriteria.Op.EQ, domainId);
61+
return listBy(sc);
62+
}
63+
5764
@Override
5865
public OauthProviderVO findByProviderAndDomainWithGlobalFallback(String provider, Long domainId) {
5966
OauthProviderVO providerVO = null;

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

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,12 @@
2020
package org.apache.cloudstack.oauth2;
2121

2222
import com.cloud.domain.Domain;
23+
import com.cloud.domain.DomainVO;
2324
import com.cloud.user.DomainService;
2425
import com.cloud.utils.exception.CloudRuntimeException;
2526
import org.apache.cloudstack.api.ApiConstants;
27+
import org.apache.cloudstack.framework.messagebus.MessageBus;
28+
import org.apache.cloudstack.framework.messagebus.MessageSubscriber;
2629
import org.apache.cloudstack.oauth2.api.command.DeleteOAuthProviderCmd;
2730
import org.apache.cloudstack.oauth2.api.command.RegisterOAuthProviderCmd;
2831
import org.apache.cloudstack.oauth2.api.command.UpdateOAuthProviderCmd;
@@ -49,6 +52,7 @@
4952
import static org.junit.Assert.assertNull;
5053
import static org.junit.Assert.assertTrue;
5154
import static org.mockito.Mockito.doNothing;
55+
import static org.mockito.Mockito.verify;
5256
import static org.mockito.Mockito.when;
5357

5458
public class OAuth2AuthManagerImplTest {
@@ -63,6 +67,9 @@ public class OAuth2AuthManagerImplTest {
6367
@Mock
6468
DomainService _domainService;
6569

70+
@Mock
71+
MessageBus _messageBus;
72+
6673
AutoCloseable closeable;
6774
@Before
6875
public void setUp() {
@@ -636,4 +643,37 @@ public void testDuplicateGlobalProviderRejected() {
636643
}
637644
}
638645

646+
@Test
647+
public void testDomainDeletionCleansUpOAuthProviders() {
648+
Long domainId = 42L;
649+
650+
OauthProviderVO provider1 = new OauthProviderVO();
651+
provider1.setProvider("github");
652+
provider1.setDomainId(domainId);
653+
654+
OauthProviderVO provider2 = new OauthProviderVO();
655+
provider2.setProvider("google");
656+
provider2.setDomainId(domainId);
657+
658+
when(_oauthProviderDao.listByDomain(domainId)).thenReturn(Arrays.asList(provider1, provider2));
659+
when(_oauthProviderDao.expunge(Mockito.anyLong())).thenReturn(true);
660+
661+
// Capture the subscriber registered during start()
662+
doNothing().when(_authManager).initializeUserOAuth2AuthenticationProvidersMap();
663+
Mockito.doAnswer(invocation -> {
664+
String subject = invocation.getArgument(0);
665+
MessageSubscriber subscriber = invocation.getArgument(1);
666+
// Simulate domain removal event
667+
DomainVO domain = Mockito.mock(DomainVO.class);
668+
when(domain.getId()).thenReturn(domainId);
669+
subscriber.onPublishMessage("", subject, domain);
670+
return null;
671+
}).when(_messageBus).subscribe(Mockito.eq(com.cloud.user.DomainManager.MESSAGE_PRE_REMOVE_DOMAIN_EVENT), Mockito.any());
672+
673+
_authManager.start();
674+
675+
verify(_oauthProviderDao).listByDomain(domainId);
676+
verify(_oauthProviderDao, Mockito.times(2)).expunge(Mockito.anyLong());
677+
}
678+
639679
}

0 commit comments

Comments
 (0)