General Fixes (#117)

- Remove Spring Security filter for VIEW_COURSE_FINAL_INSCRIPTIONS, VIEW_STUDENTS_APPROVED and VIEW_COURSE_STUDENT authorities
- To qualify a final for a student, the user must be have ADMIN authority
- Fix admin / student creation endpoints to support address
- Fix case where a student is tried to be created  with an admin's dni or viceversa (the check was being made only for the corresponding Role and not for users in general)
- Return 409 CONFLICT and return a JSON with conflictField when trying to update a course's information (courseId or semester fields)
- Return 409 CONFLICT when trying to change password and the old password does not match the one in the database
- Return 404 NOT FOUND in case the user does not exist when resetting or changing password
- Changed permissions to change other user's password
- Restrict student access for GET /students/{docket} to itself and to admins
- Allow updating information with the same courseId and returning the Location header only if the courseId is changed, else return 204 No content

interfaces/src/main/java/ar/edu/itba/paw/interfaces/UserDao.java

@@ -80,4 +80,5 @@ public interface UserDao {
 	boolean existsEmail(String email);
 
 
+	boolean userExists(int dni);
 }

interfaces/src/main/java/ar/edu/itba/paw/interfaces/UserService.java

@@ -35,4 +35,5 @@ public interface UserService {
 
 	boolean existsEmail(String email);
 
+	boolean userExists(int dni);
 }

persistence/src/main/java/ar/edu/itba/paw/persistence/UserHibernateDao.java

@@ -84,9 +84,6 @@ public boolean changePassword(final int dni, final String prevPassword, final St
 		query.setParameter(DNI_PARAM, dni);
 		query.setMaxResults(ONE);
 		final List<User> users = query.getResultList();
-		if (users.isEmpty()) {
-			return false;
-		}
 		final User user = users.get(FIRST);
 
 		if(prevPassword != null && !user.getPassword().equals(prevPassword)) {
@@ -123,6 +120,16 @@ public boolean existsEmail(final String email) {
 		return !emails.isEmpty();
 	}
 
+	@Override
+	public boolean userExists(final int dni) {
+		final TypedQuery<User> query = em.createQuery(GET_BY_DNI, User.class);
+		query.setParameter(DNI_PARAM, dni);
+		query.setMaxResults(ONE);
+		final List<User> users = query.getResultList();
+
+		return !users.isEmpty();
+	}
+
 	// QueryFilter
 	/* package-private */static class QueryFilter<T extends User> {
 

services/src/main/java/ar/edu/itba/paw/services/AdminServiceImpl.java

@@ -6,6 +6,7 @@
 import ar.edu.itba.paw.models.Procedure;
 import ar.edu.itba.paw.models.Role;
 import ar.edu.itba.paw.models.users.Admin;
+import ar.edu.itba.paw.models.users.User;
 import ar.edu.itba.paw.shared.AdminFilter;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.stereotype.Service;
@@ -33,7 +34,9 @@ public List<Admin> getAllAdmins() {
     @Override
     public boolean create(final Admin admin) {
         admin.setRole(Role.ADMIN);
-        if (admin.getEmail() == null || Objects.equals(admin.getEmail(), "")) {
+	    admin.getAddress().setDni(admin.getDni());
+	    admin.setPassword(User.DEFAULT_PASSWORD);
+	    if (admin.getEmail() == null || Objects.equals(admin.getEmail(), "")) {
             admin.setEmail(userService.createEmail(admin));
         }
         return adminDao.create(admin);

services/src/main/java/ar/edu/itba/paw/services/StudentServiceImpl.java

@@ -6,6 +6,7 @@
 import ar.edu.itba.paw.interfaces.UserService;
 import ar.edu.itba.paw.models.*;
 import ar.edu.itba.paw.models.users.Student;
+import ar.edu.itba.paw.models.users.User;
 import ar.edu.itba.paw.shared.CourseFilter;
 import ar.edu.itba.paw.shared.StudentFilter;
 import org.springframework.beans.factory.annotation.Autowired;
@@ -65,7 +66,9 @@ public List<Student> getByFilter(final StudentFilter studentFilter) {
   @Override
   public boolean create(final Student student) {
     student.setRole(Role.STUDENT);
-    if (student.getEmail() == null || Objects.equals(student.getEmail(), "")) {
+	  student.getAddress().setDni(student.getDni());
+	  student.setPassword(User.DEFAULT_PASSWORD);
+	  if (student.getEmail() == null || Objects.equals(student.getEmail(), "")) {
       student.setEmail(userService.createEmail(student));
     }
     if(student.getAddress() != null){

services/src/main/java/ar/edu/itba/paw/services/UserServiceImpl.java

@@ -53,6 +53,11 @@ public boolean existsEmail(final String email) {
 		return userDao.existsEmail(email);
 	}
 
+	@Override
+	public boolean userExists(final int dni) {
+		return userDao.userExists(dni);
+	}
+
 	/* Test purpose only */
 	/* default */ void setUserDao(UserDao userDao) {
 		this.userDao = userDao;

webapp/src/main/java/ar/edu/itba/paw/webapp/config/WebAuthConfig.java

@@ -46,10 +46,7 @@ protected void configure(final HttpSecurity http) throws Exception {
 						.antMatchers(HttpMethod.DELETE, API_PREFIX_VERSION + "/students").hasAuthority("DELETE_STUDENT")
 
 						// CourseController permissions
-						.antMatchers(HttpMethod.POST,   API_PREFIX_VERSION + "courses/finalInscriptions/*/grades").hasAuthority("QUALIFY_COURSE_FINAL")
-						.antMatchers(HttpMethod.GET,    API_PREFIX_VERSION + "courses/finalInscriptions/*").hasAuthority("VIEW_COURSE_FINAL_INSCRIPTIONS")
-						.antMatchers(HttpMethod.GET,    API_PREFIX_VERSION + "/courses/*/students/passed").hasAuthority("VIEW_STUDENTS_APPROVED")
-						.antMatchers(HttpMethod.GET,    API_PREFIX_VERSION + "/courses/*/students").hasAuthority("VIEW_COURSE_STUDENTS")
+						.antMatchers(HttpMethod.POST, API_PREFIX_VERSION + "courses/finalInscriptions/*/grades").hasAuthority("ADMIN")
 						.antMatchers(HttpMethod.DELETE, API_PREFIX_VERSION + "/courses/*/correlatives/*").hasAuthority("DELETE_CORRELATIVE")
 						.antMatchers(HttpMethod.POST,   API_PREFIX_VERSION + "/courses/*/correlatives").hasAuthority("ADD_CORRELATIVE")
 						.antMatchers(HttpMethod.POST,   API_PREFIX_VERSION + "/courses/*").hasAuthority("EDIT_COURSE")

webapp/src/main/java/ar/edu/itba/paw/webapp/controllers/AdminController.java

@@ -1,6 +1,7 @@
 package ar.edu.itba.paw.webapp.controllers;
 
 import ar.edu.itba.paw.interfaces.AdminService;
+import ar.edu.itba.paw.interfaces.UserService;
 import ar.edu.itba.paw.models.users.Admin;
 import ar.edu.itba.paw.shared.AdminFilter;
 import ar.edu.itba.paw.webapp.models.*;
@@ -30,6 +31,9 @@ public class AdminController {
 
   private final static Logger LOGGER = LoggerFactory.getLogger(AdminController.class);
 
+  @Autowired
+  private UserService us;
+
   @Autowired
   private AdminService as;
 
@@ -65,7 +69,7 @@ public Response adminsIndex(@QueryParam("dni") final Integer dni,
   public Response adminsNew(@Valid AdminNewDTO adminNewDTO) {
     final Admin admin = mapper.convertToAdmin(adminNewDTO);
 
-    if(as.getByDni(admin.getDni()) != null) {
+    if (us.userExists(adminNewDTO.getDni())) {
       return status(Status.CONFLICT).build();
     }
     if(!as.create(admin)) {

webapp/src/main/java/ar/edu/itba/paw/webapp/controllers/CourseController.java

@@ -104,21 +104,45 @@ public Response coursesShow(@PathParam("courseId") final String courseId) {
   @Consumes(MediaType.APPLICATION_JSON)
   public Response coursesUpdate(@PathParam("courseId") final String courseId,
                                 @Valid final CourseDTO courseDTO) {
-
     if(cs.getByCourseID(courseId) == null) {
       return status(Status.NOT_FOUND).build();
     }
-    final Course courseUpdated = mapper.convertToCourse(courseDTO);
 
+	  if (cs.getByCourseID(courseDTO.getCourseId()) != null && !courseId.equals(courseDTO.getCourseId())) {
+		  LOGGER.debug("Attempting to update course {} with courseId {}, which already exists", courseId, courseDTO.getCourseId());
+		  return status(Status.CONFLICT).entity(createJSONEntryMessage("conflictField", "courseId")).build();
+	  }
+
+    final Course courseUpdated = mapper.convertToCourse(courseDTO);
     if(!cs.update(courseId, courseUpdated)) {
-      return status(Status.BAD_REQUEST).build(); //TODO: Decide what to return
+	    return status(Status.CONFLICT).entity(createJSONEntryMessage("conflictField", "semester")).build();
     }
 
-    final URI uri = uriInfo.getAbsolutePathBuilder().path(String.valueOf(courseUpdated.getCourseId())).build();
+	  if (!courseId.equals(courseDTO.getCourseId())) {
+		  final URI uri = uriInfo.getBaseUriBuilder().path("/courses/" + String.valueOf(courseUpdated.getCourseId())).build();
+		  return ok(uri).build();
+	  }
 
-    return ok(uri).build();
+	  return noContent().build();
   }
 
+	private String createJSONEntryMessage(final String key, final String value) {
+		final StringBuilder json = new StringBuilder();
+
+		json
+						.append("{")
+						.append("\"")
+						.append(key)
+						.append("\"")
+						.append(":")
+						.append("\"")
+						.append(value)
+						.append("\"")
+						.append("}");
+
+		return json.toString();
+	}
+
   @DELETE
   @Path("/{courseId}")
   public Response coursesDestroy(@PathParam("courseId") final String courseId) {

webapp/src/main/java/ar/edu/itba/paw/webapp/controllers/DTOEntityMapper.java

@@ -78,7 +78,11 @@ FinalInscriptionDTO convertToFinalInscriptionDTO(FinalInscription finalInscripti
   }
 
   FinalInscriptionIndexDTO convertToFinalInscriptionIndexDTO(FinalInscription finalInscription) {
-    return modelMapper.map(finalInscription, FinalInscriptionIndexDTO.class);
+	  final FinalInscriptionIndexDTO finalInscriptionIndexDTO = modelMapper.map(finalInscription, FinalInscriptionIndexDTO.class);
+
+	  finalInscriptionIndexDTO.setName(finalInscription.getCourse().getName());
+
+	  return finalInscriptionIndexDTO;
   }
 
   FinalInscription convertToFinalInscription(FinalInscriptionDTO finalinscriptionDTO, Course course) {

webapp/src/main/java/ar/edu/itba/paw/webapp/controllers/StudentController.java

@@ -3,6 +3,7 @@
 
 import ar.edu.itba.paw.interfaces.CourseService;
 import ar.edu.itba.paw.interfaces.StudentService;
+import ar.edu.itba.paw.interfaces.UserService;
 import ar.edu.itba.paw.models.Course;
 import ar.edu.itba.paw.models.FinalInscription;
 import ar.edu.itba.paw.models.Grade;
@@ -42,6 +43,9 @@ public class StudentController {
 
   private final static org.slf4j.Logger LOGGER = LoggerFactory.getLogger(StudentController.class);
 
+  @Autowired
+  private UserService us;
+
   @Autowired
   private StudentService ss;
 
@@ -58,13 +62,9 @@ public class StudentController {
 
   @GET
   @Produces(MediaType.APPLICATION_JSON)
-  public Response studentsIndex(
-          @Min(value = 1)
-          @QueryParam("docket") final Integer docket,
-          @Size(max=50)
-          @QueryParam("firstName") final String firstName,
-          @Size(max=50)
-          @QueryParam("lastName") final String lastName){
+  public Response studentsIndex(@Min(value = 1) @QueryParam("docket") final Integer docket,
+                                @Size(max = 50) @QueryParam("firstName") final String firstName,
+                                @Size(max = 50) @QueryParam("lastName") final String lastName) {
     final StudentFilter studentFilter = new StudentFilter.StudentFilterBuilder()
             .docket(docket).firstName(firstName).lastName(lastName).build();
 
@@ -77,10 +77,10 @@ public Response studentsIndex(
 
   @POST
   @Consumes(MediaType.APPLICATION_JSON)
-  public Response studentsNew(@Valid final UserNewDTO userNewDTO) throws ValidationException{
+  public Response studentsNew(@Valid final UserNewDTO userNewDTO) throws ValidationException {
     final Student student = mapper.convertToStudent(userNewDTO);
 
-    if(ss.getByDni(student.getDni()) != null) {
+    if (us.userExists(student.getDni())) {
       return status(Status.CONFLICT).build();
     }
     if(!ss.create(student)) {
@@ -96,13 +96,17 @@ public Response studentsNew(@Valid final UserNewDTO userNewDTO) throws Validatio
   @Produces(MediaType.APPLICATION_JSON)
   public Response studentsShow(@PathParam("docket") final int docket) {
     final Student student = ss.getByDocket(docket);
+
     if(student == null) {
       return status(Status.NOT_FOUND).build();
     }
 
-
-
+	  if (student.getDni() != LoggedUser.getDni() && !LoggedUser.isAdmin()) {
+		  LOGGER.debug("Student with dni {} tried to access student with dni {}", LoggedUser.getDni(), student.getDni());
+		  return status(Status.FORBIDDEN).build();
+	  }
     final StudentShowDTO studentShowDTO = mapper.convertToStudentShowDTO(student);
+
     return ok(studentShowDTO).build();
   }
 
@@ -119,7 +123,7 @@ public Response studentsUpdate(@PathParam("docket") final int docket,
     }
 
 	  final int dni = LoggedUser.getDni();
-	  if(dni != oldStudent.getDni() && !LoggedUser.isAdmin()) {
+	  if (dni != oldStudent.getDni() && !LoggedUser.isAdmin()) {
 		  return status(Status.FORBIDDEN).build();
 	  }
 
@@ -134,7 +138,7 @@ public Response studentsUpdate(@PathParam("docket") final int docket,
   @Path("/{docket}")
   public Response studentsDestroy(@PathParam("docket") final int docket) {
 	  ss.deleteStudent(docket);
-    return noContent().build();
+	  return noContent().build();
   }
 
   @GET
@@ -178,20 +182,19 @@ public Response studentsCoursesNew(
 
     final int dni = LoggedUser.getDni();
     if(dni != student.getDni() && !LoggedUser.isAdmin()) {
-    	return status(Status.FORBIDDEN).build();
+	    return status(Status.FORBIDDEN).build();
     }
 
     final Course course = cs.getByCourseID(inscriptionDTO.getCourseId());
     if(course == null){
       return status(Status.BAD_REQUEST).build();
     }
 
-    boolean result = ss.enroll(student, course);
+	  boolean enrolled = ss.enroll(student, course);
 
-    if(!result){
-      return status(Status.BAD_REQUEST).build();
-      // TODO: Decide what to return
-    }
+	  if (!enrolled) {
+		  return status(Status.CONFLICT).build();
+	  }
     return status(Status.OK).build();
   }
 
@@ -202,13 +205,13 @@ public Response studentsCoursesDestroy(
           @Pattern(regexp="\\d{2}\\.\\d{2}")
           @PathParam("courseId") final String courseId){
 	  final Student student = ss.getByDocket(docket);
-	  if(student == null){
+	  if (student == null) {
 		  return status(Status.NOT_FOUND).build();
 	  }
 
     final int dni = LoggedUser.getDni();
     if(dni != student.getDni() && !LoggedUser.isAdmin()) {
-    	return status(Status.FORBIDDEN).build();
+	    return status(Status.FORBIDDEN).build();
     }
 
     final Course course = cs.getByCourseID(courseId);
@@ -278,7 +281,7 @@ public Response studentsGradesIndex(@PathParam("docket") final Integer docket){
   @Consumes(MediaType.APPLICATION_JSON)
   @Path("/{docket}/grades")
   public Response studentsGradesNew(@PathParam("docket") final Integer docket,
-                                       @Valid final GradeDTO gradeDTO){
+                                    @Valid final GradeDTO gradeDTO) {
 
     final Student student = ss.getByDocket(docket);
     if(student == null){
@@ -319,7 +322,7 @@ public Response studentsGradesUpdate(
     }
 
 	  final int dni = LoggedUser.getDni();
-	  if(dni != student.getDni() && !LoggedUser.isAdmin()) {
+	  if (dni != student.getDni() && !LoggedUser.isAdmin()) {
 		  return status(Status.FORBIDDEN).build();
 	  }
 

webapp/src/main/java/ar/edu/itba/paw/webapp/controllers/UserController.java

@@ -34,14 +34,18 @@ public Response usersPasswordChange(@PathParam("dni") final int dni,
                                       @Valid PasswordDTO passwordDTO) {
 
     final int loggedDni = LoggedUser.getDni();
-    if(loggedDni != dni  && !LoggedUser.isAdmin()) {
-      return status(Status.FORBIDDEN).build();
+	  if (loggedDni != dni) {
+		  return status(Status.FORBIDDEN).build();
     }
 
+	  if (!us.userExists(dni)) {
+		  return status(Status.NOT_FOUND).build();
+	  }
+
     passwordDTO.setDni(loggedDni);
 
     if(!us.changePassword(loggedDni, passwordDTO.getCurrentPassword(), passwordDTO.getNewPassword())) {
-      return status(Status.BAD_REQUEST).build();
+	    return status(Status.CONFLICT).build();
     }
 
     return noContent().build();
@@ -50,6 +54,10 @@ public Response usersPasswordChange(@PathParam("dni") final int dni,
   @POST
   @Path("/{dni}/password/reset")
   public Response usersPasswordReset(@PathParam("dni") final int dni) {
+	  if (!us.userExists(dni)) {
+		  return status(Status.NOT_FOUND).build();
+	  }
+
     us.resetPassword(dni);
 
     return noContent().build();