[Review] ATDD 3주차 1차 PR

 

권승철 리뷰어 님께 감사의 말을 전하고 싶다!!

 

와 개인적으로 이번 3주차는 Spring Security를 비슷하게 만든 뼈대 코드 위에서 인증 처리 테스트를 작성하는데, 진짜 어려우면서 꿀잼이다.

아직 Spring Security를 공부해본적은 없지만, 큰 틀을 배워가는 것 같아 너무 재미있다.

 

1. 질문

1-1) 인터셉터 처리 방식

public void addInterceptors(InterceptorRegistry registry) {
  registry.addInterceptor(new SecurityContextPersistenceFilter());
  registry.addInterceptor(new UsernamePasswordAuthenticationFilter(loginMemberService)).addPathPatterns("/login/form");
  registry.addInterceptor(new TokenAuthenticationInterceptor(loginMemberService, jwtTokenProvider)).addPathPatterns("/login/token");
  registry.addInterceptor(new BasicAuthenticationFilter(loginMemberService));
  registry.addInterceptor(new BearerTokenAuthenticationFilter(jwtTokenProvider));
}

위와 같은 인터셉트 등록에서, UsernamePasswordAuthenticationFilter, TokenAuthenticationInterceptor 같은 경우
로그인 처리를 인터셉터에서 해주는 격이니까? 바로 false를 반환하도록 처리 했는데, 이게 맞는 방식일까요?

 

답변:

 

1-2) 요구사항 질문

우리의 요구사항을 보면 "지하철역, 노선, 구간을 변경하는 API는 관리자만 접근이 가능하도록 수정하세요."
라고 명시되어 있는데, MemberAcceptanceTest를 보면 다음과 같이 비어 있는 메서드 들이 있습니다.

    @DisplayName("회원 정보를 관리한다.")
    @Test
    void manageMember() {
    }

    @DisplayName("나의 정보를 관리한다.")
    @Test
    void manageMyInfo() {
    }

비어 있는 이유가 회원 정보도 관리자만 수정 가능하도록 작성해보라는 의미인가요??

회원 관리 기능의 CRUD를 하나의 큰 흐름으로 인수테스트를 작성해보자는 의미였다!

인수테스트 합치기에 해당되는 내용이다.

 

2. 리뷰 정리

2-1) 예외 catch하기

 

음 사실 이런 리뷰가 달릴거라는 생각은 했던 부분이다.

하지만 나 또한 Exception으로 놔둔 이유가 있었다.

 

위 인터셉터 에서 로그인 처리를 하던 도중 어떠한 이유든 예외가 발생한다면 false를 반환하도록 catch부분에서 작석해주는 것이 적합하지 않을까?

로그인 과정에서 오류가 나면 일단 튕겨주는게 적합하지 않나? 란 생각이 들었다.

 

하지만 리뷰어의 말을 듣고보니 생각이 조금 바뀌었는데

모든 예외를 다 튕겨버리면, 이게 로그인이 잘못되서 튕긴건지? 아니면 다른 부분에서 예외가 발생하여(ex Json 파싱이라던가?) 튕긴건지?

구분할수가 없다. 로그인 오류가 아니라면 true를 반환하여 다음 interceptor로 넘어가는 것 또한 적합한것 같다?

 

2-2) 매직 리터럴 분리

 

흠... 사실 분리한다 해도 다음과 같이 분리할 것 같은데

private static final String EMAIL = "email";
private static final String PASSWORD = "password";

꼭 분리해야하나? 란 생각이 들었습니다.

문자 자체로 의미가 충분하게 전달된다 생각되어 변경하지 않았었습니다 ㅎㅎ

이에 대한 리뷰어님의 의견이 궁금합니다!!

 

답변:

흠 principal 객체 자체에 담을 수 있는 정보가 단지 email/password가 아닌, 다른 전략 또한 사용이 가능하며

이에 따라 다형적으로 적용할 수 있다는 리뷰어의 답문인것 같다.

 

따라서 username, credentail로 상수를 추출하여 사용하면 더 좋을것 같다.

말씀해주신 것 처럼 email/password 은 너무 세부적이다.

 

2-3) import에서 * (wild card) 사용 금지

나:

좋은 리뷰 감사합니다 ㅎㅎ
혹시 리뷰어님은 그럼 *은 아예 사용하지 않는 편이신가요?
혹 가끔 사용하신다면 기준이 있을까요? 몇개 이상 정도면 그냥 *(wild card)를 사용한다던가? 하는

 

답변: