Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 8 additions & 11 deletions src/main/java/com/mycom/socket/auth/jwt/JWTFilter.java
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,10 @@ protected void doFilterInternal(HttpServletRequest request,
HttpServletResponse response,
FilterChain filterChain) throws ServletException, IOException {
try {
String token = resolveTokenFromCookie(request);
if (StringUtils.hasText(token) && jwtUtil.validateToken(token, "ACCESS_TOKEN")) {
setAuthentication(token);
// Bearer 토큰 확인
String bearerToken = resolveTokenFromHeader(request);
if (StringUtils.hasText(bearerToken) && jwtUtil.validateToken(bearerToken, "ACCESS_TOKEN")) {
setAuthentication(bearerToken);
Copy link

Choose a reason for hiding this comment

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

πŸ› οΈ Refactor suggestion

JWT 토큰 처리 둜직 λ³΄μ•ˆ κ°•ν™”κ°€ ν•„μš”ν•©λ‹ˆλ‹€.

ν˜„μž¬ κ΅¬ν˜„μ—μ„œ λ‹€μŒκ³Ό 같은 λ³΄μ•ˆ κ΄€λ ¨ κ°œμ„ μ‚¬ν•­μ΄ ν•„μš”ν•©λ‹ˆλ‹€:

  • Bearer 접두사 μƒμˆ˜ν™”
  • 토큰 ν˜•μ‹ 검증 κ°•ν™”
  • ꡬ체적인 μ˜ˆμ™Έ 처리

λ‹€μŒκ³Ό 같이 κ°œμ„ μ„ μ œμ•ˆλ“œλ¦½λ‹ˆλ‹€:

+    private static final String BEARER_PREFIX = "Bearer ";
+    private static final String TOKEN_TYPE = "ACCESS_TOKEN";
+
     try {
-        // Bearer 토큰 확인
         String bearerToken = resolveTokenFromHeader(request);
-        if (StringUtils.hasText(bearerToken) && jwtUtil.validateToken(bearerToken, "ACCESS_TOKEN")) {
+        if (isValidBearerToken(bearerToken)) {
             setAuthentication(bearerToken);
         }
     } catch (Exception e) {
private boolean isValidBearerToken(String token) {
    return StringUtils.hasText(token) &&
           token.matches("^[A-Za-z0-9-_=]+\\.[A-Za-z0-9-_=]+\\.?[A-Za-z0-9-_.+/=]*$") &&
           jwtUtil.validateToken(token, TOKEN_TYPE);
}

}
} catch (Exception e) {
log.warn("인증 처리 μ‹€νŒ¨", e);
Expand All @@ -42,14 +43,10 @@ protected void doFilterInternal(HttpServletRequest request,
filterChain.doFilter(request, response);
}

private String resolveTokenFromCookie(HttpServletRequest request) {
Cookie[] cookies = request.getCookies();
if (cookies != null) {
for (Cookie cookie : cookies) {
if (jwtProperties.getAccessTokenCookieName().equals(cookie.getName())) {
return cookie.getValue();
}
}
private String resolveTokenFromHeader(HttpServletRequest request) {
String bearerToken = request.getHeader("Authorization");
if (StringUtils.hasText(bearerToken) && bearerToken.startsWith("Bearer ")) {
return bearerToken.substring(7);
}
Copy link

Choose a reason for hiding this comment

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

πŸ› οΈ Refactor suggestion

토큰 μΆ”μΆœ λ©”μ„œλ“œμ˜ λ³΄μ•ˆμ„± κ°•ν™”κ°€ ν•„μš”ν•©λ‹ˆλ‹€.

토큰 μΆ”μΆœ μ‹œ λ‹€μŒκ³Ό 같은 λ³΄μ•ˆ 검증이 μΆ”κ°€λ˜μ–΄μ•Ό ν•©λ‹ˆλ‹€:

  • 헀더 쑴재 μ—¬λΆ€ 검증
  • 토큰 길이 μ œν•œ
  • XSS λ°©μ§€λ₯Ό μœ„ν•œ λ¬Έμžμ—΄ μ΄μŠ€μΌ€μ΄ν”„

λ‹€μŒκ³Ό 같이 κ°œμ„ μ„ μ œμ•ˆλ“œλ¦½λ‹ˆλ‹€:

     private String resolveTokenFromHeader(HttpServletRequest request) {
         String bearerToken = request.getHeader("Authorization");
-        if (StringUtils.hasText(bearerToken) && bearerToken.startsWith("Bearer ")) {
-            return bearerToken.substring(7);
+        if (!StringUtils.hasText(bearerToken)) {
+            return null;
+        }
+        if (!bearerToken.startsWith(BEARER_PREFIX)) {
+            log.warn("잘λͺ»λœ 토큰 ν˜•μ‹: Bearer 접두사 μ—†μŒ");
+            return null;
+        }
+        String token = bearerToken.substring(BEARER_PREFIX.length());
+        if (token.length() > 1000) {  // μ΅œλŒ€ 토큰 길이 μ œν•œ
+            log.warn("토큰 길이 초과");
+            return null;
         }
-        return null;
+        return HtmlUtils.htmlEscape(token);
     }

Committable suggestion skipped: line range outside the PR's diff.

return null;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
package com.mycom.socket.go_socket.controller;

import com.mycom.socket.auth.security.MemberDetails;
import com.mycom.socket.go_socket.dto.response.ProfileResponse;
import lombok.RequiredArgsConstructor;
import org.springframework.security.core.annotation.AuthenticationPrincipal;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.RestController;

@RestController
@RequiredArgsConstructor
@RequestMapping("/api/profile")
public class ProfileController {

@GetMapping
public ProfileResponse getProfile(@AuthenticationPrincipal MemberDetails memberDetails) {
return ProfileResponse.of(memberDetails.getMember());
}
Comment on lines +16 to +19
Copy link

Choose a reason for hiding this comment

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

πŸ› οΈ Refactor suggestion

μ»¨νŠΈλ‘€λŸ¬μ— μ˜ˆμ™Έ μ²˜λ¦¬μ™€ API λ¬Έμ„œν™”κ°€ ν•„μš”ν•©λ‹ˆλ‹€.

λ‹€μŒ 사항듀이 λˆ„λ½λ˜μ–΄ μžˆμŠ΅λ‹ˆλ‹€:

  • 응닡 μƒνƒœ μ½”λ“œ λͺ…μ‹œ
  • Swagger/OpenAPI λ¬Έμ„œν™”
  • μ˜ˆμ™Έ 처리
  • λ‘œκΉ…

λ‹€μŒκ³Ό 같이 κ°œμ„ μ„ μ œμ•ˆλ“œλ¦½λ‹ˆλ‹€:

+    private final Logger log = LoggerFactory.getLogger(ProfileController.class);
+
+    /**
+     * ν˜„μž¬ 인증된 μ‚¬μš©μžμ˜ ν”„λ‘œν•„ 정보λ₯Ό μ‘°νšŒν•©λ‹ˆλ‹€.
+     *
+     * @param memberDetails 인증된 μ‚¬μš©μž 정보
+     * @return μ‚¬μš©μž ν”„λ‘œν•„ 정보
+     */
     @GetMapping
+    @ResponseStatus(HttpStatus.OK)
+    @Operation(summary = "ν”„λ‘œν•„ 쑰회", description = "ν˜„μž¬ λ‘œκ·ΈμΈν•œ μ‚¬μš©μžμ˜ ν”„λ‘œν•„ 정보λ₯Ό μ‘°νšŒν•©λ‹ˆλ‹€.")
+    @ApiResponse(responseCode = "200", description = "ν”„λ‘œν•„ 쑰회 성곡")
+    @ApiResponse(responseCode = "401", description = "μΈμ¦λ˜μ§€ μ•Šμ€ μ‚¬μš©μž")
     public ProfileResponse getProfile(@AuthenticationPrincipal MemberDetails memberDetails) {
+        log.debug("ν”„λ‘œν•„ 쑰회 μš”μ²­: {}", memberDetails.getUsername());
+        if (memberDetails == null) {
+            throw new UnauthorizedException("인증 정보가 μ—†μŠ΅λ‹ˆλ‹€.");
+        }
         return ProfileResponse.of(memberDetails.getMember());
     }

Committable suggestion skipped: line range outside the PR's diff.

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
package com.mycom.socket.go_socket.dto.response;

import com.mycom.socket.go_socket.entity.Member;

public record ProfileResponse(
String email,
String nickname,
String intro
) {
public static ProfileResponse of(Member member) {
return new ProfileResponse(
member.getEmail(),
member.getNickname(),
member.getIntro()
);
}
Comment on lines +10 to +16
Copy link

Choose a reason for hiding this comment

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

πŸ› οΈ Refactor suggestion

νŒ©ν† λ¦¬ λ©”μ„œλ“œμ— μœ νš¨μ„± 검사와 λ¬Έμ„œν™”κ°€ ν•„μš”ν•©λ‹ˆλ‹€.

of λ©”μ„œλ“œμ— λ‹€μŒ 사항듀이 λˆ„λ½λ˜μ–΄ μžˆμŠ΅λ‹ˆλ‹€:

  • null 체크
  • JavaDoc λ¬Έμ„œν™”
  • λ―Όκ°ν•œ 정보 λ…ΈμΆœ κ°€λŠ₯μ„± κ²€ν† 

λ‹€μŒκ³Ό 같이 κ°œμ„ μ„ μ œμ•ˆλ“œλ¦½λ‹ˆλ‹€:

+    /**
+     * Member μ—”ν‹°ν‹°λ‘œλΆ€ν„° ProfileResponseλ₯Ό μƒμ„±ν•©λ‹ˆλ‹€.
+     *
+     * @param member ν”„λ‘œν•„ 정보λ₯Ό κ°€μ Έμ˜¬ νšŒμ› μ—”ν‹°ν‹° (null이 μ•„λ‹ˆμ–΄μ•Ό 함)
+     * @return μƒμ„±λœ ProfileResponse 객체
+     * @throws IllegalArgumentException memberκ°€ null인 경우
+     */
     public static ProfileResponse of(Member member) {
+        if (member == null) {
+            throw new IllegalArgumentException("Member must not be null");
+        }
+        
         return new ProfileResponse(
                 member.getEmail(),
                 member.getNickname(),
                 member.getIntro()
         );
     }
πŸ“ 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.

Suggested change
public static ProfileResponse of(Member member) {
return new ProfileResponse(
member.getEmail(),
member.getNickname(),
member.getIntro()
);
}
/**
* Member μ—”ν‹°ν‹°λ‘œλΆ€ν„° ProfileResponseλ₯Ό μƒμ„±ν•©λ‹ˆλ‹€.
*
* @param member ν”„λ‘œν•„ 정보λ₯Ό κ°€μ Έμ˜¬ νšŒμ› μ—”ν‹°ν‹° (null이 μ•„λ‹ˆμ–΄μ•Ό 함)
* @return μƒμ„±λœ ProfileResponse 객체
* @throws IllegalArgumentException memberκ°€ null인 경우
*/
public static ProfileResponse of(Member member) {
if (member == null) {
throw new IllegalArgumentException("Member must not be null");
}
return new ProfileResponse(
member.getEmail(),
member.getNickname(),
member.getIntro()
);
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package com.mycom.socket.go_socket.service;

import com.mycom.socket.go_socket.repository.MemberRepository;
import lombok.RequiredArgsConstructor;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;

@Service
@RequiredArgsConstructor
@Transactional(readOnly = true)
public class MemberService {

private final MemberRepository memberRepository;
}
Comment on lines +11 to +14
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

μ„œλΉ„μŠ€ ν΄λž˜μŠ€μ— ν•„μš”ν•œ λ©”μ„œλ“œ κ΅¬ν˜„μ΄ λˆ„λ½λ˜μ—ˆμŠ΅λ‹ˆλ‹€.

ν˜„μž¬ MemberService ν΄λž˜μŠ€κ°€ λΉ„μ–΄μžˆμŠ΅λ‹ˆλ‹€. ν”„λ‘œν•„ 쑰회 κΈ°λŠ₯을 μœ„ν•΄ λ‹€μŒκ³Ό 같은 λ©”μ„œλ“œλ“€μ˜ κ΅¬ν˜„μ΄ ν•„μš”ν•©λ‹ˆλ‹€:

  • νšŒμ› ν”„λ‘œν•„ 쑰회
  • νšŒμ› 쑴재 μ—¬λΆ€ 확인
  • μ˜ˆμ™Έ 처리 둜직

λ‹€μŒκ³Ό 같은 λ©”μ„œλ“œ κ΅¬ν˜„μ„ μ œμ•ˆλ“œλ¦½λ‹ˆλ‹€:

 @Service
 @RequiredArgsConstructor
 @Transactional(readOnly = true)
 public class MemberService {
     private final MemberRepository memberRepository;
+    
+    public Member getMemberByEmail(String email) {
+        return memberRepository.findByEmail(email)
+            .orElseThrow(() -> new EntityNotFoundException("νšŒμ›μ„ 찾을 수 μ—†μŠ΅λ‹ˆλ‹€."));
+    }
+    
+    public boolean existsByEmail(String email) {
+        return memberRepository.existsByEmail(email);
+    }
 }

Committable suggestion skipped: line range outside the PR's diff.

2 changes: 1 addition & 1 deletion src/main/resources/yaml/application-dev.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,4 @@ spring:
jpa:
show-sql: true
hibernate:
ddl-auto: create
ddl-auto: update
Loading