From 17c2044eb517e61f73ef8d7610b5796ec24efc22 Mon Sep 17 00:00:00 2001 From: SanriaArgos Date: Mon, 10 Aug 2026 00:55:19 +0300 Subject: [PATCH 1/9] fix(api): map API errors without exposing internals --- src/main/java/goodroad/api/ApiErrors.java | 93 ++++++++++++++++++++--- 1 file changed, 84 insertions(+), 9 deletions(-) diff --git a/src/main/java/goodroad/api/ApiErrors.java b/src/main/java/goodroad/api/ApiErrors.java index 96670b0..c72cda2 100644 --- a/src/main/java/goodroad/api/ApiErrors.java +++ b/src/main/java/goodroad/api/ApiErrors.java @@ -1,10 +1,23 @@ package goodroad.api; +import jakarta.validation.ConstraintViolationException; +import lombok.extern.slf4j.Slf4j; +import org.springframework.dao.DataIntegrityViolationException; import org.springframework.http.HttpStatus; import org.springframework.http.ResponseEntity; +import org.springframework.http.converter.HttpMessageNotReadableException; +import org.springframework.security.access.AccessDeniedException; +import org.springframework.web.HttpMediaTypeNotSupportedException; +import org.springframework.web.bind.MethodArgumentNotValidException; +import org.springframework.web.bind.MissingServletRequestParameterException; import org.springframework.web.bind.annotation.ExceptionHandler; import org.springframework.web.bind.annotation.RestControllerAdvice; -import lombok.extern.slf4j.Slf4j; +import org.springframework.web.method.annotation.HandlerMethodValidationException; +import org.springframework.web.method.annotation.MethodArgumentTypeMismatchException; +import org.springframework.web.multipart.MaxUploadSizeExceededException; +import org.springframework.web.multipart.support.MissingServletRequestPartException; +import org.springframework.web.servlet.resource.NoResourceFoundException; + import java.time.Instant; @Slf4j @@ -23,8 +36,8 @@ public static class ApiException extends RuntimeException { private final HttpStatus status; private final String code; - public ApiException(HttpStatus status, String code, String msg) { - super(msg); + public ApiException(HttpStatus status, String code, String message) { + super(message); this.status = status; this.code = code; } @@ -42,16 +55,78 @@ public String code() { public static class GlobalHandler { @ExceptionHandler(ApiException.class) - public ResponseEntity handleApiException(ApiException e) { - log.error("API exception: code={}, msg={}", e.code(), e.getMessage(), e); - return ResponseEntity.status(e.status()).body(ApiError.of(e.code(), e.getMessage())); + public ResponseEntity handleApiException(ApiException exception) { + if (exception.status().is5xxServerError()) { + log.error("API exception: code={}, msg={}", exception.code(), exception.getMessage(), exception); + } + return ResponseEntity.status(exception.status()) + .body(ApiError.of(exception.code(), exception.getMessage())); + } + + @ExceptionHandler(MethodArgumentNotValidException.class) + public ResponseEntity handleValidation(MethodArgumentNotValidException exception) { + return badRequest("REQUEST_VALIDATION_FAILED", "Request fields are invalid"); + } + + @ExceptionHandler(HttpMessageNotReadableException.class) + public ResponseEntity handleUnreadableBody(HttpMessageNotReadableException exception) { + return badRequest("REQUEST_BODY_INVALID", "Request body is missing or contains invalid JSON"); + } + + @ExceptionHandler({ + MethodArgumentTypeMismatchException.class, + MissingServletRequestParameterException.class, + ConstraintViolationException.class, + HandlerMethodValidationException.class + }) + public ResponseEntity handleInvalidParameters(Exception exception) { + return badRequest("REQUEST_VALIDATION_FAILED", "Request parameters are invalid"); + } + + @ExceptionHandler(MissingServletRequestPartException.class) + public ResponseEntity handleMissingPart(MissingServletRequestPartException exception) { + return badRequest("REQUEST_PART_MISSING", "Required request part is missing"); + } + + @ExceptionHandler(MaxUploadSizeExceededException.class) + public ResponseEntity handleUploadTooLarge(MaxUploadSizeExceededException exception) { + return ResponseEntity.status(HttpStatus.PAYLOAD_TOO_LARGE) + .body(ApiError.of("FILE_TOO_LARGE", "Uploaded file is too large")); + } + + @ExceptionHandler(HttpMediaTypeNotSupportedException.class) + public ResponseEntity handleUnsupportedMediaType(HttpMediaTypeNotSupportedException exception) { + return ResponseEntity.status(HttpStatus.UNSUPPORTED_MEDIA_TYPE) + .body(ApiError.of("CONTENT_TYPE_UNSUPPORTED", "Content type is not supported")); + } + + @ExceptionHandler(DataIntegrityViolationException.class) + public ResponseEntity handleConflict(DataIntegrityViolationException exception) { + return ResponseEntity.status(HttpStatus.CONFLICT) + .body(ApiError.of("DATA_CONFLICT", "Operation conflicts with current data")); + } + + @ExceptionHandler(AccessDeniedException.class) + public ResponseEntity handleAccessDenied(AccessDeniedException exception) { + return ResponseEntity.status(HttpStatus.FORBIDDEN) + .body(ApiError.of("ACCESS_DENIED", "Access denied")); + } + + @ExceptionHandler(NoResourceFoundException.class) + public ResponseEntity handleNotFound(NoResourceFoundException exception) { + return ResponseEntity.status(HttpStatus.NOT_FOUND) + .body(ApiError.of("ENDPOINT_NOT_FOUND", "Endpoint not found")); } @ExceptionHandler(Exception.class) - public ResponseEntity handleServerError(Exception e) { - log.error("Unexpected exception", e); + public ResponseEntity handleServerError(Exception exception) { + log.error("Unexpected exception", exception); return ResponseEntity.status(HttpStatus.INTERNAL_SERVER_ERROR) .body(ApiError.of("SERVER_INTERNAL_ERROR", "Server internal error")); } + + private ResponseEntity badRequest(String code, String message) { + return ResponseEntity.badRequest().body(ApiError.of(code, message)); + } } -} \ No newline at end of file +} From abb41977954b6351f02baf8be6f9851cbfd6a42b Mon Sep 17 00:00:00 2001 From: SanriaArgos Date: Mon, 10 Aug 2026 00:56:26 +0300 Subject: [PATCH 2/9] fix(validation): bound shared text inputs --- src/main/java/goodroad/validation/InputRules.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/goodroad/validation/InputRules.java b/src/main/java/goodroad/validation/InputRules.java index 0f357dd..5745e6c 100644 --- a/src/main/java/goodroad/validation/InputRules.java +++ b/src/main/java/goodroad/validation/InputRules.java @@ -16,7 +16,7 @@ private InputRules() { public static String requireCyrillicText(String value, String code, String fieldName) { String normalized = trimToNull(value); - if (normalized == null || !CYRILLIC_TEXT.matcher(normalized).matches()) { + if (normalized == null || normalized.length() > 80 || !CYRILLIC_TEXT.matcher(normalized).matches()) { throw new ApiException(HttpStatus.BAD_REQUEST, code, fieldName + " is invalid"); } return normalized; From f983f9c69db8c4aacfeb10b5fa662d26197b26e8 Mon Sep 17 00:00:00 2001 From: SanriaArgos Date: Mon, 10 Aug 2026 00:57:05 +0300 Subject: [PATCH 3/9] fix(validation): validate coordinates and distance calculations --- .../java/goodroad/validation/GeoUtils.java | 63 +++++++++++++++++++ 1 file changed, 63 insertions(+) create mode 100644 src/main/java/goodroad/validation/GeoUtils.java diff --git a/src/main/java/goodroad/validation/GeoUtils.java b/src/main/java/goodroad/validation/GeoUtils.java new file mode 100644 index 0000000..310ab78 --- /dev/null +++ b/src/main/java/goodroad/validation/GeoUtils.java @@ -0,0 +1,63 @@ +package goodroad.validation; + +import goodroad.api.ApiErrors.ApiException; +import org.springframework.http.HttpStatus; + +public final class GeoUtils { + private static final double EARTH_RADIUS_KM = 6371.0; + + private GeoUtils() { + } + + public static Coordinates requireCoordinates(Double latitude, Double longitude, String code) { + if (latitude == null || longitude == null + || !Double.isFinite(latitude) || !Double.isFinite(longitude) + || latitude < -90 || latitude > 90 + || longitude < -180 || longitude > 180) { + throw new ApiException(HttpStatus.BAD_REQUEST, code, "Координаты должны содержать допустимые широту и долготу"); + } + return new Coordinates(latitude, longitude); + } + + public static Coordinates parseLatLon(String value, String fieldName) { + if (value == null) { + throw invalidPoint(fieldName); + } + String[] parts = value.split(",", -1); + if (parts.length != 2) { + throw invalidPoint(fieldName); + } + try { + return requireCoordinates( + Double.parseDouble(parts[0].trim()), + Double.parseDouble(parts[1].trim()), + "ROUTE_POINT_INVALID" + ); + } catch (NumberFormatException e) { + throw invalidPoint(fieldName); + } + } + + public static double distanceKm(double firstLat, double firstLon, double secondLat, double secondLon) { + double latitudeDelta = Math.toRadians(secondLat - firstLat); + double longitudeDelta = Math.toRadians(secondLon - firstLon); + double a = Math.sin(latitudeDelta / 2) * Math.sin(latitudeDelta / 2) + + Math.cos(Math.toRadians(firstLat)) * Math.cos(Math.toRadians(secondLat)) + * Math.sin(longitudeDelta / 2) * Math.sin(longitudeDelta / 2); + return EARTH_RADIUS_KM * 2 * Math.atan2(Math.sqrt(a), Math.sqrt(1 - a)); + } + + private static ApiException invalidPoint(String fieldName) { + return new ApiException( + HttpStatus.BAD_REQUEST, + "ROUTE_POINT_INVALID", + "Поле " + fieldName + " должно иметь формат latitude,longitude" + ); + } + + public record Coordinates(double latitude, double longitude) { + public String asLatLon() { + return latitude + "," + longitude; + } + } +} From f75ae5121532caecdfdd8342a4c1d8624faa82ae Mon Sep 17 00:00:00 2001 From: SanriaArgos Date: Mon, 10 Aug 2026 00:57:56 +0300 Subject: [PATCH 4/9] fix(data): add row locks for user state changes --- src/main/java/goodroad/users/repository/UserRepo.java | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/main/java/goodroad/users/repository/UserRepo.java b/src/main/java/goodroad/users/repository/UserRepo.java index 47fe3cb..1277548 100644 --- a/src/main/java/goodroad/users/repository/UserRepo.java +++ b/src/main/java/goodroad/users/repository/UserRepo.java @@ -2,6 +2,7 @@ import org.springframework.data.jpa.repository.*; import org.springframework.data.repository.query.Param; +import jakarta.persistence.LockModeType; import java.time.Instant; import java.util.List; import java.util.Optional; @@ -10,6 +11,14 @@ public interface UserRepo extends JpaRepository { Optional findByPhoneHash(String phoneHash); + @Lock(LockModeType.PESSIMISTIC_WRITE) + @Query("select user from UserEntity user where user.phoneHash = :phoneHash") + Optional findByPhoneHashForUpdate(@Param("phoneHash") String phoneHash); + + @Lock(LockModeType.PESSIMISTIC_WRITE) + @Query("select user from UserEntity user where user.id = :id") + Optional findByIdForUpdate(@Param("id") Long id); + List findByRoleIn(List roles); @Modifying @@ -20,4 +29,4 @@ public interface UserRepo extends JpaRepository { and user.lastActiveAt < :cutoff """) int deleteInactiveBefore(@Param("cutoff") Instant cutoff); -} \ No newline at end of file +} From 417ed59bbdfc96b115fdcc16af1cb2d5033b66f0 Mon Sep 17 00:00:00 2001 From: SanriaArgos Date: Mon, 10 Aug 2026 02:05:06 +0300 Subject: [PATCH 5/9] fix(validation): restrict external and owned storage URLs --- .../validation/TrustedUrlService.java | 71 +++++++++++++++++++ .../validation/TrustedUrlServiceTest.java | 50 +++++++++++++ 2 files changed, 121 insertions(+) create mode 100644 src/main/java/goodroad/validation/TrustedUrlService.java create mode 100644 src/test/java/goodroad/validation/TrustedUrlServiceTest.java diff --git a/src/main/java/goodroad/validation/TrustedUrlService.java b/src/main/java/goodroad/validation/TrustedUrlService.java new file mode 100644 index 0000000..2ed6d52 --- /dev/null +++ b/src/main/java/goodroad/validation/TrustedUrlService.java @@ -0,0 +1,71 @@ +package goodroad.validation; + +import goodroad.api.ApiErrors.ApiException; +import org.springframework.beans.factory.annotation.Value; +import org.springframework.http.HttpStatus; +import org.springframework.stereotype.Service; + +import java.net.URI; +import java.net.URISyntaxException; +import java.util.Locale; +import java.util.Set; + +@Service +public class TrustedUrlService { + private static final String STORAGE_HOST = "storage.yandexcloud.net"; + private static final Set DOBRO_HOSTS = Set.of("dobro.ru", "www.dobro.ru"); + + private final String storageBucket; + + public TrustedUrlService(@Value("${yandex.storage.bucket}") String storageBucket) { + this.storageBucket = storageBucket; + } + + public String requireDobroProfileUrl(String rawUrl) { + URI uri = parseHttpsUrl(rawUrl, "DOBRO_URL_INVALID", "Укажите корректную HTTPS-ссылку на профиль dobro.ru"); + String host = uri.getHost().toLowerCase(Locale.ROOT); + if (!DOBRO_HOSTS.contains(host) || uri.getPort() != -1 || uri.getUserInfo() != null) { + throw invalid("DOBRO_URL_INVALID", "Ссылка должна вести на домен dobro.ru"); + } + return uri.normalize().toString(); + } + + public String requireOwnedStorageUrl(String rawUrl, String directory, Long userId, String code) { + URI uri = parseHttpsUrl(rawUrl, code, "Ссылка на файл имеет неверный формат"); + String expectedPrefix = "/" + storageBucket + "/" + directory + "/" + userId + "/"; + String rawPath = uri.getRawPath() == null ? "" : uri.getRawPath().toLowerCase(Locale.ROOT); + boolean containsEncodedSeparator = rawPath.contains("%2f") || rawPath.contains("%5c") || rawPath.contains("%2e"); + if (!STORAGE_HOST.equalsIgnoreCase(uri.getHost()) + || uri.getPort() != -1 + || uri.getUserInfo() != null + || uri.getQuery() != null + || uri.getFragment() != null + || containsEncodedSeparator + || uri.normalize().getPath() == null + || !uri.normalize().getPath().startsWith(expectedPrefix) + || uri.normalize().getPath().equals(expectedPrefix)) { + throw invalid(code, "Разрешены только файлы, загруженные текущим пользователем через GoodRoad"); + } + return uri.normalize().toString(); + } + + private URI parseHttpsUrl(String rawUrl, String code, String message) { + String value = InputRules.trimToNull(rawUrl); + if (value == null || value.length() > 512) { + throw invalid(code, message); + } + try { + URI uri = new URI(value); + if (!"https".equalsIgnoreCase(uri.getScheme()) || uri.getHost() == null) { + throw invalid(code, message); + } + return uri; + } catch (URISyntaxException e) { + throw invalid(code, message); + } + } + + private ApiException invalid(String code, String message) { + return new ApiException(HttpStatus.BAD_REQUEST, code, message); + } +} diff --git a/src/test/java/goodroad/validation/TrustedUrlServiceTest.java b/src/test/java/goodroad/validation/TrustedUrlServiceTest.java new file mode 100644 index 0000000..9c71af6 --- /dev/null +++ b/src/test/java/goodroad/validation/TrustedUrlServiceTest.java @@ -0,0 +1,50 @@ +package goodroad.validation; + +import goodroad.api.ApiErrors.ApiException; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; + +class TrustedUrlServiceTest { + private final TrustedUrlService service = new TrustedUrlService("goodroad-bucket"); + + @Test + void acceptsDobroHttpsProfile() { + assertEquals( + "https://dobro.ru/volunteer/123", + service.requireDobroProfileUrl("https://dobro.ru/volunteer/123") + ); + } + + @Test + void rejectsLookalikeDobroDomain() { + assertThrows(ApiException.class, () -> service.requireDobroProfileUrl( + "https://dobro.ru.evil.example/payload.exe" + )); + assertThrows(ApiException.class, () -> service.requireDobroProfileUrl( + "https://attacker@dobro.ru/volunteer/123" + )); + } + + @Test + void acceptsOnlyCurrentUsersUploadedCertificate() { + String expected = "https://storage.yandexcloud.net/goodroad-bucket/volunteer-certificates/10/cert.jpg"; + assertEquals(expected, service.requireOwnedStorageUrl( + expected, "volunteer-certificates", 10L, "CERTIFICATE_URL_INVALID" + )); + + assertThrows(ApiException.class, () -> service.requireOwnedStorageUrl( + "https://storage.yandexcloud.net/goodroad-bucket/volunteer-certificates/11/malware.jpg", + "volunteer-certificates", + 10L, + "CERTIFICATE_URL_INVALID" + )); + assertThrows(ApiException.class, () -> service.requireOwnedStorageUrl( + "https://storage.yandexcloud.net/goodroad-bucket/volunteer-certificates/10/%2e%2e/reviews/file.jpg", + "volunteer-certificates", + 10L, + "CERTIFICATE_URL_INVALID" + )); + } +} From b4ee0a54bfb60a33ada1aabf2fb5717b057d7da6 Mon Sep 17 00:00:00 2001 From: SanriaArgos Date: Mon, 10 Aug 2026 02:11:01 +0300 Subject: [PATCH 6/9] fix(storage): validate image uploads before storage --- .../java/goodroad/storage/StorageService.java | 57 +++++----- .../validation/UploadValidationService.java | 106 ++++++++++++++++++ .../UploadValidationServiceTest.java | 51 +++++++++ 3 files changed, 187 insertions(+), 27 deletions(-) create mode 100644 src/main/java/goodroad/validation/UploadValidationService.java create mode 100644 src/test/java/goodroad/validation/UploadValidationServiceTest.java diff --git a/src/main/java/goodroad/storage/StorageService.java b/src/main/java/goodroad/storage/StorageService.java index e7b630b..fada9e7 100644 --- a/src/main/java/goodroad/storage/StorageService.java +++ b/src/main/java/goodroad/storage/StorageService.java @@ -1,7 +1,12 @@ package goodroad.storage; +import goodroad.api.ApiErrors.ApiException; +import goodroad.validation.UploadValidationService; +import goodroad.validation.UploadValidationService.UploadPurpose; +import goodroad.validation.UploadValidationService.VerifiedUpload; import lombok.RequiredArgsConstructor; import org.springframework.beans.factory.annotation.Value; +import org.springframework.http.HttpStatus; import org.springframework.stereotype.Service; import org.springframework.web.multipart.MultipartFile; import software.amazon.awssdk.core.sync.RequestBody; @@ -15,55 +20,58 @@ public class StorageService { private final S3Client s3Client; + private final UploadValidationService uploadValidator; @Value("${yandex.storage.bucket}") private String bucket; public String uploadAvatar(MultipartFile file, String userId) { - try { + VerifiedUpload verified = uploadValidator.validate(file, UploadPurpose.AVATAR); - String ext = getExt(file.getOriginalFilename()); + try { - String key = "avatars/" + userId + "/" + UUID.randomUUID() + ext; + String key = "avatars/" + userId + "/" + UUID.randomUUID() + verified.extension(); s3Client.putObject( PutObjectRequest.builder() .bucket(bucket) .key(key) - .contentType(file.getContentType()) + .contentType(verified.contentType()) + .contentLength((long) verified.bytes().length) .build(), - RequestBody.fromBytes(file.getBytes()) + RequestBody.fromBytes(verified.bytes()) ); return "https://storage.yandexcloud.net/" + bucket + "/" + key; - } catch (Exception e) { - throw new RuntimeException("Upload failed", e); + } catch (RuntimeException e) { + throw new ApiException(HttpStatus.BAD_GATEWAY, "STORAGE_UNAVAILABLE", "File storage is unavailable"); } } public String uploadReviewPhoto(MultipartFile file, String userId) { - try { + VerifiedUpload verified = uploadValidator.validate(file, UploadPurpose.REVIEW_PHOTO); - String ext = getExt(file.getOriginalFilename()); + try { String key = "reviews/" + userId + "/" + UUID.randomUUID() - + ext; + + verified.extension(); s3Client.putObject( PutObjectRequest.builder() .bucket(bucket) .key(key) - .contentType(file.getContentType()) + .contentType(verified.contentType()) + .contentLength((long) verified.bytes().length) .build(), - RequestBody.fromBytes(file.getBytes()) + RequestBody.fromBytes(verified.bytes()) ); return "https://storage.yandexcloud.net/" @@ -71,30 +79,31 @@ public String uploadReviewPhoto(MultipartFile file, String userId) { + "/" + key; - } catch (Exception e) { - throw new RuntimeException("Upload failed", e); + } catch (RuntimeException e) { + throw new ApiException(HttpStatus.BAD_GATEWAY, "STORAGE_UNAVAILABLE", "File storage is unavailable"); } } public String uploadVolunteerCertificate(MultipartFile file, String userId) { - try { + VerifiedUpload verified = uploadValidator.validate(file, UploadPurpose.VOLUNTEER_CERTIFICATE); - String ext = getExt(file.getOriginalFilename()); + try { String key = "volunteer-certificates/" + userId + "/" + UUID.randomUUID() - + ext; + + verified.extension(); s3Client.putObject( PutObjectRequest.builder() .bucket(bucket) .key(key) - .contentType(file.getContentType()) + .contentType(verified.contentType()) + .contentLength((long) verified.bytes().length) .build(), - RequestBody.fromBytes(file.getBytes()) + RequestBody.fromBytes(verified.bytes()) ); return "https://storage.yandexcloud.net/" @@ -102,14 +111,8 @@ public String uploadVolunteerCertificate(MultipartFile file, String userId) { + "/" + key; - } catch (Exception e) { - throw new RuntimeException("Upload failed", e); + } catch (RuntimeException e) { + throw new ApiException(HttpStatus.BAD_GATEWAY, "STORAGE_UNAVAILABLE", "File storage is unavailable"); } } - - private String getExt(String name) { - if (name == null) return ""; - int i = name.lastIndexOf("."); - return i == -1 ? "" : name.substring(i); - } } \ No newline at end of file diff --git a/src/main/java/goodroad/validation/UploadValidationService.java b/src/main/java/goodroad/validation/UploadValidationService.java new file mode 100644 index 0000000..634e4d6 --- /dev/null +++ b/src/main/java/goodroad/validation/UploadValidationService.java @@ -0,0 +1,106 @@ +package goodroad.validation; + +import goodroad.api.ApiErrors.ApiException; +import org.springframework.http.HttpStatus; +import org.springframework.stereotype.Service; +import org.springframework.web.multipart.MultipartFile; + +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.util.Set; + +@Service +public class UploadValidationService { + private static final long MAX_FILE_SIZE = 10L * 1024 * 1024; + + public VerifiedUpload validate(MultipartFile file, UploadPurpose purpose) { + if (file == null || file.isEmpty()) { + throw new ApiException(HttpStatus.BAD_REQUEST, "FILE_EMPTY", "Выберите непустой файл для загрузки"); + } + + if (file.getSize() > MAX_FILE_SIZE) { + throw new ApiException(HttpStatus.PAYLOAD_TOO_LARGE, "FILE_TOO_LARGE", "Размер файла не должен превышать 10 МБ"); + } + + byte[] bytes; + try { + bytes = file.getBytes(); + } catch (IOException e) { + throw new ApiException(HttpStatus.UNPROCESSABLE_ENTITY, "FILE_READ_FAILED", "Не удалось прочитать загруженный файл"); + } + + DetectedType detectedType = detectType(bytes); + if (!purpose.allowedTypes.contains(detectedType)) { + throw new ApiException( + HttpStatus.UNSUPPORTED_MEDIA_TYPE, + "FILE_CONTENT_TYPE_INVALID", + purpose == UploadPurpose.VOLUNTEER_CERTIFICATE + ? "Сертификат должен быть файлом JPEG или PNG" + : "Изображение должно быть файлом JPEG, PNG или WEBP" + ); + } + + return new VerifiedUpload(bytes, detectedType.contentType, detectedType.extension); + } + + private DetectedType detectType(byte[] bytes) { + if (startsWith(bytes, new int[] {0xFF, 0xD8, 0xFF})) { + return DetectedType.JPEG; + } + if (startsWith(bytes, new int[] {0x89, 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A})) { + return DetectedType.PNG; + } + if (bytes.length >= 12 + && ascii(bytes, 0, 4).equals("RIFF") + && ascii(bytes, 8, 4).equals("WEBP")) { + return DetectedType.WEBP; + } + return DetectedType.UNKNOWN; + } + + private boolean startsWith(byte[] bytes, int[] signature) { + if (bytes.length < signature.length) { + return false; + } + for (int index = 0; index < signature.length; index++) { + if ((bytes[index] & 0xFF) != signature[index]) { + return false; + } + } + return true; + } + + private String ascii(byte[] bytes, int offset, int length) { + return new String(bytes, offset, length, StandardCharsets.US_ASCII); + } + + public enum UploadPurpose { + AVATAR(Set.of(DetectedType.JPEG, DetectedType.PNG, DetectedType.WEBP)), + REVIEW_PHOTO(Set.of(DetectedType.JPEG, DetectedType.PNG, DetectedType.WEBP)), + VOLUNTEER_CERTIFICATE(Set.of(DetectedType.JPEG, DetectedType.PNG)); + + private final Set allowedTypes; + + UploadPurpose(Set allowedTypes) { + this.allowedTypes = allowedTypes; + } + } + + private enum DetectedType { + JPEG("image/jpeg", ".jpg"), + PNG("image/png", ".png"), + WEBP("image/webp", ".webp"), + UNKNOWN("application/octet-stream", ""); + + private final String contentType; + private final String extension; + + DetectedType(String contentType, String extension) { + this.contentType = contentType; + this.extension = extension; + } + } + + public record VerifiedUpload(byte[] bytes, String contentType, String extension) { + } +} diff --git a/src/test/java/goodroad/validation/UploadValidationServiceTest.java b/src/test/java/goodroad/validation/UploadValidationServiceTest.java new file mode 100644 index 0000000..514784f --- /dev/null +++ b/src/test/java/goodroad/validation/UploadValidationServiceTest.java @@ -0,0 +1,51 @@ +package goodroad.validation; + +import goodroad.api.ApiErrors.ApiException; +import org.junit.jupiter.api.Test; +import org.springframework.mock.web.MockMultipartFile; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; + +class UploadValidationServiceTest { + private final UploadValidationService service = new UploadValidationService(); + + @Test + void acceptsPngBySignatureEvenWhenClientContentTypeIsWrong() { + byte[] png = new byte[] {(byte) 0x89, 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A, 1, 2}; + MockMultipartFile file = new MockMultipartFile("file", "image.bin", "application/octet-stream", png); + + UploadValidationService.VerifiedUpload result = service.validate( + file, + UploadValidationService.UploadPurpose.REVIEW_PHOTO + ); + + assertEquals("image/png", result.contentType()); + assertEquals(".png", result.extension()); + } + + @Test + void rejectsExecutableRenamedToJpeg() { + byte[] executable = new byte[] {'M', 'Z', 1, 2, 3, 4}; + MockMultipartFile file = new MockMultipartFile("file", "certificate.jpg", "image/jpeg", executable); + + ApiException exception = assertThrows(ApiException.class, () -> service.validate( + file, + UploadValidationService.UploadPurpose.VOLUNTEER_CERTIFICATE + )); + + assertEquals("FILE_CONTENT_TYPE_INVALID", exception.code()); + } + + @Test + void rejectsPdfAsVolunteerCertificateToAvoidActiveDocumentContent() { + MockMultipartFile file = new MockMultipartFile( + "file", "certificate.pdf", "application/pdf", "%PDF-1.7".getBytes() + ); + + assertThrows(ApiException.class, () -> service.validate( + file, + UploadValidationService.UploadPurpose.VOLUNTEER_CERTIFICATE + )); + } +} From 61f3bbbf01d97e78f404b317c16a73f0eb525b5f Mon Sep 17 00:00:00 2001 From: SanriaArgos Date: Mon, 10 Aug 2026 02:18:26 +0300 Subject: [PATCH 7/9] fix(users): protect phone password and avatar updates --- .../goodroad/users/users/UserController.java | 5 +- .../users/users/UserSettingsService.java | 67 ++++++++++++------- .../goodroad/api/HttpApiScenarioTest.java | 14 ++-- .../users/users/UserSettingsServiceTest.java | 25 ++++--- 4 files changed, 67 insertions(+), 44 deletions(-) diff --git a/src/main/java/goodroad/users/users/UserController.java b/src/main/java/goodroad/users/users/UserController.java index 43a31e9..c6eabaa 100644 --- a/src/main/java/goodroad/users/users/UserController.java +++ b/src/main/java/goodroad/users/users/UserController.java @@ -31,11 +31,10 @@ public UserSettingsService.SettingsView updateCurrentUser( @PostMapping("") public void changePassword( - @RequestParam String oldPassword, - @RequestParam String newPassword + @RequestBody UserSettingsService.ChangePasswordReq req ) { String currentUsername = SecurityContextHolder.getContext().getAuthentication().getName(); - service.changePassword(currentUsername, oldPassword, newPassword); + service.changePassword(currentUsername, req); } @PostMapping(value = "/avatar", consumes = MediaType.MULTIPART_FORM_DATA_VALUE) diff --git a/src/main/java/goodroad/users/users/UserSettingsService.java b/src/main/java/goodroad/users/users/UserSettingsService.java index ccd8bf0..8e3f4eb 100644 --- a/src/main/java/goodroad/users/users/UserSettingsService.java +++ b/src/main/java/goodroad/users/users/UserSettingsService.java @@ -8,38 +8,38 @@ import goodroad.users.repository.UserEntity; import goodroad.users.repository.UserRepo; import goodroad.validation.InputRules; +import goodroad.validation.TrustedUrlService; import org.springframework.http.HttpStatus; -import org.springframework.http.ResponseEntity; import org.springframework.security.crypto.password.PasswordEncoder; import org.springframework.stereotype.Service; import org.springframework.transaction.annotation.Transactional; import org.springframework.web.multipart.MultipartFile; +import java.nio.charset.StandardCharsets; import java.time.Instant; -import java.util.Set; @SuppressWarnings({"DuplicatedCode", "SpellCheckingInspection"}) @Service public class UserSettingsService { - private static final long MAX_AVATAR_SIZE = 10 * 1024 * 1024; - private static final Set ALLOWED_AVATAR_TYPES = Set.of("image/jpeg", "image/png", "image/webp"); - private final UserRepo users; private final PasswordEncoder passwordEncoder; private final AuthService authService; private final StorageService storageService; + private final TrustedUrlService trustedUrls; public UserSettingsService( UserRepo users, PasswordEncoder passwordEncoder, AuthService authService, - StorageService storageService + StorageService storageService, + TrustedUrlService trustedUrls ) { this.users = users; this.passwordEncoder = passwordEncoder; this.authService = authService; this.storageService = storageService; + this.trustedUrls = trustedUrls; } public record SettingsView( @@ -56,7 +56,8 @@ public record UpdateSettingsReq( String firstName, String lastName, String photoUrl, - String phone + String phone, + String currentPassword ) { } @@ -65,6 +66,9 @@ public record AvatarUploadResp( ) { } + public record ChangePasswordReq(String oldPassword, String newPassword) { + } + public record DeleteAccountReq( String password ) { @@ -89,7 +93,7 @@ public SettingsView updateCurrentUserSettings(String phoneFromAuth, UpdateSettin throw new ApiException(HttpStatus.BAD_REQUEST, "USER_UPDATE_EMPTY", "No fields provided to update"); } - UserEntity user = findCurrent(phoneFromAuth); + UserEntity user = phone == null ? findCurrent(phoneFromAuth) : findCurrentForUpdate(phoneFromAuth); if (req.firstName() != null) { String firstName = InputRules.requireCyrillicText(req.firstName(), "USER_FIRST_NAME_INVALID", "First name"); @@ -100,9 +104,19 @@ public SettingsView updateCurrentUserSettings(String phoneFromAuth, UpdateSettin user.setLastName(lastName); } if (req.photoUrl() != null) { - user.setPhotoUrl(photoUrl); + user.setPhotoUrl(photoUrl == null ? null : trustedUrls.requireOwnedStorageUrl( + photoUrl, + "avatars", + user.getId(), + "AVATAR_URL_INVALID" + )); } if (phone != null) { + if (req.currentPassword() == null || req.currentPassword().isBlank() + || req.currentPassword().getBytes(StandardCharsets.UTF_8).length > 72 + || !passwordEncoder.matches(req.currentPassword(), user.getPassHash())) { + throw new ApiException(HttpStatus.UNAUTHORIZED, "CREDENTIALS_INVALID", "Credentials are invalid"); + } String newPhoneNorm = Crypto.normPhone(phone); if (newPhoneNorm.isEmpty()) { throw new ApiException(HttpStatus.BAD_REQUEST, "PHONE_INVALID", "Phone number is invalid"); @@ -124,22 +138,15 @@ public SettingsView updateCurrentUserSettings(String phoneFromAuth, UpdateSettin } @Transactional - public void changePassword(String phoneFromAuth, String oldPassword, String newPassword) { - authService.changePass(phoneFromAuth, oldPassword, newPassword); + public void changePassword(String phoneFromAuth, ChangePasswordReq req) { + if (req == null) { + throw new ApiException(HttpStatus.BAD_REQUEST, "PASSWORD_CHANGE_EMPTY", "Password change request is empty"); + } + authService.changePass(phoneFromAuth, req.oldPassword(), req.newPassword()); } @Transactional public AvatarUploadResp uploadAvatar(String phoneFromAuth, MultipartFile file) { - if (file == null || file.isEmpty()) { - throw new ApiException(HttpStatus.BAD_REQUEST, "AVATAR_EMPTY", "Avatar file is empty"); - } - if (file.getSize() > MAX_AVATAR_SIZE) { - throw new ApiException(HttpStatus.BAD_REQUEST, "AVATAR_TOO_LARGE", "Avatar file is too large"); - } - if (file.getContentType() == null || !ALLOWED_AVATAR_TYPES.contains(file.getContentType())) { - throw new ApiException(HttpStatus.BAD_REQUEST, "AVATAR_TYPE_INVALID", "Avatar file type is invalid"); - } - UserEntity user = findCurrent(phoneFromAuth); String photoUrl = storageService.uploadAvatar(file, user.getId().toString()); @@ -152,11 +159,11 @@ public AvatarUploadResp uploadAvatar(String phoneFromAuth, MultipartFile file) { @Transactional public void deleteCurrent(String phoneFromAuth, DeleteAccountReq req) { UserEntity user = requireCurrentWithPassword(phoneFromAuth, req); - if (!Role.USER.name().equals(user.getRole())) { + if (Role.MODERATOR.name().equals(user.getRole()) || Role.MODERATOR_ADMIN.name().equals(user.getRole())) { throw new ApiException( HttpStatus.FORBIDDEN, "USER_CANT_DELETE", - "Only regular users can delete their account" + "Moderator accounts can only be deleted by an administrator" ); } @@ -182,7 +189,8 @@ public void deleteByAdmin(String phoneFromAuth, String id, DeleteAccountReq req) } private UserEntity requireCurrentWithPassword(String phoneFromAuth, DeleteAccountReq req) { - if (req == null || req.password() == null || req.password().isBlank()) { + if (req == null || req.password() == null || req.password().isBlank() + || req.password().getBytes(StandardCharsets.UTF_8).length > 72) { throw new ApiException(HttpStatus.BAD_REQUEST, "PASSWORD_INVALID", "Password is invalid"); } @@ -213,6 +221,17 @@ private UserEntity findCurrent(String phoneFromAuth) { String phoneHash = Crypto.sha256Hex(phoneNorm); return users.findByPhoneHash(phoneHash) + .filter(UserEntity::isActive) + .orElseThrow(() -> new ApiException(HttpStatus.UNAUTHORIZED, "USER_PHONE_NOT_FOUND", "User with given phone not found")); + } + + private UserEntity findCurrentForUpdate(String phoneFromAuth) { + String phoneNorm = Crypto.normPhone(phoneFromAuth); + if (phoneNorm.isEmpty()) { + throw new ApiException(HttpStatus.UNAUTHORIZED, "USER_PHONE_NOT_FOUND", "User with given phone not found"); + } + return users.findByPhoneHashForUpdate(Crypto.sha256Hex(phoneNorm)) + .filter(UserEntity::isActive) .orElseThrow(() -> new ApiException(HttpStatus.UNAUTHORIZED, "USER_PHONE_NOT_FOUND", "User with given phone not found")); } diff --git a/src/test/java/goodroad/api/HttpApiScenarioTest.java b/src/test/java/goodroad/api/HttpApiScenarioTest.java index 0a9ba51..5319ec6 100644 --- a/src/test/java/goodroad/api/HttpApiScenarioTest.java +++ b/src/test/java/goodroad/api/HttpApiScenarioTest.java @@ -191,17 +191,23 @@ void shouldUseUserEndpoints() throws Exception { "firstName": "Иван", "lastName": "Иванов", "photoUrl": null, - "phone": "+79990000001" + "phone": "+79990000001", + "currentPassword": "123" } """)) .andExpect(status().isOk()) .andExpect(jsonPath("$.lastName").value("Иванов")); mvc.perform(post("/users") - .param("oldPassword", "123") - .param("newPassword", "1234")) + .contentType(MediaType.APPLICATION_JSON) + .content(""" + { + "oldPassword": "123", + "newPassword": "1234" + } + """)) .andExpect(status().isOk()); - verify(userSettingsService).changePassword("+79990000001", "123", "1234"); + verify(userSettingsService).changePassword(eq("+79990000001"), any(UserSettingsService.ChangePasswordReq.class)); MockMultipartFile file = new MockMultipartFile( "file", "avatar.png", "image/png", new byte[]{1, 2, 3} diff --git a/src/test/java/goodroad/users/users/UserSettingsServiceTest.java b/src/test/java/goodroad/users/users/UserSettingsServiceTest.java index 95ebf1a..dbd1852 100644 --- a/src/test/java/goodroad/users/users/UserSettingsServiceTest.java +++ b/src/test/java/goodroad/users/users/UserSettingsServiceTest.java @@ -6,6 +6,7 @@ import goodroad.storage.StorageService; import goodroad.users.repository.UserEntity; import goodroad.users.repository.UserRepo; +import goodroad.validation.TrustedUrlService; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.InjectMocks; @@ -34,6 +35,9 @@ class UserSettingsServiceTest { @Mock private StorageService storageService; + @Mock + private TrustedUrlService trustedUrls; + @InjectMocks private UserSettingsService service; @@ -52,10 +56,12 @@ void shouldGetCurrentUser() { @Test void shouldUpdateCurrentUser() { UserEntity user = user(1L, Role.USER.name()); - when(users.findByPhoneHash(anyString())).thenReturn(Optional.of(user)); + user.setPassHash("hash"); + when(users.findByPhoneHashForUpdate(anyString())).thenReturn(Optional.of(user)); + when(passwordEncoder.matches("pass", "hash")).thenReturn(true); UserSettingsService.UpdateSettingsReq req = new UserSettingsService.UpdateSettingsReq( - "Мария", "Петрова", "http://photo", "+79990000002" + "Мария", "Петрова", null, "+79990000002", "pass" ); UserSettingsService.SettingsView view = service.updateCurrentUserSettings("+79990000001", req); @@ -68,7 +74,10 @@ void shouldUpdateCurrentUser() { @Test void shouldChangePasswordThroughAuthService() { - service.changePassword("+79990000001", "old", "new"); + service.changePassword( + "+79990000001", + new UserSettingsService.ChangePasswordReq("old", "new") + ); verify(authService).changePass("+79990000001", "old", "new"); } @@ -89,16 +98,6 @@ void shouldUploadAvatar() { verify(users).save(user); } - @Test - void shouldRejectBadAvatarType() { - MockMultipartFile file = new MockMultipartFile( - "file", "avatar.txt", "text/plain", new byte[] {1} - ); - - assertThrows(RuntimeException.class, - () -> service.uploadAvatar("+79990000001", file)); - } - @Test void shouldDeleteCurrentUser() { UserEntity user = user(1L, Role.USER.name()); From 8dbda6b0f130275d0a26ffde4ce4114a764efcb5 Mon Sep 17 00:00:00 2001 From: VictoriaGrudtsyna Date: Sun, 30 Aug 2026 14:06:40 +0300 Subject: [PATCH 8/9] fix: separate changePhone from updateCurrentUserSettings --- docker-compose.yml | 1 + .../goodroad/users/users/UserController.java | 8 + .../users/users/UserSettingsService.java | 173 +++++++++++++++--- 3 files changed, 153 insertions(+), 29 deletions(-) diff --git a/docker-compose.yml b/docker-compose.yml index 93d646d..ac528de 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -53,6 +53,7 @@ services: YC_ACCESS_KEY: ${YC_ACCESS_KEY} YC_SECRET_KEY: ${YC_SECRET_KEY} YC_BUCKET: ${YC_BUCKET} + GRAPHHOPPER_API_KEY: ${GRAPHHOPPER_API_KEY} depends_on: db: condition: service_healthy diff --git a/src/main/java/goodroad/users/users/UserController.java b/src/main/java/goodroad/users/users/UserController.java index c6eabaa..c85caab 100644 --- a/src/main/java/goodroad/users/users/UserController.java +++ b/src/main/java/goodroad/users/users/UserController.java @@ -29,6 +29,14 @@ public UserSettingsService.SettingsView updateCurrentUser( return service.updateCurrentUserSettings(currentUsername, req); } + @PutMapping("/phone") + public UserSettingsService.SettingsView changePhone( + @RequestBody UserSettingsService.ChangePhoneReq req + ) { + String currentUsername = SecurityContextHolder.getContext().getAuthentication().getName(); + return service.changePhone(currentUsername, req); + } + @PostMapping("") public void changePassword( @RequestBody UserSettingsService.ChangePasswordReq req diff --git a/src/main/java/goodroad/users/users/UserSettingsService.java b/src/main/java/goodroad/users/users/UserSettingsService.java index 8e3f4eb..5d47122 100644 --- a/src/main/java/goodroad/users/users/UserSettingsService.java +++ b/src/main/java/goodroad/users/users/UserSettingsService.java @@ -14,6 +14,8 @@ import org.springframework.stereotype.Service; import org.springframework.transaction.annotation.Transactional; import org.springframework.web.multipart.MultipartFile; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import java.nio.charset.StandardCharsets; import java.time.Instant; @@ -28,6 +30,8 @@ public class UserSettingsService { private final StorageService storageService; private final TrustedUrlService trustedUrls; + private static final Logger log = LoggerFactory.getLogger(UserSettingsService.class); + public UserSettingsService( UserRepo users, PasswordEncoder passwordEncoder, @@ -55,9 +59,7 @@ public record SettingsView( public record UpdateSettingsReq( String firstName, String lastName, - String photoUrl, - String phone, - String currentPassword + String photoUrl ) { } @@ -69,6 +71,12 @@ public record AvatarUploadResp( public record ChangePasswordReq(String oldPassword, String newPassword) { } + public record ChangePhoneReq( + String phone, + String currentPassword + ) { + } + public record DeleteAccountReq( String password ) { @@ -87,22 +95,31 @@ public SettingsView updateCurrentUserSettings(String phoneFromAuth, UpdateSettin } String photoUrl = blankToNull(req.photoUrl()); - String phone = blankToNull(req.phone()); - if (req.firstName() == null && req.lastName() == null && req.photoUrl() == null && req.phone() == null) { + if (req.firstName() == null && req.lastName() == null && req.photoUrl() == null) { throw new ApiException(HttpStatus.BAD_REQUEST, "USER_UPDATE_EMPTY", "No fields provided to update"); } - UserEntity user = phone == null ? findCurrent(phoneFromAuth) : findCurrentForUpdate(phoneFromAuth); + UserEntity user = findCurrent(phoneFromAuth); if (req.firstName() != null) { - String firstName = InputRules.requireCyrillicText(req.firstName(), "USER_FIRST_NAME_INVALID", "First name"); + String firstName = InputRules.requireCyrillicText( + req.firstName(), + "USER_FIRST_NAME_INVALID", + "First name" + ); user.setFirstName(firstName); } + if (req.lastName() != null) { - String lastName = InputRules.requireCyrillicText(req.lastName(), "USER_LAST_NAME_INVALID", "Last name"); + String lastName = InputRules.requireCyrillicText( + req.lastName(), + "USER_LAST_NAME_INVALID", + "Last name" + ); user.setLastName(lastName); } + if (req.photoUrl() != null) { user.setPhotoUrl(photoUrl == null ? null : trustedUrls.requireOwnedStorageUrl( photoUrl, @@ -111,37 +128,93 @@ public SettingsView updateCurrentUserSettings(String phoneFromAuth, UpdateSettin "AVATAR_URL_INVALID" )); } - if (phone != null) { - if (req.currentPassword() == null || req.currentPassword().isBlank() - || req.currentPassword().getBytes(StandardCharsets.UTF_8).length > 72 - || !passwordEncoder.matches(req.currentPassword(), user.getPassHash())) { - throw new ApiException(HttpStatus.UNAUTHORIZED, "CREDENTIALS_INVALID", "Credentials are invalid"); + + user.setLastActiveAt(Instant.now()); + users.save(user); + + return toView(user); + } + + @Transactional + public SettingsView changePhone(String phoneFromAuth, ChangePhoneReq req) { + try { + if (req == null) { + throw new ApiException( + HttpStatus.BAD_REQUEST, + "PHONE_CHANGE_EMPTY", + "Phone change request is empty" + ); } + + String phone = req.phone(); + + if (phone == null || phone.isBlank()) { + throw new ApiException( + HttpStatus.BAD_REQUEST, + "PHONE_INVALID", + "Phone number is invalid" + ); + } + + UserEntity user = findCurrentForUpdate(phoneFromAuth); + + String currentPassword = req.currentPassword(); + + if (currentPassword == null || currentPassword.isBlank() + || currentPassword.getBytes(StandardCharsets.UTF_8).length > 72 + || !passwordEncoder.matches(currentPassword, user.getPassHash())) { + throw new ApiException( + HttpStatus.UNAUTHORIZED, + "CREDENTIALS_INVALID", + "Credentials are invalid" + ); + } + String newPhoneNorm = Crypto.normPhone(phone); + if (newPhoneNorm.isEmpty()) { - throw new ApiException(HttpStatus.BAD_REQUEST, "PHONE_INVALID", "Phone number is invalid"); + throw new ApiException( + HttpStatus.BAD_REQUEST, + "PHONE_INVALID", + "Phone number is invalid" + ); } String newPhoneHash = Crypto.sha256Hex(newPhoneNorm); + users.findByPhoneHash(newPhoneHash) .filter(other -> !other.getId().equals(user.getId())) .ifPresent(other -> { - throw new ApiException(HttpStatus.CONFLICT, "PHONE_ALREADY_USED", "Phone number already used"); + throw new ApiException( + HttpStatus.CONFLICT, + "PHONE_ALREADY_USED", + "Phone number already used" + ); }); user.setPhoneHash(newPhoneHash); - } + user.setLastActiveAt(Instant.now()); + users.save(user); - user.setLastActiveAt(Instant.now()); - users.save(user); - return toView(user); + return toView(user); + } + catch (Exception e) { + log.error("Error changing phone", e); // ← Добавить это + throw e; + } } + @Transactional public void changePassword(String phoneFromAuth, ChangePasswordReq req) { if (req == null) { - throw new ApiException(HttpStatus.BAD_REQUEST, "PASSWORD_CHANGE_EMPTY", "Password change request is empty"); + throw new ApiException( + HttpStatus.BAD_REQUEST, + "PASSWORD_CHANGE_EMPTY", + "Password change request is empty" + ); } + authService.changePass(phoneFromAuth, req.oldPassword(), req.newPassword()); } @@ -159,7 +232,9 @@ public AvatarUploadResp uploadAvatar(String phoneFromAuth, MultipartFile file) { @Transactional public void deleteCurrent(String phoneFromAuth, DeleteAccountReq req) { UserEntity user = requireCurrentWithPassword(phoneFromAuth, req); - if (Role.MODERATOR.name().equals(user.getRole()) || Role.MODERATOR_ADMIN.name().equals(user.getRole())) { + + if (Role.MODERATOR.name().equals(user.getRole()) + || Role.MODERATOR_ADMIN.name().equals(user.getRole())) { throw new ApiException( HttpStatus.FORBIDDEN, "USER_CANT_DELETE", @@ -173,6 +248,7 @@ public void deleteCurrent(String phoneFromAuth, DeleteAccountReq req) { @Transactional public void deleteByAdmin(String phoneFromAuth, String id, DeleteAccountReq req) { UserEntity admin = requireCurrentWithPassword(phoneFromAuth, req); + if (!Role.MODERATOR_ADMIN.name().equals(admin.getRole())) { throw new ApiException( HttpStatus.FORBIDDEN, @@ -182,8 +258,13 @@ public void deleteByAdmin(String phoneFromAuth, String id, DeleteAccountReq req) } Long userId = parseId(id); + UserEntity user = users.findById(userId) - .orElseThrow(() -> new ApiException(HttpStatus.NOT_FOUND, "USER_ID_NOT_FOUND", "User id not found")); + .orElseThrow(() -> new ApiException( + HttpStatus.NOT_FOUND, + "USER_ID_NOT_FOUND", + "User id not found" + )); users.delete(user); } @@ -191,12 +272,21 @@ public void deleteByAdmin(String phoneFromAuth, String id, DeleteAccountReq req) private UserEntity requireCurrentWithPassword(String phoneFromAuth, DeleteAccountReq req) { if (req == null || req.password() == null || req.password().isBlank() || req.password().getBytes(StandardCharsets.UTF_8).length > 72) { - throw new ApiException(HttpStatus.BAD_REQUEST, "PASSWORD_INVALID", "Password is invalid"); + throw new ApiException( + HttpStatus.BAD_REQUEST, + "PASSWORD_INVALID", + "Password is invalid" + ); } UserEntity user = findCurrent(phoneFromAuth); + if (!passwordEncoder.matches(req.password(), user.getPassHash())) { - throw new ApiException(HttpStatus.UNAUTHORIZED, "CREDENTIALS_INVALID", "Credentials are invalid"); + throw new ApiException( + HttpStatus.UNAUTHORIZED, + "CREDENTIALS_INVALID", + "Credentials are invalid" + ); } return user; @@ -215,31 +305,55 @@ private SettingsView toView(UserEntity user) { private UserEntity findCurrent(String phoneFromAuth) { String phoneNorm = Crypto.normPhone(phoneFromAuth); + if (phoneNorm.isEmpty()) { - throw new ApiException(HttpStatus.UNAUTHORIZED, "USER_PHONE_NOT_FOUND", "User with given phone not found"); + throw new ApiException( + HttpStatus.UNAUTHORIZED, + "USER_PHONE_NOT_FOUND", + "User with given phone not found" + ); } String phoneHash = Crypto.sha256Hex(phoneNorm); + return users.findByPhoneHash(phoneHash) .filter(UserEntity::isActive) - .orElseThrow(() -> new ApiException(HttpStatus.UNAUTHORIZED, "USER_PHONE_NOT_FOUND", "User with given phone not found")); + .orElseThrow(() -> new ApiException( + HttpStatus.UNAUTHORIZED, + "USER_PHONE_NOT_FOUND", + "User with given phone not found" + )); } private UserEntity findCurrentForUpdate(String phoneFromAuth) { String phoneNorm = Crypto.normPhone(phoneFromAuth); + if (phoneNorm.isEmpty()) { - throw new ApiException(HttpStatus.UNAUTHORIZED, "USER_PHONE_NOT_FOUND", "User with given phone not found"); + throw new ApiException( + HttpStatus.UNAUTHORIZED, + "USER_PHONE_NOT_FOUND", + "User with given phone not found" + ); } + return users.findByPhoneHashForUpdate(Crypto.sha256Hex(phoneNorm)) .filter(UserEntity::isActive) - .orElseThrow(() -> new ApiException(HttpStatus.UNAUTHORIZED, "USER_PHONE_NOT_FOUND", "User with given phone not found")); + .orElseThrow(() -> new ApiException( + HttpStatus.UNAUTHORIZED, + "USER_PHONE_NOT_FOUND", + "User with given phone not found" + )); } private Long parseId(String raw) { try { return Long.parseLong(raw); } catch (NumberFormatException e) { - throw new ApiException(HttpStatus.BAD_REQUEST, "ID_INVALID", "Id is invalid"); + throw new ApiException( + HttpStatus.BAD_REQUEST, + "ID_INVALID", + "Id is invalid" + ); } } @@ -247,6 +361,7 @@ private static String blankToNull(String value) { if (value == null) { return null; } + String s = value.trim(); return s.isEmpty() ? null : s; } From 2198990c9182902303c551f6f0357ee4fcfa8c8a Mon Sep 17 00:00:00 2001 From: VictoriaGrudtsyna Date: Sun, 30 Aug 2026 14:55:45 +0300 Subject: [PATCH 9/9] fix: naming in UserController and UserProfileService, fix: tests for UserProfileService --- .../goodroad/users/users/UserController.java | 24 +- ...gsService.java => UserProfileService.java} | 22 +- .../goodroad/api/HttpApiScenarioTest.java | 72 +++-- ...eTest.java => UserProfileServiceTest.java} | 263 +++++++++--------- 4 files changed, 205 insertions(+), 176 deletions(-) rename src/main/java/goodroad/users/users/{UserSettingsService.java => UserProfileService.java} (92%) rename src/test/java/goodroad/users/users/{UserSettingsServiceTest.java => UserProfileServiceTest.java} (70%) diff --git a/src/main/java/goodroad/users/users/UserController.java b/src/main/java/goodroad/users/users/UserController.java index c85caab..915e378 100644 --- a/src/main/java/goodroad/users/users/UserController.java +++ b/src/main/java/goodroad/users/users/UserController.java @@ -9,29 +9,29 @@ @RequestMapping("/users") public class UserController { - private final UserSettingsService service; + private final UserProfileService service; - public UserController(UserSettingsService service) { + public UserController(UserProfileService service) { this.service = service; } @GetMapping("") - public UserSettingsService.SettingsView getCurrentUser() { + public UserProfileService.ProfileView getCurrentUser() { String currentUsername = SecurityContextHolder.getContext().getAuthentication().getName(); return service.getCurrentUser(currentUsername); } @PutMapping("") - public UserSettingsService.SettingsView updateCurrentUser( - @RequestBody UserSettingsService.UpdateSettingsReq req + public UserProfileService.ProfileView updateProfile( + @RequestBody UserProfileService.UpdateProfileReq req ) { String currentUsername = SecurityContextHolder.getContext().getAuthentication().getName(); - return service.updateCurrentUserSettings(currentUsername, req); + return service.updateProfile(currentUsername, req); } @PutMapping("/phone") - public UserSettingsService.SettingsView changePhone( - @RequestBody UserSettingsService.ChangePhoneReq req + public UserProfileService.ProfileView changePhone( + @RequestBody UserProfileService.ChangePhoneReq req ) { String currentUsername = SecurityContextHolder.getContext().getAuthentication().getName(); return service.changePhone(currentUsername, req); @@ -39,14 +39,14 @@ public UserSettingsService.SettingsView changePhone( @PostMapping("") public void changePassword( - @RequestBody UserSettingsService.ChangePasswordReq req + @RequestBody UserProfileService.ChangePasswordReq req ) { String currentUsername = SecurityContextHolder.getContext().getAuthentication().getName(); service.changePassword(currentUsername, req); } @PostMapping(value = "/avatar", consumes = MediaType.MULTIPART_FORM_DATA_VALUE) - public UserSettingsService.AvatarUploadResp uploadAvatar( + public UserProfileService.AvatarUploadResp uploadAvatar( @RequestParam("file") MultipartFile file ) { String currentUsername = SecurityContextHolder.getContext().getAuthentication().getName(); @@ -55,7 +55,7 @@ public UserSettingsService.AvatarUploadResp uploadAvatar( @DeleteMapping("") public void deleteCurrentUser( - @RequestBody UserSettingsService.DeleteAccountReq req + @RequestBody UserProfileService.DeleteAccountReq req ) { String currentUsername = SecurityContextHolder.getContext().getAuthentication().getName(); service.deleteCurrent(currentUsername, req); @@ -64,7 +64,7 @@ public void deleteCurrentUser( @DeleteMapping("/{id}") public void deleteUserByAdmin( @PathVariable String id, - @RequestBody UserSettingsService.DeleteAccountReq req + @RequestBody UserProfileService.DeleteAccountReq req ) { String currentUsername = SecurityContextHolder.getContext().getAuthentication().getName(); service.deleteByAdmin(currentUsername, id, req); diff --git a/src/main/java/goodroad/users/users/UserSettingsService.java b/src/main/java/goodroad/users/users/UserProfileService.java similarity index 92% rename from src/main/java/goodroad/users/users/UserSettingsService.java rename to src/main/java/goodroad/users/users/UserProfileService.java index 5d47122..f2716e6 100644 --- a/src/main/java/goodroad/users/users/UserSettingsService.java +++ b/src/main/java/goodroad/users/users/UserProfileService.java @@ -22,7 +22,7 @@ @SuppressWarnings({"DuplicatedCode", "SpellCheckingInspection"}) @Service -public class UserSettingsService { +public class UserProfileService { private final UserRepo users; private final PasswordEncoder passwordEncoder; @@ -30,9 +30,9 @@ public class UserSettingsService { private final StorageService storageService; private final TrustedUrlService trustedUrls; - private static final Logger log = LoggerFactory.getLogger(UserSettingsService.class); + private static final Logger log = LoggerFactory.getLogger(UserProfileService.class); - public UserSettingsService( + public UserProfileService( UserRepo users, PasswordEncoder passwordEncoder, AuthService authService, @@ -46,7 +46,7 @@ public UserSettingsService( this.trustedUrls = trustedUrls; } - public record SettingsView( + public record ProfileView( String id, String role, String firstName, @@ -56,7 +56,7 @@ public record SettingsView( ) { } - public record UpdateSettingsReq( + public record UpdateProfileReq( String firstName, String lastName, String photoUrl @@ -83,13 +83,13 @@ public record DeleteAccountReq( } @Transactional(readOnly = true) - public SettingsView getCurrentUser(String phoneFromAuth) { + public ProfileView getCurrentUser(String phoneFromAuth) { UserEntity user = findCurrent(phoneFromAuth); return toView(user); } @Transactional - public SettingsView updateCurrentUserSettings(String phoneFromAuth, UpdateSettingsReq req) { + public ProfileView updateProfile(String phoneFromAuth, UpdateProfileReq req) { if (req == null) { throw new ApiException(HttpStatus.BAD_REQUEST, "USER_UPDATE_EMPTY", "No fields provided to update"); } @@ -136,7 +136,7 @@ public SettingsView updateCurrentUserSettings(String phoneFromAuth, UpdateSettin } @Transactional - public SettingsView changePhone(String phoneFromAuth, ChangePhoneReq req) { + public ProfileView changePhone(String phoneFromAuth, ChangePhoneReq req) { try { if (req == null) { throw new ApiException( @@ -199,7 +199,7 @@ public SettingsView changePhone(String phoneFromAuth, ChangePhoneReq req) { return toView(user); } catch (Exception e) { - log.error("Error changing phone", e); // ← Добавить это + log.error("Error changing phone", e); throw e; } } @@ -292,8 +292,8 @@ private UserEntity requireCurrentWithPassword(String phoneFromAuth, DeleteAccoun return user; } - private SettingsView toView(UserEntity user) { - return new SettingsView( + private ProfileView toView(UserEntity user) { + return new ProfileView( user.getId().toString(), user.getRole(), user.getFirstName(), diff --git a/src/test/java/goodroad/api/HttpApiScenarioTest.java b/src/test/java/goodroad/api/HttpApiScenarioTest.java index 5319ec6..04090aa 100644 --- a/src/test/java/goodroad/api/HttpApiScenarioTest.java +++ b/src/test/java/goodroad/api/HttpApiScenarioTest.java @@ -19,7 +19,7 @@ import goodroad.users.moderators.ModeratorController; import goodroad.users.moderators.ModeratorService; import goodroad.users.users.UserController; -import goodroad.users.users.UserSettingsService; +import goodroad.users.users.UserProfileService; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -52,7 +52,7 @@ class HttpApiScenarioTest { private AuthService authService; @Mock - private UserSettingsService userSettingsService; + private UserProfileService userProfileService; @Mock private ModeratorService moderatorService; @@ -165,19 +165,23 @@ void shouldRecoverPassword() throws Exception { @Test void shouldUseUserEndpoints() throws Exception { - MockMvc mvc = standaloneSetup(new UserController(userSettingsService)).build(); + MockMvc mvc = standaloneSetup(new UserController(userProfileService)).build(); setCurrentUser("+79990000001"); - when(userSettingsService.getCurrentUser("+79990000001")) - .thenReturn(new UserSettingsService.SettingsView( + when(userProfileService.getCurrentUser("+79990000001")) + .thenReturn(new UserProfileService.ProfileView( "10", "USER", "Иван", "Петров", null, true )); - when(userSettingsService.updateCurrentUserSettings(eq("+79990000001"), any(UserSettingsService.UpdateSettingsReq.class))) - .thenReturn(new UserSettingsService.SettingsView( + when(userProfileService.updateProfile(eq("+79990000001"), any(UserProfileService.UpdateProfileReq.class))) + .thenReturn(new UserProfileService.ProfileView( "10", "USER", "Иван", "Иванов", null, true )); - when(userSettingsService.uploadAvatar(eq("+79990000001"), any())) - .thenReturn(new UserSettingsService.AvatarUploadResp("https://storage/avatar.png")); + when(userProfileService.changePhone(eq("+79990000001"), any(UserProfileService.ChangePhoneReq.class))) + .thenReturn(new UserProfileService.ProfileView( + "10", "USER", "Иван", "Петров", null, true + )); + when(userProfileService.uploadAvatar(eq("+79990000001"), any())) + .thenReturn(new UserProfileService.AvatarUploadResp("https://storage/avatar.png")); mvc.perform(get("/users")) .andExpect(status().isOk()) @@ -187,27 +191,37 @@ void shouldUseUserEndpoints() throws Exception { mvc.perform(put("/users") .contentType(MediaType.APPLICATION_JSON) .content(""" - { - "firstName": "Иван", - "lastName": "Иванов", - "photoUrl": null, - "phone": "+79990000001", - "currentPassword": "123" - } - """)) + { + "firstName": "Иван", + "lastName": "Иванов", + "photoUrl": null + } + """)) .andExpect(status().isOk()) .andExpect(jsonPath("$.lastName").value("Иванов")); + mvc.perform(put("/users/phone") + .contentType(MediaType.APPLICATION_JSON) + .content(""" + { + "phone": "+79990000002", + "currentPassword": "123" + } + """)) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.id").value("10")); + verify(userProfileService).changePhone(eq("+79990000001"), any(UserProfileService.ChangePhoneReq.class)); + mvc.perform(post("/users") .contentType(MediaType.APPLICATION_JSON) .content(""" - { - "oldPassword": "123", - "newPassword": "1234" - } - """)) + { + "oldPassword": "123", + "newPassword": "1234" + } + """)) .andExpect(status().isOk()); - verify(userSettingsService).changePassword(eq("+79990000001"), any(UserSettingsService.ChangePasswordReq.class)); + verify(userProfileService).changePassword(eq("+79990000001"), any(UserProfileService.ChangePasswordReq.class)); MockMultipartFile file = new MockMultipartFile( "file", "avatar.png", "image/png", new byte[]{1, 2, 3} @@ -219,12 +233,12 @@ void shouldUseUserEndpoints() throws Exception { mvc.perform(delete("/users") .contentType(MediaType.APPLICATION_JSON) .content(""" - { - "password": "123" - } - """)) + { + "password": "123" + } + """)) .andExpect(status().isOk()); - verify(userSettingsService).deleteCurrent(eq("+79990000001"), any(UserSettingsService.DeleteAccountReq.class)); + verify(userProfileService).deleteCurrent(eq("+79990000001"), any(UserProfileService.DeleteAccountReq.class)); } @Test @@ -547,4 +561,4 @@ private String reviewJson() { } """; } -} +} \ No newline at end of file diff --git a/src/test/java/goodroad/users/users/UserSettingsServiceTest.java b/src/test/java/goodroad/users/users/UserProfileServiceTest.java similarity index 70% rename from src/test/java/goodroad/users/users/UserSettingsServiceTest.java rename to src/test/java/goodroad/users/users/UserProfileServiceTest.java index dbd1852..2eb4457 100644 --- a/src/test/java/goodroad/users/users/UserSettingsServiceTest.java +++ b/src/test/java/goodroad/users/users/UserProfileServiceTest.java @@ -1,124 +1,139 @@ -package goodroad.users.users; - -import goodroad.auth.AuthService; -import goodroad.model.Role; -import goodroad.security.Crypto; -import goodroad.storage.StorageService; -import goodroad.users.repository.UserEntity; -import goodroad.users.repository.UserRepo; -import goodroad.validation.TrustedUrlService; -import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.extension.ExtendWith; -import org.mockito.InjectMocks; -import org.mockito.Mock; -import org.mockito.junit.jupiter.MockitoExtension; -import org.springframework.mock.web.MockMultipartFile; -import org.springframework.security.crypto.password.PasswordEncoder; - -import java.util.Optional; - -import static org.junit.jupiter.api.Assertions.*; -import static org.mockito.Mockito.*; - -@ExtendWith(MockitoExtension.class) -class UserSettingsServiceTest { - - @Mock - private UserRepo users; - - @Mock - private PasswordEncoder passwordEncoder; - - @Mock - private AuthService authService; - - @Mock - private StorageService storageService; - - @Mock - private TrustedUrlService trustedUrls; - - @InjectMocks - private UserSettingsService service; - - @Test - void shouldGetCurrentUser() { - UserEntity user = user(1L, Role.USER.name()); - when(users.findByPhoneHash(anyString())).thenReturn(Optional.of(user)); - - UserSettingsService.SettingsView view = service.getCurrentUser("+79990000001"); - - assertEquals("1", view.id()); - assertEquals(Role.USER.name(), view.role()); - assertTrue(view.active()); - } - - @Test - void shouldUpdateCurrentUser() { - UserEntity user = user(1L, Role.USER.name()); - user.setPassHash("hash"); - when(users.findByPhoneHashForUpdate(anyString())).thenReturn(Optional.of(user)); - when(passwordEncoder.matches("pass", "hash")).thenReturn(true); - - UserSettingsService.UpdateSettingsReq req = new UserSettingsService.UpdateSettingsReq( - "Мария", "Петрова", null, "+79990000002", "pass" - ); - - UserSettingsService.SettingsView view = service.updateCurrentUserSettings("+79990000001", req); - - assertEquals("Мария", view.firstName()); - assertEquals("Петрова", view.lastName()); - assertEquals(Crypto.sha256Hex("79990000002"), user.getPhoneHash()); - verify(users).save(user); - } - - @Test - void shouldChangePasswordThroughAuthService() { - service.changePassword( - "+79990000001", - new UserSettingsService.ChangePasswordReq("old", "new") - ); - - verify(authService).changePass("+79990000001", "old", "new"); - } - - @Test - void shouldUploadAvatar() { - UserEntity user = user(1L, Role.USER.name()); - MockMultipartFile file = new MockMultipartFile( - "file", "avatar.png", "image/png", new byte[] {1, 2, 3} - ); - when(users.findByPhoneHash(anyString())).thenReturn(Optional.of(user)); - when(storageService.uploadAvatar(file, "1")).thenReturn("http://avatar"); - - UserSettingsService.AvatarUploadResp resp = service.uploadAvatar("+79990000001", file); - - assertEquals("http://avatar", resp.photoUrl()); - assertEquals("http://avatar", user.getPhotoUrl()); - verify(users).save(user); - } - - @Test - void shouldDeleteCurrentUser() { - UserEntity user = user(1L, Role.USER.name()); - user.setPassHash("hash"); - when(users.findByPhoneHash(anyString())).thenReturn(Optional.of(user)); - when(passwordEncoder.matches("pass", "hash")).thenReturn(true); - - service.deleteCurrent("+79990000001", new UserSettingsService.DeleteAccountReq("pass")); - - verify(users).delete(user); - } - - private UserEntity user(Long id, String role) { - UserEntity user = UserEntity.builder() - .firstName("Анна") - .lastName("Иванова") - .phoneHash(Crypto.sha256Hex("79990000001")) - .role(role) - .active(true) - .build(); - user.setId(id); - return user; - } -} +package goodroad.users.users; + +import goodroad.auth.AuthService; +import goodroad.model.Role; +import goodroad.security.Crypto; +import goodroad.storage.StorageService; +import goodroad.users.repository.UserEntity; +import goodroad.users.repository.UserRepo; +import goodroad.validation.TrustedUrlService; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import org.springframework.mock.web.MockMultipartFile; +import org.springframework.security.crypto.password.PasswordEncoder; + +import java.util.Optional; + +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.Mockito.*; + +@ExtendWith(MockitoExtension.class) +class UserProfileServiceTest { + + @Mock + private UserRepo users; + + @Mock + private PasswordEncoder passwordEncoder; + + @Mock + private AuthService authService; + + @Mock + private StorageService storageService; + + @Mock + private TrustedUrlService trustedUrls; + + @InjectMocks + private UserProfileService service; + + @Test + void shouldGetCurrentUser() { + UserEntity user = user(1L, Role.USER.name()); + when(users.findByPhoneHash(anyString())).thenReturn(Optional.of(user)); + + UserProfileService.ProfileView view = service.getCurrentUser("+79990000001"); + + assertEquals("1", view.id()); + assertEquals(Role.USER.name(), view.role()); + assertTrue(view.active()); + } + + @Test + void shouldUpdateProfile() { + UserEntity user = user(1L, Role.USER.name()); + when(users.findByPhoneHash(anyString())).thenReturn(Optional.of(user)); + + UserProfileService.UpdateProfileReq req = new UserProfileService.UpdateProfileReq( + "Мария", "Петрова", null + ); + + UserProfileService.ProfileView view = service.updateProfile("+79990000001", req); + + assertEquals("Мария", view.firstName()); + assertEquals("Петрова", view.lastName()); + verify(users).save(user); + } + + @Test + void shouldChangePhone() { + UserEntity user = user(1L, Role.USER.name()); + user.setPassHash("hash"); + when(users.findByPhoneHashForUpdate(anyString())).thenReturn(Optional.of(user)); + when(passwordEncoder.matches("pass", "hash")).thenReturn(true); + when(users.findByPhoneHash(anyString())).thenReturn(Optional.empty()); + + UserProfileService.ChangePhoneReq req = new UserProfileService.ChangePhoneReq( + "+79990000002", "pass" + ); + + UserProfileService.ProfileView view = service.changePhone("+79990000001", req); + + assertNotNull(view); + verify(users).save(user); + } + + @Test + void shouldChangePasswordThroughAuthService() { + service.changePassword( + "+79990000001", + new UserProfileService.ChangePasswordReq("old", "new") + ); + + verify(authService).changePass("+79990000001", "old", "new"); + } + + @Test + void shouldUploadAvatar() { + UserEntity user = user(1L, Role.USER.name()); + MockMultipartFile file = new MockMultipartFile( + "file", "avatar.png", "image/png", new byte[] {1, 2, 3} + ); + when(users.findByPhoneHash(anyString())).thenReturn(Optional.of(user)); + when(storageService.uploadAvatar(file, "1")).thenReturn("http://avatar"); + + UserProfileService.AvatarUploadResp resp = service.uploadAvatar("+79990000001", file); + + assertEquals("http://avatar", resp.photoUrl()); + assertEquals("http://avatar", user.getPhotoUrl()); + verify(users).save(user); + } + + @Test + void shouldDeleteCurrentUser() { + UserEntity user = user(1L, Role.USER.name()); + user.setPassHash("hash"); + when(users.findByPhoneHash(anyString())).thenReturn(Optional.of(user)); + when(passwordEncoder.matches("pass", "hash")).thenReturn(true); + + service.deleteCurrent("+79990000001", new UserProfileService.DeleteAccountReq("pass")); + + verify(users).delete(user); + } + + private UserEntity user(Long id, String role) { + UserEntity user = UserEntity.builder() + .firstName("Анна") + .lastName("Иванова") + .phoneHash(Crypto.sha256Hex("79990000001")) + .role(role) + .active(true) + .build(); + user.setId(id); + return user; + } +} \ No newline at end of file