Fix #1795: Validate user ID for operation approve and reject before u…

…pdating operation (#1796)
wultra · Dec 3, 2024 · 6ffadee · 6ffadee
1 parent ee6c266
commit 6ffadee
Show file tree

Hide file tree

Showing 2 changed files with 20 additions and 12 deletions.
diff --git a/...etlime/security/powerauth/app/server/service/behavior/tasks/OperationServiceBehavior.java b/...etlime/security/powerauth/app/server/service/behavior/tasks/OperationServiceBehavior.java
@@ -371,7 +371,12 @@ public OperationUserActionResponse attemptApproveOperation(OperationApproveReque
 
             final String expectedUserId = operationEntity.getUserId();
             if (expectedUserId == null) {
-                logger.warn("Operation ID: {} cannot be approved, because user ID is not set.", operationId);
+                logger.warn("Operation with operationId: {} cannot be approved, because user ID is not set.", operationId);
+                throw localizationProvider.buildExceptionForCode(ServiceError.OPERATION_APPROVE_FAILURE);
+            }
+
+            if (!expectedUserId.equals(userId)) {
+                logger.warn("Operation with operationId: {} cannot be approved, because user ID from the request '{}' does not match user ID from the operation '{}'.", operationId, userId, expectedUserId);
                 throw localizationProvider.buildExceptionForCode(ServiceError.OPERATION_APPROVE_FAILURE);
             }
 
@@ -380,8 +385,7 @@ public OperationUserActionResponse attemptApproveOperation(OperationApproveReque
             final ProximityCheckResult proximityCheckResult = fetchProximityCheckResult(operationEntity, request, currentInstant);
             final boolean activationIdMatches = activationIdMatches(request, operationEntity.getActivationId());
             final boolean operationShouldFail = operationApprovalCustomizer.operationShouldFail(operationEntity, request);
-            if (expectedUserId.equals(userId) // correct user approved the operation
-                    && operationEntity.getApplications().contains(application.get()) // operation is approved by the expected application
+            if (operationEntity.getApplications().contains(application.get()) // operation is approved by the expected application
                     && isDataEqual(operationEntity, data) // operation data matched the expected value
                     && factorsAcceptable(operationEntity, factorEnum) // auth factors are acceptable
                     && operationEntity.getMaxFailureCount() > operationEntity.getFailureCount() // operation has sufficient attempts left (redundant check)
@@ -539,12 +543,16 @@ public OperationUserActionResponse rejectOperation(OperationRejectRequest reques
 
             final String expectedUserId = operationEntity.getUserId();
             if (expectedUserId == null) {
-                logger.warn("Operation ID: {} cannot be rejected, because user ID is not set.", operationId);
-                throw localizationProvider.buildExceptionForCode(ServiceError.OPERATION_APPROVE_FAILURE);
+                logger.warn("Operation with operationId: {} cannot be rejected, because user ID is not set.", operationId);
+                throw localizationProvider.buildExceptionForCode(ServiceError.OPERATION_REJECT_FAILURE);
+            }
+
+            if (!expectedUserId.equals(userId)) {
+                logger.warn("Operation with operationId: {} cannot be rejected, because user ID from the request '{}' does not match user ID from the operation '{}'.", operationId, userId, expectedUserId);
+                throw localizationProvider.buildExceptionForCode(ServiceError.OPERATION_REJECT_FAILURE);
             }
 
-            if ((expectedUserId.equals(userId)) // correct user rejects the operation
-                    && operationEntity.getApplications().contains(application.get())) { // operation is rejected by the expected application
+            if (operationEntity.getApplications().contains(application.get())) { // operation is rejected by the expected application
 
                 // Reject the operation
                 operationEntity.setStatus(OperationStatusDo.REJECTED);

diff --git a/...me/security/powerauth/app/server/service/behavior/tasks/OperationServiceBehaviorTest.java b/...me/security/powerauth/app/server/service/behavior/tasks/OperationServiceBehaviorTest.java
@@ -747,8 +747,8 @@ void testOperationApprovalFailDifferentUserId() throws GenericServiceException {
         approveRequest.setData("A2");
         approveRequest.setApplicationId("PA_Tests");
         approveRequest.setSignatureType(SignatureType.POSSESSION_KNOWLEDGE);
-        final OperationUserActionResponse response = operationService.attemptApproveOperation(approveRequest);
-        assertEquals(UserActionResult.APPROVAL_FAILED, response.getResult());
+        final Exception exception = assertThrows(GenericServiceException.class, () -> operationService.attemptApproveOperation(approveRequest));
+        assertEquals("Operation cannot be approved.", exception.getMessage());
     }
 
     @Test
@@ -762,7 +762,7 @@ void testAnonymousOperationRejectError() throws GenericServiceException {
         rejectRequest.setUserId("test_user");
         rejectRequest.setApplicationId("PA_Tests");
         final Exception exception = assertThrows(GenericServiceException.class, () -> operationService.rejectOperation(rejectRequest));
-        assertEquals("Operation cannot be approved.", exception.getMessage());
+        assertEquals("Operation cannot be rejected.", exception.getMessage());
     }
 
     @Test
@@ -776,8 +776,8 @@ void testOperationRejectFailedDifferentUserId() throws GenericServiceException {
         rejectRequest.setOperationId(operation.getId());
         rejectRequest.setUserId("test_user_2");
         rejectRequest.setApplicationId("PA_Tests");
-        final OperationUserActionResponse response = operationService.rejectOperation(rejectRequest);
-        assertEquals(UserActionResult.REJECT_FAILED, response.getResult());
+        final Exception exception = assertThrows(GenericServiceException.class, () -> operationService.rejectOperation(rejectRequest));
+        assertEquals("Operation cannot be rejected.", exception.getMessage());
     }
 
     private void createApplication() throws GenericServiceException {