[BE] 회원가입 기능을 구현한다. #774

tkdgur0906 · 2024-10-09T08:46:48Z

❗ Issue

[BE] 회원가입 기능을 구현한다. #729

✨ 구현한 기능

이메일, 비밀번호 클래스 분리
비밀번호 암호화
회원가입

📢 논의하고 싶은 내용

비밀번호 암호화 시 필요한 Salt 생성 방법

현재 비밀번호 암호화 시 Salt가 필요한데, 사용자마다 고유한 값이 필요해 이메일을 사용해 생성하고 있습니다.
그래서 PasswordEncoder의 encode() 호출시 email, password를 넘겨야합니다.
하지만 비밀번호 정책을 검증하는 책임이 Password 클래스에 있다보니, 순수한 비밀번호 값을 생성자에 넘겨야합니다.
그래서 Password 클래스가 PasswordEncoder를 필드로 가지고 있게 되는데, 암호화 시 이메일이 필요해서 Password 생성자에 email을 같이 넘겨야 되는 상황입니다.

public Password(String email, String value) {
        validatePassword(value);
        this.value = passwordEncoder.encode(email, value);
    }

개선 가능 방안은 다음과 같습니다.

Salt를 email을 통해 생성하는 것이 아닌 랜덤 생성으로 바꾼다.
랜덤 생성 객체는 PasswordEncoder 내부 필드로 가지고 있을 것 같습니다.

추가적인 다른 의견 있으시면 논의하면 좋을 것 같습니다.

비밀번호 암호화 시 필요한 Salt 저장 방법

만약 이메일 대신 랜덤하고 유저마다 고유한 값으로 Salt를 생성한다면 저장할 방법이 필요합니다.
세가지 방안을 생각했습니다.

password와 같이 저장

db에 비밀번호가 vI8aWBnW3fID.ZQ4/zo1G.q1lRps.9cGLcZEiGDMVr5yUP1KUOYTa 같이 저장됐을 시 처음 22개 문자는 salt값, 나머지는 암호화된 비밀번호입니다.
salt값은 16바이트 값으로 디코딩하여 다시 사용하는 방식입니다.
해당 방식이 Spring Security에서 Salt를 관리하는 방식이라고 합니다.

User 테이블 컬럼으로 Salt 추가
Salt 테이블 따로 생성해 저장

Salt 생성 시 email을 사용하지 않도록 바뀐다면 고민해봐야할 것 같습니다.

회원가입 동시성 처리

현재 코드에서 회원 가입시 같은 이메일로 요청이 동시에 온다면 둘다 회원가입 되는 문제가 발생할 수 있습니다.

@Transactional
    public Long register(RegisterRequestV1 request) {
        if (userRepository.existsByEmailAndUserType(new Email(request.email()), LoginType.LOCAL)) {
            throw new BangggoodException(ExceptionCode.USER_EMAIL_ALREADY_USED);
        }

        return userRepository.save(request.toUserEntity()).getId();
    }

User 테이블의 email과 LoginType을 같이 unique 제약조건을 설정하면 해당 문제를 해결할 수 있습니다.
만약 제약 조건 추가시 코드는 다음과 같이 작성할 수 있을 것 같습니다.

@Transactional
public void register(RegisterRequestV1 request) {
    try {
        userRepository.save(request.toUserEntity()).getId();
    } catch (DataIntegrityViolationException e) {
        // 유니크 제약 조건 위반 시 처리
        throw new BangggoodException(ExceptionCode.USER_EMAIL_ALREADY_USED);
    }
}

현재 테이블에 유니크 제약조건이 안걸려 있기 때문에 제약조건 거는 것에 대해 의견 부탁드립니다.

🎸 기타

shin-jisong

수고하셨습니다 카피!

비밀번호 암호화 시 필요한 Salt
찾아보니 이메일로 salt를 생성하는 건 보안적으로 위험한 방법이라고 하더라고요
따라서 랜덤으로 생성한 뒤에 구분자를 통해서 비밀번호와 salt를 분리해서 저장하는 것이 좋을 것 같아요
이때 제가 생각하는 문제점은 password에 해당 구분자가 들어가는 경우가 있을 수 있어서 저희가 구분자로 사용하는 문자는 password에서 사용할 수 없도록 정책을 지정해야 될 것 같습니다

제약 조건
저는 PASSWORD랑 LoginType 유니크 제약 조건 좋습니다~

shin-jisong · 2024-10-10T01:15:51Z

backend/bang-ggood/src/main/java/com/bang_ggood/auth/controller/AuthController.java

-        this.authService = authService;
-        this.cookieProvider = cookieProvider;
-        this.cookieResolver = cookieResolver;
+    @PostMapping("/v1/register")


oauth와 구분되는 path를 사용하는 것이 좋을 것 같은데 /v1/local-auth/register 어떨까요?
혹은 다른 적절한 path가 있다면 좋을 것 같네요!

반영했습니다!

shin-jisong · 2024-10-10T01:19:11Z

backend/bang-ggood/src/main/java/com/bang_ggood/user/domain/Password.java

+public class Password {
+
+    private static final Pattern PASSWORD_PATTERN =
+            Pattern.compile("^(?=.*[A-Za-z])(?=.*\\d)[A-Za-z\\d]{6,}$");


사용한 PASSWORD 패턴 전략도 본문에 적어 주시면 좋을 거서 같아요!

shin-jisong · 2024-10-10T01:20:21Z

backend/bang-ggood/src/main/java/com/bang_ggood/user/repository/UserRepository.java

+            + "WHERE u.email = :email "
+            + "AND u.loginType = :loginType "
+            + "AND u.deleted = false")
+    boolean existsByEmailAndUserType(Email email, LoginType loginType);


저희 쿼리문에서는 JPA 사용하지 않고 실제 쿼리를 작성하기로 했어서 value값으로 조회해야 될 것 같아요!

확인해 봤는데 그렇게 작성하려면 nativeQuery 옵션을 켜야 되더라고요!
ENUM에 대해서는 따로 값을 꺼낼 필요는 없을 것 같아요~
실제 쿼리는 Id 같은 부분에 대해서만 적용하면 될 것 같습니다

(Email email, LoginType loginType) 대신에 (String email, LoginType loginType)로 받으라는 것인가요??
일단 해담 메서드가 로직이 바뀜에 따라 사용되지 않아 삭제했습니다!

shin-jisong · 2024-10-10T01:21:01Z

backend/bang-ggood/src/main/resources/data.sql

-INSERT INTO users(name, email, user_type, login_type, created_at, modified_at, deleted)
-VALUES ('방방이', '[email protected]', 'USER', 'LOCAL', '2024-07-22 07:56:42', '2024-07-22 07:56:42', false);
+INSERT INTO users(name, email, password, user_type, login_type, created_at, modified_at, deleted)
+VALUES ('방방이', '[email protected]', 'password1234', 'USER', 'LOCAL', '2024-07-22 07:56:42', '2024-07-22 07:56:42', false);


이거 VALUE 넣을 때 암호화된 값으로 넣어야 될 것 같아요!

shin-jisong · 2024-10-10T01:23:11Z

backend/bang-ggood/src/test/java/com/bang_ggood/auth/service/AuthServiceTest.java

+
+        //then
+        User findUser = userRepository.findById(userId).orElseThrow();
+        assertThat(findUser.getPassword().getValue()).isNotEqualTo(password);


서비스 테스트니까 암호화한 결과값이랑 같은지 보는 게 좋을 것 같아요!

내부에서 salt를 생성하기 때문에 암호화한 결과값을 동일하게 생성하는게 마땅치 않은 것 같네요
PasswordEncoder의 메서드 시그니처를 바꾸지 않는 이상 테스트 하기가 어려워 현재로써는 방법이 떠오르지 않아서 일단 유지했습니다!

Salt를 주입받는 식으로 변경해서 값이 같은지에 대한 테스트로 변경했습니다!!!

shin-jisong · 2024-10-10T01:24:03Z

backend/bang-ggood/src/test/java/com/bang_ggood/user/domain/EmailTest.java

+import static org.assertj.core.api.Assertions.assertThatCode;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
+class EmailTest {


테스트를 꼼꼽하게 해 주셨네요 👍

tsulocalize · 2024-10-10T01:27:29Z

랜덤에 한표 드립니다. 랜덤도 요소를 여러가지 섞으면 pseudo-random으로 보이게 할 수 있을 겁니다.
password와 함께 저장 한표. 필요한 곳에 데이터가 있어야 된다고 생각하기 때문
유니크 거시죠~

JINU-CHANG

카피 전체적으로 잘 구현해주셨네요 👍

랜덤으로 Salt 생성하는게 안전할 것 같아요.
하나의 암호화된 문자열이라는 생각이 들어 굳이 따로 테이블을 빼지 않고 password와 같이 저장하는게 좋을 것 같아요.
안 그래도 필요했는데 유니크 제약조건 좋습니다!

JINU-CHANG · 2024-10-10T01:26:24Z

backend/bang-ggood/src/main/java/com/bang_ggood/user/domain/Email.java

+    public void validateEmail(String email) {
+        if(!EMAIL_PATTERN.matcher(email).matches()) {
+            throw new BangggoodException(ExceptionCode.EMAIL_INVALID_FORMAT);
+        }
+    }


Suggested change

public void validateEmail(String email) {

if(!EMAIL_PATTERN.matcher(email).matches()) {

throw new BangggoodException(ExceptionCode.EMAIL_INVALID_FORMAT);

}

}

public void validateEmailPattern(String email) {

if(!EMAIL_PATTERN.matcher(email).matches()) {

throw new BangggoodException(ExceptionCode.EMAIL_INVALID_FORMAT);

}

}

아래 Password 포함해서 무엇을 검증하는지 이름으로 바로 알 수 있으면 좋겠어요!

JINU-CHANG · 2024-10-10T01:27:12Z

backend/bang-ggood/src/main/java/com/bang_ggood/auth/service/PasswordEncoder.java

+            return Base64.getEncoder().encodeToString(hash);
+        } catch (NoSuchAlgorithmException | UnsupportedEncodingException |
+                 InvalidKeySpecException e) {
+            throw new RuntimeException(e);


여기는 왜 커스텀 예외로 던지지 않나용?

해시 에러로 바꿔서 던졌습니다!

JINU-CHANG · 2024-10-10T01:45:16Z

backend/bang-ggood/src/main/java/com/bang_ggood/user/domain/Password.java

+
+    private static final Pattern PASSWORD_PATTERN =
+            Pattern.compile("^(?=.*[A-Za-z])(?=.*\\d)[A-Za-z\\d]{6,}$");
+    private static final PasswordEncoder passwordEncoder = new PasswordEncoder();


PasswordEncoder가 이미 빈에 등록되어 있는데 계속 생성하도록 구현한 이유가 있나요??
PasswordEncoder.encode()값으로 Password 객체를 생성하는게 더 자연스러울 것 같아요!

PasswordEncoder를 static 클래스로 변경하였습니다!

JINU-CHANG · 2024-10-10T01:46:39Z

backend/bang-ggood/src/main/java/com/bang_ggood/user/domain/Email.java

+    private static final Pattern EMAIL_PATTERN =
+            Pattern.compile("^[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\\.[a-zA-Z]{2,}$");


주석 사용을 지양하긴 하지만, 빠르게 이해하기 쉽게 주석으로 간단히 설명 적어주면 좋을 것 같아요~

# Conflicts: # backend/bang-ggood/src/test/java/com/bang_ggood/auth/service/AuthServiceTest.java

shin-jisong · 2024-10-11T01:03:11Z

backend/bang-ggood/src/main/java/com/bang_ggood/auth/service/PasswordEncoder.java

+@Component
+public class PasswordEncoder {
+
+    public String encode(String password) {


카피! 해당 부분 잘 바꿔주셨는데 추후 재사용성을 고려하여 encode 로직에서 salt를 받아야 될 것 같은데 어떻게 생각하시나요? 🤔

좋은 방법인 것 같아요!
encode를 외부에서 salt를 받도록 변경했습니다!

shin-jisong

수고하셨습니다~~!!

tsulocalize

沐雨櫛風(목우즐풍)하셨습니다!

tsulocalize · 2024-10-11T03:08:40Z

backend/bang-ggood/src/main/java/com/bang_ggood/user/domain/Password.java

+
+    public Password(String value) {
+        validatePasswordPattern(value);
+        this.value = passwordEncoder.encode(value, passwordEncoder.getSalt());


static으로 만들어도 무방할 것 같은데, 추후에 bean이 필요한 상황을 생각하고 있다면 반영하지 않으셔도 됩니다.

encode(String value) 메서드도 있으면 사용하기 편할 것 같아요~

빈에 대한 판단은 로그인 PR에서 반영하는 게 좋을 것 같아 그때 생각해보겠습니다!

encodeWithGeneralSalt(String password) 메서드를 만들어서 따로 처리했습니다!

tsulocalize · 2024-10-11T03:10:24Z

backend/bang-ggood/src/main/resources/schema.sql

+    password    VARCHAR(255),
+    user_type   VARCHAR(255) NOT NULL,
+    login_type  VARCHAR(255) NOT NULL,
    created_at  TIMESTAMP(6),
    modified_at TIMESTAMP(6),
-    deleted     BOOLEAN
+    deleted     BOOLEAN,
+    CONSTRAINT unique_email_login_type UNIQUE (email, login_type)


prod 환경에 배포할 때 데이터 정합성 일치하도록 주의해주세요!

tkdgur0906 added 6 commits October 7, 2024 17:33

feat: PasswordEncoder 구현

f898fbb

test: e2e 테스트 헤더 로직 통일

633fa50

feat: User에 Password 필드 추가 및 형식 검증

06c633c

feat: password 컬럼 sql 설정에 추가

6e5fda6

feat: email 클래스 생성하여 역할 분리

fe6ab1c

feat: 회원가입 기능 구현

47bc872

tkdgur0906 added the 🔒 BE label Oct 9, 2024

tkdgur0906 added this to the 최종 데모데이 milestone Oct 9, 2024

tkdgur0906 requested review from shin-jisong, tsulocalize and JINU-CHANG October 9, 2024 08:46

tkdgur0906 self-assigned this Oct 9, 2024

shin-jisong requested changes Oct 10, 2024

View reviewed changes

JINU-CHANG requested changes Oct 10, 2024

View reviewed changes

tkdgur0906 added 11 commits October 10, 2024 18:01

feat: User의 email, loginType 유니크 제약조건 추가

8380e95

feat: 동일한 이메일로 회원가입 하지 못하도록 구현

1493ad1

feat: salt 랜덤으로 생성하도록 변경

8e08cb3

refactor: 회원가입 엔드포인트 변경

28135cf

style: 이메일, 비밀번호 정책 주석 추가

b00c448

feat: data.sql 비밀번호 암호화하여 삽입

935ee5b

test: 중복 테스트 삭제

bc0484b

refactor: Email, Password 검증 메서드명 명확히 수정

d900f0f

feat: PasswordEncoder 커스텀 예외 처리 적용

233ed59

Merge remote-tracking branch 'origin/dev-be' into feat/729-register

fc5ff92

# Conflicts: # backend/bang-ggood/src/test/java/com/bang_ggood/auth/service/AuthServiceTest.java

refactor: 미사용 db 메서드 제거

4a412db

shin-jisong requested changes Oct 11, 2024

View reviewed changes

refactor: PasswordEncoder encode시 외부에서 salt 주입받도록 변경

e09660c

shin-jisong approved these changes Oct 11, 2024

View reviewed changes

tsulocalize approved these changes Oct 11, 2024

View reviewed changes

tkdgur0906 added 2 commits October 11, 2024 13:35

feat: 기본 salt로 encode하는 메서드 추가

4f10903

refactor: PasswordEncoder 정적 클래스로 변경

9b29556

JINU-CHANG approved these changes Oct 11, 2024

View reviewed changes

JINU-CHANG merged commit 351a7d9 into dev-be Oct 11, 2024
2 checks passed

JINU-CHANG deleted the feat/729-register branch October 11, 2024 05:19

tkdgur0906 added a commit that referenced this pull request Oct 15, 2024

[BE] 회원가입 기능을 구현한다. (#774)

26e599d

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[BE] 회원가입 기능을 구현한다. #774

[BE] 회원가입 기능을 구현한다. #774

tkdgur0906 commented Oct 9, 2024

shin-jisong left a comment •

edited

Loading

shin-jisong Oct 10, 2024

tkdgur0906 Oct 10, 2024

shin-jisong Oct 10, 2024

shin-jisong Oct 10, 2024

shin-jisong Oct 10, 2024

tkdgur0906 Oct 10, 2024 •

edited

Loading

shin-jisong Oct 10, 2024

shin-jisong Oct 10, 2024

tkdgur0906 Oct 10, 2024

tkdgur0906 Oct 11, 2024

shin-jisong Oct 10, 2024

tsulocalize commented Oct 10, 2024 •

edited

Loading

JINU-CHANG left a comment

JINU-CHANG Oct 10, 2024

JINU-CHANG Oct 10, 2024

tkdgur0906 Oct 10, 2024

JINU-CHANG Oct 10, 2024

tkdgur0906 Oct 10, 2024 •

edited

Loading

JINU-CHANG Oct 10, 2024

shin-jisong Oct 11, 2024

tkdgur0906 Oct 11, 2024

shin-jisong left a comment

tsulocalize left a comment

tsulocalize Oct 11, 2024

tkdgur0906 Oct 11, 2024

tsulocalize Oct 11, 2024

		private static final Pattern EMAIL_PATTERN =
		Pattern.compile("^[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\\.[a-zA-Z]{2,}$");

[BE] 회원가입 기능을 구현한다. #774

[BE] 회원가입 기능을 구현한다. #774

Conversation

tkdgur0906 commented Oct 9, 2024

❗ Issue

✨ 구현한 기능

📢 논의하고 싶은 내용

비밀번호 암호화 시 필요한 Salt 생성 방법

비밀번호 암호화 시 필요한 Salt 저장 방법

회원가입 동시성 처리

🎸 기타

shin-jisong left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tkdgur0906 Oct 10, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tsulocalize commented Oct 10, 2024 • edited Loading

JINU-CHANG left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tkdgur0906 Oct 10, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shin-jisong left a comment

Choose a reason for hiding this comment

tsulocalize left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shin-jisong left a comment •

edited

Loading

tkdgur0906 Oct 10, 2024 •

edited

Loading

tsulocalize commented Oct 10, 2024 •

edited

Loading

tkdgur0906 Oct 10, 2024 •

edited

Loading