-
Notifications
You must be signed in to change notification settings - Fork 7
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
로그인 관련 코드 리팩토링 및 사용자 전환 플로우 수정에 따른 로직 추가 #691
Conversation
updateLoginDetail 로직은 여전히 필요할 것 같아 그대로 두었습니다
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.
고생하셨습니다 안나~~!
코멘트 몇 개 있는데 사소해서 approve 할게요~~
return ResponseEntity.ok().body(new RestResponse<>(response)); | ||
@PostMapping("/apple") | ||
public ResponseEntity<Void> loginAppleOauth( | ||
@RequestParam("id_token") String id_token, |
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.
@RequestParam
에 따로 지정해줘서 변수명은 카멜케이스 사용해도 될 것 같아요
} | ||
String accessToken = appleAuthService.login(id_token); | ||
HttpHeaders httpHeaders = new HttpHeaders(); | ||
httpHeaders.add("Location", "https://dev.mouda.site/oauth/apple?token=" + accessToken); |
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.
항상 dev 로 보내나용?
private final MemberWriter memberWriter; | ||
|
||
public Member join(String name, OauthType oauthType, String identifier) { | ||
if (OauthType.KAKAO.equals(oauthType)) { |
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.
사소하지만 다른 부분에서는 enum 비교 시 ==
를 사용하고 있는데, 통일하는 건 어떤가요?
@@ -19,36 +19,19 @@ public interface AuthSwagger { | |||
@ApiResponses({ | |||
@ApiResponse(responseCode = "200", description = "로그인 성공!"), | |||
}) | |||
ResponseEntity<RestResponse<LoginResponse>> loginKakaoOauth(@RequestBody OauthRequest oauthRequest); | |||
ResponseEntity<Void> convert( |
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.
바뀐 기능에 따라 swagger description 도 변경해주시면 좋을 것 같아여!~
개인적으로는 예외를 던지는 게 낫지 않나 생각해요. 테스트하느라 prod에서 로그인을 못하는 건 슬프지만(ㅠㅠ) 그건 모우다팀 + 우테코 소수 인원으로 한정될 것이고 데이터를 직접 조작할 수 있는 범위이지 않을까요? 실제 저희 서비스에서는 실명이 중요한 요소인만큼 대충 저장하기엔 위험요소가 있지 않나 싶네요! |
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.
작업을 반대로 돌리시느라 고생하셨을 것 같네요..ㅠ
질문 남겨주신것은 만나서 한번 이야기 해봅시다!
@Service | ||
@RequiredArgsConstructor | ||
public class TestAuthService { | ||
|
||
private final MemberFinder memberFinder; | ||
private final MemberWriter memberWriter; | ||
private final AccessTokenProvider accessTokenProvider; | ||
|
||
public LoginResponse basicLoginAnna() { | ||
Member member = memberFinder.findByIdentifier("identifier"); | ||
return new LoginResponse(accessTokenProvider.provide(member)); | ||
} | ||
|
||
public LoginResponse basicLoginHogee() { | ||
Member member = Member.builder() | ||
.name("조호연") | ||
.loginDetail(new LoginDetail(OauthType.GOOGLE, UUID.randomUUID().toString())) | ||
.build(); | ||
memberWriter.append(member); | ||
return new LoginResponse(accessTokenProvider.provide(member)); | ||
} | ||
} |
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.
스프링의 profile을 잘 설정하면 prod 용 배포에서 이 클래스를 뺄 수 있겠네요!
APPLE_USER_BAD_REQUEST("잘못된 애플 로그인 요청입니다."); | ||
KAKAO_CANNOT_JOIN("기존 카카오 로그인 이력이 있는 사용자만 이용할 수 있는 서비스입니다. 새로운 회원은 다른 로그인 서비스를 이용해주세요."), | ||
APPLE_USER_BAD_REQUEST("사용자의 이름을 가져오는 과정에서 오류가 발생하였습니다."), | ||
CANNOT_FIND_APPLE_MEMBER("애플 로그인 이력이 있지만 회원 정보를 조회할 수 없습니다."); |
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.
이게 그 이슈인가보네요 ㅋ큐ㅠ
말씀해주신 건은 월요일에 만나서 이야기 나눠보시죠!
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.
고생했습니다 안나~
애플 user 없을 때 현재는 예외를 던지는데 어떻게 해야할지 고민좀 해봅시바.
PR의 목적이 무엇인가요?
이슈 ID는 무엇인가요?
설명
구글, 애플로 로그인한 후 기존 카카오 인증 API로 데이터 이전 요청을 보냅니다.
서버에서는 카카오 통신 후 사용자의 identifier를 얻습니다.
그 다음 카카오 회원 데이터를 구글, 애플 회원 데이터로 갈아끼웁니다.
이 경우 같은 구글, 애플 회원이 두 행 존재하게 되므로 기존 구글, 애플 데이터는 삭제합니다.
이 과정을 통해 기존 카카오 회원을 구글, 애플 회원으로 이전하도록 합니다.
질문 혹은 공유 사항 (Optional)
<질문>
애플 로그인시
user
가 전달되지 않고 (=최초 로그인이 아닌 경우) 회원 DB에도 없는 경우에 예외를 던지는 게 맞을까요?지금 dev에서 한 번이라도 애플 로그인한 회원은 prod에서 애플 로그인 시 무조건 로그인이 실패하는데, 엣지 케이스를 어떻게 대응해야할 지 모르겠습니다. 이름이 없어도 대충 저장하고 따로 찾아가서 수정하는 게 좋을까요? 😅