[REFACTOR] Auth 도메인 리펙토링 #220

wook099 · 2026-01-09T06:37:50Z

Auth Service 기능 개선 및 구조 분리

RedisService의 isBlacklisted메서드는 jwt도메인 리펙토링 머지 후 doFilterInternal에 추가할 로직입니다

dnwls16071

수고하셨습니다.
내용 한 번 검토해주세요.

src/main/java/org/atdev/artrip/controller/dto/request/ReissueRequest.java

dnwls16071 · 2026-01-09T10:32:25Z

src/main/java/org/atdev/artrip/global/apipayload/code/status/AuthErrorCode.java

이 부분 좋네요👍
네이밍은 ~ErrorCode로 일관성있게 가면 좋을 것 같아요.

AuthError->AuthErrorCode 이런식으로 전부 통일을 말씀하시는걸까요?
Error로도 가독성 측면에서 충분하지 않을까요?

Error라 함은 애플리케이션에서 복구 불가능한 심각한 오류를 말해요.
아예 의미가 다르다고 생각하는데 어떻게 생각하실까요?

네 이해했습니다! 수정하겠습니다

dnwls16071 · 2026-01-09T10:33:06Z

src/main/java/org/atdev/artrip/jwt/JwtProvider.java


+            return (expirationTime - now);
+        } catch (ExpiredJwtException e){
+            return 0;


토큰이 만료되었다면 예외를 반환하는게 좋지 않을까요?

해당 로직은 로그아웃시 엑세스토큰 만료까지 남은 기간을 구하는 로직입니다
엑세스토큰이 만료되었다면 사용 할 수 없는 상태로 관리할 필요가 없어 0을 반환하여 건너 뛸 수 있게 설계하였습니다

API에서 활용되는 부분이라면 예외가 맞을 것 같고 그게 아니라 단순 계산이라면 지금처럼 해도 될 것 같네요.

네 만료예외를 제외한 예외에는 에러처리를 하겠습니다

src/main/java/org/atdev/artrip/service/AuthService.java

dnwls16071 · 2026-01-09T11:34:59Z

src/main/java/org/atdev/artrip/service/RedisService.java

+    public void saveBlacklist(String accessToken, long expirationMillis) {
+        save("BLACKLIST:" + accessToken, "logout", expirationMillis);
+    }
+
+    public boolean isBlacklisted(String accessToken) {
+        return getValue("BLACKLIST:" + accessToken) != null;
+    }


범용 Redis를 다루는 곳에서 Token을 알 필요가 있을까요?

제거하고 save를 재사용할수있도록 수정하겠습니다

dnwls16071 · 2026-01-09T11:36:06Z

src/main/java/org/atdev/artrip/service/social/GoogleValidator.java

+    public SocialUserInfo verify(String idToken) {
+
+        try {
+            String jwksUrl = "https://www.googleapis.com/oauth2/v3/certs";


상수로 분리 혹은 보안이 중요하다면 환경변수로 분리하는 것도 좋을 것 같아요.

넵 고정된 식별자이고 공개되는 정보이기때문에 상수로 분리하겠습니다

src/main/java/org/atdev/artrip/service/social/GoogleValidator.java

dnwls16071 · 2026-01-09T11:38:34Z

src/main/java/org/atdev/artrip/service/social/KakaoValidator.java

+import java.util.List;
+
+@Service
+public class KakaoValidator implements SocialVerifier{


네이밍이 검증기라고 쓰셨는데 @Service 어노테이션이 붙은 이유가 있을까요? 서비스라면 네이밍을 바꿀 수 있을 것 같아요.

유효성을 검사하는 비즈니스로직이 들어가 @service를 선언했고 로직 내용을 네이밍따서 vaildator라고 지었습니다

초반 네이밍작업때 현재 서비스 패키지는 도메인명을 따 service로 통일되어있기에 vaildator 클래스에도 service를 붙이면 관계가 좀 어색해지지 않을까란 생각을 했었습니다

그럼에도 @service 어노테이션이 있으니 service을 네이밍하는게 일관성 있는 설계일까요?

파라미터의 검증이 프레젠테이션 레이어에 있느냐 서비스 레이어에 있느냐는 정확하게 답이 있는 것은 아니지만 서비스 레이어의 경우 비즈니스 요구사항 코드만 있어야 유지보수가 용이하다고 생각해요.

만약 다른 파라미터의 검증이 들어온다거나 하게 된다면 서비스 레이어에서 코드를 수정해야하니까요.
검증기를 써야한다면 패키지로 분리를 한다든가하는 방법도 고민을 해 볼 순 있을 것 같아요.

네 컴포넌트로 수정하겠습니다 패키지분리는 global- social 로 두려는데 괜찮을까요?

global은 공용 느낌이 나는 네이밍이라 다른 패키지로 빼는게 좋을 것 같아요.

넵 validator로 따로 빼보겠습니다

dnwls16071 · 2026-01-09T11:39:56Z

src/main/java/org/atdev/artrip/validator/social/SocialVerifier.java

+
+public interface SocialVerifier {
+
+    Provider getProvider();


이 부분은 단순히 자격증명 Enum을 리턴하는 것으로 보이는데 굳이 있을 필요가 있을까요?
각 구현체에서 상수로 선언해서 사용해도 되지 않을까요?

확장성과 유지보수 측면에서 상수로 선언하는것보다 타입안정성과 재사용성을 고려해 enum으로 선언하였습니다

다만 enum이 여러 계층에 결합되어 OCP가 지켜지지 않은거 같습니다
현재 구조 :provider → SocialVerifier → validator / authService

현재 역설적이게도 결합도가 높지만 변경비용은 크지 않다고 생각을 하고 있습니다
(애플 확장시provider,validator1개 추가)
위 이슈에 대해서 어떤 방법이 적절한지 고민이 많이 되네요

getProvider()의 경우 단순 가져오는 형식으로만 쓰이는 것으로 이해했는데 어느 부분에서의 확장성과 유지보수가 저하된다고 보시는 것인지 알 수 있을까요?

getProvider() 를 통해 authservice에서 리스트로 순회하며 동적 주입받고있습니다
상수로 선언하면 애플 추가시 이에 대해 if else로 관리하게되어 확장성, 유지보수측면에서 어려움이 생길거라 생각했습니다

SocialVerifier verifier = socialVerifiers.stream() .filter(v -> v.getProvider() == provider) .findFirst() .orElseThrow(() -> new GeneralException(AuthErrorCode._UNSUPPORTED_SOCIAL_PROVIDER));

매번 돌기는 해야하는 구조네요. 그렇게 많이 늘 것 같진 않으니 다른 패턴을 사용하기보다는 지금 구조도 괜찮을 것 같네요.

dnwls16071

수고하셨습니다.
한 번 검토해주세요.

dnwls16071 · 2026-01-11T03:31:44Z

src/main/java/org/atdev/artrip/controller/AuthController.java

    @PostMapping("/app/logout")
-    public ResponseEntity<String> appLogout(@RequestBody(required = false) ReissueRequest refreshToken) {
+    public ResponseEntity<String> appLogout(@RequestBody(required = false) ReissueRequest token) {

-        authService.appLogout(refreshToken);
+        authService.appLogout(token);

        return ResponseEntity.ok("로그아웃 완료");


단순 String을 리턴하는거라면 로그아웃은 반환 타입이 필요없지않을까요?

넵 null로 반환하도록 하겠습니다

null을 리턴하는게 좋진 않아요.
없다면 void로 하는게 좋을 것 같아요.

ResponseEntity<void> (X)
void (O)

넵 혹시 return ResponseEntity.noContent().build(); 이런식의 반환은 어떤가요?
200 OK, 204 No Content의 차이인거같은데 성공의 응답 코드를 세분화해야한다면 더 좋지않을까 생각되어 여쭤봅니다!

dnwls16071 · 2026-01-11T08:18:28Z

src/main/java/org/atdev/artrip/jwt/JwtProvider.java


+            return (expirationTime - now);
+        } catch (ExpiredJwtException e){
+            return 0;


API에서 활용되는 부분이라면 예외가 맞을 것 같고 그게 아니라 단순 계산이라면 지금처럼 해도 될 것 같네요.

dnwls16071 · 2026-01-11T08:19:31Z

src/main/java/org/atdev/artrip/constants/Provider.java

+        if(providerName==null){
+            throw new GeneralException(AuthError._SOCIAL_EMAIL_NOT_PROVIDED);
+        }
+        try {
+            return Provider.valueOf(providerName.toUpperCase());
+        } catch (IllegalArgumentException | NullPointerException e) {
+            throw new GeneralException(AuthError._UNSUPPORTED_SOCIAL_PROVIDER);
+        }


Provider.values()로 순회해서 찾으면 되지 않을까요?

네 훨씬 깔끔해질거같습니다

src/main/java/org/atdev/artrip/controller/dto/response/SocialUserInfo.java

dnwls16071 · 2026-01-11T08:24:11Z

src/main/java/org/atdev/artrip/global/apipayload/code/status/AuthErrorCode.java

Error라 함은 애플리케이션에서 복구 불가능한 심각한 오류를 말해요.
아예 의미가 다르다고 생각하는데 어떻게 생각하실까요?

src/main/java/org/atdev/artrip/service/social/GoogleValidator.java

dnwls16071 · 2026-01-11T08:28:41Z

src/main/java/org/atdev/artrip/service/social/KakaoValidator.java

+import java.util.List;
+
+@Service
+public class KakaoValidator implements SocialVerifier{


파라미터의 검증이 프레젠테이션 레이어에 있느냐 서비스 레이어에 있느냐는 정확하게 답이 있는 것은 아니지만 서비스 레이어의 경우 비즈니스 요구사항 코드만 있어야 유지보수가 용이하다고 생각해요.

만약 다른 파라미터의 검증이 들어온다거나 하게 된다면 서비스 레이어에서 코드를 수정해야하니까요.
검증기를 써야한다면 패키지로 분리를 한다든가하는 방법도 고민을 해 볼 순 있을 것 같아요.

dnwls16071 · 2026-01-11T08:29:41Z

src/main/java/org/atdev/artrip/validator/social/SocialVerifier.java

+
+public interface SocialVerifier {
+
+    Provider getProvider();


getProvider()의 경우 단순 가져오는 형식으로만 쓰이는 것으로 이해했는데 어느 부분에서의 확장성과 유지보수가 저하된다고 보시는 것인지 알 수 있을까요?

dnwls16071

수고하셨습니다.
거진 수정은 되었지만 GET을 제외한 모든 API에서 응답을 반환하는게 보이는데 해당 응답이 필요없다면 굳이 리턴을 할 필요는 없어보여요.

이건 나중에 수정하는 것으로 하시죠

wook099 added 2 commits January 9, 2026 13:50

[REFACTOR] AuthService 로직 리펙토링

93c3c3c

[REFACTOR] redis Service SRP 책임 분리

487fea6

wook099 self-assigned this Jan 9, 2026

wook099 added the ♻️ Refactor label Jan 9, 2026

wook099 added 3 commits January 9, 2026 15:45

[CHORE] 공백 제거

3c299fa

[CHORE] 수정

1f08206

[FIX] SocialUserInfo provider누락 수정

5339bf9

dnwls16071 requested changes Jan 9, 2026

View reviewed changes

wook099 added 6 commits January 10, 2026 03:24

[REFACTOR] 토큰 Dto -> record 타입으로 전환

935ae97

[REFACTOR] 쿠키utils 구조 분리, Provider변환 로직 enum으로 분리

65a2f05

[REFACTOR] user생성 로직 엔티티 내부에서 동작하도록 분리

859593c

[REFACTOR] SocialUserInfo 정적팩토리메서드 도입

a94e16f

[REFACTOR] 레디스service 책임분리

6341418

[REFACTOR] resolve merge conflict, 블랙리스트 토큰 접근 제어

e0776b3

dnwls16071 requested changes Jan 11, 2026

View reviewed changes

wook099 added 4 commits January 11, 2026 19:21

[REFACTOR] error -> errorCode 네이밍 수정, 토큰 만료외에 예외시 에러

04b2c0f

[REFACTOR] SocialUserInfo 책임원칙 위반 코드 제거

c0ccf24

[REFACOTR] verifier component로 변경 후 패키지 분리

01cd395

[REFACTOR] verifier 패키지 이동

181848e

dnwls16071 approved these changes Jan 13, 2026

View reviewed changes

wook099 added 2 commits January 13, 2026 16:37

[REFACTOR] JwtAuthenticationEntryPoint 인증 토큰 부재 담당

e1fa5f1

[REFACTOR] 로그아웃 api 반환형 void 수정

63301d6

dnwls16071 approved these changes Jan 13, 2026

View reviewed changes

wook099 merged commit f6944e9 into developer Jan 13, 2026

[REFACTOR] Auth 도메인 리펙토링 #220

[REFACTOR] Auth 도메인 리펙토링 #220

Uh oh!

Conversation

wook099 commented Jan 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dnwls16071 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dnwls16071 Jan 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dnwls16071 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

wook099 commented Jan 9, 2026 •

edited

Loading

dnwls16071 Jan 11, 2026 •

edited

Loading

dnwls16071 Jan 11, 2026 •

edited

Loading