Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed Issue in idempotency validation for APIs without a payload #57

Merged
merged 2 commits into from
Apr 26, 2024
Merged

Fixed Issue in idempotency validation for APIs without a payload #57

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5b0da62
Fixed Issue in idempotency validation for APIs without a payload
Ashi1993 Apr 25, 2024
b97b4cf
Fixed review comments
Ashi1993 Apr 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions ...2/openbanking/accelerator/consent/extensions/common/idempotency/IdempotencyConstants.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ public class IdempotencyConstants {
public static final String X_IDEMPOTENCY_KEY = "x-idempotency-key";
public static final String IDEMPOTENCY_KEY_NAME = "IdempotencyKey";
public static final String ISO_FORMAT = "yyyy-MM-dd'T'HH:mm:ssXXX";
public static final String EMPTY_OBJECT = "{}";
public static final String ERROR_PAYLOAD_NOT_SIMILAR = "Payloads are not similar. Hence this is not a valid" +
" idempotent request";
public static final String ERROR_AFTER_ALLOWED_TIME = "Request received after the allowed time., Hence this is" +
Expand All @@ -36,4 +37,6 @@ public class IdempotencyConstants {
" is not a valid idempotent request";
public static final String JSON_COMPARING_ERROR = "Error occurred while comparing JSON payloads";
public static final String CONSENT_RETRIEVAL_ERROR = "Error while retrieving detailed consent data";
public static final String SAME_CONSENT_ID_ERROR = "Cannot use different unique identifier for the same" +
" consent ID when the request does not contain a payload.";
}
17 changes: 17 additions & 0 deletions ...banking/accelerator/consent/extensions/common/idempotency/IdempotencyValidationUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@
import java.time.OffsetDateTime;
import java.util.ArrayList;
import java.util.Date;
import java.util.HashMap;
import java.util.Map;

/**
* Class to hold idempotency validation utils.
Expand Down Expand Up @@ -60,6 +62,21 @@ static ArrayList<String> getConsentIdsFromIdempotencyKey(String idempotencyKeyNa
}
}

/**
* Method to retrieve the consent ids and idempotency key value using the idempotency key.
*
* @param idempotencyKeyName Idempotency Key Name
* @return Map of consent ids and idempotency key vallue if available, else an empty map will be returned
*/
static Map<String, String> getAttributesFromIdempotencyKey(String idempotencyKeyName) {
try {
return consentCoreService.getConsentAttributesByName(idempotencyKeyName);
} catch (ConsentManagementException e) {
log.debug("No consent ids found for the idempotency key value");
return new HashMap<>();
}
}

/**
* Method to compare the client ID sent in the request and client id retrieved from the database.
*
Expand Down
88 changes: 70 additions & 18 deletions ...2/openbanking/accelerator/consent/extensions/common/idempotency/IdempotencyValidator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@

import java.io.IOException;
import java.util.ArrayList;
import java.util.Map;

/**
* Class to handle idempotency related operations.
Expand Down Expand Up @@ -81,26 +82,30 @@ public IdempotencyValidationResult validateIdempotency(ConsentManageData consent
}
try {
String idempotencyKeyName = getIdempotencyAttributeName(consentManageData.getRequestPath());
// Retrieve consent ids that have the idempotency key name and value as attribute
ArrayList<String> consentIds = IdempotencyValidationUtils
.getConsentIdsFromIdempotencyKey(idempotencyKeyName, idempotencyKeyValue);
// Check whether the consent id list is not empty. If idempotency key exists in the database then
// the consent Id list will be not empty.
if (!consentIds.isEmpty()) {
if (log.isDebugEnabled()) {
log.debug(String.format("Idempotency Key %s exists in the database. Hence this is an" +
" idempotent request", idempotencyKeyValue));
}
for (String consentId : consentIds) {
DetailedConsentResource consentRequest = consentCoreService.getDetailedConsent(consentId);
if (consentRequest != null) {
return validateIdempotencyConditions(consentManageData, consentRequest);
} else {
String errorMsg = String.format(IdempotencyConstants.ERROR_NO_CONSENT_DETAILS, consentId);
log.error(errorMsg);
throw new IdempotencyValidationException(errorMsg);
if (!IdempotencyConstants.EMPTY_OBJECT.equals(consentManageData.getPayload().toString())) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couldn't this be null? consentManageData.getPayload()

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is checked upfront in line 68

aka4rKO marked this conversation as resolved.
Show resolved Hide resolved
// Retrieve consent ids that have the idempotency key name and value as attribute
ArrayList<String> consentIds = IdempotencyValidationUtils
Ashi1993 marked this conversation as resolved.
Show resolved Hide resolved
.getConsentIdsFromIdempotencyKey(idempotencyKeyName, idempotencyKeyValue);
// Check whether the consent id list is not empty. If idempotency key exists in the database then
// the consent Id list will be not empty.
if (!consentIds.isEmpty()) {
if (log.isDebugEnabled()) {
log.debug(String.format("Idempotency Key %s exists in the database. Hence this is an" +
" idempotent request", idempotencyKeyValue));
}
for (String consentId : consentIds) {
DetailedConsentResource consentRequest = consentCoreService.getDetailedConsent(consentId);
Ashi1993 marked this conversation as resolved.
Show resolved Hide resolved
if (consentRequest != null) {
return validateIdempotencyConditions(consentManageData, consentRequest);
} else {
String errorMsg = String.format(IdempotencyConstants.ERROR_NO_CONSENT_DETAILS, consentId);
log.error(errorMsg);
throw new IdempotencyValidationException(errorMsg);
}
}
}
} else {
return validateIdempotencyWithoutPayload(consentManageData, idempotencyKeyName, idempotencyKeyValue);
}
} catch (IOException e) {
log.error(IdempotencyConstants.JSON_COMPARING_ERROR, e);
Expand All @@ -112,6 +117,53 @@ public IdempotencyValidationResult validateIdempotency(ConsentManageData consent
return new IdempotencyValidationResult(false, false);
}

/**
* Method to check whether the idempotency conditions are met for requests without payload.
* This method will validate the following conditions.
* - Whether the idempotency key value is different for the same consent id
* - Whether the client id sent in the request and client id retrieved from the database are equal
* - Whether the difference between two dates is less than the configured time
* - Whether payloads are equal
*
* @param consentManageData Consent Manage Data
* @param idempotencyKeyName Idempotency Key Name
* @param idempotencyKeyValue Idempotency Key value
* @return IdempotencyValidationResult
*/
private IdempotencyValidationResult validateIdempotencyWithoutPayload(ConsentManageData consentManageData,
String idempotencyKeyName,
String idempotencyKeyValue)
throws IdempotencyValidationException, IOException, ConsentManagementException {

// Retrieve consent ids and idempotency key values that have the idempotency key name
Map<String, String> attributes = IdempotencyValidationUtils.getAttributesFromIdempotencyKey(idempotencyKeyName);
// Check whether the attributes map is not empty. If idempotency key exists in the database then
// the consent Id list will be not empty.
if (!attributes.isEmpty()) {
if (log.isDebugEnabled()) {
log.debug(String.format("Idempotency Key %s exists in the database. Hence this is an" +
" idempotent request", idempotencyKeyValue));
}
for (Map.Entry<String, String> entry : attributes.entrySet()) {
// If the idempotency key value is different for the same consent id then it is not a valid idempotent
if (consentManageData.getRequestPath().contains(entry.getKey()) &&
!idempotencyKeyValue.equals(entry.getValue())) {
throw new IdempotencyValidationException(IdempotencyConstants.SAME_CONSENT_ID_ERROR);
}
DetailedConsentResource consentRequest = consentCoreService.getDetailedConsent(entry.getKey());
if (consentRequest != null) {
return validateIdempotencyConditions(consentManageData, consentRequest);
} else {
String errorMsg = String.format(IdempotencyConstants.ERROR_NO_CONSENT_DETAILS, entry.getKey());
log.error(errorMsg);
throw new IdempotencyValidationException(errorMsg);
}
}

}
return new IdempotencyValidationResult(false, false);
}

/**
* Method to check whether the idempotency conditions are met.
* This method will validate the following conditions.
Expand Down
20 changes: 20 additions & 0 deletions ...nbanking/accelerator/consent/extensions/common/idempotency/IdempotencyValidatorTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ public class IdempotencyValidatorTests extends PowerMockTestCase {
private ConsentManageData consentManageData;
private ConsentCoreServiceImpl consentCoreServiceImpl;
private ArrayList<String> consentIdList;
private Map<String, String> attributeList;
private String consentId;
private Map<String, Object> configs;
private Map<String, String> headers;
Expand Down Expand Up @@ -126,6 +127,9 @@ public void beforeTest() {
consentId = UUID.randomUUID().toString();
consentIdList = new ArrayList<>();
consentIdList.add(consentId);

attributeList = new HashMap<>();
attributeList.put(consentId, "123456");
}

@BeforeMethod
Expand Down Expand Up @@ -164,6 +168,22 @@ public void testValidateIdempotency() throws ConsentManagementException, Idempot
Assert.assertEquals(consentId, result.getConsentId());
}

@Test(expectedExceptions = IdempotencyValidationException.class)
public void testValidateIdempotencyForRequestsWithoutPayload() throws ConsentManagementException,
IdempotencyValidationException {
OffsetDateTime offsetDateTime = OffsetDateTime.now();

Mockito.doReturn(attributeList).when(consentCoreServiceImpl).getConsentAttributesByName(Mockito.anyString());
Mockito.doReturn(getConsent(offsetDateTime.toEpochSecond())).when(consentCoreServiceImpl)
.getDetailedConsent(Mockito.anyString());
Mockito.doReturn(headers).when(consentManageData).getHeaders();
Mockito.doReturn(CLIENT_ID).when(consentManageData).getClientId();
Mockito.doReturn("{}").when(consentManageData).getPayload();
Mockito.doReturn("{}").when(consentManageData).getPayload();
Mockito.doReturn("/payments/".concat(consentId)).when(consentManageData).getRequestPath();
new IdempotencyValidator().validateIdempotency(consentManageData);
}

@Test
public void testValidateIdempotencyWithoutIdempotencyKeyValue() throws IdempotencyValidationException {

Expand Down
Loading