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 @@ -17,9 +17,10 @@ public class CustomUserDetailsService implements UserDetailsService {

private final UserRepository userRepository;

// OrderByIdAsc로 해줌으로써 처음 생성된 계정으로 연결된다.

Choose a reason for hiding this comment

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

medium

리포지토리 스타일 가이드 규칙 #8에 따라 주석에 작성자 이름과 날짜를 포함해야 합니다.

References
  1. Each comment must include the author's name and the date. (link)

@Override
public UserDetails loadUserByUsername(String username) {
User user = userRepository.findByEmail(username)
User user = userRepository.findFirstByEmailOrderByIdAsc(username)

Choose a reason for hiding this comment

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

security-high high

The change to return the first user found by email (findFirstByEmailOrderByIdAsc) breaks authentication for users who have multiple accounts with the same email but different providers (e.g., one Apple account and one Kakao account). When such a user attempts to log in with their newer account, Spring Security's AuthenticationManager will retrieve the oldest account's details and attempt to verify the current provider's credentials against the oldest account's stored credentials (which are provider-specific, e.g., APPLE + appleId vs KAKAO + kakaoId). This will result in a BadCredentialsException, effectively locking users out of all but their oldest account. A proper integration should either allow multiple credentials per user or handle provider-specific lookups during authentication.

.orElseThrow(() -> new BaseException(BaseResponseStatus.NOT_FOUND_USER));
return new CustomUserDetails(user);
}
Expand Down
21 changes: 11 additions & 10 deletions src/main/java/ssu/eatssu/domain/auth/service/OAuthService.java
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@ public class OAuthService {

public Tokens kakaoLogin(KakaoLoginRequest request) {
User user = userRepository.findByProviderId(request.providerId())
.orElseGet(() -> userService.join(request.email(), KAKAO, request.providerId()));
.orElseGet(() -> userRepository.findFirstByEmailOrderByIdAsc(request.email())
.orElseGet(() -> userService.join(request.email(), KAKAO, request.providerId())));
Comment on lines +37 to +38

Choose a reason for hiding this comment

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

security-critical critical

The kakaoLogin and kakaoLoginV2 methods accept email and providerId directly from the client request without server-side verification. This trusts unvalidated user input, allowing an attacker to 'squat' on any unassigned email address by providing an existing user's email, preventing legitimate owners from using the service with a different provider. All OAuth-based logins must verify the provider's token on the backend to retrieve a verified email and provider ID. Additionally, a functional bug exists: when attempting a different social login with the same email, authentication fails. Even if a user is found with findFirstByEmailOrderByIdAsc, their credentials are stored with previous social login information. generateOauthJwtTokens then attempts authentication with new social login info, causing a BadCredentialsException. To resolve this, when a user is found by email, the logic needs to update their provider, providerId, and credentials with the new social login information. This issue affects kakaoLogin, kakaoLoginV2, appleLogin, and appleLoginV2 methods.


return generateOauthJwtTokens(user.getEmail(), KAKAO, request.providerId());
}
Expand All @@ -44,11 +45,11 @@ public Tokens kakaoLogin(KakaoLoginRequest request) {
*/
public Tokens kakaoLoginV2(KakaoLoginRequestV2 request) {
User user = userRepository.findByProviderId(request.providerId())
.orElseGet(() -> userService.joinV2(request.email(), KAKAO, request.providerId(),request.deviceType()));
.orElseGet(() -> userRepository.findFirstByEmailOrderByIdAsc(request.email())
.orElseGet(() -> userService.joinV2(request.email(), KAKAO, request.providerId(),request.deviceType())));
Comment on lines 47 to +49

Choose a reason for hiding this comment

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

medium

사용자 조회 및 가입 로직이 kakaoLogin, kakaoLoginV2, appleLogin, appleLoginV2 네 개의 메소드에 걸쳐 중복되고 있습니다. 이 로직을 별도의 private 메소드로 추출하여 코드 중복을 줄이고 유지보수성을 향상시키는 것을 고려해보세요.


user.updateDeviceType(request.deviceType());

if (user.getDeviceType() == null) {
user.updateDeviceType(request.deviceType());
}

return generateOauthJwtTokens(user.getEmail(), KAKAO, request.providerId());
}
Expand All @@ -58,7 +59,8 @@ public Tokens appleLogin(AppleLoginRequest request) {
OAuthInfo oAuthInfo = appleAuthenticator.getOAuthInfoByIdentityToken(request.identityToken());

User user = userRepository.findByProviderId(oAuthInfo.providerId())
.orElseGet(() -> userService.join(oAuthInfo.email(), APPLE, oAuthInfo.providerId()));
.orElseGet(() -> userRepository.findFirstByEmailOrderByIdAsc(oAuthInfo.email())
.orElseGet(() -> userService.join(oAuthInfo.email(), APPLE, oAuthInfo.providerId())));

updateAppleUserEmail(user, oAuthInfo.email());

Expand All @@ -72,13 +74,12 @@ public Tokens appleLoginV2(AppleLoginRequestV2 request) {
OAuthInfo oAuthInfo = appleAuthenticator.getOAuthInfoByIdentityToken(request.identityToken());

User user = userRepository.findByProviderId(oAuthInfo.providerId())
.orElseGet(() -> userService.joinV2(oAuthInfo.email(), APPLE, oAuthInfo.providerId(),request.deviceType()));
.orElseGet(() -> userRepository.findFirstByEmailOrderByIdAsc(oAuthInfo.email())
.orElseGet(() -> userService.joinV2(oAuthInfo.email(), APPLE, oAuthInfo.providerId(),request.deviceType())));

updateAppleUserEmail(user, oAuthInfo.email());

if (user.getDeviceType() == null) {
user.updateDeviceType(request.deviceType());
}
user.updateDeviceType(request.deviceType());

return generateOauthJwtTokens(user.getEmail(), APPLE, oAuthInfo.providerId());
}
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/ssu/eatssu/domain/user/entity/User.java
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,6 @@ public void updateDepartment(Department department) {
this.department = department;
}

// 회원 가입 v2 마이그레이션을 위한 메서드 (원래는 가입시 DeviceType을 받아야 합니다.)
// 추후에 기종이 바뀌더라도 업데이트합니다. + V1-> V2 마이그레이션을 수행하는 역할을 하기도 합니다.

Choose a reason for hiding this comment

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

medium

리포지토리 스타일 가이드 규칙 #8에 따라 주석에 작성자 이름과 날짜를 포함해야 합니다.

References
  1. Each comment must include the author's name and the date. (link)

public void updateDeviceType(DeviceType deviceType) { this.deviceType = deviceType; }
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,7 @@ public interface UserRepository extends JpaRepository<User, Long> {

boolean existsByNickname(String nickname);

Optional<User> findByEmail(String email);

Optional<User> findByProviderId(String providerId);

Optional<User> findByNickname(String nickname);
Optional<User> findFirstByEmailOrderByIdAsc(String email);
}
4 changes: 2 additions & 2 deletions src/main/resources/application-dev.yml
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,8 @@ spring:
jwt:
secret:
key: ${EATSSU_JWT_SECRET_DEV}
token-validity-in-seconds: 86400
refresh-token-validity-in-seconds: 604800
token-validity-in-seconds: 60
refresh-token-validity-in-seconds: 180

#S3
cloud:
Expand Down