Open
Conversation
- 통합 및 단위 테스트 코드 수정
- 통합 및 단위 테스트 코드 수정
- API 네이밍 변경 - 통합 및 단위 테스트 코드 수정 - Cookie 생성 메서드를 CookieUtils로 이동 - CookieFixture 추가
- OAuth2ClientValidationFilter 테스트 코드 수정
mono0801
requested review from
CUCU7103,
exjuu,
kimbro97,
kimjongsoo97 and
kmindev
1 task
Walkthrough새로운 Public OAuth 엔드포인트( Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant Controller as OAuthController
participant Facade as OAuthFacade
participant Service as OAuthService
participant Repo as RefreshTokenRepository
participant TokenProv as TokenProvider
Client->>Controller: POST /api/v1/auth/reissue (cookie: refreshToken)
Controller->>Facade: reissueToken(refreshToken)
Facade->>TokenProv: parseRefreshToken(refreshToken)
TokenProv-->>Facade: TokenClaims(userId, role)
Facade->>Service: refreshTokenValidate(refreshToken)
Service->>Repo: findByRefreshToken(refreshToken)
Repo-->>Service: RefreshToken entity (exists)
Facade->>Service: deleteRefreshToken(refreshToken)
Service->>Repo: deleteByRefreshToken(refreshToken)
Facade->>Service: generateRefreshToken(userId, role)
Service->>TokenProv: generateToken(claims, duration=REFRESH_TOKEN_DURATION)
TokenProv-->>Service: newRefreshToken
Facade->>Service: generateAccessToken(userId, role)
Service->>TokenProv: generateToken(claims, duration=ACCESS_TOKEN_DURATION)
TokenProv-->>Service: newAccessToken
Facade-->>Controller: TokenResponse(access, refresh)
Controller->>Client: 200 OK (Header: Authorization, Set-Cookie: refreshToken)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
시 🐰
🚥 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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/main/java/com/bbangle/bbangle/config/security/PublicApiPath.java (1)
9-9:⚠️ Potential issue | 🟡 Minor
/api/v1/oauth/**와일드카드는 보안 노출 범위가 지나치게 넓습니다현재
/api/v1/oauth/**는ANY_METHOD에 추가되어 해당 경로 하위의 모든 엔드포인트를 인증 없이 공개합니다. 현재 노출되어야 하는 엔드포인트가/api/v1/oauth/reissue와/api/v1/oauth/logout뿐이라면, 와일드카드 대신 구체적인 경로를 나열하는 것이 더 안전합니다. 향후/api/v1/oauth/하위에 인증이 필요한 엔드포인트가 추가될 경우, 이 와일드카드에 의해 의도치 않게 공개될 위험이 있습니다.🛡️ 더 좁은 경로로 수정 제안
- "/api/v1/oauth/**", + "/api/v1/oauth/reissue", + "/api/v1/oauth/logout",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/bbangle/bbangle/config/security/PublicApiPath.java` at line 9, The "/api/v1/oauth/**" wildcard in PublicApiPath (the list used for ANY_METHOD public endpoints) is too broad; replace the wildcard entry with the specific endpoints that should be public (e.g., "/api/v1/oauth/reissue" and "/api/v1/oauth/logout") by editing the collection/array in PublicApiPath where "/api/v1/oauth/**" appears so only those explicit paths are exposed without authentication. Ensure you update the constant/list used by the security configuration (the ANY_METHOD public endpoints) to remove the wildcard and add the two explicit paths.src/main/java/com/bbangle/bbangle/auth/seller/controller/SellerOauthController.java (1)
39-40: 🛠️ Refactor suggestion | 🟠 Major
Duration.ofDays(14)이OAuthService.REFRESH_TOKEN_DURATION과 중복되며, 다른 컨트롤러에서도 같은 문제가 있습니다.
OAuthService에 이미REFRESH_TOKEN_DURATION상수가 정의되어 있으므로,SellerOauthController와OAuthController에서 이 상수를 참조하면 유지보수 시 한 곳만 수정하면 됩니다. 현재 세 개의 서비스(OAuthService,AdminAuthService,CustomerOauthService)에서 같은 값으로 중복 정의되어 있으므로, 중앙 집중식 상수로 통합하는 것을 권장합니다.SellerOauthController 수정안
+import static com.bbangle.bbangle.auth.oauth.client.service.OAuthService.REFRESH_TOKEN_DURATION; ... - response.addHeader(HttpHeaders.SET_COOKIE, createCookie(dto.refreshToken(), Duration.ofDays(14)).toString()); + response.addHeader(HttpHeaders.SET_COOKIE, createCookie(dto.refreshToken(), REFRESH_TOKEN_DURATION).toString());🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/bbangle/bbangle/auth/seller/controller/SellerOauthController.java` around lines 39 - 40, 응답 쿠키에 하드코딩된 Duration.ofDays(14)이 여러 컨트롤러에 중복되어 있으므로 SellerOauthController와 OAuthController(및 AdminAuthService, CustomerOauthService에 있는 중복 정의)를 OAuthService에 정의된 REFRESH_TOKEN_DURATION 상수를 참조하도록 변경하세요; 즉, createCookie(dto.refreshToken(), Duration.ofDays(14)) 호출들을 createCookie(dto.refreshToken(), OAuthService.REFRESH_TOKEN_DURATION)로 바꾸고, 중복으로 정의된 상수들은 제거하거나 중앙상수로 통합하여 한 곳(OAuthService)에만 유지되게 하십시오.
🧹 Nitpick comments (6)
src/main/java/com/bbangle/bbangle/util/CookieUtils.java (2)
14-15:secure(true)를 로컬에서 수동으로 주석 처리해야 하는 TODO는 유지보수 위험 요소입니다개발자가 주석 처리를 잊거나 실수로
secure=false인 상태로 배포할 수 있습니다. Spring의 프로파일 기반 설정으로 관리하는 방식을 권장합니다.♻️ 프로파일 기반 secure 플래그 적용 예시
-public class CookieUtils { +public class CookieUtils { + + private CookieUtils() {} - public static ResponseCookie createCookie(String value, Duration duration) { + public static ResponseCookie createCookie(String value, Duration duration, boolean secure) { return ResponseCookie.from("refreshToken", value) .httpOnly(true) - // TODO : 로컬에서 사용할 때는 주석처리 - .secure(true) + .secure(secure) .path("/") .maxAge(duration) .sameSite("Strict") .build(); }호출부 (예:
OAuthController)에서는@Value("${server.ssl.enabled:true}")등으로 주입받아 사용:CookieUtils.createCookie(value, duration, secureEnabled);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/bbangle/bbangle/util/CookieUtils.java` around lines 14 - 15, The TODO to manually comment out .secure(true) is unsafe; change CookieUtils to drive the cookie secure flag from a Spring property instead of hardcoding it: add a configurable boolean (e.g., server.ssl.enabled or custom cookie.secure) injected into the component or passed into CookieUtils.createCookie, then replace the hardcoded .secure(true) with that property (use a default of true for production). Update callers (e.g., OAuthController) to pass the injected secure flag (or let CookieUtils read Environment/@Value) so local/dev profiles can override to false while prod remains true.
9-9: 유틸리티 클래스에private생성자 추가 필요
CookieFixture.java는 올바르게private생성자를 갖추었지만CookieUtils는 그렇지 않아 인스턴스 생성이 가능합니다.♻️ 수정 제안
public class CookieUtils { + + private CookieUtils() {} + public static ResponseCookie createCookie(String value, Duration duration) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/bbangle/bbangle/util/CookieUtils.java` at line 9, CookieUtils currently lacks a private constructor so it can be instantiated; add a private no-arg constructor to the CookieUtils class (similar to CookieFixture) to prevent instantiation — implement a private CookieUtils() { /* optional: throw new AssertionError("no instances"); */ } inside the CookieUtils class body.src/main/java/com/bbangle/bbangle/config/security/auth/CustomOAuth2AuthorizationRequestResolver.java (1)
19-21: OAuth 경로 문자열 상수화 고려
"/api/v1/oauth/authorization"이 현재CustomOAuth2AuthorizationRequestResolver(Line 20),OAuth2SecurityConfig(Line 40),OAuth2ClientValidationFilter(부분 경로/oauth/authorization/) 세 곳에 분산되어 하드코딩되어 있습니다. 경로가 변경될 경우 세 파일을 모두 수정해야 합니다.
OAuthApiPath같은 상수 클래스를 추가하고 공통 상수로 관리하는 방안을 고려해 주세요.♻️ 상수 추출 예시
새 상수 클래스 (예시):
// src/main/java/com/bbangle/bbangle/config/security/OAuthApiPath.java public class OAuthApiPath { public static final String AUTHORIZATION_BASE = "/api/v1/oauth/authorization"; }
CustomOAuth2AuthorizationRequestResolver.java:- clientRegistrationRepository, "/api/v1/oauth/authorization" + clientRegistrationRepository, OAuthApiPath.AUTHORIZATION_BASE
OAuth2SecurityConfig.java:- "/api/v1/oauth/authorization/**", + OAuthApiPath.AUTHORIZATION_BASE + "/**",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/bbangle/bbangle/config/security/auth/CustomOAuth2AuthorizationRequestResolver.java` around lines 19 - 21, The OAuth authorization path string is hardcoded in multiple places (CustomOAuth2AuthorizationRequestResolver, OAuth2SecurityConfig, OAuth2ClientValidationFilter); extract it into a shared constant class (e.g., create OAuthApiPath with public static final String AUTHORIZATION_BASE) and replace literal "/api/v1/oauth/authorization" occurrences with OAuthApiPath.AUTHORIZATION_BASE in CustomOAuth2AuthorizationRequestResolver (the DefaultOAuth2AuthorizationRequestResolver constructor call), OAuth2SecurityConfig (where the same path is used), and OAuth2ClientValidationFilter (where the partial path is checked), ensuring imports and references are updated accordingly.src/main/java/com/bbangle/bbangle/auth/oauth/client/service/OAuthService.java (1)
26-43: 신규 생성 경로에서token.update(refreshToken)호출이 불필요합니다.
RefreshToken.create(userId, role, refreshToken)로 이미 토큰 값이 설정된 상태에서 Line 38의update는 같은 값을 다시 설정합니다. 기존 토큰 업데이트 경로에서만 필요한 호출입니다.두 경로를 명시적으로 분리하면 의도가 더 명확해집니다:
제안된 수정
`@Transactional` public String generateRefreshToken(Long userId, Role role) { String refreshToken = tokenProvider.generateToken(userId, role, REFRESH_TOKEN_DURATION); - RefreshToken token = refreshTokenRepository + refreshTokenRepository .findByUserIdAndUserRole(userId, role) - .orElseGet(() -> RefreshToken.create( - userId, - role, - refreshToken - )); - - token.update(refreshToken); - - refreshTokenRepository.save(token); + .ifPresentOrElse( + existing -> { + existing.update(refreshToken); + refreshTokenRepository.save(existing); + }, + () -> refreshTokenRepository.save( + RefreshToken.create(userId, role, refreshToken) + ) + ); return refreshToken; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/bbangle/bbangle/auth/oauth/client/service/OAuthService.java` around lines 26 - 43, The generateRefreshToken method currently always calls token.update(refreshToken) even when a new RefreshToken was just created (via RefreshToken.create), causing a redundant assignment; change the logic so you fetch with refreshTokenRepository.findByUserIdAndUserRole(userId, role) and ifPresent call token.update(refreshToken) then save, otherwise create a new RefreshToken with RefreshToken.create(userId, role, refreshToken) and save it without calling update — i.e., separate the "existing token" and "new token" branches so token.update(...) is only invoked for existing tokens.src/test/java/com/bbangle/bbangle/auth/oauth/client/service/OAuthServiceUnitTest.java (1)
104-115:existToken테스트에서 mock 매처와 테스트 데이터 불일치.Line 111에서
findByRefreshToken(any())를 사용하고 있어,refreshToken변수의 실제 값("validRefreshToken")과 무관하게 mock이 동작합니다. 실제 값을 매칭하도록 수정하면 테스트 의도가 더 명확해집니다.제안된 수정
- given(refreshTokenRepository.findByRefreshToken(any())).willReturn(Optional.of(existing)); + given(refreshTokenRepository.findByRefreshToken(refreshToken)).willReturn(Optional.of(existing));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/com/bbangle/bbangle/auth/oauth/client/service/OAuthServiceUnitTest.java` around lines 104 - 115, The test existToken should use a precise mock matcher and aligned test data: change the repository stub to match the actual refreshToken variable (use eq(refreshToken) with given(refreshTokenRepository.findByRefreshToken(...))).willReturn(...)) and ensure the RefreshToken created via RefreshToken.create(...) uses the same token string as the refreshToken variable (replace "refreshToken" with "validRefreshToken") so the mocked findByRefreshToken and test data are consistent; update references in the test method existToken that call findByRefreshToken and RefreshToken.create accordingly.src/main/java/com/bbangle/bbangle/auth/oauth/client/controller/OAuthController.java (1)
69-69: 쿠키 삭제 시Duration.ZERO사용 권장
Duration.ofMillis(1)은Max-Age로 변환 시 0초로 잘려 쿠키를 삭제하는 효과를 냅니다. 의도를 명확히 하려면Duration.ZERO를 사용하는 것이 좋습니다.♻️ 제안 수정
- response.addHeader(HttpHeaders.SET_COOKIE, createCookie("", Duration.ofMillis(1)).toString()); + response.addHeader(HttpHeaders.SET_COOKIE, createCookie("", Duration.ZERO).toString());🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/bbangle/bbangle/auth/oauth/client/controller/OAuthController.java` at line 69, The response currently sets a deletion cookie using Duration.ofMillis(1) which is ambiguous; update the call in OAuthController where response.addHeader(HttpHeaders.SET_COOKIE, createCookie("", Duration.ofMillis(1)).toString()) to use Duration.ZERO instead (i.e., call createCookie("", Duration.ZERO)) so the intent to delete the cookie is explicit; ensure the change is made where createCookie and HttpHeaders.SET_COOKIE are used together in that controller.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/java/com/bbangle/bbangle/auth/oauth/client/facade/OAuthFacade.java`:
- Around line 17-31: 현재 reissueToken 메서드에서 oAuthService.deleteRefreshToken(...)
호출 후 새로운 토큰 생성(generateRefreshToken, generateAccessToken) 도중 실패하면 기존 토큰이 삭제되어 토큰
유실이 발생할 수 있으므로 reissueToken 메서드에 트랜잭션 경계를 추가하세요: reissueToken 메서드에
`@Transactional` 애너테이션을 적용해 tokenProvider.parseRefreshToken,
oAuthService.refreshTokenValidate, oAuthService.deleteRefreshToken,
oAuthService.generateRefreshToken 및 oAuthService.generateAccessToken 호출이 하나의 원자적
트랜잭션으로 실행되도록 하고, 필요한 경우 트랜잭션 전파 정책(예: REQUIRED)과 롤백 대상(예: Exception)을 설정하세요.
In
`@src/test/java/com/bbangle/bbangle/auth/oauth/client/controller/OAuthControllerTest.java`:
- Line 97: Update the outdated comment in OAuthControllerTest: replace the old
class name "OAuth2SellerFacade" with the current "OAuthFacade" (the line that
currently reads "// 쿠키가 없을 경우 OAuth2SellerFacade는 호출조차 하지 않음"). Ensure any other
nearby comments in OAuthControllerTest that reference OAuth2SellerFacade are
similarly updated to OAuthFacade so comments match the refactored code.
- Around line 117-130: The test in OAuthControllerTest.logout currently asserts
cookie expiration and response body but doesn't verify the controller called
oAuthService.deleteRefreshToken; after the mockMvc.perform(...) assertions add a
Mockito verification that oAuthService.deleteRefreshToken was invoked once with
the refresh token from CookieFixture.defaultCookie() (e.g.
then(oAuthService).should(times(1)).deleteRefreshToken("refreshToken") or the
exact token value returned by CookieFixture.defaultCookie()), so the server-side
token invalidation is asserted.
---
Outside diff comments:
In
`@src/main/java/com/bbangle/bbangle/auth/seller/controller/SellerOauthController.java`:
- Around line 39-40: 응답 쿠키에 하드코딩된 Duration.ofDays(14)이 여러 컨트롤러에 중복되어 있으므로
SellerOauthController와 OAuthController(및 AdminAuthService, CustomerOauthService에
있는 중복 정의)를 OAuthService에 정의된 REFRESH_TOKEN_DURATION 상수를 참조하도록 변경하세요; 즉,
createCookie(dto.refreshToken(), Duration.ofDays(14)) 호출들을
createCookie(dto.refreshToken(), OAuthService.REFRESH_TOKEN_DURATION)로 바꾸고, 중복으로
정의된 상수들은 제거하거나 중앙상수로 통합하여 한 곳(OAuthService)에만 유지되게 하십시오.
In `@src/main/java/com/bbangle/bbangle/config/security/PublicApiPath.java`:
- Line 9: The "/api/v1/oauth/**" wildcard in PublicApiPath (the list used for
ANY_METHOD public endpoints) is too broad; replace the wildcard entry with the
specific endpoints that should be public (e.g., "/api/v1/oauth/reissue" and
"/api/v1/oauth/logout") by editing the collection/array in PublicApiPath where
"/api/v1/oauth/**" appears so only those explicit paths are exposed without
authentication. Ensure you update the constant/list used by the security
configuration (the ANY_METHOD public endpoints) to remove the wildcard and add
the two explicit paths.
---
Nitpick comments:
In
`@src/main/java/com/bbangle/bbangle/auth/oauth/client/controller/OAuthController.java`:
- Line 69: The response currently sets a deletion cookie using
Duration.ofMillis(1) which is ambiguous; update the call in OAuthController
where response.addHeader(HttpHeaders.SET_COOKIE, createCookie("",
Duration.ofMillis(1)).toString()) to use Duration.ZERO instead (i.e., call
createCookie("", Duration.ZERO)) so the intent to delete the cookie is explicit;
ensure the change is made where createCookie and HttpHeaders.SET_COOKIE are used
together in that controller.
In
`@src/main/java/com/bbangle/bbangle/auth/oauth/client/service/OAuthService.java`:
- Around line 26-43: The generateRefreshToken method currently always calls
token.update(refreshToken) even when a new RefreshToken was just created (via
RefreshToken.create), causing a redundant assignment; change the logic so you
fetch with refreshTokenRepository.findByUserIdAndUserRole(userId, role) and
ifPresent call token.update(refreshToken) then save, otherwise create a new
RefreshToken with RefreshToken.create(userId, role, refreshToken) and save it
without calling update — i.e., separate the "existing token" and "new token"
branches so token.update(...) is only invoked for existing tokens.
In
`@src/main/java/com/bbangle/bbangle/config/security/auth/CustomOAuth2AuthorizationRequestResolver.java`:
- Around line 19-21: The OAuth authorization path string is hardcoded in
multiple places (CustomOAuth2AuthorizationRequestResolver, OAuth2SecurityConfig,
OAuth2ClientValidationFilter); extract it into a shared constant class (e.g.,
create OAuthApiPath with public static final String AUTHORIZATION_BASE) and
replace literal "/api/v1/oauth/authorization" occurrences with
OAuthApiPath.AUTHORIZATION_BASE in CustomOAuth2AuthorizationRequestResolver (the
DefaultOAuth2AuthorizationRequestResolver constructor call),
OAuth2SecurityConfig (where the same path is used), and
OAuth2ClientValidationFilter (where the partial path is checked), ensuring
imports and references are updated accordingly.
In `@src/main/java/com/bbangle/bbangle/util/CookieUtils.java`:
- Around line 14-15: The TODO to manually comment out .secure(true) is unsafe;
change CookieUtils to drive the cookie secure flag from a Spring property
instead of hardcoding it: add a configurable boolean (e.g., server.ssl.enabled
or custom cookie.secure) injected into the component or passed into
CookieUtils.createCookie, then replace the hardcoded .secure(true) with that
property (use a default of true for production). Update callers (e.g.,
OAuthController) to pass the injected secure flag (or let CookieUtils read
Environment/@Value) so local/dev profiles can override to false while prod
remains true.
- Line 9: CookieUtils currently lacks a private constructor so it can be
instantiated; add a private no-arg constructor to the CookieUtils class (similar
to CookieFixture) to prevent instantiation — implement a private CookieUtils() {
/* optional: throw new AssertionError("no instances"); */ } inside the
CookieUtils class body.
In
`@src/test/java/com/bbangle/bbangle/auth/oauth/client/service/OAuthServiceUnitTest.java`:
- Around line 104-115: The test existToken should use a precise mock matcher and
aligned test data: change the repository stub to match the actual refreshToken
variable (use eq(refreshToken) with
given(refreshTokenRepository.findByRefreshToken(...))).willReturn(...)) and
ensure the RefreshToken created via RefreshToken.create(...) uses the same token
string as the refreshToken variable (replace "refreshToken" with
"validRefreshToken") so the mocked findByRefreshToken and test data are
consistent; update references in the test method existToken that call
findByRefreshToken and RefreshToken.create accordingly.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (26)
src/main/java/com/bbangle/bbangle/auth/oauth/client/controller/OAuthController.javasrc/main/java/com/bbangle/bbangle/auth/oauth/client/controller/swagger/OAuthApi.javasrc/main/java/com/bbangle/bbangle/auth/oauth/client/facade/OAuthFacade.javasrc/main/java/com/bbangle/bbangle/auth/oauth/client/service/OAuthService.javasrc/main/java/com/bbangle/bbangle/auth/seller/controller/SellerOauthController.javasrc/main/java/com/bbangle/bbangle/auth/seller/controller/swagger/SellerOauthApi.javasrc/main/java/com/bbangle/bbangle/auth/seller/facade/OAuth2SellerFacade.javasrc/main/java/com/bbangle/bbangle/auth/seller/service/OAuthSellerService.javasrc/main/java/com/bbangle/bbangle/config/security/OAuth2SecurityConfig.javasrc/main/java/com/bbangle/bbangle/config/security/PublicApiPath.javasrc/main/java/com/bbangle/bbangle/config/security/auth/CustomOAuth2AuthorizationRequestResolver.javasrc/main/java/com/bbangle/bbangle/config/security/auth/OAuth2ClientValidationFilter.javasrc/main/java/com/bbangle/bbangle/util/CookieUtils.javasrc/test/java/com/bbangle/bbangle/auth/oauth/client/controller/OAuthControllerTest.javasrc/test/java/com/bbangle/bbangle/auth/oauth/client/facade/OAuthFacadeIntegrationTest.javasrc/test/java/com/bbangle/bbangle/auth/oauth/client/facade/OAuthFacadeUnitTest.javasrc/test/java/com/bbangle/bbangle/auth/oauth/client/service/OAuthServiceIntegrationTest.javasrc/test/java/com/bbangle/bbangle/auth/oauth/client/service/OAuthServiceUnitTest.javasrc/test/java/com/bbangle/bbangle/auth/seller/controller/SellerOauthControllerTest.javasrc/test/java/com/bbangle/bbangle/auth/seller/facade/OAuth2SellerFacadeIntegrationTest.javasrc/test/java/com/bbangle/bbangle/auth/seller/facade/OAuth2SellerFacadeUnitTest.javasrc/test/java/com/bbangle/bbangle/auth/seller/service/OAuthSellerServiceIntegrationTest.javasrc/test/java/com/bbangle/bbangle/auth/seller/service/OAuthSellerServiceUnitTest.javasrc/test/java/com/bbangle/bbangle/claim/repository/ClaimRepositoryDataJpaTest.javasrc/test/java/com/bbangle/bbangle/config/security/auth/OAuth2ClientValidationFilterUnitTest.javasrc/test/java/com/bbangle/bbangle/fixture/oauth/CookieFixture.java
💤 Files with no reviewable changes (7)
- src/main/java/com/bbangle/bbangle/auth/seller/controller/swagger/SellerOauthApi.java
- src/test/java/com/bbangle/bbangle/claim/repository/ClaimRepositoryDataJpaTest.java
- src/test/java/com/bbangle/bbangle/auth/seller/service/OAuthSellerServiceIntegrationTest.java
- src/main/java/com/bbangle/bbangle/auth/seller/service/OAuthSellerService.java
- src/test/java/com/bbangle/bbangle/auth/seller/controller/SellerOauthControllerTest.java
- src/test/java/com/bbangle/bbangle/auth/seller/service/OAuthSellerServiceUnitTest.java
- src/test/java/com/bbangle/bbangle/auth/seller/facade/OAuth2SellerFacadeIntegrationTest.java
src/main/java/com/bbangle/bbangle/auth/oauth/client/facade/OAuthFacade.java
Show resolved
Hide resolved
src/test/java/com/bbangle/bbangle/auth/oauth/client/controller/OAuthControllerTest.java
Outdated
Show resolved
Hide resolved
src/test/java/com/bbangle/bbangle/auth/oauth/client/controller/OAuthControllerTest.java
Show resolved
Hide resolved
- CookieUtils 주석 삭제 - OAuth API URI를 상수로 변경 - Refresh Token 발급 로직 리팩토링
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/main/java/com/bbangle/bbangle/auth/oauth/client/controller/OAuthController.java (1)
57-57: 쿠키 유효 기간을OAuthService.REFRESH_TOKEN_DURATION상수로 교체하세요.
Duration.ofDays(14)가 하드코딩되어 있지만, 이미OAuthService.REFRESH_TOKEN_DURATION으로 동일한 값이 상수로 정의되어 있습니다. 추후 유효 기간이 변경될 경우 이 코드가 함께 갱신되지 않을 수 있습니다.♻️ 제안된 수정
- response.addHeader(HttpHeaders.SET_COOKIE, createCookie(dto.refreshToken(), Duration.ofDays(14)).toString()); + response.addHeader(HttpHeaders.SET_COOKIE, createCookie(dto.refreshToken(), OAuthService.REFRESH_TOKEN_DURATION).toString());참고:
SellerOauthController.java(line 38)에서도 동일하게Duration.ofDays(14)가 하드코딩되어 있으므로 함께 수정하는 것을 권장합니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/bbangle/bbangle/auth/oauth/client/controller/OAuthController.java` at line 57, Replace the hardcoded Duration.ofDays(14) used when creating the refresh-token cookie with the shared constant OAuthService.REFRESH_TOKEN_DURATION; specifically update the call in OAuthController where response.addHeader(... createCookie(dto.refreshToken(), Duration.ofDays(14)) ...) to use createCookie(dto.refreshToken(), OAuthService.REFRESH_TOKEN_DURATION) and make the same change in SellerOauthController to ensure both controllers rely on the single source of truth.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/test/java/com/bbangle/bbangle/auth/oauth/client/service/OAuthServiceUnitTest.java`:
- Around line 60-77: The test success_update currently omits verifying that the
repository persists the updated token; update the unit test to assert that
RefreshTokenRepository.save(...) is called when
OAuthService.generateRefreshToken(...) finds an existing RefreshToken (created
via RefreshToken.create(...) and updated via existing.update(...));
specifically, after calling service.generateRefreshToken(1L, Role.ROLE_SELLER)
add a verify(refreshTokenRepository).save(...) (or equivalent with any() /
argThat for the RefreshToken) to ensure the update path in
OAuthService.generateRefreshToken actually invokes refreshTokenRepository.save
and would catch persistence bugs for non-JPA stores.
---
Nitpick comments:
In
`@src/main/java/com/bbangle/bbangle/auth/oauth/client/controller/OAuthController.java`:
- Line 57: Replace the hardcoded Duration.ofDays(14) used when creating the
refresh-token cookie with the shared constant
OAuthService.REFRESH_TOKEN_DURATION; specifically update the call in
OAuthController where response.addHeader(... createCookie(dto.refreshToken(),
Duration.ofDays(14)) ...) to use createCookie(dto.refreshToken(),
OAuthService.REFRESH_TOKEN_DURATION) and make the same change in
SellerOauthController to ensure both controllers rely on the single source of
truth.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
src/main/java/com/bbangle/bbangle/auth/oauth/client/controller/OAuthController.javasrc/main/java/com/bbangle/bbangle/auth/oauth/client/service/OAuthService.javasrc/main/java/com/bbangle/bbangle/config/security/PublicApiPath.javasrc/main/java/com/bbangle/bbangle/config/security/auth/CustomOAuth2AuthorizationRequestResolver.javasrc/main/java/com/bbangle/bbangle/config/security/auth/OAuth2ClientValidationFilter.javasrc/main/java/com/bbangle/bbangle/util/CookieUtils.javasrc/test/java/com/bbangle/bbangle/auth/oauth/client/controller/OAuthControllerTest.javasrc/test/java/com/bbangle/bbangle/auth/oauth/client/service/OAuthServiceUnitTest.javasrc/test/java/com/bbangle/bbangle/config/security/auth/OAuth2ClientValidationFilterUnitTest.java
🚧 Files skipped from review as they are similar to previous changes (3)
- src/main/java/com/bbangle/bbangle/util/CookieUtils.java
- src/main/java/com/bbangle/bbangle/config/security/auth/CustomOAuth2AuthorizationRequestResolver.java
- src/main/java/com/bbangle/bbangle/config/security/PublicApiPath.java
src/test/java/com/bbangle/bbangle/auth/oauth/client/service/OAuthServiceUnitTest.java
Show resolved
Hide resolved
- Token 유효 기간을 상수로 변경
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/main/java/com/bbangle/bbangle/auth/oauth/client/controller/OAuthController.java (1)
32-32:@RequestMapping("")제거 권장클래스 레벨의
@RequestMapping("")은 각 메서드가 이미 절대 경로를 직접 지정하고 있어 아무런 효과가 없습니다. 불필요한 노이즈를 줄이기 위해 제거하거나 아예 생략하는 것이 더 명확합니다.♻️ Proposed fix
`@RequiredArgsConstructor` `@RestController` -@RequestMapping("") public class OAuthController implements OAuthApi {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/bbangle/bbangle/auth/oauth/client/controller/OAuthController.java` at line 32, Remove the no-op class-level annotation `@RequestMapping`("") from the OAuthController class (symbol: OAuthController) since individual handler methods already declare absolute paths; delete the annotation from the class declaration and remove the unused import org.springframework.web.bind.annotation.RequestMapping if it becomes unused to avoid dead imports and keep the controller clean.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/main/java/com/bbangle/bbangle/auth/oauth/client/controller/OAuthController.java`:
- Around line 41-45: OAuthController의 login 구현에서 Spring이 인터페이스(`@OAuthApi`) 메서드의
어노테이션을 상속하지 못하므로 UserType과 ProfileType 파라미터에 명시적으로 `@RequestParam을` 추가하십시오:
login(OauthServerType oauthServerType, `@RequestParam`("user") UserType userType,
`@RequestParam`(value="profile", required=false, defaultValue="prod") ProfileType
profileType). 이렇게 해서 OAuthController.login, UserType, ProfileType 시그니처가
OAuthApi의 기대와 일치하도록 만드세요.
---
Nitpick comments:
In
`@src/main/java/com/bbangle/bbangle/auth/oauth/client/controller/OAuthController.java`:
- Line 32: Remove the no-op class-level annotation `@RequestMapping`("") from the
OAuthController class (symbol: OAuthController) since individual handler methods
already declare absolute paths; delete the annotation from the class declaration
and remove the unused import
org.springframework.web.bind.annotation.RequestMapping if it becomes unused to
avoid dead imports and keep the controller clean.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/main/java/com/bbangle/bbangle/auth/oauth/client/controller/OAuthController.javasrc/main/java/com/bbangle/bbangle/auth/oauth/client/service/OAuthService.javasrc/main/java/com/bbangle/bbangle/auth/seller/controller/SellerOauthController.javasrc/main/java/com/bbangle/bbangle/config/security/jwt/TokenProvider.java
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/com/bbangle/bbangle/auth/oauth/client/service/OAuthService.java
src/main/java/com/bbangle/bbangle/auth/oauth/client/controller/OAuthController.java
Show resolved
Hide resolved
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
History
🚀 Major Changes & Explanations
📷 Test Image
💡 ETC
Summary by CodeRabbit
New Features
Refactor
Tests