-
Notifications
You must be signed in to change notification settings - Fork 6
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.
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
[BE] Refactor/#547 로그인 회원 프로필 조회 API 명세 변경 #561
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
간단한 질문 및 제안 남겼습니다!
답변 주시면 바로 Approve 드리도록 하겠습니다!
항상 고생많으십니다 또이또이또이~~ 내일 생일 재미있게 즐기십셔~
return MemberDetailResponse.from(member); | ||
} | ||
throw new MemberForbiddenException(MemberErrorCode.FORBIDDEN_ACCESS, id); | ||
public MemberDetailResponse findProfile(AuthMember authMember) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
해당 메서드는 무조건 본인의 profile 을 찾아오니까, findMyProfile
도 괜찮을 것 같아요!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
아래 코멘트 남겠습니다!
public ResponseEntity<MemberDetailResponse> findMemberById(AuthMember authMember, @PathVariable Long memberId) { | ||
MemberDetailResponse response = memberQueryService.findById(authMember, memberId); | ||
@GetMapping("/my/profiles") | ||
public ResponseEntity<MemberDetailResponse> findMyProfile(AuthMember authMember) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
앗 Controller 의 메서드 명이 findMyProfile 이라서 Service Method 명은 다르게 하신건가??
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
저는 컨트롤러 메서드는 API 의미에 가깝게 하는 걸 선호해서 그렇게 하고, 서비스메서드는 다르게 했는데
오히려 좀 헷갈렸을 것 같네요!
서비스 메서드명은 개발 로직에 더 가깝게 findMemberDetail로 하겠습니당
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
좋습니당!
@DisplayName("로그인한 식별값(ID)에 해당하는 회원 정보를 찾을 수 없는 경우 예외를 반환한다.") | ||
void findMyProfile_whenNoneExists_thenFail() { | ||
// given | ||
AuthMember otherAuthMember = new User(Long.MAX_VALUE, Collections.emptyList(), Collections.emptyList()); | ||
|
||
@Test | ||
@DisplayName("조회하려는 회원이 본인이 아닌 경우 예외를 반환한다.") | ||
void findMemberById_whenNotSameMember_thenFail() { | ||
// given when then | ||
assertThatThrownBy(() -> memberQueryService.findById(authMember, Long.MAX_VALUE)) | ||
.isInstanceOf(MemberForbiddenException.class); | ||
// when then | ||
assertThatThrownBy(() -> memberQueryService.findProfile(otherAuthMember)) | ||
.isInstanceOf(MemberNotFoundException.class); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
현재 이런 경우가 발생할 수 있을까요 ??
Interceptor 를 거치면서 Token 에 담겨있는 memberId 가 유효한지 확인하니까요!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
맞아요 정상적인 상황에서는 발생할 일이 없는데, 서비스 단에서도 검증을 해주자는 이전의 논의가 생각나서 검증 로직을 유지해봤어요.
만에 하나 jwt 시크릿 키를 탈취당해 외부에서 토큰을 만드는 경우를 생각해봤습니다!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
도이 (유도영) 님이 입력중입니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ㅋㅋㅋㅋㅋㅋ입력 완료상태입니다
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
좋습니다! 이전에 쥬니가 adminService 에서 해줬던 고민과 동일하군요!
저는 OK 입니다~!!
근데, jwt 시크릿 키를 탈취당한 경우를 생각해보셨다고 하셨는데, 조금만 더 자세히 설명 해주실 수 있을까요?
시크릿 키를 탈취 당해서 외부에서 토큰을 만든다고 하더라도 Controller 단에서 먼저 검증을 진행하고, 만일 유효한 MemberId 로 토큰을 만들어 Controller 단이 뚫린다면 현재 Service 도 뚫리지 않나 싶어서요!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
제 이해력이 딸려서 잘 못 이해한 것 같기도 합니다 허허허
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
만일 유효한 MemberId 로 토큰을 만들어 Controller 단이 뚫린다면 현재 Service 도 뚫리지 않나 싶어서요!
맞아요! 잘 말씀해주신 것 같아요. 위 검증 로직은 유효하지 않은 232392024 이런 아이디로 보내는 경우만 막아줘요.
근데 말씀해주신 경우를 막으려면 AuthMember가 정말 본인인지 아닌지 확인하는 수단이 하나 더 필요해지기 때문에
쉽지 않겠네용...
생각해보니 위 검증 로직은 차선책이고, 시크릿 키 탈취에 대한 대책은 별도의 문제인 것 같네요! (그리고 지금은 시크릿 키가 탈취될 가능성이 거의 적으니..)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
시크릿 키는 탈취당하면 admin 큰일 나버렷...
쉬는 날에 긴 답변 정말 감사합니다 도이!
푹 쉬십쇼!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
도이 너무 고생하셨어요~~~!!!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
수고하셨습니다 👍👍👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
고생하셨습미다 도이 !
public MemberDetailResponse findMemberDetail(AuthMember authMember) { | ||
Member member = findMemberById(authMember.getMemberId()); | ||
|
||
return MemberDetailResponse.from(member); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
작업 대상
#547 참조
📄 작업 내용
#547 참조
🙋🏻 주의 사항
API 명세의 변경으로, 프론트 측 해당 코드 변경 PR과 시점을 맞추어 배포할 필요가 있습니다.(현재 해당 API 사용하지 않음)스크린샷
📎 관련 이슈
closed #547
레퍼런스