From bc42cd18470581e503d40626c6eacbc8aef9284b Mon Sep 17 00:00:00 2001 From: Daan Hoogland Date: Sat, 22 Aug 2026 21:32:47 +0200 Subject: [PATCH 1/4] make sure pre saml login is enabled after saml is disabled --- .../cloudstack/saml/SAML2AuthManagerImpl.java | 30 +++++++++- .../cloudstack/SAML2AuthManagerImplTest.java | 60 +++++++++++++++++++ 2 files changed, 89 insertions(+), 1 deletion(-) diff --git a/plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/saml/SAML2AuthManagerImpl.java b/plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/saml/SAML2AuthManagerImpl.java index 9f8101b867d4..0994571ec84e 100644 --- a/plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/saml/SAML2AuthManagerImpl.java +++ b/plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/saml/SAML2AuthManagerImpl.java @@ -54,6 +54,8 @@ import org.apache.cloudstack.framework.config.Configurable; import org.apache.cloudstack.framework.security.keystore.KeystoreDao; import org.apache.cloudstack.framework.security.keystore.KeystoreVO; +import org.apache.cloudstack.resourcedetail.UserDetailVO; +import org.apache.cloudstack.resourcedetail.dao.UserDetailsDao; import org.apache.cloudstack.utils.security.CertUtils; import org.apache.commons.codec.binary.Base64; import org.apache.commons.httpclient.HttpClient; @@ -92,6 +94,10 @@ @Component public class SAML2AuthManagerImpl extends AdapterBase implements SAML2AuthManager, Configurable { + /** Remembers the user's Source (e.g. LDAP) from before SAML was authorized, so disabling + * SAML can fall back to it instead of always defaulting to {@link User.Source#UNKNOWN}. */ + private static final String PRE_SAML_SOURCE_DETAIL_KEY = "PreSamlSource"; + private SAMLProviderMetadata _spMetadata = new SAMLProviderMetadata(); private Map _idpMetadataMap = new HashMap(); @@ -115,6 +121,9 @@ public String getSAMLIdentityProviderMetadataURL(){ @Inject private UserDao _userDao; + @Inject + private UserDetailsDao userDetailsDao; + @Inject DomainManager _domainMgr; @@ -448,13 +457,16 @@ public boolean authorizeUser(Long userId, String entityId, boolean enable) { UserVO user = _userDao.getUser(userId); if (user != null) { if (enable) { + if (user.getSource() != null && !User.Source.SAML2.equals(user.getSource()) && !User.Source.SAML2DISABLED.equals(user.getSource())) { + userDetailsDao.addDetail(user.getId(), PRE_SAML_SOURCE_DETAIL_KEY, user.getSource().toString(), false); + } user.setExternalEntity(entityId); user.setSource(User.Source.SAML2); } else { boolean enableLoginAfterSAMLDisable = SAML2AuthManager.EnableLoginAfterSAMLDisable.value(); if (user.getSource().equals(User.Source.SAML2)) { if(enableLoginAfterSAMLDisable) { - user.setSource(User.Source.UNKNOWN); + user.setSource(getPreSamlSource(user.getId())); } else { user.setSource(User.Source.SAML2DISABLED); } @@ -468,6 +480,22 @@ public boolean authorizeUser(Long userId, String entityId, boolean enable) { return false; } + /** + * The Source (e.g. LDAP) the user had before SAML was authorized for them, so disabling + * SAML can restore it instead of always falling back to {@link User.Source#UNKNOWN}. + */ + private User.Source getPreSamlSource(long userId) { + UserDetailVO preSamlSource = userDetailsDao.findDetail(userId, PRE_SAML_SOURCE_DETAIL_KEY); + if (preSamlSource != null) { + try { + return User.Source.valueOf(preSamlSource.getValue()); + } catch (IllegalArgumentException e) { + logger.warn("Unrecognized pre-SAML source '{}' stored for user {}; falling back to UNKNOWN", preSamlSource.getValue(), userId); + } + } + return User.Source.UNKNOWN; + } + @Override public void saveToken(String authnId, String domainPath, String entity) { Long domainId = null; diff --git a/plugins/user-authenticators/saml2/src/test/java/org/apache/cloudstack/SAML2AuthManagerImplTest.java b/plugins/user-authenticators/saml2/src/test/java/org/apache/cloudstack/SAML2AuthManagerImplTest.java index 81594db1009f..5b06072620b6 100644 --- a/plugins/user-authenticators/saml2/src/test/java/org/apache/cloudstack/SAML2AuthManagerImplTest.java +++ b/plugins/user-authenticators/saml2/src/test/java/org/apache/cloudstack/SAML2AuthManagerImplTest.java @@ -20,8 +20,11 @@ package org.apache.cloudstack; import java.lang.reflect.Field; +import java.lang.reflect.Method; import org.apache.cloudstack.framework.security.keystore.KeystoreDao; +import org.apache.cloudstack.resourcedetail.UserDetailVO; +import org.apache.cloudstack.resourcedetail.dao.UserDetailsDao; import org.apache.cloudstack.saml.SAML2AuthManagerImpl; import org.apache.cloudstack.saml.SAMLTokenDao; import org.apache.cloudstack.saml.SAMLTokenVO; @@ -50,6 +53,9 @@ public class SAML2AuthManagerImplTest extends TestCase { @Mock private UserDao userDao; + @Mock + private UserDetailsDao userDetailsDao; + @Mock DomainManager domainMgr; @@ -72,6 +78,10 @@ public void setUp() throws NoSuchFieldException, IllegalAccessException { userDaoField.setAccessible(true); userDaoField.set(saml2AuthManager, userDao); + Field userDetailsDaoField = SAML2AuthManagerImpl.class.getDeclaredField("userDetailsDao"); + userDetailsDaoField.setAccessible(true); + userDetailsDaoField.set(saml2AuthManager, userDetailsDao); + Field domainMgrField = SAML2AuthManagerImpl.class.getDeclaredField("_domainMgr"); domainMgrField.setAccessible(true); domainMgrField.set(saml2AuthManager, domainMgr); @@ -117,7 +127,57 @@ public void testAuthorizeUser() { Mockito.verify(userDao, Mockito.atLeastOnce()).update(Mockito.anyLong(), Mockito.any(user.getClass())); } + @Test + public void testAuthorizeUserStoresPreSamlSourceOnEnable() { + UserVO user = new UserVO(200L); + user.setUsername("someuser"); + user.setSource(User.Source.LDAP); + Mockito.when(userDao.getUser(Mockito.anyLong())).thenReturn(user); + + saml2AuthManager.authorizeUser(200L, "someID", true); + + Mockito.verify(userDetailsDao).addDetail(200L, "PreSamlSource", "LDAP", false); + assertEquals(User.Source.SAML2, user.getSource()); + } + + @Test + public void testAuthorizeUserDoesNotRestorePreSamlSourceWhenAlreadyAuthorized() { + UserVO user = new UserVO(200L); + user.setUsername("someuser"); + user.setSource(User.Source.SAML2); + Mockito.when(userDao.getUser(Mockito.anyLong())).thenReturn(user); + + saml2AuthManager.authorizeUser(200L, "someID", true); + + Mockito.verify(userDetailsDao, Mockito.never()).addDetail(Mockito.anyLong(), Mockito.anyString(), Mockito.anyString(), Mockito.anyBoolean()); + } + + @Test + public void testGetPreSamlSourceRestoresStoredSource() throws Exception { + Mockito.when(userDetailsDao.findDetail(200L, "PreSamlSource")).thenReturn(new UserDetailVO(200L, "PreSamlSource", "LDAP")); + + assertEquals(User.Source.LDAP, invokeGetPreSamlSource(200L)); + } + + @Test + public void testGetPreSamlSourceDefaultsToUnknownWhenNothingStored() throws Exception { + Mockito.when(userDetailsDao.findDetail(200L, "PreSamlSource")).thenReturn(null); + + assertEquals(User.Source.UNKNOWN, invokeGetPreSamlSource(200L)); + } + + @Test + public void testGetPreSamlSourceDefaultsToUnknownOnGarbageValue() throws Exception { + Mockito.when(userDetailsDao.findDetail(200L, "PreSamlSource")).thenReturn(new UserDetailVO(200L, "PreSamlSource", "not-a-real-source")); + + assertEquals(User.Source.UNKNOWN, invokeGetPreSamlSource(200L)); + } + private User.Source invokeGetPreSamlSource(long userId) throws Exception { + Method method = SAML2AuthManagerImpl.class.getDeclaredMethod("getPreSamlSource", long.class); + method.setAccessible(true); + return (User.Source) method.invoke(saml2AuthManager, userId); + } @Test public void testSaveToken() { From 08f943d56a28cf701cee6a225793d04fdcc81ebb Mon Sep 17 00:00:00 2001 From: Daan Hoogland Date: Tue, 25 Aug 2026 11:58:48 +0200 Subject: [PATCH 2/4] sonarqube --- .../cloudstack/saml/SAML2AuthManagerImpl.java | 51 +++++++++++-------- .../cloudstack/SAML2AuthManagerImplTest.java | 18 ++++--- 2 files changed, 40 insertions(+), 29 deletions(-) diff --git a/plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/saml/SAML2AuthManagerImpl.java b/plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/saml/SAML2AuthManagerImpl.java index 0994571ec84e..87b40b393622 100644 --- a/plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/saml/SAML2AuthManagerImpl.java +++ b/plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/saml/SAML2AuthManagerImpl.java @@ -455,29 +455,36 @@ public boolean isUserAuthorized(Long userId, String entityId) { @Override public boolean authorizeUser(Long userId, String entityId, boolean enable) { UserVO user = _userDao.getUser(userId); - if (user != null) { - if (enable) { - if (user.getSource() != null && !User.Source.SAML2.equals(user.getSource()) && !User.Source.SAML2DISABLED.equals(user.getSource())) { - userDetailsDao.addDetail(user.getId(), PRE_SAML_SOURCE_DETAIL_KEY, user.getSource().toString(), false); - } - user.setExternalEntity(entityId); - user.setSource(User.Source.SAML2); - } else { - boolean enableLoginAfterSAMLDisable = SAML2AuthManager.EnableLoginAfterSAMLDisable.value(); - if (user.getSource().equals(User.Source.SAML2)) { - if(enableLoginAfterSAMLDisable) { - user.setSource(getPreSamlSource(user.getId())); - } else { - user.setSource(User.Source.SAML2DISABLED); - } - } else { - return false; - } - } - _userDao.update(user.getId(), user); - return true; + if (user == null) { + return false; } - return false; + if (enable) { + enableSAMLForUser(user, entityId); + } else if (!disableSAMLForUser(user)) { + return false; + } + _userDao.update(user.getId(), user); + return true; + } + + private void enableSAMLForUser(UserVO user, String entityId) { + if (user.getSource() != null && !User.Source.SAML2.equals(user.getSource()) && !User.Source.SAML2DISABLED.equals(user.getSource())) { + userDetailsDao.addDetail(user.getId(), PRE_SAML_SOURCE_DETAIL_KEY, user.getSource().toString(), false); + } + user.setExternalEntity(entityId); + user.setSource(User.Source.SAML2); + } + + private boolean disableSAMLForUser(UserVO user) { + if (!user.getSource().equals(User.Source.SAML2)) { + return false; + } + if (SAML2AuthManager.EnableLoginAfterSAMLDisable.value()) { + user.setSource(getPreSamlSource(user.getId())); + } else { + user.setSource(User.Source.SAML2DISABLED); + } + return true; } /** diff --git a/plugins/user-authenticators/saml2/src/test/java/org/apache/cloudstack/SAML2AuthManagerImplTest.java b/plugins/user-authenticators/saml2/src/test/java/org/apache/cloudstack/SAML2AuthManagerImplTest.java index 5b06072620b6..161b20fcd756 100644 --- a/plugins/user-authenticators/saml2/src/test/java/org/apache/cloudstack/SAML2AuthManagerImplTest.java +++ b/plugins/user-authenticators/saml2/src/test/java/org/apache/cloudstack/SAML2AuthManagerImplTest.java @@ -19,6 +19,10 @@ package org.apache.cloudstack; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + import java.lang.reflect.Field; import java.lang.reflect.Method; @@ -132,11 +136,11 @@ public void testAuthorizeUserStoresPreSamlSourceOnEnable() { UserVO user = new UserVO(200L); user.setUsername("someuser"); user.setSource(User.Source.LDAP); - Mockito.when(userDao.getUser(Mockito.anyLong())).thenReturn(user); + when(userDao.getUser(Mockito.anyLong())).thenReturn(user); saml2AuthManager.authorizeUser(200L, "someID", true); - Mockito.verify(userDetailsDao).addDetail(200L, "PreSamlSource", "LDAP", false); + verify(userDetailsDao).addDetail(200L, "PreSamlSource", "LDAP", false); assertEquals(User.Source.SAML2, user.getSource()); } @@ -145,30 +149,30 @@ public void testAuthorizeUserDoesNotRestorePreSamlSourceWhenAlreadyAuthorized() UserVO user = new UserVO(200L); user.setUsername("someuser"); user.setSource(User.Source.SAML2); - Mockito.when(userDao.getUser(Mockito.anyLong())).thenReturn(user); + when(userDao.getUser(Mockito.anyLong())).thenReturn(user); saml2AuthManager.authorizeUser(200L, "someID", true); - Mockito.verify(userDetailsDao, Mockito.never()).addDetail(Mockito.anyLong(), Mockito.anyString(), Mockito.anyString(), Mockito.anyBoolean()); + verify(userDetailsDao, never()).addDetail(Mockito.anyLong(), Mockito.anyString(), Mockito.anyString(), Mockito.anyBoolean()); } @Test public void testGetPreSamlSourceRestoresStoredSource() throws Exception { - Mockito.when(userDetailsDao.findDetail(200L, "PreSamlSource")).thenReturn(new UserDetailVO(200L, "PreSamlSource", "LDAP")); + when(userDetailsDao.findDetail(200L, "PreSamlSource")).thenReturn(new UserDetailVO(200L, "PreSamlSource", "LDAP")); assertEquals(User.Source.LDAP, invokeGetPreSamlSource(200L)); } @Test public void testGetPreSamlSourceDefaultsToUnknownWhenNothingStored() throws Exception { - Mockito.when(userDetailsDao.findDetail(200L, "PreSamlSource")).thenReturn(null); + when(userDetailsDao.findDetail(200L, "PreSamlSource")).thenReturn(null); assertEquals(User.Source.UNKNOWN, invokeGetPreSamlSource(200L)); } @Test public void testGetPreSamlSourceDefaultsToUnknownOnGarbageValue() throws Exception { - Mockito.when(userDetailsDao.findDetail(200L, "PreSamlSource")).thenReturn(new UserDetailVO(200L, "PreSamlSource", "not-a-real-source")); + when(userDetailsDao.findDetail(200L, "PreSamlSource")).thenReturn(new UserDetailVO(200L, "PreSamlSource", "not-a-real-source")); assertEquals(User.Source.UNKNOWN, invokeGetPreSamlSource(200L)); } From 7eadb76cec07ab8db8b14daf3b8332bf52a9e0e8 Mon Sep 17 00:00:00 2001 From: Daan Hoogland Date: Tue, 20 Oct 2026 09:54:15 +0200 Subject: [PATCH 3/4] extra test for different scenarios, including not enabling fallback login after disabling saml --- .../cloudstack/SAML2AuthManagerImplTest.java | 121 ++++++++++++++++++ 1 file changed, 121 insertions(+) diff --git a/plugins/user-authenticators/saml2/src/test/java/org/apache/cloudstack/SAML2AuthManagerImplTest.java b/plugins/user-authenticators/saml2/src/test/java/org/apache/cloudstack/SAML2AuthManagerImplTest.java index 161b20fcd756..c573dd982bdf 100644 --- a/plugins/user-authenticators/saml2/src/test/java/org/apache/cloudstack/SAML2AuthManagerImplTest.java +++ b/plugins/user-authenticators/saml2/src/test/java/org/apache/cloudstack/SAML2AuthManagerImplTest.java @@ -26,12 +26,15 @@ import java.lang.reflect.Field; import java.lang.reflect.Method; +import org.apache.cloudstack.framework.config.ConfigKey; import org.apache.cloudstack.framework.security.keystore.KeystoreDao; import org.apache.cloudstack.resourcedetail.UserDetailVO; import org.apache.cloudstack.resourcedetail.dao.UserDetailsDao; +import org.apache.cloudstack.saml.SAML2AuthManager; import org.apache.cloudstack.saml.SAML2AuthManagerImpl; import org.apache.cloudstack.saml.SAMLTokenDao; import org.apache.cloudstack.saml.SAMLTokenVO; +import org.junit.After; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -94,6 +97,39 @@ public void setUp() throws NoSuchFieldException, IllegalAccessException { Mockito.doReturn(true).when(saml2AuthManager).isSAMLPluginEnabled(); } + @After + public void tearDown() { + resetConfigKeyValue(SAML2AuthManager.EnableLoginAfterSAMLDisable); + } + + private static void setConfigKeyValue(ConfigKey configKey, T value) { + try { + Field valueField = ConfigKey.class.getDeclaredField("_value"); + valueField.setAccessible(true); + valueField.set(configKey, value); + + Field dynamicField = ConfigKey.class.getDeclaredField("_isDynamic"); + dynamicField.setAccessible(true); + dynamicField.setBoolean(configKey, false); + } catch (IllegalAccessException | NoSuchFieldException e) { + throw new RuntimeException("Failed to set ConfigKey value", e); + } + } + + private static void resetConfigKeyValue(ConfigKey configKey) { + try { + Field valueField = ConfigKey.class.getDeclaredField("_value"); + valueField.setAccessible(true); + valueField.set(configKey, null); + + Field dynamicField = ConfigKey.class.getDeclaredField("_isDynamic"); + dynamicField.setAccessible(true); + dynamicField.setBoolean(configKey, true); + } catch (IllegalAccessException | NoSuchFieldException e) { + throw new RuntimeException("Failed to reset ConfigKey value", e); + } + } + @Test public void testIsUserAuthorized() { final String entityID = "some IDP ID"; @@ -156,6 +192,91 @@ public void testAuthorizeUserDoesNotRestorePreSamlSourceWhenAlreadyAuthorized() verify(userDetailsDao, never()).addDetail(Mockito.anyLong(), Mockito.anyString(), Mockito.anyString(), Mockito.anyBoolean()); } + @Test + public void testAuthorizeUserEnableWithNullSourceSkipsStore() { + UserVO user = new UserVO(200L); + user.setUsername("someuser"); + user.setSource(null); + when(userDao.getUser(Mockito.anyLong())).thenReturn(user); + + assertTrue(saml2AuthManager.authorizeUser(200L, "someID", true)); + + verify(userDetailsDao, never()).addDetail(Mockito.anyLong(), Mockito.anyString(), Mockito.anyString(), Mockito.anyBoolean()); + assertEquals(User.Source.SAML2, user.getSource()); + } + + @Test + public void testAuthorizeUserEnableSkipsStoreWhenAlreadyDisabled() { + UserVO user = new UserVO(200L); + user.setUsername("someuser"); + user.setSource(User.Source.SAML2DISABLED); + when(userDao.getUser(Mockito.anyLong())).thenReturn(user); + + saml2AuthManager.authorizeUser(200L, "someID", true); + + verify(userDetailsDao, never()).addDetail(Mockito.anyLong(), Mockito.anyString(), Mockito.anyString(), Mockito.anyBoolean()); + assertEquals(User.Source.SAML2, user.getSource()); + } + + @Test + public void testAuthorizeUserDisableReturnsFalseWhenUserNotCurrentlySaml() { + UserVO user = new UserVO(200L); + user.setUsername("someuser"); + user.setSource(User.Source.LDAP); + when(userDao.getUser(Mockito.anyLong())).thenReturn(user); + + assertFalse(saml2AuthManager.authorizeUser(200L, "someID", false)); + + assertEquals(User.Source.LDAP, user.getSource()); + Mockito.verify(userDao, never()).update(Mockito.anyLong(), Mockito.any(UserVO.class)); + } + + // With enable.login.with.disabled.saml left at its default (false), disabling SAML must still + // block login via SAML2DISABLED, regardless of any PreSamlSource recorded. + @Test + public void testAuthorizeUserDisableSetsSaml2DisabledWhenFlagOff() { + UserVO user = new UserVO(200L); + user.setUsername("someuser"); + user.setSource(User.Source.SAML2); + when(userDao.getUser(Mockito.anyLong())).thenReturn(user); + + assertTrue(saml2AuthManager.authorizeUser(200L, "someID", false)); + + assertEquals(User.Source.SAML2DISABLED, user.getSource()); + verify(userDetailsDao, never()).findDetail(Mockito.anyLong(), Mockito.anyString()); + } + + @Test + public void testAuthorizeUserDisableRestoresPreSamlSourceWhenFlagOn() { + setConfigKeyValue(SAML2AuthManager.EnableLoginAfterSAMLDisable, true); + + UserVO user = new UserVO(200L); + user.setUsername("someuser"); + user.setSource(User.Source.SAML2); + when(userDao.getUser(Mockito.anyLong())).thenReturn(user); + when(userDetailsDao.findDetail(200L, "PreSamlSource")).thenReturn(new UserDetailVO(200L, "PreSamlSource", "LDAP")); + + assertTrue(saml2AuthManager.authorizeUser(200L, "someID", false)); + + assertEquals(User.Source.LDAP, user.getSource()); + Mockito.verify(userDao, Mockito.atLeastOnce()).update(Mockito.anyLong(), Mockito.any(UserVO.class)); + } + + @Test + public void testAuthorizeUserDisableFallsBackToUnknownWhenFlagOnButNothingStored() { + setConfigKeyValue(SAML2AuthManager.EnableLoginAfterSAMLDisable, true); + + UserVO user = new UserVO(200L); + user.setUsername("someuser"); + user.setSource(User.Source.SAML2); + when(userDao.getUser(Mockito.anyLong())).thenReturn(user); + when(userDetailsDao.findDetail(200L, "PreSamlSource")).thenReturn(null); + + assertTrue(saml2AuthManager.authorizeUser(200L, "someID", false)); + + assertEquals(User.Source.UNKNOWN, user.getSource()); + } + @Test public void testGetPreSamlSourceRestoresStoredSource() throws Exception { when(userDetailsDao.findDetail(200L, "PreSamlSource")).thenReturn(new UserDetailVO(200L, "PreSamlSource", "LDAP")); From a35ca4156c8b204ee8c253988f4102baa1c7bffd Mon Sep 17 00:00:00 2001 From: dahn Date: Wed, 21 Oct 2026 10:51:13 +0200 Subject: [PATCH 4/4] Apply batched suggestions from code review Co-authored-by: dahn --- .../apache/cloudstack/saml/SAML2AuthManagerImpl.java | 10 +++++----- .../apache/cloudstack/SAML2AuthManagerImplTest.java | 4 ++-- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/saml/SAML2AuthManagerImpl.java b/plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/saml/SAML2AuthManagerImpl.java index 87b40b393622..380df4fef7f8 100644 --- a/plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/saml/SAML2AuthManagerImpl.java +++ b/plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/saml/SAML2AuthManagerImpl.java @@ -469,16 +469,16 @@ public boolean authorizeUser(Long userId, String entityId, boolean enable) { private void enableSAMLForUser(UserVO user, String entityId) { if (user.getSource() != null && !User.Source.SAML2.equals(user.getSource()) && !User.Source.SAML2DISABLED.equals(user.getSource())) { - userDetailsDao.addDetail(user.getId(), PRE_SAML_SOURCE_DETAIL_KEY, user.getSource().toString(), false); + userDetailsDao.addDetail(user.getId(), PRE_SAML_SOURCE_DETAIL_KEY, user.getSource().name(), false); } user.setExternalEntity(entityId); user.setSource(User.Source.SAML2); } - private boolean disableSAMLForUser(UserVO user) { - if (!user.getSource().equals(User.Source.SAML2)) { - return false; - } + private boolean disableSAMLForUser(UserVO user) { + if (!User.Source.SAML2.equals(user.getSource())) { + return false; + } if (SAML2AuthManager.EnableLoginAfterSAMLDisable.value()) { user.setSource(getPreSamlSource(user.getId())); } else { diff --git a/plugins/user-authenticators/saml2/src/test/java/org/apache/cloudstack/SAML2AuthManagerImplTest.java b/plugins/user-authenticators/saml2/src/test/java/org/apache/cloudstack/SAML2AuthManagerImplTest.java index c573dd982bdf..0abe42a231ec 100644 --- a/plugins/user-authenticators/saml2/src/test/java/org/apache/cloudstack/SAML2AuthManagerImplTest.java +++ b/plugins/user-authenticators/saml2/src/test/java/org/apache/cloudstack/SAML2AuthManagerImplTest.java @@ -180,8 +180,8 @@ public void testAuthorizeUserStoresPreSamlSourceOnEnable() { assertEquals(User.Source.SAML2, user.getSource()); } - @Test - public void testAuthorizeUserDoesNotRestorePreSamlSourceWhenAlreadyAuthorized() { + @Test + public void testAuthorizeUserDoesNotStorePreSamlSourceWhenAlreadySaml() { UserVO user = new UserVO(200L); user.setUsername("someuser"); user.setSource(User.Source.SAML2);