Skip to content

Conversation

@sapiens2000
Copy link
Collaborator

@sapiens2000 sapiens2000 commented Apr 2, 2025

📌 PR 제목

리프레시 토큰을 활용해 토큰 재발급 시 발생하는 오류 수정

✨ 변경 사항

리프레시 토큰 엔티티 생성할 때 id 값을 잘못 넣어서 오류가 발생해 id값 넣지 않도록 수정했음

🔍 변경 이유

리프레시 토큰 재발급 시 발생하는 오류 수정

✅ 체크리스트

  • 코드가 정상적으로 동작하는지 확인
  • 관련 테스트 코드 작성 및 통과 여부 확인

@github-actions
Copy link

github-actions bot commented Apr 2, 2025

Claude의 전체 변경사항 및 관련 파일에 대한 리뷰:

개선된 사항:

  • TokenConstants 클래스를 사용하여 문자열 상수를 관리하고 있습니다.
  • RefreshTokenService를 통해 리프레시 토큰 관련 로직을 분리하여 관심사 분리를 잘 구현했습니다.
  • @transactional 어노테이션을 사용하여 데이터 일관성을 보장하고 있습니다.

주요 이슈:

  1. 보안 취약점: 쿠키 설정에서 보안 관련 옵션이 누락되어 있습니다.

    • 제안: createCookie 메소드에서 보안 관련 옵션을 추가하세요.
    private Cookie createCookie(String key, String value) {
        Cookie cookie = new Cookie(key, value);
        cookie.setMaxAge(60 * 60 * 60);
        cookie.setSecure(true); // HTTPS에서만 전송
        cookie.setPath("/");
        cookie.setHttpOnly(true);
        cookie.setSameSite("Strict"); // CSRF 방지
        return cookie;
    }
  2. 에러 처리: 쿠키가 null일 경우 NPE가 발생할 수 있습니다.

    • 제안: 쿠키 배열이 null인 경우를 처리하세요.
    Cookie[] cookies = request.getCookies();
    if (cookies != null) {
        for (Cookie cookie : cookies) {
            // 기존 코드
        }
    } else {
        return new ResponseEntity<>("쿠키가 없습니다.", HttpStatus.BAD_REQUEST);
    }
  3. 토큰 만료 시간: 액세스 토큰과 리프레시 토큰의 만료 시간이 동일합니다.

    • 제안: 리프레시 토큰의 만료 시간을 더 길게 설정하세요.
    String newAccessToken = jwtUtil.createJwt(TokenConstants.ACCESS_TOKEN, userId, name, role, 600000L);
    String newRefreshToken = jwtUtil.createJwt(TokenConstants.REFRESH_TOKEN, userId, name, role, 3600000L); // 1시간으로 설정

관련 파일에 대한 영향 분석:

  • OAuth2AuthenticationSuccessHandler: RefreshTokenService의 변경으로 인해 saveRefreshToken 메소드 호출 부분을 수정해야 할 수 있습니다.
  • RefreshTokenRepository: RefreshToken 엔티티의 변경으로 인해 쿼리 메소드를 수정해야 할 수 있습니다.

전반적인 의견:
코드 품질이 전반적으로 향상되었으며, 보안과 에러 처리에 조금 더 신경 쓰면 더욱 견고한 시스템이 될 것 같습니다. 토큰 관리와 쿠키 설정에 대한 추가적인 보안 강화가 필요해 보입니다.

@sonarqubecloud
Copy link

sonarqubecloud bot commented Apr 2, 2025

Quality Gate Failed Quality Gate failed

Failed conditions
2 Security Hotspots
B Maintainability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

@github-actions
Copy link

github-actions bot commented Apr 2, 2025

Claude의 전체 변경사항 및 관련 파일에 대한 리뷰:

개선된 사항:

  • 토큰 관련 상수를 TokenConstants 클래스로 분리하여 코드의 일관성과 유지보수성이 향상되었습니다.
  • RefreshToken 엔티티의 refresh 필드에 unique 제약 조건을 추가하여 데이터 무결성이 개선되었습니다.
  • RefreshTokenService에 deleteRefreshToken 메서드를 추가하여 토큰 관리 기능이 강화되었습니다.

주요 이슈:

  1. OAuth2Controller의 쿠키 처리 로직 개선 필요

    • 제안: 쿠키 처리를 위한 별도의 유틸리티 메서드를 만들어 코드 중복을 줄이고 가독성을 높입니다.
    private String getTokenFromCookie(HttpServletRequest request, String tokenName) {
        Cookie[] cookies = request.getCookies();
        if (cookies != null) {
            for (Cookie cookie : cookies) {
                if (cookie.getName().equals(tokenName)) {
                    return cookie.getValue();
                }
            }
        }
        return null;
    }
  2. RefreshTokenService의 saveRefreshToken 메서드 개선

    • 제안: 기존 토큰이 있는 경우 업데이트하도록 로직을 수정하여 중복 저장을 방지합니다.
    @Transactional
    public void saveRefreshToken(String name, String refresh) {
        Date expirationDate = new Date(System.currentTimeMillis() + refreshTokenValidityInSeconds);
        refreshTokenRepository.findByName(name)
            .ifPresentOrElse(
                token -> {
                    token.setRefresh(refresh);
                    token.setExpiration(expirationDate.toString());
                },
                () -> refreshTokenRepository.save(new RefreshToken(null, name, refresh, expirationDate.toString()))
            );
    }

관련 파일에 대한 영향 분석:

  • RefreshToken 엔티티의 변경으로 인해 RefreshTokenRepository와 관련 서비스 클래스들의 쿼리 메서드를 확인하고 필요시 수정해야 합니다.
  • TokenConstants 클래스의 도입으로 토큰 관련 상수를 사용하는 모든 클래스들(예: SecurityConfig, JwtAuthenticationFilter 등)을 업데이트해야 합니다.
  • RefreshTokenService의 변경으로 인해 이를 사용하는 OAuth2Controller와 OAuth2AuthenticationSuccessHandler의 호출 부분을 확인하고 필요시 수정해야 합니다.

전반적인 의견:
코드의 일관성과 보안성이 전반적으로 향상되었습니다. 토큰 관리 로직의 중앙화와 엔티티 설계 개선은 긍정적인 변화입니다. 다만, 쿠키 처리와 토큰 저장 로직에 대한 추가적인 개선이 필요해 보입니다.

@sapiens2000 sapiens2000 merged commit 57acba8 into develop Apr 2, 2025
3 of 4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants