remove todo and fix review comments

modules/charon-core/src/main/java/org/wso2/charon3/core/objects/Group.java

@@ -490,18 +490,18 @@ private SimpleAttribute getSimpleAttribute(String attributeName, String attribut
  new SimpleAttribute(attributeName, attributeValue));
  }
 
- private void setRoleV2(ComplexAttribute groupPropertiesAttribute) throws CharonException, BadRequestException {
+ private void setRoleV2(ComplexAttribute rolePropertiesAttribute) throws CharonException, BadRequestException {
 
- MultiValuedAttribute groupsAttribute;
+ MultiValuedAttribute rolesAttribute;
  if (this.attributeList.containsKey(SCIMConstants.GroupSchemaConstants.ROLES)) {
- groupsAttribute = (MultiValuedAttribute) this.attributeList.get(SCIMConstants.GroupSchemaConstants.ROLES);
- groupsAttribute.setAttributeValue(groupPropertiesAttribute);
+ rolesAttribute = (MultiValuedAttribute) this.attributeList.get(SCIMConstants.GroupSchemaConstants.ROLES);
+ rolesAttribute.setAttributeValue(rolePropertiesAttribute);
  } else {
- groupsAttribute = new MultiValuedAttribute(SCIMConstants.GroupSchemaConstants.ROLES);
- groupsAttribute.setAttributeValue(groupPropertiesAttribute);
- groupsAttribute = (MultiValuedAttribute) DefaultAttributeFactory
- .createAttribute(SCIMSchemaDefinitions.SCIMGroupSchemaDefinition.ROLES_SCHEMA, groupsAttribute);
- this.attributeList.put(SCIMConstants.GroupSchemaConstants.ROLES, groupsAttribute);
+ rolesAttribute = new MultiValuedAttribute(SCIMConstants.GroupSchemaConstants.ROLES);
+ rolesAttribute.setAttributeValue(rolePropertiesAttribute);
+ rolesAttribute = (MultiValuedAttribute) DefaultAttributeFactory
+ .createAttribute(SCIMSchemaDefinitions.SCIMGroupSchemaDefinition.ROLES_SCHEMA, rolesAttribute);
+ this.attributeList.put(SCIMConstants.GroupSchemaConstants.ROLES, rolesAttribute);
  }
  }
 }

modules/charon-core/src/main/java/org/wso2/charon3/core/objects/RoleV2.java

@@ -385,7 +385,6 @@ public List<String> getPermissionValues() {
  }
  permissionValuesList.add((String) ((SimpleAttribute) (subAttributesList
  .get(SCIMConstants.CommonSchemaConstants.VALUE))).getValue());
-
  }
  return permissionValuesList;
  }
@@ -406,7 +405,6 @@ public List<String> getPermissionDisplayNames() {
  List<String> permissionDisplayNames = new ArrayList<>();
  if (values == null) {
  return Collections.emptyList();
-
  }
  List<Attribute> subValuesList = permissions.getAttributeValues();
  for (Attribute subValue : subValuesList) {
@@ -614,7 +612,6 @@ protected boolean isAudienceAttributeExist() {
  return attributeList.containsKey(SCIMConstants.RoleSchemaConstants.AUDIENCE);
  }
 
- // TODO: having this kind of custom property in meta is a spec violation. Check whether we need to fix.
 
  /**
  * Set the systemRole attribute of the meta attribute.

modules/charon-core/src/main/java/org/wso2/charon3/core/protocol/endpoints/RoleResourceV2Manager.java

@@ -18,8 +18,6 @@
 
 package org.wso2.charon3.core.protocol.endpoints;
 
-// TODO check the comment. possible to move this out from charon, since it's an extended resource
-
 import org.apache.commons.lang.ArrayUtils;
 import org.apache.commons.lang.StringUtils;
 import org.json.JSONArray;
@@ -95,8 +93,7 @@ public SCIMResponse getRole(String id, RoleV2Manager roleManager, String attribu
  throw new NotFoundException(message);
  }
  ServerSideValidator.validateRetrievedSCIMObject(role, schema, attributes, excludeAttributes);
- // TODO
-// ServerSideValidator.validateRetrievedSCIMRoleObject(role, attributes, excludeAttributes);
+ ServerSideValidator.validateRetrievedSCIMRoleV2Object(role, attributes, excludeAttributes);
  String encodedRole = encoder.encodeSCIMObject(role);
  Map<String, String> httpHeaders = new HashMap<>();
  httpHeaders.put(SCIMConstants.CONTENT_TYPE_HEADER, SCIMConstants.APPLICATION_JSON);
@@ -261,13 +258,12 @@ public SCIMResponse updateWithPUTRole(String id, String putRequest, RoleV2Manage
 
  // Retrieve the old object.
  RoleV2 oldRole = roleManager.getRole(id, requestAttributes);
- if (oldRole != null) {
- RoleV2 newRole = (RoleV2) ServerSideValidator.validateUpdatedSCIMObject(oldRole, role, schema);
- updatedRole = roleManager.updateRole(oldRole, newRole);
- } else {
+ if (oldRole == null) {
  String error = "No role exists with the given id: " + id;
  throw new NotFoundException(error);
  }
+ RoleV2 newRole = (RoleV2) ServerSideValidator.validateUpdatedSCIMObject(oldRole, role, schema);
+ updatedRole = roleManager.updateRole(oldRole, newRole);
  return getScimResponse(encoder, updatedRole);
  } catch (NotFoundException | BadRequestException | CharonException | ConflictException | InternalErrorException
  | NotImplementedException e) {

modules/charon-core/src/main/java/org/wso2/charon3/core/schema/SCIMConstants.java

@@ -560,7 +560,7 @@ public static class RoleSchemaConstants {
  public static final String GROUPS_DESC = "A list of groups of the role.";
  public static final String PERMISSIONS_DESC = "A list of permissions of the role.";
  public static final String ASC_APPLICATIONS_DESC = "A list of associated applications of the role.";
- public static final String AUDIENCE_DESC = "The role usable scope.";
+ public static final String AUDIENCE_DESC = "The role usable audience.";
  public static final String USERS_VALUE_DESC = "Identifier of the user of this role.";
  public static final String GROUPS_VALUE_DESC = "Identifier of the group of this role.";
  public static final String PERMISSIONS_VALUE_DESC = "Identifier of the permissions of this role.";

modules/charon-core/src/main/java/org/wso2/charon3/core/schema/SCIMResourceSchemaManager.java

@@ -221,6 +221,7 @@ public SCIMResourceTypeSchema getRoleResourceV2Schema() {
 
  return SCIMSchemaDefinitions.SCIM_ROLE_V2_SCHEMA;
  }
+
  public SCIMResourceTypeSchema getResourceTypeResourceSchema() {
 
  return SCIMSchemaDefinitions.SCIM_RESOURCE_TYPE_SCHEMA;

modules/charon-core/src/main/java/org/wso2/charon3/core/schema/SCIMSchemaDefinitions.java

@@ -513,6 +513,7 @@ public static class SCIMUserSchemaDefinition {
  SCIMDefinitions.Uniqueness.NONE, null, new ArrayList<>
  (Arrays.asList(SCIMDefinitions.ReferenceType.USER, SCIMDefinitions.ReferenceType
  .ROLE)), null);
+
  public static final SCIMAttributeSchema ROLES_AUDIENCE_VALUE = SCIMAttributeSchema.createSCIMAttributeSchema(
  SCIMConstants.UserSchemaConstants.ROLES_AUDIENCE_VALUE_URI,
  SCIMConstants.CommonSchemaConstants.AUDIENCE_VALUE, SCIMDefinitions.DataType.STRING, false,

modules/charon-core/src/main/java/org/wso2/charon3/core/schema/ServerSideValidator.java

@@ -21,6 +21,7 @@
 import org.wso2.charon3.core.exceptions.NotFoundException;
 import org.wso2.charon3.core.objects.AbstractSCIMObject;
 import org.wso2.charon3.core.objects.Role;
+import org.wso2.charon3.core.objects.RoleV2;
 import org.wso2.charon3.core.objects.User;
 import org.wso2.charon3.core.protocol.endpoints.AbstractResourceManager;
 import org.wso2.charon3.core.utils.AttributeUtil;
@@ -161,6 +162,35 @@ public static void validateRetrievedSCIMRoleObject(Role scimObject, String reque
  }
  }
 
+ /**
+ * Validate Retrieved SCIM Role V2 Object.
+ *
+ * @param scimObject RoleV2 object.
+ * @param requestedAttributes RequestedAttributes.
+ * @param requestedExcludingAttributes RequestedExcludingAttributes.
+ */
+ public static void validateRetrievedSCIMRoleV2Object(RoleV2 scimObject, String requestedAttributes,
+ String requestedExcludingAttributes) {
+
+ List<String> requestedExcludingAttributesList = null;
+ List<String> requestedAttributesList = null;
+ if (requestedExcludingAttributes != null) {
+ // Make a list from the comma separated requestedExcludingAttributes.
+ requestedExcludingAttributesList = Arrays.asList(requestedExcludingAttributes.split(","));
+ }
+ if (requestedAttributes != null) {
+ // Make a list from the comma separated requestedAttributes.
+ requestedAttributesList = Arrays.asList(requestedAttributes.split(","));
+ }
+ if (requestedAttributesList != null && requestedAttributesList.
+ stream().noneMatch(SCIMConstants.RoleSchemaConstants.PERMISSIONS::equalsIgnoreCase)) {
+ scimObject.setPermissions(new ArrayList<>());
+ } else if (requestedExcludingAttributesList != null && requestedExcludingAttributesList.
+ stream().anyMatch(SCIMConstants.RoleSchemaConstants.PERMISSIONS::equalsIgnoreCase)) {
+ scimObject.setPermissions(new ArrayList<>());
+ }
+ }
+
  /**
  * Perform validation on SCIM Object update on service provider side.
  *