Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,6 @@ public class CustomOAuth2UserService implements OAuth2UserService <OAuth2UserReq
private final UserRepository userRepository;
private final UserOauthAccountRepository oauthAccountRepository;



@Override
public OAuth2User loadUser(OAuth2UserRequest req) throws OAuth2AuthenticationException {
// log.info("[CustomOAuth2UserService] loadUser START");
Expand All @@ -47,7 +45,6 @@ public OAuth2User loadUser(OAuth2UserRequest req) throws OAuth2AuthenticationExc
String email;
String name;


// log.info("[OAuth2] Provider = {}", provider);
if (log.isDebugEnabled()) {
log.debug("[OAuth2] Attributes = {}", attrs);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ public OidcUser loadUser(OidcUserRequest req) throws OAuth2AuthenticationExcepti
.build();
oauthAccountRepository.save(oauth);

// log.info("[CustomOidcUserService] 신규 User 및 UserOauthAccount 생성됨");
// log.info("[CustomOidcUserService] 신규 User 및 UserOauthAccount 생성됨");
return savedUser;
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,15 @@
import com.fasterxml.jackson.databind.ObjectMapper;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.springframework.core.env.Environment;
import org.sejongisc.backend.auth.dao.UserOauthAccountRepository;
import org.sejongisc.backend.auth.entity.AuthProvider;
import org.sejongisc.backend.auth.entity.UserOauthAccount;
import org.sejongisc.backend.common.auth.jwt.JwtProvider;
import org.sejongisc.backend.auth.service.RefreshTokenService;
import org.sejongisc.backend.user.dao.UserRepository;
import org.sejongisc.backend.user.entity.User;
import org.springframework.beans.factory.annotation.Value;
import org.springframework.http.HttpHeaders;
import org.springframework.http.ResponseCookie;
import org.springframework.security.core.Authentication;
Expand All @@ -26,6 +28,7 @@
import java.io.IOException;
import java.net.URLEncoder;
import java.nio.charset.StandardCharsets;
import java.util.Arrays;
import java.util.Map;
import java.util.UUID;

Expand All @@ -38,6 +41,10 @@ public class OAuth2SuccessHandler extends SimpleUrlAuthenticationSuccessHandler
private final RefreshTokenService refreshTokenService;
private final UserRepository userRepository;
private final UserOauthAccountRepository userOauthAccountRepository;
private final Environment env;

@Value("${app.oauth2.redirect-success}")
private String redirectSuccessBase;
Comment on lines +44 to +47
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

환경별 설정을 위한 필드 추가를 확인하세요.

Environment@Value 주입이 적절하게 추가되었습니다. 다만 app.oauth2.redirect-success 속성이 모든 환경 설정 파일(application-local.yml, application-prod.yml 등)에 정의되어 있는지 확인이 필요합니다.

다음 스크립트로 속성 정의를 확인하세요:


🏁 Script executed:

#!/bin/bash
# Description: Verify that app.oauth2.redirect-success property is defined in all environment config files

# Search for the property definition in YAML files
rg -n "redirect-success" --type=yaml

Length of output: 0



🏁 Script executed:

#!/bin/bash
# Find all application config files
echo "=== Searching for application config files ==="
fd -e yml -e yaml -e properties | head -20

echo "=== Searching for any oauth2 configuration ==="
rg -n "oauth2" --type=yaml --type=properties -i

echo "=== Checking Java config files for OAuth2 ==="
rg -n "oauth2|redirect" --type=java -i -C2 | head -50

Length of output: 5925


🏁 Script executed:

#!/bin/bash
# Verify if the field will cause runtime failures
echo "=== Checking for Spring @Value usage patterns in the file ==="
cat backend/src/main/java/org/sejongisc/backend/common/auth/config/OAuth2SuccessHandler.java | grep -n "@Value" -A2 -B2

Length of output: 198


🏁 Script executed:

#!/bin/bash
# Search for any configuration classes or properties
echo "=== Looking for application configuration files ==="
find . -name "application*.yml" -o -name "application*.yaml" -o -name "application*.properties" 2>/dev/null | head -20

Length of output: 96


🏁 Script executed:

#!/bin/bash
# Find all application config files and search for OAuth2/redirect configuration
echo "=== All application config files ==="
fd "application.*\.(yml|yaml|properties)$"

echo "=== Search for oauth2 configuration anywhere ==="
rg -i "oauth2|redirect-success" -C2

echo "=== Check the OAuth2SuccessHandler file for @Value annotations ==="
fd "OAuth2SuccessHandler.java$" -x cat {} | grep -n "@Value" -A1 -B1

Length of output: 31566


🏁 Script executed:

#!/bin/bash
# Verify Spring @Value behavior with missing properties
echo "=== Understanding @Value with missing property ==="
# When Spring finds @Value with a property that doesn't exist, it throws:
# BeanCreationException or IllegalArgumentException at startup
echo "Missing @Value properties cause startup failure in Spring"

Length of output: 108


app.oauth2.redirect-success 속성이 설정 파일에 정의되지 않았습니다. 애플리케이션 시작 실패를 유발합니다.

backend/src/main/resources/application.ymlapp.oauth2.redirect-success 속성이 정의되어 있지 않습니다. Spring의 @Value 주입은 해당 속성이 존재하지 않을 경우 시작 시점에 BeanCreationException을 발생시켜 애플리케이션이 구동되지 않습니다.

application.yml에 다음을 추가하세요:

app:
  oauth2:
    redirect-success: <실제 리다이렉트 URL>
🤖 Prompt for AI Agents
In
backend/src/main/java/org/sejongisc/backend/common/auth/config/OAuth2SuccessHandler.java
around lines 44 to 47, the @Value("${app.oauth2.redirect-success}") injection
fails at startup because the app.oauth2.redirect-success property is missing;
add the property to backend/src/main/resources/application.yml under app.oauth2
with an appropriate redirect URL (e.g., set app.oauth2.redirect-success:
<your-redirect-url>) so Spring can inject the value and the application can
start successfully.


@Override
public void onAuthenticationSuccess(
Expand Down Expand Up @@ -94,36 +101,46 @@ public void onAuthenticationSuccess(
// 5. RefreshToken 저장(DB or Redis)
refreshTokenService.saveOrUpdateToken(user.getUserId(), refreshToken);

boolean isProd = Arrays.asList(env.getActiveProfiles()).contains("prod");

String sameSite = isProd ? "None" : "Lax";
boolean secure = isProd;
String domain = isProd ? "sisc-web.duckdns.org" : "localhost";
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

도메인을 하드코딩하지 말고 설정으로 외부화하세요.

도메인 "sisc-web.duckdns.org"가 하드코딩되어 있습니다. 환경이나 도메인이 변경될 때 코드 수정이 필요하므로, application.yml에 속성으로 정의하는 것이 좋습니다.

예시:

+    @Value("${app.oauth2.cookie-domain:localhost}")
+    private String cookieDomain;
+
     @Override
     public void onAuthenticationSuccess(...) {
-        String domain = isProd ? "sisc-web.duckdns.org" : "localhost";
+        String domain = isProd ? cookieDomain : "localhost";

또는 환경별 설정 파일에서:

# application-prod.yml
app:
  oauth2:
    cookie-domain: sisc-web.duckdns.org
🤖 Prompt for AI Agents
In
backend/src/main/java/org/sejongisc/backend/common/auth/config/OAuth2SuccessHandler.java
around line 108 the domain "sisc-web.duckdns.org" is hardcoded; externalize it
by adding a property (e.g. app.oauth2.cookie-domain) to
application.yml/application-prod.yml and read it in the class (via
@Value("${app.oauth2.cookie-domain}") or a @ConfigurationProperties bean) with a
sensible default or profile-specific values; replace the hardcoded literal with
the injected property so domain changes are driven by configuration, not code.



// 6. HttpOnly 쿠키로 refreshToken 저장
ResponseCookie accessCookie = ResponseCookie.from("access", accessToken)
.httpOnly(true)
.secure(false) // 로컬 개발
.sameSite("Lax") // 로컬에서는 None 비추천
.secure(secure) // 로컬=false, 배포=true
.sameSite(sameSite) // 로컬= "Lax", 배포="None"
.path("/")
.maxAge(60L * 60) // 1 hour
.build();

ResponseCookie refreshCookie = ResponseCookie.from("refresh", refreshToken)
.httpOnly(true)
.secure(false)
.sameSite("Lax")
.secure(secure)
.sameSite(sameSite)
.path("/")
.maxAge(60L * 60 * 24 * 14) // 2 weeks
.build();
Comment on lines 112 to 126
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

쿠키에 domain 속성이 설정되지 않았습니다.

Line 108에서 domain 변수를 계산했지만, 쿠키 생성 시 실제로 적용하지 않았습니다. 이로 인해 쿠키가 의도한 도메인에서 작동하지 않을 수 있습니다.

다음 diff를 적용하여 수정하세요:

         ResponseCookie accessCookie = ResponseCookie.from("access", accessToken)
                 .httpOnly(true)
                 .secure(secure)    // 로컬=false, 배포=true
                 .sameSite(sameSite)  // 로컬= "Lax", 배포="None"
                 .path("/")
+                .domain(domain)
                 .maxAge(60L * 60)  // 1 hour
                 .build();
 
         ResponseCookie refreshCookie = ResponseCookie.from("refresh", refreshToken)
                 .httpOnly(true)
                 .secure(secure)
                 .sameSite(sameSite)
                 .path("/")
+                .domain(domain)
                 .maxAge(60L * 60 * 24 * 14) // 2 weeks
                 .build();
🤖 Prompt for AI Agents
In
backend/src/main/java/org/sejongisc/backend/common/auth/config/OAuth2SuccessHandler.java
around lines 112 to 126, the ResponseCookie builders for access and refresh
cookies omit the domain setting calculated earlier; update both builders to call
.domain(domain) so the cookies are scoped to the intended domain (ensure the
domain variable is correctly computed/non-null before use, or conditionally
apply .domain(domain) only when valid).



response.addHeader(HttpHeaders.SET_COOKIE, accessCookie.toString());
response.addHeader(HttpHeaders.SET_COOKIE, refreshCookie.toString());


// 7. 프론트로 redirect
String redirectUrl = "http://localhost:5173/oauth/success";
// application-local.yml → http://localhost:5173/oauth/success
// application-prod.yml → https://sisc-web.duckdns.org/oauth/success
//String redirectUrl = redirectSuccessBase;
// + "?accessToken=" + accessToken
// + "&name=" + URLEncoder.encode(name, StandardCharsets.UTF_8)
// + "&userId=" + userId;

// log.info("[OAuth2 Redirect] {}", redirectUrl);

getRedirectStrategy().sendRedirect(request, response, redirectUrl);
getRedirectStrategy().sendRedirect(request, response, redirectSuccessBase);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -111,12 +111,13 @@ public SecurityFilterChain securityFilterChain(HttpSecurity http) throws Excepti
public CorsConfigurationSource corsConfigurationSource() {
CorsConfiguration config = new CorsConfiguration();
config.setAllowedOrigins(List.of(
"http://localhost:5173" // 허용할 프론트 주소
"http://localhost:5173", // 허용할 프론트 주소
"https://sisc-web.duckdns.org"
));
config.setAllowedOriginPatterns(List.of("*"));
config.setAllowedMethods(List.of("GET", "POST", "PUT", "DELETE", "PATCH", "OPTIONS"));
config.setAllowedHeaders(List.of("*"));
config.setAllowCredentials(true);
config.addExposedHeader("Authorization");
config.setMaxAge(3600L); // 캐시 시간(초)

UrlBasedCorsConfigurationSource source = new UrlBasedCorsConfigurationSource();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import io.jsonwebtoken.JwtException;
import jakarta.servlet.FilterChain;
import jakarta.servlet.ServletException;
import jakarta.servlet.http.Cookie;
import jakarta.servlet.http.HttpServletRequest;
import jakarta.servlet.http.HttpServletResponse;
import jakarta.validation.constraints.NotNull;
Expand Down Expand Up @@ -40,15 +41,15 @@ public class JwtAuthenticationFilter extends OncePerRequestFilter {
"/api/auth/signup",
"/api/auth/login",
"/api/auth/login/**",
"/api/auth/oauth",
"/api/auth/oauth/**",
"/api/auth/logout",
"/api/auth/reissue",
"/v3/api-docs/**",
"/swagger-ui/**",
"/swagger-ui/index.html",
"/swagger-resources/**",
"/webjars/**"
"/webjars/**",
"/login/**",
"/oauth2/**"
);
Comment on lines +50 to 53
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

쿠키 기반 access 토큰 허용에 따른 CSRF 리스크 점검 필요

이제 Authorization 헤더가 없을 때 "access" HttpOnly 쿠키의 JWT로도 인증이 되는데, 현재 설정을 보면:

  • SecurityConfig에서 CSRF가 disable 상태이고,
  • prod에서 SameSite = "None", secure = true 로 설정되어,
  • 브라우저는 다른 사이트에서 발생한 요청에도 자동으로 이 쿠키를 첨부합니다.

이 조합이면 공격자가 별도 토큰 탈취 없이도 사용자의 브라우저를 이용해 상태 변경 요청(POST/PUT/DELETE 등)을 보내도록 유도하는 전형적인 CSRF 공격 가능성이 생깁니다.

옵션 예시입니다.

  1. 가장 확실한 방법

    • 쿠키 기반 access 토큰을 유지할 거라면,
      • state-changing 엔드포인트에 대해 CSRF 토큰(더블 서브밋, 헤더+쿠키 패턴 등)을 도입하거나,
      • 최소한 특정 경로(ex: /oauth2/** 이후 프론트 초기 진입)에서만 쿠키 토큰을 허용하고, 나머지는 여전히 Authorization 헤더만 허용하도록 분리하는 방식을 검토해 주세요.
  2. 단기 완화책

    • 당장 구조 변경이 어렵다면,
      • 우선 resolveTokenFromCookie 사용을 로그인 직후 특정 redirect 처리 등 정말 필요한 최소 경로로 한정하고,
      • 일반 API 호출에서는 여전히 Authorization 헤더만 허용하는 쪽으로 조정할 수 있습니다.

현재 변경은 보안 모델을 꽤 크게 바꾸는 부분이라, 실제 운영 전 보안 관점에서 한 번 더 논의/검증하는 것을 강하게 추천드립니다.

Also applies to: 77-80, 130-141

🤖 Prompt for AI Agents
In
backend/src/main/java/org/sejongisc/backend/common/auth/springsecurity/JwtAuthenticationFilter.java
around lines 50-53 (and also review similar logic at 77-80 and 130-141), the new
logic that resolves access tokens from an HttpOnly "access" cookie exposes CSRF
risk because CSRF is disabled and SameSite=None with secure=true will send the
cookie on cross-site requests; to fix this either: 1) implement CSRF protection
for state-changing endpoints (e.g., double-submit cookie or header+cookie token
validation) before accepting cookie-based authentication, or 2) restrict
cookie-based token resolution to a very small set of safe endpoints (for example
only on the initial OAuth2 redirect/SPA bootstrap path) and keep all ordinary
API endpoints requiring Authorization header only; as a short-term mitigation,
limit resolveTokenFromCookie usage to post-login redirect handling and ensure
server sets stricter cookie attributes (SameSite=Lax/Strict where feasible) and
document the change for a security review before production rollout.


@Override
Expand All @@ -73,6 +74,10 @@ protected void doFilterInternal(@NotNull HttpServletRequest request,
try {
String token = resolveToken(request);

if (token == null) {
token = resolveTokenFromCookie(request);
}

if (token != null && jwtParser.validationToken(token) ) {
UsernamePasswordAuthenticationToken authentication = jwtParser.getAuthentication(token);
SecurityContextHolder.getContext().setAuthentication(authentication);
Expand Down Expand Up @@ -122,6 +127,19 @@ private String resolveToken(HttpServletRequest request) {
return null;
}

private String resolveTokenFromCookie(HttpServletRequest request) {
if (request.getCookies() == null) return null;

for (Cookie cookie : request.getCookies()) {
if ("access".equals(cookie.getName())) {
log.info("쿠키에서 access token 추출됨");
return cookie.getValue();
}
}

return null;
}

private String toJson(ErrorResponse errorResponse) throws JsonProcessingException {
ObjectMapper mapper = new ObjectMapper();
mapper.registerModule(new JavaTimeModule());
Expand Down