From add77636d9866f4364401104e27507a599e7728d Mon Sep 17 00:00:00 2001 From: Manoj Kumar Date: Wed, 25 Feb 2026 11:38:38 +0530 Subject: [PATCH] acl related changes, fixes, mistakes --- .../command/user/dns/DeleteDnsServerCmd.java | 5 +- .../command/user/dns/ListDnsRecordsCmd.java | 2 - .../command/user/dns/ListDnsServersCmd.java | 2 - .../api/command/user/dns/ListDnsZonesCmd.java | 3 - .../command/user/dns/UpdateDnsServerCmd.java | 9 +- .../cloudstack/dns/DnsProviderManager.java | 6 +- .../org/apache/cloudstack/dns/DnsServer.java | 2 +- ...UtilTest.java => DnsProviderUtilTest.java} | 18 +- .../java/com/cloud/acl/DomainChecker.java | 12 +- .../network/firewall/FirewallManagerImpl.java | 2 +- .../com/cloud/user/AccountManagerImpl.java | 4 +- .../dns/DnsProviderManagerImpl.java | 174 +++++++++++++----- .../{DnsUtil.java => DnsProviderUtil.java} | 3 +- .../cloudstack/dns/dao/DnsServerDao.java | 2 + .../cloudstack/dns/dao/DnsServerDaoImpl.java | 16 ++ .../apache/cloudstack/dns/dao/DnsZoneDao.java | 4 +- .../cloudstack/dns/dao/DnsZoneDaoImpl.java | 29 ++- .../apache/cloudstack/dns/vo/DnsServerVO.java | 12 +- 18 files changed, 217 insertions(+), 88 deletions(-) rename plugins/dns/powerdns/src/test/java/org/apache/cloudstack/dns/{DnsUtilTest.java => DnsProviderUtilTest.java} (89%) rename server/src/main/java/org/apache/cloudstack/dns/{DnsUtil.java => DnsProviderUtil.java} (97%) diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/dns/DeleteDnsServerCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/dns/DeleteDnsServerCmd.java index 6cc86312f24..2fb8a9903e4 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/dns/DeleteDnsServerCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/dns/DeleteDnsServerCmd.java @@ -80,13 +80,10 @@ public class DeleteDnsServerCmd extends BaseAsyncCmd { @Override public long getEntityOwnerId() { - // Look up the server to find its owner. - // This allows the Framework to check: Is Caller == Owner? - DnsServer server = dnsProviderManager.getDnsServer(id); + DnsServer server = _entityMgr.findById(DnsServer.class, id); if (server != null) { return server.getAccountId(); } - // If server not found, return System to fail safely (or let manager handle 404) return Account.ACCOUNT_ID_SYSTEM; } diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/dns/ListDnsRecordsCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/dns/ListDnsRecordsCmd.java index 3bfa4af18db..e1c6b6b42e7 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/dns/ListDnsRecordsCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/dns/ListDnsRecordsCmd.java @@ -18,7 +18,6 @@ package org.apache.cloudstack.api.command.user.dns; import org.apache.cloudstack.acl.RoleType; -import org.apache.cloudstack.api.ACL; import org.apache.cloudstack.api.APICommand; import org.apache.cloudstack.api.ApiConstants; import org.apache.cloudstack.api.BaseListCmd; @@ -37,7 +36,6 @@ import org.apache.cloudstack.dns.DnsRecord; authorized = {RoleType.Admin, RoleType.ResourceAdmin, RoleType.DomainAdmin, RoleType.User}) public class ListDnsRecordsCmd extends BaseListCmd { - @ACL @Parameter(name = ApiConstants.DNS_ZONE_ID, type = CommandType.UUID, entityType = DnsZoneResponse.class, required = true, description = "ID of the DNS zone to list records from") private Long dnsZoneId; diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/dns/ListDnsServersCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/dns/ListDnsServersCmd.java index fa301bf2851..39cb24a3ec3 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/dns/ListDnsServersCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/dns/ListDnsServersCmd.java @@ -18,7 +18,6 @@ package org.apache.cloudstack.api.command.user.dns; import org.apache.cloudstack.acl.RoleType; -import org.apache.cloudstack.api.ACL; import org.apache.cloudstack.api.APICommand; import org.apache.cloudstack.api.ApiConstants; import org.apache.cloudstack.api.BaseListAccountResourcesCmd; @@ -40,7 +39,6 @@ public class ListDnsServersCmd extends BaseListAccountResourcesCmd { //////////////// API Parameters ///////////////////// ///////////////////////////////////////////////////// - @ACL @Parameter(name = ApiConstants.ID, type = CommandType.UUID, entityType = DnsServerResponse.class, description = "the ID of the DNS server") private Long id; diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/dns/ListDnsZonesCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/dns/ListDnsZonesCmd.java index f848c3525d2..7d6f17a28f0 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/dns/ListDnsZonesCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/dns/ListDnsZonesCmd.java @@ -18,7 +18,6 @@ package org.apache.cloudstack.api.command.user.dns; import org.apache.cloudstack.acl.RoleType; -import org.apache.cloudstack.api.ACL; import org.apache.cloudstack.api.APICommand; import org.apache.cloudstack.api.ApiConstants; import org.apache.cloudstack.api.BaseListAccountResourcesCmd; @@ -40,12 +39,10 @@ public class ListDnsZonesCmd extends BaseListAccountResourcesCmd { //////////////// API parameters ///////////////////// ///////////////////////////////////////////////////// - @ACL @Parameter(name = ApiConstants.ID, type = CommandType.UUID, entityType = DnsZoneResponse.class, description = "List DNS zone by ID") private Long id; - @ACL @Parameter(name = "dnsserverid", type = CommandType.UUID, entityType = DnsServerResponse.class, description = "List DNS zones belonging to a specific DNS server") private Long dnsServerId; diff --git a/api/src/main/java/org/apache/cloudstack/api/command/user/dns/UpdateDnsServerCmd.java b/api/src/main/java/org/apache/cloudstack/api/command/user/dns/UpdateDnsServerCmd.java index 4077f7f1b91..52ba0497e74 100644 --- a/api/src/main/java/org/apache/cloudstack/api/command/user/dns/UpdateDnsServerCmd.java +++ b/api/src/main/java/org/apache/cloudstack/api/command/user/dns/UpdateDnsServerCmd.java @@ -27,11 +27,11 @@ import org.apache.cloudstack.api.BaseCmd; import org.apache.cloudstack.api.Parameter; import org.apache.cloudstack.api.ServerApiException; import org.apache.cloudstack.api.response.DnsServerResponse; -import org.apache.cloudstack.context.CallContext; import org.apache.cloudstack.dns.DnsServer; import org.apache.commons.lang3.BooleanUtils; import org.apache.commons.lang3.StringUtils; +import com.cloud.user.Account; import com.cloud.utils.EnumUtils; @APICommand(name = "updateDnsServer", @@ -99,7 +99,12 @@ public class UpdateDnsServerCmd extends BaseCmd { @Override public long getEntityOwnerId() { - return CallContext.current().getCallingAccount().getId(); + DnsServer server = _entityMgr.findById(DnsServer.class, id); + if (server != null) { + return server.getAccountId(); + } + // If server not found, return System to fail safely (or let manager handle 404) + return Account.ACCOUNT_ID_SYSTEM; } @Override diff --git a/api/src/main/java/org/apache/cloudstack/dns/DnsProviderManager.java b/api/src/main/java/org/apache/cloudstack/dns/DnsProviderManager.java index 2adb1d8f3c5..72a793328af 100644 --- a/api/src/main/java/org/apache/cloudstack/dns/DnsProviderManager.java +++ b/api/src/main/java/org/apache/cloudstack/dns/DnsProviderManager.java @@ -39,6 +39,7 @@ import org.apache.cloudstack.api.response.DnsZoneNetworkMapResponse; import org.apache.cloudstack.api.response.DnsZoneResponse; import org.apache.cloudstack.api.response.ListResponse; +import com.cloud.user.Account; import com.cloud.utils.component.Manager; import com.cloud.utils.component.PluggableService; @@ -50,8 +51,6 @@ public interface DnsProviderManager extends Manager, PluggableService { boolean deleteDnsServer(DeleteDnsServerCmd cmd); DnsServerResponse createDnsServerResponse(DnsServer server); - DnsServer getDnsServer(Long id); - // Allocates the DB row (State: Inactive) DnsZone allocateDnsZone(CreateDnsZoneCmd cmd); // Calls the Plugin (State: Inactive -> Active) @@ -78,4 +77,7 @@ public interface DnsProviderManager extends Manager, PluggableService { boolean registerDnsRecordForVm(RegisterDnsRecordForVmCmd cmd); boolean removeDnsRecordForVm(RemoveDnsRecordForVmCmd cmd); + + void checkDnsServerPermissions(Account caller, DnsServer server); + void checkDnsZonePermission(Account caller, DnsZone zone); } diff --git a/api/src/main/java/org/apache/cloudstack/dns/DnsServer.java b/api/src/main/java/org/apache/cloudstack/dns/DnsServer.java index 49f7ddea5b4..0261902ef98 100644 --- a/api/src/main/java/org/apache/cloudstack/dns/DnsServer.java +++ b/api/src/main/java/org/apache/cloudstack/dns/DnsServer.java @@ -41,7 +41,7 @@ public interface DnsServer extends InternalIdentity, Identity, ControlledEntity long getAccountId(); - boolean isPublic(); + boolean isPublicServer(); Date getCreated(); diff --git a/plugins/dns/powerdns/src/test/java/org/apache/cloudstack/dns/DnsUtilTest.java b/plugins/dns/powerdns/src/test/java/org/apache/cloudstack/dns/DnsProviderUtilTest.java similarity index 89% rename from plugins/dns/powerdns/src/test/java/org/apache/cloudstack/dns/DnsUtilTest.java rename to plugins/dns/powerdns/src/test/java/org/apache/cloudstack/dns/DnsProviderUtilTest.java index bc155e8d982..25d8f158372 100644 --- a/plugins/dns/powerdns/src/test/java/org/apache/cloudstack/dns/DnsUtilTest.java +++ b/plugins/dns/powerdns/src/test/java/org/apache/cloudstack/dns/DnsProviderUtilTest.java @@ -17,8 +17,8 @@ package org.apache.cloudstack.dns; -import static org.apache.cloudstack.dns.DnsUtil.appendPublicSuffixToZone; -import static org.apache.cloudstack.dns.DnsUtil.normalizeDomain; +import static org.apache.cloudstack.dns.DnsProviderUtil.appendPublicSuffixToZone; +import static org.apache.cloudstack.dns.DnsProviderUtil.normalizeDomain; import static org.junit.Assert.assertEquals; import static org.junit.Assert.fail; @@ -31,16 +31,16 @@ import org.junit.runner.RunWith; import org.junit.runners.Parameterized; @RunWith(Parameterized.class) -public class DnsUtilTest { +public class DnsProviderUtilTest { private final String userZoneName; private final String publicSuffix; private final String expectedResult; private final boolean expectException; - public DnsUtilTest(String userZoneName, - String publicSuffix, - String expectedResult, - boolean expectException) { + public DnsProviderUtilTest(String userZoneName, + String publicSuffix, + String expectedResult, + boolean expectException) { this.userZoneName = userZoneName; this.publicSuffix = publicSuffix; this.expectedResult = expectedResult; @@ -96,8 +96,6 @@ public class DnsUtilTest { } String executeAppendSuffixTest(String zoneName, String domainSuffix) { - return appendPublicSuffixToZone( - normalizeDomain(zoneName), - normalizeDomain(domainSuffix)); + return appendPublicSuffixToZone(normalizeDomain(zoneName), domainSuffix); } } diff --git a/server/src/main/java/com/cloud/acl/DomainChecker.java b/server/src/main/java/com/cloud/acl/DomainChecker.java index 0500960abb1..ba4f5e53031 100644 --- a/server/src/main/java/com/cloud/acl/DomainChecker.java +++ b/server/src/main/java/com/cloud/acl/DomainChecker.java @@ -28,6 +28,9 @@ import org.apache.cloudstack.acl.RolePermissionEntity; import org.apache.cloudstack.acl.SecurityChecker; import org.apache.cloudstack.affinity.AffinityGroup; import org.apache.cloudstack.context.CallContext; +import org.apache.cloudstack.dns.DnsProviderManager; +import org.apache.cloudstack.dns.DnsServer; +import org.apache.cloudstack.dns.DnsZone; import org.apache.cloudstack.query.QueryService; import org.apache.cloudstack.resourcedetail.dao.DiskOfferingDetailsDao; import org.springframework.stereotype.Component; @@ -101,6 +104,8 @@ public class DomainChecker extends AdapterBase implements SecurityChecker { private ProjectDao projectDao; @Inject private AccountService accountService; + @Inject + private DnsProviderManager dnsProviderManager; protected DomainChecker() { super(); @@ -216,7 +221,12 @@ public class DomainChecker extends AdapterBase implements SecurityChecker { _networkMgr.checkRouterPermissions(caller, (VirtualRouter)entity); } else if (entity instanceof AffinityGroup) { return false; - } else { + } else if (entity instanceof DnsServer) { + dnsProviderManager.checkDnsServerPermissions(caller, (DnsServer) entity); + } else if (entity instanceof DnsZone) { + dnsProviderManager.checkDnsZonePermission(caller, (DnsZone) entity); + } + else { validateCallerHasAccessToEntityOwner(caller, entity, accessType); } return true; diff --git a/server/src/main/java/com/cloud/network/firewall/FirewallManagerImpl.java b/server/src/main/java/com/cloud/network/firewall/FirewallManagerImpl.java index 00863c28dd2..32a7a972129 100644 --- a/server/src/main/java/com/cloud/network/firewall/FirewallManagerImpl.java +++ b/server/src/main/java/com/cloud/network/firewall/FirewallManagerImpl.java @@ -311,7 +311,7 @@ public class FirewallManagerImpl extends ManagerBase implements FirewallService, sb.and("id", sb.entity().getId(), Op.EQ); sb.and("trafficType", sb.entity().getTrafficType(), Op.EQ); - sb.and("networkId", sb.entity().getNetworkId(), Op.EQ); + sb.and("networkId", sb.entity().getNetworkId(), Op.EQ); sb.and("ip", sb.entity().getSourceIpAddressId(), Op.EQ); sb.and("purpose", sb.entity().getPurpose(), Op.EQ); sb.and("display", sb.entity().isDisplay(), Op.EQ); diff --git a/server/src/main/java/com/cloud/user/AccountManagerImpl.java b/server/src/main/java/com/cloud/user/AccountManagerImpl.java index 53b88690654..e7395b69131 100644 --- a/server/src/main/java/com/cloud/user/AccountManagerImpl.java +++ b/server/src/main/java/com/cloud/user/AccountManagerImpl.java @@ -70,6 +70,7 @@ import org.apache.cloudstack.auth.UserTwoFactorAuthenticator; import org.apache.cloudstack.backup.BackupOffering; import org.apache.cloudstack.config.ApiServiceConfiguration; import org.apache.cloudstack.context.CallContext; +import org.apache.cloudstack.dns.DnsServer; import org.apache.cloudstack.engine.orchestration.service.NetworkOrchestrationService; import org.apache.cloudstack.framework.config.ConfigKey; import org.apache.cloudstack.framework.config.dao.ConfigurationDao; @@ -744,7 +745,8 @@ public class AccountManagerImpl extends ManagerBase implements AccountManager, M } if (entity.getAccountId() != -1 && domainId != -1 && !(entity instanceof VirtualMachineTemplate) && !(entity instanceof Network && accessType != null && (accessType == AccessType.UseEntry || accessType == AccessType.OperateEntry)) - && !(entity instanceof AffinityGroup) && !(entity instanceof VirtualRouter)) { + && !(entity instanceof AffinityGroup) && !(entity instanceof VirtualRouter) + && !(entity instanceof DnsServer)) { List toBeChecked = domains.get(entity.getDomainId()); // for templates, we don't have to do cross domains check if (toBeChecked == null) { diff --git a/server/src/main/java/org/apache/cloudstack/dns/DnsProviderManagerImpl.java b/server/src/main/java/org/apache/cloudstack/dns/DnsProviderManagerImpl.java index b48eece57aa..b79a59342df 100644 --- a/server/src/main/java/org/apache/cloudstack/dns/DnsProviderManagerImpl.java +++ b/server/src/main/java/org/apache/cloudstack/dns/DnsProviderManagerImpl.java @@ -55,17 +55,23 @@ import org.apache.cloudstack.dns.vo.DnsZoneNetworkMapVO; import org.apache.cloudstack.dns.vo.DnsZoneVO; import org.springframework.stereotype.Component; +import com.cloud.domain.DomainVO; +import com.cloud.domain.dao.DomainDao; import com.cloud.exception.InvalidParameterValueException; import com.cloud.exception.PermissionDeniedException; import com.cloud.network.dao.NetworkDao; import com.cloud.network.dao.NetworkVO; +import com.cloud.projects.Project; import com.cloud.user.Account; import com.cloud.user.AccountManager; import com.cloud.utils.Pair; import com.cloud.utils.StringUtils; +import com.cloud.utils.Ternary; import com.cloud.utils.component.ManagerBase; import com.cloud.utils.component.PluggableService; import com.cloud.utils.db.Filter; +import com.cloud.utils.db.SearchBuilder; +import com.cloud.utils.db.SearchCriteria; import com.cloud.utils.exception.CloudRuntimeException; import com.cloud.vm.NicVO; import com.cloud.vm.UserVmVO; @@ -89,6 +95,8 @@ public class DnsProviderManagerImpl extends ManagerBase implements DnsProviderMa UserVmDao userVmDao; @Inject NicDao nicDao; + @Inject + DomainDao domainDao; private DnsProvider getProvider(DnsProviderType type) { if (type == null) { @@ -120,7 +128,7 @@ public class DnsProviderManagerImpl extends ManagerBase implements DnsProviderMa } if (StringUtils.isNotBlank(publicDomainSuffix)) { - publicDomainSuffix = DnsUtil.normalizeDomain(publicDomainSuffix); + publicDomainSuffix = DnsProviderUtil.normalizeDomain(publicDomainSuffix); } DnsProviderType type = DnsProviderType.fromString(cmd.getProvider()); @@ -142,26 +150,7 @@ public class DnsProviderManagerImpl extends ManagerBase implements DnsProviderMa @Override public ListResponse listDnsServers(ListDnsServersCmd cmd) { - Account caller = CallContext.current().getCallingAccount(); - Long accountIdFilter = null; - if (accountMgr.isRootAdmin(caller.getId())) { - // Root Admin: Can see all, unless they specifically ask for an account - if (cmd.getAccountName() != null) { - Account target = accountMgr.getActiveAccountByName(cmd.getAccountName(), cmd.getDomainId()); - if (target == null) { - return new ListResponse<>(); // Account not found - } - accountIdFilter = target.getId(); - } - } else { - // Regular User / Domain Admin: STRICTLY restricted to their own account - accountIdFilter = caller.getId(); - } - - Filter searchFilter = new Filter(DnsServerVO.class, "id", true, cmd.getStartIndex(), cmd.getPageSizeVal()); - Pair, Integer> result = dnsServerDao.searchDnsServers(cmd.getId(), cmd.getKeyword(), - cmd.getProvider(), accountIdFilter, searchFilter); - + Pair, Integer> result = searchForDnsServerInternal(cmd); ListResponse response = new ListResponse<>(); List serverResponses = new ArrayList<>(); for (DnsServerVO server : result.first()) { @@ -171,6 +160,76 @@ public class DnsProviderManagerImpl extends ManagerBase implements DnsProviderMa return response; } + private Pair, Integer> searchForDnsServerInternal(ListDnsServersCmd cmd) { + Long dnsServerId = cmd.getId(); + Account caller = CallContext.current().getCallingAccount(); + Long domainId = cmd.getDomainId(); + boolean isRecursive = cmd.isRecursive(); + + // Step 1: Build ACL search parameters based on caller permissions + List permittedAccountIds = new ArrayList<>(); + Ternary domainIdRecursiveListProject = new Ternary<>( + domainId, isRecursive, null); + accountMgr.buildACLSearchParameters(caller, dnsServerId, cmd.getAccountName(), null, permittedAccountIds, + domainIdRecursiveListProject, cmd.listAll(), false); + + domainId = domainIdRecursiveListProject.first(); + isRecursive = domainIdRecursiveListProject.second(); + Project.ListProjectResourcesCriteria listProjectResourcesCriteria = domainIdRecursiveListProject.third(); + Filter searchFilter = new Filter(DnsServerVO.class, "id", true, cmd.getStartIndex(), cmd.getPageSizeVal()); + + // Step 2: Search for caller's own DNS servers using standard ACL pattern + SearchBuilder sb = dnsServerDao.createSearchBuilder(); + accountMgr.buildACLSearchBuilder(sb, domainId, isRecursive, permittedAccountIds, listProjectResourcesCriteria); + sb.and("state", sb.entity().getState(), SearchCriteria.Op.EQ); + sb.done(); + + SearchCriteria sc = sb.create(); + accountMgr.buildACLSearchCriteria(sc, domainId, isRecursive, permittedAccountIds, listProjectResourcesCriteria); + sc.setParameters("state", DnsServer.State.Enabled); + + Pair, Integer> ownServersPair = dnsServerDao.searchAndCount(sc, searchFilter); + List dnsServers = new ArrayList<>(ownServersPair.first()); + int count = ownServersPair.second(); + + // Step 3: Search for public DNS servers from caller's domain and children + // domains + Long callerDomainId = caller.getDomainId(); + DomainVO callerDomain = domainDao.findById(callerDomainId); + if (callerDomain != null) { + List domainIds = new ArrayList<>(); + domainIds.add(callerDomainId); + List childDomains = domainDao.findAllChildren(callerDomain.getPath(), callerDomainId); + for (DomainVO childDomain : childDomains) { + domainIds.add(childDomain.getId()); + } + + SearchBuilder publicSb = dnsServerDao.createSearchBuilder(); + publicSb.and("publicDns", publicSb.entity().isPublicServer(), SearchCriteria.Op.EQ); + publicSb.and("publicDomainId", publicSb.entity().getDomainId(), SearchCriteria.Op.IN); + publicSb.and("publicState", publicSb.entity().getState(), SearchCriteria.Op.EQ); + publicSb.done(); + + SearchCriteria publicSc = publicSb.create(); + publicSc.setParameters("publicDns", 1); + publicSc.setParameters("publicDomainId", domainIds.toArray()); + publicSc.setParameters("publicState", DnsServer.State.Enabled); + + List publicServers = dnsServerDao.search(publicSc, null); + + // Deduplicate: add only public servers not already in the own servers list + List ownServerIds = dnsServers.stream().map(DnsServerVO::getId).collect(Collectors.toList()); + for (DnsServerVO publicServer : publicServers) { + if (!ownServerIds.contains(publicServer.getId())) { + dnsServers.add(publicServer); + count++; + } + } + } + + return new Pair<>(dnsServers, count); + } + @Override public DnsServer updateDnsServer(UpdateDnsServerCmd cmd) { Long dnsServerId = cmd.getId(); @@ -214,7 +273,7 @@ public class DnsProviderManagerImpl extends ManagerBase implements DnsProviderMa } if (cmd.getPublicDomainSuffix() != null) { - dnsServer.setPublicDomainSuffix(DnsUtil.normalizeDomain(cmd.getPublicDomainSuffix())); + dnsServer.setPublicDomainSuffix(DnsProviderUtil.normalizeDomain(cmd.getPublicDomainSuffix())); } if (cmd.getNameServers() != null) { @@ -235,7 +294,6 @@ public class DnsProviderManagerImpl extends ManagerBase implements DnsProviderMa } boolean updateStatus = dnsServerDao.update(dnsServerId, dnsServer); - if (updateStatus) { return dnsServerDao.findById(dnsServerId); } else { @@ -263,18 +321,13 @@ public class DnsProviderManagerImpl extends ManagerBase implements DnsProviderMa response.setUrl(server.getUrl()); response.setPort(server.getPort()); response.setProvider(server.getProviderType()); - response.setPublic(server.isPublic()); + response.setPublic(server.isPublicServer()); response.setNameServers(server.getNameServers()); response.setPublicDomainSuffix(server.getPublicDomainSuffix()); response.setObjectName("dnsserver"); return response; } - @Override - public DnsServer getDnsServer(Long id) { - return null; - } - @Override public boolean deleteDnsZone(Long zoneId) { DnsZoneVO zone = dnsZoneDao.findById(zoneId); @@ -335,14 +388,8 @@ public class DnsProviderManagerImpl extends ManagerBase implements DnsProviderMa @Override public ListResponse listDnsZones(ListDnsZonesCmd cmd) { - Account caller = CallContext.current().getCallingAccount(); - Filter searchFilter = new Filter(DnsZoneVO.class, ApiConstants.ID, true, cmd.getStartIndex(), cmd.getPageSizeVal()); - Long accountIdFilter = caller.getAccountId(); - String keyword = cmd.getKeyword(); - if (cmd.getName() != null) { - keyword = cmd.getName(); - } - Pair, Integer> result = dnsZoneDao.searchZones(cmd.getId(), cmd.getDnsServerId(), keyword, accountIdFilter, searchFilter); + Pair, Integer> result = searchForDnsZonesInternal(cmd); + List zoneResponses = new ArrayList<>(); for (DnsZoneVO zone : result.first()) { zoneResponses.add(createDnsZoneResponse(zone)); @@ -352,6 +399,27 @@ public class DnsProviderManagerImpl extends ManagerBase implements DnsProviderMa return response; } + private Pair, Integer> searchForDnsZonesInternal(ListDnsZonesCmd cmd) { + if (cmd.getId() != null) { + DnsZone dnsZone = dnsZoneDao.findById(cmd.getId()); + if (dnsZone == null) { + throw new InvalidParameterValueException("DNS zone not found for the given ID"); + } + } + Account caller = CallContext.current().getCallingAccount(); + if (cmd.getDnsServerId() != null) { + DnsServer dnsServer = dnsServerDao.findById(cmd.getDnsServerId()); + accountMgr.checkAccess(caller, null, false, dnsServer); + } + List ownDnsServerIds = dnsServerDao.listDnsServerIdsByAccountId(caller.getAccountId()); + String keyword = cmd.getKeyword(); + if (cmd.getName() != null) { + keyword = cmd.getName(); + } + Filter searchFilter = new Filter(DnsZoneVO.class, ApiConstants.ID, true, cmd.getStartIndex(), cmd.getPageSizeVal()); + return dnsZoneDao.searchZones(cmd.getId(), caller.getAccountId(), ownDnsServerIds, cmd.getDnsServerId(), keyword, searchFilter); + } + @Override public DnsRecordResponse createDnsRecord(CreateDnsRecordCmd cmd) { String recordName = StringUtils.trimToEmpty(cmd.getName()).toLowerCase(); @@ -368,7 +436,7 @@ public class DnsProviderManagerImpl extends ManagerBase implements DnsProviderMa try { DnsRecord.RecordType type = cmd.getType(); List normalizedContents = cmd.getContents().stream() - .map(value -> DnsUtil.normalizeDnsRecordValue(value, type)).collect(Collectors.toList()); + .map(value -> DnsProviderUtil.normalizeDnsRecordValue(value, type)).collect(Collectors.toList()); DnsRecord record = new DnsRecord(recordName, type, normalizedContents, cmd.getTtl()); DnsProvider provider = getProvider(server.getProviderType()); String normalizedRecordName = provider.addRecord(server, zone, record); @@ -452,19 +520,18 @@ public class DnsProviderManagerImpl extends ManagerBase implements DnsProviderMa throw new InvalidParameterValueException("DNS zone name cannot be empty"); } - String dnsZoneName = DnsUtil.normalizeDomain(cmd.getName()); + String dnsZoneName = DnsProviderUtil.normalizeDomain(cmd.getName()); DnsServerVO server = dnsServerDao.findById(cmd.getDnsServerId()); if (server == null) { throw new InvalidParameterValueException(String.format("DNS server not found for the given ID: %s", cmd.getDnsServerId())); } - Account caller = CallContext.current().getCallingAccount(); boolean isOwner = (server.getAccountId() == caller.getId()); if (!isOwner) { - if (!server.isPublic()) { + if (!server.isPublicServer()) { throw new PermissionDeniedException("You do not have permission to use this DNS server."); } - dnsZoneName = DnsUtil.appendPublicSuffixToZone(dnsZoneName, DnsUtil.normalizeDomain(server.getPublicDomainSuffix())); + dnsZoneName = DnsProviderUtil.appendPublicSuffixToZone(dnsZoneName, server.getPublicDomainSuffix()); } DnsZone.ZoneType type = cmd.getType(); DnsZoneVO existing = dnsZoneDao.findByNameServerAndType(dnsZoneName, server.getId(), type); @@ -577,6 +644,27 @@ public class DnsProviderManagerImpl extends ManagerBase implements DnsProviderMa return processDnsRecordForInstance(cmd.getVmId(), cmd.getNetworkId(), false); } + @Override + public void checkDnsServerPermissions(Account caller, DnsServer server) { + if (caller.getId() == server.getAccountId()) { + return; + } + if (!server.isPublicServer()) { + throw new PermissionDeniedException(caller + "is not allowed to access the DNS server " + server.getName()); + } + Account owner = accountMgr.getAccount(server.getAccountId()); + if (!domainDao.isChildDomain(owner.getDomainId(), caller.getDomainId())) { + throw new PermissionDeniedException(caller + "is not allowed to access the DNS server " + server.getName()); + } + } + + @Override + public void checkDnsZonePermission(Account caller, DnsZone zone) { + if (caller.getId() != zone.getAccountId()) { + throw new PermissionDeniedException(caller + "is not allowed to access the DNS Zone " + zone.getName()); + } + } + /** * Helper method to handle both Register and Remove logic for Instance */ @@ -623,7 +711,7 @@ public class DnsProviderManagerImpl extends ManagerBase implements DnsProviderMa // Construct FQDN Prefix (e.g., "instance-id" or "instance-id.subdomain") String recordName = String.valueOf(instance.getInstanceName()); - if (StringUtils.isNotBlank(map.getSubDomain() )) { + if (StringUtils.isNotBlank(map.getSubDomain())) { recordName = recordName + "." + map.getSubDomain(); } diff --git a/server/src/main/java/org/apache/cloudstack/dns/DnsUtil.java b/server/src/main/java/org/apache/cloudstack/dns/DnsProviderUtil.java similarity index 97% rename from server/src/main/java/org/apache/cloudstack/dns/DnsUtil.java rename to server/src/main/java/org/apache/cloudstack/dns/DnsProviderUtil.java index dc312f55287..95f04fe0940 100644 --- a/server/src/main/java/org/apache/cloudstack/dns/DnsUtil.java +++ b/server/src/main/java/org/apache/cloudstack/dns/DnsProviderUtil.java @@ -21,13 +21,14 @@ import org.apache.commons.validator.routines.DomainValidator; import com.cloud.utils.StringUtils; -public class DnsUtil { +public class DnsProviderUtil { static DomainValidator validator = DomainValidator.getInstance(true); public static String appendPublicSuffixToZone(String zoneName, String suffixDomain) { if (StringUtils.isBlank(suffixDomain)) { return zoneName; } + suffixDomain = DnsProviderUtil.normalizeDomain(suffixDomain); // Already suffixed → return as-is if (zoneName.toLowerCase().endsWith("." + suffixDomain.toLowerCase())) { return zoneName; diff --git a/server/src/main/java/org/apache/cloudstack/dns/dao/DnsServerDao.java b/server/src/main/java/org/apache/cloudstack/dns/dao/DnsServerDao.java index d68e456a6dd..c9cd68c0513 100644 --- a/server/src/main/java/org/apache/cloudstack/dns/dao/DnsServerDao.java +++ b/server/src/main/java/org/apache/cloudstack/dns/dao/DnsServerDao.java @@ -29,5 +29,7 @@ import com.cloud.utils.db.GenericDao; public interface DnsServerDao extends GenericDao { DnsServer findByUrlAndAccount(String url, long accountId); + List listDnsServerIdsByAccountId(Long accountId); + Pair, Integer> searchDnsServers(Long id, String keyword, String provider, Long accountId, Filter filter); } diff --git a/server/src/main/java/org/apache/cloudstack/dns/dao/DnsServerDaoImpl.java b/server/src/main/java/org/apache/cloudstack/dns/dao/DnsServerDaoImpl.java index 7e4559c51fd..8a7311e2f1a 100644 --- a/server/src/main/java/org/apache/cloudstack/dns/dao/DnsServerDaoImpl.java +++ b/server/src/main/java/org/apache/cloudstack/dns/dao/DnsServerDaoImpl.java @@ -27,6 +27,7 @@ import org.springframework.stereotype.Component; import com.cloud.utils.Pair; import com.cloud.utils.db.Filter; import com.cloud.utils.db.GenericDaoBase; +import com.cloud.utils.db.GenericSearchBuilder; import com.cloud.utils.db.SearchBuilder; import com.cloud.utils.db.SearchCriteria; @@ -34,6 +35,7 @@ import com.cloud.utils.db.SearchCriteria; public class DnsServerDaoImpl extends GenericDaoBase implements DnsServerDao { SearchBuilder AllFieldsSearch; SearchBuilder AccountUrlSearch; + GenericSearchBuilder DnsServerIdsByAccountSearch; public DnsServerDaoImpl() { @@ -51,6 +53,11 @@ public class DnsServerDaoImpl extends GenericDaoBase implemen AllFieldsSearch.and(ApiConstants.ACCOUNT_ID, AllFieldsSearch.entity().getAccountId(), SearchCriteria.Op.EQ); AllFieldsSearch.done(); + DnsServerIdsByAccountSearch = createSearchBuilder(Long.class); + DnsServerIdsByAccountSearch.selectFields(DnsServerIdsByAccountSearch.entity().getId()); + DnsServerIdsByAccountSearch.and(ApiConstants.ACCOUNT_ID, DnsServerIdsByAccountSearch.entity().getAccountId(), SearchCriteria.Op.EQ); + DnsServerIdsByAccountSearch.done(); + } @Override @@ -61,6 +68,15 @@ public class DnsServerDaoImpl extends GenericDaoBase implemen return findOneBy(sc); } + @Override + public List listDnsServerIdsByAccountId(Long accountId) { + SearchCriteria sc = DnsServerIdsByAccountSearch.create(); + if (accountId != null) { + sc.setParameters(ApiConstants.ACCOUNT_ID, accountId); + } + return customSearch(sc, null); + } + @Override public Pair, Integer> searchDnsServers(Long id, String keyword, String provider, Long accountId, Filter filter) { SearchCriteria sc = AllFieldsSearch.create(); diff --git a/server/src/main/java/org/apache/cloudstack/dns/dao/DnsZoneDao.java b/server/src/main/java/org/apache/cloudstack/dns/dao/DnsZoneDao.java index 1d58b527503..05fb799a663 100644 --- a/server/src/main/java/org/apache/cloudstack/dns/dao/DnsZoneDao.java +++ b/server/src/main/java/org/apache/cloudstack/dns/dao/DnsZoneDao.java @@ -29,5 +29,7 @@ import com.cloud.utils.db.GenericDao; public interface DnsZoneDao extends GenericDao { List listByAccount(long accountId); DnsZoneVO findByNameServerAndType(String name, long dnsServerId, DnsZone.ZoneType type); - Pair, Integer> searchZones(Long id, Long dnsServerId, String keyword, Long accountId, Filter filter); + + Pair, Integer> searchZones(Long id, Long accountId, List ownDnsServerIds, Long targetDnsServerId, + String keyword, Filter filter); } diff --git a/server/src/main/java/org/apache/cloudstack/dns/dao/DnsZoneDaoImpl.java b/server/src/main/java/org/apache/cloudstack/dns/dao/DnsZoneDaoImpl.java index 2658b8e2987..9ad1217cb71 100644 --- a/server/src/main/java/org/apache/cloudstack/dns/dao/DnsZoneDaoImpl.java +++ b/server/src/main/java/org/apache/cloudstack/dns/dao/DnsZoneDaoImpl.java @@ -22,6 +22,7 @@ import java.util.List; import org.apache.cloudstack.api.ApiConstants; import org.apache.cloudstack.dns.DnsZone; import org.apache.cloudstack.dns.vo.DnsZoneVO; +import org.apache.commons.collections.CollectionUtils; import org.springframework.stereotype.Component; import com.cloud.utils.Pair; @@ -32,7 +33,6 @@ import com.cloud.utils.db.SearchCriteria; @Component public class DnsZoneDaoImpl extends GenericDaoBase implements DnsZoneDao { - static final String DNS_SERVER_ID = "dnsServerId"; SearchBuilder AccountSearch; SearchBuilder NameServerTypeSearch; SearchBuilder AllFieldsSearch; @@ -42,19 +42,24 @@ public class DnsZoneDaoImpl extends GenericDaoBase implements D AccountSearch = createSearchBuilder(); AccountSearch.and(ApiConstants.ACCOUNT_ID, AccountSearch.entity().getAccountId(), SearchCriteria.Op.EQ); + AccountSearch.and(ApiConstants.STATE, AccountSearch.entity().getState(), SearchCriteria.Op.EQ); AccountSearch.done(); NameServerTypeSearch = createSearchBuilder(); NameServerTypeSearch.and(ApiConstants.NAME, NameServerTypeSearch.entity().getName(), SearchCriteria.Op.EQ); - NameServerTypeSearch.and(DNS_SERVER_ID, NameServerTypeSearch.entity().getDnsServerId(), SearchCriteria.Op.EQ); + NameServerTypeSearch.and(ApiConstants.DNS_SERVER_ID, NameServerTypeSearch.entity().getDnsServerId(), SearchCriteria.Op.EQ); NameServerTypeSearch.and(ApiConstants.TYPE, NameServerTypeSearch.entity().getType(), SearchCriteria.Op.EQ); + NameServerTypeSearch.and(ApiConstants.STATE, NameServerTypeSearch.entity().getState(), SearchCriteria.Op.EQ); NameServerTypeSearch.done(); AllFieldsSearch = createSearchBuilder(); + AllFieldsSearch.and().op(ApiConstants.DNS_SERVER_ID, AllFieldsSearch.entity().getDnsServerId(), SearchCriteria.Op.IN); + AllFieldsSearch.or(ApiConstants.ACCOUNT_ID, AllFieldsSearch.entity().getAccountId(), SearchCriteria.Op.EQ); + AllFieldsSearch.cp(); + AllFieldsSearch.and(ApiConstants.STATE, AllFieldsSearch.entity().getState(), SearchCriteria.Op.EQ); AllFieldsSearch.and(ApiConstants.ID, AllFieldsSearch.entity().getId(), SearchCriteria.Op.EQ); - AllFieldsSearch.and(DNS_SERVER_ID, AllFieldsSearch.entity().getDnsServerId(), SearchCriteria.Op.EQ); AllFieldsSearch.and(ApiConstants.NAME, AllFieldsSearch.entity().getName(), SearchCriteria.Op.LIKE); - AllFieldsSearch.and(ApiConstants.ACCOUNT_ID, AllFieldsSearch.entity().getAccountId(), SearchCriteria.Op.EQ); + AllFieldsSearch.and(ApiConstants.TARGET_ID, AllFieldsSearch.entity().getDnsServerId(), SearchCriteria.Op.EQ); AllFieldsSearch.done(); } @@ -62,6 +67,7 @@ public class DnsZoneDaoImpl extends GenericDaoBase implements D public List listByAccount(long accountId) { SearchCriteria sc = AccountSearch.create(); sc.setParameters(ApiConstants.ACCOUNT_ID, accountId); + sc.setParameters(ApiConstants.STATE, DnsZone.State.Active); return listBy(sc); } @@ -69,19 +75,22 @@ public class DnsZoneDaoImpl extends GenericDaoBase implements D public DnsZoneVO findByNameServerAndType(String name, long dnsServerId, DnsZone.ZoneType type) { SearchCriteria sc = NameServerTypeSearch.create(); sc.setParameters(ApiConstants.NAME, name); - sc.setParameters(DNS_SERVER_ID, dnsServerId); + sc.setParameters(ApiConstants.DNS_SERVER_ID, dnsServerId); sc.setParameters(ApiConstants.TYPE, type); + sc.setParameters(ApiConstants.STATE, DnsZone.State.Active); return findOneBy(sc); } @Override - public Pair, Integer> searchZones(Long id, Long dnsServerId, String keyword, Long accountId, Filter filter) { + public Pair, Integer> searchZones(Long id, Long accountId, List ownDnsServerIds, Long targetDnsServerId, + String keyword, Filter filter) { + SearchCriteria sc = AllFieldsSearch.create(); if (id != null) { sc.setParameters(ApiConstants.ID, id); } - if (dnsServerId != null) { - sc.setParameters(DNS_SERVER_ID, dnsServerId); + if (!CollectionUtils.isEmpty(ownDnsServerIds)) { + sc.setParameters(ApiConstants.DNS_SERVER_ID, ownDnsServerIds.toArray()); } if (keyword != null) { sc.setParameters(ApiConstants.NAME, "%" + keyword + "%"); @@ -89,6 +98,10 @@ public class DnsZoneDaoImpl extends GenericDaoBase implements D if (accountId != null) { sc.setParameters(ApiConstants.ACCOUNT_ID, accountId); } + if (targetDnsServerId != null) { + sc.setParameters(ApiConstants.TARGET_ID, targetDnsServerId); + } + sc.setParameters(ApiConstants.STATE, DnsZone.State.Active); return searchAndCount(sc, filter); } } diff --git a/server/src/main/java/org/apache/cloudstack/dns/vo/DnsServerVO.java b/server/src/main/java/org/apache/cloudstack/dns/vo/DnsServerVO.java index d27b3486612..5ccc5bf91f9 100644 --- a/server/src/main/java/org/apache/cloudstack/dns/vo/DnsServerVO.java +++ b/server/src/main/java/org/apache/cloudstack/dns/vo/DnsServerVO.java @@ -44,7 +44,7 @@ import com.cloud.utils.db.GenericDao; @Entity @Table(name = "dns_server") -public class DnsServerVO implements DnsServer { +public class DnsServerVO implements DnsServer { @Id @GeneratedValue(strategy = GenerationType.IDENTITY) @Column(name = "id") @@ -77,7 +77,7 @@ public class DnsServerVO implements DnsServer { private String externalServerId; @Column(name = "is_public") - private boolean isPublic; + private boolean publicServer; @Column(name = "public_domain_suffix") private String publicDomainSuffix; @@ -121,7 +121,7 @@ public class DnsServerVO implements DnsServer { this.accountId = accountId; this.domainId = domainId; this.publicDomainSuffix = publicDomainSuffix; - this.isPublic = isPublic; + this.publicServer = isPublic; this.state = State.Enabled; this.nameServers = String.join(",", nameServers);; } @@ -176,8 +176,8 @@ public class DnsServerVO implements DnsServer { return uuid; } - public boolean isPublic() { - return isPublic; + public boolean isPublicServer() { + return publicServer; } public State getState() { @@ -203,7 +203,7 @@ public class DnsServerVO implements DnsServer { } public void setIsPublic(boolean value) { - isPublic = value; + publicServer = value; } public void setPublicDomainSuffix(String publicDomainSuffix) {