Authorization 헤더를 응답하도록 변경 #981

zangsu · 2024-12-19T11:14:41Z

⚡️ 관련 이슈

close #908

📍주요 변경 사항

바로 이전 pr에서 추가했던 CredentialManager를 함께 사용하도록 하여 Cookie / Authorization 헤더 응답을 모두 줄 수 있도록 변경하였습니다.
이후 프론트 작업이 끝나면 Cookie 관련 클래스를 삭제 예정입니다.

🎸기타

코멘트로 남겨둘게욥

🍗 PR 첫 리뷰 마감 기한

12/21(토) 15:00

내일 중으로 리뷰 반영 가능한 여유시간이 없어 주말까지 조금 더 기한을 늘려두겠습니닷

zangsu · 2024-12-19T11:19:02Z

backend/src/main/java/codezap/auth/controller/AuthController.java

+        //ArgumentResolver 와 동작이 일치
+        CredentialManager credentialManager = credentialManagers.stream()
+                .filter(eachCredentialManager -> eachCredentialManager.hasCredential(httpServletRequest))
+                .findFirst()
+                .orElseThrow(() -> new CodeZapException(ErrorCode.UNAUTHORIZED_USER, "인증 정보가 없습니다. 다시 로그인 해 주세요."));


이 부분의 코드가 ArgumentResolver의 코드와 완전 동일합니다.
즉, 파라미터로 @AuthenticationPrinciple Member member만 추가해주고 아무 로직도 작성하지 않아도 동일한 동작을 하는데요.
이 방향으로의 수정은 다들 어떻게 생각하시나요?

리팩토링 진행 시:

@GetMapping("/login/check") public ResponseEntity<Void> checkLogin( @AuthenticationPrinciple Member member HttpServletRequest httpServletRequest) { return ResponseEntity.ok().build(); }

저는 좋다고 생각합니다~ 재사용할 수 있다면 하면 좋죠~

member가 null로 들어온 경우에는 어떻게 동작할지 궁금해요!

member가 null로 들어온 경우에는 어떻게 동작할지 궁금해요!

오~ 미처 생각을 못했군요~!!
Null 여부를 확인하여 예외 처리 추가해 두겠습니닷

코드를 보고 생길 인지부조화도 훨씬 줄어든 것 같군요!! 👍

zangsu · 2024-12-19T11:20:57Z

backend/src/test/java/codezap/auth/AuthTest.java

+import org.springframework.test.web.servlet.MvcResult;
+import org.springframework.test.web.servlet.ResultActions;
+
+class AuthTest extends MvcTest {


인수테스트를 작성한다는 관점으로 작성해 보았습니다.
통합 테스트를 작성하자는 논의 결과에 한번 작성해 보았는데, 편하게 많은 의견 남겨주시면 최대한 반영해볼게요~

클래스명에 Acceptance라는 단어가 들어가면 인수테스트라는 정보를 더 명확히 알 수 있을 것 같아요!

저도 제우스 의견처럼 이름이 조금 더 명확하면 좋을 것 같아요!
AuthAcceptanceTest 좋네요!

zangsu · 2024-12-19T11:22:23Z

backend/src/test/java/codezap/auth/AuthTest.java

+
+    @Nested
+    @DisplayName("로그인 확인")
+    class CheckLogin {


성공 테스트가 위에서 존재하는 Login의 성공 테스트와 함께 테스트가 됩니다. 그래서 작성하지 않았는데,,, 뭔가 이래도되나 싶네요.

"로그인이 잘 되었다" 와 "로그인 유지가 잘 된다" 를 따로 테스트 해야 할 필요가 있을까요?

그래도 api가 따로 존재하는 것이니 테스트 코드를 추가하는게 맞을 것 같아요~

사용자의 로그인 상태가 유효한지 검증한다 라는 시나리오를 만들어 테스트하면 좋을 것 같아요.

로그인 API와 로그인 확인 API의 목적은 다르니까요.

zangsu · 2024-12-19T11:24:07Z

backend/src/test/java/codezap/auth/AuthTest.java

+            String password = "password123!";
+            signup(name, password);
+            MvcResult loginResponse = requestLogin(name, password).andReturn();
+            Pattern expireCookieRegex = Pattern.compile("credential=.*?; Max-Age=0;.*?");


로그아웃을 검증하기 위해 인증정보 쿠키의 이름이 노출되어야 합니다. (credential <- 쿠키 이름)
이 부분을 어떻게 해야 캡슐화 되어있는 쿠키의 이름을 재사용하지 않고 테스트를 잘 할 수 있을지 고민이네용

음,, 저희 DB 변수 숨기는 것처럼 yml 파일에 환경변수로 해당 변수를 등록해놓는건 어떤가요?
이 방법 외에는 떠오르지 않네요~

인수 테스트이므로 애초에 테스트하기 좋은 시나리오를 만드는 것도 방법이라고 생각해요.

로그인 상태에서는 정상적으로 접근할 수 있던 자원 또는 기능에 대해서 로그아웃 후에는 접근하지 못하게 하는 테스트가 되면 좋을 것 같아요.

저는 인수테스트를 사용자에게 초점을 둔 테스트라고 이해했습니다.
그런 관점에서 쿠키 정보가 삭제되었는지를 사용자가 테스트할 필요는 없다고 생각해요.
제우스 말대로 로그인 전에는 내 템플릿을 조회할 수 있었는데 로그아웃을 하고 나면 조회할 수 없는 시나리오 등으로 테스트 해봐도 좋을 것 같네요.
근데 이게 인수테스트가 맞는지는 모르겠어요. 나중에 같이 인수테스트에 대해 얘기해봐요.

아,, 설명이 조금 부족했네요!!

로그인 상태에서는 정상적으로 접근할 수 있던 자원 또는 기능에 대해서 로그아웃 후에는 접근하지 못하게 하는 테스트가 되면 좋을 것 같아요.

저도 이 의견에 공감합니닷! 다만, 문제가 되는 지점은 "쿠키 인증정보를 만료시키는 방법" 이에요.
지금의 동작은 SET-COOKIE 헤더를 사용해 기존의 인증 정보 쿠키를 만료시키는 방법인데요.
이 동작은 브라우저에서 알아서 처리해주지만, 우리는 브라우저 없이 테스트를 진행해야 하기 때문에 SET-COOKIE 헤더를 확인하지 않고는 로그아웃을 테스트 하기가 어려운 상태라서 질문 드리게 되었어요~!

zangsu · 2024-12-19T11:25:12Z

backend/src/test/java/codezap/global/MvcTest.java

+
+@SpringBootTest
+@EnableConfigurationProperties(CorsProperties.class)
+public abstract class MvcTest {


이 클래스도 통합테스트를 위해 만들어 본 기반 환경이에요. MockMvcTest 에서 모킹 부분만 지운 상태입니다.
어때보이시는지,,,? 수정 보완 제안하실 부분도 편하게 말해주세요~~

일단 동작한다면 충분하다고 생각해요! 간결하고 좋네요 ㅎㅎ

저는 개인적으로 통합테스트에서는 RestAssured를 사용하는 것을 선호합니다.
테스트의 가독성이 좋고 json 데이터도 더 쉽게 검증할 수 있기 때문입니다.
더군다나 RestAssured는 BDD 스타일로 작성할 수 있는데, 인수테스트가 사용자 시나리오를 테스트하는 것이라면 BDD의 장점을 더 잘 활용할 수 있다고 생각해요.

다른 분들 의견은 어떤가요?

kyum-q

짱수 수고했어용~ 간단한 코멘트만 남겨뒀어요 확인 부탁드려요 !

backend/src/main/java/codezap/auth/configuration/AuthArgumentResolver.java

kyum-q · 2024-12-19T12:06:59Z

backend/src/main/java/codezap/auth/configuration/AuthArgumentResolver.java

+        CredentialManager credentialManager = credentialManagers.stream()
+                .filter(eachCredentialManager -> eachCredentialManager.hasCredential(request))
+                .findFirst()
+                .orElseThrow(() -> new CodeZapException(ErrorCode.UNAUTHORIZED_USER, "인증 정보가 없습니다. 다시 로그인 해 주세요."));


찾아보니 인증 정보가 없습니다. 다시 로그인해 주세요. 가 맞는 맞춤법이라네요~

kyum-q · 2024-12-19T12:08:04Z

backend/src/main/java/codezap/auth/controller/AuthController.java

+        //ArgumentResolver 와 동작이 일치
+        CredentialManager credentialManager = credentialManagers.stream()
+                .filter(eachCredentialManager -> eachCredentialManager.hasCredential(httpServletRequest))
+                .findFirst()
+                .orElseThrow(() -> new CodeZapException(ErrorCode.UNAUTHORIZED_USER, "인증 정보가 없습니다. 다시 로그인 해 주세요."));


저는 좋다고 생각합니다~ 재사용할 수 있다면 하면 좋죠~

kyum-q · 2024-12-19T12:09:53Z

backend/src/test/java/codezap/auth/AuthTest.java

+
+    @Nested
+    @DisplayName("로그인 확인")
+    class CheckLogin {


그래도 api가 따로 존재하는 것이니 테스트 코드를 추가하는게 맞을 것 같아요~

kyum-q · 2024-12-19T12:12:45Z

backend/src/test/java/codezap/auth/AuthTest.java

+            String password = "password123!";
+            signup(name, password);
+            MvcResult loginResponse = requestLogin(name, password).andReturn();
+            Pattern expireCookieRegex = Pattern.compile("credential=.*?; Max-Age=0;.*?");


음,, 저희 DB 변수 숨기는 것처럼 yml 파일에 환경변수로 해당 변수를 등록해놓는건 어떤가요?
이 방법 외에는 떠오르지 않네요~

zeus6768

이제보니 짱수의 코드는 컨벤션으로 정한 Code Style이 적용되지 않았네요.

아래의 텍스트를 복사해 CodeZapStyle.xml 파일로 저장한 뒤,

인텔리제이 설정(⌘,) > Editor > Code Style 에서 import 하면 좋겠어요.

그리고 추후 다른 팀원의 작업 이후 불필요하게 많은 변경사항이 생기는 것을 방지하기 위해, 짱수가 변경한 파일들을 포맷팅해서 반영하면 좋겠어요.

인텔리제이에서 파일을 열고 ⌘⌥L 을 누르면 된답니당

<code_scheme name="ZeusStyle" version="173">
  <option name="KEEP_CONTROL_STATEMENT_IN_ONE_LINE" value="false" />
  <option name="KEEP_BLANK_LINES_BEFORE_RBRACE" value="0" />
  <option name="SPACE_BEFORE_ARRAY_INITIALIZER_LBRACE" value="true" />
  <option name="CALL_PARAMETERS_WRAP" value="1" />
  <option name="METHOD_PARAMETERS_WRAP" value="1" />
  <option name="EXTENDS_LIST_WRAP" value="1" />
  <option name="THROWS_KEYWORD_WRAP" value="1" />
  <option name="METHOD_CALL_CHAIN_WRAP" value="1" />
  <option name="BINARY_OPERATION_WRAP" value="1" />
  <option name="BINARY_OPERATION_SIGN_ON_NEXT_LINE" value="true" />
  <option name="TERNARY_OPERATION_WRAP" value="1" />
  <option name="TERNARY_OPERATION_SIGNS_ON_NEXT_LINE" value="true" />
  <option name="FOR_STATEMENT_WRAP" value="1" />
  <option name="ARRAY_INITIALIZER_WRAP" value="1" />
  <option name="WRAP_COMMENTS" value="true" />
  <option name="IF_BRACE_FORCE" value="3" />
  <option name="DOWHILE_BRACE_FORCE" value="3" />
  <option name="WHILE_BRACE_FORCE" value="3" />
  <option name="FOR_BRACE_FORCE" value="3" />
  <DBN-PSQL>
    <case-options enabled="true">
      <option name="KEYWORD_CASE" value="lower" />
      <option name="FUNCTION_CASE" value="lower" />
      <option name="PARAMETER_CASE" value="lower" />
      <option name="DATATYPE_CASE" value="lower" />
      <option name="OBJECT_CASE" value="preserve" />
    </case-options>
    <formatting-settings enabled="false" />
  </DBN-PSQL>
  <DBN-SQL>
    <case-options enabled="true">
      <option name="KEYWORD_CASE" value="lower" />
      <option name="FUNCTION_CASE" value="lower" />
      <option name="PARAMETER_CASE" value="lower" />
      <option name="DATATYPE_CASE" value="lower" />
      <option name="OBJECT_CASE" value="preserve" />
    </case-options>
    <formatting-settings enabled="false">
      <option name="STATEMENT_SPACING" value="one_line" />
      <option name="CLAUSE_CHOP_DOWN" value="chop_down_if_statement_long" />
      <option name="ITERATION_ELEMENTS_WRAPPING" value="chop_down_if_not_single" />
    </formatting-settings>
  </DBN-SQL>
  <JavaCodeStyleSettings>
    <option name="NEW_LINE_WHEN_BODY_IS_PRESENTED" value="true" />
    <option name="INSERT_INNER_CLASS_IMPORTS" value="true" />
    <option name="CLASS_COUNT_TO_USE_IMPORT_ON_DEMAND" value="999" />
    <option name="NAMES_COUNT_TO_USE_IMPORT_ON_DEMAND" value="999" />
    <option name="PACKAGES_TO_USE_IMPORT_ON_DEMAND">
      <value />
    </option>
    <option name="IMPORT_LAYOUT_TABLE">
      <value>
        <package name="java" withSubpackages="true" static="true" />
        <emptyLine />
        <package name="javax" withSubpackages="true" static="true" />
        <emptyLine />
        <package name="jakarta" withSubpackages="true" static="true" />
        <emptyLine />
        <package name="org" withSubpackages="true" static="true" />
        <emptyLine />
        <package name="net" withSubpackages="true" static="true" />
        <emptyLine />
        <package name="com" withSubpackages="true" static="true" />
        <emptyLine />
        <package name="" withSubpackages="true" static="true" />
        <emptyLine />
        <package name="java" withSubpackages="true" static="false" />
        <emptyLine />
        <package name="javax" withSubpackages="true" static="false" />
        <emptyLine />
        <package name="jakarta" withSubpackages="true" static="false" />
        <emptyLine />
        <package name="org" withSubpackages="true" static="false" />
        <emptyLine />
        <package name="net" withSubpackages="true" static="false" />
        <emptyLine />
        <package name="com" withSubpackages="true" static="false" />
        <emptyLine />
        <package name="" withSubpackages="true" static="false" />
      </value>
    </option>
    <option name="ALIGN_MULTILINE_RECORDS" value="false" />
    <option name="JD_PRESERVE_LINE_FEEDS" value="true" />
  </JavaCodeStyleSettings>
  <codeStyleSettings language="HTML">
    <indentOptions>
      <option name="INDENT_SIZE" value="2" />
      <option name="CONTINUATION_INDENT_SIZE" value="4" />
      <option name="TAB_SIZE" value="2" />
    </indentOptions>
  </codeStyleSettings>
  <codeStyleSettings language="JAVA">
    <option name="KEEP_CONTROL_STATEMENT_IN_ONE_LINE" value="false" />
    <option name="KEEP_BLANK_LINES_IN_CODE" value="1" />
    <option name="KEEP_BLANK_LINES_BEFORE_RBRACE" value="1" />
    <option name="ALIGN_MULTILINE_PARAMETERS" value="false" />
    <option name="CALL_PARAMETERS_WRAP" value="1" />
    <option name="METHOD_PARAMETERS_WRAP" value="1" />
    <option name="METHOD_PARAMETERS_RPAREN_ON_NEXT_LINE" value="true" />
    <option name="EXTENDS_LIST_WRAP" value="1" />
    <option name="THROWS_KEYWORD_WRAP" value="1" />
    <option name="METHOD_CALL_CHAIN_WRAP" value="1" />
    <option name="BINARY_OPERATION_WRAP" value="1" />
    <option name="BINARY_OPERATION_SIGN_ON_NEXT_LINE" value="true" />
    <option name="TERNARY_OPERATION_WRAP" value="1" />
    <option name="TERNARY_OPERATION_SIGNS_ON_NEXT_LINE" value="true" />
    <option name="KEEP_SIMPLE_METHODS_IN_ONE_LINE" value="true" />
    <option name="KEEP_SIMPLE_LAMBDAS_IN_ONE_LINE" value="true" />
    <option name="KEEP_SIMPLE_CLASSES_IN_ONE_LINE" value="true" />
    <option name="FOR_STATEMENT_WRAP" value="1" />
    <option name="ARRAY_INITIALIZER_WRAP" value="1" />
    <option name="WRAP_COMMENTS" value="true" />
    <option name="IF_BRACE_FORCE" value="3" />
    <option name="DOWHILE_BRACE_FORCE" value="3" />
    <option name="WHILE_BRACE_FORCE" value="3" />
    <option name="FOR_BRACE_FORCE" value="3" />
    <option name="WRAP_LONG_LINES" value="true" />
    <arrangement>
      <groups>
        <group>
          <type>GETTERS_AND_SETTERS</type>
          <order>KEEP</order>
        </group>
        <group>
          <type>OVERRIDDEN_METHODS</type>
          <order>KEEP</order>
        </group>
      </groups>
    </arrangement>
  </codeStyleSettings>
  <codeStyleSettings language="Markdown">
    <indentOptions>
      <option name="INDENT_SIZE" value="2" />
    </indentOptions>
  </codeStyleSettings>
</code_scheme>

인수 테스트 리뷰에는 우리의 브라운께서 작년에 발표하신 영상 자료의 4:13부터 4:56의 내용을 참고했어요. 🔥

zeus6768 · 2024-12-20T17:44:06Z

backend/src/main/java/codezap/auth/controller/AuthController.java

+        //ArgumentResolver 와 동작이 일치
+        CredentialManager credentialManager = credentialManagers.stream()
+                .filter(eachCredentialManager -> eachCredentialManager.hasCredential(httpServletRequest))
+                .findFirst()
+                .orElseThrow(() -> new CodeZapException(ErrorCode.UNAUTHORIZED_USER, "인증 정보가 없습니다. 다시 로그인 해 주세요."));


member가 null로 들어온 경우에는 어떻게 동작할지 궁금해요!

zeus6768 · 2024-12-20T17:51:09Z

backend/src/test/java/codezap/auth/AuthTest.java

+    class Login {
+
+        @Nested
+        @DisplayName("로그인에 성공:")


정상적인 ID와 패스워드를 입력해야 로그인에 성공할 수 있다는 내용이 들어가면 좋겠어요~!

zeus6768 · 2024-12-20T17:55:56Z

backend/src/test/java/codezap/auth/AuthTest.java

+            @Test
+            @DisplayName("정상적인 인증 쿠키 반환")
+            void responseCookie() throws Exception {
+                //when
+                Cookie[] cookies = loginResult.getResponse().getCookies();
+
+                //then
+                mvc.perform(get("/login/check")
+                                .cookie(cookies))
+                        .andExpect(status().isOk());
+            }
+
+            @Test
+            @DisplayName("정상적인 인증 헤더 반환")
+            void responseHeader() throws Exception {
+                //when & then
+                MockHttpServletResponse response = loginResult.getResponse();
+                String authorizationHeader = response.getHeader(HttpHeaders.AUTHORIZATION);
+
+                mvc.perform(get("/login/check")
+                                .header(HttpHeaders.AUTHORIZATION, authorizationHeader))
+                        .andExpect(status().isOk());
+            }


헤더, 쿠키는 인수테스트의 설명을 위해 사용하기에 적절한 단어가 아니라는 생각이 들어요.

로그인 전에는 접근할 수 없는 자원에 로그인 이후 접근할 수 있게 된다는 것을 보여주는 테스트가 되면 좋을 것 같아요.

저도 제우스 의견에 동의해요!
지금의 테스트는 컨트롤러 단위 테스트와 흡사해보이는 것 같아요!

음.. 뭔가 인수테스트의 개념이 명확하지 않은 것 같아요. 인수테스트의 개념에 대해 백엔드 내에서 정의를 해보면 좋을 것 같네요.

우선 제가 테스트 하고 싶었던 부분은 Cookie / Authorization 헤더 모두 정상적으로 credential 정보가 설정이 되는것이었어요.
즉, Controller 하나의 단위가 아닌 두 개의 CredentialManager 까지 함께 동작해야 하는 통합테스트에 해당한다고 생각했습니다.

다만, 헤더 / 쿠키 라는 이름이 인수테스트에 적합하지 않다는 의견에도 동의해요.
일단 인수테스트와 단순 통합 테스트를 분리해 두겠습니다!!

zeus6768 · 2024-12-20T18:02:53Z

backend/src/test/java/codezap/auth/AuthTest.java

+
+    @Nested
+    @DisplayName("로그인 확인")
+    class CheckLogin {


사용자의 로그인 상태가 유효한지 검증한다 라는 시나리오를 만들어 테스트하면 좋을 것 같아요.

로그인 API와 로그인 확인 API의 목적은 다르니까요.

zeus6768 · 2024-12-20T18:04:18Z

backend/src/test/java/codezap/auth/AuthTest.java

+            String password = "password123!";
+            signup(name, password);
+            MvcResult loginResponse = requestLogin(name, password).andReturn();
+            Pattern expireCookieRegex = Pattern.compile("credential=.*?; Max-Age=0;.*?");


인수 테스트이므로 애초에 테스트하기 좋은 시나리오를 만드는 것도 방법이라고 생각해요.

로그인 상태에서는 정상적으로 접근할 수 있던 자원 또는 기능에 대해서 로그아웃 후에는 접근하지 못하게 하는 테스트가 되면 좋을 것 같아요.

zeus6768 · 2024-12-20T18:05:43Z

backend/src/test/java/codezap/auth/AuthTest.java

+import org.springframework.test.web.servlet.MvcResult;
+import org.springframework.test.web.servlet.ResultActions;
+
+class AuthTest extends MvcTest {


클래스명에 Acceptance라는 단어가 들어가면 인수테스트라는 정보를 더 명확히 알 수 있을 것 같아요!

zeus6768 · 2024-12-20T18:06:28Z

backend/src/test/java/codezap/global/MvcTest.java

+public abstract class MvcTest {
+
+    protected MockMvc mvc;
+    protected ObjectMapper objectMapper;


혹시 objectMapper는 @Autowired를 사용하는 대신 직접 생성하는 이유가 뭘까요??

오, 단순히 MockMvcTest 의 코드를 최대한 재활용했는데요!
생각해 보니 이 클래스는 실제로 사용되는 모든 클래스를 그대로 사용하기 위한 테스트이니 @Autowired를 사용하는 것이 더 좋을 것 같네용

zeus6768 · 2024-12-20T18:07:23Z

backend/src/test/java/codezap/global/MvcTest.java

+
+@SpringBootTest
+@EnableConfigurationProperties(CorsProperties.class)
+public abstract class MvcTest {


일단 동작한다면 충분하다고 생각해요! 간결하고 좋네요 ㅎㅎ

jminkkk

안녕하세요 짱수!
소소한 코멘트 남겨보았어요!
리뷰 시간이 지나서 선택적으로 반영해주시면 좋을 것 같아요 수고하셨습니다 👍👍

backend/src/test/java/codezap/global/MvcTest.java

jminkkk · 2024-12-21T09:19:09Z

backend/src/test/java/codezap/auth/AuthTest.java

+            @Test
+            @DisplayName("정상적인 인증 쿠키 반환")
+            void responseCookie() throws Exception {
+                //when
+                Cookie[] cookies = loginResult.getResponse().getCookies();
+
+                //then
+                mvc.perform(get("/login/check")
+                                .cookie(cookies))
+                        .andExpect(status().isOk());
+            }
+
+            @Test
+            @DisplayName("정상적인 인증 헤더 반환")
+            void responseHeader() throws Exception {
+                //when & then
+                MockHttpServletResponse response = loginResult.getResponse();
+                String authorizationHeader = response.getHeader(HttpHeaders.AUTHORIZATION);
+
+                mvc.perform(get("/login/check")
+                                .header(HttpHeaders.AUTHORIZATION, authorizationHeader))
+                        .andExpect(status().isOk());
+            }


저도 제우스 의견에 동의해요!
지금의 테스트는 컨트롤러 단위 테스트와 흡사해보이는 것 같아요!

jminkkk · 2024-12-21T09:19:41Z

backend/src/test/java/codezap/auth/AuthTest.java

+import org.springframework.test.web.servlet.MvcResult;
+import org.springframework.test.web.servlet.ResultActions;
+
+class AuthTest extends MvcTest {


저도 제우스 의견처럼 이름이 조금 더 명확하면 좋을 것 같아요!
AuthAcceptanceTest 좋네요!

jminkkk · 2024-12-21T09:22:52Z

backend/src/main/java/codezap/auth/controller/AuthController.java

@@ -21,7 +24,7 @@
 @RequiredArgsConstructor
 public class AuthController implements SpringDocAuthController {

-    private final CredentialManager credentialManager;
+    private final List<CredentialManager> credentialManagers;


이 리스트는 혹시 페어할 때 이야기 나누었던 것처럼 Adapter로 발전할 예정일까요?
Adapter 같은 일급 컬렉션으로 추출되면 적절한 CredentialManager를 반환하는지 테스트하기 더 좋을 것 같네요👍

아마 프론트 코드 변경 이후엔 다시 List<CredentialManager> -> CredentialManager 로 변경될 것 같아요. 그래서 지금 예상은 Adapter 로 변경되지 않을 것 같네용

오홍... 뭔가 우리 클라이언트의 환경이 무려 4개(웹, 익스텐션, vs 플러그인, intellij 플러그인)이기도 하고 ....!
호환성을 생각한다면 Mapping 객체를 하나 두는 게 더 낫다고 생각하긴 하는데 짱수는 어떤가요...!

+) 다시 생각해보니 객체 이름은 Adapter보다는 Mapping이 더 의미가 맞는 것 같아요 정정할게요!
(스프링 MVC 미션할 때 구현했던 HandlerMapping에서 착안)

backend/src/test/java/codezap/global/MvcTest.java

HoeSeong123

고생하셨습니다 짱수.
수정사항은 딱히 없는 것 같고 간단하게 제 의견만 몇 개 달아두었습니다.
인수테스트 관련해서 피드백들이 많네요.
개인적으로 인수테스트의 정의가 명확하게 내려졌으면 좋겠습니다ㅎㅎ
나중에 같이 얘기해봐요

고생하셨습니다!!

HoeSeong123 · 2024-12-21T10:17:24Z

backend/src/test/java/codezap/auth/AuthTest.java

+            @Test
+            @DisplayName("정상적인 인증 쿠키 반환")
+            void responseCookie() throws Exception {
+                //when
+                Cookie[] cookies = loginResult.getResponse().getCookies();
+
+                //then
+                mvc.perform(get("/login/check")
+                                .cookie(cookies))
+                        .andExpect(status().isOk());
+            }
+
+            @Test
+            @DisplayName("정상적인 인증 헤더 반환")
+            void responseHeader() throws Exception {
+                //when & then
+                MockHttpServletResponse response = loginResult.getResponse();
+                String authorizationHeader = response.getHeader(HttpHeaders.AUTHORIZATION);
+
+                mvc.perform(get("/login/check")
+                                .header(HttpHeaders.AUTHORIZATION, authorizationHeader))
+                        .andExpect(status().isOk());
+            }


음.. 뭔가 인수테스트의 개념이 명확하지 않은 것 같아요. 인수테스트의 개념에 대해 백엔드 내에서 정의를 해보면 좋을 것 같네요.

HoeSeong123 · 2024-12-21T10:24:42Z

backend/src/test/java/codezap/auth/AuthTest.java

+            String password = "password123!";
+            signup(name, password);
+            MvcResult loginResponse = requestLogin(name, password).andReturn();
+            Pattern expireCookieRegex = Pattern.compile("credential=.*?; Max-Age=0;.*?");


저는 인수테스트를 사용자에게 초점을 둔 테스트라고 이해했습니다.
그런 관점에서 쿠키 정보가 삭제되었는지를 사용자가 테스트할 필요는 없다고 생각해요.
제우스 말대로 로그인 전에는 내 템플릿을 조회할 수 있었는데 로그아웃을 하고 나면 조회할 수 없는 시나리오 등으로 테스트 해봐도 좋을 것 같네요.
근데 이게 인수테스트가 맞는지는 모르겠어요. 나중에 같이 인수테스트에 대해 얘기해봐요.

HoeSeong123 · 2024-12-21T10:33:46Z

backend/src/test/java/codezap/global/MvcTest.java

+
+@SpringBootTest
+@EnableConfigurationProperties(CorsProperties.class)
+public abstract class MvcTest {


저는 개인적으로 통합테스트에서는 RestAssured를 사용하는 것을 선호합니다.
테스트의 가독성이 좋고 json 데이터도 더 쉽게 검증할 수 있기 때문입니다.
더군다나 RestAssured는 BDD 스타일로 작성할 수 있는데, 인수테스트가 사용자 시나리오를 테스트하는 것이라면 BDD의 장점을 더 잘 활용할 수 있다고 생각해요.

다른 분들 의견은 어떤가요?

zangsu added 4 commits December 18, 2024 16:34

refactor: AuthorizationHeader 를 함께 사용하도록 변경

c4a885a

test: 테스트 코드에서 재사용성을 위한 리팩토링

1766805

test: 통합 테스트 관점으로 테스트 수정

b61fdf2

test: Auth 관련 통합 테스트 작성

a24bdcc

zangsu added refactor 요구사항이 바뀌지 않은 변경사항 BE 백엔드 labels Dec 19, 2024

zangsu added this to the 7차 스프린트 💭 milestone Dec 19, 2024

zangsu requested review from zeus6768, jminkkk, kyum-q and HoeSeong123 December 19, 2024 11:14

zangsu self-assigned this Dec 19, 2024

zangsu commented Dec 19, 2024

View reviewed changes

test: 잘못된 비밀번호 부분에 잘못된 값을 사용하도록 변경

6e8bd43

kyum-q approved these changes Dec 19, 2024

View reviewed changes

zeus6768 requested changes Dec 20, 2024

View reviewed changes

jminkkk approved these changes Dec 21, 2024

View reviewed changes

HoeSeong123 approved these changes Dec 21, 2024

View reviewed changes

Authorization 헤더를 응답하도록 변경 #981

Are you sure you want to change the base?

Authorization 헤더를 응답하도록 변경 #981

Conversation

zangsu commented Dec 19, 2024

⚡️ 관련 이슈

📍주요 변경 사항

🎸기타

🍗 PR 첫 리뷰 마감 기한

zangsu Dec 19, 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

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

kyum-q 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

zeus6768 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

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

jminkkk 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

zangsu Dec 23, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

HoeSeong123 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

zangsu Dec 19, 2024 •

edited

Loading

zangsu Dec 23, 2024 •

edited

Loading