Skip to content

Commit b631da2

Browse files
Kshitij Kansalwilderrodrigues
Kshitij Kansal
authored and
wilderrodrigues
committed
Coverity Issue: Null Pointer Dereferencing fixed and Test cases added
Signed-off-by: wilderrodrigues <wrodrigues@schubergphilis.com> This closes #628
1 parent c30308d commit b631da2

2 files changed

Lines changed: 96 additions & 6 deletions

File tree

‎plugins/user-authenticators/saml2/src/org/apache/cloudstack/saml/SAML2AuthManagerImpl.java‎

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,10 @@ public class SAML2AuthManagerImpl extends AdapterBase implements SAML2AuthManage
104104
private int _refreshInterval = SAMLPluginConstants.SAML_REFRESH_INTERVAL;
105105
private AbstractReloadingMetadataProvider _idpMetaDataProvider;
106106

107+
public String getSAMLIdentityProviderMetadataURL(){
108+
return SAMLIdentityProviderMetadataURL.value();
109+
}
110+
107111
@Inject
108112
private KeystoreDao _ksDao;
109113

@@ -119,12 +123,12 @@ public class SAML2AuthManagerImpl extends AdapterBase implements SAML2AuthManage
119123
@Override
120124
public boolean start() {
121125
if (isSAMLPluginEnabled()) {
122-
setup();
123126
s_logger.info("SAML auth plugin loaded");
127+
return setup();
124128
} else {
125129
s_logger.info("SAML auth plugin not enabled so not loading");
130+
return super.start();
126131
}
127-
return super.start();
128132
}
129133

130134
@Override
@@ -135,7 +139,7 @@ public boolean stop() {
135139
return super.stop();
136140
}
137141

138-
private boolean initSP() {
142+
protected boolean initSP() {
139143
KeystoreVO keyStoreVO = _ksDao.findByName(SAMLPluginConstants.SAMLSP_KEYPAIR);
140144
if (keyStoreVO == null) {
141145
try {
@@ -338,6 +342,7 @@ public void run() {
338342
return;
339343
}
340344
s_logger.debug("Starting SAML IDP Metadata Refresh Task");
345+
341346
Map <String, SAMLProviderMetadata> metadataMap = new HashMap<String, SAMLProviderMetadata>();
342347
try {
343348
discoverAndAddIdp(_idpMetaDataProvider.getMetadata(), metadataMap);
@@ -358,7 +363,7 @@ private boolean setup() {
358363
}
359364
_timer = new Timer();
360365
final HttpClient client = new HttpClient();
361-
final String idpMetaDataUrl = SAMLIdentityProviderMetadataURL.value();
366+
final String idpMetaDataUrl = getSAMLIdentityProviderMetadataURL();
362367
if (SAMLTimeout.value() != null && SAMLTimeout.value() > SAMLPluginConstants.SAML_REFRESH_INTERVAL) {
363368
_refreshInterval = SAMLTimeout.value();
364369
}
@@ -368,21 +373,31 @@ private boolean setup() {
368373
_idpMetaDataProvider = new HTTPMetadataProvider(_timer, client, idpMetaDataUrl);
369374
} else {
370375
File metadataFile = PropertiesUtil.findConfigFile(idpMetaDataUrl);
371-
s_logger.debug("Provided Metadata is not a URL, trying to read metadata file from local path: " + metadataFile.getAbsolutePath());
372-
_idpMetaDataProvider = new FilesystemMetadataProvider(_timer, metadataFile);
376+
if (metadataFile == null) {
377+
s_logger.error("Provided Metadata is not a URL, Unable to locate metadata file from local path: " + idpMetaDataUrl);
378+
return false;
379+
}
380+
else{
381+
s_logger.debug("Provided Metadata is not a URL, trying to read metadata file from local path: " + metadataFile.getAbsolutePath());
382+
_idpMetaDataProvider = new FilesystemMetadataProvider(_timer, metadataFile);
383+
}
373384
}
374385
_idpMetaDataProvider.setRequireValidMetadata(true);
375386
_idpMetaDataProvider.setParserPool(new BasicParserPool());
376387
_idpMetaDataProvider.initialize();
377388
_timer.scheduleAtFixedRate(new MetadataRefreshTask(), 0, _refreshInterval * 1000);
389+
378390
} catch (MetadataProviderException e) {
379391
s_logger.error("Unable to read SAML2 IDP MetaData URL, error:" + e.getMessage());
380392
s_logger.error("SAML2 Authentication may be unavailable");
393+
return false;
381394
} catch (ConfigurationException | FactoryConfigurationError e) {
382395
s_logger.error("OpenSAML bootstrapping failed: error: " + e.getMessage());
396+
return false;
383397
} catch (NullPointerException e) {
384398
s_logger.error("Unable to setup SAML Auth Plugin due to NullPointerException" +
385399
" please check the SAML global settings: " + e.getMessage());
400+
return false;
386401
}
387402
return true;
388403
}

‎plugins/user-authenticators/saml2/test/org/apache/cloudstack/saml/SAML2AuthManagerImplTest.java‎

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
// Licensed to the Apache Software Foundation (ASF) under one
2+
// or more contributor license agreements. See the NOTICE file
3+
// distributed with this work for additional information
4+
// regarding copyright ownership. The ASF licenses this file
5+
// to you under the Apache License, Version 2.0 (the
6+
// "License"); you may not use this file except in compliance
7+
// with the License. You may obtain a copy of the License at
8+
//
9+
// http://www.apache.org/licenses/LICENSE-2.0
10+
//
11+
// Unless required by applicable law or agreed to in writing,
12+
// software distributed under the License is distributed on an
13+
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
14+
// KIND, either express or implied. See the License for the
15+
// specific language governing permissions and limitations
16+
// under the License.
17+
package org.apache.cloudstack.saml;
18+
19+
import com.cloud.user.DomainManager;
20+
import com.cloud.user.dao.UserDao;
21+
import org.apache.cloudstack.framework.security.keystore.KeystoreDao;
22+
import org.junit.Before;
23+
import org.junit.Test;
24+
import org.junit.runner.RunWith;
25+
import org.mockito.InjectMocks;
26+
import org.mockito.Mock;
27+
import org.mockito.Spy;
28+
import org.mockito.runners.MockitoJUnitRunner;
29+
30+
import static org.junit.Assert.assertFalse;
31+
import static org.mockito.Mockito.doReturn;
32+
import static org.mockito.Mockito.when;
33+
34+
35+
@RunWith(MockitoJUnitRunner.class)
36+
public class SAML2AuthManagerImplTest {
37+
38+
@Mock
39+
private KeystoreDao _ksDao;
40+
41+
@Mock
42+
private SAMLTokenDao _samlTokenDao;
43+
44+
@Mock
45+
private UserDao _userDao;
46+
47+
@Mock
48+
DomainManager _domainMgr;
49+
50+
@InjectMocks
51+
@Spy
52+
SAML2AuthManagerImpl saml2AuthManager = new SAML2AuthManagerImpl();
53+
54+
@Before
55+
public void setUp() {
56+
doReturn(true).when(saml2AuthManager).isSAMLPluginEnabled();
57+
doReturn(true).when(saml2AuthManager).initSP();
58+
}
59+
60+
@Test
61+
public void testStart() {
62+
when(saml2AuthManager.getSAMLIdentityProviderMetadataURL()).thenReturn("file://does/not/exist");
63+
boolean started = saml2AuthManager.start();
64+
assertFalse("saml2authmanager should not start as the file doesnt exist", started);
65+
66+
when(saml2AuthManager.getSAMLIdentityProviderMetadataURL()).thenReturn(" ");
67+
started = saml2AuthManager.start();
68+
assertFalse("saml2authmanager should not start as the file doesnt exist", started);
69+
70+
when(saml2AuthManager.getSAMLIdentityProviderMetadataURL()).thenReturn("");
71+
started = saml2AuthManager.start();
72+
assertFalse("saml2authmanager should not start as the file doesnt exist", started);
73+
74+
}
75+
}

0 commit comments

Comments
 (0)