[USER] 이메일 인증 개선, 쿠키(HttpOnly) 기반 JWT 인증 및 GitHub OAuth 로그인 추가#25
[USER] 이메일 인증 개선, 쿠키(HttpOnly) 기반 JWT 인증 및 GitHub OAuth 로그인 추가#25s0ooo0k merged 9 commits intoPETTY-HUB:mainfrom
Conversation
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Summary by CodeRabbit
Walkthrough이번 변경사항에서는 OAuth2(GitHub) 기반 소셜 로그인 기능이 프로젝트에 통합되었으며, JWT 발급 및 인증 방식을 쿠키 기반으로 전환하였습니다. 사용자 프로필 수정 기능이 추가되어, 사용자는 자신의 이름과 전화번호를 웹에서 직접 수정할 수 있습니다. 데이터베이스 엔티티에는 컬럼 제약조건이 강화되고, OAuth2 관련 필드가 추가되었습니다. 이메일 인증 방식이 6자리 코드 및 HTML 이메일로 개선되었으며, 회원가입 및 로그인, 메인 페이지의 인증 흐름이 쿠키 기반으로 일원화되었습니다. 관련 서비스, DTO, 핸들러, 템플릿 파일이 신설 및 수정되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Browser
participant AppServer
participant OAuth2Provider as "GitHub OAuth2"
participant DB
User->Browser: 로그인 페이지 접속
Browser->User: 로그인 또는 GitHub 버튼 클릭 요청
User->Browser: GitHub 소셜 로그인 클릭
Browser->AppServer: /oauth2/authorization/github 요청
AppServer->OAuth2Provider: OAuth2 인증 요청
OAuth2Provider->User: GitHub 로그인/동의
OAuth2Provider->AppServer: 인증 코드 전달
AppServer->OAuth2Provider: 토큰 교환
AppServer->OAuth2Provider: 사용자 정보 조회
AppServer->DB: 사용자 존재 여부 확인
alt 신규 사용자
AppServer->DB: 사용자 자동 생성 (provider, providerId, email 등)
end
AppServer->Browser: JWT 쿠키 발급(Set-Cookie), 루트로 리다이렉트
Browser->AppServer: 메인 페이지 등 접근 (쿠키 포함)
AppServer->DB: JWT 검증 및 사용자 인증
AppServer->Browser: 인증된 페이지 응답
sequenceDiagram
participant User
participant Browser
participant AppServer
participant DB
User->Browser: 프로필 수정 클릭
Browser->AppServer: GET /profile/edit
AppServer->DB: 사용자 정보 조회
AppServer->Browser: 프로필 수정 폼 반환
User->Browser: 정보 입력 후 저장 클릭
Browser->AppServer: POST /profile/update
AppServer->DB: 사용자 정보 업데이트
AppServer->Browser: 결과 메시지 및 리다이렉트
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chainI'll need to gather information about this PR to provide a thorough review. Let me fetch the details. 🏁 Script executed: Length of output: 4206 Now I need to examine the key files that were changed in this PR to provide a thorough review. Let me first check some of the most important ones related to authentication and security. 🏁 Script executed: Length of output: 20824 Let me examine the login filter and user service for a more comprehensive review. 🏁 Script executed: Length of output: 6730 PR 리뷰: 인증 시스템 개선이 PR은 사용자 인증 및 보안 시스템을 크게 개선했습니다. 전반적으로 잘 구현되었으며, 몇 가지 개선점을 제안합니다. 좋은 점 👍
개선 제안 🔄
전체적으로 보안과 사용자 경험을 크게 개선한 좋은 PR입니다. 몇 가지 작은 개선 사항만 반영하면 더욱 완성도 높은 코드가 될 것 같습니다. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
🔭 Outside diff range comments (1)
src/main/resources/templates/login.html (1)
115-142:⚠️ Potential issue로그인 함수와 쿠키 기반 인증 간의 불일치가 있습니다.
PR 목표에 따르면 JWT 토큰 저장 방식이 localStorage에서 HttpOnly 쿠키로 변경되었다고 되어 있으나, 여전히 로그인 함수는 토큰을 localStorage에 저장하고 있습니다. 쿠키 기반 인증으로의 완전한 전환을 위해 이 부분도 수정이 필요합니다.
if (response.ok) { - const token = response.headers.get('Authorization'); - if (token) { - const jwtToken = token.startsWith('Bearer ') ? token.slice(7) : token; - localStorage.setItem('jwt', jwtToken); window.location.href = '/'; - } }참고: 쿠키 기반 인증에서는 서버가 쿠키를 설정하므로 클라이언트에서 토큰을 추출하고 저장하는 로직은 필요하지 않습니다.
🧹 Nitpick comments (13)
src/main/java/io/github/petty/users/jwt/JWTFilter.java (1)
60-68: 토큰 만료 처리가 개선되었습니다.토큰 만료 시 보안 컨텍스트를 명시적으로 초기화하고 적절한 응답 코드와 메시지를 반환하도록 개선되었습니다. 클라이언트에 명확한 오류 피드백을 제공합니다.
다만, HTTPS 환경에서의 보안을 더욱 강화하기 위해 쿠키에 'secure' 속성 확인을 추가하는 것을 고려해보세요.
src/main/java/io/github/petty/users/entity/EmailVerification.java (2)
22-24: 인증 코드 길이 검증 로직 개선.PR 목표에 명시된 대로 인증 코드 길이를 6자리로 제한하는
@Size어노테이션을 적용했습니다. 이는 보안성 향상을 위한 조치입니다.다만, 현재 어노테이션은 문자열 길이만 검증하고 숫자만 포함하는지는 검증하지 않습니다. 숫자로만 구성된 코드가 필요하다면 추가 검증이 필요합니다.
@Column(nullable = false) @Size(min = 6, max = 6) +@Pattern(regexp = "^[0-9]{6}$", message = "인증 코드는 6자리 숫자여야 합니다") private String code;
26-28: 만료 시간 관리 기능 추가 고려.현재
createdAt필드는 있지만, 인증 코드의 명시적인 만료 시간 필드가 없습니다. PR 목표 중 "서버에서 인증 코드 만료 관리"를 위해 전용 필드 추가를 고려해보세요.@CreationTimestamp private LocalDateTime createdAt; +private LocalDateTime expiresAt;src/main/java/io/github/petty/users/jwt/JWTUtil.java (1)
32-41: 토큰 만료 검증 로직이 크게 개선되었습니다.예외 처리를 추가하여 토큰 만료 확인을 더 정확하게 처리하도록 개선되었습니다. 특히:
ExpiredJwtException을 명시적으로 처리하여 만료된 토큰을 정확히 식별- 다른 예외는 별도로 처리하여 만료 이외의 토큰 문제를 구분
다만, 다른 예외를 모두
false로 처리하는 것은 다양한 토큰 문제를 구분하지 못하게 할 수 있습니다. 주요 JWT 예외를 구체적으로 처리하는 것을 고려해보세요.try { return Jwts.parser().verifyWith(secretKey).build().parseSignedClaims(token).getPayload().getExpiration().before(new Date()); } catch (ExpiredJwtException e) { return true; // 토큰 만료됨 } catch (Exception e) { // 다른 예외 처리 (토큰 형식 오류 등) + // 예외 유형에 따라 로그 기록 + // SignatureException, MalformedJwtException 등 특정 예외 처리 return false; }src/main/resources/templates/profile_edit.html (2)
7-63: 일관된 디자인 시스템 적용 필요프로필 수정 페이지의 스타일이 인라인 CSS로 구현되어 있습니다. 프로젝트 전체의 일관성을 위해 별도의 CSS 파일로 분리하거나 공통 스타일을 활용하는 것이 좋습니다. 현재 방식은 유지보수성을 저하시킬 수 있습니다.
- <style> - body { - font-family: Arial, sans-serif; - background-color: #fdf7e4; - color: #333; - padding: 20px; - } - /* ... 중략 ... */ - </style> + <link rel="stylesheet" th:href="@{/css/common.css}"> + <link rel="stylesheet" th:href="@{/css/profile.css}">
83-104: 폼 유효성 검증 로직 개선 필요현재 유효성 검증 로직이 DOM 조작에 의존하고 있습니다. 모던 웹 개발 방식으로 개선하고, 이름 필드에 대한 유효성 검증도 추가하는 것이 좋습니다.
document.addEventListener('DOMContentLoaded', function() { const form = document.getElementById('editProfileForm'); const displayNameInput = document.getElementById('displayName'); const phoneInput = document.getElementById('phone'); const phoneErrorElement = document.getElementById('phoneError'); const nameErrorElement = document.createElement('div'); // 이름 필드 오류 메시지 요소 추가 nameErrorElement.className = 'error'; nameErrorElement.id = 'nameError'; nameErrorElement.textContent = '이름을 입력해주세요.'; displayNameInput.parentNode.appendChild(nameErrorElement); // 폼 제출 이벤트 form.addEventListener('submit', function(e) { let isValid = true; // 이름 검증 if (displayNameInput.value.trim() === '') { displayNameInput.classList.add('invalid'); nameErrorElement.style.display = 'block'; isValid = false; } else { displayNameInput.classList.remove('invalid'); nameErrorElement.style.display = 'none'; } // 전화번호 검증 const phone = phoneInput.value.trim(); const phoneRegex = /^01[016789][-\s]?\d{3,4}[-\s]?\d{4}$/; if (!phoneRegex.test(phone)) { phoneInput.classList.add('invalid'); phoneErrorElement.style.display = 'block'; isValid = false; } else { phoneInput.classList.remove('invalid'); phoneErrorElement.style.display = 'none'; } if (!isValid) { e.preventDefault(); } }); });src/main/resources/templates/index.html (2)
85-105: 쿠키 기반 인증 방식으로 전환localStorage에서 쿠키 기반 JWT 인증 방식으로 전환한 점은 보안성 측면에서 좋은 개선입니다. 특히 HttpOnly 쿠키를 사용함으로써 XSS 공격으로부터 토큰을 보호할 수 있게 되었습니다.
다만, 주석으로 표시된 "쿠키가 만료되었거나 유효하지 않은 경우"에 대한 추가 처리가 없는 점이 아쉽습니다. 사용자 경험 향상을 위해 만료된 세션에 대한 적절한 메시지와 리다이렉션을 추가하는 것이 좋겠습니다.
} else { showLoginMenu(); if (response.status === 401) { - // 쿠키가 만료되었거나 유효하지 않은 경우 + // 쿠키가 만료되었거나 유효하지 않은 경우 + console.log('인증 세션이 만료되었습니다.'); + // 선택적으로 사용자에게 알림 + // alert('로그인 세션이 만료되었습니다. 다시 로그인해주세요.'); } }
117-133: 로그아웃 처리 개선로그아웃 처리가 fetch API를 사용한 비동기 POST 요청으로 개선되었습니다. 이는 CSRF 보호 측면에서도 좋은 접근 방식입니다. 성공 및 실패 시 사용자 피드백도 적절히 구현되었습니다.
다만, 로그아웃 성공 후 메인 페이지 리다이렉션 전에 약간의 지연을 두어 사용자에게 알림 메시지가 충분히 보이도록 하는 것이 좋겠습니다.
if (response.ok) { alert('로그아웃 성공'); - window.location.href = '/'; // 로그아웃 성공 시 메인 페이지로 이동 + setTimeout(() => { + window.location.href = '/'; // 로그아웃 성공 시 메인 페이지로 이동 + }, 500); // 0.5초 후 리다이렉션 } else {src/main/java/io/github/petty/users/controller/UsersController.java (1)
67-73: 인증 확인 로직 중복을 개선할 수 있습니다.두 엔드포인트에서 동일한 인증 확인 로직이 중복되어 있습니다. 이를 별도의 메서드로 추출하여 코드 재사용성을 높이는 것이 좋습니다.
+ private String checkAuthentication() { + Authentication authentication = SecurityContextHolder.getContext().getAuthentication(); + + if (authentication == null || !authentication.isAuthenticated() + || authentication instanceof AnonymousAuthenticationToken) { + return "redirect:/login"; + } + return null; + } @GetMapping("/profile/edit") public String editProfileForm(Model model) { - Authentication authentication = SecurityContextHolder.getContext().getAuthentication(); - - if (authentication == null || !authentication.isAuthenticated() - || authentication instanceof AnonymousAuthenticationToken) { - return "redirect:/login"; - } + String redirectResult = checkAuthentication(); + if (redirectResult != null) { + return redirectResult; + } + Authentication authentication = SecurityContextHolder.getContext().getAuthentication(); Object principal = authentication.getPrincipal(); UUID currentUserId = userService.getCurrentUserId(principal); UserProfileEditDTO userProfile = userService.getUserById(currentUserId);Also applies to: 86-91
src/main/java/io/github/petty/users/oauth2/CustomOAuth2UserService.java (1)
9-9: 불필요한 어노테이션이 사용되었습니다.
@RequiredArgsConstructor를 사용할 때는@Autowired어노테이션이 필요하지 않습니다. Lombok이 final 필드에 대한 생성자 주입을 자동으로 처리합니다.-import org.springframework.beans.factory.annotation.Autowired;src/main/resources/templates/join.html (1)
327-560: 스크립트 코드를 별도 파일로 분리하는 것이 좋습니다.현재 JavaScript 코드가 HTML 파일 내에 직접 작성되어 있어 파일이 매우 길고 유지보수가 어렵습니다. 스크립트 코드를 별도의 .js 파일로 분리하여 관리하는 것이 좋습니다.
-<script> - // 현재 JavaScript 코드 전체 -</script> +<script src="/js/join.js"></script>src/main/java/io/github/petty/users/service/UserService.java (2)
9-9: 불필요한 어노테이션이 사용되었습니다.
@RequiredArgsConstructor를 사용할 때는@Autowired어노테이션이 필요하지 않습니다. Lombok이 final 필드에 대한 생성자 주입을 자동으로 처리합니다.-import org.springframework.beans.factory.annotation.Autowired;
51-55: 불필요한 빈 줄이 있습니다.코드 가독성을 위해 일부 공백은 좋지만, 너무 많은 빈 줄은 코드 읽기를 오히려 방해할 수 있습니다. 불필요한 빈 줄을 제거하는 것이 좋습니다.
} - - - - - // 사용자 정보 가져오기 public UserProfileEditDTO getUserById(UUID userId) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
build.gradle(1 hunks)src/main/java/io/github/petty/config/SecurityConfig.java(3 hunks)src/main/java/io/github/petty/config/SupabaseDataSourceConfig.java(2 hunks)src/main/java/io/github/petty/users/controller/UsersController.java(3 hunks)src/main/java/io/github/petty/users/dto/UserProfileEditDTO.java(1 hunks)src/main/java/io/github/petty/users/entity/EmailVerification.java(2 hunks)src/main/java/io/github/petty/users/entity/Users.java(2 hunks)src/main/java/io/github/petty/users/jwt/JWTFilter.java(2 hunks)src/main/java/io/github/petty/users/jwt/JWTUtil.java(3 hunks)src/main/java/io/github/petty/users/jwt/LoginFilter.java(3 hunks)src/main/java/io/github/petty/users/oauth2/CustomOAuth2User.java(1 hunks)src/main/java/io/github/petty/users/oauth2/CustomOAuth2UserService.java(1 hunks)src/main/java/io/github/petty/users/oauth2/OAuth2SuccessHandler.java(1 hunks)src/main/java/io/github/petty/users/service/EmailService.java(2 hunks)src/main/java/io/github/petty/users/service/JoinService.java(1 hunks)src/main/java/io/github/petty/users/service/UserService.java(1 hunks)src/main/resources/templates/index.html(3 hunks)src/main/resources/templates/join.html(1 hunks)src/main/resources/templates/login.html(2 hunks)src/main/resources/templates/profile_edit.html(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
src/main/java/io/github/petty/users/oauth2/OAuth2SuccessHandler.java (1)
src/main/java/io/github/petty/users/jwt/JWTUtil.java (1)
Component(15-53)
src/main/java/io/github/petty/users/controller/UsersController.java (1)
src/main/java/io/github/petty/users/dto/CustomUserDetails.java (1)
CustomUserDetails(10-63)
🪛 ast-grep (0.31.1)
src/main/java/io/github/petty/users/oauth2/OAuth2SuccessHandler.java
[warning] 36-36: The application does not appear to verify inbound requests which can lead to a Cross-site request forgery (CSRF) vulnerability. If the application uses cookie-based authentication, an attacker can trick users into sending authenticated HTTP requests without their knowledge from any arbitrary domain they visit. To prevent this vulnerability start by identifying if the framework or library leveraged has built-in features or offers plugins for CSRF protection. CSRF tokens should be unique and securely random. The Synchronizer Token or Double Submit Cookie patterns with defense-in-depth mechanisms such as the sameSite cookie flag can help prevent CSRF. For more information, see: [Cross-site request forgery prevention](https://cheatsheetseries.owasp.org/cheatsheets/Cross-Site_Req\ uest_Forgery_Prevention_Cheat_Sheet.html).
Context: response.addCookie(jwtCookie);
Note: [CWE-352] Cross-Site Request Forgery (CSRF). [REFERENCES]
- https://stackoverflow.com/questions/42717210/samesite-cookie-in-java-application
(cookie-missing-samesite-java)
[warning] 36-36: A cookie was detected without setting the 'secure' flag. The 'secure' flag for cookies prevents the client from transmitting the cookie over insecure channels such as HTTP. Set the 'secure' flag by calling '.setSecure(true);'.
Context: response.addCookie(jwtCookie);
Note: [CWE-614] Sensitive Cookie in HTTPS Session Without 'Secure' Attribute. [REFERENCES]
- https://owasp.org/www-community/controls/SecureCookieAttribute
(cookie-missing-secure-flag-java)
src/main/java/io/github/petty/users/jwt/LoginFilter.java
[warning] 80-80: The application does not appear to verify inbound requests which can lead to a Cross-site request forgery (CSRF) vulnerability. If the application uses cookie-based authentication, an attacker can trick users into sending authenticated HTTP requests without their knowledge from any arbitrary domain they visit. To prevent this vulnerability start by identifying if the framework or library leveraged has built-in features or offers plugins for CSRF protection. CSRF tokens should be unique and securely random. The Synchronizer Token or Double Submit Cookie patterns with defense-in-depth mechanisms such as the sameSite cookie flag can help prevent CSRF. For more information, see: [Cross-site request forgery prevention](https://cheatsheetseries.owasp.org/cheatsheets/Cross-Site_Req\ uest_Forgery_Prevention_Cheat_Sheet.html).
Context: response.addCookie(jwtCookie);
Note: [CWE-352] Cross-Site Request Forgery (CSRF). [REFERENCES]
- https://stackoverflow.com/questions/42717210/samesite-cookie-in-java-application
(cookie-missing-samesite-java)
[warning] 80-80: A cookie was detected without setting the 'secure' flag. The 'secure' flag for cookies prevents the client from transmitting the cookie over insecure channels such as HTTP. Set the 'secure' flag by calling '.setSecure(true);'.
Context: response.addCookie(jwtCookie);
Note: [CWE-614] Sensitive Cookie in HTTPS Session Without 'Secure' Attribute. [REFERENCES]
- https://owasp.org/www-community/controls/SecureCookieAttribute
(cookie-missing-secure-flag-java)
🔇 Additional comments (40)
build.gradle (1)
59-60: OAuth2 지원을 위한 적절한 의존성 추가.GitHub OAuth 로그인 기능 구현을 위해 필요한 Spring Security OAuth2 Client 의존성이 올바르게 추가되었습니다. 이 의존성은 OAuth2 클라이언트 관련 기능을 사용하기 위한 필수 요소입니다.
src/main/java/io/github/petty/users/service/JoinService.java (1)
36-36: OAuth2 통합을 위한 적절한 provider 설정.일반 회원가입 사용자를 "local" provider로 표시하는 것은 OAuth 기반 사용자와 구분하기 위한 좋은 접근법입니다. 이는 소셜 로그인과 로컬 로그인 사용자를 구분하는 데 도움이 됩니다.
src/main/java/io/github/petty/users/service/EmailService.java (1)
46-46: 인증 코드 길이 증가로 보안 강화.4자리에서 6자리로 인증 코드 길이를 늘린 것은 보안을 강화하는 좋은 개선사항입니다. 무작위 대입 공격에 대한 저항력이 높아집니다.
src/main/java/io/github/petty/users/jwt/JWTFilter.java (4)
7-11: 로그 기능 추가로 디버깅이 개선되었습니다.SLF4J 로거를 추가하여 인증 과정의 디버깅이 용이해졌습니다. 이는 개발 과정에서 토큰 관련 문제를 쉽게 파악할 수 있게 합니다.
Also applies to: 22-22
31-42: 쿠키 기반 토큰 추출 로직이 잘 구현되었습니다.JWT 토큰을 먼저 쿠키에서 확인하고, 없을 경우 Authorization 헤더로 폴백하는 접근 방식이 적절합니다. 이는 PR 목표인 "localStorage에서 HttpOnly 쿠키로 토큰 저장 방식 변경"에 부합합니다.
45-51: 헤더 기반 토큰 추출 로직을 유지하여 하위 호환성을 제공합니다.쿠키와 헤더 두 방식 모두 지원하여 원활한 전환이 가능하도록 한 점이 좋습니다. 기존 클라이언트에 대한 하위 호환성을 유지하면서 점진적 마이그레이션이 가능합니다.
54-58: 토큰 부재 상황에 대한 로깅이 개선되었습니다.토큰이 없는 경우를 명확히 로깅하여 사용자 인증 상태를 파악하기 쉽게 하였습니다. 디버깅 과정에서 유용합니다.
src/main/java/io/github/petty/users/entity/EmailVerification.java (2)
8-9: Bean Validation 의존성 추가.이메일 인증 코드 길이 검증을 위한
@Size어노테이션을 사용하기 위해 적절한 의존성을 추가했습니다.
19-20: 이메일 필드에 적절한 길이 제한 추가.이메일 필드에 길이 제한(255)을 추가하여 데이터베이스 스키마를 더 명확하게 정의했습니다. 이 길이는 일반적인 이메일 주소의 최대 길이를 충분히 수용합니다.
src/main/java/io/github/petty/users/jwt/JWTUtil.java (2)
3-4: JWT 만료 예외 처리를 위한 적절한 예외 클래스 추가.
ExpiredJwtException을 명시적으로 가져와서 토큰 만료를 정확하게 처리할 수 있게 되었습니다.
49-49: 토큰 만료 시간 계산 로직 수정.토큰 만료 시간 계산에서 불필요한 1000 곱하기를 제거하여 정확한 만료 시간을 설정할 수 있게 되었습니다. 이제
expiredMs파라미터를 밀리초 단위로 직접 사용합니다.src/main/resources/templates/login.html (2)
54-83: 소셜 로그인을 위한 스타일이 적절하게 추가되었습니다.GitHub 로그인 버튼과 소셜 로그인 섹션을 위한 CSS 스타일이 깔끔하게 정의되었습니다. 특히:
- 소셜 로그인 섹션이 기존 로그인 폼과 시각적으로 잘 분리됨
- GitHub 버튼 스타일이 GitHub의 브랜드 색상과 일관성 있게 정의됨
- 아이콘 배치 및 크기가 적절하게 설정됨
101-110: GitHub OAuth2 로그인 기능이 적절하게 구현되었습니다.GitHub 로그인 버튼이 소셜 로그인 섹션에 추가되었으며, 올바른 OAuth2 인증 엔드포인트(
/oauth2/authorization/github)로 연결됩니다. SVG 아이콘을 사용하여 시각적으로 명확하게 식별할 수 있습니다.src/main/java/io/github/petty/users/oauth2/CustomOAuth2User.java (1)
1-28: 코드가 잘 구현되었습니다.DefaultOAuth2User를 확장하여 필요한 사용자 정보(이메일, 제공자, 제공자 ID)를 추가한 클래스가 잘 구현되었습니다. Lombok의 @Getter 어노테이션을 적절하게 사용하고 있으며, 생성자에서 super 호출과 필드 초기화가 올바르게 이루어지고 있습니다.
src/main/java/io/github/petty/config/SupabaseDataSourceConfig.java (3)
5-5: Value 어노테이션 추가 - 좋은 접근법입니다.설정 값을 외부화하기 위한 @value 어노테이션을 추가한 것은 좋은 접근법입니다. 이를 통해 환경별로 다른 설정을 적용할 수 있게 됩니다.
66-67: DDL-AUTO 설정 외부화 - 좋은 변경사항입니다.Hibernate의 ddl-auto 설정을 하드코딩에서 외부 설정으로 변경한 것은 좋은 접근법입니다. 이를 통해 개발, 테스트, 운영 환경에 따라 적절한 설정을 유연하게 적용할 수 있습니다.
72-72: 외부화된 DDL-AUTO 설정 적용 - 적절합니다.하드코딩된 값 대신 주입받은 ddlAuto 변수를 사용하여 Hibernate 설정을 구성하는 것은 적절합니다.
src/main/java/io/github/petty/users/jwt/LoginFilter.java (1)
27-29: JWT 만료 시간 주입 - 좋은 접근법입니다.JWT 만료 시간을 외부 설정에서 주입받는 방식은 환경별로 다른 설정을 적용할 수 있어 좋은 접근법입니다.
src/main/java/io/github/petty/users/oauth2/OAuth2SuccessHandler.java (2)
15-20: 컴포넌트 구조 및 의존성 주입 - 적절합니다.
@Component와@RequiredArgsConstructor를 사용한 의존성 주입 방식은 적절합니다. OAuth2 인증 성공 시 JWT 토큰 발급을 위한JWTUtil의존성이 잘 주입되었습니다.
39-42: 리다이렉션 처리 - 적절합니다.인증 성공 후 홈 페이지로 리다이렉션하는 로직은 간단하고 명확하게 구현되었습니다.
src/main/java/io/github/petty/config/SecurityConfig.java (5)
6-7: OAuth2 통합을 위한 적절한 클래스 임포트GitHub OAuth2 로그인 기능 구현을 위한 필수 클래스들을 적절히 임포트하였습니다. CustomOAuth2UserService와 OAuth2SuccessHandler의 도입은 소셜 로그인 기능을 체계적으로 구현하기 위한 좋은 접근 방식입니다.
25-26: OAuth2 관련 의존성 주입 구현OAuth2 사용자 서비스와 인증 성공 핸들러를 의존성 주입으로 적절하게 구현하였습니다. 이는 SOLID 원칙 중 의존성 역전 원칙(DIP)을 잘 따르고 있으며, 테스트 용이성도 향상시킵니다.
Also applies to: 35-36
52-55: 최신 Spring Security 람다 스타일 적용Spring Security 구성을 최신 람다 스타일로 변경하여 가독성과 유지보수성이 향상되었습니다. 이는 Spring Security 6.x 버전의 권장 사항을 따른 좋은 개선입니다.
58-58: 로그인 및 OAuth2 엔드포인트 접근 권한 설정OAuth2 인증 관련 엔드포인트에 대한 접근 권한을 적절히 설정하였습니다. 인증 관련 경로를 모든 사용자에게 허용하는 것은 로그인 기능의 기본 요구사항입니다.
67-73: 쿠키 기반 JWT 로그아웃 처리 구현쿠키 기반 JWT 인증에 맞춰 로그아웃 처리를 적절히 구성하였습니다. 로그아웃 시 JWT 쿠키 삭제, 세션 무효화, 인증 정보 제거 등을 통해 보안을 강화하였습니다. 이는 기존 localStorage 기반 방식보다 보안성이 향상된 접근법입니다.
src/main/resources/templates/profile_edit.html (1)
69-80: 프로필 수정 폼 구성Thymeleaf 바인딩을 통한 프로필 수정 폼이 적절히 구현되었습니다.
th:field속성을 사용해 모델-뷰 바인딩을 구현한 점이 좋습니다.src/main/java/io/github/petty/users/entity/Users.java (3)
3-5: 적절한 임포트 구성임포트 구성이 적절하게 정리되어 있으며, 필요한 어노테이션과 클래스가 잘 포함되어 있습니다.
19-20: 보안 관련 필드 제약조건 개선사용자 인증과 관련된 중요 필드(username, password)에 대한 제약조건이 적절히 설정되었습니다. 특히
@JsonIgnore와@ToString.Exclude를 통해 비밀번호가 노출되지 않도록 처리한 점이 보안적으로 우수합니다.Also applies to: 22-25
27-34: 사용자 프로필 필드 제약조건사용자 프로필 관련 필드(displayName, phone, role)에 대한 길이 제약이 적절히 설정되었습니다. 특히 phone 필드를 nullable로 설정하여 선택적 입력이 가능하도록 한 점이 사용자 경험 측면에서 좋습니다.
src/main/resources/templates/index.html (3)
66-67: 사용자 메뉴 확장프로필 수정 기능 링크를 사용자 메뉴에 적절히 추가하였습니다. 로그아웃 이벤트 처리도 개선되었습니다.
80-83: Thymeleaf 인라인 자바스크립트 적용Thymeleaf의 인라인 자바스크립트 모드(
th:inline="javascript")를 사용하여 서버 측 변수를 자바스크립트에서 접근할 수 있도록 개선한 점이 좋습니다.
113-115: 프로필 편집 페이지 이동 함수프로필 편집 페이지로 이동하는 함수가 간결하게 구현되었습니다. 브라우저가 쿠키를 자동으로 전송하는 방식을 활용한 점이 좋습니다.
src/main/java/io/github/petty/users/controller/UsersController.java (1)
65-81: 프로필 수정 기능 구현이 잘 되었습니다.인증 확인 로직이 적절하게 구현되어 있으며, 비인증 사용자를 로그인 페이지로 리다이렉트하는 보안 처리가 좋습니다. 사용자 ID를 안전하게 추출하고 프로필 데이터를 가져오는 과정이 명확합니다.
src/main/java/io/github/petty/users/oauth2/CustomOAuth2UserService.java (2)
57-66: 이메일 추출 로직이 잘 구현되었습니다.GitHub에서 이메일을 가져오는 로직이 명확하며 이메일이 없는 경우에 대한 대체 방안도 잘 구현되어 있습니다. 추후 다른 OAuth2 제공자를 추가하기 쉽도록 확장성을 고려한 구조입니다.
78-78: 보안 관련 고려사항: 랜덤 비밀번호 설정UUID를 비밀번호로 설정하는 방식은 충분히 복잡하고 안전하지만, 이 사용자가 나중에 일반 로그인을 시도할 경우 문제가 발생할 수 있습니다. 소셜 로그인만 사용하도록 제한하거나, 최초 로그인 시 비밀번호 변경을 요구하는 메커니즘을 고려해 보세요.
src/main/resources/templates/join.html (4)
227-228: 6자리 인증코드 변경 및 타이머 UI 추가가 잘 되었습니다.이메일 인증 코드가 4자리에서 6자리로 변경되었고, 인증 만료 시간을 표시하는 타이머 UI가 추가되어 사용자 경험이 향상되었습니다.
478-517: 타이머 기능이 잘 구현되었습니다.이메일 인증 코드의 만료 시간을 관리하는 타이머 기능이 체계적으로 구현되어 있습니다. 시작, 재설정, 정지 기능이 모두 포함되어 있어 사용자 경험이 향상되었습니다.
534-540: 비밀번호 정책이 적절하게 구현되었습니다.비밀번호 정책이 영문, 숫자, 특수문자를 포함한 8자 이상으로 설정되어 있어 보안 수준이 높습니다. 사용자에게 명확한 오류 메시지도 제공하고 있어 좋습니다.
401-430: 인증코드 재발송 기능이 잘 구현되었습니다.이메일 인증코드 재발송 기능이 적절하게 구현되어 있으며, 재발송 버튼의 비활성화 처리와 타이머 재설정 등 사용자 경험을 고려한 부분이 좋습니다.
src/main/java/io/github/petty/users/service/UserService.java (1)
20-49: 다양한 인증 타입 처리가 잘 구현되었습니다.다양한 인증 방식(일반 로그인, 소셜 로그인)에 따른 사용자 ID 추출 로직이 체계적으로 구현되어 있습니다. 각 타입별 처리 방식이 명확하고 null 체크도 적절히 수행하고 있습니다.
| @Getter | ||
| @Setter | ||
| @NoArgsConstructor | ||
| @AllArgsConstructor | ||
| public class UserProfileEditDTO { | ||
| private String displayName; // 사용자 이름 | ||
| private String phone; // 전화번호 | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
유효성 검증 추가 필요.
사용자 프로필 편집을 위한 DTO 생성은 좋은 접근법이지만, 입력값에 대한 유효성 검증이 누락되어 있습니다. Jakarta Validation API를 사용하여 필드에 유효성 검증 어노테이션을 추가하는 것이 좋습니다.
다음과 같이 개선할 수 있습니다:
+import jakarta.validation.constraints.NotBlank;
+import jakarta.validation.constraints.Pattern;
import lombok.AllArgsConstructor;
import lombok.Getter;
import lombok.NoArgsConstructor;
import lombok.Setter;
@Getter
@Setter
@NoArgsConstructor
@AllArgsConstructor
public class UserProfileEditDTO {
+ @NotBlank(message = "이름은 필수 입력 항목입니다")
private String displayName; // 사용자 이름
+ @NotBlank(message = "전화번호는 필수 입력 항목입니다")
+ @Pattern(regexp = "^\\d{3}-\\d{3,4}-\\d{4}$", message = "유효한 전화번호 형식이 아닙니다")
private String phone; // 전화번호
}이렇게 하면 컨트롤러에서 @Valid 어노테이션을 사용하여 입력값을 검증할 수 있으며, 사용자에게 명확한 오류 메시지를 제공할 수 있습니다.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @Getter | |
| @Setter | |
| @NoArgsConstructor | |
| @AllArgsConstructor | |
| public class UserProfileEditDTO { | |
| private String displayName; // 사용자 이름 | |
| private String phone; // 전화번호 | |
| } | |
| import jakarta.validation.constraints.NotBlank; | |
| import jakarta.validation.constraints.Pattern; | |
| import lombok.AllArgsConstructor; | |
| import lombok.Getter; | |
| import lombok.NoArgsConstructor; | |
| import lombok.Setter; | |
| @Getter | |
| @Setter | |
| @NoArgsConstructor | |
| @AllArgsConstructor | |
| public class UserProfileEditDTO { | |
| @NotBlank(message = "이름은 필수 입력 항목입니다") | |
| private String displayName; // 사용자 이름 | |
| @NotBlank(message = "전화번호는 필수 입력 항목입니다") | |
| @Pattern(regexp = "^\\d{3}-\\d{3,4}-\\d{4}$", message = "유효한 전화번호 형식이 아닙니다") | |
| private String phone; // 전화번호 | |
| } |
| String token = jwtUtil.createJwt(username, role, 3600000L); // expirationTime | ||
|
|
There was a problem hiding this comment.
주입된 만료 시간 사용 필요
@value로 주입받은 expirationTime 변수를 사용하지 않고 하드코딩된 값(3600000L)을 사용하고 있습니다. 설정의 일관성을 위해 주입받은 변수를 사용하도록 수정해주세요.
다음과 같이 수정하는 것이 좋습니다:
- String token = jwtUtil.createJwt(username, role, 3600000L); // expirationTime
+ String token = jwtUtil.createJwt(username, role, expirationTime); // 주입받은 만료 시간 사용📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| String token = jwtUtil.createJwt(username, role, 3600000L); // expirationTime | |
| String token = jwtUtil.createJwt(username, role, expirationTime); // 주입받은 만료 시간 사용 |
| // JWT 토큰을 쿠키에 저장 | ||
| Cookie jwtCookie = new Cookie("jwt", token); | ||
| jwtCookie.setHttpOnly(true); // JavaScript 접근 방지 (XSS 방어) | ||
| jwtCookie.setPath("/"); // 쿠키의 유효 경로 | ||
| // jwtCookie.setSecure(true); // HTTPS 환경에서만 전송 (로컬호스트에서는 생략) | ||
| int maxAgeSeconds = (int) (3600000L / 1000); // 만료 시간을 초 단위로 변환 | ||
| jwtCookie.setMaxAge(maxAgeSeconds); // 쿠키의 만료 시간 설정 | ||
| response.addCookie(jwtCookie); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
쿠키 보안 강화 필요
JWT 토큰을 쿠키에 저장하는 방식으로 변경한 것은 좋은 접근이지만, 다음과 같은 보안 강화가 필요합니다:
- SameSite 속성 추가 - CSRF 공격 방지를 위해 필요합니다.
- Secure 플래그 활성화 - 운영 환경에서는 반드시 활성화해야 합니다.
- 만료 시간 계산에서도 주입된 expirationTime 변수를 사용해야 합니다.
다음과 같이 수정하는 것을 권장합니다:
// JWT 토큰을 쿠키에 저장
Cookie jwtCookie = new Cookie("jwt", token);
jwtCookie.setHttpOnly(true); // JavaScript 접근 방지 (XSS 방어)
jwtCookie.setPath("/"); // 쿠키의 유효 경로
- // jwtCookie.setSecure(true); // HTTPS 환경에서만 전송 (로컬호스트에서는 생략)
- int maxAgeSeconds = (int) (3600000L / 1000); // 만료 시간을 초 단위로 변환
+ // 운영 환경에서는 Secure 활성화
+ if (!"localhost".equals(request.getServerName()) && !"127.0.0.1".equals(request.getServerName())) {
+ jwtCookie.setSecure(true);
+ }
+ // SameSite 속성 설정 (서블릿 API에서 직접 지원하지 않으므로 헤더로 설정)
+ response.setHeader("Set-Cookie", response.getHeader("Set-Cookie") + "; SameSite=Lax");
+ int maxAgeSeconds = (int) (expirationTime / 1000); // 주입된 만료 시간 사용
jwtCookie.setMaxAge(maxAgeSeconds); // 쿠키의 만료 시간 설정
response.addCookie(jwtCookie);참고: Spring Boot 2.6 이상에서는 spring.servlet.session.cookie.same-site 속성으로 SameSite를 설정할 수 있습니다.
Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 ast-grep (0.31.1)
[warning] 80-80: The application does not appear to verify inbound requests which can lead to a Cross-site request forgery (CSRF) vulnerability. If the application uses cookie-based authentication, an attacker can trick users into sending authenticated HTTP requests without their knowledge from any arbitrary domain they visit. To prevent this vulnerability start by identifying if the framework or library leveraged has built-in features or offers plugins for CSRF protection. CSRF tokens should be unique and securely random. The Synchronizer Token or Double Submit Cookie patterns with defense-in-depth mechanisms such as the sameSite cookie flag can help prevent CSRF. For more information, see: [Cross-site request forgery prevention](https://cheatsheetseries.owasp.org/cheatsheets/Cross-Site_Req\ uest_Forgery_Prevention_Cheat_Sheet.html).
Context: response.addCookie(jwtCookie);
Note: [CWE-352] Cross-Site Request Forgery (CSRF). [REFERENCES]
- https://stackoverflow.com/questions/42717210/samesite-cookie-in-java-application
(cookie-missing-samesite-java)
[warning] 80-80: A cookie was detected without setting the 'secure' flag. The 'secure' flag for cookies prevents the client from transmitting the cookie over insecure channels such as HTTP. Set the 'secure' flag by calling '.setSecure(true);'.
Context: response.addCookie(jwtCookie);
Note: [CWE-614] Sensitive Cookie in HTTPS Session Without 'Secure' Attribute. [REFERENCES]
- https://owasp.org/www-community/controls/SecureCookieAttribute
(cookie-missing-secure-flag-java)
| @Override | ||
| public void onAuthenticationSuccess(HttpServletRequest request, HttpServletResponse response, | ||
| Authentication authentication) throws IOException, ServletException { | ||
|
|
||
| CustomOAuth2User oAuth2User = (CustomOAuth2User) authentication.getPrincipal(); | ||
|
|
||
| // JWT 발급 | ||
| String token = jwtUtil.createJwt(oAuth2User.getEmail(), oAuth2User.getAuthorities().iterator().next().getAuthority(), 3600000L); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
OAuth2 인증 성공 처리 및 JWT 토큰 생성
OAuth2 인증 성공 시 JWT 토큰을 생성하는 로직은 잘 구현되었습니다. 다만, 토큰 만료 시간이 하드코딩(3600000L)되어 있습니다. 이 값은 LoginFilter에서와 같이 설정 파일에서 주입받아 사용하는 것이 좋습니다.
다음과 같이 수정하는 것을 권장합니다:
+ @Value("${jwt.expiration-time}")
+ private long expirationTime;
+
@Override
public void onAuthenticationSuccess(HttpServletRequest request, HttpServletResponse response,
Authentication authentication) throws IOException, ServletException {
CustomOAuth2User oAuth2User = (CustomOAuth2User) authentication.getPrincipal();
// JWT 발급
- String token = jwtUtil.createJwt(oAuth2User.getEmail(), oAuth2User.getAuthorities().iterator().next().getAuthority(), 3600000L);
+ String token = jwtUtil.createJwt(oAuth2User.getEmail(), oAuth2User.getAuthorities().iterator().next().getAuthority(), expirationTime);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @Override | |
| public void onAuthenticationSuccess(HttpServletRequest request, HttpServletResponse response, | |
| Authentication authentication) throws IOException, ServletException { | |
| CustomOAuth2User oAuth2User = (CustomOAuth2User) authentication.getPrincipal(); | |
| // JWT 발급 | |
| String token = jwtUtil.createJwt(oAuth2User.getEmail(), oAuth2User.getAuthorities().iterator().next().getAuthority(), 3600000L); | |
| @Value("${jwt.expiration-time}") | |
| private long expirationTime; | |
| @Override | |
| public void onAuthenticationSuccess(HttpServletRequest request, HttpServletResponse response, | |
| Authentication authentication) throws IOException, ServletException { | |
| CustomOAuth2User oAuth2User = (CustomOAuth2User) authentication.getPrincipal(); | |
| // JWT 발급 | |
| String token = jwtUtil.createJwt( | |
| oAuth2User.getEmail(), | |
| oAuth2User.getAuthorities().iterator().next().getAuthority(), | |
| expirationTime | |
| ); |
| // JWT 토큰을 쿠키에 저장 | ||
| Cookie jwtCookie = new Cookie("jwt", token); | ||
| jwtCookie.setHttpOnly(true); // JavaScript 접근 방지 (XSS 방어) | ||
| jwtCookie.setPath("/"); // 쿠키의 유효 경로 | ||
| // jwtCookie.setSecure(true); // HTTPS 환경에서만 전송 (로컬호스트에서는 생략) | ||
| int maxAgeSeconds = (int) (3600000L / 1000); // 만료 시간을 초 단위로 변환 | ||
| jwtCookie.setMaxAge(maxAgeSeconds); // 쿠키의 만료 시간 설정 | ||
| response.addCookie(jwtCookie); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
쿠키 보안 강화 필요
JWT 토큰을 쿠키에 저장하는 방식은 적절하지만, 다음과 같은 보안 강화가 필요합니다:
- SameSite 속성 추가 - CSRF 공격 방지를 위해 필요합니다.
- Secure 플래그 활성화 - 운영 환경에서는 반드시 활성화해야 합니다.
- 만료 시간 계산에서도 주입된 expirationTime 변수를 사용해야 합니다.
- LoginFilter와 중복되는 코드는 공통 메서드로 분리하는 것이 좋습니다.
다음과 같이 수정하는 것을 권장합니다:
// JWT 토큰을 쿠키에 저장
Cookie jwtCookie = new Cookie("jwt", token);
jwtCookie.setHttpOnly(true); // JavaScript 접근 방지 (XSS 방어)
jwtCookie.setPath("/"); // 쿠키의 유효 경로
- // jwtCookie.setSecure(true); // HTTPS 환경에서만 전송 (로컬호스트에서는 생략)
- int maxAgeSeconds = (int) (3600000L / 1000); // 만료 시간을 초 단위로 변환
+ // 운영 환경에서는 Secure 활성화
+ if (!"localhost".equals(request.getServerName()) && !"127.0.0.1".equals(request.getServerName())) {
+ jwtCookie.setSecure(true);
+ }
+ // SameSite 속성 설정 (서블릿 API에서 직접 지원하지 않으므로 헤더로 설정)
+ response.setHeader("Set-Cookie", response.getHeader("Set-Cookie") + "; SameSite=Lax");
+ int maxAgeSeconds = (int) (expirationTime / 1000); // 주입된 만료 시간 사용
jwtCookie.setMaxAge(maxAgeSeconds); // 쿠키의 만료 시간 설정
response.addCookie(jwtCookie);참고: 쿠키 설정 로직은 LoginFilter와 중복되므로, 재사용 가능한 유틸리티 메서드로 분리하는 것이 좋습니다.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // JWT 토큰을 쿠키에 저장 | |
| Cookie jwtCookie = new Cookie("jwt", token); | |
| jwtCookie.setHttpOnly(true); // JavaScript 접근 방지 (XSS 방어) | |
| jwtCookie.setPath("/"); // 쿠키의 유효 경로 | |
| // jwtCookie.setSecure(true); // HTTPS 환경에서만 전송 (로컬호스트에서는 생략) | |
| int maxAgeSeconds = (int) (3600000L / 1000); // 만료 시간을 초 단위로 변환 | |
| jwtCookie.setMaxAge(maxAgeSeconds); // 쿠키의 만료 시간 설정 | |
| response.addCookie(jwtCookie); | |
| // JWT 토큰을 쿠키에 저장 | |
| Cookie jwtCookie = new Cookie("jwt", token); | |
| jwtCookie.setHttpOnly(true); // JavaScript 접근 방지 (XSS 방어) | |
| jwtCookie.setPath("/"); // 쿠키의 유효 경로 | |
| // 운영 환경에서는 Secure 활성화 | |
| if (!"localhost".equals(request.getServerName()) && !"127.0.0.1".equals(request.getServerName())) { | |
| jwtCookie.setSecure(true); | |
| } | |
| // SameSite 속성 설정 (서블릿 API에서 직접 지원하지 않으므로 헤더로 설정) | |
| response.setHeader("Set-Cookie", response.getHeader("Set-Cookie") + "; SameSite=Lax"); | |
| int maxAgeSeconds = (int) (expirationTime / 1000); // 주입된 만료 시간 사용 | |
| jwtCookie.setMaxAge(maxAgeSeconds); // 쿠키의 만료 시간 설정 | |
| response.addCookie(jwtCookie); |
🧰 Tools
🪛 ast-grep (0.31.1)
[warning] 36-36: The application does not appear to verify inbound requests which can lead to a Cross-site request forgery (CSRF) vulnerability. If the application uses cookie-based authentication, an attacker can trick users into sending authenticated HTTP requests without their knowledge from any arbitrary domain they visit. To prevent this vulnerability start by identifying if the framework or library leveraged has built-in features or offers plugins for CSRF protection. CSRF tokens should be unique and securely random. The Synchronizer Token or Double Submit Cookie patterns with defense-in-depth mechanisms such as the sameSite cookie flag can help prevent CSRF. For more information, see: [Cross-site request forgery prevention](https://cheatsheetseries.owasp.org/cheatsheets/Cross-Site_Req\ uest_Forgery_Prevention_Cheat_Sheet.html).
Context: response.addCookie(jwtCookie);
Note: [CWE-352] Cross-Site Request Forgery (CSRF). [REFERENCES]
- https://stackoverflow.com/questions/42717210/samesite-cookie-in-java-application
(cookie-missing-samesite-java)
[warning] 36-36: A cookie was detected without setting the 'secure' flag. The 'secure' flag for cookies prevents the client from transmitting the cookie over insecure channels such as HTTP. Set the 'secure' flag by calling '.setSecure(true);'.
Context: response.addCookie(jwtCookie);
Note: [CWE-614] Sensitive Cookie in HTTPS Session Without 'Secure' Attribute. [REFERENCES]
- https://owasp.org/www-community/controls/SecureCookieAttribute
(cookie-missing-secure-flag-java)
| try { | ||
| // 사용자 정보 수정 | ||
| userService.updateUserProfile(currentUserId, userProfileEditDTO); | ||
| redirectAttributes.addFlashAttribute("successMessage", "프로필이 성공적으로 수정되었습니다."); | ||
| } catch (Exception e) { | ||
| redirectAttributes.addFlashAttribute("errorMessage", "프로필 수정 중 오류가 발생했습니다: " + e.getMessage()); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
예외 처리 범위가 너무 넓습니다.
현재 모든 예외(Exception)를 catch하고 있는데, 이는 너무 광범위합니다. UserService에서 발생할 수 있는 특정 예외들을 명시적으로 처리하는 것이 더 좋습니다.
try {
// 사용자 정보 수정
userService.updateUserProfile(currentUserId, userProfileEditDTO);
redirectAttributes.addFlashAttribute("successMessage", "프로필이 성공적으로 수정되었습니다.");
- } catch (Exception e) {
+ } catch (RuntimeException e) {
redirectAttributes.addFlashAttribute("errorMessage", "프로필 수정 중 오류가 발생했습니다: " + e.getMessage());
+ } catch (Exception e) {
+ redirectAttributes.addFlashAttribute("errorMessage", "서버 오류가 발생했습니다. 관리자에게 문의하세요.");
+ // 로깅 추가
+ // log.error("프로필 수정 중 예상치 못한 오류 발생: ", e);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| try { | |
| // 사용자 정보 수정 | |
| userService.updateUserProfile(currentUserId, userProfileEditDTO); | |
| redirectAttributes.addFlashAttribute("successMessage", "프로필이 성공적으로 수정되었습니다."); | |
| } catch (Exception e) { | |
| redirectAttributes.addFlashAttribute("errorMessage", "프로필 수정 중 오류가 발생했습니다: " + e.getMessage()); | |
| } | |
| try { | |
| // 사용자 정보 수정 | |
| userService.updateUserProfile(currentUserId, userProfileEditDTO); | |
| redirectAttributes.addFlashAttribute("successMessage", "프로필이 성공적으로 수정되었습니다."); | |
| } catch (RuntimeException e) { | |
| redirectAttributes.addFlashAttribute("errorMessage", "프로필 수정 중 오류가 발생했습니다: " + e.getMessage()); | |
| } catch (Exception e) { | |
| redirectAttributes.addFlashAttribute("errorMessage", "서버 오류가 발생했습니다. 관리자에게 문의하세요."); | |
| // 로깅 추가 | |
| // log.error("프로필 수정 중 예상치 못한 오류 발생: ", e); | |
| } |
| @Override | ||
| public OAuth2User loadUser(OAuth2UserRequest userRequest) throws OAuth2AuthenticationException { | ||
| OAuth2UserService<OAuth2UserRequest, OAuth2User> delegate = new DefaultOAuth2UserService(); | ||
| OAuth2User oAuth2User = delegate.loadUser(userRequest); | ||
|
|
||
| // OAuth2 서비스 ID (github, google, naver 등) | ||
| String registrationId = userRequest.getClientRegistration().getRegistrationId(); | ||
|
|
||
| // OAuth2 로그인 진행 시 키가 되는 필드 값(PK) | ||
| String userNameAttributeName = userRequest.getClientRegistration() | ||
| .getProviderDetails().getUserInfoEndpoint().getUserNameAttributeName(); | ||
|
|
||
| // Github 유저 정보 추출 | ||
| Map<String, Object> attributes = oAuth2User.getAttributes(); | ||
|
|
||
| // GitHub 이메일 및 ID 추출 | ||
| String email = extractEmail(registrationId, attributes); | ||
| String providerId = attributes.get(userNameAttributeName).toString(); | ||
|
|
||
| // 기존 회원인지 확인하고 없으면 회원가입 진행 | ||
| Users user = saveOrUpdate(email, registrationId, providerId, attributes); | ||
|
|
||
| return new CustomOAuth2User( | ||
| Collections.singleton(new SimpleGrantedAuthority(user.getRole())), | ||
| attributes, | ||
| userNameAttributeName, | ||
| user.getUsername(), | ||
| registrationId, | ||
| providerId | ||
| ); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
트랜잭션 관리가 명시되지 않았습니다.
사용자 생성 및 업데이트 작업은 데이터베이스 트랜잭션 내에서 이루어져야 합니다. @Transactional 어노테이션을 메서드에 추가하는 것이 좋습니다.
+ @Transactional
@Override
public OAuth2User loadUser(OAuth2UserRequest userRequest) throws OAuth2AuthenticationException {
// 기존 코드
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @Override | |
| public OAuth2User loadUser(OAuth2UserRequest userRequest) throws OAuth2AuthenticationException { | |
| OAuth2UserService<OAuth2UserRequest, OAuth2User> delegate = new DefaultOAuth2UserService(); | |
| OAuth2User oAuth2User = delegate.loadUser(userRequest); | |
| // OAuth2 서비스 ID (github, google, naver 등) | |
| String registrationId = userRequest.getClientRegistration().getRegistrationId(); | |
| // OAuth2 로그인 진행 시 키가 되는 필드 값(PK) | |
| String userNameAttributeName = userRequest.getClientRegistration() | |
| .getProviderDetails().getUserInfoEndpoint().getUserNameAttributeName(); | |
| // Github 유저 정보 추출 | |
| Map<String, Object> attributes = oAuth2User.getAttributes(); | |
| // GitHub 이메일 및 ID 추출 | |
| String email = extractEmail(registrationId, attributes); | |
| String providerId = attributes.get(userNameAttributeName).toString(); | |
| // 기존 회원인지 확인하고 없으면 회원가입 진행 | |
| Users user = saveOrUpdate(email, registrationId, providerId, attributes); | |
| return new CustomOAuth2User( | |
| Collections.singleton(new SimpleGrantedAuthority(user.getRole())), | |
| attributes, | |
| userNameAttributeName, | |
| user.getUsername(), | |
| registrationId, | |
| providerId | |
| ); | |
| } | |
| @Transactional | |
| @Override | |
| public OAuth2User loadUser(OAuth2UserRequest userRequest) throws OAuth2AuthenticationException { | |
| OAuth2UserService<OAuth2UserRequest, OAuth2User> delegate = new DefaultOAuth2UserService(); | |
| OAuth2User oAuth2User = delegate.loadUser(userRequest); | |
| // OAuth2 서비스 ID (github, google, naver 등) | |
| String registrationId = userRequest.getClientRegistration().getRegistrationId(); | |
| // OAuth2 로그인 진행 시 키가 되는 필드 값(PK) | |
| String userNameAttributeName = userRequest.getClientRegistration() | |
| .getProviderDetails().getUserInfoEndpoint().getUserNameAttributeName(); | |
| // Github 유저 정보 추출 | |
| Map<String, Object> attributes = oAuth2User.getAttributes(); | |
| // GitHub 이메일 및 ID 추출 | |
| String email = extractEmail(registrationId, attributes); | |
| String providerId = attributes.get(userNameAttributeName).toString(); | |
| // 기존 회원인지 확인하고 없으면 회원가입 진행 | |
| Users user = saveOrUpdate(email, registrationId, providerId, attributes); | |
| return new CustomOAuth2User( | |
| Collections.singleton(new SimpleGrantedAuthority(user.getRole())), | |
| attributes, | |
| userNameAttributeName, | |
| user.getUsername(), | |
| registrationId, | |
| providerId | |
| ); | |
| } |
| private Users saveOrUpdate(String email, String provider, String providerId, Map<String, Object> attributes) { | ||
| Users user = usersRepository.findByUsername(email); | ||
|
|
||
| if (user == null) { | ||
| // 새 사용자 생성 | ||
| user = new Users(); | ||
| user.setUsername(email); | ||
| user.setProvider(provider); | ||
| user.setProviderId(providerId); | ||
| user.setPassword(UUID.randomUUID().toString()); // 임의의 패스워드 설정 | ||
| user.setRole(Role.ROLE_USER.name()); | ||
|
|
||
| // GitHub 사용자명을 displayName으로 사용 | ||
| if (attributes.containsKey("name") && attributes.get("name") != null) { | ||
| user.setDisplayName((String) attributes.get("name")); | ||
| } else if (attributes.containsKey("login")) { | ||
| user.setDisplayName((String) attributes.get("login")); | ||
| } else { | ||
| user.setDisplayName(email.split("@")[0]); | ||
| } | ||
|
|
||
| return usersRepository.save(user); | ||
| } | ||
|
|
||
| // 기존 사용자 업데이트 로직 (필요한 경우) | ||
| return user; | ||
| } |
There was a problem hiding this comment.
사용자 업데이트 로직이 구현되지 않았습니다.
주석(Line 93)에는 "기존 사용자 업데이트 로직"이라고 되어 있지만, 실제로는 아무런 업데이트 로직이 구현되어 있지 않습니다. 기존 사용자의 정보를 업데이트하는 코드를 추가해야 합니다.
// 기존 사용자 업데이트 로직 (필요한 경우)
+ // 소셜 로그인 정보가 변경되었거나 추가 정보가 필요한 경우
+ if (user.getProvider() == null || !user.getProvider().equals(provider)) {
+ user.setProvider(provider);
+ user.setProviderId(providerId);
+ }
+
+ // 이름이 없거나 업데이트가 필요한 경우 GitHub 이름으로 업데이트
+ if (user.getDisplayName() == null || user.getDisplayName().isEmpty()) {
+ if (attributes.containsKey("name") && attributes.get("name") != null) {
+ user.setDisplayName((String) attributes.get("name"));
+ } else if (attributes.containsKey("login")) {
+ user.setDisplayName((String) attributes.get("login"));
+ }
+ }
+
+ return usersRepository.save(user);
- return user;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private Users saveOrUpdate(String email, String provider, String providerId, Map<String, Object> attributes) { | |
| Users user = usersRepository.findByUsername(email); | |
| if (user == null) { | |
| // 새 사용자 생성 | |
| user = new Users(); | |
| user.setUsername(email); | |
| user.setProvider(provider); | |
| user.setProviderId(providerId); | |
| user.setPassword(UUID.randomUUID().toString()); // 임의의 패스워드 설정 | |
| user.setRole(Role.ROLE_USER.name()); | |
| // GitHub 사용자명을 displayName으로 사용 | |
| if (attributes.containsKey("name") && attributes.get("name") != null) { | |
| user.setDisplayName((String) attributes.get("name")); | |
| } else if (attributes.containsKey("login")) { | |
| user.setDisplayName((String) attributes.get("login")); | |
| } else { | |
| user.setDisplayName(email.split("@")[0]); | |
| } | |
| return usersRepository.save(user); | |
| } | |
| // 기존 사용자 업데이트 로직 (필요한 경우) | |
| return user; | |
| } | |
| private Users saveOrUpdate(String email, String provider, String providerId, Map<String, Object> attributes) { | |
| Users user = usersRepository.findByUsername(email); | |
| if (user == null) { | |
| // 새 사용자 생성 | |
| user = new Users(); | |
| user.setUsername(email); | |
| user.setProvider(provider); | |
| user.setProviderId(providerId); | |
| user.setPassword(UUID.randomUUID().toString()); // 임의의 패스워드 설정 | |
| user.setRole(Role.ROLE_USER.name()); | |
| // GitHub 사용자명을 displayName으로 사용 | |
| if (attributes.containsKey("name") && attributes.get("name") != null) { | |
| user.setDisplayName((String) attributes.get("name")); | |
| } else if (attributes.containsKey("login")) { | |
| user.setDisplayName((String) attributes.get("login")); | |
| } else { | |
| user.setDisplayName(email.split("@")[0]); | |
| } | |
| return usersRepository.save(user); | |
| } | |
| // 기존 사용자 업데이트 로직 (필요한 경우) | |
| // 소셜 로그인 정보가 변경되었거나 추가 정보가 필요한 경우 | |
| if (user.getProvider() == null || !user.getProvider().equals(provider)) { | |
| user.setProvider(provider); | |
| user.setProviderId(providerId); | |
| } | |
| // 이름이 없거나 업데이트가 필요한 경우 GitHub 이름으로 업데이트 | |
| if (user.getDisplayName() == null || user.getDisplayName().isEmpty()) { | |
| if (attributes.containsKey("name") && attributes.get("name") != null) { | |
| user.setDisplayName((String) attributes.get("name")); | |
| } else if (attributes.containsKey("login")) { | |
| user.setDisplayName((String) attributes.get("login")); | |
| } | |
| } | |
| return usersRepository.save(user); | |
| } |
| public UserProfileEditDTO getUserById(UUID userId) { | ||
| Users user = usersRepository.findById(userId) | ||
| .orElseThrow(() -> new RuntimeException("사용자를 찾을 수 없습니다.")); | ||
| return new UserProfileEditDTO(user.getDisplayName(), user.getPhone()); | ||
|
|
||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
예외 메시지에 상세 정보 추가가 필요합니다.
현재 예외 메시지가 너무 일반적입니다. 사용자 ID를 포함한 더 상세한 메시지를 제공하면 디버깅이 용이해집니다. 또한 트랜잭션 관리가 필요합니다.
+ @Transactional(readOnly = true)
public UserProfileEditDTO getUserById(UUID userId) {
Users user = usersRepository.findById(userId)
- .orElseThrow(() -> new RuntimeException("사용자를 찾을 수 없습니다."));
+ .orElseThrow(() -> new RuntimeException("사용자를 찾을 수 없습니다. ID: " + userId));
return new UserProfileEditDTO(user.getDisplayName(), user.getPhone());
}
+ @Transactional
public Users updateUserProfile(UUID userId, UserProfileEditDTO userProfileEditDTO) {
Users user = usersRepository.findById(userId)
- .orElseThrow(() -> new RuntimeException("사용자를 찾을 수 없습니다."));
+ .orElseThrow(() -> new RuntimeException("사용자를 찾을 수 없습니다. ID: " + userId));
user.setDisplayName(userProfileEditDTO.getDisplayName());
user.setPhone(userProfileEditDTO.getPhone());
return usersRepository.save(user);
}Also applies to: 65-73
| public Users updateUserProfile(UUID userId, UserProfileEditDTO userProfileEditDTO) { | ||
| Users user = usersRepository.findById(userId) | ||
| .orElseThrow(() -> new RuntimeException("사용자를 찾을 수 없습니다.")); | ||
|
|
||
| user.setDisplayName(userProfileEditDTO.getDisplayName()); | ||
| user.setPhone(userProfileEditDTO.getPhone()); | ||
|
|
||
| return usersRepository.save(user); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
입력 데이터 검증이 누락되었습니다.
사용자 프로필 업데이트 시 입력값에 대한 유효성 검증이 누락되었습니다. 최소한 null 체크와 기본적인 유효성 검증을 추가하는 것이 좋습니다.
public Users updateUserProfile(UUID userId, UserProfileEditDTO userProfileEditDTO) {
+ if (userId == null || userProfileEditDTO == null) {
+ throw new IllegalArgumentException("유효하지 않은 사용자 ID 또는 프로필 데이터입니다.");
+ }
+
+ // 이름 유효성 검사
+ if (userProfileEditDTO.getDisplayName() == null || userProfileEditDTO.getDisplayName().trim().isEmpty()) {
+ throw new IllegalArgumentException("이름은 필수 입력 항목입니다.");
+ }
+
+ // 전화번호 유효성 검사 (기본 패턴)
+ if (userProfileEditDTO.getPhone() != null && !userProfileEditDTO.getPhone().matches("^01[016789]\\d{7,8}$")) {
+ throw new IllegalArgumentException("유효하지 않은 전화번호 형식입니다.");
+ }
+
Users user = usersRepository.findById(userId)
.orElseThrow(() -> new RuntimeException("사용자를 찾을 수 없습니다."));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public Users updateUserProfile(UUID userId, UserProfileEditDTO userProfileEditDTO) { | |
| Users user = usersRepository.findById(userId) | |
| .orElseThrow(() -> new RuntimeException("사용자를 찾을 수 없습니다.")); | |
| user.setDisplayName(userProfileEditDTO.getDisplayName()); | |
| user.setPhone(userProfileEditDTO.getPhone()); | |
| return usersRepository.save(user); | |
| } | |
| public Users updateUserProfile(UUID userId, UserProfileEditDTO userProfileEditDTO) { | |
| if (userId == null || userProfileEditDTO == null) { | |
| throw new IllegalArgumentException("유효하지 않은 사용자 ID 또는 프로필 데이터입니다."); | |
| } | |
| // 이름 유효성 검사 | |
| if (userProfileEditDTO.getDisplayName() == null || userProfileEditDTO.getDisplayName().trim().isEmpty()) { | |
| throw new IllegalArgumentException("이름은 필수 입력 항목입니다."); | |
| } | |
| // 전화번호 유효성 검사 (기본 패턴) | |
| if (userProfileEditDTO.getPhone() != null && !userProfileEditDTO.getPhone().matches("^01[016789]\\d{7,8}$")) { | |
| throw new IllegalArgumentException("유효하지 않은 전화번호 형식입니다."); | |
| } | |
| Users user = usersRepository.findById(userId) | |
| .orElseThrow(() -> new RuntimeException("사용자를 찾을 수 없습니다.")); | |
| user.setDisplayName(userProfileEditDTO.getDisplayName()); | |
| user.setPhone(userProfileEditDTO.getPhone()); | |
| return usersRepository.save(user); | |
| } |
✅ 리뷰
|
2 participants
📜 PR 내용 요약
⚒️ 작업 및 변경 내용
이메일 인증 시스템 개선
JWT 인증 시스템 고도화
토큰 저장 방식 변경
만료 시간 yml 설정화
application.yml에서 관리할 수 있도록 변경JWT 인증 필터 개선
회원 정보 관리 기능 추가
추가 기능
개발 환경 개선
ddl-auto설정 적용📚 기타 참고 사항
리뷰 포인트
환경 설정 관련
application-secret.yml에 Gmail 계정 및 앱 비밀번호 추가 필요