Skip to content

SSLCertService: Fix account id requirement by using caller account id as fallback #13818

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
resmo wants to merge 13 commits into
base: main
Choose a base branch
from
+90 −20
Open

SSLCertService: Fix account id requirement by using caller account id as fallback #13818

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
334dc48
CertService: Fix account requirement by using caller account fallback
resmo Aug 6, 2026
a9dc8bf
revert to deprecated closeQuietly
resmo Aug 6, 2026
8a3871f
sslcert: add account and domainid to api
resmo Aug 7, 2026
bddcf1e
undeprecate account id
resmo Aug 10, 2026
5763612
verify accountid / account are mutually exclusive
resmo Aug 10, 2026
c23976d
finalizeOwner add additional common checks
resmo Aug 10, 2026
0aed4d0
verify only one or zero of the params are given
resmo Aug 10, 2026
db72cc6
address copilot review concerns
resmo Aug 10, 2026
574b250
upload cert: ensure checks in finalizeOwner are executed
resmo Aug 10, 2026
c731613
fix NPE, finalizeOwner is nullable
resmo Aug 11, 2026
321fe2f
ensure owner not null
resmo Aug 11, 2026
d08de80
check for nullable projectId
resmo Aug 11, 2026
247b534
also take domainId into account
resmo Aug 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 16 additions & 1 deletion api/src/main/java/org/apache/cloudstack/api/command/user/loadbalancer/ListSslCertsCmd.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import org.apache.cloudstack.api.BaseCmd;
import org.apache.cloudstack.api.Parameter;
import org.apache.cloudstack.api.response.AccountResponse;
import org.apache.cloudstack.api.response.DomainResponse;
import org.apache.cloudstack.api.response.FirewallRuleResponse;
import org.apache.cloudstack.api.response.ListResponse;
import org.apache.cloudstack.api.response.ProjectResponse;
Expand All @@ -50,9 +51,15 @@ public class ListSslCertsCmd extends BaseCmd {
@Parameter(name = ApiConstants.CERTIFICATE_ID, type = CommandType.UUID, entityType = SslCertResponse.class, required = false, description = "ID of SSL certificate")
private Long certId;

@Parameter(name = ApiConstants.ACCOUNT_ID, type = CommandType.UUID, entityType = AccountResponse.class, required = false, description = "Account ID")
@Parameter(name = ApiConstants.ACCOUNT_ID, type = CommandType.UUID, entityType = AccountResponse.class, required = false, description = "Account ID and " + ApiConstants.ACCOUNT + " are mutually exclusive.")
private Long accountId;

@Parameter(name = ApiConstants.ACCOUNT, type = CommandType.STRING, since = "4.24", description = "Account owning the SSL certificate")
private String accountName;

@Parameter(name = ApiConstants.DOMAIN_ID, type = CommandType.UUID, since = "4.24", entityType = DomainResponse.class, description = "Domain ID of the account owning the SSL certificate")
private Long domainId;

@Parameter(name = ApiConstants.LBID, type = CommandType.UUID, entityType = FirewallRuleResponse.class, required = false, description = "Load balancer rule ID")
private Long lbId;

Expand All @@ -79,6 +86,14 @@ public Long getProjectId() {
return projectId;
}

public String getAccountName() {
return accountName;
}

public Long getDomainId() {
return domainId;
}

/////////////////////////////////////////////////////
/////////////// API Implementation///////////////////
/////////////////////////////////////////////////////
Expand Down
66 changes: 47 additions & 19 deletions server/src/main/java/org/apache/cloudstack/network/ssl/CertServiceImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,18 @@ public CertServiceImpl() {
public SslCertResponse uploadSslCert(final UploadSslCertCmd certCmd) {
Preconditions.checkNotNull(certCmd);

final CallContext ctx = CallContext.current();
final Account caller = ctx.getCallingAccount();

Account owner;
if ((StringUtils.isNotBlank(certCmd.getAccountName()) && certCmd.getDomainId() != null) || certCmd.getProjectId() != null) {
owner = _accountMgr.finalizeOwner(caller, certCmd.getAccountName(), certCmd.getDomainId(), certCmd.getProjectId());
} else {
owner = caller;
}

Preconditions.checkNotNull(owner);

final String cert = certCmd.getCert();
final String key = certCmd.getKey();
final String password = certCmd.getPassword();
Expand All @@ -138,17 +150,6 @@ public SslCertResponse uploadSslCert(final UploadSslCertCmd certCmd) {
logger.debug("Certificate Validation succeeded");

final String fingerPrint = CertificateHelper.generateFingerPrint(parseCertificate(cert));

final CallContext ctx = CallContext.current();
final Account caller = ctx.getCallingAccount();

Account owner = null;
if (StringUtils.isNotEmpty(certCmd.getAccountName()) && certCmd.getDomainId() != null || certCmd.getProjectId() != null) {
owner = _accountMgr.finalizeOwner(caller, certCmd.getAccountName(), certCmd.getDomainId(), certCmd.getProjectId());
} else {
owner = caller;
}

final Long accountId = owner.getId();
final Long domainId = owner.getDomainId();

Expand Down Expand Up @@ -199,16 +200,44 @@ public List<SslCertResponse> listSslCerts(final ListSslCertsCmd listSslCertCmd)
final Account caller = ctx.getCallingAccount();

final Long certId = listSslCertCmd.getCertId();
final Long accountId = listSslCertCmd.getAccountId();
final Long lbRuleId = listSslCertCmd.getLbId();
final Long projectId = listSslCertCmd.getProjectId();
final Long accountId = listSslCertCmd.getAccountId();
final String accountName = listSslCertCmd.getAccountName();
final Long domainId = listSslCertCmd.getDomainId();
Comment thread
resmo marked this conversation as resolved.

final List<SslCertResponse> certResponseList = new ArrayList<SslCertResponse>();
if (accountId != null && (StringUtils.isNotBlank(accountName) || domainId != null)) {
throw new InvalidParameterValueException("The accountid and account/domainid are mutually exclusive");
}

if (certId == null && accountId == null && lbRuleId == null && projectId == null) {
throw new InvalidParameterValueException("Invalid parameters either certificate ID or Account ID or Loadbalancer ID or Project ID required");
// Validate that only one of certid, lbid, projectid, or accountid/account can be specified
ArrayList<Object> params = new ArrayList<>();
params.add(certId);
params.add(accountId != null ? accountId : accountName);
params.add(lbRuleId);
params.add(projectId);
Comment thread
resmo marked this conversation as resolved.

int nonNullIds = 0;
for (Object param : params) {
if (param != null) {
nonNullIds++;
}
}
if (nonNullIds > 1) {
throw new InvalidParameterValueException("Only one of certid, lbid, projectid, or accountid/account can be specified");
}

Account owner;
if ((StringUtils.isNotBlank(accountName) && domainId != null) || projectId != null) {
owner = _accountMgr.finalizeOwner(caller, accountName, domainId, projectId);
} else {
owner = caller;
}
Comment thread
resmo marked this conversation as resolved.

Preconditions.checkNotNull(owner);

final List<SslCertResponse> certResponseList = new ArrayList<SslCertResponse>();

List<LoadBalancerCertMapVO> certLbMap = null;
SslCertVO certVO = null;

Expand Down Expand Up @@ -241,7 +270,7 @@ public List<SslCertResponse> listSslCerts(final ListSslCertsCmd listSslCertCmd)
lbCertMapRule = _lbCertDao.findByLbRuleId(lbRuleId);

if (lbCertMapRule == null) {
logger.debug("No certificate bound to loadbalancer id: " + lbRuleId);
logger.debug("No certificate bound to loadbalancer id: {}", lbRuleId);
return certResponseList;
}

Expand Down Expand Up @@ -273,8 +302,7 @@ public List<SslCertResponse> listSslCerts(final ListSslCertsCmd listSslCertCmd)
return certResponseList;
}

//reached here look by accountId
final List<SslCertVO> certVOList = _sslCertDao.listByAccountId(accountId);
final List<SslCertVO> certVOList = _sslCertDao.listByAccountId(accountId != null ? accountId : owner.getId());
if (certVOList == null || certVOList.isEmpty()) {
return certResponseList;
}
Expand Down Expand Up @@ -374,7 +402,7 @@ private void validateKeys(final PublicKey pubKey, final PrivateKey privKey) {
}

// No encryption for DSA
if (pubKey.getAlgorithm() != "RSA") {
if (!pubKey.getAlgorithm().equals("RSA")) {
return;
}

Expand Down
27 changes: 27 additions & 0 deletions server/src/test/java/org/apache/cloudstack/network/ssl/CertServiceTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import com.cloud.utils.db.EntityManager;
import com.cloud.utils.db.TransactionLegacy;
import org.apache.cloudstack.api.command.user.loadbalancer.DeleteSslCertCmd;
import org.apache.cloudstack.api.command.user.loadbalancer.ListSslCertsCmd;
import org.apache.cloudstack.api.command.user.loadbalancer.UploadSslCertCmd;
import org.apache.cloudstack.context.CallContext;
import org.bouncycastle.openssl.PKCS8Generator;
Expand Down Expand Up @@ -819,6 +820,25 @@ public void runDeleteSslCertInvalidId() throws NoSuchFieldException, IllegalAcce

}

@Test
public void runListSslCertsUsesCallerAccountWhenNoFilters() {
final long callerAccountId = 42L;
final CertServiceImpl certService = new CertServiceImpl();

certService._sslCertDao = Mockito.mock(SslCertDao.class);
when(certService._sslCertDao.listByAccountId(anyLong())).thenReturn(new ArrayList<>());

final AccountVO callerAccount = new AccountVO("testaccount", 1, "networkdomain", Account.Type.NORMAL, UUID.randomUUID().toString());
callerAccount.setId(callerAccountId);
final UserVO user = new UserVO(1, "testuser", "password", "firstname", "lastName", "email", "timezone", UUID.randomUUID().toString(), User.Source.UNKNOWN);
CallContext.unregister();
CallContext.register(user, callerAccount);

certService.listSslCerts(new ListSslCertsCmdExtn());

Mockito.verify(certService._sslCertDao).listByAccountId(callerAccountId);
}

public class UploadSslCertCmdExtn extends UploadSslCertCmd {
@Override
public long getEntityOwnerId() {
Expand All @@ -833,6 +853,13 @@ public long getEntityOwnerId() {
}
}

public class ListSslCertsCmdExtn extends ListSslCertsCmd {
@Override
public long getEntityOwnerId() {
return 1;
}
}

private String generateEncryptedPrivateKey(String password) throws NoSuchAlgorithmException, OperatorCreationException, IOException {
// Generate RSA key pair
KeyPairGenerator kpg = KeyPairGenerator.getInstance("RSA");
Expand Down
Loading