Skip to content

Commit

Permalink
Merge pull request #1759 from /issues/1755-backport-1753
Browse files Browse the repository at this point in the history
Fix #1755: Backport fix of #1753 to 1.8.x branch
  • Loading branch information
romanstrobl authored Oct 24, 2024
2 parents 38a9c04 + 99e2d6c commit 698055c
Show file tree
Hide file tree
Showing 11 changed files with 116 additions and 13 deletions.
2 changes: 1 addition & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@

<groupId>io.getlime.security</groupId>
<artifactId>powerauth-server-parent</artifactId>
<version>1.8.2</version>
<version>1.8.3</version>
<packaging>pom</packaging>

<parent>
Expand Down
2 changes: 1 addition & 1 deletion powerauth-admin/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
<parent>
<groupId>io.getlime.security</groupId>
<artifactId>powerauth-server-parent</artifactId>
<version>1.8.2</version>
<version>1.8.3</version>
</parent>

<dependencies>
Expand Down
2 changes: 1 addition & 1 deletion powerauth-client-model/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
<parent>
<groupId>io.getlime.security</groupId>
<artifactId>powerauth-server-parent</artifactId>
<version>1.8.2</version>
<version>1.8.3</version>
</parent>

<dependencies>
Expand Down
2 changes: 1 addition & 1 deletion powerauth-fido2-model/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
<parent>
<groupId>io.getlime.security</groupId>
<artifactId>powerauth-server-parent</artifactId>
<version>1.8.2</version>
<version>1.8.3</version>
</parent>

<artifactId>powerauth-fido2-model</artifactId>
Expand Down
2 changes: 1 addition & 1 deletion powerauth-fido2/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
<parent>
<groupId>io.getlime.security</groupId>
<artifactId>powerauth-server-parent</artifactId>
<version>1.8.2</version>
<version>1.8.3</version>
</parent>

<dependencies>
Expand Down
2 changes: 1 addition & 1 deletion powerauth-java-server/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
<parent>
<groupId>io.getlime.security</groupId>
<artifactId>powerauth-server-parent</artifactId>
<version>1.8.2</version>
<version>1.8.3</version>
</parent>

<dependencies>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -374,7 +374,7 @@ public OperationUserActionResponse attemptApproveOperation(OperationApproveReque
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 == null || 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 @@ -419,7 +419,6 @@ && proximityCheckPassed(proximityCheckResult)
final Long maxFailureCount = operationEntity.getMaxFailureCount();

if (failureCount < maxFailureCount) {
operationEntity.setUserId(userId);
operationEntity.setFailureCount(failureCount);
operationEntity.setAdditionalData(mapMerge(operationEntity.getAdditionalData(), additionalData));

Expand Down Expand Up @@ -450,7 +449,6 @@ && proximityCheckPassed(proximityCheckResult)
response.setOperation(operationDetailResponse);
return response;
} else {
operationEntity.setUserId(userId);
operationEntity.setStatus(OperationStatusDo.FAILED);
operationEntity.setTimestampFinalized(currentTimestamp);
operationEntity.setFailureCount(maxFailureCount); // just in case, set the failure count to max value
Expand Down Expand Up @@ -535,7 +533,7 @@ public OperationUserActionResponse rejectOperation(OperationRejectRequest reques
}

final String expectedUserId = operationEntity.getUserId();
if (expectedUserId == null || expectedUserId.equals(userId) // correct user rejects the operation
if ((expectedUserId == null || expectedUserId.equals(userId)) // correct user rejects the operation
&& operationEntity.getApplications().contains(application.get())) { // operation is rejected by the expected application

// Reject the operation
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,10 @@ public AssertionChallenge approveAssertion(String challengeValue, AuthenticatorD
@SuppressWarnings("unchecked")
final List<String> allowCredentials = (List<String>) operationEntity.getAdditionalData().get(ATTR_ALLOW_CREDENTIALS);
final String credentialId = (String) request.getAdditionalData().get(ATTR_CREDENTIAL_ID);
return allowCredentials == null || allowCredentials.isEmpty() || allowCredentials.contains(credentialId);
final boolean allowCredentialsMatches = allowCredentials == null // no allow credentials at all are expected (null)
|| allowCredentials.isEmpty() // no allow credentials at all are expected (empty collection)
|| allowCredentials.contains(credentialId); // used credential matches one of expected values
return !allowCredentialsMatches;
});
final UserActionResult result = approveOperation.getResult();
final OperationDetailResponse operation = approveOperation.getOperation();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -544,7 +544,7 @@ private void createOperationTemplate() throws Exception {
templateCreateRequest.setDataTemplate("A2");
templateCreateRequest.setMaxFailureCount(5L);
templateCreateRequest.setExpiration(300L);
templateCreateRequest.getSignatureType().add(SignatureType.POSSESSION_KNOWLEDGE);
templateCreateRequest.getSignatureType().add(SignatureType.POSSESSION);
operationTemplateService.createOperationTemplate(templateCreateRequest);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -699,6 +699,108 @@ void testParsingDeviceOperationCancelDetail() throws Exception {
assertEquals(expectedDevice, detailResponse.getAdditionalData().get("device"));
}

@Test
void testAnonymousOperationApprovedUserChanged() throws GenericServiceException {
final OperationCreateRequest operationCreateRequest = new OperationCreateRequest();
operationCreateRequest.setApplications(List.of("PA_Tests"));
operationCreateRequest.setTemplateName("test-template");
final OperationDetailResponse operation = operationService.createOperation(operationCreateRequest);
final OperationApproveRequest approveRequest = new OperationApproveRequest();
approveRequest.setOperationId(operation.getId());
approveRequest.setUserId("test_user");
approveRequest.setData("A2");
approveRequest.setApplicationId("PA_Tests");
approveRequest.setSignatureType(SignatureType.POSSESSION_KNOWLEDGE);
final OperationUserActionResponse response = operationService.attemptApproveOperation(approveRequest);
assertEquals(UserActionResult.APPROVED, response.getResult());
final OperationDetailRequest detailRequest = new OperationDetailRequest();
detailRequest.setOperationId(operation.getId());
final OperationDetailResponse operationDetail = operationService.operationDetail(detailRequest);
assertEquals("test_user", operationDetail.getUserId());
}

@Test
void testAnonymousOperationFailedApproveUserNotChanged() throws GenericServiceException {
final OperationCreateRequest operationCreateRequest = new OperationCreateRequest();
operationCreateRequest.setApplications(List.of("PA_Tests"));
operationCreateRequest.setTemplateName("test-template");
final OperationDetailResponse operation = operationService.createOperation(operationCreateRequest);
final OperationApproveRequest approveRequest = new OperationApproveRequest();
approveRequest.setOperationId(operation.getId());
approveRequest.setUserId("invalid_user");
approveRequest.setData("invalid_data");
approveRequest.setApplicationId("PA_Tests");
approveRequest.setSignatureType(SignatureType.POSSESSION_KNOWLEDGE);
final OperationUserActionResponse response = operationService.attemptApproveOperation(approveRequest);
assertEquals(UserActionResult.APPROVAL_FAILED, response.getResult());
final OperationDetailRequest detailRequest = new OperationDetailRequest();
detailRequest.setOperationId(operation.getId());
final OperationDetailResponse operationDetail = operationService.operationDetail(detailRequest);
assertNull(operationDetail.getUserId());
}

@Test
void testAnonymousOperationFailedOperationUserNotChanged() throws GenericServiceException {
final OperationCreateRequest operationCreateRequest = new OperationCreateRequest();
operationCreateRequest.setApplications(List.of("PA_Tests"));
operationCreateRequest.setTemplateName("test-template");
final OperationDetailResponse operation = operationService.createOperation(operationCreateRequest);
for (int i = 0; i < 5; i++) {
final OperationApproveRequest approveRequest = new OperationApproveRequest();
approveRequest.setOperationId(operation.getId());
approveRequest.setUserId("invalid_user");
approveRequest.setData("invalid_data");
approveRequest.setApplicationId("PA_Tests");
approveRequest.setSignatureType(SignatureType.POSSESSION_KNOWLEDGE);
final OperationUserActionResponse response = operationService.attemptApproveOperation(approveRequest);
if (i == 4) {
assertEquals(UserActionResult.OPERATION_FAILED, response.getResult());
} else {
assertEquals(UserActionResult.APPROVAL_FAILED, response.getResult());
}
}
final OperationDetailRequest detailRequest = new OperationDetailRequest();
detailRequest.setOperationId(operation.getId());
final OperationDetailResponse operationDetail = operationService.operationDetail(detailRequest);
assertNull(operationDetail.getUserId());
}

@Test
void testAnonymousOperationRejectUserChanged() throws GenericServiceException {
final OperationCreateRequest operationCreateRequest = new OperationCreateRequest();
operationCreateRequest.setApplications(List.of("PA_Tests"));
operationCreateRequest.setTemplateName("test-template");
final OperationDetailResponse operation = operationService.createOperation(operationCreateRequest);
final OperationRejectRequest rejectRequest = new OperationRejectRequest();
rejectRequest.setOperationId(operation.getId());
rejectRequest.setUserId("test_user");
rejectRequest.setApplicationId("PA_Tests");
final OperationUserActionResponse response = operationService.rejectOperation(rejectRequest);
assertEquals(UserActionResult.REJECTED, response.getResult());
final OperationDetailRequest detailRequest = new OperationDetailRequest();
detailRequest.setOperationId(operation.getId());
final OperationDetailResponse operationDetail = operationService.operationDetail(detailRequest);
assertEquals("test_user", operationDetail.getUserId());
}

@Test
void testAnonymousOperationRejectFailedUserNotChanged() throws GenericServiceException {
final OperationCreateRequest operationCreateRequest = new OperationCreateRequest();
operationCreateRequest.setApplications(List.of("PA_Tests"));
operationCreateRequest.setTemplateName("test-template");
final OperationDetailResponse operation = operationService.createOperation(operationCreateRequest);
final OperationRejectRequest rejectRequest = new OperationRejectRequest();
rejectRequest.setOperationId(operation.getId());
rejectRequest.setUserId("test_user");
rejectRequest.setApplicationId(APP_ID);
final OperationUserActionResponse response = operationService.rejectOperation(rejectRequest);
assertEquals(UserActionResult.REJECT_FAILED, response.getResult());
final OperationDetailRequest detailRequest = new OperationDetailRequest();
detailRequest.setOperationId(operation.getId());
final OperationDetailResponse operationDetail = operationService.operationDetail(detailRequest);
assertNull(operationDetail.getUserId());
}

private void createApplication() throws GenericServiceException {
boolean appExists = applicationService.getApplicationList().getApplications().stream()
.anyMatch(app -> app.getApplicationId().equals(APP_ID));
Expand Down
2 changes: 1 addition & 1 deletion powerauth-rest-client-spring/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
<parent>
<groupId>io.getlime.security</groupId>
<artifactId>powerauth-server-parent</artifactId>
<version>1.8.2</version>
<version>1.8.3</version>
</parent>

<dependencies>
Expand Down

0 comments on commit 698055c

Please sign in to comment.