[BE] 일반 로그인을 구현한다. #775

shin-jisong · 2024-10-10T04:06:11Z

❗ Issue

[BE] 일반 로그인을 구현한다. #755

✨ 구현한 기능

일반 로그인 구현

📢 논의하고 싶은 내용

#774에서 반영되는 내용에 따라 코드 수정이 많을 듯합니다!
먼저 머지되고 관련 내용 합쳐야 될 것 같습니다 🥲

🎸 기타

# Conflicts: # backend/bang-ggood/src/main/java/com/bang_ggood/auth/controller/AuthController.java # backend/bang-ggood/src/main/java/com/bang_ggood/checklist/controller/ChecklistController.java # backend/bang-ggood/src/main/java/com/bang_ggood/global/config/WebMvcConfig.java # backend/bang-ggood/src/test/java/com/bang_ggood/AcceptanceTest.java # backend/bang-ggood/src/test/java/com/bang_ggood/auth/JwtTokenFixture.java # backend/bang-ggood/src/test/java/com/bang_ggood/auth/controller/cookie/CookieResolverTest.java # backend/bang-ggood/src/test/java/com/bang_ggood/auth/service/AuthServiceTest.java # backend/bang-ggood/src/test/java/com/bang_ggood/station/service/SubwayStationServiceTest.java

JINU-CHANG

시소 고생많으셨습니다 👍
전반적으로 잘 작성해주셔서 사소한 코멘트 몇가지만 남겨보았어요!

JINU-CHANG · 2024-10-10T23:58:07Z

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


    @Transactional
-    public AuthTokenResponse login(OauthLoginRequest request) {
+    public AuthTokenResponse authLogin(OauthLoginRequest request) {


Suggested change

public AuthTokenResponse authLogin(OauthLoginRequest request) {

public AuthTokenResponse outhLogin(OauthLoginRequest request) {

JINU-CHANG · 2024-10-10T23:58:27Z

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

+        User user = userRepository.findByEmailAndLoginType(oauthInfoApiResponse.kakao_account().email(),
+                        LoginType.KAKAO)


👍 함께 변경해주셨네요

JINU-CHANG · 2024-10-11T00:03:22Z

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

+        return AuthTokenResponse.of(accessToken, refreshToken);
+    }
+
+    private void checkPassword(LocalLoginRequestV1 request, User user) {


Suggested change

private void checkPassword(LocalLoginRequestV1 request, User user) {

private void validatePassword(LocalLoginRequestV1 request, User user) {

지금까지는 검증 메서드를 validate라고 했던 것 같아서 제안드려봅니다~!

해당 부분은 정책에 따라 규칙에 맞는가 확인하는 것이 아닌 맞는가 체크하는 부분이라 생각해서 validate 명명보다는 비즈니스 로직 명명을 가질 수 있도록 구현하였습니다!

제가 이해한 바에 따르면, 단순히 비밀번호가 다른지 체크하는 부분은 정책적인 부분이 아니라 네이밍을 다르게 가져가고 싶다는 의미로 이해했어요! 하지만 null, empty 체크도 비슷한 경우라고 생각되는데 validate 메서드명을 사용했던 것 같아서 이 부분은 어떻게 생각하는지 궁금합니다 ~

null 혹은 empty 체크는 값 자체의 형식에 대한 검증이라 생각해요!
validate 검증 로직은 주로 입력 데이터가 특정 형식, 패턴 또는 규칙에 부합하는지 확인 등 비즈니스 로직을 시작하기 직전에 값을 선별하는 데 사용하는 명명에 가깝다고 생각해요
예를 들어 저장한다는 비즈니스 로직 전 해당 값을 저장하기에 유효한지 검증하는 validate 검증 로직을 작성하는 것처럼요 🤔

반면에 비밀번호 일치 여부를 확인하는 로직은 단순히 입력 데이터의 형식 검증이 아닌, 기존 데이터와 비교하여 중요한 비즈니스 결정을 내리는 과정이라 생각했어요!
validate라는 검증의 의미만 담기에는 다소 의미가 축소되는 느낌이 있다고 판단하였습니다

JINU-CHANG · 2024-10-11T00:04:30Z

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

+    private final PasswordEncoder passwordEncoder;
+    private final UserRepository userRepository;
+
+    private static void validateTokenOwnership(User user, AuthUser accessAuthUser, AuthUser refreshAuthUser) {


위로 올라온 이유가 있나요??

머지하다가 그렇게 됐네요.... 다시 내려두었습니다 🔽

JINU-CHANG · 2024-10-11T00:09:57Z

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

@@ -100,23 +132,23 @@ void login_default_checklist_question() {
        assertThat(sum).isEqualTo(Question.findDefaultQuestions().size());
    }

-    @DisplayName("로그인 성공 : 회원 가입시 디폴트 체크리스트 추가")
+    @DisplayName("카카오 로그인 성공 : 회원 가입시 디폴트 체크리스트를 추가한다.")


컨벤션이 명사구 형태여서 저번 PR에서 수정했던 기억이 있는데 다시 문장형으로 수정하신 이유가 있나요?-?

이것도 머지하다가... 충돌 해결한다고... 🥲
수정해 두었습니다!

tkdgur0906

고생많으셨습니다!
충돌 해결된 후에 한번더 볼게요!

tkdgur0906 · 2024-10-11T02:54:17Z

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

@@ -51,6 +54,10 @@ public User(Long id, String name, String email) { // TODO 테스트용
        this.email = email;
    }

+    public boolean isDifferentPassword(String password) {
+        return !Objects.equals(this.password, password);


if both arguments are null, true is returned.
password가 둘 다 null이면 true가 반환되는데, 의도되었나요 ?

로직이 바뀌었습니다!

# Conflicts: # backend/bang-ggood/src/main/java/com/bang_ggood/auth/service/AuthService.java # backend/bang-ggood/src/main/java/com/bang_ggood/auth/service/PasswordEncoder.java # backend/bang-ggood/src/main/java/com/bang_ggood/global/DBInitializer.java # backend/bang-ggood/src/main/java/com/bang_ggood/global/exception/ExceptionCode.java # backend/bang-ggood/src/main/java/com/bang_ggood/user/domain/User.java # backend/bang-ggood/src/main/java/com/bang_ggood/user/repository/UserRepository.java # backend/bang-ggood/src/main/resources/data.sql # backend/bang-ggood/src/test/java/com/bang_ggood/auth/service/AuthServiceTest.java # backend/bang-ggood/src/test/java/com/bang_ggood/user/UserFixture.java # backend/bang-ggood/src/test/resources/data-test.sql # backend/bang-ggood/src/test/resources/schema-test.sql

tkdgur0906

잘 수정해주셨네요 고생많으셨어요!!
간단한 코멘트 남겼으니 확인 부탁드립니다~

tkdgur0906 · 2024-10-11T09:23:40Z

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

-    private PasswordEncoder() {
+    private static byte[] extractSaltByPassword(String encodedPassword) {
+        String[] parts = encodedPassword.split(DELIMITER);
+        if (parts.length != 2) {


2도 상수 분리하는 것은 어떤가요?

PASSWORD_AND_SALT_LENGTH로 상수화하였습니다!

tkdgur0906 · 2024-10-11T09:27:06Z

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

@@ -25,11 +25,20 @@ public class Password {
    private String value;

    public Password(String value) {
+        if (value == null || value.isEmpty()) {
+            this.value = value;
+            return;


비밀번호가 비어있어도 null로 들어가야하지 않나요?

각 입력값을 그대로 넣어 주는 것이 좋을 것 같아서 해당 로직으로 작성했습니다
다만 isEmpty에 해당할 경우가 빈 문자열인 "" 에 해당할 것 같은데 null을 넣는 것이 더욱 명확한 코드 전달일 것 같아 null을 넣는 것으로 대체하였습니다!

Password라는 도메인이 null 허용이라는 게 null로 로그인 가능하다? 는 느낌이 들었어요.
개인적인 의견이지만 exception을 던지면 좋을 것 같아요!

반영하였습니다~!

tkdgur0906

짱짱 고생많으셨습니다~

tsulocalize

수고하셨어요~
간단한 코멘트만 확인하고 바로 approve 할게요!

tsulocalize · 2024-10-14T04:12:28Z

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

@@ -25,11 +25,20 @@ public class Password {
    private String value;

    public Password(String value) {
+        if (value == null || value.isEmpty()) {
+            this.value = value;
+            return;


Password라는 도메인이 null 허용이라는 게 null로 로그인 가능하다? 는 느낌이 들었어요.
개인적인 의견이지만 exception을 던지면 좋을 것 같아요!

shin-jisong added 7 commits October 7, 2024 17:16

feat: user 필드 추가에 따른 sql 수정

bca3bf1

feat(UserRepository): 이메일과 로그인 타입으로 조회 구현

b44cc92

feat(User): 비밀번호 불일치 확인 구현

5cc7c23

feat(PasswordEncoder): 비밀번호 암호화 구현

c90960d

fix(UserRepository): findByEmailAndLoginType 쿼리문 오류 수정

a790116

feat(AuthService): 로컬 로그인 구현

22b26c2

feat(AuthController): 로컬 로그인 구현

e488edc

shin-jisong added 🔒 BE 😊 Feature labels Oct 10, 2024

shin-jisong requested review from tkdgur0906, tsulocalize and JINU-CHANG October 10, 2024 04:06

shin-jisong self-assigned this Oct 10, 2024

shin-jisong added 3 commits October 10, 2024 13:06

style: 코드 재정렬

8a63fa9

test: 충돌 해결

e680d8e

JINU-CHANG requested changes Oct 11, 2024

View reviewed changes

tkdgur0906 requested changes Oct 11, 2024

View reviewed changes

shin-jisong added 12 commits October 11, 2024 14:35

fix: 충돌 해결

08b86b6

test: 깨진 테스트 수정

4419c2e

feat(AuthService): localLogin 수정 구현

5fa12f4

feat(ArticleRepository): 시간이 같을 경우, 아이디로 정렬

005aeb6

feat(PasswordEncoder): 암호화 구현

4472507

feat(ChecklistRepository): 시간이 같을 경우, id로 정렬

8f2dcd4

feat(Password): isDifferent 구현

c37e5c1

feat(User): isDifferent 구현

2d1d5bc

test: 깨지는 테스트 수정

6dd181c

style: 코드 재정렬

b2da013

style: oauth 명명 변경

53e3204

style: 함수 순서 변경

bd4c012

tkdgur0906 requested changes Oct 11, 2024

View reviewed changes

shin-jisong added 2 commits October 13, 2024 01:16

feat(PasswordEncoder): 상수화

fb3b2a1

feat(Password): 비어있을 시 null 처리

7c465e3

tkdgur0906 approved these changes Oct 14, 2024

View reviewed changes

JINU-CHANG approved these changes Oct 14, 2024

View reviewed changes

tsulocalize reviewed Oct 14, 2024

View reviewed changes

shin-jisong added 3 commits October 14, 2024 16:03

feat(Password): null이 들어오면 예외 발생

3e717e4

feat(AuthController): 로그인 path 변경

b21c9af

test(AuthE2ETest): 로컬 로그인 path 변경

6859f83

tsulocalize approved these changes Oct 15, 2024

View reviewed changes

tsulocalize merged commit 59eb9ac into dev-be Oct 15, 2024
2 checks passed

tsulocalize deleted the feat/755-local-login branch October 15, 2024 01:02

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

[BE] 일반 로그인을 구현한다. (#775)

f7a83ec

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[BE] 일반 로그인을 구현한다. #775

[BE] 일반 로그인을 구현한다. #775

shin-jisong commented Oct 10, 2024

JINU-CHANG left a comment

JINU-CHANG Oct 10, 2024

JINU-CHANG Oct 10, 2024

JINU-CHANG Oct 11, 2024

shin-jisong Oct 11, 2024

JINU-CHANG Oct 13, 2024

shin-jisong Oct 13, 2024 •

edited

Loading

JINU-CHANG Oct 11, 2024

shin-jisong Oct 11, 2024

JINU-CHANG Oct 11, 2024

shin-jisong Oct 11, 2024

tkdgur0906 left a comment •

edited

Loading

tkdgur0906 Oct 11, 2024

shin-jisong Oct 11, 2024

tkdgur0906 left a comment

tkdgur0906 Oct 11, 2024

shin-jisong Oct 12, 2024

tkdgur0906 Oct 11, 2024

shin-jisong Oct 12, 2024

tsulocalize Oct 14, 2024

shin-jisong Oct 14, 2024

tkdgur0906 left a comment

tsulocalize left a comment

tsulocalize Oct 14, 2024

	public AuthTokenResponse authLogin(OauthLoginRequest request) {
	public AuthTokenResponse outhLogin(OauthLoginRequest request) {

		User user = userRepository.findByEmailAndLoginType(oauthInfoApiResponse.kakao_account().email(),
		LoginType.KAKAO)

	private void checkPassword(LocalLoginRequestV1 request, User user) {
	private void validatePassword(LocalLoginRequestV1 request, User user) {

[BE] 일반 로그인을 구현한다. #775

[BE] 일반 로그인을 구현한다. #775

Conversation

shin-jisong commented Oct 10, 2024

❗ Issue

✨ 구현한 기능

📢 논의하고 싶은 내용

🎸 기타

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

Choose a reason for hiding this comment

shin-jisong Oct 13, 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

tkdgur0906 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

tkdgur0906 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tkdgur0906 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

shin-jisong Oct 13, 2024 •

edited

Loading

tkdgur0906 left a comment •

edited

Loading