debugjavaMinor
Using a Boolean method that never returns "false" to check user permissions
Viewed 0 times
neverpermissionsbooleanmethoduserfalsethatusingreturnscheck
Problem
I need to check that a user is allowed to save/retrieve contacts to/from the database by calling a web service, and return an HTTP403 with an explanation if it is not the case.
So for the sake of factorizing the code, I decided to use a boolean method that "never" returns
Then in
I do the same thing in
And finally these methods are called in the web service:
```
@GET
@Path("{id}")
@Produces(MediaType.APPLICATION_JSON)
public Response retrieveContact(@PathParam("contactId") String contactId) {
try{
Person contact= retrieveContact(getUser(), contactId);
return Response.status(Response.Status.OK).entity(contact).build();
}catch(ForbiddenActionException e){
return Response.status(Response.Status.FORBIDDEN).entity(e.getMessage()).build();
}
}
@POST
@Path("{id}")
@Consumes(MediaType.APPLICATION_JSON)
@Produces(MediaType.APPLICATION_JSON)
public Response saveContact(@PathParam("id") String id, Person contact) {
try{
Person contact= saveContact(getUser(), contact);
return Response.status(Response.Status.OK).entity(contact).build();
}catch(ForbiddenActi
So for the sake of factorizing the code, I decided to use a boolean method that "never" returns
false and is there only to check this, and throws a corresponding exception.public boolean canPerformAction(User authenticatedUser) throws ForbiddenActionException{
if(authenticatedUser == null){
throw new ForbiddenActionException("There must be a user to perform an action !");
}
if(!authenticatedUser.BelongToCompany(company)){
throw new ForbiddenActionException("The user doesn't belong to the company, therefore he can't perform the action");
}
return true;
}Then in
saveContact, I call it as follows: public Person saveContact(User user, Person contact) throws ForbiddenActionException{
canPerformAction(user);
contact= persistContact(contact);
return contact;
}I do the same thing in
retrieveContact: public Person retrieveContact(User user, String contactId) throws ForbiddenActionException{
canPerformAction(user);
return fetchContact(contactId);
}And finally these methods are called in the web service:
```
@GET
@Path("{id}")
@Produces(MediaType.APPLICATION_JSON)
public Response retrieveContact(@PathParam("contactId") String contactId) {
try{
Person contact= retrieveContact(getUser(), contactId);
return Response.status(Response.Status.OK).entity(contact).build();
}catch(ForbiddenActionException e){
return Response.status(Response.Status.FORBIDDEN).entity(e.getMessage()).build();
}
}
@POST
@Path("{id}")
@Consumes(MediaType.APPLICATION_JSON)
@Produces(MediaType.APPLICATION_JSON)
public Response saveContact(@PathParam("id") String id, Person contact) {
try{
Person contact= saveContact(getUser(), contact);
return Response.status(Response.Status.OK).entity(contact).build();
}catch(ForbiddenActi
Solution
The Question
The approach is acceptable and it's perfectly valid to use a method like
Other Stuff
The boilerplate response building instructions
1) Create an ad-hoc shortcut method like
Or, better
2) If you are using Jersey behind your JAX-RS annotations, add an exception mapper class:
In this second case the service method can be shortened as follows:
The approach is acceptable and it's perfectly valid to use a method like
canPerformAction. But it shouldn't return an unused boolean and can be static:private static void ensureUserAuthorized(User user) throws ForbiddenActionException {
if (user == null) {
throw new ForbiddenActionException("message");
}
if (!user.belongsToCompany(company)) {
throw new ForbiddenActionException("message");
}
}Other Stuff
The boilerplate response building instructions
return Response.status... in service methods can be avoided in two ways:1) Create an ad-hoc shortcut method like
private static Response buildResponse(Status status, Object entity) {
return Response.status(status).entity(entity).build();
}Or, better
2) If you are using Jersey behind your JAX-RS annotations, add an exception mapper class:
@Provider
public class ForbiddenActionMapper implements ExceptionMapper {
@Override
public Response toResponse(ForbiddenActionException ex) {
return Response.status(Response.Status.FORBIDDEN)
.entity(ex.getMessage())
.type(MediaType.APPLICATION_JSON)
.build();
}
}In this second case the service method can be shortened as follows:
@POST
@annotations...
public Contact saveContact(@PathParam("id") String id, Person contact) throws ForbiddenActionException {
return saveContact(getUser(), contact);
}Code Snippets
private static void ensureUserAuthorized(User user) throws ForbiddenActionException {
if (user == null) {
throw new ForbiddenActionException("message");
}
if (!user.belongsToCompany(company)) {
throw new ForbiddenActionException("message");
}
}private static Response buildResponse(Status status, Object entity) {
return Response.status(status).entity(entity).build();
}@Provider
public class ForbiddenActionMapper implements ExceptionMapper<ForbiddenActionException> {
@Override
public Response toResponse(ForbiddenActionException ex) {
return Response.status(Response.Status.FORBIDDEN)
.entity(ex.getMessage())
.type(MediaType.APPLICATION_JSON)
.build();
}
}@POST
@annotations...
public Contact saveContact(@PathParam("id") String id, Person contact) throws ForbiddenActionException {
return saveContact(getUser(), contact);
}Context
StackExchange Code Review Q#123640, answer score: 3
Revisions (0)
No revisions yet.