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

[BG-133]: 구글 OAuth2 연동하기 (6.5h / 5h) #5

Merged
merged 8 commits into from
Jun 26, 2024

Conversation

Dltmd202
Copy link
Member

Why

소셜 로그인을 위한 구글 OAuth2 연동

How

스프링에서 제공하는 OAuth2 클라이언트를 사용했을 때 커스텀이 어려웠던 것 같아서 이를 사용하지 않고 직접 연동했다.

Result

image

Prize

외부 API 호출파트가 있기 때문에 테스트가 어렵고, 또 에러 처리 또한 확실히 하려고 노력했다.

@Service
class AuthService(
    val googleOAuthClient: GoogleOAuthClient,
    val googleApiClient: GoogleApiClient,
    val authConfig: AuthConfig,
) {
    fun googleLogin(authorizationCode: String): String? {
        val accessTokenDto: GoogleOAuth2AccessTokenDto =
            googleOAuthClient.getGoogleOAuth2(
                authorizationCode,
                authConfig.redirectUri,
                authConfig.grantType,
                authConfig.clientSecret,
                authConfig.clientId,
            ) ?: throw IllegalArgumentException("Failed to get access token")

        val userInfo: GoogleUserInfoDto =
            googleApiClient.getUserInfo(accessTokenDto.getBearerToken())
                ?: throw IllegalArgumentException("Failed to get user information")

        return userInfo.email
    }
}

기존에 로직에서 별도의 처리를 거치지 않았으면 요청 결과가 실패일 경우 exception을 터뜨린다. 때문에 try-catch 처리를 해주어야 하고, 또 그 결과를 일괄적으로 처리하기 힘들다.

이를 해결하기 위해 AOP를 사용해 결과를 일괄적으로 value 또는 null이 반환되도록 강제하였다.(exception이 터지지 않도록)

Link

BG-132

Copy link
Member

@GGHDMS GGHDMS left a comment

Choose a reason for hiding this comment

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

사용자를 회원가입 시키고
가입되어 있는지 안되어 있는지 확인하는 것은 추후에 연동 할거야?

googleApiClient.getUserInfo(accessTokenDto.getBearerToken())
?: throw IllegalArgumentException("Failed to get user information")

return userInfo.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 Author

@Dltmd202 Dltmd202 Jun 26, 2024

Choose a reason for hiding this comment

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

그래서 aop를 어떻게 짤지 고민을 좀 했는데 aop에서 exception을 변환해서 터뜨릴까 그냥 null을 반환할까

아직 우리 팀 예외전략같은게 안정해져서 일단 저렇게 처리해 놨엉!

Copy link
Member

Choose a reason for hiding this comment

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

👍

@Dltmd202
Copy link
Member Author

사용자를 회원가입 시키고

가입되어 있는지 안되어 있는지 확인하는 것은 추후에 연동 할거야?

아 그게 처음부터 oauth 연동이랑 인증이 다른 이슈로 구준되어 있더라구!

Copy link
Member

@GGHDMS GGHDMS left a comment

Choose a reason for hiding this comment

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

지라 티켓이 잘못 연동되어 있어

@Dltmd202 Dltmd202 changed the title [BG-132]: 구글 OAuth2 연동하기 (6.5h / 5h) [BG-133]: 구글 OAuth2 연동하기 (6.5h / 5h) Jun 26, 2024
Copy link
Member

@GGHDMS GGHDMS left a comment

Choose a reason for hiding this comment

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

👍

@Dltmd202 Dltmd202 merged commit ada0d24 into develop Jun 26, 2024
1 check passed
@Dltmd202 Dltmd202 deleted the BG-133-link-oauth branch June 26, 2024 05:19
@Dltmd202 Dltmd202 self-assigned this Jun 26, 2024
Comment on lines +8 to +30
@Value("\${oauth.google.client-id}")
lateinit var clientId: String

@Value("\${oauth.google.client-secret}")
lateinit var clientSecret: String

@Value("\${oauth.google.redirect-uri}")
lateinit var redirectUri: String

@Value("\${oauth.google.client-name}")
lateinit var clientName: String

@Value("\${oauth.google.base-url}")
lateinit var baseUrl: String

@Value("\${oauth.google.scope}")
lateinit var scope: String

@Value("\${oauth.google.oauth-url}")
lateinit var oauthUrl: String

@Value("\${oauth.google.api-url}")
lateinit var apiUrl: String
Copy link
Member

Choose a reason for hiding this comment

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

이거 불편해 👎
@ConfigurationProperties로 묶어줘

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.

3 participants