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: IdTokenResolver가 Provider에 맞게 동작한다 #314

Merged
merged 4 commits into from
Sep 21, 2023
Merged

Conversation

0chil
Copy link
Collaborator

@0chil 0chil commented Sep 19, 2023

관련 이슈번호

작업 사항

IdTokenResolver 및 그 하위 객체들이 OAuth Provider에 맞게 동작한다.
따라서 단일 endpoint를 통해 소셜 로그인을 수행할 수 있다.

  • 기존에 테스트하지 못했던 부분 최대한 보완

기존 구조

CachedJwkProvider는 거의 파워와 콩하나가 잘 구현해주셔서 그 개념을 거의 그대로 이어받았습니다!
대신 예외 처리 등 조금 더 wrapping을 해보았어요.

그리고 IdTokenResolver와 CachedJwkProvider 사이는 Provider에 따라 유연하게 처리할 수 있도록 채워넣었습니다.

구조

빨간색MemberService에서 참조하는 Class, Interface입니다~!

image

기타 사항

파워랑 콩하나 말대로 com.auth0:java-jwt 라이브러리 좀 똥이네요 ㅋㅋ
코드 사용자 입장에서 서타일이 좀 중구난방.. 어떤건 별 의미없는 빌더 패턴.. 어떤건 정적 팩토리.. ㅋㅋ;
다음에는 jsonwebtoken:jjwt를 사용합시다~

@0chil 0chil added backend 백엔드 분야 feat 기능 개발 labels Sep 19, 2023
@0chil 0chil added this to the 스프린트 5 milestone Sep 19, 2023
@0chil 0chil self-assigned this Sep 19, 2023
Comment on lines 10 to +11
import com.digginroom.digginroom.oauth.IdTokenResolver;
import com.digginroom.digginroom.oauth.payload.IdTokenPayload;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

MemberService에서는 이 두가지 Class, Interface만 알고있으면 됩니다!

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 Author

@0chil 0chil left a comment

Choose a reason for hiding this comment

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

이건 오늘 강의듣고 떠오른게 있어서 좀만 더 개선해보고 리뷰 요청하겠습니다~

@0chil 0chil requested review from kong-hana01, Songusika and thdwoqor and removed request for kong-hana01 and Songusika September 20, 2023 06:50
Copy link
Collaborator

@kong-hana01 kong-hana01 left a comment

Choose a reason for hiding this comment

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

땡칠 코드 정말 잘 짜셨네요 ㅋㅋㅋㅋㅋ
땡칠의 솜씨를 잘 볼 수 있었습니다.

구조적으로도 아주 깔끔하고 오늘 배운 DI도 잘 적용되어있어서
앞으로 OIDC 추가할 때에도 서비스/컨트롤러의 수정 없이 도메인 쪽만 건들면 되겠다는 생각이 드네요 ㅋㅋㅋㅋㅋㅋ
추상화 너무 잘 하셨고 보면서 많이 배웠습니다.
어떻게 이렇게 짧은 시간동안 이렇게 잘 짜셨죠?
고생하셨습니다~~
저는 리뷰를 드리고 싶지만 제 수준에서는 너무 깔끔해보이기도 하고 뭐라 추가해드릴 말씀이 없어서 바로 어프로브하겠습니다~!

final만 반영해주세용 👍

Comment on lines 10 to +11
import com.digginroom.digginroom.oauth.IdTokenResolver;
import com.digginroom.digginroom.oauth.payload.IdTokenPayload;
Copy link
Collaborator

Choose a reason for hiding this comment

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

ㅋㅋㅋㅋㅋㅋ 개쩌네요

import lombok.RequiredArgsConstructor;
import org.springframework.stereotype.Component;

@Component
@RequiredArgsConstructor
public class IdTokenResolver {

private final CachedJwkProviders cachedJwkProviders;
private final ThirdPartyJwkProviders thirdPartyJwkProviders;
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 Author

Choose a reason for hiding this comment

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

우리 입장에선 캐시 여부는 별로 안 중요할 것이라고 생각했습니다~

return verifier.verify(idToken).getClaims();

verifier.verify(rawIdToken);
return issuer;
Copy link
Collaborator

Choose a reason for hiding this comment

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

verify를 리턴하던게

Algorithm algorithm = Algorithm.RSA256((RSAPublicKey) jwk.getPublicKey(), null);

JWTVerifier verifier = JWT.require(algorithm).build();
verifier.verify(rawIdToken);
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 +29 to +32
private final static Map<Provider, Function<Map<String, Claim>, IdTokenPayload>> PAYLOAD_CONSTRUCTORS = Map.of(
Provider.KAKAO, KakaoIdTokenPayload::new,
Provider.GOOGLE, GoogleIdTokenPayload::new
);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Function 오랜만에 봐서 기억이 하나도 안났는데 덕분에 공부 좀 했네요 ㅋㅋㅋㅋ

Comment on lines +41 to +47
void IdToken_Issuer_GOOGLE에_알맞는_Payload를_생성한다() {
IdTokenPayload payload = THIRD_PARTY_JWK_PROVIDERS.resolve(ID_TOKEN_SAMPLE_GOOGLE);

assertThat(payload.getProvider()).isEqualTo(Provider.GOOGLE);
assertThat(payload.getUsername()).isEqualTo("104106003661228920371");
assertThat(payload.getNickname()).isEqualTo("김진욱");
}
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 63 to 70
public IdTokenPayload resolve(String rawIdToken) {
DecodedJWT decoded = JWT.decode(rawIdToken);
Provider provider = Provider.of(decoded.getIssuer())
.orElseThrow(() -> new UnsupportedIdTokenException(decoded.getIssuer()));
return getConstructorFor(provider).apply(
decoded.getClaims()
);
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

대신 verify와 resolve를 외부에서 점검해야한다는 잠재적 위험성이 있을 수는 있겠지만,
서비스의 역할이 흐름 제어니까 그런 측면에서 볼 때는 괜찮아보이네요!
그리고 어차피 이를 사용하는 서비스 코드의 수정은 앞으로 크게 없을 것으로 보여서 괜찮을 것 같기도 하고요!


private final String issueUrl;

Provider(String issueUrl) {
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 Author

Choose a reason for hiding this comment

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

완료입니다~!

@@ -37,25 +36,12 @@ public ResponseEntity<MemberLoginResponse> login(
.body(member);
}

@PostMapping("/google")
@PostMapping("/oauth")
Copy link
Collaborator

Choose a reason for hiding this comment

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

oauth라고 통일하는것보다
@ PathVariable 을 통해서 kakao인지 google인지 입력받으면 provider에 iss url 작성이나 검증 과정도 줄어들거같아요

@RequiredArgsConstructor
public class ThirdPartyJwkProviders {

private final static Map<Provider, Function<Map<String, Claim>, IdTokenPayload>> PAYLOAD_CONSTRUCTORS = Map.of(
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

@0chil 0chil merged commit 8888dce into backend Sep 21, 2023
3 checks passed
0chil added a commit that referenced this pull request Sep 21, 2023
* feat: IdTokenResolver가 Provider에 맞게 동작한다

* test: 지원하지 않는 Issuer 예외 테스트 추가

* refactor: 유지보수가 용이하도록 구조 개선

* refactor: 누락된 final 추가
0chil added a commit that referenced this pull request Sep 21, 2023
* feat: id를 사용하는 entity는 BaseEntity를 상속하도록 수정 (#293)

* feat: id를 사용하는 entity는 BaseEntity를 상속하도록 수정

* refactor: flyway 사용 규칙에 따라 스크립트 수정

* refactor: 룸 업로드 페이지 리팩터링 (#307)

* refactor: 룸, 트랙, 미디어소스 역정규화 (#309)

* feat: 카카오 소셜 로그인 기능을 구현한다. (#301)

* refactor: 패키지 및 클래스 명 수정

* feat: 카카오 소셜로그인 idToken 검증 및 payload 추출 기능 구현

* feat: 카카오 소셜로그인 기능 구현

* refactor: jwkProvider를 static 변수로 가지도록 수정

* refactor: 구글 소셜로그인 검증 방식 통일

* refactor: 구글 소셜로그인 검증 방식 추상화

* feat: 멤버 객체 내 nickname 필드 추가

* feat: 닉네임 객체 구현

* refactor: 클래스 네이밍 변경

* feat: JwkProvider를 찾지 못할 때 예외 구현 및 record -> class로 변경

* refactor: final 키워드 추가

* chore: 닉네임 추가 sql 버전 변경 5.3 -> 5.4

---------

Co-authored-by: thdwoqor <celebarte01@gmail.com>

* chore: 데이터 베이스 테이블 컬럼 수정 (#311)

* feat: IdTokenResolver가 Provider에 맞게 동작한다 (#314)

* feat: IdTokenResolver가 Provider에 맞게 동작한다

* test: 지원하지 않는 Issuer 예외 테스트 추가

* refactor: 유지보수가 용이하도록 구조 개선

* refactor: 누락된 final 추가

* feat: 게스트로 로그인한다 (#316)

* feat: 게스트로 로그인한다

* feat: 빈 패스워드는 어떤 문자열과도 일치하지 않는다

* refactor: Member 클래스 내 생성자를 정적 팩토리 메소드로 변경

* refactor: final 키워드 추가

* fix: 머지 중 누락

---------

Co-authored-by: Kong-hana01 <gksqlsl11@khu.ac.kr>

* chore: 중복된 ci 파일 제거 (#323)

---------

Co-authored-by: Jae-Baek Song <83541246+thdwoqor@users.noreply.github.com>
Co-authored-by: Songusika <74398096+Songusika@users.noreply.github.com>
Co-authored-by: 0chil <gorae02@gmail.com>
Co-authored-by: thdwoqor <celebarte01@gmail.com>
kong-hana01 added a commit that referenced this pull request Jan 24, 2024
* feat: id를 사용하는 entity는 BaseEntity를 상속하도록 수정 (#293)

* feat: id를 사용하는 entity는 BaseEntity를 상속하도록 수정

* refactor: flyway 사용 규칙에 따라 스크립트 수정

* refactor: 룸 업로드 페이지 리팩터링 (#307)

* refactor: 룸, 트랙, 미디어소스 역정규화 (#309)

* feat: 카카오 소셜 로그인 기능을 구현한다. (#301)

* refactor: 패키지 및 클래스 명 수정

* feat: 카카오 소셜로그인 idToken 검증 및 payload 추출 기능 구현

* feat: 카카오 소셜로그인 기능 구현

* refactor: jwkProvider를 static 변수로 가지도록 수정

* refactor: 구글 소셜로그인 검증 방식 통일

* refactor: 구글 소셜로그인 검증 방식 추상화

* feat: 멤버 객체 내 nickname 필드 추가

* feat: 닉네임 객체 구현

* refactor: 클래스 네이밍 변경

* feat: JwkProvider를 찾지 못할 때 예외 구현 및 record -> class로 변경

* refactor: final 키워드 추가

* chore: 닉네임 추가 sql 버전 변경 5.3 -> 5.4

---------

Co-authored-by: thdwoqor <celebarte01@gmail.com>

* chore: 데이터 베이스 테이블 컬럼 수정 (#311)

* feat: IdTokenResolver가 Provider에 맞게 동작한다 (#314)

* feat: IdTokenResolver가 Provider에 맞게 동작한다

* test: 지원하지 않는 Issuer 예외 테스트 추가

* refactor: 유지보수가 용이하도록 구조 개선

* refactor: 누락된 final 추가

* feat: 게스트로 로그인한다 (#316)

* feat: 게스트로 로그인한다

* feat: 빈 패스워드는 어떤 문자열과도 일치하지 않는다

* refactor: Member 클래스 내 생성자를 정적 팩토리 메소드로 변경

* refactor: final 키워드 추가

* fix: 머지 중 누락

---------

Co-authored-by: Kong-hana01 <gksqlsl11@khu.ac.kr>

* chore: 중복된 ci 파일 제거 (#323)

* fix: 피드백 검증을 하지 않는 문제 해결

* chore: 컨벤션 통일

* docs: ci dev db 파일 수정

* fix: db url 수정

* fix: runs-on 수정

* chore: ci 스크립트 수정 복원

* chore: DB CI 복구

---------

Co-authored-by: Jae-Baek Song <83541246+thdwoqor@users.noreply.github.com>
Co-authored-by: Songusika <74398096+Songusika@users.noreply.github.com>
Co-authored-by: 0chil <gorae02@gmail.com>
Co-authored-by: thdwoqor <celebarte01@gmail.com>
kong-hana01 added a commit that referenced this pull request Mar 4, 2024
* feat: id를 사용하는 entity는 BaseEntity를 상속하도록 수정 (#293)

* feat: id를 사용하는 entity는 BaseEntity를 상속하도록 수정

* refactor: flyway 사용 규칙에 따라 스크립트 수정

* refactor: 룸 업로드 페이지 리팩터링 (#307)

* refactor: 룸, 트랙, 미디어소스 역정규화 (#309)

* feat: 카카오 소셜 로그인 기능을 구현한다. (#301)

* refactor: 패키지 및 클래스 명 수정

* feat: 카카오 소셜로그인 idToken 검증 및 payload 추출 기능 구현

* feat: 카카오 소셜로그인 기능 구현

* refactor: jwkProvider를 static 변수로 가지도록 수정

* refactor: 구글 소셜로그인 검증 방식 통일

* refactor: 구글 소셜로그인 검증 방식 추상화

* feat: 멤버 객체 내 nickname 필드 추가

* feat: 닉네임 객체 구현

* refactor: 클래스 네이밍 변경

* feat: JwkProvider를 찾지 못할 때 예외 구현 및 record -> class로 변경

* refactor: final 키워드 추가

* chore: 닉네임 추가 sql 버전 변경 5.3 -> 5.4

---------

Co-authored-by: thdwoqor <celebarte01@gmail.com>

* chore: 데이터 베이스 테이블 컬럼 수정 (#311)

* feat: IdTokenResolver가 Provider에 맞게 동작한다 (#314)

* feat: IdTokenResolver가 Provider에 맞게 동작한다

* test: 지원하지 않는 Issuer 예외 테스트 추가

* refactor: 유지보수가 용이하도록 구조 개선

* refactor: 누락된 final 추가

* feat: 게스트로 로그인한다 (#316)

* feat: 게스트로 로그인한다

* feat: 빈 패스워드는 어떤 문자열과도 일치하지 않는다

* refactor: Member 클래스 내 생성자를 정적 팩토리 메소드로 변경

* refactor: final 키워드 추가

* fix: 머지 중 누락

---------

Co-authored-by: Kong-hana01 <gksqlsl11@khu.ac.kr>

* chore: 중복된 ci 파일 제거 (#323)

* fix: 피드백 검증을 하지 않는 문제 해결

* chore: 컨벤션 통일

* refactor: DatabaseCleaner 인터페이스 및 구현체 구현

* refactor: AuthDatabaseCleaner 구현

* refactor: 외부에서 값을 주입받아 빈을 생성하도록 변경

* chore: 패키지 구조 변경(util -> config)

* refactor: ControllerTest의 profile 설정

* feat: MockLogin 객체 구현

* feat: MockLogin 객체 구현

* refactor: default 환경을 구성하는 컨트롤러 테스트 격리

* feat: 로그인을 담당하는 mock 서버 구현

* refactor: 로그인 환경을 설정하는 ControllerTestConfig 객체 구현

* refactor: 멤버 정보를 재사용해서 테스트를 하도록 변경

* fix: FeedbackControllerTest에서 캐싱사용하도록 변경

* docs: 예외 로그 확인 ci 파일 추가

* refactor: TestInstance 단위를 class로 변경

* refactor: MockLoginServer의 빈 스코프를 프로토타입으로 변경

* docs: ci 파일 복구

* refactor: 테스트 내 환경을 @TestConfiguration으로 변경

* chore: 사용하지 않는 콘솔 출력 제거

---------

Co-authored-by: Jae-Baek Song <83541246+thdwoqor@users.noreply.github.com>
Co-authored-by: Songusika <74398096+Songusika@users.noreply.github.com>
Co-authored-by: 0chil <gorae02@gmail.com>
Co-authored-by: thdwoqor <celebarte01@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend 백엔드 분야 feat 기능 개발
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants