简柏林 / thingskit · Commits

Commit 78436f9baa4af41b114b8ff7e8a06bb1595cd141

Authored by Andrew Shvayka
1 parent a64d2123

Critical security fixes

Inline Side-by-side
Showing 3 changed files with 54 additions and 43 deletions
@@ -259,7 +259,6 @@ public abstract class BaseController { @@ -259,7 +259,6 @@ public abstract class BaseController {
259 259
260 Customer checkCustomerId(CustomerId customerId) throws ThingsboardException { 260 Customer checkCustomerId(CustomerId customerId) throws ThingsboardException {
261 try { 261 try {
262 - validateId(customerId, "Incorrect customerId " + customerId);  
263 SecurityUser authUser = getCurrentUser(); 262 SecurityUser authUser = getCurrentUser();
264 if (authUser.getAuthority() == Authority.SYS_ADMIN || 263 if (authUser.getAuthority() == Authority.SYS_ADMIN ||
265 (authUser.getAuthority() != Authority.TENANT_ADMIN && 264 (authUser.getAuthority() != Authority.TENANT_ADMIN &&
@@ -267,9 +266,13 @@ public abstract class BaseController { @@ -267,9 +266,13 @@ public abstract class BaseController {
267 throw new ThingsboardException(YOU_DON_T_HAVE_PERMISSION_TO_PERFORM_THIS_OPERATION, 266 throw new ThingsboardException(YOU_DON_T_HAVE_PERMISSION_TO_PERFORM_THIS_OPERATION,
268 ThingsboardErrorCode.PERMISSION_DENIED); 267 ThingsboardErrorCode.PERMISSION_DENIED);
269 } 268 }
270 - Customer customer = customerService.findCustomerById(customerId);  
271 - checkCustomer(customer);  
272 - return customer; 269 + if (customerId != null && !customerId.isNullUid()) {
  270 + Customer customer = customerService.findCustomerById(customerId);
  271 + checkCustomer(customer);
  272 + return customer;
  273 + } else {
  274 + return null;
  275 + }
273 } catch (Exception e) { 276 } catch (Exception e) {
274 throw handleException(e, false); 277 throw handleException(e, false);
275 } 278 }
@@ -350,9 +353,7 @@ public abstract class BaseController { @@ -350,9 +353,7 @@ public abstract class BaseController {
350 protected void checkDevice(Device device) throws ThingsboardException { 353 protected void checkDevice(Device device) throws ThingsboardException {
351 checkNotNull(device); 354 checkNotNull(device);
352 checkTenantId(device.getTenantId()); 355 checkTenantId(device.getTenantId());
353 - if (device.getCustomerId() != null && !device.getCustomerId().getId().equals(ModelConstants.NULL_UUID)) {  
354 - checkCustomerId(device.getCustomerId());  
355 - } 356 + checkCustomerId(device.getCustomerId());
356 } 357 }
357 358
358 protected EntityView checkEntityViewId(EntityViewId entityViewId) throws ThingsboardException { 359 protected EntityView checkEntityViewId(EntityViewId entityViewId) throws ThingsboardException {
@@ -369,9 +370,7 @@ public abstract class BaseController { @@ -369,9 +370,7 @@ public abstract class BaseController {
369 protected void checkEntityView(EntityView entityView) throws ThingsboardException { 370 protected void checkEntityView(EntityView entityView) throws ThingsboardException {
370 checkNotNull(entityView); 371 checkNotNull(entityView);
371 checkTenantId(entityView.getTenantId()); 372 checkTenantId(entityView.getTenantId());
372 - if (entityView.getCustomerId() != null && !entityView.getCustomerId().getId().equals(ModelConstants.NULL_UUID)) {  
373 - checkCustomerId(entityView.getCustomerId());  
374 - } 373 + checkCustomerId(entityView.getCustomerId());
375 } 374 }
376 375
377 Asset checkAssetId(AssetId assetId) throws ThingsboardException { 376 Asset checkAssetId(AssetId assetId) throws ThingsboardException {
@@ -388,9 +387,7 @@ public abstract class BaseController { @@ -388,9 +387,7 @@ public abstract class BaseController {
388 protected void checkAsset(Asset asset) throws ThingsboardException { 387 protected void checkAsset(Asset asset) throws ThingsboardException {
389 checkNotNull(asset); 388 checkNotNull(asset);
390 checkTenantId(asset.getTenantId()); 389 checkTenantId(asset.getTenantId());
391 - if (asset.getCustomerId() != null && !asset.getCustomerId().getId().equals(ModelConstants.NULL_UUID)) {  
392 - checkCustomerId(asset.getCustomerId());  
393 - } 390 + checkCustomerId(asset.getCustomerId());
394 } 391 }
395 392
396 Alarm checkAlarmId(AlarmId alarmId) throws ThingsboardException { 393 Alarm checkAlarmId(AlarmId alarmId) throws ThingsboardException {
@@ -499,8 +496,7 @@ public abstract class BaseController { @@ -499,8 +496,7 @@ public abstract class BaseController {
499 ComponentDescriptor checkComponentDescriptorByClazz(String clazz) throws ThingsboardException { 496 ComponentDescriptor checkComponentDescriptorByClazz(String clazz) throws ThingsboardException {
500 try { 497 try {
501 log.debug("[{}] Lookup component descriptor", clazz); 498 log.debug("[{}] Lookup component descriptor", clazz);
502 - ComponentDescriptor componentDescriptor = checkNotNull(componentDescriptorService.getComponent(clazz));  
503 - return componentDescriptor; 499 + return checkNotNull(componentDescriptorService.getComponent(clazz));
504 } catch (Exception e) { 500 } catch (Exception e) {
505 throw handleException(e, false); 501 throw handleException(e, false);
506 } 502 }
@@ -564,16 +560,16 @@ public abstract class BaseController { @@ -564,16 +560,16 @@ public abstract class BaseController {
564 } 560 }
565 561
566 protected <I extends EntityId> I emptyId(EntityType entityType) { 562 protected <I extends EntityId> I emptyId(EntityType entityType) {
567 - return (I)EntityIdFactory.getByTypeAndUuid(entityType, ModelConstants.NULL_UUID); 563 + return (I) EntityIdFactory.getByTypeAndUuid(entityType, ModelConstants.NULL_UUID);
568 } 564 }
569 565
570 protected <E extends HasName, I extends EntityId> void logEntityAction(I entityId, E entity, CustomerId customerId, 566 protected <E extends HasName, I extends EntityId> void logEntityAction(I entityId, E entity, CustomerId customerId,
571 - ActionType actionType, Exception e, Object... additionalInfo) throws ThingsboardException { 567 + ActionType actionType, Exception e, Object... additionalInfo) throws ThingsboardException {
572 logEntityAction(getCurrentUser(), entityId, entity, customerId, actionType, e, additionalInfo); 568 logEntityAction(getCurrentUser(), entityId, entity, customerId, actionType, e, additionalInfo);
573 } 569 }
574 570
575 protected <E extends HasName, I extends EntityId> void logEntityAction(User user, I entityId, E entity, CustomerId customerId, 571 protected <E extends HasName, I extends EntityId> void logEntityAction(User user, I entityId, E entity, CustomerId customerId,
576 - ActionType actionType, Exception e, Object... additionalInfo) throws ThingsboardException { 572 + ActionType actionType, Exception e, Object... additionalInfo) throws ThingsboardException {
577 if (customerId == null || customerId.isNullUid()) { 573 if (customerId == null || customerId.isNullUid()) {
578 customerId = user.getCustomerId(); 574 customerId = user.getCustomerId();
579 } 575 }
@@ -589,7 +585,7 @@ public abstract class BaseController { @@ -589,7 +585,7 @@ public abstract class BaseController {
589 } 585 }
590 586
591 private <E extends HasName, I extends EntityId> void pushEntityActionToRuleEngine(I entityId, E entity, User user, CustomerId customerId, 587 private <E extends HasName, I extends EntityId> void pushEntityActionToRuleEngine(I entityId, E entity, User user, CustomerId customerId,
592 - ActionType actionType, Object... additionalInfo) { 588 + ActionType actionType, Object... additionalInfo) {
593 String msgType = null; 589 String msgType = null;
594 switch (actionType) { 590 switch (actionType) {
595 case ADDED: 591 case ADDED:
@@ -668,7 +664,7 @@ public abstract class BaseController { @@ -668,7 +664,7 @@ public abstract class BaseController {
668 String scope = extractParameter(String.class, 0, additionalInfo); 664 String scope = extractParameter(String.class, 0, additionalInfo);
669 List<String> keys = extractParameter(List.class, 1, additionalInfo); 665 List<String> keys = extractParameter(List.class, 1, additionalInfo);
670 metaData.putValue("scope", scope); 666 metaData.putValue("scope", scope);
671 - ArrayNode attrsArrayNode = entityNode.putArray("attributes"); 667 + ArrayNode attrsArrayNode = entityNode.putArray("attributes");
672 if (keys != null) { 668 if (keys != null) {
673 keys.forEach(attrsArrayNode::add); 669 keys.forEach(attrsArrayNode::add);
674 } 670 }
@@ -36,29 +36,10 @@ public class ValidationCallback<T> implements FutureCallback<ValidationResult> { @@ -36,29 +36,10 @@ public class ValidationCallback<T> implements FutureCallback<ValidationResult> {
36 36
37 @Override 37 @Override
38 public void onSuccess(ValidationResult result) { 38 public void onSuccess(ValidationResult result) {
39 - ValidationResultCode resultCode = result.getResultCode();  
40 - if (resultCode == ValidationResultCode.OK) { 39 + if (result.getResultCode() == ValidationResultCode.OK) {
41 action.onSuccess(response); 40 action.onSuccess(response);
42 } else { 41 } else {
43 - Exception e;  
44 - switch (resultCode) {  
45 - case ENTITY_NOT_FOUND:  
46 - e = new EntityNotFoundException(result.getMessage());  
47 - break;  
48 - case UNAUTHORIZED:  
49 - e = new UnauthorizedException(result.getMessage());  
50 - break;  
51 - case ACCESS_DENIED:  
52 - e = new AccessDeniedException(result.getMessage());  
53 - break;  
54 - case INTERNAL_ERROR:  
55 - e = new InternalErrorException(result.getMessage());  
56 - break;  
57 - default:  
58 - e = new UnauthorizedException("Permission denied.");  
59 - break;  
60 - }  
61 - onFailure(e); 42 + onFailure(getException(result));
62 } 43 }
63 } 44 }
64 45
@@ -66,4 +47,28 @@ public class ValidationCallback<T> implements FutureCallback<ValidationResult> { @@ -66,4 +47,28 @@ public class ValidationCallback<T> implements FutureCallback<ValidationResult> {
66 public void onFailure(Throwable e) { 47 public void onFailure(Throwable e) {
67 action.onFailure(e); 48 action.onFailure(e);
68 } 49 }
  50 +
  51 + public static Exception getException(ValidationResult result) {
  52 + ValidationResultCode resultCode = result.getResultCode();
  53 + Exception e;
  54 + switch (resultCode) {
  55 + case ENTITY_NOT_FOUND:
  56 + e = new EntityNotFoundException(result.getMessage());
  57 + break;
  58 + case UNAUTHORIZED:
  59 + e = new UnauthorizedException(result.getMessage());
  60 + break;
  61 + case ACCESS_DENIED:
  62 + e = new AccessDeniedException(result.getMessage());
  63 + break;
  64 + case INTERNAL_ERROR:
  65 + e = new InternalErrorException(result.getMessage());
  66 + break;
  67 + default:
  68 + e = new UnauthorizedException("Permission denied.");
  69 + break;
  70 + }
  71 + return e;
  72 + }
  73 +
69 } 74 }
@@ -37,13 +37,18 @@ import org.thingsboard.server.common.data.kv.TsKvEntry; @@ -37,13 +37,18 @@ import org.thingsboard.server.common.data.kv.TsKvEntry;
37 import org.thingsboard.server.dao.attributes.AttributesService; 37 import org.thingsboard.server.dao.attributes.AttributesService;
38 import org.thingsboard.server.dao.timeseries.TimeseriesService; 38 import org.thingsboard.server.dao.timeseries.TimeseriesService;
39 import org.thingsboard.server.service.security.AccessValidator; 39 import org.thingsboard.server.service.security.AccessValidator;
  40 +import org.thingsboard.server.service.security.ValidationCallback;
40 import org.thingsboard.server.service.security.ValidationResult; 41 import org.thingsboard.server.service.security.ValidationResult;
  42 +import org.thingsboard.server.service.security.ValidationResultCode;
41 import org.thingsboard.server.service.telemetry.cmd.AttributesSubscriptionCmd; 43 import org.thingsboard.server.service.telemetry.cmd.AttributesSubscriptionCmd;
42 import org.thingsboard.server.service.telemetry.cmd.GetHistoryCmd; 44 import org.thingsboard.server.service.telemetry.cmd.GetHistoryCmd;
43 import org.thingsboard.server.service.telemetry.cmd.SubscriptionCmd; 45 import org.thingsboard.server.service.telemetry.cmd.SubscriptionCmd;
44 import org.thingsboard.server.service.telemetry.cmd.TelemetryPluginCmd; 46 import org.thingsboard.server.service.telemetry.cmd.TelemetryPluginCmd;
45 import org.thingsboard.server.service.telemetry.cmd.TelemetryPluginCmdsWrapper; 47 import org.thingsboard.server.service.telemetry.cmd.TelemetryPluginCmdsWrapper;
46 import org.thingsboard.server.service.telemetry.cmd.TimeseriesSubscriptionCmd; 48 import org.thingsboard.server.service.telemetry.cmd.TimeseriesSubscriptionCmd;
  49 +import org.thingsboard.server.service.telemetry.exception.AccessDeniedException;
  50 +import org.thingsboard.server.service.telemetry.exception.EntityNotFoundException;
  51 +import org.thingsboard.server.service.telemetry.exception.InternalErrorException;
47 import org.thingsboard.server.service.telemetry.exception.UnauthorizedException; 52 import org.thingsboard.server.service.telemetry.exception.UnauthorizedException;
48 import org.thingsboard.server.service.telemetry.sub.SubscriptionErrorCode; 53 import org.thingsboard.server.service.telemetry.sub.SubscriptionErrorCode;
49 import org.thingsboard.server.service.telemetry.sub.SubscriptionState; 54 import org.thingsboard.server.service.telemetry.sub.SubscriptionState;
@@ -535,11 +540,16 @@ public class DefaultTelemetryWebSocketService implements TelemetryWebSocketServi @@ -535,11 +540,16 @@ public class DefaultTelemetryWebSocketService implements TelemetryWebSocketServi
535 }; 540 };
536 } 541 }
537 542
538 - private FutureCallback<ValidationResult> on(Consumer<ValidationResult> success, Consumer<Throwable> failure) { 543 + private FutureCallback<ValidationResult> on(Consumer<Void> success, Consumer<Throwable> failure) {
539 return new FutureCallback<ValidationResult>() { 544 return new FutureCallback<ValidationResult>() {
540 @Override 545 @Override
541 public void onSuccess(@Nullable ValidationResult result) { 546 public void onSuccess(@Nullable ValidationResult result) {
542 - success.accept(result); 547 + ValidationResultCode resultCode = result.getResultCode();
  548 + if (resultCode == ValidationResultCode.OK) {
  549 + success.accept(null);
  550 + } else {
  551 + onFailure(ValidationCallback.getException(result));
  552 + }
543 } 553 }
544 554
545 @Override 555 @Override