feat-be: 동아리 이메일, 비밀번호 정보 수정 #962

github-actions · 2024-12-18T05:57:56Z

Original issue description

목적

동아리 계정 이메일, 비밀번호를 수정할 수 있는 API를 개발합니다.

작업 세부사항

계정 정보 수정 API
- method: PATCH
- end point: /v1/member/{memberId}
비밀번호 수정 API
- method: PATCH
- end point: /v1/member/{memberId}/password

참고 사항

계정 정보와 비밀번호 수정 API를 분리했습니다.
계정 정보의 이메일 변경 시, 이메일 검증이 필요할 것 같아 적용했습니다.
비밀번호 수정 시, 정책이 정해지지 않아서 논의가 필요합니다.

아래의 별표줄 밑에 요구사항 ID만 작성해주세요. Prefix 금지!

FIX_AUTH_02

closes #961

github-actions · 2024-12-21T06:25:05Z

1734762304.726159

github-actions · 2024-12-21T06:25:08Z

1734762308.103079

github-actions · 2024-12-21T06:25:13Z

1734762313.152619

xogns1514

수고하셨습니다~ 초코칩!
인가 부분에 대해서 문제가 있는거 같아서 답변 달아두었습니다.
이해 안가는 부분 있으면 질문 주세요~

xogns1514 · 2024-12-23T08:04:13Z

backend/src/main/java/com/cruru/member/controller/MemberController.java

@@ -23,4 +29,20 @@ public ResponseEntity<Void> create(@RequestBody @Valid MemberCreateRequest reque
        long memberId = memberFacade.create(request);
        return ResponseEntity.created(URI.create("/v1/members/" + memberId)).build();
    }
+
+    @PatchMapping("/{memberId}/email")


이부분 동작확인 해보셨나요~?
지금 구현된 AOP 동작을 봤을때 정상동작하지 않을 것 같아요.

첫번째로 인가 AOP 동작 Joinpoint가 @ValidAuth 이 존재한 컨트롤러 메서드입니다. AOP를 동작시키려면 메서드단에 해당 어노테이션이 추가되어야 합니다.

인가 AOP 동작은 ArgumentResolver가 넘겨준 LoginProfile을 기준으로 인가를 진행합니다. 현재 컨트롤러는 LoginProfile을 받지 않고 있습니다.

LoginProfile을 통해 찾은 멤버와 @RequiredAuth로 넘겨진 id와 findByIdFetchingMember 메서드를 통해 찾은 맴버를 비교해서 인가를 진행합니다. 현재 MemberRepository에는 해당 메서드가 존재하지 않습니다.

AOP로 인가를 진행하려면 다음 사항이 추가되어야 할 것 같습니다.

@ValidAuth 추가

LoginProfile을 파라미터로 받아와 통해 현재 로그인한 멤버 정보를 받아온다.

MemberRepository에 findByIdFetchingMember 메서드 추가

AOP 인가가 적용된 다른 컨트롤러 보시면 이해하기 쉬울 것 같습니다.!
이번에는 secureResource가 Member 자체라서 어색한 부분이 있긴하네요

적용했습니다!

타사용자가 비밀번호를 변경할 수 있는지의 테스트도 인수테스트 계층으로 추가했습니다.
혹시 다른 분들도 reviewer로 지정하는게 좋을까요?

인가 작동방식에 대한 공유 목적이라면 좋은것 같아요~

xogns1514 · 2024-12-23T08:05:05Z

backend/src/main/java/com/cruru/member/domain/repository/MemberRepository.java


 public interface MemberRepository extends JpaRepository<Member, Long> {

    boolean existsByEmail(String email);

    Optional<Member> findByEmail(String email);
+
+    @Transactional
+    @Modifying


제가 기억이 잘 안나서 그러는데, 변경감지 말고 jpql로 update 하기로 통일했었나요..?ㅎㅎ 가물가물합니다

저도 가물가물하네요! 저는 가독성이 좋아서 일단 이렇게 작성했습니다!

저는 통일 관련 얘기는 들은 적이 없지만, 예전에 변경감지를 사용하지 말자는 제안을 초코칩이 했다가 그냥 사라졌던 기억은 있네요. 다시 이야기해보면 좋을 것 같아요.

HyungHoKim00

고생하셨습니다. 여행 때문에 늦게 확인해서 죄송해요 ㅎㅎ. 코멘트 남긴거 확인해주시고 리뷰요청 주세용~

HyungHoKim00 · 2024-12-24T09:40:33Z

backend/src/test/java/com/cruru/member/acceptance/MemberAcceptanceTest.java

+
+    @DisplayName("사용자는 다른 사용자의 이메일을 변경할 수 없다.")
+    @Test
+    void userCannotChangeAnotherUsersEmail() {


이 실패 케이스들은 문서화하지 않는 이유가 뭔가요? 이 실패 케이스에 어떤 응답이 돌아오는지 프론트 측도 아는 것이 좋을 것 같아서요 ㅎㅎ.

인수(시나리오) 테스트가 API 문서화를 하는지에 대한 개념과 팀 문화가 없어서 저도 고민했던 부분이에요!

문서화하지 않은 이유는

API응답의 모든 것을 문서화하면 문서의 양이 방대해져서 백엔드의 관리가 힘들어진다.

프론트가 찾아야할 부분이 많다. -> 어느 정도는 문서화하지 않아도 될듯. -> "그럼 어떤 기준으로 문서화하지 않을 것인가?"에 대한 기준은 딱히 없음... 인가는 대부분 API에서 프론트가 직관적으로 알수 있을 것 같아서....

시나리오 테스트도 문서화 하나요? (진짜 모름)

정도네용

@xogns1514 어떻게 생각하시나용

저도 2번 의견에 동의해서 꼭 문서화가 필요한 부분인지 느끼지 못했습니다.
인가부분은 공통적으로 접근하지 못하면 403을 던져주고 있어서, 모든 상황마다 문서를 만들 필요는 없을 것 같다고 생각했습니다.

HyungHoKim00 · 2024-12-24T09:41:50Z

backend/src/test/java/com/cruru/member/service/MemberServiceTest.java

+
+    @DisplayName("회원의 이메일을 갱신한다.")
+    @Test
+    void updateEmail() {


비밀번호 갱신 테스트도 있으면 좋을 것 같아요~

비밀번호 갱신 테스트는 비밀번호 변경 후, 변경되었는지 확인하기 위해 비밀번호 검색이 필요한데 검색 기능이 없어서 불가능하더라고요 🥲

HyungHoKim00 · 2024-12-24T09:44:47Z

backend/src/main/java/com/cruru/member/domain/repository/MemberRepository.java


 public interface MemberRepository extends JpaRepository<Member, Long> {

    boolean existsByEmail(String email);

    Optional<Member> findByEmail(String email);
+
+    @Transactional
+    @Modifying


저는 통일 관련 얘기는 들은 적이 없지만, 예전에 변경감지를 사용하지 말자는 제안을 초코칩이 했다가 그냥 사라졌던 기억은 있네요. 다시 이야기해보면 좋을 것 같아요.

HyungHoKim00 · 2024-12-24T09:46:24Z

backend/src/main/java/com/cruru/member/service/MemberService.java

-        validatePassword(rawPassword);
-        return passwordValidator.encode(rawPassword);
+    private String encodePassword(String password) {
+        validatePassword(password);


passwordValidator가 존재하는데 검증은 memberService가 하고 encode를 validator가 하는게 뭔가 어색하네요 ㅎㅎ.. 어떻게 생각하시나요?

음 이게 클래스명 때문에 더 그렇게 느껴지겠네요. 비밀번호 정책관련 검증은 비즈니스로직이라서 서비스에 위치하고, 단순 인코딩, 인코딩시 동일한지 판단은 유틸적인 기능이라 생각해서 passwordValidator가 담당하고 있었습니다.

xogns1514

수고하셨습니다 초코칩~!

xogns1514 · 2024-12-24T11:46:38Z

backend/src/main/java/com/cruru/member/controller/MemberController.java

@@ -23,4 +29,20 @@ public ResponseEntity<Void> create(@RequestBody @Valid MemberCreateRequest reque
        long memberId = memberFacade.create(request);
        return ResponseEntity.created(URI.create("/v1/members/" + memberId)).build();
    }
+
+    @PatchMapping("/{memberId}/email")


인가 작동방식에 대한 공유 목적이라면 좋은것 같아요~

xogns1514 · 2024-12-24T11:51:42Z

backend/src/main/java/com/cruru/member/service/MemberService.java

-        validatePassword(rawPassword);
-        return passwordValidator.encode(rawPassword);
+    private String encodePassword(String password) {
+        validatePassword(password);


음 이게 클래스명 때문에 더 그렇게 느껴지겠네요. 비밀번호 정책관련 검증은 비즈니스로직이라서 서비스에 위치하고, 단순 인코딩, 인코딩시 동일한지 판단은 유틸적인 기능이라 생각해서 passwordValidator가 담당하고 있었습니다.

backend/src/test/java/com/cruru/member/acceptance/MemberAcceptanceTest.java

HyungHoKim00

고생하셨습니당. 문서화에 대해선 다시 이야기해보면 좋을 것 같네요~

Create draft PR for #961

a7f500b

github-actions bot added backend 백엔드 feature 새로운 기능 labels Dec 18, 2024

github-actions bot assigned Chocochip101 Dec 18, 2024

Chocochip101 added 5 commits December 18, 2024 16:12

feat(Member): SecureSource 적용

f0bc442

feat(Member): 이메일 변경 API 구현

27b0258

docs: 이메일 변경 API 문서화

1fbd065

feat(Member): 비밀번호 변경 API 구현

b31e430

docs: 비밀번호 변경 API 문서화

0205ec8

Chocochip101 requested review from xogns1514 and HyungHoKim00 December 21, 2024 06:24

Chocochip101 marked this pull request as ready for review December 21, 2024 06:24

xogns1514 requested changes Dec 23, 2024

View reviewed changes

Chocochip101 added 2 commits December 23, 2024 19:30

fix: 인가 로직 추가

41da345

test: 타사용자의 이메일, 비밀번호 변경 불가 검증

a96bdc5

HyungHoKim00 reviewed Dec 24, 2024

View reviewed changes

xogns1514 approved these changes Dec 24, 2024

View reviewed changes

Chocochip101 requested a review from HyungHoKim00 December 30, 2024 03:16

HyungHoKim00 approved these changes Dec 30, 2024

View reviewed changes

Chocochip101 merged commit 8397c42 into be/develop Jan 1, 2025
35 checks passed

Chocochip101 deleted the 961-be-FIX_AUTH_02 branch January 1, 2025 12:13

xogns1514 mentioned this pull request Jan 2, 2025

feat-be: 이메일 발송 내역 조회 기능 #974

Merged

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat-be: 동아리 이메일, 비밀번호 정보 수정 #962

feat-be: 동아리 이메일, 비밀번호 정보 수정 #962

github-actions bot commented Dec 18, 2024 •

edited by Chocochip101

Loading

github-actions bot commented Dec 21, 2024

github-actions bot commented Dec 21, 2024

github-actions bot commented Dec 21, 2024

xogns1514 left a comment

xogns1514 Dec 23, 2024

Chocochip101 Dec 23, 2024

xogns1514 Dec 24, 2024

xogns1514 Dec 23, 2024

Chocochip101 Dec 23, 2024

HyungHoKim00 Dec 24, 2024

HyungHoKim00 left a comment •

edited

Loading

HyungHoKim00 Dec 24, 2024

Chocochip101 Dec 30, 2024

xogns1514 Jan 1, 2025

HyungHoKim00 Dec 24, 2024

Chocochip101 Dec 30, 2024

HyungHoKim00 Dec 24, 2024

HyungHoKim00 Dec 24, 2024 •

edited

Loading

xogns1514 Dec 24, 2024

xogns1514 left a comment

xogns1514 Dec 24, 2024

xogns1514 Dec 24, 2024

HyungHoKim00 left a comment

feat-be: 동아리 이메일, 비밀번호 정보 수정 #962

feat-be: 동아리 이메일, 비밀번호 정보 수정 #962

Conversation

github-actions bot commented Dec 18, 2024 • edited by Chocochip101 Loading

목적

작업 세부사항

참고 사항

github-actions bot commented Dec 21, 2024

github-actions bot commented Dec 21, 2024

github-actions bot commented Dec 21, 2024

xogns1514 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

HyungHoKim00 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

Choose a reason for hiding this comment

HyungHoKim00 Dec 24, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xogns1514 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

HyungHoKim00 left a comment

Choose a reason for hiding this comment

github-actions bot commented Dec 18, 2024 •

edited by Chocochip101

Loading

HyungHoKim00 left a comment •

edited

Loading

HyungHoKim00 Dec 24, 2024 •

edited

Loading