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 @@ -7,8 +7,8 @@
import life.mosu.mosuserver.domain.user.entity.UserRole;
import life.mosu.mosuserver.global.exception.CustomRuntimeException;
import life.mosu.mosuserver.global.exception.ErrorCode;
import life.mosu.mosuserver.presentation.auth.dto.LoginCommandResponse;
import life.mosu.mosuserver.presentation.auth.dto.LoginRequest;
import life.mosu.mosuserver.presentation.auth.dto.request.LoginRequest;
import life.mosu.mosuserver.presentation.auth.dto.response.LoginCommandResponse;
import lombok.RequiredArgsConstructor;
import org.springframework.security.crypto.password.PasswordEncoder;
import org.springframework.stereotype.Service;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@
import life.mosu.mosuserver.application.auth.processor.SignUpAccountStepProcessor;
import life.mosu.mosuserver.application.auth.provider.AuthTokenManager;
import life.mosu.mosuserver.domain.user.entity.UserJpaEntity;
import life.mosu.mosuserver.presentation.auth.dto.SignUpAccountRequest;
import life.mosu.mosuserver.presentation.auth.dto.Token;
import life.mosu.mosuserver.presentation.auth.dto.request.SignUpAccountRequest;
import lombok.RequiredArgsConstructor;
import org.springframework.security.crypto.password.PasswordEncoder;
import org.springframework.stereotype.Service;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,20 +17,23 @@ public record OAuthUserInfo(
String name,
Gender gender,
String phoneNumber,
LocalDate birthDay
LocalDate birthDay,
boolean marketingAgreed
) {

public static OAuthUserInfo of(
final OAuthProvider oAuthProvider,
final Map<String, Object> attributes
final Map<String, Object> attributes,
final boolean marketingAgreed
) {
return switch (oAuthProvider) {
case KAKAO -> ofKakao(attributes);
case KAKAO -> ofKakao(attributes, marketingAgreed);
default -> throw new CustomRuntimeException(ErrorCode.UNSUPPORTED_OAUTH2_PROVIDER);
};
}

private static OAuthUserInfo ofKakao(final Map<String, Object> attributes) {
private static OAuthUserInfo ofKakao(final Map<String, Object> attributes,
final boolean marketingAgreed) {
Map<String, Object> account = Optional.ofNullable(
(Map<String, Object>) attributes.get("kakao_account"))
.orElse(Map.of());
Expand Down Expand Up @@ -68,6 +71,7 @@ private static OAuthUserInfo ofKakao(final Map<String, Object> attributes) {
.phoneNumber(phoneNumber)
.gender(gender)
.birthDay(birthDate)
.marketingAgreed(marketingAgreed)
.build();
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package life.mosu.mosuserver.application.oauth;

import java.time.LocalDate;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import life.mosu.mosuserver.domain.profile.entity.Gender;
import life.mosu.mosuserver.domain.profile.repository.ProfileJpaRepository;
Expand All @@ -10,11 +12,13 @@
import life.mosu.mosuserver.domain.user.repository.UserJpaRepository;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.springframework.core.ParameterizedTypeReference;
import org.springframework.security.oauth2.client.userinfo.DefaultOAuth2UserService;
import org.springframework.security.oauth2.client.userinfo.OAuth2UserRequest;
import org.springframework.security.oauth2.core.OAuth2AuthenticationException;
import org.springframework.security.oauth2.core.user.OAuth2User;
import org.springframework.stereotype.Service;
import org.springframework.web.reactive.function.client.WebClient;

@Slf4j
@Service
Expand All @@ -23,14 +27,28 @@ public class OAuthUserService extends DefaultOAuth2UserService {

private final UserJpaRepository userRepository;
private final ProfileJpaRepository profileRepository;
private final WebClient webClient;

@Override
public OAuth2User loadUser(final OAuth2UserRequest userRequest)
throws OAuth2AuthenticationException {
final OAuth2User user = super.loadUser(userRequest);

final Map<String, Object> oAuth2UserAttributes = user.getAttributes();
log.info("KKK OAuth2User attributes: {}", oAuth2UserAttributes);

Map<String, Object> serviceTermsAttributes = getServiceTerms(
userRequest.getAccessToken().getTokenValue());

boolean agreedToMarketing = false;
if (serviceTermsAttributes.get("service_terms") instanceof List<?> termsList) {
agreedToMarketing = termsList.stream()
.filter(term -> term instanceof Map)
.map(term -> (Map<String, Object>) term)
.filter(termMap -> "terms_03".equals(termMap.get("tag")))
.findFirst()
.map(termMap -> (Boolean) termMap.get("agreed"))
.orElse(false);
}
Comment on lines +43 to +51

Choose a reason for hiding this comment

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

medium

The logic to parse the marketing agreement status relies on instanceof checks and casting from Map<String, Object>. This approach is not type-safe and can be brittle if the API response structure changes.

To improve robustness and readability, I recommend creating dedicated DTOs for the Kakao service terms API response. This would allow you to leverage your JSON mapper (e.g., Jackson) for safe deserialization and eliminate manual casting.

For example, you could define classes like this:

@JsonIgnoreProperties(ignoreUnknown = true)
public record KakaoServiceTermsResponse(List<ServiceTerm> service_terms) {}

@JsonIgnoreProperties(ignoreUnknown = true)
public record ServiceTerm(String tag, boolean agreed) {}

Using these DTOs in your WebClient call would make this parsing logic much cleaner and safer.


final String registrationId = userRequest.getClientRegistration().getRegistrationId();
final String userNameAttributeName = userRequest.getClientRegistration()
Expand All @@ -39,7 +57,7 @@ public OAuth2User loadUser(final OAuth2UserRequest userRequest)
.getUserNameAttributeName();

final OAuthUserInfo userInfo = OAuthUserInfo.of(OAuthProvider.from(registrationId),
oAuth2UserAttributes);
oAuth2UserAttributes, agreedToMarketing);

final UserJpaEntity oAuthUser = updateOrWrite(userInfo);

Expand All @@ -61,9 +79,8 @@ private UserJpaEntity updateOrWrite(final OAuthUserInfo info) {
})
.orElseGet(() -> {
final UserJpaEntity newUser = UserJpaEntity.builder()
//TODO kakao 정보 null일 경우 후처리 필요, ServiceTerm 승인 시 처리 구현
.loginId(info.email() != null ? info.email() : "NA")
.gender(info.gender() != null ? info.gender() : Gender.MALE)
.gender(info.gender() != null ? info.gender() : Gender.PENDING)
.name(info.name() != null ? info.name() : "NA")
.birth(info.birthDay() != null ? info.birthDay()
: LocalDate.EPOCH)
Expand All @@ -73,9 +90,30 @@ private UserJpaEntity updateOrWrite(final OAuthUserInfo info) {
.provider(AuthProvider.KAKAO)
.agreedToTermsOfService(true)
.agreedToPrivacyPolicy(true)
.agreedToMarketing(false)
.agreedToMarketing(info.marketingAgreed())
.build();
return userRepository.save(newUser);
});
}

private Map<String, Object> getServiceTerms(String accessToken) {

String url = "https://kapi.kakao.com/v2/user/service_terms";

try {
Map<String, Object> response = webClient.get()
.uri(url)
.header("Authorization", "Bearer " + accessToken)
.retrieve()
.bodyToMono(new ParameterizedTypeReference<Map<String, Object>>() {
})
.block();

log.info(response.toString());
return response != null ? response : Collections.emptyMap();
} catch (Exception e) {
log.error("Failed to fetch service terms from Kakao with WebClient", e);
return Collections.emptyMap();
}
}
Comment on lines +99 to +118

Choose a reason for hiding this comment

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

medium

This method has a couple of areas for improvement:

  1. Hardcoded URL: The Kakao API URL on line 101 is hardcoded. It's better to externalize this to a configuration file (e.g., application.yml) to improve maintainability and flexibility between different environments.
  2. Generic Exception Handling: The catch (Exception e) on line 114 is too broad. It's better to catch more specific exceptions, such as WebClientResponseException from Spring's WebClient, to handle different HTTP error statuses (e.g., 4xx vs. 5xx) appropriately.

}

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -47,18 +47,10 @@ public class SecurityConfig {

public static final List<String> clients = List.of(
"http://localhost:3000",
"http://localhost:8080",
"https://mosuedu.com",
"http://api.mosuedu.com",
"https://api.mosuedu.com",
"https://www.mosuedu.com",
"http://www.mosuedu.com:3000",
"https://test.mosuedu.com",
"http://localhost:5173",
"https://partnership.mosuedu.com",
"http://dev.mosuedu.com:3000",
"https://dev.mosuedu.com",
"http://admin.mosuedu.com:3000",
"https://admin.mosuedu.com"
);
private final OAuthUserService userService;
Expand Down
16 changes: 3 additions & 13 deletions src/main/java/life/mosu/mosuserver/global/config/WebMvcConfig.java
Original file line number Diff line number Diff line change
@@ -1,16 +1,14 @@
package life.mosu.mosuserver.global.config;

import java.util.List;

import life.mosu.mosuserver.global.resolver.PhoneNumberArgumentResolver;
import life.mosu.mosuserver.global.resolver.UserIdArgumentResolver;
import lombok.RequiredArgsConstructor;
import org.springframework.context.annotation.Configuration;
import org.springframework.web.method.support.HandlerMethodArgumentResolver;
import org.springframework.web.servlet.config.annotation.CorsRegistry;
import org.springframework.web.servlet.config.annotation.WebMvcConfigurer;

import life.mosu.mosuserver.global.resolver.UserIdArgumentResolver;
import lombok.RequiredArgsConstructor;

@Configuration
@RequiredArgsConstructor
public class WebMvcConfig implements WebMvcConfigurer {
Expand All @@ -22,7 +20,6 @@ public class WebMvcConfig implements WebMvcConfigurer {
public void addArgumentResolvers(final List<HandlerMethodArgumentResolver> resolvers) {
resolvers.add(userIdResolver);
resolvers.add(phoneNumberResolver);

}

@Override
Expand All @@ -32,17 +29,10 @@ public void addCorsMappings(CorsRegistry registry) {
.allowedHeaders("*")
.allowedOrigins(
"http://localhost:3000",
"http://localhost:8080",
"http://api.mosuedu.com",
"https://mosuedu.com",
"https://api.mosuedu.com",
"https://www.mosuedu.com",
"http://www.mosuedu.com:3000",
"https://test.mosuedu.com",
"http://localhost:5173",
"https://partnership.mosuedu.com",
"http://dev.mosuedu.com:3000",
"https://dev.mosuedu.com",
"http://admin.mosuedu.com:3000",
"https://admin.mosuedu.com"
)
Comment on lines 30 to 37

Choose a reason for hiding this comment

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

medium

The list of allowed origins is duplicated here and in SecurityConfig.java. To avoid inconsistencies and improve maintainability, you should reuse the clients list defined in SecurityConfig.

You can convert the list to an array as required by the allowedOrigins method.

                .allowedOrigins(SecurityConfig.clients.toArray(new String[0]))

.allowCredentials(true)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ public enum Whitelist {
SWAGGER_UI("/api/v1/swagger-ui", WhitelistMethod.ALL),
VIRTUAL_ACCOUNT("/api/v1/virtual-account", WhitelistMethod.ALL),
ADMISSION_TICKET("/api/v1/admission-ticket", WhitelistMethod.ALL),
APPLICATION_GUEST("/api/v1/applications/guest", WhitelistMethod.ALL),

// 정적 리소스
CSS("/api/v1/css", WhitelistMethod.GET),
Expand All @@ -32,15 +33,11 @@ public enum Whitelist {
OAUTH2("/api/v1/oauth2", WhitelistMethod.ALL),
OAUTH("/api/v1/oauth", WhitelistMethod.ALL),

// 삭제 예정
MASTER("/api/v1/master", WhitelistMethod.ALL),

// 조회만 가능한 PATH
EVENT("/api/v1/event", WhitelistMethod.GET),
FAQ("/api/v1/faq", WhitelistMethod.GET),
NOTICE("/api/v1/notice", WhitelistMethod.GET),
NOTICE("/api/v1/notice", WhitelistMethod.GET);

APPLICATION_GUEST("/api/v1/applications/guest", WhitelistMethod.ALL);
private final String path;
private final WhitelistMethod method;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@
import life.mosu.mosuserver.application.auth.provider.AuthTokenManager;
import life.mosu.mosuserver.application.oauth.OAuthUser;
import life.mosu.mosuserver.global.util.CookieBuilderUtil;
import life.mosu.mosuserver.presentation.auth.dto.LoginResponse;
import life.mosu.mosuserver.presentation.auth.dto.Token;
import life.mosu.mosuserver.presentation.auth.dto.request.LoginResponse;
import lombok.RequiredArgsConstructor;
import org.springframework.http.HttpHeaders;
import org.springframework.http.ResponseCookie;
Expand Down
Loading