Commit 6508a634 authored by Paul-Christian Volkmer's avatar Paul-Christian Volkmer
Browse files

Use exceptions instead of checking for null to send correct http code

If a session was not found in database this will result in http 404,
if the session is not accessable for the current user the request will
result in http 403.

The old behavior was to check for null. This was the response if the
session was not found or the current user could not access this session
(not owner and inactive sessions). Both ended up with http 404 - not found.
parent 1a847feb
......@@ -39,6 +39,8 @@ import org.springframework.web.bind.annotation.ResponseBody;
import de.thm.arsnova.entities.Feedback;
import de.thm.arsnova.entities.Session;
import de.thm.arsnova.entities.User;
import de.thm.arsnova.exceptions.ForbiddenException;
import de.thm.arsnova.exceptions.NotFoundException;
import de.thm.arsnova.services.ISessionService;
import de.thm.arsnova.services.IUserService;
import de.thm.arsnova.socket.ARSnovaSocketIOServer;
......@@ -73,10 +75,14 @@ public class SessionController {
@RequestMapping(value="/session/{sessionkey}", method=RequestMethod.GET)
@ResponseBody
public Session getSession(@PathVariable String sessionkey, HttpServletResponse response) {
Session session = sessionService.getSession(sessionkey);
if (session != null) return session;
response.setStatus(HttpStatus.NOT_FOUND.value());
try {
Session session = sessionService.getSession(sessionkey);
if (session != null) return session;
} catch (NotFoundException e) {
response.setStatus(HttpStatus.NOT_FOUND.value());
} catch (ForbiddenException e) {
response.setStatus(HttpStatus.FORBIDDEN.value());
}
return null;
}
......
......@@ -51,6 +51,8 @@ import com.fourspaces.couchdb.ViewResults;
import de.thm.arsnova.entities.Feedback;
import de.thm.arsnova.entities.Session;
import de.thm.arsnova.entities.User;
import de.thm.arsnova.exceptions.ForbiddenException;
import de.thm.arsnova.exceptions.NotFoundException;
import de.thm.arsnova.services.ISessionService;
import de.thm.arsnova.services.IUserService;
import de.thm.arsnova.socket.message.Question;
......@@ -148,14 +150,14 @@ public class CouchDBDao implements IDatabaseDao {
public Session getSession(String keyword) {
Session result = this.getSessionFromKeyword(keyword);
if(result == null) {
return null;
throw new NotFoundException();
}
if (result.isActive() || result.getCreator().equals(this.actualUserName())) {
sessionService.addUserToSessionMap(this.actualUserName(), keyword);
return result;
}
return null;
throw new ForbiddenException();
}
@Override
......
package de.thm.arsnova.exceptions;
public class ForbiddenException extends RuntimeException {
private static final long serialVersionUID = 1L;
}
package de.thm.arsnova.exceptions;
public class NotFoundException extends RuntimeException {
private static final long serialVersionUID = 1L;
}
package de.thm.arsnova.controller;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
import javax.inject.Inject;
......@@ -19,6 +20,7 @@ import org.springframework.web.servlet.ModelAndView;
import org.springframework.web.servlet.mvc.annotation.AnnotationMethodHandlerAdapter;
import de.thm.arsnova.SessionController;
import de.thm.arsnova.exceptions.NotFoundException;
@RunWith(SpringJUnit4ClassRunner.class)
@ContextConfiguration(locations={
......@@ -45,16 +47,30 @@ public class SessionControllerTest {
}
@Test
public void testShouldNotGetMissingSession() {
public void testShouldNotGetUnknownSession() {
request.setMethod("GET");
request.setRequestURI("/session/12345678");
request.setRequestURI("/session/00000000");
try {
final ModelAndView mav = handlerAdapter.handle(request, response, sessionController);
assertNull(mav);
assertTrue(response.getStatus() == 404);
} catch (Exception e) {
e.printStackTrace();
fail("An exception occured");
}
}
}
@Test
public void testShouldNotGetForbiddenSession() {
request.setMethod("GET");
request.setRequestURI("/session/99999999");
try {
final ModelAndView mav = handlerAdapter.handle(request, response, sessionController);
assertNull(mav);
assertTrue(response.getStatus() == 403);
} catch (Exception e) {
e.printStackTrace();
fail("An exception occured");
}
}
}
......@@ -4,12 +4,16 @@ import java.util.List;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.context.annotation.Scope;
import org.springframework.stereotype.Component;
import de.thm.arsnova.entities.Feedback;
import de.thm.arsnova.entities.Session;
import de.thm.arsnova.entities.User;
import de.thm.arsnova.exceptions.ForbiddenException;
import de.thm.arsnova.exceptions.NotFoundException;
import de.thm.arsnova.socket.message.Question;
@Component
......@@ -20,6 +24,8 @@ public class StubDatabaseDao implements IDatabaseDao {
private Map<String, Feedback> stubFeedbacks = new ConcurrentHashMap<String, Feedback>();
private Map<Session, Question> stubQuestions = new ConcurrentHashMap<Session, Question>();
private final Logger logger = LoggerFactory.getLogger(getClass());
public StubDatabaseDao() {
fillWithDummySessions();
fillWithDummyFeedbacks();
......@@ -56,7 +62,13 @@ public class StubDatabaseDao implements IDatabaseDao {
@Override
public Session getSession(String keyword) {
return stubSessions.get(keyword);
// Magic keyword for forbidden session
if (keyword.equals("99999999")) throw new ForbiddenException();
Session session = stubSessions.get(keyword);
if (session == null) throw new NotFoundException();
return session;
}
@Override
......@@ -108,4 +120,9 @@ public class StubDatabaseDao implements IDatabaseDao {
return stubQuestions.get(session) != null;
}
@Override
public Question getQuestion(String id) {
// Simply ... no such question ;-)
return null;
}
}
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment