-
Notifications
You must be signed in to change notification settings - Fork 1
feat: #172 Apple 로그인 구현 #227
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
base: develop
Are you sure you want to change the base?
Changes from 17 commits
93ada66
151222a
c09842e
de038fe
1f8c967
bb57b7d
254433f
38cf547
0ee4797
43b57c2
2882fbe
d29fea2
1703ce7
01aebac
4b4937b
ea99690
c9133a6
848ecc9
5886099
f903972
11dd186
fa36216
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,8 @@ | ||
| package org.appjam.smashing.domain.auth.dto.command | ||
|
|
||
| import org.appjam.smashing.domain.auth.enums.ProviderType | ||
|
|
||
| data class SignInRequestCommand( | ||
| val accessToken: String, | ||
| val idToken: String, | ||
| val provider: ProviderType, | ||
| ) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| package org.appjam.smashing.domain.auth.dto.response | ||
|
|
||
| import org.appjam.smashing.domain.auth.enums.ProviderType | ||
|
|
||
| data class SocialType( | ||
| val provider: ProviderType, | ||
| val socialId: String, | ||
| ) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| package org.appjam.smashing.domain.auth.enums | ||
|
|
||
| enum class ProviderType { | ||
| KAKAO, APPLE | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -42,12 +42,12 @@ class AuthService( | |
| fun signIn( | ||
| requestCommand: SignInRequestCommand, | ||
| ): SignInResponse { | ||
| val kakaoId = socialAuthServiceManager.getKakaoId(requestCommand.accessToken) | ||
| val (provider, socialId) = socialAuthServiceManager.getSocialId(requestCommand) | ||
|
|
||
| val user = userRepository.findByKakaoId(kakaoId) | ||
| ?: return SignInResponse.from( | ||
| kakaoId = kakaoId, | ||
| ) | ||
| val user = userRepository.findBySocialIdAndProvider( | ||
| socialId = socialId, | ||
| provider = provider, | ||
| ) ?: return SignInResponse.from(socialId = socialId) | ||
|
|
||
| val userId = user.id ?: throw CustomException(ErrorCode.USER_NOT_FOUND) | ||
|
|
||
|
|
@@ -56,7 +56,7 @@ class AuthService( | |
| return SignInResponse.from( | ||
| accessToken = token.accessToken.token, | ||
| refreshToken = token.refreshToken.token, | ||
| kakaoId = kakaoId, | ||
| socialId = socialId, | ||
| userId = userId, | ||
| nickname = user.nickname, | ||
| ) | ||
|
|
@@ -81,7 +81,8 @@ class AuthService( | |
|
|
||
| val user = userRepository.save( | ||
| User.create( | ||
| kakaoId = requestCommand.kakaoId, | ||
| socialId = requestCommand.socialId, | ||
| provider = requestCommand.provider, | ||
| nickname = requestCommand.nickname, | ||
| gender = requestCommand.gender, | ||
| openchatUrl = requestCommand.openChatUrl.trim(), | ||
|
|
@@ -173,8 +174,8 @@ class AuthService( | |
| } | ||
|
|
||
| private fun validateUser(requestCommand: SignUpRequestCommand) { | ||
| if (userRepository.existsByKakaoId(requestCommand.kakaoId)) { | ||
| throw CustomException(ErrorCode.DUPLICATE_KAKAO_ID) | ||
| if (userRepository.existsBySocialId(requestCommand.socialId)) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 로그인 조회는 (socialId, provider) 조합으로 하고 있는데, 가입 시 중복 체크는 existsBySocialId()만 사용하고 있어서 기준이 조금 달라질 수 있을 것같아요.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 좋은 의견 감사합니다 :) |
||
| throw CustomException(ErrorCode.DUPLICATE_SOCIAL_ID) | ||
| } | ||
|
Comment on lines
176
to
179
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 중복 체크는
예시 수정- if (userRepository.existsBySocialId(requestCommand.socialId)) {
+ if (userRepository.existsBySocialIdAndProvider(
+ socialId = requestCommand.socialId,
+ provider = requestCommand.provider,
+ )) {
throw CustomException(ErrorCode.DUPLICATE_SOCIAL_ID)
}🤖 Prompt for AI Agents |
||
|
|
||
| if (userRepository.existsByNickname(requestCommand.nickname)) { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,68 @@ | ||
| package org.appjam.smashing.domain.auth.social | ||
|
|
||
| import com.fasterxml.jackson.databind.JsonNode | ||
| import com.fasterxml.jackson.databind.ObjectMapper | ||
| import io.jsonwebtoken.Jwts | ||
| import org.appjam.smashing.global.exception.CustomException | ||
| import org.appjam.smashing.global.exception.ErrorCode | ||
| import org.springframework.stereotype.Component | ||
| import org.springframework.web.client.RestTemplate | ||
| import java.math.BigInteger | ||
| import java.security.KeyFactory | ||
| import java.security.PublicKey | ||
| import java.security.spec.RSAPublicKeySpec | ||
| import java.util.* | ||
|
|
||
| @Component | ||
| class OidcTokenValidator { | ||
| fun extractSocialId( | ||
| idToken: String, | ||
| jwksUri: String, | ||
| iss: String, | ||
| ): String = try { | ||
| val publicKey = getPublicKey( | ||
| idToken = idToken, | ||
| jwksUri = jwksUri, | ||
| ) | ||
|
|
||
| val claims = Jwts.parserBuilder() | ||
| .setSigningKey(publicKey) | ||
| .build() | ||
| .parseClaimsJws(idToken) | ||
| .body | ||
|
|
||
| if (claims.issuer != iss) throw CustomException(ErrorCode.INVALID_ISS) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OIDC 공통화한 방향 너무너무 좋습니다! 다만 현재 iss만 검증하고 aud 검증이 빠져 있는데, ID Token은 우리 client를 대상으로 발급된 토큰인지도 함께 확인해야 해서 provider별 clientId 기준으로 aud까지 검증해주면 더 안전할 것 같아요. 또 지금 구조상 Apple/Kakao가 각각 따로 처리하기보다, provider별 설정값만 넘기고 OidcTokenValidator에서 공통 검증하도록 두는 쪽이 더 자연스러울 수 있을 것같은데, 혹시 유빈님은 어떠실까요?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 좋습니다! |
||
|
|
||
| claims.subject | ||
|
coderabbitai[bot] marked this conversation as resolved.
Outdated
|
||
| } catch (e: Exception) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 여기서 내부에서 던진 CustomException도 마지막 catch (e: Exception)에 같이 잡히면서 INVALID_ID_TOKEN으로 덮일 수 있을 것같아요. 의도적인 부분이 아니었다면 CustomException은 그대로 던지고, try catch문을 지양하고 예상하지 못한 예외만 공통 에러로 변환해도 좋을 것 같습니다! |
||
| throw CustomException(ErrorCode.INVALID_ID_TOKEN) | ||
| } | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
|
|
||
| private fun getPublicKey( | ||
| idToken: String, | ||
| jwksUri: String, | ||
| ): PublicKey { | ||
| val header = String(Base64.getUrlDecoder().decode(idToken.split(".")[0])) | ||
| val kid = ObjectMapper().readTree(header).get(KID).asText() | ||
|
|
||
| val jwks = RestTemplate().getForObject(jwksUri, JsonNode::class.java) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 여기 JWKS 조회가 인증 요청마다 외부 호출로 이어질 수 있어서 운영 측면에서는 조금 걱정되는 부분이 있을 것같아요. RestTemplate()를 직접 생성하기보다는 timeout이 설정된 클라이언트를 주입받고, 가능하면 provider 단위 캐싱도 같이 두면 더 안정적일 수 있을 것같다는 생각이 들어요!
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 오호 생각치 못한 방향이네요! |
||
| ?: throw CustomException(ErrorCode.INVALID_ID_TOKEN) | ||
|
coderabbitai[bot] marked this conversation as resolved.
Outdated
|
||
|
|
||
| val key = jwks[KEYS].find { it[KID].asText() == kid } | ||
| ?: throw CustomException(ErrorCode.INVALID_ID_TOKEN) | ||
|
|
||
| val n = BigInteger(1, Base64.getUrlDecoder().decode(key[N].asText())) | ||
| val e = BigInteger(1, Base64.getUrlDecoder().decode(key[E].asText())) | ||
|
|
||
| return KeyFactory.getInstance(RSA) | ||
| .generatePublic(RSAPublicKeySpec(n, e)) | ||
| } | ||
|
|
||
| companion object { | ||
| private const val KID = "kid" | ||
| private const val KEYS = "keys" | ||
| private const val N = "n" | ||
| private const val E = "e" | ||
| private const val RSA = "RSA" | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,11 +1,31 @@ | ||
| package org.appjam.smashing.domain.auth.social | ||
|
|
||
| import org.appjam.smashing.domain.auth.social.kakao.KakaoAuthTokenValidator | ||
| import org.appjam.smashing.domain.auth.dto.command.SignInRequestCommand | ||
| import org.appjam.smashing.domain.auth.dto.response.SocialType | ||
| import org.appjam.smashing.domain.auth.enums.ProviderType.APPLE | ||
| import org.appjam.smashing.domain.auth.enums.ProviderType.KAKAO | ||
| import org.appjam.smashing.domain.auth.social.apple.AppleOidcValidator | ||
| import org.appjam.smashing.domain.auth.social.kakao.KakaoOidcValidator | ||
| import org.springframework.stereotype.Component | ||
|
|
||
| @Component | ||
| class SocialAuthServiceManager( | ||
| private val kakaoAuthTokenValidator: KakaoAuthTokenValidator, | ||
| private val kakaoOidcValidator: KakaoOidcValidator, | ||
| private val appleOidcValidator: AppleOidcValidator, | ||
| ) { | ||
| fun getKakaoId(authAccessToken: String): String = kakaoAuthTokenValidator.extractKakaoId(authAccessToken) | ||
| fun getSocialId(command: SignInRequestCommand): SocialType { | ||
| val idToken = command.idToken | ||
|
|
||
| return when (command.provider) { | ||
| KAKAO -> SocialType( | ||
| provider = KAKAO, | ||
| socialId = kakaoOidcValidator.extractKakaoId(idToken), | ||
| ) | ||
|
|
||
| APPLE -> SocialType( | ||
| provider = APPLE, | ||
| socialId = appleOidcValidator.extractAppleId(idToken), | ||
| ) | ||
| } | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| package org.appjam.smashing.domain.auth.social.apple | ||
|
|
||
| import org.appjam.smashing.domain.auth.social.OidcTokenValidator | ||
| import org.springframework.stereotype.Component | ||
|
|
||
| @Component | ||
| class AppleOidcValidator( | ||
| private val oidcTokenValidator: OidcTokenValidator, | ||
| ) { | ||
| fun extractAppleId(idToken: String): String = | ||
| oidcTokenValidator.extractSocialId( | ||
| idToken = idToken, | ||
| jwksUri = JWKS_URI, | ||
| iss = ISS, | ||
| ) | ||
|
|
||
| companion object { | ||
| private const val JWKS_URI = "https://appleid.apple.com/auth/keys" | ||
| private const val ISS = "https://appleid.apple.com" | ||
| } | ||
| } |
This file was deleted.
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| package org.appjam.smashing.domain.auth.social.kakao | ||
|
|
||
| import org.appjam.smashing.domain.auth.social.OidcTokenValidator | ||
| import org.springframework.stereotype.Component | ||
|
|
||
| @Component | ||
| class KakaoOidcValidator( | ||
| private val oidcTokenValidator: OidcTokenValidator, | ||
| ) { | ||
| fun extractKakaoId(idToken: String): String = | ||
| oidcTokenValidator.extractSocialId( | ||
| idToken = idToken, | ||
| jwksUri = JWKS_URI, | ||
| iss = ISS, | ||
| ) | ||
|
|
||
| companion object { | ||
| private const val JWKS_URI = "https://kauth.kakao.com/.well-known/jwks.json" | ||
| private const val ISS = "https://kauth.kakao.com" | ||
| } | ||
| } |
This file was deleted.
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.
provider는 현재 enum validation만 하고 toCommand()에서 바로 provider!!로 사용하고 있어서, null/blank가 validation에서 걸러지지 않으면 500으로 떨어질 수 있을 것 같아요.
@notblank까지 같이 두어서 요청 검증 단계에서 400으로 막아주면 더 안전할 수 있을 것같습니다!
Uh oh!
There was an error while loading. Please reload this page.
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.
예리하십니다.. 추가했습니다! f903972