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

[JT-29] JWT 로그인 기능 구현 #9

Merged
merged 7 commits into from
Sep 6, 2023
Merged

[JT-29] JWT 로그인 기능 구현 #9

merged 7 commits into from
Sep 6, 2023

Conversation

parksey
Copy link
Collaborator

@parksey parksey commented Sep 5, 2023

📌 개발 내용

  • JWT를 이욯한 토큰 생성
  • 로그인 기능 구현
  • Spring security: security config추가 및 필터 생성

📑 PR 포인트

  • Security 필터 부분

추후 기능

  • InterceptorThreadLocal 사용 예정
  • Logout기능 추가

👥 협업을 위한 코드리뷰

  1. 세상에 바보같은 질문은 없다.
  2. 실수를 예방하는 팀이 아니라, 실수를 잘 다루는 팀이 되자.
  3. 분업이 아닌 협업을 하자. 우린 모두 같은 문제를 이겨내기 위해 모였다.
  4. 영원한 것은 없으니 대화를 하자.
  5. 서로 존중하자.
  6. 많이 부딪치고 깨닫고 배우자.
  7. 부드럽게 설득하고 열린 마음으로 설득당해보자.

✅ 리마인더

  • 본인의 로컬에서 정상 동작하는지 확인해주세요.
  • 최신 브랜치를 Pull 받고 PR을 요청했는지 확인해주세요.
  • API가 추가되었을 경우 테스트를 하고 PR을 올려주세요.
  • Conflict가 났을 때, UI상에서 해결하지 말고, 본인 local에서 해결해주세요.
  • Commit 메시지 제대로 작성해주세요.

⚙️ 코드리뷰 룰

  • R(Request Change): 해당 블럭은 꼭 변경해주셨으면 좋겠습니다.
  • C(Comment): 웬만하면 고려해주시면 좋겠습니다.
  • Q(Question) : 해당 라인이 궁금합니다.
  • A(Approve): 반영해도 좋고 넘어가도 좋습니다. 혹은 사소한 의견입니다.

Copy link
Member

@kmebin kmebin left a comment

Choose a reason for hiding this comment

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

고생하셧심다 🫡

Comment on lines 33 to 34
member.updateLastLogin();
return jwtProvider.generateToken(logInReq.email());
Copy link
Member

Choose a reason for hiding this comment

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

R: return 전에 줄바꿈 해주시요

Copy link
Member

Choose a reason for hiding this comment

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

R:

Member member = memberRepository.findByEmail(logInReq.email()).orElseThrow(
			() -> new BadCredentialsException("너 안돼!")
		);
		
Member member = memberRepository.findByEmail(logInReq.email())
                                        .orElseThrow(() -> new BadCredentialsException("너 안돼!"));

Comment on lines 24 to 26
@Override
protected void doFilterInternal(HttpServletRequest request, @NotNull HttpServletResponse response,
@NotNull FilterChain filterChain) throws ServletException, IOException {
Copy link
Member

Choose a reason for hiding this comment

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

R: 파라미터 길어지면 코틀린 컨벤션으루

Comment on lines 37 to 38

}
Copy link
Member

Choose a reason for hiding this comment

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

R: 함정 있음

Copy link
Member

Choose a reason for hiding this comment

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

R: 예외 시, 다음 필터안가게 리턴!

Comment on lines 29 to 36
@Value("${jwt.secret.key}")
private String salt;

@Value("${jwt.iss}")
private String iss;

@Value("${jwt.expire}")
private long expire;
Copy link
Member

Choose a reason for hiding this comment

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

R: 설정 파일에서 @Value로 값 가져오는 경우엔 상수인걸 명시하기 위해서 대문자로 표기하기로 햇슴다!

Comment on lines 60 to 61
headers.put("typ", "JWT");
return headers;
Copy link
Member

Choose a reason for hiding this comment

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

R: 줄바꿈 해주세용

Comment on lines 67 to 70
.build().parseClaimsJws(token);
if (claimsJws.getBody().getExpiration().before(new Date())) {
throw new RuntimeException("Token Expired");
}
Copy link
Member

Choose a reason for hiding this comment

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

R: 요기도 블록일 때 줄바꿈 하기로 했던 듯요

Copy link
Member

Choose a reason for hiding this comment

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

A: claimsJws.getBody().getExpiration().before(new Date()) 요거 따로 private 메서드로 빼도 좋을 것 같아요! 음 예를들면 isTokenExpired ..

Comment on lines 33 to 34
member.updateLastLogin();
return jwtProvider.generateToken(logInReq.email());
Copy link
Member

Choose a reason for hiding this comment

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

R:

Member member = memberRepository.findByEmail(logInReq.email()).orElseThrow(
			() -> new BadCredentialsException("너 안돼!")
		);
		
Member member = memberRepository.findByEmail(logInReq.email())
                                        .orElseThrow(() -> new BadCredentialsException("너 안돼!"));

Comment on lines 37 to 38

}
Copy link
Member

Choose a reason for hiding this comment

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

R: 예외 시, 다음 필터안가게 리턴!

String email = Jwts.parserBuilder()
.setSigningKey(secretKey)
.build().parseClaimsJws(token).getBody().getSubject();

Copy link
Member

Choose a reason for hiding this comment

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

R: 의미없는 줄바꿈!~

@Override
public UserDetails loadUserByUsername(String email) throws UsernameNotFoundException {
Member member = memberRepository.findByEmail(email).orElseThrow(
() -> new UsernameNotFoundException("Invalid Email")
Copy link
Member

Choose a reason for hiding this comment

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

위에서 말했어용!

Copy link
Member

Choose a reason for hiding this comment

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

체이닝~

Copy link
Collaborator

@Shin-Jae-Yoon Shin-Jae-Yoon left a comment

Choose a reason for hiding this comment

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

다른분들이 이미 열시미 남겨주시기도 했고 컨벤션 관련된거 말고는 크게 수정할 건 없을 것 같아요
고생하셨씀다 갠적인 궁금증은 물어보러 가겠슴두

public String login(LogInReq logInReq) {
Member member = memberRepository.findByEmail(logInReq.email()).orElseThrow(
() -> new BadCredentialsException("너 안돼!")
);
Copy link
Collaborator

Choose a reason for hiding this comment

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

R: 요런식으로 많이 썼던 것 같아요 !

	Member member = memberRepository.findByEmail(logInReq.email())
		.orElseThrow(() -> new BadCredentialsException("너 안돼!"));

public UserDetails loadUserByUsername(String email) throws UsernameNotFoundException {
Member member = memberRepository.findByEmail(email).orElseThrow(
() -> new UsernameNotFoundException("Invalid Email")
);
Copy link
Collaborator

Choose a reason for hiding this comment

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

R: 요기도 마찬가지 !!

@ymkim97 ymkim97 merged commit 6b25818 into develop Sep 6, 2023
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants