-
Notifications
You must be signed in to change notification settings - Fork 0
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
feat: 관리자 인증 기능을 구현한다 #37
Conversation
- AdminAccessTokenResponse - AdminTokenResponse
- AdminAuthArgumentResolver - AdminAuthConfig - AdminAuthenticationContext - AdminAuthenticationContext - AdminLoginValidCheckerInterceptor - AdminRefreshTokenExtractionArgumentResolver
- AdminExceptionHandler - AdminLoginInvalidException - AdminNotFoundException - InvalidPasswordException - PasswordMismatchException - UnauthorizedAccessToAdminException
- AdminRepository - AdminJpaRepository - AdminRepositoryImpl
- AdminTokenProvider - AdminJwtTokenProvider
- AdminAuthService
- AdminAuthController
- AdminSignUpRequest - AdminProfileSignUpRequest - AdminLoginRequest
- Department: 관리자의 부서 - AdminStatus: 관리자 계정 사용 가능 여부 - Authority: 관리자의 권한
…기 위해 어노테이션 생성 - AdminRefreshToken
- AdminFixture - AdminRequestFixture - AdminTokenResponseFixture
- AdminFixture - AdminRequestFixture - AdminTokenResponseFixture
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.
아주 최고네요~ Approve 해도 될 것 같아서 누릅니다! 다만 리뷰 내용 확인해보시고 고쳐도 될 것 같다라고 생각하시면 그 부분만 고쳐주세요! 잘못된 패키징 고친 것도 그렇고 수고 많으셨습니다 👍
private final AdminRepository adminRepository; | ||
private final AdminTokenProvider adminTokenProvider; | ||
|
||
public AdminTokenResponse signUp(final AdminSignUpRequest adminSignUpRequest) { |
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.
dto로의 변환은 레이어드 아키텍처에서 컨트롤러에서 해주는 것이 책임이라고 생각하는데 어떻게 생각하시나요?
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.
응용레이어에서 AT와 RT를 만들어서 컨트롤러로 넘겨주기 때문에 이를 컨트롤러에서 받기 위한 DTO를 만들어서 사용했습니다. 응답 바디에 담기는 DTO 타입은 저도 컨트롤러에서 하는게 맞다고 생각합니다! 하지만 지금은 응용 레이어에서 컨트롤러로 넘겨주기 위한 DTO이기 때문에 큰 문제가 없어보이는데 어떻게 생각하시나요? 컨트롤러에서 응답 바디에 넘겨주는 DTO의 타입은 AdminAccessTokenResponse입니다!
import com.atwoz.admin.application.auth.dto.AdminTokenResponse; | ||
import com.atwoz.admin.domain.admin.Admin; | ||
import com.atwoz.admin.domain.admin.AdminRepository; | ||
import com.atwoz.admin.domain.admin.AdminTokenProvider; |
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.
토큰을 주는 것은 도메인 보다는 서비스가 낫지 않을까 조심스럽게 의견 드려봅니다..! 다르게 생각한다면 편하게 말씀해주세요!
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.
저는 토큰을 암호화하는 부분이 외부라이브러리를 이용해서 인프라 레이어로 두고 이를 인터페이스화시킨 것을 도메인 레이어에서 둬서 응용 계층에서 호출해서 사용하도록 구현하려고 했는데 혹시 서비스 레이어에 두는게 더 좋은 것 같다고 생각한 이유를 알 수 있을까요?
@@ -0,0 +1,7 @@ | |||
package com.atwoz.admin.application.auth.dto; | |||
|
|||
public record AdminProfileSignUpRequest( |
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.
여기도 validation 추가 해주세요!
|
||
String createRefreshToken(Long id); | ||
|
||
<T> T extract(String token, String claimName, Class<T> classType); |
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.
제네릭을 사용하신 이유가 따로 있을까요?!
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.
클레임에는 문자열(이메일), 숫자(id)등이 들어값니다. 그래서 추출할 때 만약 문자열 값과 숫자 값을 모두 추출할 일이 있으면 해당 메서드를 여러개 만들어야 해서 중복을 최소한해보고자 제네릭을 이용해서 구현해보았습니다.
private static final String ID = "id"; | ||
private static final String TOKEN_TYPE = "token type"; | ||
private static final String REFRESH_TOKEN = "refresh token"; | ||
private static final String ACCESS_TOKEN = "access token"; | ||
private static final String ROLE = "role"; | ||
private static final String ADMIN = "admin"; |
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.
👍
@PostMapping("/access-token-regeneration") | ||
public ResponseEntity<AdminAccessTokenResponse> reGenerateAccessToken( | ||
@AdminRefreshToken final String refreshToken) { | ||
System.out.println("hihi : " + refreshToken); |
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.
System.out.println("hihi : " + refreshToken); |
@@ -1,5 +1,7 @@ | |||
package com.atwoz.member.ui.auth.interceptor; | |||
package com.atwoz.global.config.interceptor; |
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.
이거 항상 거슬렸는데 👍
|
||
private static final String DEFAULT_PHONE_NUMBER = "01011111111"; | ||
|
||
private final TokenProvider tokenProvider; | ||
private final MemberTokenProvider memberTokenProvider; |
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.
어차피 토큰 주는게 똑같다면 기존과 같이 같은 인터페이스를 사용하고 DI하는 시점에만 다른 빈이 주입되도록 하는 건 어떻게 생각하시나요?
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.
오 좋은 방법인 것 같습니다. 한번 고민해보겠습니다!
.orElseThrow(MemberLoginInvalidException::new); | ||
Long extractedId = memberTokenProvider.extract(token, MEMBER_ID, Long.class); | ||
memberAuthenticationContext.setAuthentication(extractedId); | ||
|
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.
토큰의 형식은 일치하는데 id를 추출할 수 없으면 어떻게 될까요?
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.
음 그 부분은 고려해본 적이 없는 것 같습니다. 찾아보고 수정해보겠습니다!
📄 Summary
관리자 인증 기능을 구현하였습니다.
관리자의 회원가입은 이메일, 비밀번호, 비밀번호 확인, 프로필 정보를 올바르게 입력하면 회원가입이 되게 됩니다.
AT가 만료되면 쿠키에 답긴 RT를 이용하여 AT를 재발급해주게 됩니다.
🙋🏻 More
이번에 개발을 진행하며 회원과 관리자의 테이블 구조의 다른 점이 많아 하나의 바운디드 컨텍스트로 관리되는데 문제가 있는 것 같다고 판단하여 분리하는 작업을 진행하였습니다.
기존 방식에서는 관리자와 회원을 하나의 테이블로 관리하여, 토큰 검증 시 ID만 확인하면 되는 이점이 있었습니다. 또한, 인증에 사용되는 여러 기능을 재사용할 수 있었습니다. 예를 들어, 토큰 암호화 및 Argument Resolver에서 필요한 URI 검증 기능을 재사용할 수 있었습니다. 그러나 관리자와 회원을 하나의 테이블로 관리할 때, 서로 다른 컬럼들이 null 값이나 기본 값으로 남겨지는 단점이 있었습니다.
이를 해결하기 위해 테이블을 분리하고, 각각 다른 도메인으로 개발해 보았습니다. 그 결과, 관리자와 회원 테이블을 null 값 없이 관리할 수 있었고, 서로 다른 인증 방식을 같은 루트 도메인에서 개발할 필요가 없어졌습니다.
이 외에 발생할 수 있는 문제점이나 고려하지 못한 사항이 있다면 댓글로 남겨주시면 감사하겠습니다.
추가적으로 관리자와 회원의 도메인을 따로 개발하는 것에 대한 의견을 남겨주시면 감사하겠습니다!