Skip to content

Commit

Permalink
Fix #1771: Enforce presence of user ID before operation approval
Browse files Browse the repository at this point in the history
  • Loading branch information
romanstrobl committed Nov 15, 2024
1 parent 1fc61a9 commit b5c02af
Show file tree
Hide file tree
Showing 11 changed files with 225 additions and 72 deletions.
46 changes: 43 additions & 3 deletions docs/WebServices-Methods.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
@@ -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;

/**
* Optional user identifier of the user who is claiming the operation.
*/
private String userId;

}
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,4 @@ public class OperationDetailRequest {

private String operationId;

/**
* Optional user identifier of the user who is requesting the operation.
*/
private String userId;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
/*
* 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) {
return "Operation ID must not be null when requesting operation claim";
}
if (source.getOperationId().isEmpty()) {
return "Operation ID must not be empty when requesting operation claim";
}
if (source.getUserId() == null || source.getUserId().isEmpty()) {
return "User ID must be specified when requesting operation claim";
}
return null;
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 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
Expand Down Expand Up @@ -763,6 +768,38 @@ public OperationDetailResponse operationDetail(OperationDetailRequest request) t
return localizationProvider.buildExceptionForCode(ServiceError.OPERATION_NOT_FOUND);
});

final OperationEntity operationEntity = expireOperation(operation, currentTimestamp);
final OperationDetailResponse operationDetailResponse = convertFromEntityAndFillOtp(operationEntity);
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 // 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 operationEntity = expireOperation(
claimOperation(operation, userId, currentTimestamp),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
Loading

0 comments on commit b5c02af

Please sign in to comment.