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

[PC-379] 어드민 모듈 로그인 기능 #34

Merged

Conversation

Lujaec
Copy link
Member

@Lujaec Lujaec commented Jan 23, 2025

🔗 관련 이슈

PC-379

✨ 작업 내용

  • api 모듈에 있던 jwt 기능을 common:auth 모듈로 리팩토링
  • common:auth에 Token 생성 기능 추가
  • admin 모듈 로그인 기능

✅ 체크리스트

  • 코드가 정상적으로 컴파일되나요?
  • 테스트 코드를 통과했나요?
  • merge할 브랜치의 위치를 확인했나요?
  • Label을 지정했나요?

🎃 새롭게 알게된 사항

📋 참고 사항

  • 코드 리뷰 시 논의가 필요한 사항이나 배포 관련 주의 사항을 추가합니다.
    로그인하는 사용자를 위해 common:auth 모듈을 만들고 admin 모듈에서는 common:auth를 사용하도록 하였습니다.
    토큰을 생성하는 로직을 api 모듈에서도 common:auth 기능을 사용하도록 수정해야합니다.

Copy link
Member

@devchlee12 devchlee12 left a comment

Choose a reason for hiding this comment

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

고생하셨습니다!

Comment on lines +10 to +30
public class AuthTokenGenerator {

private final JwtUtil jwtUtil;

@Value("${jwt.accessToken.expiration}")
private Long accessTokenExpiration;

@Value("${jwt.refreshToken.expiration}")
private Long refreshTokenExpiration;


public AuthToken generate(Long userId, String oauthId, String role) {
String accessToken = jwtUtil.createJwt("access_token", userId, oauthId, role,
accessTokenExpiration);

String refreshToken = jwtUtil.createJwt("refesh_token", userId, oauthId, role,
refreshTokenExpiration);

return new AuthToken(accessToken, refreshToken);
}
}
Copy link
Member

Choose a reason for hiding this comment

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

책임 분리 좋은 것 같습니다!

@@ -9,8 +9,10 @@
@RequiredArgsConstructor
public enum AuthErrorCode implements ErrorCode {
OAUTH_ERROR(HttpStatus.FORBIDDEN, "Oauth Error"),
ACCESS_DENIED(HttpStatus.FORBIDDEN, "권한이 없습니다."),
Copy link
Member

Choose a reason for hiding this comment

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

에러 메시지 한글로 하는 거 좋은 것 같습니다!
앞으로는 에러메시지 한글로 할까요?

Copy link
Member Author

Choose a reason for hiding this comment

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

에러 메시지가 한글인게 더 명확한 것 같습니다 !

그리고, 에러가 발생한 응답이 전달될때 클라이언트 분들은 저희 서비스만의 에러 코드(ex. 1404, 1402 ...)가 같이 전달되었으면 하는 바람이 있으신 것 같습니다. 한글로 변환하는 김에 저희 서비스 에러코드도 지정하는 것이 어떨까요 ?

@Lujaec Lujaec merged commit 50629e7 into feature/PC-382-admin-update-member-status Jan 25, 2025
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.

2 participants