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

[FEAT] 애플 로그인 api #128

Merged
merged 5 commits into from
Jan 17, 2024
Merged

[FEAT] 애플 로그인 api #128

merged 5 commits into from
Jan 17, 2024

Conversation

Chan531
Copy link
Contributor

@Chan531 Chan531 commented Jan 16, 2024

✨ Related Issue

📝 기능 구현 명세

image

🐥 추가적인 언급 사항

제 나름대로 appleServiceImpl의 코드를 리팩토링해봤는데 한 번 봐주시면 감사하겠습니다.

@Chan531 Chan531 added chan 찬 작업 feat 기능 개발 labels Jan 16, 2024
@Chan531 Chan531 self-assigned this Jan 16, 2024
Copy link
Contributor

@csb9427 csb9427 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 +31 to +34
private String kakaoUri;

@Value("${jwt.APPLE_URL}")
private String appleUri;
Copy link
Contributor

Choose a reason for hiding this comment

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

이름도 두 개 다 분리해서 바뀐 거 좋아요~

Copy link
Member

@thguss thguss left a comment

Choose a reason for hiding this comment

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

수고하셨습니다~!! 메서드도 깔끔하게 분리되어 좋네요 :)
예외만 내부에서 따로 처리 부탁드리고, 재언급해주세요~

@@ -21,7 +23,7 @@ public class AuthController {
private final AuthService authService;

@PostMapping
public ResponseEntity<Response> signIn(@RequestHeader("Authorization") String socialAccessToken, @RequestBody SignInRequest request) {
public ResponseEntity<Response> signIn(@RequestHeader("Authorization") String socialAccessToken, @RequestBody SignInRequest request) throws NoSuchAlgorithmException, InvalidKeySpecException {
Copy link
Member

Choose a reason for hiding this comment

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

throws 예외를 Controller까지 들고오는 방향보다, 애플 서비스 내에서 try-catch문으로 잡아서 AuthException을 발생시키는 방법은 어떤가요?? 외부 API에서 발생하는 예외이다보니 서비스 내에서 잡아주면 좋을 것 같아요!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

좋은 거 같습니다! 저도 컨트롤러까지 들고 오는게 좀 그랬는데 훨 나은 거 같아요!

Comment on lines 133 to 147
private PublicKey getPublicKey(JsonObject object) throws NoSuchAlgorithmException, InvalidKeySpecException {
val modulus = object.get(MODULUS).toString();
val exponent = object.get(EXPONENT).toString();

val modulusBytes = Base64.getUrlDecoder().decode(modulus.substring(QUOTES, modulus.length() - QUOTES));
val exponentBytes = Base64.getUrlDecoder().decode(exponent.substring(QUOTES, exponent.length() - QUOTES));

val modulusValue = new BigInteger(POSITIVE_NUMBER, modulusBytes);
val exponentValue = new BigInteger(POSITIVE_NUMBER, exponentBytes);

val publicKeySpec = new RSAPublicKeySpec(modulusValue, exponentValue);
val keyFactory = KeyFactory.getInstance(RSA);

return keyFactory.generatePublic(publicKeySpec);
}
Copy link
Member

Choose a reason for hiding this comment

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

여기서 NoSuchAlgorithmException, InvalidKeySpecException 예외를 try-catch문으로 잡아주면 좋을 것 같아요 :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

좋아요!

Copy link
Member

@thguss thguss 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 14 to 16
/* 400 BAD_REQUEST : 잘못된 요청 */
INVALID_TOKEN(BAD_REQUEST, "유효하지 않은 토큰입니다."),
INVALID_KEY(BAD_REQUEST, "유효하지 않은 키입니다."),
Copy link
Member

@thguss thguss Jan 17, 2024

Choose a reason for hiding this comment

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

헛 다시 생각해보니 2개 에러의 상태 코드를 401 Unauthorized(BAD_REQUEST에서 변경)로 두어도 좋을 것 같아요!
병합 전에 한 번 고민해보고 괜찮으면 반영 해주세요~

Copy link
Contributor Author

Choose a reason for hiding this comment

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

INVALID_KEY는 클라에서 제공한 identity token의 인증정보가 잘못되어 발생한 인증문제로 401이 맞는 거 같습니다!

근데 개인적으로 INVALID_TOKEN은 클라에서 유효하지 않은 토큰을 던져줘서 요청 자체가 유효하지 않음을 나타내야 하기 때문에 400이 맞아보이는데 어떻게 생각하시나요?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

다시 생각해보니 유효하지 않은 토큰도 인증문제라 401로 해도 괜찮을 거 같습니다! 401로 수정할게요

@Chan531 Chan531 merged commit 13d2980 into develop Jan 17, 2024
1 check passed
@Chan531 Chan531 deleted the feature/#118-apple-login-api branch January 17, 2024 07:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chan 찬 작업 feat 기능 개발
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEAT] 애플 로그인 api
3 participants