-
Notifications
You must be signed in to change notification settings - Fork 414
Enable modification of sub-attributes in complex OIDC claims & fix org switch default resolution in pre-issue id token action #3026
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.
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
base: master
Are you sure you want to change the base?
Changes from 8 commits
2f36a50
022c723
fddad67
f342e8f
8345297
c61dd39
56f326c
e213858
fce6933
1169d0d
09a24f8
f03437f
ec3730e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -63,7 +63,6 @@ | |
| import java.util.HashMap; | ||
| import java.util.List; | ||
| import java.util.Map; | ||
| import java.util.stream.Collectors; | ||
|
|
||
| import javax.servlet.http.HttpServletRequestWrapper; | ||
|
|
||
|
|
@@ -207,24 +206,45 @@ private List<AllowedOperation> getAllowedOperations(Map<String, Object> claimsTo | |
|
|
||
| private List<String> getRemoveOrReplacePaths(Map<String, Object> oidcClaims) { | ||
|
|
||
| List<String> removeOrReplacePaths = oidcClaims.entrySet().stream() | ||
| .filter(entry -> entry.getValue() instanceof String || | ||
| entry.getValue() instanceof Number || | ||
| entry.getValue() instanceof Boolean || entry.getValue() instanceof List || | ||
| entry.getValue() instanceof String[]) | ||
| .map(this::generatePathForClaim) | ||
| .collect(Collectors.toList()); | ||
| List<String> removeOrReplacePaths = new ArrayList<>(); | ||
| for (Map.Entry<String, Object> entry : oidcClaims.entrySet()) { | ||
|
|
||
| String basePath = CLAIMS_PATH_PREFIX + entry.getKey(); | ||
| Object value = entry.getValue(); | ||
|
|
||
| removeOrReplacePaths.add(basePath); | ||
| if (value instanceof String || value instanceof Number || value instanceof Boolean) { | ||
| continue; | ||
| } | ||
| if (value instanceof List || value instanceof String[]) { | ||
| removeOrReplacePaths.add(basePath + "/"); | ||
| continue; | ||
| } | ||
|
|
||
| // Handle nested objects | ||
| if (value instanceof Map) { | ||
| collectNestedClaimPaths(basePath, value, removeOrReplacePaths); | ||
| } | ||
Lashen1227 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
| removeOrReplacePaths.add(CLAIMS_PATH_PREFIX + AccessToken.ClaimNames.AUD.getName() + "/"); | ||
| return removeOrReplacePaths; | ||
| } | ||
|
|
||
| private String generatePathForClaim(Map.Entry<String, Object> entry) { | ||
| private void collectNestedClaimPaths(String basePath, Object value, List<String> paths) { | ||
|
|
||
| if (value instanceof Map) { | ||
| Map<?, ?> map = (Map<?, ?>) value; | ||
|
|
||
| String basePath = CLAIMS_PATH_PREFIX + entry.getKey(); | ||
| if (entry.getValue() instanceof List || entry.getValue() instanceof String[]) { | ||
| basePath += "/"; | ||
| for (Map.Entry<?, ?> entry : map.entrySet()) { | ||
| if (!(entry.getKey() instanceof String)) { | ||
| continue; | ||
| } | ||
| String childPath = basePath + "/" + entry.getKey(); | ||
| paths.add(childPath); | ||
|
|
||
| collectNestedClaimPaths(childPath, entry.getValue(), paths); | ||
| } | ||
| } | ||
| return basePath; | ||
| } | ||
|
Comment on lines
+233
to
248
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nested lists within map claims won't have array-indexed allowed paths.
If nested list operations should be supported, add the trailing-slash variant here: Proposed fix private void collectNestedClaimPaths(String basePath, Object value, List<String> paths) {
if (value instanceof Map) {
Map<?, ?> map = (Map<?, ?>) value;
for (Map.Entry<?, ?> entry : map.entrySet()) {
if (!(entry.getKey() instanceof String)) {
continue;
}
String childPath = basePath + "/" + entry.getKey();
paths.add(childPath);
+ Object childValue = entry.getValue();
+ if (childValue instanceof List || childValue instanceof String[]) {
+ paths.add(childPath + "/");
+ }
collectNestedClaimPaths(childPath, entry.getValue(), paths);
}
}
}🤖 Prompt for AI Agents |
||
|
|
||
| private AllowedOperation createAllowedOperation(Operation op, List<String> paths) { | ||
|
|
@@ -298,8 +318,17 @@ private User resolveLocalUser(AuthenticatedUser authenticatedUser, String grantT | |
| userBuilder.organization(resolveUserAuthenticatedOrganization(authenticatedUser)); | ||
|
|
||
| if (OAuthConstants.GrantTypes.ORGANIZATION_SWITCH.equals(grantType)) { | ||
| userBuilder.accessingOrganization(buildOrganization(authenticatedUser.getAccessingOrganization(), | ||
| authenticatedUser.getTenantDomain())); | ||
| Organization accessingOrg; | ||
| if (authenticatedUser.getAccessingOrganization() != null) { | ||
| accessingOrg = buildOrganization(authenticatedUser.getAccessingOrganization(), | ||
| authenticatedUser.getTenantDomain()); | ||
| // In case of org switch, if accessing org is not set, it means user is switching to root org. | ||
| } else { | ||
| accessingOrg = buildOrganization( | ||
| resolveOrganizationId(authenticatedUser.getTenantDomain()), | ||
| authenticatedUser.getTenantDomain()); | ||
| } | ||
| userBuilder.accessingOrganization(accessingOrg); | ||
| } | ||
| return userBuilder.build(); | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -54,6 +54,7 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import org.wso2.carbon.utils.DiagnosticLog; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import java.util.ArrayList; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import java.util.HashMap; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import java.util.List; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import java.util.Map; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import java.util.regex.Matcher; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -351,7 +352,9 @@ private OperationExecutionResult addToOtherClaims(PerformableOperation operation | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Object claimValue = claim.getValue(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (isValidPrimitiveValue(claimValue) || isValidListValue(claimValue)) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (isValidPrimitiveValue(claimValue) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| || isValidListValue(claimValue) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| || isValidMapValue(claimValue)) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| responseIDTokenDTO.getCustomOIDCClaims().put(claim.getName(), claim.getValue()); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return new OperationExecutionResult(operation, OperationExecutionResult.Status.SUCCESS, "Claim added."); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -402,6 +405,15 @@ private boolean isValidListValue(Object value) { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return list.stream().allMatch(item -> item instanceof String); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private boolean isValidMapValue(Object value) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (!(value instanceof Map<?, ?>)) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return false; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Map<?, ?> map = (Map<?, ?>) value; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return true; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private OperationExecutionResult handleRemoveOperation(PerformableOperation operation, IDToken requestedIDToken, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| IDTokenDTO responseIDTokenDTO) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -455,9 +467,15 @@ private OperationExecutionResult removeOtherClaims(PerformableOperation operatio | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| IDToken requestIDToken, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| IDTokenDTO responseIDTokenDTO) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ClaimPathInfo claimPathInfo = parseOperationPath(operation.getPath()); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| IDToken.Claim claim = requestIDToken.getClaim(claimPathInfo.getClaimName()); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| List<String> pathSegments = extractNestedClaimPath(operation.getPath()); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Nested removal | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (pathSegments.size() > 1 && !isArrayIndexPath(pathSegments)) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return removeNestedClaim(pathSegments, requestIDToken, responseIDTokenDTO, operation); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ClaimPathInfo claimPathInfo = parseOperationPath(operation.getPath()); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| IDToken.Claim claim = requestIDToken.getClaim(claimPathInfo.getClaimName()); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (claim == null) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return new OperationExecutionResult(operation, OperationExecutionResult.Status.FAILURE, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "Claim not found."); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -471,6 +489,57 @@ private OperationExecutionResult removeOtherClaims(PerformableOperation operatio | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private OperationExecutionResult removeNestedClaim(List<String> pathSegments, IDToken requestIDToken, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| IDTokenDTO responseIDTokenDTO, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| PerformableOperation operation) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| String rootClaimName = pathSegments.get(0); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| List<String> nestedPath = pathSegments.subList(1, pathSegments.size()); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| IDToken.Claim rootClaim = requestIDToken.getClaim(rootClaimName); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (rootClaim == null || !(rootClaim.getValue() instanceof Map)) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return new OperationExecutionResult(operation, OperationExecutionResult.Status.FAILURE, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Lashen1227 marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "Root claim is not a complex object."); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Map<String, Object> rootValue = new HashMap<>((Map<String, Object>) rootClaim.getValue()); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| boolean removed = removeFromNestedMap(rootValue, nestedPath, 0); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (!removed) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Lashen1227 marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return new OperationExecutionResult(operation, OperationExecutionResult.Status.FAILURE, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "Nested claim not found."); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (rootValue.isEmpty()) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| responseIDTokenDTO.getCustomOIDCClaims().remove(rootClaimName); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| responseIDTokenDTO.getCustomOIDCClaims().put(rootClaimName, rootValue); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return new OperationExecutionResult(operation, OperationExecutionResult.Status.SUCCESS, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "Nested claim removed."); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+492
to
+520
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shallow copy of root map means nested maps are mutated in-place. Both Consider performing a deep copy instead: Example deep copy helper`@SuppressWarnings`("unchecked")
private Map<String, Object> deepCopyMap(Map<String, Object> original) {
Map<String, Object> copy = new HashMap<>();
for (Map.Entry<String, Object> entry : original.entrySet()) {
if (entry.getValue() instanceof Map) {
copy.put(entry.getKey(), deepCopyMap((Map<String, Object>) entry.getValue()));
} else {
copy.put(entry.getKey(), entry.getValue());
}
}
return copy;
}Also applies to: 664-693 🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private boolean removeFromNestedMap(Map<String, Object> current, List<String> path, int index) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| String key = path.get(index); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (index == path.size() - 1) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return current.remove(key) != null; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Object next = current.get(key); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (!(next instanceof Map)) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return false; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return removeFromNestedMap((Map<String, Object>) next, path, index + 1); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+522
to
+535
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
After removing a leaf key, any intermediate 🐛 Proposed fix — propagate empty-map cleanup on the way back up private boolean removeFromNestedMap(Map<String, Object> current, List<String> path, int index) {
String key = path.get(index);
if (index == path.size() - 1) {
return current.remove(key) != null;
}
Object next = current.get(key);
if (!(next instanceof Map)) {
return false;
}
- return removeFromNestedMap((Map<String, Object>) next, path, index + 1);
+ boolean removed = removeFromNestedMap((Map<String, Object>) next, path, index + 1);
+ Map<String, Object> nextMap = (Map<String, Object>) next;
+ if (removed && nextMap.isEmpty()) {
+ current.remove(key);
+ }
+ return removed;
}🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private List<String> extractNestedClaimPath(String operationPath) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| String relativePath = operationPath.substring(CLAIMS_PATH_PREFIX.length()); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return List.of(relativePath.split("/")); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+537
to
+541
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
A path like Proposed fix private List<String> extractNestedClaimPath(String operationPath) {
String relativePath = operationPath.substring(CLAIMS_PATH_PREFIX.length());
- return List.of(relativePath.split("/"));
+ return Arrays.stream(relativePath.split("/"))
+ .filter(s -> !s.isEmpty())
+ .collect(Collectors.toList());
}🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private OperationExecutionResult removeClaimValueAtIndexFromArrayTypeClaim(PerformableOperation operation, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ClaimPathInfo claimPathInfo, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| IDToken.Claim claim, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -570,6 +639,13 @@ private OperationExecutionResult replaceOtherClaims(PerformableOperation operati | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| IDToken requestIDToken, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| IDTokenDTO responseIDTokenDTO) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| List<String> pathSegments = extractNestedClaimPath(operation.getPath()); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Nested replace | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (pathSegments.size() > 1 && !isArrayIndexPath(pathSegments)) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return replaceNestedClaim(pathSegments, requestIDToken, responseIDTokenDTO, operation); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ClaimPathInfo claimPathInfo = parseOperationPath(operation.getPath()); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| IDToken.Claim claim = requestIDToken.getClaim(claimPathInfo.getClaimName()); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -585,6 +661,63 @@ private OperationExecutionResult replaceOtherClaims(PerformableOperation operati | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private OperationExecutionResult replaceNestedClaim(List<String> pathSegments, IDToken requestIDToken, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| IDTokenDTO responseIDTokenDTO, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| PerformableOperation operation) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| String rootClaimName = pathSegments.get(0); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| List<String> nestedPath = pathSegments.subList(1, pathSegments.size()); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| IDToken.Claim rootClaim = requestIDToken.getClaim(rootClaimName); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (rootClaim == null || !(rootClaim.getValue() instanceof Map)) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return new OperationExecutionResult(operation, OperationExecutionResult.Status.FAILURE, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "Root claim is not a complex object."); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Map<String, Object> rootValue = new HashMap<>((Map<String, Object>) rootClaim.getValue()); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| boolean replaced = replaceInNestedMap(rootValue, nestedPath, 0, operation.getValue()); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (!replaced) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return new OperationExecutionResult(operation, OperationExecutionResult.Status.FAILURE, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "Nested claim not found."); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (rootValue.isEmpty()) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| responseIDTokenDTO.getCustomOIDCClaims().remove(rootClaimName); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| responseIDTokenDTO.getCustomOIDCClaims().put(rootClaimName, rootValue); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return new OperationExecutionResult(operation, OperationExecutionResult.Status.SUCCESS, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "Nested claim replaced."); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private boolean replaceInNestedMap(Map<String, Object> current, List<String> path, int index, Object newValue) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| String key = path.get(index); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (index == path.size() - 1) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (newValue == null) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return current.remove(key) != null; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| current.put(key, newValue); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return true; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Object next = current.get(key); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (!(next instanceof Map)) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return false; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| boolean updated = replaceInNestedMap((Map<String, Object>) next, path, index + 1, newValue); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Map<String, Object> nextMap = (Map<String, Object>) next; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (updated && nextMap.isEmpty()) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| current.remove(key); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return updated; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+695
to
+719
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
At line 703, when at the leaf level ( Proposed fix: check key existence before replacing if (index == path.size() - 1) {
if (newValue == null) {
return current.remove(key) != null;
}
+ if (!current.containsKey(key)) {
+ return false;
+ }
current.put(key, newValue);
return true;
}📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private OperationExecutionResult replaceClaimValueAtIndexFromArrayTypeClaim(PerformableOperation operation, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ClaimPathInfo claimPathInfo, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| IDToken.Claim claim, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -680,6 +813,25 @@ private boolean isValidStringOrURI(String input) { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return matcher.matches(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private boolean isArrayIndexPath(List<String> pathSegments) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (pathSegments.size() != 2) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return false; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| String lastSegment = pathSegments.get(1); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (LAST_ELEMENT_CHARACTER.equals(lastSegment)) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return true; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Integer.parseInt(lastSegment); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return true; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } catch (NumberFormatException e) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return false; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+816
to
+833
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# First, let's locate and examine the file mentioned in the review
find . -type f -name "PreIssueIDTokenResponseProcessor.java" | head -5Repository: wso2-extensions/identity-inbound-auth-oauth Length of output: 254 🏁 Script executed: # Look for the methods mentioned in the review
rg -n "removeNestedClaim|replaceNestedClaim|removeFromNestedMap|replaceInNestedMap" --type java | head -20Repository: wso2-extensions/identity-inbound-auth-oauth Length of output: 2766 🏁 Script executed: # Search for test files related to this processor
fd -t f ".*PreIssueIDTokenResponseProcessor.*Test.*\.java$"Repository: wso2-extensions/identity-inbound-auth-oauth Length of output: 256 🏁 Script executed: # Search for any test that might exercise nested array paths
rg -n "address.*phones.*[0-9]|claims.*\[\d+\]" --type java | head -20Repository: wso2-extensions/identity-inbound-auth-oauth Length of output: 813 🏁 Script executed: #!/bin/bash
# Let's examine the broader context around isArrayIndexPath to understand how it's used
sed -n '750,900p' ./components/org.wso2.carbon.identity.oauth/src/main/java/org/wso2/carbon/identity/openidconnect/action/preissueidtoken/execution/PreIssueIDTokenResponseProcessor.javaRepository: wso2-extensions/identity-inbound-auth-oauth Length of output: 4583 🏁 Script executed: # Let's look at the beginning of the file to understand the path construction
head -100 ./components/org.wso2.carbon.identity.oauth/src/main/java/org/wso2/carbon/identity/openidconnect/action/preissueidtoken/execution/PreIssueIDTokenResponseProcessor.javaRepository: wso2-extensions/identity-inbound-auth-oauth Length of output: 5439 🏁 Script executed: # Let's examine the test file to see what test cases exist
wc -l ./components/org.wso2.carbon.identity.oauth/src/test/java/org/wso2/carbon/identity/openidconnect/action/preissueidtoken/execution/PreIssueIDTokenResponseProcessorTest.javaRepository: wso2-extensions/identity-inbound-auth-oauth Length of output: 263 🏁 Script executed: # Check test coverage for nested paths and arrays
rg -n "pathSegments|isArrayIndexPath" ./components/org.wso2.carbon.identity.oauth/src/test/java/org/wso2/carbon/identity/openidconnect/action/preissueidtoken/execution/PreIssueIDTokenResponseProcessorTest.java | head -30Repository: wso2-extensions/identity-inbound-auth-oauth Length of output: 69 🏁 Script executed: # Search for where pathSegments is constructed
rg -n "pathSegments.*split|split.*pathSegments" --type java -B 2 -A 2Repository: wso2-extensions/identity-inbound-auth-oauth Length of output: 2133 🏁 Script executed: #!/bin/bash
# Find where isArrayIndexPath is called
rg -n "isArrayIndexPath\|handleArrayIndexPath" ./components/org.wso2.carbon.identity.oauth/src/main/java/org/wso2/carbon/identity/openidconnect/action/preissueidtoken/execution/PreIssueIDTokenResponseProcessor.java -B 5 -A 5Repository: wso2-extensions/identity-inbound-auth-oauth Length of output: 69 🏁 Script executed: # Look for the method that constructs pathSegments to understand the flow
rg -n "toList\(\)|Arrays.asList\|new.*List.*path" ./components/org.wso2.carbon.identity.oauth/src/main/java/org/wso2/carbon/identity/openidconnect/action/preissueidtoken/execution/PreIssueIDTokenResponseProcessor.java -B 2 -A 2 | head -50Repository: wso2-extensions/identity-inbound-auth-oauth Length of output: 69 🏁 Script executed: # Look at specific line ranges around the method calls (470-520 and 640-700)
sed -n '450,530p' ./components/org.wso2.carbon.identity.oauth/src/main/java/org/wso2/carbon/identity/openidconnect/action/preissueidtoken/execution/PreIssueIDTokenResponseProcessor.javaRepository: wso2-extensions/identity-inbound-auth-oauth Length of output: 3648 🏁 Script executed: # Check how the nested path is processed in removeNestedClaim
sed -n '492,520p' ./components/org.wso2.carbon.identity.oauth/src/main/java/org/wso2/carbon/identity/openidconnect/action/preissueidtoken/execution/PreIssueIDTokenResponseProcessor.javaRepository: wso2-extensions/identity-inbound-auth-oauth Length of output: 1543 🏁 Script executed: # Search for tests involving nested paths or array indices
rg -n "phones|address.*\[|nestedPath|pathSegment" ./components/org.wso2.carbon.identity.oauth/src/test/java/org/wso2/carbon/identity/openidconnect/action/preissueidtoken/execution/PreIssueIDTokenResponseProcessorTest.java | head -30Repository: wso2-extensions/identity-inbound-auth-oauth Length of output: 69 🏁 Script executed: #!/bin/bash
# Find the extractNestedClaimPath method
rg -n "extractNestedClaimPath" ./components/org.wso2.carbon.identity.oauth/src/main/java/org/wso2/carbon/identity/openidconnect/action/preissueidtoken/execution/PreIssueIDTokenResponseProcessor.java -B 2 -A 15Repository: wso2-extensions/identity-inbound-auth-oauth Length of output: 2921 🏁 Script executed: # Now let's look at the removeFromNestedMap method to see what happens with List
sed -n '522,550p' ./components/org.wso2.carbon.identity.oauth/src/main/java/org/wso2/carbon/identity/openidconnect/action/preissueidtoken/execution/PreIssueIDTokenResponseProcessor.javaRepository: wso2-extensions/identity-inbound-auth-oauth Length of output: 1411 🏁 Script executed: # Look at the replaceNestedClaim method as well
sed -n '664,715p' ./components/org.wso2.carbon.identity.oauth/src/main/java/org/wso2/carbon/identity/openidconnect/action/preissueidtoken/execution/PreIssueIDTokenResponseProcessor.javaRepository: wso2-extensions/identity-inbound-auth-oauth Length of output: 2306 🏁 Script executed: # Check the test file for scenarios with array elements in nested claims
grep -n "phones\|address\|0\|-" ./components/org.wso2.carbon.identity.oauth/src/test/java/org/wso2/carbon/identity/openidconnect/action/preissueidtoken/execution/PreIssueIDTokenResponseProcessorTest.java | head -40Repository: wso2-extensions/identity-inbound-auth-oauth Length of output: 3289 🏁 Script executed: #!/bin/bash
# Let's search for tests with 3+ segment paths
rg -n "CLAIMS_PATH_PREFIX.*/.*/.*/" ./components/org.wso2.carbon.identity.oauth/src/test/java/org/wso2/carbon/identity/openidconnect/action/preissueidtoken/execution/PreIssueIDTokenResponseProcessorTest.javaRepository: wso2-extensions/identity-inbound-auth-oauth Length of output: 929 🏁 Script executed: # Let's look at test cases around nested claims with complex structures
sed -n '750,830p' ./components/org.wso2.carbon.identity.oauth/src/test/java/org/wso2/carbon/identity/openidconnect/action/preissueidtoken/execution/PreIssueIDTokenResponseProcessorTest.javaRepository: wso2-extensions/identity-inbound-auth-oauth Length of output: 4074 🏁 Script executed: # Check if there are any tests that set up nested List structures (like address/phones)
rg -n "List.*new ArrayList\|Arrays.asList" ./components/org.wso2.carbon.identity.oauth/src/test/java/org/wso2/carbon/identity/openidconnect/action/preissueidtoken/execution/PreIssueIDTokenResponseProcessorTest.java -B 2 -A 5 | head -60Repository: wso2-extensions/identity-inbound-auth-oauth Length of output: 69 🏁 Script executed: #!/bin/bash
# Let's look more carefully at the logic flow.
# First, let's see the complete condition in removeOtherClaims
sed -n '465,490p' ./components/org.wso2.carbon.identity.oauth/src/main/java/org/wso2/carbon/identity/openidconnect/action/preissueidtoken/execution/PreIssueIDTokenResponseProcessor.javaRepository: wso2-extensions/identity-inbound-auth-oauth Length of output: 1284 🏁 Script executed: # Let's check replaceOtherClaims to see if it has the same logic
sed -n '638,660p' ./components/org.wso2.carbon.identity.oauth/src/main/java/org/wso2/carbon/identity/openidconnect/action/preissueidtoken/execution/PreIssueIDTokenResponseProcessor.javaRepository: wso2-extensions/identity-inbound-auth-oauth Length of output: 1254 🏁 Script executed: # Now let's understand: if a path like "address/phones/0" comes in:
# extractNestedClaimPath removes "/idToken/claims/" and splits by "/"
# So we need to trace through what the actual issue is.
# Let me look at a complete example in the tests for nested structures
sed -n '900,970p' ./components/org.wso2.carbon.identity.oauth/src/test/java/org/wso2/carbon/identity/openidconnect/action/preissueidtoken/execution/PreIssueIDTokenResponseProcessorTest.javaRepository: wso2-extensions/identity-inbound-auth-oauth Length of output: 3852 🏁 Script executed: # Let's see if there are any tests with nested Maps containing Lists
grep -n "phones\|address\|List" ./components/org.wso2.carbon.identity.oauth/src/test/java/org/wso2/carbon/identity/openidconnect/action/preissueidtoken/execution/PreIssueIDTokenResponseProcessorTest.java | grep -i "map\|put" | head -20Repository: wso2-extensions/identity-inbound-auth-oauth Length of output: 577 🏁 Script executed: # Check what the actual nested claim tests look like
sed -n '900,1000p' ./components/org.wso2.carbon.identity.oauth/src/test/java/org/wso2/carbon/identity/openidconnect/action/preissueidtoken/execution/PreIssueIDTokenResponseProcessorTest.javaRepository: wso2-extensions/identity-inbound-auth-oauth Length of output: 5234 🏁 Script executed: #!/bin/bash
# Let me trace through the actual code flow with a concrete example.
# If path = "/idToken/claims/address/phones/0"
# extractNestedClaimPath gives ["address", "phones", "0"]
# isArrayIndexPath(["address", "phones", "0"]) checks size != 2, so returns false
# So it goes to removeNestedClaim with pathSegments=["address", "phones", "0"]
# In removeNestedClaim, nestedPath = pathSegments.subList(1, 3) = ["phones", "0"]
# Then removeFromNestedMap is called with path=["phones", "0"]
# Let's verify what happens in removeFromNestedMap with path ["phones", "0"]
# Looking at the code, on first iteration:
# key = path.get(0) = "phones"
# next = current.get("phones")
# If "phones" is a List (array), then !(next instanceof Map) is true, so return false
# Let's create a test scenario to confirm this behavior
cat > /tmp/test_analysis.txt << 'EOF'
Scenario: Path "/idToken/claims/address/phones/0"
address claim structure: {
"address": {
"phones": ["555-1234", "555-5678"] // LIST, not MAP
}
}
Flow:
1. extractNestedClaimPath("/idToken/claims/address/phones/0")
-> removes prefix, splits by "/"
-> pathSegments = ["address", "phones", "0"]
2. isArrayIndexPath(["address", "phones", "0"])
-> checks size != 2
-> returns false (size is 3, not 2)
3. removeOtherClaims condition: pathSegments.size() > 1 && !isArrayIndexPath(pathSegments)
-> true && true = true
-> calls removeNestedClaim(["address", "phones", "0"], ...)
4. removeNestedClaim:
- rootClaimName = "address"
- nestedPath = ["phones", "0"]
- rootValue = {"phones": ["555-1234", "555-5678"]}
- calls removeFromNestedMap(rootValue, ["phones", "0"], 0)
5. removeFromNestedMap(current, ["phones", "0"], 0):
- key = "phones"
- next = current.get("phones") = ["555-1234", "555-5678"] // LIST
- !(next instanceof Map) = true
- returns false // FAILS HERE
Result: "Nested claim not found" error instead of removing the array element
The reviewer's concern is VALID - isArrayIndexPath doesn't recognize this pattern.
EOF
cat /tmp/test_analysis.txtRepository: wso2-extensions/identity-inbound-auth-oauth Length of output: 1265 🏁 Script executed: # Let's check if there are any tests with nested Maps containing Lists
rg -n "nestedValue\|innerMap" ./components/org.wso2.carbon.identity.oauth/src/test/java/org/wso2/carbon/identity/openidconnect/action/preissueidtoken/execution/PreIssueIDTokenResponseProcessorTest.java -B 2 -A 5 | head -80Repository: wso2-extensions/identity-inbound-auth-oauth Length of output: 69 🏁 Script executed: # Let me check what happens with the current test structure
# The test "testProcessSuccessResponse_RemoveClaim_FromArrayWithValidIndex" uses:
# Map<String, Object> nestedValue = new HashMap<>();
# nestedValue.put("0", "value1");
# This is a Map with string keys "0", "1", NOT a List
# Let's verify the distinction by looking at how "arrayClaim" is used in tests
grep -n "customClaims.put.*arrayClaim" ./components/org.wso2.carbon.identity.oauth/src/test/java/org/wso2/carbon/identity/openidconnect/action/preissueidtoken/execution/PreIssueIDTokenResponseProcessorTest.javaRepository: wso2-extensions/identity-inbound-auth-oauth Length of output: 721 🏁 Script executed: #!/bin/bash
# Let's look more carefully at what cases are tested.
# Check if there's any test that puts a List inside a nested Map structure
sed -n '750,820p' ./components/org.wso2.carbon.identity.oauth/src/test/java/org/wso2/carbon/identity/openidconnect/action/preissueidtoken/execution/PreIssueIDTokenResponseProcessorTest.javaRepository: wso2-extensions/identity-inbound-auth-oauth Length of output: 3696 🏁 Script executed: # Check what the complete structure looks like for the nested claim test
sed -n '725,760p' ./components/org.wso2.carbon.identity.oauth/src/test/java/org/wso2/carbon/identity/openidconnect/action/preissueidtoken/execution/PreIssueIDTokenResponseProcessorTest.javaRepository: wso2-extensions/identity-inbound-auth-oauth Length of output: 1607 🏁 Script executed: # Now let's verify the logic one more time by examining what conditions lead to each code path
sed -n '468,485p' ./components/org.wso2.carbon.identity.oauth/src/main/java/org/wso2/carbon/identity/openidconnect/action/preissueidtoken/execution/PreIssueIDTokenResponseProcessor.javaRepository: wso2-extensions/identity-inbound-auth-oauth Length of output: 952 🏁 Script executed: # Let me check the actual commit to see what was intended
git log --oneline --all | grep -i "array\|nested.*claim" | head -5Repository: wso2-extensions/identity-inbound-auth-oauth Length of output: 492 🏁 Script executed: #!/bin/bash
# Let's examine the commit to understand the intent
git show e21385896 --stat | head -20Repository: wso2-extensions/identity-inbound-auth-oauth Length of output: 1145 🏁 Script executed: # Let's look at the key insight: what does isArrayIndexPath actually check?
# It checks if pathSegments.size() == 2 AND the last element is numeric or "-"
# But for a path like /idToken/claims/address/phones/0
# pathSegments = ["address", "phones", "0"] (size 3)
# So isArrayIndexPath returns false, sending it to removeNestedClaim
# Which expects all intermediate values to be Maps, but "phones" is a List
# The reviewer's claim: there's no test covering nested Lists
# Current tests only have:
# 1. Root level arrays: customClaims.put("arrayClaim", Arrays.asList(...))
# 2. Maps with numeric string keys: nestedValue.put("0", ...)
# But NO: {"address": {"phones": [...]}} where phones is a List
# Let me verify by checking if any test has a nested structure with a List
rg -n "new HashMap\|new ArrayList" ./components/org.wso2.carbon.identity.oauth/src/test/java/org/wso2/carbon/identity/openidconnect/action/preissueidtoken/execution/PreIssueIDTokenResponseProcessorTest.java | grep -A 5 -B 5 "phones\|address" | head -30Repository: wso2-extensions/identity-inbound-auth-oauth Length of output: 69 🏁 Script executed: # Final check: Are there tests with 3+ segment paths?
rg -n 'CLAIMS_PATH_PREFIX.*".*/.*/.*' ./components/org.wso2.carbon.identity.oauth/src/test/java/org/wso2/carbon/identity/openidconnect/action/preissueidtoken/execution/PreIssueIDTokenResponseProcessorTest.javaRepository: wso2-extensions/identity-inbound-auth-oauth Length of output: 929
The hard The commit message ("fix: handle array index paths in nested claim removal and replace") suggests this case should be handled. No tests currently exercise 3+ segment paths ending in array indices. 🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| private ClaimPathInfo parseOperationPath(String operationPath) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| String[] pathSegments = operationPath.split("/"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Uh oh!
There was an error while loading. Please reload this page.