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

카카오 IdToken 검증 #250

Merged
merged 10 commits into from
Dec 11, 2023
Merged

카카오 IdToken 검증 #250

merged 10 commits into from
Dec 11, 2023

Conversation

msjang4
Copy link
Collaborator

@msjang4 msjang4 commented Dec 11, 2023

relatedTo #40

작업 내용

  • 카카오 Id 토큰 유효성 검사

전달 사항

@msjang4 msjang4 added backend feat 새로운 기능 추가 labels Dec 11, 2023
@msjang4 msjang4 added this to the Week 6 milestone Dec 11, 2023
@msjang4 msjang4 requested a review from 5tarry December 11, 2023 08:32
Copy link
Collaborator

@5tarry 5tarry left a comment

Choose a reason for hiding this comment

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

아까 잠깐 봤었을 때 공개키 만드는 부분이 어려웠는데 깔끔하게 잘하셨네요 🙌
이미 아시겠지만 공개키는 일정 기간 캐싱(Caching)하여 사용할 것을 권장하며, 지나치게 빈번한 요청 시 요청이 차단될 수 있으므로 유의 해야 하는 점은 언젠가 개선해봐도 좋을 것 같아요~

Comment on lines 51 to 54
const newUser = new this.UserModel({
...signupRequestDto,
profileImageExtension: profileImageExtension ?? null,
});
Copy link
Collaborator

Choose a reason for hiding this comment

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

signupRequestDto에 profileImageExtension가 포함되어 있는데 이렇게 변경하신 이유가 있나요?!

/**
   * 프로필 이미지 확장자
   * @example 'webp'
   */
  profileImageExtension?: string | null = null;

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

?!? 이런코드를 짠 기억이 없는데..

Copy link
Collaborator

Choose a reason for hiding this comment

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

ㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋ

Comment on lines 116 to 118
const jwks = await axios
.get(process.env.KAKAO_KEY_URL)
.then((response) => response.data.keys);
Copy link
Collaborator

Choose a reason for hiding this comment

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

큰 상관은 없지만 이렇게 하는 것도 괜찮을 것 같아요!

const {keys} = (
        await axios
        .get(process.env.KAKAO_KEY_URL)
        ).data;

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

구조분해할당을 바로 활용하는 방법이 있었군요
jwks라는 변수명이 더 맞는것 같아서 나중에 변수명을 그대로 쓰는 경우가 있다면 꼭 사용해볼게요!!
JWK = Json Web Key, 암호화 키를 표현하기 위한 다양한 정보를 담은 JSON 객체에 관한 표준

Copy link
Collaborator

Choose a reason for hiding this comment

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

네네 그리고 await랑 then 같이 사용하는 부분도 확인해주시면 좋을 것 같아요~

Comment on lines +122 to +130
const keyObject = createPublicKey({
key,
format: 'jwk',
});
const secret = keyObject.export({ type: 'pkcs1', format: 'pem' });
await this.jwtService.verifyAsync(idToken, {
algorithms: [key.alg],
secret,
});
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
Collaborator

Choose a reason for hiding this comment

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

pkcs1 은 어떤 타입인가요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

pkcs는 Public-Key Cryptography Standard로 공개키 암호화의 프로토콜이에요.
pkcs는 15가지가 있고 각 번호별로 사용되는 알고리즘이 있어요. 몇가지 예로 들면, pkcs1이 젤 유명한 RSA고 pkcs13은 ECC (타원곡선 난해를 이용한 비대칭키 알고리즘)입니다

5tarry
5tarry previously approved these changes Dec 11, 2023
@msjang4 msjang4 merged commit e547dab into develop Dec 11, 2023
@msjang4 msjang4 deleted the feat/verify_kakao_token branch December 11, 2023 10:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend feat 새로운 기능 추가
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants