Skip to content

Improve error handling #437

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

Draft
asha15 wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

Improve error handling #437

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b9c9a9d
Throw ForbiddenException
asha15 Jun 3, 2025
67e404e
Refactor the code
asha15 Jun 5, 2025
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
9 changes: 6 additions & 3 deletions modules/charon-core/src/main/java/org/wso2/charon3/core/extensions/RoleManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,8 @@ public interface RoleManager {
* @throws NotImplementedException NotImplementedException.
* @throws BadRequestException BadRequestException.
*/
Role createRole(Role role) throws CharonException, ConflictException, NotImplementedException, BadRequestException;
Role createRole(Role role) throws CharonException, ConflictException, NotImplementedException,
BadRequestException, ForbiddenException;
Copy link
Copy Markdown
Contributor

@piraveena piraveena Jul 8, 2025

Choose a reason for hiding this comment

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

won't this introduce any behavioural change? Then we have to do a version bump

Copy link
Copy Markdown
Contributor

@mpmadhavig mpmadhavig Aug 6, 2025

Choose a reason for hiding this comment

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

So far ForbiddenException was not thrown.

Now also throwing for the cases introduced in this effort.

Do we still need to consider this as a behavioural change?


/**
* Get the role for the given ID.
Expand All @@ -73,7 +74,8 @@ Role getRole(String id, Map<String, Boolean> requiredAttributes)
* @throws NotImplementedException NotImplementedException.
* @throws BadRequestException BadRequestException.
*/
void deleteRole(String id) throws NotFoundException, CharonException, NotImplementedException, BadRequestException;
void deleteRole(String id) throws NotFoundException, CharonException, NotImplementedException,
BadRequestException, ForbiddenException;

/**
* List roles with Get.
Expand Down Expand Up @@ -104,7 +106,8 @@ RolesGetResponse listRolesWithGET(Node node, Integer startIndex, Integer count,
* @throws NotFoundException NotFoundException.
*/
Role updateRole(Role oldRole, Role newRole)
throws NotImplementedException, BadRequestException, CharonException, ConflictException, NotFoundException;
throws NotImplementedException, BadRequestException, CharonException, ConflictException,
NotFoundException, ForbiddenException;

/**
* List roles with Post.
Expand Down
8 changes: 5 additions & 3 deletions modules/charon-core/src/main/java/org/wso2/charon3/core/extensions/RoleV2Manager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ public interface RoleV2Manager {
* @throws BadRequestException BadRequestException.
*/
RoleV2 createRole(RoleV2 role)
throws CharonException, ConflictException, NotImplementedException, BadRequestException;
throws CharonException, ConflictException, NotImplementedException, BadRequestException, ForbiddenException;

/**
* Get the role for the given ID.
Expand All @@ -74,7 +74,8 @@ RoleV2 getRole(String id, Map<String, Boolean> requiredAttributes)
* @throws NotImplementedException NotImplementedException.
* @throws BadRequestException BadRequestException.
*/
void deleteRole(String id) throws NotFoundException, CharonException, NotImplementedException, BadRequestException;
void deleteRole(String id) throws NotFoundException, CharonException, NotImplementedException, BadRequestException,
ForbiddenException;

/**
* List roles with Get.
Expand Down Expand Up @@ -107,7 +108,8 @@ RolesV2GetResponse listRolesWithGET(Node node, Integer startIndex, Integer count
* @throws NotFoundException NotFoundException.
*/
RoleV2 updateRole(RoleV2 oldRole, RoleV2 newRole)
throws NotImplementedException, BadRequestException, CharonException, ConflictException, NotFoundException;
throws NotImplementedException, BadRequestException, CharonException, ConflictException,
NotFoundException, ForbiddenException;

/**
* List roles with Post.
Expand Down
6 changes: 4 additions & 2 deletions modules/charon-core/src/main/java/org/wso2/charon3/core/extensions/UserManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,8 @@ default void updateGroup(Group oldGroup, Group newGroup)
*/
default Group patchGroup(String groupId, String currentGroupName, Map<String, List<PatchOperation>> patchOperations,
Map<String, Boolean> requiredAttributes)
throws NotImplementedException, BadRequestException, CharonException, NotFoundException {
throws NotImplementedException, BadRequestException, CharonException,
NotFoundException, ForbiddenException {

throw new NotImplementedException();
}
Expand All @@ -234,7 +235,8 @@ default Group patchGroup(String groupId, String currentGroupName, Map<String, Li
* @throws NotImplementedException Functionality no implemented exception.
*/
default void patchGroup(String groupId, String currentGroupName, Map<String, List<PatchOperation>> patchOperations)
throws NotImplementedException, BadRequestException, CharonException, NotFoundException {
throws NotImplementedException, BadRequestException, CharonException,
NotFoundException, ForbiddenException {

throw new NotImplementedException();
}
Expand Down
8 changes: 5 additions & 3 deletions ...ron-core/src/main/java/org/wso2/charon3/core/protocol/endpoints/GroupResourceManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import org.wso2.charon3.core.exceptions.BadRequestException;
import org.wso2.charon3.core.exceptions.CharonException;
import org.wso2.charon3.core.exceptions.ConflictException;
import org.wso2.charon3.core.exceptions.ForbiddenException;
import org.wso2.charon3.core.exceptions.InternalErrorException;
import org.wso2.charon3.core.exceptions.NotFoundException;
import org.wso2.charon3.core.exceptions.NotImplementedException;
Expand Down Expand Up @@ -747,7 +748,8 @@ public SCIMResponse updateWithPatchForAddRemoveOperations(String existingGroupId
String error = "Updated group resource is null.";
throw new CharonException(error);
}
} catch (NotFoundException | BadRequestException | NotImplementedException | CharonException e) {
} catch (NotFoundException | BadRequestException | NotImplementedException |
CharonException | ForbiddenException e) {
return AbstractResourceManager.encodeSCIMException(e);
} catch (RuntimeException e) {
CharonException e1 = new CharonException("Error in performing the patch operation on group resource.", e);
Expand Down Expand Up @@ -785,7 +787,7 @@ private Map<String, List<PatchOperation>> buildPatchOperationsMap(List<PatchOper

private void updateWithPatchForAddRemoveOperations(String existingGroupId, List<PatchOperation> opList,
UserManager userManager) throws BadRequestException,
NotImplementedException, NotFoundException, CharonException {
NotImplementedException, NotFoundException, CharonException, ForbiddenException {

Map<String, List<PatchOperation>> patchOperations = buildPatchOperationsMap(opList);
SCIMResourceTypeSchema schema = SCIMResourceSchemaManager.getInstance().getGroupResourceSchema();
Expand Down Expand Up @@ -1011,7 +1013,7 @@ public SCIMResponse updateWithPATCH(String existingGroupId, String patchRequest,
httpHeaders.put(SCIMConstants.CONTENT_TYPE_HEADER, SCIMConstants.APPLICATION_JSON);
return new SCIMResponse(ResponseCodeConstants.CODE_NO_CONTENT, null, httpHeaders);
} catch (NotFoundException | BadRequestException | NotImplementedException | CharonException |
InternalErrorException e) {
InternalErrorException | ForbiddenException e) {
return AbstractResourceManager.encodeSCIMException(e);
} catch (RuntimeException e) {
CharonException ex = new CharonException("Error in performing the patch operation on group resource.", e);
Expand Down
8 changes: 4 additions & 4 deletions ...aron-core/src/main/java/org/wso2/charon3/core/protocol/endpoints/RoleResourceManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ public SCIMResponse createRole(String postRequest, RoleManager roleManager) {
return new SCIMResponse(ResponseCodeConstants.CODE_CREATED, encodedRole, httpHeaders);

} catch (InternalErrorException | BadRequestException | ConflictException | CharonException | NotFoundException
| NotImplementedException e) {
| NotImplementedException | ForbiddenException e) {
return encodeSCIMException(e);
}
}
Expand All @@ -152,7 +152,7 @@ public SCIMResponse deleteRole(String id, RoleManager roleManager) {
return new SCIMResponse(ResponseCodeConstants.CODE_NO_CONTENT, null, null);

} catch (InternalErrorException | CharonException | NotFoundException | NotImplementedException
| BadRequestException e) {
| BadRequestException | ForbiddenException e) {
return encodeSCIMException(e);
}
}
Expand Down Expand Up @@ -363,7 +363,7 @@ public SCIMResponse updateWithPUTRole(String id, String putRequest, RoleManager
return getScimResponse(encoder, updatedRole);

} catch (NotFoundException | BadRequestException | CharonException | ConflictException | InternalErrorException
| NotImplementedException e) {
| NotImplementedException | ForbiddenException e) {
return encodeSCIMException(e);
}
}
Expand Down Expand Up @@ -397,7 +397,7 @@ public SCIMResponse updateWithPATCHRole(String id, String patchRequest, RoleMana
return getScimResponse(encoder, updatedRole);

} catch (NotFoundException | BadRequestException | NotImplementedException | CharonException | ConflictException
| InternalErrorException e) {
| InternalErrorException | ForbiddenException e) {
return AbstractResourceManager.encodeSCIMException(e);
} catch (RuntimeException e) {
CharonException ex = new CharonException("Error in performing the patch operation on role resource.", e);
Expand Down
8 changes: 4 additions & 4 deletions ...on-core/src/main/java/org/wso2/charon3/core/protocol/endpoints/RoleResourceV2Manager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ public SCIMResponse createRole(String postRequest, RoleV2Manager roleManager) {
return new SCIMResponse(ResponseCodeConstants.CODE_CREATED, encodedRole, httpHeaders);

} catch (InternalErrorException | BadRequestException | ConflictException | CharonException | NotFoundException
| NotImplementedException e) {
| NotImplementedException | ForbiddenException e) {
return encodeSCIMException(e);
}
}
Expand All @@ -151,7 +151,7 @@ public SCIMResponse deleteRole(String id, RoleV2Manager roleManager) {
roleManager.deleteRole(id);
return new SCIMResponse(ResponseCodeConstants.CODE_NO_CONTENT, null, null);
} catch (InternalErrorException | CharonException | NotFoundException | NotImplementedException
| BadRequestException e) {
| BadRequestException | ForbiddenException e) {
return encodeSCIMException(e);
}
}
Expand Down Expand Up @@ -287,7 +287,7 @@ public SCIMResponse updateWithPUTRole(String id, String putRequest, RoleV2Manage
updatedRole = roleManager.updateRole(oldRole, newRole);
return getScimResponse(encoder, updatedRole);
} catch (NotFoundException | BadRequestException | CharonException | ConflictException | InternalErrorException
| NotImplementedException e) {
| NotImplementedException | ForbiddenException e) {
return encodeSCIMException(e);
}
}
Expand Down Expand Up @@ -320,7 +320,7 @@ public SCIMResponse updateWithPATCHRole(String id, String patchRequest, RoleV2Ma
RoleV2 updatedRole = roleManager.updateRole(originalRole, patchedRole);
return getScimResponse(encoder, updatedRole);
} catch (NotFoundException | BadRequestException | NotImplementedException | CharonException | ConflictException
| InternalErrorException e) {
| InternalErrorException | ForbiddenException e) {
return AbstractResourceManager.encodeSCIMException(e);
} catch (RuntimeException e) {
CharonException ex = new CharonException("Error in performing the patch operation on role resource.", e);
Expand Down
7 changes: 5 additions & 2 deletions ...core/src/test/java/org/wso2/charon3/core/protocol/endpoints/GroupResourceManagerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import org.wso2.charon3.core.exceptions.BadRequestException;
import org.wso2.charon3.core.exceptions.CharonException;
import org.wso2.charon3.core.exceptions.ConflictException;
import org.wso2.charon3.core.exceptions.ForbiddenException;
import org.wso2.charon3.core.exceptions.InternalErrorException;
import org.wso2.charon3.core.exceptions.NotFoundException;
import org.wso2.charon3.core.exceptions.NotImplementedException;
Expand Down Expand Up @@ -786,7 +787,8 @@ public Object[][] dataToUpdateWithPATCH() throws BadRequestException, CharonExce
@Test(dataProvider = "dataForUpdateWithPATCH")
public void testUpdateWithPATCH(String existingId, String patchRequest, String attributes,
String excludeAttributes, Object scimNewGroupObject, Object scimOldGroupObject)
throws BadRequestException, CharonException, NotImplementedException, NotFoundException {
throws BadRequestException, CharonException, NotImplementedException,
NotFoundException, ForbiddenException {

Group groupNew = (Group) scimNewGroupObject;
Group groupOld = (Group) scimOldGroupObject;
Expand Down Expand Up @@ -817,7 +819,8 @@ public Object[][] dataToUpdateWithPATCHOverride()
@Test(dataProvider = "dataForUpdateWithPATCHOverride")
public void testUpdateWithPATCHOverride(String existingId, String patchRequest,
Object scimNewGroupObject, Object scimOldGroupObject)
throws BadRequestException, CharonException, NotImplementedException, NotFoundException {
throws BadRequestException, CharonException, NotImplementedException,
NotFoundException, ForbiddenException {

Group groupNew = (Group) scimNewGroupObject;
Group groupOld = (Group) scimOldGroupObject;
Expand Down