Skip to content

[Feat] 회원가입 시 필수 약관 동의 추가 #171

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.

Sign up for GitHub

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

Open
chaechaen wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

[Feat] 회원가입 시 필수 약관 동의 추가 #171

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
3 changes: 2 additions & 1 deletion src/main/java/checkmo/authentication/AuthenticationEvent.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
package checkmo.authentication;

import java.util.List;
import lombok.Builder;

public class AuthenticationEvent {

@Builder
public record CreateMember(String id, String email) {
public record CreateMember(String id, String email, List<Long> agreedTermsIds) {
}
Comment on lines 8 to 10
Copy link

@coderabbitai coderabbitai bot Feb 11, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

rg -n "CreateMember" --type java -C3

Repository: checkmo2025/BE

Length of output: 3426

🏁 Script executed:

rg -n "memberCommandService.createMember|createMember\(" --type java -C5 src/main/java/checkmo/member/

Repository: checkmo2025/BE

Length of output: 2314

🏁 Script executed:

rg -n "validateRequiredTerms" --type java -A10 src/main/java/checkmo/member/

Repository: checkmo2025/BE

Length of output: 2758

Fix social login signup: supply agreedTermsIds in CustomOAuth2UserService.CreateMember builder.

Social login flow in CustomOAuth2UserService (line 56) publishes CreateMember without agreedTermsIds, causing it to be null. The event listener then calls MemberCommandService.createMember(), which validates required terms at line 74: if (agreedTermsIds == null || !agreedTermsIds.containsAll(requiredTermsIds)) throws MemberException. This breaks the social login signup flow. Either supply the agreed terms list (if social login should require terms agreement) or an empty list (if social login should skip this validation).

🤖 Prompt for AI Agents 
In `@src/main/java/checkmo/authentication/AuthenticationEvent.java` around lines 8
- 10, The CreateMember event built in CustomOAuth2UserService omits
agreedTermsIds, causing MemberCommandService.createMember's validation (if
(agreedTermsIds == null || !agreedTermsIds.containsAll(requiredTermsIds))) to
throw; fix by supplying a non-null list when constructing
CustomOAuth2UserService.CreateMember (either the actual agreed terms if social
login should require them or an explicit empty list like
Collections.emptyList()/List.of() to skip validation), updating the builder call
in CustomOAuth2UserService to set agreedTermsIds accordingly so
MemberCommandService.createMember receives a non-null value.


@Builder
Expand Down
3 changes: 1 addition & 2 deletions src/main/java/checkmo/authentication/internal/service/command/AuthUserCommandService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ public class AuthUserCommandService {
private final ApplicationEventPublisher eventPublisher;

public AuthUser signUp(AuthRequestDTO.SignUp request) {
// TODO: Member 모듈의 API를 통해 필수 약관 동의 여부 체크

// 이메일 중복 확인
if (authRepository.existsByEmail(request.getEmail())) {
Expand All @@ -54,7 +53,7 @@ public AuthUser signUp(AuthRequestDTO.SignUp request) {
AuthenticationEvent.CreateMember.builder()
.id(savedUser.getId())
.email(savedUser.getEmail())
// TODO: 동의한 약관 ID 리스트 보냄
.agreedTermsIds(request.getAgreedTermsIds())
.build());

return newUser;
Expand Down
5 changes: 5 additions & 0 deletions src/main/java/checkmo/authentication/web/dto/AuthRequestDTO.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,10 @@
import io.swagger.v3.oas.annotations.media.Schema;
import jakarta.validation.constraints.Email;
import jakarta.validation.constraints.NotBlank;
import jakarta.validation.constraints.NotEmpty;
import jakarta.validation.constraints.Pattern;
import jakarta.validation.constraints.Size;
import java.util.List;
import lombok.AllArgsConstructor;
import lombok.Getter;
import lombok.NoArgsConstructor;
Expand All @@ -24,6 +26,9 @@ public static class SignUp {
@Pattern(regexp = "^(?=.*[a-zA-Z])(?=.*[!@#$%^&*]).*$", message = "비밀번호는 영어 및 특수문자를 포함해야 합니다")
@Schema(description = "비밀번호(영어+특수문자 포함 6~12자", example = "pass123!")
private String password;

@NotEmpty(message = "필수 약관 동의 정보가 필요합니다.")
private List<Long> agreedTermsIds;
}

@Getter
Expand Down
6 changes: 5 additions & 1 deletion src/main/java/checkmo/member/internal/exception/MemberErrorStatus.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,11 @@ public enum MemberErrorStatus implements BaseErrorCode {
MULTIPLE_ACCOUNTS_FOUND(HttpStatus.BAD_REQUEST, "MEMBER_409", "해당 정보로 가입된 계정이 여러 개입니다. 관리자에게 문의해주세요."),

// 신고
CANNOT_REPORT_SELF(HttpStatus.BAD_REQUEST, "REPORT_400", "자기 자신을 신고할 수 없습니다.");
CANNOT_REPORT_SELF(HttpStatus.BAD_REQUEST, "REPORT_400", "자기 자신을 신고할 수 없습니다."),

// 약관
TERMS_NOT_FOUND(HttpStatus.NOT_FOUND, "TERMS_400", "해당 약관을 찾을 수 없습니다."),
REQUIRED_TERMS_NOT_AGREE(HttpStatus.BAD_REQUEST, "TERMS_401", "필수 약관에 동의해야 회원가입이 가능합니다.");

private final HttpStatus httpStatus;
private final String code;
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/checkmo/member/internal/listener/MemberEventListener.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,6 @@ public class MemberEventListener {
@EventListener
@Transactional(propagation = Propagation.MANDATORY)
public void createMember(AuthenticationEvent.CreateMember event) {
memberCommandService.createMember(event.id(), event.email());
memberCommandService.createMember(event.id(), event.email(), event.agreedTermsIds());
}
}
8 changes: 8 additions & 0 deletions src/main/java/checkmo/member/internal/repository/MemberTermsRepository.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
package checkmo.member.internal.repository;

import checkmo.member.internal.entity.MemberTerms;
import org.springframework.data.jpa.repository.JpaRepository;

public interface MemberTermsRepository extends JpaRepository<MemberTerms, Long> {

}
8 changes: 8 additions & 0 deletions src/main/java/checkmo/member/internal/repository/TermsRepository.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
package checkmo.member.internal.repository;

import checkmo.member.internal.entity.Terms;
import org.springframework.data.jpa.repository.JpaRepository;

public interface TermsRepository extends JpaRepository<Terms, Long> {

}
38 changes: 36 additions & 2 deletions src/main/java/checkmo/member/internal/service/command/MemberCommandService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,17 @@
import checkmo.member.MemberEvent;
import checkmo.member.internal.converter.MemberConverter;
import checkmo.member.internal.entity.Member;
import checkmo.member.internal.entity.MemberTerms;
import checkmo.member.internal.entity.Terms;
import checkmo.member.internal.exception.MemberErrorStatus;
import checkmo.member.internal.exception.MemberException;
import checkmo.member.internal.repository.MemberRepository;
import checkmo.member.internal.repository.MemberTermsRepository;
import checkmo.member.internal.repository.TermsRepository;
import checkmo.member.web.dto.MemberRequestDTO;
import checkmo.member.web.dto.MemberResponseDTO.DetailInfo;
import java.util.HashSet;
import java.util.List;
import lombok.RequiredArgsConstructor;
import org.springframework.context.ApplicationEventPublisher;
import org.springframework.stereotype.Service;
Expand All @@ -26,7 +31,13 @@ public class MemberCommandService {

private final ApplicationEventPublisher eventPublisher;

public void createMember(String memberId, String email) {
private final MemberTermsRepository memberTermsRepository;

private final TermsRepository termsRepository;

public void createMember(String memberId, String email, List<Long> agreedTermsIds) {
validateRequiredTerms(agreedTermsIds);
Comment on lines +38 to +39
Copy link

@coderabbitai coderabbitai bot Feb 11, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Consider deduplicating agreedTermsIds at entry to prevent duplicate MemberTerms rows.

If the caller passes [1, 1, 2], two MemberTerms rows are created for terms 1. Dedup early:

-    public void createMember(String memberId, String email, List<Long> agreedTermsIds) {
-        validateRequiredTerms(agreedTermsIds);
+    public void createMember(String memberId, String email, List<Long> agreedTermsIds) {
+        List<Long> distinctTermsIds = agreedTermsIds != null
+                ? agreedTermsIds.stream().distinct().toList()
+                : null;
+        validateRequiredTerms(distinctTermsIds);

Then use distinctTermsIds for the rest of the method.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
public void createMember(String memberId, String email, List<Long> agreedTermsIds) {
validateRequiredTerms(agreedTermsIds);
public void createMember(String memberId, String email, List<Long> agreedTermsIds) {
List<Long> distinctTermsIds = agreedTermsIds != null
? agreedTermsIds.stream().distinct().toList()
: null;
validateRequiredTerms(distinctTermsIds);
🤖 Prompt for AI Agents 
In
`@src/main/java/checkmo/member/internal/service/command/MemberCommandService.java`
around lines 38 - 39, In createMember, deduplicate the incoming agreedTermsIds
immediately (e.g., produce a distinctTermsIds collection) and use that for
validation and all subsequent logic so you don't create duplicate MemberTerms
rows; call validateRequiredTerms(distinctTermsIds) (instead of
validateRequiredTerms(agreedTermsIds)) and replace further usages of
agreedTermsIds in MemberTerms creation and related code with distinctTermsIds to
ensure each term results in at most one MemberTerms entry.


Member member = Member.builder()
.id(memberId)
.email(email)
Expand All @@ -39,7 +50,30 @@ public void createMember(String memberId, String email) {

memberRepository.save(member);

// TODO: 여기서 약관 내역 DB에 저장 (멤버 생성 후)
// 여기서 약관 내역 DB에 저장 (멤버 생성 후)
for (Long termsId : agreedTermsIds) {
Terms terms = termsRepository.findById(termsId)
.orElseThrow(() -> new MemberException(MemberErrorStatus.TERMS_NOT_FOUND));

MemberTerms memberTerms = MemberTerms.builder()
.member(member)
.terms(terms)
.isAgreed(true)
.build();

memberTermsRepository.save(memberTerms);
}
Comment on lines +54 to +65
Copy link

@gemini-code-assist gemini-code-assist bot Feb 11, 2026

Choose a reason for hiding this comment

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

security-high high

이 코드는 서비스 거부(DoS) 공격에 취약합니다. createMember 메서드는 사용자 제어 하에 있는 agreedTermsIds 목록을 반복하며 각 ID에 대해 데이터베이스 쿼리를 수행합니다. 공격자가 매우 많은 수의 ID를 제공하면 과도한 데이터베이스 쿼리가 발생하여 데이터베이스 리소스가 고갈되고 애플리케이션이 다른 사용자에게 제공되지 않을 수 있습니다. 현재 약관 동의 내역을 저장하는 로직은 동의한 약관의 수만큼 데이터베이스 조회가 발생하여 N+1 문제가 발생할 수 있습니다. findAllById와 saveAll을 사용하여 여러 약관을 한 번에 조회하고 저장하는 것이 성능에 더 효율적입니다.

        List<Terms> allTerms = termsRepository.findAllById(agreedTermsIds);
        if (allTerms.size() != agreedTermsIds.size()) {
            throw new MemberException(MemberErrorStatus.TERMS_NOT_FOUND);
        }

        List<MemberTerms> memberTermsToSave = allTerms.stream()
            .map(term -> MemberTerms.builder()
                .member(member)
                .terms(term)
                .isAgreed(true)
                .build())
            .collect(Collectors.toList());

        memberTermsRepository.saveAll(memberTermsToSave);

}

private void validateRequiredTerms(List<Long> agreedTermsIds) {
List<Long> requiredTermsIds = termsRepository.findAll().stream()
.filter(Terms::isRequired)
.map(Terms::getId)
.toList();

if (agreedTermsIds == null || !agreedTermsIds.containsAll(requiredTermsIds)) {
throw new MemberException(MemberErrorStatus.REQUIRED_TERMS_NOT_AGREE);
}
Comment on lines +69 to +76
Copy link

@gemini-code-assist gemini-code-assist bot Feb 11, 2026

Choose a reason for hiding this comment

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

high

필수 약관을 검증하는 로직이 비효율적입니다.

  1. termsRepository.findAll()은 모든 약관을 DB에서 조회하므로 약관이 많아지면 성능 저하를 유발할 수 있습니다. TermsRepository에 findAllByIsRequired(true)와 같은 메서드를 추가하여 DB에서 직접 필터링하는 것이 좋습니다.
  2. List.containsAll은 시간 복잡도가 O(n*m)으로 비효율적입니다. agreedTermsIds를 HashSet으로 변환하여 containsAll을 호출하면 평균 시간 복잡도를 O(n+m)으로 개선할 수 있습니다.
        List<Long> requiredTermsIds = termsRepository.findAllByIsRequired(true).stream()
                                                     .map(Terms::getId)
                                                     .toList();

        if (agreedTermsIds == null || !new HashSet<>(agreedTermsIds).containsAll(requiredTermsIds)) {
            throw new MemberException(MemberErrorStatus.REQUIRED_TERMS_NOT_AGREE);
        }

}

/**
Expand Down
4 changes: 4 additions & 0 deletions src/main/resources/db/migration/V20260211__insert_default_terms.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
INSERT INTO terms (id, term_url, is_required) VALUES
(1, 'https://www.notion.so/2cd7445c73f780dab5c5ccfbf5ce6101', true), -- 서비스 이용을 위한 필수 개인정보 수집 이용 동의(필수)
(2, 'https://www.notion.so/2cd7445c73f780e3a653ead96321331f', false), -- 마케팅 및 이벤트 정보 수신 동의 (선택)
(3, 'https://www.notion.so/3-2cd7445c73f7806f9b37f75eea6acb30', false); -- 개인정보 제3자 제공 동의(선택)
Comment on lines +1 to +4
Copy link

@coderabbitai coderabbitai bot Feb 11, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Find and check Terms entity for ID generation strategy
find . -type f -name "Terms.java" | head -5 | xargs -I {} sh -c 'echo "=== File: {} ===" && cat -n {}'

Repository: checkmo2025/BE

Length of output: 1048

🏁 Script executed:

#!/bin/bash
# Find CREATE TABLE migration for terms
rg -n "CREATE TABLE" --type sql -A10 | grep -A10 "terms"

Repository: checkmo2025/BE

Length of output: 2687

🏁 Script executed:

#!/bin/bash
# Check the exact last character(s) of the V20260211 file to confirm newline
hexdump -C src/main/resources/db/migration/V20260211__insert_default_terms.sql | tail -3

Repository: checkmo2025/BE

Length of output: 103

🏁 Script executed:

#!/bin/bash
# Check trailing newline using od
tail -c 20 src/main/resources/db/migration/V20260211__insert_default_terms.sql | od -c

Repository: checkmo2025/BE

Length of output: 100

🏁 Script executed:

#!/bin/bash
# Check if other seed data migrations also use explicit IDs
rg -n "INSERT INTO" --type sql -A1 src/main/resources/db/migration/ | head -40

Repository: checkmo2025/BE

Length of output: 966

🏁 Script executed:

#!/bin/bash
# Check trailing newline using tail with -c and wc
file="src/main/resources/db/migration/V20260211__insert_default_terms.sql"
last_char=$(tail -c 1 "$file" | wc -l)
echo "Lines at end: $last_char"
# Also show last few characters using sed
echo "Last 5 chars:"
tail -c 5 "$file" | sed 's/.$/[END]/g' | cat -A

Repository: checkmo2025/BE

Length of output: 107

🏁 Script executed:

#!/bin/bash
# Check all INSERT migrations to see if hardcoded IDs are the standard pattern
find src/main/resources/db/migration -name "*.sql" -type f -exec grep -l "INSERT INTO" {} \; | while read f; do
  echo "=== $f ===" 
  grep -A2 "INSERT INTO" "$f" | head -6
done

Repository: checkmo2025/BE

Length of output: 863

Add trailing newline to the file; reconsider hardcoded IDs if sequence management is a project concern.

The file is missing a trailing newline (line 4 ends without one).

Regarding the hardcoded IDs (1, 2, 3): The Terms entity uses GenerationType.IDENTITY with AUTO_INCREMENT on the id column. Explicitly inserting IDs can technically advance the sequence; however, for static reference data like these three terms, hardcoded IDs are a standard pattern to ensure consistency across environments. This is only problematic if the application later inserts Terms via JPA without specifying IDs and sequence management becomes inconsistent. If your project avoids explicit IDs in seed migrations, consider using IDENTITY generation without pre-set values or resetting the sequence after insertion.

🤖 Prompt for AI Agents 
In `@src/main/resources/db/migration/V20260211__insert_default_terms.sql` around
lines 1 - 4, Add a trailing newline to the migration file so it ends with a
newline character and ensure the INSERT INTO terms statement is terminated
properly; additionally, if your project prefers not to hardcode PKs, remove the
explicit id values from the INSERT INTO terms (...) VALUES (...) (or adjust to
use DEFAULT for id) so GenerationType.IDENTITY/AUTO_INCREMENT manages IDs, or
add a sequence-reset statement after inserting fixed rows to keep auto-increment
in sync — update the INSERT INTO terms and any post-insert sequence maintenance
accordingly.