Fix #1771: Enforce presence of user ID before operation approval (#1773)

docs/WebServices-Methods.md

@@ -88,6 +88,7 @@ The following `v3` methods are published using the service:
 - Operations
     - [createOperation](#method-createoperation)
     - [operationDetail](#method-operationdetail)
+    - [operationClaim](#method-operationclaim)
     - [findPendingOperationsForUser](#method-findpendingoperationsforuser)
     - [findAllOperationsForUser](#method-findalloperationsforuser)
     - [findAllOperationsByExternalId](#method-findalloperationsbyexternalid)
@@ -2232,10 +2233,49 @@ REST endpoint: `POST /rest/v3/operation/detail`
 
 `OperationDetailRequest`
 
-| Type | Name | Description |
-|------|------|-------------|
+| Type     | Name          | Description                     |
+|----------|---------------|---------------------------------|
 | `String` | `operationId` | The identifier of the operation |
-| `String` | `userId` | Optional user identifier of the user, used for operation claim. |
+
+#### Response
+
+`OperationDetailResponse`
+
+| Type                  | Name                 | Description                                                                                                                      |
+|-----------------------|----------------------|----------------------------------------------------------------------------------------------------------------------------------|
+| `String`              | `id`                 | The operation ID                                                                                                                 |
+| `String`              | `userId`             | The identifier of the user                                                                                                       |
+| `String`              | `applicationId`      | The identifier of the application                                                                                                |
+| `String`              | `externalId`         | External identifier of the operation, i.e., ID from transaction system                                                           |
+| `String`              | `operationType`      | Type of the operation created based on the template                                                                              |
+| `String`              | `data`               | Operation data                                                                                                                   |
+| `Map<String, String>` | `parameters`         | Parameters of the operation, will be filled to the operation data                                                                |
+| `OperationStatus`     | `status`             | Status of the operation                                                                                                          |
+| `String`              | `statusReason`       | Optional details why the status changed. The value should be sent in the form of a computer-readable code, not a free-form text. |
+| `List<SignatureType>` | `signatureType`      | Allowed types of signature                                                                                                       |
+| `Long`                | `failureCount`       | The current number of the failed approval attempts                                                                               |
+| `Long`                | `maxFailureCount`    | The maximum allowed number of the failed approval attempts                                                                       |
+| `Date`                | `timestampCreated`   | Timestamp of when the operation was created                                                                                      |
+| `Date`                | `timestampExpires`   | Timestamp of when the operation will expires / expired                                                                           |
+| `Date`                | `timestampFinalized` | Timestamp of when the operation was switched to a terminating status                                                             |
+| `String`              | `riskFlags`          | Risk flags for offline QR code. Uppercase letters without separator, e.g. `XFC`.                                                 |
+| `String`              | `proximityOtp`       | TOTP for proximity check (if enabled) valid for the current time step.                                                           |
+| `String`              | `activationId`       | Activation Id of the activation scoped for the operation                                                                         |
+
+### Method 'operationClaim'
+
+Claim the operation for a user.
+
+#### Request
+
+REST endpoint: `POST /rest/v3/operation/claim`
+
+`OperationClaimRequest`
+
+| Type     | Name          | Description                                            |
+|----------|---------------|--------------------------------------------------------|
+| `String` | `operationId` | The identifier of the operation                        |
+| `String` | `userId`      | User identifier of the user, used for operation claim. |
 
 #### Response
 

powerauth-client-model/src/main/java/com/wultra/security/powerauth/client/PowerAuthClient.java

@@ -1976,6 +1976,24 @@ RecoveryCodeActivationResponse createActivationUsingRecoveryCode(String recovery
      */
     OperationDetailResponse operationDetail(OperationDetailRequest request, MultiValueMap<String, String> queryParams, MultiValueMap<String, String> httpHeaders) throws PowerAuthClientException;
 
+    /**
+     * Claim operation for a user.
+     * @param request Operation detail request.
+     * @return Operation detail response.
+     * @throws PowerAuthClientException In case REST API call fails.
+     */
+    OperationDetailResponse operationClaim(OperationClaimRequest request) throws PowerAuthClientException;
+
+    /**
+     * Claim operation for a user.
+     * @param request Operation detail request.
+     * @param queryParams HTTP query parameters.
+     * @param httpHeaders HTTP headers.
+     * @return Operation detail response.
+     * @throws PowerAuthClientException In case REST API call fails.
+     */
+    OperationDetailResponse operationClaim(OperationClaimRequest request, MultiValueMap<String, String> queryParams, MultiValueMap<String, String> httpHeaders) throws PowerAuthClientException;
+
     /**
      * Get list with all operations for provided user.
      * @param request Get operation list request.

powerauth-client-model/src/main/java/com/wultra/security/powerauth/client/model/request/OperationClaimRequest.java

@@ -0,0 +1,41 @@
+/*
+ * PowerAuth Server and related software components
+ * Copyright (C) 2024 Wultra s.r.o.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as published
+ * by the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+package com.wultra.security.powerauth.client.model.request;
+
+import lombok.Data;
+
+/**
+ * Request for operation claim.
+ *
+ * @author Roman Strobl, [email protected]
+ */
+@Data
+public class OperationClaimRequest  {
+
+    /**
+     * Operation identifier.
+     */
+    private String operationId;
+
+    /**
+     * User identifier of the user who is claiming the operation.
+     */
+    private String userId;
+
+}

powerauth-client-model/src/main/java/com/wultra/security/powerauth/client/model/request/OperationDetailRequest.java

@@ -30,8 +30,4 @@ public class OperationDetailRequest {
 
     private String operationId;
 
-    /**
-     * Optional user identifier of the user who is requesting the operation.
-     */
-    private String userId;
 }

powerauth-client-model/src/main/java/com/wultra/security/powerauth/client/model/validator/OperationClaimRequestValidator.java

@@ -0,0 +1,43 @@
+/*
+ * PowerAuth Server and related software components
+ * Copyright (C) 2024 Wultra s.r.o.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as published
+ * by the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+package com.wultra.security.powerauth.client.model.validator;
+
+import com.wultra.security.powerauth.client.model.request.OperationClaimRequest;
+
+/**
+ * Validator for OperationClaimRequest class.
+ *
+ * @author Roman Strobl, [email protected]
+ */
+public class OperationClaimRequestValidator {
+
+    public static String validate(OperationClaimRequest source) {
+        if (source == null) {
+            return "Operation claim request must not be null";
+        }
+        if (source.getOperationId() == null || source.getOperationId().isEmpty()) {
+            return "Operation ID must be specified when requesting operation claim";
+        }
+        if (source.getUserId() == null || source.getUserId().isEmpty()) {
+            return "User ID must be specified when requesting operation claim";
+        }
+        return null;
+    }
+
+}

powerauth-java-server/src/main/java/io/getlime/security/powerauth/app/server/controller/api/OperationsController.java

@@ -81,6 +81,21 @@ public ObjectResponse<OperationDetailResponse> operationDetail(@RequestBody Obje
         return response;
     }
 
+    /**
+     * Claim operation for a user.
+     *
+     * @param request Claim operation for a user.
+     * @return Get operation response.
+     * @throws Exception In case the service throws exception.
+     */
+    @PostMapping("/claim")
+    public ObjectResponse<OperationDetailResponse> operationClaim(@RequestBody ObjectRequest<OperationClaimRequest> request) throws Exception {
+        logger.info("OperationClaimRequest received: {}", request);
+        final ObjectResponse<OperationDetailResponse> response = new ObjectResponse<>(service.operationClaim(request.getRequestObject()));
+        logger.info("OperationClaimRequest succeeded: {}", response);
+        return response;
+    }
+
     /**
      * Find all operations for given user.
      *

powerauth-java-server/src/main/java/io/getlime/security/powerauth/app/server/service/behavior/tasks/OperationServiceBehavior.java

@@ -369,13 +369,18 @@ public OperationUserActionResponse attemptApproveOperation(OperationApproveReque
                 throw localizationProvider.buildExceptionForCode(ServiceError.OPERATION_APPROVE_FAILURE);
             }
 
+            final String expectedUserId = operationEntity.getUserId();
+            if (expectedUserId == null) {
+                logger.warn("Operation ID: {} cannot be approved, because user ID is not set.", operationId);
+                throw localizationProvider.buildExceptionForCode(ServiceError.OPERATION_APPROVE_FAILURE);
+            }
+
             // Check the operation properties match the request
             final PowerAuthSignatureTypes factorEnum = PowerAuthSignatureTypes.getEnumFromString(signatureType.toString());
             final ProximityCheckResult proximityCheckResult = fetchProximityCheckResult(operationEntity, request, currentInstant);
-            final String expectedUserId = operationEntity.getUserId();
             final boolean activationIdMatches = activationIdMatches(request, operationEntity.getActivationId());
             final boolean operationShouldFail = operationApprovalCustomizer.operationShouldFail(operationEntity, request);
-            if ((expectedUserId == null || expectedUserId.equals(userId)) // correct user approved the operation
+            if (expectedUserId.equals(userId) // correct user approved the operation
                     && 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
@@ -385,7 +390,6 @@ && proximityCheckPassed(proximityCheckResult)
                     && !operationShouldFail) { // operation customizer can change the approval status by an external impulse
 
                 // Approve the operation
-                operationEntity.setUserId(userId);
                 operationEntity.setStatus(OperationStatusDo.APPROVED);
                 operationEntity.setTimestampFinalized(currentTimestamp);
                 operationEntity.setAdditionalData(mapMerge(operationEntity.getAdditionalData(), additionalData));
@@ -534,11 +538,15 @@ public OperationUserActionResponse rejectOperation(OperationRejectRequest reques
             }
 
             final String expectedUserId = operationEntity.getUserId();
-            if ((expectedUserId == null || expectedUserId.equals(userId)) // correct user rejects the operation
+            if (expectedUserId == null) {
+                logger.warn("Operation ID: {} cannot be rejected, because user ID is not set.", operationId);
+                throw localizationProvider.buildExceptionForCode(ServiceError.OPERATION_APPROVE_FAILURE);
+            }
+
+            if ((expectedUserId.equals(userId)) // correct user rejects the operation
                     && operationEntity.getApplications().contains(application.get())) { // operation is rejected by the expected application
 
                 // Reject the operation
-                operationEntity.setUserId(userId);
                 operationEntity.setStatus(OperationStatusDo.REJECTED);
                 operationEntity.setTimestampFinalized(currentTimestamp);
                 operationEntity.setAdditionalData(mapMerge(operationEntity.getAdditionalData(), additionalData));
@@ -763,11 +771,7 @@ public OperationDetailResponse operationDetail(OperationDetailRequest request) t
                 return localizationProvider.buildExceptionForCode(ServiceError.OPERATION_NOT_FOUND);
             });
 
-            final String userId = request.getUserId();
-            final OperationEntity operationEntity = expireOperation(
-                    claimOperation(operation, userId, currentTimestamp),
-                    currentTimestamp
-            );
+            final OperationEntity operationEntity = expireOperation(operation, currentTimestamp);
             final OperationDetailResponse operationDetailResponse = convertFromEntityAndFillOtp(operationEntity);
             extendAndSetOperationDetailData(operationDetailResponse);
             return operationDetailResponse;
@@ -783,6 +787,40 @@ public OperationDetailResponse operationDetail(OperationDetailRequest request) t
         }
     }
 
+    @Transactional // operation is modified when expiration happens
+    public OperationDetailResponse operationClaim(OperationClaimRequest request) throws GenericServiceException {
+        try {
+            final String error = OperationClaimRequestValidator.validate(request);
+            if (error != null) {
+                throw new GenericServiceException(ServiceError.INVALID_REQUEST, error);
+            }
+
+            final Date currentTimestamp = new Date();
+            final String operationId = request.getOperationId();
+
+            final OperationEntity operation = operationQueryService.findOperationForUpdate(operationId).orElseThrow(() -> {
+                logger.warn("Operation was not found for ID: {}", operationId);
+                return localizationProvider.buildExceptionForCode(ServiceError.OPERATION_NOT_FOUND);
+            });
+
+            final String userId = request.getUserId();
+            final OperationEntity operationClaimed = claimOperation(operation, userId, currentTimestamp);
+            final OperationEntity operationUpdated = expireOperation(operationClaimed, currentTimestamp);
+            final OperationDetailResponse operationDetailResponse = convertFromEntityAndFillOtp(operationUpdated);
+            extendAndSetOperationDetailData(operationDetailResponse);
+            return operationDetailResponse;
+        } catch (GenericServiceException ex) {
+            // already logged
+            throw ex;
+        } catch (RuntimeException ex) {
+            logger.error("Runtime exception or error occurred, transaction will be rolled back", ex);
+            throw ex;
+        } catch (Exception ex) {
+            logger.error("Unknown error occurred", ex);
+            throw new GenericServiceException(ServiceError.UNKNOWN_ERROR, ex.getMessage());
+        }
+    }
+
     @Transactional
     public OperationListResponse findAllOperationsForUser(final OperationListForUserRequest request) throws GenericServiceException {
         try {

powerauth-java-server/src/main/java/io/getlime/security/powerauth/app/server/service/fido2/PowerAuthAssertionProvider.java

@@ -31,10 +31,7 @@
 import com.wultra.security.powerauth.client.model.enumeration.OperationStatus;
 import com.wultra.security.powerauth.client.model.enumeration.SignatureType;
 import com.wultra.security.powerauth.client.model.enumeration.UserActionResult;
-import com.wultra.security.powerauth.client.model.request.OperationApproveRequest;
-import com.wultra.security.powerauth.client.model.request.OperationCreateRequest;
-import com.wultra.security.powerauth.client.model.request.OperationDetailRequest;
-import com.wultra.security.powerauth.client.model.request.OperationFailApprovalRequest;
+import com.wultra.security.powerauth.client.model.request.*;
 import com.wultra.security.powerauth.client.model.response.OperationDetailResponse;
 import com.wultra.security.powerauth.client.model.response.OperationUserActionResponse;
 import com.wultra.security.powerauth.fido2.model.entity.AuthenticatorDetail;
@@ -133,6 +130,12 @@ public AssertionChallenge approveAssertion(String challengeValue, AuthenticatorD
             final String operationId = split[0];
             final String operationData = split[1];
 
+            // Claim operation to set user ID for this operation before approval
+            final OperationClaimRequest claimRequest = new OperationClaimRequest();
+            claimRequest.setOperationId(operationId);
+            claimRequest.setUserId(authenticatorDetail.getUserId());
+            operationServiceBehavior.operationClaim(claimRequest);
+
             final OperationApproveRequest operationApproveRequest = new OperationApproveRequest();
             operationApproveRequest.setOperationId(operationId);
             operationApproveRequest.setData(operationData);

powerauth-java-server/src/test/java/io/getlime/security/powerauth/app/server/controller/api/PowerAuthControllerTest.java

@@ -454,7 +454,6 @@ void testGetOperationDetail() throws Exception {
         final OperationDetailRequest detailRequest = new OperationDetailRequest();
         final String operationId = operation.getId();
         detailRequest.setOperationId(operationId);
-        detailRequest.setUserId(PowerAuthControllerTestConfig.USER_ID);
 
         final OperationDetailResponse detailResponse = powerAuthClient.operationDetail(detailRequest);
         assertEquals(OperationStatus.PENDING, detailResponse.getStatus());