From c8642d5722bae3e819f7c5189dd4219216bf49df Mon Sep 17 00:00:00 2001 From: Rafael Zubairov Date: Mon, 5 Oct 2015 21:32:08 +0300 Subject: [PATCH 1/4] #102 forcing any network to be specified for the device created by non admin user --- .../com/devicehive/service/DeviceService.java | 43 ++++++++++++--- .../devicehive/service/DeviceServiceTest.java | 52 ++++++++++--------- 2 files changed, 64 insertions(+), 31 deletions(-) diff --git a/src/main/java/com/devicehive/service/DeviceService.java b/src/main/java/com/devicehive/service/DeviceService.java index 7813183f8..617a58a94 100644 --- a/src/main/java/com/devicehive/service/DeviceService.java +++ b/src/main/java/com/devicehive/service/DeviceService.java @@ -13,6 +13,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.security.core.context.SecurityContextHolder; import org.springframework.stereotype.Component; import org.springframework.transaction.annotation.Propagation; import org.springframework.transaction.annotation.Transactional; @@ -84,8 +85,8 @@ public DeviceNotification deviceSaveByUser(DeviceUpdate deviceUpdate, User user) { logger.debug("Device save executed for device: id {}, user: {}", deviceUpdate.getGuid(), user.getId()); Network network = networkService.createOrUpdateNetworkByUser(deviceUpdate.getNetwork(), user); - DeviceClass deviceClass = deviceClassService - .createOrUpdateDeviceClass(deviceUpdate.getDeviceClass(), equipmentSet); + network = findNetworkForAuth(network); + DeviceClass deviceClass = deviceClassService.createOrUpdateDeviceClass(deviceUpdate.getDeviceClass(), equipmentSet); Device existingDevice = genericDAO.createNamedQuery(Device.class, "Device.findByUUID", Optional.of(CacheConfig.refresh())) .setParameter("guid", deviceUpdate.getGuid().getValue()) .getResultList() @@ -143,12 +144,11 @@ public DeviceNotification deviceSaveByKey(DeviceUpdate deviceUpdate, .stream().findFirst().orElse(null); if (existingDevice != null && !accessKeyService.hasAccessToNetwork(key, existingDevice.getNetwork())) { logger.error("Access key {} has no access to device network {}", key, existingDevice.getGuid()); - throw new HiveException( - String.format(Messages.DEVICE_NOT_FOUND, deviceUpdate.getGuid().getValue()), UNAUTHORIZED.getStatusCode()); + throw new HiveException(String.format(Messages.DEVICE_NOT_FOUND, deviceUpdate.getGuid().getValue()), UNAUTHORIZED.getStatusCode()); } Network network = networkService.createOrVerifyNetworkByKey(deviceUpdate.getNetwork(), key); - DeviceClass deviceClass = deviceClassService - .createOrUpdateDeviceClass(deviceUpdate.getDeviceClass(), equipmentSet); + network = findNetworkForAuth(network); + DeviceClass deviceClass = deviceClassService.createOrUpdateDeviceClass(deviceUpdate.getDeviceClass(), equipmentSet); if (existingDevice == null) { Device device = deviceUpdate.convertTo(); device.setDeviceClass(deviceClass); @@ -435,4 +435,33 @@ private List getDeviceList(List guids, HivePrincipal principal) return query.getResultList(); } -} \ No newline at end of file + + private Network findNetworkForAuth(Network network) { + if (network == null) { + HivePrincipal principal = (HivePrincipal) SecurityContextHolder.getContext().getAuthentication().getPrincipal(); + User user = findUserFromAuth(principal); + if (user != null) { + if (!user.isAdmin()) { + Set userNetworks = userService.findUserWithNetworks(user.getId()).getNetworks(); + if (userNetworks.isEmpty()) { + throw new HiveException(Messages.NO_ACCESS_TO_NETWORK, PRECONDITION_FAILED.getStatusCode()); + } + + return userNetworks.iterator().next(); + } + } + } + return network; + } + + private User findUserFromAuth(HivePrincipal principal) { + if (principal.getUser() != null) { + return principal.getUser(); + } + if (principal.getKey() != null && principal.getKey().getUser() != null) { + return principal.getKey().getUser(); + } + return null; + } + +} diff --git a/src/test/java/com/devicehive/service/DeviceServiceTest.java b/src/test/java/com/devicehive/service/DeviceServiceTest.java index 4197fd114..d7cea45a7 100644 --- a/src/test/java/com/devicehive/service/DeviceServiceTest.java +++ b/src/test/java/com/devicehive/service/DeviceServiceTest.java @@ -4,13 +4,17 @@ import com.devicehive.auth.HivePrincipal; import com.devicehive.base.AbstractResourceTest; import com.devicehive.base.fixture.DeviceFixture; +import com.devicehive.exceptions.HiveException; import com.devicehive.model.*; import com.devicehive.model.enums.UserRole; import com.devicehive.model.updates.DeviceClassUpdate; import com.devicehive.model.updates.DeviceUpdate; import org.apache.commons.lang3.RandomStringUtils; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.ExpectedException; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.security.core.context.SecurityContextHolder; import java.net.InetAddress; import java.net.UnknownHostException; @@ -24,18 +28,16 @@ public class DeviceServiceTest extends AbstractResourceTest { + @Rule + public ExpectedException expectedException = ExpectedException.none(); @Autowired private DeviceService deviceService; - @Autowired private DeviceNotificationService deviceNotificationService; - @Autowired private UserService userService; - @Autowired private NetworkService networkService; - @Autowired private DeviceClassService deviceClassService; @@ -63,7 +65,9 @@ public void should_save_and_notify_role_device() { * using Client role. */ @Test - public void should_save_and_notify_role_client() { + public void should_throw_HiveException_when_role_client_creates_device_without_network() throws Exception { + expectedException.expect(HiveException.class); + final Device device = DeviceFixture.createDevice(); final DeviceClassUpdate dc = DeviceFixture.createDeviceClass(); final DeviceUpdate deviceUpdate = DeviceFixture.createDevice(device.getKey(), dc); @@ -73,11 +77,9 @@ public void should_save_and_notify_role_client() { user = userService.createUser(user, "123"); final HivePrincipal principal = new HivePrincipal(user); - deviceService.deviceSaveAndNotify(deviceUpdate, Collections.emptySet(), principal); + SecurityContextHolder.getContext().setAuthentication(new HiveAuthentication(principal)); - final DeviceNotification existingNotification = deviceNotificationService.find(null, device.getGuid()); - assertNotNull(existingNotification); - assertEquals(device.getGuid(), existingNotification.getDeviceGuid()); + deviceService.deviceSaveAndNotify(deviceUpdate, Collections.emptySet(), principal); } /** @@ -96,6 +98,8 @@ public void should_save_and_notify_role_admin() { user = userService.createUser(user, "123"); final HivePrincipal principal = new HivePrincipal(user); + SecurityContextHolder.getContext().setAuthentication(new HiveAuthentication(principal)); + deviceService.deviceSaveAndNotify(deviceUpdate, Collections.emptySet(), principal); final Device existingDevice = deviceService.getDeviceWithNetworkAndDeviceClass(device.getGuid(), principal); @@ -122,7 +126,7 @@ public void should_save_and_notify_role_key() throws UnknownHostException { user = userService.createUser(user, "123"); final Network network = new Network(); - network.setName(""+randomUUID()); + network.setName("" + randomUUID()); Network created = networkService.create(network); assertThat(created.getId(), notNullValue()); userService.assignNetwork(user.getId(), network.getId()); @@ -165,7 +169,7 @@ public void should_save_and_find_without_permissions() { user = userService.createUser(user, "123"); final Network network = new Network(); - network.setName(""+randomUUID()); + network.setName("" + randomUUID()); Network created = networkService.create(network); assertThat(created.getId(), notNullValue()); userService.assignNetwork(user.getId(), network.getId()); @@ -208,14 +212,14 @@ public void should_save_and_find_by_user() throws UnknownHostException { user1 = userService.createUser(user1, "123"); final Network network = new Network(); - network.setName(""+randomUUID()); + network.setName("" + randomUUID()); Network created = networkService.create(network); assertThat(created.getId(), notNullValue()); userService.assignNetwork(user.getId(), network.getId()); deviceUpdate.setNetwork(new NullableWrapper<>(network)); final Network network1 = new Network(); - network1.setName(""+randomUUID()); + network1.setName("" + randomUUID()); Network created1 = networkService.create(network1); assertThat(created1.getId(), notNullValue()); userService.assignNetwork(user1.getId(), network1.getId()); @@ -264,14 +268,14 @@ public void should_save_and_find_by_device_id() throws UnknownHostException { user1 = userService.createUser(user1, "123"); final Network network = new Network(); - network.setName(""+randomUUID()); + network.setName("" + randomUUID()); Network created = networkService.create(network); assertThat(created.getId(), notNullValue()); userService.assignNetwork(user.getId(), network.getId()); deviceUpdate.setNetwork(new NullableWrapper<>(network)); final Network network1 = new Network(); - network1.setName(""+randomUUID()); + network1.setName("" + randomUUID()); Network created1 = networkService.create(network1); assertThat(created1.getId(), notNullValue()); userService.assignNetwork(user1.getId(), network1.getId()); @@ -317,7 +321,7 @@ public void should_save_and_find_by_device_name() { deviceService.deviceSave(deviceUpdate1, Collections.emptySet()); deviceService.deviceSave(deviceUpdate2, Collections.emptySet()); - final List devices = deviceService.getList("DEVICE_NAME", null, null, null, null,null,null,null,null,false,null,null,null ); + final List devices = deviceService.getList("DEVICE_NAME", null, null, null, null, null, null, null, null, false, null, null, null); assertNotNull(devices); assertEquals(devices.size(), 1); assertEquals(device.getGuid(), devices.get(0).getGuid()); @@ -343,7 +347,7 @@ public void should_save_and_find_by_device_status() { deviceService.deviceSave(deviceUpdate1, Collections.emptySet()); deviceService.deviceSave(deviceUpdate2, Collections.emptySet()); - final List devices = deviceService.getList(null, null, "TEST", null, null,null,null,null,null,false,null,null,null ); + final List devices = deviceService.getList(null, null, "TEST", null, null, null, null, null, null, false, null, null, null); assertNotNull(devices); assertEquals(devices.size(), 2); assertEquals(device1.getGuid(), devices.get(0).getGuid()); @@ -371,14 +375,14 @@ public void should_save_and_find_by_network_id() { user1 = userService.createUser(user1, "123"); final Network network = new Network(); - network.setName(""+randomUUID()); + network.setName("" + randomUUID()); Network created = networkService.create(network); assertThat(created.getId(), notNullValue()); userService.assignNetwork(user.getId(), network.getId()); deviceUpdate.setNetwork(new NullableWrapper<>(network)); final Network network1 = new Network(); - network1.setName(""+randomUUID()); + network1.setName("" + randomUUID()); Network created1 = networkService.create(network1); assertThat(created1.getId(), notNullValue()); userService.assignNetwork(user1.getId(), network1.getId()); @@ -392,7 +396,7 @@ public void should_save_and_find_by_network_id() { deviceService.deviceSave(deviceUpdate, Collections.emptySet()); deviceService.deviceSave(deviceUpdate1, Collections.emptySet()); - final List devices = deviceService.getList(null, null, null, network1.getId(), null,null,null,null,null,false,null,null,null ); + final List devices = deviceService.getList(null, null, null, network1.getId(), null, null, null, null, null, false, null, null, null); assertNotNull(devices); assertEquals(device1.getGuid(), devices.get(0).getGuid()); assertNotNull(devices.get(0).getNetwork()); @@ -416,7 +420,7 @@ public void should_save_and_find_by_device_class_id() { deviceService.deviceSave(deviceUpdate, Collections.emptySet()); deviceService.deviceSave(deviceUpdate1, Collections.emptySet()); - final List devices = deviceService.getList(null, null, null, null, null, dc.getId(),null,null,null,false,null,null,null ); + final List devices = deviceService.getList(null, null, null, null, null, dc.getId(), null, null, null, false, null, null, null); assertNotNull(devices); assertEquals(device.getGuid(), devices.get(0).getGuid()); } @@ -438,7 +442,7 @@ public void should_save_and_find_by_device_class_name() { deviceService.deviceSave(deviceUpdate, Collections.emptySet()); deviceService.deviceSave(deviceUpdate1, Collections.emptySet()); - final List devices = deviceService.getList(null, null, null, null, null, null, dc.getName(),null,null,false,null,null,null ); + final List devices = deviceService.getList(null, null, null, null, null, null, dc.getName(), null, null, false, null, null, null); assertNotNull(devices); assertEquals(device.getGuid(), devices.get(0).getGuid()); } @@ -461,7 +465,7 @@ public void should_save_and_find_by_device_class_version() { deviceService.deviceSave(deviceUpdate, Collections.emptySet()); deviceService.deviceSave(deviceUpdate1, Collections.emptySet()); - final List devices = deviceService.getList(null, null, null, null, null, null, null, dc1.getVersion(),null,false,null,null,null ); + final List devices = deviceService.getList(null, null, null, null, null, null, null, dc1.getVersion(), null, false, null, null, null); assertNotNull(devices); assertEquals(device1.getGuid(), devices.get(0).getGuid()); } @@ -500,7 +504,7 @@ public void should_return_device_count() { user = userService.createUser(user, "123"); final Network network = new Network(); - network.setName(""+randomUUID()); + network.setName("" + randomUUID()); Network created = networkService.create(network); assertThat(created.getId(), notNullValue()); userService.assignNetwork(user.getId(), network.getId()); From 45fab838a4caf559669aa8239bff2c6696476332 Mon Sep 17 00:00:00 2001 From: Rafael Zubairov Date: Mon, 5 Oct 2015 22:47:42 +0300 Subject: [PATCH 2/4] #102 made key nullable on device table. --- src/main/java/com/devicehive/model/Device.java | 5 ++--- .../java/com/devicehive/service/DeviceService.java | 11 +++++++---- .../db/migration/V2_0_3__102_make_key_nullable.sql | 1 + 3 files changed, 10 insertions(+), 7 deletions(-) create mode 100644 src/main/resources/db/migration/V2_0_3__102_make_key_nullable.sql diff --git a/src/main/java/com/devicehive/model/Device.java b/src/main/java/com/devicehive/model/Device.java index 31be0597c..806393213 100644 --- a/src/main/java/com/devicehive/model/Device.java +++ b/src/main/java/com/devicehive/model/Device.java @@ -47,8 +47,7 @@ public class Device implements HiveEntity { private String guid; @SerializedName("key") @Column - @NotNull(message = "key field cannot be null.") - @Size(min = 1, max = 64, message = "Field cannot be empty. The length of key should not be more than 64 symbols.") + @Size(min = 1, max = 64, message = "Field could be empty. The length of key should not be more than 64 symbols.") private String key; @SerializedName("name") @Column @@ -178,4 +177,4 @@ public static interface Parameters { static final String ID = "id"; } } -} \ No newline at end of file +} diff --git a/src/main/java/com/devicehive/service/DeviceService.java b/src/main/java/com/devicehive/service/DeviceService.java index 617a58a94..d3490912f 100644 --- a/src/main/java/com/devicehive/service/DeviceService.java +++ b/src/main/java/com/devicehive/service/DeviceService.java @@ -199,9 +199,10 @@ public DeviceNotification deviceUpdateByDevice(DeviceUpdate deviceUpdate, throw new HiveException(String.format(Messages.DEVICE_NOT_FOUND, deviceUpdate.getGuid().getValue()), UNAUTHORIZED.getStatusCode()); } - if (deviceUpdate.getKey() != null && !device.getKey().equals(deviceUpdate.getKey().getValue())) { - logger.error("Device update key {} doesn't equal to the authenticated device key {}", - deviceUpdate.getKey().getValue(), device.getKey()); + if (deviceUpdate.getKey() == null || + device.getKey() == null || + !device.getKey().equals(deviceUpdate.getKey().getValue())) { + logger.error("Device update key {} doesn't equal to the authenticated device key {}", deviceUpdate.getKey(), device.getKey()); throw new HiveException(Messages.INCORRECT_CREDENTIALS, UNAUTHORIZED.getStatusCode()); } DeviceClass deviceClass = deviceClassService @@ -258,7 +259,9 @@ public DeviceNotification deviceSave(DeviceUpdate deviceUpdate, genericDAO.persist(device); return ServerResponsesFactory.createNotificationForDevice(device, SpecialNotifications.DEVICE_ADD); } else { - if (deviceUpdate.getKey() == null || !existingDevice.getKey().equals(deviceUpdate.getKey().getValue())) { + if (deviceUpdate.getKey() == null || + existingDevice.getKey() == null || + !existingDevice.getKey().equals(deviceUpdate.getKey().getValue())) { logger.error("Device update key is null or doesn't equal to the authenticated device key {}", existingDevice.getKey()); throw new HiveException(Messages.INCORRECT_CREDENTIALS, UNAUTHORIZED.getStatusCode()); } diff --git a/src/main/resources/db/migration/V2_0_3__102_make_key_nullable.sql b/src/main/resources/db/migration/V2_0_3__102_make_key_nullable.sql new file mode 100644 index 000000000..7fb6a89ff --- /dev/null +++ b/src/main/resources/db/migration/V2_0_3__102_make_key_nullable.sql @@ -0,0 +1 @@ +alter table device alter column key drop not null; From 1d4e186d2055dbe9b12bef9635c812ccb1feafd0 Mon Sep 17 00:00:00 2001 From: Rafael Zubairov Date: Mon, 5 Oct 2015 23:35:33 +0300 Subject: [PATCH 3/4] #102 fix nullable key conditional semantics has been restored, and corrected to nullability. --- src/main/java/com/devicehive/service/DeviceService.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/main/java/com/devicehive/service/DeviceService.java b/src/main/java/com/devicehive/service/DeviceService.java index d3490912f..374a1819f 100644 --- a/src/main/java/com/devicehive/service/DeviceService.java +++ b/src/main/java/com/devicehive/service/DeviceService.java @@ -199,9 +199,8 @@ public DeviceNotification deviceUpdateByDevice(DeviceUpdate deviceUpdate, throw new HiveException(String.format(Messages.DEVICE_NOT_FOUND, deviceUpdate.getGuid().getValue()), UNAUTHORIZED.getStatusCode()); } - if (deviceUpdate.getKey() == null || - device.getKey() == null || - !device.getKey().equals(deviceUpdate.getKey().getValue())) { + if (deviceUpdate.getKey() != null && + (device.getKey() == null || !device.getKey().equals(deviceUpdate.getKey().getValue()))) { logger.error("Device update key {} doesn't equal to the authenticated device key {}", deviceUpdate.getKey(), device.getKey()); throw new HiveException(Messages.INCORRECT_CREDENTIALS, UNAUTHORIZED.getStatusCode()); } From b4d679dfacdb880b6d4b3270ebe6ddeaeacfd8bc Mon Sep 17 00:00:00 2001 From: Sergey Demyanov Date: Mon, 5 Oct 2015 17:16:56 -0400 Subject: [PATCH 4/4] version bump for 2.0.7 --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 85d6a4168..2b289466e 100644 --- a/pom.xml +++ b/pom.xml @@ -7,7 +7,7 @@ com.devicehive devicehive-server ${packaging.type} - 2.0.6 + 2.0.7 DeviceHive Java Server