Skip to content

Commit

Permalink
Merge pull request #75 from VimukthiRajapaksha/main_internal_951
Browse files Browse the repository at this point in the history 
Prevent persisting duplicate consent mappings
  • Loading branch information
@VimukthiRajapaksha
VimukthiRajapaksha authored Jan 9, 2025
2 parents 608586e + e96c7d0 commit 24df934
Show file tree
Hide file tree
Showing 4 changed files with 131 additions and 67 deletions.
111 changes: 78 additions & 33 deletions ...wso2/openbanking/cds/consent/extensions/authorize/impl/persist/CDSConsentPersistStep.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/**
* Copyright (c) 2024, WSO2 LLC. (https://www.wso2.com).
* Copyright (c) 2024-2025, WSO2 LLC. (https://www.wso2.com).
*
* WSO2 LLC. licenses this file to you under the Apache License,
* Version 2.0 (the "License"); you may not use this file except
Expand Down Expand Up @@ -54,6 +54,7 @@
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
import java.util.stream.Collectors;


/**
Expand Down Expand Up @@ -186,8 +187,7 @@ public void execute(ConsentPersistData consentPersistData) throws ConsentExcepti
Map<String, Object> additionalAmendmentData = new HashMap<>();
if (nonPrimaryAccountIdAgainstUsersObj != null && userIdAgainstNonPrimaryAccountsObj != null) {
additionalAmendmentData = bindNonPrimaryAccountUsersToConsent(consentResource, consentData,
(Map<String, Map<String, String>>) nonPrimaryAccountIdAgainstUsersObj,
(Map<String, ArrayList<String>>) userIdAgainstNonPrimaryAccountsObj, true);
(Map<String, Map<String, List<String>>>) userIdAgainstNonPrimaryAccountsObj, true);
}
consentCoreService.amendDetailedConsent(cdrArrangementId, consentResource.getReceipt(),
validityPeriod, authResorceId, accountIdsMapWithPermissions,
Expand Down Expand Up @@ -239,8 +239,7 @@ public void execute(ConsentPersistData consentPersistData) throws ConsentExcepti
// bind user consented accounts with the created consent for non-primary users
if (nonPrimaryAccountIdAgainstUsersObj != null && userIdAgainstNonPrimaryAccountsObj != null) {
bindNonPrimaryAccountUsersToConsent(consentResource, consentData,
(Map<String, Map<String, String>>) nonPrimaryAccountIdAgainstUsersObj,
(Map<String, ArrayList<String>>) userIdAgainstNonPrimaryAccountsObj, false);
(Map<String, Map<String, List<String>>>) userIdAgainstNonPrimaryAccountsObj, false);
}
}

Expand Down Expand Up @@ -447,19 +446,18 @@ private ArrayList<String> getAccountIdList(JSONObject payloadData) throws Consen
}

@Generated(message = "Excluding from code coverage since it requires a service call")
private Map<String, Object> bindNonPrimaryAccountUsersToConsent(ConsentResource consentResource,
ConsentData consentData,
Map<String, Map<String, String>> nonPrimaryAccountIdAgainstUsers,
Map<String, ArrayList<String>> userIdAgainstNonPrimaryAccounts,
boolean isConsentAmendment)
throws ConsentManagementException {
private Map<String, Object> bindNonPrimaryAccountUsersToConsent(
ConsentResource consentResource,
ConsentData consentData,
Map<String, Map<String, List<String>>> userIdAgainstNonPrimaryAccounts,
boolean isConsentAmendment) throws ConsentManagementException {

List<String> addedUserAuthTypeMappings = new ArrayList<>();
// add primary user to already added users auth type mapping list
addedUserAuthTypeMappings.add(consentData.getUserId() + ":" +
CDSConsentExtensionConstants.AUTH_RESOURCE_TYPE_PRIMARY);
// Users who have already stored as auth resources
Map<String, AuthorizationResource> reAuthorizableResources = new HashMap<>();
List<String> reAuthorizableResources = new ArrayList<>();
// Users who need to store as auth resources
Map<String, List<String>> authorizableResources = new HashMap<>();

Expand All @@ -468,23 +466,19 @@ private Map<String, Object> bindNonPrimaryAccountUsersToConsent(ConsentResource
: consentData.getConsentId();
DetailedConsentResource detailedConsent = consentCoreService.getDetailedConsent(consentId);

for (Entry<String, Map<String, String>> entry : nonPrimaryAccountIdAgainstUsers.entrySet()) {
Map<String, String> userIdList = entry.getValue();
for (Entry userEntry : userIdList.entrySet()) {
String userId = userEntry.getKey().toString();
String authType = userEntry.getValue().toString();
for (Map.Entry<String, Map<String, List<String>>> userEntry : userIdAgainstNonPrimaryAccounts.entrySet()) {

final String userId = userEntry.getKey();
for (Map.Entry<String, List<String>> accountEntry : userEntry.getValue().entrySet()) {
final String authType = accountEntry.getKey();
String userAuthTypeMapping = userId + ":" + authType;
if (StringUtils.isNotBlank(userId) && !addedUserAuthTypeMappings.contains(userAuthTypeMapping)) {

if (isConsentAmendment) {
for (AuthorizationResource authResource : detailedConsent.getAuthorizationResources()) {
if (userId.equals(authResource.getUserID())) {
reAuthorizableResources.put(userId, authResource);
}
}
if (isConsentAmendment && isReauthorizable(userId, accountEntry.getValue(), detailedConsent)) {
reAuthorizableResources.add(userId);
}

if (!reAuthorizableResources.containsKey(userId)) {
if (!reAuthorizableResources.contains(userId)) {
authorizableResources.computeIfAbsent(userId, k -> new ArrayList<>()).add(authType);
}

Expand All @@ -498,11 +492,11 @@ private Map<String, Object> bindNonPrimaryAccountUsersToConsent(ConsentResource
}

@Generated(message = "Excluding from code coverage since it requires a service call")
private Map<String, Object> createNewAuthResources(String consentId, ConsentResource consentResource,
Map<String, List<String>> authorizableResources,
Map<String, ArrayList<String>> userIdAgainstNonPrimaryAccounts,
boolean isConsentAmendment)
throws ConsentManagementException {
private Map<String, Object> createNewAuthResources(
String consentId, ConsentResource consentResource,
Map<String, List<String>> authorizableResources,
Map<String, Map<String, List<String>>> userIdAgainstNonPrimaryAccounts,
boolean isConsentAmendment) throws ConsentManagementException {

Map<String, Object> additionalAmendmentData = new HashMap<>();
Map<String, AuthorizationResource> secondaryUserAuthResources = new HashMap<>();
Expand All @@ -522,15 +516,23 @@ private Map<String, Object> createNewAuthResources(String consentId, ConsentReso
// AuthorizationResources, ConsentMappingResources against the userId and returned.
secondaryUserAuthResources.put(userId, secondaryAuthResource);

for (String accountId : userIdAgainstNonPrimaryAccounts.get(userId)) {
for (String accountId : userIdAgainstNonPrimaryAccounts.get(userId).get(authType)) {
mappingResources.add(getSecondaryConsentMappingResource(accountId));
}
secondaryUserAccountMappings.put(userId, mappingResources);
} else {
AuthorizationResource authorizationResource = consentCoreService
.createConsentAuthorization(secondaryAuthResource);
final AuthorizationResource authorizationResource =
consentCoreService.createConsentAuthorization(secondaryAuthResource);

if (log.isDebugEnabled()) {
log.debug(String.format("Inserting consented accounts for authorizationId: %s, " +
"consentId: %s, authType %s", authorizationResource.getAuthorizationID(),
consentResource.getConsentID(), authType));
}

consentCoreService.bindUserAccountsToConsent(consentResource, userId,
authorizationResource.getAuthorizationID(), userIdAgainstNonPrimaryAccounts.get(userId),
authorizationResource.getAuthorizationID(),
new ArrayList<>(userIdAgainstNonPrimaryAccounts.get(userId).get(authType)),
CDSConsentExtensionConstants.AUTHORIZED_STATUS,
CDSConsentExtensionConstants.AUTHORIZED_STATUS);
}
Expand Down Expand Up @@ -674,4 +676,47 @@ private void publishConsentApprovalStatus(ConsentPersistData consentPersistData)
CDSDataPublishingService.getCDSDataPublishingService()
.publishAbandonedConsentFlowData(abandonedConsentFlowData);
}

/**
* Checks if the given mapping resource is re-authorizable based on the user ID and account IDs.
*
* @param newUserId the ID of the user to check.
* @param newlyConsentedAccountIDs the list of account IDs to validate.
* @param detailedConsent the detailed consent object containing authorization and mapping resources.
* @return true if the resource is re-authorizable, false otherwise.
*/
private boolean isReauthorizable(String newUserId, List<String> newlyConsentedAccountIDs,
DetailedConsentResource detailedConsent) {

final List<String> existingUserIDs = detailedConsent.getAuthorizationResources().stream()
.map(AuthorizationResource::getUserID).collect(Collectors.toList());

if (!existingUserIDs.contains(newUserId)) {
if (log.isDebugEnabled()) {
log.debug(String.format("authorize user %s for consent %s", newUserId, detailedConsent.getConsentID()));
}
return false;
}

final List<String> existingAuthIDs = detailedConsent.getAuthorizationResources().stream()
.filter(auth -> auth.getUserID().equals(newUserId))
.map(AuthorizationResource::getAuthorizationID)
.collect(Collectors.toList());

final List<String> existingAccountIDs = detailedConsent.getConsentMappingResources().stream()
.filter(mapping -> existingAuthIDs.contains(mapping.getAuthorizationID()))
.map(ConsentMappingResource::getAccountID)
.collect(Collectors.toList());

// Return false if any accountID matches an authorization ID
if (existingAccountIDs.stream().noneMatch(newlyConsentedAccountIDs::contains)) {
if (log.isDebugEnabled()) {
log.debug(String.format("authorize user %s for consent %s",
newUserId, detailedConsent.getConsentID()));
}
return false;
}

return true;
}
}
67 changes: 43 additions & 24 deletions ...va/org/wso2/openbanking/cds/consent/extensions/authorize/utils/CDSConsentPersistUtil.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/**
* Copyright (c) 2024, WSO2 LLC. (https://www.wso2.com).
* Copyright (c) 2024-2025, WSO2 LLC. (https://www.wso2.com).
*
* WSO2 LLC. licenses this file to you under the Apache License,
* Version 2.0 (the "License"); you may not use this file except
Expand Down Expand Up @@ -53,7 +53,7 @@ public static void addNonPrimaryAccountDataToPersistData(
ConsentPersistData consentPersistData) {

Map<String, Map<String, String>> currentNonPrimaryAccountIdUsersMap = new HashMap<>();
Map<String, List<String>> currentUserIdNonPrimaryAccountsMap = new HashMap<>();
Map<String, Map<String, List<String>>> currentUserIdNonPrimaryAccountsMap = new HashMap<>();

//Get existing non-primary account data from consentPersistData
if (consentPersistData.getMetadata().get(CDSConsentExtensionConstants.
Expand All @@ -67,8 +67,8 @@ public static void addNonPrimaryAccountDataToPersistData(

if (consentPersistData.getMetadata().get(CDSConsentExtensionConstants.
USER_ID_AGAINST_NON_PRIMARY_ACCOUNTS_MAP) != null) {
currentUserIdNonPrimaryAccountsMap = (Map<String, List<String>>) consentPersistData.getMetadata().
get(CDSConsentExtensionConstants.USER_ID_AGAINST_NON_PRIMARY_ACCOUNTS_MAP);
currentUserIdNonPrimaryAccountsMap = (Map<String, Map<String, List<String>>>) consentPersistData
.getMetadata().get(CDSConsentExtensionConstants.USER_ID_AGAINST_NON_PRIMARY_ACCOUNTS_MAP);
} else {
log.debug("UserIds against non-primary accountId map not available in consentPersistData. " +
"Creating new map");
Expand All @@ -85,15 +85,26 @@ public static void addNonPrimaryAccountDataToPersistData(

// Update user against accounts map
// Get UserId against non-primary accounts Map from current persistence Step
Map<String, List<String>> userIdNonPrimaryAccountsMap = getUserIdAgainstAccountsMap(
nonPrimaryAccountIdUsersMap);
for (Map.Entry<String, List<String>> entry : userIdNonPrimaryAccountsMap.entrySet()) {
final String userId = entry.getKey();
final List<String> nonPrimaryAccounts = entry.getValue();
if (currentUserIdNonPrimaryAccountsMap.containsKey(userId)) {
currentUserIdNonPrimaryAccountsMap.get(userId).addAll(nonPrimaryAccounts);
} else {
currentUserIdNonPrimaryAccountsMap.put(userId, nonPrimaryAccounts);
Map<String, Map<String, List<String>>> userIdNonPrimaryAccountsMap =
getUserIdAgainstAccountsMap(nonPrimaryAccountIdUsersMap);

for (Map.Entry<String, Map<String, List<String>>> userAndAccounts : userIdNonPrimaryAccountsMap.entrySet()) {
final String userId = userAndAccounts.getKey();
for (Map.Entry<String, List<String>> authTypeAndNonPrimaryAccount : userAndAccounts.getValue().entrySet()) {
final String authType = authTypeAndNonPrimaryAccount.getKey();
final List<String> nonPrimaryAccounts = authTypeAndNonPrimaryAccount.getValue();

if (currentUserIdNonPrimaryAccountsMap.containsKey(userId)) {
if (currentUserIdNonPrimaryAccountsMap.get(userId).containsKey(authType)) {
currentUserIdNonPrimaryAccountsMap.get(userId).get(authType).addAll(nonPrimaryAccounts);
} else {
currentUserIdNonPrimaryAccountsMap.get(userId).put(authType, nonPrimaryAccounts);
}
} else {
currentUserIdNonPrimaryAccountsMap.put(userId, new HashMap<String, List<String>>() { {
put(authType, nonPrimaryAccounts);
}});
}
}
}

Expand All @@ -110,19 +121,27 @@ public static void addNonPrimaryAccountDataToPersistData(
* @param accountIdUserMap Map of accountId against userIds
* @return a map of userId against a list of accountIds
*/
private static Map<String, List<String>> getUserIdAgainstAccountsMap(Map<String,
Map<String, String>> accountIdUserMap) {

Map<String, List<String>> userIdAccountsMap = new HashMap<>();
for (Map.Entry<String, Map<String, String>> entry : accountIdUserMap.entrySet()) {
final String accountId = entry.getKey();
final List<String> userIdList = new ArrayList<>(entry.getValue().keySet());
for (String userId : userIdList) {
private static Map<String, Map<String, List<String>>> getUserIdAgainstAccountsMap(
Map<String, Map<String, String>> accountIdUserMap) {

Map<String, Map<String, List<String>>> userIdAccountsMap = new HashMap<>();
for (Map.Entry<String, Map<String, String>> accountIdUsers : accountIdUserMap.entrySet()) {
final String accountId = accountIdUsers.getKey();
for (Map.Entry<String, String> userIdAndAuthType : accountIdUsers.getValue().entrySet()) {
final String userId = userIdAndAuthType.getKey();
final String authType = userIdAndAuthType.getValue();

if (userIdAccountsMap.containsKey(userId)) {
userIdAccountsMap.get(userId).add(accountId);
if (userIdAccountsMap.get(userId).containsKey(authType)) {
userIdAccountsMap.get(userId).get(authType).add(accountId);
} else {
userIdAccountsMap.get(userId)
.put(authType, new ArrayList<>(Collections.singletonList(accountId)));
}
} else {
userIdAccountsMap.put(userId, new ArrayList<>(Collections.
singletonList(accountId)));
userIdAccountsMap.put(userId, new HashMap<String, List<String>>() { {
put(authType, new ArrayList<>(Collections.singletonList(accountId)));
}});
}
}
}
Expand Down
10 changes: 5 additions & 5 deletions ...nsent/extensions/authorize/impl/persist/CDSBusinessAccountConsentPersistenceStepTest.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/**
* Copyright (c) 2024, WSO2 LLC. (https://www.wso2.com).
* Copyright (c) 2024-2025, WSO2 LLC. (https://www.wso2.com).
*
* WSO2 LLC. licenses this file to you under the Apache License,
* Version 2.0 (the "License"); you may not use this file except
Expand Down Expand Up @@ -165,8 +165,8 @@ public void testExecute() throws Exception {

verify(consentPersistDataMock).addMetadata(eq(CDSConsentExtensionConstants.
USER_ID_AGAINST_NON_PRIMARY_ACCOUNTS_MAP), usersWithMultipleBusinessAccountsCaptor.capture());
Map<String, List<String>> usersWithMultipleJointAccountsCaptorValue =
(Map<String, List<String>>) usersWithMultipleBusinessAccountsCaptor.getValue();
Map<String, Map<String, List<String>>> usersWithMultipleJointAccountsCaptorValue =
(Map<String, Map<String, List<String>>>) usersWithMultipleBusinessAccountsCaptor.getValue();

assertTrue(jointAccountIdWithUsersCaptorValue.containsKey("business-account-id"));
assertTrue(jointAccountIdWithUsersCaptorValue.get("business-account-id").
Expand All @@ -177,8 +177,8 @@ public void testExecute() throws Exception {
assertTrue(usersWithMultipleJointAccountsCaptorValue.containsKey("[email protected]@carbon.super"));
assertTrue(usersWithMultipleJointAccountsCaptorValue.containsKey("[email protected]@carbon.super"));
assertTrue(usersWithMultipleJointAccountsCaptorValue.get("[email protected]@carbon.super")
.contains("business-account-id"));
.get(CDSConsentExtensionConstants.NOMINATED_REPRESENTATIVE).contains("business-account-id"));
assertTrue(usersWithMultipleJointAccountsCaptorValue.get("[email protected]@carbon.super")
.contains("business-account-id"));
.get(CDSConsentExtensionConstants.BUSINESS_ACCOUNT_OWNER).contains("business-account-id"));
}
}
Loading

0 comments on commit 24df934

Please sign in to comment.