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

[27] keycloak + oauth 2.0 적용 및 회원 가입 수정 #31

Merged
merged 54 commits into from
Sep 27, 2024

Conversation

ohsuha
Copy link
Collaborator

@ohsuha ohsuha commented Sep 15, 2024

쇼핑몰에는 SSO 로그인 기능이 필요할것같아서 oauth2.0 으로 선택했습니다.
인가서버로는 키클락을 선택했고, 유저 등록 기능을 수정했습니다.
이 과정에서 user 의 정보 중 패스워드는 키클락에, 이름과 이메일 키클락 서버에 저장된 ID를 DB에 저장하려고하는데 맞는 방향인지 궁금합니다.

추후 키클락 쪽이랑 중복으로 저장되어있는 느낌이 들어서, user table 을 삭제하고 장바구니에서 FK로 잡고 있는 user_id 를 모두 이메일이나 키클락 서버에 등록된 유저ID(uuid)로 변경하려 합니다.

추가적으로 고민이.. keycloak 서버에 저장을 하고 나면 이미 그 서버에 전송되어 저장된것인데 이후 user db에 어떤 다른 이유로 저장이 실패해도 키클락에 저장된 데이터는 롤백이 되지 않을텐데 이럴경우는 어떻게 해야하나요..?

ohsuha and others added 30 commits September 1, 2024 12:15
- address 관련 테이블 칼럼 수정
- 주 사용 주소는 계정당 하나씩 존재하도록
- 불필요한 줄바꿈 삭제
- 수정시 응답 void로 변경
- 수정, 삭제에 @Transction 추가
- 불필요한 줄바꿈 삭제
- 수정시 응답 void로 변경
- 수정, 삭제에 @Transction 추가
- 불필요한 줄바꿈 삭제
- 수정시 응답 void로 변경
- 수정, 삭제에 @Transction 추가
- 리스트에서는 페이지를 0으로 받도록 했는데 1로 받는게 나은가요?
- Create 할때 reponse 는 없애려고 합니다
- 페이지를 1로 받도록하고 내부에서 0으로 처리하도록 수정
- 불필요한 주석 삭제
flyaway 선택이유
1. 명시적 마이그레이션 관리로 언제 어떤 변경이 있었는지 추적 가능
2. 롤백 가능
3. 운영환경이라고 생각하고 개발하고 싶습니다
4. 처음 보는 거라 써보고 싶어서...
- 어떻게 업데이트를 할것인지를 매개변수 통해서 받아 메서드가 어떻게 동작하는지 명확히하고자함
- readOnly = true 시 변경 감지를 위한 추가 작업(더티 체크)을 생략, 읽기 작업 수행시 실수로 데이터 수정하는 것 방지, 롤백을 고려할 필요 없어 커넥션이 효율적으로 관리됨의 이점에 따라 정보를 읽어오는 서비스 로직에 적용했습니다.
flyaway 선택이유
1. 명시적 마이그레이션 관리로 언제 어떤 변경이 있었는지 추적 가능
2. 롤백 가능
3. 운영환경이라고 생각하고 개발하고 싶습니다
4. 처음 보는 거라 써보고 싶어서...
- 어떻게 업데이트를 할것인지를 매개변수 통해서 받아 메서드가 어떻게 동작하는지 명확히하고자함
- readOnly = true 시 변경 감지를 위한 추가 작업(더티 체크)을 생략, 읽기 작업 수행시 실수로 데이터 수정하는 것 방지, 롤백을 고려할 필요 없어 커넥션이 효율적으로 관리됨의 이점에 따라 정보를 읽어오는 서비스 로직에 적용했습니다.
- 리스트에서는 페이지를 0으로 받도록 했는데 1로 받는게 나은가요?
- Create 할때 reponse 는 없애려고 합니다
- 장바구니에 상품 추가
- repository 들을 패키지 추가하여 이동
- 장바구니 내용물삭제
- 장바구니 추가시 동일한 상품이 있다면 수량을 추가하는 로직으로 변경
- map으로 파라미터를 받은적은 처음인데 혹시 이렇게 받아도 되나요...?
- user db 수정
  - 패스워드는 키클락에서만 들고 있도록 수정
  - 키클락에 저장된 userID(uuid)를 DB에 저장하도록 수정
- auth service
  - keycloak 과 관련된 클라이언트를 얻어 작업을 수행하는 서비스
@ohsuha ohsuha linked an issue Sep 15, 2024 that may be closed by this pull request
@ohsuha ohsuha self-assigned this Sep 15, 2024
@f-lab-kai-c
Copy link

추가적으로 고민이.. keycloak 서버에 저장을 하고 나면 이미 그 서버에 전송되어 저장된것인데 이후 user db에 어떤 다른 이유로 저장이 실패해도 키클락에 저장된 데이터는 롤백이 되지 않을텐데 이럴경우는 어떻게 해야하나요..?

이럴 경우는 어떻게 처리하면 좋을지 관련 내용들을 찾아보면서 고민해보면 좋을 것 같습니다. DB에 등록할 때 항상 키클락도 같이 등록해야할지? 미리 등록되어 있으면 어떻게 처리할지를 고민해보면 좋을 것 같습니다.

- DB에 유저 등록 실패시 키클락에 등록된 유저를 삭제하는 함수를 실행합니다
@ohsuha
Copy link
Collaborator Author

ohsuha commented Sep 19, 2024

@f-lab-kai-c
분산 트랜잭션의 처리 방식을 찾아보던 중 "보상 트랜잭션"이라는 것을 적용하기에 알맞을 것같아서 api 쪽 DB에 저장 실패시 키클락에 등록된 유저를 삭제하는 부분을 추가했습니다.

Comment on lines 24 to 29
private final Keycloak keycloak;
private final UserService userService;
@Value("${keycloak.realm}")
private String REALM_NAME;
private final RealmResource realmResource = keycloak.realm(REALM_NAME);
private final UsersResource usersResource = realmResource.users();

Choose a reason for hiding this comment

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

AuthService에서 실제로 사용되는건 UsersResource 뿐인데, 의존성을 분리하는게 좋지 않을까요?

Comment on lines 25 to 30
try {
keyCloakId = authService.createUser(dto);
} catch (Exception e) {
log.error("Keycloak 사용자 생성 실패: {}", e.getMessage());
throw new CustomException(ErrorCode.CREATE_KEYCLOAK_USER_ERROR);
}

Choose a reason for hiding this comment

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

위 로직에서 Exception이 발생한 후 아래로 진행되면, userService#create가 정상적으로 실행되나요?

@f-lab-kai-c
Copy link

분산 트랜잭션의 처리 방식을 찾아보던 중 "보상 트랜잭션"이라는 것을 적용하기에 알맞을 것같아서 api 쪽 DB에 저장 실패시 키클락에 등록된 유저를 삭제하는 부분을 추가했습니다.

현재와 같이 적용했을 때, 알수 없는 장애로 인하여 키클락에 등록된 유저를 삭제하는 로직까지 도달하지 못하고 api 서버가 죽는 상황이 발생한다면 어떻게 될까요?

- call back api를 통해서 토큰 정보를 리턴하도록 수정
- open id 방식을 통해 카카오 계정 사용자의 회원 가입 및 로그인 구현
- 키클락 이벤트를 통해 새로 등록된 유저가 발생하면 DB에 회원 정보 저장
- 불필요한 facade, service 삭제
@ohsuha
Copy link
Collaborator Author

ohsuha commented Sep 21, 2024

코드가 크게 수정되었는데요 말씀주신대로 로그인 구현할때 외부 계정(카카오, 구글,..) 에서 이메일을 가져와 이메일 + 이름 정보로 저장하는 방향으로 변경하고있었습니다.

그렇게 카카오 계정으로 가입을 구현하던 도중 이렇게 하면 사용자에게 키클락 쪽에 등록할 비밀번호를 따로 받을 수 없다는 것을 알게되어 openID 방식으로 키클락에 사용자를 가입시켰습니다.

그리고 키클락을 띄울때 CustomEventListenerProvider, CustomEventListenerProviderFactory 를 별도의 jar 파일을 만들어서 이걸 키클락 설정에 커스텀 이벤트로 심어주고 해당 이벤트에서 유저 REGISTER 이벤트가 발생하면 user/keycloak/webhook api 를 호출해 사용자 정보를 DB에 저장하고, api 호출이 실패하면 키클락 디비에서 삭제하는 방식으로 수정해 진행했습니다..!

@JsonProperty("id_token")
private String idToken;

@JsonProperty("not-before-policy")

Choose a reason for hiding this comment

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

이것만 kebab-case인데 맞나요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

수정했습니다!

@NoArgsConstructor
@JsonIgnoreProperties(ignoreUnknown = true)
public static class Keycloak {
@JsonProperty("access_token")

Choose a reason for hiding this comment

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

snake_case를 지원하기 위해 모든 필드에 @JsonProperty를 활용했는데, 다른 방법은 없을까요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

기존에 이미 yml 에 sping.jackson.property-naming-strategy: SNAKE_CASE 를 지정했는데 키클락에서 보내는 요청에 대해서는 어째선지 설정이 제대로 되지 않는것같아서 해당 repsonse 클래스에@JsonNaming(PropertyNamingStrategies.SnakeCaseStrategy.class) 를 설정하는 방식으로 수정했습니다!

Choose a reason for hiding this comment

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

키클락에 보내는 요청에 대해서는 왜 설정이 제대로 되지 않는걸까요?

Comment on lines 19 to 28
@Value("${oauth.keycloak.grant-type}")
private String GRANT_TYPE;
@Value("${oauth.keycloak.credentials.client}")
private String CLIENT_ID;
@Value("${oauth.keycloak.credentials.secret}")
private String CLIENT_SECRET;
@Value("${oauth.keycloak.uri.redirect}")
private String REDIRECT_URI;
@Value("${oauth.keycloak.uri.token}")
private String TOKEN_URI;

Choose a reason for hiding this comment

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

@Value를 활용해서 각각의 필드를 받았는데, 이걸 좀 더 쉽게 한 번에 받는 방법은 없을까요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

KeycloakProperties 클래스를 만들어 value 로 가져올 수 있는 것들을 @ConfigurationProperties(prefix = "oauth.keycloak") 어노테이션 사용해 이쪽에서 가져오도록 설정했습니다!

Comment on lines 31 to 43
MultiValueMap<String, String> info = new LinkedMultiValueMap<>();
info.add("grant_type", GRANT_TYPE);
info.add("client_id", CLIENT_ID);
info.add("client_secret", CLIENT_SECRET);
info.add("redirect_uri", REDIRECT_URI);
info.add("code", code);

final HttpHeaders headers = new HttpHeaders();
headers.setContentType(MediaType.APPLICATION_FORM_URLENCODED);

final HttpEntity<MultiValueMap<String, String>> httpEntity = new HttpEntity<>(info, headers);

return restTemplate.postForEntity(TOKEN_URI, httpEntity, OAuthAccessTokenResponse.Keycloak.class).getBody();

Choose a reason for hiding this comment

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

http 요청을 하기위해서 map을 활용했는데, dto로 클래스를 만드는 것과 map을 사용하는 것에는 어떤 차이가 있을까요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

DTO로 만들면 타입 안정성들을 체크할 수 있을것같습니다.
DTO를 직접 HttpEntity에 넣으면, REST API 호출 시 DTO의 필드가 JSON 형태로 변환됩니다. 하지만 application/x-www-form-urlencoded 형식으로 전송하려면 각 필드가 URL 인코딩 된 형태로 전달되어야 하므로, DTO의 필드를 그 형식에 맞게 변환해야 합니다.

맵을 사용하면 RestTemplate의 postForEntity 메서드는 MultiValueMap 형식의 요청 본문을 자동으로 URL 인코딩하여 전송합니다.


public static User toEntity(UserRequestDto.Create dto) {
public static User toEntity(UserRequestDto.Create dto, String authId) {

Choose a reason for hiding this comment

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

사용하는 곳을 보니 dto.getId()를 하는데 따로 받는 이유가 있나요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

추후 사용자 탈퇴 처리 등을 할때 키클락에 등록된 아이디로 유저를 찾아 삭제하기 위해 저장해뒀습니다

Choose a reason for hiding this comment

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

아 제 얘기는 이 함수를 호출하는 곳에서,

UserRequestDto.Create.toEntity(dto, dto.getId())

위와 같은 형태로 처리를 하고 있는데, UserRequestDto.Create만 받는게 아니라, authId를 따로 받는 이유가 궁금해서 남긴 리뷰였습니다.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

헉 dto.getId() 를 하면 되는 것인데 실수했습니다;

public ApiSuccessResponse<UserResponse.Create> createUser(@Valid @RequestBody UserRequest.Create request) {
return ApiSuccessResponse.success(
UserResponse.Create.of(userService.create(UserRequest.Create.toDTO(request))));
@PostMapping("/keycloak/webhook")

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.

헉.. 생각해보지 못했습니다..
이럴 경우 admin 계정같은걸 만들고 웹훅을 보내는 쪽에서 어드민 계정의 토큰을 발급받아 해당 토큰으로 hook api 를 호출하도록 하면 될까요..?

Choose a reason for hiding this comment

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

네, 해당 방법도 하나의 좋은 방법이 될 것 같습니다. 추가적으로 더 조치할 방법은 없는지 좀 더 찾아보면 좋을 것 같습니다.

Copy link
Collaborator Author

@ohsuha ohsuha Sep 27, 2024

Choose a reason for hiding this comment

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

  1. @RequestHeader("X-API-KEY") 를 통해 리퀘스트 헤더에 미리 지정한 키를 설정하는 방식
  2. Keycloak이 요청을 보내는 서버의 IP 주소만 허용하도록 제한하는 방식

이렇게 두가지 방식을 더 찾아봤는데 1번을 사용하도록 수정했습니다!

Copy link

@f-lab-kai-c f-lab-kai-c left a comment

Choose a reason for hiding this comment

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

수고하셨습니다

@ohsuha ohsuha merged commit 44f21bf into develop Sep 27, 2024
1 check passed
@ohsuha ohsuha deleted the feature/auth/27-oauth2.0-login branch October 19, 2024 04:17
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.

로그인
2 participants