Merge pull request #2831 from Abyss777/permissions_polishing

Permissions improvements

7 files changed, 62 insertions, 16 deletions

diff --git a/schema/changelog-3.10.xml b/schema/changelog-3.10.xml
index 137b3bc82..c4d9b7b18 100644
--- a/schema/changelog-3.10.xml
+++ b/schema/changelog-3.10.xml
@@ -64,5 +64,15 @@
     <addForeignKeyConstraint baseTableName="user_user" baseColumnNames="userid" constraintName="fk_user_user_userid" referencedTableName="users" referencedColumnNames="id" onDelete="CASCADE" />
     <addForeignKeyConstraint baseTableName="user_user" baseColumnNames="manageduserid" constraintName="fk_user_user_manageduserid" referencedTableName="users" referencedColumnNames="id" onDelete="CASCADE" />
 
+    <update tableName="users">
+        <column name="devicelimit" valueNumeric="-1" />
+        <where>devicelimit = 0</where>
+    </update>
+    <addDefaultValue tableName="users" columnName="devicelimit" defaultValueNumeric="-1" />
+
+    <addColumn tableName="users">
+      <column name="devicereadonly" type="BOOLEAN" defaultValueBoolean="false" />
+    </addColumn>
+
   </changeSet>
 </databaseChangeLog>
diff --git a/setup/default.xml b/setup/default.xml
index 945b348dc..dbd57fc99 100644
--- a/setup/default.xml
+++ b/setup/default.xml
@@ -64,8 +64,8 @@
     </entry>
 
     <entry key='database.insertUser'>
-        INSERT INTO users (name, email, hashedPassword, salt, readonly, admin, map, distanceUnit, speedUnit, latitude, longitude, zoom, twelveHourFormat, coordinateFormat, disabled, expirationTime, deviceLimit, userLimit, token, attributes)
-        VALUES (:name, :email, :hashedPassword, :salt, :readonly, :admin, :map, :distanceUnit, :speedUnit, :latitude, :longitude, :zoom, :twelveHourFormat, :coordinateFormat, :disabled, :expirationTime, :deviceLimit, :userLimit, :token, :attributes)
+        INSERT INTO users (name, email, hashedPassword, salt, readonly, admin, map, distanceUnit, speedUnit, latitude, longitude, zoom, twelveHourFormat, coordinateFormat, disabled, expirationTime, deviceLimit, userLimit, deviceReadonly, token, attributes)
+        VALUES (:name, :email, :hashedPassword, :salt, :readonly, :admin, :map, :distanceUnit, :speedUnit, :latitude, :longitude, :zoom, :twelveHourFormat, :coordinateFormat, :disabled, :expirationTime, :deviceLimit, :userLimit, :deviceReadonly, :token, :attributes)
     </entry>
 
     <entry key='database.updateUser'>
@@ -86,6 +86,7 @@
         expirationTime = :expirationTime,
         deviceLimit = :deviceLimit,
         userLimit = :userLimit,
+        deviceReadonly = :deviceReadonly,
         token = :token,
         attributes = :attributes
         WHERE id = :id
diff --git a/src/org/traccar/api/resource/DevicePermissionResource.java b/src/org/traccar/api/resource/DevicePermissionResource.java
index af38676b0..6e00dc47f 100644
--- a/src/org/traccar/api/resource/DevicePermissionResource.java
+++ b/src/org/traccar/api/resource/DevicePermissionResource.java
@@ -38,9 +38,6 @@ public class DevicePermissionResource extends BaseResource {
         Context.getPermissionsManager().checkReadonly(getUserId());
         Context.getPermissionsManager().checkUser(getUserId(), entity.getUserId());
         Context.getPermissionsManager().checkDevice(getUserId(), entity.getDeviceId());
-        if (!Context.getPermissionsManager().isAdmin(getUserId())) {
-            Context.getPermissionsManager().checkDeviceLimit(entity.getUserId());
-        }
         Context.getDataManager().linkDevice(entity.getUserId(), entity.getDeviceId());
         Context.getPermissionsManager().refreshPermissions();
         if (Context.getGeofenceManager() != null) {
@@ -52,7 +49,11 @@ public class DevicePermissionResource extends BaseResource {
     @DELETE
     public Response remove(DevicePermission entity) throws SQLException {
         Context.getPermissionsManager().checkReadonly(getUserId());
-        Context.getPermissionsManager().checkUser(getUserId(), entity.getUserId());
+        if (getUserId() != entity.getUserId()) {
+            Context.getPermissionsManager().checkUser(getUserId(), entity.getUserId());
+        } else {
+            Context.getPermissionsManager().checkAdmin(getUserId());
+        }
         Context.getPermissionsManager().checkDevice(getUserId(), entity.getDeviceId());
         Context.getDataManager().unlinkDevice(entity.getUserId(), entity.getDeviceId());
         Context.getPermissionsManager().refreshPermissions();
diff --git a/src/org/traccar/api/resource/DeviceResource.java b/src/org/traccar/api/resource/DeviceResource.java
index c9680ac77..e0c2335f9 100644
--- a/src/org/traccar/api/resource/DeviceResource.java
+++ b/src/org/traccar/api/resource/DeviceResource.java
@@ -62,6 +62,7 @@ public class DeviceResource extends BaseResource {
     @POST
     public Response add(Device entity) throws SQLException {
         Context.getPermissionsManager().checkReadonly(getUserId());
+        Context.getPermissionsManager().checkDeviceReadonly(getUserId());
         Context.getPermissionsManager().checkDeviceLimit(getUserId());
         Context.getDeviceManager().addDevice(entity);
         Context.getDataManager().linkDevice(getUserId(), entity.getId());
@@ -76,6 +77,7 @@ public class DeviceResource extends BaseResource {
     @PUT
     public Response update(Device entity) throws SQLException {
         Context.getPermissionsManager().checkReadonly(getUserId());
+        Context.getPermissionsManager().checkDeviceReadonly(getUserId());
         Context.getPermissionsManager().checkDevice(getUserId(), entity.getId());
         Context.getDeviceManager().updateDevice(entity);
         if (Context.getGeofenceManager() != null) {
@@ -88,6 +90,7 @@ public class DeviceResource extends BaseResource {
     @DELETE
     public Response remove(@PathParam("id") long id) throws SQLException {
         Context.getPermissionsManager().checkReadonly(getUserId());
+        Context.getPermissionsManager().checkDeviceReadonly(getUserId());
         Context.getPermissionsManager().checkDevice(getUserId(), id);
         Context.getDeviceManager().removeDevice(id);
         Context.getPermissionsManager().refreshPermissions();
diff --git a/src/org/traccar/api/resource/UserResource.java b/src/org/traccar/api/resource/UserResource.java
index dd59a11ee..4d8a8b3a4 100644
--- a/src/org/traccar/api/resource/UserResource.java
+++ b/src/org/traccar/api/resource/UserResource.java
@@ -64,7 +64,7 @@ public class UserResource extends BaseResource {
                 Context.getPermissionsManager().checkUserLimit(getUserId());
             } else {
                 Context.getPermissionsManager().checkRegistration(getUserId());
-                entity.setDeviceLimit(Context.getConfig().getInteger("users.defaultDeviceLimit"));
+                entity.setDeviceLimit(Context.getConfig().getInteger("users.defaultDeviceLimit", -1));
                 int expirationDays = Context.getConfig().getInteger("users.defaultExpirationDays");
                 if (expirationDays > 0) {
                     entity.setExpirationTime(
@@ -86,6 +86,7 @@ public class UserResource extends BaseResource {
     @Path("{id}")
     @PUT
     public Response update(User entity) throws SQLException {
+        Context.getPermissionsManager().checkReadonly(getUserId());
         User before = Context.getPermissionsManager().getUser(entity.getId());
         Context.getPermissionsManager().checkUser(getUserId(), entity.getId());
         Context.getPermissionsManager().checkUserUpdate(getUserId(), before, entity);
@@ -99,6 +100,7 @@ public class UserResource extends BaseResource {
     @Path("{id}")
     @DELETE
     public Response remove(@PathParam("id") long id) throws SQLException {
+        Context.getPermissionsManager().checkReadonly(getUserId());
         Context.getPermissionsManager().checkUser(getUserId(), id);
         Context.getPermissionsManager().removeUser(id);
         if (Context.getGeofenceManager() != null) {
diff --git a/src/org/traccar/database/PermissionsManager.java b/src/org/traccar/database/PermissionsManager.java
index 4a5f759a8..14cc8027a 100644
--- a/src/org/traccar/database/PermissionsManager.java
+++ b/src/org/traccar/database/PermissionsManager.java
@@ -31,7 +31,6 @@ import java.util.Collection;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Map;
-import java.util.Objects;
 import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
 
@@ -170,7 +169,7 @@ public class PermissionsManager {
     }
 
     public boolean isManager(long userId) {
-        return users.containsKey(userId) && users.get(userId).getUserLimit() > 0;
+        return users.containsKey(userId) && users.get(userId).getUserLimit() != 0;
     }
 
     public void checkManager(long userId) throws SecurityException {
@@ -187,15 +186,21 @@ public class PermissionsManager {
     }
 
     public void checkUserLimit(long userId) throws SecurityException {
-        if (!isAdmin(userId) && userPermissions.get(userId).size() >= users.get(userId).getUserLimit()) {
+        int userLimit = users.get(userId).getUserLimit();
+        if (userLimit != -1 && userPermissions.get(userId).size() >= userLimit) {
             throw new SecurityException("Manager user limit reached");
         }
     }
 
-    public void checkDeviceLimit(long userId) throws SecurityException {
+    public void checkDeviceLimit(long userId) throws SecurityException, SQLException {
         int deviceLimit = users.get(userId).getDeviceLimit();
-        if (deviceLimit != 0) {
-            int deviceCount = getDevicePermissions(userId).size();
+        if (deviceLimit != -1) {
+            int deviceCount = 0;
+            if (isManager(userId)) {
+                deviceCount = Context.getDeviceManager().getManagedDevices(userId).size();
+            } else {
+                deviceCount = getDevicePermissions(userId).size();
+            }
             if (deviceCount >= deviceLimit) {
                 throw new SecurityException("User device limit reached");
             }
@@ -206,12 +211,22 @@ public class PermissionsManager {
         return users.containsKey(userId) && users.get(userId).getReadonly();
     }
 
+    public boolean isDeviceReadonly(long userId) {
+        return users.containsKey(userId) && users.get(userId).getDeviceReadonly();
+    }
+
     public void checkReadonly(long userId) throws SecurityException {
         if (!isAdmin(userId) && (server.getReadonly() || isReadonly(userId))) {
             throw new SecurityException("Account is readonly");
         }
     }
 
+    public void checkDeviceReadonly(long userId) throws SecurityException {
+        if (!isAdmin(userId) && isDeviceReadonly(userId)) {
+            throw new SecurityException("Account is device readonly");
+        }
+    }
+
     public void checkUserEnabled(long userId) throws SecurityException {
         User user = getUser(userId);
         if (user.getDisabled()) {
@@ -228,10 +243,14 @@ public class PermissionsManager {
                 || before.getUserLimit() != after.getUserLimit()) {
             checkAdmin(userId);
         }
+        if (users.containsKey(userId) && users.get(userId).getExpirationTime() != null
+                && (after.getExpirationTime() == null
+                || users.get(userId).getExpirationTime().compareTo(after.getExpirationTime()) < 0)) {
+            checkAdmin(userId);
+        }
         if (before.getReadonly() != after.getReadonly()
-                || before.getDisabled() != after.getDisabled()
-                || !Objects.equals(before.getExpirationTime(), after.getExpirationTime())
-                || !Objects.equals(before.getToken(), after.getToken())) {
+                || before.getDeviceReadonly() != after.getDeviceReadonly()
+                || before.getDisabled() != after.getDisabled()) {
             if (userId == after.getId()) {
                 checkAdmin(userId);
             }
diff --git a/src/org/traccar/model/User.java b/src/org/traccar/model/User.java
index 72fe2c9f9..a89a75dd8 100644
--- a/src/org/traccar/model/User.java
+++ b/src/org/traccar/model/User.java
@@ -190,6 +190,16 @@ public class User extends Extensible {
         this.userLimit = userLimit;
     }
 
+    private boolean deviceReadonly;
+
+    public boolean getDeviceReadonly() {
+        return deviceReadonly;
+    }
+
+    public void setDeviceReadonly(boolean deviceReadonly) {
+        this.deviceReadonly = deviceReadonly;
+    }
+
     private String token;
 
     public String getToken() {