-
Notifications
You must be signed in to change notification settings - Fork 1
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
#261_T-10856 [feat] /token API 구현 #262
Conversation
auth service 내에 있는 플랫폼 인가코드 발급 및 검증기능을 jwt token provider 내로 옮겼습니다. 테스트를 위해서 @value 어노테이션을 생성자로 받도록 수정했습니다
테스트 코드 작성을 위해 concurrent hash map을 configuration 설정했다
concurrent hash map mock bean 추가 uri -> authorizeUri로 변수명 변경
만료된 플랫폼 인가코드가 map에 존재하는 것을 방지한다
This pull request has been linked to 1 task:
|
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.
Test까지 꼼꼼히 해주시고 너무 좋네요!!
고생 많으셨구
Test를 위한 어플리케이션 코드 추가 측면에 대해서만 한 번 애기 나눠보고 싶어용!!
우선 Test에 대한 내용이라 기능상 문제가 없어 보이니 Approve 해놓겠습니다!
operation-api/src/main/java/org/sopt/makers/operation/auth/api/AuthApiController.java
Show resolved
Hide resolved
...ation-api/src/main/java/org/sopt/makers/operation/common/config/ConcurrentHashMapConfig.java
Show resolved
Hide resolved
operation-api/src/main/java/org/sopt/makers/operation/auth/api/AuthApiController.java
Show resolved
Hide resolved
operation-api/src/main/java/org/sopt/makers/operation/auth/api/AuthApiController.java
Show resolved
Hide resolved
머지해도 괜찮을 것 같네요!! |
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.
너무너무 고생하셨습니다 ! 테스트 코드도, 로직도 모두모두 꼼꼼하게 짜주시고 사소한거까지 고려했다는게 느껴져서 많이 배웠네요 ! 👍👍
그 이번 PR에서 변경사항은 아니었지만, 제가 저번 PR을 빨리 확인 못해서 ㅠ JwtTokenProvider
클래스의 resolveToken
메서드에서 궁금한 점이 하나 있는데요 ! 저희 Authorization 클라가 헤더에 담아서 보낼 때 Bearer
붙여서 통신하게 할지 궁금합니다 ! Bearer
붙여서 통신하게 할거면 접두사 제거하는 로직이 필요할거 같기는 해서, 가볍게 여쭤봅니다 ~!
@@ -1,5 +1,5 @@ | |||
package org.sopt.makers.operation.jwt; | |||
|
|||
public enum JwtTokenType { | |||
ACCESS_TOKEN, REFRESH_TOKEN, APP_ACCESS_TOKEN | |||
ACCESS_TOKEN, REFRESH_TOKEN, APP_ACCESS_TOKEN, PLATFORM_CODE |
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.
[p5]
요 부분 제가 놓친거 같은데요 ! 혹시 APP_ACCESS_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.
app access token은 기존에 있었던 로직이라 자세한 맥락은 모르겠지만,
코드를 읽어봤을 때 playground secret key를 사용한다는 점과 앱은 운영 프로덕트 팀의 출석 API를 사용한다는 점을 봤을 때
출석 API시 유저 구분을 하기 위해 사용되는 것으로 추측 됩니다..!
@yeseul106 |
Related Issue 🚀
Work Description ✏️
PR Point 📸
1. jwt token provider 사용
기존에는 auth service 에서 플랫폼 인가코드를 생성하는 로직이 있었습니다.
개발 도중 기존에 만들어진 jwt token provider를 사용하면 코드 중복을 줄일 수 있겠다고 생각하여
Platform Code enum 값을 추가하여 플랫폼 인가코드를 발급하는 로직을 추가했습니다.
2. concurrent hash map 빈 등록
/token 테스트 코드를 작성할 때, hash map에 플랫폼 인가코드가 contain하는지 검증하는 로직을 테스트해야 했습니다.
기존에는
= new Hashmap<>()
같이 설정했기 때문에 테스트 코드를 작성하는데 어려웠습니다.이를 해결하기 위해 Configuration을 통해 ConcurrentHashMap을 bean으로 등록해 의존성을 주입받는 형태로 변경했고,
테스트 코드에서는 mockbean을 통해 테스트 가능하도록 만들었습니다.
3. refresh token 검증 과정
클라이언트가 ATK가 만료되어 RTK를 통해 토큰을 갱신 받는 과정에서 grantType 과 refreshToken만 받고 있습니다.
clientId와 redirectUri를 claims에 넣어 검증하려 했으나,
플레이그라운드에서 발급한 토큰이 모임에서 만료되었을 때, 토큰 내부의 clientId와 호출하는 팀(모임)의 clientId가 다르기 때문에 위와 같은 검증이 제대로 이뤄지지 않을 것이라 판단했습니다.
따라서 grantType 과 refreshToken만 받아서 refreshToken이 만료되지 않았다면 다시 ATK, RTK를 반환하도록 구현하려하는데,
이 방식에 동의하는지 궁금합니다..!