-
Notifications
You must be signed in to change notification settings - Fork 0
test: Auth 서비스 단위 테스트 추가 + 보안 버그 2건 수정 + Apple 서명검증 dev 반영 #221
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -52,8 +52,12 @@ public GetNewTokenResponseDto getNewToken(String accessToken, String refreshToke | |
| try { | ||
| // refreshToken으로 유저찾기 | ||
| final long userId = Long.parseLong(tokenContents); | ||
| if(redisService.getValuesByKey(String.valueOf(userId)).isBlank()){ | ||
| //탈취된 refreshToken인 경우 | ||
| final String storedRefreshToken = redisService.getValuesByKey(String.valueOf(userId)); | ||
| // Redis에 저장된 최신 refreshToken과 실제로 일치하는지까지 확인한다. | ||
| // (단순히 "뭔가 저장돼 있는지"만 보면, 재로그인 등으로 이미 무효화된 | ||
| // 예전 refreshToken도 계속 accessToken 재발급에 쓰일 수 있었음) | ||
| if (storedRefreshToken == null || storedRefreshToken.isBlank() || !storedRefreshToken.equals(refreshToken)) { | ||
| //탈취되었거나 이미 무효화된 refreshToken인 경우 | ||
|
Comment on lines
+55
to
+60
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
ast-grep outline src/main/java/org/runnect/server/auth/service/AuthService.java --items all
ast-grep outline src/main/java/org/runnect/server/config/jwt/JwtService.java --items all
ast-grep outline src/main/java/org/runnect/server/config/redis/RedisService.java --items all
rg -n -C 4 'getNewToken|issuedAccessToken|issuedRefreshToken|setValues|getValuesByKey' \
src/main/java/org/runnect/server/auth/service/AuthService.java \
src/main/java/org/runnect/server/config/jwt/JwtService.java \
src/main/java/org/runnect/server/config/redis/RedisService.javaRepository: Runnect/Runnect-Spring-Boot-Server Length of output: 9941 🏁 Script executed: #!/bin/bash
set -euo pipefail
sed -n '1,125p' src/main/java/org/runnect/server/auth/service/AuthService.java | cat -n
sed -n '1,90p' src/main/java/org/runnect/server/config/jwt/JwtService.java | cat -n
sed -n '1,55p' src/main/java/org/runnect/server/config/redis/RedisService.java | cat -n
rg -n 'setIfAbsent|setIfPresent|compareAndSet|opsForValue\(\)\.setIfAbsent|opsForValue\(\)\.compareAndSet|lock|sessionVersion|session-version|sessionVersion|invalidate|issuedRefreshToken|refreshToken' src/main/java src/test/javaRepository: Runnect/Runnect-Spring-Boot-Server Length of output: 20604 Broken Authentication (CWE-367): Time-of-check Time-of-use (TOCTOU) Race Condition Reachability: External · Exploitability: Moderate Make refresh-token reissue atomic with Redis rotation. A stolen/old refresh token can read the current Redis value, then a concurrent sign-in can replace that value before Use atomic Redis state rotation for refresh-token reissue, such as comparing against the current stored value, writing an unchanged version key, and invalidating access tokens when session rotation occurs. 🤖 Prompt for AI Agents |
||
| throw new InvalidRefreshTokenException(ErrorStatus.INVALID_REFRESH_TOKEN_EXCEPTION, ErrorStatus.INVALID_REFRESH_TOKEN_EXCEPTION.getMessage()); | ||
| } | ||
| RunnectUser user = userRepository.findById(userId) | ||
|
|
@@ -79,8 +83,6 @@ public GetNewTokenResponseDto getNewToken(String accessToken, String refreshToke | |
| public AuthResponseDto signIn(SignInRequestDto signInRequestDto) { | ||
| SocialType socialType = SocialType.valueOf(signInRequestDto.getProvider()); | ||
|
|
||
| System.out.println("타입은? "+ socialType); | ||
|
|
||
| SocialInfoResponseDto socialInfo = getSocialInfo(socialType, signInRequestDto.getToken()); | ||
|
|
||
| boolean isRegistered = userRepository.existsByEmailAndProvider(socialInfo.getEmail(), socialType); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,53 @@ | ||
| package org.runnect.server.auth.service; | ||
|
|
||
| import static org.assertj.core.api.Assertions.assertThat; | ||
| import static org.assertj.core.api.Assertions.assertThatThrownBy; | ||
|
|
||
| import java.security.KeyPair; | ||
| import java.security.KeyPairGenerator; | ||
| import java.security.spec.ECGenParameterSpec; | ||
| import java.util.Base64; | ||
| import org.junit.jupiter.api.Test; | ||
| import org.runnect.server.common.exception.UnauthorizedException; | ||
| import org.springframework.test.util.ReflectionTestUtils; | ||
|
|
||
| /** | ||
| * AppleSignInService는 OkHttpClient/JWKS 서명 검증기를 메서드 내부에서 직접 생성해서 | ||
| * (의존성 주입이 안 되어 있어서) 실제 애플 서버 통신 없이는 getSocialInfo()의 | ||
| * 정상 경로(서명 검증 성공)를 순수 단위 테스트로 재현할 수 없다. | ||
| * 네트워크 없이 검증 가능한 두 가지 — P8 키 파싱, 잘못된 idToken 형식 — 만 다룬다. | ||
| */ | ||
| class AppleSignInServiceTest { | ||
|
|
||
| private String base64EncodedEcPrivateKey() throws Exception { | ||
| KeyPairGenerator generator = KeyPairGenerator.getInstance("EC"); | ||
| generator.initialize(new ECGenParameterSpec("secp256r1")); | ||
| KeyPair keyPair = generator.generateKeyPair(); | ||
| return Base64.getEncoder().encodeToString(keyPair.getPrivate().getEncoded()); | ||
| } | ||
|
|
||
| @Test | ||
| void 유효한_EC_비밀키면_정상적으로_파싱된다() throws Exception { | ||
| AppleSignInService service = new AppleSignInService(); | ||
|
|
||
| ReflectionTestUtils.invokeMethod(service, "getPrivateKey", base64EncodedEcPrivateKey()); | ||
|
|
||
| assertThat(ReflectionTestUtils.getField(service, "PRIVATE_KEY")).isNotNull(); | ||
| } | ||
|
|
||
| @Test | ||
| void 잘못된_형식의_비밀키면_UnauthorizedException() { | ||
| AppleSignInService service = new AppleSignInService(); | ||
|
|
||
| assertThatThrownBy(() -> ReflectionTestUtils.invokeMethod(service, "getPrivateKey", "not-a-valid-key")) | ||
| .isInstanceOf(UnauthorizedException.class); | ||
| } | ||
|
|
||
| @Test | ||
| void idToken이_JWT_형식이_아니면_UnauthorizedException() { | ||
| AppleSignInService service = new AppleSignInService(); | ||
|
|
||
| assertThatThrownBy(() -> service.getSocialInfo("this-is-not-a-jwt")) | ||
| .isInstanceOf(UnauthorizedException.class); | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
Repository: Runnect/Runnect-Spring-Boot-Server
Length of output: 37295
🏁 Script executed:
Repository: Runnect/Runnect-Spring-Boot-Server
Length of output: 40437
Do not treat provider outages as invalid-provider credentials.
These catch blocks throw
INVALID_*_ID_TOKEN_EXCEPTIONfor anyException, including JWKS retrieval/network failures and Kakao API 5xx/network failures. Return a retryable provider error for connectivity/timeout/server failures while keeping signature or invalid-credential failures as invalid-token errors.📍 Affects 2 files
src/main/java/org/runnect/server/auth/service/AppleSignInService.java#L100-L106(this comment)src/main/java/org/runnect/server/auth/service/KakaoSignInService.java#L37-L49🤖 Prompt for AI Agents