20260125 #204 회원 엔티티 구조 및 회원가입 로직 개선#205
Hidden character warning
Conversation
- admin 관련 컨트롤러 스켈레톤 추가 - 개발자 어드민 페이지 위한 설정 추가 - 인증 관련 URL 클래스화 - TODO, 설명 주석 추가 -
|
Caution Review failedThe pull request is closed. Walkthrough인증/인증서(Email, OAuth2)를 common.auth로 재구조화하고 학번 기반 로그인·쿠키 흐름과 리프레시 토큰 로직을 재도입했습니다. 사용자 엔티티에 온보딩 필드·상태·역할을 추가하고 Spring Boot Admin을 활성화했습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client (브라우저)
participant AuthCtrl as AuthController
participant AuthSvc as AuthService
participant JwtProv as JwtProvider
participant RTRepo as RefreshTokenRepository
participant CookieH as AuthCookieHelper
Client->>AuthCtrl: POST /api/auth/login (AuthRequest: studentId,password)
AuthCtrl->>AuthSvc: login(request)
AuthSvc->>JwtProv: generate access & refresh tokens
JwtProv-->>AuthSvc: accessToken, refreshToken
AuthSvc->>RTRepo: saveOrUpdateToken(userId, encryptedRefreshToken)
RTRepo-->>AuthSvc: OK
AuthSvc-->>AuthCtrl: AuthResponse (user info)
AuthCtrl->>CookieH: createAccessCookie(accessToken)
AuthCtrl->>CookieH: createRefreshCookie(refreshToken)
CookieH-->>AuthCtrl: Set-Cookie headers
AuthCtrl-->>Client: 200 + Authorization header + cookies
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (18)
backend/src/main/java/org/sejongisc/backend/common/auth/jwt/JwtParser.java (1)
63-65:⚠️ Potential issue | 🟡 Minor들여쓰기가 일관되지 않습니다.
Line 64의
throw문에 불필요한 공백이 포함되어 있습니다. 다른 코드 블록과 일관성을 맞춰주세요.✏️ 수정 제안
if (userId == null) { - throw new JwtException("JWT에 userId(uid)가 없습니다."); + throw new JwtException("JWT에 userId(uid)가 없습니다."); }backend/src/test/java/org/sejongisc/backend/auth/service/OauthUnlinkServiceImplTest.java (3)
73-78:⚠️ Potential issue | 🔴 Critical테스트 로직 오류: 구현부가 예외를 던지므로
assertDoesNotThrow는 실패합니다.
OauthUnlinkServiceImpl.unlinkKakao구현을 보면 API 호출 실패 시OauthUnlinkException을 던집니다. 따라서 이 테스트는 실패할 것입니다.assertThrows를 사용해야 합니다.🐛 제안된 수정
`@Test` `@DisplayName`("카카오 연결끊기 실패 테스트 (예외 발생)") void testUnlinkKakao_Failure() { when(restTemplate.exchange(anyString(), any(), any(), eq(String.class))) .thenThrow(new RuntimeException("API 오류")); - assertDoesNotThrow(() -> oauthUnlinkService.unlinkKakao("invalid-token")); + assertThrows(org.sejongisc.backend.common.auth.service.oauth2.exception.OauthUnlinkException.class, + () -> oauthUnlinkService.unlinkKakao("invalid-token")); }
97-102:⚠️ Potential issue | 🔴 Critical테스트 로직 오류: 구현부가 예외를 던지므로
assertDoesNotThrow는 실패합니다.
unlinkGoogle구현도 마찬가지로 API 호출 실패 시OauthUnlinkException을 던집니다.🐛 제안된 수정
`@Test` `@DisplayName`("구글 연결끊기 실패 테스트") void testUnlinkGoogle_Failure() { when(restTemplate.postForEntity(anyString(), isNull(), eq(String.class))) .thenThrow(new RuntimeException("Google API 실패")); - assertDoesNotThrow(() -> oauthUnlinkService.unlinkGoogle("bad-token")); + assertThrows(org.sejongisc.backend.common.auth.service.oauth2.exception.OauthUnlinkException.class, + () -> oauthUnlinkService.unlinkGoogle("bad-token")); }
128-133:⚠️ Potential issue | 🔴 Critical테스트 로직 오류: 구현부가 예외를 던지므로
assertDoesNotThrow는 실패합니다.
unlinkGithub구현도 마찬가지로 API 호출 실패 시OauthUnlinkException을 던집니다.🐛 제안된 수정
`@Test` `@DisplayName`("깃허브 연결끊기 실패 테스트") void testUnlinkGithub_Failure() { when(restTemplate.exchange(anyString(), eq(HttpMethod.DELETE), any(HttpEntity.class), eq(String.class))) .thenThrow(new RuntimeException("GitHub API 실패")); - assertDoesNotThrow(() -> oauthUnlinkService.unlinkGithub("gh-bad-token")); + assertThrows(org.sejongisc.backend.common.auth.service.oauth2.exception.OauthUnlinkException.class, + () -> oauthUnlinkService.unlinkGithub("gh-bad-token")); }backend/src/test/java/org/sejongisc/backend/backtest/controller/BacktestControllerTest.java (1)
1-207:⚠️ Potential issue | 🟡 Minor전체 테스트 클래스가 주석 처리되어 있습니다.
BacktestControllerTest전체가 주석 처리되어 있어 테스트 커버리지가 감소합니다. 테스트를 비활성화한 이유가 있다면 TODO 주석으로 명시하거나, 리팩토링 완료 후 테스트를 복원해야 합니다.주석 처리된 테스트는 코드베이스에 남겨두면 유지보수 부담이 됩니다. 복원 계획이 없다면 삭제를 고려해 주세요.
backend/src/main/java/org/sejongisc/backend/common/auth/service/EmailService.java (1)
131-135:⚠️ Potential issue | 🟠 Major이메일 주소 로깅은 PII(개인식별정보) 노출 위험이 있습니다.
디버그 레벨이라도 이메일 주소를 로그에 기록하면 개인정보 보호 규정(GDPR, 개인정보보호법) 위반 가능성이 있습니다. 이메일을 마스킹하거나 로그에서 제거하는 것을 권장합니다.
🔒 제안된 수정
public void sendResetEmail(String email) { if(!userRepository.existsByEmail(email)){ - log.debug("Password reset requested for non-existent email {}", email); + log.debug("Password reset requested for non-existent email"); return; }backend/src/main/java/org/sejongisc/backend/common/auth/dto/oauth/OauthUserInfo.java (1)
7-7:⚠️ Potential issue | 🟡 Minor주석에 오타가 있습니다.
getEamil()→getEmail()로 수정해야 합니다.✏️ 제안된 수정
- // String getEamil(); // kakao email 승인 필요 + // String getEmail(); // kakao email 승인 필요backend/src/main/java/org/sejongisc/backend/BackendApplication.java (1)
3-13:⚠️ Potential issue | 🔴 CriticalAdmin Server/Actuator 엔드포인트가 인증 없이 공개되어 있습니다. SecurityConstants의 WHITELIST_URLS에
/actuator/**가 포함되어 모든 사용자가 접근 가능한 상태입니다. Spring Boot Admin UI 및 sensitive 정보 노출 위험이 있으므로, 관리자 전용 역할 제한, IP 화이트리스트, 또는 네트워크 분리를 적용해야 합니다. 최소한 프로덕션 환경에서는 Admin Server 활성화를 재검토하세요.backend/src/main/java/org/sejongisc/backend/user/dto/UserUpdateRequest.java (1)
12-38:⚠️ Potential issue | 🟠 Major@notblank로 인해 선택 입력 불가능합니다.
스키마 설명은 "이메일/비밀번호 중 수정할 항목만 입력"이지만,
@NotBlank제약과 컨트롤러의@Valid검증으로 인해 이메일을 생략한 요청이 실패합니다. 서비스 구현(if (request.getEmail() != null))은 이메일이 선택 입력임을 가정하고 있어, DTO 검증과 실제 의도가 일치하지 않습니다.선택 입력이라면
@NotBlank를 제거하고, 필수라면 스키마와 서비스 null-체크를 "필수"로 명확히 해주세요.backend/src/test/java/org/sejongisc/backend/auth/service/RefreshTokenServiceTest.java (1)
28-38:⚠️ Potential issue | 🟠 MajorTokenEncryptor 목이 없어서 reissueTokens 테스트가 NPE로 실패합니다.
RefreshTokenService가 tokenEncryptor를 final 의존성으로 가지고 있고 reissueTokens 메서드에서
tokenEncryptor.decrypt(encryptedRefreshToken)를 호출하는데, 현재 테스트에는 TokenEncryptor 목이 선언되어 있지 않습니다.@InjectMocks는null이 주입되므로 NPE가 발생합니다. TokenEncryptor 목을 추가하고 decrypt 스텁을 설정하세요.제안 수정(diff)
import org.sejongisc.backend.common.auth.repository.RefreshTokenRepository; import org.sejongisc.backend.common.auth.jwt.JwtProvider; +import org.sejongisc.backend.common.auth.jwt.TokenEncryptor; import org.sejongisc.backend.common.auth.service.RefreshTokenService; `@Mock` private JwtProvider jwtProvider; + `@Mock` + private TokenEncryptor tokenEncryptor; + `@InjectMocks` private RefreshTokenService refreshTokenService; `@BeforeEach` void setUp() { userId = UUID.randomUUID(); refreshToken = "dummy-refresh-token"; + lenient().when(tokenEncryptor.decrypt(anyString())) + .thenAnswer(inv -> inv.getArgument(0, String.class));backend/src/main/java/org/sejongisc/backend/board/service/PostInteractionService.java (1)
40-45:⚠️ Potential issue | 🟠 Major4개의
@Retryable메서드에서@Recover미구현으로 예외가 왜곡됩니다.createComment(40-45), deleteComment(103-107), toggleLike(149-153), toggleBookmark(183-187) 메서드에서 CustomException 등 재시도 목록에 없는 예외가 발생하면,
@Recover메서드가 없어 Spring Retry가 "Cannot locate recovery method" 오류를 던지게 됩니다. 각 메서드에 재시도 예외용(재시도 소진 후) 및 비재시도 예외용(즉시 처리)@Recover를추가하여 원본 예외가 그대로 전파되도록 구현해 주세요.backend/src/main/java/org/sejongisc/backend/common/auth/service/oauth2/GoogleServiceImpl.java (1)
109-112:⚠️ Potential issue | 🟠 Major사용자 이메일 로깅은 개인정보 보호 위험이 있습니다.
INFO레벨에서 사용자 이메일을 로깅하는 것은 GDPR/CCPA 등 개인정보 보호 규정 위반 가능성이 있습니다. 토큰은 마스킹 처리(Line 75-82)하면서 이메일은 평문으로 로깅하고 있어 일관성도 없습니다.이메일 로깅을 제거하거나, 최소한
DEBUG레벨로 변경하고 마스킹 처리를 적용해야 합니다.🔒 제안된 수정 사항
- log.info(" [Google Service] Sub(ID) ------> {}", userInfo.getSub()); - log.info(" [Google Service] Email ------> {}", userInfo.getEmail()); - log.info(" [Google Service] Name ------> {}", userInfo.getName()); - log.info(" [Google Service] Picture ------> {}", userInfo.getPicture()); + log.debug(" [Google Service] Sub(ID) ------> {}", userInfo.getSub()); + log.debug(" [Google Service] Name ------> {}", userInfo.getName()); + // 이메일은 PII이므로 로깅하지 않음backend/src/main/java/org/sejongisc/backend/common/auth/dto/oauth/KakaoUserInfoAdapter.java (1)
9-15:⚠️ Potential issue | 🟡 MinorLine 12의 불필요한 공백 제거
String accessToken파라미터에 불필요한 공백(두 칸)이 있습니다.String accessToken으로 수정 필요합니다.참고:
KakaoUserInfoResponse는 동일 패키지(org.sejongisc.backend.common.auth.dto.oauth)에 위치하므로 import 문이 불필요합니다.backend/src/main/java/org/sejongisc/backend/common/auth/service/AuthService.java (1)
66-71:⚠️ Potential issue | 🟡 MinoraccessToken이 null일 때 예외 발생 가능
logout메서드에서accessToken이null이거나 빈 문자열인 경우jwtParser.getUserIdFromToken(accessToken)에서 예외가 발생합니다.AuthController의logout에서 null이 전달될 수 있으므로 방어적 처리가 필요합니다.🛡️ 제안된 수정사항
`@Transactional` public void logout(String accessToken) { + if (accessToken == null || accessToken.isBlank()) { + log.warn("로그아웃 요청에 유효한 토큰이 없습니다."); + return; + } UUID userId = jwtParser.getUserIdFromToken(accessToken); refreshTokenRepository.deleteByUserId(userId); log.info("로그아웃 완료: userId={}", userId); }backend/src/test/java/org/sejongisc/backend/auth/service/AuthServiceTest.java (4)
70-90:⚠️ Potential issue | 🔴 Critical테스트와 실제 서비스 코드 간 Repository 메서드 불일치
테스트 코드가
userRepository.findUserByEmail()을 모킹하고 있지만,AuthService.login()은userRepository.findByStudentId()를 호출합니다. 이 불일치로 인해 테스트가 실패하거나 의도한 동작을 검증하지 못합니다.
- Line 71:
request.setStudentId("test@example.com")- Line 74:
given(userRepository.findUserByEmail("test@example.com"))← 잘못된 메서드- AuthService Line 34:
userRepository.findByStudentId(request.getStudentId())← 실제 호출🐛 제안된 수정사항
AuthRequest request = new AuthRequest(); - request.setStudentId("test@example.com"); + request.setStudentId("20231234"); // studentId는 학번 형식이어야 함 request.setPassword(rawPassword); - given(userRepository.findUserByEmail("test@example.com")) + given(userRepository.findByStudentId("20231234")) .willReturn(Optional.of(user));동일한 수정이
login_userNotFound,login_wrongPassword,login_nullPassword테스트에도 필요합니다.
92-108:⚠️ Potential issue | 🔴 Critical동일한 Repository 메서드 불일치 - login_userNotFound
이 테스트도
findUserByEmail대신findByStudentId를 모킹해야 합니다.🐛 제안된 수정사항
AuthRequest request = new AuthRequest(); - request.setStudentId("notfound@example.com"); + request.setStudentId("99999999"); request.setPassword("password"); - given(userRepository.findUserByEmail("notfound@example.com")) + given(userRepository.findByStudentId("99999999")) .willReturn(Optional.empty());
110-139:⚠️ Potential issue | 🔴 Critical동일한 Repository 메서드 불일치 - login_wrongPassword
🐛 제안된 수정사항
AuthRequest request = new AuthRequest(); - request.setStudentId("test@example.com"); + request.setStudentId("20231234"); request.setPassword("wrongPassword"); - given(userRepository.findUserByEmail("test@example.com")) + given(userRepository.findByStudentId("20231234")) .willReturn(Optional.of(user));
141-167:⚠️ Potential issue | 🔴 Critical동일한 Repository 메서드 불일치 - login_nullPassword
🐛 제안된 수정사항
AuthRequest request = new AuthRequest(); - request.setStudentId("test@example.com"); + request.setStudentId("20231234"); request.setPassword("somePassword"); - given(userRepository.findUserByEmail("test@example.com")) + given(userRepository.findByStudentId("20231234")) .willReturn(Optional.of(user));
🤖 Fix all issues with AI agents
In `@backend/build.gradle`:
- Line 31: The dependency
'org.springframework.boot:spring-boot-starter-validation' is declared twice;
remove the duplicate declaration and keep only a single implementation
'org.springframework.boot:spring-boot-starter-validation' entry in the
dependencies block so the dependency is not duplicated (delete the extra
occurrence).
- Around line 68-69: Update the Spring Boot Admin dependency versions: replace
the two occurrences of implementation
'de.codecentric:spring-boot-admin-starter-server:3.2.1' and implementation
'de.codecentric:spring-boot-admin-starter-client:3.2.1' with the matching Spring
Boot 3.5.x series by changing both to version 3.5.7 so the server and client
artifacts use 'de.codecentric:spring-boot-admin-starter-server:3.5.7' and
'de.codecentric:spring-boot-admin-starter-client:3.5.7'.
In
`@backend/src/main/java/org/sejongisc/backend/admin/controller/AdminUserController.java`:
- Around line 49-77: The three controller methods updateUserStatus,
updateUserRole, and forceDeleteUser currently return success messages while
their service calls (userService.updateUserStatus, userService.updateUserRole,
userService.deleteUserWithOauth) are commented out; either re-enable and use
those service methods (uncomment or invoke them) and propagate/handle errors and
return the real outcome, or explicitly mark them unimplemented by returning 501
Not Implemented and add TODO comments indicating the missing implementation;
ensure you reference the exact service methods (userService.updateUserStatus,
userService.updateUserRole, userService.deleteUserWithOauth) and adjust
ResponseEntity responses accordingly.
- Around line 34-39: The method uploadMemberExcel in AdminUserController
currently returns null which causes NPE at runtime; either restore the intended
implementation by calling adminUserService.syncMembersFromExcel(file), wrap the
result (ExcelSyncResultDto) in ResponseEntity.ok(result) and return it, or if
the feature is intentionally unfinished return a proper HTTP 501 by returning
ResponseEntity.status(HttpStatus.NOT_IMPLEMENTED).body(...) instead; ensure
imports for HttpStatus and ExcelSyncResultDto are present and remove the null
return.
- Around line 41-47: The GET handler getAllUsers currently accepts
AdminUserRequest via `@RequestBody` and returns null; change the parameter to be
bound from query params or form data (use `@RequestParam` or `@ModelAttribute` on
AdminUserRequest) and call the service to return a real response; specifically
update the signature of getAllUsers to accept AdminUserRequest as
`@ModelAttribute` (or individual `@RequestParam` fields) and return
ResponseEntity.ok(userService.findAllUsers() or the appropriate filtered
userService method) instead of returning null.
In `@backend/src/main/java/org/sejongisc/backend/admin/dto/AdminUserRequest.java`:
- Around line 1-5: The GET endpoint is using `@RequestBody` with an empty
AdminUserRequest DTO which violates HTTP semantics; remove `@RequestBody` from the
`@GetMapping` handler, either delete the unused AdminUserRequest class or populate
it with the needed filter fields and change the controller to accept them via
`@RequestParam` or `@ModelAttribute` (or individual `@RequestParam` parameters)
instead of request body; update any controller method signatures that reference
AdminUserRequest to use the chosen binding.
In
`@backend/src/main/java/org/sejongisc/backend/common/auth/controller/AuthController.java`:
- Around line 67-75: AuthController.logout currently passes a possibly
null/invalid accessToken into authService.logout which can propagate
JwtParser.getUserIdFromToken exceptions; update logout to first check if
accessToken is non-null and non-empty (or alternatively wrap authService.logout
call in try/catch), only invoke authService.logout when a token is present and
swallow/parses token-related exceptions so they don't block response, and always
call cookieHelper.deleteCookie("access") and
cookieHelper.deleteCookie("refresh") to remove cookies and return the OK
response; reference AuthController.logout, authService.logout,
JwtParser.getUserIdFromToken, and cookieHelper.deleteCookie when making the
change.
In `@backend/src/main/java/org/sejongisc/backend/common/auth/jwt/JwtParser.java`:
- Line 49: The TODO comment in JwtParser (class JwtParser) ends with "및" and is
incomplete; either complete the sentence to state the remaining intent (e.g., "및
클레임 키 상수화를 적용한다" or the specific follow-up task) or remove/replace the dangling
"및" so the comment is a complete clear note; update the comment above JwtParser
methods/fields accordingly to reflect the intended maintenance action or delete
the TODO if it's no longer needed.
In
`@backend/src/main/java/org/sejongisc/backend/common/auth/service/oauth2/GithubServiceImpl.java`:
- Around line 196-212: User creation for OAuth paths omits the non-nullable
studentId, causing DB constraint failures; set a valid default studentId before
saving by populating studentId in the User.builder(...) used in the OAuth branch
(the block that calls userRepository.save(newUser)) and in the equivalent
creation logic in CustomOidcUserService, e.g. generate a unique placeholder
(like "oauth-{UUID}" or similar) so it satisfies `@Column`(nullable = false); also
update prePersist() to only set a default studentId when it is null to cover any
other creation flows. Ensure any reference to studentId appears on the User
object before calling userRepository.save and leave provider/account logic
(UserOauthAccount, oauthAccountRepository) unchanged.
In
`@backend/src/main/java/org/sejongisc/backend/user/controller/UserController.java`:
- Around line 44-46: The controller method withdraw currently calls
refreshTokenService.deleteByUserId after calling
userService.deleteUserSoftDelete, which already invokes
refreshTokenService.deleteByUserId internally; remove the redundant
refreshTokenService.deleteByUserId(user.getUserId()) call from
UserController.withdraw and keep only
userService.deleteUserSoftDelete(user.getUserId()); optionally add a brief
comment in withdraw noting that refresh tokens are removed inside
UserService.deleteUserSoftDelete to prevent future re-introduction of the
duplicate call.
In `@backend/src/main/java/org/sejongisc/backend/user/service/UserService.java`:
- Around line 180-186: The deleteUserSoftDelete method is missing a
write-enabled transaction, so the status change may not be flushed because the
class has `@Transactional`(readOnly = true); add a method-level `@Transactional`
(without readOnly or with readOnly = false) annotation to public void
deleteUserSoftDelete(UUID userId) so the User entity modification is persisted,
and ensure the proper import of
org.springframework.transaction.annotation.Transactional is added.
- Around line 161-169: The getEmailFromRedis method is catching all Exceptions
and masking thrown CustomException(ErrorCode.EMAIL_CODE_NOT_FOUND); change the
catch logic so CustomException is rethrown (or not caught) while only
Redis-specific exceptions (e.g., RedisConnectionFailureException or
DataAccessException) are caught and mapped to INTERNAL_SERVER_ERROR;
specifically update getEmailFromRedis to allow the CustomException from the null
email check to propagate and replace the broad catch(Exception e) with targeted
catches for Redis-related exceptions and a separate rethrow for CustomException.
- Around line 52-55: The duplicate-check branch in UserService.java currently
throws ErrorCode.DUPLICATE_PHONE for non-studentId collisions which is
incorrect; update the logic so when
userRepository.existsByEmailOrStudentId(request.getEmail(),
request.getStudentId()) is true you check
userRepository.existsByStudentId(request.getStudentId()) and throw
ErrorCode.DUPLICATE_USER for studentId collisions, otherwise throw
ErrorCode.DUPLICATE_EMAIL for email collisions (or alternatively replace the
existsByEmailOrStudentId call with explicit existsByEmail and existsByStudentId
checks and throw DUPLICATE_EMAIL / DUPLICATE_USER accordingly).
- Around line 85-88: The password comparison in UserService is wrong: it
currently calls passwordEncoder.matches(request.getNewPassword(),
request.getNewPassword()) which always fails; change the logic to verify the
provided current password against the stored hash (use
passwordEncoder.matches(request.getCurrentPassword(), user.getPasswordHash()))
before applying request.getNewPassword(), and only throw
CustomException(ErrorCode.INVALID_INPUT) when that current-password check fails;
keep the existing flow for updating the stored password with
request.getNewPassword().
In
`@backend/src/main/java/org/sejongisc/backend/user/util/PasswordPolicyValidator.java`:
- Around line 14-22: The login flow is using request.getPassword() untrimmed
which can mismatch the trimmed password stored via
PasswordPolicyValidator.getValidatedPassword(); update the AuthService login
method to normalize the incoming password before matching by calling
PasswordPolicyValidator.getValidatedPassword(request.getPassword()) (or at
minimum apply the same sanitize/trim logic) and then pass that result into
passwordEncoder.matches(), ensuring consistent use of
PasswordPolicyValidator.getValidatedPassword across authentication and
registration.
🧹 Nitpick comments (21)
backend/src/main/java/org/sejongisc/backend/common/auth/jwt/JwtParser.java (1)
68-68: DB 조회 제거 시 보안 고려사항 검토 필요TODO 주석에서 언급한 대로 성능을 위해 DB 조회를 제거하는 것은 고려해볼 수 있습니다. 다만, 이 경우 다음 사항들을 검토해야 합니다:
- 사용자 비활성화/삭제 감지 불가: JWT가 유효하더라도 사용자가 비활성화되거나 삭제된 경우 이를 감지할 수 없습니다.
- 권한 변경 즉시 반영 불가: 사용자 권한이 변경되어도 토큰 만료 전까지 이전 권한이 유지됩니다.
성능 최적화가 필요하다면 캐싱(Redis 등)을 도입하거나, 토큰 만료 시간을 짧게 설정하는 방안을 함께 고려해 보세요.
backend/src/main/java/org/sejongisc/backend/common/auth/service/oauth2/OauthUnlinkServiceImpl.java (1)
57-58: 불필요한throws선언 제거 고려
OauthUnlinkException은RuntimeException을 상속하므로 메서드 시그니처에throws선언이 필요하지 않습니다.unlinkKakao와unlinkGithub메서드는throws선언이 없으므로 일관성을 위해 제거하는 것이 좋습니다.♻️ 제안된 수정
`@Override` -public void unlinkGoogle(String accessToken) throws OauthUnlinkException{ +public void unlinkGoogle(String accessToken) { if (accessToken == null || accessToken.trim().isEmpty()) {backend/src/main/java/org/sejongisc/backend/common/config/swagger/SwaggerConfig.java (1)
34-37: 메인 서버 URL 설정이 누락되었습니다.메인 서버의
.url()호출이 주석 처리되어 있어 Swagger UI에서 URL 없이 "메인 서버"만 표시됩니다. 이는 API 문서 사용자에게 혼란을 줄 수 있습니다.배포 전에 실제 URL을 설정하거나, 준비되지 않았다면 해당 서버 항목 전체를 주석 처리하는 것을 권장합니다.
이 TODO 항목을 추적하기 위한 이슈를 생성해 드릴까요?
backend/src/main/java/org/sejongisc/backend/common/auth/entity/RefreshToken.java (1)
3-3: import 문의 공백 오류를 수정해 주세요.
jakarta.persistence. *;에 불필요한 공백이 있습니다. 컴파일에는 영향이 없지만 일관된 코드 스타일을 위해 수정이 필요합니다.🔧 수정 제안
-import jakarta.persistence. *; +import jakarta.persistence.*;backend/src/test/java/org/sejongisc/backend/attendance/service/AttendanceServiceTest.java (1)
14-29: 테스트 메서드가 없는 테스트 클래스입니다.Mock 객체들이 설정되어 있지만 실제 테스트 메서드가 없습니다. 의도적으로 작업 중인 상태라면 TODO 주석을 추가하여 향후 테스트 구현 계획을 명시하는 것이 좋습니다.
backend/src/main/java/org/sejongisc/backend/common/config/security/SecurityConstants.java (1)
28-35: MEMBER_ONLY_URLS의 TODO/주석 경로 정리 필요주석 처리된 엔드포인트가 실제로 존재한다면 권한 보호가 누락될 수 있습니다. TODO를 이슈로 남기고 실제 보호 정책을 확정하는 편이 안전합니다.
원하시면 해당 경로들을 기준으로 권한 정책 정리 및 테스트 추가안을 제안드릴게요.
backend/src/main/java/org/sejongisc/backend/common/auth/filter/JwtAuthenticationFilter.java (1)
46-50: 필터 제외 조건이 중복 평가됩니다.
OncePerRequestFilter의doFilter()메서드는shouldNotFilter()가true를 반환하면doFilterInternal()을 호출하지 않습니다. 따라서 Line 47에서shouldNotFilter(request)를 다시 확인하는 것은 불필요한 중복입니다.OPTIONS예외도shouldNotFilter()메서드로 옮기면 불필요한 조건 체크를 제거할 수 있습니다.♻️ 제안 리팩터링
- if (shouldNotFilter(request) || "OPTIONS".equalsIgnoreCase(request.getMethod())) { + if (shouldNotFilter(request)) { filterChain.doFilter(request, response); return; }`@Override` protected boolean shouldNotFilter(HttpServletRequest request) { String path = request.getRequestURI(); + // CORS Preflight 요청은 JWT 검사 제외 + if ("OPTIONS".equalsIgnoreCase(request.getMethod())) { + return true; + }backend/src/main/java/org/sejongisc/backend/admin/controller/AdminBoardController.java (1)
1-4: 빈 컨트롤러의 의도를 명확히 해주세요.현재 바디와 어노테이션이 없어 실질적으로 사용되지 않습니다. 같은 디렉토리의
AdminUserController는@RestController와@RequestMapping어노테이션으로 구성되어 있습니다. 실제 엔드포인트 구현 예정이라면 동일한 패턴으로 뼈대를 추가하거나, 아직 계획 단계면 생성을 미루는 것도 방법입니다.backend/src/main/java/org/sejongisc/backend/admin/controller/AdminPointController.java (1)
1-4: 빈 컨트롤러 의도 명확화 권장현재 스켈레톤이라 향후 추가 예정임을 최소한 주석으로 명시하면 혼동을 줄일 수 있습니다.
✍️ 제안: 의도 명시용 TODO 주석
package org.sejongisc.backend.admin.controller; public class AdminPointController { + // TODO: 관리자 포인트 API 엔드포인트 추가 예정 }backend/src/main/java/org/sejongisc/backend/admin/controller/AdminAttendanceController.java (1)
1-4: 빈 컨트롤러 의도 명확화 권장현재 스켈레톤이라 향후 추가 예정임을 주석으로 표시해 두면 좋겠습니다.
✍️ 제안: 의도 명시용 TODO 주석
package org.sejongisc.backend.admin.controller; public class AdminAttendanceController { + // TODO: 관리자 출석 API 엔드포인트 추가 예정 }backend/src/main/java/org/sejongisc/backend/admin/controller/AdminDashboardController.java (1)
1-4: 빈 컨트롤러는 목적을 명확히 해주세요.현재 어노테이션과 메서드가 없어 스프링에 등록되지도 않으므로, 계획된 역할이 없다면 삭제하거나 최소한 목적/향후 구현 계획을 주석으로 남기는 편이 좋겠습니다.
backend/src/main/java/org/sejongisc/backend/common/auth/controller/AuthCookieHelper.java (1)
9-15: 매직 넘버를 상수로 추출하는 것을 권장합니다.쿠키 만료 시간 값(
60L * 60,60L * 60 * 24 * 14)을 명명된 상수로 추출하면 가독성과 유지보수성이 향상됩니다.♻️ 제안된 리팩토링
`@Component` public class AuthCookieHelper { + + private static final long ACCESS_TOKEN_MAX_AGE = 60L * 60; // 1시간 + private static final long REFRESH_TOKEN_MAX_AGE = 60L * 60 * 24 * 14; // 14일 public ResponseCookie createAccessCookie(String token) { - return createCookie("access", token, 60L * 60); + return createCookie("access", token, ACCESS_TOKEN_MAX_AGE); } public ResponseCookie createRefreshCookie(String token) { - return createCookie("refresh", token, 60L * 60 * 24 * 14); + return createCookie("refresh", token, REFRESH_TOKEN_MAX_AGE); }backend/src/main/java/org/sejongisc/backend/common/auth/controller/AuthController.java (1)
51-65: 예외 처리 시 원본 에러 정보 손실Line 62-63에서 모든 예외를 catch하여 동일한 메시지로 반환하고 있습니다. 디버깅 및 모니터링을 위해 예외 로깅을 추가하는 것이 좋습니다.
♻️ 제안된 개선사항
} catch (Exception e) { + log.warn("토큰 재발급 실패: {}", e.getMessage()); return ResponseEntity.status(HttpStatus.UNAUTHORIZED).body(Map.of("message", "Refresh Token이 유효하지 않거나 만료되었습니다.")); }backend/src/main/java/org/sejongisc/backend/common/config/security/SecurityConfig.java (1)
42-44: 중첩 클래스 구조는 의도적이며 작동합니다
CustomOAuth2UserService,CustomOidcUserService,OAuth2SuccessHandler는 정적 중첩 클래스로 선언되어 있고@Service,@Component어노테이션을 통해 Spring의 컴포넌트 스캔에 의해 올바르게 등록됩니다. 이 패턴은 유효하며, 특히 OAuth2 관련 기능이 하나의 논리적 단위로 통합되어 있어 응집도가 높습니다.선택적으로 이들을 별도의 최상위 클래스로 분리할 수 있지만, 현재 구조도 의도적이고 잘 작동하는 Spring 표준 패턴입니다.
backend/src/main/java/org/sejongisc/backend/user/controller/UserController.java (1)
53-57:getUserInfo에서CustomUserDetails의 데이터를 직접 반환하고 있습니다.현재 구현은 인증 시점의 데이터를 반환하므로, 사용자 정보가 변경된 경우 최신 정보가 반영되지 않을 수 있습니다. 최신 데이터가 필요하다면 DB에서 조회하는 것을 고려해 주세요. 현재 방식이 의도된 것이라면 무시해도 됩니다.
backend/src/main/java/org/sejongisc/backend/user/entity/User.java (2)
52-53:isNewMember필드에@Builder.Default가 누락되었습니다.
isNewMember는nullable = false제약이 있지만,@Builder.Default없이 Builder 패턴으로 User 객체를 생성하면 primitive boolean의 기본값false가 적용됩니다. 이는createUserWithSignupAndPending에서는 명시적으로true를 설정하지만, 다른 곳에서 Builder를 사용할 때 의도치 않게false가 될 수 있습니다.명시적인 기본값을 권장합니다.
♻️ 수정 제안
`@Column`(name = "is_new_member", nullable = false) + `@Builder.Default` - private boolean isNewMember; // 신규 여부 (포인트나 이벤트 대상자 선정용) + private boolean isNewMember = false; // 신규 여부 (포인트나 이벤트 대상자 선정용)
107-124: factory 메서드와prePersist의 기본 Role 설정이 불일치합니다.
prePersist()는Role.PENDING_MEMBER를 기본값으로 설정하지만,createUserWithSignupAndPending은Role.TEAM_MEMBER를 설정합니다. TODO 주석에서 언급한 대로, 운영진 승인 로직이 추가되면 factory 메서드도PENDING_MEMBER로 변경이 필요합니다.현재 상태에서는 동작에 문제가 없지만, 향후 유지보수 시 혼란을 방지하기 위해 TODO를 추적해 주세요.
backend/src/main/java/org/sejongisc/backend/common/auth/service/oauth2/GithubServiceImpl.java (3)
81-95: 매 요청마다WebClient.create()를 호출하고 있습니다.
WebClient.create(TOKEN_URL)이getAccessToken호출 시마다 새 인스턴스를 생성합니다. WebClient는 thread-safe하고 재사용 가능하므로, 필드로 선언하여 재사용하는 것이 성능상 유리합니다.♻️ 수정 제안
+ private final WebClient tokenWebClient; + private final WebClient userInfoWebClient; `@Autowired` public GithubServiceImpl( `@Value`("${github.client.id}") String clientId, `@Value`("${github.client.secret}") String clientSecret) { this.clientId = clientId; this.clientSecret = clientSecret; this.TOKEN_URL = "https://github.com/login/oauth/access_token"; this.USERINFO_URL = "https://api.github.com/user"; + this.tokenWebClient = WebClient.create(TOKEN_URL); + this.userInfoWebClient = WebClient.create(USERINFO_URL); }
5-5:jakarta.transaction.Transactional대신 Spring의 어노테이션을 사용하세요.
jakarta.transaction.Transactional이 import되어 있는데, 프로젝트 내 다른 서비스들은org.springframework.transaction.annotation.Transactional을 사용합니다. 일관성을 위해 Spring 어노테이션 사용을 권장합니다.♻️ 수정 제안
-import jakarta.transaction.Transactional; +import org.springframework.transaction.annotation.Transactional;
381-388: 도메인 문자열이 하드코딩되어 있습니다.
"sjusisc.com","sisc-web.duckdns.org"등의 도메인이 직접 코드에 작성되어 있습니다. 환경별 설정 파일(application-{profile}.yml)에서 주입받는 것이 유지보수에 유리합니다.backend/src/test/java/org/sejongisc/backend/auth/controller/AuthControllerTest.java (1)
80-84: 사용되지 않는oauth2Services맵이 있습니다.
oauth2Services맵이 생성되지만, Line 86에서AuthController가(authService, refreshTokenService, authCookieHelper)만 받는 새 생성자를 사용하므로 해당 맵은 더 이상 사용되지 않습니다. 불필요한 코드를 제거해 주세요.🧹 수정 제안
`@BeforeEach` void setUp() { - Map<String, Oauth2Service<?, ?>> oauth2Services = Map.of( - "GOOGLE", googleService, - "KAKAO", kakaoService, - "GITHUB", githubService - ); authController = new AuthController(authService, refreshTokenService, authCookieHelper);또한 Lines 59-61의 OAuth2Service mock 필드들도 OAuth 관련 테스트에서 실제로 사용되는지 확인이 필요합니다.
backend/src/main/java/org/sejongisc/backend/admin/controller/AdminUserController.java
Show resolved
Hide resolved
backend/src/main/java/org/sejongisc/backend/admin/controller/AdminUserController.java
Show resolved
Hide resolved
backend/src/main/java/org/sejongisc/backend/admin/controller/AdminUserController.java
Show resolved
Hide resolved
backend/src/main/java/org/sejongisc/backend/user/service/UserService.java
Show resolved
Hide resolved
backend/src/main/java/org/sejongisc/backend/user/service/UserService.java
Show resolved
Hide resolved
backend/src/main/java/org/sejongisc/backend/user/service/UserService.java
Show resolved
Hide resolved
backend/src/main/java/org/sejongisc/backend/user/service/UserService.java
Show resolved
Hide resolved
backend/src/main/java/org/sejongisc/backend/user/util/PasswordPolicyValidator.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/src/main/java/org/sejongisc/backend/common/auth/service/AuthService.java (1)
68-73:⚠️ Potential issue | 🟠 Major로그아웃 시 토큰 파싱 예외를 처리하세요.
jwtParser.getUserIdFromToken(accessToken)은 만료되거나 유효하지 않은 토큰에 대해JwtException을 던집니다. 현재 코드에는 예외 처리가 없으므로 이러한 경우 500 에러가 반환되지만, 멱등성을 보장하려면 유효하지 않은 토큰으로도 로그아웃을 성공(200 OK)으로 처리해야 합니다. 테스트(logout_invalidToken)에서 기대하는 동작과 실제 구현이 맞지 않으므로, 서비스 또는 컨트롤러 레벨에서JwtException을 처리하고 정상 응답을 반환하도록 수정하시기 바랍니다.
🤖 Fix all issues with AI agents
In `@backend/src/main/java/org/sejongisc/backend/user/dto/UserUpdateRequest.java`:
- Around line 35-38: The email field in UserUpdateRequest is currently required
by `@NotBlank` but the class and UserService.updateUser() treat email as optional;
remove the `@NotBlank` annotation from the email field so null/empty values are
allowed, keep the `@Email` annotation (it permits nulls) to validate only when
present, and update the `@Schema` description for the email field to indicate it
is optional (e.g., "비밀번호 재설정용 이메일 (선택)"). Ensure this aligns with
UserService.updateUser()'s request.getEmail() != null checks.
🧹 Nitpick comments (6)
backend/src/main/java/org/sejongisc/backend/common/auth/jwt/JwtParser.java (2)
49-51: 클레임 키 상수화로 중복/오타 리스크를 줄이는 리팩터를 권장합니다.동일 문자열을 여러 곳에서 직접 사용하는 패턴이라 유지보수성이 떨어질 수 있습니다.
♻️ 제안 변경(diff)
public class JwtParser { + private static final String CLAIM_UID = "uid"; + private static final String CLAIM_ROLE = "role"; @@ - String userId = claims.get("uid", String.class); - String roleStr = claims.get("role", String.class); + String userId = claims.get(CLAIM_UID, String.class); + String roleStr = claims.get(CLAIM_ROLE, String.class);
68-68: DB 조회 제거는 권한/차단 상태 반영을 약화시킬 수 있으니 주의하세요.성능 최적화가 필요하다면 캐시 도입이나 토큰에 최소 권한 정보를 담되, 계정 비활성화/권한 변경 반영 경로를 보장하는 접근을 추천합니다.
backend/src/main/java/org/sejongisc/backend/common/auth/service/AuthService.java (1)
45-53: Refresh token 삭제 로직 일관성 개선을 권장합니다.
logout메서드(Line 71)에서는deleteByUserId를 직접 사용하는 반면,login에서는findByUserId().ifPresent(delete)패턴을 사용하고 있습니다. 일관성과 간결함을 위해deleteByUserId를 사용하는 것이 좋습니다.또한, 현재 패턴은 동일 트랜잭션 내에서
find→delete→save순서로 실행되므로, flush 타이밍에 따라 예상치 못한 동작이 발생할 수 있습니다.♻️ 제안하는 수정 사항
- // 기존 토큰 삭제 후 새로 저장 - refreshTokenRepository.findByUserId(user.getUserId()) - .ifPresent(refreshTokenRepository::delete); - - refreshTokenRepository.save( + // 기존 토큰 삭제 후 새로 저장 + refreshTokenRepository.deleteByUserId(user.getUserId()); + refreshTokenRepository.save(backend/src/main/java/org/sejongisc/backend/user/service/UserService.java (1)
85-88:currentPasswordnull 체크를 명시적으로 추가하면 UX가 개선됩니다.
currentPassword가 null인 상태로newPassword만 입력하면,passwordEncoder.matches(null, hash)는 false를 반환하여INVALID_INPUT예외가 발생합니다. 동작은 정상이나, "현재 비밀번호 미입력"과 "현재 비밀번호 불일치"를 구분하면 사용자 경험이 향상됩니다.♻️ 리팩토링 제안
if (request.getNewPassword() != null && !request.getNewPassword().isBlank()) { + if (request.getCurrentPassword() == null || request.getCurrentPassword().isBlank()) { + throw new CustomException(ErrorCode.INVALID_INPUT); // 현재 비밀번호 미입력 + } if (!passwordEncoder.matches(request.getCurrentPassword(), user.getPasswordHash())) { throw new CustomException(ErrorCode.INVALID_INPUT); // 비밀번호 불일치 에러 }backend/src/main/java/org/sejongisc/backend/user/controller/UserController.java (2)
13-13: 사용되지 않는 import가 있습니다.
RefreshTokenService가 import 되어 있지만 클래스 내에서 필드나 로컬 변수로 사용되지 않습니다.♻️ 수정 제안
-import org.sejongisc.backend.common.auth.service.RefreshTokenService;
52-54:UserInfoResponse생성 로직을 팩토리 메서드로 추출하면 가독성이 향상됩니다.생성자 파라미터가 많아 가독성이 떨어집니다.
UserInfoResponse.from(CustomUserDetails)같은 팩토리 메서드 사용을 고려해 주세요.
backend/src/main/java/org/sejongisc/backend/user/dto/UserUpdateRequest.java
Outdated
Show resolved
Hide resolved
1 participant
Summary by CodeRabbit
새로운 기능
개선 사항
✏️ Tip: You can customize this high-level summary in your review settings.