acl related changes, fixes, mistakes

Author: sudo87

api/src/main/java/org/apache/cloudstack/api/command/user/dns/DeleteDnsServerCmd.java

@@ -80,13 +80,10 @@ public void execute() {
 
     @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;
     }

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 @@
         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;

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;

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;

api/src/main/java/org/apache/cloudstack/api/command/user/dns/UpdateDnsServerCmd.java

@@ -27,11 +27,11 @@
 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 String getPublicDomainSuffix() {
 
     @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

api/src/main/java/org/apache/cloudstack/dns/DnsProviderManager.java

@@ -39,6 +39,7 @@
 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);
 }

api/src/main/java/org/apache/cloudstack/dns/DnsServer.java

@@ -41,7 +41,7 @@ enum State {
 
     long getAccountId();
 
-    boolean isPublic();
+    boolean isPublicServer();
 
     Date getCreated();
 

…g/apache/cloudstack/dns/DnsUtilTest.java …/cloudstack/dns/DnsProviderUtilTest.javaplugins/dns/powerdns/src/test/java/org/apache/cloudstack/dns/DnsUtilTest.java renamed to plugins/dns/powerdns/src/test/java/org/apache/cloudstack/dns/DnsProviderUtilTest.java plugins/dns/powerdns/src/test/java/org/apache/cloudstack/dns/DnsUtilTest.java renamed to 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.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 void testAppendPublicSuffix() {
     }
 
     String executeAppendSuffixTest(String zoneName, String domainSuffix) {
-        return appendPublicSuffixToZone(
-                normalizeDomain(zoneName),
-                normalizeDomain(domainSuffix));
+        return appendPublicSuffixToZone(normalizeDomain(zoneName), domainSuffix);
     }
 }

server/src/main/java/com/cloud/acl/DomainChecker.java

@@ -28,6 +28,9 @@
 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 boolean checkAccess(Account caller, ControlledEntity entity, AccessType a
             _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;

server/src/main/java/com/cloud/network/firewall/FirewallManagerImpl.java

@@ -311,7 +311,7 @@ public Pair<List<? extends FirewallRule>, Integer> listFirewallRules(IListFirewa
 
         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);