-
Notifications
You must be signed in to change notification settings - Fork 1
Conversation
phantom08266
commented
Mar 14, 2021
- 이메일, 비밀번호를 입력받는다.(Null 체크)
- DB에 등록되어있는 회원인지 체크한다.
- 앱 종료 후 앱을 실행하면 자동 로그인이 되어 사용 가능하도록 한다.(세션방식)
- 회원가입 기능을 통해 Careers를 로그인 한 사용자들만 이용할 수 있도록 한다. - 이름, 이메일, 비밀번호 모두를 입력 받는다.(Null 체크) - 이메일은 이메일 형식에 맞게 작성해야 한다. - 비밀번호는 문자, 숫자, 특수문자로 구성되어야 한다. - 비밀번호는 암호화하여 DB에 저장한다.(sha-256) - 중복된 이메일인지 체크하여 중복가입을 제한한다. #3
put -> post
이메일, 비밀번호 위반 시 status code 반환하도록 수정
단일책임원칙 적용
명확한 의도를 설정하기위해 Data 애노테이션 대신 각각 설정
여러 스레드가 해당 부분을 접근할 일이 현재로서는 없으므로 StringBuilder로 수정
…ion.java Fix 코딩 컨벤션 위반 수정 Co-authored-by: f-lab <54677861+f-lab-dev@users.noreply.github.com>
Encryption -> Encryptor 로 변경
- 이메일, 비밀번호를 입력받는다.(Null 체크) - DB에 등록되어있는 회원인지 체크한다. - 앱 종료 후 앱을 실행하면 자동 로그인이 되어 사용 가능하도록 한다.(세션방식) #4
# Conflicts: # src/main/java/com/dev/careers/controller/CuratorController.java # src/main/java/com/dev/careers/mapper/CuratorMapper.java # src/main/java/com/dev/careers/service/CuratorService.java # src/main/resources/mybatis/CuratorMapper.xml # src/test/java/com/dev/careers/controller/CuratorControllerTest.java # src/test/java/com/dev/careers/service/CuratorServiceTest.java
src/main/java/com/dev/careers/controller/CuratorController.java
Outdated
Show resolved
Hide resolved
exists를 사용하여 쿼리성능향상 및 이메일 존재 유무에 대해 명확하게 표현
# Conflicts: # src/main/java/com/dev/careers/service/CuratorService.java # src/main/resources/mybatis/CuratorMapper.xml
src/main/java/com/dev/careers/service/session/SessionController.java
Outdated
Show resolved
Hide resolved
src/main/java/com/dev/careers/service/session/SessionController.java
Outdated
Show resolved
Hide resolved
src/main/java/com/dev/careers/service/session/SessionController.java
Outdated
Show resolved
Hide resolved
sessionController.SetSession(Optional.ofNullable(httpSession), loginParamter); | ||
} | ||
|
||
public void logout(HttpSession httpSession){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
로그인/로그아웃은 회원정보를 담당
하는 클래스의 책임일까요? 혹은 인증/인가
를 담당하는 클래스의 책임일까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
로그인/로그아웃은 인증에 관한 문제임으로 인증/인가를 담당하는 클래스에서 책임을 지도록 수정하겠습니다.
카멜케이스 적용
로그인/로그아웃에 대한 세션처리부분을 SessionAuthenticator에서 담당하도록 수정
@@ -47,10 +47,10 @@ public void login(LoginParamter loginParamter, HttpSession httpSession) throws N | |||
memberInfo.filter(v -> hashing.equals(v.get("password"))) | |||
.orElseThrow(ViolationException::new); | |||
|
|||
sessionController.SetSession(Optional.ofNullable(httpSession), loginParamter); | |||
sessionAuthenticator.setSession(Optional.ofNullable(httpSession), loginParamter); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
회원가입시에 꼭 로그인을 시켜줘야할까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
해당부분은 코드에 반영되어있지 않습니다. 확인 부탁드립니다.
|
||
|
||
public void SetSession(Optional<HttpSession> httpSession, LoginParamter loginParamter) { | ||
public void setSession(Optional<HttpSession> httpSession, LoginParamter loginParamter) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
클래스의 이름처럼 이 클래스는 인증과 관련된 책임을 가지고있습니다.
메소드 이름도 인증 관점의 이름을 지어줘야하지 않을까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
그리고 httpSession
은 @Autowired
로 주입이 가능합니다~ 굳이 인자로 받을 필요가 없어보이네요
- SetAttribute를 호춣한 시점에 Servlet Container가 생성한 Session을 주입받도록 수정 - Session 인증 처리 메서드 이름 변경
|
||
public void setSession(Optional<HttpSession> httpSession, LoginParamter loginParamter) { | ||
httpSession.ifPresent(session -> session.setAttribute(sessionName, loginParamter)); | ||
public void accreditSession(LoginParamter loginParamter) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
인증과 관련된 부분이므로 login
/ logout
이라는 메소드명을 쓰는게 더 좋지 않을까요?
세션 불일치 문제 고민한것도 결론을 내셔서 소스코드에 반영해보면 좋겠습니다~ |
이거는 작업 진행중이신걸까요? |
병준님 로그인 작업이 완료되면 머지를 할 예정입니다. 세션불일치 문제 반영은 추후 별도의 이슈를 생성하여 처리하도록 하겠습니다. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
그런데 병준님 코드와 이 작업은 관련이 없어보이는데 어떤 이유로 기다리고 계신걸까요?
병준님이 로그인 기능까지 완료되면 회의를 통해 두 코드를 통합할 예정이었습니다. 아니면 먼저 머지하고 추후 병준님이 완료 후 통합하는 방식으로 진행할까요? |
네 일단 로그인을 제일 먼저 완료하셨으니 준희님 코드를 베이스로 진행하는게 나아보입니다~ |
# Conflicts: # build.gradle # src/main/java/com/dev/careers/controller/CuratorController.java # src/main/java/com/dev/careers/mapper/CuratorMapper.java # src/main/java/com/dev/careers/model/Curator.java # src/main/java/com/dev/careers/service/CuratorService.java # src/main/resources/application.properties # src/main/resources/mybatis/CuratorMapper.xml # src/test/java/com/dev/careers/controller/CuratorControllerTest.java # src/test/java/com/dev/careers/service/CuratorServiceTest.java