thingsboard-memoizeit

Improve Access control interfaces.

12/8/2018 7:48:29 AM

Details

diff --git a/application/src/main/java/org/thingsboard/server/controller/AdminController.java b/application/src/main/java/org/thingsboard/server/controller/AdminController.java
index b7b6f0c..72bd077 100644
--- a/application/src/main/java/org/thingsboard/server/controller/AdminController.java
+++ b/application/src/main/java/org/thingsboard/server/controller/AdminController.java
@@ -51,7 +51,7 @@ public class AdminController extends BaseController {
     @ResponseBody
     public AdminSettings getAdminSettings(@PathVariable("key") String key) throws ThingsboardException {
         try {
-            accessControlService.checkPermission(getCurrentUser(), TenantId.SYS_TENANT_ID, Resource.ADMIN_SETTINGS, Operation.READ);
+            accessControlService.checkPermission(getCurrentUser(), Resource.ADMIN_SETTINGS, Operation.READ);
             return checkNotNull(adminSettingsService.findAdminSettingsByKey(TenantId.SYS_TENANT_ID, key));
         } catch (Exception e) {
             throw handleException(e);
@@ -63,7 +63,7 @@ public class AdminController extends BaseController {
     @ResponseBody 
     public AdminSettings saveAdminSettings(@RequestBody AdminSettings adminSettings) throws ThingsboardException {
         try {
-            accessControlService.checkPermission(getCurrentUser(), TenantId.SYS_TENANT_ID, Resource.ADMIN_SETTINGS, Operation.WRITE);
+            accessControlService.checkPermission(getCurrentUser(), Resource.ADMIN_SETTINGS, Operation.WRITE);
             adminSettings = checkNotNull(adminSettingsService.saveAdminSettings(TenantId.SYS_TENANT_ID, adminSettings));
             if (adminSettings.getKey().equals("mail")) {
                 mailService.updateMailConfiguration();
@@ -78,7 +78,7 @@ public class AdminController extends BaseController {
     @RequestMapping(value = "/settings/testMail", method = RequestMethod.POST)
     public void sendTestMail(@RequestBody AdminSettings adminSettings) throws ThingsboardException {
         try {
-            accessControlService.checkPermission(getCurrentUser(), TenantId.SYS_TENANT_ID, Resource.ADMIN_SETTINGS, Operation.READ);
+            accessControlService.checkPermission(getCurrentUser(), Resource.ADMIN_SETTINGS, Operation.READ);
             adminSettings = checkNotNull(adminSettings);
             if (adminSettings.getKey().equals("mail")) {
                String email = getCurrentUser().getEmail();
diff --git a/application/src/main/java/org/thingsboard/server/controller/BaseController.java b/application/src/main/java/org/thingsboard/server/controller/BaseController.java
index 702fdd9..7dbfc38 100644
--- a/application/src/main/java/org/thingsboard/server/controller/BaseController.java
+++ b/application/src/main/java/org/thingsboard/server/controller/BaseController.java
@@ -259,9 +259,16 @@ public abstract class BaseController {
         }
     }
 
-    void checkTenantId(TenantId tenantId, Operation operation) throws ThingsboardException {
-        validateId(tenantId, INCORRECT_TENANT_ID + tenantId);
-        accessControlService.checkPermission(getCurrentUser(), tenantId, Resource.TENANT, operation, tenantId);
+    Tenant checkTenantId(TenantId tenantId, Operation operation) throws ThingsboardException {
+        try {
+            validateId(tenantId, INCORRECT_TENANT_ID + tenantId);
+            Tenant tenant = tenantService.findTenantById(tenantId);
+            checkNotNull(tenant);
+            accessControlService.checkPermission(getCurrentUser(), Resource.TENANT, operation, tenantId, tenant);
+            return tenant;
+        } catch (Exception e) {
+            throw handleException(e, false);
+        }
     }
 
     protected TenantId getTenantId() throws ThingsboardException {
@@ -270,16 +277,11 @@ public abstract class BaseController {
 
     Customer checkCustomerId(CustomerId customerId, Operation operation) throws ThingsboardException {
         try {
-            accessControlService.checkPermission(getCurrentUser(), getCurrentUser().getTenantId(), Resource.CUSTOMER, operation, customerId);
-
-            if (customerId != null && !customerId.isNullUid()) {
-                Customer customer = customerService.findCustomerById(getTenantId(), customerId);
-                checkNotNull(customer);
-                accessControlService.checkPermission(getCurrentUser(), Resource.CUSTOMER, operation, customerId, customer);
-                return customer;
-            } else {
-                return null;
-            }
+            validateId(customerId, "Incorrect customerId " + customerId);
+            Customer customer = customerService.findCustomerById(getTenantId(), customerId);
+            checkNotNull(customer);
+            accessControlService.checkPermission(getCurrentUser(), Resource.CUSTOMER, operation, customerId, customer);
+            return customer;
         } catch (Exception e) {
             throw handleException(e, false);
         }
diff --git a/application/src/main/java/org/thingsboard/server/controller/TenantController.java b/application/src/main/java/org/thingsboard/server/controller/TenantController.java
index a48bc65..1431b66 100644
--- a/application/src/main/java/org/thingsboard/server/controller/TenantController.java
+++ b/application/src/main/java/org/thingsboard/server/controller/TenantController.java
@@ -72,8 +72,8 @@ public class TenantController extends BaseController {
 
             Operation operation = newTenant ? Operation.CREATE : Operation.WRITE;
 
-            accessControlService.checkPermission(getCurrentUser(), tenant.getId(), Resource.TENANT, operation,
-                    tenant.getId());
+            accessControlService.checkPermission(getCurrentUser(), Resource.TENANT, operation,
+                    tenant.getId(), tenant);
 
             tenant = checkNotNull(tenantService.saveTenant(tenant));
             if (newTenant) {
diff --git a/application/src/main/java/org/thingsboard/server/service/security/AccessValidator.java b/application/src/main/java/org/thingsboard/server/service/security/AccessValidator.java
index 11c59df..e61b5b5 100644
--- a/application/src/main/java/org/thingsboard/server/service/security/AccessValidator.java
+++ b/application/src/main/java/org/thingsboard/server/service/security/AccessValidator.java
@@ -275,7 +275,7 @@ public class AccessValidator {
                     return ValidationResult.entityNotFound("Customer with requested id wasn't found!");
                 } else {
                     try {
-                        accessControlService.checkPermission(currentUser, customer.getTenantId(), Resource.CUSTOMER, operation, entityId);
+                        accessControlService.checkPermission(currentUser, Resource.CUSTOMER, operation, entityId, customer);
                     } catch (ThingsboardException e) {
                         return ValidationResult.accessDenied(e.getMessage());
                     }
@@ -297,7 +297,7 @@ public class AccessValidator {
                     return ValidationResult.entityNotFound("Tenant with requested id wasn't found!");
                 }
                 try {
-                    accessControlService.checkPermission(currentUser, new TenantId(entityId.getId()), Resource.TENANT, operation, entityId);
+                    accessControlService.checkPermission(currentUser, Resource.TENANT, operation, entityId, tenant);
                 } catch (ThingsboardException e) {
                     return ValidationResult.accessDenied(e.getMessage());
                 }
diff --git a/application/src/main/java/org/thingsboard/server/service/security/permission/AccessControlService.java b/application/src/main/java/org/thingsboard/server/service/security/permission/AccessControlService.java
index ccb62ec..219c57a 100644
--- a/application/src/main/java/org/thingsboard/server/service/security/permission/AccessControlService.java
+++ b/application/src/main/java/org/thingsboard/server/service/security/permission/AccessControlService.java
@@ -24,10 +24,8 @@ import org.thingsboard.server.service.security.model.SecurityUser;
 
 public interface AccessControlService {
 
-    void checkPermission(SecurityUser user, TenantId tenantId, Resource resource, Operation operation) throws ThingsboardException;
+    void checkPermission(SecurityUser user, Resource resource, Operation operation) throws ThingsboardException;
 
-    void checkPermission(SecurityUser user, TenantId tenantId, Resource resource, Operation operation, EntityId entityId) throws ThingsboardException;
-
-    <T extends HasTenantId, I extends EntityId> void checkPermission(SecurityUser user, Resource resource, Operation operation, I entityId, T entity) throws ThingsboardException;
+    <I extends EntityId, T extends HasTenantId> void checkPermission(SecurityUser user, Resource resource, Operation operation, I entityId, T entity) throws ThingsboardException;
 
 }
diff --git a/application/src/main/java/org/thingsboard/server/service/security/permission/CustomerUserPremissions.java b/application/src/main/java/org/thingsboard/server/service/security/permission/CustomerUserPremissions.java
index 355862b..d6b5d97 100644
--- a/application/src/main/java/org/thingsboard/server/service/security/permission/CustomerUserPremissions.java
+++ b/application/src/main/java/org/thingsboard/server/service/security/permission/CustomerUserPremissions.java
@@ -42,7 +42,7 @@ public class CustomerUserPremissions extends AbstractPermissions {
         put(Resource.WIDGET_TYPE, widgetsPermissionChecker);
     }
 
-    public static final PermissionChecker customerEntityPermissionChecker =
+    private static final PermissionChecker customerEntityPermissionChecker =
             new PermissionChecker.GenericPermissionChecker(Operation.READ, Operation.READ_ATTRIBUTES, Operation.READ_TELEMETRY) {
 
         @Override
@@ -68,8 +68,8 @@ public class CustomerUserPremissions extends AbstractPermissions {
             new PermissionChecker.GenericPermissionChecker(Operation.READ, Operation.READ_ATTRIBUTES, Operation.READ_TELEMETRY) {
 
                 @Override
-                public boolean hasPermission(SecurityUser user, TenantId tenantId, Operation operation, EntityId entityId) {
-                    if (!super.hasPermission(user, tenantId, operation, entityId)) {
+                public boolean hasPermission(SecurityUser user, Operation operation, EntityId entityId, HasTenantId entity) {
+                    if (!super.hasPermission(user, operation, entityId, entity)) {
                         return false;
                     }
                     if (!user.getCustomerId().equals(entityId)) {
@@ -81,7 +81,7 @@ public class CustomerUserPremissions extends AbstractPermissions {
             };
 
     private static final PermissionChecker customerDashboardPermissionChecker =
-            new PermissionChecker.GenericPermissionChecker<DashboardInfo, DashboardId>(Operation.READ, Operation.READ_ATTRIBUTES, Operation.READ_TELEMETRY) {
+            new PermissionChecker.GenericPermissionChecker<DashboardId, DashboardInfo>(Operation.READ, Operation.READ_ATTRIBUTES, Operation.READ_TELEMETRY) {
 
                 @Override
                 public boolean hasPermission(SecurityUser user, Operation operation, DashboardId dashboardId, DashboardInfo dashboard) {
@@ -100,7 +100,7 @@ public class CustomerUserPremissions extends AbstractPermissions {
 
             };
 
-    private static final PermissionChecker userPermissionChecker = new PermissionChecker<User, UserId>() {
+    private static final PermissionChecker userPermissionChecker = new PermissionChecker<UserId, User>() {
 
         @Override
         public boolean hasPermission(SecurityUser user, Operation operation, UserId userId, User userEntity) {
@@ -122,9 +122,12 @@ public class CustomerUserPremissions extends AbstractPermissions {
             if (!super.hasPermission(user, operation, entityId, entity)) {
                 return false;
             }
-            if (entity.getTenantId() == null || entity.getTenantId().isNullUid() || user.getTenantId().equals(entity.getTenantId())) {
+            if (entity.getTenantId() == null || entity.getTenantId().isNullUid()) {
                 return true;
             }
+            if (!user.getTenantId().equals(entity.getTenantId())) {
+                return false;
+            }
             return true;
         }
 
diff --git a/application/src/main/java/org/thingsboard/server/service/security/permission/DefaultAccessControlService.java b/application/src/main/java/org/thingsboard/server/service/security/permission/DefaultAccessControlService.java
index 751d375..73266ef 100644
--- a/application/src/main/java/org/thingsboard/server/service/security/permission/DefaultAccessControlService.java
+++ b/application/src/main/java/org/thingsboard/server/service/security/permission/DefaultAccessControlService.java
@@ -56,23 +56,15 @@ public class DefaultAccessControlService implements AccessControlService {
     }
 
     @Override
-    public void checkPermission(SecurityUser user, TenantId tenantId, Resource resource, Operation operation) throws ThingsboardException {
+    public void checkPermission(SecurityUser user, Resource resource, Operation operation) throws ThingsboardException {
         PermissionChecker permissionChecker = getPermissionChecker(user.getAuthority(), resource);
-        if (!permissionChecker.hasPermission(user, tenantId, operation)) {
+        if (!permissionChecker.hasPermission(user, operation)) {
             permissionDenied();
         }
     }
 
     @Override
-    public void checkPermission(SecurityUser user, TenantId tenantId, Resource resource, Operation operation, EntityId entityId) throws ThingsboardException {
-        PermissionChecker permissionChecker = getPermissionChecker(user.getAuthority(), resource);
-        if (!permissionChecker.hasPermission(user, tenantId, operation, entityId)) {
-            permissionDenied();
-        }
-    }
-
-    @Override
-    public <T extends HasTenantId, I extends EntityId> void checkPermission(SecurityUser user, Resource resource,
+    public <I extends EntityId, T extends HasTenantId> void checkPermission(SecurityUser user, Resource resource,
                                                                                             Operation operation, I entityId, T entity) throws ThingsboardException {
         PermissionChecker permissionChecker = getPermissionChecker(user.getAuthority(), resource);
         if (!permissionChecker.hasPermission(user, operation, entityId, entity)) {
diff --git a/application/src/main/java/org/thingsboard/server/service/security/permission/PermissionChecker.java b/application/src/main/java/org/thingsboard/server/service/security/permission/PermissionChecker.java
index 3b14890..d8b5ed3 100644
--- a/application/src/main/java/org/thingsboard/server/service/security/permission/PermissionChecker.java
+++ b/application/src/main/java/org/thingsboard/server/service/security/permission/PermissionChecker.java
@@ -25,13 +25,9 @@ import java.util.Arrays;
 import java.util.HashSet;
 import java.util.Set;
 
-public interface PermissionChecker<T extends HasTenantId, I extends EntityId> {
+public interface PermissionChecker<I extends EntityId, T extends HasTenantId> {
 
-    default boolean hasPermission(SecurityUser user, TenantId tenantId, Operation operation) {
-        return false;
-    }
-
-    default boolean hasPermission(SecurityUser user, TenantId tenantId, Operation operation, EntityId entityId) {
+    default boolean hasPermission(SecurityUser user, Operation operation) {
         return false;
     }
 
@@ -39,7 +35,7 @@ public interface PermissionChecker<T extends HasTenantId, I extends EntityId> {
         return false;
     }
 
-    public class GenericPermissionChecker<T extends HasTenantId, I extends EntityId> implements PermissionChecker<T, I> {
+    public class GenericPermissionChecker<I extends EntityId, T extends HasTenantId> implements PermissionChecker<I,T> {
 
         private final Set<Operation> allowedOperations;
 
@@ -48,12 +44,7 @@ public interface PermissionChecker<T extends HasTenantId, I extends EntityId> {
         }
 
         @Override
-        public boolean hasPermission(SecurityUser user, TenantId tenantId, Operation operation) {
-            return allowedOperations.contains(Operation.ALL) || allowedOperations.contains(operation);
-        }
-
-        @Override
-        public boolean hasPermission(SecurityUser user, TenantId tenantId, Operation operation, EntityId entityId) {
+        public boolean hasPermission(SecurityUser user, Operation operation) {
             return allowedOperations.contains(Operation.ALL) || allowedOperations.contains(operation);
         }
 
@@ -65,15 +56,10 @@ public interface PermissionChecker<T extends HasTenantId, I extends EntityId> {
 
     public static PermissionChecker denyAllPermissionChecker = new PermissionChecker() {};
 
-    public static PermissionChecker allowAllPermissionChecker = new PermissionChecker<HasTenantId, EntityId>() {
-
-        @Override
-        public boolean hasPermission(SecurityUser user, TenantId tenantId, Operation operation) {
-            return true;
-        }
+    public static PermissionChecker allowAllPermissionChecker = new PermissionChecker<EntityId, HasTenantId>() {
 
         @Override
-        public boolean hasPermission(SecurityUser user, TenantId tenantId, Operation operation, EntityId entityId) {
+        public boolean hasPermission(SecurityUser user, Operation operation) {
             return true;
         }
 
diff --git a/application/src/main/java/org/thingsboard/server/service/security/permission/SysAdminPermissions.java b/application/src/main/java/org/thingsboard/server/service/security/permission/SysAdminPermissions.java
index de391c2..86ca173 100644
--- a/application/src/main/java/org/thingsboard/server/service/security/permission/SysAdminPermissions.java
+++ b/application/src/main/java/org/thingsboard/server/service/security/permission/SysAdminPermissions.java
@@ -41,15 +41,7 @@ public class SysAdminPermissions extends AbstractPermissions {
         put(Resource.WIDGET_TYPE, systemEntityPermissionChecker);
     }
 
-    private static final PermissionChecker systemEntityPermissionChecker = new PermissionChecker<HasTenantId, EntityId>() {
-
-        @Override
-        public boolean hasPermission(SecurityUser user, TenantId tenantId, Operation operation, EntityId entityId) {
-            if (tenantId != null && !tenantId.isNullUid()) {
-                return false;
-            }
-            return true;
-        }
+    private static final PermissionChecker systemEntityPermissionChecker = new PermissionChecker() {
 
         @Override
         public boolean hasPermission(SecurityUser user, Operation operation, EntityId entityId, HasTenantId entity) {
@@ -61,7 +53,7 @@ public class SysAdminPermissions extends AbstractPermissions {
         }
     };
 
-    private static final PermissionChecker userPermissionChecker = new PermissionChecker<User, UserId>() {
+    private static final PermissionChecker userPermissionChecker = new PermissionChecker<UserId, User>() {
 
         @Override
         public boolean hasPermission(SecurityUser user, Operation operation, UserId userId, User userEntity) {
diff --git a/application/src/main/java/org/thingsboard/server/service/security/permission/TenantAdminPermissions.java b/application/src/main/java/org/thingsboard/server/service/security/permission/TenantAdminPermissions.java
index 2ce48f6..bab009f 100644
--- a/application/src/main/java/org/thingsboard/server/service/security/permission/TenantAdminPermissions.java
+++ b/application/src/main/java/org/thingsboard/server/service/security/permission/TenantAdminPermissions.java
@@ -44,15 +44,7 @@ public class TenantAdminPermissions extends AbstractPermissions {
         put(Resource.WIDGET_TYPE, widgetsPermissionChecker);
     }
 
-    public static final PermissionChecker tenantEntityPermissionChecker = new PermissionChecker<HasTenantId, EntityId>() {
-
-        @Override
-        public boolean hasPermission(SecurityUser user, TenantId tenantId, Operation operation, EntityId entityId) {
-            if (!user.getTenantId().equals(tenantId)) {
-                return false;
-            }
-            return true;
-        }
+    public static final PermissionChecker tenantEntityPermissionChecker = new PermissionChecker() {
 
         @Override
         public boolean hasPermission(SecurityUser user, Operation operation, EntityId entityId, HasTenantId entity) {
@@ -64,12 +56,12 @@ public class TenantAdminPermissions extends AbstractPermissions {
         }
     };
 
-    public static final PermissionChecker tenantPermissionChecker =
-            new PermissionChecker.GenericPermissionChecker(Operation.READ) {
+    private static final PermissionChecker tenantPermissionChecker =
+            new PermissionChecker.GenericPermissionChecker(Operation.READ, Operation.READ_ATTRIBUTES, Operation.READ_TELEMETRY) {
 
                 @Override
-                public boolean hasPermission(SecurityUser user, TenantId tenantId, Operation operation, EntityId entityId) {
-                    if (!super.hasPermission(user, tenantId, operation, entityId)) {
+                public boolean hasPermission(SecurityUser user, Operation operation, EntityId entityId, HasTenantId entity) {
+                    if (!super.hasPermission(user, operation, entityId, entity)) {
                         return false;
                     }
                     if (!user.getTenantId().equals(entityId)) {
@@ -80,7 +72,7 @@ public class TenantAdminPermissions extends AbstractPermissions {
 
             };
 
-    private static final PermissionChecker userPermissionChecker = new PermissionChecker<User, UserId>() {
+    private static final PermissionChecker userPermissionChecker = new PermissionChecker<UserId, User>() {
 
         @Override
         public boolean hasPermission(SecurityUser user, Operation operation, UserId userId, User userEntity) {
@@ -95,7 +87,7 @@ public class TenantAdminPermissions extends AbstractPermissions {
 
     };
 
-    private static final PermissionChecker widgetsPermissionChecker = new PermissionChecker<HasTenantId, EntityId>() {
+    private static final PermissionChecker widgetsPermissionChecker = new PermissionChecker() {
 
         @Override
         public boolean hasPermission(SecurityUser user, Operation operation, EntityId entityId, HasTenantId entity) {
diff --git a/common/data/src/main/java/org/thingsboard/server/common/data/Tenant.java b/common/data/src/main/java/org/thingsboard/server/common/data/Tenant.java
index 7f41741..0b29a0b 100644
--- a/common/data/src/main/java/org/thingsboard/server/common/data/Tenant.java
+++ b/common/data/src/main/java/org/thingsboard/server/common/data/Tenant.java
@@ -15,6 +15,7 @@
  */
 package org.thingsboard.server.common.data;
 
+import com.fasterxml.jackson.annotation.JsonIgnore;
 import com.fasterxml.jackson.annotation.JsonProperty;
 import lombok.EqualsAndHashCode;
 import org.thingsboard.server.common.data.id.TenantId;
@@ -22,7 +23,7 @@ import org.thingsboard.server.common.data.id.TenantId;
 import com.fasterxml.jackson.databind.JsonNode;
 
 @EqualsAndHashCode(callSuper = true)
-public class Tenant extends ContactBased<TenantId> implements HasName {
+public class Tenant extends ContactBased<TenantId> implements HasName, HasTenantId {
 
     private static final long serialVersionUID = 8057243243859922101L;
     
@@ -52,6 +53,12 @@ public class Tenant extends ContactBased<TenantId> implements HasName {
     }
 
     @Override
+    @JsonIgnore
+    public TenantId getTenantId() {
+        return getId();
+    }
+
+    @Override
     @JsonProperty(access = JsonProperty.Access.READ_ONLY)
     public String getName() {
         return title;