Skip to content

Commit 3818004

Browse files
authored
Enhance social login security and metadata validation with Redis TTL (#143)
* Refactor authentication API and token management system - Removed outdated Redis script executors and consolidated token management logic. - Introduced a new RedisScriptExecutor interface for better abstraction and handling of Redis scripts. - Enhanced error handling in token management, replacing BaseException with AuthException for specific authentication errors. - Updated AuthService to streamline login and sign-up processes, improving member validation and token creation. - Refactored OAuthLoginRequest to simplify OAuth handling and removed unnecessary classes for cleaner codebase. - Improved member repository methods for better retrieval and error handling. This refactor aims to enhance maintainability, security, and clarity in the authentication flow. * Refactor error handling and improve code clarity - Simplified error messages in SocialLink to enhance security and clarity. - Updated AuthService to use orElseGet for better performance and readability. - Corrected a typo in AuthConstants for the refresh token prefix. - Enhanced IpAddressExtractor to utilize orElseGet for improved null handling. - Refactored FollowReader to use orElseGet for consistency in null returns. These changes aim to improve maintainability and clarity across the authentication and follow management components. --------- Co-authored-by: Youngjun Kim <[email protected]>
1 parent 7fceebc commit 3818004

37 files changed

+598
-250
lines changed
File renamed without changes.
File renamed without changes.

docs/pr/PR-141-refactor---auth.md

Lines changed: 131 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,131 @@
1+
# 치명적인 보안 패치 및 인증/인가 리팩토링 (PR [#141](https://github.com/juulabel/juulabel-back/pull/143))
2+
3+
## TL;DR
4+
5+
이번 PR은 소셜 로그인 프로세스에 존재했던 **치명적인 보안 취약점**을 해결하고, 불필요한 데이터베이스 호출을 줄이며 도메인 책임을 명확히 했습니다.
6+
7+
| 항목 | Before | After | 결과 |
8+
| :------------------- | :------------------------ | :--------------------- | :------------------------ |
9+
| **보안 위험** | **높음** (소셜 인증 우회) | **완화됨** | **치명적 취약점 해결** |
10+
| **회원가입 DB 쿼리** | 4회 | 1회 | **75% 감소** |
11+
| **로그인 DB 쿼리** | 2회 | 1회 | **50% 감소** |
12+
| **이메일 검증** | 중복 이메일 처리 회원가입에서만 | 로그인도 같이 | **유저 경험 개선** |
13+
14+
---
15+
16+
## 💥 핵심 문제 해결
17+
18+
### 1. 소셜 로그인 우회 취약점 차단
19+
20+
**문제점:** 이전에는 사용자가 소셜 인증 핸드셰이크 과정을 완전히 우회할 수 있었습니다. `/v1/api/auth/sign-up` 엔드포인트에 조작된 데이터를 직접 호출함으로써, 실제 검증 없이 계정을 생성할 수 있었습니다.
21+
22+
```javascript
23+
// 이전 취약점: 검증되지 않은 회원가입 허용
24+
POST /v1/api/auth/sign-up
25+
{
26+
"email": "[email protected]",
27+
"nickname": "fake_name",
28+
"provider": "GOOGLE",
29+
"providerId": "fake_google_id",
30+
...
31+
}
32+
```
33+
34+
**해결**: TTL 기반 소셜 인증 상태 관리
35+
36+
- 소셜 로그인 시 로그인 정보와 request header로부터 받아올수있는 metatdata를 Redis에 30분 TTL로 저장
37+
- 회원가입 시 로그인떄와 저장된 정보와 100% 일치하는지 검증
38+
- 불일치하거나 TTL 만료 시 가입 차단
39+
40+
### 2. 인증 로직 보완
41+
42+
**Before**: 기존에는 이미 가입된 이메일의 다른 소셜 로그인을 통한 처리를 회원가입부분에서 검증.
43+
44+
```java
45+
boolean isNewMember = !memberReader.existsByEmailAndProvider(email, provider);
46+
```
47+
48+
**After**: 로그인 과정에서도 검증하여 에러 반환
49+
50+
```java
51+
public void validateLoginMember(Provider provider, String providerId) {
52+
if (this.deletedAt != null) {
53+
throw new BaseException(ErrorCode.MEMBER_WITHDRAWN);
54+
}
55+
// 사용자가 등록한 *동일한* 제공업체를 통해 로그인하는지 확인
56+
if (!this.provider.equals(provider)) {
57+
throw new BaseException(ErrorCode.MEMBER_EMAIL_DUPLICATE);
58+
}
59+
// 중요: *올바른* 제공업체의 고유 ID 확인
60+
if (!this.providerId.equals(providerId)) {
61+
throw new AuthException(ErrorCode.PROVIDER_ID_MISMATCH);
62+
}
63+
}
64+
```
65+
66+
## 3. 데이터베이스 성능 최적화
67+
68+
**Before**: 회원가입 과정에서는 INSERT를 시도하기 전에 닉네임 충돌, 이메일 충돌, 탈퇴 상태를 확인하기 위해 여러 번의 중복된 EXISTS 쿼리가 발생했습니다. 이로 인해 불필요한 데이터베이스 왕복이 발생했습니다.
69+
70+
```java
71+
// 비효율적: 삽입 전 여러 사전 확인
72+
boolean nicknameExists = memberRepository.existsByNickname(nickname); // 쿼리 1
73+
boolean emailExists = memberRepository.existsByEmail(email); // 쿼리 2
74+
boolean isWithdrawn = withdrawalRepository.existsByEmail(email); // 쿼리 3
75+
memberRepository.save(member); // 쿼리 4
76+
```
77+
78+
**After**: 데이터베이스의 내장된 제약 조건 위반 처리를 활용합니다. INSERT를 직접 시도하고 DataIntegrityViolationException (예: 고유 키 충돌)과 같은 잠재적인 예외 사례를 우아하게 처리합니다. 이 접근 방식은 일반적인 회원가입 흐름을 단일 INSERT 쿼리로 줄입니다.
79+
80+
```java
81+
// 효율적: 단일 쿼리, DB 제약 조건 활용 (낙관적)
82+
try {
83+
memberRepository.save(member); // 쿼리 1
84+
} catch (DataIntegrityViolationException e) {
85+
// 특정 제약 조건 위반 처리 (예: 이메일/닉네임 중복)
86+
handleConstraintViolation(e);
87+
}
88+
```
89+
90+
## 기타 사항
91+
92+
### 불필요한 DTO 제거
93+
94+
```java
95+
// 제거된 중간 변환 객체
96+
public class OAuthLoginInfo { /* 불필요한 래핑 */ }
97+
98+
// 불필요한 변환 발생
99+
public record OAuthLoginRequest(
100+
/* 불필요한 래핑 */
101+
) {
102+
public OAuthLoginInfo toDto() {
103+
Map<String, String> propertyMap = Map.of(
104+
AuthConstants.CODE, code,
105+
AuthConstants.REDIRECT_URI, redirectUri
106+
);
107+
return new OAuthLoginInfo(provider, propertyMap);
108+
}
109+
}
110+
111+
// 객체 변환 간소화
112+
final OAuthUser oAuthUser = providerFactory.getOAuthUser(oAuthLoginRequest);
113+
```
114+
115+
### AuthExcpetion 추가
116+
117+
기존에는 인증/인가 관련 예외(Authentication, Authorization)를 포함해 모든 예외를 동일한 수준에서 처리하고 있었습니다. 이 방식은 간단하지만 다음과 같은 단점이 있습니다:
118+
119+
❗ 문제점
120+
• 문제 추적이 어렵다: 인증 관련 문제인지, 비즈니스 로직 문제인지 구분되지 않음
121+
• 모니터링/알림 설정이 어려움: 특정 보안 이슈에 대한 빠른 탐지가 불가능
122+
• 책임 경계 불분명: 도메인 계층과 인증 계층의 에러가 동일하게 처리됨
123+
124+
125+
126+
🎯 개선 방향: AuthException 정의 및 분리
127+
• 인증/인가 실패 상황(providerId 불일치, 로그인되지 않은 사용자, 토큰 유효성 문제 등)에 대해 별도 예외 클래스를 정의
128+
• BaseException으로부터 상속, ErrorCode를 명확히 지정
129+
• 차후 로그 필터링/슬랙 알림/보안 모니터링 등에서 인증 이슈만 별도로 추적 가능
130+
131+
Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,107 @@
1+
package com.juu.juulabel.auth.domain;
2+
3+
import java.io.Serializable;
4+
import java.time.Instant;
5+
import java.util.Objects;
6+
import java.util.concurrent.TimeUnit;
7+
8+
import org.springframework.data.annotation.Id;
9+
import org.springframework.data.redis.core.RedisHash;
10+
import org.springframework.data.redis.core.TimeToLive;
11+
12+
import static com.juu.juulabel.common.constants.AuthConstants.SOCIAL_LINK_DURATION;
13+
import static com.juu.juulabel.common.constants.AuthConstants.SOCIAL_LINK_PREFIX;
14+
15+
import com.juu.juulabel.common.exception.AuthException;
16+
import com.juu.juulabel.common.exception.code.ErrorCode;
17+
import com.juu.juulabel.member.domain.Provider;
18+
19+
import lombok.Builder;
20+
import lombok.Getter;
21+
import lombok.NoArgsConstructor;
22+
import lombok.AllArgsConstructor;
23+
import lombok.AccessLevel;
24+
25+
@Getter
26+
@NoArgsConstructor(access = AccessLevel.PROTECTED)
27+
@AllArgsConstructor(access = AccessLevel.PRIVATE)
28+
@RedisHash(SOCIAL_LINK_PREFIX)
29+
public class SocialLink implements Serializable {
30+
31+
@Id
32+
private String hashedEmail;
33+
34+
private Provider provider;
35+
36+
private String providerId;
37+
38+
private String deviceId;
39+
40+
private String ipAddress;
41+
42+
private String userAgent;
43+
44+
private Long usedAt;
45+
46+
@TimeToLive(unit = TimeUnit.SECONDS)
47+
private Long ttl;
48+
49+
@Builder
50+
public SocialLink(String hashedEmail, Provider provider, String providerId, String deviceId, String userAgent,
51+
String ipAddress) {
52+
this.hashedEmail = hashedEmail;
53+
this.provider = provider;
54+
this.providerId = providerId;
55+
this.deviceId = deviceId;
56+
this.userAgent = userAgent;
57+
this.ipAddress = ipAddress;
58+
this.usedAt = null;
59+
this.ttl = SOCIAL_LINK_DURATION.getSeconds();
60+
}
61+
62+
/**
63+
* Validates the social link against provided parameters for security purposes.
64+
* Throws AuthException if validation fails.
65+
*/
66+
public void validate(Provider provider, String providerId, String deviceId, String userAgent) {
67+
// Check if already used
68+
if (isAlreadyUsed()) {
69+
throw new AuthException(ErrorCode.SOCIAL_LINK_ALREADY_USED);
70+
}
71+
72+
// Validate parameters match stored values
73+
if (!isValidationParametersMatch(provider, providerId, deviceId, userAgent)) {
74+
throw new AuthException("Validation failed due to parameter mismatch");
75+
}
76+
}
77+
78+
/**
79+
* Marks this social link as used with current timestamp.
80+
* Can only be used once.
81+
*/
82+
public void markAsUsed() {
83+
if (isAlreadyUsed()) {
84+
throw new AuthException(ErrorCode.SOCIAL_LINK_ALREADY_USED);
85+
}
86+
this.usedAt = Instant.now().getEpochSecond();
87+
}
88+
89+
/**
90+
* Checks if this social link has already been used.
91+
*/
92+
public boolean isAlreadyUsed() {
93+
return this.usedAt != null;
94+
}
95+
96+
/**
97+
* Checks if validation parameters match stored values.
98+
* Uses efficient short-circuit evaluation.
99+
*/
100+
private boolean isValidationParametersMatch(Provider provider, String providerId, String deviceId,
101+
String userAgent) {
102+
return Objects.equals(this.provider, provider) &&
103+
Objects.equals(this.providerId, providerId) &&
104+
Objects.equals(this.deviceId, deviceId) &&
105+
Objects.equals(this.userAgent, userAgent);
106+
}
107+
}

src/main/java/com/juu/juulabel/auth/executor/LoginRefreshTokenScriptExecutor.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
import org.springframework.stereotype.Component;
1313

1414
import com.juu.juulabel.auth.domain.RefreshToken;
15+
import com.juu.juulabel.redis.RedisScriptExecutor;
1516

1617
@Component
1718
public class LoginRefreshTokenScriptExecutor implements RedisScriptExecutor<Object, RefreshToken> {

src/main/java/com/juu/juulabel/auth/executor/RevokeRefreshTokenByIndexKeyExecutor.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@
1111
import org.springframework.data.redis.core.script.RedisScript;
1212
import org.springframework.stereotype.Component;
1313

14+
import com.juu.juulabel.redis.RedisScriptExecutor;
15+
1416
@Component
1517
public class RevokeRefreshTokenByIndexKeyExecutor implements RedisScriptExecutor<Object, String> {
1618

src/main/java/com/juu/juulabel/auth/executor/RotateRefreshTokenScriptExecutor.java

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,10 @@
1515
import com.juu.juulabel.auth.domain.RefreshToken;
1616
import com.juu.juulabel.common.constants.AuthConstants;
1717
import com.juu.juulabel.common.exception.BaseException;
18+
import com.juu.juulabel.common.exception.AuthException;
1819
import com.juu.juulabel.common.exception.code.ErrorCode;
1920
import com.juu.juulabel.common.util.HttpResponseUtil;
21+
import com.juu.juulabel.redis.RedisScriptExecutor;
2022

2123
@Component
2224
public class RotateRefreshTokenScriptExecutor implements RedisScriptExecutor<Object, RefreshToken> {
@@ -52,11 +54,11 @@ public Object execute(RefreshToken refreshToken, Object... args) {
5254
public void handleRedisScriptError(String errorMessage) {
5355
HttpResponseUtil.addCookie(AuthConstants.REFRESH_TOKEN_HEADER_NAME, "", 0);
5456
if (errorMessage.contains("OLD_TOKEN_NOT_FOUND")) {
55-
throw new BaseException(ErrorCode.REFRESH_TOKEN_NOT_FOUND);
57+
throw new AuthException(ErrorCode.REFRESH_TOKEN_NOT_FOUND);
5658
} else if (errorMessage.contains("OLD_TOKEN_ALREADY_REVOKED_ALL_TOKENS_INVALIDATED")) {
57-
throw new BaseException(ErrorCode.REFRESH_TOKEN_REUSE_DETECTED);
59+
throw new AuthException(ErrorCode.REFRESH_TOKEN_REUSE_DETECTED);
5860
} else if (errorMessage.contains("DEVICE_ID_MISMATCH")) {
59-
throw new BaseException(ErrorCode.DEVICE_ID_MISMATCH);
61+
throw new AuthException(ErrorCode.DEVICE_ID_MISMATCH);
6062
} else {
6163
throw new BaseException(errorMessage, ErrorCode.INTERNAL_SERVER_ERROR);
6264
}

src/main/java/com/juu/juulabel/auth/executor/SaveRefreshTokenScriptExecutor.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
import org.springframework.stereotype.Component;
1414

1515
import com.juu.juulabel.auth.domain.RefreshToken;
16+
import com.juu.juulabel.redis.RedisScriptExecutor;
1617

1718
@Component
1819
public class SaveRefreshTokenScriptExecutor implements RedisScriptExecutor<Object, RefreshToken> {

src/main/java/com/juu/juulabel/auth/repository/RedisRefreshTokenRepository.java renamed to src/main/java/com/juu/juulabel/auth/repository/RefreshTokenRepositoryImpl.java

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,16 @@
22

33
import com.juu.juulabel.auth.domain.ClientId;
44
import com.juu.juulabel.auth.domain.RefreshToken;
5-
import com.juu.juulabel.auth.executor.RedisScriptName;
6-
import com.juu.juulabel.auth.executor.ScriptRegistry;
75
import com.juu.juulabel.common.constants.AuthConstants;
6+
import com.juu.juulabel.redis.RedisScriptName;
7+
import com.juu.juulabel.redis.ScriptRegistry;
8+
89
import lombok.RequiredArgsConstructor;
910
import org.springframework.stereotype.Repository;
1011

1112
@Repository
1213
@RequiredArgsConstructor
13-
public class RedisRefreshTokenRepository implements RefreshTokenRepository {
14+
public class RefreshTokenRepositoryImpl implements RefreshTokenRepository {
1415

1516
private final ScriptRegistry scriptRegistry;
1617

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
package com.juu.juulabel.auth.repository;
2+
3+
import org.springframework.data.repository.CrudRepository;
4+
5+
import com.juu.juulabel.auth.domain.SocialLink;
6+
7+
public interface SocialLinkRepository extends CrudRepository<SocialLink, String> {
8+
9+
}

0 commit comments

Comments
 (0)