-
Notifications
You must be signed in to change notification settings - Fork 0
[Feat] 인증 도메인 (Auth) 틀 잡기 #11
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
base: develop
Are you sure you want to change the base?
Conversation
rlawngjs0313
left a comment
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을 날리는걸로 할까요?? 코멘트 남긴거 읽어보시고 나중에 개발하실때 참고하면 좋을 것 같아요!!
|
|
||
| @Operation(summary = "SMS 발송", description = "휴대폰 번호로 인증번호를 발송합니다.") | ||
| @PostMapping("/sms/send") | ||
| public ResponseEntity<ApiResponse<Void>> sendSms( |
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.
ResponseEntity.ok() 만 사용하면 차라리 ApiResponse만 이용하는게 나을 것 같아요!!
public ApiResponse
...
return ApiResponse.onSuccess(AuthSuccessCode.OK, null); 처럼요!
|
|
||
| private final AuthService authService; | ||
|
|
||
| @Operation(summary = "SMS 발송", description = "휴대폰 번호로 인증번호를 발송합니다.") |
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.
나중에 배포를 염두해서 Swagger 상에서 누가 이 API를 만들었는지 파악하기 쉽게 담당자를 한번 적어볼까요??
SMS 발송 By 장명준 처럼요!!
| package com.example.scoi.domain.auth.dto; | ||
|
|
||
| public class AuthResDTO { | ||
|
|
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.
API 명세서에는 SMS 발송에 응답으로 expiredAt, 회원가입에 memberId, koreanName 이 있는거 같은데 여기 DTO에선 안적은 거 같아요!!
|
|
||
| // 2. 로그인 실패 5회 체크 | ||
| if (member.getLoginFailCount() >= 5) { | ||
| // TODO: RT 블랙리스트 등록 (Redis) |
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.
Refresh Token말고 Access Token을 블랙리스트에 등록해야 해요!! Refresh Token은 DB에 저장되어 있고 재발급할때 DB 업데이트 하니까 Refresh Token은 따로 블랙리스트에 등록할 필요는 없는 것 같아요!!
|
📍 PR 타입 (하나 이상 선택)
❗️ 관련 이슈 링크
Close #6
📌 개요
🔁 변경 사항
의존성 추가
Auth 도메인 생성
Member 도메인 수정
📸 스크린샷
👀 기타 더 이야기해볼 점
✅ 체크 리스트