Skip to content
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.

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

Issue#301 + Issue#300 #305

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Original file line number Diff line number Diff line change
@@ -1,8 +1,14 @@
package dev.findfirst.users.controller;

import java.io.File;
import java.io.IOException;
import java.net.URI;
import java.net.URISyntaxException;
import java.nio.file.Files;
import java.rmi.UnexpectedException;
import java.util.Arrays;
import java.util.Optional;
import java.util.UUID;

import jakarta.validation.Valid;
import jakarta.validation.constraints.Email;
Expand All @@ -29,8 +35,11 @@
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.springframework.beans.factory.annotation.Value;
import org.springframework.core.io.FileSystemResource;
import org.springframework.core.io.Resource;
import org.springframework.http.HttpHeaders;
import org.springframework.http.HttpStatus;
import org.springframework.http.MediaType;
import org.springframework.http.ResponseCookie;
import org.springframework.http.ResponseEntity;
import org.springframework.web.bind.annotation.GetMapping;
Expand All @@ -40,6 +49,7 @@
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.RequestParam;
import org.springframework.web.bind.annotation.RestController;
import org.springframework.web.multipart.MultipartFile;

@RestController
@RequestMapping("/user")
Expand All @@ -60,6 +70,10 @@ public class UserController {
@Value("${findfirst.app.domain}")
private String domain;

private static final long MAX_FILE_SIZE = 2 * 1024 * 1024; // 2 MB
R-Sandor marked this conversation as resolved.
Show resolved Hide resolved
private static final String[] ALLOWED_TYPES = {"image/jpeg", "image/png"};
private static final String UPLOAD_DIR = "uploads/profile-pictures/";
Copy link
Owner

Choose a reason for hiding this comment

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

Would you mind using an application properties for that @Value

  @Value("${findfirst.screenshot.uploads.profile-pictures}")
  private String uploadProfileLoc;

Or something similar.

For example:
findfirst.local.upload.profile-pictures=../data/uploads/profile-pictures/
findfirst.upload.location=${FINDFIRST_UPLOAD_LOCATION:${findfirst.local.upload.profile-pictures}}

Then in the docker files do something like I have for the screenshot location:
environment:

  • FINDFIRST_UPLOAD_LOCATION=/app/profile-pictures/

volumes:

  • ./data/uploads/profile-pictures:/app/profile-pictures


@PostMapping("/signup")
public ResponseEntity<String> registerUser(@Valid @RequestBody SignupRequest signUpRequest) {
User user;
Expand Down Expand Up @@ -154,4 +168,85 @@ public ResponseEntity<String> refreshToken(
return ResponseEntity.ok().header(HttpHeaders.SET_COOKIE, cookie.toString()).body(token);
}).orElseThrow(() -> new TokenRefreshException(jwt, "Refresh token is not in database!"));
}

@PostMapping("/profile-picture")
public ResponseEntity<?> uploadProfilePicture(@RequestParam("file") MultipartFile file,
Copy link
Owner

Choose a reason for hiding this comment

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

After at @RequestParm use the size annotation @Size(max = MAX_FILE_SIZE)

@RequestParam("userId") int userId) {
Copy link
Owner

@R-Sandor R-Sandor Jan 14, 2025

Choose a reason for hiding this comment

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

Remove this because it makes it so we can not validate the user by their jwt credentials.
I would move all the internal logic from 186 - 213 into the userService.

Ideally the controller logic should mostly be handling errors if any occur, the parameters and different types of parameters then letting the Service level handle the doing.

// File size validation
if (file.getSize() > MAX_FILE_SIZE) {
Copy link
Owner

Choose a reason for hiding this comment

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

I would do the size check in the method parameter, also helps keep our end point from getting dsod with big files.

return ResponseEntity.badRequest().body("File size exceeds the maximum limit of 2 MB.");
}

// File type validation
String contentType = file.getContentType();
if (Arrays.stream(ALLOWED_TYPES).noneMatch(contentType::equals)) {
return ResponseEntity.badRequest().body("Invalid file type. Only JPG and PNG are allowed.");
}

try {
User user;
try {
user = userService.getUserById(userId).orElseThrow(() -> new NoUserFoundException());
Copy link
Owner

Choose a reason for hiding this comment

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

To clear up a little bit above:
Add this to the userManagement service:
https://github.com/R-Sandor/FindFirst/blob/75ada43b5d5aa7c6ea3c808519e361e45280e5fe/server/src/main/java/dev/findfirst/core/service/BookmarkService.java#L61C1-L61C38

UserContext userContext that allows us to get the user from the request to server rather than telling the server who you are, which could lead to me putting my photo on your profile since there is not any validation.

} catch (NoUserFoundException e) {
return ResponseEntity.status(HttpStatus.NOT_FOUND).body("User not found.");
}

// Create directory for uploads
File uploadDir = new File(UPLOAD_DIR);
if (!uploadDir.exists()) {
uploadDir.mkdirs();
}

// Delete old photo (if exists)
String oldPhotoPath = user.getUserPhoto();
if (oldPhotoPath != null) {
File oldPhoto = new File(oldPhotoPath);
if (oldPhoto.exists()) {
oldPhoto.delete();
}
}

// Save new photo
String fileName = UUID.randomUUID() + "_" + file.getOriginalFilename();
File destinationFile = new File(UPLOAD_DIR + fileName);
file.transferTo(destinationFile);
userService.changeUserPhoto(user, UPLOAD_DIR + fileName);

return ResponseEntity.ok("File uploaded successfully.");
} catch (Exception e) {
return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR).body("Failed to upload file.");
}
}

@GetMapping("/profile-picture")
public ResponseEntity<Resource> getUserProfilePicture(@RequestParam("userId") int userId) {
try {
User user;
try {
user = userService.getUserById(userId).orElseThrow(() -> new NoUserFoundException());
} catch (NoUserFoundException e) {
return ResponseEntity.status(HttpStatus.NOT_FOUND).body(null);
}
String userPhotoPath = user.getUserPhoto();

if (userPhotoPath == null || userPhotoPath.isEmpty()) {
return ResponseEntity.status(HttpStatus.NOT_FOUND).body(null);
}

// Load file
File photoFile = new File(userPhotoPath);
if (!photoFile.exists()) {
return ResponseEntity.status(HttpStatus.NOT_FOUND).body(null);
}

// Create response
Resource fileResource = new FileSystemResource(photoFile);
return ResponseEntity.ok()
.contentType(MediaType.parseMediaType(Files.probeContentType(photoFile.toPath())))
.body(fileResource);

} catch (IOException e) {
return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR).body(null);
}
}
}
3 changes: 3 additions & 0 deletions server/src/main/java/dev/findfirst/users/model/user/User.java
Original file line number Diff line number Diff line change
Expand Up @@ -43,4 +43,7 @@ public User(SignupRequest signup, String encodedPasswd) {
@Column("role_role_id")
private AggregateReference<Role, Integer> role;

@Column("user_photo")
private String userPhoto;

}
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package dev.findfirst.users.service;

import java.io.File;
import java.nio.charset.StandardCharsets;
import java.rmi.UnexpectedException;
import java.time.Instant;
Expand Down Expand Up @@ -81,6 +82,25 @@ public void changeUserPassword(User user, String password) {
saveUser(user);
}

public void changeUserPhoto(User user, String userPhoto) {
log.info("Changing profile picture for user ID {}: {}", user.getUserId(), userPhoto);
user.setUserPhoto(userPhoto);
saveUser(user);
}

public void removeUserPhoto(User user) {
String userPhoto = user.getUserPhoto();
if (userPhoto != null) {
File photoFile = new File(userPhoto);
if (photoFile.exists()) {
log.info("Removing profile picture for user ID {}: {}", user.getUserId(), userPhoto);
photoFile.delete();
}
}
user.setUserPhoto(null);
saveUser(user);
}

public String createVerificationToken(User user) {
String token = UUID.randomUUID().toString();
Token verificationToken = new Token(AggregateReference.to(user.getUserId()), token);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
ALTER TABLE users
ADD COLUMN user_photo varchar(255);
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@

import static org.junit.Assert.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.Mockito.*;

import java.util.Optional;
import java.util.Properties;
Expand All @@ -12,10 +14,16 @@
import dev.findfirst.security.userauth.models.payload.request.SignupRequest;
import dev.findfirst.users.model.MailHogMessage;
import dev.findfirst.users.model.user.TokenPassword;
import dev.findfirst.users.model.user.User;
import dev.findfirst.users.service.UserManagementService;

import com.fasterxml.jackson.databind.ObjectMapper;
import org.junit.jupiter.api.Order;
import org.junit.jupiter.api.Test;
import org.mockito.ArgumentCaptor;
import org.mockito.InjectMocks;
import org.mockito.Mock;
import org.mockito.MockitoAnnotations;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.test.context.SpringBootTest;
import org.springframework.boot.test.context.TestConfiguration;
Expand All @@ -26,8 +34,10 @@
import org.springframework.http.HttpHeaders;
import org.springframework.http.HttpMethod;
import org.springframework.http.HttpStatus;
import org.springframework.http.ResponseEntity;
import org.springframework.mail.javamail.JavaMailSender;
import org.springframework.mail.javamail.JavaMailSenderImpl;
import org.springframework.mock.web.MockMultipartFile;
import org.springframework.test.context.TestPropertySource;
import org.testcontainers.containers.GenericContainer;
import org.testcontainers.containers.PostgreSQLContainer;
Expand All @@ -41,7 +51,17 @@
@TestPropertySource(locations = "classpath:application-test.yml")
class UserControllerTest {

final TestRestTemplate restTemplate;
TestRestTemplate restTemplate = new TestRestTemplate();

@Mock
private UserManagementService userManagementService;

@InjectMocks
private UserController userController;

public UserControllerTest() {
MockitoAnnotations.openMocks(this);
}

@Autowired
UserControllerTest(TestRestTemplate tRestTemplate) {
Expand Down Expand Up @@ -181,4 +201,101 @@ void refreshToken() {
HttpMethod.POST, new HttpEntity<>(new HttpHeaders()), String.class, refreshTkn);
assertEquals(HttpStatus.OK, resp.getStatusCode());
}

@Test
void testUploadProfilePicture_Success() throws Exception {
MockMultipartFile file = new MockMultipartFile("file", "test.jpg", "image/jpeg", "dummy content".getBytes());
int userId = 1;

User user = new User();
user.setUserId(userId);
user.setUsername("testUser");
when(userManagementService.getUserById(userId)).thenReturn(Optional.of(user));

ResponseEntity<?> response = userController.uploadProfilePicture(file, userId);

assertEquals(HttpStatus.OK, response.getStatusCode());
assertEquals("File uploaded successfully.", response.getBody());
verify(userManagementService, times(1)).changeUserPhoto(eq(user), anyString());
}

@Test
void testRemoveUserPhoto_Success() {
User user = new User();
user.setUserId(1);
user.setUsername("testUser");
user.setUserPhoto("uploads/profile-pictures/test.jpg");

when(userManagementService.getUserById(user.getUserId())).thenReturn(Optional.of(user));

userManagementService.removeUserPhoto(user);

ArgumentCaptor<User> userCaptor = ArgumentCaptor.forClass(User.class);
verify(userManagementService, times(1)).saveUser(userCaptor.capture());
assertNull(userCaptor.getValue().getUserPhoto());
}

@Test
void testUploadProfilePicture_FileSizeExceedsLimit() throws Exception {
byte[] largeContent = new byte[3 * 1024 * 1024]; // 3 MB
MockMultipartFile file = new MockMultipartFile("file", "large.jpg", "image/jpeg", largeContent);
int userId = 1;

User user = new User();
user.setUserId(userId);
user.setUsername("testUser");
when(userManagementService.getUserById(userId)).thenReturn(Optional.of(user));

ResponseEntity<?> response = userController.uploadProfilePicture(file, userId);

assertEquals(HttpStatus.BAD_REQUEST, response.getStatusCode());
assertEquals("File size exceeds the maximum limit of 2 MB.", response.getBody());
}

@Test
void testUploadProfilePicture_InvalidFileType() throws Exception {
MockMultipartFile file = new MockMultipartFile("file", "test.txt", "text/plain", "dummy content".getBytes());
int userId = 1;

User user = new User();
user.setUserId(userId);
user.setUsername("testUser");
when(userManagementService.getUserById(userId)).thenReturn(Optional.of(user));

ResponseEntity<?> response = userController.uploadProfilePicture(file, userId);

assertEquals(HttpStatus.BAD_REQUEST, response.getStatusCode());
assertEquals("Invalid file type. Only JPG and PNG are allowed.", response.getBody());
}

@Test
void testGetUserProfilePicture_NotFound() {
int userId = 1;

User user = new User();
user.setUserId(userId);
user.setUsername("testUser");
user.setUserPhoto(null);
when(userManagementService.getUserById(userId)).thenReturn(Optional.of(user));

ResponseEntity<?> response = userController.getUserProfilePicture(userId);

assertEquals(HttpStatus.NOT_FOUND, response.getStatusCode());
}

@Test
void testGetUserProfilePicture_Success() {
int userId = 1;

User user = new User();
user.setUserId(userId);
user.setUsername("testUser");
user.setUserPhoto("uploads/profile-pictures/test.jpg");
when(userManagementService.getUserById(userId)).thenReturn(Optional.of(user));

ResponseEntity<?> response = userController.getUserProfilePicture(userId);

assertEquals(HttpStatus.OK, response.getStatusCode());
assertNotNull(response.getBody());
}
}
Loading