refactor(api): Map→SaveReq DTO 교체 + 입력 검증 + 잔여 Enum 적용 (리뷰 3b)

infra-reviewer가 지적한 입력 검증 누락 5건과 잔여 매직 스트링 정리.

Map<String,String> 입력 제거 — 신규 DTO 4종:
- auth/ChangePasswordReq (@NotBlank oldPassword/newPassword)
- auth/RefreshTokenReq (@NotBlank refreshToken)
- controller/settle/PaymentStatusUpdateReq (@NotBlank status)
- controller/receive/ResolveErrorReq (@NotBlank status, optional note)

검증 보강:
- PaymentController.generateTransferFile — settleMonth @Pattern("\d{6}")
- UploadController.preview — limit @Max(100)

잔여 매직 스트링 → 3a Enum 적용:
- AuthService 4곳 — UserStatus.LOCKED/RETIRED/ACTIVE
- UserService 2곳 — UserStatus.ACTIVE
- LedgerService:71 — ExceptionLedgerStatus.NEW
This commit is contained in:
GA Pro
2026-05-12 23:28:41 +09:00
parent 693275e9e0
commit af3c6b9d51
11 changed files with 87 additions and 24 deletions
@@ -8,8 +8,6 @@ import jakarta.validation.Valid;
import lombok.RequiredArgsConstructor; import lombok.RequiredArgsConstructor;
import org.springframework.web.bind.annotation.*; import org.springframework.web.bind.annotation.*;
import java.util.Map;
@Tag(name = "인증") @Tag(name = "인증")
@RestController @RestController
@RequestMapping("/api/auth") @RequestMapping("/api/auth")
@@ -24,8 +22,8 @@ public class AuthController {
} }
@PostMapping("/refresh") @PostMapping("/refresh")
public ApiResponse<LoginResp> refresh(@RequestBody Map<String, String> body) { public ApiResponse<LoginResp> refresh(@Valid @RequestBody RefreshTokenReq req) {
return ApiResponse.ok(authService.refresh(body.get("refreshToken"))); return ApiResponse.ok(authService.refresh(req.getRefreshToken()));
} }
@GetMapping("/me") @GetMapping("/me")
@@ -34,8 +32,8 @@ public class AuthController {
} }
@PostMapping("/password") @PostMapping("/password")
public ApiResponse<Void> changePassword(@RequestBody Map<String, String> body) { public ApiResponse<Void> changePassword(@Valid @RequestBody ChangePasswordReq req) {
authService.changePassword(body.get("oldPassword"), body.get("newPassword")); authService.changePassword(req.getOldPassword(), req.getNewPassword());
return ApiResponse.ok(); return ApiResponse.ok();
} }
@@ -8,6 +8,7 @@ import com.ga.common.system.SystemLogMapper;
import com.ga.common.util.SecurityUtil; import com.ga.common.util.SecurityUtil;
import com.ga.core.mapper.user.UserMapper; import com.ga.core.mapper.user.UserMapper;
import com.ga.core.vo.user.UserResp; import com.ga.core.vo.user.UserResp;
import com.ga.core.vo.user.UserStatus;
import com.ga.core.vo.user.UserVO; import com.ga.core.vo.user.UserVO;
import io.jsonwebtoken.Claims; import io.jsonwebtoken.Claims;
import lombok.RequiredArgsConstructor; import lombok.RequiredArgsConstructor;
@@ -38,18 +39,18 @@ public class AuthService {
saveLoginLog(null, req.getLoginId(), "FAIL", "USER_NOT_FOUND", ipAddress, userAgent); saveLoginLog(null, req.getLoginId(), "FAIL", "USER_NOT_FOUND", ipAddress, userAgent);
throw new BizException(ErrorCode.LOGIN_FAIL); throw new BizException(ErrorCode.LOGIN_FAIL);
} }
if ("LOCKED".equals(user.getStatus())) { if (UserStatus.LOCKED.name().equals(user.getStatus())) {
saveLoginLog(user.getUserId(), req.getLoginId(), "FAIL", "ACCOUNT_LOCKED", ipAddress, userAgent); saveLoginLog(user.getUserId(), req.getLoginId(), "FAIL", "ACCOUNT_LOCKED", ipAddress, userAgent);
throw new BizException(ErrorCode.ACCOUNT_LOCKED); throw new BizException(ErrorCode.ACCOUNT_LOCKED);
} }
if ("RETIRED".equals(user.getStatus())) { if (UserStatus.RETIRED.name().equals(user.getStatus())) {
throw new BizException(ErrorCode.ACCOUNT_RETIRED); throw new BizException(ErrorCode.ACCOUNT_RETIRED);
} }
if (!passwordEncoder.matches(req.getPassword(), user.getPassword())) { if (!passwordEncoder.matches(req.getPassword(), user.getPassword())) {
userMapper.incrementFailCount(user.getUserId()); userMapper.incrementFailCount(user.getUserId());
int newFail = (user.getFailCount() == null ? 0 : user.getFailCount()) + 1; int newFail = (user.getFailCount() == null ? 0 : user.getFailCount()) + 1;
if (newFail >= failLimit) { if (newFail >= failLimit) {
userMapper.updateStatus(user.getUserId(), "LOCKED"); userMapper.updateStatus(user.getUserId(), UserStatus.LOCKED.name());
} }
saveLoginLog(user.getUserId(), req.getLoginId(), "FAIL", "BAD_PASSWORD", ipAddress, userAgent); saveLoginLog(user.getUserId(), req.getLoginId(), "FAIL", "BAD_PASSWORD", ipAddress, userAgent);
throw new BizException(ErrorCode.LOGIN_FAIL); throw new BizException(ErrorCode.LOGIN_FAIL);
@@ -80,7 +81,7 @@ public class AuthService {
} }
Long userId = claims.get("uid", Long.class); Long userId = claims.get("uid", Long.class);
UserVO user = userMapper.selectById(userId); UserVO user = userMapper.selectById(userId);
if (user == null || !"ACTIVE".equals(user.getStatus())) { if (user == null || !UserStatus.ACTIVE.name().equals(user.getStatus())) {
throw new BizException(ErrorCode.UNAUTHORIZED); throw new BizException(ErrorCode.UNAUTHORIZED);
} }
List<String> roles = userMapper.selectRoleCodes(userId); List<String> roles = userMapper.selectRoleCodes(userId);
@@ -0,0 +1,16 @@
package com.ga.api.auth;
import jakarta.validation.constraints.NotBlank;
import lombok.Getter;
import lombok.Setter;
@Getter
@Setter
public class ChangePasswordReq {
@NotBlank
private String oldPassword;
@NotBlank
private String newPassword;
}
@@ -0,0 +1,13 @@
package com.ga.api.auth;
import jakarta.validation.constraints.NotBlank;
import lombok.Getter;
import lombok.Setter;
@Getter
@Setter
public class RefreshTokenReq {
@NotBlank
private String refreshToken;
}
@@ -6,11 +6,11 @@ import com.ga.common.annotation.RequirePermission;
import com.ga.common.model.ApiResponse; import com.ga.common.model.ApiResponse;
import com.ga.core.vo.receive.*; import com.ga.core.vo.receive.*;
import io.swagger.v3.oas.annotations.tags.Tag; import io.swagger.v3.oas.annotations.tags.Tag;
import jakarta.validation.Valid;
import lombok.RequiredArgsConstructor; import lombok.RequiredArgsConstructor;
import org.springframework.web.bind.annotation.*; import org.springframework.web.bind.annotation.*;
import java.util.List; import java.util.List;
import java.util.Map;
@Tag(name = "데이터 수신") @Tag(name = "데이터 수신")
@RestController @RestController
@@ -100,10 +100,8 @@ public class ReceiveController {
@PutMapping("/errors/{errorId}/resolve") @PutMapping("/errors/{errorId}/resolve")
@RequirePermission(menu = "RECEIVE_DATA", perm = "UPDATE") @RequirePermission(menu = "RECEIVE_DATA", perm = "UPDATE")
@DataChangeLog(menu = "RECEIVE_DATA", table = "parse_error_log") @DataChangeLog(menu = "RECEIVE_DATA", table = "parse_error_log")
public ApiResponse<Void> resolveError(@PathVariable Long errorId, @RequestBody Map<String, String> body) { public ApiResponse<Void> resolveError(@PathVariable Long errorId, @Valid @RequestBody ResolveErrorReq req) {
receiveService.resolveError(errorId, receiveService.resolveError(errorId, req.getStatus(), req.getNote());
body.getOrDefault("status", "RESOLVED"),
body.get("note"));
return ApiResponse.ok(); return ApiResponse.ok();
} }
} }
@@ -0,0 +1,15 @@
package com.ga.api.controller.receive;
import jakarta.validation.constraints.NotBlank;
import lombok.Getter;
import lombok.Setter;
@Getter
@Setter
public class ResolveErrorReq {
@NotBlank
private String status;
private String note;
}
@@ -11,12 +11,14 @@ import com.ga.core.vo.settle.SettleSearchParam;
import com.ga.core.vo.settle.TransferFileResp; import com.ga.core.vo.settle.TransferFileResp;
import io.swagger.v3.oas.annotations.Operation; import io.swagger.v3.oas.annotations.Operation;
import io.swagger.v3.oas.annotations.tags.Tag; import io.swagger.v3.oas.annotations.tags.Tag;
import jakarta.validation.Valid;
import jakarta.validation.constraints.Pattern;
import lombok.RequiredArgsConstructor; import lombok.RequiredArgsConstructor;
import org.springframework.validation.annotation.Validated;
import org.springframework.web.bind.annotation.*; import org.springframework.web.bind.annotation.*;
import java.util.Map;
@Tag(name = "지급 관리") @Tag(name = "지급 관리")
@Validated
@RestController @RestController
@RequestMapping("/api/payments") @RequestMapping("/api/payments")
@RequiredArgsConstructor @RequiredArgsConstructor
@@ -35,8 +37,8 @@ public class PaymentController {
@PutMapping("/{paymentId}/status") @PutMapping("/{paymentId}/status")
@RequirePermission(menu = "PAYMENT", perm = "UPDATE") @RequirePermission(menu = "PAYMENT", perm = "UPDATE")
@DataChangeLog(menu = "PAYMENT", table = "payment") @DataChangeLog(menu = "PAYMENT", table = "payment")
public ApiResponse<Void> updateStatus(@PathVariable Long paymentId, @RequestBody Map<String, String> body) { public ApiResponse<Void> updateStatus(@PathVariable Long paymentId, @Valid @RequestBody PaymentStatusUpdateReq req) {
mapper.updateStatus(paymentId, body.get("status"), body.get("failReason")); mapper.updateStatus(paymentId, req.getStatus(), req.getFailReason());
return ApiResponse.ok(); return ApiResponse.ok();
} }
@@ -45,7 +47,7 @@ public class PaymentController {
@RequirePermission(menu = "PAYMENT", perm = "EXPORT") @RequirePermission(menu = "PAYMENT", perm = "EXPORT")
@DataChangeLog(menu = "PAYMENT", table = "payment") @DataChangeLog(menu = "PAYMENT", table = "payment")
public ApiResponse<TransferFileResp> generateTransferFile( public ApiResponse<TransferFileResp> generateTransferFile(
@RequestParam String settleMonth, @Pattern(regexp = "\\d{6}", message = "settleMonth는 YYYYMM 형식이어야 합니다") @RequestParam String settleMonth,
@RequestParam String bankCode) { @RequestParam String bankCode) {
return ApiResponse.ok(transferFileService.generate(settleMonth, bankCode)); return ApiResponse.ok(transferFileService.generate(settleMonth, bankCode));
} }
@@ -0,0 +1,15 @@
package com.ga.api.controller.settle;
import jakarta.validation.constraints.NotBlank;
import lombok.Getter;
import lombok.Setter;
@Getter
@Setter
public class PaymentStatusUpdateReq {
@NotBlank
private String status;
private String failReason;
}
@@ -9,7 +9,9 @@ import com.ga.common.annotation.RequirePermission;
import com.ga.common.model.ApiResponse; import com.ga.common.model.ApiResponse;
import io.swagger.v3.oas.annotations.Operation; import io.swagger.v3.oas.annotations.Operation;
import io.swagger.v3.oas.annotations.tags.Tag; import io.swagger.v3.oas.annotations.tags.Tag;
import jakarta.validation.constraints.Max;
import lombok.RequiredArgsConstructor; import lombok.RequiredArgsConstructor;
import org.springframework.validation.annotation.Validated;
import org.springframework.web.bind.annotation.*; import org.springframework.web.bind.annotation.*;
import org.springframework.web.multipart.MultipartFile; import org.springframework.web.multipart.MultipartFile;
@@ -17,6 +19,7 @@ import java.util.List;
import java.util.Map; import java.util.Map;
@Tag(name = "엑셀 업로드") @Tag(name = "엑셀 업로드")
@Validated
@RestController @RestController
@RequiredArgsConstructor @RequiredArgsConstructor
public class UploadController { public class UploadController {
@@ -41,7 +44,7 @@ public class UploadController {
public ApiResponse<List<Map<String, Object>>> preview( public ApiResponse<List<Map<String, Object>>> preview(
@PathVariable String templateCode, @PathVariable String templateCode,
@RequestParam("file") MultipartFile file, @RequestParam("file") MultipartFile file,
@RequestParam(value = "limit", defaultValue = "5") int limit) { @Max(100) @RequestParam(value = "limit", defaultValue = "5") int limit) {
List<Map<String, Object>> rows = uploadService.preview(templateCode, file, limit); List<Map<String, Object>> rows = uploadService.preview(templateCode, file, limit);
return ApiResponse.ok(rows); return ApiResponse.ok(rows);
} }
@@ -9,6 +9,7 @@ import com.ga.core.mapper.ledger.MaintainLedgerMapper;
import com.ga.core.mapper.ledger.RecruitLedgerMapper; import com.ga.core.mapper.ledger.RecruitLedgerMapper;
import com.ga.core.vo.ledger.*; import com.ga.core.vo.ledger.*;
import com.ga.core.vo.ledger.ApproveStatus; import com.ga.core.vo.ledger.ApproveStatus;
import com.ga.core.vo.ledger.ExceptionLedgerStatus;
import lombok.RequiredArgsConstructor; import lombok.RequiredArgsConstructor;
import org.springframework.stereotype.Service; import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional; import org.springframework.transaction.annotation.Transactional;
@@ -69,7 +70,7 @@ public class LedgerService {
vo.setRecurringMonths(req.getRecurringMonths()); vo.setRecurringMonths(req.getRecurringMonths());
vo.setRecurringLeft(req.getRecurringMonths()); vo.setRecurringLeft(req.getRecurringMonths());
vo.setApproveStatus(ApproveStatus.PENDING.name()); vo.setApproveStatus(ApproveStatus.PENDING.name());
vo.setStatus("NEW"); // ExceptionLedger 전용 초기 상태 — ApproveStatus에 없음 vo.setStatus(ExceptionLedgerStatus.NEW.name());
exceptionMapper.insert(vo); exceptionMapper.insert(vo);
return vo.getLedgerId(); return vo.getLedgerId();
} }
@@ -8,6 +8,7 @@ import com.ga.core.mapper.user.UserMapper;
import com.ga.core.vo.user.UserResp; import com.ga.core.vo.user.UserResp;
import com.ga.core.vo.user.UserSaveReq; import com.ga.core.vo.user.UserSaveReq;
import com.ga.core.vo.user.UserSearchParam; import com.ga.core.vo.user.UserSearchParam;
import com.ga.core.vo.user.UserStatus;
import com.ga.core.vo.user.UserVO; import com.ga.core.vo.user.UserVO;
import lombok.RequiredArgsConstructor; import lombok.RequiredArgsConstructor;
import org.springframework.security.crypto.password.PasswordEncoder; import org.springframework.security.crypto.password.PasswordEncoder;
@@ -52,7 +53,7 @@ public class UserService {
vo.setEmail(req.getEmail()); vo.setEmail(req.getEmail());
vo.setPhone(req.getPhone()); vo.setPhone(req.getPhone());
vo.setAgentId(req.getAgentId()); vo.setAgentId(req.getAgentId());
vo.setStatus(req.getStatus() != null ? req.getStatus() : "ACTIVE"); vo.setStatus(req.getStatus() != null ? req.getStatus() : UserStatus.ACTIVE.name());
mapper.insert(vo); mapper.insert(vo);
if (req.getRoleIds() != null) { if (req.getRoleIds() != null) {
@@ -91,7 +92,7 @@ public class UserService {
@Transactional @Transactional
public void unlock(Long userId) { public void unlock(Long userId) {
mapper.resetFailCount(userId); mapper.resetFailCount(userId);
mapper.updateStatus(userId, "ACTIVE"); mapper.updateStatus(userId, UserStatus.ACTIVE.name());
} }
public List<String> roleCodes(Long userId) { public List<String> roleCodes(Long userId) {