[피오 & 노리] 웹서버 4단계 - 쿠키를 이용한 로그인 구현 by NB993 · Pull Request #67 · codesquad-members-2022/java-was

NB993 · 2022-03-31T06:35:07Z

안녕하세요!! 피오 & 노리 입니다. 👬

"웹서버 4 단계 - 쿠키를 이용한 로그인 구현" 완료하여 pull request 요청드립니다.

감사합니다! 🙇

3단계 리뷰 항목

중복 회원가입 요청 처리 보완
리다이렉션 메서드 재사용 가능하도록 보완
RequestLine 생성자 매개변수 변경

기능요구사항

회원가입한 사용자로 로그인을 할 수 있어야 한다.
“로그인” 메뉴를 클릭하면 http://localhost:8080/user/login.html 으로 이동해 로그인할 수 있다.
로그인이 성공하면 index.html로 이동하고, 로그인이 실패하면 /user/login_failed.html로 이동해야 한다.

프로그래밍 요구사항

앞 단계에서 회원가입할 때 생성한 User 객체를 DataBase.addUser() 메서드를 활용해 메모리에 저장한다.
아이디와 비밀번호가 같은지를 확인해서 로그인이 성공하면 응답 header의 Set-Cookie 값을 sessionId=적당한값으로 설정한다.
Set-Cookie 설정시 모든 요청에 대해 Cookie 처리가 가능하도록 Path 설정 값을 /(Path=/)로 설정한다.
응답 header에 Set-Cookie값을 설정한 후 요청 header에 Cookie이 전달되는지 확인한다.

추가 요구 사항

(선택) 로그아웃을 구현한다.

- 로그인이 성공하면 index.html로 이동하고, 로그인이 실패하면 /user/login_failed.html로 이동시킨다. - 아이디와 비밀번호가 같은지를 확인해서 로그인이 성공하면 응답 header의 Set-Cookie 값을 sessionId로 지정해준다. 값은 UUID로 생성한다.

- 로그아웃 요청이 오면 쿠키를 만료시킨다.

requestLine 정보를 ReqeustLine이 파싱하도록 함.

- 중복된 userId로 회원가입 요청이 들어올시, 예외를 발생시킨 후 /user/form.html로 이동시킨다. - HttpResponse클래스의 리다이렉션 메서드의 재활용성을 높이기 위해 location을 매개변수로 받는다.

ksundong

안녕하세요. 피오 & 노리, 리뷰어 dion 입니다.

4단계 미션을 수행하느라 고생하셨습니다.

기능은 문제가 없으나, 보다 좋은 코드를 작성할 필요가 있다는 생각이 들어서 변경요청 드립니다. 고생하셨습니다.

ksundong · 2022-04-02T14:10:18Z

src/main/java/db/SessionDataBase.java

+
+public class SessionDataBase {
+
+    private static final Map<String, String> SESSIONS = new HashMap<>();


HashMap 을 사용해주신 이유는 무엇인가요?

ksundong · 2022-04-02T14:10:53Z

src/main/java/db/SessionDataBase.java

+    public static void save(String sessionId, String userId) {
+        SESSIONS.put(sessionId, userId);
+    }
+
+    public static void remove(String sessionId) {
+        SESSIONS.remove(sessionId);
+    }


메서드명 👍

ksundong · 2022-04-02T14:11:39Z

src/main/java/webserver/HttpRequest.java

-        String[] requestLineSplit = requestLine.split(" ");
-        this.requestLine = new RequestLine(requestLineSplit[0], requestLineSplit[1], requestLineSplit[2]);
+        this.requestLine = new RequestLine(requestLine);
        this.headers = IOUtils.readRequestHeader(br);


init() 메서드 같은건 필요없죠. 앞으로도 계속 지양해주세요.

또 BufferedReader 가 상태로 관리되는 이유가 있나요?

ksundong · 2022-04-02T14:12:57Z

src/main/java/webserver/HttpResponse.java

+        } catch (IOException e) {
+            log.error(e.getMessage());
+        }


이건 올바른 예외처리가 아닙니다. 지양해주세요.

ksundong · 2022-04-02T14:13:35Z

src/main/java/webserver/HttpResponse.java

            dos.writeBytes("HTTP/1.1 302 FOUND \r\n");
-            dos.writeBytes("Location: http://localhost:8080/index.html\r\n");
+            dos.writeBytes("Location: http://localhost:8080" + path + "\r\n");
+            dos.writeBytes("Set-Cookie: sessionId=" + cookie + "; Max-Age=-1;  path=/");
+            dos.writeBytes("\r\n");
+        } catch (IOException e) {
+            log.error(e.getMessage());
+        }
+    }
+
+    public void response302WithCookieHeader(String path, String cookie) {
+        try {
+            dos.writeBytes("HTTP/1.1 302 FOUND \r\n");
+            dos.writeBytes("Location: http://localhost:8080" + path + "\r\n");
+            dos.writeBytes("Set-Cookie: sessionId=" + cookie + "; path=/");
+            dos.writeBytes("\r\n");
+        } catch (IOException e) {
+            log.error(e.getMessage());
+        }
+    }
+
+    public void response302Header(String path) {
+        try {
+            dos.writeBytes("HTTP/1.1 302 FOUND \r\n");
+            dos.writeBytes("Location: http://localhost:8080" + path + "\r\n");


중복되는 코드가 많네요. 재사용을 고려할 필요는 없을까요?

ksundong · 2022-04-02T14:15:06Z

src/main/java/webserver/HttpResponse.java

        try {
            dos.writeBytes("HTTP/1.1 302 FOUND \r\n");
-            dos.writeBytes("Location: http://localhost:8080/index.html\r\n");
+            dos.writeBytes("Location: http://localhost:8080" + path + "\r\n");


이 객체가 stream 을 직접 가지고서 응답을 반환하는 것은 도메인을 담당하는 객체가 출력을 하는 것과 동일합니다. 지양해주세요.

ksundong · 2022-04-02T14:15:38Z

src/main/java/webserver/RequestHandler.java

+            HttpResponse httpResponse = new HttpResponse(out);

-            if (httpRequest.getPath().contains("/user/create")) {
+            if (httpRequest.getPath().equals("/user/create")) {
                User user = new User(
                    httpRequest.getParameter("userId"),
                    httpRequest.getParameter("password"),
                    httpRequest.getParameter("name"),
                    httpRequest.getParameter("email")
                );
-                DataBase.addUser(user);
-                HttpResponse httpResponse = new HttpResponse(out);
-                httpResponse.response302Header();
+
+                try {
+                    DataBase.addUser(user);
+                    httpResponse.response302Header("/index.html");
+                } catch (IllegalArgumentException e) {
+                    log.debug("exception: {}", e.getMessage());
+                    httpResponse.response302Header("/user/form.html");
+                }
+                return;
+            }
+
+            if (httpRequest.getPath().equals("/user/login")) {
+                User user = DataBase.findUserById(httpRequest.getParameter("userId"));
+                if (user == null) {
+                    httpResponse.response302Header("/user/login_failed.html");
+                    return;
+                }
+                if (!user.getPassword().equals(httpRequest.getParameter("password"))) {
+                    httpResponse.response302Header("/user/login_failed.html");
+                    return;
+                }
+                String sessionId = UUID.randomUUID().toString();
+                log.debug("return cookie: {}", sessionId);
+                SessionDataBase.save(sessionId, user.getUserId());
+                httpResponse.response302WithCookieHeader("/index.html", sessionId);
+                return;
+            }
+
+            if (httpRequest.getPath().equals("/user/logout")) {
+                Map<String, String> cookies = HttpRequestUtils.parseCookies(
+                    httpRequest.getHeader("Cookie"));
+                String sessionId = cookies.get("sessionId");
+                log.debug("sessionId = {}", sessionId);
+                if (sessionId == null) {
+                    httpResponse.response302Header("/index.html");
+                    return;
+                }
+                httpResponse.response302WithExpiredCookieHeader("/index.html", sessionId);
+                SessionDataBase.remove(sessionId);


메서드가 너무 길죠. 리팩토링 해주세요. depth 2단계는 여기서도 지양해주셔야 합니다.

NB993 and others added 4 commits March 31, 2022 14:11

feat: 쿠키를 이용한 로그아웃 구현

34d5a01

- 로그아웃 요청이 오면 쿠키를 만료시킨다.

refactor: RequestLine 생성자 매개변수 변경

02c22ed

requestLine 정보를 ReqeustLine이 파싱하도록 함.

NB993 added the review-BE Improvements or additions to documentation label Mar 31, 2022

ksundong self-assigned this Apr 2, 2022

ksundong requested changes Apr 2, 2022

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[피오 & 노리] 웹서버 4단계 - 쿠키를 이용한 로그인 구현#67

[피오 & 노리] 웹서버 4단계 - 쿠키를 이용한 로그인 구현#67
NB993 wants to merge 4 commits intocodesquad-members-2022:pio-norifrom
nohriter:step4

NB993 commented Mar 31, 2022

Uh oh!

ksundong left a comment

Uh oh!

ksundong Apr 2, 2022

Uh oh!

ksundong Apr 2, 2022

Uh oh!

ksundong Apr 2, 2022

Uh oh!

ksundong Apr 2, 2022

Uh oh!

ksundong Apr 2, 2022

Uh oh!

ksundong Apr 2, 2022

Uh oh!

ksundong Apr 2, 2022

Uh oh!

ksundong Apr 2, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants


		public class SessionDataBase {

		private static final Map<String, String> SESSIONS = new HashMap<>();

Conversation

NB993 commented Mar 31, 2022

3단계 리뷰 항목

기능요구사항

프로그래밍 요구사항

추가 요구 사항

Uh oh!

ksundong left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants