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

회원가입 및 로그인 & 인가 기능 구현 #82

Merged
merged 32 commits into from
Jul 20, 2023
Merged

회원가입 및 로그인 & 인가 기능 구현 #82

merged 32 commits into from
Jul 20, 2023

Conversation

jeomxon
Copy link
Collaborator

@jeomxon jeomxon commented Jul 19, 2023

🔥 연관 이슈

close: #29

📝 작업 요약

멤버 인증 인가 구현했습니다.

🔎 작업 상세 설명

OAuth로 정보를 가져와서 저장하는 기능
Member의 닉네임을 랜덤으로 생성하는 기능
JWTProcessor를 통해 토큰을 생성하는 기능
JwtFilter 구현
JwtArgumentResolver 구현

test 환경분리

🌟 논의 사항

  • accessToken에 들어가야할 정보에 대해서 논의하고 싶습니다.
  • JwtAuthenticationFilter에서 허용 시킬 URI를 어떤 방식으로 관리하면 좋을지 논의하고 싶습니다.
  • 멤버의 익명 닉네임을 효과적으로 사용할 수 있는 방법을 논의해보고 싶습니다. 현재는 서버 시작시점부터 숫자를 순차적으로 부여하고 있는데,
    마음에 안들어서 좀 간지나는 랜덤 숫자를 어떻게 중복시키지 않고 붙여볼지 고민 중입니다. 의견 있으시면 말씀해주세요~

@jeomxon jeomxon added this to the 2차 데모데이 milestone Jul 19, 2023
@jeomxon jeomxon self-assigned this Jul 19, 2023
@github-actions
Copy link

github-actions bot commented Jul 19, 2023

Test Results

41 tests   41 ✔️  5s ⏱️
21 suites    0 💤
21 files      0

Results for commit c35ae22.

♻️ This comment has been updated with latest results.

Copy link
Collaborator

@woo-chang woo-chang left a comment

Choose a reason for hiding this comment

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

수고하셨습니다 저문 🙇🏻‍♂️
전체적으로 코드가 깔끔하고 가독성이 좋아서
어떤 동작을 하는지 이해하기 쉬워서 긴 코드임에도 쉽게 리뷰할 수 있었습니다~
몇 가지(적다보니 30개나 되네요..) 코멘트와 질문 남겨두었으니 확인 부탁드립니다 :)

Comment on lines 29 to 32
implementation 'io.jsonwebtoken:jjwt-api:0.11.5'

testImplementation 'io.rest-assured:rest-assured'
testImplementation 'io.rest-assured:spring-mock-mvc'

runtimeOnly 'io.jsonwebtoken:jjwt-impl:0.11.5'
runtimeOnly 'io.jsonwebtoken:jjwt-jackson:0.11.5'
Copy link
Collaborator

Choose a reason for hiding this comment

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

Q : 0.11.5 버전을 사용하신 이유가 궁금합니다!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

jjwt 최신 버전으로 가져왔습니다!
최신버전이 배포된 지 1년이 지난 시점이라 안정적이게 사용할 수 있을 것이라 판단한 것도 있습니다.

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.

오 그렇게까지 생각을 못해본 것 같아요..!
배포가 자주 일어나는 것이 안정적일 것 같다는 생각이 드네요.
최신버전이 배포된 지 1년이 지난 시점이라 안정적이게 사용할 수 있을 것이라 판단한 것도 있습니다.에 말을 좀 덧붙이자면
최신 버전을 배포하고 1년동안 사람들이 문제없이 사용해왔다면 안정적이라고 할 수 있을 것 같다는 의미였습니다.

import org.springframework.web.bind.annotation.RequestParam;
import org.springframework.web.bind.annotation.RestController;

@Tag(name = "회원가입", description = "회원가입 및 로그인 API")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
@Tag(name = "회원가입", description = "회원가입 및 로그인 API")
@Tag(name = "인증", description = "인증 API")

P2 : 인증과 관련된 로직들이 모이는 서비스인 것 같은데 인증 정보를 드러내보는건 어떨까요?!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

동의합니다.
수정해보겠습니다~

private final AuthService authService;

@Operation(summary = "카카오 로그인 하기", description = "카카오 계정으로 로그인을 한다.")
@ApiResponse(responseCode = "200", description = "로그인 성공")
Copy link
Collaborator

Choose a reason for hiding this comment

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

P2 : 실패하는 상황이 있다면 실패하는 상황에 대한 설명도 적어주면 좋을 것 같아요 :)

Copy link
Collaborator Author

@jeomxon jeomxon Jul 19, 2023

Choose a reason for hiding this comment

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

믿으실지 모르겠지만 이거 지적 안나오면 리뷰 재요청하려고 했습니다.
추가하겠습니다 ㅎㅎ

Comment on lines 25 to 26
final String token = authService.register(code);
return ResponseEntity.ok(new LoginResponse(token));
Copy link
Collaborator

Choose a reason for hiding this comment

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

Q : 바로 response를 반환하지 않고 String을 반환하신 이유가 궁금합니다!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

소셜타입별로 추상화를 해보고자 처음에 이렇게 설계했는데,
지금으로써는 response를 반환하는 것이 통일성을 좀 더 가져갈 수 있을 것 같네요.
그리고 확실하게 추상화를 시킬 수 있다는 보장이 없기도 해서 response를 반환하도록 변경해볼게요!

Comment on lines 6 to 7
@JsonProperty("id") Long id,
@JsonProperty("kakao_account") KakaoAccount kakaoAccount
Copy link
Collaborator

Choose a reason for hiding this comment

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

Q : @JsonProperty를 사용하지 않으면 카멜 케이스가 자동으로 스네이크 케이스로 변환되지 않는건가요?
Q : @JsonProperty는 직렬화/역직렬화 과정에서 둘다 동작하는건가요? 🤔

Copy link
Collaborator Author

@jeomxon jeomxon Jul 19, 2023

Choose a reason for hiding this comment

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

  1. @JsonProperty를 사용하지 않으면 자동으로 변환되지 않습니다.
  2. 찾아보니 직렬화/역직렬화 과정에서 둘 다 동작한다고 하네요.
    덕분에 다른 어노테이션을 학습할 수 있었는데요.
    @JsonNaming(value = PropertyNamingStrategies.SnakeCaseStrategy.class)이라는 어노테이션을 클래스 레벨에 붙임으로써 반복되는 필드의 @JsonProperty를 줄이고 스네이크 케이스를 사용했음을 쉽게 파악할 수 있더라구요.
    해당 어노테이션을 사용하는 것으로 변경해보았습니다!

Copy link
Collaborator

Choose a reason for hiding this comment

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

꼼꼼한 학습까지 너무 좋습니다 :)

throw new IllegalArgumentException("토큰의 유효기간이 만료되었습니다.");
} catch (final IllegalArgumentException e) {
throw new IllegalArgumentException("토큰의 내용이 비어있습니다.");
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

P2 : 혹시 확인하지 못한 예외를 위해 Exception 상황도 추가하는건 어떤가요?

Copy link
Collaborator Author

@jeomxon jeomxon Jul 19, 2023

Choose a reason for hiding this comment

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

parseClaimJws()에서 발생할 수 있는 5가지 에러에 대해서 현재 예외처리를 한 상태였습니다.
확인하지 못한 예외가 어디서 발생할지 당장은 짐작이 안가긴하지만,
혹시 어디서 발생할지 모르는 예외에 대한 대비를 위해 Exception상황도 추가해보겠습니다.

}
}

public TokenPayload parseToken(final String token) throws JsonProcessingException {
Copy link
Collaborator

Choose a reason for hiding this comment

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

P1 : 토큰을 파싱하는 상황에서 토큰의 유효성 검사가 선행되어야 하지 않을까요?

Copy link
Collaborator Author

@jeomxon jeomxon Jul 19, 2023

Choose a reason for hiding this comment

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

filter에서 토큰 유효성을 검사하고 들어오는데, 여기서도 중복하여 검증한다고 생각해서 진행하지 않았습니다.
혹시 유효성 검사를 해야할 다른 이유가 있을까요?

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

@jeomxon jeomxon Jul 19, 2023

Choose a reason for hiding this comment

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

고민 끝에 validation을 각 메서드에 추가해주는 대신에
혹여나 사용할 수 있는 토큰 유효성 검증을 위해서
validation을 하는 메서드의 접근 제어자를 public으로 열어두면 좋을 것 같다는 생각입니다.

Comment on lines +30 to +41
oauth:
kakao:
info:
grant_type: ${GRANT_TYPE}
client_id: ${CLIENT_ID}
client_secret: ${CLIENT_SECRET}
redirect_uri: ${REDIRECT_URI}

jwt:
token:
secret-key: ${SECRET_KEY}
expiration-time: ${EXPIRATION_TIME}
Copy link
Collaborator

Choose a reason for hiding this comment

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

P1 : 중요한 정보는 환경 변수 좋아요 👍🏻

Comment on lines 1 to 28
spring:
datasource:
url: jdbc:h2:~/test;MODE=MySQL
username: sa

jpa:
properties:
hibernate:
format_sql: true
dialect: org.hibernate.dialect.MySQLDialect
show-sql: true

hibernate:
ddl-auto: create

logging:
level:
org:
hibernate:
type:
descriptor:
sql:
BasicBinder: TRACE

votogether:
openapi:
dev-url: http://localhost:8080
prod-url: http://votogether.com
Copy link
Collaborator

Choose a reason for hiding this comment

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

Q : 위의 정보는 없어도 src파일의 application.yml로 동작하지 않나요? (진짜 궁금)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

맞습니다. 동작해요...
오히려 적는 것이 기존에 있던 테스트 환경을 변화시키더라구요.
환경변수 부분을 제외하고는 삭제했습니다!

);
}

}
Copy link
Collaborator

Choose a reason for hiding this comment

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

P1 : jwt 디렉토리에 있는 기능들도 테스트 작성해보면 좋을 것 같습니다 :)

Copy link
Collaborator

@tjdtls690 tjdtls690 left a comment

Choose a reason for hiding this comment

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

OAuth의 권위자 저문

@Operation(summary = "카카오 로그인 하기", description = "카카오 계정으로 로그인을 한다.")
@ApiResponse(responseCode = "200", description = "로그인 성공")
@GetMapping("/auth/kakao/callback")
public ResponseEntity<LoginResponse> loginByKakao(@RequestParam("code") final String code) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Q : code와 같이 매핑할 파라미터 이름이 같으면 @RequestParam의 name 속성은 생략해도 되는 것으로 알고 있는데, "code"를 다시 한번 지정해준 이유가 있을까요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

명시적 표현을 위해서 지정해줬는데 제거하는 것도 깔끔하고 좋아보이네요!

Comment on lines 18 to 26
public String register(final String code) {
final String accessToken = kakaoOAuthClient.getAccessToken(code);
final KakaoMemberResponse response = kakaoOAuthClient.getMemberInfo(accessToken);

final Member member = Member.createKakaoMember(response);
final Member registeredMember = memberService.register(member);

return tokenProcessor.generateToken(registeredMember);
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

저번에 OAuth 흐름도에 비추어 봤을 때, 이해하기 굉장히 편하도록 추상화가 잘 되어있네요 👍👍

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

이해가 잘 되신다니 다행입니다...ㅎㅎ


private static final RestTemplate restTemplate = new RestTemplate();

private final MultiValueMap<String, String> info = new LinkedMultiValueMap<>();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Q : Map 대신 MultiValueMap을 사용한 이유가 restTemplate의 postForObject 메서드에 MultiValueMap만 들어갈 수 있어서인가요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

그런 이유는 아니예요. 보편적으로 MultiValueMap이 사용되는 것입니다.
MultiValueMap은 하나의 key에 여러 개의 value가 들어갈 수 있는 구조입니다.
예를들어 파라미터로 하나의 키에 다중 값이 들어오는 경우를 생각해볼 수 있습니다.
해당 경우에는 단순 HashMap과 같은 자료구조로는 값을 전부 담기 힘듭니다.
따라서 이와 같은 경우를 대비해서 MultiValueMap을 사용한다고 볼 수 있습니다.


public class NicknameNumberGenerator implements NumberGenerator {

private int number = 0;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Q

제가 알기로는 number를 static으로 해놔야 위에서

final NicknameNumberGenerator nicknameNumberGenerator = new NicknameNumberGenerator();

이 코드를 실행해도 증가값이 유지되는 것으로 알고 있습니다.

이 상태라면 위의 createKakaoMember() 메서드에서 new NicknameNumberGenerator() 로 생성될 때마다 number가 0으로 초기화 될 듯 합니다.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

저의 레전드 오류를 찾아주셔서 감사합니다.
휴먼 에러 빠르게 수정하여 반영하겠습니다!


public Member findById(final Long memberId) {
return memberRepository.findById(memberId)
.orElseThrow(() -> new IllegalArgumentException("해당 Id를 가지 회원은 존재하지 않습니다."));
Copy link
Collaborator

Choose a reason for hiding this comment

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

P1 : 가지 -> 가진

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

시력테스트였습니다^^

Comment on lines 24 to 25
@Autowired
MockMvc mockMvc;
Copy link
Collaborator

Choose a reason for hiding this comment

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

P1 : RestAssuredMockMvc 쓸 때, MockMvc는 Autowired 안해줘도 됩니다!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

마이그레이션 과정에서 지워야하는데 그러지 못했네요.
꼼꼼하게 확인하도록 하겠습니다!

Comment on lines +49 to +57
// when
String responseBody = RestAssuredMockMvc
.given().log().all()
.queryParam("code", "abc1234")
.when().get("/auth/kakao/callback")
.then().log().all()
.status(HttpStatus.OK)
.extract()
.asString();
Copy link
Collaborator

Choose a reason for hiding this comment

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

extract로 꺼내서 given, when, then을 명확히 구분하는게 인상적이네요 👍

Copy link
Collaborator

@aiaiaiai1 aiaiaiai1 left a comment

Choose a reason for hiding this comment

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

저문 새로운개념 공부하면서 기능 구현하느라 고생하셨습니다 🙇
저는 머리가 어지럽네요 간단한 코멘트와 질문 남겨드렸습니다
고생하셨어요~

) throws ServletException, IOException {
final String token = request.getHeader(HttpHeaders.AUTHORIZATION);
final String tokenWithoutType = tokenProcessor.resolveToken(token);
tokenProcessor.validateToken(tokenWithoutType);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Q:TokenProcessor의 메서드 validateToken를 public으로 해서 여기서 검증한 이유가 있으시나요? resolveToken 메서드안에서 이루어질수도 있을거 같아서요

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

validateToken을 다른 부분에서도 검증용도로 사용할 수 있을 것 같아서 public으로 사용했습니다.
parseToken에서도 검증이 필요하다는 다즐의 의견도 있어서 한번 다 같이 모여서 의견을 들어보고 싶네요.
어떤 방향으로 token을 검증할지 이야기해보면 좋을 것 같아요 :)

final String tokenWithoutType = tokenProcessor.resolveToken(token);
tokenProcessor.validateToken(tokenWithoutType);
filterChain.doFilter(request, response);
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

P3 : doFilterInter 메서드에서 하는 행위가 토큰 검사인거같은데 필터에서 토큰 검사한다는걸 명시해주기 위해 메서드로 하나 빼는건 어떤가요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

어떤식으로 메서드를 분리해야할지 감이 잘 안잡히는데 혹시 예시를 보여주실 수 있나요?

Copy link
Collaborator

Choose a reason for hiding this comment

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

        final String token = request.getHeader(HttpHeaders.AUTHORIZATION);
        final String tokenWithoutType = tokenProcessor.resolveToken(token);
        tokenProcessor.validateToken(tokenWithoutType);

이 내용을 option + command + M으로 빼서 메서드명은 validate~ 라고 명시하면 어떨까 라는 의견이였습니다

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

한 단계 추상화하는 방법도 좋다는 생각같아요 ㅎㅎ
다만 조금 더 로직이 커지게 된다면 분리하는 것이 어떨까?라는 생각입니다.
토큰을 resolve하고 parsing하는 과정이 잘 드러나서 아직까지는 분리 시점이라고 느끼지 않았습니다.
이 부분은 그대로 가져가보겠습니다!
생각하지 못했던 부분이었는데 언급해주셔서 감사합니다 :)

@Operation(summary = "카카오 로그인 하기", description = "카카오 계정으로 로그인을 한다.")
@ApiResponse(responseCode = "200", description = "로그인 성공")
@GetMapping("/auth/kakao/callback")
public ResponseEntity<LoginResponse> loginByKakao(@RequestParam("code") final String code) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

P3 : 메서드이름으로 loginWithKakao 는 어떠신가요? 테스트코드에는 이렇게 작성되어있더라고요

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

좋은 지적 감사합니다 ㅎㅎ
테스트코드에서 loginByKakao로 바꾸는 것이 더 좋을 것 같아서 그렇게 변경해볼게요~


private static final RestTemplate restTemplate = new RestTemplate();

private final MultiValueMap<String, String> info = new LinkedMultiValueMap<>();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Q : @ConfigurationProperties(prefix = "oauth.kakao")이 어떤 역할인가요??
xml 에 파일에 있는 info정보가 MultiValueMap<String, String> info = new LinkedMultiValueMap<>();와 매핑되어 들어가지는 건가요??

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파일에 있는 환경변수들을 매핑해주기 위해서 사용하고 있습니다.
oauth.kakao.info에 있는 key value가 매핑이 됩니다!


public class NicknameNumberGenerator implements NumberGenerator {

private int number = 0;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Q 만약 서버를 껐다가 다시 키게되면 랜덤값이 다시 0부터 초기화 되지 않나요??

Copy link
Collaborator Author

@jeomxon jeomxon Jul 19, 2023

Choose a reason for hiding this comment

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

static을 붙여서 서버가 실행되는 도중에는 값이 바뀌지 않도록 변경했습니다.
혹시나 서버가 꺼질 상황도 있을 것 같은데 그에 대비해서 다른 정책에 대해 이야기를 해보고,
적용하는 방식으로 진행하면 좋을 것 같아요!

Copy link
Collaborator

@woo-chang woo-chang left a comment

Choose a reason for hiding this comment

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

아주아주 사소한 커멘트 몇 가지만 남겼습니다 :)
수고하셨어요 저문 🙇🏻‍♂️👏🏻

@@ -1,7 +1,7 @@
votogether:
openapi:
dev-url: http://localhost:8080
prod-url: http://votogether.com
prod-url: http://aaaaaaaa.com
Copy link
Collaborator

Choose a reason for hiding this comment

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

아아아아아아아아

private MemberRepository memberRepository;

private TokenProcessor tokenProcessor;
private ObjectMapper objectMapper;
Copy link
Collaborator

Choose a reason for hiding this comment

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

P2 : @RepositoryTest는 JpaConfig를 import하고 있으므로 빈으로 등록된 ObjectMapper를 사용해도 괜찮지 않을까요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

제가 등록해놓고도 인지를 못하고 있었네요...ㅎㅎ
수정하겠습니다!

objectMapper = new ObjectMapper();
String secretKey = "abcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcdabcd";
int expirationTime = 100000;
tokenProcessor = new TokenProcessor(
Copy link
Collaborator

Choose a reason for hiding this comment

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

P3 : 동일한 TokenProcessor를 생성하는 동작으로 이해했습니다! 테스트마다 초기화해주는 이유가 있을까요 🤔

Copy link
Collaborator Author

@jeomxon jeomxon Jul 20, 2023

Choose a reason for hiding this comment

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

좀 더 효과적인 방법으로 리팩터링 진행했습니다!
인지하지 못하고 있었네요. 덕분에 배워갑니다 :)

Copy link
Collaborator

@tjdtls690 tjdtls690 left a comment

Choose a reason for hiding this comment

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

👍👍👍👍👍

) {

@JsonNaming(value = PropertyNamingStrategies.SnakeCaseStrategy.class)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Q : SnakeCaseStrategy 전략이 camel 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.

원래는 @JsonProperty를 이용해서 각 필드를 snake case로 변경해주었는데요.
@JsonNaming을 사용하여 해당 클래스에 일괄적으로 직렬화, 역직렬화 시에 snake case로 적용할 수 있습니다.
카카오에서 제공하는 문서에 따르면 필드들이 snake case로 지정되어있어서 적용해주었습니다!

@@ -1,7 +1,7 @@
votogether:
openapi:
dev-url: http://localhost:8080
prod-url: http://votogether.com
prod-url: http://aaaaaaaa.com
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.

으어어어어~

Copy link
Collaborator

@aiaiaiai1 aiaiaiai1 left a comment

Choose a reason for hiding this comment

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

저문 확인 완료했습니다
피드백 반영하시느라 고생하셨어요~

@ParameterizedTest
@ValueSource(strings = {"Bear", "Barrier", "Baerer", "bearer"})
@DisplayName("토큰의 prefix를 제외한 값을 추출한다.")
void resolveTokenFail(String prefix) {
Copy link
Collaborator

@aiaiaiai1 aiaiaiai1 Jul 20, 2023

Choose a reason for hiding this comment

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

P2 : @DisplayName이 전의 테스트랑 복붙이 된거같아요

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

올리자마자 확인해서 수정했습니다! ㅎㅎ

final String tokenWithoutType = tokenProcessor.resolveToken(token);
tokenProcessor.validateToken(tokenWithoutType);
filterChain.doFilter(request, response);
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

        final String token = request.getHeader(HttpHeaders.AUTHORIZATION);
        final String tokenWithoutType = tokenProcessor.resolveToken(token);
        tokenProcessor.validateToken(tokenWithoutType);

이 내용을 option + command + M으로 빼서 메서드명은 validate~ 라고 명시하면 어떨까 라는 의견이였습니다

@jeomxon jeomxon merged commit 78750b5 into dev Jul 20, 2023
3 checks passed
@jeomxon jeomxon deleted the feat/#29 branch July 20, 2023 05:21
This was referenced Sep 12, 2023
This was referenced Sep 12, 2023
@tjdtls690 tjdtls690 mentioned this pull request Sep 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

회원가입 및 로그인 & 인가 기능 구현
4 participants