-
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
[🚌 Issue] dev api 로그인 요청 위치에 따라 redirect url 변환 #245
Changes from 20 commits
b99367c
4249f81
854f4a2
23aa5f9
1c7d46f
74b372b
54c8b81
ef9f69d
30b557b
89264c7
5425558
48e2709
a9135a1
63bb7e9
4c110bc
bb151c4
0a00a11
c94c5b5
e97bc4e
583201f
e307c2d
1073e18
39ce4cc
128357d
e27bb5b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,8 +1,6 @@ | ||
package io.raemian.api.goal.controller.request | ||
|
||
import java.time.LocalDateTime | ||
|
||
data class TimelinePageRequest( | ||
val cursor: LocalDateTime?, | ||
val size: Int, | ||
val page: Int = 0, | ||
val size: Int = 20, | ||
) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
package io.raemian.api.support.response | ||
|
||
data class OffsetPaginationResult<T>( | ||
val total: Long, | ||
val page: Int, | ||
val size: Int, | ||
val contents: List<T>, | ||
) { | ||
companion object { | ||
fun <T> of(page: Int, size: Int, total: Long, contents: List<T>): OffsetPaginationResult<T> { | ||
return OffsetPaginationResult(total, page, size, contents) | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,26 @@ | ||
package io.raemian.api.support.security | ||
|
||
import io.raemian.api.auth.model.Token | ||
import org.springframework.beans.factory.annotation.Value | ||
import org.springframework.stereotype.Component | ||
|
||
@Component | ||
class LoginRedirector( | ||
@Value("\${spring.profiles.active:local}") | ||
private val profile: String, | ||
val loginRequestRefererStorage: LoginRequestRefererStorage, | ||
) { | ||
fun getUrl(state: String, token: Token): String { | ||
if (profile == "live") { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 프로필을 한번 Wraaping하는 객체가 있으면 어떨까 싶은데, 한번 같이 고민해주실 수 있나요? ProfileHolder or ProfileUtil이라는 이름의 객체가 있다면 더 나아가 괜찮은 구현인지는 모르겠지만, 상태에 따라 RedirectURL을 반환하는 객체가 있는 것도 괜찮을 것 같아요. 토큰 값에 대한 파라미터는 따로 붙여주거나, 따로 붙여주는 Util 클래스가 있거나, 아니면 토큰 값들을 넣으면 토큰 값까지 모두 포함된 RedirectUrl을 반환하는 객체가 있는 것도 괜찮을 것 같아요 (마지막은 과한 것 같기도 합니다) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 객체를 어떤 단위까지 분리할까는 항상 고민이 되는 부분인 것 같습니다 ㅎㅎ
사실 저는 작업하면서 이렇게까지 객체를 분리하지는 않았던 것 같습니다. 우선은 ProfileHolder로 현재 application의 profile을 알려주는 책임을 가지는 객체 만들어 반영해보겠습니다. |
||
return "https://bandiboodi.com/oauth2/token?token=${token.accessToken}&refresh=${token.refreshToken}" | ||
} | ||
|
||
val referer = loginRequestRefererStorage.get(state) | ||
|
||
if (profile == "dev" && referer.contains("dev-bandiboodi")) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 commentThe 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 commentThe reason will be displayed to describe this comment to others. Learn more. 별거 아닌데 프로필에 대한 비교값을 문자열이 아닌 enum 으로 관리하는게 좋긴할듯 (referer 도 enum 으로) There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 commentThe reason will be displayed to describe this comment to others. Learn more. 같은 부분을 형은 enum으로, 진호는 현재 profile을 원하는 값과 비교해주는 객체 메서드로 바꾸는 방향을 제안해줬는데 |
||
return "https://dev-bandiboodi.vercel.app/oauth2/token?token=${token.accessToken}&refresh=${token.refreshToken}" | ||
} | ||
|
||
return "http://localhost:3000/oauth2/token?token=${token.accessToken}&refresh=${token.refreshToken}" | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
package io.raemian.api.support.security | ||
|
||
import com.github.benmanes.caffeine.cache.Caffeine | ||
import org.springframework.stereotype.Component | ||
import java.util.concurrent.TimeUnit | ||
|
||
@Component | ||
class LoginRequestRefererStorage { | ||
private val timedStorage = Caffeine.newBuilder() | ||
.expireAfterWrite(60L, TimeUnit.SECONDS) | ||
.build<String, String>() | ||
|
||
fun put(state: String, referer: String?) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [[ 100% 취향 제안 ]] put() = if (referer.isNullOrBlank()) return
else timedStorage.put(state, referer) 혹은.. 확장함수 활용해서 isNotNullOrBlank() 구현한 다음.. fun put() {
if (referer.isNotNullOrBlank())
timedStorage.put(state, referer)
} 완전 취향이고 지금 얼리 리턴 방식도 너무 좋아요 저도 보통 그렇게 작성해왔던 것 같아요. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 commentThe reason will be displayed to describe this comment to others. Learn more. @binary-ho 추가 제안 ㅆㄱㄴ |
||
if (referer.isNullOrBlank()) { | ||
return | ||
} | ||
|
||
timedStorage.put(state, referer) | ||
} | ||
|
||
fun get(state: String): String { | ||
return timedStorage.getIfPresent(state) ?: "" | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,13 +4,18 @@ import com.github.benmanes.caffeine.cache.Cache | |
import com.github.benmanes.caffeine.cache.Caffeine | ||
import jakarta.servlet.http.HttpServletRequest | ||
import jakarta.servlet.http.HttpServletResponse | ||
import org.springframework.beans.factory.annotation.Value | ||
import org.springframework.security.oauth2.client.web.AuthorizationRequestRepository | ||
import org.springframework.security.oauth2.core.endpoint.OAuth2AuthorizationRequest | ||
import org.springframework.stereotype.Component | ||
import java.util.concurrent.TimeUnit | ||
|
||
@Component | ||
class StateOAuth2AuthorizationRequestRepository() : AuthorizationRequestRepository<OAuth2AuthorizationRequest> { | ||
class StateOAuth2AuthorizationRequestRepository( | ||
@Value("\${spring.profiles.active:local}") | ||
private val profile: String, | ||
val loginRequestRefererStorage: LoginRequestRefererStorage, | ||
) : AuthorizationRequestRepository<OAuth2AuthorizationRequest> { | ||
private val oauthRequestStorage: Cache<String, OAuth2AuthorizationRequest> = Caffeine.newBuilder() | ||
.expireAfterWrite(60L, TimeUnit.SECONDS) | ||
.build() | ||
|
@@ -25,6 +30,11 @@ class StateOAuth2AuthorizationRequestRepository() : AuthorizationRequestReposito | |
response: HttpServletResponse, | ||
) { | ||
if (authorizationRequest != null) { | ||
/*** for frontend dev test ***/ | ||
if (profile == "dev") { | ||
loginRequestRefererStorage.put(authorizationRequest.state, request.getParameter("referer")) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 commentThe reason will be displayed to describe this comment to others. Learn more. 제안 감사합니다 👍 추가 제안 ㅆㄱㄴ |
||
} | ||
|
||
oauthRequestStorage.put(authorizationRequest.state, authorizationRequest) | ||
} | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,5 @@ | ||
package io.raemian.storage.db.core.common.pagination | ||
|
||
interface CursorExtractable<CursorType> { | ||
fun cursorId(): CursorType | ||
interface CursorExtractable { | ||
fun cursorId(): Long | ||
} |
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.
최근에 이런 방법도 봤네요. 값들이 모여있고, 객체로 관리하고, 리팩토링 하기 쉬운 장점이 있는거 같은데 한번 슥 봐주세요 절대 이거 도입해보자 아닙니다!!
https://medium.com/@mikael_55667/why-you-should-stop-using-value-annotations-in-spring-and-use-this-instead-2c8a47e5096a
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.
저는 진호님이 얘기해주신 ConfigurationProperties 사용하는 방식 좋아합니다!
저희 프로젝트에 AppleLoginProps 객체에 사용하기도 했구요
다만 저는 한번에 많은 값을 주입해야 할 때 ConfigurationProperties를 고려했던 것 같습니다!
Profile을 담을 객체에 주입할 값이 많아진다면 ConfigurationProperties 사용하는 방식 좋은 것 같아요 👍