Skip to content

Commit e326ed6

Browse files
Pearl1594Pearl1594
committed
Merge branch '4.18' of https://github.com/apache/cloudstack into 4.18
2 parents 8410707 + 33be0ae commit e326ed6

10 files changed

Lines changed: 103 additions & 45 deletions

File tree

‎api/src/main/java/org/apache/cloudstack/api/command/user/backup/ListBackupsCmd.java‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
import org.apache.cloudstack.backup.Backup;
3737
import org.apache.cloudstack.backup.BackupManager;
3838
import org.apache.cloudstack.context.CallContext;
39+
import org.apache.log4j.Logger;
3940

4041
import com.cloud.exception.ConcurrentOperationException;
4142
import com.cloud.exception.InsufficientCapacityException;
@@ -49,6 +50,7 @@
4950
responseObject = BackupResponse.class, since = "4.14.0",
5051
authorized = {RoleType.Admin, RoleType.ResourceAdmin, RoleType.DomainAdmin, RoleType.User})
5152
public class ListBackupsCmd extends BaseListProjectAndAccountResourcesCmd {
53+
private static final Logger s_logger = Logger.getLogger(ListBackupsCmd.class);
5254

5355
@Inject
5456
private BackupManager backupManager;
@@ -116,6 +118,7 @@ public void execute() throws ResourceUnavailableException, InsufficientCapacityE
116118
Pair<List<Backup>, Integer> result = backupManager.listBackups(this);
117119
setupResponseBackupList(result.first(), result.second());
118120
} catch (Exception e) {
121+
s_logger.debug("Exception while listing backups", e);
119122
throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, e.getMessage());
120123
}
121124
}

‎api/src/main/java/org/apache/cloudstack/backup/Backup.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,7 @@ public String toString() {
132132
}
133133

134134
long getVmId();
135+
long getBackupOfferingId();
135136
String getExternalId();
136137
String getType();
137138
String getDate();

‎engine/schema/src/main/java/com/cloud/usage/dao/UsageDaoImpl.java‎

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import com.cloud.utils.db.SearchCriteria;
2929
import com.cloud.utils.db.Transaction;
3030
import com.cloud.utils.db.TransactionCallback;
31+
import com.cloud.utils.db.TransactionCallbackNoReturn;
3132
import com.cloud.utils.db.TransactionLegacy;
3233
import com.cloud.utils.db.TransactionStatus;
3334
import com.cloud.utils.exception.CloudRuntimeException;
@@ -469,21 +470,20 @@ public void saveUsageRecords(List<UsageVO> usageRecords) {
469470

470471
@Override
471472
public void removeOldUsageRecords(int days) {
472-
String sql = DELETE_ALL_BY_INTERVAL;
473-
TransactionLegacy txn = TransactionLegacy.open(TransactionLegacy.USAGE_DB);
474-
PreparedStatement pstmt = null;
475-
try {
476-
txn.start();
477-
pstmt = txn.prepareAutoCloseStatement(sql);
478-
pstmt.setLong(1, days);
479-
pstmt.executeUpdate();
480-
txn.commit();
481-
} catch (Exception ex) {
482-
txn.rollback();
483-
s_logger.error("error removing old cloud_usage records for interval: " + days);
484-
} finally {
485-
txn.close();
486-
}
473+
Transaction.execute(TransactionLegacy.USAGE_DB, new TransactionCallbackNoReturn() {
474+
@Override
475+
public void doInTransactionWithoutResult(TransactionStatus status) {
476+
TransactionLegacy txn = TransactionLegacy.currentTxn();
477+
PreparedStatement pstmt = null;
478+
try {
479+
pstmt = txn.prepareAutoCloseStatement(DELETE_ALL_BY_INTERVAL);
480+
pstmt.setLong(1, days);
481+
pstmt.executeUpdate();
482+
} catch (Exception ex) {
483+
s_logger.error("error removing old cloud_usage records for interval: " + days);
484+
}
485+
}
486+
});
487487
}
488488

489489
public UsageVO persistUsage(final UsageVO usage) {

‎engine/schema/src/main/java/org/apache/cloudstack/backup/BackupVO.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,7 @@ public void setStatus(Status status) {
155155
this.status = status;
156156
}
157157

158+
@Override
158159
public long getBackupOfferingId() {
159160
return backupOfferingId;
160161
}

‎engine/schema/src/main/java/org/apache/cloudstack/backup/dao/BackupDaoImpl.java‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,11 @@ public BackupResponse newBackupResponse(Backup backup) {
145145
AccountVO account = accountDao.findByIdIncludingRemoved(vm.getAccountId());
146146
DomainVO domain = domainDao.findByIdIncludingRemoved(vm.getDomainId());
147147
DataCenterVO zone = dataCenterDao.findByIdIncludingRemoved(vm.getDataCenterId());
148-
BackupOffering offering = backupOfferingDao.findByIdIncludingRemoved(vm.getBackupOfferingId());
148+
Long offeringId = vm.getBackupOfferingId();
149+
if (offeringId == null) {
150+
offeringId = backup.getBackupOfferingId();
151+
}
152+
BackupOffering offering = backupOfferingDao.findByIdIncludingRemoved(offeringId);
149153

150154
BackupResponse response = new BackupResponse();
151155
response.setId(backup.getUuid());

‎plugins/hypervisors/kvm/src/test/java/com/cloud/hypervisor/kvm/resource/wrapper/LibvirtOvsFetchInterfaceCommandWrapperTest.java‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import java.util.UUID;
2727

2828
import org.junit.Assert;
29+
import org.junit.Assume;
2930
import org.junit.Test;
3031
import org.junit.runner.RunWith;
3132
import org.mockito.Spy;
@@ -51,8 +52,7 @@ public void testGetInterfaceDetailsValidValid() {
5152
while(interfaces.hasMoreElements()) {
5253
NetworkInterface networkInterface = interfaces.nextElement();
5354
if (networkInterface.getInetAddresses().hasMoreElements() &&
54-
(networkInterface.getName().startsWith("eth") ||
55-
networkInterface.getName().startsWith("wl"))) {
55+
networkInterface.getName().matches("^(eth|wl|en).*")) {
5656
interfaceName = networkInterface.getName();
5757
Enumeration<InetAddress> addresses = networkInterface.getInetAddresses();
5858
while(addresses.hasMoreElements()) {
@@ -62,9 +62,13 @@ public void testGetInterfaceDetailsValidValid() {
6262
break;
6363
};
6464
}
65+
if (StringUtils.isNotBlank(interfaceName) && StringUtils.isNotBlank(ipAddress)) {
66+
break;
67+
}
6568
}
6669
}
6770
} catch (SocketException ignored) {}
71+
Assume.assumeTrue(StringUtils.isNotBlank(interfaceName));
6872
Ternary<String, String, String> result = null;
6973
try {
7074
result = wrapper.getInterfaceDetails(interfaceName);

‎plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/api/command/SAML2LoginAPIAuthenticatorCmd.java‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,14 @@ public Response processSAMLResponse(String responseMessage) {
144144
return responseObject;
145145
}
146146

147+
protected void checkAndFailOnMissingSAMLSignature(Signature signature) {
148+
if (signature == null && SAML2AuthManager.SAMLCheckSignature.value()) {
149+
s_logger.error("Failing SAML login due to missing signature in the SAML response and signature check is enforced. " +
150+
"Please check and ensure the IDP configuration has signing certificate or relax the saml2.check.signature setting.");
151+
throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, "Signature is missing from the SAML Response. Please contact the Administrator");
152+
}
153+
}
154+
147155
@Override
148156
public String authenticate(final String command, final Map<String, Object[]> params, final HttpSession session, final InetAddress remoteAddress, final String responseType, final StringBuilder auditTrailSb, final HttpServletRequest req, final HttpServletResponse resp) throws ServerApiException {
149157
try {
@@ -220,11 +228,13 @@ public String authenticate(final String command, final Map<String, Object[]> par
220228
"Received SAML response for a SSO request that we may not have made or has expired, please try logging in again",
221229
params, responseType));
222230
}
231+
samlAuthManager.purgeToken(token);
223232

224233
// Set IdpId for this session
225234
session.setAttribute(SAMLPluginConstants.SAML_IDPID, issuer.getValue());
226235

227236
Signature sig = processedSAMLResponse.getSignature();
237+
checkAndFailOnMissingSAMLSignature(sig);
228238
if (idpMetadata.getSigningCertificate() != null && sig != null) {
229239
BasicX509Credential credential = new BasicX509Credential();
230240
credential.setEntityCertificate(idpMetadata.getSigningCertificate());
@@ -238,9 +248,8 @@ public String authenticate(final String command, final Map<String, Object[]> par
238248
params, responseType));
239249
}
240250
}
241-
if (username == null) {
242-
username = SAMLUtils.getValueFromAssertions(processedSAMLResponse.getAssertions(), SAML2AuthManager.SAMLUserAttributeName.value());
243-
}
251+
252+
username = SAMLUtils.getValueFromAssertions(processedSAMLResponse.getAssertions(), SAML2AuthManager.SAMLUserAttributeName.value());
244253

245254
for (Assertion assertion: processedSAMLResponse.getAssertions()) {
246255
if (assertion!= null && assertion.getSubject() != null && assertion.getSubject().getNameID() != null) {
@@ -272,6 +281,7 @@ public String authenticate(final String command, final Map<String, Object[]> par
272281
continue;
273282
}
274283
Signature encSig = assertion.getSignature();
284+
checkAndFailOnMissingSAMLSignature(encSig);
275285
if (idpMetadata.getSigningCertificate() != null && encSig != null) {
276286
BasicX509Credential sigCredential = new BasicX509Credential();
277287
sigCredential.setEntityCertificate(idpMetadata.getSigningCertificate());

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

Lines changed: 27 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -25,59 +25,63 @@
2525

2626
public interface SAML2AuthManager extends PluggableAPIAuthenticator, PluggableService {
2727

28-
public static final ConfigKey<Boolean> SAMLIsPluginEnabled = new ConfigKey<Boolean>("Advanced", Boolean.class, "saml2.enabled", "false",
28+
ConfigKey<Boolean> SAMLIsPluginEnabled = new ConfigKey<Boolean>("Advanced", Boolean.class, "saml2.enabled", "false",
2929
"Indicates whether SAML SSO plugin is enabled or not", true);
3030

31-
public static final ConfigKey<String> SAMLServiceProviderID = new ConfigKey<String>("Advanced", String.class, "saml2.sp.id", "org.apache.cloudstack",
31+
ConfigKey<String> SAMLServiceProviderID = new ConfigKey<String>("Advanced", String.class, "saml2.sp.id", "org.apache.cloudstack",
3232
"SAML2 Service Provider Identifier String", true);
3333

34-
public static final ConfigKey<String> SAMLServiceProviderContactPersonName = new ConfigKey<String>("Advanced", String.class, "saml2.sp.contact.person", "CloudStack Developers",
34+
ConfigKey<String> SAMLServiceProviderContactPersonName = new ConfigKey<String>("Advanced", String.class, "saml2.sp.contact.person", "CloudStack Developers",
3535
"SAML2 Service Provider Contact Person Name", true);
3636

37-
public static final ConfigKey<String> SAMLServiceProviderContactEmail = new ConfigKey<String>("Advanced", String.class, "saml2.sp.contact.email", "dev@cloudstack.apache.org",
37+
ConfigKey<String> SAMLServiceProviderContactEmail = new ConfigKey<String>("Advanced", String.class, "saml2.sp.contact.email", "dev@cloudstack.apache.org",
3838
"SAML2 Service Provider Contact Email Address", true);
3939

40-
public static final ConfigKey<String> SAMLServiceProviderOrgName = new ConfigKey<String>("Advanced", String.class, "saml2.sp.org.name", "Apache CloudStack",
40+
ConfigKey<String> SAMLServiceProviderOrgName = new ConfigKey<String>("Advanced", String.class, "saml2.sp.org.name", "Apache CloudStack",
4141
"SAML2 Service Provider Organization Name", true);
4242

43-
public static final ConfigKey<String> SAMLServiceProviderOrgUrl = new ConfigKey<String>("Advanced", String.class, "saml2.sp.org.url", "http://cloudstack.apache.org",
43+
ConfigKey<String> SAMLServiceProviderOrgUrl = new ConfigKey<String>("Advanced", String.class, "saml2.sp.org.url", "http://cloudstack.apache.org",
4444
"SAML2 Service Provider Organization URL", true);
4545

46-
public static final ConfigKey<String> SAMLServiceProviderSingleSignOnURL = new ConfigKey<String>("Advanced", String.class, "saml2.sp.sso.url", "http://localhost:8080/client/api?command=samlSso",
46+
ConfigKey<String> SAMLServiceProviderSingleSignOnURL = new ConfigKey<String>("Advanced", String.class, "saml2.sp.sso.url", "http://localhost:8080/client/api?command=samlSso",
4747
"SAML2 CloudStack Service Provider Single Sign On URL", true);
4848

49-
public static final ConfigKey<String> SAMLServiceProviderSingleLogOutURL = new ConfigKey<String>("Advanced", String.class, "saml2.sp.slo.url", "http://localhost:8080/client/",
49+
ConfigKey<String> SAMLServiceProviderSingleLogOutURL = new ConfigKey<String>("Advanced", String.class, "saml2.sp.slo.url", "http://localhost:8080/client/",
5050
"SAML2 CloudStack Service Provider Single Log Out URL", true);
5151

52-
public static final ConfigKey<String> SAMLCloudStackRedirectionUrl = new ConfigKey<String>("Advanced", String.class, "saml2.redirect.url", "http://localhost:8080/client",
52+
ConfigKey<String> SAMLCloudStackRedirectionUrl = new ConfigKey<String>("Advanced", String.class, "saml2.redirect.url", "http://localhost:8080/client",
5353
"The CloudStack UI url the SSO should redirected to when successful", true);
5454

55-
public static final ConfigKey<String> SAMLUserAttributeName = new ConfigKey<String>("Advanced", String.class, "saml2.user.attribute", "uid",
55+
ConfigKey<String> SAMLUserAttributeName = new ConfigKey<String>("Advanced", String.class, "saml2.user.attribute", "uid",
5656
"Attribute name to be looked for in SAML response that will contain the username", true);
5757

58-
public static final ConfigKey<String> SAMLIdentityProviderMetadataURL = new ConfigKey<String>("Advanced", String.class, "saml2.idp.metadata.url", "https://openidp.feide.no/simplesaml/saml2/idp/metadata.php",
58+
ConfigKey<String> SAMLIdentityProviderMetadataURL = new ConfigKey<String>("Advanced", String.class, "saml2.idp.metadata.url", "https://openidp.feide.no/simplesaml/saml2/idp/metadata.php",
5959
"SAML2 Identity Provider Metadata XML Url", true);
6060

61-
public static final ConfigKey<String> SAMLDefaultIdentityProviderId = new ConfigKey<String>("Advanced", String.class, "saml2.default.idpid", "https://openidp.feide.no",
61+
ConfigKey<String> SAMLDefaultIdentityProviderId = new ConfigKey<String>("Advanced", String.class, "saml2.default.idpid", "https://openidp.feide.no",
6262
"The default IdP entity ID to use only in case of multiple IdPs", true);
6363

64-
public static final ConfigKey<String> SAMLSignatureAlgorithm = new ConfigKey<>(String.class, "saml2.sigalg", "Advanced", "SHA1",
64+
ConfigKey<String> SAMLSignatureAlgorithm = new ConfigKey<>(String.class, "saml2.sigalg", "Advanced", "SHA1",
6565
"The algorithm to use to when signing a SAML request. Default is SHA1, allowed algorithms: SHA1, SHA256, SHA384, SHA512", true, ConfigKey.Scope.Global, null, null, null, null, null, ConfigKey.Kind.Select, "SHA1,SHA256,SHA384,SHA512");
6666

67-
public static final ConfigKey<Boolean> SAMLAppendDomainSuffix = new ConfigKey<Boolean>("Advanced", Boolean.class, "saml2.append.idpdomain", "false",
67+
ConfigKey<Boolean> SAMLAppendDomainSuffix = new ConfigKey<Boolean>("Advanced", Boolean.class, "saml2.append.idpdomain", "false",
6868
"If enabled, create account/user dialog with SAML SSO enabled will append the IdP domain to the user or account name in the UI dialog", true);
6969

70-
public static final ConfigKey<Integer> SAMLTimeout = new ConfigKey<Integer>("Advanced", Integer.class, "saml2.timeout", "1800",
70+
ConfigKey<Integer> SAMLTimeout = new ConfigKey<Integer>("Advanced", Integer.class, "saml2.timeout", "1800",
7171
"SAML2 IDP Metadata refresh interval in seconds, minimum value is set to 300", true);
7272

73-
public SAMLProviderMetadata getSPMetadata();
74-
public SAMLProviderMetadata getIdPMetadata(String entityId);
75-
public Collection<SAMLProviderMetadata> getAllIdPMetadata();
73+
ConfigKey<Boolean> SAMLCheckSignature = new ConfigKey<Boolean>("Advanced", Boolean.class, "saml2.check.signature", "true",
74+
"When enabled (default and recommended), SAML2 signature checks are enforced and lack of signature in the SAML SSO response will cause login exception. Disabling this is not advisable but provided for backward compatibility for users who are able to accept the risks.", false);
7675

77-
public boolean isUserAuthorized(Long userId, String entityId);
78-
public boolean authorizeUser(Long userId, String entityId, boolean enable);
76+
SAMLProviderMetadata getSPMetadata();
77+
SAMLProviderMetadata getIdPMetadata(String entityId);
78+
Collection<SAMLProviderMetadata> getAllIdPMetadata();
7979

80-
public void saveToken(String authnId, String domain, String entity);
81-
public SAMLTokenVO getToken(String authnId);
82-
public void expireTokens();
80+
boolean isUserAuthorized(Long userId, String entityId);
81+
boolean authorizeUser(Long userId, String entityId, boolean enable);
82+
83+
void saveToken(String authnId, String domain, String entity);
84+
SAMLTokenVO getToken(String authnId);
85+
void purgeToken(SAMLTokenVO token);
86+
void expireTokens();
8387
}

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

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -487,6 +487,13 @@ public SAMLTokenVO getToken(String authnId) {
487487
return _samlTokenDao.findByUuid(authnId);
488488
}
489489

490+
@Override
491+
public void purgeToken(SAMLTokenVO token) {
492+
if (token != null) {
493+
_samlTokenDao.remove(token.getId());
494+
}
495+
}
496+
490497
@Override
491498
public void expireTokens() {
492499
_samlTokenDao.expireTokens();
@@ -535,6 +542,6 @@ public ConfigKey<?>[] getConfigKeys() {
535542
SAMLServiceProviderSingleSignOnURL, SAMLServiceProviderSingleLogOutURL,
536543
SAMLCloudStackRedirectionUrl, SAMLUserAttributeName,
537544
SAMLIdentityProviderMetadataURL, SAMLDefaultIdentityProviderId,
538-
SAMLSignatureAlgorithm, SAMLAppendDomainSuffix, SAMLTimeout};
545+
SAMLSignatureAlgorithm, SAMLAppendDomainSuffix, SAMLTimeout, SAMLCheckSignature};
539546
}
540547
}

‎plugins/user-authenticators/saml2/src/test/java/org/apache/cloudstack/api/command/SAML2LoginAPIAuthenticatorCmdTest.java‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -271,6 +271,30 @@ public void whenFailToAuthenticateThrowExceptionOrRedirectToUrlTestSaml2FailedLo
271271
verifyTestWhenFailToAuthenticateThrowExceptionOrRedirectToUrl(false, hasThrownServerApiException, 0, 0);
272272
}
273273

274+
private void overrideDefaultConfigValue(final ConfigKey configKey, final String name, final Object o) throws IllegalAccessException, NoSuchFieldException {
275+
Field f = ConfigKey.class.getDeclaredField(name);
276+
f.setAccessible(true);
277+
f.set(configKey, o);
278+
}
279+
280+
@Test
281+
public void testFailOnSAMLSignatureCheckWhenFalse() throws NoSuchFieldException, IllegalAccessException {
282+
overrideDefaultConfigValue(SAML2AuthManager.SAMLCheckSignature, "_value", false);
283+
SAML2LoginAPIAuthenticatorCmd cmd = new SAML2LoginAPIAuthenticatorCmd();
284+
try {
285+
cmd.checkAndFailOnMissingSAMLSignature(null);
286+
} catch(Exception e) {
287+
Assert.fail("This shouldn't throw any exception");
288+
}
289+
}
290+
291+
@Test(expected = ServerApiException.class)
292+
public void testFailOnSAMLSignatureCheckWhenTrue() throws NoSuchFieldException, IllegalAccessException {
293+
overrideDefaultConfigValue(SAML2AuthManager.SAMLCheckSignature, "_value", true);
294+
SAML2LoginAPIAuthenticatorCmd cmd = new SAML2LoginAPIAuthenticatorCmd();
295+
cmd.checkAndFailOnMissingSAMLSignature(null);
296+
}
297+
274298
private UserAccountVO configureTestWhenFailToAuthenticateThrowExceptionOrRedirectToUrl(String entity, String configurationValue, Boolean isUserAuthorized)
275299
throws IOException {
276300
Mockito.when(samlAuthManager.isUserAuthorized(nullable(Long.class), nullable(String.class))).thenReturn(isUserAuthorized);

0 commit comments

Comments
 (0)