[배포] #265까지 병합 및 배포 #266
There are no files selected for viewing
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,47 @@ | ||
| package life.mosu.mosuserver.application.oauth; | ||
|
|
||
| import life.mosu.mosuserver.domain.user.entity.AuthProvider; | ||
| import life.mosu.mosuserver.domain.user.entity.UserJpaEntity; | ||
| import life.mosu.mosuserver.domain.user.entity.UserRole; | ||
| import life.mosu.mosuserver.domain.user.repository.UserJpaRepository; | ||
| import life.mosu.mosuserver.global.processor.StepProcessor; | ||
| import lombok.RequiredArgsConstructor; | ||
| import org.springframework.stereotype.Component; | ||
| import org.springframework.transaction.annotation.Transactional; | ||
|
|
||
| @Component | ||
| @RequiredArgsConstructor | ||
| public class OAuthUserPersistenceProcessor implements StepProcessor<OAuthUserInfo, UserJpaEntity> { | ||
|
|
||
| private final UserJpaRepository userRepository; | ||
|
|
||
| @Override | ||
| @Transactional | ||
| public UserJpaEntity process(final OAuthUserInfo info) { | ||
| return userRepository.findByLoginId(info.email()) | ||
| .map(existingUser -> { | ||
| existingUser.updateOAuthUser( | ||
| info.gender(), | ||
| info.name(), | ||
| info.phoneNumber(), | ||
| info.birthDay(), | ||
| info.marketingAgreed()); | ||
| return existingUser; | ||
| }) | ||
| .orElseGet(() -> { | ||
| final UserJpaEntity newUser = UserJpaEntity.builder() | ||
| .loginId(info.email()) | ||
| .gender(info.gender()) | ||
| .name(info.name()) | ||
| .birth(info.birthDay()) | ||
| .phoneNumber(info.phoneNumber()) | ||
| .userRole(UserRole.ROLE_PENDING) | ||
| .provider(AuthProvider.KAKAO) | ||
| .agreedToTermsOfService(true) | ||
| .agreedToPrivacyPolicy(true) | ||
| .agreedToMarketing(info.marketingAgreed()) | ||
| .build(); | ||
| return userRepository.save(newUser); | ||
| }); | ||
| } | ||
| } | ||
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,15 +6,18 @@ | |
|
|
||
| @Getter | ||
| public class BlockedIp { | ||
|
|
||
| private final TimePenalty penaltyLevel; | ||
|
|
||
| public BlockedIp(TimePenalty penaltyLevel) { | ||
| this.penaltyLevel = penaltyLevel; | ||
| } | ||
|
|
||
| public static BlockedIp init() { | ||
| return new BlockedIp(TimePenalty.LEVEL_0); | ||
| } | ||
|
|
||
| public Duration getTtl() { | ||
| return penaltyLevel.getDuration(); | ||
| } | ||
| } | ||
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,12 +1,16 @@ | ||
| package life.mosu.mosuserver.domain.caffeine.dto; | ||
|
|
||
| import java.util.concurrent.atomic.AtomicInteger; | ||
|
|
||
| public class RequestCounter { | ||
|
|
||
| private final AtomicInteger count = new AtomicInteger(); | ||
|
|
||
| public int incrementAndGet() { | ||
| return count.incrementAndGet(); | ||
| } | ||
|
|
||
| public int getCount() { | ||
| return count.get(); | ||
| } | ||
| } |
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -93,12 +93,14 @@ public void updateOAuthUser( | |
| Gender gender, | ||
| String name, | ||
| String phoneNumber, | ||
| LocalDate birth, | ||
| boolean agreedToMarketing | ||
| ) { | ||
| this.gender = gender; | ||
| this.name = name; | ||
| this.phoneNumber = phoneNumber; | ||
| this.birth = birth; | ||
| this.agreedToMarketing = agreedToMarketing; | ||
| } | ||
|
|
||
| public void updateUserInfo( | ||
|
|
||
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,8 +5,8 @@ | |
| import com.github.benmanes.caffeine.cache.Expiry; | ||
| import com.github.benmanes.caffeine.cache.LoadingCache; | ||
| import java.util.concurrent.TimeUnit; | ||
| import life.mosu.mosuserver.domain.caffeine.dto.BlockedIp; | ||
| import life.mosu.mosuserver.domain.caffeine.dto.BlockedIpHistory; | ||
| import life.mosu.mosuserver.domain.caffeine.dto.RequestCounter; | ||
| import org.springframework.context.annotation.Bean; | ||
| import org.springframework.context.annotation.Configuration; | ||
|
|
@@ -41,12 +41,14 @@ public long expireAfterCreate(String key, BlockedIp value, long currentTime) { | |
| } | ||
|
|
||
| @Override | ||
| public long expireAfterUpdate(String key, BlockedIp value, long currentTime, | ||
| long currentDuration) { | ||
| return value.getTtl().toNanos(); | ||
| } | ||
|
|
||
| @Override | ||
| public long expireAfterRead(String key, BlockedIp value, long currentTime, | ||
| long currentDuration) { | ||
| return currentDuration; | ||
| } | ||
| }) | ||
|
|
||
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,13 +9,12 @@ | |
| import jakarta.servlet.http.HttpServletRequest; | ||
| import jakarta.servlet.http.HttpServletResponse; | ||
| import java.io.IOException; | ||
| import life.mosu.mosuserver.domain.caffeine.dto.BlockedIp; | ||
| import life.mosu.mosuserver.domain.caffeine.dto.BlockedIpHistory; | ||
| import life.mosu.mosuserver.domain.caffeine.dto.RequestCounter; | ||
| import life.mosu.mosuserver.global.config.IpRateLimitingProperties; | ||
| import life.mosu.mosuserver.global.exception.CustomRuntimeException; | ||
| import life.mosu.mosuserver.global.exception.ErrorCode; | ||
| import lombok.RequiredArgsConstructor; | ||
| import lombok.extern.slf4j.Slf4j; | ||
| import org.springframework.stereotype.Component; | ||
|
|
@@ -31,59 +30,60 @@ public class IpRateLimitingFilter extends OncePerRequestFilter { | |
| private final Cache<String, BlockedIpHistory> blockedHistoryCache; | ||
| private final LoadingCache<String, BlockedIp> blockedIpCache; | ||
|
|
||
| @Override | ||
| protected void doFilterInternal( | ||
| HttpServletRequest request, | ||
| HttpServletResponse response, | ||
| FilterChain filterChain | ||
| ) throws ServletException, IOException { | ||
|
|
||
| if (!ipRateLimitingProperties.isEnabled()) { | ||
| log.debug("IpRateLimitingFilter disabled"); | ||
| filterChain.doFilter(request, response); | ||
| return; | ||
| } | ||
|
|
||
| String ip = getClientIp(request); | ||
| isAlreadyBlocked(ip); | ||
|
|
||
| RequestCounter counter = ipRequestCountsCache.get(ip, k -> new RequestCounter()); | ||
| int after = counter.incrementAndGet(); | ||
| int max = ipRateLimitingProperties.getMaxRequestsPerMinute(); | ||
|
|
||
| if (after > max) { | ||
| handleBlockedIp(ip); | ||
| } | ||
|
|
||
| log.debug("IP: {}, 요청 횟수 증가 후: {}", ip, after); | ||
| log.debug("Cache stats: {}", ipRequestCountsCache.stats()); | ||
|
|
||
| filterChain.doFilter(request, response); | ||
| } | ||
|
|
||
| private void handleBlockedIp(String ip) { | ||
| BlockedIp existing = blockedIpCache.asMap().putIfAbsent(ip, BlockedIp.init()); | ||
| if (existing != null) { | ||
| log.warn("이미 차단된 IP: {}, 차단 레벨: {}", ip, existing.getPenaltyLevel()); | ||
| throw new CustomRuntimeException(ErrorCode.TOO_MANY_REQUESTS); | ||
| } | ||
|
|
||
| TimePenalty level = blockedHistoryCache.asMap().compute(ip, (k, history) -> { | ||
| BlockedIpHistory h = (history == null) ? new BlockedIpHistory(ip) : history; | ||
| TimePenalty next = h.getPenaltyLevel().nextLevel(); | ||
| h.updateHistory(next); | ||
| return h; | ||
| }).getPenaltyLevel(); | ||
|
|
||
| blockedIpCache.asMap().computeIfPresent(ip, (k, v) -> new BlockedIp(level)); | ||
|
|
||
| throw new CustomRuntimeException(ErrorCode.TOO_MANY_REQUESTS); | ||
| } | ||
|
|
||
| private void isAlreadyBlocked(String ip) { | ||
| BlockedIp blockedIp = blockedIpCache.getIfPresent(ip); | ||
| if (blockedIp != null) { | ||
| log.warn("이미 차단된 IP: {}, 차단 레벨: {}", ip, blockedIp.getPenaltyLevel()); | ||
| throw new CustomRuntimeException(ErrorCode.TOO_MANY_REQUESTS); | ||
| } | ||
| } | ||
| } | ||
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,7 +7,7 @@ | |
| @Getter | ||
| @RequiredArgsConstructor | ||
| public enum TimePenalty { | ||
| LEVEL_0(0, Duration.ofSeconds(10)), | ||
| LEVEL_1(1, Duration.ofMinutes(1)), | ||
| LEVEL_2(2, Duration.ofMinutes(5)), | ||
| LEVEL_3(3, Duration.ofMinutes(30)), | ||
|
|
||
Oops, something went wrong.
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.
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.
The
processmethod doesn't handle the case whereinfo.email()isnull. If the OAuth provider doesn't return an email,userRepository.findByLoginId(null)will be called, which will likely cause aNullPointerExceptionor anIllegalArgumentException, breaking the login/signup flow for that user.The previous implementation had a fallback for a null email, but it was flawed as it used a non-unique value ("NA"). This refactoring seems to have lost the null-safety check entirely.
If an email is required for identifying users, you should validate that it's not null and throw an exception if it is. This makes the requirement explicit and prevents runtime errors.