Skip to content
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

[테리 & 쿠킴] 웹서버 4단계 - 쿠키를 이용한 로그인 구현 #61

Merged
merged 17 commits into from Mar 31, 2022

Conversation

mybloom
Copy link
Member

@mybloom mybloom commented Mar 30, 2022

안녕하세요 쿠킴, 테리입니다.
3단계 PR 피드백 반영과 4단계 요구사항 구현하여 PR 보내드립니다.
미리 리뷰 감사합니다. 😀


3단계 PR(#45 ) 피드백 반영

  • 프로젝트 전체에 indent를 tab으로 맞춰주었습니다. 해당 포매팅 적용으로 많은 파일이 변경되었습니다.
  • FrontController의 중복되는 if/else를 리팩토링
  • Request 객체의 생성자 구조를 읽기 쉽게 리팩토링
  • Response 객체에 StatusLine과 Headers를 속성으로 구분
  • Response 응답을 전달해주는 메서드 toHeader()를 List가 아닌 StringBuffer에 담아 전달
  • Response 의 body를 전달해주는 toBody()에서 임의로 캐리지리턴을 보내준 부분을 없애고, html/css/js 파일에 개행을 추가
  • 변수명들의 네이밍을 더 명확하게 변경

4단계 구현, 요구 사항

  • 로그인 기능 구현
    • 성공 시, 쿠키 생성 후 index.html로 이동
    • 실패 시, /user/login_failed.html로 이동
  • 모든 요청에 대해 Cookie 처리가 가능하도록 Path 설정
  • 로그아웃 시, 쿠키 삭제되도록 구현

결과

View 🖼
  • GET /user/login.html - 로그인 페이지
    Imgur

  • 로그인 실패 : /user/login_failed.html로 302 리다이렉트
    Imgur

  • 로그인 성공 : 서버로부터 쿠키 응답과 /index.html로 302 리다이렉트
    Imgur

  • 로그아웃 : 서버로부터 쿠키 기한 끝 응답과 /index.html로 302 리다이렉트
    Imgur

ku-kim and others added 15 commits March 30, 2022 11:09
- 기존 .css, .js 요청일 때 Content-Type: text/html 라서 클라이언트가 .css, .js 파일 읽지 못하는 문제 발생
- 클라이언트의 파일 요청, 확장자에 맞게 Content-Type 변경하여 전송
- Request 객체 시 fileExtension 추가에 따른 생성 테스트 코드 추가
- id/password입력 후 로그인 클릭 시, 회원가입 하여 Database 데이터와 일치 여부 판단
- 로그인 성공시 /index.html 로 이동
- 로그인 실패시 /user/login_failed.html 로 이동
- Map 형태로 세션 데이터를 관리한다. (Key : UUID.toString(), Value : User.userId() // String)
- addSession(String userId) : userId 기준 세션, 쿠키를 생성하고 생성된 UUID를 문자열로 리턴한다.
- findUserIdByCookie(String cookie) : 입력 파라미터 cookie에 따라 세션DB에 있는 userId를 옵셔널로 리턴한다.
- deleteByCookie(String cookie) : 입력 파라미터 cookie에 따라 SessionDataBase에 있는 세션을 삭제한다.
- 로그인 성공시 SessionDabase 에 쿠키 생성 후, 해당 쿠키를 Set-Cookie 에 담아 응답
- Set-Cookie 내용 : sessionId=UUID; PATH:/
- PATH:/를 추가하여 모든 요청에 cookie처리 가능하도록 설정
- ../user/login.html -> /user/login.html 변경
- 로그아웃 버튼 클릭 시 /user/logout 으로 되도록 변경
- Request 객체 생성시 cookie를 파싱하여 cookieMap을 생성한다.
- /user/logout 호출 시 cookieMap에 담은 sessionId를 SessionDabase에서 삭제한다.
- 응답 헤더 Set-Cookie에 "sessionId=deleted;"와 유효기간을 과거로 설정등을 넣어준다.
- 로그아웃 성공 시 index.html 로 리다이렉트 된다.
- 로직을 개행 단위로 구분
- 불필요한 로그 삭제
- if-else 문을 Map 을 활용해 단순하게 변경
- 기존 테스트에서 Optional 변경하고 테스트 수정하지 못한 부분 변경
- Request 생성자 구조를 메서드 추출로 가독성 향상
- BufferedReader br -> bufferedReader 으로 변경
- DataOutputStream dos -> dataOutputStream 으로 변경
- Response.responseHeaderMap에서 Status-Line 구분
- toHeader() 메서드에서 응답 헤더 생성할 때 전체를 StringBuffer 객체를 사용하여 문자 생성 후 리턴
- 기존 서버에서는 응답 바디 끝에 임의적으로 CRLF를 추가하여 전송
- RFC 2616 규약에 따라 응답 바디 끝에 CRLF를 서버에서 전송하는 것이 아닌 파일이 갖도록 변경
@ku-kim ku-kim added the review-BE Improvements or additions to documentation label Mar 30, 2022
ku-kim and others added 2 commits March 30, 2022 15:15
- Controller클래스를 interface로 바꾼 후 구현체의 생성자에서  super()제거
- ResponseTest 에 body뒤에 붙였던 개행문자 제거
method = tokens[0];
uri = parseUri(tokens[1]);
version = tokens[2];
switch (type) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

원래 코드보다 좋은 것 같아요.

Comment on lines +41 to +50
private String parseContentType(String contentType) {
if (contentType.equals("html")) {
return "text/html";
} else if (contentType.equals("js")) {
return "application/js";
} else if (contentType.equals("css")) {
return "text/css";
} else {
return "";
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

컨텐츠 타입은 enum으로 빼도 좋지 않을까요?

return body;
}

public void setCookie(String cookie) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이름은 setCookie지만 실제 하는 일은 sessionId를 쓰는 일 담당이군요. 흠... 좀더 적당한 이름과 매개변수병을 찾아보면 어떨까요?


User user = DataBase.findUserById(requestUserId);
if (isLogin(requestPassword, user)) {
String cookie = SessionDataBase.addSession(requestUserId);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
String cookie = SessionDataBase.addSession(requestUserId);
String sessionId = SessionDataBase.addSession(requestUserId);

Copy link
Contributor

@honux77 honux77 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

수고하셨습니다! 코드품질이 더 좋아진 것 같아요.

@honux77 honux77 merged commit 518efce into codesquad-members-2022:mybloom Mar 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
review-BE Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants