회원가입 및 로그인 기능 추가 #3

Daniel-Taeyoun · 2020-09-26T07:39:36Z

#2 회원가입 및 로그인 기능 추가

build.gradle

src/main/java/me/naming/onlineshoppingmall/config/MessageException.java

f-lab-dev · 2020-09-27T14:41:15Z

src/main/java/me/naming/onlineshoppingmall/config/RestResponseEntityExceptionHandler.java

+        .path(path)
+        .message(ex.getMessage())
+        .build();
+    return handleExceptionInternal(ex, gson.toJson(responseData), httpHeaders, HttpStatus.INTERNAL_SERVER_ERROR, request);


너무 문제를 복잡하게 해결하신 것 같은데 이런 에러메세지를 전달하는 로직을 더 심플하게 짜는법을 알아보시면 좋겠습니다~

src/main/java/me/naming/onlineshoppingmall/constant/CommonConstant.java

src/main/java/me/naming/onlineshoppingmall/constant/RequestUrlConstant.java

f-lab-dev · 2020-09-27T14:42:52Z

src/main/java/me/naming/onlineshoppingmall/controller/MemberController.java

+  @ResponseStatus(HttpStatus.OK)
+  public void login(@RequestBody UserLoginRequest userLoginRequest, HttpServletRequest request) {
+    HttpSession httpSession = request.getSession();
+    httpSession.setAttribute("loginInfo", memberService.getLoginInfo(userLoginRequest.getEmail(), userLoginRequest.getPassword()));


로그인 처리는 비즈니스 로직 같습니다. controller에서 처리해줘야할까요?

패스워드를 응답에 넣어주면 보안에 취약하지 않을까요?

로그인 처리는 비즈니스 로직이기 때문에 서비스 부분에서 처리해주었습니다.
또한, 패스워드의 경우 응답 값에 넣어주는 것이 아니라 회원 정보 조회의 경우에만 사용되고, 회원번호를 세션 정보로 전달합니다.

f-lab-dev · 2020-09-27T14:43:43Z

src/main/java/me/naming/onlineshoppingmall/repository/MemberRepository.java

+  Member findByEmail(String email);
+
+  @Query("select mb.memNo from Member mb where mb.email = ?1 and mb.password = ?2")
+  Long findMemnoForLoginInfo(String email, String password);


findMemnoByLoginInfo 가 더 자연스러울 것 같네요

굳이 이렇게 쿼리를 하나 선언하는것보다 email로 조회해서 password를 검증하는 메소드를 만드는게 더 낫지 않을까요?

말씀해주신 피드백 내용 반영했습니다. 감사합니다.

f-lab-dev · 2020-09-27T14:44:30Z

src/main/java/me/naming/onlineshoppingmall/service/MemberService.java

+  @Autowired
+  private MemberRepository memberRepository;
+
+  @Autowired


일반적으로 생성자 주입을 권장하고는 합니다. 이유가 무엇일까요?

생성자 주입으로 사용할 경우 주입받을 필드 값을 final로 선언이 가능하고 순환 참조를 방지 할 수 있습니다.
또한, Mockito를 활용한 테스트 코드 작성 시 원하는 구현체를 유연하게 주입 할 수 있습니다.

f-lab-dev · 2020-09-27T14:45:06Z

src/main/java/me/naming/onlineshoppingmall/service/MemberService.java

+  public String getLoginInfo(String email, String password) {
+    Long memNo = memberRepository.findMemnoForLoginInfo(email, password);
+    String encodeMemNo = encryptUtil.aesEncode(String.valueOf(memNo));
+    logger.info("** encodeMemNo : {}", encodeMemNo);


이 메소드에 많은 요청이 접근하면 엄청난 로그를 생성할 것 같네요

넵 해당 부분은 삭제했습니다. 감사합니다.

만약 디버깅에 꼭 필요하다면 레벨을 낮추는것도 괜찮습니다~

src/main/java/me/naming/onlineshoppingmall/util/EncryptUtil.java

Daniel-Taeyoun · 2020-10-11T13:29:54Z

src/test/java/me/naming/onlineshoppingmall/service/MemberServiceTest.java

+//    //given
+//    MockHttpServletRequest mockHttpServletRequest = new MockHttpServletRequest();
+//    when(memberRepository.findMemnoAndPasswordByEmail(EMAIL)).thenReturn(testMember);
+//    memberService = new MemberService(shaService, aesService, memberRepository);


로그인 성공에 대한 테스트 코드 작성 중 SHAServiceImpl 클래스에 대한 의존성으로 인해 적절한 테스트 코드를 작성하지 못해 주석처리했습니다. 이 부분은 어떻게 테스트 코드 작성해야 할지 의문입니다..

일단 memberService는 필드로 선언하는게 나을 것 같고요.
의존성은 mocking 처리하면 되는데 어디에서 헷갈리시는걸까요?

f-lab-dev · 2020-10-13T09:49:58Z

build.gradle

@@ -49,10 +47,15 @@ dependencies {
    testImplementation 'org.hamcrest:hamcrest:2.2'
    testImplementation 'org.hamcrest:hamcrest-library:2.2'

+    /**
+     * (10/11) Intellij 내부적으로 JUnit을 지원해주고 있습니다.


Intellij 지원이면 intellij가 아닌 환경에서는 에러가 나지 않나요??

해당 내용은 개인적으로 블로그 글을 잘못 이해했습니다. 스프링 내부(org.springframework.boot:spring-boot-starter-test)에서 jupiter를 제공해주고 있으며, 따라서 추가했던 jupiter는 삭제했습니다.

f-lab-dev · 2020-10-13T09:50:30Z

src/main/java/me/naming/onlineshoppingmall/config/RestResponseEntityExceptionHandler.java

@@ -18,10 +17,9 @@
 @ControllerAdvice
 public class RestResponseEntityExceptionHandler extends ResponseEntityExceptionHandler {

-  @ExceptionHandler(value = { MessageException.class })
-  protected ResponseEntity<Object> handleConflict(RuntimeException ex, WebRequest request) {
+  @ExceptionHandler(value = { RuntimeException.class })


RuntimeException의 하위 클래스에는 여러가지가 있습니다. 그 예외가 전부 서버의 책임이라고 핸들링하고 있는 것으로 보이네요~

해당 피드백 내용은 코드에 반영했습니다.

f-lab-dev · 2020-10-13T09:50:55Z

src/main/java/me/naming/onlineshoppingmall/domain/Member.java

@@ -44,8 +42,8 @@
  @Column(name = "PASSWORD")
  @NotNull
  @Setter
-  @Pattern(regexp = "^(?=.*\\d)(?=.*[~`!@#$%\\^&*()-])(?=.*[a-z])(?=.*[A-Z]).{9,12}$",
-      message = "비밀번호는 영문 대,소문자와 숫자, 특수기호가 적어도 1개 이상씩 포함된 9자 ~ 12자의 비밀번호여야 합니다.")
+//  @Pattern(regexp = "^(?=.*\\d)(?=.*[~`!@#$%\\^&*()-])(?=.*[a-z])(?=.*[A-Z]).{9,12}$",


Pull request는 머지를 위한 것입니다. 주석을 실서버 소스에 머지해도 괜찮을까요?

아닙니다. 해당 피드백 내용은 코드에 반영했습니다.

f-lab-dev · 2020-10-13T09:51:46Z

src/main/java/me/naming/onlineshoppingmall/service/AESServiceImpl.java

+      aesDecode =  new String(cipher.doFinal(byteStr), StandardCharsets.UTF_8.name());
+
+    } catch (UnsupportedEncodingException e) {
+      logger.error("** Decode UnsupportedEncodingException : {}", e);


그냥 아래의 내용들과 메세지만 약간씩 다른것같은데 한번에 처리할 수 있지 않을까요?

Exception으로 모든 예외를 한꺼번에 처리해줄 수 있을 수 있지만, 예외 상황을 세부적으로 파악하기 위해 코드 내용을 작성해주는게 좋진 않을지 궁금합니다.

피드백 해주신 내용은 현재 GeneralSecurityException와 UnsupportedEncodingException로 예외처리 했습니다.

src/main/java/me/naming/onlineshoppingmall/service/CryptoService.java

f-lab-dev · 2020-10-13T09:54:01Z

src/main/java/me/naming/onlineshoppingmall/service/MemberService.java

+      throw new RuntimeException("비밀번호가 틀렸습니다");
+    }
+
+    String encodeMemNo = aesService.encrypt(String.valueOf(member.getMemNo()));


이것은 암호화하시는 이유가 무엇일까요?

세션의 경우 서버 메모리에 존재하기 때문에 굳이 암호화 할 필요는 없다고 생각합니다.
따라서 해당 코드 내용은 암호화 하지 않은 평문으로 회원번호가 저장될 수 있게 수정했습니다.

f-lab-dev · 2020-10-13T09:54:13Z

src/main/java/me/naming/onlineshoppingmall/service/MemberService.java

+    }
+
+    String encodeMemNo = aesService.encrypt(String.valueOf(member.getMemNo()));
+    httpSession.setAttribute("loginInfo", encodeMemNo);


"loginInfo" 같은 값은 상수로 관리하는게 어떨까요?

해당 피드백 내용은 코드에 반영했습니다.

f-lab-dev · 2020-10-13T09:54:36Z

src/main/java/me/naming/onlineshoppingmall/service/SHAServiceImpl.java

+  @Value("${encrypt.shaKey}")
+  private String shaKey;
+
+  private static final Logger logger = LogManager.getLogger(SHAServiceImpl.class);


이것도 lombok을 사용할 수 있지 않을까요?

해당 피드백 내용은 코드에 반영했습니다.

f-lab-dev · 2020-10-13T09:55:11Z

src/test/java/me/naming/onlineshoppingmall/service/MemberServiceTest.java

+  }
+
+  @Test
+  public void 로그인_실패_비밀번호_일치_X() {


메소드 이름을 한글로 해주지 않아도 junit5의 기능을 이용하면 테스트에 이름을 지어줄 수 있습니다

해당 피드백 내용은 코드에 반영했습니다.

f-lab-dev · 2020-10-13T09:56:28Z

src/test/java/me/naming/onlineshoppingmall/service/MemberServiceTest.java

+//    //given
+//    MockHttpServletRequest mockHttpServletRequest = new MockHttpServletRequest();
+//    when(memberRepository.findMemnoAndPasswordByEmail(EMAIL)).thenReturn(testMember);
+//    memberService = new MemberService(shaService, aesService, memberRepository);


일단 memberService는 필드로 선언하는게 나을 것 같고요.
의존성은 mocking 처리하면 되는데 어디에서 헷갈리시는걸까요?

Daniel-Taeyoun · 2020-10-21T16:50:56Z

해당 코드에서 세션 정보 저장 시 회원번호(memNo)를 저장하게끔 코드를 작성했습니다. 하지만, 회원번호의 경우 DB에서 자동 생성해주고 있어서 이런 경우 어떻게 해야할지 의문입니다.

이런 경우 @DataJpaTest 를 추가하여 테스트 코드를 작성하는 것이 적절할까요? 아님 따로 회원번호에 관한 Mock을 생성해줄 수 있는지 궁금합니다.

f-lab-dev

질문해주신 내용은 적절하게 mocking을 해주면 될 것 같네요~
단위테스트를 작성하는데 DB에서 자동생성해준다라는 사실이 중요할까요?

f-lab-dev · 2020-10-23T13:48:26Z

src/main/java/me/naming/onlineshoppingmall/config/RestResponseEntityExceptionHandler.java


    ResponseData responseData = ResponseData.builder()
        .localDateTime(LocalDateTime.now())
-        .status(HttpStatus.INTERNAL_SERVER_ERROR)
+        .statusCode(code)


이렇게하면 status code가 body에 들어가지 않을까요?

f-lab-dev · 2020-10-23T13:49:20Z

src/test/java/me/naming/onlineshoppingmall/service/MemberServiceTest.java

@@ -99,12 +102,12 @@ public void setUp() {
  }

  @Test
-  public void 로그인_실패_비밀번호_일치_X() {
+  @DisplayName("로그인_실패_비밀번호_일치_X")


여기는 띄어쓰기를 쓰셔도 되는데 굳이 언더바를 넣어주신 이유가 무엇인가요?

서술형으로 쓰는데 X 같은 말을 쓰는것은 프로페셔널해보이지 않네요~

f-lab-dev · 2020-10-23T13:49:45Z

src/main/java/me/naming/onlineshoppingmall/service/CryptoService.java

- *  - https://d2.naver.com/helloworld/318732
- *  - https://dailyworker.github.io/AES-Algorithm-and-Chiper-mode/
- *  - https://javaplant.tistory.com/26
- */
 public interface CryptoService {


어떤 의도의 추상화인지 의도는 주석에 남겨주시는게 좋겠네요

회원가입 및 로그인 기능 추가

fc45a1d

Daniel-Taeyoun requested a review from f-lab-dev September 26, 2020 07:39

Daniel-Taeyoun self-assigned this Sep 26, 2020

f-lab-dev reviewed Sep 27, 2020

View reviewed changes

피드백 반영

6070ae1

Daniel-Taeyoun commented Oct 11, 2020

View reviewed changes

Daniel-Taeyoun requested a review from f-lab-dev October 11, 2020 13:31

f-lab-dev reviewed Oct 13, 2020

View reviewed changes

Daniel-Taeyoun added 2 commits October 22, 2020 01:20

피드백 반영

ac098bb

주석 및 불필요한 코드 삭제

7d7c9e0

Daniel-Taeyoun requested a review from f-lab-dev October 21, 2020 16:51

f-lab-dev reviewed Oct 23, 2020

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

회원가입 및 로그인 기능 추가 #3

회원가입 및 로그인 기능 추가 #3

Daniel-Taeyoun commented Sep 26, 2020

f-lab-dev Sep 27, 2020

f-lab-dev Sep 27, 2020

Daniel-Taeyoun Oct 10, 2020

f-lab-dev Sep 27, 2020

Daniel-Taeyoun Oct 11, 2020

f-lab-dev Sep 27, 2020

Daniel-Taeyoun Oct 10, 2020

f-lab-dev Sep 27, 2020

Daniel-Taeyoun Oct 11, 2020

f-lab-dev Oct 23, 2020

Daniel-Taeyoun Oct 11, 2020

f-lab-dev Oct 13, 2020

f-lab-dev Oct 13, 2020

Daniel-Taeyoun Oct 21, 2020

f-lab-dev Oct 13, 2020

Daniel-Taeyoun Oct 21, 2020

f-lab-dev Oct 13, 2020

Daniel-Taeyoun Oct 21, 2020

f-lab-dev Oct 13, 2020

Daniel-Taeyoun Oct 21, 2020

f-lab-dev Oct 13, 2020

Daniel-Taeyoun Oct 21, 2020

f-lab-dev Oct 13, 2020

Daniel-Taeyoun Oct 21, 2020

f-lab-dev Oct 13, 2020

Daniel-Taeyoun Oct 21, 2020

f-lab-dev Oct 13, 2020

Daniel-Taeyoun Oct 21, 2020

f-lab-dev Oct 13, 2020

Daniel-Taeyoun commented Oct 21, 2020

f-lab-dev left a comment

f-lab-dev Oct 23, 2020

f-lab-dev Oct 23, 2020

f-lab-dev Oct 23, 2020

회원가입 및 로그인 기능 추가 #3

Are you sure you want to change the base?

회원가입 및 로그인 기능 추가 #3

Conversation

Daniel-Taeyoun commented Sep 26, 2020

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

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

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

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

Choose a reason for hiding this comment

Daniel-Taeyoun commented Oct 21, 2020

f-lab-dev 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