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

🔀 :: google oauth2 로그인 #6

Merged
merged 29 commits into from
Oct 12, 2024
Merged

Conversation

ta2ye0n
Copy link
Contributor

@ta2ye0n ta2ye0n commented Oct 8, 2024

💡 배경 및 개요

google oauth 사용해서 로그인 기능 구현했습니다

Resolves: #5

📃 작업내용

  • google oauth2
  • security 설정
  • 예외처리
  • jwt

🙋‍♂️ 리뷰노트

Authorization Code Grant 방식을 사용해서 oauth 구현했습니다
jwt, security 설정을 같이해서 코드의 양이 많아서 천천히 확인 하셔도 괜찮을것 같아요

✅ PR 체크리스트

템플릿 체크리스트 말고도 추가적으로 필요한 체크리스트는 추가해주세요!

  • 이 작업으로 인해 변경이 필요한 문서가 변경되었나요? (e.g. .env, 노션, README)
  • 이 작업을 하고나서 공유해야할 팀원들에게 공유되었나요? (e.g. "API 개발 완료됐어요", "환경값 추가되었어요")
  • 작업한 코드가 정상적으로 동작하나요?
  • Merge 대상 브랜치가 올바른가요?
  • PR과 관련 없는 작업이 있지는 않나요?
  • 이 작업으로 인해 발생한 변경 사항이 Resource 서버에도 반영되었나요?

🎸 기타

@ta2ye0n ta2ye0n added the ✨ Feature 신규 기능 label Oct 8, 2024
@ta2ye0n ta2ye0n self-assigned this Oct 8, 2024
@ta2ye0n ta2ye0n requested a review from Umjiseung as a code owner October 8, 2024 04:36
@ta2ye0n ta2ye0n linked an issue Oct 8, 2024 that may be closed by this pull request
Comment on lines 27 to 28
@Value("${jwt.secret}")
private String secretKey;
Copy link
Collaborator

Choose a reason for hiding this comment

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

직접 값을 받는 것 보단 Properties 클래스를 만들어서 따로 관리하는 게 어떨까요?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

396b059

말하신게 이렇게 만드는게 맞나요?

Copy link
Collaborator

Choose a reason for hiding this comment

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

저는 @Value말고 @ConfigurationProperties써서 accessToken과 refreshToken을 받는 것을 말하는 거 였는데 죄송합니다
제가 너무 추상적으로 말한 것 같네요

Copy link
Contributor Author

Choose a reason for hiding this comment

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

438abb9

수정했습니다

Comment on lines 36 to 37
secret: ${JWT_SECRET}
Copy link
Collaborator

Choose a reason for hiding this comment

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

JwtTokenProvider 로직을 보니 AccessToken과 RefreshToken를 서명할 때 동일한 key를 사용하는 것으로 보여지는데 보안쪽 측면에서는 따로 Secret 값을 분리하는 게 좋을 것 같아요

Copy link
Contributor Author

Choose a reason for hiding this comment

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

31346bd

수정했어요

Comment on lines 34 to 35
private static Key key;
private final AuthDetailsService authDetailsService;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
private static Key key;
private final AuthDetailsService authDetailsService;
private static Key key;
private final AuthDetailsService authDetailsService;

Comment on lines 17 to 19
@PostMapping
public ResponseEntity<TokenInfoResponseDto> test(@RequestBody SignInRequestDto signInRequestDto) {
TokenInfoResponseDto res = signInService.execute(signInRequestDto);
Copy link
Collaborator

Choose a reason for hiding this comment

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

테스트하시고 나서 메서드 이름을 안 바꾸신 것 같아요

Copy link
Contributor Author

Choose a reason for hiding this comment

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

66605e0

수정했어요

Comment on lines 6 to 8
public class SignInRequestDto {
private String code;
}
Copy link

Choose a reason for hiding this comment

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

DTO를 정의할 때, 일반적으로 불변 필드만 선언할 수 있으며, 생성자, Getter, equals(), toString()을 자동으로 생성해 주는 record를 사용하는 것이 어떨까요? 이런 특징들이 DTO의 특성과도 잘 맞아 좋은 시너지를 낼 것 같다고 생각합니다!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

5094a59

수정했습니다!

Comment on lines 10 to 15
public class TokenInfoResponseDto {
private String accessToken;
private String refreshToken;
private LocalDateTime accessTokenExpiresIn;
private LocalDateTime refreshTokenExpiresIn;
}
Copy link

Choose a reason for hiding this comment

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

Dto라고 했으니 Response에도 똑같이 적용해 주시면 될 것 같아요!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

e06e043

response 부분은 까먹었던것 같아요 수정했습니다

Comment on lines 12 to 25
public class ErrorResponse {
private HttpStatus status;
private String message;

public static ResponseEntity<ErrorResponse> toResponseEntity(ErrorCode e){
return ResponseEntity
.status(e.getHttpStatus())
.body(ErrorResponse.builder()
.status(e.getHttpStatus())
.message(e.getMessage())
.build()
);
}
}
Copy link

Choose a reason for hiding this comment

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

여기도요

Copy link
Contributor Author

Choose a reason for hiding this comment

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

292e005

수정했습니다

Copy link

@KimTaeO KimTaeO left a comment

Choose a reason for hiding this comment

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

뭔가 한번에 리뷰를 안남겨서 많이 불편하실 텐데 양해 부탁드립니다


HttpEntity<?> request = new HttpEntity<>(body, httpHeaders);

ResponseEntity<Map> response = restTemplate.exchange(config.getUserInfoUri(), HttpMethod.GET, request, Map.class);
Copy link

Choose a reason for hiding this comment

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

RestTemplate에서 반환받은 값의 타입을 지정할 때 제네릭을 사용하려면 ParameterizedTypeReference<반환받을 값 타입>() {}과 같이 사용해서 정확한 타입을 지정할 수 있어요
이 예시에서는 Map.class 대신에 new ParameterizedTypeReference<Map<String, String>() {}를 사용하면 될 것 같네요

Copy link
Contributor Author

Choose a reason for hiding this comment

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

3943c76

수정했어요


HttpEntity<?> request = new HttpEntity<>(body, httpHeaders);

ResponseEntity<Map> response = restTemplate.postForEntity(config.getTokenUri(), request, Map.class);
Copy link

Choose a reason for hiding this comment

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

여기도 똑같이 적용해주시면 될 것 같아요

Copy link

@KimTaeO KimTaeO left a comment

Choose a reason for hiding this comment

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

늦어서 죄송합니다. 수고하셨습니다!!

@ta2ye0n ta2ye0n merged commit e0457b8 into develop Oct 12, 2024
1 check passed
@ta2ye0n ta2ye0n deleted the feature/5-google-oauth2-login branch October 12, 2024 14:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✨ Feature 신규 기능
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Google Oauth2 로그인
3 participants