HiveBrain v1.2.0
Get Started
← Back to all entries
debugjavaMinor

Using a Boolean method that never returns "false" to check user permissions

Submitted by: @import:stackexchange-codereview··
0
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 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 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.